| Message ID | 20230807-pxa1908-lkml-v4-3-cb387d73b452@skole.hr (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Initial Marvell PXA1908 support | expand |
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
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
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(+)