diff mbox series

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

Message ID 20230222183932.33267-8-arinc.unal@arinc9.com (mailing list archive)
State Superseded
Headers show
Series pinctrl: ralink: fix ABI, improve driver, move to mediatek, improve dt-bindings | expand

Commit Message

Arınç ÜNAL Feb. 22, 2023, 6:39 p.m. UTC
From: Arınç ÜNAL <arinc.unal@arinc9.com>

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@arinc9.com>
---
 .../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(-)

Comments

Rob Herring Feb. 27, 2023, 5:33 p.m. UTC | #1
On Wed, Feb 22, 2023 at 09:39:23PM +0300, arinc9.unal@gmail.com wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> 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@arinc9.com>
> ---
>  .../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.

Rob
Arınç ÜNAL Feb. 28, 2023, 4:46 p.m. UTC | #2
On 27/02/2023 20:33, Rob Herring wrote:
> On Wed, Feb 22, 2023 at 09:39:23PM +0300, arinc9.unal@gmail.com wrote:
>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> 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@arinc9.com>
>> ---
>>   .../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.

Arınç
Rob Herring March 1, 2023, 2:44 a.m. UTC | #3
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@gmail.com wrote:
> > > From: Arınç ÜNAL <arinc.unal@arinc9.com>
> > > 
> > > 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@arinc9.com>
> > > ---
> > >   .../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.

Rob
Arınç ÜNAL March 1, 2023, 8:15 a.m. UTC | #4
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@gmail.com wrote:
>>>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>>>
>>>> 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@arinc9.com>
>>>> ---
>>>>    .../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.

Arınç
Krzysztof Kozlowski March 2, 2023, 8:28 a.m. UTC | #5
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@gmail.com wrote:
>>>>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>>>>
>>>>> 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@arinc9.com>
>>>>> ---
>>>>>    .../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...

Best regards,
Krzysztof
Arınç ÜNAL March 2, 2023, 9:17 a.m. UTC | #6
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@gmail.com wrote:
>>>>>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>>>>>
>>>>>> 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@arinc9.com>
>>>>>> ---
>>>>>>     .../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.

## Incorrect naming

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

## 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, and dt-binding schemas on my next version.

Arınç
Krzysztof Kozlowski March 2, 2023, 9:58 a.m. UTC | #7
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@gmail.com wrote:
>>>>>>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>>>>>>
>>>>>>> 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@arinc9.com>
>>>>>>> ---
>>>>>>>     .../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.

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

Best regards,
Krzysztof
Arınç ÜNAL March 2, 2023, 10:22 a.m. UTC | #8
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@gmail.com wrote:
>>>>>>>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>>>>>>>
>>>>>>>> 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@arinc9.com>
>>>>>>>> ---
>>>>>>>>      .../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ç
Krzysztof Kozlowski March 2, 2023, 10:29 a.m. UTC | #9
On 02/03/2023 11:22, Arınç ÜNAL wrote:
>>>
>>> ## 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?

Misleading yes, but also does not matter. At least matter not enough to
justify ABI break, so you would need to deprecate old ones and keep
everything backwards compatible. You still would affect 3rd party users
of DTS, though...

Best regards,
Krzysztof
Arınç ÜNAL March 2, 2023, 10:47 a.m. UTC | #10
On 2.03.2023 13:29, Krzysztof Kozlowski wrote:
> On 02/03/2023 11:22, Arınç ÜNAL wrote:
>>>>
>>>> ## 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?
> 
> Misleading yes, but also does not matter. At least matter not enough to
> justify ABI break, so you would need to deprecate old ones and keep
> everything backwards compatible. You still would affect 3rd party users
> of DTS, though...

I intend to do just that. Introduce new mediatek strings, keep the old 
ones so it's backwards compatible, therefore don't break the ABI.

Instead of deprecating old strings, I intend to introduce the checks I 
mentioned, on the schema, so the pin muxing bindings only apply if the 
DT has got a string that won't match multiple schemas. This way it 
shouldn't affect 3rd party DTs.

