2024-01-23 08:22:27

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 2/3] dt-bindings: arm: mediatek: convert PCIESYS to the json-schema

From: Rafał Miłecki <[email protected]>

This helps validating DTS files. Introduced changes:
1. Documented "reg" property
2. Adjusted "reg" in example

Signed-off-by: Rafał Miłecki <[email protected]>
---
.../arm/mediatek/mediatek,mt7622-pciesys.yaml | 47 +++++++++++++++++++
.../arm/mediatek/mediatek,pciesys.txt | 25 ----------
2 files changed, 47 insertions(+), 25 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
delete mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,pciesys.txt

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
new file mode 100644
index 000000000000..7340a2512402
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/mediatek/mediatek,mt7622-pciesys.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek PCIESYS controller
+
+description:
+ The MediaTek PCIESYS controller provides various clocks to the system.
+
+maintainers:
+ - Matthias Brugger <[email protected]>
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - mediatek,mt7622-pciesys
+ - mediatek,mt7629-pciesys
+ - const: syscon
+
+ reg:
+ maxItems: 1
+
+ "#clock-cells":
+ const: 1
+ description: The available clocks are defined in dt-bindings/clock/mt*-clk.h
+
+ "#reset-cells":
+ const: 1
+
+required:
+ - reg
+ - "#clock-cells"
+ - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ pciesys@1a100800 {
+ compatible = "mediatek,mt7622-pciesys", "syscon";
+ reg = <0x1a100800 0x1000>;
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ };
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,pciesys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,pciesys.txt
deleted file mode 100644
index d179a61536f4..000000000000
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,pciesys.txt
+++ /dev/null
@@ -1,25 +0,0 @@
-MediaTek PCIESYS controller
-============================
-
-The MediaTek PCIESYS controller provides various clocks to the system.
-
-Required Properties:
-
-- compatible: Should be:
- - "mediatek,mt7622-pciesys", "syscon"
- - "mediatek,mt7629-pciesys", "syscon"
-- #clock-cells: Must be 1
-- #reset-cells: Must be 1
-
-The PCIESYS controller uses the common clk binding from
-Documentation/devicetree/bindings/clock/clock-bindings.txt
-The available clocks are defined in dt-bindings/clock/mt*-clk.h.
-
-Example:
-
-pciesys: pciesys@1a100800 {
- compatible = "mediatek,mt7622-pciesys", "syscon";
- reg = <0 0x1a100800 0 0x1000>;
- #clock-cells = <1>;
- #reset-cells = <1>;
-};
--
2.35.3



2024-01-30 20:34:25

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: arm: mediatek: convert PCIESYS to the json-schema

On Tue, Jan 23, 2024 at 12:20:29PM +0100, AngeloGioacchino Del Regno wrote:
> Il 23/01/24 09:20, Rafał Miłecki ha scritto:
> > From: Rafał Miłecki <[email protected]>
> >
> > This helps validating DTS files. Introduced changes:
> > 1. Documented "reg" property
> > 2. Adjusted "reg" in example
> >
> > Signed-off-by: Rafał Miłecki <[email protected]>
> > ---
> > .../arm/mediatek/mediatek,mt7622-pciesys.yaml | 47 +++++++++++++++++++
> > .../arm/mediatek/mediatek,pciesys.txt | 25 ----------
> > 2 files changed, 47 insertions(+), 25 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
> > delete mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,pciesys.txt
> >
> > diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
> > new file mode 100644
> > index 000000000000..7340a2512402
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
>
> I think that we should really move all those clock controller yaml files to their
> proper directory, which would be
>
> Documentation/devicetree/bindings/clock/
>
> ...because those are clock controllers anyway and the fact that they do also
> provide a reset controller doesn't really justify having them in arm/mediatek.
>
> Besides, I would appreciate if you could also move mt8186/92/95 and eventual
> others that are there to clock/.

Yes, please move it.

