diff mbox series

[v2,3/4] dt-bindings: power: reset: atmel,sama5d2-shdwc: convert to yaml

Message ID 20230524123528.439082-4-claudiu.beznea@microchip.com (mailing list archive)
State New, archived
Headers show
Series dt-bindings: power: reset: at91: convert to YAML | expand

Commit Message

Claudiu Beznea May 24, 2023, 12:35 p.m. UTC
Convert Atmel SAMA5D2 shutdown controller to YAML. SAMA7G5 SHDWC DT node
(available in arch/arm/boot/dts/sama7g5.dtsi) has syscon along with its
compatible. There is no usage of this syscon in the current code but it
may be necessary in future as some registers of SHDWC are accessed in
different drivers (at91-sama5d2_shdwc.c and arch/arm/mach-at91/pm.c).
Thus update the YAML with it to make DT checkers happy.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 .../devicetree/bindings/arm/atmel-sysregs.txt |  63 ----------
 .../power/reset/atmel,sama5d2-shdwc.yaml      | 115 ++++++++++++++++++
 2 files changed, 115 insertions(+), 63 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/reset/atmel,sama5d2-shdwc.yaml

Comments

Conor Dooley May 24, 2023, 7:19 p.m. UTC | #1
On Wed, May 24, 2023 at 03:35:27PM +0300, Claudiu Beznea wrote:
> Convert Atmel SAMA5D2 shutdown controller to YAML. SAMA7G5 SHDWC DT node
> (available in arch/arm/boot/dts/sama7g5.dtsi) has syscon along with its
> compatible. There is no usage of this syscon in the current code but it
> may be necessary in future as some registers of SHDWC are accessed in
> different drivers (at91-sama5d2_shdwc.c and arch/arm/mach-at91/pm.c).
> Thus update the YAML with it to make DT checkers happy.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>

