[U-Boot] [PATCH 06/10] dm: imx: Use gpio_request() to request GPIOs

[Simon Glass]

Hi Igor,

On 15 September 2014 11:13, Igor Grinberg <grinberg at compulab.co.il> wrote:
> Hi Simon,
>
> On 09/15/14 15:57, Simon Glass wrote:
>> GPIOs should be requested before use. Without this, driver model will not
>> permit the GPIO to be used.
>
> Right. That should have been done from the start... Sorry for that...
> A question below though..
>
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>>  arch/arm/imx-common/i2c-mxv7.c | 14 ++++++++++++++
>>  board/compulab/cm_fx6/cm_fx6.c |  9 +++++++++
>>  board/compulab/cm_fx6/common.c |  3 +++
>>  3 files changed, 26 insertions(+)
>>
>> diff --git a/arch/arm/imx-common/i2c-mxv7.c b/arch/arm/imx-common/i2c-mxv7.c
>> index a580873..7cea024 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,8 +73,21 @@ static void * const i2c_bases[] = {
>>  void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
>>               struct i2c_pads_info *p)
>>  {
>> +     char *name;
>> +
>>       if (i2c_index >= ARRAY_SIZE(i2c_bases))
>>               return;
>> +
>> +     name = malloc(9);
>> +     assert(name);
>> +     sprintf(name, "i2c_sda%d", i2c_index);
>> +     gpio_request(p->sda.gp, name);
>> +
>> +     name = malloc(9);
>> +     assert(name);
>> +     sprintf(name, "i2c_scl%d", i2c_index);
>> +     gpio_request(p->scl.gp, name);
>> +
>>       /* Enable i2c clock */
>>       enable_i2c_clk(1, i2c_index);
>>       /* Make sure bus is idle */
>> diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c
>> index fdb8ebf..80a123d 100644
>> --- a/board/compulab/cm_fx6/cm_fx6.c
>> +++ b/board/compulab/cm_fx6/cm_fx6.c
>> @@ -71,8 +71,15 @@ static iomux_v3_cfg_t const sata_pads[] = {
>>
>>  static void cm_fx6_setup_issd(void)
>>  {
>> +     int i;
>> +
>>       SETUP_IOMUX_PADS(sata_pads);
>> +
>> +     for (i = 0; i < ARRAY_SIZE(cm_fx6_issd_gpios); i++)
>> +             gpio_request(cm_fx6_issd_gpios[i], "sata");
>> +
>>       /* Make sure this gpio has logical 0 value */
>> +     gpio_request(CM_FX6_SATA_PWLOSS_INT, "sata_pwloss_int");
>>       gpio_direction_output(CM_FX6_SATA_PWLOSS_INT, 0);
>>       udelay(100);
>>
>> @@ -167,6 +174,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");
>>       gpio_direction_output(CM_FX6_USB_HUB_RST, 0);
>>       udelay(10);
>>       gpio_direction_output(CM_FX6_USB_HUB_RST, 1);
>> @@ -324,6 +332,7 @@ int board_eth_init(bd_t *bis)
>>
>>       SETUP_IOMUX_PADS(enet_pads);
>>       /* phy reset */
>> +     gpio_request(CM_FX6_ENET_NRST, "enet_nrst");
>>       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..562313b 100644
>> --- a/board/compulab/cm_fx6/common.c
>> +++ b/board/compulab/cm_fx6/common.c
>> @@ -79,6 +79,9 @@ void cm_fx6_set_ecspi_iomux(void)
>>
>>  int board_spi_cs_gpio(unsigned bus, unsigned cs)
>>  {
>> +#ifndef CONFIG_SPL_BUILD
>> +     gpio_request(CM_FX6_ECSPI_BUS0_CS0, "ecspi_bus0_cs0");
>> +#endif
>>       return (bus == 0 && cs == 0) ? (CM_FX6_ECSPI_BUS0_CS0) : -1;
>>  }
>>  #endif
>>
>
> In all the above gpio_request() calls, I think we should check for
> the return value.
> Because after patch 8 in the series it can fail if if someone
> double requests the same gpio.

That's true, although for a particular board you presumably know what
you are doing. The problem happens more when we move this sort of
thing to drivers, and there is a conflict.

In some cases there is not way to report an error (void functions),
and in others it needs additional plumbing. But I agree we should
start to fix this sort of thing.

Regards,
Simon