Add dt bindings and documentation for the Marvell PXA1908 clock
controller.
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
.../devicetree/bindings/clock/marvell,pxa1908.yaml | 48 +++++++++++
include/dt-bindings/clock/marvell,pxa1908.h | 92 ++++++++++++++++++++++
2 files changed, 140 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/marvell,pxa1908.yaml b/Documentation/devicetree/bindings/clock/marvell,pxa1908.yaml
new file mode 100644
index 000000000000..4e78933232b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/marvell,pxa1908.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/marvell,pxa1908.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell PXA1908 Clock Controllers
+
+maintainers:
+ - Duje Mihanović <duje.mihanovic@skole.hr>
+
+description: |
+ The PXA1908 clock subsystem generates and supplies clock to various
+ controllers within the PXA1908 SoC. The PXA1908 contains numerous clock
+ controller blocks, with the ones currently supported being APBC, APBCP, MPMU
+ and APMU roughly corresponding to internal buses.
+
+ All these clock identifiers could be found in <include/dt-bindings/marvell,pxa1908.h>.
+
+properties:
+ compatible:
+ enum:
+ - marvell,pxa1908-apbc
+ - marvell,pxa1908-apbcp
+ - marvell,pxa1908-mpmu
+ - marvell,pxa1908-apmu
+
+ reg:
+ maxItems: 1
+
+ '#clock-cells':
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+ # APMU block:
+ - |
+ clock-controller@d4282800 {
+ compatible = "marvell,pxa1908-apmu";
+ reg = <0xd4282800 0x400>;
+ #clock-cells = <1>;
+ };
diff --git a/include/dt-bindings/clock/marvell,pxa1908.h b/include/dt-bindings/clock/marvell,pxa1908.h
new file mode 100644
index 000000000000..0c1f328bf534
--- /dev/null
+++ b/include/dt-bindings/clock/marvell,pxa1908.h
@@ -0,0 +1,92 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+#ifndef __DTS_MARVELL_PXA1908_CLOCK_H
+#define __DTS_MARVELL_PXA1908_CLOCK_H
+
+/* plls */
+#define PXA1908_CLK_CLK32 1
+#define PXA1908_CLK_VCTCXO 2
+#define PXA1908_CLK_PLL1_624 3
+#define PXA1908_CLK_PLL1_416 4
+#define PXA1908_CLK_PLL1_499 5
+#define PXA1908_CLK_PLL1_832 6
+#define PXA1908_CLK_PLL1_1248 7
+#define PXA1908_CLK_PLL1_D2 8
+#define PXA1908_CLK_PLL1_D4 9
+#define PXA1908_CLK_PLL1_D8 10
+#define PXA1908_CLK_PLL1_D16 11
+#define PXA1908_CLK_PLL1_D6 12
+#define PXA1908_CLK_PLL1_D12 13
+#define PXA1908_CLK_PLL1_D24 14
+#define PXA1908_CLK_PLL1_D48 15
+#define PXA1908_CLK_PLL1_D96 16
+#define PXA1908_CLK_PLL1_D13 17
+#define PXA1908_CLK_PLL1_32 18
+#define PXA1908_CLK_PLL1_208 19
+#define PXA1908_CLK_PLL1_117 20
+#define PXA1908_CLK_PLL1_416_GATE 21
+#define PXA1908_CLK_PLL1_624_GATE 22
+#define PXA1908_CLK_PLL1_832_GATE 23
+#define PXA1908_CLK_PLL1_1248_GATE 24
+#define PXA1908_CLK_PLL1_D2_GATE 25
+#define PXA1908_CLK_PLL1_499_EN 26
+#define PXA1908_CLK_PLL2VCO 27
+#define PXA1908_CLK_PLL2 28
+#define PXA1908_CLK_PLL2P 29
+#define PXA1908_CLK_PLL2VCODIV3 30
+#define PXA1908_CLK_PLL3VCO 31
+#define PXA1908_CLK_PLL3 32
+#define PXA1908_CLK_PLL3P 33
+#define PXA1908_CLK_PLL3VCODIV3 34
+#define PXA1908_CLK_PLL4VCO 35
+#define PXA1908_CLK_PLL4 36
+#define PXA1908_CLK_PLL4P 37
+#define PXA1908_CLK_PLL4VCODIV3 38
+#define PXA1908_MPMU_NR_CLKS 38
+
+/* apb (apbc) peripherals */
+#define PXA1908_CLK_UART0 1
+#define PXA1908_CLK_UART1 2
+#define PXA1908_CLK_GPIO 3
+#define PXA1908_CLK_PWM0 4
+#define PXA1908_CLK_PWM1 5
+#define PXA1908_CLK_PWM2 6
+#define PXA1908_CLK_PWM3 7
+#define PXA1908_CLK_SSP0 8
+#define PXA1908_CLK_SSP1 9
+#define PXA1908_CLK_IPC_RST 10
+#define PXA1908_CLK_RTC 11
+#define PXA1908_CLK_TWSI0 12
+#define PXA1908_CLK_KPC 13
+#define PXA1908_CLK_SWJTAG 14
+#define PXA1908_CLK_SSP2 15
+#define PXA1908_CLK_TWSI1 16
+#define PXA1908_CLK_THERMAL 17
+#define PXA1908_CLK_TWSI3 18
+#define PXA1908_APBC_NR_CLKS 50
+
+/* apb (apbcp) peripherals */
+#define PXA1908_CLK_UART2 1
+#define PXA1908_CLK_TWSI2 2
+#define PXA1908_CLK_AICER 3
+#define PXA1908_APBCP_NR_CLKS 50
+
+/* axi (apmu) peripherals */
+#define PXA1908_CLK_CCIC1 1
+#define PXA1908_CLK_ISP 2
+#define PXA1908_CLK_DSI1 3
+#define PXA1908_CLK_DISP1 4
+#define PXA1908_CLK_CCIC0 5
+#define PXA1908_CLK_SDH0 6
+#define PXA1908_CLK_SDH1 7
+#define PXA1908_CLK_USB 8
+#define PXA1908_CLK_NF 9
+#define PXA1908_CLK_CORE_DEBUG 10
+#define PXA1908_CLK_VPU 11
+#define PXA1908_CLK_GC 12
+#define PXA1908_CLK_SDH2 13
+#define PXA1908_CLK_GC2D 14
+#define PXA1908_CLK_TRACE 15
+#define PXA1908_CLK_DVC_DFC_DEBUG 16
+#define PXA1908_APMU_NR_CLKS 50
+
+#endif
--
2.41.0
On Mon, Aug 07, 2023 at 05:42:37PM +0200, Duje Mihanović wrote: > diff --git a/include/dt-bindings/clock/marvell,pxa1908.h b/include/dt-bindings/clock/marvell,pxa1908.h > new file mode 100644 > index 000000000000..0c1f328bf534 > --- /dev/null > +++ b/include/dt-bindings/clock/marvell,pxa1908.h > +#define PXA1908_CLK_PLL4VCODIV3 38 > +#define PXA1908_MPMU_NR_CLKS 38 > +#define PXA1908_CLK_TWSI3 18 > +#define PXA1908_APBC_NR_CLKS 50 > +#define PXA1908_CLK_AICER 3 > +#define PXA1908_APBCP_NR_CLKS 50 > +#define PXA1908_CLK_DVC_DFC_DEBUG 16 > +#define PXA1908_APMU_NR_CLKS 50 How are these "NR_CLKS" things helpful to the binding?
On Tuesday, August 8, 2023 9:49:49 AM CEST Conor Dooley wrote: > On Mon, Aug 07, 2023 at 05:42:37PM +0200, Duje Mihanović wrote: > > diff --git a/include/dt-bindings/clock/marvell,pxa1908.h > > b/include/dt-bindings/clock/marvell,pxa1908.h new file mode 100644 > > index 000000000000..0c1f328bf534 > > --- /dev/null > > +++ b/include/dt-bindings/clock/marvell,pxa1908.h > > +#define PXA1908_CLK_PLL4VCODIV3 38 > > +#define PXA1908_MPMU_NR_CLKS 38 > > > > +#define PXA1908_CLK_TWSI3 18 > > +#define PXA1908_APBC_NR_CLKS 50 > > > > +#define PXA1908_CLK_AICER 3 > > +#define PXA1908_APBCP_NR_CLKS 50 > > > > +#define PXA1908_CLK_DVC_DFC_DEBUG 16 > > +#define PXA1908_APMU_NR_CLKS 50 > > How are these "NR_CLKS" things helpful to the binding? They are used by the clock driver when calling mmp_clk_init which then uses that as the size of a struct clk array it allocates. In retrospect, 50 for each block may be too much as from what I can tell by reading the mmp_register_* functions (number of clocks + 1) for each block should be enough, anything less than that causes a null pointer dereference sometime during clock initialization. Regards, Duje
On Tue, Aug 08, 2023 at 12:41:22PM +0200, Duje Mihanović wrote: > On Tuesday, August 8, 2023 9:49:49 AM CEST Conor Dooley wrote: > > On Mon, Aug 07, 2023 at 05:42:37PM +0200, Duje Mihanović wrote: > > > diff --git a/include/dt-bindings/clock/marvell,pxa1908.h > > > b/include/dt-bindings/clock/marvell,pxa1908.h new file mode 100644 > > > index 000000000000..0c1f328bf534 > > > --- /dev/null > > > +++ b/include/dt-bindings/clock/marvell,pxa1908.h > > > +#define PXA1908_CLK_PLL4VCODIV3 38 > > > +#define PXA1908_MPMU_NR_CLKS 38 > > > > > > +#define PXA1908_CLK_TWSI3 18 > > > +#define PXA1908_APBC_NR_CLKS 50 > > > > > > +#define PXA1908_CLK_AICER 3 > > > +#define PXA1908_APBCP_NR_CLKS 50 > > > > > > +#define PXA1908_CLK_DVC_DFC_DEBUG 16 > > > +#define PXA1908_APMU_NR_CLKS 50 > > > > How are these "NR_CLKS" things helpful to the binding? > > They are used by the clock driver when calling mmp_clk_init which then uses > that as the size of a struct clk array it allocates. In retrospect, 50 for > each block may be too much as from what I can tell by reading the > mmp_register_* functions (number of clocks + 1) for each block should be > enough, anything less than that causes a null pointer dereference sometime > during clock initialization. I think you might have misread my question, I'm not really interested in the implementation detail of the driver. If these are not used in devicetree, remove them - otherwise they are being needlessly added to the ABI.
On Tuesday, August 8, 2023 12:46:23 PM CEST Conor Dooley wrote: > On Tue, Aug 08, 2023 at 12:41:22PM +0200, Duje Mihanović wrote: > > They are used by the clock driver when calling mmp_clk_init which then uses > > that as the size of a struct clk array it allocates. In retrospect, 50 for > > each block may be too much as from what I can tell by reading the > > mmp_register_* functions (number of clocks + 1) for each block should be > > enough, anything less than that causes a null pointer dereference sometime > > during clock initialization. > > I think you might have misread my question, I'm not really interested in > the implementation detail of the driver. If these are not used in > devicetree, remove them - otherwise they are being needlessly added to > the ABI. Should I also do this in the rest of the MMP clock drivers? Regards, Duje
On Tue, Aug 08, 2023 at 01:03:33PM +0200, Duje Mihanović wrote: > On Tuesday, August 8, 2023 12:46:23 PM CEST Conor Dooley wrote: > > On Tue, Aug 08, 2023 at 12:41:22PM +0200, Duje Mihanović wrote: > > > They are used by the clock driver when calling mmp_clk_init which then > uses > > > that as the size of a struct clk array it allocates. In retrospect, 50 for > > > each block may be too much as from what I can tell by reading the > > > mmp_register_* functions (number of clocks + 1) for each block should be > > > enough, anything less than that causes a null pointer dereference sometime > > > during clock initialization. > > > > I think you might have misread my question, I'm not really interested in > > the implementation detail of the driver. If these are not used in > > devicetree, remove them - otherwise they are being needlessly added to > > the ABI. > > Should I also do this in the rest of the MMP clock drivers? Krzysztof has just done it for all of the Samsung ones! I think it wouldn't be a bad idea to get rid of them else where & new additions should definitely be avoided.
© 2016 - 2025 Red Hat, Inc.