Modulo the license thing that I mentioned on v1,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.
> ---
>  .../devicetree/bindings/arm/atmel-sysregs.txt |  63 ----------
>  .../power/reset/atmel,sama5d2-shdwc.yaml      | 115 ++++++++++++++++++
>  2 files changed, 115 insertions(+), 63 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/reset/atmel,sama5d2-shdwc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
> index e6b2fb291b45..67a66bf74895 100644
> --- a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
> +++ b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
> @@ -52,69 +52,6 @@ Example:
>  		reg = <0xe3804000 0x1000>;
>  };
>  
> -SHDWC SAMA5D2-Compatible Shutdown Controller
> -
> -1) shdwc node
> -
> -required properties:
> -- compatible: should be "atmel,sama5d2-shdwc", "microchip,sam9x60-shdwc" or
> -  "microchip,sama7g5-shdwc"
> -- reg: should contain registers location and length
> -- clocks: phandle to input clock.
> -- #address-cells: should be one. The cell is the wake-up input index.
> -- #size-cells: should be zero.
> -
> -optional properties:
> -
> -- debounce-delay-us: minimum wake-up inputs debouncer period in
> -  microseconds. It's usually a board-related property.
> -- atmel,wakeup-rtc-timer: boolean to enable Real-Time Clock wake-up.
> -
> -optional microchip,sam9x60-shdwc or microchip,sama7g5-shdwc properties:
> -- atmel,wakeup-rtt-timer: boolean to enable Real-time Timer Wake-up.
> -
> -The node contains child nodes for each wake-up input that the platform uses.
> -
> -2) input nodes
> -
> -Wake-up input nodes are usually described in the "board" part of the Device
> -Tree. Note also that input 0 is linked to the wake-up pin and is frequently
> -used.
> -
> -Required properties:
> -- reg: should contain the wake-up input index [0 - 15].
> -
> -Optional properties:
> -- atmel,wakeup-active-high: boolean, the corresponding wake-up input described
> -  by the child, forces the wake-up of the core power supply on a high level.
> -  The default is to be active low.
> -
> -Example:
> -
> -On the SoC side:
> -	shdwc@f8048010 {
> -		compatible = "atmel,sama5d2-shdwc";
> -		reg = <0xf8048010 0x10>;
> -		clocks = <&clk32k>;
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		atmel,wakeup-rtc-timer;
> -	};
> -
> -On the board side:
> -	shdwc@f8048010 {
> -		debounce-delay-us = <976>;
> -
> -		input@0 {
> -			reg = <0>;
> -		};
> -
> -		input@1 {
> -			reg = <1>;
> -			atmel,wakeup-active-high;
> -		};
> -	};
> -
>  Special Function Registers (SFR)
>  
>  Special Function Registers (SFR) manage specific aspects of the integrated
> diff --git a/Documentation/devicetree/bindings/power/reset/atmel,sama5d2-shdwc.yaml b/Documentation/devicetree/bindings/power/reset/atmel,sama5d2-shdwc.yaml
> new file mode 100644
> index 000000000000..31a16a354a3a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/atmel,sama5d2-shdwc.yaml
> @@ -0,0 +1,115 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/reset/atmel,sama5d2-shdwc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel SAMA5D2 SHDWC Shutdown Controller
> +
> +maintainers:
> +  - Claudiu Beznea <claudiu.beznea@microchip.com>
> +
> +description:
> +  Atmel SHDWC shutdown controller controls the power supplies VDDIO and VDDCORE
> +  and the wake-up detection on debounced input lines.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - const: microchip,sama7g5-shdwc
> +          - const: syscon
> +      - items:
> +          enum:
> +            - atmel,sama5d2-shdwc
> +            - microchip,sam9x60-shdwc
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  debounce-delay-us:
> +    description:
> +      Minimum wake-up inputs debouncer period in microseconds. It is usually a
> +      board-related property.
> +
> +  atmel,wakeup-rtc-timer:
> +    description: enable real-time clock wake-up
> +    type: boolean
> +
> +patternProperties:
> +  "^input@[0-15]$":
> +    description:
> +      Wake-up input nodes. These are usually described in the "board" part of
> +      the Device Tree. Note also that input 0 is linked to the wake-up pin and
> +      is frequently used.
> +    type: object
> +    properties:
> +      reg:
> +        description: contains the wake-up input index
> +        minimum: 0
> +        maximum: 15
> +
> +      atmel,wakeup-active-high:
> +        description:
> +          The corresponding wake-up input described by the child forces the
> +          wake-up of the core power supply on a high level. The default is to
> +          be active low.
> +        type: boolean
> +
> +    required:
> +      - reg
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - microchip,sam9x60-shdwc
> +              - microchip,sama7g5-shdwc
> +    then:
> +      properties:
> +        atmel,wakeup-rtt-timer:
> +          description: enable real-time timer wake-up
> +          type: boolean
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    shdwc: poweroff@f8048010 {
> +        compatible = "atmel,sama5d2-shdwc";
> +        reg = <0xf8048010 0x10>;
> +        clocks = <&clk32k>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        atmel,wakeup-rtc-timer;
> +        debounce-delay-us = <976>;
> +
> +        input@0 {
> +            reg = <0>;
> +        };
> +
> +        input@1 {
> +            reg = <1>;
> +            atmel,wakeup-active-high;
> +        };
> +    };
> +
> +...
> -- 
> 2.34.1
>
Krzysztof Kozlowski June 2, 2023, 2:53 p.m. UTC | #2
On 24/05/2023 14:35, Claudiu Beznea wrote:
> Convert Atmel SAMA5D2 shutdown controller to YAML. SAMA7G5 SHDWC DT node
> (available in arch/arm/boot/dts/sama7g5.dtsi) has syscon along with its
> compatible. There is no usage of this syscon in the current code but it
> may be necessary in future as some registers of SHDWC are accessed in
> different drivers (at91-sama5d2_shdwc.c and arch/arm/mach-at91/pm.c).
> Thus update the YAML with it to make DT checkers happy.
> 



> +---
> +$id: http://devicetree.org/schemas/power/reset/atmel,sama5d2-shdwc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel SAMA5D2 SHDWC Shutdown Controller
> +
> +maintainers:
> +  - Claudiu Beznea <claudiu.beznea@microchip.com>
> +
> +description:
> +  Atmel SHDWC shutdown controller controls the power supplies VDDIO and VDDCORE
> +  and the wake-up detection on debounced input lines.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - const: microchip,sama7g5-shdwc
> +          - const: syscon
> +      - items:

These are not items, but just enum. This could not work as intended.

