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

From: "Arınç ÜNAL" <arinc.unal@arinc9.com>
To: Rob Herring <robh@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Sergio Paracuellos <sergio.paracuellos@gmail.com>,
	Luiz Angelo Daros de Luca <luizluca@gmail.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	erkin.bozoglu@xeront.com, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-mips@vger.kernel.org
Subject: Re: [PATCH 10/14] dt-bindings: pinctrl: rt2880: fix binding name, pin groups and functions
Date: Thu, 14 Apr 2022 19:37:25 +0300	[thread overview]
Message-ID: <87c9bf7e-f290-7d38-0844-7a7243688f5a@arinc9.com> (raw)
In-Reply-To: <YlhJGerakzbsXwnv@robh.at.kernel.org>

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ç

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek