[PATCH v2 1/1] dt-bindings: rtc: nxp,pcf85063: Convert to DT schema

[PATCH v2 1/1] dt-bindings: rtc: nxp,pcf85063: Convert to DT schema

[Alexander Stein]

Convert the NXP PCF85063 RTC binding to DT schema format.

Add 'interrupts' and 'wakeup-source' as this device has an interrupt
which was not documented, but is in use.
'clock-output-names' and '#clock-cells' are added as well, those were
probably missed when adding clkout support in commit 8c229ab6048b
("rtc: pcf85063: Add pcf85063 clkout control to common clock framework")

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Krzysztof, thanks for your review.

Changes in v2:
* Sorted compatible list
* Mentioned new #clock-cells and clock-output-names properties in commit message
* Removed 'interrupt-names', not needed/used anyway
* Fixed quartz-load-femtofarads defintion/description

 .../devicetree/bindings/rtc/nxp,pcf85063.txt  | 32 --------
 .../devicetree/bindings/rtc/nxp,pcf85063.yaml | 73 +++++++++++++++++++
 2 files changed, 73 insertions(+), 32 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt
 create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf85063.yaml

diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt b/Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt
deleted file mode 100644
index 217b7cd06c11..000000000000
--- a/Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt
+++ /dev/null
@@ -1,32 +0,0 @@
-* NXP PCF85063 Real Time Clock
-
-Required properties:
-- compatible: Should one of contain:
-	"nxp,pca85073a",
-	"nxp,pcf85063",
-	"nxp,pcf85063a",
-	"nxp,pcf85063tp",
-	"microcrystal,rv8263"
-- reg: I2C address for chip.
-
-Optional property:
-- quartz-load-femtofarads: The capacitive load of the quartz(x-tal),
-  expressed in femto Farad (fF). Valid values are 7000 and 12500.
-  Default value (if no value is specified) is 7000fF.
-
-Optional child node:
-- clock: Provide this if the square wave pin is used as boot-enabled fixed clock.
-
-Example:
-
-pcf85063: rtc@51 {
-	compatible = "nxp,pcf85063";
-	reg = <0x51>;
-	quartz-load-femtofarads = <12500>;
-
-		clock {
-			compatible = "fixed-clock";
-			#clock-cells = <0>;
-			clock-frequency = <32768>;
-		};
-};
diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85063.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf85063.yaml
new file mode 100644
index 000000000000..bf86c9786b0f
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85063.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/nxp,pcf85063.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP PCF85063 Real Time Clock
+
+maintainers:
+  - Alexander Stein <alexander.stein@ew.tq-group.com>
+
+allOf:
+  - $ref: rtc.yaml#
+
+properties:
+  compatible:
+    enum:
+      - microcrystal,rv8263
+      - nxp,pcf85063
+      - nxp,pcf85063a
+      - nxp,pcf85063tp
+      - nxp,pca85073a
+
+  reg:
+    maxItems: 1
+
+  "#clock-cells":
+    const: 0
+
+  clock-output-names:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  quartz-load-femtofarads:
+    description:
+      The capacitive load of the quartz(x-tal).
+    enum: [7000, 12500]
+    default: 7000
+
+  clock:
+    $ref: /schemas/clock/fixed-clock.yaml
+    description:
+      Provide this if the square wave pin is used as boot-enabled
+      fixed clock.
+
+  wakeup-source: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc@51 {
+          compatible = "nxp,pcf85063";
+          reg = <0x51>;
+          quartz-load-femtofarads = <12500>;
+
+          clock {
+            compatible = "fixed-clock";
+            #clock-cells = <0>;
+            clock-frequency = <32768>;
+          };
+        };
+      };
-- 
2.25.1

Re: [PATCH v2 1/1] dt-bindings: rtc: nxp,pcf85063: Convert to DT schema

[Krzysztof Kozlowski]

On 21/07/2022 15:33, Alexander Stein wrote:
> Convert the NXP PCF85063 RTC binding to DT schema format.
> 
> Add 'interrupts' and 'wakeup-source' as this device has an interrupt
> which was not documented, but is in use.
> 'clock-output-names' and '#clock-cells' are added as well, those were
> probably missed when adding clkout support in commit 8c229ab6048b
> ("rtc: pcf85063: Add pcf85063 clkout control to common clock framework")

Thanks for adding it here, this sounds fine but brought my attention to
interrupts and quartz-load. It seems that only rv8263 supports
interrupts. In the same time rv8263 work only with 7000
quartz-load-femtofarads.

If that's correct, you need to put "allOf" after "required" and inside
"if:then:" restricting it. For rv8263 interrupts:true and quartz as
const 7000, for else: interrupts:false.

> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Krzysztof, thanks for your review.
> 
> Changes in v2:
> * Sorted compatible list
> * Mentioned new #clock-cells and clock-output-names properties in commit message
> * Removed 'interrupt-names', not needed/used anyway
> * Fixed quartz-load-femtofarads defintion/description
> 
>  .../devicetree/bindings/rtc/nxp,pcf85063.txt  | 32 --------
>  .../devicetree/bindings/rtc/nxp,pcf85063.yaml | 73 +++++++++++++++++++
>  2 files changed, 73 insertions(+), 32 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt
>  create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf85063.yaml
> 

Best regards,
Krzysztof

Re: [PATCH v2 1/1] dt-bindings: rtc: nxp, pcf85063: Convert to DT schema

[Alexander Stein]

Hello Krzysztof,

thanks for your feedback.

Am Donnerstag, 21. Juli 2022, 15:43:13 CEST schrieb Krzysztof Kozlowski:
> On 21/07/2022 15:33, Alexander Stein wrote:
> > Convert the NXP PCF85063 RTC binding to DT schema format.
> > 
> > Add 'interrupts' and 'wakeup-source' as this device has an interrupt
> > which was not documented, but is in use.
> > 'clock-output-names' and '#clock-cells' are added as well, those were
> > probably missed when adding clkout support in commit 8c229ab6048b
> > ("rtc: pcf85063: Add pcf85063 clkout control to common clock framework")
> 
> Thanks for adding it here, this sounds fine but brought my attention to
> interrupts and quartz-load. It seems that only rv8263 supports
> interrupts. In the same time rv8263 work only with 7000
> quartz-load-femtofarads.
> 
> If that's correct, you need to put "allOf" after "required" and inside
> "if:then:" restricting it. For rv8263 interrupts:true and quartz as
> const 7000, for else: interrupts:false.

It is slightly different. In all the datasheets I found there was an IRQ pin, 
so this applies to all models, although only some of them (PCF85063A, 
PCF85073A and RV8263) support alarms, which is what Linux cares for right now. 
But this is handles in the driver already.
quartz-load-femtofarads does not apply to RV8263, because it has no OSCI pins 
at all but uses an onboard oscillator. See commit 5b3a3ade0293 ("rtc: 
pcf85063: add Micro Crystal RV8263 support") for that. But this also handled 
in the driver already.
Apart from that apparently only PCF85063 has a fixed quartz-load of 7pF, the 
other types supported can have either 7 oder 12.5 pF.

Best regards,
Alexander

Re: [PATCH v2 1/1] dt-bindings: rtc: nxp, pcf85063: Convert to DT schema

[Krzysztof Kozlowski]

On 22/07/2022 08:02, Alexander Stein wrote:
> Hello Krzysztof,
> 
> thanks for your feedback.
> 
> Am Donnerstag, 21. Juli 2022, 15:43:13 CEST schrieb Krzysztof Kozlowski:
>> On 21/07/2022 15:33, Alexander Stein wrote:
>>> Convert the NXP PCF85063 RTC binding to DT schema format.
>>>
>>> Add 'interrupts' and 'wakeup-source' as this device has an interrupt
>>> which was not documented, but is in use.
>>> 'clock-output-names' and '#clock-cells' are added as well, those were
>>> probably missed when adding clkout support in commit 8c229ab6048b
>>> ("rtc: pcf85063: Add pcf85063 clkout control to common clock framework")
>>
>> Thanks for adding it here, this sounds fine but brought my attention to
>> interrupts and quartz-load. It seems that only rv8263 supports
>> interrupts. In the same time rv8263 work only with 7000
>> quartz-load-femtofarads.
>>
>> If that's correct, you need to put "allOf" after "required" and inside
>> "if:then:" restricting it. For rv8263 interrupts:true and quartz as
>> const 7000, for else: interrupts:false.
> 
> It is slightly different. In all the datasheets I found there was an IRQ pin, 
> so this applies to all models, although only some of them (PCF85063A, 
> PCF85073A and RV8263) support alarms, which is what Linux cares for right now. 
> But this is handles in the driver already.

OK, this is fine then.

> quartz-load-femtofarads does not apply to RV8263, because it has no OSCI pins 
> at all but uses an onboard oscillator. See commit 5b3a3ade0293 ("rtc: 
> pcf85063: add Micro Crystal RV8263 support") for that. But this also handled 
> in the driver already.

This is what I was based on, so the quartz-load-femtofarads should not
be even allowed for RV8263.

> Apart from that apparently only PCF85063 has a fixed quartz-load of 7pF, the 
> other types supported can have either 7 oder 12.5 pF.

...and for PCF85063 this should be fixed to 7.

Best regards,
Krzysztof