mbox series

[0/14] Refactor Ralink Pinctrl and Add Documentation

Message ID 20220413060729.27639-1-arinc.unal@arinc9.com (mailing list archive)
Headers show
Series Refactor Ralink Pinctrl and Add Documentation | expand

Message

Arınç ÜNAL April 13, 2022, 6:07 a.m. UTC
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
  pinctrl: ralink: add new compatible strings for each pinctrl subdriver
  MAINTAINERS: add Ralink pinctrl driver
  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

 Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml   |  87 ++++++++
 .../pinctrl/{ralink,rt2880-pinmux.yaml => ralink,mt7621-pinctrl.yaml}  |  25 ++-
 Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinctrl.yaml   |  68 ++++++
 Documentation/devicetree/bindings/pinctrl/ralink,rt305x-pinctrl.yaml   |  89 ++++++++
 Documentation/devicetree/bindings/pinctrl/ralink,rt3883-pinctrl.yaml   |  69 ++++++
 MAINTAINERS                                                            |   7 +
 arch/mips/boot/dts/ralink/mt7621.dtsi                                  |   2 +-
 drivers/pinctrl/ralink/Kconfig                                         |  28 +--
 drivers/pinctrl/ralink/Makefile                                        |   4 +-
 drivers/pinctrl/ralink/pinctrl-mt7620.c                                | 302 ++++++++++++-------------
 drivers/pinctrl/ralink/pinctrl-mt7621.c                                |  76 +++----
 drivers/pinctrl/ralink/pinctrl-ralink.c                                | 349 +++++++++++++++++++++++++++++
 drivers/pinctrl/ralink/{pinmux.h => pinctrl-ralink.h}                  |  16 +-
 drivers/pinctrl/ralink/pinctrl-rt2880.c                                | 381 ++++----------------------------
 drivers/pinctrl/ralink/pinctrl-rt288x.c                                |  60 -----
 drivers/pinctrl/ralink/pinctrl-rt305x.c                                |  66 +++---
 drivers/pinctrl/ralink/pinctrl-rt3883.c                                |  50 ++---
 17 files changed, 998 insertions(+), 681 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml
 rename Documentation/devicetree/bindings/pinctrl/{ralink,rt2880-pinmux.yaml => ralink,mt7621-pinctrl.yaml} (63%)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ralink,rt305x-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ralink,rt3883-pinctrl.yaml
 create mode 100644 drivers/pinctrl/ralink/pinctrl-ralink.c
 rename drivers/pinctrl/ralink/{pinmux.h => pinctrl-ralink.h} (75%)
 delete mode 100644 drivers/pinctrl/ralink/pinctrl-rt288x.c

Comments

Sergio Paracuellos April 13, 2022, 6:36 a.m. UTC | #1
Hi Arinç,

On Wed, Apr 13, 2022 at 8:08 AM Arınç ÜNAL <arinc.unal@arinc9.com> 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@gmail.com>

>   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
:).

Thanks for doing this.

Best regards,
    Sergio Paracuellos
Arınç ÜNAL April 13, 2022, 7:52 a.m. UTC | #2
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@arinc9.com> 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@gmail.com>
> 
>>    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ç
Krzysztof Kozlowski April 13, 2022, 3:39 p.m. UTC | #3
On 13/04/2022 08:36, Sergio Paracuellos wrote:
> 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
> :).

Breaking other users is usually not accepted, so some good reason would
be needed here...


Best regards,
Krzysztof
Arınç ÜNAL April 14, 2022, 8:26 a.m. UTC | #4
On 13/04/2022 18:39, Krzysztof Kozlowski wrote:
> On 13/04/2022 08:36, Sergio Paracuellos wrote:
>> 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
>> :).
> 
> Breaking other users is usually not accepted, so some good reason would
> be needed here...

As I explained on my response to Sergio, this doesn't necessarily break 
anything.

> 
> 
> Best regards,
> Krzysztof
Linus Walleij April 21, 2022, 2:27 p.m. UTC | #5
On Wed, Apr 13, 2022 at 8:08 AM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:

> This patch series brings complete refactoring to the Ralink pinctrl driver
> and its subdrivers.

I just merged all the patches, the comments seem minor and any further
fixes can certainly be done on top of this. Anyone interested in ralink
working nicely is likely in the thread and we mostly care about that this
works for OpenWrt, and if it works for them we are happy.

>   mips: dts: ralink: mt7621: use the new compatible string for MT7621 pinctrl

This was a bit scary since we usually take these through the respective
SoC tree, but I just applied it anyway, it makes logical sense in the
series.

I hope it will not lead to conflicts.

Good work with this series!

My personal headache with RT2880 is the Ralink RT2880F mini PCI card
which apparently contains a non-reflashable firmware. It's not
the RT2880:s fault though, I'm just always reminded of this when
working with RT2880 :/

