Re: [PATCH v2 1/4] clk: sunxi-ng: Add A64 clocks
Hi Stephen,
On Wed, Sep 14, 2016 at 02:45:54PM -0700, Stephen Boyd wrote:
> On 09/09, Maxime Ripard wrote:
> > index 106cba27c331..964f22091a10 100644
> > --- a/drivers/clk/sunxi-ng/Makefile
> > +++ b/drivers/clk/sunxi-ng/Makefile
> > @@ -22,3 +22,4 @@ obj-$(CONFIG_SUN6I_A31_CCU) += ccu-sun6i-a31.o
> > obj-$(CONFIG_SUN8I_A23_CCU)+= ccu-sun8i-a23.o
> > obj-$(CONFIG_SUN8I_A33_CCU)+= ccu-sun8i-a33.o
> > obj-$(CONFIG_SUN8I_H3_CCU) += ccu-sun8i-h3.o
> > +obj-$(CONFIG_SUN50I_A64_CCU) += ccu-sun50i-a64.o
>
> Maybe do alphanumeric ordering?
Yes, of course.
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > new file mode 100644
> > index ..d51ee416f515
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > @@ -0,0 +1,870 @@
> > +
> > +static void __init sun50i_a64_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 + SUN50I_A64_PLL_AUDIO_REG);
> > + val &= ~GENMASK(19, 16);
> > + writel(val | (3 << 16), reg + SUN50I_A64_PLL_AUDIO_REG);
> > +
> > + writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
> > +
> > + sunxi_ccu_probe(node, reg, &sun50i_a64_ccu_desc);
> > +}
> > +CLK_OF_DECLARE(sun50i_a64_ccu, "allwinner,sun50i-a64-ccu",
> > + sun50i_a64_ccu_setup);
>
> Is there a reason it can't be a platform driver?
We have timers connected to those clocks. I'm not sure we'll ever use
them, since we also have the arch timers, and we can always change
that later, I'll change that.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
signature.asc
Description: PGP signature
Re: [PATCH v2 1/4] clk: sunxi-ng: Add A64 clocks
Hi,
On Sat, Sep 10, 2016 at 11:24:48AM +0800, Chen-Yu Tsai wrote:
> > +static SUNXI_CCU_NK_WITH_GATE_LOCK_POSTDIV(pll_periph0_clk, "pll-periph0",
> > + "osc24M", 0x028,
> > + 8, 5,/* N */
> > + 4, 2,/* K */
>
> The manual says to give preference to K >= 2. I suggest swapping N/K and
> leaving a note about it, so the actual K is in the inner loop (see ccu_nk.c)
> of the factor calculation.
It also says that we should fix that frequency (and we don't), so we
don't really care.
> > +static SUNXI_CCU_NKM_WITH_GATE_LOCK(pll_mipi_clk, "pll-mipi",
> > + "pll-video0", 0x040,
> > + 8, 4, /* N */
> > + 4, 2, /* K */
>
> You need a table for K. (Manual says K >= 2).
Not really a table, but a minimum, like we have a maximum already.
> > +static SUNXI_CCU_DIV_TABLE_WITH_GATE(ths_clk, "ths", "osc24M",
> > +0x074, 0, 2, ths_div_table, BIT(31),
> > 0);
>
> Even though the mux has only one valid parent, I suggest you still implement
> it,
> in case some bogus value gets put in before the kernel loads.
Ack.
> > +static const char * const ts_parents[] = { "osc24M", "pll-periph0", };
> > +static SUNXI_CCU_MP_WITH_MUX_GATE(ts_clk, "ts", ts_parents, 0x098,
> > + 0, 4, /* M */
> > + 16, 2,/* P */
> > + 24, 2,/* mux */
>
> Manual says the mux is 4 bits wide.
Ack.
> > +static const char * const i2s_parents[] = { "pll-audio-8x", "pll-audio-4x",
> > + "pll-audio-2x", "pll-audio" };
> > +static SUNXI_CCU_MUX_WITH_GATE(i2s0_clk, "i2s0", i2s_parents,
> > + 0x0b0, 16, 2, BIT(31), 0);
> > +
> > +static SUNXI_CCU_MUX_WITH_GATE(i2s1_clk, "i2s1", i2s_parents,
> > + 0x0b4, 16, 2, BIT(31), 0);
> > +
> > +static SUNXI_CCU_MUX_WITH_GATE(i2s2_clk, "i2s2", i2s_parents,
> > + 0x0b8, 16, 2, BIT(31), 0);
> > +
> > +static SUNXI_CCU_M_WITH_GATE(spdif_clk, "spdif", "pll-audio",
> > +0x0c0, 0, 4, BIT(31), 0);
>
> CLK_SET_PARENT_RATE for the above audio clocks?
Indeed.
> > +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_hsic_clk,"usb-hsic", "pll-hsic",
> > + 0x0cc, BIT(10), 0);
> > +static SUNXI_CCU_GATE(usb_hsic_12m_clk,"usb-hsic-12M", "osc12M",
> > + 0x0cc, BIT(11), 0);
> > +static SUNXI_CCU_GATE(usb_ohci0_clk, "usb-ohci0","osc12M",
> > + 0x0cc, BIT(16), 0);
> > +static SUNXI_CCU_GATE(usb_ohci1_clk, "usb-ohci1","usb-ohci0",
> > + 0x0cc, BIT(17), 0);
>
> I guess we aren't modeling the 2 OHCI 12M muxes?
To be honest, I can't even make sense of it. Which clocks are using it
is unclear to me, so yeah, I don't know. I can probably leave some ID
for it though, just in case.
> > +static const char * const tcon1_parents[] = { "pll-video0", "pll-video1" };
> > +static const u8 tcon1_table[] = { 0, 2, };
> > +struct ccu_div tcon1_clk = {
> > + .enable = BIT(31),
> > + .div= _SUNXI_CCU_DIV(0, 4),
> > + .mux= _SUNXI_CCU_MUX_TABLE(24, 3, tcon1_table),
> > + .common = {
> > + .reg= 0x11c,
> > + .hw.init= CLK_HW_INIT_PARENTS("tcon1",
> > + tcon1_parents,
> > + &ccu_div_ops,
> > + 0),
> > + },
> > +};
>
> CLK_SET_PARENT_RATE for the TCON clocks?
Yep
> > +static SUNXI_CCU_GATE(ac_dig_clk, "ac-dig", "pll-audio",
> > + 0x140, BIT(31), 0);
> > +
> > +static SUNXI_CCU_GATE(ac_dig_4x_clk, "ac-dig-4x","pll-audio-4x",
> > + 0x140, BIT(30), 0);
>
> CLK_SET_PARENT_RATE for the above audio clocks?
Ack.
> > +static SUNXI_CCU_GATE(avs_clk, "avs", "osc24M",
> > + 0x144, BIT(31), 0);
> > +
> > +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);
>
> Might need CLK_SET_PARENT_RATE for hdmi?
Ack
> > +static SUNXI_CCU_GATE(hdmi_ddc_clk,"hdmi-ddc", "osc24M",
> > + 0x154, BIT(31), 0);
> > +
> > +static const char * const mbus_parents[] = { "os
Re: [PATCH v2 1/4] clk: sunxi-ng: Add A64 clocks
On 09/09, Maxime Ripard wrote:
> index 106cba27c331..964f22091a10 100644
> --- a/drivers/clk/sunxi-ng/Makefile
> +++ b/drivers/clk/sunxi-ng/Makefile
> @@ -22,3 +22,4 @@ obj-$(CONFIG_SUN6I_A31_CCU) += ccu-sun6i-a31.o
> obj-$(CONFIG_SUN8I_A23_CCU) += ccu-sun8i-a23.o
> obj-$(CONFIG_SUN8I_A33_CCU) += ccu-sun8i-a33.o
> obj-$(CONFIG_SUN8I_H3_CCU) += ccu-sun8i-h3.o
> +obj-$(CONFIG_SUN50I_A64_CCU) += ccu-sun50i-a64.o
Maybe do alphanumeric ordering?
> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> new file mode 100644
> index ..d51ee416f515
> --- /dev/null
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> @@ -0,0 +1,870 @@
> +
> +static void __init sun50i_a64_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 + SUN50I_A64_PLL_AUDIO_REG);
> + val &= ~GENMASK(19, 16);
> + writel(val | (3 << 16), reg + SUN50I_A64_PLL_AUDIO_REG);
> +
> + writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
> +
> + sunxi_ccu_probe(node, reg, &sun50i_a64_ccu_desc);
> +}
> +CLK_OF_DECLARE(sun50i_a64_ccu, "allwinner,sun50i-a64-ccu",
> +sun50i_a64_ccu_setup);
Is there a reason it can't be a platform driver?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v2 1/4] clk: sunxi-ng: Add A64 clocks
On Sat, Sep 10, 2016 at 4:10 AM, Maxime Ripard wrote: > Add the A64 CCU clocks set. > > Acked-by: Rob Herring > Signed-off-by: Maxime Ripard > --- > .../devicetree/bindings/clock/sunxi-ccu.txt| 1 + > drivers/clk/sunxi-ng/Kconfig | 11 + > drivers/clk/sunxi-ng/Makefile | 1 + > drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 870 > + > drivers/clk/sunxi-ng/ccu-sun50i-a64.h | 68 ++ > include/dt-bindings/clock/sun50i-a64-ccu.h | 132 > include/dt-bindings/reset/sun50i-a64-ccu.h | 97 +++ > 7 files changed, 1180 insertions(+) > create mode 100644 drivers/clk/sunxi-ng/ccu-sun50i-a64.c > create mode 100644 drivers/clk/sunxi-ng/ccu-sun50i-a64.h > create mode 100644 include/dt-bindings/clock/sun50i-a64-ccu.h > create mode 100644 include/dt-bindings/reset/sun50i-a64-ccu.h > > diff --git a/Documentation/devicetree/bindings/clock/sunxi-ccu.txt > b/Documentation/devicetree/bindings/clock/sunxi-ccu.txt > index 3868458a5feb..74d44a4273f2 100644 > --- a/Documentation/devicetree/bindings/clock/sunxi-ccu.txt > +++ b/Documentation/devicetree/bindings/clock/sunxi-ccu.txt > @@ -7,6 +7,7 @@ Required properties : > - "allwinner,sun8i-a23-ccu" > - "allwinner,sun8i-a33-ccu" > - "allwinner,sun8i-h3-ccu" > + - "allwinner,sun50i-a64-ccu" > > - reg: Must contain the registers base address and length > - clocks: phandle to the oscillators feeding the CCU. Two are needed: > diff --git a/drivers/clk/sunxi-ng/Kconfig b/drivers/clk/sunxi-ng/Kconfig > index 254d9526c018..71e0c21c3545 100644 > --- a/drivers/clk/sunxi-ng/Kconfig > +++ b/drivers/clk/sunxi-ng/Kconfig > @@ -101,4 +101,15 @@ config SUN8I_H3_CCU > select SUNXI_CCU_PHASE > default MACH_SUN8I > > +config SUN50I_A64_CCU > + bool "Support for the Allwinner A64 CCU" > + select SUNXI_CCU_DIV > + select SUNXI_CCU_NK > + select SUNXI_CCU_NKM > + select SUNXI_CCU_NKMP > + select SUNXI_CCU_NM > + select SUNXI_CCU_MP > + select SUNXI_CCU_PHASE > + default ARM64 && ARCH_SUNXI > + > endif > diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile > index 106cba27c331..964f22091a10 100644 > --- a/drivers/clk/sunxi-ng/Makefile > +++ b/drivers/clk/sunxi-ng/Makefile > @@ -22,3 +22,4 @@ obj-$(CONFIG_SUN6I_A31_CCU) += ccu-sun6i-a31.o > obj-$(CONFIG_SUN8I_A23_CCU)+= ccu-sun8i-a23.o > obj-$(CONFIG_SUN8I_A33_CCU)+= ccu-sun8i-a33.o > obj-$(CONFIG_SUN8I_H3_CCU) += ccu-sun8i-h3.o > +obj-$(CONFIG_SUN50I_A64_CCU) += ccu-sun50i-a64.o > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > new file mode 100644 > index ..d51ee416f515 > --- /dev/null > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > @@ -0,0 +1,870 @@ > +/* > + * Copyright (c) 2016 Maxime Ripard. All rights reserved. > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > + > +#include "ccu_common.h" > +#include "ccu_reset.h" > + > +#include "ccu_div.h" > +#include "ccu_gate.h" > +#include "ccu_mp.h" > +#include "ccu_mult.h" > +#include "ccu_nk.h" > +#include "ccu_nkm.h" > +#include "ccu_nkmp.h" > +#include "ccu_nm.h" > +#include "ccu_phase.h" > + > +#include "ccu-sun50i-a64.h" > + > +static SUNXI_CCU_NKMP_WITH_GATE_LOCK(pll_cpux_clk, "pll-cpux", > +"osc24M", 0x000, > +8, 5, /* N */ > +4, 2, /* K */ > +0, 2, /* M */ > +16, 2, /* P */ You need a max = 4 for P. > +BIT(31), /* gate */ > +BIT(28), /* lock */ > +0); > + > +/* > + * The Audio PLL is supposed to have 4 outputs: 3 fixed factors from > + * the base (2x, 4x and 8x), and one variable divider (the one true > + * pll audio). > + * > + * We don't have any need for the variable divider for now, so we just > + * hardcode it to match with the clock names > + */ > +#define SUN50I_A64_PLL_AUDIO_REG 0x008 > + > +static SUNXI_CCU_NM_WITH_GATE_LOCK(pll_audio_base_clk, "pll-audio-base", > + "osc24M", 0x008, > + 8, 7,/* N */ > + 0, 5,/* M */ > +
