Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220209162607.1118325-1-maz@kernel.org> <20220209162607.1118325-11-maz@kernel.org>
In-Reply-To: <20220209162607.1118325-11-maz@kernel.org>
From:   Emil Renner Berthing <kernel@esmil.dk>
Date:   Thu, 10 Feb 2022 00:30:55 +0100
Message-ID: <CANBLGcwKeLn7Q1Ra8pCw=cXy=kJeEFRmBjOxjds10+k70LvzXA@mail.gmail.com>
Subject: Re: [PATCH 10/10] pinctrl: starfive: Switch to dynamic chip name output
To:     Marc Zyngier <maz@kernel.org>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        Bartosz Golaszewski <brgl@bgdev.pl>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Grygorii Strashko <grygorii.strashko@ti.com>,
        Santosh Shilimkar <ssantosh@kernel.org>,
        Kevin Hilman <khilman@kernel.org>,
        Tony Lindgren <tony@atomide.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Vladimir Zapolskiy <vz@mleia.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Gregory Clement <gregory.clement@bootlin.com>,
        Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
        kernel-team@android.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 9 Feb 2022 at 17:49, Marc Zyngier <maz@kernel.org> wrote:
>
> Instead of overloading the name field, use the relevant callback to
> output the device name.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/pinctrl/pinctrl-starfive.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-starfive.c b/drivers/pinctrl/pinctrl-starfive.c
> index 5be9866c2b3c..f29d9ccf858b 100644
> --- a/drivers/pinctrl/pinctrl-starfive.c
> +++ b/drivers/pinctrl/pinctrl-starfive.c
> @@ -15,6 +15,7 @@
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/reset.h>
> +#include <linux/seq_file.h>
>  #include <linux/spinlock.h>
>
>  #include <linux/pinctrl/pinctrl.h>
> @@ -1163,12 +1164,20 @@ static int starfive_irq_set_type(struct irq_data *d, unsigned int trigger)
>         return 0;
>  }
>
> +static void starfive_irq_print_chip(struct irq_data *d, struct seq_file *p)
> +{
> +       struct starfive_pinctrl *sfp = starfive_from_irq_data(d);
> +
> +       seq_printf(p, sfp->gc.label);
> +}
> +
>  static struct irq_chip starfive_irq_chip = {
>         .irq_ack = starfive_irq_ack,
>         .irq_mask = starfive_irq_mask,
>         .irq_mask_ack = starfive_irq_mask_ack,
>         .irq_unmask = starfive_irq_unmask,
>         .irq_set_type = starfive_irq_set_type,
> +       .irq_print_chip = starfive_irq_print_chip,
>         .flags = IRQCHIP_SET_TYPE_MASKED,
>  };

The parent interrupt doesn't show up in /proc/interrupts anyway, so if
setting the name is considered abuse we can just drop the addition
above and just delete the two lines below.

The gpio framework seems to fill in default handlers in the struct
above, so unfortunately it can't yet be made const. Is this something
you intend to fix in the future?

> @@ -1307,8 +1316,6 @@ static int starfive_probe(struct platform_device *pdev)
>         sfp->gc.base = -1;
>         sfp->gc.ngpio = NR_GPIOS;
>
> -       starfive_irq_chip.name = sfp->gc.label;
> -
>         sfp->gc.irq.chip = &starfive_irq_chip;
>         sfp->gc.irq.parent_handler = starfive_gpio_irq_handler;
>         sfp->gc.irq.num_parents = 1;
> --
> 2.30.2
>