Meson8 and Meson8b SoCs embed a DDR clock controller in their MMCBUS registers. This series: - adds support for this DDR clock controller (patches 0 and 1) - wires up the DDR PLL as input for two audio clocks (patches 2 and 3) - adds the DDR clock controller to meson8.dtsi and meson8b.dtsi Special thanks go out to Alexandre Mergnat for switching the Amlogic clock drivers over to parent_hws and parent_data. That made this series a lot easier for me! This series depends on my other series from [0]: "provide the XTAL clock via OF on Meson8/8b/8m2" [0] https://patchwork.kernel.org/cover/11155515/ Martin Blumenstingl (6): dt-bindings: clock: add the Amlogic Meson8 DDR clock controller binding clk: meson: add a driver for the Meson8/8b/8m2 DDR clock controller clk: meson: meson8b: use of_clk_hw_register to register the clocks clk: meson: meson8b: add the ddr_pll input for the audio clocks ARM: dts: meson8: add the DDR clock controller ARM: dts: meson8b: add the DDR clock controller .../clock/amlogic,meson8-ddr-clkc.yaml | 50 ++++++ arch/arm/boot/dts/meson8.dtsi | 13 +- arch/arm/boot/dts/meson8b.dtsi | 13 +- drivers/clk/meson/Makefile | 2 +- drivers/clk/meson/meson8-ddr.c | 153 ++++++++++++++++++ drivers/clk/meson/meson8b.c | 36 ++--- include/dt-bindings/clock/meson8-ddr-clkc.h | 4 + 7 files changed, 245 insertions(+), 26 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/amlogic,meson8-ddr-clkc.yaml create mode 100644 drivers/clk/meson/meson8-ddr.c create mode 100644 include/dt-bindings/clock/meson8-ddr-clkc.h -- 2.23.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
Amlogic Meson8, Meson8b and Meson8m2 SoCs have a DDR clock controller in the MMCBUS registers. There is no public documentation on this, but the GPL u-boot sources from the Amlogic BSP show that: - it uses the same XTAL input as the main clock controller - it contains a PLL which seems to be implemented just like the other PLLs in this SoC - there is a power-of-two PLL post-divider Add the documentation and header file for this DDR clock controller. Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- .../clock/amlogic,meson8-ddr-clkc.yaml | 50 +++++++++++++++++++ include/dt-bindings/clock/meson8-ddr-clkc.h | 4 ++ 2 files changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/amlogic,meson8-ddr-clkc.yaml create mode 100644 include/dt-bindings/clock/meson8-ddr-clkc.h diff --git a/Documentation/devicetree/bindings/clock/amlogic,meson8-ddr-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,meson8-ddr-clkc.yaml new file mode 100644 index 000000000000..bf3ca5888485 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/amlogic,meson8-ddr-clkc.yaml @@ -0,0 +1,50 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/amlogic,meson8-ddr-clkc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Amlogic DDR Clock Controller Device Tree Bindings + +maintainers: + - Martin Blumenstingl <martin.blumenstingl@googlemail.com> + +properties: + compatible: + enum: + - amlogic,meson8-ddr-clkc + - amlogic,meson8b-ddr-clkc + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + clock-names: + items: + - const: xtal + + "#clock-cells": + const: 1 + +required: + - compatible + - reg + - clocks + - clock-names + - "#clock-cells" + +additionalProperties: false + +examples: + - | + ddr_clkc: clock-controller@400 { + compatible = "amlogic,meson8-ddr-clkc"; + reg = <0x400 0x20>; + clocks = <&xtal>; + clock-names = "xtal"; + #clock-cells = <1>; + }; + +... diff --git a/include/dt-bindings/clock/meson8-ddr-clkc.h b/include/dt-bindings/clock/meson8-ddr-clkc.h new file mode 100644 index 000000000000..a8e0fa2987ab --- /dev/null +++ b/include/dt-bindings/clock/meson8-ddr-clkc.h @@ -0,0 +1,4 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#define DDR_CLKID_DDR_PLL_DCO 0 +#define DDR_CLKID_DDR_PLL 1 -- 2.23.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
The Meson8/Meson8b/Meson8m2 SoCs embed a DDR clock controller in the MMCBUS registers. There is no public documentation, but the u-boot GPL sources from the Amlogic BSP show that the DDR clock controller is identical on all three SoCs: #define CFG_DDR_CLK 792 #define CFG_PLL_M (((CFG_DDR_CLK/12)*12)/24) #define CFG_PLL_N 1 #define CFG_PLL_OD 1 // from set_ddr_clock: t_ddr_pll_cntl= (CFG_PLL_OD << 16)|(CFG_PLL_N<<9)|(CFG_PLL_M<<0) writel(timing_reg->t_ddr_pll_cntl|(1<<29),AM_DDR_PLL_CNTL); writel(readl(AM_DDR_PLL_CNTL) & (~(1<<29)),AM_DDR_PLL_CNTL); // from hx_ddr_power_down_enter: shut down DDR PLL writel(readl(AM_DDR_PLL_CNTL)|(1<<30),AM_DDR_PLL_CNTL); do { ... } while((readl(AM_DDR_PLL_CNTL)&(1<<31))==0) This translates to: - AM_DDR_PLL_CNTL[29] is the reset bit - AM_DDR_PLL_CNTL[30] is the enable bit - AM_DDR_PLL_CNTL[31] is the lock bit - AM_DDR_PLL_CNTL[8:0] is the m value (assuming the width is 9 bits based on the start of the n value) - AM_DDR_PLL_CNTL[13:9] is the n value (assuming the width is 5 bits based on the start of the od) - AM_DDR_PLL_CNTL[17:16] is the od (assuming the width is 2 bits based on other PLLs on this SoC) Add a driver for this PLL setup because it's used as one of the inputs of the audio clocks. There may be more clocks inside that clock controller - those can be added in subsequent patches. Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- drivers/clk/meson/Makefile | 2 +- drivers/clk/meson/meson8-ddr.c | 153 +++++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 drivers/clk/meson/meson8-ddr.c diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile index 3939f218587a..6eca2a406ee3 100644 --- a/drivers/clk/meson/Makefile +++ b/drivers/clk/meson/Makefile @@ -18,4 +18,4 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o -obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o +obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o diff --git a/drivers/clk/meson/meson8-ddr.c b/drivers/clk/meson/meson8-ddr.c new file mode 100644 index 000000000000..64ab4c27cce0 --- /dev/null +++ b/drivers/clk/meson/meson8-ddr.c @@ -0,0 +1,153 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Amlogic Meson8 DDR clock controller + * + * Copyright (C) 2019 Martin Blumenstingl <martin.blumenstingl@googlemail.com> + */ + +#include <dt-bindings/clock/meson8-ddr-clkc.h> + +#include <linux/platform_device.h> +#include <linux/mfd/syscon.h> +#include <linux/of_device.h> +#include <linux/slab.h> + +#include "clk-regmap.h" +#include "clk-pll.h" + +#define AM_DDR_PLL_CNTL 0x00 +#define AM_DDR_PLL_CNTL1 0x04 +#define AM_DDR_PLL_CNTL2 0x08 +#define AM_DDR_PLL_CNTL3 0x0c +#define AM_DDR_PLL_CNTL4 0x10 +#define AM_DDR_PLL_STS 0x14 +#define DDR_CLK_CNTL 0x18 +#define DDR_CLK_STS 0x1c + +static struct clk_regmap meson8_ddr_pll_dco = { + .data = &(struct meson_clk_pll_data){ + .en = { + .reg_off = AM_DDR_PLL_CNTL, + .shift = 30, + .width = 1, + }, + .m = { + .reg_off = AM_DDR_PLL_CNTL, + .shift = 0, + .width = 9, + }, + .n = { + .reg_off = AM_DDR_PLL_CNTL, + .shift = 9, + .width = 5, + }, + .l = { + .reg_off = AM_DDR_PLL_CNTL, + .shift = 31, + .width = 1, + }, + .rst = { + .reg_off = AM_DDR_PLL_CNTL, + .shift = 29, + .width = 1, + }, + }, + .hw.init = &(struct clk_init_data){ + .name = "ddr_pll_dco", + .ops = &meson_clk_pll_ro_ops, + .parent_data = &(const struct clk_parent_data) { + .fw_name = "xtal", + }, + .num_parents = 1, + }, +}; + +static struct clk_regmap meson8_ddr_pll = { + .data = &(struct clk_regmap_div_data){ + .offset = AM_DDR_PLL_CNTL, + .shift = 16, + .width = 2, + .flags = CLK_DIVIDER_POWER_OF_TWO, + }, + .hw.init = &(struct clk_init_data){ + .name = "ddr_pll", + .ops = &clk_regmap_divider_ro_ops, + .parent_hws = (const struct clk_hw *[]) { + &meson8_ddr_pll_dco.hw + }, + .num_parents = 1, + }, +}; + +static struct clk_hw_onecell_data meson8_ddr_clk_hw_onecell_data = { + .hws = { + [DDR_CLKID_DDR_PLL_DCO] = &meson8_ddr_pll_dco.hw, + [DDR_CLKID_DDR_PLL] = &meson8_ddr_pll.hw, + }, + .num = 2, +}; + +static struct clk_regmap *const meson8_ddr_clk_regmaps[] = { + &meson8_ddr_pll_dco, + &meson8_ddr_pll, +}; + +static const struct regmap_config meson8_ddr_clkc_regmap_config = { + .reg_bits = 8, + .val_bits = 32, + .reg_stride = 4, + .fast_io = true, +}; + +static int meson8_ddr_clkc_probe(struct platform_device *pdev) +{ + struct regmap *regmap; + struct resource *res; + void __iomem *base; + struct clk_hw *hw; + int ret, i; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(base)) + return PTR_ERR(base); + + regmap = devm_regmap_init_mmio(&pdev->dev, base, + &meson8_ddr_clkc_regmap_config); + if (IS_ERR(regmap)) + return PTR_ERR(regmap); + + /* Populate regmap */ + for (i = 0; i < ARRAY_SIZE(meson8_ddr_clk_regmaps); i++) + meson8_ddr_clk_regmaps[i]->map = regmap; + + /* Register all clks */ + for (i = 0; i < meson8_ddr_clk_hw_onecell_data.num; i++) { + hw = meson8_ddr_clk_hw_onecell_data.hws[i]; + + ret = devm_clk_hw_register(&pdev->dev, hw); + if (ret) { + dev_err(&pdev->dev, "Clock registration failed\n"); + return ret; + } + } + + return devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get, + &meson8_ddr_clk_hw_onecell_data); +} + +static const struct of_device_id meson8_ddr_clkc_match_table[] = { + { .compatible = "amlogic,meson8-ddr-clkc" }, + { .compatible = "amlogic,meson8b-ddr-clkc" }, + { /* sentinel */ }, +}; + +static struct platform_driver meson8_ddr_clkc_driver = { + .probe = meson8_ddr_clkc_probe, + .driver = { + .name = "meson8-ddr-clkc", + .of_match_table = meson8_ddr_clkc_match_table, + }, +}; + +builtin_platform_driver(meson8_ddr_clkc_driver); -- 2.23.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
Switch from clk_hw_register to of_clk_hw_register so we can use clk_parent_data.fw_name. This will be used to get the "xtal", "ddr_pll" and possibly others from the .dtb. Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- drivers/clk/meson/meson8b.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c index 15ec14fde2a0..fefb4b7185d0 100644 --- a/drivers/clk/meson/meson8b.c +++ b/drivers/clk/meson/meson8b.c @@ -3696,7 +3696,7 @@ static void __init meson8b_clkc_init_common(struct device_node *np, if (!clk_hw_onecell_data->hws[i]) continue; - ret = clk_hw_register(NULL, clk_hw_onecell_data->hws[i]); + ret = of_clk_hw_register(np, clk_hw_onecell_data->hws[i]); if (ret) return; } -- 2.23.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
The two audio muxes cts_amclk_sel and cts_mclk_i958_sel use ddr_pll as input at index 0. Update the muxes to use clk_parent_data and add "ddr_pll" as input using clk_parent_data.fw_name because the DDR clock controller is actually separate from the main clock controller. Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- drivers/clk/meson/meson8b.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c index fefb4b7185d0..3987f4ea7378 100644 --- a/drivers/clk/meson/meson8b.c +++ b/drivers/clk/meson/meson8b.c @@ -2429,28 +2429,25 @@ static struct clk_regmap meson8b_vdec_hevc = { }, }; -/* TODO: the clock at index 0 is "DDR_PLL" which we don't support yet */ -static const struct clk_hw *meson8b_cts_amclk_parent_hws[] = { - &meson8b_mpll0.hw, - &meson8b_mpll1.hw, - &meson8b_mpll2.hw +static const struct clk_parent_data meson8b_cts_amclk_parent_data[] = { + { .fw_name = "ddr_pll", }, + { .hw = &meson8b_mpll0.hw, }, + { .hw = &meson8b_mpll1.hw, }, + { .hw = &meson8b_mpll2.hw, }, }; -static u32 meson8b_cts_amclk_mux_table[] = { 1, 2, 3 }; - static struct clk_regmap meson8b_cts_amclk_sel = { .data = &(struct clk_regmap_mux_data){ .offset = HHI_AUD_CLK_CNTL, .mask = 0x3, .shift = 9, - .table = meson8b_cts_amclk_mux_table, .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cts_amclk_sel", .ops = &clk_regmap_mux_ops, - .parent_hws = meson8b_cts_amclk_parent_hws, - .num_parents = ARRAY_SIZE(meson8b_cts_amclk_parent_hws), + .parent_data = meson8b_cts_amclk_parent_data, + .num_parents = ARRAY_SIZE(meson8b_cts_amclk_parent_data), }, }; @@ -2488,28 +2485,25 @@ static struct clk_regmap meson8b_cts_amclk = { }, }; -/* TODO: the clock at index 0 is "DDR_PLL" which we don't support yet */ -static const struct clk_hw *meson8b_cts_mclk_i958_parent_hws[] = { - &meson8b_mpll0.hw, - &meson8b_mpll1.hw, - &meson8b_mpll2.hw +static const struct clk_parent_data meson8b_cts_mclk_i958_parent_data[] = { + { .fw_name = "ddr_pll", }, + { .hw = &meson8b_mpll0.hw, }, + { .hw = &meson8b_mpll1.hw, }, + { .hw = &meson8b_mpll2.hw, }, }; -static u32 meson8b_cts_mclk_i958_mux_table[] = { 1, 2, 3 }; - static struct clk_regmap meson8b_cts_mclk_i958_sel = { .data = &(struct clk_regmap_mux_data){ .offset = HHI_AUD_CLK_CNTL2, .mask = 0x3, .shift = 25, - .table = meson8b_cts_mclk_i958_mux_table, .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data) { .name = "cts_mclk_i958_sel", .ops = &clk_regmap_mux_ops, - .parent_hws = meson8b_cts_mclk_i958_parent_hws, - .num_parents = ARRAY_SIZE(meson8b_cts_mclk_i958_parent_hws), + .parent_data = meson8b_cts_mclk_i958_parent_data, + .num_parents = ARRAY_SIZE(meson8b_cts_mclk_i958_parent_data), }, }; -- 2.23.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
Add the DDR clock controller and pass it's DDR_CLKID_DDR_PLL to the main (HHI) clock controller as "ddr_clk". The "ddr_clk" is used as one of the inputs for the audio clock muxes. Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- arch/arm/boot/dts/meson8.dtsi | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi index 4f59a4c8f036..257c1364864c 100644 --- a/arch/arm/boot/dts/meson8.dtsi +++ b/arch/arm/boot/dts/meson8.dtsi @@ -3,6 +3,7 @@ * Copyright 2014 Carlo Caione <carlo@caione.org> */ +#include <dt-bindings/clock/meson8-ddr-clkc.h> #include <dt-bindings/clock/meson8b-clkc.h> #include <dt-bindings/gpio/meson8-gpio.h> #include <dt-bindings/reset/amlogic,meson8b-clkc-reset.h> @@ -195,6 +196,14 @@ #size-cells = <1>; ranges = <0x0 0xc8000000 0x8000>; + ddr_clkc: clock-controller@400 { + compatible = "amlogic,meson8-ddr-clkc"; + reg = <0x400 0x20>; + clocks = <&xtal>; + clock-names = "xtal"; + #clock-cells = <1>; + }; + dmcbus: bus@6000 { compatible = "simple-bus"; reg = <0x6000 0x400>; @@ -455,8 +464,8 @@ &hhi { clkc: clock-controller { compatible = "amlogic,meson8-clkc"; - clocks = <&xtal>; - clock-names = "xtal"; + clocks = <&xtal>, <&ddr_clkc DDR_CLKID_DDR_PLL>; + clock-names = "xtal", "ddr_pll"; #clock-cells = <1>; #reset-cells = <1>; }; -- 2.23.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
Add the DDR clock controller and pass it's DDR_CLKID_DDR_PLL to the main (HHI) clock controller as "ddr_clk". The "ddr_clk" is used as one of the inputs for the audio clock muxes. Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- arch/arm/boot/dts/meson8b.dtsi | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi index 1934666ff60f..8ac8bdfaf58f 100644 --- a/arch/arm/boot/dts/meson8b.dtsi +++ b/arch/arm/boot/dts/meson8b.dtsi @@ -4,6 +4,7 @@ * Author: Carlo Caione <carlo@endlessm.com> */ +#include <dt-bindings/clock/meson8-ddr-clkc.h> #include <dt-bindings/clock/meson8b-clkc.h> #include <dt-bindings/gpio/meson8b-gpio.h> #include <dt-bindings/reset/amlogic,meson8b-reset.h> @@ -172,6 +173,14 @@ #size-cells = <1>; ranges = <0x0 0xc8000000 0x8000>; + ddr_clkc: clock-controller@400 { + compatible = "amlogic,meson8b-ddr-clkc"; + reg = <0x400 0x20>; + clocks = <&xtal>; + clock-names = "xtal"; + #clock-cells = <1>; + }; + dmcbus: bus@6000 { compatible = "simple-bus"; reg = <0x6000 0x400>; @@ -434,8 +443,8 @@ &hhi { clkc: clock-controller { compatible = "amlogic,meson8-clkc"; - clocks = <&xtal>; - clock-names = "xtal"; + clocks = <&xtal>, <&ddr_clkc DDR_CLKID_DDR_PLL>; + clock-names = "xtal", "ddr_pll"; #clock-cells = <1>; #reset-cells = <1>; }; -- 2.23.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
On Sat 21 Sep 2019 at 17:18, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > Meson8 and Meson8b SoCs embed a DDR clock controller in their MMCBUS > registers. This series: > - adds support for this DDR clock controller (patches 0 and 1) > - wires up the DDR PLL as input for two audio clocks (patches 2 and 3) Have you been able to validate somehow that DDR rate calculated by CCF is the actual rate that gets to the audio clocks ? While I understand the interest for completeness, I suspect the having the DDR clock as an audio parent was just for debugging purpose. IOW, I'm not sure if adding this parent is useful to an actual audio use case. As far as audio would be concerned, I think we are better of without this parent. > - adds the DDR clock controller to meson8.dtsi and meson8b.dtsi > Could you please separate the driver and DT series in the future ? Those take different paths and are meant for different maintainers. > Special thanks go out to Alexandre Mergnat for switching the Amlogic > clock drivers over to parent_hws and parent_data. That made this series > a lot easier for me! > > This series depends on my other series from [0]: > "provide the XTAL clock via OF on Meson8/8b/8m2" > > > [0] https://patchwork.kernel.org/cover/11155515/ > > > Martin Blumenstingl (6): > dt-bindings: clock: add the Amlogic Meson8 DDR clock controller > binding > clk: meson: add a driver for the Meson8/8b/8m2 DDR clock controller > clk: meson: meson8b: use of_clk_hw_register to register the clocks > clk: meson: meson8b: add the ddr_pll input for the audio clocks > ARM: dts: meson8: add the DDR clock controller > ARM: dts: meson8b: add the DDR clock controller > > .../clock/amlogic,meson8-ddr-clkc.yaml | 50 ++++++ > arch/arm/boot/dts/meson8.dtsi | 13 +- > arch/arm/boot/dts/meson8b.dtsi | 13 +- > drivers/clk/meson/Makefile | 2 +- > drivers/clk/meson/meson8-ddr.c | 153 ++++++++++++++++++ > drivers/clk/meson/meson8b.c | 36 ++--- > include/dt-bindings/clock/meson8-ddr-clkc.h | 4 + > 7 files changed, 245 insertions(+), 26 deletions(-) > create mode 100644 Documentation/devicetree/bindings/clock/amlogic,meson8-ddr-clkc.yaml > create mode 100644 drivers/clk/meson/meson8-ddr.c > create mode 100644 include/dt-bindings/clock/meson8-ddr-clkc.h > > -- > 2.23.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
Hi Jerome, On Mon, Sep 23, 2019 at 12:06 PM Jerome Brunet <jbrunet@baylibre.com> wrote: > > On Sat 21 Sep 2019 at 17:18, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > > Meson8 and Meson8b SoCs embed a DDR clock controller in their MMCBUS > > registers. This series: > > - adds support for this DDR clock controller (patches 0 and 1) > > - wires up the DDR PLL as input for two audio clocks (patches 2 and 3) > > Have you been able to validate somehow that DDR rate calculated by CCF > is the actual rate that gets to the audio clocks ? no, I haven't been able to validate this (yet) > While I understand the interest for completeness, I suspect the having > the DDR clock as an audio parent was just for debugging purpose. IOW, > I'm not sure if adding this parent is useful to an actual audio use > case. As far as audio would be concerned, I think we are better of > without this parent. there at least three other (potential) consumers of the ddr_pll clocks on the 32-bit SoCs: - CPU clock mux [0] - clk81 mux [1] - USB PHY [2] I have not validated any of these either > > - adds the DDR clock controller to meson8.dtsi and meson8b.dtsi > > > > Could you please separate the driver and DT series in the future ? Those > take different paths and are meant for different maintainers. sure - so far Kevin has been doing a great job of still tracking these but I'm happy to split this into two patchsets Martin [0] https://github.com/endlessm/u-boot-meson/blob/345ee7eb02903f5ecb1173ffb2cd36666e44ebed/board/amlogic/m8b_m201_v1/firmware/timming.c#L441 [1] https://github.com/endlessm/u-boot-meson/blob/345ee7eb02903f5ecb1173ffb2cd36666e44ebed/board/amlogic/m8b_m201_v1/firmware/timming.c#L452 [2] https://github.com/endlessm/u-boot-meson/blob/f1ee03e3f7547d03e1478cc1fc967a9e5a121d92/arch/arm/cpu/aml_meson/m8/firmware/usb_boot/platform.c#L22 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
On Sat 21 Sep 2019 at 17:18, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > The Meson8/Meson8b/Meson8m2 SoCs embed a DDR clock controller in the > MMCBUS registers. There is no public documentation, but the u-boot GPL > sources from the Amlogic BSP show that the DDR clock controller is > identical on all three SoCs: > #define CFG_DDR_CLK 792 > #define CFG_PLL_M (((CFG_DDR_CLK/12)*12)/24) > #define CFG_PLL_N 1 > #define CFG_PLL_OD 1 > > // from set_ddr_clock: > t_ddr_pll_cntl= (CFG_PLL_OD << 16)|(CFG_PLL_N<<9)|(CFG_PLL_M<<0) > writel(timing_reg->t_ddr_pll_cntl|(1<<29),AM_DDR_PLL_CNTL); > writel(readl(AM_DDR_PLL_CNTL) & (~(1<<29)),AM_DDR_PLL_CNTL); > > // from hx_ddr_power_down_enter: shut down DDR PLL > writel(readl(AM_DDR_PLL_CNTL)|(1<<30),AM_DDR_PLL_CNTL); > > do { ... } while((readl(AM_DDR_PLL_CNTL)&(1<<31))==0) > > This translates to: > - AM_DDR_PLL_CNTL[29] is the reset bit > - AM_DDR_PLL_CNTL[30] is the enable bit > - AM_DDR_PLL_CNTL[31] is the lock bit > - AM_DDR_PLL_CNTL[8:0] is the m value (assuming the width is 9 bits > based on the start of the n value) > - AM_DDR_PLL_CNTL[13:9] is the n value (assuming the width is 5 bits > based on the start of the od) > - AM_DDR_PLL_CNTL[17:16] is the od (assuming the width is 2 bits based > on other PLLs on this SoC) > > Add a driver for this PLL setup because it's used as one of the inputs > of the audio clocks. There may be more clocks inside that clock > controller - those can be added in subsequent patches. > > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > drivers/clk/meson/Makefile | 2 +- > drivers/clk/meson/meson8-ddr.c | 153 +++++++++++++++++++++++++++++++++ > 2 files changed, 154 insertions(+), 1 deletion(-) > create mode 100644 drivers/clk/meson/meson8-ddr.c > > diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile > index 3939f218587a..6eca2a406ee3 100644 > --- a/drivers/clk/meson/Makefile > +++ b/drivers/clk/meson/Makefile > @@ -18,4 +18,4 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o > obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o > obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o > obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o > -obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o > +obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o > diff --git a/drivers/clk/meson/meson8-ddr.c b/drivers/clk/meson/meson8-ddr.c > new file mode 100644 > index 000000000000..64ab4c27cce0 > --- /dev/null > +++ b/drivers/clk/meson/meson8-ddr.c > @@ -0,0 +1,153 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Amlogic Meson8 DDR clock controller > + * > + * Copyright (C) 2019 Martin Blumenstingl <martin.blumenstingl@googlemail.com> > + */ > + > +#include <dt-bindings/clock/meson8-ddr-clkc.h> > + > +#include <linux/platform_device.h> > +#include <linux/mfd/syscon.h> syscon is not used in the driver > +#include <linux/of_device.h> > +#include <linux/slab.h> > + > +#include "clk-regmap.h" > +#include "clk-pll.h" > + > +#define AM_DDR_PLL_CNTL 0x00 > +#define AM_DDR_PLL_CNTL1 0x04 > +#define AM_DDR_PLL_CNTL2 0x08 > +#define AM_DDR_PLL_CNTL3 0x0c > +#define AM_DDR_PLL_CNTL4 0x10 > +#define AM_DDR_PLL_STS 0x14 > +#define DDR_CLK_CNTL 0x18 > +#define DDR_CLK_STS 0x1c > + > +static struct clk_regmap meson8_ddr_pll_dco = { > + .data = &(struct meson_clk_pll_data){ > + .en = { > + .reg_off = AM_DDR_PLL_CNTL, > + .shift = 30, > + .width = 1, > + }, > + .m = { > + .reg_off = AM_DDR_PLL_CNTL, > + .shift = 0, > + .width = 9, > + }, > + .n = { > + .reg_off = AM_DDR_PLL_CNTL, > + .shift = 9, > + .width = 5, > + }, > + .l = { > + .reg_off = AM_DDR_PLL_CNTL, > + .shift = 31, > + .width = 1, > + }, > + .rst = { > + .reg_off = AM_DDR_PLL_CNTL, > + .shift = 29, > + .width = 1, > + }, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "ddr_pll_dco", > + .ops = &meson_clk_pll_ro_ops, > + .parent_data = &(const struct clk_parent_data) { > + .fw_name = "xtal", > + }, > + .num_parents = 1, > + }, > +}; > + > +static struct clk_regmap meson8_ddr_pll = { > + .data = &(struct clk_regmap_div_data){ > + .offset = AM_DDR_PLL_CNTL, > + .shift = 16, > + .width = 2, > + .flags = CLK_DIVIDER_POWER_OF_TWO, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "ddr_pll", > + .ops = &clk_regmap_divider_ro_ops, > + .parent_hws = (const struct clk_hw *[]) { > + &meson8_ddr_pll_dco.hw > + }, > + .num_parents = 1, > + }, > +}; > + > +static struct clk_hw_onecell_data meson8_ddr_clk_hw_onecell_data = { > + .hws = { > + [DDR_CLKID_DDR_PLL_DCO] = &meson8_ddr_pll_dco.hw, > + [DDR_CLKID_DDR_PLL] = &meson8_ddr_pll.hw, I wonder if onecell is not overkill for this driver. We won't expose the DCO, so only the post divider remains Do you expect this provider to have more than one leaf clock ? If not, maybe you could use of_clk_hw_simple_get() instead ? > + }, > + .num = 2, > +}; > + > +static struct clk_regmap *const meson8_ddr_clk_regmaps[] = { > + &meson8_ddr_pll_dco, > + &meson8_ddr_pll, > +}; > + > +static const struct regmap_config meson8_ddr_clkc_regmap_config = { > + .reg_bits = 8, > + .val_bits = 32, > + .reg_stride = 4, > + .fast_io = true, I think fast_io will default to true with memory based register. Setting the max_register would be nice > +}; > + > +static int meson8_ddr_clkc_probe(struct platform_device *pdev) > +{ > + struct regmap *regmap; > + struct resource *res; > + void __iomem *base; > + struct clk_hw *hw; > + int ret, i; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(base)) > + return PTR_ERR(base); > + > + regmap = devm_regmap_init_mmio(&pdev->dev, base, > + &meson8_ddr_clkc_regmap_config); > + if (IS_ERR(regmap)) > + return PTR_ERR(regmap); > + > + /* Populate regmap */ > + for (i = 0; i < ARRAY_SIZE(meson8_ddr_clk_regmaps); i++) > + meson8_ddr_clk_regmaps[i]->map = regmap; > + > + /* Register all clks */ > + for (i = 0; i < meson8_ddr_clk_hw_onecell_data.num; i++) { > + hw = meson8_ddr_clk_hw_onecell_data.hws[i]; > + > + ret = devm_clk_hw_register(&pdev->dev, hw); > + if (ret) { > + dev_err(&pdev->dev, "Clock registration failed\n"); > + return ret; > + } > + } > + > + return devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get, > + &meson8_ddr_clk_hw_onecell_data); > +} > + > +static const struct of_device_id meson8_ddr_clkc_match_table[] = { > + { .compatible = "amlogic,meson8-ddr-clkc" }, > + { .compatible = "amlogic,meson8b-ddr-clkc" }, > + { /* sentinel */ }, > +}; > + > +static struct platform_driver meson8_ddr_clkc_driver = { > + .probe = meson8_ddr_clkc_probe, > + .driver = { > + .name = "meson8-ddr-clkc", > + .of_match_table = meson8_ddr_clkc_match_table, > + }, > +}; > + > +builtin_platform_driver(meson8_ddr_clkc_driver); _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
On Mon 23 Sep 2019 at 22:49, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > Hi Jerome, > > On Mon, Sep 23, 2019 at 12:06 PM Jerome Brunet <jbrunet@baylibre.com> wrote: >> >> On Sat 21 Sep 2019 at 17:18, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: >> >> > Meson8 and Meson8b SoCs embed a DDR clock controller in their MMCBUS >> > registers. This series: >> > - adds support for this DDR clock controller (patches 0 and 1) >> > - wires up the DDR PLL as input for two audio clocks (patches 2 and 3) >> >> Have you been able to validate somehow that DDR rate calculated by CCF >> is the actual rate that gets to the audio clocks ? > no, I haven't been able to validate this (yet) > >> While I understand the interest for completeness, I suspect the having >> the DDR clock as an audio parent was just for debugging purpose. IOW, >> I'm not sure if adding this parent is useful to an actual audio use >> case. As far as audio would be concerned, I think we are better of >> without this parent. > there at least three other (potential) consumers of the ddr_pll clocks > on the 32-bit SoCs: > - CPU clock mux [0] > - clk81 mux [1] > - USB PHY [2] > > I have not validated any of these either > Then I would suggest to leave patch 4 out until we can somehow validate this. > > > Martin > > > [0] https://github.com/endlessm/u-boot-meson/blob/345ee7eb02903f5ecb1173ffb2cd36666e44ebed/board/amlogic/m8b_m201_v1/firmware/timming.c#L441 > [1] https://github.com/endlessm/u-boot-meson/blob/345ee7eb02903f5ecb1173ffb2cd36666e44ebed/board/amlogic/m8b_m201_v1/firmware/timming.c#L452 > [2] https://github.com/endlessm/u-boot-meson/blob/f1ee03e3f7547d03e1478cc1fc967a9e5a121d92/arch/arm/cpu/aml_meson/m8/firmware/usb_boot/platform.c#L22 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
Hi Jerome, thank you for taking the time to go through this! On Tue, Oct 1, 2019 at 3:29 PM Jerome Brunet <jbrunet@baylibre.com> wrote: [...] > > +#include <linux/platform_device.h> > > +#include <linux/mfd/syscon.h> > > syscon is not used in the driver oops, good catch - thank you [...] > > +static struct clk_hw_onecell_data meson8_ddr_clk_hw_onecell_data = { > > + .hws = { > > + [DDR_CLKID_DDR_PLL_DCO] = &meson8_ddr_pll_dco.hw, > > + [DDR_CLKID_DDR_PLL] = &meson8_ddr_pll.hw, > > I wonder if onecell is not overkill for this driver. We won't expose the > DCO, so only the post divider remains > > Do you expect this provider to have more than one leaf clock ? > If not, maybe you could use of_clk_hw_simple_get() instead ? there's some more clock bits in DDR_CLK_CNTL - the public A311D datasheet has a description for these bits but I'm not sure they do the same on Meson8/Meson8b/Meson8m2 all I know is that some magic bytes are written to DDR_CLK_CNTL in the old u-boot code that's why I don't want to make any assumptions and play safe here (by using a onecell clock provider) > > + }, > > + .num = 2, > > +}; > > + > > +static struct clk_regmap *const meson8_ddr_clk_regmaps[] = { > > + &meson8_ddr_pll_dco, > > + &meson8_ddr_pll, > > +}; > > + > > +static const struct regmap_config meson8_ddr_clkc_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 32, > > + .reg_stride = 4, > > + .fast_io = true, > > I think fast_io will default to true with memory based register. > Setting the max_register would be nice good point - I'll take care of this when sending v2 Martin _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
On Tue 01 Oct 2019 at 20:53, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > [...] >> > +static struct clk_hw_onecell_data meson8_ddr_clk_hw_onecell_data = { >> > + .hws = { >> > + [DDR_CLKID_DDR_PLL_DCO] = &meson8_ddr_pll_dco.hw, >> > + [DDR_CLKID_DDR_PLL] = &meson8_ddr_pll.hw, >> >> I wonder if onecell is not overkill for this driver. We won't expose the >> DCO, so only the post divider remains >> >> Do you expect this provider to have more than one leaf clock ? >> If not, maybe you could use of_clk_hw_simple_get() instead ? > there's some more clock bits in DDR_CLK_CNTL - the public A311D > datasheet has a description for these bits but I'm not sure they do > the same on Meson8/Meson8b/Meson8m2 > all I know is that some magic bytes are written to DDR_CLK_CNTL in the > old u-boot code > > that's why I don't want to make any assumptions and play safe here (by > using a onecell clock provider) Understood. Let's keep onecell then. _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
On Sat, Sep 21, 2019 at 05:18:30PM +0200, Martin Blumenstingl wrote: > Amlogic Meson8, Meson8b and Meson8m2 SoCs have a DDR clock controller in > the MMCBUS registers. There is no public documentation on this, but the > GPL u-boot sources from the Amlogic BSP show that: > - it uses the same XTAL input as the main clock controller > - it contains a PLL which seems to be implemented just like the other > PLLs in this SoC > - there is a power-of-two PLL post-divider > > Add the documentation and header file for this DDR clock controller. > > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > .../clock/amlogic,meson8-ddr-clkc.yaml | 50 +++++++++++++++++++ > include/dt-bindings/clock/meson8-ddr-clkc.h | 4 ++ > 2 files changed, 54 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/amlogic,meson8-ddr-clkc.yaml > create mode 100644 include/dt-bindings/clock/meson8-ddr-clkc.h > > diff --git a/Documentation/devicetree/bindings/clock/amlogic,meson8-ddr-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,meson8-ddr-clkc.yaml > new file mode 100644 > index 000000000000..bf3ca5888485 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/amlogic,meson8-ddr-clkc.yaml > @@ -0,0 +1,50 @@ > +# SPDX-License-Identifier: GPL-2.0 (GPL-2.0-only OR BSD-2-Clause) for new bindings please. With that, Reviewed-by: Rob Herring <robh@kernel.org> > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/amlogic,meson8-ddr-clkc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Amlogic DDR Clock Controller Device Tree Bindings > + > +maintainers: > + - Martin Blumenstingl <martin.blumenstingl@googlemail.com> > + > +properties: > + compatible: > + enum: > + - amlogic,meson8-ddr-clkc > + - amlogic,meson8b-ddr-clkc > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + items: > + - const: xtal > + > + "#clock-cells": > + const: 1 > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - "#clock-cells" > + > +additionalProperties: false > + > +examples: > + - | > + ddr_clkc: clock-controller@400 { > + compatible = "amlogic,meson8-ddr-clkc"; > + reg = <0x400 0x20>; > + clocks = <&xtal>; > + clock-names = "xtal"; > + #clock-cells = <1>; > + }; > + > +... > diff --git a/include/dt-bindings/clock/meson8-ddr-clkc.h b/include/dt-bindings/clock/meson8-ddr-clkc.h > new file mode 100644 > index 000000000000..a8e0fa2987ab > --- /dev/null > +++ b/include/dt-bindings/clock/meson8-ddr-clkc.h > @@ -0,0 +1,4 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#define DDR_CLKID_DDR_PLL_DCO 0 > +#define DDR_CLKID_DDR_PLL 1 > -- > 2.23.0 > _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic