Re: [PATCH 1/7] dt-bindings: rtc: sun6i: Add Allwinner A733 support

[Chen-Yu Tsai]

On Tue, Jun 16, 2026 at 1:46 AM Jerome Brunet <jbrunet@xxxxxxxxxxxx> wrote:
>
> On sam. 28 mars 2026 at 20:37, Chen-Yu Tsai <wens@xxxxxxxxxx> wrote:
>
> > On Wed, Jan 21, 2026 at 7:03 PM Junhui Liu <junhui.liu@xxxxxxxxxxxxx> wrote:
> >>
> >> The RTC module in the Allwinner A733 SoC is functionally compatible with
> >> the sun6i RTC, but its internal Clock Control Unit (CCU) has significant
> >> changes.
> >>
> >> The A733 supports selecting the oscillator between three frequencies:
> >> 19.2MHz, 24MHz, and 26MHz. The RTC CCU relies on hardware to detect
> >> which frequency is actually used on the board. By defining all three
> >> frequencies as fixed-clocks in the device tree, the driver can identify
> >> the hardware-detected frequency and expose it to the rest of the system.
> >
> > No. The board device tree shall have the exact and correct frequency
> > defined in the external crystal device node. The operating system can
> > use the hardware-detected frequency to "fix" the in-system representation
> > if it is off.
> >
> >> Additionally, the A733 RTC CCU provides several new DCXO gate clocks for
> >> specific modules, including SerDes, HDMI, and UFS.
> >>
> >> Signed-off-by: Junhui Liu <junhui.liu@xxxxxxxxxxxxx>
> >> ---
> >>  .../bindings/rtc/allwinner,sun6i-a31-rtc.yaml      | 38 ++++++++++++++++++++--
> >>  include/dt-bindings/clock/sun60i-a733-rtc.h        | 16 +++++++++
> >>  2 files changed, 52 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml b/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
> >> index 9df5cdb6f63f..b18431955783 100644
> >> --- a/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
> >> +++ b/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
> >> @@ -26,6 +26,7 @@ properties:
> >>            - allwinner,sun50i-h6-rtc
> >>            - allwinner,sun50i-h616-rtc
> >>            - allwinner,sun50i-r329-rtc
> >> +          - allwinner,sun60i-a733-rtc
> >>        - items:
> >>            - const: allwinner,sun50i-a64-rtc
> >>            - const: allwinner,sun8i-h3-rtc
> >> @@ -46,11 +47,11 @@ properties:
> >>
> >>    clocks:
> >>      minItems: 1
> >> -    maxItems: 4
> >> +    maxItems: 6
> >>
> >>    clock-names:
> >>      minItems: 1
> >> -    maxItems: 4
> >> +    maxItems: 6
> >>
> >>    clock-output-names:
> >>      minItems: 1
> >> @@ -156,6 +157,38 @@ allOf:
> >>          - clocks
> >>          - clock-names
> >>
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          contains:
> >> +            const: allwinner,sun60i-a733-rtc
> >> +
> >> +    then:
> >> +      properties:
> >> +        clocks:
> >> +          minItems: 5
> >> +          items:
> >> +            - description: Bus clock for register access
> >
> >> +            - description: 19.2 MHz oscillator
> >> +            - description: 24 MHz oscillator
> >> +            - description: 26 MHz oscillator
> >
> > No. There is only one input. As in there is only one set of pins for the
> > DCXO. The inputs are the same as on R329 / A523. Just use that list.
> >
> >> +            - description: AHB parent for internal SPI clock
> >> +            - description: External 32768 Hz oscillator
> >> +
> >> +        clock-names:
> >> +          minItems: 5
> >> +          items:
> >> +            - const: bus
> >> +            - const: osc19M
> >> +            - const: osc24M
> >> +            - const: osc26M
> >> +            - const: ahb
> >> +            - const: ext-osc32k
> >> +
> >> +      required:
> >> +        - clocks
> >> +        - clock-names
> >> +
> >>    - if:
> >>        properties:
> >>          compatible:
> >> @@ -164,6 +197,7 @@ allOf:
> >>                - allwinner,sun8i-r40-rtc
> >>                - allwinner,sun50i-h616-rtc
> >>                - allwinner,sun50i-r329-rtc
> >> +              - allwinner,sun60i-a733-rtc
> >>
> >>      then:
> >>        properties:
> >> diff --git a/include/dt-bindings/clock/sun60i-a733-rtc.h b/include/dt-bindings/clock/sun60i-a733-rtc.h
> >> new file mode 100644
> >> index 000000000000..8a2b5facad73
> >> --- /dev/null
> >> +++ b/include/dt-bindings/clock/sun60i-a733-rtc.h
> >> @@ -0,0 +1,16 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
> >> +
> >> +#ifndef _DT_BINDINGS_CLK_SUN60I_A733_RTC_H_
> >> +#define _DT_BINDINGS_CLK_SUN60I_A733_RTC_H_
> >> +
> >> +#define CLK_IOSC               0
> >> +#define CLK_OSC32K             1
> >> +#define CLK_HOSC               2
> >
> > The DCXO enable control has been present since at least the H6. We just
> > never added it, as we would never disable it anyway.
> >
> > If you compare the RTC clock trees of the A733 and A523, the only addition
> > besides the new gates seems to be the LOSC auto selection. But even that
> > is just an illusion, as the A523 has the same registers for that.
> >
> > One could say the A733 RTC is almost backward compatible to the A523, if
> > not for the two fastboot registers the A523 has at 0x120 and 0x124.
> >
> > So I ask that you try to integrate the differences into the existing
> > driver and bindings. You can tweak and export internal clks if you
> > need.
>
> I'd like to help with that. I think it is doable but I have a question
> regarding the binding of the existing driver, more precisely their usage
> here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c?h=v7.1#n370
>
> Clock indexes are supposed to be stable in DT (AFAIK) but with the code
> linked the external 32k is at:
>
> * "ext-32k" - so index 3 - if "clock-names" is present
> * index 0 if clock names is not present
>
> ... but index 0 is supposed to be the bus clock according the binding
> doc, whether "clock-names" is there or not :/
>
> So what are those old r329 bindings ? is there a documentation defining
> them somewhere ?

You can look at

    8487614a8a8a dt-bindings: rtc: sun6i: Add H616, R329, and D1 support

In hindsight maybe the two bindings should be separate. The old SoCs
did not have all these clock inputs from the main clock controller.
The only input it could possibly take was the external 32k crystal.

> Cleaning that part would help with A733 addition in the existing driver
> I think

Yeah. Also, we can treat the bindings and drivers separately. We could
have two bindings but one common driver, or vice versa. As you pointed
out, the bindings are a bit messed up, so we could consider separating
them.

If we end up with separate binding header files, maybe we could use
a different prefix for the new ones so they don't collide? That way
the driver could maybe still be shared?

As for whether to share the headers, I think they should be treated
as part of the binding, so if the bindings are shared, then they can
be shared as well; if the bindings are separate, then they should be
completely separate files as well.


And sorry for the late reply.


Thanks
ChenYu

> >
> >> +#define CLK_RTC_32K            3
> >
> > AFAICT besides being an internal clock, this is also fed to GPIO for
> > debounce? We probably need to expose this on the A523 as well.
> >
> >
> > Thanks
> > ChenYu
> >
> >
> >> +#define CLK_OSC32K_FANOUT      4
> >> +#define CLK_HOSC_SERDES1       5
> >> +#define CLK_HOSC_SERDES0       6
> >> +#define CLK_HOSC_HDMI          7
> >> +#define CLK_HOSC_UFS           8
> >> +
> >> +#endif /* _DT_BINDINGS_CLK_SUN60I_A733_RTC_H_ */
> >>
> >> --
> >> 2.52.0
> >>
> >>
>
> --
> Jerome
>