
Hi Igor,
On 17 September 2014 06:13, Igor Grinberg grinberg@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@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