mbox series

[00/20] pinctrl: ralink: fix ABI, improve driver, move to mediatek, improve dt-bindings

Message ID 20230303002850.51858-1-arinc.unal@arinc9.com
Headers show
Series pinctrl: ralink: fix ABI, improve driver, move to mediatek, improve dt-bindings | expand

Message

Arınç ÜNAL March 3, 2023, 12:28 a.m. UTC
[PATCH 00/20] pinctrl: ralink: fix ABI, improve driver, move to mediatek, improve dt-bindings

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.

This platform from Ralink was acquired by MediaTek in 2011. Then, MediaTek
introduced new SoCs which utilise this platform. We're moving the Ralink
pinctrl driver to MediaTek, and rename the schemas for MediaTek SoCs 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.

Arınç

RFC to v1:
- Address Rob and Krzysztof's reviews, add Rob's acked-by.
- More cleanups, hooray!
- Keep ralink,rt2880-pinmux deprecated.
- Deprecate ralink,mt7620-pinctrl. Another one bites the dust!
- More slight changes I can't currently remember to mention.

Arınç ÜNAL (20):
  pinctrl: ralink: reintroduce ralink,rt2880-pinmux compatible string
  pinctrl: ralink: {mt7620,mt7621}: add new mediatek compatible strings
  pinctrl: ralink: rt305x: add new compatible string for every SoC
  pinctrl: ralink: mt7620: split out to mt76x8
  pinctrl: ralink: move to mediatek as mtmips
  pinctrl: mediatek: remove OF_GPIO as reverse dependency
  dt-bindings: pinctrl: ralink: improve bindings
  dt-bindings: pinctrl: ralink: add new compatible strings
  dt-bindings: pinctrl: ralink: {mt7620,mt7621}: rename to mediatek
  dt-bindings: pinctrl: mediatek: mt6795: rename to mediatek,mt6795-pinctrl
  dt-bindings: pinctrl: mediatek: mt8186: rename to mediatek,mt8186-pinctrl
  dt-bindings: pinctrl: mediatek: mt8192: rename to mediatek,mt8192-pinctrl
  dt-bindings: pinctrl: mediatek: mt8195: rename to mediatek,mt8195-pinctrl
  dt-bindings: pinctrl: mediatek: fix naming inconsistency
  dt-bindings: pinctrl: {mediatek,ralink}: fix formatting
  dt-bindings: pinctrl: mediatek: drop quotes from referred schemas
  dt-bindings: pinctrl: mediatek: mt7986: fix patternProperties regex
  dt-bindings: pinctrl: ralink: rt305x: split binding
  dt-bindings: pinctrl: mediatek: mt7620: split binding
  MAINTAINERS: move ralink pinctrl to mediatek mips pinctrl

 .../pinctrl/mediatek,mt65xx-pinctrl.yaml        |  28 +-
 .../pinctrl/mediatek,mt6779-pinctrl.yaml        |  37 +-
 ...mt6795.yaml => mediatek,mt6795-pinctrl.yaml} |  39 +-
 .../pinctrl/mediatek,mt7620-pinctrl.yaml        | 298 ++++++++++++++
 ...inctrl.yaml => mediatek,mt7621-pinctrl.yaml} |  23 +-
 .../pinctrl/mediatek,mt7622-pinctrl.yaml        |  32 +-
 ...inctrl.yaml => mediatek,mt76x8-pinctrl.yaml} | 252 ++----------
 .../pinctrl/mediatek,mt7981-pinctrl.yaml        |  35 +-
 .../pinctrl/mediatek,mt7986-pinctrl.yaml        |  72 ++--
 .../pinctrl/mediatek,mt8183-pinctrl.yaml        |  30 +-
 ...mt8186.yaml => mediatek,mt8186-pinctrl.yaml} |  51 ++-
 .../pinctrl/mediatek,mt8188-pinctrl.yaml        |  76 ++--
 ...mt8192.yaml => mediatek,mt8192-pinctrl.yaml} |  53 +--
 ...mt8195.yaml => mediatek,mt8195-pinctrl.yaml} |  44 +--
 .../pinctrl/mediatek,mt8365-pinctrl.yaml        |  28 +-
 .../bindings/pinctrl/ralink,rt2880-pinctrl.yaml |  11 +-
 .../bindings/pinctrl/ralink,rt305x-pinctrl.yaml |  89 +----
 .../bindings/pinctrl/ralink,rt3352-pinctrl.yaml | 243 ++++++++++++
 .../bindings/pinctrl/ralink,rt3883-pinctrl.yaml |  11 +-
 .../bindings/pinctrl/ralink,rt5350-pinctrl.yaml | 206 ++++++++++
 MAINTAINERS                                     |  29 +-
 drivers/pinctrl/Kconfig                         |   1 -
 drivers/pinctrl/Makefile                        |   1 -
 drivers/pinctrl/mediatek/Kconfig                |  54 ++-
 drivers/pinctrl/mediatek/Makefile               |  63 +--
 drivers/pinctrl/mediatek/pinctrl-mt7620.c       | 138 +++++++
 .../{ralink => mediatek}/pinctrl-mt7621.c       |  32 +-
 drivers/pinctrl/mediatek/pinctrl-mt76x8.c       | 283 ++++++++++++++
 .../pinctrl-mtmips.c}                           |  90 ++---
 .../pinctrl-mtmips.h}                           |  16 +-
 .../{ralink => mediatek}/pinctrl-rt2880.c       |  21 +-
 .../{ralink => mediatek}/pinctrl-rt305x.c       |  47 +--
 .../{ralink => mediatek}/pinctrl-rt3883.c       |  29 +-
 drivers/pinctrl/ralink/Kconfig                  |  35 --
 drivers/pinctrl/ralink/Makefile                 |   8 -
 drivers/pinctrl/ralink/pinctrl-mt7620.c         | 391 -------------------
 36 files changed, 1723 insertions(+), 1173 deletions(-)

Comments

