[PATCH v3 0/5] dt-bindings: clocks: at91: convert to yaml

[PATCH v3 0/5] dt-bindings: clocks: at91: convert to yaml

[Claudiu Beznea]

Hi,

This series converts atmel clocks bindings (PMC and slow clock
controller) to YAML. Along with it updated device trees to cope
with the dt-binding requirements.

Thank you,
Claudiu Beznea

Changes in v3:
- in patch 2/5:
	- get rid of 1st "items" section and embedd it in the last compatible
	  enum
	- sort alphanumerically the compatibles in allOf
- collected tags

Changes in v2:
- in patch 2/5:
	- dropped quotes from $id and $schema
	- get rid of 1st "items" sections corresponding to "atmel,at91sam9260-pmc",
	  "syscon" compatible and move it to the proper enum
	- ordered compatibles by name
	- add description for #clock-cells
	- remove blank lines
	- keep order in required (same order that the properties were
	  defined)
	- dropped required from allOf

- in patch 5/5:
	- dropped quotes from $id and $schema
	- drop first "items:" in compatible:oneOf section
	- ordered compatibles by name
	- moved additionalProperties after allOf
	- dropped microchip,sama7g5-sckc from first allOf:if section
	- moved "required" section from allOf to global "required" section
	- dropped if:then from the last if:then:else in allOf

Claudiu Beznea (5):
  ARM: dts: at91: use clock-controller name for PMC nodes
  dt-bindings: clocks: atmel,at91rm9200-pmc: convert to yaml
  ARM: dts: at91: at91sam9n12: witch sckc to new clock bindings
  ARM: dts: at91: use clock-controller name for sckc nodes
  dt-bindings: clocks: at91sam9x5-sckc: convert to yaml

 .../devicetree/bindings/clock/at91-clock.txt  |  58 -------
 .../bindings/clock/atmel,at91rm9200-pmc.yaml  | 152 ++++++++++++++++++
 .../bindings/clock/atmel,at91sam9x5-sckc.yaml |  70 ++++++++
 arch/arm/boot/dts/at91rm9200.dtsi             |   2 +-
 arch/arm/boot/dts/at91sam9260.dtsi            |   2 +-
 arch/arm/boot/dts/at91sam9261.dtsi            |   2 +-
 arch/arm/boot/dts/at91sam9263.dtsi            |   2 +-
 arch/arm/boot/dts/at91sam9g20.dtsi            |   2 +-
 arch/arm/boot/dts/at91sam9g25.dtsi            |   2 +-
 arch/arm/boot/dts/at91sam9g35.dtsi            |   2 +-
 arch/arm/boot/dts/at91sam9g45.dtsi            |   4 +-
 arch/arm/boot/dts/at91sam9n12.dtsi            |  25 +--
 arch/arm/boot/dts/at91sam9rl.dtsi             |   4 +-
 arch/arm/boot/dts/at91sam9x25.dtsi            |   2 +-
 arch/arm/boot/dts/at91sam9x35.dtsi            |   2 +-
 arch/arm/boot/dts/at91sam9x5.dtsi             |   4 +-
 arch/arm/boot/dts/sam9x60.dtsi                |   4 +-
 arch/arm/boot/dts/sama5d2.dtsi                |   4 +-
 arch/arm/boot/dts/sama5d3.dtsi                |   4 +-
 arch/arm/boot/dts/sama5d3_emac.dtsi           |   2 +-
 arch/arm/boot/dts/sama5d4.dtsi                |   4 +-
 arch/arm/boot/dts/sama7g5.dtsi                |   2 +-
 22 files changed, 251 insertions(+), 104 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/at91-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/atmel,at91sam9x5-sckc.yaml

-- 
2.34.1

[PATCH v3 1/5] ARM: dts: at91: use clock-controller name for PMC nodes

[Claudiu Beznea]

Use clock-controller generic name for PMC nodes.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 arch/arm/boot/dts/at91rm9200.dtsi   | 2 +-
 arch/arm/boot/dts/at91sam9260.dtsi  | 2 +-
 arch/arm/boot/dts/at91sam9261.dtsi  | 2 +-
 arch/arm/boot/dts/at91sam9263.dtsi  | 2 +-
 arch/arm/boot/dts/at91sam9g20.dtsi  | 2 +-
 arch/arm/boot/dts/at91sam9g25.dtsi  | 2 +-
 arch/arm/boot/dts/at91sam9g35.dtsi  | 2 +-
 arch/arm/boot/dts/at91sam9g45.dtsi  | 2 +-
 arch/arm/boot/dts/at91sam9n12.dtsi  | 2 +-
 arch/arm/boot/dts/at91sam9rl.dtsi   | 2 +-
 arch/arm/boot/dts/at91sam9x25.dtsi  | 2 +-
 arch/arm/boot/dts/at91sam9x35.dtsi  | 2 +-
 arch/arm/boot/dts/at91sam9x5.dtsi   | 2 +-
 arch/arm/boot/dts/sam9x60.dtsi      | 2 +-
 arch/arm/boot/dts/sama5d2.dtsi      | 2 +-
 arch/arm/boot/dts/sama5d3.dtsi      | 2 +-
 arch/arm/boot/dts/sama5d3_emac.dtsi | 2 +-
 arch/arm/boot/dts/sama5d4.dtsi      | 2 +-
 arch/arm/boot/dts/sama7g5.dtsi      | 2 +-
 19 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/arch/arm/boot/dts/at91rm9200.dtsi b/arch/arm/boot/dts/at91rm9200.dtsi
