Re: [RFC PATCH 07/16] dt-bindings: pinctrl: ralink: add new compatible strings

Arınç ÜNAL <arinc.unal@xxxxxxxxxx> · Thu, 2 Mar 2023 13:22:19 +0300

On 2.03.2023 12:58, Krzysztof Kozlowski wrote:
On 02/03/2023 10:17, Arınç ÜNAL wrote:
On 2.03.2023 11:28, Krzysztof Kozlowski wrote:
On 01/03/2023 09:15, Arınç ÜNAL wrote:
On 1.03.2023 05:44, Rob Herring wrote:
On Tue, Feb 28, 2023 at 07:46:36PM +0300, Arınç ÜNAL wrote:
On 27/02/2023 20:33, Rob Herring wrote:
On Wed, Feb 22, 2023 at 09:39:23PM +0300, arinc9.unal@xxxxxxxxx wrote:
From: Arınç ÜNAL <arinc.unal@xxxxxxxxxx>

Add the ralink,rt2880-pinmux compatible string. It had been removed from
the driver which broke the ABI.

Add the mediatek compatible strings. Change the compatible string on the
examples with the mediatek compatible strings.

Signed-off-by: Arınç ÜNAL <arinc.unal@xxxxxxxxxx>
---
     .../devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml | 7 +++++--
     .../devicetree/bindings/pinctrl/ralink,mt7621-pinctrl.yaml | 7 +++++--
     .../devicetree/bindings/pinctrl/ralink,rt2880-pinctrl.yaml | 7 +++++--
     .../devicetree/bindings/pinctrl/ralink,rt305x-pinctrl.yaml | 7 +++++--
     .../devicetree/bindings/pinctrl/ralink,rt3883-pinctrl.yaml | 7 +++++--
     5 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml
index 1e63ea34146a..531b5f616c3d 100644
--- a/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml
@@ -17,7 +17,10 @@ description:
     properties:
       compatible:
-    const: ralink,mt7620-pinctrl
+    enum:
+      - mediatek,mt7620-pinctrl
+      - ralink,mt7620-pinctrl

We don't update compatible strings based on acquistions nor marketing
whims. If you want to use 'mediatek' for new things, then fine.

Understood. Only the SoCs with rtXXXX were rebranded, the mtXXXX SoCs share
the same architecture from Ralink, so they were incorrectly called Ralink
SoCs.

I can remove the new strings from Ralink SoCs and add them only for MediaTek
SoCs. Or you could make an exception for this one, regarding the situation.
Whatever you think is best.

I'm not in a position to make an exception as I know little about this
platform. Carrying both strings is a NAK. Either you (and everyone using
these platforms) care about the ABI and are stuck with the "wrong"
string. In the end, they are just unique identifiers. Or you don't care
and break the ABI and rename everything. If you do that, do just that in
your patches and make it crystal clear in the commit msg that is your
intention and why that is okay.

Ralink had their MIPS SoCs pre-acquisition, RT2880, etc. MediaTek
introduced new SoCs post-acquisition, MT7620, MT7621, MT7628, and
MT7688, utilising the same platform from Ralink, sharing the same
architecture code, pinctrl core driver, etc.

I don't intend to break the ABI at all. On the contrary, I fix it where
possible.

If I understand correctly, from this conversation and what Krzysztof
said, all strings must be kept on the schemas so I can do what I said on
the composed mail. Only match the pin muxing information on the strings
that won't match multiple pin muxing information from other schemas.

This way we don't break the ABI, introduce new compatible strings while
keeping the remaining ones, and make schemas match correctly.

Let me know if this is acceptable to you.

If by "introduce new compatible strings" you mean duplicate compatibles
to fix the ralink->mediatek, then you ignored entire email from Rob -
this and previous. We don't do this. Leave them as is.

If you meant something else, explain more...

Let me put them in a group to better explain.

## Fix ABI

ralink,rt2880-pinmux was there before, it was removed which broke the
ABI. I'm reintroducing it to fix it.

## New strings to be able to split bindings

New strings are needed for MT7628/MT7688 and some RT SoCs to be able to
properly document the pin muxing information.

Then ok.


## Incorrect naming

MT7620, MT7621, MT7628, and MT7688 SoCs are incorrectly called Ralink,
introduce new ralink->mediatek compatible strings to address it.

So this part was addressed by Rob - we don't do it, because it does not
matter. Ralink is now Mediatek, thus there is no conflict and no issues
with different vendor used.

I think Rob was rather addressing that updating compatible strings based 
on acquisition or marketing whims is not permitted. This condition does 
not apply here as these SoCs were never Ralink.

I understand your point that Ralink is now MediaTek but still, calling 
these SoCs Ralink would be a bit misleading, don't you think?



## Exception for RT SoCs to be called MediaTek

This is where I was asking an exception to be made. Rob told us here
they know little about the platform so I explained it.

MediaTek acquired Ralink and then introduced new MediaTek SoCs utilising
the same platform from Ralink.

Anyway, now that I look at this again, it makes sense to me as well not
to rename the Ralink SoCs. I'll call the RT SoCs Ralink on the kconfig,
pinctrl driver,

These are separate. We did not comment on how you call Linux drivers.
The mail thread was only about:

and dt-binding schemas on my next version.

Understood, thanks.

Arınç






