[U-Boot] [PATCH v2 08/11] dm: imx: Use gpio_request() to request GPIOs

[Simon Glass]

Hi Igor,

On 17 September 2014 06:13, Igor Grinberg <grinberg at compulab.co.il> wrote:

> Hi Simon,
>
> On 09/17/14 06:51, Simon Glass wrote:
> > GPIOs should be requested before use. Without this, driver model will not
> > permit the GPIO to be used.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > Changes in v2:
> > - Check return values of gpio_request()
> >
> >  arch/arm/imx-common/i2c-mxv7.c | 24 ++++++++++++++++++++++++
> >  board/compulab/cm_fx6/cm_fx6.c | 15 +++++++++++++++
> >  board/compulab/cm_fx6/common.c |  7 +++++++
> >  3 files changed, 46 insertions(+)
> >
> > diff --git a/arch/arm/imx-common/i2c-mxv7.c
> b/arch/arm/imx-common/i2c-mxv7.c
> > index 70cff5c..aaf6936 100644
> > --- a/arch/arm/imx-common/i2c-mxv7.c
> > +++ b/arch/arm/imx-common/i2c-mxv7.c
> > @@ -4,6 +4,7 @@
> >   * SPDX-License-Identifier:  GPL-2.0+
> >   */
> >  #include <common.h>
> > +#include <malloc.h>
> >  #include <asm/arch/clock.h>
> >  #include <asm/arch/imx-regs.h>
> >  #include <asm/errno.h>
> > @@ -72,10 +73,26 @@ static void * const i2c_bases[] = {
> >  int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> >             struct i2c_pads_info *p)
> >  {
> > +     char *name1, *name2;
> >       int ret;
> >
> >       if (i2c_index >= ARRAY_SIZE(i2c_bases))
> >               return -EINVAL;
> > +
> > +     name1 = malloc(9);
> > +     name2 = malloc(9);
> > +     if (!name1 || !name2)
> > +             return -ENOMEM;
> > +     sprintf(name1, "i2c_sda%d", i2c_index);
> > +     sprintf(name2, "i2c_scl%d", i2c_index);
> > +     ret = gpio_request(p->sda.gp, name1);
> > +     if (ret)
> > +             goto err_req1;
> > +
> > +     ret = gpio_request(p->scl.gp, name2);
> > +     if (ret)
> > +             goto err_req2;
> > +
> >       /* Enable i2c clock */
> >       ret = enable_i2c_clk(1, i2c_index);
> >       if (ret)
> > @@ -93,5 +110,12 @@ int setup_i2c(unsigned i2c_index, int speed, int
> slave_addr,
> >
> >  err_idle:
> >  err_clk:
> > +     gpio_free(p->scl.gp);
> > +err_req2:
> > +     gpio_free(p->sda.gp);
> > +err_req1:
> > +     free(name1);
> > +     free(name2);
> > +
> >       return ret;
> >  }
> > diff --git a/board/compulab/cm_fx6/cm_fx6.c
> b/board/compulab/cm_fx6/cm_fx6.c
> > index 62c625a..10e31b6 100644
> > --- a/board/compulab/cm_fx6/cm_fx6.c
> > +++ b/board/compulab/cm_fx6/cm_fx6.c
> > @@ -71,8 +71,21 @@ static iomux_v3_cfg_t const sata_pads[] = {
> >
> >  static int cm_fx6_setup_issd(void)
> >  {
> > +     int ret;
> > +     int i;
> > +
> >       SETUP_IOMUX_PADS(sata_pads);
> > +
> > +     for (i = 0; i < ARRAY_SIZE(cm_fx6_issd_gpios); i++) {
> > +             ret = gpio_request(cm_fx6_issd_gpios[i], "sata");
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> >       /* Make sure this gpio has logical 0 value */
> > +     ret = gpio_request(CM_FX6_SATA_PWLOSS_INT, "sata_pwloss_int");
> > +     if (ret)
> > +             return ret;
> >       gpio_direction_output(CM_FX6_SATA_PWLOSS_INT, 0);
> >       udelay(100);
> >
> > @@ -182,6 +195,7 @@ static int cm_fx6_usb_hub_reset(void)
> >       }
> >
> >       SETUP_IOMUX_PAD(PAD_SD3_RST__GPIO7_IO08 |
> MUX_PAD_CTRL(NO_PAD_CTRL));
> > +     gpio_request(CM_FX6_USB_HUB_RST, "usb_hub_rst");
>
> This is confusing... This GPIO should be already requested
> several lines above.
>

OK

>
> >       gpio_direction_output(CM_FX6_USB_HUB_RST, 0);
> >       udelay(10);
> >       gpio_direction_output(CM_FX6_USB_HUB_RST, 1);
> > @@ -339,6 +353,7 @@ int board_eth_init(bd_t *bis)
> >
> >       SETUP_IOMUX_PADS(enet_pads);
> >       /* phy reset */
> > +     gpio_request(CM_FX6_ENET_NRST, "enet_nrst");
>
> Error check?
> This one also should not fail the boot process, just abort the
> Ethernet init.
>

OK


>
> >       gpio_direction_output(CM_FX6_ENET_NRST, 0);
> >       udelay(500);
> >       gpio_set_value(CM_FX6_ENET_NRST, 1);
> > diff --git a/board/compulab/cm_fx6/common.c
> b/board/compulab/cm_fx6/common.c
> > index 1f39679..e4d7e2e 100644
> > --- a/board/compulab/cm_fx6/common.c
> > +++ b/board/compulab/cm_fx6/common.c
> > @@ -79,6 +79,13 @@ void cm_fx6_set_ecspi_iomux(void)
> >
> >  int board_spi_cs_gpio(unsigned bus, unsigned cs)
> >  {
> > +#ifndef CONFIG_SPL_BUILD
> > +     int ret;
> > +
> > +     ret = gpio_request(CM_FX6_ECSPI_BUS0_CS0, "ecspi_bus0_cs0");
> > +     if (ret)
> > +             return ret;
> > +#endif
>
> Is there a reason for excluding this request from spl builds?
> We do build with CONFIG_SPL_GPIO_SUPPORT and we don't have a
> problem with spl size.
> So, if there is no strong reason to exclude it from spl build,
> I'd like to remove the #ifndef.
>

It isn't supported by DM yet.


>
> >       return (bus == 0 && cs == 0) ? (CM_FX6_ECSPI_BUS0_CS0) : -1;
> >  }
> >  #endif
> >
>
> --
> Regards,
> Igor.
>

Regards,
Simon