diff mbox

[U-Boot,v2,07/11] dm: imx: Add error checking to setup_i2c()

Message ID 1410925884-8767-8-git-send-email-sjg@chromium.org
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

Simon Glass Sept. 17, 2014, 3:51 a.m. UTC
Since this function can fail, check its return value.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

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            | 35 +++++++++++++++++++++----------
 3 files changed, 45 insertions(+), 18 deletions(-)

Comments

Igor Grinberg Sept. 17, 2014, 8:37 a.m. UTC | #1
Hi Simon,

On 09/17/14 06:51, Simon Glass wrote:
> Since this function can fail, check its return value.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> 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            | 35 +++++++++++++++++++++----------
>  3 files changed, 45 insertions(+), 18 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..62c625a 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,18 @@ 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();
> +	ret = cm_fx6_setup_issd();
> +	if (ret)
> +		return ret;

Hmm.. 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 of course is not controlled by those GPIOs.
Therefore, I think, it would be incorrect to fail the boot process
if there is a problem with an iSSD GPIO...
Instead a warning message will be enough. Something like:

ret = cm_fx6_setup_issd();
if (ret)
	printf("Warning: iSSD setup failed!\n");

and then continue to the rest of SATA init.

>  	for (i = 0; i < CM_FX6_SATA_INIT_RETRIES; i++) {
>  		err = setup_sata();
>  		if (err) {
> @@ -141,14 +145,25 @@ I2C_PADS(i2c2_pads,
>  	 IMX_GPIO_NR(1, 6));
>  
>  
> -static void cm_fx6_setup_i2c(void)
> +static int cm_fx6_setup_i2c(void)
>  {
> -	setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c0_pads));
> -	setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c1_pads));
> -	setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c2_pads));
> +	int ret;
> +
> +	ret = setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f,
> +			I2C_PADS_INFO(i2c0_pads));
> +	if (!ret) {
> +		ret = setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f,
> +				I2C_PADS_INFO(i2c1_pads));
> +	}
> +	if (!ret) {
> +		ret = setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f,
> +				I2C_PADS_INFO(i2c2_pads));
> +	}

Almost same here, the fact that one of the (or even all) i2c buses
fails to initialize, should not lead to hang()...
The decision if this is a critical error or not should be decided
by the board code.
In this case, this is not a critical error and if one of the buses
fails to initialize, the others should still try.
So, here also, a warning message would be appropriate instead of
abort.

> +
> +	return ret;
>  }
>  #else
> -static void cm_fx6_setup_i2c(void) { }
> +static int cm_fx6_setup_i2c(void) { return 0; }
>  #endif
>  
>  #ifdef CONFIG_USB_EHCI_MX6
> @@ -411,9 +426,7 @@ int board_init(void)
>  {
>  	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
>  	cm_fx6_setup_gpmi_nand();
> -	cm_fx6_setup_i2c();
> -
> -	return 0;
> +	return cm_fx6_setup_i2c();

Same here, on cm-fx6, i2c is not that critical to U-Boot boot
process, and should not lead to hang().

>  }
>  
>  int checkboard(void)
>
Simon Glass Sept. 17, 2014, 1:56 p.m. UTC | #2
Hi Igor,

On 17 September 2014 02:37, Igor Grinberg <grinberg@compulab.co.il> wrote:

> Hi Simon,
>
> On 09/17/14 06:51, Simon Glass wrote:
> > Since this function can fail, check its return value.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > 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            | 35
> +++++++++++++++++++++----------
> >  3 files changed, 45 insertions(+), 18 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..62c625a 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,18 @@ 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();
> > +     ret = cm_fx6_setup_issd();
> > +     if (ret)
> > +             return ret;
>
> Hmm.. 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 of course is not controlled by those GPIOs.
> Therefore, I think, it would be incorrect to fail the boot process
> if there is a problem with an iSSD GPIO...
> Instead a warning message will be enough. Something like:
>
> ret = cm_fx6_setup_issd();
> if (ret)
>         printf("Warning: iSSD setup failed!\n");
>

