pinctrl: intel: Pass irqchip when adding gpiochip

Message ID 20191229013059.495767-1-linus.walleij@linaro.org
State New
Headers show
Series
  • pinctrl: intel: Pass irqchip when adding gpiochip
Related show

Commit Message

Linus Walleij Dec. 29, 2019, 1:30 a.m.
We need to convert all old gpio irqchips to pass the irqchip
setup along when adding the gpio_chip. For more info see
drivers/gpio/TODO.

Set up the pin ranges using the new callback.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 drivers/pinctrl/intel/pinctrl-intel.c | 61 +++++++++++++++------------
 1 file changed, 35 insertions(+), 26 deletions(-)

-- 
2.23.0

Comments

Mika Westerberg Dec. 30, 2019, 10:20 a.m. | #1
On Sun, Dec 29, 2019 at 02:30:59AM +0100, Linus Walleij wrote:
> We need to convert all old gpio irqchips to pass the irqchip

> setup along when adding the gpio_chip. For more info see

> drivers/gpio/TODO.

> 

> Set up the pin ranges using the new callback.


Maybe have this one split as a separate patch? Same what we do for
Baytrail and Cherryview.

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---

>  drivers/pinctrl/intel/pinctrl-intel.c | 61 +++++++++++++++------------

>  1 file changed, 35 insertions(+), 26 deletions(-)

> 

> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c

> index 4860bc9a4e48..ffacd77861f7 100644

> --- a/drivers/pinctrl/intel/pinctrl-intel.c

> +++ b/drivers/pinctrl/intel/pinctrl-intel.c

> @@ -1160,8 +1160,8 @@ static irqreturn_t intel_gpio_irq(int irq, void *data)

>  	return ret;

>  }

>  

> -static int intel_gpio_add_pin_ranges(struct intel_pinctrl *pctrl,

> -				     const struct intel_community *community)

> +static int intel_gpio_add_community_ranges(struct intel_pinctrl *pctrl,

> +				const struct intel_community *community)

>  {

>  	int ret = 0, i;

>  

> @@ -1181,6 +1181,24 @@ static int intel_gpio_add_pin_ranges(struct intel_pinctrl *pctrl,

>  	return ret;

>  }

>  

> +static int intel_gpio_add_pin_ranges(struct gpio_chip *gc)

> +{

> +	struct intel_pinctrl *pctrl = gpiochip_get_data(gc);

> +	int ret, i;

> +

> +	for (i = 0; i < pctrl->ncommunities; i++) {

> +		struct intel_community *community = &pctrl->communities[i];

> +

> +		ret = intel_gpio_add_community_ranges(pctrl, community);

> +		if (ret) {

> +			dev_err(pctrl->dev, "failed to add GPIO pin range\n");

> +			return ret;

> +		}

> +	}

> +

> +	return 0;

> +}

> +

>  static unsigned int intel_gpio_ngpio(const struct intel_pinctrl *pctrl)

>  {

>  	const struct intel_community *community;

> @@ -1205,7 +1223,8 @@ static unsigned int intel_gpio_ngpio(const struct intel_pinctrl *pctrl)

>  

>  static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)

>  {

> -	int ret, i;

> +	int ret;

> +	struct gpio_irq_chip *girq;


Nit:

Can you order these in "reverse christmas tree" like,

	struct gpio_irq_chip *girq;
	int ret;

Otherwise looks good to me, thanks!

Patch

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 4860bc9a4e48..ffacd77861f7 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1160,8 +1160,8 @@  static irqreturn_t intel_gpio_irq(int irq, void *data)
 	return ret;
 }
 
-static int intel_gpio_add_pin_ranges(struct intel_pinctrl *pctrl,
-				     const struct intel_community *community)
+static int intel_gpio_add_community_ranges(struct intel_pinctrl *pctrl,
+				const struct intel_community *community)
 {
 	int ret = 0, i;
 
@@ -1181,6 +1181,24 @@  static int intel_gpio_add_pin_ranges(struct intel_pinctrl *pctrl,
 	return ret;
 }
 
+static int intel_gpio_add_pin_ranges(struct gpio_chip *gc)
+{
+	struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
+	int ret, i;
+
+	for (i = 0; i < pctrl->ncommunities; i++) {
+		struct intel_community *community = &pctrl->communities[i];
+
+		ret = intel_gpio_add_community_ranges(pctrl, community);
+		if (ret) {
+			dev_err(pctrl->dev, "failed to add GPIO pin range\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static unsigned int intel_gpio_ngpio(const struct intel_pinctrl *pctrl)
 {
 	const struct intel_community *community;
@@ -1205,7 +1223,8 @@  static unsigned int intel_gpio_ngpio(const struct intel_pinctrl *pctrl)
 
 static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 {
-	int ret, i;
+	int ret;
+	struct gpio_irq_chip *girq;
 
 	pctrl->chip = intel_gpio_chip;
 
@@ -1214,6 +1233,7 @@  static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 	pctrl->chip.label = dev_name(pctrl->dev);
 	pctrl->chip.parent = pctrl->dev;
 	pctrl->chip.base = -1;
+	pctrl->chip.add_pin_ranges = intel_gpio_add_pin_ranges;
 	pctrl->irq = irq;
 
 	/* Setup IRQ chip */
@@ -1225,26 +1245,9 @@  static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 	pctrl->irqchip.irq_set_wake = intel_gpio_irq_wake;
 	pctrl->irqchip.flags = IRQCHIP_MASK_ON_SUSPEND;
 
-	ret = devm_gpiochip_add_data(pctrl->dev, &pctrl->chip, pctrl);
-	if (ret) {
-		dev_err(pctrl->dev, "failed to register gpiochip\n");
-		return ret;
-	}
-
-	for (i = 0; i < pctrl->ncommunities; i++) {
-		struct intel_community *community = &pctrl->communities[i];
-
-		ret = intel_gpio_add_pin_ranges(pctrl, community);
-		if (ret) {
-			dev_err(pctrl->dev, "failed to add GPIO pin range\n");
-			return ret;
-		}
-	}
-
 	/*
-	 * We need to request the interrupt here (instead of providing chip
-	 * to the irq directly) because on some platforms several GPIO
-	 * controllers share the same interrupt line.
+	 * On some platforms several GPIO controllers share the same interrupt
+	 * line.
 	 */
 	ret = devm_request_irq(pctrl->dev, irq, intel_gpio_irq,
 			       IRQF_SHARED | IRQF_NO_THREAD,
@@ -1254,14 +1257,20 @@  static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 		return ret;
 	}
 
-	ret = gpiochip_irqchip_add(&pctrl->chip, &pctrl->irqchip, 0,
-				   handle_bad_irq, IRQ_TYPE_NONE);
+	girq = &pctrl->chip.irq;
+	girq->chip = &pctrl->irqchip;
+	/* This will let us handle the IRQ in the driver */
+	girq->parent_handler = NULL;
+	girq->num_parents = 0;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_bad_irq;
+
+	ret = devm_gpiochip_add_data(pctrl->dev, &pctrl->chip, pctrl);
 	if (ret) {
-		dev_err(pctrl->dev, "failed to add irqchip\n");
+		dev_err(pctrl->dev, "failed to register gpiochip\n");
 		return ret;
 	}
 
-	gpiochip_set_chained_irqchip(&pctrl->chip, &pctrl->irqchip, irq, NULL);
 	return 0;
 }