Yours,
Linus Walleij
Arınç ÜNAL April 21, 2022, 2:43 p.m. UTC | #6
On 21/04/2022 17:27, Linus Walleij wrote:
> On Wed, Apr 13, 2022 at 8:08 AM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
> 
>> This patch series brings complete refactoring to the Ralink pinctrl driver
>> and its subdrivers.
> 
> I just merged all the patches, the comments seem minor and any further
> fixes can certainly be done on top of this. Anyone interested in ralink
> working nicely is likely in the thread and we mostly care about that this
> works for OpenWrt, and if it works for them we are happy.
> 
>>    mips: dts: ralink: mt7621: use the new compatible string for MT7621 pinctrl
> 
> This was a bit scary since we usually take these through the respective
> SoC tree, but I just applied it anyway, it makes logical sense in the
> series.
> 
> I hope it will not lead to conflicts.
> 
> Good work with this series!

Thanks. There is a v2 of this series which has been waiting for a week, 
I hope that was the one you applied as you replied under v1 (I'm not 
sure which repository you applied this so I can't check myself).

https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=632370

Arınç
Sergio Paracuellos April 22, 2022, 5:21 a.m. UTC | #7
Hi Arinç,

On Thu, Apr 21, 2022 at 4:44 PM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
>
> On 21/04/2022 17:27, Linus Walleij wrote:
> > On Wed, Apr 13, 2022 at 8:08 AM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
> >
> >> This patch series brings complete refactoring to the Ralink pinctrl driver
> >> and its subdrivers.
> >
> > I just merged all the patches, the comments seem minor and any further
> > fixes can certainly be done on top of this. Anyone interested in ralink
> > working nicely is likely in the thread and we mostly care about that this
> > works for OpenWrt, and if it works for them we are happy.
> >
> >>    mips: dts: ralink: mt7621: use the new compatible string for MT7621 pinctrl
> >
> > This was a bit scary since we usually take these through the respective
> > SoC tree, but I just applied it anyway, it makes logical sense in the
> > series.
> >
> > I hope it will not lead to conflicts.
> >
> > Good work with this series!
>
> Thanks. There is a v2 of this series which has been waiting for a week,
> I hope that was the one you applied as you replied under v1 (I'm not
> sure which repository you applied this so I can't check myself).

Linus adds patches through the linux-pinctrl tree as pinctrl maintainer. Check:

https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel

Best regards,
    Sergio Paracuellos
>
> https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=632370
>
> Arınç
Arınç ÜNAL April 22, 2022, 6:13 a.m. UTC | #8
On 22/04/2022 08:21, Sergio Paracuellos wrote:
> Hi Arinç,
> 
> On Thu, Apr 21, 2022 at 4:44 PM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
>>
>> On 21/04/2022 17:27, Linus Walleij wrote:
>>> On Wed, Apr 13, 2022 at 8:08 AM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
>>>
>>>> This patch series brings complete refactoring to the Ralink pinctrl driver
>>>> and its subdrivers.
>>>
>>> I just merged all the patches, the comments seem minor and any further
>>> fixes can certainly be done on top of this. Anyone interested in ralink
>>> working nicely is likely in the thread and we mostly care about that this
>>> works for OpenWrt, and if it works for them we are happy.
>>>
>>>>     mips: dts: ralink: mt7621: use the new compatible string for MT7621 pinctrl
>>>
>>> This was a bit scary since we usually take these through the respective
>>> SoC tree, but I just applied it anyway, it makes logical sense in the
>>> series.
>>>
>>> I hope it will not lead to conflicts.
>>>
>>> Good work with this series!
>>
>> Thanks. There is a v2 of this series which has been waiting for a week,
>> I hope that was the one you applied as you replied under v1 (I'm not
>> sure which repository you applied this so I can't check myself).
> 
> Linus adds patches through the linux-pinctrl tree as pinctrl maintainer. Check:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel

Thanks Sergio. I see v1 was applied, oops. What to do?

Arınç
Linus Walleij April 22, 2022, 10:07 p.m. UTC | #9
On Fri, Apr 22, 2022 at 8:14 AM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:

> >> Thanks. There is a v2 of this series which has been waiting for a week,
> >> I hope that was the one you applied as you replied under v1 (I'm not
> >> sure which repository you applied this so I can't check myself).
> >
> > Linus adds patches through the linux-pinctrl tree as pinctrl maintainer. Check:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel
>
> Thanks Sergio. I see v1 was applied, oops. What to do?

No problem, I backed it out (had to back out some stuff pulled on top too...)
then re-pulled the stuff I pulled on top, then applied v2 on top.
Let's see how much I screwed up :D

Yours,
Linus Walleij