OK, my intent here was to add the missing gpio_request() calls so that
things would work with DM. I think I am being drawn into board error
policy! I'll give it another spin based on your comments.


>
> and then continue to the rest of SATA init.
>
> >       for (i = 0; i < CM_FX6_SATA_INIT_RETRIES; i++) {
> >               err = setup_sata();
> >               if (err) {
> > @@ -141,14 +145,25 @@ I2C_PADS(i2c2_pads,
> >        IMX_GPIO_NR(1, 6));
> >
> >
> > -static void cm_fx6_setup_i2c(void)
> > +static int cm_fx6_setup_i2c(void)
> >  {
> > -     setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c0_pads));
> > -     setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c1_pads));
> > -     setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c2_pads));
> > +     int ret;
> > +
> > +     ret = setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f,
> > +                     I2C_PADS_INFO(i2c0_pads));
> > +     if (!ret) {
> > +             ret = setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f,
> > +                             I2C_PADS_INFO(i2c1_pads));
> > +     }
> > +     if (!ret) {
> > +             ret = setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f,
> > +                             I2C_PADS_INFO(i2c2_pads));
> > +     }
>
> Almost same here, the fact that one of the (or even all) i2c buses
> fails to initialize, should not lead to hang()...
> The decision if this is a critical error or not should be decided
> by the board code.
> In this case, this is not a critical error and if one of the buses
> fails to initialize, the others should still try.
> So, here also, a warning message would be appropriate instead of
> abort.
>
> > +
> > +     return ret;
> >  }
> >  #else
> > -static void cm_fx6_setup_i2c(void) { }
> > +static int cm_fx6_setup_i2c(void) { return 0; }
> >  #endif
> >
> >  #ifdef CONFIG_USB_EHCI_MX6
> > @@ -411,9 +426,7 @@ int board_init(void)
> >  {
> >       gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
> >       cm_fx6_setup_gpmi_nand();
> > -     cm_fx6_setup_i2c();
> > -
> > -     return 0;
> > +     return cm_fx6_setup_i2c();
>
> Same here, on cm-fx6, i2c is not that critical to U-Boot boot
> process, and should not lead to hang().
>
> >  }
> >
> >  int checkboard(void)
> >
>
> --
> Regards,
> Igor.
>


Regards,
Simon
Igor Grinberg Sept. 17, 2014, 2:16 p.m. UTC | #3
Hi Simon,

On 09/17/14 16:56, Simon Glass wrote:
> Hi Igor,
> 
> On 17 September 2014 02:37, Igor Grinberg <grinberg@compulab.co.il <mailto:grinberg@compulab.co.il>> wrote:
> 
>     Hi Simon,
> 
>     On 09/17/14 06:51, Simon Glass wrote:
>     > Since this function can fail, check its return value.
>     >
>     > Signed-off-by: Simon Glass <sjg@chromium.org <mailto:sjg@chromium.org>>
>     > ---
>     >
>     > 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            | 35 +++++++++++++++++++++----------
>     >  3 files changed, 45 insertions(+), 18 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..62c625a 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,18 @@ 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();
>     > +     ret = cm_fx6_setup_issd();
>     > +     if (ret)
>     > +             return ret;
> 
>     Hmm.. 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 of course is not controlled by those GPIOs.
>     Therefore, I think, it would be incorrect to fail the boot process
>     if there is a problem with an iSSD GPIO...
>     Instead a warning message will be enough. Something like:
> 
>     ret = cm_fx6_setup_issd();
>     if (ret)
>             printf("Warning: iSSD setup failed!\n");
> 
> 
> OK, my intent here was to add the missing gpio_request() calls so that things would work with DM. I think I am being drawn into board error policy! I'll give it another spin based on your comments.

