Re: [PATCH 10/14] dt-bindings: pinctrl: rt2880: fix binding name, pin groups and functions

[Arınç ÜNAL]

On 13/04/2022 18:25, Krzysztof Kozlowski wrote:
> On 13/04/2022 08:07, Arınç ÜNAL wrote:
> > Change binding name from ralink,rt2880-pinmux to ralink,rt2880-pinctrl.
> > This is the binding for the Ralink RT2880 pinctrl subdriver.
>
> What I don't see here is why you are doing this. pinmux/pinctrl have the
> same meaning, I guess?

What I understand is pinmux is rather a specific term for the muxing of 
pins or pin groups. Pinctrl is what we prefer here since the term is 
more inclusive of what the subdriver does: controlling pins. Any 
mediatek driver/subdriver is called pinctrl so I'm not doing something 
uncommon.

> > Current pin group and function bindings are for MT7621. Put bindings for
> > RT2880 instead.
> >
> > Signed-off-by: Arınç ÜNAL <arinc.unal@xxxxxxxxxx>
> > ---
> >   ...pinmux.yaml => ralink,rt2880-pinctrl.yaml} | 24 +++++++++----------
> >   1 file changed, 12 insertions(+), 12 deletions(-)
> >   rename Documentation/devicetree/bindings/pinctrl/{ralink,rt2880-pinmux.yaml => ralink,rt2880-pinctrl.yaml} (56%)
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinctrl.yaml
> > similarity index 56%
> > rename from Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
> > rename to Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinctrl.yaml
> > index 9de8b0c075e2..c657bbf9fdda 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinmux.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinctrl.yaml
> > @@ -1,21 +1,23 @@
> >   # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> >   %YAML 1.2
> >   ---
> > -$id: http://devicetree.org/schemas/pinctrl/ralink,rt2880-pinmux.yaml#
> > +$id: http://devicetree.org/schemas/pinctrl/ralink,rt2880-pinctrl.yaml#
> >   $schema: http://devicetree.org/meta-schemas/core.yaml#
> >   
> > -title: Ralink rt2880 pinmux controller
> > +title: Ralink RT2880 Pin Controller
> >   
> >   maintainers:
> > +  - Arınç ÜNAL <arinc.unal@xxxxxxxxxx>
>
> Mention this in commit msg.

Will do.

> >     - Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> >   
> >   description:
> > -  The rt2880 pinmux can only set the muxing of pin groups. Muxing indiviual pins
> > +  Ralink RT2880 pin controller for RT2880 SoC.
> > +  The pin controller can only set the muxing of pin groups. Muxing indiviual pins
> >     is not supported. There is no pinconf support.
> >   
> >   properties:
> >     compatible:
> > -    const: ralink,rt2880-pinmux
> > +    const: ralink,rt2880-pinctrl
>
> you need to deprecate old property and add a new one.

Do we really have to? That property name was inaccurate from the start. 
I don't see a reason to keep it being referred to on the binding.

> >   
> >   patternProperties:
> >     '-pins$':
> > @@ -28,14 +30,12 @@ patternProperties:
> >   
> >           properties:
> >             groups:
> > -            description: Name of the pin group to use for the functions.
> > -            enum: [i2c, jtag, mdio, pcie, rgmii1, rgmii2, sdhci, spi,
> > -                   uart1, uart2, uart3, wdt]
> > +            description: The pin group to select.
> > +            enum: [i2c, spi, uartlite, jtag, mdio, sdram, pci]
> > +
> >             function:
> > -            description: The mux function to select
> > -            enum: [gpio, i2c, i2s, jtag, mdio, nand1, nand2, pcie refclk,
> > -                   pcie rst, pcm, rgmii1, rgmii2, sdhci, spdif2, spdif3,
> > -                   spi, uart1, uart2, uart3, wdt refclk, wdt rst]
> > +            description: The mux function to select.
> > +            enum: [gpio, i2c, spi, uartlite, jtag, mdio, sdram, pci]
> >   
>
> These were all incorrect for rt2880, I understand?

Pretty much.

Arınç