| Message ID | 20220413060729.27639-11-arinc.unal@arinc9.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Refactor Ralink Pinctrl and Add Documentation | expand |
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? > > Current pin group and function bindings are for MT7621. Put bindings for > RT2880 instead. > > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> > --- > ...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@arinc9.com> Mention this in commit msg. > - Sergio Paracuellos <sergio.paracuellos@gmail.com> > > 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. > > 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? Best regards, Krzysztof
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@arinc9.com> >> --- >> ...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@arinc9.com> > > Mention this in commit msg. Will do. > >> - Sergio Paracuellos <sergio.paracuellos@gmail.com> >> >> 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ç
On Thu, Apr 14, 2022 at 11:34:31AM +0300, Arınç ÜNAL wrote: > 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. The correct name is really whatever the h/w block is called, not whatever we've come up with for some class of devices. > > > > > > > > > Current pin group and function bindings are for MT7621. Put bindings for > > > RT2880 instead. > > > > > > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> > > > --- > > > ...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@arinc9.com> > > > > Mention this in commit msg. > > Will do. > > > > > > - Sergio Paracuellos <sergio.paracuellos@gmail.com> > > > 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. It's an ABI. There are exceptions, but you've got to spell out the reasoning in the commit message. Really, who cares. It's just a unique identifier. Unless you also had a h/w block called 'pinmux' in addition to a 'pinctrl' block it doesn't matter. We could use just GUIDs instead. Rob
On 14/04/2022 19:17, Rob Herring wrote: > On Thu, Apr 14, 2022 at 11:34:31AM +0300, Arınç ÜNAL wrote: >> 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. > > The correct name is really whatever the h/w block is called, not > whatever we've come up with for some class of devices. > >> >>> >>>> >>>> Current pin group and function bindings are for MT7621. Put bindings for >>>> RT2880 instead. >>>> >>>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> >>>> --- >>>> ...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@arinc9.com> >>> >>> Mention this in commit msg. >> >> Will do. >> >>> >>>> - Sergio Paracuellos <sergio.paracuellos@gmail.com> >>>> 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. > > It's an ABI. There are exceptions, but you've got to spell out the > reasoning in the commit message. Oh, I thought by deprecating, I was supposed to keep the old one on the YAML binding. I'll properly explain the reason in the commit message. > > Really, who cares. It's just a unique identifier. Unless you also had a > h/w block called 'pinmux' in addition to a 'pinctrl' block it doesn't > matter. We could use just GUIDs instead. Understood, thanks Rob! Arınç
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@arinc9.com> - Sergio Paracuellos <sergio.paracuellos@gmail.com> 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 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] required: - groups @@ -57,7 +57,7 @@ examples: # Pinmux controller node - | pinctrl { - compatible = "ralink,rt2880-pinmux"; + compatible = "ralink,rt2880-pinctrl"; i2c_pins: i2c0-pins { pinmux {
Change binding name from ralink,rt2880-pinmux to ralink,rt2880-pinctrl. This is the binding for the Ralink RT2880 pinctrl subdriver. Current pin group and function bindings are for MT7621. Put bindings for RT2880 instead. Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> --- ...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%)