diff mbox series

[v2,1/4] dt-bindings: rtc: ingenic: Rework compatible strings and add #clock-cells

Message ID 20221028225519.89210-2-paul@crapouillou.net (mailing list archive)
State Handled Elsewhere
Headers show
Series rtc: ingenic: various updates | expand

Commit Message

Paul Cercueil Oct. 28, 2022, 10:55 p.m. UTC
The RTC in the JZ4770 is compatible with the JZ4760, but has an extra
register that permits to configure the behaviour of the CLK32K pin. The
same goes for the RTC in the JZ4780.

Therefore, the ingenic,jz4770-rtc and ingenic,jz4780-rtc strings do not
fall back anymore to ingenic,jz4760-rtc. The ingenic,jz4780-rtc string
now falls back to the ingenic,jz4770-rtc string.

Additionally, since the RTCs in the JZ4770 and JZ4780 support outputting
the input oscillator's clock to the CLK32K pin, the RTC node is now also
a clock provider on these SoCs, so a #clock-cells property is added.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

 v2: - add constraint on which SoCs can have the #clock-cells property
     - add JZ4780 example which has a #clock-cells

 .../devicetree/bindings/rtc/ingenic,rtc.yaml  | 32 +++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

Comments

Rob Herring (Arm) Oct. 31, 2022, 7:23 p.m. UTC | #1
On Fri, Oct 28, 2022 at 11:55:16PM +0100, Paul Cercueil wrote:
> The RTC in the JZ4770 is compatible with the JZ4760, but has an extra
> register that permits to configure the behaviour of the CLK32K pin. The
> same goes for the RTC in the JZ4780.
> 
> Therefore, the ingenic,jz4770-rtc and ingenic,jz4780-rtc strings do not
> fall back anymore to ingenic,jz4760-rtc. The ingenic,jz4780-rtc string
> now falls back to the ingenic,jz4770-rtc string.

This is a compatibility mess. There is no driver support in v6.1-rc for 
ingenic,jz4770-rtc, so a new DT would not work with existing kernels. It 
sounds like you need 3 compatibles for 4780.

> 
> Additionally, since the RTCs in the JZ4770 and JZ4780 support outputting
> the input oscillator's clock to the CLK32K pin, the RTC node is now also
> a clock provider on these SoCs, so a #clock-cells property is added.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> 
>  v2: - add constraint on which SoCs can have the #clock-cells property
>      - add JZ4780 example which has a #clock-cells
> 
>  .../devicetree/bindings/rtc/ingenic,rtc.yaml  | 32 +++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
Paul Cercueil Oct. 31, 2022, 7:40 p.m. UTC | #2
Hi Rob,

Le lun. 31 oct. 2022 à 14:23:41 -0500, Rob Herring <robh@kernel.org> a 
écrit :
> On Fri, Oct 28, 2022 at 11:55:16PM +0100, Paul Cercueil wrote:
>>  The RTC in the JZ4770 is compatible with the JZ4760, but has an 
>> extra
>>  register that permits to configure the behaviour of the CLK32K pin. 
>> The
>>  same goes for the RTC in the JZ4780.
>> 
>>  Therefore, the ingenic,jz4770-rtc and ingenic,jz4780-rtc strings do 
>> not
>>  fall back anymore to ingenic,jz4760-rtc. The ingenic,jz4780-rtc 
>> string
>>  now falls back to the ingenic,jz4770-rtc string.
> 
> This is a compatibility mess. There is no driver support in v6.1-rc 
> for
> ingenic,jz4770-rtc, so a new DT would not work with existing kernels. 
> It
> sounds like you need 3 compatibles for 4780.

Do newer DTs need to work with older kernels? I always assumed the 
compatibility was only enforced for the other way around.

If that's the case though, I guess I can enforce 3 compatibles for 4780 
and 2 for 4770 like  you suggested.

Cheers,
-Paul

>> 
>>  Additionally, since the RTCs in the JZ4770 and JZ4780 support 
>> outputting
>>  the input oscillator's clock to the CLK32K pin, the RTC node is now 
>> also
>>  a clock provider on these SoCs, so a #clock-cells property is added.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>> 
>>   v2: - add constraint on which SoCs can have the #clock-cells 
>> property
>>       - add JZ4780 example which has a #clock-cells
>> 
>>   .../devicetree/bindings/rtc/ingenic,rtc.yaml  | 32 
>> +++++++++++++++++--
>>   1 file changed, 30 insertions(+), 2 deletions(-)
Rob Herring (Arm) Nov. 1, 2022, 5:32 p.m. UTC | #3
On Mon, Oct 31, 2022 at 2:41 PM Paul Cercueil <paul@crapouillou.net> wrote:
>
> Hi Rob,
>
> Le lun. 31 oct. 2022 à 14:23:41 -0500, Rob Herring <robh@kernel.org> a
> écrit :
> > On Fri, Oct 28, 2022 at 11:55:16PM +0100, Paul Cercueil wrote:
> >>  The RTC in the JZ4770 is compatible with the JZ4760, but has an
> >> extra
> >>  register that permits to configure the behaviour of the CLK32K pin.
> >> The
> >>  same goes for the RTC in the JZ4780.
> >>
> >>  Therefore, the ingenic,jz4770-rtc and ingenic,jz4780-rtc strings do
> >> not
> >>  fall back anymore to ingenic,jz4760-rtc. The ingenic,jz4780-rtc
> >> string
> >>  now falls back to the ingenic,jz4770-rtc string.
> >
> > This is a compatibility mess. There is no driver support in v6.1-rc
> > for
> > ingenic,jz4770-rtc, so a new DT would not work with existing kernels.
> > It
> > sounds like you need 3 compatibles for 4780.
>
> Do newer DTs need to work with older kernels? I always assumed the
> compatibility was only enforced for the other way around.

For a stable platform, yes. Would you want a firmware update carrying
the DT to break an existing OS install?

Compatibility either way ultimately is up to the platform whether you
care or not. I just ask that the commit msg spell that out. In this
case, it is easily avoided.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml b/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
index b235b2441997..d63bb727cee5 100644
--- a/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
@@ -11,6 +11,16 @@  maintainers:
 
 allOf:
   - $ref: rtc.yaml#
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              enum:
+                - ingenic,jz4770-rtc
+    then:
+      properties:
+        "#clock-cells": false
 
 properties:
   compatible:
@@ -18,14 +28,14 @@  properties:
       - enum:
           - ingenic,jz4740-rtc
           - ingenic,jz4760-rtc
+          - ingenic,jz4770-rtc
       - items:
           - const: ingenic,jz4725b-rtc
           - const: ingenic,jz4740-rtc
       - items:
           - enum:
-              - ingenic,jz4770-rtc
               - ingenic,jz4780-rtc
-          - const: ingenic,jz4760-rtc
+          - const: ingenic,jz4770-rtc
 
   reg:
     maxItems: 1
@@ -39,6 +49,9 @@  properties:
   clock-names:
     const: rtc
 
+  "#clock-cells":
+    const: 0
+
   system-power-controller:
     description: |
       Indicates that the RTC is responsible for powering OFF
@@ -83,3 +96,18 @@  examples:
       clocks = <&cgu JZ4740_CLK_RTC>;
       clock-names = "rtc";
     };
+
+  - |
+    #include <dt-bindings/clock/ingenic,jz4780-cgu.h>
+    rtc: rtc@10003000 {
+      compatible = "ingenic,jz4780-rtc", "ingenic,jz4770-rtc";
+      reg = <0x10003000 0x4c>;
+
+      interrupt-parent = <&intc>;
+      interrupts = <32>;
+
+      clocks = <&cgu JZ4780_CLK_RTCLK>;
+      clock-names = "rtc";
+
+      #clock-cells = <0>;
+    };