Hi Linus, On Fri, Dec 27, 2024 at 6:19 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > Newcomers, latest patch set: > https://lore.kernel.org/linux-gpio/20241226-amlogic-pinctrl-v2-0-cdae42a67b76@xxxxxxxxxxx/ > > 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@xxxxxxxxxx> 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@xxxxxxxxxxx/ [1] https://lore.kernel.org/all/20241101-a4_pinctrl-v4-0-efd98edc3ad4@xxxxxxxxxxx/ [2] https://lore.kernel.org/all/20241112-a4_pinctrl-v5-0-3460ce10c480@xxxxxxxxxxx/ [3] https://lore.kernel.org/all/20241113-a4_pinctrl-v6-0-35ba2401ee35@xxxxxxxxxxx/ [4] https://lore.kernel.org/all/20241211-amlogic-pinctrl-v1-0-410727335119@xxxxxxxxxxx/ [5] https://lore.kernel.org/linux-gpio/20241226-amlogic-pinctrl-v2-0-cdae42a67b76@xxxxxxxxxxx/