index 6f9004ebf424..37b500f6f395 100644
--- a/arch/arm/boot/dts/at91rm9200.dtsi
+++ b/arch/arm/boot/dts/at91rm9200.dtsi
@@ -102,7 +102,7 @@ ramc0: ramc@ffffff00 {
 				reg = <0xffffff00 0x100>;
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,at91rm9200-pmc", "syscon";
 				reg = <0xfffffc00 0x100>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
diff --git a/arch/arm/boot/dts/at91sam9260.dtsi b/arch/arm/boot/dts/at91sam9260.dtsi
index 789fe356dbf6..16e3b24b4ddd 100644
--- a/arch/arm/boot/dts/at91sam9260.dtsi
+++ b/arch/arm/boot/dts/at91sam9260.dtsi
@@ -115,7 +115,7 @@ matrix: matrix@ffffee00 {
 				reg = <0xffffee00 0x200>;
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,at91sam9260-pmc", "syscon";
 				reg = <0xfffffc00 0x100>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
diff --git a/arch/arm/boot/dts/at91sam9261.dtsi b/arch/arm/boot/dts/at91sam9261.dtsi
index ee0bd1aceb3f..fe9ead867e2a 100644
--- a/arch/arm/boot/dts/at91sam9261.dtsi
+++ b/arch/arm/boot/dts/at91sam9261.dtsi
@@ -599,7 +599,7 @@ pioC: gpio@fffff800 {
 				};
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,at91sam9261-pmc", "syscon";
 				reg = <0xfffffc00 0x100>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
diff --git a/arch/arm/boot/dts/at91sam9263.dtsi b/arch/arm/boot/dts/at91sam9263.dtsi
index 3ce9ea987312..ee5e6ed44dd4 100644
--- a/arch/arm/boot/dts/at91sam9263.dtsi
+++ b/arch/arm/boot/dts/at91sam9263.dtsi
@@ -101,7 +101,7 @@ aic: interrupt-controller@fffff000 {
 				atmel,external-irqs = <30 31>;
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,at91sam9263-pmc", "syscon";
 				reg = <0xfffffc00 0x100>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
index 708e1646b7f4..738a43ffd228 100644
--- a/arch/arm/boot/dts/at91sam9g20.dtsi
+++ b/arch/arm/boot/dts/at91sam9g20.dtsi
@@ -41,7 +41,7 @@ adc0: adc@fffe0000 {
 				atmel,adc-startup-time = <40>;
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,at91sam9g20-pmc", "atmel,at91sam9260-pmc", "syscon";
 			};
 		};
diff --git a/arch/arm/boot/dts/at91sam9g25.dtsi b/arch/arm/boot/dts/at91sam9g25.dtsi
index d2f13afb35ea..ec3c77221881 100644
--- a/arch/arm/boot/dts/at91sam9g25.dtsi
+++ b/arch/arm/boot/dts/at91sam9g25.dtsi
@@ -26,7 +26,7 @@ pinctrl@fffff400 {
 				      >;
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,at91sam9g25-pmc", "atmel,at91sam9x5-pmc", "syscon";
 			};
 		};
diff --git a/arch/arm/boot/dts/at91sam9g35.dtsi b/arch/arm/boot/dts/at91sam9g35.dtsi
index 48c2bc4a7753..c9cfb93092ee 100644
--- a/arch/arm/boot/dts/at91sam9g35.dtsi
+++ b/arch/arm/boot/dts/at91sam9g35.dtsi
@@ -25,7 +25,7 @@ pinctrl@fffff400 {
 				      >;
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,at91sam9g35-pmc", "atmel,at91sam9x5-pmc", "syscon";
 			};
 		};
diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index 95f5d76234db..76afeb31b7f5 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -129,7 +129,7 @@ matrix: matrix@ffffea00 {
 				reg = <0xffffea00 0x200>;
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,at91sam9g45-pmc", "syscon";
 				reg = <0xfffffc00 0x100>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
diff --git a/arch/arm/boot/dts/at91sam9n12.dtsi b/arch/arm/boot/dts/at91sam9n12.dtsi
index 83114d26f10d..c2e7460fb7ff 100644
--- a/arch/arm/boot/dts/at91sam9n12.dtsi
+++ b/arch/arm/boot/dts/at91sam9n12.dtsi
@@ -118,7 +118,7 @@ smc: smc@ffffea00 {
 				reg = <0xffffea00 0x200>;
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,at91sam9n12-pmc", "syscon";
 				reg = <0xfffffc00 0x200>;
 				#clock-cells = <2>;
diff --git a/arch/arm/boot/dts/at91sam9rl.dtsi b/arch/arm/boot/dts/at91sam9rl.dtsi
index 364a2ff0a763..a12e6c419fe3 100644
--- a/arch/arm/boot/dts/at91sam9rl.dtsi
+++ b/arch/arm/boot/dts/at91sam9rl.dtsi
@@ -763,7 +763,7 @@ pioD: gpio@fffffa00 {
 				};
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,at91sam9rl-pmc", "syscon";
 				reg = <0xfffffc00 0x100>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
diff --git a/arch/arm/boot/dts/at91sam9x25.dtsi b/arch/arm/boot/dts/at91sam9x25.dtsi
index 0fe8802e1242..7036f5f04571 100644
--- a/arch/arm/boot/dts/at91sam9x25.dtsi
+++ b/arch/arm/boot/dts/at91sam9x25.dtsi
@@ -27,7 +27,7 @@ pinctrl@fffff400 {
 				      >;
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,at91sam9x25-pmc", "atmel,at91sam9x5-pmc", "syscon";
 			};
 		};
diff --git a/arch/arm/boot/dts/at91sam9x35.dtsi b/arch/arm/boot/dts/at91sam9x35.dtsi
index 0bfa21f18f87..eb03b0497e37 100644
--- a/arch/arm/boot/dts/at91sam9x35.dtsi
+++ b/arch/arm/boot/dts/at91sam9x35.dtsi
@@ -26,7 +26,7 @@ pinctrl@fffff400 {
 				      >;
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,at91sam9x35-pmc", "atmel,at91sam9x5-pmc", "syscon";
 			};
 		};
diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
index 0c26c925761b..af19ef2a875c 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -126,7 +126,7 @@ smc: smc@ffffea00 {
 				reg = <0xffffea00 0x200>;
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,at91sam9x5-pmc", "syscon";
 				reg = <0xfffffc00 0x200>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
diff --git a/arch/arm/boot/dts/sam9x60.dtsi b/arch/arm/boot/dts/sam9x60.dtsi
index 8f5477e307dd..6f5177df01bc 100644
--- a/arch/arm/boot/dts/sam9x60.dtsi
+++ b/arch/arm/boot/dts/sam9x60.dtsi
@@ -660,7 +660,7 @@ pioD: gpio@fffffa00 {
 				};
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "microchip,sam9x60-pmc", "syscon";
 				reg = <0xfffffc00 0x200>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index 14c35c12a115..86009dd28e62 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -284,7 +284,7 @@ dma1: dma-controller@f0004000 {
 				clock-names = "dma_clk";
 			};
 
-			pmc: pmc@f0014000 {
+			pmc: clock-controller@f0014000 {
 				compatible = "atmel,sama5d2-pmc", "syscon";
 				reg = <0xf0014000 0x160>;
 				interrupts = <74 IRQ_TYPE_LEVEL_HIGH 7>;
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index bde8e92d60bb..4524a16322d1 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -1001,7 +1001,7 @@ pioE: gpio@fffffa00 {
 				};
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 				compatible = "atmel,sama5d3-pmc", "syscon";
 				reg = <0xfffffc00 0x120>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
diff --git a/arch/arm/boot/dts/sama5d3_emac.dtsi b/arch/arm/boot/dts/sama5d3_emac.dtsi
index 45226108850d..5d7ce13de8cc 100644
--- a/arch/arm/boot/dts/sama5d3_emac.dtsi
+++ b/arch/arm/boot/dts/sama5d3_emac.dtsi
@@ -30,7 +30,7 @@ AT91_PIOC 8 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PC8 periph A EMDC, conflicts with
 				};
 			};
 
-			pmc: pmc@fffffc00 {
+			pmc: clock-controller@fffffc00 {
 			};
 
 			macb1: ethernet@f802c000 {
diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index af62157ae214..e94f3a661f4b 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -250,7 +250,7 @@ dma0: dma-controller@f0014000 {
 				clock-names = "dma_clk";
 			};
 
-			pmc: pmc@f0018000 {
+			pmc: clock-controller@f0018000 {
 				compatible = "atmel,sama5d4-pmc", "syscon";
 				reg = <0xf0018000 0x120>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
diff --git a/arch/arm/boot/dts/sama7g5.dtsi b/arch/arm/boot/dts/sama7g5.dtsi
index ab131762ecb5..f0478a43edc2 100644
--- a/arch/arm/boot/dts/sama7g5.dtsi
+++ b/arch/arm/boot/dts/sama7g5.dtsi
@@ -241,7 +241,7 @@ pioA: pinctrl@e0014000 {
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 11>;
 		};
 
-		pmc: pmc@e0018000 {
+		pmc: clock-controller@e0018000 {
 			compatible = "microchip,sama7g5-pmc", "syscon";
 			reg = <0xe0018000 0x200>;
 			interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.34.1

[PATCH v3 2/5] dt-bindings: clocks: atmel,at91rm9200-pmc: convert to yaml

[Claudiu Beznea]

Convert Atmel PMC documentation to yaml. Along with it clock names
were adapted according to the current available device trees as
different controller versions accept different clocks (some of them
have 3 clocks as input, some has 2 clocks as inputs and some with 2
input clocks uses different clock names).

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 .../devicetree/bindings/clock/at91-clock.txt  |  28 ----
 .../bindings/clock/atmel,at91rm9200-pmc.yaml  | 152 ++++++++++++++++++
 2 files changed, 152 insertions(+), 28 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml

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..aa727dee6729
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml
@@ -0,0 +1,152 @@
+# 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:
+          - enum:
+              - atmel,at91sam9g15-pmc
+              - atmel,at91sam9g20-pmc
+              - atmel,at91sam9g25-pmc
+              - atmel,at91sam9g35-pmc
+              - atmel,at91sam9x25-pmc
+              - atmel,at91sam9x35-pmc
+          - enum:
+              - atmel,at91sam9260-pmc
+              - atmel,at91sam9x5-pmc
+          - const: syscon
+      - items:
+          - enum:
+              - atmel,at91rm9200-pmc
+              - atmel,at91sam9260-pmc
+              - atmel,at91sam9g45-pmc
+              - atmel,at91sam9n12-pmc
+              - atmel,at91sam9rl-pmc
+              - atmel,sama5d2-pmc
+              - atmel,sama5d3-pmc
+              - atmel,sama5d4-pmc
+              - microchip,sam9x60-pmc
+              - microchip,sama7g5-pmc
+          - const: syscon
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  "#clock-cells":
+    description: |
+      - 1st cell is the clock type, one of PMC_TYPE_CORE, PMC_TYPE_SYSTEM,
+        PMC_TYPE_PERIPHERAL, PMC_TYPE_GCK, PMC_TYPE_PROGRAMMABLE (as defined
+        in <dt-bindings/clock/at91.h>)
+      - 2nd cell is the clock identifier as defined in <dt-bindings/clock/at91.h
+        (for core clocks) or as defined in datasheet (for system, peripheral,
+        gck and programmable clocks).
+    const: 2
+
+  clocks:
+    minItems: 2
+    maxItems: 3
+
+  clock-names:
+    minItems: 2
+    maxItems: 3
+
+  atmel,osc-bypass:
+    description: set when a clock signal is directly provided on XIN
+    type: boolean
+
+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
+
+  - 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
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - atmel,sama5d2-pmc
+              - atmel,sama5d3-pmc
+              - atmel,sama5d4-pmc
+    then:
+      properties:
+        clocks:
+          minItems: 2
+          maxItems: 2
+        clock-names:
+          items:
+            - const: slow_clk
+            - const: main_xtal
+
+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";
+    };
+
+...
-- 
2.34.1

[PATCH v3 3/5] ARM: dts: at91: at91sam9n12: witch sckc to new clock bindings

[Claudiu Beznea]

Switch slow clock controller to new clock bindings.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 arch/arm/boot/dts/at91sam9n12.dtsi | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/arch/arm/boot/dts/at91sam9n12.dtsi b/arch/arm/boot/dts/at91sam9n12.dtsi
index c2e7460fb7ff..0e28101b26bf 100644
--- a/arch/arm/boot/dts/at91sam9n12.dtsi
+++ b/arch/arm/boot/dts/at91sam9n12.dtsi
@@ -146,28 +146,11 @@ shdwc@fffffe10 {
 				clocks = <&clk32k>;
 			};
 
-			sckc@fffffe50 {
+			clk32k: clock-controller@fffffe50 {
 				compatible = "atmel,at91sam9x5-sckc";
 				reg = <0xfffffe50 0x4>;
-
-				slow_osc: slow_osc {
-					compatible = "atmel,at91sam9x5-clk-slow-osc";
-					#clock-cells = <0>;
-					clocks = <&slow_xtal>;
-				};
-
-				slow_rc_osc: slow_rc_osc {
-					compatible = "atmel,at91sam9x5-clk-slow-rc-osc";
-					#clock-cells = <0>;
-					clock-frequency = <32768>;
-					clock-accuracy = <50000000>;
-				};
-
-				clk32k: slck {
-					compatible = "atmel,at91sam9x5-clk-slow";
-					#clock-cells = <0>;
-					clocks = <&slow_rc_osc>, <&slow_osc>;
-				};
+				clocks = <&slow_xtal>;
+				#clock-cells = <0>;
 			};
 
 			mmc0: mmc@f0008000 {
-- 
2.34.1

[PATCH v3 4/5] ARM: dts: at91: use clock-controller name for sckc nodes

[Claudiu Beznea]

Use clock-controller generic name for slow clock controller nodes.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 arch/arm/boot/dts/at91sam9g45.dtsi | 2 +-
 arch/arm/boot/dts/at91sam9rl.dtsi  | 2 +-
 arch/arm/boot/dts/at91sam9x5.dtsi  | 2 +-
 arch/arm/boot/dts/sam9x60.dtsi     | 2 +-
 arch/arm/boot/dts/sama5d2.dtsi     | 2 +-
 arch/arm/boot/dts/sama5d3.dtsi     | 2 +-
 arch/arm/boot/dts/sama5d4.dtsi     | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index 76afeb31b7f5..498cb92b29f9 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -923,7 +923,7 @@ usb2: gadget@fff78000 {
 				status = "disabled";
 			};
 
-			clk32k: sckc@fffffd50 {
+			clk32k: clock-controller@fffffd50 {
 				compatible = "atmel,at91sam9x5-sckc";
 				reg = <0xfffffd50 0x4>;
 				clocks = <&slow_xtal>;
diff --git a/arch/arm/boot/dts/at91sam9rl.dtsi b/arch/arm/boot/dts/at91sam9rl.dtsi
index a12e6c419fe3..d7e8a115c916 100644
--- a/arch/arm/boot/dts/at91sam9rl.dtsi
+++ b/arch/arm/boot/dts/at91sam9rl.dtsi
@@ -799,7 +799,7 @@ watchdog@fffffd40 {
 				status = "disabled";
 			};
 
-			clk32k: sckc@fffffd50 {
+			clk32k: clock-controller@fffffd50 {
 				compatible = "atmel,at91sam9x5-sckc";
 				reg = <0xfffffd50 0x4>;
 				clocks = <&slow_xtal>;
diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
index af19ef2a875c..0123ee47151c 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -154,7 +154,7 @@ pit: timer@fffffe30 {
 				clocks = <&pmc PMC_TYPE_CORE PMC_MCK>;
 			};
 
-			clk32k: sckc@fffffe50 {
+			clk32k: clock-controller@fffffe50 {
 				compatible = "atmel,at91sam9x5-sckc";
 				reg = <0xfffffe50 0x4>;
 				clocks = <&slow_xtal>;
diff --git a/arch/arm/boot/dts/sam9x60.dtsi b/arch/arm/boot/dts/sam9x60.dtsi
index 6f5177df01bc..933d73505a8b 100644
--- a/arch/arm/boot/dts/sam9x60.dtsi
+++ b/arch/arm/boot/dts/sam9x60.dtsi
@@ -700,7 +700,7 @@ pit: timer@fffffe40 {
 				clocks = <&pmc PMC_TYPE_CORE PMC_MCK>;
 			};
 
-			clk32k: sckc@fffffe50 {
+			clk32k: clock-controller@fffffe50 {
 				compatible = "microchip,sam9x60-sckc";
 				reg = <0xfffffe50 0x4>;
 				clocks = <&slow_xtal>;
diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index 86009dd28e62..5f632e3f039e 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -704,7 +704,7 @@ watchdog: watchdog@f8048040 {
 				status = "disabled";
 			};
 
-			clk32k: sckc@f8048050 {
+			clk32k: clock-controller@f8048050 {
 				compatible = "atmel,sama5d4-sckc";
 				reg = <0xf8048050 0x4>;
 
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index 4524a16322d1..0eebf6c760b3 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -1040,7 +1040,7 @@ watchdog: watchdog@fffffe40 {
 				status = "disabled";
 			};
 
-			clk32k: sckc@fffffe50 {
+			clk32k: clock-controller@fffffe50 {
 				compatible = "atmel,sama5d3-sckc";
 				reg = <0xfffffe50 0x4>;
 				clocks = <&slow_xtal>;
diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index e94f3a661f4b..de6c82969232 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -761,7 +761,7 @@ watchdog: watchdog@fc068640 {
 				status = "disabled";
 			};
 
-			clk32k: sckc@fc068650 {
+			clk32k: clock-controller@fc068650 {
 				compatible = "atmel,sama5d4-sckc";
 				reg = <0xfc068650 0x4>;
 				#clock-cells = <0>;
-- 
2.34.1

[PATCH v3 5/5] dt-bindings: clocks: at91sam9x5-sckc: convert to yaml

[Claudiu Beznea]

Convert Atmel slow clock controller documentation to yaml.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/clock/at91-clock.txt  | 30 --------
 .../bindings/clock/atmel,at91sam9x5-sckc.yaml | 70 +++++++++++++++++++
 2 files changed, 70 insertions(+), 30 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/at91-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/atmel,at91sam9x5-sckc.yaml

diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt b/Documentation/devicetree/bindings/clock/at91-clock.txt
deleted file mode 100644
index 57394785d3b0..000000000000
--- a/Documentation/devicetree/bindings/clock/at91-clock.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-Device Tree Clock bindings for arch-at91
-
-This binding uses the common clock binding[1].
-
-[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-Slow Clock controller:
-
-Required properties:
-- compatible : shall be one of the following:
-	"atmel,at91sam9x5-sckc",
-	"atmel,sama5d3-sckc",
-	"atmel,sama5d4-sckc" or
-	"microchip,sam9x60-sckc":
-		at91 SCKC (Slow Clock Controller)
-- #clock-cells : shall be 1 for "microchip,sam9x60-sckc" otherwise shall be 0.
-- clocks : shall be the input parent clock phandle for the clock.
-
-Optional properties:
-- atmel,osc-bypass : boolean property. Set this when a clock signal is directly
-  provided on XIN.
-
-For example:
-	sckc@fffffe50 {
-		compatible = "atmel,at91sam9x5-sckc";
-		reg = <0xfffffe50 0x4>;
-		clocks = <&slow_xtal>;
-		#clock-cells = <0>;
-	};
-
diff --git a/Documentation/devicetree/bindings/clock/atmel,at91sam9x5-sckc.yaml b/Documentation/devicetree/bindings/clock/atmel,at91sam9x5-sckc.yaml
new file mode 100644
index 000000000000..7be29877e6d2
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/atmel,at91sam9x5-sckc.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/atmel,at91sam9x5-sckc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel Slow Clock Controller (SCKC)
+
+maintainers:
+  - Claudiu Beznea <claudiu.beznea@microchip.com>
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - atmel,at91sam9x5-sckc
+          - atmel,sama5d3-sckc
+          - atmel,sama5d4-sckc
+          - microchip,sam9x60-sckc
+      - items:
+          - const: microchip,sama7g5-sckc
+          - const: microchip,sam9x60-sckc
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  "#clock-cells":
+    enum: [0, 1]
+
+  atmel,osc-bypass:
+    type: boolean
+    description: set when a clock signal is directly provided on XIN
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - "#clock-cells"
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - microchip,sam9x60-sckc
+    then:
+      properties:
+        "#clock-cells":
+          const: 1
+    else:
+      properties:
+        "#clock-cells":
+          const: 0
+
+additionalProperties: false
+
+examples:
+  - |
+    clk32k: clock-controller@fffffe50 {
+        compatible = "microchip,sam9x60-sckc";
+        reg = <0xfffffe50 0x4>;
+        clocks = <&slow_xtal>;
+        #clock-cells = <1>;
+    };
+
+...
-- 
2.34.1

Re: [PATCH v3 2/5] dt-bindings: clocks: atmel,at91rm9200-pmc: convert to yaml

[Krzysztof Kozlowski]

On 09/05/2023 07:27, Claudiu Beznea wrote:
> Convert Atmel PMC documentation to yaml. Along with it clock names
> were adapted according to the current available device trees as
> different controller versions accept different clocks (some of them
> have 3 clocks as input, some has 2 clocks as inputs and some with 2
> input clocks uses different clock names).
> 

Thank you for your patch. There is something to discuss/improve.

> +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:
> +          - enum:
> +              - atmel,at91sam9g15-pmc
> +              - atmel,at91sam9g20-pmc
> +              - atmel,at91sam9g25-pmc
> +              - atmel,at91sam9g35-pmc
> +              - atmel,at91sam9x25-pmc
> +              - atmel,at91sam9x35-pmc
> +          - enum:
> +              - atmel,at91sam9260-pmc
> +              - atmel,at91sam9x5-pmc

I missed it last time - why you have two enums? We never talked about
this. It's usually wrong... are you sure this is real hardware:
atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
?


> +          - const: syscon
> +      - items:
> +          - enum:
> +              - atmel,at91rm9200-pmc
> +              - atmel,at91sam9260-pmc
> +              - atmel,at91sam9g45-pmc
> +              - atmel,at91sam9n12-pmc
> +              - atmel,at91sam9rl-pmc
> +              - atmel,sama5d2-pmc
> +              - atmel,sama5d3-pmc
> +              - atmel,sama5d4-pmc
> +              - microchip,sam9x60-pmc
> +              - microchip,sama7g5-pmc
> +          - const: syscon
> +


Best regards,
Krzysztof

Re: [PATCH v3 2/5] dt-bindings: clocks: atmel,at91rm9200-pmc: convert to yaml

[Claudiu.Beznea]

On 09.05.2023 09:25, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 09/05/2023 07:27, Claudiu Beznea wrote:
>> Convert Atmel PMC documentation to yaml. Along with it clock names
>> were adapted according to the current available device trees as
>> different controller versions accept different clocks (some of them
>> have 3 clocks as input, some has 2 clocks as inputs and some with 2
>> input clocks uses different clock names).
>>
> 
> Thank you for your patch. There is something to discuss/improve.
> 
>> +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:
>> +          - enum:
>> +              - atmel,at91sam9g15-pmc
>> +              - atmel,at91sam9g20-pmc
>> +              - atmel,at91sam9g25-pmc
>> +              - atmel,at91sam9g35-pmc
>> +              - atmel,at91sam9x25-pmc
>> +              - atmel,at91sam9x35-pmc
>> +          - enum:
>> +              - atmel,at91sam9260-pmc
>> +              - atmel,at91sam9x5-pmc
> 
> I missed it last time - why you have two enums? We never talked about
> this. It's usually wrong... are you sure this is real hardware:
> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
> ?

I have 2 enums because there are some hardware covered by:
"vendor-name,hardware-v1-pmc", "syscon" and some covered by:
"vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon".

Many AT91 device trees compatibles were written in this way. Thus when new
versions of the same IP has been introduced the drivers were not
necessarily updated but the compatibles in device trees were updated e.g.
with "vendor-name,hardware-v2-pmc" (the full compatible becoming
"vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon") and
let the drivers fall back to already in driver supported compatible
"vendor-name,hardware-v1-pmc", "syscon". In general v2 comes with new
features in addition to v1.

That way they AT91 ensures the ABI properties of DT and thus when the
drivers were finally updated with the new features of the
"vendor-name,hardware-v2-pmc" DT remained in place.

Please let me know if these could be handled better in YAML.

Thank you,
Claudiu

> 
> 
>> +          - const: syscon
>> +      - items:
>> +          - enum:
>> +              - atmel,at91rm9200-pmc
>> +              - atmel,at91sam9260-pmc
>> +              - atmel,at91sam9g45-pmc
>> +              - atmel,at91sam9n12-pmc
>> +              - atmel,at91sam9rl-pmc
>> +              - atmel,sama5d2-pmc
>> +              - atmel,sama5d3-pmc
>> +              - atmel,sama5d4-pmc
>> +              - microchip,sam9x60-pmc
>> +              - microchip,sama7g5-pmc
>> +          - const: syscon
>> +
> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v3 2/5] dt-bindings: clocks: atmel,at91rm9200-pmc: convert to yaml

[Krzysztof Kozlowski]

On 10/05/2023 09:00, Claudiu.Beznea@microchip.com wrote:
> On 09.05.2023 09:25, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 09/05/2023 07:27, Claudiu Beznea wrote:
>>> Convert Atmel PMC documentation to yaml. Along with it clock names
>>> were adapted according to the current available device trees as
>>> different controller versions accept different clocks (some of them
>>> have 3 clocks as input, some has 2 clocks as inputs and some with 2
>>> input clocks uses different clock names).
>>>
>>
>> Thank you for your patch. There is something to discuss/improve.
>>
>>> +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:
>>> +          - enum:
>>> +              - atmel,at91sam9g15-pmc
>>> +              - atmel,at91sam9g20-pmc
>>> +              - atmel,at91sam9g25-pmc
>>> +              - atmel,at91sam9g35-pmc
>>> +              - atmel,at91sam9x25-pmc
>>> +              - atmel,at91sam9x35-pmc
>>> +          - enum:
>>> +              - atmel,at91sam9260-pmc
>>> +              - atmel,at91sam9x5-pmc
>>
>> I missed it last time - why you have two enums? We never talked about
>> this. It's usually wrong... are you sure this is real hardware:
>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
>> ?
> 
> I have 2 enums because there are some hardware covered by:
> "vendor-name,hardware-v1-pmc", "syscon" and some covered by:
> "vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon".

The enum does not say this. At all.

So again, answer, do not ignore:
is this valid setup:
atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
?

> 
> Many AT91 device trees compatibles were written in this way. Thus when new
> versions of the same IP has been introduced the drivers were not
> necessarily updated but the compatibles in device trees were updated e.g.
> with "vendor-name,hardware-v2-pmc" (the full compatible becoming
> "vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon") and
> let the drivers fall back to already in driver supported compatible
> "vendor-name,hardware-v1-pmc", "syscon". In general v2 comes with new
> features in addition to v1.
> 
> That way they AT91 ensures the ABI properties of DT and thus when the
> drivers were finally updated with the new features of the
> "vendor-name,hardware-v2-pmc" DT remained in place.
> 
> Please let me know if these could be handled better in YAML.


enum + const + syscon, like every binding that type does in all
bindings. Don't invent some new syntax.

Best regards,
Krzysztof

Re: [PATCH v3 2/5] dt-bindings: clocks: atmel,at91rm9200-pmc: convert to yaml

[Claudiu.Beznea]

On 10.05.2023 10:06, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 10/05/2023 09:00, Claudiu.Beznea@microchip.com wrote:
>> On 09.05.2023 09:25, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 09/05/2023 07:27, Claudiu Beznea wrote:
>>>> Convert Atmel PMC documentation to yaml. Along with it clock names
>>>> were adapted according to the current available device trees as
>>>> different controller versions accept different clocks (some of them
>>>> have 3 clocks as input, some has 2 clocks as inputs and some with 2
>>>> input clocks uses different clock names).
>>>>
>>>
>>> Thank you for your patch. There is something to discuss/improve.
>>>
>>>> +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:
>>>> +          - enum:
>>>> +              - atmel,at91sam9g15-pmc
>>>> +              - atmel,at91sam9g20-pmc
>>>> +              - atmel,at91sam9g25-pmc
>>>> +              - atmel,at91sam9g35-pmc
>>>> +              - atmel,at91sam9x25-pmc
>>>> +              - atmel,at91sam9x35-pmc
>>>> +          - enum:
>>>> +              - atmel,at91sam9260-pmc
>>>> +              - atmel,at91sam9x5-pmc
>>>
>>> I missed it last time - why you have two enums? We never talked about
>>> this. It's usually wrong... are you sure this is real hardware:
>>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
>>> ?
>>
>> I have 2 enums because there are some hardware covered by:
>> "vendor-name,hardware-v1-pmc", "syscon" and some covered by:
>> "vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon".
> 
> The enum does not say this. At all.
> 
> So again, answer, do not ignore:
> is this valid setup:
> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
> ?

Not w/o syscon. This is valid:

compatible = "atmel,at91sam9g20-pmc", "atmel,at91sam9260-pmc", "syscon";

available in arch/arm/boot/dts/at91sam9g20.dtsi +45

> 
>>
>> Many AT91 device trees compatibles were written in this way. Thus when new
>> versions of the same IP has been introduced the drivers were not
>> necessarily updated but the compatibles in device trees were updated e.g.
>> with "vendor-name,hardware-v2-pmc" (the full compatible becoming
>> "vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon") and
>> let the drivers fall back to already in driver supported compatible
>> "vendor-name,hardware-v1-pmc", "syscon". In general v2 comes with new
>> features in addition to v1.
>>
>> That way they AT91 ensures the ABI properties of DT and thus when the
>> drivers were finally updated with the new features of the
>> "vendor-name,hardware-v2-pmc" DT remained in place.
>>
>> Please let me know if these could be handled better in YAML.
> 
> 
> enum + const + syscon, like every binding that type does in all
> bindings. Don't invent some new syntax.
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v3 2/5] dt-bindings: clocks: atmel,at91rm9200-pmc: convert to yaml

[Krzysztof Kozlowski]

On 10/05/2023 09:14, Claudiu.Beznea@microchip.com wrote:
> On 10.05.2023 10:06, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 10/05/2023 09:00, Claudiu.Beznea@microchip.com wrote:
>>> On 09.05.2023 09:25, Krzysztof Kozlowski wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> On 09/05/2023 07:27, Claudiu Beznea wrote:
>>>>> Convert Atmel PMC documentation to yaml. Along with it clock names
>>>>> were adapted according to the current available device trees as
>>>>> different controller versions accept different clocks (some of them
>>>>> have 3 clocks as input, some has 2 clocks as inputs and some with 2
>>>>> input clocks uses different clock names).
>>>>>
>>>>
>>>> Thank you for your patch. There is something to discuss/improve.
>>>>
>>>>> +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:
>>>>> +          - enum:
>>>>> +              - atmel,at91sam9g15-pmc
>>>>> +              - atmel,at91sam9g20-pmc
>>>>> +              - atmel,at91sam9g25-pmc
>>>>> +              - atmel,at91sam9g35-pmc
>>>>> +              - atmel,at91sam9x25-pmc
>>>>> +              - atmel,at91sam9x35-pmc
>>>>> +          - enum:
>>>>> +              - atmel,at91sam9260-pmc
>>>>> +              - atmel,at91sam9x5-pmc
>>>>
>>>> I missed it last time - why you have two enums? We never talked about
>>>> this. It's usually wrong... are you sure this is real hardware:
>>>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
>>>> ?
>>>
>>> I have 2 enums because there are some hardware covered by:
>>> "vendor-name,hardware-v1-pmc", "syscon" and some covered by:
>>> "vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon".
>>
>> The enum does not say this. At all.
>>
>> So again, answer, do not ignore:
>> is this valid setup:
>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
>> ?
> 
> Not w/o syscon. This is valid:

Syscon is not important here, but indeed I missed it.

> 
> compatible = "atmel,at91sam9g20-pmc", "atmel,at91sam9260-pmc", "syscon";
> 
> available in arch/arm/boot/dts/at91sam9g20.dtsi +45

Nice, so my random choice was actually correct. Ok, so another:

atmel,at91sam9g15-pmc, atmel,at91sam9260-pmc, syscon

Is it valid hardware?

Best regards,
Krzysztof

Re: [PATCH v3 2/5] dt-bindings: clocks: atmel,at91rm9200-pmc: convert to yaml

[Claudiu.Beznea]

On 10.05.2023 10:58, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 10/05/2023 09:14, Claudiu.Beznea@microchip.com wrote:
>> On 10.05.2023 10:06, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 10/05/2023 09:00, Claudiu.Beznea@microchip.com wrote:
>>>> On 09.05.2023 09:25, Krzysztof Kozlowski wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>
>>>>> On 09/05/2023 07:27, Claudiu Beznea wrote:
>>>>>> Convert Atmel PMC documentation to yaml. Along with it clock names
>>>>>> were adapted according to the current available device trees as
>>>>>> different controller versions accept different clocks (some of them
>>>>>> have 3 clocks as input, some has 2 clocks as inputs and some with 2
>>>>>> input clocks uses different clock names).
>>>>>>
>>>>>
>>>>> Thank you for your patch. There is something to discuss/improve.
>>>>>
>>>>>> +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:
>>>>>> +          - enum:
>>>>>> +              - atmel,at91sam9g15-pmc
>>>>>> +              - atmel,at91sam9g20-pmc
>>>>>> +              - atmel,at91sam9g25-pmc
>>>>>> +              - atmel,at91sam9g35-pmc
>>>>>> +              - atmel,at91sam9x25-pmc
>>>>>> +              - atmel,at91sam9x35-pmc
>>>>>> +          - enum:
>>>>>> +              - atmel,at91sam9260-pmc
>>>>>> +              - atmel,at91sam9x5-pmc
>>>>>
>>>>> I missed it last time - why you have two enums? We never talked about
>>>>> this. It's usually wrong... are you sure this is real hardware:
>>>>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
>>>>> ?
>>>>
>>>> I have 2 enums because there are some hardware covered by:
>>>> "vendor-name,hardware-v1-pmc", "syscon" and some covered by:
>>>> "vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon".
>>>
>>> The enum does not say this. At all.
>>>
>>> So again, answer, do not ignore:
>>> is this valid setup:
>>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
>>> ?
>>
>> Not w/o syscon. This is valid:
> 
> Syscon is not important here, but indeed I missed it.
> 
>>
>> compatible = "atmel,at91sam9g20-pmc", "atmel,at91sam9260-pmc", "syscon";
>>
>> available in arch/arm/boot/dts/at91sam9g20.dtsi +45
> 
> Nice, so my random choice was actually correct. Ok, so another:
> 
> atmel,at91sam9g15-pmc, atmel,at91sam9260-pmc, syscon
> 
> Is it valid hardware?

This one, no. So, I guess, the wrong here is that there could be
combinations that are not for actual hardware and yet considered valid by
changes in this patch?

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v3 2/5] dt-bindings: clocks: atmel,at91rm9200-pmc: convert to yaml

[Krzysztof Kozlowski]

On 10/05/2023 10:31, Claudiu.Beznea@microchip.com wrote:
> On 10.05.2023 10:58, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 10/05/2023 09:14, Claudiu.Beznea@microchip.com wrote:
>>> On 10.05.2023 10:06, Krzysztof Kozlowski wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> On 10/05/2023 09:00, Claudiu.Beznea@microchip.com wrote:
>>>>> On 09.05.2023 09:25, Krzysztof Kozlowski wrote:
>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>>
>>>>>> On 09/05/2023 07:27, Claudiu Beznea wrote:
>>>>>>> Convert Atmel PMC documentation to yaml. Along with it clock names
>>>>>>> were adapted according to the current available device trees as
>>>>>>> different controller versions accept different clocks (some of them
>>>>>>> have 3 clocks as input, some has 2 clocks as inputs and some with 2
>>>>>>> input clocks uses different clock names).
>>>>>>>
>>>>>>
>>>>>> Thank you for your patch. There is something to discuss/improve.
>>>>>>
>>>>>>> +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:
>>>>>>> +          - enum:
>>>>>>> +              - atmel,at91sam9g15-pmc
>>>>>>> +              - atmel,at91sam9g20-pmc
>>>>>>> +              - atmel,at91sam9g25-pmc
>>>>>>> +              - atmel,at91sam9g35-pmc
>>>>>>> +              - atmel,at91sam9x25-pmc
>>>>>>> +              - atmel,at91sam9x35-pmc
>>>>>>> +          - enum:
>>>>>>> +              - atmel,at91sam9260-pmc
>>>>>>> +              - atmel,at91sam9x5-pmc
>>>>>>
>>>>>> I missed it last time - why you have two enums? We never talked about
>>>>>> this. It's usually wrong... are you sure this is real hardware:
>>>>>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
>>>>>> ?
>>>>>
>>>>> I have 2 enums because there are some hardware covered by:
>>>>> "vendor-name,hardware-v1-pmc", "syscon" and some covered by:
>>>>> "vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon".
>>>>
>>>> The enum does not say this. At all.
>>>>
>>>> So again, answer, do not ignore:
>>>> is this valid setup:
>>>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
>>>> ?
>>>
>>> Not w/o syscon. This is valid:
>>
>> Syscon is not important here, but indeed I missed it.
>>
>>>
>>> compatible = "atmel,at91sam9g20-pmc", "atmel,at91sam9260-pmc", "syscon";
>>>
>>> available in arch/arm/boot/dts/at91sam9g20.dtsi +45
>>
>> Nice, so my random choice was actually correct. Ok, so another:
>>
>> atmel,at91sam9g15-pmc, atmel,at91sam9260-pmc, syscon
>>
>> Is it valid hardware?
> 
> This one, no. So, I guess, the wrong here is that there could be
> combinations that are not for actual hardware and yet considered valid by
> changes in this patch?

I just don't understand why you have two enums. This is not a pattern
which is allowed anywhere. It might appear but only as exception or mistake.


Best regards,
Krzysztof

Re: [PATCH v3 2/5] dt-bindings: clocks: atmel,at91rm9200-pmc: convert to yaml

[Claudiu.Beznea]

On 10.05.2023 13:12, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 10/05/2023 10:31, Claudiu.Beznea@microchip.com wrote:
>> On 10.05.2023 10:58, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 10/05/2023 09:14, Claudiu.Beznea@microchip.com wrote:
>>>> On 10.05.2023 10:06, Krzysztof Kozlowski wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>
>>>>> On 10/05/2023 09:00, Claudiu.Beznea@microchip.com wrote:
>>>>>> On 09.05.2023 09:25, Krzysztof Kozlowski wrote:
>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>>>
>>>>>>> On 09/05/2023 07:27, Claudiu Beznea wrote:
>>>>>>>> Convert Atmel PMC documentation to yaml. Along with it clock names
>>>>>>>> were adapted according to the current available device trees as
>>>>>>>> different controller versions accept different clocks (some of them
>>>>>>>> have 3 clocks as input, some has 2 clocks as inputs and some with 2
>>>>>>>> input clocks uses different clock names).
>>>>>>>>
>>>>>>>
>>>>>>> Thank you for your patch. There is something to discuss/improve.
>>>>>>>
>>>>>>>> +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:
>>>>>>>> +          - enum:
>>>>>>>> +              - atmel,at91sam9g15-pmc
>>>>>>>> +              - atmel,at91sam9g20-pmc
>>>>>>>> +              - atmel,at91sam9g25-pmc
>>>>>>>> +              - atmel,at91sam9g35-pmc
>>>>>>>> +              - atmel,at91sam9x25-pmc
>>>>>>>> +              - atmel,at91sam9x35-pmc
>>>>>>>> +          - enum:
>>>>>>>> +              - atmel,at91sam9260-pmc
>>>>>>>> +              - atmel,at91sam9x5-pmc
>>>>>>>
>>>>>>> I missed it last time - why you have two enums? We never talked about
>>>>>>> this. It's usually wrong... are you sure this is real hardware:
>>>>>>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
>>>>>>> ?
>>>>>>
>>>>>> I have 2 enums because there are some hardware covered by:
>>>>>> "vendor-name,hardware-v1-pmc", "syscon" and some covered by:
>>>>>> "vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon".
>>>>>
>>>>> The enum does not say this. At all.
>>>>>
>>>>> So again, answer, do not ignore:
>>>>> is this valid setup:
>>>>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
>>>>> ?
>>>>
>>>> Not w/o syscon. This is valid:
>>>
>>> Syscon is not important here, but indeed I missed it.
>>>
>>>>
>>>> compatible = "atmel,at91sam9g20-pmc", "atmel,at91sam9260-pmc", "syscon";
>>>>
>>>> available in arch/arm/boot/dts/at91sam9g20.dtsi +45
>>>
>>> Nice, so my random choice was actually correct. Ok, so another:
>>>
>>> atmel,at91sam9g15-pmc, atmel,at91sam9260-pmc, syscon
>>>
>>> Is it valid hardware?
>>
>> This one, no. So, I guess, the wrong here is that there could be
>> combinations that are not for actual hardware and yet considered valid by
>> changes in this patch?
> 
> I just don't understand why you have two enums. This is not a pattern
> which is allowed anywhere. It might appear but only as exception or mistake.

I'm not at all an YAML expert and this is how I've managed to make
dt_binding_check/dtbs_check happy.

> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v3 2/5] dt-bindings: clocks: atmel,at91rm9200-pmc: convert to yaml

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4682 bytes --]

On Thu, May 11, 2023 at 06:29:39AM +0000, Claudiu.Beznea@microchip.com wrote:
> On 10.05.2023 13:12, Krzysztof Kozlowski wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On 10/05/2023 10:31, Claudiu.Beznea@microchip.com wrote:
> >> On 10.05.2023 10:58, Krzysztof Kozlowski wrote:
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >>>
> >>> On 10/05/2023 09:14, Claudiu.Beznea@microchip.com wrote:
> >>>> On 10.05.2023 10:06, Krzysztof Kozlowski wrote:
> >>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >>>>>
> >>>>> On 10/05/2023 09:00, Claudiu.Beznea@microchip.com wrote:
> >>>>>> On 09.05.2023 09:25, Krzysztof Kozlowski wrote:
> >>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >>>>>>>
> >>>>>>> On 09/05/2023 07:27, Claudiu Beznea wrote:
> >>>>>>>> Convert Atmel PMC documentation to yaml. Along with it clock names
> >>>>>>>> were adapted according to the current available device trees as
> >>>>>>>> different controller versions accept different clocks (some of them
> >>>>>>>> have 3 clocks as input, some has 2 clocks as inputs and some with 2
> >>>>>>>> input clocks uses different clock names).
> >>>>>>>>
> >>>>>>>
> >>>>>>> Thank you for your patch. There is something to discuss/improve.
> >>>>>>>
> >>>>>>>> +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:
> >>>>>>>> +          - enum:
> >>>>>>>> +              - atmel,at91sam9g15-pmc
> >>>>>>>> +              - atmel,at91sam9g20-pmc
> >>>>>>>> +              - atmel,at91sam9g25-pmc
> >>>>>>>> +              - atmel,at91sam9g35-pmc
> >>>>>>>> +              - atmel,at91sam9x25-pmc
> >>>>>>>> +              - atmel,at91sam9x35-pmc
> >>>>>>>> +          - enum:
> >>>>>>>> +              - atmel,at91sam9260-pmc
> >>>>>>>> +              - atmel,at91sam9x5-pmc
> >>>>>>>
> >>>>>>> I missed it last time - why you have two enums? We never talked about
> >>>>>>> this. It's usually wrong... are you sure this is real hardware:
> >>>>>>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
> >>>>>>> ?
> >>>>>>
> >>>>>> I have 2 enums because there are some hardware covered by:
> >>>>>> "vendor-name,hardware-v1-pmc", "syscon" and some covered by:
> >>>>>> "vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon".
> >>>>>
> >>>>> The enum does not say this. At all.
> >>>>>
> >>>>> So again, answer, do not ignore:
> >>>>> is this valid setup:
> >>>>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
> >>>>> ?
> >>>>
> >>>> Not w/o syscon. This is valid:
> >>>
> >>> Syscon is not important here, but indeed I missed it.
> >>>
> >>>>
> >>>> compatible = "atmel,at91sam9g20-pmc", "atmel,at91sam9260-pmc", "syscon";
> >>>>
> >>>> available in arch/arm/boot/dts/at91sam9g20.dtsi +45
> >>>
> >>> Nice, so my random choice was actually correct. Ok, so another:
> >>>
> >>> atmel,at91sam9g15-pmc, atmel,at91sam9260-pmc, syscon
> >>>
> >>> Is it valid hardware?
> >>
> >> This one, no. So, I guess, the wrong here is that there could be
> >> combinations that are not for actual hardware and yet considered valid by
> >> changes in this patch?
> > 
> > I just don't understand why you have two enums. This is not a pattern
> > which is allowed anywhere. It might appear but only as exception or mistake.
> 
> I'm not at all an YAML expert and this is how I've managed to make
> dt_binding_check/dtbs_check happy.

Picking one item at random, do the devicetrees contain stuff like:
"atmel,at91sam9g35-pmc", "atmel,at91sam9260-pmc", "syscon"
//AND//
"atmel,at91sam9g35-pmc", "atmel,at91sam9x5-pmc", "syscon"
?

If not, why do you not break it down to something like:
- items:
    - enum:
        - atmel,compatible
        - atmel,with
        - atmel,sam9260's pmc
    - const: atmel,at91sam9260-pmc
    - const: syscon

- items:
    - enum:
        - atmel,compatible
        - atmel,with
        - atmel,sam9x5's pmc
    - const: atmel,at91sam9x5-pmc
    - const: syscon

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 2/5] dt-bindings: clocks: atmel,at91rm9200-pmc: convert to yaml

[Claudiu.Beznea]

On 11.05.2023 11:58, Conor Dooley wrote:
> On Thu, May 11, 2023 at 06:29:39AM +0000, Claudiu.Beznea@microchip.com wrote:
>> On 10.05.2023 13:12, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 10/05/2023 10:31, Claudiu.Beznea@microchip.com wrote:
>>>> On 10.05.2023 10:58, Krzysztof Kozlowski wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>
>>>>> On 10/05/2023 09:14, Claudiu.Beznea@microchip.com wrote:
>>>>>> On 10.05.2023 10:06, Krzysztof Kozlowski wrote:
>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>>>
>>>>>>> On 10/05/2023 09:00, Claudiu.Beznea@microchip.com wrote:
>>>>>>>> On 09.05.2023 09:25, Krzysztof Kozlowski wrote:
>>>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>>>>>
>>>>>>>>> On 09/05/2023 07:27, Claudiu Beznea wrote:
>>>>>>>>>> Convert Atmel PMC documentation to yaml. Along with it clock names
>>>>>>>>>> were adapted according to the current available device trees as
>>>>>>>>>> different controller versions accept different clocks (some of them
>>>>>>>>>> have 3 clocks as input, some has 2 clocks as inputs and some with 2
>>>>>>>>>> input clocks uses different clock names).
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thank you for your patch. There is something to discuss/improve.
>>>>>>>>>
>>>>>>>>>> +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:
>>>>>>>>>> +          - enum:
>>>>>>>>>> +              - atmel,at91sam9g15-pmc
>>>>>>>>>> +              - atmel,at91sam9g20-pmc
>>>>>>>>>> +              - atmel,at91sam9g25-pmc
>>>>>>>>>> +              - atmel,at91sam9g35-pmc
>>>>>>>>>> +              - atmel,at91sam9x25-pmc
>>>>>>>>>> +              - atmel,at91sam9x35-pmc
>>>>>>>>>> +          - enum:
>>>>>>>>>> +              - atmel,at91sam9260-pmc
>>>>>>>>>> +              - atmel,at91sam9x5-pmc
>>>>>>>>>
>>>>>>>>> I missed it last time - why you have two enums? We never talked about
>>>>>>>>> this. It's usually wrong... are you sure this is real hardware:
>>>>>>>>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
>>>>>>>>> ?
>>>>>>>>
>>>>>>>> I have 2 enums because there are some hardware covered by:
>>>>>>>> "vendor-name,hardware-v1-pmc", "syscon" and some covered by:
>>>>>>>> "vendor-name,hardware-v2-pmc", "vendor-name,hardware-v1-pmc", "syscon".
>>>>>>>
>>>>>>> The enum does not say this. At all.
>>>>>>>
>>>>>>> So again, answer, do not ignore:
>>>>>>> is this valid setup:
>>>>>>> atmel,at91sam9g20-pmc, atmel,at91sam9260-pmc
>>>>>>> ?
>>>>>>
>>>>>> Not w/o syscon. This is valid:
>>>>>
>>>>> Syscon is not important here, but indeed I missed it.
>>>>>
>>>>>>
>>>>>> compatible = "atmel,at91sam9g20-pmc", "atmel,at91sam9260-pmc", "syscon";
>>>>>>
>>>>>> available in arch/arm/boot/dts/at91sam9g20.dtsi +45
>>>>>
>>>>> Nice, so my random choice was actually correct. Ok, so another:
>>>>>
>>>>> atmel,at91sam9g15-pmc, atmel,at91sam9260-pmc, syscon
>>>>>
>>>>> Is it valid hardware?
>>>>
>>>> This one, no. So, I guess, the wrong here is that there could be
>>>> combinations that are not for actual hardware and yet considered valid by
>>>> changes in this patch?
>>>
>>> I just don't understand why you have two enums. This is not a pattern
>>> which is allowed anywhere. It might appear but only as exception or mistake.
>>
>> I'm not at all an YAML expert and this is how I've managed to make
>> dt_binding_check/dtbs_check happy.
> 
> Picking one item at random, do the devicetrees contain stuff like:
> "atmel,at91sam9g35-pmc", "atmel,at91sam9260-pmc", "syscon"
> //AND//
> "atmel,at91sam9g35-pmc", "atmel,at91sam9x5-pmc", "syscon"
> ?
> 
> If not, why do you not break it down to something like:
> - items:
>     - enum:
>         - atmel,compatible
>         - atmel,with
>         - atmel,sam9260's pmc
>     - const: atmel,at91sam9260-pmc
>     - const: syscon
> 
> - items:
>     - enum:
>         - atmel,compatible
>         - atmel,with
>         - atmel,sam9x5's pmc
>     - const: atmel,at91sam9x5-pmc
>     - const: syscon
> 

I'll check it out, thank you!

> Cheers,
> Conor.