| Message ID | 20230504060729.689579-3-claudiu.beznea@microchip.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | dt-bindings: clocks: at91: convert to yaml | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 04/05/2023 08:07, Claudiu Beznea wrote: > Convert Atmel PMC documentation to yaml. > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> Thank you for your patch. There is something to discuss/improve. > diff --git a/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml > new file mode 100644 > index 000000000000..c4023c3a85f2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml > @@ -0,0 +1,161 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/clock/atmel,at91rm9200-pmc.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" Drop quotes from both. > + > +title: Atmel Power Management Controller (PMC) > + > +maintainers: > + - Claudiu Beznea <claudiu.beznea@microchip.com> > + > +description: > + The power management controller optimizes power consumption by controlling all > + system and user peripheral clocks. The PMC enables/disables the clock inputs > + to many of the peripherals and to the processor. > + > +properties: > + compatible: > + oneOf: > + - items: > + - const: atmel,at91sam9260-pmc Why this is separate, not part of bottom enum? > + - const: syscon > + - items: > + - enum: > + - atmel,at91sam9g20-pmc > + - enum: > + - atmel,at91sam9260-pmc This should be const. You cannot have here different compatibles. > + - const: syscon > + - items: > + - enum: > + - atmel,at91sam9g15-pmc > + - atmel,at91sam9g25-pmc > + - atmel,at91sam9g35-pmc > + - atmel,at91sam9x25-pmc > + - atmel,at91sam9x35-pmc > + - enum: > + - atmel,at91sam9x5-pmc > + - const: syscon > + - items: > + - enum: > + - atmel,at91sam9g45-pmc > + - atmel,at91sam9n12-pmc > + - atmel,at91sam9rl-pmc > + - atmel,at91rm9200-pmc Order by name? > + - atmel,sama5d4-pmc > + - atmel,sama5d3-pmc > + - atmel,sama5d2-pmc > + - microchip,sam9x60-pmc > + - microchip,sama7g5-pmc > + - const: syscon > + > + reg: > + maxItems: 1 > + > + "#clock-cells": > + const: 2 Explain what the cells are for in description. Having '2' for clock controller is not obvious. > + > + clocks: > + minItems: 2 > + maxItems: 3 > + > + clock-names: > + minItems: 2 > + maxItems: 3 > + > + interrupts: > + maxItems: 1 > + > + atmel,osc-bypass: > + type: boolean > + description: set when a clock signal is directly provided on XIN > + > + Just one blank line. > +required: > + - compatible > + - reg > + - interrupts > + - "#clock-cells" > + - clocks > + - clock-names Keep the same order here as they appear in properties:. > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - microchip,sam9x60-pmc > + - microchip,sama7g5-pmc > + then: > + properties: > + clocks: > + minItems: 3 > + maxItems: 3 > + clock-names: > + items: > + - const: td_slck > + - const: md_slck > + - const: main_xtal > + required: > + - clock-names > + - clocks Drop required: here. It's already in top-level. Same in places below. > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - atmel,at91rm9200-pmc > + - atmel,at91sam9260-pmc > + - atmel,at91sam9g20-pmc > + then: > + properties: > + clocks: > + minItems: 2 > + maxItems: 2 > + clock-names: > + items: > + - const: slow_xtal > + - const: main_xtal > + required: > + - clock-names > + - clocks > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - atmel,sama5d4-pmc > + - atmel,sama5d3-pmc > + - atmel,sama5d2-pmc > + then: > + properties: > + clocks: > + minItems: 2 > + maxItems: 2 > + clock-names: > + items: > + - const: slow_clk > + - const: main_xtal > + required: > + - clock-names > + - clocks > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + > + pmc: clock-controller@f0018000 { > + compatible = "atmel,sama5d4-pmc", "syscon"; > + reg = <0xf0018000 0x120>; > + interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>; interrupt looks a bit odd. Are you sure it is correct? > + #clock-cells = <2>; > + clocks = <&clk32k>, <&main_xtal>; > + clock-names = "slow_clk", "main_xtal"; > + }; > + > +... Best regards, Krzysztof
On 04.05.2023 15:47, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 04/05/2023 08:07, Claudiu Beznea wrote: >> Convert Atmel PMC documentation to yaml. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > > Thank you for your patch. There is something to discuss/improve. > > >> diff --git a/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml >> new file mode 100644 >> index 000000000000..c4023c3a85f2 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml >> @@ -0,0 +1,161 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: "http://devicetree.org/schemas/clock/atmel,at91rm9200-pmc.yaml#" >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > Drop quotes from both. > >> + >> +title: Atmel Power Management Controller (PMC) >> + >> +maintainers: >> + - Claudiu Beznea <claudiu.beznea@microchip.com> >> + >> +description: >> + The power management controller optimizes power consumption by controlling all >> + system and user peripheral clocks. The PMC enables/disables the clock inputs >> + to many of the peripherals and to the processor. >> + >> +properties: >> + compatible: >> + oneOf: >> + - items: >> + - const: atmel,at91sam9260-pmc > > Why this is separate, not part of bottom enum? Current device trees uses the following compatibles (among others): - "atmel,at91sam9260-pmc, "syscon" or - "atmel,at91sam9g20-pmc", "atmel,at91sam9260-pmc, "syscon" I haven't found another way to make dtbs_check happy. Is there another way for this? > >> + - const: syscon > >> + - items: >> + - enum: >> + - atmel,at91sam9g20-pmc >> + - enum: >> + - atmel,at91sam9260-pmc > > This should be const. You cannot have here different compatibles. > >> + - const: syscon >> + - items: >> + - enum: >> + - atmel,at91sam9g15-pmc >> + - atmel,at91sam9g25-pmc >> + - atmel,at91sam9g35-pmc >> + - atmel,at91sam9x25-pmc >> + - atmel,at91sam9x35-pmc >> + - enum: >> + - atmel,at91sam9x5-pmc >> + - const: syscon >> + - items: >> + - enum: >> + - atmel,at91sam9g45-pmc >> + - atmel,at91sam9n12-pmc >> + - atmel,at91sam9rl-pmc >> + - atmel,at91rm9200-pmc > > Order by name? > >> + - atmel,sama5d4-pmc >> + - atmel,sama5d3-pmc >> + - atmel,sama5d2-pmc >> + - microchip,sam9x60-pmc >> + - microchip,sama7g5-pmc >> + - const: syscon >> + >> + reg: >> + maxItems: 1 >> + >> + "#clock-cells": >> + const: 2 > > Explain what the cells are for in description. Having '2' for clock > controller is not obvious. > >> + >> + clocks: >> + minItems: 2 >> + maxItems: 3 >> + >> + clock-names: >> + minItems: 2 >> + maxItems: 3 >> + >> + interrupts: >> + maxItems: 1 >> + >> + atmel,osc-bypass: >> + type: boolean >> + description: set when a clock signal is directly provided on XIN >> + >> + > > Just one blank line. > >> +required: >> + - compatible >> + - reg >> + - interrupts >> + - "#clock-cells" >> + - clocks >> + - clock-names > > Keep the same order here as they appear in properties:. > > >> + >> +allOf: >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - microchip,sam9x60-pmc >> + - microchip,sama7g5-pmc >> + then: >> + properties: >> + clocks: >> + minItems: 3 >> + maxItems: 3 >> + clock-names: >> + items: >> + - const: td_slck >> + - const: md_slck >> + - const: main_xtal >> + required: >> + - clock-names >> + - clocks > > Drop required: here. It's already in top-level. Same in places below. > >> + >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - atmel,at91rm9200-pmc >> + - atmel,at91sam9260-pmc >> + - atmel,at91sam9g20-pmc >> + then: >> + properties: >> + clocks: >> + minItems: 2 >> + maxItems: 2 >> + clock-names: >> + items: >> + - const: slow_xtal >> + - const: main_xtal >> + required: >> + - clock-names >> + - clocks >> + >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - atmel,sama5d4-pmc >> + - atmel,sama5d3-pmc >> + - atmel,sama5d2-pmc >> + then: >> + properties: >> + clocks: >> + minItems: 2 >> + maxItems: 2 >> + clock-names: >> + items: >> + - const: slow_clk >> + - const: main_xtal >> + required: >> + - clock-names >> + - clocks >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/interrupt-controller/irq.h> >> + >> + pmc: clock-controller@f0018000 { >> + compatible = "atmel,sama5d4-pmc", "syscon"; >> + reg = <0xf0018000 0x120>; >> + interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>; > > interrupt looks a bit odd. Are you sure it is correct? This example is from SAMA5D4 SoC which uses a vendor specific interrupt controller (Atmel AIC) where: - 1st cell is the interrupt number - 2nd cell is the interrupt type (level/edge sensitive) - 3rd cell is the IRQ priority Thank you, Claudiu > >> + #clock-cells = <2>; >> + clocks = <&clk32k>, <&main_xtal>; >> + clock-names = "slow_clk", "main_xtal"; >> + }; >> + >> +... > > Best regards, > Krzysztof >
On 05/05/2023 14:46, Claudiu.Beznea@microchip.com wrote: > On 04.05.2023 15:47, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On 04/05/2023 08:07, Claudiu Beznea wrote: >>> Convert Atmel PMC documentation to yaml. >>> >>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> >> >> Thank you for your patch. There is something to discuss/improve. >> >> >>> diff --git a/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml >>> new file mode 100644 >>> index 000000000000..c4023c3a85f2 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml >>> @@ -0,0 +1,161 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: "http://devicetree.org/schemas/clock/atmel,at91rm9200-pmc.yaml#" >>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >> >> Drop quotes from both. >> >>> + >>> +title: Atmel Power Management Controller (PMC) >>> + >>> +maintainers: >>> + - Claudiu Beznea <claudiu.beznea@microchip.com> >>> + >>> +description: >>> + The power management controller optimizes power consumption by controlling all >>> + system and user peripheral clocks. The PMC enables/disables the clock inputs >>> + to many of the peripherals and to the processor. >>> + >>> +properties: >>> + compatible: >>> + oneOf: >>> + - items: >>> + - const: atmel,at91sam9260-pmc >> >> Why this is separate, not part of bottom enum? > > Current device trees uses the following compatibles (among others): > - "atmel,at91sam9260-pmc, "syscon" or > - "atmel,at91sam9g20-pmc", "atmel,at91sam9260-pmc, "syscon" > > I haven't found another way to make dtbs_check happy. > Is there another way for this? I mean the enum at the bottom of all compatibles. >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + #include <dt-bindings/interrupt-controller/irq.h> >>> + >>> + pmc: clock-controller@f0018000 { >>> + compatible = "atmel,sama5d4-pmc", "syscon"; >>> + reg = <0xf0018000 0x120>; >>> + interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>; >> >> interrupt looks a bit odd. Are you sure it is correct? > > This example is from SAMA5D4 SoC which uses a vendor specific interrupt > controller (Atmel AIC) where: > - 1st cell is the interrupt number > - 2nd cell is the interrupt type (level/edge sensitive) > - 3rd cell is the IRQ priority ok Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt b/Documentation/devicetree/bindings/clock/at91-clock.txt index 13f45db3b66d..57394785d3b0 100644 --- a/Documentation/devicetree/bindings/clock/at91-clock.txt +++ b/Documentation/devicetree/bindings/clock/at91-clock.txt @@ -28,31 +28,3 @@ For example: #clock-cells = <0>; }; -Power Management Controller (PMC): - -Required properties: -- compatible : shall be "atmel,<chip>-pmc", "syscon" or - "microchip,sam9x60-pmc" - <chip> can be: at91rm9200, at91sam9260, at91sam9261, - at91sam9263, at91sam9g45, at91sam9n12, at91sam9rl, at91sam9g15, - at91sam9g25, at91sam9g35, at91sam9x25, at91sam9x35, at91sam9x5, - sama5d2, sama5d3 or sama5d4. -- #clock-cells : from common clock binding; shall be set to 2. The first entry - is the type of the clock (core, system, peripheral or generated) and the - second entry its index as provided by the datasheet -- clocks : Must contain an entry for each entry in clock-names. -- clock-names: Must include the following entries: "slow_clk", "main_xtal" - -Optional properties: -- atmel,osc-bypass : boolean property. Set this when a clock signal is directly - provided on XIN. - -For example: - pmc: pmc@f0018000 { - compatible = "atmel,sama5d4-pmc", "syscon"; - reg = <0xf0018000 0x120>; - interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>; - #clock-cells = <2>; - clocks = <&clk32k>, <&main_xtal>; - clock-names = "slow_clk", "main_xtal"; - }; diff --git a/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml new file mode 100644 index 000000000000..c4023c3a85f2 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml @@ -0,0 +1,161 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/clock/atmel,at91rm9200-pmc.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Atmel Power Management Controller (PMC) + +maintainers: + - Claudiu Beznea <claudiu.beznea@microchip.com> + +description: + The power management controller optimizes power consumption by controlling all + system and user peripheral clocks. The PMC enables/disables the clock inputs + to many of the peripherals and to the processor. + +properties: + compatible: + oneOf: + - items: + - const: atmel,at91sam9260-pmc + - const: syscon + - items: + - enum: + - atmel,at91sam9g20-pmc + - enum: + - atmel,at91sam9260-pmc + - const: syscon + - items: + - enum: + - atmel,at91sam9g15-pmc + - atmel,at91sam9g25-pmc + - atmel,at91sam9g35-pmc + - atmel,at91sam9x25-pmc + - atmel,at91sam9x35-pmc + - enum: + - atmel,at91sam9x5-pmc + - const: syscon + - items: + - enum: + - atmel,at91sam9g45-pmc + - atmel,at91sam9n12-pmc + - atmel,at91sam9rl-pmc + - atmel,at91rm9200-pmc + - atmel,sama5d4-pmc + - atmel,sama5d3-pmc + - atmel,sama5d2-pmc + - microchip,sam9x60-pmc + - microchip,sama7g5-pmc + - const: syscon + + reg: + maxItems: 1 + + "#clock-cells": + const: 2 + + clocks: + minItems: 2 + maxItems: 3 + + clock-names: + minItems: 2 + maxItems: 3 + + interrupts: + maxItems: 1 + + atmel,osc-bypass: + type: boolean + description: set when a clock signal is directly provided on XIN + + +required: + - compatible + - reg + - interrupts + - "#clock-cells" + - clocks + - clock-names + +allOf: + - if: + properties: + compatible: + contains: + enum: + - microchip,sam9x60-pmc + - microchip,sama7g5-pmc + then: + properties: + clocks: + minItems: 3 + maxItems: 3 + clock-names: + items: + - const: td_slck + - const: md_slck + - const: main_xtal + required: + - clock-names + - clocks + + - if: + properties: + compatible: + contains: + enum: + - atmel,at91rm9200-pmc + - atmel,at91sam9260-pmc + - atmel,at91sam9g20-pmc + then: + properties: + clocks: + minItems: 2 + maxItems: 2 + clock-names: + items: + - const: slow_xtal + - const: main_xtal + required: + - clock-names + - clocks + + - if: + properties: + compatible: + contains: + enum: + - atmel,sama5d4-pmc + - atmel,sama5d3-pmc + - atmel,sama5d2-pmc + then: + properties: + clocks: + minItems: 2 + maxItems: 2 + clock-names: + items: + - const: slow_clk + - const: main_xtal + required: + - clock-names + - clocks + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + pmc: clock-controller@f0018000 { + compatible = "atmel,sama5d4-pmc", "syscon"; + reg = <0xf0018000 0x120>; + interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>; + #clock-cells = <2>; + clocks = <&clk32k>, <&main_xtal>; + clock-names = "slow_clk", "main_xtal"; + }; + +...
Convert Atmel PMC documentation to yaml. Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> --- .../devicetree/bindings/clock/at91-clock.txt | 28 --- .../bindings/clock/atmel,at91rm9200-pmc.yaml | 161 ++++++++++++++++++ 2 files changed, 161 insertions(+), 28 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml