[U-Boot] [PATCH v3 07/11] imx: Add error checking to setup_i2c()

[Simon Glass]

Hi Nikita,

On 1 October 2014 05:31, Nikita Kiryanov <nikita at compulab.co.il> wrote:
> Hi Simon,
>
>
> On 17/09/14 18:02, Simon Glass wrote:
>>
>> Since this function can fail, check its return value.
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>> Changes in v3:
>> - Just warn when one of the board init stages fails
>>
>> Changes in v2:
>> - Add new patch to add error checking to setup_i2c()
>>
>>   arch/arm/imx-common/i2c-mxv7.c            | 24 ++++++++++---
>>   arch/arm/include/asm/imx-common/mxc_i2c.h |  4 +--
>>   board/compulab/cm_fx6/cm_fx6.c            | 56
>> ++++++++++++++++++++++++++-----
>>   3 files changed, 68 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm/imx-common/i2c-mxv7.c
>> b/arch/arm/imx-common/i2c-mxv7.c
>> index a580873..70cff5c 100644
>> --- a/arch/arm/imx-common/i2c-mxv7.c
>> +++ b/arch/arm/imx-common/i2c-mxv7.c
>> @@ -69,15 +69,29 @@ static void * const i2c_bases[] = {
>>   };
>>
>>   /* i2c_index can be from 0 - 2 */
>> -void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
>> -               struct i2c_pads_info *p)
>> +int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
>> +             struct i2c_pads_info *p)
>>   {
>> +       int ret;
>> +
>>         if (i2c_index >= ARRAY_SIZE(i2c_bases))
>> -               return;
>> +               return -EINVAL;
>>         /* Enable i2c clock */
>> -       enable_i2c_clk(1, i2c_index);
>> +       ret = enable_i2c_clk(1, i2c_index);
>> +       if (ret)
>> +               goto err_clk;
>> +
>>         /* Make sure bus is idle */
>> -       force_idle_bus(p);
>> +       ret = force_idle_bus(p);
>> +       if (ret)
>> +               goto err_idle;
>> +
>>         bus_i2c_init(i2c_bases[i2c_index], speed, slave_addr,
>>                         force_idle_bus, p);
>> +
>> +       return 0;
>> +
>> +err_idle:
>> +err_clk:
>> +       return ret;
>>   }
>> diff --git a/arch/arm/include/asm/imx-common/mxc_i2c.h
>> b/arch/arm/include/asm/imx-common/mxc_i2c.h
>> index 182c2f3..af86163 100644
>> --- a/arch/arm/include/asm/imx-common/mxc_i2c.h
>> +++ b/arch/arm/include/asm/imx-common/mxc_i2c.h
>> @@ -52,8 +52,8 @@ struct i2c_pads_info {
>>                                         &mx6q_##name : &mx6s_##name
>>   #endif
>>
>> -void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
>> -               struct i2c_pads_info *p);
>> +int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
>> +             struct i2c_pads_info *p);
>>   void bus_i2c_init(void *base, int speed, int slave_addr,
>>                 int (*idle_bus_fn)(void *p), void *p);
>>   int bus_i2c_read(void *base, uchar chip, uint addr, int alen, uchar
>> *buf,
>> diff --git a/board/compulab/cm_fx6/cm_fx6.c
>> b/board/compulab/cm_fx6/cm_fx6.c
>> index fdb8ebf..10a568f 100644
>> --- a/board/compulab/cm_fx6/cm_fx6.c
>> +++ b/board/compulab/cm_fx6/cm_fx6.c
>> @@ -69,7 +69,7 @@ static iomux_v3_cfg_t const sata_pads[] = {
>>         IOMUX_PADS(PAD_EIM_BCLK__GPIO6_IO31   |
>> MUX_PAD_CTRL(NO_PAD_CTRL)),
>>   };
>>
>> -static void cm_fx6_setup_issd(void)
>> +static int cm_fx6_setup_issd(void)
>>   {
>>         SETUP_IOMUX_PADS(sata_pads);
>>         /* Make sure this gpio has logical 0 value */
>> @@ -79,14 +79,24 @@ static void cm_fx6_setup_issd(void)
>>         cm_fx6_sata_power(0);
>>         mdelay(250);
>>         cm_fx6_sata_power(1);
>> +
>> +       return 0;
>>   }
>>
>>   #define CM_FX6_SATA_INIT_RETRIES      10
>>   int sata_initialize(void)
>>   {
>> -       int err, i;
>> +       int err, i, ret;
>>
>> -       cm_fx6_setup_issd();
>> +       /*
>> +        * cm-fx6 may have iSSD not assembled and in this case it has
>> +        * bypasses for a (m)SATA socket on the baseboard. The socketed
>> +        * device is not controlled by those GPIOs. So just print a
>> warning
>> +        * if the setup fails.
>> +        */
>> +       ret = cm_fx6_setup_issd();
>> +       if (ret)
>> +               printf("Warning: iSSD setup failed!\n");
>>         for (i = 0; i < CM_FX6_SATA_INIT_RETRIES; i++) {
>>                 err = setup_sata();
>>                 if (err) {
>
>
> The issd stuff above is unrelated to the subject of this patch. It
> should be part of the next patch.
>
> Aside from that,
> Tested-by: Nikita Kiryanov <nikita at compulab.co.il>
>

Thanks - I'll take another look later on today.

Regards,
Simon