Arınç
Krzysztof Kozlowski March 2, 2023, 11:36 a.m. UTC | #11
On 02/03/2023 11:47, Arınç ÜNAL wrote:
> On 2.03.2023 13:29, Krzysztof Kozlowski wrote:
>> On 02/03/2023 11:22, Arınç ÜNAL wrote:
>>>>>
>>>>> ## 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?
>>
>> Misleading yes, but also does not matter. At least matter not enough to
>> justify ABI break, so you would need to deprecate old ones and keep
>> everything backwards compatible. You still would affect 3rd party users
>> of DTS, though...
> 
> I intend to do just that. Introduce new mediatek strings, keep the old 
> ones so it's backwards compatible, therefore don't break the ABI.
> 
> Instead of deprecating old strings, I intend to introduce the checks I 
> mentioned, on the schema, so the pin muxing bindings only apply if the 
> DT has got a string that won't match multiple schemas. This way it 
> shouldn't affect 3rd party DTs.

I meant, 3rd party users of DTS. You will replace the compatible in the
DTS, right? So the DTS exported and used in all other projects, OS,
firmwares, bootloaders, out of tree kernel forks will stop working.

Best regards,
Krzysztof
Arınç ÜNAL March 2, 2023, 11:50 a.m. UTC | #12
On 2.03.2023 14:36, Krzysztof Kozlowski wrote:
> On 02/03/2023 11:47, Arınç ÜNAL wrote:
>> On 2.03.2023 13:29, Krzysztof Kozlowski wrote:
>>> On 02/03/2023 11:22, Arınç ÜNAL wrote:
>>>>>>
>>>>>> ## 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?
>>>
>>> Misleading yes, but also does not matter. At least matter not enough to
>>> justify ABI break, so you would need to deprecate old ones and keep
>>> everything backwards compatible. You still would affect 3rd party users
>>> of DTS, though...
>>
>> I intend to do just that. Introduce new mediatek strings, keep the old
>> ones so it's backwards compatible, therefore don't break the ABI.
>>
>> Instead of deprecating old strings, I intend to introduce the checks I
>> mentioned, on the schema, so the pin muxing bindings only apply if the
>> DT has got a string that won't match multiple schemas. This way it
>> shouldn't affect 3rd party DTs.
> 
> I meant, 3rd party users of DTS. You will replace the compatible in the
> DTS, right? So the DTS exported and used in all other projects, OS,
> firmwares, bootloaders, out of tree kernel forks will stop working.

I plan to change it on the DTs for MediaTek SoCs, yes. Is this a 
problem? From what I can tell, what must be ensured is that old DTs must 
work with newer kernels, not new DTs on older kernels.

Arınç
Arınç ÜNAL March 2, 2023, 10:33 p.m. UTC | #13
On 2.03.2023 13:47, Arınç ÜNAL wrote:
> On 2.03.2023 13:29, Krzysztof Kozlowski wrote:
>> On 02/03/2023 11:22, Arınç ÜNAL wrote:
>>>>>
>>>>> ## 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?
>>
>> Misleading yes, but also does not matter. At least matter not enough to
>> justify ABI break, so you would need to deprecate old ones and keep
>> everything backwards compatible. You still would affect 3rd party users
>> of DTS, though...
> 
> I intend to do just that. Introduce new mediatek strings, keep the old 
> ones so it's backwards compatible, therefore don't break the ABI.
> 
> Instead of deprecating old strings, I intend to introduce the checks I 
> mentioned, on the schema, so the pin muxing bindings only apply if the 
> DT has got a string that won't match multiple schemas. This way it 
> shouldn't affect 3rd party DTs.

I'm looking at this again and I see that doing this brings more issues 
than it solves. I think deprecating the old strings from the schemas is 
better. They will be on the driver anyway so newer kernels will still 
work fine with old DTs.

Arınç
Krzysztof Kozlowski March 3, 2023, 7:05 a.m. UTC | #14
On 02/03/2023 12:50, Arınç ÜNAL wrote:
> On 2.03.2023 14:36, Krzysztof Kozlowski wrote:
>> On 02/03/2023 11:47, Arınç ÜNAL wrote:
>>> On 2.03.2023 13:29, Krzysztof Kozlowski wrote:
>>>> On 02/03/2023 11:22, Arınç ÜNAL wrote:
>>>>>>>
>>>>>>> ## 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?
>>>>
>>>> Misleading yes, but also does not matter. At least matter not enough to
>>>> justify ABI break, so you would need to deprecate old ones and keep
>>>> everything backwards compatible. You still would affect 3rd party users
>>>> of DTS, though...
>>>
>>> I intend to do just that. Introduce new mediatek strings, keep the old
>>> ones so it's backwards compatible, therefore don't break the ABI.
>>>
>>> Instead of deprecating old strings, I intend to introduce the checks I
>>> mentioned, on the schema, so the pin muxing bindings only apply if the
>>> DT has got a string that won't match multiple schemas. This way it
>>> shouldn't affect 3rd party DTs.
>>
>> I meant, 3rd party users of DTS. You will replace the compatible in the
>> DTS, right? So the DTS exported and used in all other projects, OS,
>> firmwares, bootloaders, out of tree kernel forks will stop working.
> 
> I plan to change it on the DTs for MediaTek SoCs, yes. Is this a 
> problem? From what I can tell, what must be ensured is that old DTs must 
> work with newer kernels, not new DTs on older kernels.

Can I be clearer than this?

" So the DTS exported and used in all other projects, OS,
firmwares, bootloaders, out of tree kernel forks will stop working."

Yes, this is a problem - they will stop working.

Best regards,
Krzysztof
Arınç ÜNAL March 3, 2023, 7:44 a.m. UTC | #15
On 3.03.2023 10:05, Krzysztof Kozlowski wrote:
> On 02/03/2023 12:50, Arınç ÜNAL wrote:
>> On 2.03.2023 14:36, Krzysztof Kozlowski wrote:
>>> On 02/03/2023 11:47, Arınç ÜNAL wrote:
>>>> On 2.03.2023 13:29, Krzysztof Kozlowski wrote:
>>>>> On 02/03/2023 11:22, Arınç ÜNAL wrote:
>>>>>>>>
>>>>>>>> ## 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?
>>>>>
>>>>> Misleading yes, but also does not matter. At least matter not enough to
>>>>> justify ABI break, so you would need to deprecate old ones and keep
>>>>> everything backwards compatible. You still would affect 3rd party users
>>>>> of DTS, though...
>>>>
>>>> I intend to do just that. Introduce new mediatek strings, keep the old
>>>> ones so it's backwards compatible, therefore don't break the ABI.
>>>>
>>>> Instead of deprecating old strings, I intend to introduce the checks I
>>>> mentioned, on the schema, so the pin muxing bindings only apply if the
>>>> DT has got a string that won't match multiple schemas. This way it
>>>> shouldn't affect 3rd party DTs.
>>>
>>> I meant, 3rd party users of DTS. You will replace the compatible in the
>>> DTS, right? So the DTS exported and used in all other projects, OS,
>>> firmwares, bootloaders, out of tree kernel forks will stop working.
>>
>> I plan to change it on the DTs for MediaTek SoCs, yes. Is this a
>> problem? From what I can tell, what must be ensured is that old DTs must
>> work with newer kernels, not new DTs on older kernels.
> 
> Can I be clearer than this?
> 
> " So the DTS exported and used in all other projects, OS,
> firmwares, bootloaders, out of tree kernel forks will stop working."
> 
> Yes, this is a problem - they will stop working.

I've never seen any project just exporting DTs from the latest kernel 
version and slap it onto old versions, as a new devicetree that was 
introduced with a newer kernel version is not guaranteed to work with 
older kernel versions.

If someone is actually doing this on a project, I think it's the 
responsibility of the maintainers of these said projects to account for 
this and modify the DT for the kernel version they're running it on.

What's more usual is one'd run the kernel version where the new DT was 
introduced, which will work fine.

On to real life implications, popular projects like U-Boot and OpenWrt 
maintain their own DTs for this platform so I think the impact is very 
minimal.

Anyway, I'm not doing this change on this patch series so why don't we 
cross this bridge when we get to it.

