[PATCH 08/20] dt-bindings: pinctrl: ralink: add new compatible strings

arinc9.unal@gmail.com posted 20 patches 1 year, 6 months ago
There is a newer version of this series
[PATCH 08/20] dt-bindings: pinctrl: ralink: add new compatible strings
Posted by arinc9.unal@gmail.com 1 year, 6 months ago
From: Arınç ÜNAL <arinc.unal@arinc9.com>

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

Add the new compatible strings for mt7620, mt76x8, and rt305x to be able to
properly document the pin muxing information of each SoC, or SoCs that use
the same pinmux data.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml | 7 +++++--
 .../devicetree/bindings/pinctrl/ralink,mt7621-pinctrl.yaml | 6 ++++--
 .../devicetree/bindings/pinctrl/ralink,rt305x-pinctrl.yaml | 5 ++++-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml
index cde6de77e228..a94d2e7a5f37 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
+      - mediatek,mt76x8-pinctrl
+      - ralink,mt7620-pinctrl
 
 patternProperties:
   '-pins$':
@@ -646,7 +649,7 @@ additionalProperties: false
 examples:
   - |
     pinctrl {
-      compatible = "ralink,mt7620-pinctrl";
+      compatible = "mediatek,mt7620-pinctrl";
 
       i2c_pins: i2c0-pins {
         pinmux {
diff --git a/Documentation/devicetree/bindings/pinctrl/ralink,mt7621-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,mt7621-pinctrl.yaml
index fb8c5459ea93..eb0746cfc6d6 100644
--- a/Documentation/devicetree/bindings/pinctrl/ralink,mt7621-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/ralink,mt7621-pinctrl.yaml
@@ -17,7 +17,9 @@ description:
 
 properties:
   compatible:
-    const: ralink,mt7621-pinctrl
+    enum:
+      - mediatek,mt7621-pinctrl
+      - ralink,mt7621-pinctrl
 
 patternProperties:
   '-pins$':
@@ -250,7 +252,7 @@ additionalProperties: false
 examples:
   - |
     pinctrl {
-      compatible = "ralink,mt7621-pinctrl";
+      compatible = "mediatek,mt7621-pinctrl";
 
       i2c_pins: i2c0-pins {
         pinmux {
diff --git a/Documentation/devicetree/bindings/pinctrl/ralink,rt305x-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,rt305x-pinctrl.yaml
index 8b1256af09c3..23fb82f9959c 100644
--- a/Documentation/devicetree/bindings/pinctrl/ralink,rt305x-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/ralink,rt305x-pinctrl.yaml
@@ -18,7 +18,10 @@ description:
 
 properties:
   compatible:
-    const: ralink,rt305x-pinctrl
+    enum:
+      - ralink,rt305x-pinctrl
+      - ralink,rt3352-pinctrl
+      - ralink,rt5350-pinctrl
 
 patternProperties:
   '-pins$':
-- 
2.37.2

Re: [PATCH 08/20] dt-bindings: pinctrl: ralink: add new compatible strings
Posted by Rob Herring 1 year, 6 months ago
On Fri, Mar 03, 2023 at 03:28:37AM +0300, arinc9.unal@gmail.com wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> Add the new mediatek compatible strings. Change the compatible string on
> the examples with the mediatek compatible strings.
> 
> Add the new compatible strings for mt7620, mt76x8, and rt305x to be able to
> properly document the pin muxing information of each SoC, or SoCs that use
> the same pinmux data.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml | 7 +++++--
>  .../devicetree/bindings/pinctrl/ralink,mt7621-pinctrl.yaml | 6 ++++--
>  .../devicetree/bindings/pinctrl/ralink,rt305x-pinctrl.yaml | 5 ++++-
>  3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml
> index cde6de77e228..a94d2e7a5f37 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
> +      - mediatek,mt76x8-pinctrl
> +      - ralink,mt7620-pinctrl

To repeat the options from last time:

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

Marketing/acquistion renames was just an example and common reason. That 
doesn't make other reasons okay. I don't see any reason given here.

If you want to break the ABI (do you??, because the commit message 
still doesn't say), then you don't need "ralink,mt7620-pinctrl".

Rob
Re: [PATCH 08/20] dt-bindings: pinctrl: ralink: add new compatible strings
Posted by Arınç ÜNAL 1 year, 6 months ago
On 9.03.2023 00:00, Rob Herring wrote:
> On Fri, Mar 03, 2023 at 03:28:37AM +0300, arinc9.unal@gmail.com wrote:
>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> Add the new mediatek compatible strings. Change the compatible string on
>> the examples with the mediatek compatible strings.
>>
>> Add the new compatible strings for mt7620, mt76x8, and rt305x to be able to
>> properly document the pin muxing information of each SoC, or SoCs that use
>> the same pinmux data.
>>
>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> ---
>>   .../devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml | 7 +++++--
>>   .../devicetree/bindings/pinctrl/ralink,mt7621-pinctrl.yaml | 6 ++++--
>>   .../devicetree/bindings/pinctrl/ralink,rt305x-pinctrl.yaml | 5 ++++-
>>   3 files changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,mt7620-pinctrl.yaml
>> index cde6de77e228..a94d2e7a5f37 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
>> +      - mediatek,mt76x8-pinctrl
>> +      - ralink,mt7620-pinctrl
> 
> To repeat the options from last time:
> 
>> 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.
> 
> Marketing/acquistion renames was just an example and common reason. That
> doesn't make other reasons okay. I don't see any reason given here.

I did give the reason on patch 2 and 9 but I should've put it on this 
patch as well.

> This platform from Ralink was acquired by MediaTek in 2011. Then, MediaTek
> introduced these SoCs which utilise this platform. Add new compatible
> strings to address the incorrect naming.

I'm continuing the conversation regarding this under patch 9.

> 
> If you want to break the ABI (do you??, because the commit message
> still doesn't say), then you don't need "ralink,mt7620-pinctrl".

I don't want to break the ABI. But I deprecate ralink,mt7620-pinctrl on 
later patches. The driver still has it though, so old DTs will keep 
working. That keeps the ABI intact regardless of deprecating strings on 
the dt-binding schema, right?

Arınç
Re: [PATCH 08/20] dt-bindings: pinctrl: ralink: add new compatible strings
Posted by Krzysztof Kozlowski 1 year, 6 months ago
On 08/03/2023 22:19, Arınç ÜNAL wrote:
> 
>>
>> If you want to break the ABI (do you??, because the commit message
>> still doesn't say), then you don't need "ralink,mt7620-pinctrl".
> 
> I don't want to break the ABI. But I deprecate ralink,mt7620-pinctrl on 
> later patches.

Deprecation should happen here. Otherwise you have now two valid
compatibles which contradicts previous Rob's comments.


> The driver still has it though, so old DTs will keep 
> working. That keeps the ABI intact regardless of deprecating strings on 
> the dt-binding schema, right?

Yes, but deprecation is missing.


Best regards,
Krzysztof