[PATCH v3 00/12] Add RTC support for the Renesas RZ/G3S SoC

[PATCH v3 00/12] Add RTC support for the Renesas RZ/G3S SoC

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Hi,

On the Renesas RZ/G3S SoC the RTC clock is provided by the VBATTB
IP. A 32 KHz crystall oscillator could be connected to the VBATTB
input pins. The logic to control this clock (and pass it to RTC)
is inside the VBATTB IP. For this, the clk-vbattb driver was added
(patches 01-04/12).

Patches 05-06/12 add the RTC driver.

Patches 07-10/12 update the device trees with proper nodes to enable RTC.

Patches 11-12/12 enable proper config flags for RTC to work on RZ/G3S SoC.

Merge strategy, if any:
- clock patches (01-04/12) need to go though the same tree because of
  patch 04/12 using the devm_clk_hw_register_gate_parent_hw() introduced
  in patch 03/12
- RTC patches (03-04/12) can go though RTC tree
- DTS and defconfig patches can go though Renesas tree

Thank you,
Claudiu Beznea

Changes in v3:
- dropped patches "mfd: renesas-vbattb: Add a MFD driver for the Renesas
  VBATTB IP"
- added patches:
-- dt-bindings: clock: r9a08g045-vbattb: Add clock IDs for
   the VBATTB controller
-- clk: linux/clk-provider.h: Add devm_clk_hw_register_gate_parent_hw()
- moved Documentation/devicetree/bindings/mfd/renesas,r9a08g045-vbattb.yaml
  to Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
- addressed review comments
- per patch changes are listed in individual patches

Changes in v2:
- dropped patch "clk: renesas: r9a08g045: Add clock, reset and power domain
  support for the VBATTB IP" as it was already integrated
- kept only a documentation file for both VBATT MFD and clock drivers as
  suggested
- addressed review comments
- used cleanup.h lock helpers
- update startup sequence for the RTC driver
- switch to 24 hours mode on the RTC driver
- fixed range for the RTC driver
- added a generic compatible for the RTC driver as this will also be
  used by RZ/V2H
- used clkin/xin clock names for the VBATTB clock driver to determine
  if bypass should be configured on registers instead of having
  dedicated DT property
- added mfd driver for VBATTB
- updated Kconfig flag names to include vendor name
- removed DT node labels from Documentation files
- used items to describe the interrupts and clocks

Claudiu Beznea (12):
  dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB
  dt-bindings: clock: r9a08g045-vbattb: Add clock IDs for the VBATTB
    controller
  clk: linux/clk-provider.h: Add devm_clk_hw_register_gate_parent_hw()
  clk: renesas: clk-vbattb: Add VBATTB clock driver
  dt-bindings: rtc: renesas,rzg3s-rtc: Document the Renesas RTCA-3 IP
  rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S
    SoC
  arm64: dts: renesas: r9a08g045: Add VBATTB node
  arm64: dts: renesas: r9a08g045: Add RTC node
  arm64: dts: renesas: rzg3s-smarc-som: Enable VBATTB
  arm64: dts: renesas: rzg3s-smarc-som: Enable RTC
  arm64: defconfig: Enable VBATTB clock
  arm64: defconfig: Enable Renesas RTCA-3 flag

 .../clock/renesas,r9a08g045-vbattb.yaml       |  81 ++
 .../bindings/rtc/renesas,rz-rtca3.yaml        |  86 ++
 MAINTAINERS                                   |   8 +
 arch/arm64/boot/dts/renesas/r9a08g045.dtsi    |  36 +
 .../boot/dts/renesas/rzg3s-smarc-som.dtsi     |  13 +
 arch/arm64/configs/defconfig                  |   2 +
 drivers/clk/renesas/Kconfig                   |   4 +
 drivers/clk/renesas/Makefile                  |   1 +
 drivers/clk/renesas/clk-vbattb.c              | 204 ++++
 drivers/rtc/Kconfig                           |  10 +
 drivers/rtc/Makefile                          |   1 +
 drivers/rtc/rtc-renesas-rtca3.c               | 892 ++++++++++++++++++
 include/dt-bindings/clock/r9a08g045-vbattb.h  |  13 +
 include/linux/clk-provider.h                  |  18 +
 14 files changed, 1369 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
 create mode 100644 Documentation/devicetree/bindings/rtc/renesas,rz-rtca3.yaml
 create mode 100644 drivers/clk/renesas/clk-vbattb.c
 create mode 100644 drivers/rtc/rtc-renesas-rtca3.c
 create mode 100644 include/dt-bindings/clock/r9a08g045-vbattb.h

-- 
2.39.2

[PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC,
the tamper detector and a small general usage memory of 128B. Add
documentation for it.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- moved the file to clock dt bindings directory as it is the
  only functionality supported at the moment; the other functionalities
  (tamper detector, SRAM) are offered though register spreaded
  though the address space of the VBATTB IP and not actually
  individual devices; the other functionalities are not
  planned to be supported soon and if they will be I think they
  fit better on auxiliary bus than MFD
- dropped interrupt names as requested in the review process
- dropped the inner node for clock controller
- added #clock-cells
- added rtx clock
- updated description for renesas,vbattb-load-nanofarads
- included dt-bindings/interrupt-controller/irq.h in examples section

Changes in v2:
- changed file name and compatible
- updated title, description sections
- added clock controller part documentation and drop dedicated file
  for it included in v1
- used items to describe interrupts, interrupt-names, clocks, clock-names,
  resets
- dropped node labels and status
- updated clock-names for clock controller to cope with the new
  logic on detecting the necessity to setup bypass

 .../clock/renesas,r9a08g045-vbattb.yaml       | 81 +++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml

diff --git a/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
new file mode 100644
index 000000000000..29df0e01fae5
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/renesas,r9a08g045-vbattb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas Battery Backup Function (VBATTB)
+
+description:
+  Renesas VBATTB is an always on powered module (backed by battery) which
+  controls the RTC clock (VBATTCLK), tamper detection logic and a small
+  general usage memory (128B).
+
+maintainers:
+  - Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
+
+properties:
+  compatible:
+    const: renesas,r9a08g045-vbattb
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: tamper detector interrupt
+
+  clocks:
+    items:
+      - description: VBATTB module clock
+      - description: RTC input clock (crystal oscillator or external clock device)
+
+  clock-names:
+    items:
+      - const: bclk
+      - const: rtx
+
+  '#clock-cells':
+    const: 1
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    items:
+      - description: VBATTB module reset
+
+  renesas,vbattb-load-nanofarads:
+    description: load capacitance of the on board crystal oscillator
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 4000, 7000, 9000, 12500 ]
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - '#clock-cells'
+  - power-domains
+  - resets
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r9a08g045-cpg.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    vbattb@1005c000 {
+        compatible = "renesas,r9a08g045-vbattb";
+        reg = <0x1005c000 0x1000>;
+        interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb_xtal>;
+        clock-names = "bclk", "rtx";
+        #clock-cells = <1>;
+        power-domains = <&cpg>;
+        resets = <&cpg R9A08G045_VBAT_BRESETN>;
+        renesas,vbattb-load-nanofarads = <12500>;
+    };
-- 
2.39.2

Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[Rob Herring]

On Fri, Aug 30, 2024 at 04:02:07PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC,
> the tamper detector and a small general usage memory of 128B. Add
> documentation for it.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
> 
> Changes in v3:
> - moved the file to clock dt bindings directory as it is the
>   only functionality supported at the moment; the other functionalities
>   (tamper detector, SRAM) are offered though register spreaded
>   though the address space of the VBATTB IP and not actually
>   individual devices; the other functionalities are not
>   planned to be supported soon and if they will be I think they
>   fit better on auxiliary bus than MFD
> - dropped interrupt names as requested in the review process
> - dropped the inner node for clock controller
> - added #clock-cells
> - added rtx clock
> - updated description for renesas,vbattb-load-nanofarads
> - included dt-bindings/interrupt-controller/irq.h in examples section
> 
> Changes in v2:
> - changed file name and compatible
> - updated title, description sections
> - added clock controller part documentation and drop dedicated file
>   for it included in v1
> - used items to describe interrupts, interrupt-names, clocks, clock-names,
>   resets
> - dropped node labels and status
> - updated clock-names for clock controller to cope with the new
>   logic on detecting the necessity to setup bypass
> 
>  .../clock/renesas,r9a08g045-vbattb.yaml       | 81 +++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
> new file mode 100644
> index 000000000000..29df0e01fae5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/renesas,r9a08g045-vbattb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas Battery Backup Function (VBATTB)
> +
> +description:
> +  Renesas VBATTB is an always on powered module (backed by battery) which
> +  controls the RTC clock (VBATTCLK), tamper detection logic and a small
> +  general usage memory (128B).
> +
> +maintainers:
> +  - Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> +
> +properties:
> +  compatible:
> +    const: renesas,r9a08g045-vbattb
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: tamper detector interrupt
> +
> +  clocks:
> +    items:
> +      - description: VBATTB module clock
> +      - description: RTC input clock (crystal oscillator or external clock device)
> +
> +  clock-names:
> +    items:
> +      - const: bclk
> +      - const: rtx
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    items:
> +      - description: VBATTB module reset
> +
> +  renesas,vbattb-load-nanofarads:

Use defined units, don't add your own. So -picofarads should work for 
you.

> +    description: load capacitance of the on board crystal oscillator
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [ 4000, 7000, 9000, 12500 ]
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +  - power-domains
> +  - resets
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/r9a08g045-cpg.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    vbattb@1005c000 {

clock-controller@...

> +        compatible = "renesas,r9a08g045-vbattb";
> +        reg = <0x1005c000 0x1000>;
> +        interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb_xtal>;
> +        clock-names = "bclk", "rtx";
> +        #clock-cells = <1>;
> +        power-domains = <&cpg>;
> +        resets = <&cpg R9A08G045_VBAT_BRESETN>;
> +        renesas,vbattb-load-nanofarads = <12500>;
> +    };
> -- 
> 2.39.2
> 

Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[Alexandre Belloni]

On 30/08/2024 12:46:33-0500, Rob Herring wrote:
> On Fri, Aug 30, 2024 at 04:02:07PM +0300, Claudiu wrote:
> > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > 
> > The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC,
> > the tamper detector and a small general usage memory of 128B. Add
> > documentation for it.
> > 
> > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > ---
> > 
> > Changes in v3:
> > - moved the file to clock dt bindings directory as it is the
> >   only functionality supported at the moment; the other functionalities
> >   (tamper detector, SRAM) are offered though register spreaded
> >   though the address space of the VBATTB IP and not actually
> >   individual devices; the other functionalities are not
> >   planned to be supported soon and if they will be I think they
> >   fit better on auxiliary bus than MFD
> > - dropped interrupt names as requested in the review process
> > - dropped the inner node for clock controller
> > - added #clock-cells
> > - added rtx clock
> > - updated description for renesas,vbattb-load-nanofarads
> > - included dt-bindings/interrupt-controller/irq.h in examples section
> > 
> > Changes in v2:
> > - changed file name and compatible
> > - updated title, description sections
> > - added clock controller part documentation and drop dedicated file
> >   for it included in v1
> > - used items to describe interrupts, interrupt-names, clocks, clock-names,
> >   resets
> > - dropped node labels and status
> > - updated clock-names for clock controller to cope with the new
> >   logic on detecting the necessity to setup bypass
> > 
> >  .../clock/renesas,r9a08g045-vbattb.yaml       | 81 +++++++++++++++++++
> >  1 file changed, 81 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
> > new file mode 100644
> > index 000000000000..29df0e01fae5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
> > @@ -0,0 +1,81 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/renesas,r9a08g045-vbattb.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas Battery Backup Function (VBATTB)
> > +
> > +description:
> > +  Renesas VBATTB is an always on powered module (backed by battery) which
> > +  controls the RTC clock (VBATTCLK), tamper detection logic and a small
> > +  general usage memory (128B).
> > +
> > +maintainers:
> > +  - Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: renesas,r9a08g045-vbattb
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    items:
> > +      - description: tamper detector interrupt
> > +
> > +  clocks:
> > +    items:
> > +      - description: VBATTB module clock
> > +      - description: RTC input clock (crystal oscillator or external clock device)
> > +
> > +  clock-names:
> > +    items:
> > +      - const: bclk
> > +      - const: rtx
> > +
> > +  '#clock-cells':
> > +    const: 1
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    items:
> > +      - description: VBATTB module reset
> > +
> > +  renesas,vbattb-load-nanofarads:
> 
> Use defined units, don't add your own. So -picofarads should work for 
> you.

We have a generic quartz-load-femtofarads property for RTCs which is
what you define because the driver has VBATTB_XOSCCR_XSEL_4_PF which I
guess is 4 pF which is 0.004 nF and 4000 fF.

> 
> > +    description: load capacitance of the on board crystal oscillator
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [ 4000, 7000, 9000, 12500 ]
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - '#clock-cells'
> > +  - power-domains
> > +  - resets
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/r9a08g045-cpg.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    vbattb@1005c000 {
> 
> clock-controller@...
> 
> > +        compatible = "renesas,r9a08g045-vbattb";
> > +        reg = <0x1005c000 0x1000>;
> > +        interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> > +        clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb_xtal>;
> > +        clock-names = "bclk", "rtx";
> > +        #clock-cells = <1>;
> > +        power-domains = <&cpg>;
> > +        resets = <&cpg R9A08G045_VBAT_BRESETN>;
> > +        renesas,vbattb-load-nanofarads = <12500>;
> > +    };
> > -- 
> > 2.39.2
> > 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[claudiu beznea]

On 31.08.2024 01:06, Alexandre Belloni wrote:
> On 30/08/2024 12:46:33-0500, Rob Herring wrote:
>> On Fri, Aug 30, 2024 at 04:02:07PM +0300, Claudiu wrote:
>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>
>>> The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC,
>>> the tamper detector and a small general usage memory of 128B. Add
>>> documentation for it.
>>>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>> ---
>>>
>>> Changes in v3:
>>> - moved the file to clock dt bindings directory as it is the
>>>   only functionality supported at the moment; the other functionalities
>>>   (tamper detector, SRAM) are offered though register spreaded
>>>   though the address space of the VBATTB IP and not actually
>>>   individual devices; the other functionalities are not
>>>   planned to be supported soon and if they will be I think they
>>>   fit better on auxiliary bus than MFD
>>> - dropped interrupt names as requested in the review process
>>> - dropped the inner node for clock controller
>>> - added #clock-cells
>>> - added rtx clock
>>> - updated description for renesas,vbattb-load-nanofarads
>>> - included dt-bindings/interrupt-controller/irq.h in examples section
>>>
>>> Changes in v2:
>>> - changed file name and compatible
>>> - updated title, description sections
>>> - added clock controller part documentation and drop dedicated file
>>>   for it included in v1
>>> - used items to describe interrupts, interrupt-names, clocks, clock-names,
>>>   resets
>>> - dropped node labels and status
>>> - updated clock-names for clock controller to cope with the new
>>>   logic on detecting the necessity to setup bypass
>>>
>>>  .../clock/renesas,r9a08g045-vbattb.yaml       | 81 +++++++++++++++++++
>>>  1 file changed, 81 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
>>> new file mode 100644
>>> index 000000000000..29df0e01fae5
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
>>> @@ -0,0 +1,81 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/renesas,r9a08g045-vbattb.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Renesas Battery Backup Function (VBATTB)
>>> +
>>> +description:
>>> +  Renesas VBATTB is an always on powered module (backed by battery) which
>>> +  controls the RTC clock (VBATTCLK), tamper detection logic and a small
>>> +  general usage memory (128B).
>>> +
>>> +maintainers:
>>> +  - Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: renesas,r9a08g045-vbattb
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    items:
>>> +      - description: tamper detector interrupt
>>> +
>>> +  clocks:
>>> +    items:
>>> +      - description: VBATTB module clock
>>> +      - description: RTC input clock (crystal oscillator or external clock device)
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: bclk
>>> +      - const: rtx
>>> +
>>> +  '#clock-cells':
>>> +    const: 1
>>> +
>>> +  power-domains:
>>> +    maxItems: 1
>>> +
>>> +  resets:
>>> +    items:
>>> +      - description: VBATTB module reset
>>> +
>>> +  renesas,vbattb-load-nanofarads:
>>
>> Use defined units, don't add your own. So -picofarads should work for 
>> you.
> 
> We have a generic quartz-load-femtofarads property for RTCs which is
> what you define because the driver has VBATTB_XOSCCR_XSEL_4_PF which I
> guess is 4 pF which is 0.004 nF and 4000 fF.

I'll use this one in the next version.

Thank you for your review,
Claudiu Beznea

> 
>>
>>> +    description: load capacitance of the on board crystal oscillator
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [ 4000, 7000, 9000, 12500 ]
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - interrupts
>>> +  - clocks
>>> +  - clock-names
>>> +  - '#clock-cells'
>>> +  - power-domains
>>> +  - resets
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/clock/r9a08g045-cpg.h>
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +    #include <dt-bindings/interrupt-controller/irq.h>
>>> +
>>> +    vbattb@1005c000 {
>>
>> clock-controller@...
>>
>>> +        compatible = "renesas,r9a08g045-vbattb";
>>> +        reg = <0x1005c000 0x1000>;
>>> +        interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
>>> +        clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb_xtal>;
>>> +        clock-names = "bclk", "rtx";
>>> +        #clock-cells = <1>;
>>> +        power-domains = <&cpg>;
>>> +        resets = <&cpg R9A08G045_VBAT_BRESETN>;
>>> +        renesas,vbattb-load-nanofarads = <12500>;
>>> +    };
>>> -- 
>>> 2.39.2
>>>
> 

RE: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[Biju Das]

Hi Claudiu,

> -----Original Message-----
> From: Claudiu <claudiu.beznea@tuxon.dev>
> Sent: Friday, August 30, 2024 2:02 PM
> Subject: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB
> 
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC, the tamper detector and a small
> general usage memory of 128B. Add documentation for it.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
> 
> Changes in v3:
> - moved the file to clock dt bindings directory as it is the
>   only functionality supported at the moment; the other functionalities
>   (tamper detector, SRAM) are offered though register spreaded
>   though the address space of the VBATTB IP and not actually
>   individual devices; the other functionalities are not
>   planned to be supported soon and if they will be I think they
>   fit better on auxiliary bus than MFD
> - dropped interrupt names as requested in the review process
> - dropped the inner node for clock controller
> - added #clock-cells
> - added rtx clock
> - updated description for renesas,vbattb-load-nanofarads
> - included dt-bindings/interrupt-controller/irq.h in examples section
> 
> Changes in v2:
> - changed file name and compatible
> - updated title, description sections
> - added clock controller part documentation and drop dedicated file
>   for it included in v1
> - used items to describe interrupts, interrupt-names, clocks, clock-names,
>   resets
> - dropped node labels and status
> - updated clock-names for clock controller to cope with the new
>   logic on detecting the necessity to setup bypass
> 
>  .../clock/renesas,r9a08g045-vbattb.yaml       | 81 +++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
> b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
> new file mode 100644
> index 000000000000..29df0e01fae5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.y
> +++ aml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/renesas,r9a08g045-vbattb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas Battery Backup Function (VBATTB)
> +
> +description:
> +  Renesas VBATTB is an always on powered module (backed by battery)
> +which
> +  controls the RTC clock (VBATTCLK), tamper detection logic and a small
> +  general usage memory (128B).
> +
> +maintainers:
> +  - Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> +
> +properties:
> +  compatible:
> +    const: renesas,r9a08g045-vbattb
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: tamper detector interrupt
> +
> +  clocks:
> +    items:
> +      - description: VBATTB module clock
> +      - description: RTC input clock (crystal oscillator or external
> + clock device)
> +
> +  clock-names:
> +    items:
> +      - const: bclk
> +      - const: rtx
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  power-domains:
> +    maxItems: 1

Not sure, you need to document "PD_VBATT" power domain 
as per Table 41.2, this LSI supports 3 power domains(PD_ISOVCC, PD_VCC, PD_VBATT)

Power Mode PD_ISOVCC PD_VCC PD_VBATT
ALL_ON      ON          ON    ON
AWO         OFF         ON    ON
VBATT       OFF         OFF   ON
ALL_OFF     OFF         OFF   OFF

PD_VBATT domain is the area where the RTC/backup register is located, works on battery power when the power of
PD_VCC and PD_ISOVCC domain are turned off.

Cheers,
Biju

> +
> +  resets:
> +    items:
> +      - description: VBATTB module reset
> +
> +  renesas,vbattb-load-nanofarads:
> +    description: load capacitance of the on board crystal oscillator
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [ 4000, 7000, 9000, 12500 ]
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +  - power-domains
> +  - resets
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/r9a08g045-cpg.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    vbattb@1005c000 {
> +        compatible = "renesas,r9a08g045-vbattb";
> +        reg = <0x1005c000 0x1000>;
> +        interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb_xtal>;
> +        clock-names = "bclk", "rtx";
> +        #clock-cells = <1>;
> +        power-domains = <&cpg>;
> +        resets = <&cpg R9A08G045_VBAT_BRESETN>;
> +        renesas,vbattb-load-nanofarads = <12500>;
> +    };
> --
> 2.39.2
> 

Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[Geert Uytterhoeven]

Hi Biju,

On Tue, Sep 3, 2024 at 8:58 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > -----Original Message-----
> > From: Claudiu <claudiu.beznea@tuxon.dev>
> > Sent: Friday, August 30, 2024 2:02 PM
> > Subject: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB
> >
> > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >
> > The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC, the tamper detector and a small
> > general usage memory of 128B. Add documentation for it.
> >
> > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
> > +  power-domains:
> > +    maxItems: 1
>
> Not sure, you need to document "PD_VBATT" power domain
> as per Table 41.2, this LSI supports 3 power domains(PD_ISOVCC, PD_VCC, PD_VBATT)
>
> Power Mode PD_ISOVCC PD_VCC PD_VBATT
> ALL_ON      ON          ON    ON
> AWO         OFF         ON    ON
> VBATT       OFF         OFF   ON
> ALL_OFF     OFF         OFF   OFF
>
> PD_VBATT domain is the area where the RTC/backup register is located, works on battery power when the power of
> PD_VCC and PD_ISOVCC domain are turned off.

AFAIU, PD_VBATT cannot be controlled by the user, and is just on
if main or battery power is supplied. So I don't think there is a need
to describe it in DT.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[Biju Das]

Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Tuesday, September 3, 2024 8:23 AM
> Subject: Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB
> 
> Hi Biju,
> 
> On Tue, Sep 3, 2024 at 8:58 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > -----Original Message-----
> > > From: Claudiu <claudiu.beznea@tuxon.dev>
> > > Sent: Friday, August 30, 2024 2:02 PM
> > > Subject: [PATCH v3 01/12] dt-bindings: clock:
> > > renesas,r9a08g045-vbattb: Document VBATTB
> > >
> > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > >
> > > The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC,
> > > the tamper detector and a small general usage memory of 128B. Add documentation for it.
> > >
> > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbat
> > > +++ tb.yaml
> > > +  power-domains:
> > > +    maxItems: 1
> >
> > Not sure, you need to document "PD_VBATT" power domain as per Table
> > 41.2, this LSI supports 3 power domains(PD_ISOVCC, PD_VCC, PD_VBATT)
> >
> > Power Mode PD_ISOVCC PD_VCC PD_VBATT
> > ALL_ON      ON          ON    ON
> > AWO         OFF         ON    ON
> > VBATT       OFF         OFF   ON
> > ALL_OFF     OFF         OFF   OFF
> >
> > PD_VBATT domain is the area where the RTC/backup register is located,
> > works on battery power when the power of PD_VCC and PD_ISOVCC domain are turned off.
> 
> AFAIU, PD_VBATT cannot be controlled by the user, and is just on if main or battery power is supplied.
> So I don't think there is a need to describe it in DT.

