[v2,4/7] gpio: merrifield: Add GPIO <-> pin mapping ranges via callback
diff mbox series

Message ID 20191105203557.78562-5-andriy.shevchenko@linux.intel.com
State New
Headers show
Series
  • gpiolib: fix GPIO <-> pin mapping registration
Related show

Commit Message

Andy Shevchenko Nov. 5, 2019, 8:35 p.m. UTC
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@linux.intel.com>
---
 drivers/gpio/gpio-merrifield.c | 43 ++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 17 deletions(-)

Comments

Mika Westerberg Nov. 6, 2019, 1:54 p.m. UTC | #1
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@linux.intel.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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;
> +}
Andy Shevchenko Nov. 6, 2019, 4:52 p.m. UTC | #2
On Wed, Nov 06, 2019 at 03:54:36PM +0200, Mika Westerberg wrote:
> 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@linux.intel.com>
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thanks.

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

I did this one...

> 					     range->gpio_base, range->pin_base,

...but this, since over going thru 80 limit.

> 					     range->npins);

Patch
diff mbox series

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);
+		if (retval) {
+			dev_err(priv->dev, "failed to add GPIO pin range\n");
+			return retval;
+		}
+	}
+
+	return 0;
+}
+
+static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
 	struct mrfld_gpio *priv;
 	u32 gpio_base, irq_base;
 	void __iomem *base;
-	unsigned int i;
 	int retval;
 
 	retval = pcim_enable_device(pdev);
@@ -441,30 +463,16 @@  static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id
 	priv->chip.base = gpio_base;
 	priv->chip.ngpio = MRFLD_NGPIO;
 	priv->chip.can_sleep = false;
+	priv->chip.add_pin_ranges = mrfld_gpio_add_pin_ranges;
 
 	raw_spin_lock_init(&priv->lock);
 
-	pci_set_drvdata(pdev, priv);
 	retval = devm_gpiochip_add_data(&pdev->dev, &priv->chip, priv);
 	if (retval) {
 		dev_err(&pdev->dev, "gpiochip_add error %d\n", retval);
 		return 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);
-		if (retval) {
-			dev_err(&pdev->dev, "failed to add GPIO pin range\n");
-			return retval;
-		}
-	}
-
 	retval = gpiochip_irqchip_add(&priv->chip, &mrfld_irqchip, irq_base,
 				      handle_bad_irq, IRQ_TYPE_NONE);
 	if (retval) {
@@ -477,6 +485,7 @@  static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id
 	gpiochip_set_chained_irqchip(&priv->chip, &mrfld_irqchip, pdev->irq,
 				     mrfld_irq_handler);
 
+	pci_set_drvdata(pdev, priv);
 	return 0;
 }