Sergio Paracuellos March 6, 2023, 2:07 p.m. UTC | #1
On Fri, Mar 3, 2023 at 3:18 PM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
>
> Heyo,
>
> On 3.03.2023 13:57, Sergio Paracuellos wrote:
> > Hi Arınç,
> >
> > On Fri, Mar 3, 2023 at 9:16 AM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
> >>
> >> Hey Sergio,
> >>
> >> On 3.03.2023 09:34, Sergio Paracuellos wrote:
> >>> On Fri, Mar 3, 2023 at 7:17 AM Sergio Paracuellos
> >>> <sergio.paracuellos@gmail.com> wrote:
> >>>>
> >>>>    Hi Arınç,
> >>>>
> >>>> On Fri, Mar 3, 2023 at 1:30 AM <arinc9.unal@gmail.com> wrote:
> >>>>>
> >>>>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> >>>>>
> >>>>> This platform from Ralink was acquired by MediaTek in 2011. Then, MediaTek
> >>>>> introduced new SoCs which utilise this platform. Move the driver to
> >>>>> mediatek pinctrl directory. Rename the ralink core driver to mtmips.
> >>>>>
> >>>>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> >>>>> ---
> >>>>>    drivers/pinctrl/Kconfig                       |  1 -
> >>>>>    drivers/pinctrl/Makefile                      |  1 -
> >>>>>    drivers/pinctrl/mediatek/Kconfig              | 51 ++++++++++-
> >>>>>    drivers/pinctrl/mediatek/Makefile             | 63 +++++++------
> >>>>>    .../{ralink => mediatek}/pinctrl-mt7620.c     | 34 +++----
> >>>>>    .../{ralink => mediatek}/pinctrl-mt7621.c     | 30 +++----
> >>>>>    .../{ralink => mediatek}/pinctrl-mt76x8.c     | 60 ++++++-------
> >>>>>    .../pinctrl-mtmips.c}                         | 90 +++++++++----------
> >>>>>    .../pinctrl-mtmips.h}                         | 16 ++--
> >>>>>    .../{ralink => mediatek}/pinctrl-rt2880.c     | 20 ++---
> >>>>>    .../{ralink => mediatek}/pinctrl-rt305x.c     | 44 ++++-----
> >>>>>    .../{ralink => mediatek}/pinctrl-rt3883.c     | 28 +++---
> >>>>>    drivers/pinctrl/ralink/Kconfig                | 40 ---------
> >>>>>    drivers/pinctrl/ralink/Makefile               |  9 --
> >>>>>    14 files changed, 246 insertions(+), 241 deletions(-)
> >>>>>    rename drivers/pinctrl/{ralink => mediatek}/pinctrl-mt7620.c (81%)
> >>>>>    rename drivers/pinctrl/{ralink => mediatek}/pinctrl-mt7621.c (80%)
> >>>>>    rename drivers/pinctrl/{ralink => mediatek}/pinctrl-mt76x8.c (81%)
> >>>>>    rename drivers/pinctrl/{ralink/pinctrl-ralink.c => mediatek/pinctrl-mtmips.c} (74%)
> >>>>>    rename drivers/pinctrl/{ralink/pinctrl-ralink.h => mediatek/pinctrl-mtmips.h} (75%)
> >>>>>    rename drivers/pinctrl/{ralink => mediatek}/pinctrl-rt2880.c (71%)
> >>>>>    rename drivers/pinctrl/{ralink => mediatek}/pinctrl-rt305x.c (75%)
> >>>>>    rename drivers/pinctrl/{ralink => mediatek}/pinctrl-rt3883.c (80%)
> >>>>>    delete mode 100644 drivers/pinctrl/ralink/Kconfig
> >>>>>    delete mode 100644 drivers/pinctrl/ralink/Makefile
> >>>>>
> >>>>> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> >>>>> index dcb53c4a9584..8a6012770640 100644
> >>>>> --- a/drivers/pinctrl/Kconfig
> >>>>> +++ b/drivers/pinctrl/Kconfig
> >>>>> @@ -537,7 +537,6 @@ source "drivers/pinctrl/nomadik/Kconfig"
> >>>>>    source "drivers/pinctrl/nuvoton/Kconfig"
> >>>>>    source "drivers/pinctrl/pxa/Kconfig"
> >>>>>    source "drivers/pinctrl/qcom/Kconfig"
> >>>>> -source "drivers/pinctrl/ralink/Kconfig"
> >>>>>    source "drivers/pinctrl/renesas/Kconfig"
> >>>>>    source "drivers/pinctrl/samsung/Kconfig"
> >>>>>    source "drivers/pinctrl/spear/Kconfig"
> >>>>> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> >>>>> index d5939840bb2a..ada6ed1d4e91 100644
> >>>>> --- a/drivers/pinctrl/Makefile
> >>>>> +++ b/drivers/pinctrl/Makefile
> >>>>> @@ -66,7 +66,6 @@ obj-y                         += nomadik/
> >>>>>    obj-y                          += nuvoton/
> >>>>>    obj-$(CONFIG_PINCTRL_PXA)      += pxa/
> >>>>>    obj-$(CONFIG_ARCH_QCOM)                += qcom/
> >>>>> -obj-$(CONFIG_PINCTRL_RALINK)   += ralink/
> >>>>>    obj-$(CONFIG_PINCTRL_RENESAS)  += renesas/
> >>>>>    obj-$(CONFIG_PINCTRL_SAMSUNG)  += samsung/
> >>>>>    obj-$(CONFIG_PINCTRL_SPEAR)    += spear/
> >>>>> diff --git a/drivers/pinctrl/mediatek/Kconfig b/drivers/pinctrl/mediatek/Kconfig
> >>>>> index a71874fed3d6..2eeb55010563 100644
> >>>>> --- a/drivers/pinctrl/mediatek/Kconfig
> >>>>> +++ b/drivers/pinctrl/mediatek/Kconfig
> >>>>> @@ -1,6 +1,6 @@
> >>>>>    # SPDX-License-Identifier: GPL-2.0-only
> >>>>>    menu "MediaTek pinctrl drivers"
> >>>>> -       depends on ARCH_MEDIATEK || COMPILE_TEST
> >>>>> +       depends on ARCH_MEDIATEK || RALINK || COMPILE_TEST
> >>>>>
> >>>>>    config EINT_MTK
> >>>>>           tristate "MediaTek External Interrupt Support"
> >>>>> @@ -22,6 +22,12 @@ config PINCTRL_MTK
> >>>>>    config PINCTRL_MTK_V2
> >>>>>           tristate
> >>>>>
> >>>>> +config PINCTRL_MTK_MTMIPS
> >>>>> +       bool
> >>>>> +       depends on RALINK
> >>>>> +       select PINMUX
> >>>>> +       select GENERIC_PINCONF
> >>>>> +
> >>>>>    config PINCTRL_MTK_MOORE
> >>>>>           bool
> >>>>>           depends on OF
> >>>>> @@ -43,6 +49,49 @@ config PINCTRL_MTK_PARIS
> >>>>>           select OF_GPIO
> >>>>>           select PINCTRL_MTK_V2
> >>>>>
> >>>>> +# For MIPS SoCs
> >>>>> +config PINCTRL_MT7620
> >>>>> +       bool "MediaTek MT7620 pin control"
> >>>>> +       depends on SOC_MT7620 || COMPILE_TEST
> >>>>> +       depends on RALINK
> >>>>> +       default SOC_MT7620
> >>>>> +       select PINCTRL_MTK_MTMIPS
> >>>>> +
> >>>>> +config PINCTRL_MT7621
> >>>>> +       bool "MediaTek MT7621 pin control"
> >>>>> +       depends on SOC_MT7621 || COMPILE_TEST
> >>>>> +       depends on RALINK
> >>>>> +       default SOC_MT7621
> >>>>> +       select PINCTRL_MTK_MTMIPS
> >>>>> +
> >>>>> +config PINCTRL_MT76X8
> >>>>> +       bool "MediaTek MT76X8 pin control"
> >>>>> +       depends on SOC_MT7620 || COMPILE_TEST
> >>>>> +       depends on RALINK
> >>>>> +       default SOC_MT7620
> >>>>> +       select PINCTRL_MTK_MTMIPS
> >>>>> +
> >>>>> +config PINCTRL_RT2880
> >>>>> +       bool "Ralink RT2880 pin control"
> >>>>> +       depends on SOC_RT288X || COMPILE_TEST
> >>>>> +       depends on RALINK
> >>>>> +       default SOC_RT288X
> >>>>> +       select PINCTRL_MTK_MTMIPS
> >>>>> +
> >>>>> +config PINCTRL_RT305X
> >>>>> +       bool "Ralink RT305X pin control"
> >>>>> +       depends on SOC_RT305X || COMPILE_TEST
> >>>>> +       depends on RALINK
> >>>>> +       default SOC_RT305X
> >>>>> +       select PINCTRL_MTK_MTMIPS
> >>>>> +
> >>>>> +config PINCTRL_RT3883
> >>>>> +       bool "Ralink RT3883 pin control"
> >>>>> +       depends on SOC_RT3883 || COMPILE_TEST
> >>>>> +       depends on RALINK
> >>>>> +       default SOC_RT3883
> >>>>> +       select PINCTRL_MTK_MTMIPS
> >>>>> +
> >>>>
> >>>> I am not a Kconfig expert at all but...
> >>>>
> >>>> Should not all of these be depends on SOC_XXX || (COMPILE_TEST &&
> >>>> RALINK) and avoid the " depends on RALINK" next line in all of them?
> >>
> >> This seems to do the same thing but I'm following the "either change
> >> them all or fit into the crowd" ideology.
> >>
> >>>>
> >>>> Just asking since we have yet arch read and write register operations
> >>>> in pinctrl common ralink code. Having in this way, when we address
> >>>> this arch thing  in the next series just removing the "&& RALINK" part
> >>>> makes the review pretty obvious.
> >>
> >> You'd have to change RALINK with OF since we're still depending on that.
> >> RALINK selects OF by default so it's currently a hidden dependency.
> >>
> >>>>
> >>>> Other than that, changes look good to me.
> >>>
> >>> I think "depends on SOC_XXX || (COMPILE_TEST && MIPS)" would work also
> >>> and might be more accurate for compile testing targets.
> >
> > Are you sure? SOC_XXX here is already being enabled only if RALINK is
> > already enabled, right? [0]
>
> I'm not sure who's your reply to, or what it's about here.