OK, Just thought since DT is describing hardware, better to document this.

I am not an expert, So I agree with you.

Cheers,
Biju

RE: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[Biju Das]

Hi Geert,

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Tuesday, September 3, 2024 8:25 AM
> Subject: RE: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB
> 
> Hi Geert,
> 
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > Sent: Tuesday, September 3, 2024 8:23 AM
> > Subject: Re: [PATCH v3 01/12] dt-bindings: clock:
> > renesas,r9a08g045-vbattb: Document VBATTB
> >
> > Hi Biju,
> >
> > On Tue, Sep 3, 2024 at 8:58 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > -----Original Message-----
> > > > From: Claudiu <claudiu.beznea@tuxon.dev>
> > > > Sent: Friday, August 30, 2024 2:02 PM
> > > > Subject: [PATCH v3 01/12] dt-bindings: clock:
> > > > renesas,r9a08g045-vbattb: Document VBATTB
> > > >
> > > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > > >
> > > > The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for
> > > > RTC, the tamper detector and a small general usage memory of 128B. Add documentation for it.
> > > >
> > > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vb
> > > > +++ at
> > > > +++ tb.yaml
> > > > +  power-domains:
> > > > +    maxItems: 1
> > >
> > > Not sure, you need to document "PD_VBATT" power domain as per Table
> > > 41.2, this LSI supports 3 power domains(PD_ISOVCC, PD_VCC, PD_VBATT)
> > >
> > > Power Mode PD_ISOVCC PD_VCC PD_VBATT
> > > ALL_ON      ON          ON    ON
> > > AWO         OFF         ON    ON
> > > VBATT       OFF         OFF   ON
> > > ALL_OFF     OFF         OFF   OFF
> > >
> > > PD_VBATT domain is the area where the RTC/backup register is
> > > located, works on battery power when the power of PD_VCC and PD_ISOVCC domain are turned off.
> >
> > AFAIU, PD_VBATT cannot be controlled by the user, and is just on if main or battery power is
> supplied.

As per Figure 41.1 Power Domain and Power Supply,

If both PMIC PWS off, PD_VBATT is active, so looks like it is controllable
Outside Linux.

Cheers,
Biju

Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[claudiu beznea]

On 03.09.2024 09:58, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: Claudiu <claudiu.beznea@tuxon.dev>
>> Sent: Friday, August 30, 2024 2:02 PM
>> Subject: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB
>>
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC, the tamper detector and a small
>> general usage memory of 128B. Add documentation for it.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>
>> Changes in v3:
>> - moved the file to clock dt bindings directory as it is the
>>   only functionality supported at the moment; the other functionalities
>>   (tamper detector, SRAM) are offered though register spreaded
>>   though the address space of the VBATTB IP and not actually
>>   individual devices; the other functionalities are not
>>   planned to be supported soon and if they will be I think they
>>   fit better on auxiliary bus than MFD
>> - dropped interrupt names as requested in the review process
>> - dropped the inner node for clock controller
>> - added #clock-cells
>> - added rtx clock
>> - updated description for renesas,vbattb-load-nanofarads
>> - included dt-bindings/interrupt-controller/irq.h in examples section
>>
>> Changes in v2:
>> - changed file name and compatible
>> - updated title, description sections
>> - added clock controller part documentation and drop dedicated file
>>   for it included in v1
>> - used items to describe interrupts, interrupt-names, clocks, clock-names,
>>   resets
>> - dropped node labels and status
>> - updated clock-names for clock controller to cope with the new
>>   logic on detecting the necessity to setup bypass
>>
>>  .../clock/renesas,r9a08g045-vbattb.yaml       | 81 +++++++++++++++++++
>>  1 file changed, 81 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
>> b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
>> new file mode 100644
>> index 000000000000..29df0e01fae5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.y
>> +++ aml
>> @@ -0,0 +1,81 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/renesas,r9a08g045-vbattb.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Renesas Battery Backup Function (VBATTB)
>> +
>> +description:
>> +  Renesas VBATTB is an always on powered module (backed by battery)
>> +which
>> +  controls the RTC clock (VBATTCLK), tamper detection logic and a small
>> +  general usage memory (128B).
>> +
>> +maintainers:
>> +  - Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: renesas,r9a08g045-vbattb
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    items:
>> +      - description: tamper detector interrupt
>> +
>> +  clocks:
>> +    items:
>> +      - description: VBATTB module clock
>> +      - description: RTC input clock (crystal oscillator or external
>> + clock device)
>> +
>> +  clock-names:
>> +    items:
>> +      - const: bclk
>> +      - const: rtx
>> +
>> +  '#clock-cells':
>> +    const: 1
>> +
>> +  power-domains:
>> +    maxItems: 1
> 
> Not sure, you need to document "PD_VBATT" power domain 
> as per Table 41.2, this LSI supports 3 power domains(PD_ISOVCC, PD_VCC, PD_VBATT)
> 
> Power Mode PD_ISOVCC PD_VCC PD_VBATT
> ALL_ON      ON          ON    ON
> AWO         OFF         ON    ON
> VBATT       OFF         OFF   ON
> ALL_OFF     OFF         OFF   OFF
> 
> PD_VBATT domain is the area where the RTC/backup register is located, works on battery power when the power of
> PD_VCC and PD_ISOVCC domain are turned off.

In Linux, the CPG is the power domain provider for all the IPs in RZ/G3S
SoC (modeled though MSTOP CPG support). This is how it is currently
implemented.

Then groups of IPs are part of power domains PD_ISOVCC, PD_VCC, PD_VBATT.
These power domains are i2c controlled with the help of firmware (at least
at the moment).

From HW manual:
- PD_VCC domain always powered on area.

- PD_ISOVCC domain is the area where the power can be turned off.

- PD_VBATT domain is the area where the RTC/backup register is located,
  works on battery power when the power of
.

The power to these domains are controlled with the help of firmware. Linux
cannot do control itself as the CPU is in the PD_ISOVCC. If you look at
picture 41.3 Power mode transition [1] it is mentioned the relation b/w
these power domains (controlled by PMIC though firmware) and the supported
power saving modes: ALL_ON, AWO, VBATT.

Thank you,
Claudiu Beznea

[1] https://pasteboard.co/4ureEUnyCfV8.png

> 
> Cheers,
> Biju
> 
>> +
>> +  resets:
>> +    items:
>> +      - description: VBATTB module reset
>> +
>> +  renesas,vbattb-load-nanofarads:
>> +    description: load capacitance of the on board crystal oscillator
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [ 4000, 7000, 9000, 12500 ]
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - clocks
>> +  - clock-names
>> +  - '#clock-cells'
>> +  - power-domains
>> +  - resets
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/r9a08g045-cpg.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +    vbattb@1005c000 {
>> +        compatible = "renesas,r9a08g045-vbattb";
>> +        reg = <0x1005c000 0x1000>;
>> +        interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
>> +        clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb_xtal>;
>> +        clock-names = "bclk", "rtx";
>> +        #clock-cells = <1>;
>> +        power-domains = <&cpg>;
>> +        resets = <&cpg R9A08G045_VBAT_BRESETN>;
>> +        renesas,vbattb-load-nanofarads = <12500>;
>> +    };
>> --
>> 2.39.2
>>
> 

RE: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[Biju Das]

Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> Sent: Tuesday, September 3, 2024 8:28 AM
> Subject: Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB
> 
> 
> 
> On 03.09.2024 09:58, Biju Das wrote:
> > Hi Claudiu,
> >
> >> -----Original Message-----
> >> From: Claudiu <claudiu.beznea@tuxon.dev>
> >> Sent: Friday, August 30, 2024 2:02 PM
> >> Subject: [PATCH v3 01/12] dt-bindings: clock:
> >> renesas,r9a08g045-vbattb: Document VBATTB
> >>
> >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>
> >> The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC,
> >> the tamper detector and a small general usage memory of 128B. Add documentation for it.
> >>
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >> ---
> >>
> >> Changes in v3:
> >> - moved the file to clock dt bindings directory as it is the
> >>   only functionality supported at the moment; the other functionalities
> >>   (tamper detector, SRAM) are offered though register spreaded
> >>   though the address space of the VBATTB IP and not actually
> >>   individual devices; the other functionalities are not
> >>   planned to be supported soon and if they will be I think they
> >>   fit better on auxiliary bus than MFD
> >> - dropped interrupt names as requested in the review process
> >> - dropped the inner node for clock controller
> >> - added #clock-cells
> >> - added rtx clock
> >> - updated description for renesas,vbattb-load-nanofarads
> >> - included dt-bindings/interrupt-controller/irq.h in examples section
> >>
> >> Changes in v2:
> >> - changed file name and compatible
> >> - updated title, description sections
> >> - added clock controller part documentation and drop dedicated file
> >>   for it included in v1
> >> - used items to describe interrupts, interrupt-names, clocks, clock-names,
> >>   resets
> >> - dropped node labels and status
> >> - updated clock-names for clock controller to cope with the new
> >>   logic on detecting the necessity to setup bypass
> >>
> >>  .../clock/renesas,r9a08g045-vbattb.yaml       | 81 +++++++++++++++++++
> >>  1 file changed, 81 insertions(+)
> >>  create mode 100644
> >> Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.ya
> >> ml
> >> b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.ya
> >> ml
> >> new file mode 100644
> >> index 000000000000..29df0e01fae5
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbatt
> >> +++ b.y
> >> +++ aml
> >> @@ -0,0 +1,81 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> >> +---
> >> +$id:
> >> +http://devicetree.org/schemas/clock/renesas,r9a08g045-vbattb.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Renesas Battery Backup Function (VBATTB)
> >> +
> >> +description:
> >> +  Renesas VBATTB is an always on powered module (backed by battery)
> >> +which
> >> +  controls the RTC clock (VBATTCLK), tamper detection logic and a
> >> +small
> >> +  general usage memory (128B).
> >> +
> >> +maintainers:
> >> +  - Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >> +
> >> +properties:
> >> +  compatible:
> >> +    const: renesas,r9a08g045-vbattb
> >> +
> >> +  reg:
> >> +    maxItems: 1
> >> +
> >> +  interrupts:
> >> +    items:
> >> +      - description: tamper detector interrupt
> >> +
> >> +  clocks:
> >> +    items:
> >> +      - description: VBATTB module clock
> >> +      - description: RTC input clock (crystal oscillator or external
> >> + clock device)
> >> +
> >> +  clock-names:
> >> +    items:
> >> +      - const: bclk
> >> +      - const: rtx
> >> +
> >> +  '#clock-cells':
> >> +    const: 1
> >> +
> >> +  power-domains:
> >> +    maxItems: 1
> >
> > Not sure, you need to document "PD_VBATT" power domain as per Table
> > 41.2, this LSI supports 3 power domains(PD_ISOVCC, PD_VCC, PD_VBATT)
> >
> > Power Mode PD_ISOVCC PD_VCC PD_VBATT
> > ALL_ON      ON          ON    ON
> > AWO         OFF         ON    ON
> > VBATT       OFF         OFF   ON
> > ALL_OFF     OFF         OFF   OFF
> >
> > PD_VBATT domain is the area where the RTC/backup register is located,
> > works on battery power when the power of PD_VCC and PD_ISOVCC domain are turned off.
> 
> In Linux, the CPG is the power domain provider for all the IPs in RZ/G3S SoC (modeled though MSTOP CPG
> support). This is how it is currently implemented.
> 
> Then groups of IPs are part of power domains PD_ISOVCC, PD_VCC, PD_VBATT.
> These power domains are i2c controlled with the help of firmware (at least at the moment).
> 
> From HW manual:
> - PD_VCC domain always powered on area.
> 
> - PD_ISOVCC domain is the area where the power can be turned off.
> 
> - PD_VBATT domain is the area where the RTC/backup register is located,
>   works on battery power when the power of .
> 
> The power to these domains are controlled with the help of firmware. Linux cannot do control itself as
> the CPU is in the PD_ISOVCC. If you look at picture 41.3 Power mode transition [1] it is mentioned the
> relation b/w these power domains (controlled by PMIC though firmware) and the supported power saving
> modes: ALL_ON, AWO, VBATT.
> 

DT describes hardware. So, the question was, from that perspective, do we need to document PD_VBATT domain,
as it can be controlled outside linux??

Cheers,
Biju

Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[Geert Uytterhoeven]

Hi Biju,

On Tue, Sep 3, 2024 at 9:36 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > -----Original Message-----
> > From: claudiu beznea <claudiu.beznea@tuxon.dev>
> > Sent: Tuesday, September 3, 2024 8:28 AM
> > Subject: Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB
> >
> > On 03.09.2024 09:58, Biju Das wrote:
> > >> -----Original Message-----
> > >> From: Claudiu <claudiu.beznea@tuxon.dev>
> > >> Sent: Friday, August 30, 2024 2:02 PM
> > >> Subject: [PATCH v3 01/12] dt-bindings: clock:
> > >> renesas,r9a08g045-vbattb: Document VBATTB
> > >>
> > >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > >>
> > >> The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC,
> > >> the tamper detector and a small general usage memory of 128B. Add documentation for it.
> > >>
> > >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > >> ---
> > >>
> > >> Changes in v3:
> > >> - moved the file to clock dt bindings directory as it is the
> > >>   only functionality supported at the moment; the other functionalities
> > >>   (tamper detector, SRAM) are offered though register spreaded
> > >>   though the address space of the VBATTB IP and not actually
> > >>   individual devices; the other functionalities are not
> > >>   planned to be supported soon and if they will be I think they
> > >>   fit better on auxiliary bus than MFD
> > >> - dropped interrupt names as requested in the review process
> > >> - dropped the inner node for clock controller
> > >> - added #clock-cells
> > >> - added rtx clock
> > >> - updated description for renesas,vbattb-load-nanofarads
> > >> - included dt-bindings/interrupt-controller/irq.h in examples section
> > >>
> > >> Changes in v2:
> > >> - changed file name and compatible
> > >> - updated title, description sections
> > >> - added clock controller part documentation and drop dedicated file
> > >>   for it included in v1
> > >> - used items to describe interrupts, interrupt-names, clocks, clock-names,
> > >>   resets
> > >> - dropped node labels and status
> > >> - updated clock-names for clock controller to cope with the new
> > >>   logic on detecting the necessity to setup bypass
> > >>
> > >>  .../clock/renesas,r9a08g045-vbattb.yaml       | 81 +++++++++++++++++++
> > >>  1 file changed, 81 insertions(+)
> > >>  create mode 100644
> > >> Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
> > >>
> > >> diff --git
> > >> a/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.ya
> > >> ml
> > >> b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.ya
> > >> ml
> > >> new file mode 100644
> > >> index 000000000000..29df0e01fae5
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbatt
> > >> +++ b.y
> > >> +++ aml
> > >> @@ -0,0 +1,81 @@
> > >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > >> +---
> > >> +$id:
> > >> +http://devicetree.org/schemas/clock/renesas,r9a08g045-vbattb.yaml#
> > >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > >> +
> > >> +title: Renesas Battery Backup Function (VBATTB)
> > >> +
> > >> +description:
> > >> +  Renesas VBATTB is an always on powered module (backed by battery)
> > >> +which
> > >> +  controls the RTC clock (VBATTCLK), tamper detection logic and a
> > >> +small
> > >> +  general usage memory (128B).
> > >> +
> > >> +maintainers:
> > >> +  - Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > >> +
> > >> +properties:
> > >> +  compatible:
> > >> +    const: renesas,r9a08g045-vbattb
> > >> +
> > >> +  reg:
> > >> +    maxItems: 1
> > >> +
> > >> +  interrupts:
> > >> +    items:
> > >> +      - description: tamper detector interrupt
> > >> +
> > >> +  clocks:
> > >> +    items:
> > >> +      - description: VBATTB module clock
> > >> +      - description: RTC input clock (crystal oscillator or external
> > >> + clock device)
> > >> +
> > >> +  clock-names:
> > >> +    items:
> > >> +      - const: bclk
> > >> +      - const: rtx
> > >> +
> > >> +  '#clock-cells':
> > >> +    const: 1
> > >> +
> > >> +  power-domains:
> > >> +    maxItems: 1
> > >
> > > Not sure, you need to document "PD_VBATT" power domain as per Table
> > > 41.2, this LSI supports 3 power domains(PD_ISOVCC, PD_VCC, PD_VBATT)
> > >
> > > Power Mode PD_ISOVCC PD_VCC PD_VBATT
> > > ALL_ON      ON          ON    ON
> > > AWO         OFF         ON    ON
> > > VBATT       OFF         OFF   ON
> > > ALL_OFF     OFF         OFF   OFF
> > >
> > > PD_VBATT domain is the area where the RTC/backup register is located,
> > > works on battery power when the power of PD_VCC and PD_ISOVCC domain are turned off.
> >
> > In Linux, the CPG is the power domain provider for all the IPs in RZ/G3S SoC (modeled though MSTOP CPG
> > support). This is how it is currently implemented.
> >
> > Then groups of IPs are part of power domains PD_ISOVCC, PD_VCC, PD_VBATT.
> > These power domains are i2c controlled with the help of firmware (at least at the moment).
> >
> > From HW manual:
> > - PD_VCC domain always powered on area.
> >
> > - PD_ISOVCC domain is the area where the power can be turned off.
> >
> > - PD_VBATT domain is the area where the RTC/backup register is located,
> >   works on battery power when the power of .
> >
> > The power to these domains are controlled with the help of firmware. Linux cannot do control itself as
> > the CPU is in the PD_ISOVCC. If you look at picture 41.3 Power mode transition [1] it is mentioned the
> > relation b/w these power domains (controlled by PMIC though firmware) and the supported power saving
> > modes: ALL_ON, AWO, VBATT.
>
> DT describes hardware. So, the question was, from that perspective, do we need to document PD_VBATT domain,
> as it can be controlled outside linux??

No, as it is controlled by an external entity, outside the system you
are describing.

DT also doesn't describe external power input, power switches, power cords,
low/medium/high voltage circuit breakers, nuclear power plants, the Sun, ...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[claudiu beznea]

On 03.09.2024 10:36, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>> Sent: Tuesday, September 3, 2024 8:28 AM
>> Subject: Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB
>>
>>
>>
>> On 03.09.2024 09:58, Biju Das wrote:
>>> Hi Claudiu,
>>>
>>>> -----Original Message-----
>>>> From: Claudiu <claudiu.beznea@tuxon.dev>
>>>> Sent: Friday, August 30, 2024 2:02 PM
>>>> Subject: [PATCH v3 01/12] dt-bindings: clock:
>>>> renesas,r9a08g045-vbattb: Document VBATTB
>>>>
>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>
>>>> The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC,
>>>> the tamper detector and a small general usage memory of 128B. Add documentation for it.
>>>>
>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>> ---
>>>>
>>>> Changes in v3:
>>>> - moved the file to clock dt bindings directory as it is the
>>>>   only functionality supported at the moment; the other functionalities
>>>>   (tamper detector, SRAM) are offered though register spreaded
>>>>   though the address space of the VBATTB IP and not actually
>>>>   individual devices; the other functionalities are not
>>>>   planned to be supported soon and if they will be I think they
>>>>   fit better on auxiliary bus than MFD
>>>> - dropped interrupt names as requested in the review process
>>>> - dropped the inner node for clock controller
>>>> - added #clock-cells
>>>> - added rtx clock
>>>> - updated description for renesas,vbattb-load-nanofarads
>>>> - included dt-bindings/interrupt-controller/irq.h in examples section
>>>>
>>>> Changes in v2:
>>>> - changed file name and compatible
>>>> - updated title, description sections
>>>> - added clock controller part documentation and drop dedicated file
>>>>   for it included in v1
>>>> - used items to describe interrupts, interrupt-names, clocks, clock-names,
>>>>   resets
>>>> - dropped node labels and status
>>>> - updated clock-names for clock controller to cope with the new
>>>>   logic on detecting the necessity to setup bypass
>>>>
>>>>  .../clock/renesas,r9a08g045-vbattb.yaml       | 81 +++++++++++++++++++
>>>>  1 file changed, 81 insertions(+)
>>>>  create mode 100644
>>>> Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.yaml
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.ya
>>>> ml
>>>> b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.ya
>>>> ml
>>>> new file mode 100644
>>>> index 000000000000..29df0e01fae5
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbatt
>>>> +++ b.y
>>>> +++ aml
>>>> @@ -0,0 +1,81 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
>>>> +---
>>>> +$id:
>>>> +http://devicetree.org/schemas/clock/renesas,r9a08g045-vbattb.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Renesas Battery Backup Function (VBATTB)
>>>> +
>>>> +description:
>>>> +  Renesas VBATTB is an always on powered module (backed by battery)
>>>> +which
>>>> +  controls the RTC clock (VBATTCLK), tamper detection logic and a
>>>> +small
>>>> +  general usage memory (128B).
>>>> +
>>>> +maintainers:
>>>> +  - Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: renesas,r9a08g045-vbattb
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  interrupts:
>>>> +    items:
>>>> +      - description: tamper detector interrupt
>>>> +
>>>> +  clocks:
>>>> +    items:
>>>> +      - description: VBATTB module clock
>>>> +      - description: RTC input clock (crystal oscillator or external
>>>> + clock device)
>>>> +
>>>> +  clock-names:
>>>> +    items:
>>>> +      - const: bclk
>>>> +      - const: rtx
>>>> +
>>>> +  '#clock-cells':
>>>> +    const: 1
>>>> +
>>>> +  power-domains:
>>>> +    maxItems: 1
>>>
>>> Not sure, you need to document "PD_VBATT" power domain as per Table
>>> 41.2, this LSI supports 3 power domains(PD_ISOVCC, PD_VCC, PD_VBATT)
>>>
>>> Power Mode PD_ISOVCC PD_VCC PD_VBATT
>>> ALL_ON      ON          ON    ON
>>> AWO         OFF         ON    ON
>>> VBATT       OFF         OFF   ON
>>> ALL_OFF     OFF         OFF   OFF
>>>
>>> PD_VBATT domain is the area where the RTC/backup register is located,
>>> works on battery power when the power of PD_VCC and PD_ISOVCC domain are turned off.
>>
>> In Linux, the CPG is the power domain provider for all the IPs in RZ/G3S SoC (modeled though MSTOP CPG
>> support). This is how it is currently implemented.
>>
>> Then groups of IPs are part of power domains PD_ISOVCC, PD_VCC, PD_VBATT.
>> These power domains are i2c controlled with the help of firmware (at least at the moment).
>>
>> From HW manual:
>> - PD_VCC domain always powered on area.
>>
>> - PD_ISOVCC domain is the area where the power can be turned off.
>>
>> - PD_VBATT domain is the area where the RTC/backup register is located,
>>   works on battery power when the power of .
>>
>> The power to these domains are controlled with the help of firmware. Linux cannot do control itself as
>> the CPU is in the PD_ISOVCC. If you look at picture 41.3 Power mode transition [1] it is mentioned the
>> relation b/w these power domains (controlled by PMIC though firmware) and the supported power saving
>> modes: ALL_ON, AWO, VBATT.
>>
> 
> DT describes hardware. So, the question was, from that perspective, do we need to document PD_VBATT domain,
> as it can be controlled outside linux??

The control to these domains is passed to firmware.

From my point of view these will never be used by Linux becuase:
- the PD_ISOVCC is where the CPU resides and it cannot cut itself its power
- the PD_VCC is a domain where critical IPs like CPG, SYSC resides
- the VBATT is where the RTC resides, RTC that should stay on forever

Should we document something that will be never used?

> 
> Cheers,
> Biju

