diff mbox series

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

Message ID 20230516051836.2511149-3-claudiu.beznea@microchip.com (mailing list archive)
State Changes Requested, archived
Headers show
Series dt-bindings: clocks: at91: convert to yaml | expand

Commit Message

Claudiu Beznea May 16, 2023, 5:18 a.m. UTC 
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 clock (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  | 153 ++++++++++++++++++
 2 files changed, 153 insertions(+), 28 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml

Comments

Conor Dooley May 16, 2023, noon UTC | #1 
Hey Claudiu,

On Tue, May 16, 2023 at 08:18:33AM +0300, 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 clock (some of them
> have 3 clocks as input, some has 2 clocks as inputs and some with 2
> input clocks uses different clock names).
> 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..e5f514bc4bf7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml
> @@ -0,0 +1,153 @@
> +# 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,at91sam9g20-pmc
> +          - const: atmel,at91sam9260-pmc
> +          - const: syscon
> +      - items:
> +          - enum:
> +              - atmel,at91sam9g15-pmc
> +              - atmel,at91sam9g25-pmc
> +              - atmel,at91sam9g35-pmc
> +              - atmel,at91sam9x25-pmc
> +              - atmel,at91sam9x35-pmc
> +          - const: atmel,at91sam9x5-pmc

Yet another combinations question for you...
With this binding the following is not possible:

"atmel,at91sam9x5-pmc", "syscon"

Is that intended?
I notice "atmel,at91sam9260-pmc" is able to appear as:

"atmel,at91sam9260-pmc", "syscon"

So the inconsistency stands out.

> +          - 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

Otherwise, this looks grand to me.

Cheers,
Conor.
Claudiu Beznea May 16, 2023, 12:58 p.m. UTC | #2 
Hi, Conor,

On 16.05.2023 15:00, Conor Dooley wrote:
> Hey Claudiu,
> 
> On Tue, May 16, 2023 at 08:18:33AM +0300, 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 clock (some of them
>> have 3 clocks as input, some has 2 clocks as inputs and some with 2
>> input clocks uses different clock names).
>> 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..e5f514bc4bf7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml
>> @@ -0,0 +1,153 @@
>> +# 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,at91sam9g20-pmc
>> +          - const: atmel,at91sam9260-pmc
>> +          - const: syscon
>> +      - items:
>> +          - enum:
>> +              - atmel,at91sam9g15-pmc
>> +              - atmel,at91sam9g25-pmc
>> +              - atmel,at91sam9g35-pmc
>> +              - atmel,at91sam9x25-pmc
>> +              - atmel,at91sam9x35-pmc
>> +          - const: atmel,at91sam9x5-pmc
> Yet another combinations question for you...
> With this binding the following is not possible:
> 
> "atmel,at91sam9x5-pmc", "syscon"
> 
> Is that intended?

No, I've just missed it. Same for the above. I'll have a new round and fix it.

Thanks for having a look at this,
Claudiu

> I notice "atmel,at91sam9260-pmc" is able to appear as:
> 
> "atmel,at91sam9260-pmc", "syscon"
> 
> So the inconsistency stands out.
> 
>> +          - 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
> Otherwise, this looks grand to me.
> 
> Cheers,
> Conor.
Claudiu Beznea May 17, 2023, 8:48 a.m. UTC | #3 
On 16.05.2023 15:58, Claudiu Beznea - M18063 wrote:
> Hi, Conor,
> 
> On 16.05.2023 15:00, Conor Dooley wrote:
>> Hey Claudiu,
>>
>> On Tue, May 16, 2023 at 08:18:33AM +0300, 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 clock (some of them
>>> have 3 clocks as input, some has 2 clocks as inputs and some with 2
>>> input clocks uses different clock names).
>>> 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..e5f514bc4bf7
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml
>>> @@ -0,0 +1,153 @@
>>> +# 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,at91sam9g20-pmc
>>> +          - const: atmel,at91sam9260-pmc
>>> +          - const: syscon
>>> +      - items:
>>> +          - enum:
>>> +              - atmel,at91sam9g15-pmc
>>> +              - atmel,at91sam9g25-pmc
>>> +              - atmel,at91sam9g35-pmc
>>> +              - atmel,at91sam9x25-pmc
>>> +              - atmel,at91sam9x35-pmc
>>> +          - const: atmel,at91sam9x5-pmc
>> Yet another combinations question for you...
>> With this binding the following is not possible:
>>
>> "atmel,at91sam9x5-pmc", "syscon"
>>
>> Is that intended?
> 
> No, I've just missed it. Same for the above. I'll have a new round and fix it.

Though... shouldn't this have been detected by make dtbs_check?

> 
> Thanks for having a look at this,
> Claudiu
> 
>> I notice "atmel,at91sam9260-pmc" is able to appear as:
>>
>> "atmel,at91sam9260-pmc", "syscon"
>>
>> So the inconsistency stands out.
>>
>>> +          - 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
>> Otherwise, this looks grand to me.
>>
>> Cheers,
>> Conor.
>
Conor Dooley May 17, 2023, 8:55 a.m. UTC | #4 
On Wed, May 17, 2023 at 08:48:04AM +0000, Claudiu.Beznea@microchip.com wrote:
> On 16.05.2023 15:58, Claudiu Beznea - M18063 wrote:
> > Hi, Conor,
> > 
> > On 16.05.2023 15:00, Conor Dooley wrote:
> >> Hey Claudiu,
> >>
> >> On Tue, May 16, 2023 at 08:18:33AM +0300, 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 clock (some of them
> >>> have 3 clocks as input, some has 2 clocks as inputs and some with 2
> >>> input clocks uses different clock names).
> >>> 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..e5f514bc4bf7
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml
> >>> @@ -0,0 +1,153 @@
> >>> +# 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,at91sam9g20-pmc
> >>> +          - const: atmel,at91sam9260-pmc
> >>> +          - const: syscon
> >>> +      - items:
> >>> +          - enum:
> >>> +              - atmel,at91sam9g15-pmc
> >>> +              - atmel,at91sam9g25-pmc
> >>> +              - atmel,at91sam9g35-pmc
> >>> +              - atmel,at91sam9x25-pmc
> >>> +              - atmel,at91sam9x35-pmc
> >>> +          - const: atmel,at91sam9x5-pmc
> >> Yet another combinations question for you...
> >> With this binding the following is not possible:
> >>
> >> "atmel,at91sam9x5-pmc", "syscon"
> >>
> >> Is that intended?
> > 
> > No, I've just missed it. Same for the above. I'll have a new round and fix it.
> 
> Though... shouldn't this have been detected by make dtbs_check?

Only if there actually exists a dtb containing
compatible = "atmel,at91sam9x5-pmc", "syscon";
that is enabled by the config that you are building with.

From taking a quick look:
git grep "\"atmel,at91sam9x5-pmc\", \"syscon\""
arch/arm/boot/dts/at91sam9g15.dtsi:    compatible = "atmel,at91sam9g15-pmc", "atmel,at91sam9x5-pmc", "syscon";
arch/arm/boot/dts/at91sam9g25.dtsi:    compatible = "atmel,at91sam9g25-pmc", "atmel,at91sam9x5-pmc", "syscon";
arch/arm/boot/dts/at91sam9g35.dtsi:    compatible = "atmel,at91sam9g35-pmc", "atmel,at91sam9x5-pmc", "syscon";
arch/arm/boot/dts/at91sam9x25.dtsi:    compatible = "atmel,at91sam9x25-pmc", "atmel,at91sam9x5-pmc", "syscon";
arch/arm/boot/dts/at91sam9x35.dtsi:    compatible = "atmel,at91sam9x35-pmc", "atmel,at91sam9x5-pmc", "syscon";
arch/arm/boot/dts/at91sam9x5.dtsi:     compatible = "atmel,at91sam9x5-pmc", "syscon";

There's only actually one place where you have this combination & it
seems to get overridden by all of the more specific dtsi files that
include at91sam9x5.dtsi.

Hope that helps,
Conor.
diff mbox series

Patch

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..e5f514bc4bf7
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml
@@ -0,0 +1,153 @@ 
+# 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,at91sam9g20-pmc
+          - const: atmel,at91sam9260-pmc
+          - const: syscon
+      - items:
+          - enum:
+              - atmel,at91sam9g15-pmc
+              - atmel,at91sam9g25-pmc
+              - atmel,at91sam9g35-pmc
+              - atmel,at91sam9x25-pmc
+              - atmel,at91sam9x35-pmc
+          - const: 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";
+    };
+
+...