Message-ID: <CAFBinCAP_ohHiUEv_r+mEMVBz_XDNdAjw0FjeUhwPY_U1KiXuw@mail.gmail.com>
Date: Fri, 27 Dec 2024 22:01:21 +0100
From: Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To: Linus Walleij <linus.walleij@...aro.org>
Cc: Xianwei Zhao <xianwei.zhao@...ogic.com>, Huqiang Qin <huqiang.qin@...ogic.com>, 
	Qianggui Song <qianggui.song@...ogic.com>, Hyeonki Hong <hhk7734@...il.com>, 
	Beniamino Galvani <b.galvani@...il.com>, Carlo Caione <carlo@...lessm.com>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Kevin Hilman <khilman@...libre.com>, 
	Jerome Brunet <jbrunet@...libre.com>, linux-gpio@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-amlogic@...ts.infradead.org
Subject: Re: [PATCH RFC 2/3] pinctrl: Add driver support for Amlogic SoCs

Hi Linus,

On Fri, Dec 27, 2024 at 6:19 PM Linus Walleij <linus.walleij@...aro.org> wrote:
>
> Newcomers, latest patch set:
> https://lore.kernel.org/linux-gpio/20241226-amlogic-pinctrl-v2-0-cdae42a67b76@amlogic.com/
>
> I included some of the prior meson authors on the to line to see if
> their mail addresses still work and if they have some feedback on this.
>
> On Sun, Dec 22, 2024 at 10:08 AM Linus Walleij <linus.walleij@...aro.org> wrote:
>
> > > > - Renaming drivers/pinctrl/sunxi to drivers/pinctrl/amlogic
> > > >    so we keep this sorted by actual vendor, sunxi is apparently
> > > >    yours (AMlogic:s) isn't it?
> > >
> > > It isn't. Sunxi is Allwinner SoCs.
> >
> > My apologies. I mixed it up completely. :(
>
> But wait a minute. I see there is meson. And in the "meson" subdirectory
> there is stuff named "amlogic" ...
Until some point Amlogic had all of their SoCs named meson along with
a suffix which identifies the silicon die (for example: Amlogic
Meson8, Amlogic Meson GXM, ...).
The "meson" name was dropped at some point. Taking the example from
this series, the SoC name is: Amlogic A4 (which would have been called
Amlogic Meson A4 previously).

[...]
> > What do you think of the idea of a separate drivers/pinctrl/amlogic directory
> > though? I think there are already quite a few amlogic SoCs that need
> > to be supported and more will come.
>
> So what about renaming the existing subdir "meson" to "amlogic"
> and put the driver there.
That will make it consistent with other Amlogic related directories
within the Linux kernel:
$ find {arch,drivers} -type d -name "amlogic"
arch/arm64/boot/dts/amlogic
arch/arm/boot/dts/amlogic
drivers/soc/amlogic
drivers/phy/amlogic
drivers/pmdomain/amlogic
drivers/reset/amlogic
drivers/media/platform/amlogic
drivers/crypto/amlogic
drivers/perf/amlogic

> Also I want to know if this driver and hardware shares anything with
> the existing drivers in that directory. It sometimes happen that
> developers start something from scratch despite the existence of
> prior art simply because of organizational issues, and we don't want
> that kind of situation to leak over to the kernel.
I will summarize the situation in my own words. If there are any
inconsistencies then I'm asking others (Xianwei, Rob, Neil, ...) to
point them out and correct me.

Initially Xianwei started adding support for the Amlogic A4 SoC in June: [0]
In this initial series he re-used all the support code from
drivers/pinctrl/meson (the new A4 SoC driver simply selected
PINCTRL_MESON_AXG_PMX).
This means that we have two gpio-cells:
- the first is a number which identifies the pin/pad (which included
the bank and pin/pad within that bank as a consecutive numbering
starting at 0)
- the GPIO flags

Up until v4 of that series [1] the driver side was pretty much the
same as it was in v1.
However, even with v1 there sparked some feedback on the dt-bindings.

With v5 of that series [2] Xianwei changed the dt-binding (based on
the input from the DT maintainers) to take the GPIO to take three
gpio-cells:
- first the GPIO bank number
- then the pin/pad within that bank
- the GPIO flags

An overview of the changes up until this point can be found in the
cover-letter of v6 of that series: [3]

With the current series Xianwei started over, with a new driver (v1:
[4] - and Linus, you have linked to v2 yourself: [5]) so it fits (to
what I understand is) the desired dt-bindings.
The main difference to the "old" dt-bindings is that driver changes
are not needed to add or fix pin muxes (old binding: .dts only had
references to pin groups and pin functions, the mapping to register
bits happened within the driver - new binding: all pin muxing
registers and bits are described in .dts).
The "old" approach is more similar to what Allwinner SoC
drivers/dt-bindings use whereas the "new" approach is more similar to
what Rockchip SoC drivers/dt-bindings use.

To put it into other words: the new driver (from this series) is a new
dt-binding to hardware translation layer. The hardware side matches
what we already have in
drivers/pinctrl/meson/{pinctrl-meson.c,pinctrl-meson-axg-pmx.c} but
the dt-binding side is completely different.

Generally I expect the new driver to be backwards compatible to all
SoCs that used the old dt-binding and the PINCTRL_MESON_AXG_PMX
sub-driver in drivers/pinctrl/meson/
Those are:
$ git grep -l '&meson_axg_pmx_ops' drivers/pinctrl/meson/
drivers/pinctrl/meson/pinctrl-amlogic-c3.c
drivers/pinctrl/meson/pinctrl-amlogic-t7.c
drivers/pinctrl/meson/pinctrl-meson-a1.c
drivers/pinctrl/meson/pinctrl-meson-axg.c
drivers/pinctrl/meson/pinctrl-meson-g12a.c
drivers/pinctrl/meson/pinctrl-meson-s4.c

(That said, I am not aware of any plans to migrate these SoCs over)

So to conclude: This whole series is not about a new pinmux controller
IP but a new dt-binding for the existing IP.
In the old series (v1 at [0] - which shows that the underlying pinmux
IP has not changed since the AXG generation) Xianwei tried to add
support for the new SoC with the dt-binding that other Amlogic SoCs
are already using - but this got rejected by the DT maintainers.


Best regards,
Martin


[0] https://lore.kernel.org/all/20240611-a4_pinctrl-v1-2-dc487b1977b3@amlogic.com/
[1] https://lore.kernel.org/all/20241101-a4_pinctrl-v4-0-efd98edc3ad4@amlogic.com/
[2] https://lore.kernel.org/all/20241112-a4_pinctrl-v5-0-3460ce10c480@amlogic.com/
[3] https://lore.kernel.org/all/20241113-a4_pinctrl-v6-0-35ba2401ee35@amlogic.com/
[4] https://lore.kernel.org/all/20241211-amlogic-pinctrl-v1-0-410727335119@amlogic.com/
[5] https://lore.kernel.org/linux-gpio/20241226-amlogic-pinctrl-v2-0-cdae42a67b76@amlogic.com/

Powered by blists - more mailing lists