Re: [PATCH v4 3/3] clk: sunxi-ng: support R40 SoC

From: Chen-Yu Tsai
Date: Mon Aug 14 2017 - 12:13:20 EST

Next message: Jim Mattson: "Re: [PATCH v1 1/4] KVM: MMU: check guest CR3 reserved bits based on its physical address width."
Previous message: Eugeniy Paltsev: "[PATCH 5/5] ARC: AXS103: use cpu-freq param instead of /cpu_card/core_clk"
In reply to: Icenowy Zheng: "[PATCH v4 3/3] clk: sunxi-ng: support R40 SoC"
Next in thread: icenowy: "Re: [PATCH v4 3/3] clk: sunxi-ng: support R40 SoC"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

)On Sat, Aug 12, 2017 at 8:43 PM, Icenowy Zheng <icenowy@xxxxxxx> wrote:
> Allwinner R40 SoC have a clock controller module in the style of the
> SoCs beyond sun6i, however, it's more rich and complex.
>
> Add support for it.
>
> Signed-off-by: Icenowy Zheng <icenowy@xxxxxxx>
> ---
> Changes in v4:
> - Removed usb-ohci-12M mux clocks.
> - Removed unused (and not in user manual) adda-4x clock.
> - Implemented proper SATA PLL system.
> - Renamed MP (Mixed Processor) clock names to drop the extra "DE_".
> - Renamed TCONs' clock names to "tcon-lcdX" or "tcon-tvX".
> - Added missing RST_DRAM.
> - Several clock post/pre-dividers and constraints fixes.
> Changes in v3:
> - Rebased on current linux-next.
> Changes in v2:
> - Fixes according to the SoC's user manual.
>
> drivers/clk/sunxi-ng/Kconfig | 5 +
> drivers/clk/sunxi-ng/Makefile | 1 +
> drivers/clk/sunxi-ng/ccu-sun8i-r40.c | 1240 +++++++++++++++++++++++++++++
> drivers/clk/sunxi-ng/ccu-sun8i-r40.h | 69 ++
> include/dt-bindings/clock/sun8i-r40-ccu.h | 187 +++++
> include/dt-bindings/reset/sun8i-r40-ccu.h | 130 +++
> 6 files changed, 1632 insertions(+)
> create mode 100644 drivers/clk/sunxi-ng/ccu-sun8i-r40.c
> create mode 100644 drivers/clk/sunxi-ng/ccu-sun8i-r40.h
> create mode 100644 include/dt-bindings/clock/sun8i-r40-ccu.h
> create mode 100644 include/dt-bindings/reset/sun8i-r40-ccu.h
>
> diff --git a/drivers/clk/sunxi-ng/Kconfig b/drivers/clk/sunxi-ng/Kconfig
> index 7342928c35cd..7a360737fe3c 100644
> --- a/drivers/clk/sunxi-ng/Kconfig
> +++ b/drivers/clk/sunxi-ng/Kconfig
> @@ -48,6 +48,11 @@ config SUN8I_V3S_CCU
> config SUN8I_DE2_CCU
> bool "Support for the Allwinner SoCs DE2 CCU"
>
> +config SUN8I_R40_CCU
> + bool "Support for the Allwinner R40 CCU"
> + default MACH_SUN8I
> + depends on MACH_SUN8I || COMPILE_TEST
> +
> config SUN9I_A80_CCU
> bool "Support for the Allwinner A80 CCU"
> default MACH_SUN9I
> diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
> index 45a5910379a5..b1267fe68a8f 100644
> --- a/drivers/clk/sunxi-ng/Makefile
> +++ b/drivers/clk/sunxi-ng/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_SUN8I_H3_CCU) += ccu-sun8i-h3.o
> obj-$(CONFIG_SUN8I_V3S_CCU) += ccu-sun8i-v3s.o
> obj-$(CONFIG_SUN8I_DE2_CCU) += ccu-sun8i-de2.o
> obj-$(CONFIG_SUN8I_R_CCU) += ccu-sun8i-r.o
> +obj-$(CONFIG_SUN8I_R40_CCU) += ccu-sun8i-r40.o
> obj-$(CONFIG_SUN9I_A80_CCU) += ccu-sun9i-a80.o
> obj-$(CONFIG_SUN9I_A80_CCU) += ccu-sun9i-a80-de.o
> obj-$(CONFIG_SUN9I_A80_CCU) += ccu-sun9i-a80-usb.o
> diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-r40.c b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
> new file mode 100644
> index 000000000000..9dc6ad419f9a
> --- /dev/null
> +++ b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c

[...]

> +/*
> + * The MIPI PLL has 2 modes: "MIPI" and "HDMI".
> + *
> + * The MIPI mode is a standard NKM-style clock. The HDMI mode is an
> + * integer / fractional clock with switchable multipliers and dividers.
> + * This is not supported here. We hardcode the PLL to MIPI mode.
> + */
> +#define SUN8I_R40_PLL_MIPI_REG 0x040
> +
> +static const char * const pll_mipi_parents[] = { "pll-video0" };
> +static SUNXI_CCU_NKM_WITH_MUX_GATE_LOCK(pll_mipi_clk, "pll-mipi",
> + pll_mipi_parents, 0x040,
> + 8, 4, /* N */
> + 4, 2, /* K */
> + 0, 4, /* M */
> + 21, 1, /* mux */
> + BIT(31) | BIT(23) | BIT(22), /* gate */
> + BIT(28), /* lock */
> + CLK_SET_RATE_UNGATE);

K has a minimum of 2 here.

> +
> +static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK(pll_de_clk, "pll-de",
> + "osc24M", 0x0048,
> + 8, 7, /* N */
> + 0, 4, /* M */
> + BIT(24), /* frac enable */
> + BIT(25), /* frac select */
> + 270000000, /* frac rate 0 */
> + 297000000, /* frac rate 1 */
> + BIT(31), /* gate */
> + BIT(28), /* lock */
> + 0);

CLK_SET_RATE_UNGATE should be set for _all_ PLLs. The lock status bit
depends on the PLL running, and we check the lock status whenever set_rate
is done.

> +
> +static SUNXI_CCU_NM_WITH_GATE_LOCK(pll_ddr1_clk, "pll-ddr1",
> + "osc24M", 0x04c,
> + 8, 7, /* N */
> + 0, 2, /* M */
> + BIT(31), /* gate */
> + BIT(28), /* lock */
> + CLK_SET_RATE_UNGATE);

Manual says 16 <= N <= 75.

[...]

> +
> +static struct clk_div_table ths_div_table[] = {
> + { .val = 0, .div = 1 },
> + { .val = 1, .div = 2 },
> + { .val = 2, .div = 4 },
> + { .val = 3, .div = 8 },
> +};
> +static const char * const ths_parents[] = { "osc24M" };
> +static struct ccu_div ths_clk = {
> + .enable = BIT(31),
> + .div = _SUNXI_CCU_DIV_TABLE(0, 2, ths_div_table),

You could use _SUNXI_CCU_DIV_FLAGS(0, 2, CLK_DIVIDER_POWER_OF_TWO)
instead of div_table.

> + .mux = _SUNXI_CCU_MUX(24, 2),
> + .common = {
> + .reg = 0x074,
> + .hw.init = CLK_HW_INIT_PARENTS("ths",
> + ths_parents,
> + &ccu_div_ops,
> + 0),
> + },
> +};

[...]

> +static const char * const keypad_parents[] = { "osc24M", "osc32k", };

Nit: you can drop the extra comma.

[...]

> +static const char * const sata_parents[] = { "pll-sata-out", "sata-ext" };
> +static SUNXI_CCU_MUX_WITH_GATE(sata_clk, "sata", sata_parents,
> + 0x0c8, 24, 1, BIT(31), CLK_SET_RATE_PARENT);

You don't have CLK_SET_RATE_PARENT for the parent, which is also a mux+gate.
I'm not sure what you are expecting to happen here.

> +
> +static SUNXI_CCU_GATE(usb_phy0_clk, "usb-phy0", "osc24M",
> + 0x0cc, BIT(8), 0);
> +static SUNXI_CCU_GATE(usb_phy1_clk, "usb-phy1", "osc24M",
> + 0x0cc, BIT(9), 0);
> +static SUNXI_CCU_GATE(usb_phy2_clk, "usb-phy2", "osc24M",
> + 0x0cc, BIT(10), 0);
> +static SUNXI_CCU_GATE(usb_ohci0_clk, "usb-ohci0", "osc12M",
> + 0x0cc, BIT(16), 0);
> +static SUNXI_CCU_GATE(usb_ohci1_clk, "usb-ohci1", "osc12M",
> + 0x0cc, BIT(17), 0);
> +static SUNXI_CCU_GATE(usb_ohci2_clk, "usb-ohci2", "osc12M",
> + 0x0cc, BIT(18), 0);

Maybe you should force the OHCI muxes to 0?

[...]

> +static const char * const tcon_parents[] = { "pll-video0", "pll-video1",
> + "pll-video0-2x", "pll-video1-2x",
> + "pll-mipi" };
> +static SUNXI_CCU_MUX_WITH_GATE(tcon_lcd0_clk, "tcon-lcd0", tcon_parents,
> + 0x110, 24, 3, BIT(31), CLK_SET_RATE_PARENT);
> +static SUNXI_CCU_MUX_WITH_GATE(tcon_lcd1_clk, "tcon-lcd1", tcon_parents,
> + 0x114, 24, 3, BIT(31), CLK_SET_RATE_PARENT);
> +static SUNXI_CCU_M_WITH_MUX_GATE(tcon_tv0_clk, "tcon-tv0", tcon_parents,
> + 0x118, 0, 4, 24, 3, BIT(31), 0);
> +static SUNXI_CCU_M_WITH_MUX_GATE(tcon_tv1_clk, "tcon-tv1", tcon_parents,
> + 0x11c, 0, 4, 24, 3, BIT(31), 0);

You will likely need CLK_SET_RATE_PARENT for tcon-tv* as well.

[...]

> +static const char * const csi_mclk_parents[] = { "osc24M", "pll-video",

pll-video1?

> + "pll-periph1" };
> +static SUNXI_CCU_M_WITH_MUX_GATE(csi1_mclk_clk, "csi1-mclk", csi_mclk_parents,
> + 0x130, 0, 5, 8, 3, BIT(15), 0);
> +
> +static const char * const csi_sclk_parents[] = { "pll-periph0", "pll-periph1" };
> +static SUNXI_CCU_M_WITH_MUX_GATE(csi_sclk_clk, "csi-sclk", csi_sclk_parents,
> + 0x134, 16, 4, 24, 3, BIT(31), 0);
> +
> +static SUNXI_CCU_M_WITH_MUX_GATE(csi0_mclk_clk, "csi0-mclk", csi_mclk_parents,
> + 0x134, 0, 5, 8, 3, BIT(15), 0);

[...]

> +
> +static SUNXI_CCU_M_WITH_GATE(ve_clk, "ve", "pll-ve",
> + 0x13c, 16, 3, BIT(31), 0);

CLK_SET_RATE_PARENT?

> +
> +static SUNXI_CCU_GATE(adda_clk, "adda", "pll-audio",
> + 0x140, BIT(31), CLK_SET_RATE_PARENT);

You call the bus clock "bus-codec", but here you call it "adda". Pick one and
be consistent.

[...]

> +static const char * const hdmi_parents[] = { "pll-video0", "pll-video1" };
> +static SUNXI_CCU_M_WITH_MUX_GATE(hdmi_clk, "hdmi", hdmi_parents,
> + 0x150, 0, 4, 24, 2, BIT(31), 0);

CLK_SET_RATE_PARENT?

[...]

> +static const char * const mbus_parents[] = { "osc24M", "pll-periph0-2x",
> + "pll-ddr0" };
> +static SUNXI_CCU_MP_WITH_MUX_GATE(mbus_clk, "mbus", mbus_parents, 0x15c,
> + 0, 4, /* M */
> + 16, 2, /* P */
> + 24, 2, /* mux */
> + BIT(31), /* gate */
> + CLK_IS_CRITICAL);

The documentation for this one is fuzzy. It mentions the N (or P) factor,
but does not include it in the formula. Can you double check it against
the BSP and add a comment about it?

> +
> +static const char * const dsi_dphy_parents[] = { "pll-video0", "pll-video1",
> + "pll-periph0" };
> +static SUNXI_CCU_M_WITH_MUX_GATE(dsi_dphy_clk, "dsi-dphy", dsi_dphy_parents,
> + 0x168, 0, 4, 8, 2, BIT(15), 0);

Note: we might need CLK_SET_RATE_PARENT later on, but we will need to find
a way to keep pll-periph* fixed.