RE: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[Biju Das]

Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> Sent: Tuesday, September 3, 2024 8:44 AM
> Subject: Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB
> 
> 
> 
> On 03.09.2024 10:36, Biju Das wrote:
> > Hi Claudiu,
> >
> >> -----Original Message-----
> >> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> >> Sent: Tuesday, September 3, 2024 8:28 AM
> >> Subject: Re: [PATCH v3 01/12] dt-bindings: clock:
> >> renesas,r9a08g045-vbattb: Document VBATTB
> >>
> >>
> >>
> >> On 03.09.2024 09:58, Biju Das wrote:
> >>> Hi Claudiu,
> >>>
> >>>> -----Original Message-----
> >>>> From: Claudiu <claudiu.beznea@tuxon.dev>
> >>>> Sent: Friday, August 30, 2024 2:02 PM
> >>>> Subject: [PATCH v3 01/12] dt-bindings: clock:
> >>>> renesas,r9a08g045-vbattb: Document VBATTB
> >>>>
> >>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>>
> >>>> The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC,
> >>>> the tamper detector and a small general usage memory of 128B. Add documentation for it.
> >>>>
> >>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>> ---
> >>>>
> >>>> Changes in v3:
> >>>> - moved the file to clock dt bindings directory as it is the
> >>>>   only functionality supported at the moment; the other functionalities
> >>>>   (tamper detector, SRAM) are offered though register spreaded
> >>>>   though the address space of the VBATTB IP and not actually
> >>>>   individual devices; the other functionalities are not
> >>>>   planned to be supported soon and if they will be I think they
> >>>>   fit better on auxiliary bus than MFD
> >>>> - dropped interrupt names as requested in the review process
> >>>> - dropped the inner node for clock controller
> >>>> - added #clock-cells
> >>>> - added rtx clock
> >>>> - updated description for renesas,vbattb-load-nanofarads
> >>>> - included dt-bindings/interrupt-controller/irq.h in examples
> >>>> section
> >>>>
> >>>> Changes in v2:
> >>>> - changed file name and compatible
> >>>> - updated title, description sections
> >>>> - added clock controller part documentation and drop dedicated file
> >>>>   for it included in v1
> >>>> - used items to describe interrupts, interrupt-names, clocks, clock-names,
> >>>>   resets
> >>>> - dropped node labels and status
> >>>> - updated clock-names for clock controller to cope with the new
> >>>>   logic on detecting the necessity to setup bypass
> >>>>
> >>>>  .../clock/renesas,r9a08g045-vbattb.yaml       | 81 +++++++++++++++++++
> >>>>  1 file changed, 81 insertions(+)
> >>>>  create mode 100644
> >>>> Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.ya
> >>>> ml
> >>>>
> >>>> diff --git
> >>>> a/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.
> >>>> ya
> >>>> ml
> >>>> b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vbattb.
> >>>> ya
> >>>> ml
> >>>> new file mode 100644
> >>>> index 000000000000..29df0e01fae5
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/clock/renesas,r9a08g045-vba
> >>>> +++ tt
> >>>> +++ b.y
> >>>> +++ aml
> >>>> @@ -0,0 +1,81 @@
> >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML
> >>>> +1.2
> >>>> +---
> >>>> +$id:
> >>>> +http://devicetree.org/schemas/clock/renesas,r9a08g045-vbattb.yaml#
> >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>> +
> >>>> +title: Renesas Battery Backup Function (VBATTB)
> >>>> +
> >>>> +description:
> >>>> +  Renesas VBATTB is an always on powered module (backed by
> >>>> +battery) which
> >>>> +  controls the RTC clock (VBATTCLK), tamper detection logic and a
> >>>> +small
> >>>> +  general usage memory (128B).
> >>>> +
> >>>> +maintainers:
> >>>> +  - Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>> +
> >>>> +properties:
> >>>> +  compatible:
> >>>> +    const: renesas,r9a08g045-vbattb
> >>>> +
> >>>> +  reg:
> >>>> +    maxItems: 1
> >>>> +
> >>>> +  interrupts:
> >>>> +    items:
> >>>> +      - description: tamper detector interrupt
> >>>> +
> >>>> +  clocks:
> >>>> +    items:
> >>>> +      - description: VBATTB module clock
> >>>> +      - description: RTC input clock (crystal oscillator or
> >>>> + external clock device)
> >>>> +
> >>>> +  clock-names:
> >>>> +    items:
> >>>> +      - const: bclk
> >>>> +      - const: rtx
> >>>> +
> >>>> +  '#clock-cells':
> >>>> +    const: 1
> >>>> +
> >>>> +  power-domains:
> >>>> +    maxItems: 1
> >>>
> >>> Not sure, you need to document "PD_VBATT" power domain as per Table
> >>> 41.2, this LSI supports 3 power domains(PD_ISOVCC, PD_VCC, PD_VBATT)
> >>>
> >>> Power Mode PD_ISOVCC PD_VCC PD_VBATT
> >>> ALL_ON      ON          ON    ON
> >>> AWO         OFF         ON    ON
> >>> VBATT       OFF         OFF   ON
> >>> ALL_OFF     OFF         OFF   OFF
> >>>
> >>> PD_VBATT domain is the area where the RTC/backup register is
> >>> located, works on battery power when the power of PD_VCC and PD_ISOVCC domain are turned off.
> >>
> >> In Linux, the CPG is the power domain provider for all the IPs in
> >> RZ/G3S SoC (modeled though MSTOP CPG support). This is how it is currently implemented.
> >>
> >> Then groups of IPs are part of power domains PD_ISOVCC, PD_VCC, PD_VBATT.
> >> These power domains are i2c controlled with the help of firmware (at least at the moment).
> >>
> >> From HW manual:
> >> - PD_VCC domain always powered on area.
> >>
> >> - PD_ISOVCC domain is the area where the power can be turned off.
> >>
> >> - PD_VBATT domain is the area where the RTC/backup register is located,
> >>   works on battery power when the power of .
> >>
> >> The power to these domains are controlled with the help of firmware.
> >> Linux cannot do control itself as the CPU is in the PD_ISOVCC. If you
> >> look at picture 41.3 Power mode transition [1] it is mentioned the
> >> relation b/w these power domains (controlled by PMIC though firmware)
> >> and the supported power saving
> >> modes: ALL_ON, AWO, VBATT.
> >>
> >
> > DT describes hardware. So, the question was, from that perspective, do
> > we need to document PD_VBATT domain, as it can be controlled outside linux??
> 
> The control to these domains is passed to firmware.
> 
> From my point of view these will never be used by Linux becuase:
> - the PD_ISOVCC is where the CPU resides and it cannot cut itself its power
> - the PD_VCC is a domain where critical IPs like CPG, SYSC resides
> - the VBATT is where the RTC resides, RTC that should stay on forever
> 
> Should we document something that will be never used?

If it is inside the system, you are describing to avoid ABI breakages any future.

Eg [1], there are interrupts we documented, Don't know we will ever use it in linux.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml#n58

Cheers,
Biju

Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[Geert Uytterhoeven]

Hi Claudiu,

On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC,
> the tamper detector and a small general usage memory of 128B. Add
> documentation for it.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v3:
> - moved the file to clock dt bindings directory as it is the
>   only functionality supported at the moment; the other functionalities
>   (tamper detector, SRAM) are offered though register spreaded
>   though the address space of the VBATTB IP and not actually
>   individual devices; the other functionalities are not
>   planned to be supported soon and if they will be I think they
>   fit better on auxiliary bus than MFD
> - dropped interrupt names as requested in the review process
> - dropped the inner node for clock controller
> - added #clock-cells
> - added rtx clock
> - updated description for renesas,vbattb-load-nanofarads
> - included dt-bindings/interrupt-controller/irq.h in examples section

Thanks for the update!

LGTM, modulo issues pointed out in other review comments.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 01/12] dt-bindings: clock: renesas,r9a08g045-vbattb: Document VBATTB

[Geert Uytterhoeven]

Hi Claudiu,

On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The VBATTB IP of the Renesas RZ/G3S SoC controls the clock for RTC,
> the tamper detector and a small general usage memory of 128B. Add
> documentation for it.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v3:
> - moved the file to clock dt bindings directory as it is the
>   only functionality supported at the moment; the other functionalities
>   (tamper detector, SRAM) are offered though register spreaded
>   though the address space of the VBATTB IP and not actually
>   individual devices; the other functionalities are not
>   planned to be supported soon and if they will be I think they
>   fit better on auxiliary bus than MFD