>
> > @@ -0,0 +1,47 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/arm/mediatek/mediatek,mt7622-pciesys.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek PCIESYS controller
> > +
> > +description:
> > + The MediaTek PCIESYS controller provides various clocks to the system.
> > +
> > +maintainers:
> > + - Matthias Brugger <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - mediatek,mt7622-pciesys
> > + - mediatek,mt7629-pciesys
> > + - const: syscon
>
> I know that there's syscon all over the place and, even if I admit I didn't check,
> I am fairly sure that there's absolutely no reason to have syscon there, and that
> the syscon compatible never did anything for (most of, or all of) those clock
> controllers, at all.
>
> I'm not sure - though - if removing syscon during the txt->yaml conversion is
> acceptable (yeah we'd be cheating a bit), but something makes me say it is, because
> the bindings couldn't validate before that one as well.

As long as you state why you are removing it in the commit msg.

>
> Of course you'd have to remove the syscon compatible from the affected device trees
> as well as omitting it here.

You could also do 'minItems: 1' and 'deprecated' in the 2nd item.

> However, to be sure that we're doing the right thing here, I have to summon someone
> that can actually give a definitive answer to what I just said.....
>
> Krzysztof, please? :-)

2024-01-30 21:58:03

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: arm: mediatek: convert PCIESYS to the json-schema

