[U-Boot] [PATCH v4 6/9] dm: imx: Use gpio_request() to request GPIOs

[Nikita Kiryanov]

Hi Simon,

On 02/10/14 04:57, 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 v4:
> - Adjust error checking to permit calling gpio_request() multiple times
> - Avoid doing low-level SATA init multiple times
> - Move SATA changes into this patch
>
> Changes in v3:
> - Add a check for the Ethernet gpio_request() also
> - Add a comment for the CONFIG_SPL_BUILD #ifdef
> - Just warn when one of the board init stages fails
>
> Changes in v2:
> - Check return values of gpio_request()
>
>   arch/arm/imx-common/i2c-mxv7.c | 24 +++++++++++++++++
>   board/compulab/cm_fx6/cm_fx6.c | 61 ++++++++++++++++++++++++++++++++----------
>   board/compulab/cm_fx6/common.c | 14 +++++++++-
>   3 files changed, 84 insertions(+), 15 deletions(-)
>
> 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;

You have a memory leak here if name1 is allocated but name2 is not.

> +	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 9c6e686..53681d4 100644
> --- a/board/compulab/cm_fx6/cm_fx6.c
> +++ b/board/compulab/cm_fx6/cm_fx6.c

While this patch addresses the errors I mentioned in V3, I think that
the amount of additional checks this required demonstrates that these
init functions (which can be called multiple times) are not the best
place to do gpio requests.

I'm open to the idea that these requests will be handled by the
respective drivers (where applicable), but until that functionality is
implemented I think it's best to do them in board_init().

I'm attaching 2 patches, which split this patch into two, one for
i2c-mxv7.c, and the other for cm_fx6.c.

-- 
Regards,
Nikita Kiryanov