I understand. Thanks for going for another round.
If you feel like it gets too much board specific, we can send a follow up.

>  
> 
> 
>     and then continue to the rest of SATA init.
> 
>     >       for (i = 0; i < CM_FX6_SATA_INIT_RETRIES; i++) {
>     >               err = setup_sata();
>     >               if (err) {
>     > @@ -141,14 +145,25 @@ I2C_PADS(i2c2_pads,
>     >        IMX_GPIO_NR(1, 6));
>     >
>     >
>     > -static void cm_fx6_setup_i2c(void)
>     > +static int cm_fx6_setup_i2c(void)
>     >  {
>     > -     setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c0_pads));
>     > -     setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c1_pads));
>     > -     setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c2_pads));
>     > +     int ret;
>     > +
>     > +     ret = setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f,
>     > +                     I2C_PADS_INFO(i2c0_pads));
>     > +     if (!ret) {
>     > +             ret = setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f,
>     > +                             I2C_PADS_INFO(i2c1_pads));
>     > +     }
>     > +     if (!ret) {
>     > +             ret = setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f,
>     > +                             I2C_PADS_INFO(i2c2_pads));
>     > +     }
> 
>     Almost same here, the fact that one of the (or even all) i2c buses
>     fails to initialize, should not lead to hang()...
>     The decision if this is a critical error or not should be decided
>     by the board code.
>     In this case, this is not a critical error and if one of the buses
>     fails to initialize, the others should still try.
>     So, here also, a warning message would be appropriate instead of
>     abort.
> 
>     > +
>     > +     return ret;
>     >  }
>     >  #else
>     > -static void cm_fx6_setup_i2c(void) { }
>     > +static int cm_fx6_setup_i2c(void) { return 0; }
>     >  #endif
>     >
>     >  #ifdef CONFIG_USB_EHCI_MX6
>     > @@ -411,9 +426,7 @@ int board_init(void)
>     >  {
>     >       gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
>     >       cm_fx6_setup_gpmi_nand();
>     > -     cm_fx6_setup_i2c();
>     > -
>     > -     return 0;
>     > +     return cm_fx6_setup_i2c();
> 
>     Same here, on cm-fx6, i2c is not that critical to U-Boot boot
>     process, and should not lead to hang().
> 
>     >  }
>     >
>     >  int checkboard(void)
>     >
> 
>     --
>     Regards,
>     Igor.
> 
> 
> 
> Regards,
> Simon
>
diff mbox

Patch

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..62c625a 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,18 @@  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();
+	ret = cm_fx6_setup_issd();
+	if (ret)
+		return ret;
 	for (i = 0; i < CM_FX6_SATA_INIT_RETRIES; i++) {
 		err = setup_sata();
 		if (err) {
@@ -141,14 +145,25 @@  I2C_PADS(i2c2_pads,
 	 IMX_GPIO_NR(1, 6));
 
 
-static void cm_fx6_setup_i2c(void)
+static int cm_fx6_setup_i2c(void)
 {
-	setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c0_pads));
-	setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c1_pads));
-	setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c2_pads));
+	int ret;
+
+	ret = setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f,
+			I2C_PADS_INFO(i2c0_pads));
+	if (!ret) {
+		ret = setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f,
+				I2C_PADS_INFO(i2c1_pads));
+	}
+	if (!ret) {
+		ret = setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f,
+				I2C_PADS_INFO(i2c2_pads));
+	}
+
+	return ret;
 }
 #else
-static void cm_fx6_setup_i2c(void) { }
+static int cm_fx6_setup_i2c(void) { return 0; }
 #endif
 
 #ifdef CONFIG_USB_EHCI_MX6
@@ -411,9 +426,7 @@  int board_init(void)
 {
 	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
 	cm_fx6_setup_gpmi_nand();
-	cm_fx6_setup_i2c();
-
-	return 0;
+	return cm_fx6_setup_i2c();
 }
 
 int checkboard(void)