Messages in this thread | | | Date | Thu, 23 Feb 2023 09:01:59 +0300 | Subject | Re: [RFC PATCH 00/16] pinctrl: ralink: fix ABI, improve driver, move to mediatek, improve dt-bindings | From | 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@gmail.com> 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ç
| |