[v1] pinctrl: at91: Minor cleanups

[PATCH 1/2] pinctrl: at91: Make the irqchip immutable

Posted by Mark Brown 2 years, 6 months ago

To help gpiolib not fiddle around with the internals of the irqchip
flag the chip as immutable, adding the calls into the gpiolib core
required to do so.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/pinctrl/pinctrl-at91.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 1e1813d7c550..8ecf52ec9b9b 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -1536,6 +1536,20 @@ static void at91_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 #define at91_gpio_dbg_show	NULL
 #endif
 
+static int gpio_irq_request_resources(struct irq_data *d)
+{
+	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
+
+	return gpiochip_lock_as_irq(&at91_gpio->chip, irqd_to_hwirq(d));
+}
+
+static void gpio_irq_release_resources(struct irq_data *d)
+{
+	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
+
+	gpiochip_unlock_as_irq(&at91_gpio->chip, irqd_to_hwirq(d));
+}
+
 /* Several AIC controller irqs are dispatched through this GPIO handler.
  * To use any AT91_PIN_* as an externally triggered IRQ, first call
  * at91_set_gpio_input() then maybe enable its glitch filter.
@@ -1555,6 +1569,9 @@ static void gpio_irq_mask(struct irq_data *d)
 	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
 	void __iomem	*pio = at91_gpio->regbase;
 	unsigned	mask = 1 << d->hwirq;
+	unsigned        gpio = irqd_to_hwirq(d);
+
+	gpiochip_disable_irq(&at91_gpio->chip, gpio);
 
 	if (pio)
 		writel_relaxed(mask, pio + PIO_IDR);
@@ -1565,6 +1582,9 @@ static void gpio_irq_unmask(struct irq_data *d)
 	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
 	void __iomem	*pio = at91_gpio->regbase;
 	unsigned	mask = 1 << d->hwirq;
+	unsigned        gpio = irqd_to_hwirq(d);
+
+	gpiochip_enable_irq(&at91_gpio->chip, gpio);
 
 	if (pio)
 		writel_relaxed(mask, pio + PIO_IER);
@@ -1731,12 +1751,15 @@ static int at91_gpio_of_irq_setup(struct platform_device *pdev,
 	at91_gpio->pioc_hwirq = irqd_to_hwirq(d);
 
 	gpio_irqchip->name = "GPIO";
+	gpio_irqchip->irq_request_resources = gpio_irq_request_resources;
+	gpio_irqchip->irq_release_resources = gpio_irq_release_resources;
 	gpio_irqchip->irq_ack = gpio_irq_ack;
 	gpio_irqchip->irq_disable = gpio_irq_mask;
 	gpio_irqchip->irq_mask = gpio_irq_mask;
 	gpio_irqchip->irq_unmask = gpio_irq_unmask;
 	gpio_irqchip->irq_set_wake = pm_ptr(gpio_irq_set_wake);
 	gpio_irqchip->irq_set_type = at91_gpio->ops->irq_type;
+	gpio_irqchip->flags = IRQCHIP_IMMUTABLE;
 
 	/* Disable irqs of this PIO controller */
 	writel_relaxed(~0, at91_gpio->regbase + PIO_IDR);
@@ -1747,7 +1770,7 @@ static int at91_gpio_of_irq_setup(struct platform_device *pdev,
 	 * interrupt.
 	 */
 	girq = &at91_gpio->chip.irq;
-	girq->chip = gpio_irqchip;
+	gpio_irq_chip_set_chip(girq, gpio_irqchip);
 	girq->default_type = IRQ_TYPE_NONE;
 	girq->handler = handle_edge_irq;
 

-- 
2.34.1

Re: [PATCH 1/2] pinctrl: at91: Make the irqchip immutable

Posted by Linus Walleij 2 years, 6 months ago

On Thu, Feb 16, 2023 at 4:49 PM Mark Brown <broonie@kernel.org> wrote:

> To help gpiolib not fiddle around with the internals of the irqchip
> flag the chip as immutable, adding the calls into the gpiolib core
> required to do so.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>

1) I'm impressed that you're using AT91 hardware

2) Can you respin this on top of my pinctrl devel branch:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel
There are some Andy-cleanups already queued for AT91 so I am a bit
worried of collisions. (If you feel confident they are orthogonal just
use v6.3-rc1)

Yours,
Linus Walleij

Re: [PATCH 1/2] pinctrl: at91: Make the irqchip immutable

Posted by Nicolas Ferre 2 years, 6 months ago

On 06/03/2023 at 18:50, Linus Walleij wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Thu, Feb 16, 2023 at 4:49 PM Mark Brown <broonie@kernel.org> wrote:
> 
>> To help gpiolib not fiddle around with the internals of the irqchip
>> flag the chip as immutable, adding the calls into the gpiolib core
>> required to do so.
>>
>> Signed-off-by: Mark Brown <broonie@kernel.org>
> 
> 1) I'm impressed that you're using AT91 hardware

