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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <550b20f2-098e-0f25-ad9f-3ff523879cb7@arinc9.com>
Date:   Thu, 14 Apr 2022 11:34:31 +0300
From:   Arınç ÜNAL <arinc.unal@...nc9.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Sergio Paracuellos <sergio.paracuellos@...il.com>,
        Luiz Angelo Daros de Luca <luizluca@...il.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Matthias Brugger <matthias.bgg@...il.com>
Cc:     erkin.bozoglu@...ont.com, linux-gpio@...r.kernel.org,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-mips@...r.kernel.org
Subject: Re: [PATCH 10/14] dt-bindings: pinctrl: rt2880: fix binding name, pin
 groups and functions

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@...nc9.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@...nc9.com>
> 
> Mention this in commit msg.

Will do.

> 
>>     - Sergio Paracuellos <sergio.paracuellos@...il.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ç