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

[Rob Herring]

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@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.

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