> +
> +static SUNXI_CCU_M_WITH_MUX_GATE(tve0_clk, "tve0", tcon_parents,
> + 0x180, 0, 4, 24, 3, BIT(31), 0);
> +static SUNXI_CCU_M_WITH_MUX_GATE(tve1_clk, "tve1", tcon_parents,
> + 0x184, 0, 4, 24, 3, BIT(31), 0);

CLK_SET_RATE_PARENT?

> +
> +static const char * const tvd_parents[] = { "pll-video0", "pll-video1",
> + "pll-video0-2x", "pll-video1-2x" };
> +static SUNXI_CCU_M_WITH_MUX_GATE(tvd0_clk, "tvd0", tvd_parents,
> + 0x188, 0, 4, 24, 3, BIT(31), 0);
> +static SUNXI_CCU_M_WITH_MUX_GATE(tvd1_clk, "tvd1", tvd_parents,
> + 0x18c, 0, 4, 24, 3, BIT(31), 0);
> +static SUNXI_CCU_M_WITH_MUX_GATE(tvd2_clk, "tvd2", tvd_parents,
> + 0x190, 0, 4, 24, 3, BIT(31), 0);
> +static SUNXI_CCU_M_WITH_MUX_GATE(tvd3_clk, "tvd3", tvd_parents,
> + 0x194, 0, 4, 24, 3, BIT(31), 0);

CLK_SET_RATE_PARENT?

[...]

You are missing the SYS_32k mux at register 0x310. According to
the diagrams, this is the actual clock that supplies the LOSC
parent of various muxes in the CCU. It itself is a mux between
an internal 2M RC oscillator, and LOSC from the RTC module.

> +
> +static struct ccu_reset_map sun8i_r40_ccu_resets[] = {

[...]

> + [RST_BUS_HDMI_SLOW] = { 0x2c4, BIT(10) },
> + [RST_BUS_HDMI] = { 0x2c4, BIT(11) },

Manual says bit 10 is HDMI0, bit 11 is HDMI1. Looking at the clocks,
bit 10 would be RST_BUS_HDMI, while bit 11 would be RST_BUS_HDMI_SLOW.

[...]

> +
> +static const struct sunxi_ccu_desc sun8i_r40_ccu_desc = {
> + .ccu_clks = sun8i_r40_ccu_clks,
> + .num_ccu_clks = ARRAY_SIZE(sun8i_r40_ccu_clks),
> +
> + .hw_clks = &sun8i_r40_hw_clks,
> +
> + .resets = sun8i_r40_ccu_resets,
> + .num_resets = ARRAY_SIZE(sun8i_r40_ccu_resets),
> +};
> +
> +static struct ccu_mux_nb sun8i_r40_cpu_nb = {
> + .common = &cpu_clk.common,
> + .cm = &cpu_clk.mux,
> + .delay_us = 1, /* > 8 clock cycles at 24 MHz */
> + .bypass_index = 1, /* index of 24 MHz oscillator */
> +};
> +
> +static void __init sun8i_r40_ccu_setup(struct device_node *node)
> +{
> + void __iomem *reg;
> + u32 val;
> +
> + reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> + if (IS_ERR(reg)) {
> + pr_err("%s: Could not map the clock registers\n",
> + of_node_full_name(node));
> + return;
> + }
> +
> + /* Force the PLL-Audio-1x divider to 4 */
> + val = readl(reg + SUN8I_R40_PLL_AUDIO_REG);
> + val &= ~GENMASK(19, 16);
> + writel(val | (3 << 16), reg + SUN8I_R40_PLL_AUDIO_REG);

You should also force pll-mipi to mipi mode. And the OHCI muxes.

> +
> + sunxi_ccu_probe(node, reg, &sun8i_r40_ccu_desc);
> +
> + ccu_mux_notifier_register(pll_cpu_clk.common.hw.clk,
> + &sun8i_r40_cpu_nb);

Do you also need the gate-then-ungate notifier?

Thanks
ChenYu

Next message: Jim Mattson: "Re: [PATCH v1 1/4] KVM: MMU: check guest CR3 reserved bits based on its physical address width."
Previous message: Eugeniy Paltsev: "[PATCH 5/5] ARC: AXS103: use cpu-freq param instead of /cpu_card/core_clk"
In reply to: Icenowy Zheng: "[PATCH v4 3/3] clk: sunxi-ng: support R40 SoC"
Next in thread: icenowy: "Re: [PATCH v4 3/3] clk: sunxi-ng: support R40 SoC"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]