Bad insertion between lines, sorry :). I was just trying to explain to
you that SOC_RTXX ralink stuff is only available when RALINK is
already selected.

>
> >
> >>
> >> This is not OK in both cases. If the driver is dependent on Ralink
> >> architecture code, choosing any other MIPS platform will make the driver
> >> available to compile, which will fail.
> >
> > SOC_XXX is already dependent on RALINK for real uses but the driver is
> > going to be selected for other MIPS platforms only for COMPILE_TEST
> > targets. Ideally drivers should be arch agnostic so can be selected
> > for any single arch build. Now we have arch dependent read and write
> > calls in the code, so you need the right headers to be properly found
> > to be able to compile testing. I think MIPS is enough dependency here
> > to properly find them. But if not, this should be (COMPILE_TEST &&
> > RALINK)
>
> I expect below to work without requiring the MIPS option.
>
> ifeq ($(CONFIG_COMPILE_TEST),y)
> CFLAGS_pinctrl-mtmips.o         += -I$(srctree)/arch/mips/include
> endif

Yes, this will work but won't be necessary at all when we get rid of
ralink arch dependent code in the next series.

>
> >
> >>
> >> If the driver is independent of Ralink architecture code, you're
> >> limiting the driver to be compiled only when a MIPS platform is selected.
> >
> > So... how are you planning to allow compile testing of the driver in
> > any single arch when we get rid of all the arch dependent code? If you
> > make everything dependent on RALINK you cannot.
>
> I intend to make it dependent on OF, not RALINK.

Ok, I see, thanks for clarification.

>
> Arınç

Best regards,
    Sergio Paracuellos
Sergio Paracuellos March 6, 2023, 2:17 p.m. UTC | #2
On Mon, Mar 6, 2023 at 2:50 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Fri, Mar 3, 2023 at 1:29 AM <arinc9.unal@gmail.com> wrote:
>
> > [PATCH 00/20] pinctrl: ralink: fix ABI, improve driver, move to mediatek, improve dt-bindings
> >
> > This is an ambitious effort I've been wanting to do for months.
>
> Good with ambitions :)
>
> As long as Sergio is on board and can test the changes and as long
> as the DT maintainers do not explicitly disapprove, I'm game to merge
> this.

For sure I am :-). Changes in code are pretty straightforward. The
real big change is just a split of mt7620 and mt76x8 in two different
files. But the split looks good to me. The other bulk of changes is
just moving files around. If DT maintainers are finally ok with the
changes I will try to make time to test this series in a rt5350 SoC
based board I think I have somewhere and on mt7621 SoC based one. Most
of my ralink boards are based on mt7621 so I don't have more platforms
to test this pinctrl changes :(.

>
> I guess you will respin on top of v6.3-rc1 when the first round of
> non-RFC feedback is collected.
>
> Yours,
> Linus Walleij

Thanks,
    Sergio Paracuellos
Arınç ÜNAL March 6, 2023, 3:05 p.m. UTC | #3
On 6.03.2023 17:07, Sergio Paracuellos wrote:
> On Fri, Mar 3, 2023 at 3:18 PM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
>>
>> Heyo,
>>
>> On 3.03.2023 13:57, Sergio Paracuellos wrote:
>>> Hi Arınç,
>>>
>>> On Fri, Mar 3, 2023 at 9:16 AM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
>>>>
>>>> Hey Sergio,
>>>>
>>>> On 3.03.2023 09:34, Sergio Paracuellos wrote:
>>>>> On Fri, Mar 3, 2023 at 7:17 AM Sergio Paracuellos
>>>>> <sergio.paracuellos@gmail.com> wrote:
>>>>>>
>>>>>>     Hi Arınç,
>>>>>>
>>>>>> On Fri, Mar 3, 2023 at 1:30 AM <arinc9.unal@gmail.com> wrote:
>>>>>>>
>>>>>>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>>>>>>
>>>>>>> This platform from Ralink was acquired by MediaTek in 2011. Then, MediaTek
>>>>>>> introduced new SoCs which utilise this platform. Move the driver to
>>>>>>> mediatek pinctrl directory. Rename the ralink core driver to mtmips.
>>>>>>>
>>>>>>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>>>>>>> ---
>>>>>>>     drivers/pinctrl/Kconfig                       |  1 -
>>>>>>>     drivers/pinctrl/Makefile                      |  1 -
>>>>>>>     drivers/pinctrl/mediatek/Kconfig              | 51 ++++++++++-
>>>>>>>     drivers/pinctrl/mediatek/Makefile             | 63 +++++++------
>>>>>>>     .../{ralink => mediatek}/pinctrl-mt7620.c     | 34 +++----
>>>>>>>     .../{ralink => mediatek}/pinctrl-mt7621.c     | 30 +++----
>>>>>>>     .../{ralink => mediatek}/pinctrl-mt76x8.c     | 60 ++++++-------
>>>>>>>     .../pinctrl-mtmips.c}                         | 90 +++++++++----------
>>>>>>>     .../pinctrl-mtmips.h}                         | 16 ++--
>>>>>>>     .../{ralink => mediatek}/pinctrl-rt2880.c     | 20 ++---
>>>>>>>     .../{ralink => mediatek}/pinctrl-rt305x.c     | 44 ++++-----
>>>>>>>     .../{ralink => mediatek}/pinctrl-rt3883.c     | 28 +++---
>>>>>>>     drivers/pinctrl/ralink/Kconfig                | 40 ---------
>>>>>>>     drivers/pinctrl/ralink/Makefile               |  9 --
>>>>>>>     14 files changed, 246 insertions(+), 241 deletions(-)
>>>>>>>     rename drivers/pinctrl/{ralink => mediatek}/pinctrl-mt7620.c (81%)
>>>>>>>     rename drivers/pinctrl/{ralink => mediatek}/pinctrl-mt7621.c (80%)
>>>>>>>     rename drivers/pinctrl/{ralink => mediatek}/pinctrl-mt76x8.c (81%)
>>>>>>>     rename drivers/pinctrl/{ralink/pinctrl-ralink.c => mediatek/pinctrl-mtmips.c} (74%)
>>>>>>>     rename drivers/pinctrl/{ralink/pinctrl-ralink.h => mediatek/pinctrl-mtmips.h} (75%)
>>>>>>>     rename drivers/pinctrl/{ralink => mediatek}/pinctrl-rt2880.c (71%)
>>>>>>>     rename drivers/pinctrl/{ralink => mediatek}/pinctrl-rt305x.c (75%)
>>>>>>>     rename drivers/pinctrl/{ralink => mediatek}/pinctrl-rt3883.c (80%)
>>>>>>>     delete mode 100644 drivers/pinctrl/ralink/Kconfig
>>>>>>>     delete mode 100644 drivers/pinctrl/ralink/Makefile
>>>>>>>
>>>>>>> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
>>>>>>> index dcb53c4a9584..8a6012770640 100644
>>>>>>> --- a/drivers/pinctrl/Kconfig
>>>>>>> +++ b/drivers/pinctrl/Kconfig
>>>>>>> @@ -537,7 +537,6 @@ source "drivers/pinctrl/nomadik/Kconfig"
>>>>>>>     source "drivers/pinctrl/nuvoton/Kconfig"
>>>>>>>     source "drivers/pinctrl/pxa/Kconfig"
>>>>>>>     source "drivers/pinctrl/qcom/Kconfig"
>>>>>>> -source "drivers/pinctrl/ralink/Kconfig"
>>>>>>>     source "drivers/pinctrl/renesas/Kconfig"
>>>>>>>     source "drivers/pinctrl/samsung/Kconfig"
>>>>>>>     source "drivers/pinctrl/spear/Kconfig"
>>>>>>> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
>>>>>>> index d5939840bb2a..ada6ed1d4e91 100644
>>>>>>> --- a/drivers/pinctrl/Makefile
>>>>>>> +++ b/drivers/pinctrl/Makefile
>>>>>>> @@ -66,7 +66,6 @@ obj-y                         += nomadik/
>>>>>>>     obj-y                          += nuvoton/
>>>>>>>     obj-$(CONFIG_PINCTRL_PXA)      += pxa/
>>>>>>>     obj-$(CONFIG_ARCH_QCOM)                += qcom/
>>>>>>> -obj-$(CONFIG_PINCTRL_RALINK)   += ralink/
>>>>>>>     obj-$(CONFIG_PINCTRL_RENESAS)  += renesas/
>>>>>>>     obj-$(CONFIG_PINCTRL_SAMSUNG)  += samsung/
>>>>>>>     obj-$(CONFIG_PINCTRL_SPEAR)    += spear/
>>>>>>> diff --git a/drivers/pinctrl/mediatek/Kconfig b/drivers/pinctrl/mediatek/Kconfig
>>>>>>> index a71874fed3d6..2eeb55010563 100644
>>>>>>> --- a/drivers/pinctrl/mediatek/Kconfig
>>>>>>> +++ b/drivers/pinctrl/mediatek/Kconfig
>>>>>>> @@ -1,6 +1,6 @@
>>>>>>>     # SPDX-License-Identifier: GPL-2.0-only
>>>>>>>     menu "MediaTek pinctrl drivers"
>>>>>>> -       depends on ARCH_MEDIATEK || COMPILE_TEST
>>>>>>> +       depends on ARCH_MEDIATEK || RALINK || COMPILE_TEST
>>>>>>>
>>>>>>>     config EINT_MTK
>>>>>>>            tristate "MediaTek External Interrupt Support"
>>>>>>> @@ -22,6 +22,12 @@ config PINCTRL_MTK
>>>>>>>     config PINCTRL_MTK_V2
>>>>>>>            tristate
>>>>>>>
>>>>>>> +config PINCTRL_MTK_MTMIPS
>>>>>>> +       bool
>>>>>>> +       depends on RALINK
>>>>>>> +       select PINMUX
>>>>>>> +       select GENERIC_PINCONF
>>>>>>> +
>>>>>>>     config PINCTRL_MTK_MOORE
>>>>>>>            bool
>>>>>>>            depends on OF
>>>>>>> @@ -43,6 +49,49 @@ config PINCTRL_MTK_PARIS
>>>>>>>            select OF_GPIO
>>>>>>>            select PINCTRL_MTK_V2
>>>>>>>
>>>>>>> +# For MIPS SoCs
>>>>>>> +config PINCTRL_MT7620
>>>>>>> +       bool "MediaTek MT7620 pin control"
>>>>>>> +       depends on SOC_MT7620 || COMPILE_TEST
>>>>>>> +       depends on RALINK
>>>>>>> +       default SOC_MT7620
>>>>>>> +       select PINCTRL_MTK_MTMIPS
>>>>>>> +
>>>>>>> +config PINCTRL_MT7621
>>>>>>> +       bool "MediaTek MT7621 pin control"
>>>>>>> +       depends on SOC_MT7621 || COMPILE_TEST
>>>>>>> +       depends on RALINK
>>>>>>> +       default SOC_MT7621
>>>>>>> +       select PINCTRL_MTK_MTMIPS
>>>>>>> +
>>>>>>> +config PINCTRL_MT76X8
>>>>>>> +       bool "MediaTek MT76X8 pin control"
>>>>>>> +       depends on SOC_MT7620 || COMPILE_TEST
>>>>>>> +       depends on RALINK
>>>>>>> +       default SOC_MT7620
>>>>>>> +       select PINCTRL_MTK_MTMIPS
>>>>>>> +
>>>>>>> +config PINCTRL_RT2880
>>>>>>> +       bool "Ralink RT2880 pin control"
>>>>>>> +       depends on SOC_RT288X || COMPILE_TEST
>>>>>>> +       depends on RALINK
>>>>>>> +       default SOC_RT288X
>>>>>>> +       select PINCTRL_MTK_MTMIPS
>>>>>>> +
>>>>>>> +config PINCTRL_RT305X
>>>>>>> +       bool "Ralink RT305X pin control"
>>>>>>> +       depends on SOC_RT305X || COMPILE_TEST
>>>>>>> +       depends on RALINK
>>>>>>> +       default SOC_RT305X
>>>>>>> +       select PINCTRL_MTK_MTMIPS
>>>>>>> +
>>>>>>> +config PINCTRL_RT3883
>>>>>>> +       bool "Ralink RT3883 pin control"
>>>>>>> +       depends on SOC_RT3883 || COMPILE_TEST
>>>>>>> +       depends on RALINK
>>>>>>> +       default SOC_RT3883
>>>>>>> +       select PINCTRL_MTK_MTMIPS
>>>>>>> +
>>>>>>
>>>>>> I am not a Kconfig expert at all but...
>>>>>>
>>>>>> Should not all of these be depends on SOC_XXX || (COMPILE_TEST &&
>>>>>> RALINK) and avoid the " depends on RALINK" next line in all of them?
>>>>
>>>> This seems to do the same thing but I'm following the "either change
>>>> them all or fit into the crowd" ideology.
>>>>
>>>>>>
>>>>>> Just asking since we have yet arch read and write register operations
>>>>>> in pinctrl common ralink code. Having in this way, when we address
>>>>>> this arch thing  in the next series just removing the "&& RALINK" part
>>>>>> makes the review pretty obvious.
>>>>
>>>> You'd have to change RALINK with OF since we're still depending on that.
>>>> RALINK selects OF by default so it's currently a hidden dependency.
>>>>
>>>>>>
>>>>>> Other than that, changes look good to me.
>>>>>
>>>>> I think "depends on SOC_XXX || (COMPILE_TEST && MIPS)" would work also
>>>>> and might be more accurate for compile testing targets.
>>>
>>> Are you sure? SOC_XXX here is already being enabled only if RALINK is
>>> already enabled, right? [0]
>>
>> I'm not sure who's your reply to, or what it's about here.
> 
> Bad insertion between lines, sorry :). I was just trying to explain to
> you that SOC_RTXX ralink stuff is only available when RALINK is
> already selected.

Makes sense. However, I believe what I said below is still true. This 
option will be available to compile if a Ralink SoC (and therefore 
RALINK) is enabled, OR, COMPILE_TEST and MIPS is enabled. The latter 
will fail to compile if the enabled MIPS platform is not RALINK.

> 
>>
>>>
>>>>
>>>> This is not OK in both cases. If the driver is dependent on Ralink
>>>> architecture code, choosing any other MIPS platform will make the driver
>>>> available to compile, which will fail.
>>>
>>> SOC_XXX is already dependent on RALINK for real uses but the driver is
>>> going to be selected for other MIPS platforms only for COMPILE_TEST
>>> targets. Ideally drivers should be arch agnostic so can be selected
>>> for any single arch build. Now we have arch dependent read and write
>>> calls in the code, so you need the right headers to be properly found
>>> to be able to compile testing. I think MIPS is enough dependency here
>>> to properly find them. But if not, this should be (COMPILE_TEST &&
>>> RALINK)
>>
>> I expect below to work without requiring the MIPS option.
>>
>> ifeq ($(CONFIG_COMPILE_TEST),y)
>> CFLAGS_pinctrl-mtmips.o         += -I$(srctree)/arch/mips/include
>> endif
> 
> Yes, this will work but won't be necessary at all when we get rid of
> ralink arch dependent code in the next series.

Oh, you plan to completely get rid of it, including headers. That's better!

However, rt305x_pinctrl_probe() on pinctrl-rt305x.c needs them to find 
out the SoC to match the pinmux data. Sure, splitting the driver further 
will work but I'm wondering if you've got something else in mind to 
address this.

Arınç
Sergio Paracuellos March 6, 2023, 4:15 p.m. UTC | #4
On Mon, Mar 6, 2023 at 4:06 PM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
>
> On 6.03.2023 17:07, Sergio Paracuellos wrote:
> > On Fri, Mar 3, 2023 at 3:18 PM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
> >>
> >> Heyo,
> >>
> >> On 3.03.2023 13:57, Sergio Paracuellos wrote:
> >>> Hi Arınç,
> >>>
> >>> On Fri, Mar 3, 2023 at 9:16 AM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
> >>>>
> >>>> Hey Sergio,
> >>>>
> >>>> On 3.03.2023 09:34, Sergio Paracuellos wrote:
> >>>>> On Fri, Mar 3, 2023 at 7:17 AM Sergio Paracuellos
> >>>>> <sergio.paracuellos@gmail.com> wrote:
> >>>>>>
> >>>>>>     Hi Arınç,
> >>>>>>
> >>>>>> On Fri, Mar 3, 2023 at 1:30 AM <arinc9.unal@gmail.com> wrote:
> >>>>>>>
> >>>>>>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> >>>>>>>
> >>>>>>> This platform from Ralink was acquired by MediaTek in 2011. Then, MediaTek
> >>>>>>> introduced new SoCs which utilise this platform. Move the driver to
> >>>>>>> mediatek pinctrl directory. Rename the ralink core driver to mtmips.
> >>>>>>>
> >>>>>>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> >>>>>>> ---
> >>>>>>>     drivers/pinctrl/Kconfig                       |  1 -
> >>>>>>>     drivers/pinctrl/Makefile                      |  1 -
> >>>>>>>     drivers/pinctrl/mediatek/Kconfig              | 51 ++++++++++-
> >>>>>>>     drivers/pinctrl/mediatek/Makefile             | 63 +++++++------
> >>>>>>>     .../{ralink => mediatek}/pinctrl-mt7620.c     | 34 +++----
> >>>>>>>     .../{ralink => mediatek}/pinctrl-mt7621.c     | 30 +++----
> >>>>>>>     .../{ralink => mediatek}/pinctrl-mt76x8.c     | 60 ++++++-------
> >>>>>>>     .../pinctrl-mtmips.c}                         | 90 +++++++++----------
> >>>>>>>     .../pinctrl-mtmips.h}                         | 16 ++--
> >>>>>>>     .../{ralink => mediatek}/pinctrl-rt2880.c     | 20 ++---
> >>>>>>>     .../{ralink => mediatek}/pinctrl-rt305x.c     | 44 ++++-----
> >>>>>>>     .../{ralink => mediatek}/pinctrl-rt3883.c     | 28 +++---
> >>>>>>>     drivers/pinctrl/ralink/Kconfig                | 40 ---------
> >>>>>>>     drivers/pinctrl/ralink/Makefile               |  9 --
> >>>>>>>     14 files changed, 246 insertions(+), 241 deletions(-)
> >>>>>>>     rename drivers/pinctrl/{ralink => mediatek}/pinctrl-mt7620.c (81%)
> >>>>>>>     rename drivers/pinctrl/{ralink => mediatek}/pinctrl-mt7621.c (80%)
> >>>>>>>     rename drivers/pinctrl/{ralink => mediatek}/pinctrl-mt76x8.c (81%)
> >>>>>>>     rename drivers/pinctrl/{ralink/pinctrl-ralink.c => mediatek/pinctrl-mtmips.c} (74%)
> >>>>>>>     rename drivers/pinctrl/{ralink/pinctrl-ralink.h => mediatek/pinctrl-mtmips.h} (75%)
> >>>>>>>     rename drivers/pinctrl/{ralink => mediatek}/pinctrl-rt2880.c (71%)
> >>>>>>>     rename drivers/pinctrl/{ralink => mediatek}/pinctrl-rt305x.c (75%)
> >>>>>>>     rename drivers/pinctrl/{ralink => mediatek}/pinctrl-rt3883.c (80%)
> >>>>>>>     delete mode 100644 drivers/pinctrl/ralink/Kconfig
> >>>>>>>     delete mode 100644 drivers/pinctrl/ralink/Makefile
> >>>>>>>
> >>>>>>> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> >>>>>>> index dcb53c4a9584..8a6012770640 100644
> >>>>>>> --- a/drivers/pinctrl/Kconfig
> >>>>>>> +++ b/drivers/pinctrl/Kconfig
> >>>>>>> @@ -537,7 +537,6 @@ source "drivers/pinctrl/nomadik/Kconfig"
> >>>>>>>     source "drivers/pinctrl/nuvoton/Kconfig"
> >>>>>>>     source "drivers/pinctrl/pxa/Kconfig"
> >>>>>>>     source "drivers/pinctrl/qcom/Kconfig"
> >>>>>>> -source "drivers/pinctrl/ralink/Kconfig"
> >>>>>>>     source "drivers/pinctrl/renesas/Kconfig"
> >>>>>>>     source "drivers/pinctrl/samsung/Kconfig"
> >>>>>>>     source "drivers/pinctrl/spear/Kconfig"
> >>>>>>> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> >>>>>>> index d5939840bb2a..ada6ed1d4e91 100644
> >>>>>>> --- a/drivers/pinctrl/Makefile
> >>>>>>> +++ b/drivers/pinctrl/Makefile
> >>>>>>> @@ -66,7 +66,6 @@ obj-y                         += nomadik/
> >>>>>>>     obj-y                          += nuvoton/
> >>>>>>>     obj-$(CONFIG_PINCTRL_PXA)      += pxa/
> >>>>>>>     obj-$(CONFIG_ARCH_QCOM)                += qcom/
> >>>>>>> -obj-$(CONFIG_PINCTRL_RALINK)   += ralink/
> >>>>>>>     obj-$(CONFIG_PINCTRL_RENESAS)  += renesas/
> >>>>>>>     obj-$(CONFIG_PINCTRL_SAMSUNG)  += samsung/
> >>>>>>>     obj-$(CONFIG_PINCTRL_SPEAR)    += spear/
> >>>>>>> diff --git a/drivers/pinctrl/mediatek/Kconfig b/drivers/pinctrl/mediatek/Kconfig
> >>>>>>> index a71874fed3d6..2eeb55010563 100644
> >>>>>>> --- a/drivers/pinctrl/mediatek/Kconfig
> >>>>>>> +++ b/drivers/pinctrl/mediatek/Kconfig
> >>>>>>> @@ -1,6 +1,6 @@
> >>>>>>>     # SPDX-License-Identifier: GPL-2.0-only
> >>>>>>>     menu "MediaTek pinctrl drivers"
> >>>>>>> -       depends on ARCH_MEDIATEK || COMPILE_TEST
> >>>>>>> +       depends on ARCH_MEDIATEK || RALINK || COMPILE_TEST
> >>>>>>>
> >>>>>>>     config EINT_MTK
> >>>>>>>            tristate "MediaTek External Interrupt Support"
> >>>>>>> @@ -22,6 +22,12 @@ config PINCTRL_MTK
> >>>>>>>     config PINCTRL_MTK_V2
> >>>>>>>            tristate
> >>>>>>>
> >>>>>>> +config PINCTRL_MTK_MTMIPS
> >>>>>>> +       bool
> >>>>>>> +       depends on RALINK
> >>>>>>> +       select PINMUX
> >>>>>>> +       select GENERIC_PINCONF
> >>>>>>> +
> >>>>>>>     config PINCTRL_MTK_MOORE
> >>>>>>>            bool
> >>>>>>>            depends on OF
> >>>>>>> @@ -43,6 +49,49 @@ config PINCTRL_MTK_PARIS
> >>>>>>>            select OF_GPIO
> >>>>>>>            select PINCTRL_MTK_V2
> >>>>>>>
> >>>>>>> +# For MIPS SoCs
> >>>>>>> +config PINCTRL_MT7620
> >>>>>>> +       bool "MediaTek MT7620 pin control"
> >>>>>>> +       depends on SOC_MT7620 || COMPILE_TEST
> >>>>>>> +       depends on RALINK
> >>>>>>> +       default SOC_MT7620
> >>>>>>> +       select PINCTRL_MTK_MTMIPS
> >>>>>>> +
> >>>>>>> +config PINCTRL_MT7621
> >>>>>>> +       bool "MediaTek MT7621 pin control"
> >>>>>>> +       depends on SOC_MT7621 || COMPILE_TEST
> >>>>>>> +       depends on RALINK
> >>>>>>> +       default SOC_MT7621
> >>>>>>> +       select PINCTRL_MTK_MTMIPS
> >>>>>>> +
> >>>>>>> +config PINCTRL_MT76X8
> >>>>>>> +       bool "MediaTek MT76X8 pin control"
> >>>>>>> +       depends on SOC_MT7620 || COMPILE_TEST
> >>>>>>> +       depends on RALINK
> >>>>>>> +       default SOC_MT7620
> >>>>>>> +       select PINCTRL_MTK_MTMIPS
> >>>>>>> +
> >>>>>>> +config PINCTRL_RT2880
> >>>>>>> +       bool "Ralink RT2880 pin control"
> >>>>>>> +       depends on SOC_RT288X || COMPILE_TEST
> >>>>>>> +       depends on RALINK
> >>>>>>> +       default SOC_RT288X
> >>>>>>> +       select PINCTRL_MTK_MTMIPS
> >>>>>>> +
> >>>>>>> +config PINCTRL_RT305X
> >>>>>>> +       bool "Ralink RT305X pin control"
> >>>>>>> +       depends on SOC_RT305X || COMPILE_TEST
> >>>>>>> +       depends on RALINK
> >>>>>>> +       default SOC_RT305X
> >>>>>>> +       select PINCTRL_MTK_MTMIPS
> >>>>>>> +
> >>>>>>> +config PINCTRL_RT3883
> >>>>>>> +       bool "Ralink RT3883 pin control"
> >>>>>>> +       depends on SOC_RT3883 || COMPILE_TEST
> >>>>>>> +       depends on RALINK
> >>>>>>> +       default SOC_RT3883
> >>>>>>> +       select PINCTRL_MTK_MTMIPS
> >>>>>>> +
> >>>>>>
> >>>>>> I am not a Kconfig expert at all but...
> >>>>>>
> >>>>>> Should not all of these be depends on SOC_XXX || (COMPILE_TEST &&
> >>>>>> RALINK) and avoid the " depends on RALINK" next line in all of them?
> >>>>
> >>>> This seems to do the same thing but I'm following the "either change
> >>>> them all or fit into the crowd" ideology.
> >>>>
> >>>>>>
> >>>>>> Just asking since we have yet arch read and write register operations
> >>>>>> in pinctrl common ralink code. Having in this way, when we address
> >>>>>> this arch thing  in the next series just removing the "&& RALINK" part
> >>>>>> makes the review pretty obvious.
> >>>>
> >>>> You'd have to change RALINK with OF since we're still depending on that.
> >>>> RALINK selects OF by default so it's currently a hidden dependency.
> >>>>
> >>>>>>
> >>>>>> Other than that, changes look good to me.
> >>>>>
> >>>>> I think "depends on SOC_XXX || (COMPILE_TEST && MIPS)" would work also
> >>>>> and might be more accurate for compile testing targets.
> >>>
> >>> Are you sure? SOC_XXX here is already being enabled only if RALINK is
> >>> already enabled, right? [0]
> >>
> >> I'm not sure who's your reply to, or what it's about here.
> >
> > Bad insertion between lines, sorry :). I was just trying to explain to
> > you that SOC_RTXX ralink stuff is only available when RALINK is
> > already selected.
>
> Makes sense. However, I believe what I said below is still true. This
> option will be available to compile if a Ralink SoC (and therefore
> RALINK) is enabled, OR, COMPILE_TEST and MIPS is enabled. The latter
> will fail to compile if the enabled MIPS platform is not RALINK.
>
> >
> >>
> >>>
> >>>>
> >>>> This is not OK in both cases. If the driver is dependent on Ralink
> >>>> architecture code, choosing any other MIPS platform will make the driver
> >>>> available to compile, which will fail.
> >>>
> >>> SOC_XXX is already dependent on RALINK for real uses but the driver is
> >>> going to be selected for other MIPS platforms only for COMPILE_TEST
> >>> targets. Ideally drivers should be arch agnostic so can be selected
> >>> for any single arch build. Now we have arch dependent read and write
> >>> calls in the code, so you need the right headers to be properly found
> >>> to be able to compile testing. I think MIPS is enough dependency here
> >>> to properly find them. But if not, this should be (COMPILE_TEST &&
> >>> RALINK)
> >>
> >> I expect below to work without requiring the MIPS option.
> >>
> >> ifeq ($(CONFIG_COMPILE_TEST),y)
> >> CFLAGS_pinctrl-mtmips.o         += -I$(srctree)/arch/mips/include
> >> endif
> >
> > Yes, this will work but won't be necessary at all when we get rid of
> > ralink arch dependent code in the next series.
>
> Oh, you plan to completely get rid of it, including headers. That's better!

I'd really love to get rid of all of that, yes.

>
> However, rt305x_pinctrl_probe() on pinctrl-rt305x.c needs them to find
> out the SoC to match the pinmux data. Sure, splitting the driver further
> will work but I'm wondering if you've got something else in mind to
> address this.

I know. Sharing the same compatible string makes really hard to do
this easily. One of my thoughts was to split also that in the driver
as you are pointing out here. I have also submitted this series [0] to
be able to make use of soc_match stuff instead of relying on
compatible strings for these kinds of situations. However I am not
also sure that would be a valid approach. Let's see. At the end we can
end up splitting the driver if nothing seems to work.

Thanks,
    Sergio Paracuellos

[0]: https://lore.kernel.org/linux-mips/20230227105806.2394101-1-sergio.paracuellos@gmail.com/T/#t

>
> Arınç
Linus Walleij March 6, 2023, 10:09 p.m. UTC | #5
On Mon, Mar 6, 2023 at 3:56 PM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:

> Sure, if it's necessary. Once I get feedback, I'll rebase it to your
> linusw/linux-pinctrl.git for-next tree, see if it needs manual changes.
> I'll let you know.

Hm my for-next branch is a mixdown for linux-next so use the branch
named "devel" instead.

Thanks!

Linus Walleij
Rob Herring (Arm) March 8, 2023, 8:42 p.m. UTC | #6
On Fri, Mar 03, 2023 at 03:28:36AM +0300, arinc9.unal@gmail.com wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> Move additionalProperties to the top. It's easier to read than after a long
> indented section.
> 
> Drop the quotes from the referred schemas.

