diff mbox series

[v4,3/8] dt-bindings: clock: Add Marvell PXA1908 clock bindings

Message ID 20230807-pxa1908-lkml-v4-3-cb387d73b452@skole.hr
State Superseded
Headers show
Series Initial Marvell PXA1908 support | expand

Commit Message

Duje Mihanović Aug. 7, 2023, 3:42 p.m. UTC 
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(+)

Comments

Duje Mihanović Aug. 8, 2023, 10:41 a.m. UTC | #1 
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
Duje Mihanović Aug. 8, 2023, 11:03 a.m. UTC | #2 
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
Conor Dooley Aug. 8, 2023, 11:23 a.m. UTC | #3 
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.
diff mbox series

Patch

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