> +          enum:
> +            - atmel,sama5d2-shdwc
> +            - microchip,sam9x60-shdwc
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  debounce-delay-us:
> +    description:
> +      Minimum wake-up inputs debouncer period in microseconds. It is usually a
> +      board-related property.
> +
> +  atmel,wakeup-rtc-timer:
> +    description: enable real-time clock wake-up
> +    type: boolean
> +
> +patternProperties:
> +  "^input@[0-15]$":
> +    description:
> +      Wake-up input nodes. These are usually described in the "board" part of
> +      the Device Tree. Note also that input 0 is linked to the wake-up pin and
> +      is frequently used.
> +    type: object
> +    properties:
> +      reg:
> +        description: contains the wake-up input index
> +        minimum: 0
> +        maximum: 15
> +
> +      atmel,wakeup-active-high:
> +        description:
> +          The corresponding wake-up input described by the child forces the
> +          wake-up of the core power supply on a high level. The default is to
> +          be active low.
> +        type: boolean
> +
> +    required:
> +      - reg
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - microchip,sam9x60-shdwc
> +              - microchip,sama7g5-shdwc
> +    then:
> +      properties:
> +        atmel,wakeup-rtt-timer:
> +          description: enable real-time timer wake-up
> +          type: boolean

Don't define properties in allOf. This should be in top-level.


Missing additionalProperties: false.

Best regards,
Krzysztof
Rob Herring (Arm) June 7, 2023, 8:43 p.m. UTC | #3
On Wed, May 24, 2023 at 08:19:08PM +0100, Conor Dooley wrote:
> On Wed, May 24, 2023 at 03:35:27PM +0300, Claudiu Beznea wrote:
> > Convert Atmel SAMA5D2 shutdown controller to YAML. SAMA7G5 SHDWC DT node
> > (available in arch/arm/boot/dts/sama7g5.dtsi) has syscon along with its
> > compatible. There is no usage of this syscon in the current code but it
> > may be necessary in future as some registers of SHDWC are accessed in
> > different drivers (at91-sama5d2_shdwc.c and arch/arm/mach-at91/pm.c).
> > Thus update the YAML with it to make DT checkers happy.
> > 
> > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Modulo the license thing that I mentioned on v1,

Should be fine given it's an Microchip employee changing a Microchip 
binding.

Rob
Conor Dooley June 7, 2023, 8:48 p.m. UTC | #4
On Wed, Jun 07, 2023 at 02:43:51PM -0600, Rob Herring wrote:
> On Wed, May 24, 2023 at 08:19:08PM +0100, Conor Dooley wrote:
> > On Wed, May 24, 2023 at 03:35:27PM +0300, Claudiu Beznea wrote:
> > > Convert Atmel SAMA5D2 shutdown controller to YAML. SAMA7G5 SHDWC DT node
> > > (available in arch/arm/boot/dts/sama7g5.dtsi) has syscon along with its
> > > compatible. There is no usage of this syscon in the current code but it
> > > may be necessary in future as some registers of SHDWC are accessed in
> > > different drivers (at91-sama5d2_shdwc.c and arch/arm/mach-at91/pm.c).
> > > Thus update the YAML with it to make DT checkers happy.
> > > 
> > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> > 
> > Modulo the license thing that I mentioned on v1,
> 
> Should be fine given it's an Microchip employee changing a Microchip 
> binding.

Aye, that part I figured was fine - it was the when I looked at the
blame for the files & they were filled with your name that I wondered
about the licensing.
If you're okay with it though then clearly there's not an issue :)

Cheers,
Conor.
Rob Herring (Arm) June 8, 2023, 2:38 p.m. UTC | #5
On Wed, Jun 7, 2023 at 2:48 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Wed, Jun 07, 2023 at 02:43:51PM -0600, Rob Herring wrote:
> > On Wed, May 24, 2023 at 08:19:08PM +0100, Conor Dooley wrote:
> > > On Wed, May 24, 2023 at 03:35:27PM +0300, Claudiu Beznea wrote:
> > > > Convert Atmel SAMA5D2 shutdown controller to YAML. SAMA7G5 SHDWC DT node
> > > > (available in arch/arm/boot/dts/sama7g5.dtsi) has syscon along with its
> > > > compatible. There is no usage of this syscon in the current code but it
> > > > may be necessary in future as some registers of SHDWC are accessed in
> > > > different drivers (at91-sama5d2_shdwc.c and arch/arm/mach-at91/pm.c).
> > > > Thus update the YAML with it to make DT checkers happy.
> > > >
> > > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> > >
> > > Modulo the license thing that I mentioned on v1,
> >
> > Should be fine given it's an Microchip employee changing a Microchip
> > binding.
>
> Aye, that part I figured was fine - it was the when I looked at the
> blame for the files & they were filled with your name that I wondered
> about the licensing.

