[v2,1/1] gpio: tegra: Convert to immutable irq chip
Commit Message
Update the driver to use an immutable IRQ chip to fix this warning:
"not an immutable chip, please consider fixing it!"
Preserve per-chip labels by adding an ->irq_print_chip() callback.
Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # TF201 T30
Tested-by: Robert Eckelmann <longnoserob@gmail.com> # TF101 T20
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
---
Changes since V1:
- Added tested-by info from testers
- Patch itself is same, it had no review for 3 weeks
- Test robot is triggering for pm_sleep_ptr, should not it be used to omit CONFIG_PM_SLEEP ifdefs?
drivers/gpio/gpio-tegra.c | 56 ++++++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 16 deletions(-)
Comments
On Sat, Oct 15, 2022 at 7:26 PM Svyatoslav Ryhel <clamor95@gmail.com> wrote:
>
> Update the driver to use an immutable IRQ chip to fix this warning:
>
> "not an immutable chip, please consider fixing it!"
>
> Preserve per-chip labels by adding an ->irq_print_chip() callback.
>
> Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # TF201 T30
> Tested-by: Robert Eckelmann <longnoserob@gmail.com> # TF101 T20
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> ---
Applied, thanks!
Bart
Hi Svyatoslav,
On Sat, Oct 15, 2022 at 7:30 PM Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> Update the driver to use an immutable IRQ chip to fix this warning:
>
> "not an immutable chip, please consider fixing it!"
>
> Preserve per-chip labels by adding an ->irq_print_chip() callback.
>
> Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # TF201 T30
> Tested-by: Robert Eckelmann <longnoserob@gmail.com> # TF101 T20
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Thanks for your patch!
Thanks for your patch, which is now commit 6ebd28bd087127ab
("gpio: tegra: Convert to immutable irq chip") in next-20221018.
noreply@ellerman.id.au reports a build failure introduced by
this commit on e.g. m68k-allmodconfig:
drivers/gpio/gpio-tegra.c:616:48: error: 'tegra_gpio_irq_set_wake'
undeclared here (not in a function); did you mean
'tegra_gpio_irq_set_type'?
> index e4fb4cb38a0f..6b469253fad8 100644
> --- a/drivers/gpio/gpio-tegra.c
> +++ b/drivers/gpio/gpio-tegra.c
> @@ -598,10 +600,43 @@ static void tegra_gpio_irq_release_resources(struct irq_data *d)
> tegra_gpio_enable(tgi, d->hwirq);
> }
>
> +static void tegra_gpio_irq_print_chip(struct irq_data *d, struct seq_file *s)
> +{
> + struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> +
> + seq_printf(s, dev_name(chip->parent));
> +}
> +
> +static const struct irq_chip tegra_gpio_irq_chip = {
> + .irq_shutdown = tegra_gpio_irq_shutdown,
> + .irq_ack = tegra_gpio_irq_ack,
> + .irq_mask = tegra_gpio_irq_mask,
> + .irq_unmask = tegra_gpio_irq_unmask,
> + .irq_set_type = tegra_gpio_irq_set_type,
> + .irq_set_wake = pm_sleep_ptr(tegra_gpio_irq_set_wake),
This is an unrelated change, breaking the build if CONFIG_PM_SLEEP
is not set: the function pointer argument of pm_sleep_ptr() is always
referenced, so the function definition must not be protected by #ifdef
CONFIG_PM_SLEEP.
As tegra_gpio_{resume,suspend}() are also in that section, you probably
want to convert the references to them to the new macros introduced by
commit 1a3c7bb088266fa2 ("PM: core: Add new *_PM_OPS macros,
deprecate old ones"), too.
Or just revert this change for now.
> + .irq_print_chip = tegra_gpio_irq_print_chip,
> + .irq_request_resources = tegra_gpio_irq_request_resources,
> + .irq_release_resources = tegra_gpio_irq_release_resources,
> + .flags = IRQCHIP_IMMUTABLE,
> +};
> +
> +static const struct irq_chip tegra210_gpio_irq_chip = {
> + .irq_shutdown = tegra_gpio_irq_shutdown,
> + .irq_ack = tegra_gpio_irq_ack,
> + .irq_mask = tegra_gpio_irq_mask,
> + .irq_unmask = tegra_gpio_irq_unmask,
> + .irq_set_affinity = tegra_gpio_irq_set_affinity,
> + .irq_set_type = tegra_gpio_irq_set_type,
> + .irq_set_wake = pm_sleep_ptr(tegra_gpio_irq_set_wake),
> + .irq_print_chip = tegra_gpio_irq_print_chip,
> + .irq_request_resources = tegra_gpio_irq_request_resources,
> + .irq_release_resources = tegra_gpio_irq_release_resources,
> + .flags = IRQCHIP_IMMUTABLE,
> +};
> +
> #ifdef CONFIG_DEBUG_FS
>
> #include <linux/debugfs.h>
> -#include <linux/seq_file.h>
>
> static int tegra_dbg_gpio_show(struct seq_file *s, void *unused)
> {
> @@ -689,18 +724,6 @@ static int tegra_gpio_probe(struct platform_device *pdev)
> tgi->gc.ngpio = tgi->bank_count * 32;
> tgi->gc.parent = &pdev->dev;
>
> - tgi->ic.name = "GPIO";
> - tgi->ic.irq_ack = tegra_gpio_irq_ack;
> - tgi->ic.irq_mask = tegra_gpio_irq_mask;
> - tgi->ic.irq_unmask = tegra_gpio_irq_unmask;
> - tgi->ic.irq_set_type = tegra_gpio_irq_set_type;
> - tgi->ic.irq_shutdown = tegra_gpio_irq_shutdown;
> -#ifdef CONFIG_PM_SLEEP
> - tgi->ic.irq_set_wake = tegra_gpio_irq_set_wake;
> -#endif
> - tgi->ic.irq_request_resources = tegra_gpio_irq_request_resources;
> - tgi->ic.irq_release_resources = tegra_gpio_irq_release_resources;
> -
> platform_set_drvdata(pdev, tgi);
>
> if (tgi->soc->debounce_supported)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
@@ -18,6 +18,7 @@
#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/module.h>
+#include <linux/seq_file.h>
#include <linux/irqdomain.h>
#include <linux/irqchip/chained_irq.h>
#include <linux/pinctrl/consumer.h>
@@ -94,7 +95,6 @@ struct tegra_gpio_info {
struct tegra_gpio_bank *bank_info;
const struct tegra_gpio_soc_config *soc;
struct gpio_chip gc;
- struct irq_chip ic;
u32 bank_count;
unsigned int *irqs;
};
@@ -288,6 +288,7 @@ static void tegra_gpio_irq_mask(struct irq_data *d)
unsigned int gpio = d->hwirq;
tegra_gpio_mask_write(tgi, GPIO_MSK_INT_ENB(tgi, gpio), gpio, 0);
+ gpiochip_disable_irq(chip, gpio);
}
static void tegra_gpio_irq_unmask(struct irq_data *d)
@@ -296,6 +297,7 @@ static void tegra_gpio_irq_unmask(struct irq_data *d)
struct tegra_gpio_info *tgi = gpiochip_get_data(chip);
unsigned int gpio = d->hwirq;
+ gpiochip_enable_irq(chip, gpio);
tegra_gpio_mask_write(tgi, GPIO_MSK_INT_ENB(tgi, gpio), gpio, 1);
}
@@ -598,10 +600,43 @@ static void tegra_gpio_irq_release_resources(struct irq_data *d)
tegra_gpio_enable(tgi, d->hwirq);
}
+static void tegra_gpio_irq_print_chip(struct irq_data *d, struct seq_file *s)
+{
+ struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+
+ seq_printf(s, dev_name(chip->parent));
+}
+
+static const struct irq_chip tegra_gpio_irq_chip = {
+ .irq_shutdown = tegra_gpio_irq_shutdown,
+ .irq_ack = tegra_gpio_irq_ack,
+ .irq_mask = tegra_gpio_irq_mask,
+ .irq_unmask = tegra_gpio_irq_unmask,
+ .irq_set_type = tegra_gpio_irq_set_type,
+ .irq_set_wake = pm_sleep_ptr(tegra_gpio_irq_set_wake),
+ .irq_print_chip = tegra_gpio_irq_print_chip,
+ .irq_request_resources = tegra_gpio_irq_request_resources,
+ .irq_release_resources = tegra_gpio_irq_release_resources,
+ .flags = IRQCHIP_IMMUTABLE,
+};
+
+static const struct irq_chip tegra210_gpio_irq_chip = {
+ .irq_shutdown = tegra_gpio_irq_shutdown,
+ .irq_ack = tegra_gpio_irq_ack,
+ .irq_mask = tegra_gpio_irq_mask,
+ .irq_unmask = tegra_gpio_irq_unmask,
+ .irq_set_affinity = tegra_gpio_irq_set_affinity,
+ .irq_set_type = tegra_gpio_irq_set_type,
+ .irq_set_wake = pm_sleep_ptr(tegra_gpio_irq_set_wake),
+ .irq_print_chip = tegra_gpio_irq_print_chip,
+ .irq_request_resources = tegra_gpio_irq_request_resources,
+ .irq_release_resources = tegra_gpio_irq_release_resources,
+ .flags = IRQCHIP_IMMUTABLE,
+};
+
#ifdef CONFIG_DEBUG_FS
#include <linux/debugfs.h>
-#include <linux/seq_file.h>
static int tegra_dbg_gpio_show(struct seq_file *s, void *unused)
{
@@ -689,18 +724,6 @@ static int tegra_gpio_probe(struct platform_device *pdev)
tgi->gc.ngpio = tgi->bank_count * 32;
tgi->gc.parent = &pdev->dev;
- tgi->ic.name = "GPIO";
- tgi->ic.irq_ack = tegra_gpio_irq_ack;
- tgi->ic.irq_mask = tegra_gpio_irq_mask;
- tgi->ic.irq_unmask = tegra_gpio_irq_unmask;
- tgi->ic.irq_set_type = tegra_gpio_irq_set_type;
- tgi->ic.irq_shutdown = tegra_gpio_irq_shutdown;
-#ifdef CONFIG_PM_SLEEP
- tgi->ic.irq_set_wake = tegra_gpio_irq_set_wake;
-#endif
- tgi->ic.irq_request_resources = tegra_gpio_irq_request_resources;
- tgi->ic.irq_release_resources = tegra_gpio_irq_release_resources;
-
platform_set_drvdata(pdev, tgi);
if (tgi->soc->debounce_supported)
@@ -733,7 +756,6 @@ static int tegra_gpio_probe(struct platform_device *pdev)
}
irq = &tgi->gc.irq;
- irq->chip = &tgi->ic;
irq->fwnode = of_node_to_fwnode(pdev->dev.of_node);
irq->child_to_parent_hwirq = tegra_gpio_child_to_parent_hwirq;
irq->populate_parent_alloc_arg = tegra_gpio_populate_parent_fwspec;
@@ -752,7 +774,9 @@ static int tegra_gpio_probe(struct platform_device *pdev)
if (!irq->parent_domain)
return -EPROBE_DEFER;
- tgi->ic.irq_set_affinity = tegra_gpio_irq_set_affinity;
+ gpio_irq_chip_set_chip(irq, &tegra210_gpio_irq_chip);
+ } else {
+ gpio_irq_chip_set_chip(irq, &tegra_gpio_irq_chip);
}
tgi->regs = devm_platform_ioremap_resource(pdev, 0);