On 30.01.2024 21:34, Rob Herring wrote:
> On Tue, Jan 23, 2024 at 12:20:29PM +0100, AngeloGioacchino Del Regno wrote:
>> Il 23/01/24 09:20, Rafał Miłecki ha scritto:
>>> From: Rafał Miłecki <[email protected]>
>>>
>>> This helps validating DTS files. Introduced changes:
>>> 1. Documented "reg" property
>>> 2. Adjusted "reg" in example
>>>
>>> Signed-off-by: Rafał Miłecki <[email protected]>
>>> ---
>>> .../arm/mediatek/mediatek,mt7622-pciesys.yaml | 47 +++++++++++++++++++
>>> .../arm/mediatek/mediatek,pciesys.txt | 25 ----------
>>> 2 files changed, 47 insertions(+), 25 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
>>> delete mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,pciesys.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
>>> new file mode 100644
>>> index 000000000000..7340a2512402
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
>>
>> I think that we should really move all those clock controller yaml files to their
>> proper directory, which would be
>>
>> Documentation/devicetree/bindings/clock/
>>
>> ...because those are clock controllers anyway and the fact that they do also
>> provide a reset controller doesn't really justify having them in arm/mediatek.
>>
>> Besides, I would appreciate if you could also move mt8186/92/95 and eventual
>> others that are there to clock/.
>
> Yes, please move it.
>
>>
>>> @@ -0,0 +1,47 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/arm/mediatek/mediatek,mt7622-pciesys.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MediaTek PCIESYS controller
>>> +
>>> +description:
>>> + The MediaTek PCIESYS controller provides various clocks to the system.
>>> +
>>> +maintainers:
>>> + - Matthias Brugger <[email protected]>
>>> +
>>> +properties:
>>> + compatible:
>>> + items:
>>> + - enum:
>>> + - mediatek,mt7622-pciesys
>>> + - mediatek,mt7629-pciesys
>>> + - const: syscon
>>
>> I know that there's syscon all over the place and, even if I admit I didn't check,
>> I am fairly sure that there's absolutely no reason to have syscon there, and that
>> the syscon compatible never did anything for (most of, or all of) those clock
>> controllers, at all.
>>
>> I'm not sure - though - if removing syscon during the txt->yaml conversion is
>> acceptable (yeah we'd be cheating a bit), but something makes me say it is, because
>> the bindings couldn't validate before that one as well.
>
> As long as you state why you are removing it in the commit msg.
>
>>
>> Of course you'd have to remove the syscon compatible from the affected device trees
>> as well as omitting it here.
>
> You could also do 'minItems: 1' and 'deprecated' in the 2nd item.

I expected this to be quite straightforward but I can't get it to work.

Can you tell me if I'm making some mistake or is that some meta schema
limitation?

properties:
compatible:
oneOf:
- minItems: 1
items:
- enum:
- mediatek,mt2701-hifsys
- mediatek,mt7622-hifsys
- const: syscon
deprecated: true
- minItems: 2
items:
- enum:
- mediatek,mt7623-hifsys
- const: mediatek,mt2701-hifsys
- const: syscon
deprecated: true

Gives me:

Documentation/devicetree/bindings/clock/mediatek,mt2701-hifsys.yaml: properties:compatible:oneOf:0:items: 'oneOf' conditional failed, one must be fixed:
[{'enum': ['mediatek,mt2701-hifsys', 'mediatek,mt7622-hifsys']}, {'const': 'syscon', 'deprecated': True}] is not of type 'object'
Additional properties are not allowed ('deprecated' was unexpected)
from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
Documentation/devicetree/bindings/clock/mediatek,mt2701-hifsys.yaml: properties:compatible:oneOf:1:items: 'oneOf' conditional failed, one must be fixed:
[{'enum': ['mediatek,mt7623-hifsys']}, {'const': 'mediatek,mt2701-hifsys'}, {'const': 'syscon', 'deprecated': True}] is not of type 'object'
Additional properties are not allowed ('deprecated' was unexpected)
from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#

Subject: Re: [PATCH 2/3] dt-bindings: arm: mediatek: convert PCIESYS to the json-schema

Il 30/01/24 21:34, Rob Herring ha scritto:
> On Tue, Jan 23, 2024 at 12:20:29PM +0100, AngeloGioacchino Del Regno wrote:
>> Il 23/01/24 09:20, Rafał Miłecki ha scritto:
>>> From: Rafał Miłecki <[email protected]>
>>>
>>> This helps validating DTS files. Introduced changes:
>>> 1. Documented "reg" property
>>> 2. Adjusted "reg" in example
>>>
>>> Signed-off-by: Rafał Miłecki <[email protected]>
>>> ---
>>> .../arm/mediatek/mediatek,mt7622-pciesys.yaml | 47 +++++++++++++++++++
>>> .../arm/mediatek/mediatek,pciesys.txt | 25 ----------
>>> 2 files changed, 47 insertions(+), 25 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
>>> delete mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,pciesys.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
>>> new file mode 100644
>>> index 000000000000..7340a2512402
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7622-pciesys.yaml
>>
>> I think that we should really move all those clock controller yaml files to their
>> proper directory, which would be
>>
>> Documentation/devicetree/bindings/clock/
>>
>> ...because those are clock controllers anyway and the fact that they do also
>> provide a reset controller doesn't really justify having them in arm/mediatek.
>>
>> Besides, I would appreciate if you could also move mt8186/92/95 and eventual
>> others that are there to clock/.
>
> Yes, please move it.
>
>>
>>> @@ -0,0 +1,47 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/arm/mediatek/mediatek,mt7622-pciesys.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MediaTek PCIESYS controller
>>> +
>>> +description:
>>> + The MediaTek PCIESYS controller provides various clocks to the system.
>>> +
>>> +maintainers:
>>> + - Matthias Brugger <[email protected]>
>>> +
>>> +properties:
>>> + compatible:
>>> + items:
>>> + - enum:
>>> + - mediatek,mt7622-pciesys
>>> + - mediatek,mt7629-pciesys
>>> + - const: syscon
>>
>> I know that there's syscon all over the place and, even if I admit I didn't check,
>> I am fairly sure that there's absolutely no reason to have syscon there, and that
>> the syscon compatible never did anything for (most of, or all of) those clock
>> controllers, at all.
>>
>> I'm not sure - though - if removing syscon during the txt->yaml conversion is
>> acceptable (yeah we'd be cheating a bit), but something makes me say it is, because
>> the bindings couldn't validate before that one as well.
>
> As long as you state why you are removing it in the commit msg.
>
>>
>> Of course you'd have to remove the syscon compatible from the affected device trees
>> as well as omitting it here.
>
> You could also do 'minItems: 1' and 'deprecated' in the 2nd item.
>

That too. Yes. But I still really want to see the syscon compatible removed from
the device trees on those nodes because it doesn't make any sense to have it :-)

>> However, to be sure that we're doing the right thing here, I have to summon someone
>> that can actually give a definitive answer to what I just said.....
>>
>> Krzysztof, please? :-)