Arınç
Krzysztof Kozlowski March 3, 2023, 7:53 a.m. UTC | #16
On 03/03/2023 08:44, Arınç ÜNAL wrote:
> On 3.03.2023 10:05, Krzysztof Kozlowski wrote:
>> On 02/03/2023 12:50, Arınç ÜNAL wrote:
>>> On 2.03.2023 14:36, Krzysztof Kozlowski wrote:
>>>> On 02/03/2023 11:47, Arınç ÜNAL wrote:
>>>>> On 2.03.2023 13:29, Krzysztof Kozlowski wrote:
>>>>>> On 02/03/2023 11:22, Arınç ÜNAL wrote:
>>>>>>>>>
>>>>>>>>> ## 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?
>>>>>>
>>>>>> Misleading yes, but also does not matter. At least matter not enough to
>>>>>> justify ABI break, so you would need to deprecate old ones and keep
>>>>>> everything backwards compatible. You still would affect 3rd party users
>>>>>> of DTS, though...
>>>>>
>>>>> I intend to do just that. Introduce new mediatek strings, keep the old
>>>>> ones so it's backwards compatible, therefore don't break the ABI.
>>>>>
>>>>> Instead of deprecating old strings, I intend to introduce the checks I
>>>>> mentioned, on the schema, so the pin muxing bindings only apply if the
>>>>> DT has got a string that won't match multiple schemas. This way it
>>>>> shouldn't affect 3rd party DTs.
>>>>
>>>> I meant, 3rd party users of DTS. You will replace the compatible in the
>>>> DTS, right? So the DTS exported and used in all other projects, OS,
>>>> firmwares, bootloaders, out of tree kernel forks will stop working.
>>>
>>> I plan to change it on the DTs for MediaTek SoCs, yes. Is this a
>>> problem? From what I can tell, what must be ensured is that old DTs must
>>> work with newer kernels, not new DTs on older kernels.
>>
>> Can I be clearer than this?
>>
>> " So the DTS exported and used in all other projects, OS,
>> firmwares, bootloaders, out of tree kernel forks will stop working."
>>
>> Yes, this is a problem - they will stop working.
> 
> I've never seen any project just exporting DTs from the latest kernel 
> version and slap it onto old versions, as a new devicetree that was 

Really? U-Boot does it all the time, other projects (like BSD) do it
periodically to some extend as well.

> introduced with a newer kernel version is not guaranteed to work with 
> older kernel versions.

Not guaranteed but it is expected, though, to some level and under some
conditions. Therefore it might be or might not be a problem. For some
platforms no one cares. For some people care.

> 
> If someone is actually doing this on a project, I think it's the 
> responsibility of the maintainers of these said projects to account for 
> this and modify the DT for the kernel version they're running it on.
> 
> What's more usual is one'd run the kernel version where the new DT was 
> introduced, which will work fine.

"kernel" as Linux is only one part of it. I mentioned several other
projects.

> 
> On to real life implications, popular projects like U-Boot and OpenWrt 
> maintain their own DTs for this platform so I think the impact is very 
> minimal.

And they sync with Linux kernel DTS.

> 
> Anyway, I'm not doing this change on this patch series so why don't we 
> cross this bridge when we get to it.


Best regards,
Krzysztof
Arınç ÜNAL March 3, 2023, 8:03 a.m. UTC | #17
On 3.03.2023 10:53, Krzysztof Kozlowski wrote:
> On 03/03/2023 08:44, Arınç ÜNAL wrote:
>> On 3.03.2023 10:05, Krzysztof Kozlowski wrote:
>>> On 02/03/2023 12:50, Arınç ÜNAL wrote:
>>>> On 2.03.2023 14:36, Krzysztof Kozlowski wrote:
>>>>> On 02/03/2023 11:47, Arınç ÜNAL wrote:
>>>>>> On 2.03.2023 13:29, Krzysztof Kozlowski wrote:
>>>>>>> On 02/03/2023 11:22, Arınç ÜNAL wrote:
>>>>>>>>>>
>>>>>>>>>> ## 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?
>>>>>>>
>>>>>>> Misleading yes, but also does not matter. At least matter not enough to
>>>>>>> justify ABI break, so you would need to deprecate old ones and keep
>>>>>>> everything backwards compatible. You still would affect 3rd party users
>>>>>>> of DTS, though...
>>>>>>
>>>>>> I intend to do just that. Introduce new mediatek strings, keep the old
>>>>>> ones so it's backwards compatible, therefore don't break the ABI.
>>>>>>
>>>>>> Instead of deprecating old strings, I intend to introduce the checks I
>>>>>> mentioned, on the schema, so the pin muxing bindings only apply if the
>>>>>> DT has got a string that won't match multiple schemas. This way it
>>>>>> shouldn't affect 3rd party DTs.
>>>>>
>>>>> I meant, 3rd party users of DTS. You will replace the compatible in the
>>>>> DTS, right? So the DTS exported and used in all other projects, OS,
>>>>> firmwares, bootloaders, out of tree kernel forks will stop working.
>>>>
>>>> I plan to change it on the DTs for MediaTek SoCs, yes. Is this a
>>>> problem? From what I can tell, what must be ensured is that old DTs must
>>>> work with newer kernels, not new DTs on older kernels.
>>>
>>> Can I be clearer than this?
>>>
>>> " So the DTS exported and used in all other projects, OS,
>>> firmwares, bootloaders, out of tree kernel forks will stop working."
>>>
>>> Yes, this is a problem - they will stop working.
>>
>> I've never seen any project just exporting DTs from the latest kernel
>> version and slap it onto old versions, as a new devicetree that was
> 
> Really? U-Boot does it all the time, other projects (like BSD) do it
> periodically to some extend as well.

They must do heavy reviewing before shipping it. Drivers like MediaTek 
ethernet on U-Boot is different than in Linux, the dt-bindings are all 
different. Under a review, these changes will pop out for them to 
address so there're no problems.

> 
>> introduced with a newer kernel version is not guaranteed to work with
>> older kernel versions.
> 
> Not guaranteed but it is expected, though, to some level and under some
> conditions. Therefore it might be or might not be a problem. For some
> platforms no one cares. For some people care.

I'm going to assume there's not much care for this platform, at least 
for mt7621, as I've heard no complaints when I did this before.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/mips/boot/dts/ralink/mt7621.dtsi?id=b4f209e32ba5c283e7b1dd00d867b0536d3e215e

> 
>>
>> If someone is actually doing this on a project, I think it's the
>> responsibility of the maintainers of these said projects to account for
>> this and modify the DT for the kernel version they're running it on.
>>
>> What's more usual is one'd run the kernel version where the new DT was
>> introduced, which will work fine.
> 
> "kernel" as Linux is only one part of it. I mentioned several other
> projects.
> 
>>
>> On to real life implications, popular projects like U-Boot and OpenWrt
>> maintain their own DTs for this platform so I think the impact is very
>> minimal.
> 
> And they sync with Linux kernel DTS.

Again, the DTs must be reviewed so they will be modified and the 
potential issue will be addressed.

Arınç
diff mbox series

Patch

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
+      - ralink,rt2880-pinmux
 
 patternProperties:
   '-pins$':
@@ -647,7 +650,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 1b1d37b981d9..74923ca35c81 100644
--- a/Documentation/devicetree/bindings/pinctrl/ralink,mt7621-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/ralink,mt7621-pinctrl.yaml
@@ -17,7 +17,10 @@  description:
 
 properties:
   compatible:
-    const: ralink,mt7621-pinctrl
+    enum:
+      - mediatek,mt7621-pinctrl
+      - ralink,mt7621-pinctrl
+      - ralink,rt2880-pinmux
 
 patternProperties:
   '-pins$':
@@ -251,7 +254,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,rt2880-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinctrl.yaml
index 7fd0df880a76..aceea6248614 100644
--- a/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/ralink,rt2880-pinctrl.yaml
@@ -17,7 +17,10 @@  description:
 
 properties:
   compatible:
-    const: ralink,rt2880-pinctrl
+    enum:
+      - mediatek,rt2880-pinctrl
+      - ralink,rt2880-pinctrl
+      - ralink,rt2880-pinmux
 
 patternProperties:
   '-pins$':
@@ -131,7 +134,7 @@  additionalProperties: false
 examples:
   - |
     pinctrl {
-      compatible = "ralink,rt2880-pinctrl";
+      compatible = "mediatek,rt2880-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 4d66ca752a30..4c87fe201809 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:
+      - mediatek,rt305x-pinctrl
+      - ralink,rt305x-pinctrl
+      - ralink,rt2880-pinmux
 
 patternProperties:
   '-pins$':
@@ -264,7 +267,7 @@  additionalProperties: false
 examples:
   - |
     pinctrl {
-      compatible = "ralink,rt305x-pinctrl";
+      compatible = "mediatek,rt305x-pinctrl";
 
       i2c_pins: i2c0-pins {
         pinmux {
diff --git a/Documentation/devicetree/bindings/pinctrl/ralink,rt3883-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/ralink,rt3883-pinctrl.yaml
index 008d93181aea..71049a2b2779 100644
--- a/Documentation/devicetree/bindings/pinctrl/ralink,rt3883-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/ralink,rt3883-pinctrl.yaml
@@ -17,7 +17,10 @@  description:
 
 properties:
   compatible:
-    const: ralink,rt3883-pinctrl
+    enum:
+      - mediatek,rt3883-pinctrl
+      - ralink,rt3883-pinctrl
+      - ralink,rt2880-pinmux
 
 patternProperties:
   '-pins$':
@@ -251,7 +254,7 @@  additionalProperties: false
 examples:
   - |
     pinctrl {
-      compatible = "ralink,rt3883-pinctrl";
+      compatible = "mediatek,rt3883-pinctrl";
 
       i2c_pins: i2c0-pins {
         pinmux {