Message ID | 20220413060729.27639-1-arinc.unal@arinc9.com (mailing list archive) |
---|---|
Headers | show |
Series | Refactor Ralink Pinctrl and Add Documentation | expand |
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
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ç
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
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
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
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ç
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ç
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ç
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