Error in HAL I2C Driver

Tip / Sign in to post questions, reply, level up, and achieve exciting badges. Know more

cross mob
NiBu_4687336
Level 5
Level 5
5 solutions authored 50 replies posted 25 replies posted

Hi.

In cyhal_i2c_master_read() (and cyhal_i2c_master_write()), a failure returned by Cy_SCB_I2C_MasterSendStart() (or Cy_SCB_I2C_MasterSendReStart()) will cause the value returned by cyhal_i2c_master_read() not to be a legal cy_rslt_t.

Cy_SCB_I2C_MasterSendStart() returns a cy_en_scb_i2c_status_t, and if this value is anything other than CY_SCB_I2C_SUCCESS, it will become the return value of cyhal_i2c_master_read(), which is defined as returning a cy_rslt_t.

 

cy_rslt_t cyhal_i2c_master_read(cyhal_i2c_t *obj, uint16_t dev_addr, uint8_t *data, uint16_t size, uint32_t timeout, bool send_stop)
{
    if (_cyhal_scb_pm_transition_pending())
        return CYHAL_SYSPM_RSLT_ERR_PM_PENDING;

    cy_en_scb_i2c_command_t ack = CY_SCB_I2C_ACK;

    /* Start transaction, send dev_addr */
    cy_en_scb_i2c_status_t status = obj->context.state == CY_SCB_I2C_IDLE
        ? Cy_SCB_I2C_MasterSendStart(obj->base, dev_addr, CY_SCB_I2C_READ_XFER, timeout, &obj->context)
        : Cy_SCB_I2C_MasterSendReStart(obj->base, dev_addr, CY_SCB_I2C_READ_XFER, timeout, &obj->context);

    if (status == CY_SCB_I2C_SUCCESS)
    {
        while (size > 0) {
            if (size == 1)
            {
                ack = CY_SCB_I2C_NAK;
            }
            status = Cy_SCB_I2C_MasterReadByte(obj->base, ack, (uint8_t *)data, timeout, &obj->context);
            if (status != CY_SCB_I2C_SUCCESS)
            {
                break;
            }
            --size;
            ++data;
        }
    }

    if (send_stop)
    {
        /* SCB in I2C mode is very time sensitive. In practice we have to request STOP after */
        /* each block, otherwise it may break the transmission */
        Cy_SCB_I2C_MasterSendStop(obj->base, timeout, &obj->context);
    }
    return status;
}

 

-Nick

0 Likes
1 Solution

Hello Nick,

Yes, you are right. This is a bug from our side. There should have been an appropriate result code added for this in the hal header files and then the status of cy_en_scb_i2c_status_t mapped to it. I see other functions affected by this too. I have filed an internal ticket to fix all of them. 

For now, please consider these as PDL status codes. You can find the enums for these in this file here

Thanks for pointing this out. Appreciate the feedback 🙂

Regards,
Dheeraj 

View solution in original post

4 Replies
NiBu_4687336
Level 5
Level 5
5 solutions authored 50 replies posted 25 replies posted

Hi @MotooTanaka.

Thanks for the reply, but you may have misunderstood my post.

I'm not reporting a problem with my code; I'm reporting an error in Cypress's HAL implementation. The functions I referenced are documented as returning a cy_rslt_t, but actually return the value of a variable of type cy_en_scb_i2c_status_t.

C considers enums to be integer types, and will implicitly cast between enums and integers. Because cy_rslt_t is simply a typedef alias for uint32_t, the compiler doesn't detect the error.

If you attempt to use the parsing macros CY_RSLT_GET_MODULE, and CY_RSLT_GET_CODE on the returned value, you'll get nonsensical results.

-Nick

0 Likes
MotooTanaka
Level 9
Level 9
Distributor - Marubun (Japan)
First comment on blog Beta tester First comment on KBA

Dear Nick-san,

Thanks for the reply, but you may have misunderstood my post.

That is right. I am sorry for my short-sight.

 

If you attempt to use the parsing macros CY_RSLT_GET_MODULE,

> and CY_RSLT_GET_CODE on the returned value, you'll get nonsensical results.

So in this case, we (the programmer) must know that the result was returned from i2c function,

and interpret the value as cy_en_scb_i2c_status_t.

Probably the designer of HAL did not like to carry cy_en_scb_i2c_status_t all over the place,

I'm not sure if this was a reasonable decision or not.

 

I wonder how expensive it is to declare

cy_en_scb_i2c_status_t cyhal_i2c_master_write(...)

instead of

cy_rslt_t cyhal_i2c_master_write(...)

Well, at least more typing though.

 

Best Regards,

8-Feb-2021

Motoo Tanaka

P.S. To avoid confusion, I deleted my first (impertinent) response.

Hello @MotooTanaka ,

You could modify them it is not a problem. It is just that these are internal files and in case the project libraries are reimported or shared with someone else, all these changes are gone. As a temporary solution, this is okay 🙂

Regards,
Dheeraj 

Hello Nick,

Yes, you are right. This is a bug from our side. There should have been an appropriate result code added for this in the hal header files and then the status of cy_en_scb_i2c_status_t mapped to it. I see other functions affected by this too. I have filed an internal ticket to fix all of them. 

For now, please consider these as PDL status codes. You can find the enums for these in this file here

Thanks for pointing this out. Appreciate the feedback 🙂

Regards,
Dheeraj