A few cleanups for the at91 driver, making the GPIO irqchip
immutable and removing an unused member from the driver data.
The driver is still using statically assigned GPIO numbers, we
can't just remove that since the driver itself is still relying
on them even if there are no longer board files for this
platform.
Signed-off-by: Mark Brown <[email protected]>
---
Mark Brown (2):
pinctrl: at91: Make the irqchip immutable
pinctrl: at91: Remove pioc_index from struct at91_gpio_chip
drivers/pinctrl/pinctrl-at91.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
---
base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
change-id: 20230216-gpio-at91-immutable-53fcb995b285
Best regards,
--
Mark Brown <[email protected]>
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 <[email protected]>
---
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
The pioc_idx member of struct at91_gpio_chip is write only, just remove it.
Signed-off-by: Mark Brown <[email protected]>
---
drivers/pinctrl/pinctrl-at91.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 8ecf52ec9b9b..a1db6ac9bd2e 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -41,7 +41,6 @@ struct at91_pinctrl_mux_ops;
* @next: bank sharing same clock
* @pioc_hwirq: PIO bank interrupt identifier on AIC
* @pioc_virq: PIO bank Linux virtual interrupt
- * @pioc_idx: PIO bank index
* @regbase: PIO bank virtual address
* @clock: associated clock
* @ops: at91 pinctrl mux ops
@@ -55,7 +54,6 @@ struct at91_gpio_chip {
struct at91_gpio_chip *next;
int pioc_hwirq;
int pioc_virq;
- int pioc_idx;
void __iomem *regbase;
struct clk *clock;
const struct at91_pinctrl_mux_ops *ops;
@@ -1867,7 +1865,6 @@ static int at91_gpio_probe(struct platform_device *pdev)
at91_chip->ops = (const struct at91_pinctrl_mux_ops *)
of_match_device(at91_gpio_of_match, &pdev->dev)->data;
at91_chip->pioc_virq = irq;
- at91_chip->pioc_idx = alias_idx;
at91_chip->clock = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(at91_chip->clock)) {
--
2.34.1
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 <[email protected]>
> ---
> 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
>
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
>
> The pioc_idx member of struct at91_gpio_chip is write only, just remove it.
>
> Signed-off-by: Mark Brown <[email protected]>
Reviewed-by: Claudiu Beznea <[email protected]>
> ---
> drivers/pinctrl/pinctrl-at91.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 8ecf52ec9b9b..a1db6ac9bd2e 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -41,7 +41,6 @@ struct at91_pinctrl_mux_ops;
> * @next: bank sharing same clock
> * @pioc_hwirq: PIO bank interrupt identifier on AIC
> * @pioc_virq: PIO bank Linux virtual interrupt
> - * @pioc_idx: PIO bank index
> * @regbase: PIO bank virtual address
> * @clock: associated clock
> * @ops: at91 pinctrl mux ops
> @@ -55,7 +54,6 @@ struct at91_gpio_chip {
> struct at91_gpio_chip *next;
> int pioc_hwirq;
> int pioc_virq;
> - int pioc_idx;
> void __iomem *regbase;
> struct clk *clock;
> const struct at91_pinctrl_mux_ops *ops;
> @@ -1867,7 +1865,6 @@ static int at91_gpio_probe(struct platform_device *pdev)
> at91_chip->ops = (const struct at91_pinctrl_mux_ops *)
> of_match_device(at91_gpio_of_match, &pdev->dev)->data;
> at91_chip->pioc_virq = irq;
> - at91_chip->pioc_idx = alias_idx;
>
> at91_chip->clock = devm_clk_get(&pdev->dev, NULL);
> if (IS_ERR(at91_chip->clock)) {
>
> --
> 2.34.1
>
On Thu, Feb 16, 2023 at 4:49 PM Mark Brown <[email protected]> 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 <[email protected]>
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
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).
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 <[email protected]> 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 <[email protected]>
>
> 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