Re: [U-Boot] [PATCH v2 09/11] dm: imx: gpio: Support driver model in MXC gpio driver

On 09/17/14 17:03, Simon Glass wrote:

Hi Igor,

On 17 September 2014 07:00, Igor Grinberg <grinberg@compulab.co.il mailto:grinberg@compulab.co.il> wrote:

On 09/17/14 06:51, Simon Glass wrote:
> Add driver model support with this driver. In this case the platform data
> is in the driver. It would be better to put this into an SOC-specific file,
> but this is best attempted when more boards are moved over to use driver
> model.
>
> Signed-off-by: Simon Glass <sjg@chromium.org <mailto:sjg@chromium.org>>
> ---
>
> Changes in v2:
> - Change 'reserved' to 'requested'
> - Add an internal function to check if a GPIO is requested
> - Tidy up confusing code that creates names for gpio_request()
>
>  drivers/gpio/mxc_gpio.c | 302 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 301 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c
> index 6a572d5..9435d2f 100644
> --- a/drivers/gpio/mxc_gpio.c
> +++ b/drivers/gpio/mxc_gpio.c

[...]

[...]

> +static int mxc_gpio_free(struct udevice *dev, unsigned offset)
> +{
> +     struct mxc_bank_info *bank = dev_get_priv(dev);
> +     int ret;
> +
> +     ret = check_requested(dev, offset, __func__);
> +     if (ret)
> +             return ret;

In case of not requested gpio,
should we really print the error message and return -EPERM?
Or may be adopt free() behavior and just return silently?
Linux gpiolib gpio_free() uses WARN_ON(extra_checks) for
"not requested" cases, so it shouts only in DEBUG cases.

I'm not sure - I intend to push this up to the DM layer at some point. I feel that keeping track of GPIO requested/free should be a common feature of the uclass rather than implemented in each driver.

Agreed. I've got the same feeling while looking at this.

But until we have enough drivers to make it clear that this is acceptable, I'm leaving it as it is.

So for at least now I think this is correct. We might consider bringing in some sort of WARN system to U-Boot. The caller can ignore the return value anyway.

Ok.