Not the greatest subject as every change improves something and bindings 
is stated twice. Hard to come up with something better since you've 
combined 2 different kinds of changes which you shouldn't do either. So 
I'd split this patch into 2.

> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../bindings/pinctrl/ralink,mt7620-pinctrl.yaml          | 9 ++++-----
>  .../bindings/pinctrl/ralink,mt7621-pinctrl.yaml          | 9 ++++-----
>  .../bindings/pinctrl/ralink,rt2880-pinctrl.yaml          | 9 ++++-----
>  .../bindings/pinctrl/ralink,rt305x-pinctrl.yaml          | 9 ++++-----
>  .../bindings/pinctrl/ralink,rt3883-pinctrl.yaml          | 9 ++++-----
>  5 files changed, 20 insertions(+), 25 deletions(-)
Rob Herring (Arm) March 8, 2023, 9:11 p.m. UTC | #7
On Fri, Mar 03, 2023 at 03:28:44AM +0300, arinc9.unal@gmail.com wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> Change the style of description properties to plain style where there's no
> need to preserve the line endings, and vice versa.
> 
> Fit the schemas to 80 columns for each line.

I would just leave the enum cases alone and make this patch just about 
reformatting 'description' entries. Either way:

Reviewed-by: Rob Herring <robh@kernel.org>

> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../pinctrl/mediatek,mt65xx-pinctrl.yaml      | 20 ++---
>  .../pinctrl/mediatek,mt6779-pinctrl.yaml      | 31 ++++----
>  .../pinctrl/mediatek,mt6795-pinctrl.yaml      | 29 ++++----
>  .../pinctrl/mediatek,mt7620-pinctrl.yaml      |  2 +-
>  .../pinctrl/mediatek,mt7621-pinctrl.yaml      |  2 +-
>  .../pinctrl/mediatek,mt7622-pinctrl.yaml      | 24 +++---
>  .../pinctrl/mediatek,mt7981-pinctrl.yaml      | 33 +++++----
>  .../pinctrl/mediatek,mt7986-pinctrl.yaml      | 60 +++++++--------
>  .../pinctrl/mediatek,mt8183-pinctrl.yaml      | 24 +++---
>  .../pinctrl/mediatek,mt8186-pinctrl.yaml      | 43 ++++++-----
>  .../pinctrl/mediatek,mt8188-pinctrl.yaml      | 74 ++++++++++---------
>  .../pinctrl/mediatek,mt8192-pinctrl.yaml      | 43 +++++------
>  .../pinctrl/mediatek,mt8195-pinctrl.yaml      | 34 ++++-----
>  .../pinctrl/mediatek,mt8365-pinctrl.yaml      | 26 ++++---
>  .../pinctrl/ralink,rt2880-pinctrl.yaml        |  2 +-
>  .../pinctrl/ralink,rt305x-pinctrl.yaml        |  2 +-
>  .../pinctrl/ralink,rt3883-pinctrl.yaml        |  2 +-
>  17 files changed, 236 insertions(+), 215 deletions(-)
Arınç ÜNAL March 8, 2023, 9:22 p.m. UTC | #8
On 9.03.2023 00:15, Rob Herring wrote:
> On Fri, Mar 03, 2023 at 03:28:46AM +0300, arinc9.unal@gmail.com wrote:
>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> Set second level patternProperties to '^.*mux.*$' and '^.*conf.*$' on
>> mediatek,mt7986-pinctrl.yaml.
>>
>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> ---
>>   .../devicetree/bindings/pinctrl/mediatek,mt7986-pinctrl.yaml  | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt7986-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt7986-pinctrl.yaml
>> index 46b7228920ed..e937881210c5 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt7986-pinctrl.yaml
>> +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt7986-pinctrl.yaml
>> @@ -72,7 +72,7 @@ patternProperties:
>>       additionalProperties: false
>>   
>>       patternProperties:
>> -      '.*mux.*':
>> +      '^.*mux.*$':
> 
> These are equivalent (so is just 'mux', but that's ambiguous). Why are
> we changing them? Ideally, we'd only have a wildcard on one end.

I've seen your review on Daniel's patch for adding mt7981 pinctrl schema 
so I wanted other schemas to be on par with your review.

https://lore.kernel.org/linux-mediatek/20230123225943.GA2781371-robh@kernel.org/

Arınç
Arınç ÜNAL March 8, 2023, 9:30 p.m. UTC | #9
On 9.03.2023 00:11, Rob Herring wrote:
> On Fri, Mar 03, 2023 at 03:28:44AM +0300, arinc9.unal@gmail.com wrote:
>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> Change the style of description properties to plain style where there's no
>> need to preserve the line endings, and vice versa.
>>
>> Fit the schemas to 80 columns for each line.
> 
> I would just leave the enum cases alone and make this patch just about
> reformatting 'description' entries. Either way:
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

I guess I've got a bit of OCD so it would really annoy me knowing that 
I've kept some lines exceeding 80 columns. It seems OK with you either 
way so I'm going to keep it. ;)

Arınç