linux-kernel - Re: [PATCH v2 1/1] gpio: tegra: Convert to immutable irq chip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAMuHMdVy2YPY=35tBUj-_p=VvkGpJH4=OrDQLjFvr6+pKv3JRA@mail.gmail.com>
Date:   Tue, 18 Oct 2022 09:28:23 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Svyatoslav Ryhel <clamor95@...il.com>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Dmitry Osipenko <digetx@...il.com>, linux-gpio@...r.kernel.org,
        linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] gpio: tegra: Convert to immutable irq chip

Hi Svyatoslav,

On Sat, Oct 15, 2022 at 7:30 PM Svyatoslav Ryhel <clamor95@...il.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@...il.com> # TF201 T30
> Tested-by: Robert Eckelmann <longnoserob@...il.com> # TF101 T20
> Signed-off-by: Svyatoslav Ryhel <clamor95@...il.com>
> Reviewed-by: Dmitry Osipenko <digetx@...il.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@...erman.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@...ux-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