For reference, anything done by Arm, Linaro or NVIDIA employees is
okay to relicense to dual license.

Rob
Conor Dooley June 8, 2023, 4:49 p.m. UTC | #6
On Thu, Jun 08, 2023 at 08:38:10AM -0600, Rob Herring wrote:

> For reference, anything done by Arm, Linaro or NVIDIA employees is
> okay to relicense to dual license.

Ah cool, that's good to know, thanks.
Perhaps I should try to get a similar edict issued for Microchip ones.
@Nicolas, does that sound reasonable?

Cheers,
Conor.
Nicolas Ferre June 9, 2023, 10:09 a.m. UTC | #7
On 08/06/2023 at 18:49, Conor Dooley wrote:
> 
> On Thu, Jun 08, 2023 at 08:38:10AM -0600, Rob Herring wrote:
> 
>> For reference, anything done by Arm, Linaro or NVIDIA employees is
>> okay to relicense to dual license.
> Ah cool, that's good to know, thanks.
> Perhaps I should try to get a similar edict issued for Microchip ones.
> @Nicolas, does that sound reasonable?

Well, we can work it out internally, indeed. But is there a public 
statement about this somewhere?

Regards,
   Nicolas
Conor Dooley June 13, 2023, 6:44 p.m. UTC | #8
On Fri, Jun 09, 2023 at 12:09:24PM +0200, Nicolas Ferre wrote:
> On 08/06/2023 at 18:49, Conor Dooley wrote:
> > 
> > On Thu, Jun 08, 2023 at 08:38:10AM -0600, Rob Herring wrote:
> > 
> > > For reference, anything done by Arm, Linaro or NVIDIA employees is
> > > okay to relicense to dual license.
> > Ah cool, that's good to know, thanks.
> > Perhaps I should try to get a similar edict issued for Microchip ones.
> > @Nicolas, does that sound reasonable?
> 
> Well, we can work it out internally, indeed. But is there a public statement
> about this somewhere?

You mean, is there a public statement about Arm/Linaro/Nvidia being okay
with relicensing bindings as dual license?
Conor Dooley July 10, 2023, 5:12 p.m. UTC | #9
Hey Nicolas,

On Tue, Jun 13, 2023 at 07:44:58PM +0100, Conor Dooley wrote:
> On Fri, Jun 09, 2023 at 12:09:24PM +0200, Nicolas Ferre wrote:
> > On 08/06/2023 at 18:49, Conor Dooley wrote:
> > > 
> > > On Thu, Jun 08, 2023 at 08:38:10AM -0600, Rob Herring wrote:
> > > 
> > > > For reference, anything done by Arm, Linaro or NVIDIA employees is
> > > > okay to relicense to dual license.
> > > Ah cool, that's good to know, thanks.
> > > Perhaps I should try to get a similar edict issued for Microchip ones.
> > > @Nicolas, does that sound reasonable?
> > 
> > Well, we can work it out internally, indeed. But is there a public statement
> > about this somewhere?
> 
> You mean, is there a public statement about Arm/Linaro/Nvidia being okay
> with relicensing bindings as dual license?

This was sitting at the end of my queue, don't recall following this up
internally. Getting an edict might be more hassle than it is worth & I
guess I am always CCed on bindings to Ack them.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
index e6b2fb291b45..67a66bf74895 100644
--- a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
+++ b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
@@ -52,69 +52,6 @@  Example:
 		reg = <0xe3804000 0x1000>;
 };
 
