Re: [PATCH v1 01/11] dt-bindings: clock: add Allwinner B288 CCU
From: Pablo Mazzini
Date: Thu Sep 24 2026 - 16:17:34 EST
Hi Andre,
thanks for the review.
> please add a cover letter
Done for v2.
> One problem in general is the lack of access to a manual [...]
The descriptions come from the BSP's clk-sun8iw10.{c,h}, cross-checked
against ccu-sun50i-a64.c - nine of the eleven PLLs are bit-identical -
and exercised on the device up to userspace.
> I am afraid you cannot push off the responsibility to other people
Copy-paste mistake, sorry. Moot now: folding both bindings into the
existing files removes those entries.
> why do you need a separate binding?
You're right, it's a strict subset; v2 just adds the compatible to
allwinner,sun4i-a10-ccu.yaml.
Thanks,
Pablo
On Thu, Sep 24, 2026 at 10:03 AM Andre Przywara <andre.przywara@xxxxxxx> wrote:
>
> Hi Pablo,
>
> many thanks for sending this series, on a first glance this looks pretty
> good.
> As a general comment: please add a cover letter (git format-patch ...
> --cover-letter, then edit the generated file), and explain a bit about
> the background of this work. This would be a good place to briefly
> describe what the B288 is, how it's related to other SoCs, how you came
> to all this information, the firmware story, and the existing
> limitations of this port.
>
> One problem in general is the lack of access to a manual, I am not even
> sure we ever upstreamed SoC support without one. Especially the clock
> driver is quite tricky, we would love to compare the clock descriptions
> against some documentation.
>
> On 9/23/26 22:16, Pablo Mazzini wrote:
> > The B288 (sun8iw10p1) is a dual Cortex-A7 SoC used in e-book readers.
> >
> > Signed-off-by: Pablo Mazzini <pmazzini@xxxxxxxxx>
> > ---
> > .../clock/allwinner,sun8i-b288-ccu.yaml | 55 ++++++++++++
> > include/dt-bindings/clock/sun8i-b288-ccu.h | 90 +++++++++++++++++++
> > include/dt-bindings/reset/sun8i-b288-ccu.h | 45 ++++++++++
> > 3 files changed, 190 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/clock/allwinner,sun8i-b288-ccu.yaml
> > create mode 100644 include/dt-bindings/clock/sun8i-b288-ccu.h
> > create mode 100644 include/dt-bindings/reset/sun8i-b288-ccu.h
> >
> > diff --git a/Documentation/devicetree/bindings/clock/allwinner,sun8i-b288-ccu.yaml b/Documentation/devicetree/bindings/clock/allwinner,sun8i-b288-ccu.yaml
> > new file mode 100644
> > index 000000000000..22b59d5b652c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/allwinner,sun8i-b288-ccu.yaml
> > @@ -0,0 +1,55 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/allwinner,sun8i-b288-ccu.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Allwinner B288 Clock Control Unit
> > +
> > +maintainers:
> > + - Chen-Yu Tsai <wens@xxxxxxxx>
> > + - Maxime Ripard <mripard@xxxxxxxxxx>
>
> I am afraid you cannot push off the responsibility to other people,
> especially I think Maxime wouldn't be really interested any more ;-)
>
> Actually, why do you need a separate binding? Wouldn't it be enough to
> just add the compatible string to the existing
> allwinner,sun4i-a10-ccu.yaml file? This one here looks like a subset of
> this, and would blend in there easily.
>
> Cheers,
> Andre
>
> > +
> > +properties:
> > + "#clock-cells":
> > + const: 1
> > +
> > + "#reset-cells":
> > + const: 1
> > +
> > + compatible:
> > + const: allwinner,sun8i-b288-ccu
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + items:
> > + - description: High Frequency Oscillator
> > + - description: Low Frequency Oscillator
> > +
> > + clock-names:
> > + items:
> > + - const: hosc
> > + - const: losc
> > +
> > +required:
> > + - "#clock-cells"
> > + - "#reset-cells"
> > + - compatible
> > + - reg
> > + - clocks
> > + - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + ccu: clock-controller@1c20000 {
> > + compatible = "allwinner,sun8i-b288-ccu";
> > + reg = <0x01c20000 0x400>;
> > + clocks = <&osc24M>, <&osc32k>;
> > + clock-names = "hosc", "losc";
> > + #clock-cells = <1>;
> > + #reset-cells = <1>;
> > + };
> > diff --git a/include/dt-bindings/clock/sun8i-b288-ccu.h b/include/dt-bindings/clock/sun8i-b288-ccu.h
> > new file mode 100644
> > index 000000000000..3dc1704d001e
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/sun8i-b288-ccu.h
> > @@ -0,0 +1,90 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> > +#ifndef _DT_BINDINGS_CLK_SUN8I_B288_H_
> > +#define _DT_BINDINGS_CLK_SUN8I_B288_H_
> > +
> > +#define CLK_PLL_CPUX 0
> > +#define CLK_PLL_AUDIO_BASE 1
> > +#define CLK_PLL_VIDEO0 2
> > +#define CLK_PLL_DDR0 3
> > +#define CLK_PLL_PERIPH0 4
> > +#define CLK_PLL_VIDEO1 5
> > +#define CLK_PLL_24M 6
> > +#define CLK_PLL_PERIPH1 7
> > +#define CLK_PLL_DE 8
> > +#define CLK_PLL_DDR1 9
> > +
> > +#define CLK_PLL_PERIPH0_2X 10
> > +#define CLK_PLL_PERIPH1_2X 11
> > +#define CLK_CPUX 12
> > +#define CLK_AXI 13
> > +#define CLK_CPUAPB 14
> > +#define CLK_AHB1 15
> > +#define CLK_APB1 16
> > +#define CLK_APB2 17
> > +
> > +#define CLK_NAND 18
> > +#define CLK_MMC0 19
> > +#define CLK_MMC1 20
> > +#define CLK_MMC2 21
> > +#define CLK_MMC3 22
> > +#define CLK_SPI0 23
> > +#define CLK_SPI1 24
> > +#define CLK_SPI2 25
> > +#define CLK_DRAM 26
> > +#define CLK_DE 27
> > +#define CLK_EE 28
> > +#define CLK_EDMA 29
> > +#define CLK_TCON0 30
> > +#define CLK_MBUS 31
> > +
> > +#define CLK_BUS_THS 32
> > +#define CLK_BUS_NAND 33
> > +#define CLK_BUS_SDMMC0_BUS 34
> > +#define CLK_BUS_SDMMC1_BUS 35
> > +#define CLK_BUS_SDMMC2_BUS 36
> > +#define CLK_BUS_SDMMC3_BUS 37
> > +#define CLK_BUS_SPI0 38
> > +#define CLK_BUS_SPI1 39
> > +#define CLK_BUS_SPI2 40
> > +#define CLK_BUS_I2S0 41
> > +#define CLK_BUS_I2S1 42
> > +#define CLK_BUS_SPDIF 43
> > +#define CLK_BUS_DSD 44
> > +#define CLK_BUS_DMIC 45
> > +#define CLK_BUS_USBOHCI0 46
> > +#define CLK_BUS_USBEHCI0 47
> > +#define CLK_BUS_USBOTG 48
> > +#define CLK_BUS_DE 49
> > +#define CLK_BUS_EE 50
> > +#define CLK_BUS_TCON0 51
> > +#define CLK_BUS_CSI_S 52
> > +#define CLK_BUS_CSI_M 53
> > +#define CLK_BUS_CSI_MISC 54
> > +#define CLK_BUS_ADDA 55
> > +#define CLK_BUS_SDRAM 56
> > +#define CLK_BUS_PSRAM 57
> > +#define CLK_BUS_DMA 58
> > +#define CLK_BUS_UART0 59
> > +#define CLK_BUS_UART1 60
> > +#define CLK_BUS_UART2 61
> > +#define CLK_BUS_UART3 62
> > +#define CLK_BUS_UART4 63
> > +#define CLK_BUS_UART5 64
> > +#define CLK_BUS_TWI0 65
> > +#define CLK_BUS_TWI1 66
> > +#define CLK_BUS_TWI2 67
> > +#define CLK_BUS_PIO 68
> > +#define CLK_BUS_GPADC 69
> > +#define CLK_BUS_KEYADC 70
> > +
> > +/*
> > + * mmc0 and mmc1 are v4p1x controllers driven in the old timing mode, which
> > + * needs the sample and output phase clocks. mmc2 and mmc3 are newer blocks
> > + * that do their own delay calibration and have none.
> > + */
> > +#define CLK_MMC0_SAMPLE 71
> > +#define CLK_MMC0_OUTPUT 72
> > +#define CLK_MMC1_SAMPLE 73
> > +#define CLK_MMC1_OUTPUT 74
> > +
> > +#endif
> > diff --git a/include/dt-bindings/reset/sun8i-b288-ccu.h b/include/dt-bindings/reset/sun8i-b288-ccu.h
> > new file mode 100644
> > index 000000000000..675145654548
> > --- /dev/null
> > +++ b/include/dt-bindings/reset/sun8i-b288-ccu.h
> > @@ -0,0 +1,45 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> > +#ifndef _DT_BINDINGS_RST_SUN8I_B288_H_
> > +#define _DT_BINDINGS_RST_SUN8I_B288_H_
> > +
> > +#define RST_BUS_THS 0
> > +#define RST_BUS_NAND 1
> > +#define RST_BUS_MMC0 2
> > +#define RST_BUS_MMC1 3
> > +#define RST_BUS_MMC2 4
> > +#define RST_BUS_MMC3 5
> > +#define RST_BUS_SPI0 6
> > +#define RST_BUS_SPI1 7
> > +#define RST_BUS_SPI2 8
> > +#define RST_BUS_I2S0 9
> > +#define RST_BUS_I2S1 10
> > +#define RST_BUS_SPDIF 11
> > +#define RST_BUS_DSD 12
> > +#define RST_BUS_DMIC 13
> > +#define RST_USBPHY0 14
> > +#define RST_BUS_OHCI0 15
> > +#define RST_BUS_EHCI0 16
> > +#define RST_BUS_OTG 17
> > +#define RST_BUS_DE 18
> > +#define RST_BUS_EE 19
> > +#define RST_BUS_TCON0 20
> > +#define RST_BUS_CSI 21
> > +#define RST_BUS_ADDA 22
> > +#define RST_BUS_WLAN 23
> > +#define RST_MBUS 24
> > +#define RST_BUS_DRAM 25
> > +#define RST_BUS_PSRAM 26
> > +#define RST_BUS_DMA 27
> > +#define RST_BUS_UART0 28
> > +#define RST_BUS_UART1 29
> > +#define RST_BUS_UART2 30
> > +#define RST_BUS_UART3 31
> > +#define RST_BUS_UART4 32
> > +#define RST_BUS_UART5 33
> > +#define RST_BUS_I2C0 34
> > +#define RST_BUS_I2C1 35
> > +#define RST_BUS_I2C2 36
> > +#define RST_BUS_GPADC 37
> > +#define RST_BUS_KEYADC 38
> > +
> > +#endif
>