Re: [PATCH 10/10] pinctrl: starfive: Switch to dynamic chip name output

From: Marc Zyngier <maz@kernel.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Emil Renner Berthing <kernel@esmil.dk>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.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
Subject: Re: [PATCH 10/10] pinctrl: starfive: Switch to dynamic chip name output
Date: Fri, 11 Feb 2022 09:20:17 +0000	[thread overview]
Message-ID: <87ee49lpym.wl-maz@kernel.org> (raw)
In-Reply-To: <CACRpkdbo4seF6t6iFgKhaTZ_PKMdCt+2gSmdCaM+vDeHGZfHcQ@mail.gmail.com>

On Fri, 11 Feb 2022 00:15:25 +0000,
Linus Walleij <linus.walleij@linaro.org> wrote:
> 
> On Thu, Feb 10, 2022 at 10:06 AM Marc Zyngier <maz@kernel.org> wrote:
> > On Wed, 09 Feb 2022 23:30:55 +0000,
> > Emil Renner Berthing <kernel@esmil.dk> wrote:
> 
> > > 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?
> >
> > This is next on my list of things to address. The whole 'let's copy a
> > whole irqchip structure and hijack random pointers' should not have
> > happened, and it certainly is going to be an interesting ride.
> 
> Sorry about that... Probably my bad idea. The only upside is that the
> things that are ugly are centralized to one spot.

No worries. I should have kept an eye on that too and spotted it
earlier. It is only when helping with the M1 GPIO driver that this was
brought to my attention.

My current approach is to move each and every driver to using the
existing helpers, and advertise to the core GPIO code that they don't
need to be fiddled with a new irqchip flag. It is a rather simple
change, but there are a lot of drivers (I have so far converted the
Apple, Qualcomm and Tegra186 drivers, as this is the HW I have
around), allowing their irq_chip structures to be made const and only
used by reference.

Once all drivers are converted (one day), we can drop the flag and the
pointer swapping code.

The alternative approach was to use a hierarchical irqchip provided by
the GPIO code, but this proved difficult as it would need to know
which methods to implement depending on the flow used. There are also
only a handful of drivers using the hierarchical mode anyway, and we'd
be stuck for all the other drivers.

Anyway, I'll post something shortly with the stuff I have changed, and
we can happily repaint that bike shed.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.