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

Hi Nikita,

On 1 October 2014 05:31, Nikita Kiryanov nikita@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@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@compulab.co.il

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

Regards, Simon