1. Patchew
  2. linux
  3. [PATCH 0/14] Refactor Ralink Pinctrl and Add Documentation
  4. View patch

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

Arınç ÜNAL posted 14 patches 2 years, 5 months ago
There is a newer version of this series
[PATCH 10/14] dt-bindings: pinctrl: rt2880: fix binding name, pin groups and functions
Posted by Arınç ÜNAL 2 years, 5 months ago 
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%)

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 {
-- 
2.25.1
Re: [PATCH 10/14] dt-bindings: pinctrl: rt2880: fix binding name, pin groups and functions
Posted by Krzysztof Kozlowski 2 years, 5 months ago 
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
Re: [PATCH 10/14] dt-bindings: pinctrl: rt2880: fix binding name, pin groups and functions
Posted by Arınç ÜNAL 2 years, 5 months ago 
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ç
Re: [PATCH 10/14] dt-bindings: pinctrl: rt2880: fix binding name, pin groups and functions
Posted by Rob Herring 2 years, 5 months ago 
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
Re: [PATCH 10/14] dt-bindings: pinctrl: rt2880: fix binding name, pin groups and functions
Posted by Arınç ÜNAL 2 years, 5 months ago 
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ç

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.