Re: [RFC PATCH 00/16] pinctrl: ralink: fix ABI, improve driver, move to mediatek, improve dt-bindings

[Arınç ÜNAL]

On 23.02.2023 07:58, Sergio Paracuellos wrote:
> Hi Arınç,
>
> All of this looks pretty good to me. You did a really big effort with
> this series. Thanks for doing this!
>
> On Wed, Feb 22, 2023 at 7:39 PM <arinc9.unal@xxxxxxxxx> wrote:
> > This is an ambitious effort I've been wanting to do for months.
> >
> > Straight off the bat, I'm fixing the ABI that I broke a while back, by
> > reintroducing the ralink,rt2880-pinmux compatible string.
> >
> > If you take a look at the schema for mt7620 and rt305x, some functions got
> > multiple lists for groups. Like refclk on mt7620. Because mt7620 and
> > mt7628/mt7688 SoCs use the same compatible string, it's impossible to
> > differentiate on the binding which SoC a devicetree is actually for.
> > Therefore, the binding will allow all groups listed for that function. For
> > example, if the SoC is mt7620, only the refclk function for the mdio group
> > can be used. If one were to put "spi cs1" as the function there, there
> > wouldn't be a warning.
> >
> > I address this by introducing new compatible strings for these SoCs, then
> > split the schemas. I also separate mt7628/mt7688 from mt7620 pinctrl
> > subdriver in the process.
> >
> > I wanted to split the rt305x driver too but too much code would be reused
> > so I backed down from that.
> >
> > Ralink was acquired by MediaTek in 2011. These SoCs have been rebranded as
> > MediaTek. We're moving the Ralink pinctrl driver to MediaTek, and rename
> > the schemas to mediatek.
> >
> > I've renamed the ralink core driver to mtmips. I decided to call the core
> > mtmips as I've seen folks from MediaTek use the same name when they added
> > support for MT7621 pinctrl on U-Boot. Feel free to comment on this.
> >
> > The MTMIPS pinctrl driver requires rt_sysc_membase from
> > arch/mips/ralink/of.c, so, for COMPILE_TEST to be useful, RALINK must be
> > selected. These headers, asm/mach-ralink/ralink_regs.h and
> > asm/mach-ralink/mt7620.h, from arch/mips/include are also required but
> > they can easily be included:
> >
> > ifeq ($(CONFIG_COMPILE_TEST),y)
> > CFLAGS_pinctrl-mtmips.o                 += -I$(srctree)/arch/mips/include
> > endif
> >
> > Sergio, do you see a way to make the pinctrl driver independent of
> > architecture code? At least avoid using rt_sysc_membase.
>
> The only really dependent architecture code in these drivers now is
> because of the use of
> 'rt_sysc_r32()' and 'rt_sysc_w32()' in 'ralink_pmx_group_enable()'
> function [0]. This is just to set the gpio mode. The read and write
> registers here  SYSC_REG_GPIO_MODE and  SYSC_REG_GPIO_MODE2 are in the
> system controller area. In all single ralink platform 'sysc' nodes
> should be a syscon that can be accessed from the driver side. That way
> you can just get those syscon areas via regmap APIs and properly read
> and write desired registers. For the mt7621.dtsi file, the node is
> already a syscon [1]. Other ralink device tree files should also be
> modified to include this in its 'sysc' node (I think in openWRT dts
> files at least for mt7620 is already included). You have to add that
> in all of them since 'ralink_pmx_group_enable()' is common code for
> all. I think this can be done in a different patch series. I can help
> you to do this after this series is reviewed and accepted.

Awesome, thanks a lot! Note to self, "depends on RALINK" menu entries 
should be changed to "depends on OF" when this happens.

Arınç