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

[Igor Grinberg <grinberg@compulab.co.il>]

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>
> ---
> 
> 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_get_state(struct udevice *dev, unsigned int offset,
> +			      char *buf, int bufsize)
> +{
> +	struct gpio_dev_priv *uc_priv = dev->uclass_priv;
> +	struct mxc_bank_info *bank = dev_get_priv(dev);
> +	const char *label;
> +	bool is_output;
> +	int size;
> +
> +	label = bank->label[offset];
> +	is_output = mxc_gpio_is_output(bank->regs, offset);
> +	size = snprintf(buf, bufsize, "%s%d: ",
> +			uc_priv->bank_name ? uc_priv->bank_name : "", offset);
> +	buf += size;
> +	bufsize -= size;
> +	snprintf(buf, bufsize, "%s: %d [%c]%s%s",
> +		 is_output ? "out" : " in",
> +		 is_output ?
> +			mxc_gpio_bank_get_output_value(bank->regs, offset) :
> +			mxc_gpio_bank_get_value(bank->regs, offset),
> +		 *label ? 'x' : ' ',
> +		 *label ? " " : "",

This is a check if the gpio is requested, right?
I think the new function gpio_is_requested() can be of hand here
instead of open coding this.

> +		 label);
> +
> +	return 0;
> +}
> +

[...]

> +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.

> +	bank->label[offset][0] = '\0';
> +
> +	return 0;
> +}
> +

[...]


-- 
Regards,
Igor.