[PATCH v2 1/9] dt-bindings: clock: add mtmips SoCs system controller

Sergio Paracuellos posted 9 patches 2 years, 10 months ago
There is a newer version of this series
[PATCH v2 1/9] dt-bindings: clock: add mtmips SoCs system controller
Posted by Sergio Paracuellos 2 years, 10 months ago
Adds device tree binding documentation for system controller node present
in Mediatek MIPS and Ralink SOCs. This node is a clock and reset provider
for the rest of the world. This covers RT2880, RT3050, RT3052, RT3350,
RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 .../bindings/clock/mediatek,mtmips-sysc.yaml  | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml

diff --git a/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
new file mode 100644
index 000000000000..f07e1652723b
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/mediatek,mtmips-sysc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MTMIPS SoCs System Controller
+
+maintainers:
+  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
+
+description: |
+  MediaTek MIPS and Ralink SoCs provides a system controller to allow
+  to access to system control registers. These registers include clock
+  and reset related ones so this node is both clock and reset provider
+  for the rest of the world.
+
+  These SoCs have an XTAL from where the cpu clock is
+  provided as well as derived clocks for the bus and the peripherals.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - ralink,mt7620-sysc
+          - ralink,mt7620a-sysc
+          - ralink,mt7628-sysc
+          - ralink,mt7688-sysc
+          - ralink,rt2880-sysc
+          - ralink,rt3050-sysc
+          - ralink,rt3052-sysc
+          - ralink,rt3352-sysc
+          - ralink,rt3883-sysc
+          - ralink,rt5350-sysc
+      - const: syscon
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    description:
+      The first cell indicates the clock number.
+    const: 1
+
+  '#reset-cells':
+    description:
+      The first cell indicates the reset bit within the register.
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+  - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    syscon@0 {
+      compatible = "ralink,rt5350-sysc", "syscon";
+      reg = <0x0 0x100>;
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+    };
-- 
2.25.1
Re: [PATCH v2 1/9] dt-bindings: clock: add mtmips SoCs system controller
Posted by Krzysztof Kozlowski 2 years, 10 months ago
On 21/03/2023 06:00, Sergio Paracuellos wrote:
> Adds device tree binding documentation for system controller node present
> in Mediatek MIPS and Ralink SOCs. This node is a clock and reset provider
> for the rest of the world. This covers RT2880, RT3050, RT3052, RT3350,
> RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  .../bindings/clock/mediatek,mtmips-sysc.yaml  | 65 +++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
> new file mode 100644
> index 000000000000..f07e1652723b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/mediatek,mtmips-sysc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MTMIPS SoCs System Controller
> +
> +maintainers:
> +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> +
> +description: |
> +  MediaTek MIPS and Ralink SoCs provides a system controller to allow
> +  to access to system control registers. These registers include clock
> +  and reset related ones so this node is both clock and reset provider
> +  for the rest of the world.
> +
> +  These SoCs have an XTAL from where the cpu clock is
> +  provided as well as derived clocks for the bus and the peripherals.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - ralink,mt7620-sysc

Since you decided to send it before we finish discussion:
NAK - this is already used as mediatek

> +          - ralink,mt7620a-sysc
> +          - ralink,mt7628-sysc

Same here.

> +          - ralink,mt7688-sysc

I expect you to check the others.



Best regards,
Krzysztof
Re: [PATCH v2 1/9] dt-bindings: clock: add mtmips SoCs system controller
Posted by Sergio Paracuellos 2 years, 10 months ago
On Tue, Mar 21, 2023 at 7:45 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 21/03/2023 06:00, Sergio Paracuellos wrote:
> > Adds device tree binding documentation for system controller node present
> > in Mediatek MIPS and Ralink SOCs. This node is a clock and reset provider
> > for the rest of the world. This covers RT2880, RT3050, RT3052, RT3350,
> > RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  .../bindings/clock/mediatek,mtmips-sysc.yaml  | 65 +++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
> > new file mode 100644
> > index 000000000000..f07e1652723b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
> > @@ -0,0 +1,65 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/mediatek,mtmips-sysc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MTMIPS SoCs System Controller
> > +
> > +maintainers:
> > +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > +
> > +description: |
> > +  MediaTek MIPS and Ralink SoCs provides a system controller to allow
> > +  to access to system control registers. These registers include clock
> > +  and reset related ones so this node is both clock and reset provider
> > +  for the rest of the world.
> > +
> > +  These SoCs have an XTAL from where the cpu clock is
> > +  provided as well as derived clocks for the bus and the peripherals.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - ralink,mt7620-sysc
>
> Since you decided to send it before we finish discussion:
> NAK - this is already used as mediatek

Sorry, there was too much stuff commented so I preferred to clean up
all of them while maintaining the compatibles with the ralink prefix
instead since that was where the current discussion was at that point.

>
> > +          - ralink,mt7620a-sysc

As I have said, this one exists:

arch/mips/ralink/mt7620.c:      rt_sysc_membase =
plat_of_remap_node("ralink,mt7620a-sysc");


> > +          - ralink,mt7628-sysc
>
> Same here.
>
> > +          - ralink,mt7688-sysc
>
> I expect you to check the others.

I can change others to mediatek but that would be a bit weird, don't you think?

>
>
>
> Best regards,
> Krzysztof
>

Thanks,
    Sergio Paracuellos
Re: [PATCH v2 1/9] dt-bindings: clock: add mtmips SoCs system controller
Posted by Krzysztof Kozlowski 2 years, 10 months ago
On 21/03/2023 08:00, Sergio Paracuellos wrote:
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - ralink,mt7620-sysc
>>
>> Since you decided to send it before we finish discussion:
>> NAK - this is already used as mediatek
> 
> Sorry, there was too much stuff commented so I preferred to clean up
> all of them while maintaining the compatibles with the ralink prefix
> instead since that was where the current discussion was at that point.

You did not even wait for me to send feedback on this, in old thread.

> 
>>
>>> +          - ralink,mt7620a-sysc
> 
> As I have said, this one exists:
> 
> arch/mips/ralink/mt7620.c:      rt_sysc_membase =
> plat_of_remap_node("ralink,mt7620a-sysc");

And why do you ignore others which have mediatek?

> 
> 
>>> +          - ralink,mt7628-sysc
>>
>> Same here.

Same problem.

>>
>>> +          - ralink,mt7688-sysc
>>
>> I expect you to check the others.
> 
> I can change others to mediatek but that would be a bit weird, don't you think?

No, I expect to have mediatek where the model is already used with
mediatek prefix.



Best regards,
Krzysztof
Re: [PATCH v2 1/9] dt-bindings: clock: add mtmips SoCs system controller
Posted by Sergio Paracuellos 2 years, 10 months ago
On Tue, Mar 21, 2023 at 8:16 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 21/03/2023 08:00, Sergio Paracuellos wrote:
> >>> +properties:
> >>> +  compatible:
> >>> +    items:
> >>> +      - enum:
> >>> +          - ralink,mt7620-sysc
> >>
> >> Since you decided to send it before we finish discussion:
> >> NAK - this is already used as mediatek
> >
> > Sorry, there was too much stuff commented so I preferred to clean up
> > all of them while maintaining the compatibles with the ralink prefix
> > instead since that was where the current discussion was at that point.
>
> You did not even wait for me to send feedback on this, in old thread.

My apologies, I thought it was better to send it at that point. Will
wait for feedback from now on before sending anything.

