Re: [PATCH v2 4/7] gpio: merrifield: Add GPIO <-> pin mapping ranges via callback

[Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>]

On Tue, Nov 05, 2019 at 10:35:54PM +0200, Andy Shevchenko wrote:
> When IRQ chip is instantiated via GPIO library flow, the few functions,
> in particular the ACPI event registration mechanism, on some of ACPI based
> platforms expect that the pin ranges are initialized to that point.
> 
> Add GPIO <-> pin mapping ranges via callback in the GPIO library flow.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

Reviewed-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>

One minor comment below.

> ---
>  drivers/gpio/gpio-merrifield.c | 43 ++++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-merrifield.c b/drivers/gpio/gpio-merrifield.c
> index 3302125e5265..e96d8e517e26 100644
> --- a/drivers/gpio/gpio-merrifield.c
> +++ b/drivers/gpio/gpio-merrifield.c
> @@ -393,14 +393,36 @@ static const char *mrfld_gpio_get_pinctrl_dev_name(struct mrfld_gpio *priv)
>  	return name;
>  }
>  
> -static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +static int mrfld_gpio_add_pin_ranges(struct gpio_chip *chip)
>  {
> +	struct mrfld_gpio *priv = gpiochip_get_data(chip);
>  	const struct mrfld_gpio_pinrange *range;
>  	const char *pinctrl_dev_name;
> +	unsigned int i;
> +	int retval;
> +
> +	pinctrl_dev_name = mrfld_gpio_get_pinctrl_dev_name(priv);
> +	for (i = 0; i < ARRAY_SIZE(mrfld_gpio_ranges); i++) {
> +		range = &mrfld_gpio_ranges[i];
> +		retval = gpiochip_add_pin_range(&priv->chip,
> +						pinctrl_dev_name,
> +						range->gpio_base,
> +						range->pin_base,
> +						range->npins);

IMHO the below looks slightly better:

		ret = gpiochip_add_pin_range(&priv->chip, pinctrl_dev_name,
					     range->gpio_base, range->pin_base,
					     range->npins);

> +		if (retval) {
> +			dev_err(priv->dev, "failed to add GPIO pin range\n");
> +			return retval;
> +		}
> +	}
> +
> +	return 0;
> +}