-SHDWC SAMA5D2-Compatible Shutdown Controller
-
-1) shdwc node
-
-required properties:
-- compatible: should be "atmel,sama5d2-shdwc", "microchip,sam9x60-shdwc" or
-  "microchip,sama7g5-shdwc"
-- reg: should contain registers location and length
-- clocks: phandle to input clock.
-- #address-cells: should be one. The cell is the wake-up input index.
-- #size-cells: should be zero.
-
-optional properties:
-
-- debounce-delay-us: minimum wake-up inputs debouncer period in
-  microseconds. It's usually a board-related property.
-- atmel,wakeup-rtc-timer: boolean to enable Real-Time Clock wake-up.
-
-optional microchip,sam9x60-shdwc or microchip,sama7g5-shdwc properties:
-- atmel,wakeup-rtt-timer: boolean to enable Real-time Timer Wake-up.
-
-The node contains child nodes for each wake-up input that the platform uses.
-
-2) input nodes
-
-Wake-up input nodes are usually described in the "board" part of the Device
-Tree. Note also that input 0 is linked to the wake-up pin and is frequently
-used.
-
-Required properties:
-- reg: should contain the wake-up input index [0 - 15].
-
-Optional properties:
-- atmel,wakeup-active-high: boolean, the corresponding wake-up input described
-  by the child, forces the wake-up of the core power supply on a high level.
-  The default is to be active low.
-
-Example:
-
-On the SoC side:
-	shdwc@f8048010 {
-		compatible = "atmel,sama5d2-shdwc";
-		reg = <0xf8048010 0x10>;
-		clocks = <&clk32k>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-		atmel,wakeup-rtc-timer;
-	};
-
-On the board side:
-	shdwc@f8048010 {
-		debounce-delay-us = <976>;
-
-		input@0 {
-			reg = <0>;
-		};
-
-		input@1 {
-			reg = <1>;
-			atmel,wakeup-active-high;
-		};
-	};
-
 Special Function Registers (SFR)
 
 Special Function Registers (SFR) manage specific aspects of the integrated
diff --git a/Documentation/devicetree/bindings/power/reset/atmel,sama5d2-shdwc.yaml b/Documentation/devicetree/bindings/power/reset/atmel,sama5d2-shdwc.yaml
new file mode 100644
index 000000000000..31a16a354a3a
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/atmel,sama5d2-shdwc.yaml
@@ -0,0 +1,115 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/reset/atmel,sama5d2-shdwc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel SAMA5D2 SHDWC Shutdown Controller
+
+maintainers:
+  - Claudiu Beznea <claudiu.beznea@microchip.com>
+
+description:
+  Atmel SHDWC shutdown controller controls the power supplies VDDIO and VDDCORE
+  and the wake-up detection on debounced input lines.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - const: microchip,sama7g5-shdwc
+          - const: syscon
+      - items:
+          enum:
+            - atmel,sama5d2-shdwc
+            - microchip,sam9x60-shdwc
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  debounce-delay-us:
+    description:
+      Minimum wake-up inputs debouncer period in microseconds. It is usually a
+      board-related property.
+
+  atmel,wakeup-rtc-timer:
+    description: enable real-time clock wake-up
+    type: boolean
+
+patternProperties:
+  "^input@[0-15]$":
+    description:
+      Wake-up input nodes. These are usually described in the "board" part of
+      the Device Tree. Note also that input 0 is linked to the wake-up pin and
+      is frequently used.
+    type: object
+    properties:
+      reg:
+        description: contains the wake-up input index
+        minimum: 0
+        maximum: 15
+
+      atmel,wakeup-active-high:
+        description:
+          The corresponding wake-up input described by the child forces the
+          wake-up of the core power supply on a high level. The default is to
+          be active low.
+        type: boolean
+
+    required:
+      - reg
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - microchip,sam9x60-shdwc
+              - microchip,sama7g5-shdwc
+    then:
+      properties:
+        atmel,wakeup-rtt-timer:
+          description: enable real-time timer wake-up
+          type: boolean
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    shdwc: poweroff@f8048010 {
+        compatible = "atmel,sama5d2-shdwc";
+        reg = <0xf8048010 0x10>;
+        clocks = <&clk32k>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        atmel,wakeup-rtc-timer;
+        debounce-delay-us = <976>;
+
+        input@0 {
+            reg = <0>;
+        };
+
+        input@1 {
+            reg = <1>;
+            atmel,wakeup-active-high;
+        };
+    };
+
+...