Re: [PATCH 2/5] dt-bindings: clocks: atmel,at91rm9200-pmc: convert to yaml
From: Krzysztof Kozlowski <hidden>
Date: 2023-05-04 12:47:59
Also in:
linux-clk, linux-devicetree, lkml
On 04/05/2023 08:07, Claudiu Beznea wrote:
Convert Atmel PMC documentation to yaml. Signed-off-by: Claudiu Beznea <redacted>
Thank you for your patch. There is something to discuss/improve.
quoted hunk ↗ jump to hunk
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 [off-list ref] + +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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel