Re: [PATCH 0/14] Refactor Ralink Pinctrl and Add Documentation

[Arınç ÜNAL]

Hi Sergio,

On 13/04/2022 09:36, Sergio Paracuellos wrote:
> Hi Arinç,
>
> On Wed, Apr 13, 2022 at 8:08 AM Arınç ÜNAL <arinc.unal@xxxxxxxxxx> wrote:
> > Hey everyone.
> >
> > This patch series brings complete refactoring to the Ralink pinctrl driver
> > and its subdrivers.
> >
> > The mt7620 pinctrl subdriver supports MT7628 and MT7688 SoCs along with
> > MT7620. These two share the same pin layout. The code used for MT7628 and
> > MT7688 is renamed from MT7628/mt7628an to MT76X8.
> >
> > Ralink pinctrl driver is called rt2880 which is the name of the Ralink
> > RT2880 SoC. A subdriver for the Ralink RT2880 SoC is called rt288x. Rename
> > rt2880 to ralink.
> >
> > Rename code from pinmux to pinctrl for where the operation is not about the
> > muxing of pins.
> >
> > Rename rt288x pinctrl subdriver for the RT2880 SoC to rt2880.
> >
> > Variables for functions include "grp" on the Ralink MT7620 and MT7621
> > subdrivers. Rename them to "func" instead as they define the functions for
> > the pin groups. This is already the case for the other 3 subdrivers;
> > RT2880, RT305x, RT3883.
> >
> > Fix Kconfig to call the subdrivers, well, subdrivers.
> >
> > Add new compatible strings for each subdriver and update DT binding
> > accordingly.
> >
> > Add Ralink pinctrl driver to MAINTAINERS and add me and Sergio as the
> > maintainers.
> >
> > Finally, fix the current rt2880 documentation and add binding for all of
> > the subdrivers.
> >
> > I have the patches here should anyone prefer to read them there:
> > https://github.com/arinc9/linux/commits/ralink-pinctrl-refactor
> >
> > Ralink pinctrl driver and the subdrivers were compile tested.
> > MT7621 pinctrl subdriver was tested on a private mt7621 board.
> > YAML bindings checked with:
> > ARCH=mips CROSS_COMPILE=mips-linux-gnu- make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/pinctrl/ -j$(nproc)
> >
> > Arınç ÜNAL (14):
> >    pinctrl: ralink: rename MT7628(an) functions to MT76X8
> >    pinctrl: ralink: rename pinctrl-rt2880 to pinctrl-ralink
> >    pinctrl: ralink: rename pinmux functions to pinctrl
> >    pinctrl: ralink: rename pinctrl-rt288x to pinctrl-rt2880
> >    pinctrl: ralink: rename variable names for functions on MT7620 and MT7621
> >    pinctrl: ralink: rename driver names to subdrivers
> >    MAINTAINERS: add Ralink pinctrl driver
>
> For all these rename stuff and MAINTAINERS change:
>
> Reviewed-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
>
> >    pinctrl: ralink: add new compatible strings for each pinctrl subdriver
> >    mips: dts: ralink: mt7621: use the new compatible string for MT7621 pinctrl
> >    dt-bindings: pinctrl: rt2880: fix binding name, pin groups and functions
> >    dt-bindings: pinctrl: add binding for Ralink MT7620 pinctrl
> >    dt-bindings: pinctrl: add binding for Ralink MT7621 pinctrl
> >    dt-bindings: pinctrl: add binding for Ralink RT305X pinctrl
> >    dt-bindings: pinctrl: add binding for Ralink RT3883 pinctrl
>
> I think you cannot change compatible strings because you have to be
> compatible with previous stuff. That is the reason why when I
> refactored all of this stuff from 'arch/mips/ralink' into
> 'drivers/pinctrl' I maintained the same for all of them and only
> created one binding for all. I know that these SoCs are mostly used in
> openWRT and the way of doing things there is that when a new version
> is released a new dtb is also compiled so I understand the motivation
> of the change itself. In any case, Rob has the last word here, not me
> :).

I looked around pinctrl drivers for mediatek. What I see there is that 
each subdriver has its own compatible string. There's a documentation 
for each subdriver. Each subdriver contains different pin groups and 
functions like on ralink. My patch series basically does the same for 
Ralink.

I don't see this patch series causing much of an issue for OpenWrt. 
They're going to have to update their Ralink DTs with the new compatible 
strings when they either switch to the next LTS kernel or decide to 
backport Ralink pinctrl changes to the current LTS kernel.

Cheers.
Arınç