I'm delighted that you're using AT91 hardware ;-)

Thanks for your help Mark!

Regards,
   Nicolas

> 2) Can you respin this on top of my pinctrl devel branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel
> There are some Andy-cleanups already queued for AT91 so I am a bit
> worried of collisions. (If you feel confident they are orthogonal just
> use v6.3-rc1)
> 
> Yours,
> Linus Walleij

-- 
Nicolas Ferre

Re: [PATCH 1/2] pinctrl: at91: Make the irqchip immutable

Posted by Mark Brown 2 years, 6 months ago

On Mon, Mar 06, 2023 at 02:20:56PM +0100, Linus Walleij wrote:

> 2) Can you respin this on top of my pinctrl devel branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel
> There are some Andy-cleanups already queued for AT91 so I am a bit
> worried of collisions. (If you feel confident they are orthogonal just
> use v6.3-rc1)

Argh, I'd already started testing with a rebase onto 6.3-rc1.  I've
restarted for your devel branch (neither rebase caused any conflicts).

Re: [PATCH 1/2] pinctrl: at91: Make the irqchip immutable

Posted by Claudiu.Beznea@microchip.com 2 years, 6 months ago

On 16.02.2023 17:43, Mark Brown wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> To help gpiolib not fiddle around with the internals of the irqchip
> flag the chip as immutable, adding the calls into the gpiolib core
> required to do so.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  drivers/pinctrl/pinctrl-at91.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 1e1813d7c550..8ecf52ec9b9b 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -1536,6 +1536,20 @@ static void at91_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
>  #define at91_gpio_dbg_show     NULL
>  #endif
> 
> +static int gpio_irq_request_resources(struct irq_data *d)
> +{
> +       struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> +
> +       return gpiochip_lock_as_irq(&at91_gpio->chip, irqd_to_hwirq(d));
> +}
> +
> +static void gpio_irq_release_resources(struct irq_data *d)
> +{
> +       struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> +
> +       gpiochip_unlock_as_irq(&at91_gpio->chip, irqd_to_hwirq(d));
> +}
> +
>  /* Several AIC controller irqs are dispatched through this GPIO handler.
>   * To use any AT91_PIN_* as an externally triggered IRQ, first call
>   * at91_set_gpio_input() then maybe enable its glitch filter.
> @@ -1555,6 +1569,9 @@ static void gpio_irq_mask(struct irq_data *d)
>         struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
>         void __iomem    *pio = at91_gpio->regbase;
>         unsigned        mask = 1 << d->hwirq;
> +       unsigned        gpio = irqd_to_hwirq(d);

There are spaces b/w unsigned and gpio. Also, irqd_to_hwirq() returns a
irq_hw_number_t.

> +
> +       gpiochip_disable_irq(&at91_gpio->chip, gpio);
> 
>         if (pio)
>                 writel_relaxed(mask, pio + PIO_IDR);
> @@ -1565,6 +1582,9 @@ static void gpio_irq_unmask(struct irq_data *d)
>         struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
>         void __iomem    *pio = at91_gpio->regbase;
>         unsigned        mask = 1 << d->hwirq;
> +       unsigned        gpio = irqd_to_hwirq(d);

ditto.

> +
> +       gpiochip_enable_irq(&at91_gpio->chip, gpio);
> 
>         if (pio)
>                 writel_relaxed(mask, pio + PIO_IER);
> @@ -1731,12 +1751,15 @@ static int at91_gpio_of_irq_setup(struct platform_device *pdev,
>         at91_gpio->pioc_hwirq = irqd_to_hwirq(d);
> 
>         gpio_irqchip->name = "GPIO";
> +       gpio_irqchip->irq_request_resources = gpio_irq_request_resources;
> +       gpio_irqchip->irq_release_resources = gpio_irq_release_resources;
>         gpio_irqchip->irq_ack = gpio_irq_ack;
>         gpio_irqchip->irq_disable = gpio_irq_mask;
>         gpio_irqchip->irq_mask = gpio_irq_mask;
>         gpio_irqchip->irq_unmask = gpio_irq_unmask;
>         gpio_irqchip->irq_set_wake = pm_ptr(gpio_irq_set_wake);
>         gpio_irqchip->irq_set_type = at91_gpio->ops->irq_type;
> +       gpio_irqchip->flags = IRQCHIP_IMMUTABLE;
> 
>         /* Disable irqs of this PIO controller */
>         writel_relaxed(~0, at91_gpio->regbase + PIO_IDR);
> @@ -1747,7 +1770,7 @@ static int at91_gpio_of_irq_setup(struct platform_device *pdev,
>          * interrupt.
>          */
>         girq = &at91_gpio->chip.irq;
> -       girq->chip = gpio_irqchip;
> +       gpio_irq_chip_set_chip(girq, gpio_irqchip);
>         girq->default_type = IRQ_TYPE_NONE;
>         girq->handler = handle_edge_irq;
> 
> 
> --
> 2.34.1
>