>
> >
> >>
> >>> +          - ralink,mt7620a-sysc
> >
> > As I have said, this one exists:
> >
> > arch/mips/ralink/mt7620.c:      rt_sysc_membase =
> > plat_of_remap_node("ralink,mt7620a-sysc");
>
> And why do you ignore others which have mediatek?
>
> >
> >
> >>> +          - ralink,mt7628-sysc
> >>
> >> Same here.
>
> Same problem.
>
> >>
> >>> +          - ralink,mt7688-sysc
> >>
> >> I expect you to check the others.
> >
> > I can change others to mediatek but that would be a bit weird, don't you think?
>
> No, I expect to have mediatek where the model is already used with
> mediatek prefix.

It is clear now, thanks.

>
>
>
> Best regards,
> Krzysztof
>

Thanks Krzysztof.

Best regards,
    Sergio Paracuellos
Re: [PATCH v2 1/9] dt-bindings: clock: add mtmips SoCs system controller
Posted by Arınç ÜNAL 2 years, 10 months ago
On 21.03.2023 10:00, Sergio Paracuellos wrote:
> On Tue, Mar 21, 2023 at 7:45 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 21/03/2023 06:00, Sergio Paracuellos wrote:
>>> Adds device tree binding documentation for system controller node present
>>> in Mediatek MIPS and Ralink SOCs. This node is a clock and reset provider
>>> for the rest of the world. This covers RT2880, RT3050, RT3052, RT3350,
>>> RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.
>>>
>>> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
>>> ---
>>>   .../bindings/clock/mediatek,mtmips-sysc.yaml  | 65 +++++++++++++++++++
>>>   1 file changed, 65 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
>>> new file mode 100644
>>> index 000000000000..f07e1652723b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
>>> @@ -0,0 +1,65 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/mediatek,mtmips-sysc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MTMIPS SoCs System Controller
>>> +
>>> +maintainers:
>>> +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
>>> +
>>> +description: |
>>> +  MediaTek MIPS and Ralink SoCs provides a system controller to allow
>>> +  to access to system control registers. These registers include clock
>>> +  and reset related ones so this node is both clock and reset provider
>>> +  for the rest of the world.
>>> +
>>> +  These SoCs have an XTAL from where the cpu clock is
>>> +  provided as well as derived clocks for the bus and the peripherals.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - ralink,mt7620-sysc
>>
>> Since you decided to send it before we finish discussion:
>> NAK - this is already used as mediatek
> 
> Sorry, there was too much stuff commented so I preferred to clean up
> all of them while maintaining the compatibles with the ralink prefix
> instead since that was where the current discussion was at that point.
> 
>>
>>> +          - ralink,mt7620a-sysc
> 
> As I have said, this one exists:
> 
> arch/mips/ralink/mt7620.c:      rt_sysc_membase =
> plat_of_remap_node("ralink,mt7620a-sysc");
> 
> 
>>> +          - ralink,mt7628-sysc
>>
>> Same here.
>>
>>> +          - ralink,mt7688-sysc
>>
>> I expect you to check the others.
> 
> I can change others to mediatek but that would be a bit weird, don't you think?

I've seen some parts of the MTMIPS platform use mediatek compatible 
strings thanks to Krzysztof pointing them out. I don't like having some 
parts of the MTMIPS platform (pci, mmc, usbphy, etc.) with mediatek 
compatible string while others are ralink.

Like Krzysztof said [0], Ralink is now Mediatek, thus there is no 
conflict and no issues with different vendor used. So I'd rather keep 
new things Ralink and gradually change these mediatek strings to ralink.

[0] https://patchwork.kernel.org/comment/25232828/

Arınç
Re: [PATCH v2 1/9] dt-bindings: clock: add mtmips SoCs system controller
Posted by Rob Herring 2 years, 10 months ago
On Tue, Mar 21, 2023 at 10:09:59AM +0300, Arınç ÜNAL wrote:
> On 21.03.2023 10:00, Sergio Paracuellos wrote:
> > On Tue, Mar 21, 2023 at 7:45 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > > 
> > > On 21/03/2023 06:00, Sergio Paracuellos wrote:
> > > > Adds device tree binding documentation for system controller node present
> > > > in Mediatek MIPS and Ralink SOCs. This node is a clock and reset provider
> > > > for the rest of the world. This covers RT2880, RT3050, RT3052, RT3350,
> > > > RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.
> > > > 
> > > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > > ---
> > > >   .../bindings/clock/mediatek,mtmips-sysc.yaml  | 65 +++++++++++++++++++
> > > >   1 file changed, 65 insertions(+)
> > > >   create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
> > > > new file mode 100644
> > > > index 000000000000..f07e1652723b
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
> > > > @@ -0,0 +1,65 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/clock/mediatek,mtmips-sysc.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: MTMIPS SoCs System Controller
> > > > +
> > > > +maintainers:
> > > > +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > > +
> > > > +description: |
> > > > +  MediaTek MIPS and Ralink SoCs provides a system controller to allow
> > > > +  to access to system control registers. These registers include clock
> > > > +  and reset related ones so this node is both clock and reset provider
> > > > +  for the rest of the world.
> > > > +
> > > > +  These SoCs have an XTAL from where the cpu clock is
> > > > +  provided as well as derived clocks for the bus and the peripherals.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    items:
> > > > +      - enum:
> > > > +          - ralink,mt7620-sysc
> > > 
> > > Since you decided to send it before we finish discussion:
> > > NAK - this is already used as mediatek
> > 
> > Sorry, there was too much stuff commented so I preferred to clean up
> > all of them while maintaining the compatibles with the ralink prefix
> > instead since that was where the current discussion was at that point.
> > 
> > > 
> > > > +          - ralink,mt7620a-sysc
> > 
> > As I have said, this one exists:
> > 
> > arch/mips/ralink/mt7620.c:      rt_sysc_membase =
> > plat_of_remap_node("ralink,mt7620a-sysc");
> > 
> > 
> > > > +          - ralink,mt7628-sysc
> > > 
> > > Same here.
> > > 
> > > > +          - ralink,mt7688-sysc
> > > 
> > > I expect you to check the others.
> > 
> > I can change others to mediatek but that would be a bit weird, don't you think?
> 
> I've seen some parts of the MTMIPS platform use mediatek compatible strings
> thanks to Krzysztof pointing them out. I don't like having some parts of the
> MTMIPS platform (pci, mmc, usbphy, etc.) with mediatek compatible string
> while others are ralink.

That's unfortunate, but again, compatibles are just unique identifiers. 
They are only wrong if they aren't unique...

> Like Krzysztof said [0], Ralink is now Mediatek, thus there is no conflict
> and no issues with different vendor used. So I'd rather keep new things
> Ralink and gradually change these mediatek strings to ralink.

So break the ABI multiple times slowly. Again, either you live with 
*all* the existing compatible strings or you declare it is fine to break 
the ABI on these platforms and switch everything at once. Carrying both 
strings (in bindings or drivers) and breaking the ABI is lose-lose.

Rob
Re: [PATCH v2 1/9] dt-bindings: clock: add mtmips SoCs system controller
Posted by Arınç ÜNAL 2 years, 10 months ago
On 22.03.2023 01:18, Rob Herring wrote:
> On Tue, Mar 21, 2023 at 10:09:59AM +0300, Arınç ÜNAL wrote:
>> On 21.03.2023 10:00, Sergio Paracuellos wrote:
>>> On Tue, Mar 21, 2023 at 7:45 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 21/03/2023 06:00, Sergio Paracuellos wrote:
>>>>> Adds device tree binding documentation for system controller node present
>>>>> in Mediatek MIPS and Ralink SOCs. This node is a clock and reset provider
>>>>> for the rest of the world. This covers RT2880, RT3050, RT3052, RT3350,
>>>>> RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.
>>>>>
>>>>> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
>>>>> ---
>>>>>    .../bindings/clock/mediatek,mtmips-sysc.yaml  | 65 +++++++++++++++++++
>>>>>    1 file changed, 65 insertions(+)
>>>>>    create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..f07e1652723b
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
>>>>> @@ -0,0 +1,65 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/clock/mediatek,mtmips-sysc.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: MTMIPS SoCs System Controller
>>>>> +
>>>>> +maintainers:
>>>>> +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
>>>>> +
>>>>> +description: |
>>>>> +  MediaTek MIPS and Ralink SoCs provides a system controller to allow
>>>>> +  to access to system control registers. These registers include clock
>>>>> +  and reset related ones so this node is both clock and reset provider
>>>>> +  for the rest of the world.
>>>>> +
>>>>> +  These SoCs have an XTAL from where the cpu clock is
>>>>> +  provided as well as derived clocks for the bus and the peripherals.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    items:
>>>>> +      - enum:
>>>>> +          - ralink,mt7620-sysc
>>>>
>>>> Since you decided to send it before we finish discussion:
>>>> NAK - this is already used as mediatek
>>>
>>> Sorry, there was too much stuff commented so I preferred to clean up
>>> all of them while maintaining the compatibles with the ralink prefix
>>> instead since that was where the current discussion was at that point.
>>>
>>>>
>>>>> +          - ralink,mt7620a-sysc
>>>
>>> As I have said, this one exists:
>>>
>>> arch/mips/ralink/mt7620.c:      rt_sysc_membase =
>>> plat_of_remap_node("ralink,mt7620a-sysc");
>>>
>>>
>>>>> +          - ralink,mt7628-sysc
>>>>
>>>> Same here.
>>>>
>>>>> +          - ralink,mt7688-sysc
>>>>
>>>> I expect you to check the others.
>>>
>>> I can change others to mediatek but that would be a bit weird, don't you think?
>>
>> I've seen some parts of the MTMIPS platform use mediatek compatible strings
>> thanks to Krzysztof pointing them out. I don't like having some parts of the
>> MTMIPS platform (pci, mmc, usbphy, etc.) with mediatek compatible string
>> while others are ralink.
> 
> That's unfortunate, but again, compatibles are just unique identifiers.
> They are only wrong if they aren't unique...

Understood. Sergio, please keep the new strings here ralink.

> 
>> Like Krzysztof said [0], Ralink is now Mediatek, thus there is no conflict
>> and no issues with different vendor used. So I'd rather keep new things
>> Ralink and gradually change these mediatek strings to ralink.
> 
> So break the ABI multiple times slowly. Again, either you live with
> *all* the existing compatible strings or you declare it is fine to break
> the ABI on these platforms and switch everything at once. Carrying both
> strings (in bindings or drivers) and breaking the ABI is lose-lose.

If removing the mediatek strings from the drivers and bindings is better 
than keeping both strings on the drivers except the bindings, which 
would keep the ABI intact, I'll do the prior and do it all at once.

Arınç
Re: [PATCH v2 1/9] dt-bindings: clock: add mtmips SoCs system controller
Posted by Sergio Paracuellos 2 years, 10 months ago
On Wed, Mar 22, 2023 at 9:36 AM Arınç ÜNAL <arinc.unal@arinc9.com> wrote:
>
> On 22.03.2023 01:18, Rob Herring wrote:
> > On Tue, Mar 21, 2023 at 10:09:59AM +0300, Arınç ÜNAL wrote:
> >> On 21.03.2023 10:00, Sergio Paracuellos wrote:
> >>> On Tue, Mar 21, 2023 at 7:45 AM Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>
> >>>> On 21/03/2023 06:00, Sergio Paracuellos wrote:
> >>>>> Adds device tree binding documentation for system controller node present
> >>>>> in Mediatek MIPS and Ralink SOCs. This node is a clock and reset provider
> >>>>> for the rest of the world. This covers RT2880, RT3050, RT3052, RT3350,
> >>>>> RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs.
> >>>>>
> >>>>> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> >>>>> ---
> >>>>>    .../bindings/clock/mediatek,mtmips-sysc.yaml  | 65 +++++++++++++++++++
> >>>>>    1 file changed, 65 insertions(+)
> >>>>>    create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
> >>>>> new file mode 100644
> >>>>> index 000000000000..f07e1652723b
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/clock/mediatek,mtmips-sysc.yaml
> >>>>> @@ -0,0 +1,65 @@
> >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>>> +%YAML 1.2
> >>>>> +---
> >>>>> +$id: http://devicetree.org/schemas/clock/mediatek,mtmips-sysc.yaml#
> >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>> +
> >>>>> +title: MTMIPS SoCs System Controller
> >>>>> +
> >>>>> +maintainers:
> >>>>> +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> >>>>> +
> >>>>> +description: |
> >>>>> +  MediaTek MIPS and Ralink SoCs provides a system controller to allow
> >>>>> +  to access to system control registers. These registers include clock
> >>>>> +  and reset related ones so this node is both clock and reset provider
> >>>>> +  for the rest of the world.
> >>>>> +
> >>>>> +  These SoCs have an XTAL from where the cpu clock is
> >>>>> +  provided as well as derived clocks for the bus and the peripherals.
> >>>>> +
> >>>>> +properties:
> >>>>> +  compatible:
> >>>>> +    items:
> >>>>> +      - enum:
> >>>>> +          - ralink,mt7620-sysc
> >>>>
> >>>> Since you decided to send it before we finish discussion:
> >>>> NAK - this is already used as mediatek
> >>>
> >>> Sorry, there was too much stuff commented so I preferred to clean up
> >>> all of them while maintaining the compatibles with the ralink prefix
> >>> instead since that was where the current discussion was at that point.
> >>>
> >>>>
> >>>>> +          - ralink,mt7620a-sysc
> >>>
> >>> As I have said, this one exists:
> >>>
> >>> arch/mips/ralink/mt7620.c:      rt_sysc_membase =
> >>> plat_of_remap_node("ralink,mt7620a-sysc");
> >>>
> >>>
> >>>>> +          - ralink,mt7628-sysc
> >>>>
> >>>> Same here.
> >>>>
> >>>>> +          - ralink,mt7688-sysc
> >>>>
> >>>> I expect you to check the others.
> >>>
> >>> I can change others to mediatek but that would be a bit weird, don't you think?
> >>
> >> I've seen some parts of the MTMIPS platform use mediatek compatible strings
> >> thanks to Krzysztof pointing them out. I don't like having some parts of the
> >> MTMIPS platform (pci, mmc, usbphy, etc.) with mediatek compatible string
> >> while others are ralink.
> >
> > That's unfortunate, but again, compatibles are just unique identifiers.
> > They are only wrong if they aren't unique...
>
> Understood. Sergio, please keep the new strings here ralink.

So if I have to maintain this as "ralink" I think this patch is ok as
it is now? If that is the case, I prefer to get Reviewed-by for this
patch as it is now by Krzysztof or Rob before changing anything in my
current code.

>
> >
> >> Like Krzysztof said [0], Ralink is now Mediatek, thus there is no conflict
> >> and no issues with different vendor used. So I'd rather keep new things
> >> Ralink and gradually change these mediatek strings to ralink.
> >
> > So break the ABI multiple times slowly. Again, either you live with
> > *all* the existing compatible strings or you declare it is fine to break
> > the ABI on these platforms and switch everything at once. Carrying both
> > strings (in bindings or drivers) and breaking the ABI is lose-lose.
>
> If removing the mediatek strings from the drivers and bindings is better
> than keeping both strings on the drivers except the bindings, which
> would keep the ABI intact, I'll do the prior and do it all at once.
>
> Arınç

Thanks,
    Sergio Paracuellos