The battery-backed-up SRAM could be exported through the
NVMEM framework. I am not sure it offers some way to export the tamper
flag (to indicate it's erased, i.e. empty).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 02/12] dt-bindings: clock: r9a08g045-vbattb: Add clock IDs for the VBATTB controller

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The VBATTB controllers controls the clock for the RTC on the Renesas
RZ/G3S. The HW block diagram for the clock logic is as follows:

           +----------+ XC   `\
RTXIN  --->|          |----->| \       +----+  VBATTCLK
           | 32K clock|      |  |----->|gate|----------->
	   | osc      | XBYP |  |      +----+
RTXOUT --->|          |----->| /
           +----------+      ,/

One could connect as input to this HW block either a crystal oscillator or
an external clock device.

After discussions w/ Stephen Boyd the clock tree associated with this
hardware block was exported in Linux as:

input-xtal
  xbyp
  xc
     mux
        vbattclk

where:
- input-xtal is the input clock (connected to RTXIN, RTXOUT pins)
- xc, xbyp are mux inputs
- mux is the internal mux
- vbattclk is the gate clock that feeds in the end the RTC

to allow selecting the input of the MUX though assigned-clock DT
properties, using the already existing clock drivers and avoid adding
other DT properties.

This allows select the input of the mux based on the type of the
connected input clock:
- if the 32768 crystal oscillator is connected as input for the VBATTB,
  the input of the mux should be xc
- if an external clock device is connected as input for the VBATTB the
  input of the mux should be xbyp

Add clock IDs for the VBATTB controller.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- none; this patch is new

 include/dt-bindings/clock/r9a08g045-vbattb.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100644 include/dt-bindings/clock/r9a08g045-vbattb.h

diff --git a/include/dt-bindings/clock/r9a08g045-vbattb.h b/include/dt-bindings/clock/r9a08g045-vbattb.h
new file mode 100644
index 000000000000..67774eafad06
--- /dev/null
+++ b/include/dt-bindings/clock/r9a08g045-vbattb.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+ *
+ * Copyright (C) 2024 Renesas Electronics Corp.
+ */
+#ifndef __DT_BINDINGS_CLOCK_R9A08G045_VBATTB_H__
+#define __DT_BINDINGS_CLOCK_R9A08G045_VBATTB_H__
+
+#define VBATTB_XC		0
+#define VBATTB_XBYP		1
+#define VBATTB_MUX		2
+#define VBATTB_VBATTCLK		3
+
+#endif /* __DT_BINDINGS_CLOCK_R9A08G045_VBATTB_H__ */
-- 
2.39.2

Re: [PATCH v3 02/12] dt-bindings: clock: r9a08g045-vbattb: Add clock IDs for the VBATTB controller

[Rob Herring]

On Fri, Aug 30, 2024 at 04:02:08PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The VBATTB controllers controls the clock for the RTC on the Renesas
> RZ/G3S. The HW block diagram for the clock logic is as follows:
> 
>            +----------+ XC   `\
> RTXIN  --->|          |----->| \       +----+  VBATTCLK
>            | 32K clock|      |  |----->|gate|----------->
> 	   | osc      | XBYP |  |      +----+
> RTXOUT --->|          |----->| /
>            +----------+      ,/
> 
> One could connect as input to this HW block either a crystal oscillator or
> an external clock device.
> 
> After discussions w/ Stephen Boyd the clock tree associated with this
> hardware block was exported in Linux as:
> 
> input-xtal
>   xbyp
>   xc
>      mux
>         vbattclk
> 
> where:
> - input-xtal is the input clock (connected to RTXIN, RTXOUT pins)
> - xc, xbyp are mux inputs
> - mux is the internal mux
> - vbattclk is the gate clock that feeds in the end the RTC
> 
> to allow selecting the input of the MUX though assigned-clock DT
> properties, using the already existing clock drivers and avoid adding
> other DT properties.
> 
> This allows select the input of the mux based on the type of the
> connected input clock:
> - if the 32768 crystal oscillator is connected as input for the VBATTB,
>   the input of the mux should be xc
> - if an external clock device is connected as input for the VBATTB the
>   input of the mux should be xbyp
> 
> Add clock IDs for the VBATTB controller.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
> 
> Changes in v3:
> - none; this patch is new
> 
>  include/dt-bindings/clock/r9a08g045-vbattb.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>  create mode 100644 include/dt-bindings/clock/r9a08g045-vbattb.h

Squash this into the previous patch. It is part of the binding.

Rob

Re: [PATCH v3 02/12] dt-bindings: clock: r9a08g045-vbattb: Add clock IDs for the VBATTB controller

[Geert Uytterhoeven]

Hi Claudiu,

Thanks for your patch!

On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The VBATTB controllers controls the clock for the RTC on the Renesas

controller

> RZ/G3S. The HW block diagram for the clock logic is as follows:
>
>            +----------+ XC   `\
> RTXIN  --->|          |----->| \       +----+  VBATTCLK
>            | 32K clock|      |  |----->|gate|----------->
>            | osc      | XBYP |  |      +----+
> RTXOUT --->|          |----->| /
>            +----------+      ,/
>
> One could connect as input to this HW block either a crystal oscillator or

Please drop "oscillator". An external crystal is used with the internal
crystal oscillator.

> an external clock device.
>
> After discussions w/ Stephen Boyd the clock tree associated with this
> hardware block was exported in Linux as:
>
> input-xtal
>   xbyp
>   xc
>      mux
>         vbattclk
>
> where:
> - input-xtal is the input clock (connected to RTXIN, RTXOUT pins)
> - xc, xbyp are mux inputs
> - mux is the internal mux
> - vbattclk is the gate clock that feeds in the end the RTC
>
> to allow selecting the input of the MUX though assigned-clock DT
> properties, using the already existing clock drivers and avoid adding
> other DT properties.
>
> This allows select the input of the mux based on the type of the
> connected input clock:
> - if the 32768 crystal oscillator is connected as input for the VBATTB,

Please drop "oscillator".

>   the input of the mux should be xc
> - if an external clock device is connected as input for the VBATTB the
>   input of the mux should be xbyp
>
> Add clock IDs for the VBATTB controller.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v3:
> - none; this patch is new

> --- /dev/null
> +++ b/include/dt-bindings/clock/r9a08g045-vbattb.h

renesas,r9a08g045-vbattb.h

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 03/12] clk: linux/clk-provider.h: Add devm_clk_hw_register_gate_parent_hw()

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Add devm_clk_hw_register_gate_parent_hw() macro to allow registering
devres managed gate clocks providing struct clk_hw object as parent.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- none; this patch is new

 include/linux/clk-provider.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 4a537260f655..824b62059364 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -608,6 +608,24 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
 	__devm_clk_hw_register_gate((dev), NULL, (name), (parent_name), NULL, \
 			       NULL, (flags), (reg), (bit_idx),		      \
 			       (clk_gate_flags), (lock))
+/**
+ * devm_clk_hw_register_gate_parent_hw - register a gate clock with the clock
+ * framework
+ * @dev: device that is registering this clock
+ * @name: name of this clock
+ * @parent_hw: pointer to parent clk
+ * @flags: framework-specific flags for this clock
+ * @reg: register address to control gating of this clock
+ * @bit_idx: which bit in the register controls gating of this clock
+ * @clk_gate_flags: gate-specific flags for this clock
+ * @lock: shared register lock for this clock
+ */
+#define devm_clk_hw_register_gate_parent_hw(dev, name, parent_hw, flags,      \
+					    reg, bit_idx, clk_gate_flags,     \
+					    lock)			      \
+	__devm_clk_hw_register_gate((dev), NULL, (name), NULL, (parent_hw),   \
+				    NULL, (flags), (reg), (bit_idx),	      \
+				    (clk_gate_flags), (lock))
 /**
  * devm_clk_hw_register_gate_parent_data - register a gate clock with the
  * clock framework
-- 
2.39.2

Re: [PATCH v3 03/12] clk: linux/clk-provider.h: Add devm_clk_hw_register_gate_parent_hw()

[Geert Uytterhoeven]

On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Add devm_clk_hw_register_gate_parent_hw() macro to allow registering
> devres managed gate clocks providing struct clk_hw object as parent.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v3:
> - none; this patch is new

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 04/12] clk: renesas: clk-vbattb: Add VBATTB clock driver

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The VBATTB IP of the Renesas RZ/G3S SoC controls the clock that is used
by the RTC. The input to the VBATTB could be a 32KHz crystal oscillator
or an external clock device.

The HW block diagram for the clock generator is as follows:

           +----------+ XC   `\
RTXIN  --->|          |----->| \       +----+  VBATTCLK
           | 32K clock|      |  |----->|gate|----------->
           | osc      | XBYP |  |      +----+
RTXOUT --->|          |----->| /
           +----------+      ,

After discussions w/ Stephen Boyd the clock tree associated with this
hardware block was exported in Linux as:

vbattb-xtal
   xbyp
   xc
      mux
         vbattbclk

where:
- input-xtal is the input clock (connected to RTXIN, RTXOUT pins)
- xc, xbyp are mux inputs
- mux is the internal mux
- vbattclk is the gate clock that feeds in the end the RTC

to allow selecting the input of the MUX though assigned-clock DT
properties, using the already existing clock drivers and avoid adding
other DT properties. If the crystal oscillator is connected as on RTXIN,
RTXOUT pins the XC will be selected as mux input. If an external clock
device is connected on RTXIN, RTXOUT pins the XBYP will be selected as
mux input.

The load capacitance of the on-board crystal oscillator can be configured
with renesas,vbattb-load-nanofarads DT property.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- updated patch description
- dropped dependency on MFD_RENESAS_VBATTB as now there is no
  driver built under this flag
- dropped include/clk.h
- added pm_runtime and reset control support
- updated register offsets
- registered 4 clocks: xc, xbyp, mux, vbattclk using generic
  clock drivers
- added MODULE_DEVICE_TABLE()

Changes in v2:
- updated patch description
- added vendor name in Kconfig flag
- used cleanup.h lock helpers
- dropped the MFD code
- updated registers offsets
- added vbattb_clk_update_bits() and used it where possible
- added vbattb_clk_need_bypass() to detect the bypass setup necessity
- changed the compatible and driver names


 drivers/clk/renesas/Kconfig      |   4 +
 drivers/clk/renesas/Makefile     |   1 +
 drivers/clk/renesas/clk-vbattb.c | 204 +++++++++++++++++++++++++++++++
 3 files changed, 209 insertions(+)
 create mode 100644 drivers/clk/renesas/clk-vbattb.c

diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig
index 76791a1c50ac..4e835a3f1ab4 100644
--- a/drivers/clk/renesas/Kconfig
+++ b/drivers/clk/renesas/Kconfig
@@ -237,6 +237,10 @@ config CLK_RZV2H
 	bool "RZ/V2H(P) family clock support" if COMPILE_TEST
 	select RESET_CONTROLLER
 
+config CLK_RENESAS_VBATTB
+	bool "Renesas VBATTB clock controller"
+	select RESET_CONTROLLER
+
 # Generic
 config CLK_RENESAS_CPG_MSSR
 	bool "CPG/MSSR clock support" if COMPILE_TEST
diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile
index 23d2e26051c8..82efaa835ac7 100644
--- a/drivers/clk/renesas/Makefile
+++ b/drivers/clk/renesas/Makefile
@@ -53,3 +53,4 @@ obj-$(CONFIG_CLK_RZV2H)			+= rzv2h-cpg.o
 obj-$(CONFIG_CLK_RENESAS_CPG_MSSR)	+= renesas-cpg-mssr.o
 obj-$(CONFIG_CLK_RENESAS_CPG_MSTP)	+= clk-mstp.o
 obj-$(CONFIG_CLK_RENESAS_DIV6)		+= clk-div6.o
+obj-$(CONFIG_CLK_RENESAS_VBATTB)	+= clk-vbattb.o
diff --git a/drivers/clk/renesas/clk-vbattb.c b/drivers/clk/renesas/clk-vbattb.c
new file mode 100644
index 000000000000..22147f4983d7
--- /dev/null
+++ b/drivers/clk/renesas/clk-vbattb.c
@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * VBATTB clock driver
+ *
+ * Copyright (C) 2024 Renesas Electronics Corp.
+ */
+
+#include <linux/cleanup.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+
+#include <dt-bindings/clock/r9a08g045-vbattb.h>
+
+#define VBATTB_BKSCCR			0x1c
+#define VBATTB_BKSCCR_SOSEL_BIT		6
+#define VBATTB_SOSCCR2			0x24
+#define VBATTB_SOSCCR2_SOSTP2_BIT	0
+#define VBATTB_XOSCCR			0x30
+#define VBATTB_XOSCCR_OUTEN_BIT		16
+#define VBATTB_XOSCCR_XSEL		GENMASK(1, 0)
+#define VBATTB_XOSCCR_XSEL_4_PF		0x0
+#define VBATTB_XOSCCR_XSEL_7_PF		0x1
+#define VBATTB_XOSCCR_XSEL_9_PF		0x2
+#define VBATTB_XOSCCR_XSEL_12_5_PF	0x3
+
+/**
+ * struct vbattb_clk - VBATTB clock data structure
+ * @base: base address
+ * @lock: lock
+ */
+struct vbattb_clk {
+	void __iomem *base;
+	spinlock_t lock;
+};
+
+static int vbattb_clk_validate_load_capacitance(u32 *reg_lc, u32 of_lc)
+{
+	switch (of_lc) {
+	case 4000:
+		*reg_lc = VBATTB_XOSCCR_XSEL_4_PF;
+		break;
+	case 7000:
+		*reg_lc = VBATTB_XOSCCR_XSEL_7_PF;
+		break;
+	case 9000:
+		*reg_lc = VBATTB_XOSCCR_XSEL_9_PF;
+		break;
+	case 12500:
+		*reg_lc = VBATTB_XOSCCR_XSEL_12_5_PF;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void vbattb_clk_action(void *data)
+{
+	struct device *dev = data;
+	struct reset_control *rstc = dev_get_drvdata(dev);
+	int ret;
+
+	ret = reset_control_assert(rstc);
+	if (ret)
+		dev_err(dev, "Failed to de-assert reset!");
+
+	ret = pm_runtime_put_sync(dev);
+	if (ret < 0)
+		dev_err(dev, "Failed to runtime suspend!");
+
+	of_clk_del_provider(dev->of_node);
+}
+
+static int vbattb_clk_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct clk_parent_data parent_data = {};
+	struct clk_hw_onecell_data *clk_data;
+	const struct clk_hw *parent_hws[2];
+	struct device *dev = &pdev->dev;
+	struct reset_control *rstc;
+	struct vbattb_clk *vbclk;
+	u32 of_lc, reg_lc;
+	struct clk_hw *hw;
+	/* 4 clocks are exported: VBATTB_XC, VBATTB_XBYP, VBATTB_MUX, VBATTB_VBATTCLK. */
+	u8 num_clks = 4;
+	int ret;
+
+	/* Default to 4pF as this is not needed if external clock device is connected. */
+	of_lc = 4000;
+	of_property_read_u32(np, "renesas,vbattb-load-nanofarads", &of_lc);
+
+	ret = vbattb_clk_validate_load_capacitance(&reg_lc, of_lc);
+	if (ret)
+		return ret;
+
+	vbclk = devm_kzalloc(dev, sizeof(*vbclk), GFP_KERNEL);
+	if (!vbclk)
+		return -ENOMEM;
+
+	clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, num_clks), GFP_KERNEL);
+	if (!clk_data)
+		return -ENOMEM;
+	clk_data->num = num_clks;
+
+	vbclk->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(vbclk->base))
+		return PTR_ERR(vbclk->base);
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
+	rstc = devm_reset_control_get_shared(dev, NULL);
+	if (IS_ERR(rstc))
+		return PTR_ERR(rstc);
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return ret;
+
+	ret = reset_control_deassert(rstc);
+	if (ret) {
+		pm_runtime_put_sync(dev);
+		return ret;
+	}
+
+	dev_set_drvdata(dev, rstc);
+	ret = devm_add_action_or_reset(dev, vbattb_clk_action, dev);
+	if (ret)
+		return ret;
+
+	spin_lock_init(&vbclk->lock);
+
+	parent_data.fw_name = "rtx";
+	hw = devm_clk_hw_register_gate_parent_data(dev, "xc", &parent_data, 0,
+						   vbclk->base + VBATTB_SOSCCR2,
+						   VBATTB_SOSCCR2_SOSTP2_BIT,
+						   CLK_GATE_SET_TO_DISABLE, &vbclk->lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	clk_data->hws[VBATTB_XC] = hw;
+
+	hw = devm_clk_hw_register_fixed_factor_fwname(dev, np, "xbyp", "rtx", 0, 1, 1);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	clk_data->hws[VBATTB_XBYP] = hw;
+
+	parent_hws[0] = clk_data->hws[VBATTB_XC];
+	parent_hws[1] = clk_data->hws[VBATTB_XBYP];
+	hw = devm_clk_hw_register_mux_parent_hws(dev, "mux", parent_hws, 2, 0,
+						 vbclk->base + VBATTB_BKSCCR,
+						 VBATTB_BKSCCR_SOSEL_BIT,
+						 1, 0, &vbclk->lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	clk_data->hws[VBATTB_MUX] = hw;
+
+	/* Set load capacitance before registering the VBATTCLK clock. */
+	scoped_guard(spinlock, &vbclk->lock) {
+		u32 val = readl_relaxed(vbclk->base + VBATTB_XOSCCR);
+
+		val &= ~VBATTB_XOSCCR_XSEL;
+		val |= reg_lc;
+		writel_relaxed(val, vbclk->base + VBATTB_XOSCCR);
+	}
+
+	hw = devm_clk_hw_register_gate_parent_hw(dev, "vbattclk", hw, 0,
+						 vbclk->base + VBATTB_XOSCCR,
+						 VBATTB_XOSCCR_OUTEN_BIT, 0,
+						 &vbclk->lock);
+
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	clk_data->hws[VBATTB_VBATTCLK] = hw;
+
+	return of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_data);
+}
+
+static const struct of_device_id vbattb_clk_match[] = {
+	{ .compatible = "renesas,r9a08g045-vbattb" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, vbattb_clk_match);
+
+static struct platform_driver vbattb_clk_driver = {
+	.driver		= {
+		.name	= "renesas-vbattb-clk",
+		.of_match_table = vbattb_clk_match,
+	},
+	.probe = vbattb_clk_probe,
+};
+module_platform_driver(vbattb_clk_driver);
+
+MODULE_DESCRIPTION("Renesas VBATTB Clock Driver");
+MODULE_AUTHOR("Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>");
+MODULE_LICENSE("GPL");
-- 
2.39.2

Re: [PATCH v3 04/12] clk: renesas: clk-vbattb: Add VBATTB clock driver

[Geert Uytterhoeven]

Hi Claudiu,

Thanks for your patch!

On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The VBATTB IP of the Renesas RZ/G3S SoC controls the clock that is used
> by the RTC. The input to the VBATTB could be a 32KHz crystal oscillator

Please drop "oscillator".

> or an external clock device.
>
> The HW block diagram for the clock generator is as follows:
>
>            +----------+ XC   `\
> RTXIN  --->|          |----->| \       +----+  VBATTCLK
>            | 32K clock|      |  |----->|gate|----------->
>            | osc      | XBYP |  |      +----+
> RTXOUT --->|          |----->| /
>            +----------+      ,
>
> After discussions w/ Stephen Boyd the clock tree associated with this
> hardware block was exported in Linux as:
>
> vbattb-xtal
>    xbyp
>    xc
>       mux
>          vbattbclk
>
> where:
> - input-xtal is the input clock (connected to RTXIN, RTXOUT pins)
> - xc, xbyp are mux inputs
> - mux is the internal mux
> - vbattclk is the gate clock that feeds in the end the RTC
>
> to allow selecting the input of the MUX though assigned-clock DT
> properties, using the already existing clock drivers and avoid adding
> other DT properties. If the crystal oscillator is connected as on RTXIN,

Please drop "oscillator".

> RTXOUT pins the XC will be selected as mux input. If an external clock
> device is connected on RTXIN, RTXOUT pins the XBYP will be selected as
> mux input.
>
> The load capacitance of the on-board crystal oscillator can be configured

s/on-board/internal/

> with renesas,vbattb-load-nanofarads DT property.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

> --- /dev/null
> +++ b/drivers/clk/renesas/clk-vbattb.c
> @@ -0,0 +1,204 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * VBATTB clock driver
> + *
> + * Copyright (C) 2024 Renesas Electronics Corp.
> + */
> +
> +#include <linux/cleanup.h>
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +
> +#include <dt-bindings/clock/r9a08g045-vbattb.h>
> +
> +#define VBATTB_BKSCCR                  0x1c
> +#define VBATTB_BKSCCR_SOSEL_BIT                6
> +#define VBATTB_SOSCCR2                 0x24
> +#define VBATTB_SOSCCR2_SOSTP2_BIT      0
> +#define VBATTB_XOSCCR                  0x30
> +#define VBATTB_XOSCCR_OUTEN_BIT                16

Please either drop the "_BIT"-suffixes...

> +#define VBATTB_XOSCCR_XSEL             GENMASK(1, 0)

or add a "_MASK"-suffix here.

> +#define VBATTB_XOSCCR_XSEL_4_PF                0x0
> +#define VBATTB_XOSCCR_XSEL_7_PF                0x1
> +#define VBATTB_XOSCCR_XSEL_9_PF                0x2
> +#define VBATTB_XOSCCR_XSEL_12_5_PF     0x3

The rest LGTM.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 05/12] dt-bindings: rtc: renesas,rzg3s-rtc: Document the Renesas RTCA-3 IP

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Document the RTC IP (RTCA-3) available on the Renesas RZ/G3S SoC.
The RTC IP available on Renesas RZ/V2H is almost identical with the
one found on Renesas RZ/G3S (it misses the time capture functionality
which is not yet implemented on proposed driver). For this, added also a
generic compatible that will be used at the moment as fallback for both
RZ/G3S and RZ/V2H.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- added RTC bus clock, reset and power-domain; it has been detected
  by reverse engineering that RTC and VBATTB clock, reset and power
  domain are shared; HW manual doesn't mention it
- updated example with these and with assigned-clock properties
  needed to configure the VBATTCLK MUX with proper parent
- updated example section with dt-bindings/clock/r9a08g045-cpg.h
  and dt-bindings/clock/r9a08g045-vbattb.h includes
- for all these, dropped Conor's Rb tag

Changes in v2:
- updated patch description and title
- included reference to rtc.yaml
- updated compatible list with a generic compatible as explained in
  patch description; with this the node in examples section has also been
  updated
- used items to describe interrupts, interrupt-names, clock, clock-names
- updated title section

 .../bindings/rtc/renesas,rz-rtca3.yaml        | 86 +++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/renesas,rz-rtca3.yaml

diff --git a/Documentation/devicetree/bindings/rtc/renesas,rz-rtca3.yaml b/Documentation/devicetree/bindings/rtc/renesas,rz-rtca3.yaml
new file mode 100644
index 000000000000..bc99795dfb6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/renesas,rz-rtca3.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/renesas,rz-rtca3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RTCA-3 Real Time Clock
+
+maintainers:
+  - Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
+
+allOf:
+  - $ref: rtc.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,r9a08g045-rtca3 # RZ/G3S
+      - const: renesas,rz-rtca3
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: Alarm interrupt
+      - description: Periodic interrupt
+      - description: Carry interrupt
+
+  interrupt-names:
+    items:
+      - const: alarm
+      - const: period
+      - const: carry
+
+  clocks:
+    items:
+      - description: RTC bus clock
+      - description: RTC counter clock
+
+  clock-names:
+    items:
+      - const: bus
+      - const: counter
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    items:
+      - description: VBATTB module reset
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r9a08g045-cpg.h>
+    #include <dt-bindings/clock/r9a08g045-vbattb.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    rtc@1004ec00 {
+        compatible = "renesas,r9a08g045-rtca3", "renesas,rz-rtca3";
+        reg = <0x1004ec00 0x400>;
+        interrupts = <GIC_SPI 315 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "alarm", "period", "carry";
+        clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattclk VBATTB_VBATTCLK>;
+        clock-names = "bus", "counter";
+        assigned-clocks = <&vbattb VBATTB_MUX>;
+        assigned-clock-parents = <&vbattb VBATTB_XC>;
+        power-domains = <&cpg>;
+        resets = <&cpg R9A08G045_VBAT_BRESETN>;
+    };
-- 
2.39.2

Re: [PATCH v3 05/12] dt-bindings: rtc: renesas,rzg3s-rtc: Document the Renesas RTCA-3 IP

[Rob Herring (Arm)]

On Fri, 30 Aug 2024 16:02:11 +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Document the RTC IP (RTCA-3) available on the Renesas RZ/G3S SoC.
> The RTC IP available on Renesas RZ/V2H is almost identical with the
> one found on Renesas RZ/G3S (it misses the time capture functionality
> which is not yet implemented on proposed driver). For this, added also a
> generic compatible that will be used at the moment as fallback for both
> RZ/G3S and RZ/V2H.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
> 
> Changes in v3:
> - added RTC bus clock, reset and power-domain; it has been detected
>   by reverse engineering that RTC and VBATTB clock, reset and power
>   domain are shared; HW manual doesn't mention it
> - updated example with these and with assigned-clock properties
>   needed to configure the VBATTCLK MUX with proper parent
> - updated example section with dt-bindings/clock/r9a08g045-cpg.h
>   and dt-bindings/clock/r9a08g045-vbattb.h includes
> - for all these, dropped Conor's Rb tag
> 
> Changes in v2:
> - updated patch description and title
> - included reference to rtc.yaml
> - updated compatible list with a generic compatible as explained in
>   patch description; with this the node in examples section has also been
>   updated
> - used items to describe interrupts, interrupt-names, clock, clock-names
> - updated title section
> 
>  .../bindings/rtc/renesas,rz-rtca3.yaml        | 86 +++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/renesas,rz-rtca3.yaml
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v3 05/12] dt-bindings: rtc: renesas,rzg3s-rtc: Document the Renesas RTCA-3 IP

[Geert Uytterhoeven]

Hi Claudiu,

On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Document the RTC IP (RTCA-3) available on the Renesas RZ/G3S SoC.
> The RTC IP available on Renesas RZ/V2H is almost identical with the
> one found on Renesas RZ/G3S (it misses the time capture functionality
> which is not yet implemented on proposed driver). For this, added also a
> generic compatible that will be used at the moment as fallback for both
> RZ/G3S and RZ/V2H.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v3:
> - added RTC bus clock, reset and power-domain; it has been detected
>   by reverse engineering that RTC and VBATTB clock, reset and power
>   domain are shared; HW manual doesn't mention it
> - updated example with these and with assigned-clock properties
>   needed to configure the VBATTCLK MUX with proper parent
> - updated example section with dt-bindings/clock/r9a08g045-cpg.h
>   and dt-bindings/clock/r9a08g045-vbattb.h includes
> - for all these, dropped Conor's Rb tag

Thanks for the update!

Sorry for chiming in late, but this RTCA-3 block seems to be a
derivative of the RTC blocks found on older SuperH SoCs, and on RZ/A1
and RZ/A2 ARM SoCs.  Differences are found in (lack of)
100/1000-year-count parts and the Year Alarm Enable Register, and in
some control register bits.

The SuperH and RZ/A1 variant is supported by drivers/rtc/rtc-sh.c;
DT bindings for the latter are found in
Documentation/devicetree/bindings/rtc/renesas,sh-rtc.yaml.

(My first guess was that RTC-A1 is used on RZ/A1, RTC-A2 on RZ/A2,
 and RTC-A3 on RZ/A3, but apparently RZ/A3UL does not have an RTC...
 Oh well, at least it is used on later RZ series SoCs...)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 05/12] dt-bindings: rtc: renesas,rzg3s-rtc: Document the Renesas RTCA-3 IP

[claudiu beznea]

Hi, Geert,

On 10.10.2024 12:29, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Document the RTC IP (RTCA-3) available on the Renesas RZ/G3S SoC.
>> The RTC IP available on Renesas RZ/V2H is almost identical with the
>> one found on Renesas RZ/G3S (it misses the time capture functionality
>> which is not yet implemented on proposed driver). For this, added also a
>> generic compatible that will be used at the moment as fallback for both
>> RZ/G3S and RZ/V2H.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>
>> Changes in v3:
>> - added RTC bus clock, reset and power-domain; it has been detected
>>   by reverse engineering that RTC and VBATTB clock, reset and power
>>   domain are shared; HW manual doesn't mention it
>> - updated example with these and with assigned-clock properties
>>   needed to configure the VBATTCLK MUX with proper parent
>> - updated example section with dt-bindings/clock/r9a08g045-cpg.h
>>   and dt-bindings/clock/r9a08g045-vbattb.h includes
>> - for all these, dropped Conor's Rb tag
> 
> Thanks for the update!
> 
> Sorry for chiming in late, but this RTCA-3 block seems to be a
> derivative of the RTC blocks found on older SuperH SoCs, and on RZ/A1
> and RZ/A2 ARM SoCs.  Differences are found in (lack of)
> 100/1000-year-count parts and the Year Alarm Enable Register, and in
> some control register bits.

At a 1st look it seems so, yes. I was inclined at the beginning to just use
the rtc-sh but the RZ/G3S HW manual mentions a lot of restrictions that
need to be followed when configuring the IP. Because of these restrictions
I chose to have a different driver. Otherwise the rtc-sh would have become
way too complication as far as I can tell.

From these restriction I can mention:
- wait for 2*1/64 periods when configuring the alarm
- from HW manual: When the RCR1 register is modified,
 check that all the
  bits have been updated before proceeding to the next processing
- from HW manual: section 22.6.4. Notes on writing to and
  reading from registers) after writing to count registers, alarm
  registers, year alarm enable register, bits RCR2.AADJE, AADJP,
  and HR24 register, we need to do 3 empty reads before being
  able to fetch the registers content.
- updates to RCR2.START need to be checked before continuing (note 2 from
  HW manual on RCR2 register: After writing to this bit, confirm that its
  value has actually changed before proceeding with further processing)
- there are many bits that are synchronized w/ count source and need to be
  checked before proceeding (when setting it up).
- According to HW manual (section 22.4.2. Clock and count mode setting
  procedure) we need to wait at least 6 cycles of the 32KHz clock after
  clock was enabled.
- According to HW manual (section 22.3.19. RTC Control
  Register 2) when set 24 hours mode this needs to be done separate from
  stop operation.
- According to HW manual (section 22.6.3. Notes on writing to and reading
  from registers) after reset we need to wait 6 clock cycles before
  writing to RTC registers.
- According to HW manual (section 22.6.4. Notes on writing to and reading
  from registers) we need to wait 1/128 seconds while the clock is
  operating (RCR2.START bit = 1) to be able to read the counters after a
  return from reset.
- and there are other restrictions

Thank you,
Claudiu Beznea

> 
> The SuperH and RZ/A1 variant is supported by drivers/rtc/rtc-sh.c;
> DT bindings for the latter are found in
> Documentation/devicetree/bindings/rtc/renesas,sh-rtc.yaml.
> 
> (My first guess was that RTC-A1 is used on RZ/A1, RTC-A2 on RZ/A2,
>  and RTC-A3 on RZ/A3, but apparently RZ/A3UL does not have an RTC...
>  Oh well, at least it is used on later RZ series SoCs...)
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Re: [PATCH v3 05/12] dt-bindings: rtc: renesas,rzg3s-rtc: Document the Renesas RTCA-3 IP

[Geert Uytterhoeven]

Hi Claudiu,

On Thu, Oct 10, 2024 at 11:52 AM claudiu beznea
<claudiu.beznea@tuxon.dev> wrote:
> On 10.10.2024 12:29, Geert Uytterhoeven wrote:
> > On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> >> Document the RTC IP (RTCA-3) available on the Renesas RZ/G3S SoC.
> >> The RTC IP available on Renesas RZ/V2H is almost identical with the
> >> one found on Renesas RZ/G3S (it misses the time capture functionality
> >> which is not yet implemented on proposed driver). For this, added also a
> >> generic compatible that will be used at the moment as fallback for both
> >> RZ/G3S and RZ/V2H.

> > Sorry for chiming in late, but this RTCA-3 block seems to be a
> > derivative of the RTC blocks found on older SuperH SoCs, and on RZ/A1
> > and RZ/A2 ARM SoCs.  Differences are found in (lack of)
> > 100/1000-year-count parts and the Year Alarm Enable Register, and in
> > some control register bits.
>
> At a 1st look it seems so, yes. I was inclined at the beginning to just use
> the rtc-sh but the RZ/G3S HW manual mentions a lot of restrictions that
> need to be followed when configuring the IP. Because of these restrictions
> I chose to have a different driver. Otherwise the rtc-sh would have become
> way too complication as far as I can tell.

[...]

Thank you, makes sense!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The RTC IP (RTCA-3) available on the Renesas RZ/G3S SoC has calendar count
mode and binary count mode (selectable though RCR2.CNTMD) capabilities,
alarm capabilities, clock error correction capabilities. It can generate
alarm, period, carry interrupts.

Add a driver for RTCA-3 IP. The driver implements calendar count mode (as
the conversion b/w RTC and system time is simpler, done with bcd2bin(),
bin2bcd()), read and set time, read and set alarm, read and set
an offset.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- added pm runtime and reset control support; probe() was changed
  and rtca3_action() was added as for cleanup; VBATTB and RTC shares
  the clock, reset and PM domain; hw manual doesn't specifies
  this; it has been detected though reverse engineering
- dropped clk member of struct rtca3_priv as it is used only in
  probe()
- renamed rtca3_alarm_irq_enable_helper() to rtca3_alarm_irq_set_helper()
  and used it all over the places
- fixed the issue:
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202407191156.wJPjHtKG-lkp@intel.com/
- removed __maybe_unused from suspend/resume function and
  use DEFINE_SIMPLE_DEV_PM_OPS() along with pm_ptr()

Changes in v2:
- used cleanup.h helpers for locking
- updated the MAINTAINERS entry with the new name for RTCA-3 documentation
  file and a new title (from "RENESAS RZ/G3S RTC DRIVER" to
  "RENESAS RTCA-3 RTC DRIVER")
- used 24 hours mode
- changed startup sequence (rtca3_initial_setup()) to avoid stopping
  the RTC if it's already configured
- updated the RTC range to 2000-2099
- updated the compatible with the generic one (renesas,rz-rtca3) in the
  idea the driver will be also used by the RZ/V2H w/o the necessity to
  add a new compatible

 MAINTAINERS                     |   8 +
 drivers/rtc/Kconfig             |  10 +
 drivers/rtc/Makefile            |   1 +
 drivers/rtc/rtc-renesas-rtca3.c | 892 ++++++++++++++++++++++++++++++++
 4 files changed, 911 insertions(+)
 create mode 100644 drivers/rtc/rtc-renesas-rtca3.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 73008c27fcac..6866502e2653 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19591,6 +19591,14 @@ S:	Supported
 F:	Documentation/devicetree/bindings/timer/renesas,rz-mtu3.yaml
 F:	drivers/counter/rz-mtu3-cnt.c
 
+RENESAS RTCA-3 RTC DRIVER
+M:	Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
+L:	linux-rtc@vger.kernel.org
+L:	linux-renesas-soc@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/rtc/renesas,rz-rtca3.yaml
+F:	drivers/rtc/rtc-renesas-rtca3.c
+
 RENESAS RZ/N1 A5PSW SWITCH DRIVER
 M:	Clément Léger <clement.leger@bootlin.com>
 L:	linux-renesas-soc@vger.kernel.org
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index b3469f6986e9..e43997a6c05d 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1984,6 +1984,16 @@ config RTC_DRV_MA35D1
 	   This driver can also be built as a module, if so, the module
 	   will be called "rtc-ma35d1".
 
+config RTC_DRV_RENESAS_RTCA3
+	tristate "Renesas RTCA-3 RTC"
+	depends on ARCH_RENESAS
+	help
+	  If you say yes here you get support for the Renesas RTCA-3 RTC
+	  available on the Renesas RZ/G3S SoC.
+
+	  This driver can also be built as a module, if so, the module
+	  will be called "rtc-rtca3".
+
 comment "HID Sensor RTC drivers"
 
 config RTC_DRV_HID_SENSOR_TIME
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 3004e372f25f..52844f13b247 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -157,6 +157,7 @@ obj-$(CONFIG_RTC_DRV_RX8025)	+= rtc-rx8025.o
 obj-$(CONFIG_RTC_DRV_RX8111)	+= rtc-rx8111.o
 obj-$(CONFIG_RTC_DRV_RX8581)	+= rtc-rx8581.o
 obj-$(CONFIG_RTC_DRV_RZN1)	+= rtc-rzn1.o
+obj-$(CONFIG_RTC_DRV_RENESAS_RTCA3)	+= rtc-renesas-rtca3.o
 obj-$(CONFIG_RTC_DRV_S35390A)	+= rtc-s35390a.o
 obj-$(CONFIG_RTC_DRV_S3C)	+= rtc-s3c.o
 obj-$(CONFIG_RTC_DRV_S5M)	+= rtc-s5m.o
diff --git a/drivers/rtc/rtc-renesas-rtca3.c b/drivers/rtc/rtc-renesas-rtca3.c
new file mode 100644
index 000000000000..822c055b6e4d
--- /dev/null
+++ b/drivers/rtc/rtc-renesas-rtca3.c
@@ -0,0 +1,892 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * On-Chip RTC Support available on RZ/G3S SoC
+ *
+ * Copyright (C) 2024 Renesas Electronics Corp.
+ */
+#include <linux/bcd.h>
+#include <linux/cleanup.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/iopoll.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <linux/rtc.h>
+
+/* Counter registers. */
+#define RTCA3_RSECCNT			0x2
+#define RTCA3_RSECCNT_SEC		GENMASK(6, 0)
+#define RTCA3_RMINCNT			0x4
+#define RTCA3_RMINCNT_MIN		GENMASK(6, 0)
+#define RTCA3_RHRCNT			0x6
+#define RTCA3_RHRCNT_HR			GENMASK(5, 0)
+#define RTCA3_RHRCNT_PM			BIT(6)
+#define RTCA3_RWKCNT			0x8
+#define RTCA3_RWKCNT_WK			GENMASK(2, 0)
+#define RTCA3_RDAYCNT			0xa
+#define RTCA3_RDAYCNT_DAY		GENMASK(5, 0)
+#define RTCA3_RMONCNT			0xc
+#define RTCA3_RMONCNT_MONTH		GENMASK(4, 0)
+#define RTCA3_RYRCNT			0xe
+#define RTCA3_RYRCNT_YEAR		GENMASK(7, 0)
+
+/* Alarm registers. */
+#define RTCA3_RSECAR			0x10
+#define RTCA3_RSECAR_SEC		GENMASK(6, 0)
+#define RTCA3_RMINAR			0x12
+#define RTCA3_RMINAR_MIN		GENMASK(6, 0)
+#define RTCA3_RHRAR			0x14
+#define RTCA3_RHRAR_HR			GENMASK(5, 0)
+#define RTCA3_RHRAR_PM			BIT(6)
+#define RTCA3_RWKAR			0x16
+#define RTCA3_RWKAR_DAYW		GENMASK(2, 0)
+#define RTCA3_RDAYAR			0x18
+#define RTCA3_RDAYAR_DATE		GENMASK(5, 0)
+#define RTCA3_RMONAR			0x1a
+#define RTCA3_RMONAR_MON		GENMASK(4, 0)
+#define RTCA3_RYRAR			0x1c
+#define RTCA3_RYRAR_YR			GENMASK(7, 0)
+#define RTCA3_RYRAREN			0x1e
+
+/* Alarm enable bit (for all alarm registers). */
+#define RTCA3_AR_ENB			BIT(7)
+
+/* Control registers. */
+#define RTCA3_RCR1			0x22
+#define RTCA3_RCR1_AIE			BIT(0)
+#define RTCA3_RCR1_CIE			BIT(1)
+#define RTCA3_RCR1_PIE			BIT(2)
+#define RTCA3_RCR1_PES			GENMASK(7, 4)
+#define RTCA3_RCR1_PES_1_64_SEC		0x8
+#define RTCA3_RCR2			0x24
+#define RTCA3_RCR2_START		BIT(0)
+#define RTCA3_RCR2_RESET		BIT(1)
+#define RTCA3_RCR2_AADJE		BIT(4)
+#define RTCA3_RCR2_ADJP			BIT(5)
+#define RTCA3_RCR2_HR24			BIT(6)
+#define RTCA3_RCR2_CNTMD		BIT(7)
+#define RTCA3_RSR			0x20
+#define RTCA3_RSR_AF			BIT(0)
+#define RTCA3_RSR_CF			BIT(1)
+#define RTCA3_RSR_PF			BIT(2)
+#define RTCA3_RADJ			0x2e
+#define RTCA3_RADJ_ADJ			GENMASK(5, 0)
+#define RTCA3_RADJ_ADJ_MAX		0x3f
+#define RTCA3_RADJ_PMADJ		GENMASK(7, 6)
+#define RTCA3_RADJ_PMADJ_NONE		0
+#define RTCA3_RADJ_PMADJ_ADD		1
+#define RTCA3_RADJ_PMADJ_SUB		2
+
+/* Polling operation timeouts. */
+#define RTCA3_DEFAULT_TIMEOUT_US	150
+#define RTCA3_IRQSET_TIMEOUT_US		5000
+#define RTCA3_START_TIMEOUT_US		150000
+#define RTCA3_RESET_TIMEOUT_US		200000
+
+/**
+ * enum rtca3_alrm_set_step - RTCA3 alarm set steps
+ * @RTCA3_ALRM_SSTEP_DONE: alarm setup done step
+ * @RTCA3_ALRM_SSTEP_IRQ: two 1/64 periodic IRQs were generated step
+ * @RTCA3_ALRM_SSTEP_INIT: alarm setup initialization step
+ */
+enum rtca3_alrm_set_step {
+	RTCA3_ALRM_SSTEP_DONE = 0,
+	RTCA3_ALRM_SSTEP_IRQ = 1,
+	RTCA3_ALRM_SSTEP_INIT = 3,
+};
+
+/**
+ * struct rtca3_ppb_per_cycle - PPB per cycle
+ * @ten_sec: PPB per cycle in 10 seconds adjutment mode
+ * @sixty_sec: PPB per cycle in 60 seconds adjustment mode
+ */
+struct rtca3_ppb_per_cycle {
+	int ten_sec;
+	int sixty_sec;
+};
+
+/**
+ * struct rtca3_priv - RTCA3 private data structure
+ * @base: base address
+ * @rtc_dev: RTC device
+ * @rstc: reset control
+ * @set_alarm_completion: alarm setup completion
+ * @alrm_sstep: alarm setup step (see enum rtca3_alrm_set_step)
+ * @lock: device lock
+ * @ppb: ppb per cycle for each the available adjustment modes
+ * @wakeup_irq: wakeup IRQ
+ */
+struct rtca3_priv {
+	void __iomem *base;
+	struct rtc_device *rtc_dev;
+	struct reset_control *rstc;
+	struct completion set_alarm_completion;
+	atomic_t alrm_sstep;
+	spinlock_t lock;
+	struct rtca3_ppb_per_cycle ppb;
+	int wakeup_irq;
+};
+
+static void rtca3_byte_update_bits(struct rtca3_priv *priv, u8 off, u8 mask, u8 val)
+{
+	u8 tmp;
+
+	tmp = readb(priv->base + off);
+	tmp &= ~mask;
+	tmp |= (val & mask);
+	writeb(tmp, priv->base + off);
+}
+
+static u8 rtca3_alarm_handler_helper(struct rtca3_priv *priv)
+{
+	u8 val, pending;
+
+	val = readb(priv->base + RTCA3_RSR);
+	pending = val & RTCA3_RSR_AF;
+	writeb(val & ~pending, priv->base + RTCA3_RSR);
+
+	if (pending)
+		rtc_update_irq(priv->rtc_dev, 1, RTC_AF | RTC_IRQF);
+
+	return pending;
+}
+
+static irqreturn_t rtca3_alarm_handler(int irq, void *dev_id)
+{
+	struct rtca3_priv *priv = dev_id;
+	u8 pending;
+
+	guard(spinlock)(&priv->lock);
+
+	pending = rtca3_alarm_handler_helper(priv);
+
+	return IRQ_RETVAL(pending);
+}
+
+static irqreturn_t rtca3_periodic_handler(int irq, void *dev_id)
+{
+	struct rtca3_priv *priv = dev_id;
+	u8 val, pending;
+
+	guard(spinlock)(&priv->lock);
+
+	val = readb(priv->base + RTCA3_RSR);
+	pending = val & RTCA3_RSR_PF;
+
+	if (pending) {
+		writeb(val & ~pending, priv->base + RTCA3_RSR);
+
+		if (atomic_read(&priv->alrm_sstep) > RTCA3_ALRM_SSTEP_IRQ) {
+			/* Alarm setup in progress. */
+			atomic_dec(&priv->alrm_sstep);
+
+			if (atomic_read(&priv->alrm_sstep) == RTCA3_ALRM_SSTEP_IRQ) {
+				/*
+				 * We got 2 * 1/64 periodic interrupts. Disable
+				 * interrupt and let alarm setup continue.
+				 */
+				rtca3_byte_update_bits(priv, RTCA3_RCR1,
+						       RTCA3_RCR1_PIE, 0);
+				readb_poll_timeout_atomic(priv->base + RTCA3_RCR1, val,
+							  !(val & RTCA3_RCR1_PIE),
+							  10, RTCA3_DEFAULT_TIMEOUT_US);
+				complete(&priv->set_alarm_completion);
+			}
+		}
+	}
+
+	return IRQ_RETVAL(pending);
+}
+
+static void rtca3_prepare_cntalrm_regs_for_read(struct rtca3_priv *priv, bool cnt)
+{
+	/* Offset b/w time and alarm registers. */
+	u8 offset = cnt ? 0 : 0xe;
+
+	/*
+	 * According to HW manual (section 22.6.4. Notes on writing to and
+	 * reading from registers) after writing to count registers, alarm
+	 * registers, year alarm enable register, bits RCR2.AADJE, AADJP,
+	 * and HR24 register, we need to do 3 empty reads before being
+	 * able to fetch the registers content.
+	 */
+	for (u8 i = 0; i < 3; i++) {
+		readb(priv->base + RTCA3_RSECCNT + offset);
+		readb(priv->base + RTCA3_RMINCNT + offset);
+		readb(priv->base + RTCA3_RHRCNT  + offset);
+		readb(priv->base + RTCA3_RWKCNT  + offset);
+		readb(priv->base + RTCA3_RDAYCNT + offset);
+		readw(priv->base + RTCA3_RYRCNT  + offset);
+		if (!cnt)
+			readb(priv->base + RTCA3_RYRAREN);
+	}
+}
+
+static int rtca3_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct rtca3_priv *priv = dev_get_drvdata(dev);
+	u8 sec, min, hour, wday, mday, month, tmp;
+	u8 trials = 0;
+	u32 year100;
+	u16 year;
+
+	guard(spinlock_irqsave)(&priv->lock);
+
+	tmp = readb(priv->base + RTCA3_RCR2);
+	if (!(tmp & RTCA3_RCR2_START))
+		return -EINVAL;
+
+	do {
+		/* Clear carry interrupt. */
+		rtca3_byte_update_bits(priv, RTCA3_RSR, RTCA3_RSR_CF, 0);
+
+		/* Read counters. */
+		sec = readb(priv->base + RTCA3_RSECCNT);
+		min = readb(priv->base + RTCA3_RMINCNT);
+		hour = readb(priv->base + RTCA3_RHRCNT);
+		wday = readb(priv->base + RTCA3_RWKCNT);
+		mday = readb(priv->base + RTCA3_RDAYCNT);
+		month = readb(priv->base + RTCA3_RMONCNT);
+		year = readw(priv->base + RTCA3_RYRCNT);
+
+		tmp = readb(priv->base + RTCA3_RSR);
+
+		/*
+		 * We cannot generate carries due to reading 64Hz counter as
+		 * the driver doesn't implement carry, thus, carries will be
+		 * generated once per seconds. Add a timeout of 5 trials here
+		 * to avoid infinite loop, if any.
+		 */
+	} while ((tmp & RTCA3_RSR_CF) && ++trials < 5);
+
+	if (trials >= 5)
+		return -ETIMEDOUT;
+
+	tm->tm_sec = bcd2bin(FIELD_GET(RTCA3_RSECCNT_SEC, sec));
+	tm->tm_min = bcd2bin(FIELD_GET(RTCA3_RMINCNT_MIN, min));
+	tm->tm_hour = bcd2bin(FIELD_GET(RTCA3_RHRCNT_HR, hour));
+	tm->tm_wday = bcd2bin(FIELD_GET(RTCA3_RWKCNT_WK, wday));
+	tm->tm_mday = bcd2bin(FIELD_GET(RTCA3_RDAYCNT_DAY, mday));
+	tm->tm_mon = bcd2bin(FIELD_GET(RTCA3_RMONCNT_MONTH, month)) - 1;
+	year = FIELD_GET(RTCA3_RYRCNT_YEAR, year);
+	year100 = bcd2bin((year == 0x99) ? 0x19 : 0x20);
+	tm->tm_year = (year100 * 100 + bcd2bin(year)) - 1900;
+
+	return 0;
+}
+
+static int rtca3_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct rtca3_priv *priv = dev_get_drvdata(dev);
+	u8 rcr2, tmp;
+	int ret;
+
+	guard(spinlock_irqsave)(&priv->lock);
+
+	/* Stop the RTC. */
+	rcr2 = readb(priv->base + RTCA3_RCR2);
+	writeb(rcr2 & ~RTCA3_RCR2_START, priv->base + RTCA3_RCR2);
+	ret = readb_poll_timeout_atomic(priv->base + RTCA3_RCR2, tmp,
+					!(tmp & RTCA3_RCR2_START),
+					10, RTCA3_DEFAULT_TIMEOUT_US);
+	if (ret)
+		return ret;
+
+	/* Update time. */
+	writeb(bin2bcd(tm->tm_sec), priv->base + RTCA3_RSECCNT);
+	writeb(bin2bcd(tm->tm_min), priv->base + RTCA3_RMINCNT);
+	writeb(bin2bcd(tm->tm_hour), priv->base + RTCA3_RHRCNT);
+	writeb(bin2bcd(tm->tm_wday), priv->base + RTCA3_RWKCNT);
+	writeb(bin2bcd(tm->tm_mday), priv->base + RTCA3_RDAYCNT);
+	writeb(bin2bcd(tm->tm_mon + 1), priv->base + RTCA3_RMONCNT);
+	writew(bin2bcd(tm->tm_year % 100), priv->base + RTCA3_RYRCNT);
+
+	/* Make sure we can read back the counters. */
+	rtca3_prepare_cntalrm_regs_for_read(priv, true);
+
+	/* Start RTC. */
+	writeb(rcr2 | RTCA3_RCR2_START, priv->base + RTCA3_RCR2);
+	return readb_poll_timeout_atomic(priv->base + RTCA3_RCR2, tmp,
+					 (tmp & RTCA3_RCR2_START),
+					 10, RTCA3_DEFAULT_TIMEOUT_US);
+}
+
+static int rtca3_alarm_irq_set_helper(struct rtca3_priv *priv,
+				      u8 interrupts,
+				      unsigned int enabled)
+{
+	u8 tmp, val;
+
+	if (enabled) {
+		/*
+		 * AIE, CIE, PIE bit indexes in RSR corresponds with
+		 * those on RCR1. Same interrupts mask can be used.
+		 */
+		rtca3_byte_update_bits(priv, RTCA3_RSR, interrupts, 0);
+		val = interrupts;
+	} else {
+		val = 0;
+	}
+
+	rtca3_byte_update_bits(priv, RTCA3_RCR1, interrupts, val);
+	return readb_poll_timeout_atomic(priv->base + RTCA3_RCR1, tmp,
+					 ((tmp & interrupts) == val),
+					 10, RTCA3_IRQSET_TIMEOUT_US);
+}
+
+static int rtca3_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+	struct rtca3_priv *priv = dev_get_drvdata(dev);
+
+	guard(spinlock_irqsave)(&priv->lock);
+
+	return rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE, enabled);
+}
+
+static int rtca3_read_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+	struct rtca3_priv *priv = dev_get_drvdata(dev);
+	u8 sec, min, hour, wday, mday, month;
+	struct rtc_time *tm = &wkalrm->time;
+	u32 year100;
+	u16 year;
+
+	guard(spinlock_irqsave)(&priv->lock);
+
+	sec = readb(priv->base + RTCA3_RSECAR);
+	min = readb(priv->base + RTCA3_RMINAR);
+	hour = readb(priv->base + RTCA3_RHRAR);
+	wday = readb(priv->base + RTCA3_RWKAR);
+	mday = readb(priv->base + RTCA3_RDAYAR);
+	month = readb(priv->base + RTCA3_RMONAR);
+	year = readw(priv->base + RTCA3_RYRAR);
+
+	tm->tm_sec = bcd2bin(FIELD_GET(RTCA3_RSECAR_SEC, sec));
+	tm->tm_min = bcd2bin(FIELD_GET(RTCA3_RMINAR_MIN, min));
+	tm->tm_hour = bcd2bin(FIELD_GET(RTCA3_RHRAR_HR, hour));
+	tm->tm_wday = bcd2bin(FIELD_GET(RTCA3_RWKAR_DAYW, wday));
+	tm->tm_mday = bcd2bin(FIELD_GET(RTCA3_RDAYAR_DATE, mday));
+	tm->tm_mon = bcd2bin(FIELD_GET(RTCA3_RMONAR_MON, month)) - 1;
+	year = FIELD_GET(RTCA3_RYRAR_YR, year);
+	year100 = bcd2bin((year == 0x99) ? 0x19 : 0x20);
+	tm->tm_year = (year100 * 100 + bcd2bin(year)) - 1900;
+
+	wkalrm->enabled = !!(readb(priv->base + RTCA3_RCR1) & RTCA3_RCR1_AIE);
+
+	return 0;
+}
+
+static int rtca3_set_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+	struct rtca3_priv *priv = dev_get_drvdata(dev);
+	struct rtc_time *tm = &wkalrm->time;
+	u8 rcr1, tmp;
+	int ret;
+
+	scoped_guard(spinlock_irqsave, &priv->lock) {
+		tmp = readb(priv->base + RTCA3_RCR2);
+		if (!(tmp & RTCA3_RCR2_START))
+			return -EPERM;
+
+		/* Disable AIE to prevent false interrupts. */
+		rcr1 = readb(priv->base + RTCA3_RCR1);
+		rcr1 &= ~RTCA3_RCR1_AIE;
+		writeb(rcr1, priv->base + RTCA3_RCR1);
+		ret = readb_poll_timeout_atomic(priv->base + RTCA3_RCR1, tmp,
+						!(tmp & RTCA3_RCR1_AIE),
+						10, RTCA3_DEFAULT_TIMEOUT_US);
+		if (ret)
+			return ret;
+
+		/* Set the time and enable the alarm. */
+		writeb(RTCA3_AR_ENB | bin2bcd(tm->tm_sec), priv->base + RTCA3_RSECAR);
+		writeb(RTCA3_AR_ENB | bin2bcd(tm->tm_min), priv->base + RTCA3_RMINAR);
+		writeb(RTCA3_AR_ENB | bin2bcd(tm->tm_hour), priv->base + RTCA3_RHRAR);
+		writeb(RTCA3_AR_ENB | bin2bcd(tm->tm_wday), priv->base + RTCA3_RWKAR);
+		writeb(RTCA3_AR_ENB | bin2bcd(tm->tm_mday), priv->base + RTCA3_RDAYAR);
+		writeb(RTCA3_AR_ENB | bin2bcd(tm->tm_mon + 1), priv->base + RTCA3_RMONAR);
+
+		writew(bin2bcd(tm->tm_year % 100), priv->base + RTCA3_RYRAR);
+		writeb(RTCA3_AR_ENB, priv->base + RTCA3_RYRAREN);
+
+		/* Make sure we can read back the counters. */
+		rtca3_prepare_cntalrm_regs_for_read(priv, false);
+
+		/* Need to wait for 2 * 1/64 periodic interrupts to be generated. */
+		atomic_set(&priv->alrm_sstep, RTCA3_ALRM_SSTEP_INIT);
+		reinit_completion(&priv->set_alarm_completion);
+
+		/* Enable periodic interrupt. */
+		rcr1 |= RTCA3_RCR1_PIE;
+		writeb(rcr1, priv->base + RTCA3_RCR1);
+		ret = readb_poll_timeout_atomic(priv->base + RTCA3_RCR1, tmp,
+						(tmp & RTCA3_RCR1_PIE),
+						10, RTCA3_IRQSET_TIMEOUT_US);
+	}
+
+	if (ret)
+		goto setup_failed;
+
+	/* Wait for the 2 * 1/64 periodic interrupts. */
+	ret = wait_for_completion_interruptible_timeout(&priv->set_alarm_completion,
+							msecs_to_jiffies(500));
+	if (ret <= 0) {
+		ret = -ETIMEDOUT;
+		goto setup_failed;
+	}
+
+	scoped_guard(spinlock_irqsave, &priv->lock) {
+		ret = rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE, wkalrm->enabled);
+		atomic_set(&priv->alrm_sstep, RTCA3_ALRM_SSTEP_DONE);
+	}
+
+	return ret;
+
+setup_failed:
+	scoped_guard(spinlock_irqsave, &priv->lock) {
+		/*
+		 * Disable PIE to avoid interrupt storm in case HW needed more than
+		 * specified timeout for setup.
+		 */
+		writeb(rcr1 & ~RTCA3_RCR1_PIE, priv->base + RTCA3_RCR1);
+		readb_poll_timeout_atomic(priv->base + RTCA3_RCR1, tmp, !(tmp & ~RTCA3_RCR1_PIE),
+					  10, RTCA3_DEFAULT_TIMEOUT_US);
+		atomic_set(&priv->alrm_sstep, RTCA3_ALRM_SSTEP_DONE);
+	}
+
+	return ret;
+}
+
+static int rtca3_read_offset(struct device *dev, long *offset)
+{
+	struct rtca3_priv *priv = dev_get_drvdata(dev);
+	u8 val, radj, cycles;
+	u32 ppb_per_cycle;
+
+	scoped_guard(spinlock_irqsave, &priv->lock) {
+		radj = readb(priv->base + RTCA3_RADJ);
+		val = readb(priv->base + RTCA3_RCR2);
+	}
+
+	cycles = FIELD_GET(RTCA3_RADJ_ADJ, radj);
+
+	if (!cycles) {
+		*offset = 0;
+		return 0;
+	}
+
+	if (val & RTCA3_RCR2_ADJP)
+		ppb_per_cycle = priv->ppb.ten_sec;
+	else
+		ppb_per_cycle = priv->ppb.sixty_sec;
+
+	*offset = cycles * ppb_per_cycle;
+	val = FIELD_GET(RTCA3_RADJ_PMADJ, radj);
+	if (val == RTCA3_RADJ_PMADJ_SUB)
+		*offset = -(*offset);
+
+	return 0;
+}
+
+static int rtca3_set_offset(struct device *dev, long offset)
+{
+	struct rtca3_priv *priv = dev_get_drvdata(dev);
+	int cycles, cycles10, cycles60;
+	u8 radj, adjp, tmp;
+	int ret;
+
+	/*
+	 * Automatic time error adjustment could be set at intervals of 10
+	 * or 60 seconds.
+	 */
+	cycles10 = DIV_ROUND_CLOSEST(offset, priv->ppb.ten_sec);
+	cycles60 = DIV_ROUND_CLOSEST(offset, priv->ppb.sixty_sec);
+
+	/* We can set b/w 1 and 63 clock cycles. */
+	if (cycles60 >= -RTCA3_RADJ_ADJ_MAX &&
+	    cycles60 <= RTCA3_RADJ_ADJ_MAX) {
+		cycles = cycles60;
+		adjp = 0;
+	} else if (cycles10 >= -RTCA3_RADJ_ADJ_MAX &&
+		   cycles10 <= RTCA3_RADJ_ADJ_MAX) {
+		cycles = cycles10;
+		adjp = RTCA3_RCR2_ADJP;
+	} else {
+		return -ERANGE;
+	}
+
+	radj = FIELD_PREP(RTCA3_RADJ_ADJ, abs(cycles));
+	if (!cycles)
+		radj |= FIELD_PREP(RTCA3_RADJ_PMADJ, RTCA3_RADJ_PMADJ_NONE);
+	else if (cycles > 0)
+		radj |= FIELD_PREP(RTCA3_RADJ_PMADJ, RTCA3_RADJ_PMADJ_ADD);
+	else
+		radj |= FIELD_PREP(RTCA3_RADJ_PMADJ, RTCA3_RADJ_PMADJ_SUB);
+
+	guard(spinlock_irqsave)(&priv->lock);
+
+	tmp = readb(priv->base + RTCA3_RCR2);
+
+	if ((tmp & RTCA3_RCR2_ADJP) != adjp) {
+		/* RADJ.PMADJ need to be set to zero before setting RCR2.ADJP. */
+		writeb(0, priv->base + RTCA3_RADJ);
+		ret = readb_poll_timeout_atomic(priv->base + RTCA3_RADJ, tmp, !tmp,
+						10, RTCA3_DEFAULT_TIMEOUT_US);
+		if (ret)
+			return ret;
+
+		rtca3_byte_update_bits(priv, RTCA3_RCR2, RTCA3_RCR2_ADJP, adjp);
+		ret = readb_poll_timeout_atomic(priv->base + RTCA3_RCR2, tmp,
+						((tmp & RTCA3_RCR2_ADJP) == adjp),
+						10, RTCA3_DEFAULT_TIMEOUT_US);
+		if (ret)
+			return ret;
+	}
+
+	writeb(radj, priv->base + RTCA3_RADJ);
+	return readb_poll_timeout_atomic(priv->base + RTCA3_RADJ, tmp, (tmp == radj),
+					 10, RTCA3_DEFAULT_TIMEOUT_US);
+}
+
+static const struct rtc_class_ops rtca3_ops = {
+	.read_time = rtca3_read_time,
+	.set_time = rtca3_set_time,
+	.read_alarm = rtca3_read_alarm,
+	.set_alarm = rtca3_set_alarm,
+	.alarm_irq_enable = rtca3_alarm_irq_enable,
+	.set_offset = rtca3_set_offset,
+	.read_offset = rtca3_read_offset,
+};
+
+static int rtca3_initial_setup(struct clk *clk, struct rtca3_priv *priv)
+{
+	unsigned long osc32k_rate;
+	u8 val, tmp, mask;
+	u32 sleep_us;
+	int ret;
+
+	osc32k_rate = clk_get_rate(clk);
+	if (!osc32k_rate)
+		return -EINVAL;
+
+	sleep_us = DIV_ROUND_UP_ULL(1000000ULL, osc32k_rate) * 6;
+
+	priv->ppb.ten_sec = DIV_ROUND_CLOSEST_ULL(1000000000ULL, (osc32k_rate * 10));
+	priv->ppb.sixty_sec = DIV_ROUND_CLOSEST_ULL(1000000000ULL, (osc32k_rate * 60));
+
+	/*
+	 * According to HW manual (section 22.4.2. Clock and count mode setting procedure)
+	 * we need to wait at least 6 cycles of the 32KHz clock after clock was enabled.
+	 */
+	usleep_range(sleep_us, sleep_us + 10);
+
+	/* Disable alarm and carry interrupts. */
+	mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE;
+	ret = rtca3_alarm_irq_set_helper(priv, mask, 0);
+	if (ret)
+		return ret;
+
+	mask = RTCA3_RCR2_START | RTCA3_RCR2_HR24;
+	val = readb(priv->base + RTCA3_RCR2);
+	/* Nothing to do if already started in 24 hours and calendar count mode. */
+	if ((val & mask) == mask)
+		return 0;
+
+	/* Reconfigure the RTC in 24 hours and calendar count mode. */
+	mask = RTCA3_RCR2_START | RTCA3_RCR2_CNTMD;
+	writeb(0, priv->base + RTCA3_RCR2);
+	ret = readb_poll_timeout(priv->base + RTCA3_RCR2, tmp, !(tmp & mask),
+				 10, RTCA3_DEFAULT_TIMEOUT_US);
+	if (ret)
+		return ret;
+
+	/*
+	 * Set 24 hours mode. According to HW manual (section 22.3.19. RTC Control
+	 * Register 2) this needs to be done separate from stop operation.
+	 */
+	mask = RTCA3_RCR2_HR24;
+	val = RTCA3_RCR2_HR24;
+	writeb(val, priv->base + RTCA3_RCR2);
+	ret = readb_poll_timeout(priv->base + RTCA3_RCR2, tmp, (tmp & mask),
+				 10, RTCA3_DEFAULT_TIMEOUT_US);
+	if (ret)
+		return ret;
+
+	/* Execute reset. */
+	mask = RTCA3_RCR2_RESET;
+	writeb(val | RTCA3_RCR2_RESET, priv->base + RTCA3_RCR2);
+	ret = readb_poll_timeout(priv->base + RTCA3_RCR2, tmp, !(tmp & mask),
+				 10, RTCA3_RESET_TIMEOUT_US);
+	if (ret)
+		return ret;
+
+	/*
+	 * According to HW manual (section 22.6.3. Notes on writing to and reading
+	 * from registers) after reset we need to wait 6 clock cycles before
+	 * writing to RTC registers.
+	 */
+	usleep_range(sleep_us, sleep_us + 10);
+
+	/* Set no adjustment. */
+	writeb(0, priv->base + RTCA3_RADJ);
+	ret = readb_poll_timeout(priv->base + RTCA3_RADJ, tmp, !tmp, 10,
+				 RTCA3_DEFAULT_TIMEOUT_US);
+
+	/* Start the RTC and enable automatic time error adjustment. */
+	mask = RTCA3_RCR2_START | RTCA3_RCR2_AADJE;
+	val |= RTCA3_RCR2_START | RTCA3_RCR2_AADJE;
+	writeb(val, priv->base + RTCA3_RCR2);
+	ret = readb_poll_timeout(priv->base + RTCA3_RCR2, tmp, ((tmp & mask) == mask),
+				 10, RTCA3_START_TIMEOUT_US);
+	if (ret)
+		return ret;
+
+	/*
+	 * According to HW manual (section 22.6.4. Notes on writing to and reading
+	 * from registers) we need to wait 1/128 seconds while the clock is operating
+	 * (RCR2.START bit = 1) to be able to read the counters after a return from
+	 * reset.
+	 */
+	usleep_range(8000, 9000);
+
+	/* Set period interrupt to 1/64 seconds. It is necessary for alarm setup. */
+	val = FIELD_PREP(RTCA3_RCR1_PES, RTCA3_RCR1_PES_1_64_SEC);
+	rtca3_byte_update_bits(priv, RTCA3_RCR1, RTCA3_RCR1_PES, val);
+	return readb_poll_timeout(priv->base + RTCA3_RCR1, tmp, ((tmp & RTCA3_RCR1_PES) == val),
+				  10, RTCA3_DEFAULT_TIMEOUT_US);
+}
+
+static int rtca3_request_irqs(struct platform_device *pdev, struct rtca3_priv *priv)
+{
+	struct device *dev = &pdev->dev;
+	int ret, irq;
+
+	irq = platform_get_irq_byname(pdev, "alarm");
+	if (irq < 0)
+		return dev_err_probe(dev, irq, "Failed to get alarm IRQ!\n");
+
+	ret = devm_request_irq(dev, irq, rtca3_alarm_handler, 0, "rtca3-alarm", priv);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to request alarm IRQ!\n");
+	priv->wakeup_irq = irq;
+
+	irq = platform_get_irq_byname(pdev, "period");
+	if (irq < 0)
+		return dev_err_probe(dev, irq, "Failed to get period IRQ!\n");
+
+	ret = devm_request_irq(dev, irq, rtca3_periodic_handler, 0, "rtca3-period", priv);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to request period IRQ!\n");
+
+	/*
+	 * Driver doesn't implement carry handler. Just get the IRQ here
+	 * for backward compatibility, in case carry support will be added later.
+	 */
+	irq = platform_get_irq_byname(pdev, "carry");
+	if (irq < 0)
+		return dev_err_probe(dev, irq, "Failed to get carry IRQ!\n");
+
+	return 0;
+}
+
+static void rtca3_action(void *data)
+{
+	struct device *dev = data;
+	struct rtca3_priv *priv = dev_get_drvdata(dev);
+	int ret;
+
+	ret = reset_control_assert(priv->rstc);
+	if (ret)
+		dev_err(dev, "Failed to de-assert reset!");
+
+	ret = pm_runtime_put_sync(dev);
+	if (ret < 0)
+		dev_err(dev, "Failed to runtime suspend!");
+}
+
+static int rtca3_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rtca3_priv *priv;
+	struct clk *clk;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
+	priv->rstc = devm_reset_control_get_shared(dev, NULL);
+	if (IS_ERR(priv->rstc))
+		return PTR_ERR(priv->rstc);
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return ret;
+
+	ret = reset_control_deassert(priv->rstc);
+	if (ret) {
+		pm_runtime_put_sync(dev);
+		return ret;
+	}
+
+	dev_set_drvdata(dev, priv);
+	ret = devm_add_action_or_reset(dev, rtca3_action, dev);
+	if (ret)
+		return ret;
+
+	clk = devm_clk_get_enabled(dev, "counter");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	spin_lock_init(&priv->lock);
+	atomic_set(&priv->alrm_sstep, RTCA3_ALRM_SSTEP_DONE);
+	init_completion(&priv->set_alarm_completion);
+
+	ret = rtca3_initial_setup(clk, priv);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to setup the RTC!\n");
+
+	ret = rtca3_request_irqs(pdev, priv);
+	if (ret)
+		return ret;
+
+	device_init_wakeup(&pdev->dev, 1);
+
+	priv->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
+	if (IS_ERR(priv->rtc_dev))
+		return PTR_ERR(priv->rtc_dev);
+
+	priv->rtc_dev->ops = &rtca3_ops;
+	priv->rtc_dev->max_user_freq = 256;
+	priv->rtc_dev->range_min = mktime64(2000, 1, 1, 0, 0, 0);
+	priv->rtc_dev->range_max = mktime64(2099, 12, 31, 23, 59, 59);
+
+	return devm_rtc_register_device(priv->rtc_dev);
+}
+
+static void rtca3_remove(struct platform_device *pdev)
+{
+	struct rtca3_priv *priv = platform_get_drvdata(pdev);
+
+	guard(spinlock_irqsave)(&priv->lock);
+
+	/* Disable alarm, periodic interrupts. */
+	rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
+}
+
+static int rtca3_suspend(struct device *dev)
+{
+	struct rtca3_priv *priv = dev_get_drvdata(dev);
+
+	if (!device_may_wakeup(dev))
+		return 0;
+
+	/* Alarm setup in progress. */
+	if (atomic_read(&priv->alrm_sstep) != RTCA3_ALRM_SSTEP_DONE)
+		return -EBUSY;
+
+	enable_irq_wake(priv->wakeup_irq);
+
+	return 0;
+}
+
+static int rtca3_clean_alarm(struct rtca3_priv *priv)
+{
+	struct rtc_device *rtc_dev = priv->rtc_dev;
+	time64_t alarm_time, now;
+	struct rtc_wkalrm alarm;
+	struct rtc_time tm;
+	u8 pending;
+	int ret;
+
+	ret = rtc_read_alarm(rtc_dev, &alarm);
+	if (ret)
+		return ret;
+
+	if (!alarm.enabled)
+		return 0;
+
+	ret = rtc_read_time(rtc_dev, &tm);
+	if (ret)
+		return ret;
+
+	alarm_time = rtc_tm_to_time64(&alarm.time);
+	now = rtc_tm_to_time64(&tm);
+	if (alarm_time >= now)
+		return 0;
+
+	/*
+	 * Heuristically, it has been determined that when returning from deep
+	 * sleep state the RTCA3_RSR.AF is zero even though the alarm expired.
+	 * Call again the rtc_update_irq() if alarm helper detects this.
+	 */
+
+	guard(spinlock_irqsave)(&priv->lock);
+
+	pending = rtca3_alarm_handler_helper(priv);
+	if (!pending)
+		rtc_update_irq(priv->rtc_dev, 1, RTC_AF | RTC_IRQF);
+
+	return 0;
+}
+
+static int rtca3_resume(struct device *dev)
+{
+	struct rtca3_priv *priv = dev_get_drvdata(dev);
+
+	if (!device_may_wakeup(dev))
+		return 0;
+
+	disable_irq_wake(priv->wakeup_irq);
+
+	/*
+	 * According to the HW manual (section 22.6.4 Notes on writing to
+	 * and reading from registers) we need to wait 1/128 seconds while
+	 * RCR2.START = 1 to be able to read the counters after a return from low
+	 * power consumption state.
+	 */
+	mdelay(8);
+
+	/*
+	 * The alarm cannot wake the system from deep sleep states. In case
+	 * we return from deep sleep states and the alarm expired we need
+	 * to disable it to avoid failures when setting another alarm.
+	 */
+	return rtca3_clean_alarm(priv);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(rtca3_pm_ops, rtca3_suspend, rtca3_resume);
+
+static const struct of_device_id rtca3_of_match[] = {
+	{ .compatible = "renesas,rz-rtca3", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rtca3_of_match);
+
+static struct platform_driver rtca3_platform_driver = {
+	.driver = {
+		.name = "rtc-rtca3",
+		.pm = pm_ptr(&rtca3_pm_ops),
+		.of_match_table = rtca3_of_match,
+	},
+	.probe = rtca3_probe,
+	.remove_new = rtca3_remove,
+};
+module_platform_driver(rtca3_platform_driver);
+
+MODULE_DESCRIPTION("Renesas RTCA-3 RTC driver");
+MODULE_AUTHOR("Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>");
+MODULE_LICENSE("GPL");
-- 
2.39.2

Re: [PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC

[Alexandre Belloni]

On 30/08/2024 16:02:12+0300, Claudiu wrote:
> +	priv->rtc_dev->range_min = mktime64(2000, 1, 1, 0, 0, 0);

RTC_TIMESTAMP_BEGIN_2000

> +	priv->rtc_dev->range_max = mktime64(2099, 12, 31, 23, 59, 59);

RTC_TIMESTAMP_END_2099

> +
> +	return devm_rtc_register_device(priv->rtc_dev);
> +}
> +
> +static void rtca3_remove(struct platform_device *pdev)
> +{
> +	struct rtca3_priv *priv = platform_get_drvdata(pdev);
> +
> +	guard(spinlock_irqsave)(&priv->lock);
> +
> +	/* Disable alarm, periodic interrupts. */
> +	rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);

Why do you disable alarms on driver remove? I think you need to add a
comment if this is because it can't system up, else this is a bad
practice.

> +
> +static int rtca3_clean_alarm(struct rtca3_priv *priv)
> +{
> +	struct rtc_device *rtc_dev = priv->rtc_dev;
> +	time64_t alarm_time, now;
> +	struct rtc_wkalrm alarm;
> +	struct rtc_time tm;
> +	u8 pending;
> +	int ret;
> +
> +	ret = rtc_read_alarm(rtc_dev, &alarm);
> +	if (ret)
> +		return ret;
> +
> +	if (!alarm.enabled)
> +		return 0;
> +
> +	ret = rtc_read_time(rtc_dev, &tm);
> +	if (ret)
> +		return ret;
> +
> +	alarm_time = rtc_tm_to_time64(&alarm.time);
> +	now = rtc_tm_to_time64(&tm);
> +	if (alarm_time >= now)
> +		return 0;
> +
> +	/*
> +	 * Heuristically, it has been determined that when returning from deep
> +	 * sleep state the RTCA3_RSR.AF is zero even though the alarm expired.
> +	 * Call again the rtc_update_irq() if alarm helper detects this.
> +	 */
> +
> +	guard(spinlock_irqsave)(&priv->lock);
> +
> +	pending = rtca3_alarm_handler_helper(priv);
> +	if (!pending)
> +		rtc_update_irq(priv->rtc_dev, 1, RTC_AF | RTC_IRQF);
> +
> +	return 0;
> +}
> +
> +static int rtca3_resume(struct device *dev)
> +{
> +	struct rtca3_priv *priv = dev_get_drvdata(dev);
> +
> +	if (!device_may_wakeup(dev))
> +		return 0;
> +
> +	disable_irq_wake(priv->wakeup_irq);
> +
> +	/*
> +	 * According to the HW manual (section 22.6.4 Notes on writing to
> +	 * and reading from registers) we need to wait 1/128 seconds while
> +	 * RCR2.START = 1 to be able to read the counters after a return from low
> +	 * power consumption state.
> +	 */
> +	mdelay(8);
> +
> +	/*
> +	 * The alarm cannot wake the system from deep sleep states. In case
> +	 * we return from deep sleep states and the alarm expired we need
> +	 * to disable it to avoid failures when setting another alarm.
> +	 */
> +	return rtca3_clean_alarm(priv);
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(rtca3_pm_ops, rtca3_suspend, rtca3_resume);
> +
> +static const struct of_device_id rtca3_of_match[] = {
> +	{ .compatible = "renesas,rz-rtca3", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, rtca3_of_match);
> +
> +static struct platform_driver rtca3_platform_driver = {
> +	.driver = {
> +		.name = "rtc-rtca3",
> +		.pm = pm_ptr(&rtca3_pm_ops),
> +		.of_match_table = rtca3_of_match,
> +	},
> +	.probe = rtca3_probe,
> +	.remove_new = rtca3_remove,
> +};
> +module_platform_driver(rtca3_platform_driver);
> +
> +MODULE_DESCRIPTION("Renesas RTCA-3 RTC driver");
> +MODULE_AUTHOR("Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.39.2
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC

[claudiu beznea]

Hi, Alexandre,

On 31.08.2024 01:25, Alexandre Belloni wrote:
> On 30/08/2024 16:02:12+0300, Claudiu wrote:
>> +	priv->rtc_dev->range_min = mktime64(2000, 1, 1, 0, 0, 0);
> 
> RTC_TIMESTAMP_BEGIN_2000

OK

> 
>> +	priv->rtc_dev->range_max = mktime64(2099, 12, 31, 23, 59, 59);
> 
> RTC_TIMESTAMP_END_2099

OK

> 
>> +
>> +	return devm_rtc_register_device(priv->rtc_dev);
>> +}
>> +
>> +static void rtca3_remove(struct platform_device *pdev)
>> +{
>> +	struct rtca3_priv *priv = platform_get_drvdata(pdev);
>> +
>> +	guard(spinlock_irqsave)(&priv->lock);
>> +
>> +	/* Disable alarm, periodic interrupts. */
>> +	rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
> 
> Why do you disable alarms on driver remove? I think you need to add a
> comment if this is because it can't system up, else this is a bad
> practice.

The RTC cannot power on the system after a power off. It can't also resume
it from a deep sleep state (when only the SoC area where the RTC resides
remains power on (there is no way to signal from RTC to the power supply
chain that an alarm happened)). It can only wake it up from s2idle mode
where all SoC components remains powered on.

Also, w/o this change the RTC remains blocked under the following scenarios
if the interrupts are not disabled in remove:

1/ Configure wake alarm and unbind the RTC driver with the following commands:
# echo +10 > /sys/class/rtc/rtc0/wakealarm
# echo /sys/bus/platform/drivers/rtc-rtca3/1004ec00.rtc > unbind
# sleep 12
# echo /sys/bus/platform/drivers/rtc-rtca3/1004ec00.rtc > bind

When rebinding the re-configuration of the RTC device times out:
[  121.854190] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup
the RTC!
[  121.861511] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed
with error -110
-sh: echo: write error: Connection timed out

2/ Configure wake alarm, unbind the RTC driver and switch to s2idle with
the following commands:
# echo s2idle > /sys/power/mem_sleep
# echo +10 > /sys/class/rtc/rtc0/wakealarm
# echo /sys/bus/platform/drivers/rtc-rtca/31004ec00.rtc > unbind
# echo mem > /sys/power/state
# #system is resumed by non RTC wakeup source (as the RTC alarm is not
working anymore in this case)
# echo /sys/bus/platform/drivers/rtc-rtca/1004ec00.rtc > bind

The system is not waked up from RTC alarm (as expected) and the rebinding
fails again:

[  172.483688] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup
the RTC!
[  172.491003] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed
with error -110
-sh: echo: write error: Connection timed out

3/ configure the RTC alarm, unbind and power off (with the following commands):
# echo +60 > /sys/class/rtc/rtc0/wakealarm
# echo /sys/bus/platform/drivers/rtc-rtca/1004ec00.rtc > unbind
# poweroff

The system is not started after 60 seconds and at the next reboot the RTC
configuration on probe is failing the same:

[    0.292068] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup
the RTC!
[    0.292182] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed
with error -110

In all scenarios the RTC is recovered only if removing/re-applying the
power to the SoC area where it resides.

These tests were done with the patches in this series and then I tried it
with the following diff on top of the patches in this series. The results
were the same:

diff --git a/drivers/rtc/rtc-renesas-rtca3.c b/drivers/rtc/rtc-renesas-rtca3.c
index 822c055b6e4d..720fdac3adc6 100644
--- a/drivers/rtc/rtc-renesas-rtca3.c
+++ b/drivers/rtc/rtc-renesas-rtca3.c
@@ -586,7 +586,7 @@ static int rtca3_initial_setup(struct clk *clk, struct
rtca3_priv *priv)
        usleep_range(sleep_us, sleep_us + 10);

        /* Disable alarm and carry interrupts. */
-       mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE;
+       mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE | RTCA3_RCR1_PIE;
        ret = rtca3_alarm_irq_set_helper(priv, mask, 0);
        if (ret)
                return ret;
@@ -784,7 +784,7 @@ static void rtca3_remove(struct platform_device *pdev)
        guard(spinlock_irqsave)(&priv->lock);

        /* Disable alarm, periodic interrupts. */
-       rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
+       //rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
 }

Thank you,
Claudiu Beznea

Re: [PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC

[Alexandre Belloni]

On 02/09/2024 17:49:14+0300, claudiu beznea wrote:
> >> +	/* Disable alarm, periodic interrupts. */
> >> +	rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
> > 
> > Why do you disable alarms on driver remove? I think you need to add a
> > comment if this is because it can't system up, else this is a bad
> > practice.
> 
> The RTC cannot power on the system after a power off. It can't also resume
> it from a deep sleep state (when only the SoC area where the RTC resides
> remains power on (there is no way to signal from RTC to the power supply
> chain that an alarm happened)). It can only wake it up from s2idle mode
> where all SoC components remains powered on.
> 
> Also, w/o this change the RTC remains blocked under the following scenarios
> if the interrupts are not disabled in remove:
> 
> 1/ Configure wake alarm and unbind the RTC driver with the following commands:
> # echo +10 > /sys/class/rtc/rtc0/wakealarm
> # echo /sys/bus/platform/drivers/rtc-rtca3/1004ec00.rtc > unbind
> # sleep 12
> # echo /sys/bus/platform/drivers/rtc-rtca3/1004ec00.rtc > bind
> 
> When rebinding the re-configuration of the RTC device times out:
> [  121.854190] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup
> the RTC!
> [  121.861511] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed
> with error -110
> -sh: echo: write error: Connection timed out
> 
> 2/ Configure wake alarm, unbind the RTC driver and switch to s2idle with
> the following commands:
> # echo s2idle > /sys/power/mem_sleep
> # echo +10 > /sys/class/rtc/rtc0/wakealarm
> # echo /sys/bus/platform/drivers/rtc-rtca/31004ec00.rtc > unbind
> # echo mem > /sys/power/state
> # #system is resumed by non RTC wakeup source (as the RTC alarm is not
> working anymore in this case)
> # echo /sys/bus/platform/drivers/rtc-rtca/1004ec00.rtc > bind
> 
> The system is not waked up from RTC alarm (as expected) and the rebinding
> fails again:
> 
> [  172.483688] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup
> the RTC!
> [  172.491003] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed
> with error -110
> -sh: echo: write error: Connection timed out
> 
> 3/ configure the RTC alarm, unbind and power off (with the following commands):
> # echo +60 > /sys/class/rtc/rtc0/wakealarm
> # echo /sys/bus/platform/drivers/rtc-rtca/1004ec00.rtc > unbind
> # poweroff
> 
> The system is not started after 60 seconds and at the next reboot the RTC
> configuration on probe is failing the same:
> 
> [    0.292068] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup
> the RTC!
> [    0.292182] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed
> with error -110
> 
> In all scenarios the RTC is recovered only if removing/re-applying the
> power to the SoC area where it resides.
> 
> These tests were done with the patches in this series and then I tried it
> with the following diff on top of the patches in this series. The results
> were the same:
> 
> diff --git a/drivers/rtc/rtc-renesas-rtca3.c b/drivers/rtc/rtc-renesas-rtca3.c
> index 822c055b6e4d..720fdac3adc6 100644
> --- a/drivers/rtc/rtc-renesas-rtca3.c
> +++ b/drivers/rtc/rtc-renesas-rtca3.c
> @@ -586,7 +586,7 @@ static int rtca3_initial_setup(struct clk *clk, struct
> rtca3_priv *priv)
>         usleep_range(sleep_us, sleep_us + 10);
> 
>         /* Disable alarm and carry interrupts. */
> -       mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE;
> +       mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE | RTCA3_RCR1_PIE;
>         ret = rtca3_alarm_irq_set_helper(priv, mask, 0);
>         if (ret)
>                 return ret;
> @@ -784,7 +784,7 @@ static void rtca3_remove(struct platform_device *pdev)
>         guard(spinlock_irqsave)(&priv->lock);
> 
>         /* Disable alarm, periodic interrupts. */
> -       rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
> +       //rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
>  }

Thanks for the detailed explanation. Can you add a small comment, I
really want t avoid people cargo-culting this behavior as this has
already been the case.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC

[claudiu beznea]

On 02.09.2024 23:28, Alexandre Belloni wrote:
> Thanks for the detailed explanation. Can you add a small comment, I
> really want t avoid people cargo-culting this behavior as this has
> already been the case.

Sure, I'll update it in the next version.

Thank you,
Claudiu Beznea

Re: [PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC

[claudiu beznea]

Hi, Alexandre,

On 02.09.2024 17:49, claudiu beznea wrote:
> Hi, Alexandre,
> 
> On 31.08.2024 01:25, Alexandre Belloni wrote:
>> On 30/08/2024 16:02:12+0300, Claudiu wrote:
>>> +	priv->rtc_dev->range_min = mktime64(2000, 1, 1, 0, 0, 0);
>>
>> RTC_TIMESTAMP_BEGIN_2000
> 
> OK
> 
>>
>>> +	priv->rtc_dev->range_max = mktime64(2099, 12, 31, 23, 59, 59);
>>
>> RTC_TIMESTAMP_END_2099
> 
> OK
> 
>>
>>> +
>>> +	return devm_rtc_register_device(priv->rtc_dev);
>>> +}
>>> +
>>> +static void rtca3_remove(struct platform_device *pdev)
>>> +{
>>> +	struct rtca3_priv *priv = platform_get_drvdata(pdev);
>>> +
>>> +	guard(spinlock_irqsave)(&priv->lock);
>>> +
>>> +	/* Disable alarm, periodic interrupts. */
>>> +	rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
>>
>> Why do you disable alarms on driver remove? I think you need to add a
>> comment if this is because it can't system up, else this is a bad
>> practice.
> 
> The RTC cannot power on the system after a power off. It can't also resume
> it from a deep sleep state (when only the SoC area where the RTC resides
> remains power on (there is no way to signal from RTC to the power supply
> chain that an alarm happened)). It can only wake it up from s2idle mode
> where all SoC components remains powered on.

FTR, this is still valid.

> 
> Also, w/o this change the RTC remains blocked under the following scenarios
> if the interrupts are not disabled in remove:

This intrigued me and did some further investigation. I found that
something is wrong on the driver as described bellow.

The failures described in the previous emails were due to the fact that the
RTC counter clock was requested by the driver as devres managed resource.
This is the clock that feeds the RTC counting logic.

With the bellow diff applied on top of this series scenarios 1/ and 2/
described in the previous email works just fine.

For scenario 3/ the system is not powered up (as expected) but there are no
more failures on the next boots.

diff --git a/drivers/rtc/rtc-renesas-rtca3.c b/drivers/rtc/rtc-renesas-rtca3.c
index f908d2a1017d..c9adcadc58c0 100644
--- a/drivers/rtc/rtc-renesas-rtca3.c
+++ b/drivers/rtc/rtc-renesas-rtca3.c
@@ -747,10 +747,14 @@ static int rtca3_probe(struct platform_device *pdev)
        if (ret)
                return ret;

-       clk = devm_clk_get_enabled(dev, "counter");
+       clk = devm_clk_get(dev, "counter");
        if (IS_ERR(clk))
                return PTR_ERR(clk);

+       ret = clk_prepare_enable(clk);
+       if (ret)
+               return ret;
+
        spin_lock_init(&priv->lock);
        atomic_set(&priv->alrm_sstep, RTCA3_ALRM_SSTEP_DONE);
        init_completion(&priv->set_alarm_completion);
@@ -783,7 +787,7 @@ static void rtca3_remove(struct platform_device *pdev)

        guard(spinlock_irqsave)(&priv->lock);

-       rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
+       //rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
 }

Question is: the RTC counter clock should stay on when the driver is
unbinded, right?

Thank you,
Claudiu Beznea

> 
> 1/ Configure wake alarm and unbind the RTC driver with the following commands:
> # echo +10 > /sys/class/rtc/rtc0/wakealarm
> # echo /sys/bus/platform/drivers/rtc-rtca3/1004ec00.rtc > unbind
> # sleep 12
> # echo /sys/bus/platform/drivers/rtc-rtca3/1004ec00.rtc > bind
> 
> When rebinding the re-configuration of the RTC device times out:
> [  121.854190] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup
> the RTC!
> [  121.861511] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed
> with error -110
> -sh: echo: write error: Connection timed out
> 
> 2/ Configure wake alarm, unbind the RTC driver and switch to s2idle with
> the following commands:
> # echo s2idle > /sys/power/mem_sleep
> # echo +10 > /sys/class/rtc/rtc0/wakealarm
> # echo /sys/bus/platform/drivers/rtc-rtca/31004ec00.rtc > unbind
> # echo mem > /sys/power/state
> # #system is resumed by non RTC wakeup source (as the RTC alarm is not
> working anymore in this case)
> # echo /sys/bus/platform/drivers/rtc-rtca/1004ec00.rtc > bind
> 
> The system is not waked up from RTC alarm (as expected) and the rebinding
> fails again:
> 
> [  172.483688] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup
> the RTC!
> [  172.491003] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed
> with error -110
> -sh: echo: write error: Connection timed out
> 
> 3/ configure the RTC alarm, unbind and power off (with the following commands):
> # echo +60 > /sys/class/rtc/rtc0/wakealarm
> # echo /sys/bus/platform/drivers/rtc-rtca/1004ec00.rtc > unbind
> # poweroff
> 
> The system is not started after 60 seconds and at the next reboot the RTC
> configuration on probe is failing the same:
> 
> [    0.292068] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup
> the RTC!
> [    0.292182] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed
> with error -110
> 
> In all scenarios the RTC is recovered only if removing/re-applying the
> power to the SoC area where it resides.
> 
> These tests were done with the patches in this series and then I tried it
> with the following diff on top of the patches in this series. The results
> were the same:
> 
> diff --git a/drivers/rtc/rtc-renesas-rtca3.c b/drivers/rtc/rtc-renesas-rtca3.c
> index 822c055b6e4d..720fdac3adc6 100644
> --- a/drivers/rtc/rtc-renesas-rtca3.c
> +++ b/drivers/rtc/rtc-renesas-rtca3.c
> @@ -586,7 +586,7 @@ static int rtca3_initial_setup(struct clk *clk, struct
> rtca3_priv *priv)
>         usleep_range(sleep_us, sleep_us + 10);
> 
>         /* Disable alarm and carry interrupts. */
> -       mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE;
> +       mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE | RTCA3_RCR1_PIE;
>         ret = rtca3_alarm_irq_set_helper(priv, mask, 0);
>         if (ret)
>                 return ret;
> @@ -784,7 +784,7 @@ static void rtca3_remove(struct platform_device *pdev)
>         guard(spinlock_irqsave)(&priv->lock);
> 
>         /* Disable alarm, periodic interrupts. */
> -       rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
> +       //rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
>  }
> 
> Thank you,
> Claudiu Beznea
> 

Re: [PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC

[Alexandre Belloni]

On 03/09/2024 19:09:00+0300, claudiu beznea wrote:
> > 
> > Also, w/o this change the RTC remains blocked under the following scenarios
> > if the interrupts are not disabled in remove:
> 
> This intrigued me and did some further investigation. I found that
> something is wrong on the driver as described bellow.
> 
> The failures described in the previous emails were due to the fact that the
> RTC counter clock was requested by the driver as devres managed resource.
> This is the clock that feeds the RTC counting logic.
> 
> With the bellow diff applied on top of this series scenarios 1/ and 2/
> described in the previous email works just fine.
> 
> For scenario 3/ the system is not powered up (as expected) but there are no
> more failures on the next boots.
> 
> diff --git a/drivers/rtc/rtc-renesas-rtca3.c b/drivers/rtc/rtc-renesas-rtca3.c
> index f908d2a1017d..c9adcadc58c0 100644
> --- a/drivers/rtc/rtc-renesas-rtca3.c
> +++ b/drivers/rtc/rtc-renesas-rtca3.c
> @@ -747,10 +747,14 @@ static int rtca3_probe(struct platform_device *pdev)
>         if (ret)
>                 return ret;
> 
> -       clk = devm_clk_get_enabled(dev, "counter");
> +       clk = devm_clk_get(dev, "counter");
>         if (IS_ERR(clk))
>                 return PTR_ERR(clk);
> 
> +       ret = clk_prepare_enable(clk);
> +       if (ret)
> +               return ret;
> +
>         spin_lock_init(&priv->lock);
>         atomic_set(&priv->alrm_sstep, RTCA3_ALRM_SSTEP_DONE);
>         init_completion(&priv->set_alarm_completion);
> @@ -783,7 +787,7 @@ static void rtca3_remove(struct platform_device *pdev)
> 
>         guard(spinlock_irqsave)(&priv->lock);
> 
> -       rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
> +       //rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
>  }
> 
> Question is: the RTC counter clock should stay on when the driver is
> unbinded, right?

Yes, the lifecycle of the RTC and its ressources is longer than the
system's as the whole point is to keep the time and date running while
the system is off.

> 
> Thank you,
> Claudiu Beznea
> 
> > 
> > 1/ Configure wake alarm and unbind the RTC driver with the following commands:
> > # echo +10 > /sys/class/rtc/rtc0/wakealarm
> > # echo /sys/bus/platform/drivers/rtc-rtca3/1004ec00.rtc > unbind
> > # sleep 12
> > # echo /sys/bus/platform/drivers/rtc-rtca3/1004ec00.rtc > bind
> > 
> > When rebinding the re-configuration of the RTC device times out:
> > [  121.854190] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup
> > the RTC!
> > [  121.861511] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed
> > with error -110
> > -sh: echo: write error: Connection timed out
> > 
> > 2/ Configure wake alarm, unbind the RTC driver and switch to s2idle with
> > the following commands:
> > # echo s2idle > /sys/power/mem_sleep
> > # echo +10 > /sys/class/rtc/rtc0/wakealarm
> > # echo /sys/bus/platform/drivers/rtc-rtca/31004ec00.rtc > unbind
> > # echo mem > /sys/power/state
> > # #system is resumed by non RTC wakeup source (as the RTC alarm is not
> > working anymore in this case)
> > # echo /sys/bus/platform/drivers/rtc-rtca/1004ec00.rtc > bind
> > 
> > The system is not waked up from RTC alarm (as expected) and the rebinding
> > fails again:
> > 
> > [  172.483688] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup
> > the RTC!
> > [  172.491003] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed
> > with error -110
> > -sh: echo: write error: Connection timed out
> > 
> > 3/ configure the RTC alarm, unbind and power off (with the following commands):
> > # echo +60 > /sys/class/rtc/rtc0/wakealarm
> > # echo /sys/bus/platform/drivers/rtc-rtca/1004ec00.rtc > unbind
> > # poweroff
> > 
> > The system is not started after 60 seconds and at the next reboot the RTC
> > configuration on probe is failing the same:
> > 
> > [    0.292068] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup
> > the RTC!
> > [    0.292182] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed
> > with error -110
> > 
> > In all scenarios the RTC is recovered only if removing/re-applying the
> > power to the SoC area where it resides.
> > 
> > These tests were done with the patches in this series and then I tried it
> > with the following diff on top of the patches in this series. The results
> > were the same:
> > 
> > diff --git a/drivers/rtc/rtc-renesas-rtca3.c b/drivers/rtc/rtc-renesas-rtca3.c
> > index 822c055b6e4d..720fdac3adc6 100644
> > --- a/drivers/rtc/rtc-renesas-rtca3.c
> > +++ b/drivers/rtc/rtc-renesas-rtca3.c
> > @@ -586,7 +586,7 @@ static int rtca3_initial_setup(struct clk *clk, struct
> > rtca3_priv *priv)
> >         usleep_range(sleep_us, sleep_us + 10);
> > 
> >         /* Disable alarm and carry interrupts. */
> > -       mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE;
> > +       mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE | RTCA3_RCR1_PIE;
> >         ret = rtca3_alarm_irq_set_helper(priv, mask, 0);
> >         if (ret)
> >                 return ret;
> > @@ -784,7 +784,7 @@ static void rtca3_remove(struct platform_device *pdev)
> >         guard(spinlock_irqsave)(&priv->lock);
> > 
> >         /* Disable alarm, periodic interrupts. */
> > -       rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
> > +       //rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);
> >  }
> > 
> > Thank you,
> > Claudiu Beznea
> > 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC

[Wolfram Sang]

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

Hi,

sorry for being late to the game. I just noticed this series and
wonder...

> +/* Counter registers. */
> +#define RTCA3_RSECCNT			0x2
> +#define RTCA3_RSECCNT_SEC		GENMASK(6, 0)
> +#define RTCA3_RMINCNT			0x4
> +#define RTCA3_RMINCNT_MIN		GENMASK(6, 0)
> +#define RTCA3_RHRCNT			0x6
> +#define RTCA3_RHRCNT_HR			GENMASK(5, 0)
> +#define RTCA3_RHRCNT_PM			BIT(6)
> +#define RTCA3_RWKCNT			0x8
> +#define RTCA3_RWKCNT_WK			GENMASK(2, 0)
> +#define RTCA3_RDAYCNT			0xa
> +#define RTCA3_RDAYCNT_DAY		GENMASK(5, 0)
> +#define RTCA3_RMONCNT			0xc
> +#define RTCA3_RMONCNT_MONTH		GENMASK(4, 0)
> +#define RTCA3_RYRCNT			0xe
> +#define RTCA3_RYRCNT_YEAR		GENMASK(7, 0)

... if it has been considered to expand the existing rtc-sh driver? The
register set looks identical up to RMONAR. From that on, it looks a
different. Maybe the different offsets and bit positions could be
abstracted away by some sh_rtc_{read|write} helper? Or is there some
other new handling which makes re-using rtc-sh cumbersome?

Thanks and happy hacking,

   Wolfram


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

Re: [PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC

[Geert Uytterhoeven]

Hi Wolfram,

On Thu, Oct 17, 2024 at 12:45 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> sorry for being late to the game. I just noticed this series and
> wonder...

> ... if it has been considered to expand the existing rtc-sh driver? The
> register set looks identical up to RMONAR. From that on, it looks a
> different. Maybe the different offsets and bit positions could be
> abstracted away by some sh_rtc_{read|write} helper? Or is there some
> other new handling which makes re-using rtc-sh cumbersome?

Yes it has ;-)

https://lore.kernel.org/linux-renesas-soc/1c733190-bd46-43d0-8f3d-62e0ed5fde42@tuxon.dev

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC

[Wolfram Sang]

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


> > ... if it has been considered to expand the existing rtc-sh driver? The

...

> Yes it has ;-)

Thanks for the pointer. Well, a few of the limitations apply for RZA1 as
well but they are not handled in the rtc-sh driver yet (from a glimpse).

Nonetheless, given the other limitations and the bit shuffling in the
control registers, I agree that a new driver makes sense here.


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

[PATCH v3 07/12] arm64: dts: renesas: r9a08g045: Add VBATTB node

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Add the DT node for the VBATTB IP along with DT bindings for the clock
it provides.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- dropped the child nodes of vbattb; along with this dropped ranges,
  interrupt-names, #address-cells, #size-cells
- added vbattb_xtal as input clock for vbattb

Changes in v2:
- update compatibles
- updated clocks and clock-names for clock-controller node
- removed the power domain from the clock-controller as this is
  controlled by parent node in v2

 arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
index 067a26a66c24..247fa80a4f53 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
@@ -160,6 +160,18 @@ i2c3: i2c@10090c00 {
 			status = "disabled";
 		};
 
+		vbattb: vbattb@1005c000 {
+			compatible = "renesas,r9a08g045-vbattb";
+			reg = <0 0x1005c000 0 0x1000>;
+			interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb_xtal>;
+			clock-names = "bclk", "rtx";
+			#clock-cells = <1>;
+			power-domains = <&cpg>;
+			resets = <&cpg R9A08G045_VBAT_BRESETN>;
+			status = "disabled";
+		};
+
 		cpg: clock-controller@11010000 {
 			compatible = "renesas,r9a08g045-cpg";
 			reg = <0 0x11010000 0 0x10000>;
@@ -425,4 +437,11 @@ timer {
 		interrupt-names = "sec-phys", "phys", "virt", "hyp-phys",
 				  "hyp-virt";
 	};
+
+	vbattb_xtal: vbattb-xtal {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		/* This value must be overridden by the board. */
+		clock-frequency = <0>;
+	};
 };
-- 
2.39.2

Re: [PATCH v3 07/12] arm64: dts: renesas: r9a08g045: Add VBATTB node

[Stephen Boyd]

Quoting Claudiu (2024-08-30 06:02:13)
> diff --git a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
> index 067a26a66c24..247fa80a4f53 100644
> --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
> @@ -160,6 +160,18 @@ i2c3: i2c@10090c00 {
>                         status = "disabled";
>                 };
>  
> +               vbattb: vbattb@1005c000 {
> +                       compatible = "renesas,r9a08g045-vbattb";
> +                       reg = <0 0x1005c000 0 0x1000>;
> +                       interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb_xtal>;
> +                       clock-names = "bclk", "rtx";
> +                       #clock-cells = <1>;
> +                       power-domains = <&cpg>;
> +                       resets = <&cpg R9A08G045_VBAT_BRESETN>;
> +                       status = "disabled";
> +               };
> +
>                 cpg: clock-controller@11010000 {
>                         compatible = "renesas,r9a08g045-cpg";
>                         reg = <0 0x11010000 0 0x10000>;
> @@ -425,4 +437,11 @@ timer {
>                 interrupt-names = "sec-phys", "phys", "virt", "hyp-phys",
>                                   "hyp-virt";
>         };
> +
> +       vbattb_xtal: vbattb-xtal {

The node name should be something like clock-<frequency> but if the
frequency is different per-board then I don't know what should happen
here. Can you leave the vbattb_xtal phandle up above and then require
the node to be defined in the board with the proper frequency after the
dash?

> +               compatible = "fixed-clock";
> +               #clock-cells = <0>;
> +               /* This value must be overridden by the board. */
> +               clock-frequency = <0>;

Re: [PATCH v3 07/12] arm64: dts: renesas: r9a08g045: Add VBATTB node

[claudiu beznea]

Hi, Stephen,

On 03.09.2024 22:48, Stephen Boyd wrote:
> Quoting Claudiu (2024-08-30 06:02:13)
>> diff --git a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
>> index 067a26a66c24..247fa80a4f53 100644
>> --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
>> @@ -160,6 +160,18 @@ i2c3: i2c@10090c00 {
>>                         status = "disabled";
>>                 };
>>  
>> +               vbattb: vbattb@1005c000 {
>> +                       compatible = "renesas,r9a08g045-vbattb";
>> +                       reg = <0 0x1005c000 0 0x1000>;
>> +                       interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
>> +                       clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb_xtal>;
>> +                       clock-names = "bclk", "rtx";
>> +                       #clock-cells = <1>;
>> +                       power-domains = <&cpg>;
>> +                       resets = <&cpg R9A08G045_VBAT_BRESETN>;
>> +                       status = "disabled";
>> +               };
>> +
>>                 cpg: clock-controller@11010000 {
>>                         compatible = "renesas,r9a08g045-cpg";
>>                         reg = <0 0x11010000 0 0x10000>;
>> @@ -425,4 +437,11 @@ timer {
>>                 interrupt-names = "sec-phys", "phys", "virt", "hyp-phys",
>>                                   "hyp-virt";
>>         };
>> +
>> +       vbattb_xtal: vbattb-xtal {
> 
> The node name should be something like clock-<frequency> but if the
> frequency is different per-board then I don't know what should happen
> here.

The frequency should be always around 32768 Hz but not necessarily exactly
32768 Hz. It depends on what is installed on the board, indeed. RTC can do
time error adjustments based on the variations around 32768 Hz.

> Can you leave the vbattb_xtal phandle up above and then require
> the node to be defined in the board with the proper frequency after the
> dash?

Is it OK for you something like this (applied on top of this series)?

diff --git a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
index f31ec08a1e1d..60679211dc48 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
@@ -181,7 +181,8 @@ vbattb: clock-controller@1005c000 {
                        compatible = "renesas,r9a08g045-vbattb";
                        reg = <0 0x1005c000 0 0x1000>;
                        interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
-                       clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>,
<&vbattb_xtal>;
+                       /* rtx clock must be overridden by the board. */
+                       clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <0>;
                        clock-names = "bclk", "rtx";
                        #clock-cells = <1>;
                        power-domains = <&cpg>;
@@ -454,11 +455,4 @@ timer {
                interrupt-names = "sec-phys", "phys", "virt", "hyp-phys",
                                  "hyp-virt";
        };
-
-       vbattb_xtal: vbattb-xtal {
-               compatible = "fixed-clock";
-               #clock-cells = <0>;
-               /* This value must be overridden by the board. */
-               clock-frequency = <0>;
-       };
 };
diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
index 95b79a03d3d5..46cce0d48ddc 100644
--- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
@@ -47,6 +47,12 @@ chosen {
                stdout-path = "serial0:115200n8";
        };

+       vbattb_xtal: clock-32768 {
+               compatible = "fixed-clock";
+               #clock-cells = <0>;
+               clock-frequency = <32768>;
+       };
+
        memory@48000000 {
                device_type = "memory";
                /* First 128MB is reserved for secure area. */
@@ -351,14 +357,11 @@ &rtc {
 };

 &vbattb {
+       clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb_xtal>;
        renesas,vbattb-load-nanofarads = <12500>;
        status = "okay";
 };

-&vbattb_xtal {
-       clock-frequency = <32768>;
-};
-
 &wdt0 {
        timeout-sec = <60>;
        status = "okay";


Thank you for your review,
Claudiu Beznea

> 
>> +               compatible = "fixed-clock";
>> +               #clock-cells = <0>;
>> +               /* This value must be overridden by the board. */
>> +               clock-frequency = <0>;

Re: [PATCH v3 07/12] arm64: dts: renesas: r9a08g045: Add VBATTB node

[Stephen Boyd]

Quoting claudiu beznea (2024-09-04 05:17:30)
> On 03.09.2024 22:48, Stephen Boyd wrote:
> > 
> > The node name should be something like clock-<frequency> but if the
> > frequency is different per-board then I don't know what should happen
> > here.
> 
> The frequency should be always around 32768 Hz but not necessarily exactly
> 32768 Hz. It depends on what is installed on the board, indeed. RTC can do
> time error adjustments based on the variations around 32768 Hz.
> 
> > Can you leave the vbattb_xtal phandle up above and then require
> > the node to be defined in the board with the proper frequency after the
> > dash?
> 
> Is it OK for you something like this (applied on top of this series)?

Yes, it's too bad we can't append to a property in DT, or somehow leave
alone certain cells and only modify one of them.

Re: [PATCH v3 07/12] arm64: dts: renesas: r9a08g045: Add VBATTB node

[Geert Uytterhoeven]

Hi Stephen,

On Thu, Sep 5, 2024 at 8:09 PM Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting claudiu beznea (2024-09-04 05:17:30)
> > On 03.09.2024 22:48, Stephen Boyd wrote:
> > > The node name should be something like clock-<frequency> but if the
> > > frequency is different per-board then I don't know what should happen
> > > here.
> >
> > The frequency should be always around 32768 Hz but not necessarily exactly
> > 32768 Hz. It depends on what is installed on the board, indeed. RTC can do
> > time error adjustments based on the variations around 32768 Hz.
> >
> > > Can you leave the vbattb_xtal phandle up above and then require
> > > the node to be defined in the board with the proper frequency after the
> > > dash?
> >
> > Is it OK for you something like this (applied on top of this series)?
>
> Yes, it's too bad we can't append to a property in DT, or somehow leave
> alone certain cells and only modify one of them.

My main objections are that (1) this approach is different than the one used
for all other external clock inputs on Renesas SoCs, and (2) this requires
duplicating part of the clocks property in all board DTS files.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 07/12] arm64: dts: renesas: r9a08g045: Add VBATTB node

[Stephen Boyd]

Quoting Geert Uytterhoeven (2024-09-06 00:28:38)
> Hi Stephen,
> 
> On Thu, Sep 5, 2024 at 8:09 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > Quoting claudiu beznea (2024-09-04 05:17:30)
> > > On 03.09.2024 22:48, Stephen Boyd wrote:
> > > > The node name should be something like clock-<frequency> but if the
> > > > frequency is different per-board then I don't know what should happen
> > > > here.
> > >
> > > The frequency should be always around 32768 Hz but not necessarily exactly
> > > 32768 Hz. It depends on what is installed on the board, indeed. RTC can do
> > > time error adjustments based on the variations around 32768 Hz.
> > >
> > > > Can you leave the vbattb_xtal phandle up above and then require
> > > > the node to be defined in the board with the proper frequency after the
> > > > dash?
> > >
> > > Is it OK for you something like this (applied on top of this series)?
> >
> > Yes, it's too bad we can't append to a property in DT, or somehow leave
> > alone certain cells and only modify one of them.
> 
> My main objections are that (1) this approach is different than the one used
> for all other external clock inputs on Renesas SoCs, and (2) this requires
> duplicating part of the clocks property in all board DTS files.
> 

Can 'clock-ranges' be used here? Leave the cell as null in the SoC dtsi
file and then fill it in with clocks property at the parent node. I
think you'd have to use clock-names for this though.

Re: [PATCH v3 07/12] arm64: dts: renesas: r9a08g045: Add VBATTB node

[Geert Uytterhoeven]

Hi Stephen,

On Sat, Sep 7, 2024 at 1:01 AM Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Geert Uytterhoeven (2024-09-06 00:28:38)
> > On Thu, Sep 5, 2024 at 8:09 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > > Quoting claudiu beznea (2024-09-04 05:17:30)
> > > > On 03.09.2024 22:48, Stephen Boyd wrote:
> > > > > The node name should be something like clock-<frequency> but if the
> > > > > frequency is different per-board then I don't know what should happen
> > > > > here.
> > > >
> > > > The frequency should be always around 32768 Hz but not necessarily exactly
> > > > 32768 Hz. It depends on what is installed on the board, indeed. RTC can do
> > > > time error adjustments based on the variations around 32768 Hz.
> > > >
> > > > > Can you leave the vbattb_xtal phandle up above and then require
> > > > > the node to be defined in the board with the proper frequency after the
> > > > > dash?
> > > >
> > > > Is it OK for you something like this (applied on top of this series)?
> > >
> > > Yes, it's too bad we can't append to a property in DT, or somehow leave
> > > alone certain cells and only modify one of them.
> >
> > My main objections are that (1) this approach is different than the one used
> > for all other external clock inputs on Renesas SoCs, and (2) this requires
> > duplicating part of the clocks property in all board DTS files.
>
> Can 'clock-ranges' be used here? Leave the cell as null in the SoC dtsi
> file and then fill it in with clocks property at the parent node. I
> think you'd have to use clock-names for this though.

"clock-ranges" does not seem to be well-documented...

IUIC, your suggestion is to:
  1. Add "clock-ranges" to the /soc subnode,
  2. Completely leave out the "rtx" clock from the clocks property
     of the vbattb@1005c000 node,
  3. Add the following to the board DTS:

        &soc {
                clocks = <&vbattb_xtal>;
                clock-names = "rtx";
        };

Then, when resolving "rtx" for the vbattb@1005c000 node,
of_parse_clkspec() would iterate up and find the proper vbattb_xtal.
Is that correct? And probably that should be done for other external
clock inputs as well?

Still, it looks a bit complicated and un-intuitive. And what about
e.g. carrier boards with a SoM, where some clocks are provided by
the SoM, and some by the carrier? In that case you still have to
override the clock and clock-names properties in the carrier .dts,
thus duplicating all clocks provided by the SoM.

So I prefer the original approach, like is done for all other external
SoC clock inputs on Renesas SoCs.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 07/12] arm64: dts: renesas: r9a08g045: Add VBATTB node

[Stephen Boyd]

Quoting Geert Uytterhoeven (2024-09-09 05:11:03)
> Hi Stephen,
> 
> On Sat, Sep 7, 2024 at 1:01 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > Quoting Geert Uytterhoeven (2024-09-06 00:28:38)
> > >
> > > My main objections are that (1) this approach is different than the one used
> > > for all other external clock inputs on Renesas SoCs, and (2) this requires
> > > duplicating part of the clocks property in all board DTS files.
> >
> > Can 'clock-ranges' be used here? Leave the cell as null in the SoC dtsi
> > file and then fill it in with clocks property at the parent node. I
> > think you'd have to use clock-names for this though.
> 
> "clock-ranges" does not seem to be well-documented...

Yeah, I wasn't aware of it for years!

> 
> IUIC, your suggestion is to:
>   1. Add "clock-ranges" to the /soc subnode,
>   2. Completely leave out the "rtx" clock from the clocks property
>      of the vbattb@1005c000 node,
>   3. Add the following to the board DTS:
> 
>         &soc {
>                 clocks = <&vbattb_xtal>;
>                 clock-names = "rtx";
>         };
> 
> Then, when resolving "rtx" for the vbattb@1005c000 node,
> of_parse_clkspec() would iterate up and find the proper vbattb_xtal.
> Is that correct? And probably that should be done for other external
> clock inputs as well?

Sounds about right.

> 
> Still, it looks a bit complicated and un-intuitive. And what about
> e.g. carrier boards with a SoM, where some clocks are provided by
> the SoM, and some by the carrier? In that case you still have to
> override the clock and clock-names properties in the carrier .dts,
> thus duplicating all clocks provided by the SoM.

This is the same case as the board wanting to override the soc node?
When it's a SoM is there a node for the SoM? Is the clock on the SoM?
Does this case exist? Hopefully this isn't a straw man.

> 
> So I prefer the original approach, like is done for all other external
> SoC clock inputs on Renesas SoCs.
> 

Sure. I'm just suggesting to follow the preferred approach by DT
maintainers. I don't feel strongly either way and I'm not the SoC
maintainer so feel free to do what you want.

Re: [PATCH v3 07/12] arm64: dts: renesas: r9a08g045: Add VBATTB node

[Geert Uytterhoeven]

Hi Stephen,

On Mon, Sep 9, 2024 at 11:18 PM Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Geert Uytterhoeven (2024-09-09 05:11:03)
> > On Sat, Sep 7, 2024 at 1:01 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > > Quoting Geert Uytterhoeven (2024-09-06 00:28:38)
> > > >
> > > > My main objections are that (1) this approach is different than the one used
> > > > for all other external clock inputs on Renesas SoCs, and (2) this requires
> > > > duplicating part of the clocks property in all board DTS files.
> > >
> > > Can 'clock-ranges' be used here? Leave the cell as null in the SoC dtsi
> > > file and then fill it in with clocks property at the parent node. I
> > > think you'd have to use clock-names for this though.
> >
> > "clock-ranges" does not seem to be well-documented...
>
> Yeah, I wasn't aware of it for years!
>
> > IUIC, your suggestion is to:
> >   1. Add "clock-ranges" to the /soc subnode,
> >   2. Completely leave out the "rtx" clock from the clocks property
> >      of the vbattb@1005c000 node,
> >   3. Add the following to the board DTS:
> >
> >         &soc {
> >                 clocks = <&vbattb_xtal>;
> >                 clock-names = "rtx";
> >         };
> >
> > Then, when resolving "rtx" for the vbattb@1005c000 node,
> > of_parse_clkspec() would iterate up and find the proper vbattb_xtal.
> > Is that correct? And probably that should be done for other external
> > clock inputs as well?
>
> Sounds about right.
>
> > Still, it looks a bit complicated and un-intuitive. And what about
> > e.g. carrier boards with a SoM, where some clocks are provided by
> > the SoM, and some by the carrier? In that case you still have to
> > override the clock and clock-names properties in the carrier .dts,
> > thus duplicating all clocks provided by the SoM.
>
> This is the same case as the board wanting to override the soc node?

Yes, but more complicated,

> When it's a SoM is there a node for the SoM? Is the clock on the SoM?
> Does this case exist? Hopefully this isn't a straw man.

E.g. the White Hawk CPU board[1] contains extal_clk, extalr_clk, and
scif_clk, so it would need something like:

    &soc {
            clocks = <&extal_clk>, <&extalr_clk>, <&scif_clk>;
            clocks-names = "extal", "extalr", "scif";
    };

The White Hawk Break-Out board[2] contains can_clk, so it would
need to append that, by overriding (duplicate + append):

    &soc {
            clocks = <&extal_clk>, <&extalr_clk>, <&scif_clk>, <&can_clk>;
            clocks-names = "extal", "extalr", "scif", "can";
    };

Currently, [1] does:

    &extal_clk {
            clock-frequency = <16666666>;
    };

    &extalr_clk {
            clock-frequency = <32768>;
    };

    &scif_clk {
            clock-frequency = <24000000>;
    };

And [2] does:

    &can_clk {
            clock-frequency = <40000000>;
    };

[1] arch/arm64/boot/dts/renesas/white-hawk-cpu-common.dtsi:
[2] arch/arm64/boot/dts/renesas/white-hawk-common.dtsi

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 07/12] arm64: dts: renesas: r9a08g045: Add VBATTB node

[Geert Uytterhoeven]

Hi Claudiu,

On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Add the DT node for the VBATTB IP along with DT bindings for the clock
> it provides.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v3:
> - dropped the child nodes of vbattb; along with this dropped ranges,
>   interrupt-names, #address-cells, #size-cells
> - added vbattb_xtal as input clock for vbattb

Thanks for the update!

> --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
> @@ -160,6 +160,18 @@ i2c3: i2c@10090c00 {
>                         status = "disabled";
>                 };
>
> +               vbattb: vbattb@1005c000 {

Please insert this after serial@1004b800, to preserve sort order.

> +                       compatible = "renesas,r9a08g045-vbattb";
> +                       reg = <0 0x1005c000 0 0x1000>;
> +                       interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb_xtal>;
> +                       clock-names = "bclk", "rtx";
> +                       #clock-cells = <1>;
> +                       power-domains = <&cpg>;
> +                       resets = <&cpg R9A08G045_VBAT_BRESETN>;
> +                       status = "disabled";
> +               };
> +
>                 cpg: clock-controller@11010000 {
>                         compatible = "renesas,r9a08g045-cpg";
>                         reg = <0 0x11010000 0 0x10000>;

The rest LGTM, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 08/12] arm64: dts: renesas: r9a08g045: Add RTC node

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Add the DT node for the RTC IP available on the Renesas RZ/G3S SoC.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- added CPG clock, power domain, reset
- and assigned-clocks, assigned-clock-parents to configure the
  VBATTCLK
- included dt-bindings/clock/r9a08g045-vbattb.h

Changes in v2:
- updated compatibles

 arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
index 247fa80a4f53..ac9b6733d289 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
@@ -7,6 +7,7 @@
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/r9a08g045-cpg.h>
+#include <dt-bindings/clock/r9a08g045-vbattb.h>
 
 / {
 	compatible = "renesas,r9a08g045";
@@ -160,6 +161,22 @@ i2c3: i2c@10090c00 {
 			status = "disabled";
 		};
 
+		rtc: rtc@1004ec00 {
+			compatible = "renesas,r9a08g045-rtca3", "renesas,rz-rtca3";
+			reg = <0 0x1004ec00 0 0x400>;
+			interrupts = <GIC_SPI 315 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "alarm", "period", "carry";
+			clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb VBATTB_VBATTCLK>;
+			clock-names = "bus", "counter";
+			assigned-clocks = <&vbattb VBATTB_MUX>;
+			assigned-clock-parents = <&vbattb VBATTB_XC>;
+			power-domains = <&cpg>;
+			resets = <&cpg R9A08G045_VBAT_BRESETN>;
+			status = "disabled";
+		};
+
 		vbattb: vbattb@1005c000 {
 			compatible = "renesas,r9a08g045-vbattb";
 			reg = <0 0x1005c000 0 0x1000>;
-- 
2.39.2

Re: [PATCH v3 08/12] arm64: dts: renesas: r9a08g045: Add RTC node

[Geert Uytterhoeven]

Hi Claudiu,

On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Add the DT node for the RTC IP available on the Renesas RZ/G3S SoC.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v3:
> - added CPG clock, power domain, reset
> - and assigned-clocks, assigned-clock-parents to configure the
>   VBATTCLK
> - included dt-bindings/clock/r9a08g045-vbattb.h

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
> @@ -160,6 +161,22 @@ i2c3: i2c@10090c00 {
>                         status = "disabled";
>                 };
>
> +               rtc: rtc@1004ec00 {

Please insert this after serial@1004b800, to preserve sort order.

> +                       compatible = "renesas,r9a08g045-rtca3", "renesas,rz-rtca3";
> +                       reg = <0 0x1004ec00 0 0x400>;
> +                       interrupts = <GIC_SPI 315 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>;
> +                       interrupt-names = "alarm", "period", "carry";
> +                       clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb VBATTB_VBATTCLK>;
> +                       clock-names = "bus", "counter";
> +                       assigned-clocks = <&vbattb VBATTB_MUX>;
> +                       assigned-clock-parents = <&vbattb VBATTB_XC>;

Don't the assigned-clock* properties belong in the board DTS?
In addition, I think they should be documented in the DT bindings,
and be made required, so board developers don't forget about them.

> +                       power-domains = <&cpg>;
> +                       resets = <&cpg R9A08G045_VBAT_BRESETN>;
> +                       status = "disabled";
> +               };
> +
>                 vbattb: vbattb@1005c000 {
>                         compatible = "renesas,r9a08g045-vbattb";
>                         reg = <0 0x1005c000 0 0x1000>;

The rest LGTM.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 08/12] arm64: dts: renesas: r9a08g045: Add RTC node

[claudiu beznea]

Hi, Geert,

On 10.10.2024 18:22, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Add the DT node for the RTC IP available on the Renesas RZ/G3S SoC.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>
>> Changes in v3:
>> - added CPG clock, power domain, reset
>> - and assigned-clocks, assigned-clock-parents to configure the
>>   VBATTCLK
>> - included dt-bindings/clock/r9a08g045-vbattb.h
> 
> Thanks for your patch!
> 
>> --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
>> @@ -160,6 +161,22 @@ i2c3: i2c@10090c00 {
>>                         status = "disabled";
>>                 };
>>
>> +               rtc: rtc@1004ec00 {
> 
> Please insert this after serial@1004b800, to preserve sort order.

You're right. I though I have already checked this.

> 
>> +                       compatible = "renesas,r9a08g045-rtca3", "renesas,rz-rtca3";
>> +                       reg = <0 0x1004ec00 0 0x400>;
>> +                       interrupts = <GIC_SPI 315 IRQ_TYPE_LEVEL_HIGH>,
>> +                                    <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
>> +                                    <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>;
>> +                       interrupt-names = "alarm", "period", "carry";
>> +                       clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb VBATTB_VBATTCLK>;
>> +                       clock-names = "bus", "counter";
>> +                       assigned-clocks = <&vbattb VBATTB_MUX>;
>> +                       assigned-clock-parents = <&vbattb VBATTB_XC>;
> 
> Don't the assigned-clock* properties belong in the board DTS?

It makes sense to be in the board DTS, indeed.

> In addition, I think they should be documented in the DT bindings,
> and be made required, so board developers don't forget about them.

It would be better, indeed.

Thank you,
Claudiu Beznea

> 
>> +                       power-domains = <&cpg>;
>> +                       resets = <&cpg R9A08G045_VBAT_BRESETN>;
>> +                       status = "disabled";
>> +               };
>> +
>>                 vbattb: vbattb@1005c000 {
>>                         compatible = "renesas,r9a08g045-vbattb";
>>                         reg = <0 0x1005c000 0 0x1000>;
> 
> The rest LGTM.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Re: [PATCH v3 08/12] arm64: dts: renesas: r9a08g045: Add RTC node

[Alexandre Belloni]

On 11/10/2024 13:28:55+0300, claudiu beznea wrote:
> >> +                       compatible = "renesas,r9a08g045-rtca3", "renesas,rz-rtca3";
> >> +                       reg = <0 0x1004ec00 0 0x400>;
> >> +                       interrupts = <GIC_SPI 315 IRQ_TYPE_LEVEL_HIGH>,
> >> +                                    <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
> >> +                                    <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>;
> >> +                       interrupt-names = "alarm", "period", "carry";
> >> +                       clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb VBATTB_VBATTCLK>;
> >> +                       clock-names = "bus", "counter";
> >> +                       assigned-clocks = <&vbattb VBATTB_MUX>;
> >> +                       assigned-clock-parents = <&vbattb VBATTB_XC>;
> > 
> > Don't the assigned-clock* properties belong in the board DTS?
> 
> It makes sense to be in the board DTS, indeed.
> 
> > In addition, I think they should be documented in the DT bindings,
> > and be made required, so board developers don't forget about them.
> 
> It would be better, indeed.

There were multiple recent emails from Rob saying that assigned-clocks
should not be part of the bindings.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 08/12] arm64: dts: renesas: r9a08g045: Add RTC node

[Geert Uytterhoeven]

Hi Alexandre,

On Thu, Oct 17, 2024 at 12:03 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> On 11/10/2024 13:28:55+0300, claudiu beznea wrote:
> > >> +                       compatible = "renesas,r9a08g045-rtca3", "renesas,rz-rtca3";
> > >> +                       reg = <0 0x1004ec00 0 0x400>;
> > >> +                       interrupts = <GIC_SPI 315 IRQ_TYPE_LEVEL_HIGH>,
> > >> +                                    <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
> > >> +                                    <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>;
> > >> +                       interrupt-names = "alarm", "period", "carry";
> > >> +                       clocks = <&cpg CPG_MOD R9A08G045_VBAT_BCLK>, <&vbattb VBATTB_VBATTCLK>;
> > >> +                       clock-names = "bus", "counter";
> > >> +                       assigned-clocks = <&vbattb VBATTB_MUX>;
> > >> +                       assigned-clock-parents = <&vbattb VBATTB_XC>;
> > >
> > > Don't the assigned-clock* properties belong in the board DTS?
> >
> > It makes sense to be in the board DTS, indeed.
> >
> > > In addition, I think they should be documented in the DT bindings,
> > > and be made required, so board developers don't forget about them.
> >
> > It would be better, indeed.
>
> There were multiple recent emails from Rob saying that assigned-clocks
> should not be part of the bindings.

You mean "In general, assigned-clocks* do not need to be documented
and should never be required."[1] and "Whatever clock setup needed is
outside the scope of a binding"[2]?

In this case, it's not generic, and not specific to "renesas,rz-rtca3",
but related to the integration of RTC-A3 on the RZ/G3S SoC.
Which is indeed not relevant for the RTC-A3 bindings, so I agree the
assigned-clock* properties should not be part of this binding.
At least they're present in the example in the bindings ;-)

[1] https://lore.kernel.org/all/20241015211540.GA1968867-robh@kernel.org
[2] https://lore.kernel.org/all/20241015164609.GA1235530-robh@kernel.org

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 09/12] arm64: dts: renesas: rzg3s-smarc-som: Enable VBATTB

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Enable the VBATTB controller. It provides the clock for RTC.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- updated patch description
- dropped vbattclk
- added renesas,vbattb-load-nanofarads on vbattb
- moved vbattb before vbattb_xtal

Changes in v2:
- none

 arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
index 21bfa4e03972..591a90ccfe3c 100644
--- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
@@ -346,6 +346,15 @@ mux {
 	};
 };
 
+&vbattb {
+	renesas,vbattb-load-nanofarads = <12500>;
+	status = "okay";
+};
+
+&vbattb_xtal {
+	clock-frequency = <32768>;
+};
+
 &wdt0 {
 	timeout-sec = <60>;
 	status = "okay";
-- 
2.39.2

Re: [PATCH v3 09/12] arm64: dts: renesas: rzg3s-smarc-som: Enable VBATTB

[Geert Uytterhoeven]

On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Enable the VBATTB controller. It provides the clock for RTC.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v3:
> - updated patch description
> - dropped vbattclk
> - added renesas,vbattb-load-nanofarads on vbattb
> - moved vbattb before vbattb_xtal

With assigned-clock* resolved
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 10/12] arm64: dts: renesas: rzg3s-smarc-som: Enable RTC

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Enable RTC.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- none

Changes in v2:
- none

 arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
index 591a90ccfe3c..f911c2aa5e66 100644
--- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
@@ -346,6 +346,10 @@ mux {
 	};
 };
 
+&rtc {
+	status = "okay";
+};
+
 &vbattb {
 	renesas,vbattb-load-nanofarads = <12500>;
 	status = "okay";
-- 
2.39.2

Re: [PATCH v3 10/12] arm64: dts: renesas: rzg3s-smarc-som: Enable RTC

[Geert Uytterhoeven]

On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Enable RTC.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v3:
> - none

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 11/12] arm64: defconfig: Enable VBATTB clock

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Enable VBATTB clock driver flag.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- update patch title and description
- dropped CONFIG_MFD_RENESAS_VBATTB

Changes in v2:
- added CONFIG_MFD_RENESAS_VBATTB
- added vendor name in the VBATTB clock flag

 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 81ca46e3ab4b..465b70a06c33 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1361,6 +1361,7 @@ CONFIG_SM_VIDEOCC_8250=y
 CONFIG_QCOM_HFPLL=y
 CONFIG_CLK_GFM_LPASS_SM8250=m
 CONFIG_CLK_RCAR_USB2_CLOCK_SEL=y
+CONFIG_CLK_RENESAS_VBATTB=y
 CONFIG_HWSPINLOCK=y
 CONFIG_HWSPINLOCK_QCOM=y
 CONFIG_TEGRA186_TIMER=y
-- 
2.39.2

Re: [PATCH v3 11/12] arm64: defconfig: Enable VBATTB clock

[Geert Uytterhoeven]

On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Enable VBATTB clock driver flag.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v3:
> - update patch title and description
> - dropped CONFIG_MFD_RENESAS_VBATTB

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 12/12] arm64: defconfig: Enable Renesas RTCA-3 flag

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Enable Renesas RTCA-3 flag for the Renesas RZ/G3S SoC.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- none

Changes in v2:
- none

 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 465b70a06c33..40165e06c98f 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1215,6 +1215,7 @@ CONFIG_RTC_DRV_IMX_SC=m
 CONFIG_RTC_DRV_MT6397=m
 CONFIG_RTC_DRV_XGENE=y
 CONFIG_RTC_DRV_TI_K3=m
+CONFIG_RTC_DRV_RENESAS_RTCA3=y
 CONFIG_DMADEVICES=y
 CONFIG_DMA_BCM2835=y
 CONFIG_DMA_SUN6I=m
-- 
2.39.2

Re: [PATCH v3 12/12] arm64: defconfig: Enable Renesas RTCA-3 flag

[Geert Uytterhoeven]

On Fri, Aug 30, 2024 at 3:02 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Enable Renesas RTCA-3 flag for the Renesas RZ/G3S SoC.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v3:
> - none

May be combined with [11/12].

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds