[PATCH v2 0/5] ARM: sun9i: Add support for PRCM on Allwinner A80 SoC

[PATCH v2 0/5] ARM: sun9i: Add support for PRCM on Allwinner A80 SoC

[Chen-Yu Tsai]

Hi everyone,

This is v2 of the Allwinner A80 PRCM support series. Thanks to
Allwinner updating their documents, we can now support the essential
parts of the PRCM.

This series adds support for the Power Reset and Clock Management
module on Allwinner's A80 SoC. The PRCM manages clocks and resets
for the "special" block of peripherals, or the R/RTC block in
earlier SoCs, as well as power domain and resets for various parts
of the SoC, such as the 2 processor clusters, the GPU and others.

The special peripherals include a standby processor core, a timer
block, a watchdog, pin controller, 1 wire interface, PS/2 interface,
a UART, the RSB controller, a DMA controller, a consumer IR receiver
block, 2 I2C controllers, and 2 I2S controllers. We do not have
documents for all the peripherals. Support will be added where
possible.

The first 2 patches add drivers for new clocks.

The 3rd patch adds a compatible string and associated list of sub-
devices to the prcm mfd driver.

The 4th patch adds the PRCM device nodes to the A80 dtsi.

The last patch adds some comments regarding the 2 system oscillators.

Changes since v1:

    - Added missing clock gates based on updated documents

    - Added new cpus clock driver based on updated documents

    - Added pll3 clock placeholder

    - Added comments about 24M & 32k oscillators

Regards
ChenYu


Chen-Yu Tsai (5):
  clk: sunxi: sun6i-apb0: Add support for sun9i A80 apbs gates
  clk: sunxi: support the cpus (cpu special) clock on the Allwinner A80
  mfd: sun6i-prcm: Add support for PRCM found on Allwinner A80 SoC
  ARM: dts: sun9i: Add A80 PRCM clocks and reset control nodes
  ARM: dts: sun9i: Add TODO comments for the main and low power clocks

 Documentation/devicetree/bindings/clock/sunxi.txt  |   2 +
 .../devicetree/bindings/mfd/sun6i-prcm.txt         |   3 +-
 arch/arm/boot/dts/sun9i-a80.dtsi                   |  79 ++++++-
 drivers/clk/sunxi/Makefile                         |   2 +-
 drivers/clk/sunxi/clk-sun6i-apb0-gates.c           |   5 +
 drivers/clk/sunxi/clk-sun9i-cpus.c                 | 243 +++++++++++++++++++++
 drivers/mfd/sun6i-prcm.c                           |  58 +++++
 7 files changed, 389 insertions(+), 3 deletions(-)
 create mode 100644 drivers/clk/sunxi/clk-sun9i-cpus.c

-- 
2.1.4

[PATCH v2 1/5] clk: sunxi: sun6i-apb0: Add support for sun9i A80 apbs gates

[Chen-Yu Tsai]

This patch adds support for the PRCM apbs clock gates found on the
Allwinner A80 SoC.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
 drivers/clk/sunxi/clk-sun6i-apb0-gates.c          | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
index 4fa11af3d378..2015b2beb957 100644
--- a/Documentation/devicetree/bindings/clock/sunxi.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi.txt
@@ -55,6 +55,7 @@ Required properties:
 	"allwinner,sun9i-a80-apb1-gates-clk" - for the APB1 gates on A80
 	"allwinner,sun6i-a31-apb2-gates-clk" - for the APB2 gates on A31
 	"allwinner,sun8i-a23-apb2-gates-clk" - for the APB2 gates on A23
+	"allwinner,sun9i-a80-apbs-gates-clk" - for the APBS gates on A80
 	"allwinner,sun5i-a13-mbus-clk" - for the MBUS clock on A13
 	"allwinner,sun4i-a10-mmc-clk" - for the MMC clock
 	"allwinner,sun9i-a80-mmc-clk" - for mmc module clocks on A80
diff --git a/drivers/clk/sunxi/clk-sun6i-apb0-gates.c b/drivers/clk/sunxi/clk-sun6i-apb0-gates.c
index 64f3e46d383c..ef5560c10447 100644
--- a/drivers/clk/sunxi/clk-sun6i-apb0-gates.c
+++ b/drivers/clk/sunxi/clk-sun6i-apb0-gates.c
@@ -29,9 +29,14 @@ static const struct gates_data sun8i_a23_apb0_gates __initconst = {
 	.mask = {0x5D},
 };
 
+static const struct gates_data sun9i_a80_apbs_gates __initconst = {
+	.mask = {0x1730FF},
+};
+
 static const struct of_device_id sun6i_a31_apb0_gates_clk_dt_ids[] = {
 	{ .compatible = "allwinner,sun6i-a31-apb0-gates-clk", .data = &sun6i_a31_apb0_gates },
 	{ .compatible = "allwinner,sun8i-a23-apb0-gates-clk", .data = &sun8i_a23_apb0_gates },
+	{ .compatible = "allwinner,sun9i-a80-apbs-gates-clk", .data = &sun9i_a80_apbs_gates },
 	{ /* sentinel */ }
 };
 
-- 
2.1.4

[PATCH v2 2/5] clk: sunxi: support the cpus (cpu special) clock on the Allwinner A80

[Chen-Yu Tsai]

The "cpus" clock is the clock for the embedded processor in the A80.
It is also part of the PRCM clock tree.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 Documentation/devicetree/bindings/clock/sunxi.txt |   1 +
 drivers/clk/sunxi/Makefile                        |   2 +-
 drivers/clk/sunxi/clk-sun9i-cpus.c                | 243 ++++++++++++++++++++++
 3 files changed, 245 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/sunxi/clk-sun9i-cpus.c

diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
index 2015b2beb957..c52735b0b924 100644
--- a/Documentation/devicetree/bindings/clock/sunxi.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi.txt
@@ -27,6 +27,7 @@ Required properties:
 	"allwinner,sun5i-a10s-ahb-gates-clk" - for the AHB gates on A10s
 	"allwinner,sun7i-a20-ahb-gates-clk" - for the AHB gates on A20
 	"allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
+	"allwinner,sun9i-a80-cpus-clk" - for the CPUS on A80
 	"allwinner,sun6i-a31-ahb1-clk" - for the AHB1 clock on A31
 	"allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
 	"allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index 058f273d6154..f0f33131b048 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -13,4 +13,4 @@ obj-y += clk-usb.o
 
 obj-$(CONFIG_MFD_SUN6I_PRCM) += \
 	clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \
-	clk-sun8i-apb0.o
+	clk-sun8i-apb0.o clk-sun9i-cpus.o
diff --git a/drivers/clk/sunxi/clk-sun9i-cpus.c b/drivers/clk/sunxi/clk-sun9i-cpus.c
new file mode 100644
index 000000000000..1ec61ccf8cbf
--- /dev/null
+++ b/drivers/clk/sunxi/clk-sun9i-cpus.c
@@ -0,0 +1,243 @@
+/*
+ * Copyright (C) 2015 Chen-Yu Tsai
+ *
+ * Chen-Yu Tsai <wens@csie.org>
+ *
+ * Allwinner A80 CPUS clock driver
+ *
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+static DEFINE_SPINLOCK(sun9i_a80_cpus_lock);
+
+/**
+ * sun9i_a80_cpus_clk_setup() - Setup function for a80 cpus composite clk
+ */
+
+#define SUN9I_CPUS_MAX_PARENTS		4
+#define SUN9I_CPUS_MUX_PARENT_PLL4	3
+#define SUN9I_CPUS_MUX_SHIFT		16
+/* un-shifted mask is what mux_clk expects */
+#define SUN9I_CPUS_MUX_MASK		0x3
+#define SUN9I_CPUS_MUX_GET_PARENT(reg)	((reg >> SUN9I_CPUS_MUX_SHIFT) & \
+					 SUN9I_CPUS_MUX_MASK)
+
+#define SUN9I_CPUS_DIV_SHIFT		4
+#define SUN9I_CPUS_DIV_MASK		(0x3 << SUN9I_CPUS_DIV_SHIFT)
+#define SUN9I_CPUS_DIV_GET(reg)		((reg & SUN9I_CPUS_DIV_MASK) >> \
+						SUN9I_CPUS_DIV_SHIFT)
+#define SUN9I_CPUS_DIV_SET(reg, div)	((reg & ~SUN9I_CPUS_DIV_MASK) | \
+						(div << SUN9I_CPUS_DIV_SHIFT))
+#define SUN9I_CPUS_PLL4_DIV_SHIFT	8
+#define SUN9I_CPUS_PLL4_DIV_MASK	(0x1f << SUN9I_CPUS_PLL4_DIV_SHIFT)
+#define SUN9I_CPUS_PLL4_DIV_GET(reg)	((reg & SUN9I_CPUS_PLL4_DIV_MASK) >> \
+						SUN9I_CPUS_PLL4_DIV_SHIFT)
+#define SUN9I_CPUS_PLL4_DIV_SET(reg, div) ((reg & ~SUN9I_CPUS_PLL4_DIV_MASK) | \
+						(div << SUN9I_CPUS_PLL4_DIV_SHIFT))
+
+struct sun9i_a80_cpus_clk {
+	struct clk_hw hw;
+	void __iomem *reg;
+};
+
+#define to_sun9i_a80_cpus_clk(_hw) container_of(_hw, struct sun9i_a80_cpus_clk, hw)
+
+static unsigned long sun9i_a80_cpus_clk_recalc_rate(struct clk_hw *hw,
+						unsigned long parent_rate)
+{
+	struct sun9i_a80_cpus_clk *cpus = to_sun9i_a80_cpus_clk(hw);
+	unsigned long rate;
+	u32 reg;
+
+	/* Fetch the register value */
+	reg = readl(cpus->reg);
+
+	/* apply pre-divider first if parent is pll4 */
+	if (SUN9I_CPUS_MUX_GET_PARENT(reg) == SUN9I_CPUS_MUX_PARENT_PLL4)
+		parent_rate /= SUN9I_CPUS_PLL4_DIV_GET(reg) + 1;
+
+	/* clk divider */
+	rate = parent_rate / (SUN9I_CPUS_DIV_GET(reg) + 1);
+
+	return rate;
+}
+
+static long sun9i_a80_cpus_clk_round(unsigned long rate, u8 *divp, u8 *pre_divp,
+				 u8 parent, unsigned long parent_rate)
+{
+	u8 div, pre_div = 1;
+
+	/*
+	 * clock can only divide, so we will never be able to achieve
+	 * frequencies higher than the parent frequency
+	 */
+	if (parent_rate && rate > parent_rate)
+		rate = parent_rate;
+
+	div = DIV_ROUND_UP(parent_rate, rate);
+
+	/* calculate pre-divider if parent is pll4 */
+	if (parent == SUN9I_CPUS_MUX_PARENT_PLL4 && div > 4) {
+		/* pre-divider is 1 ~ 32 */
+		if (div < 32) {
+			pre_div = div;
+			div = 1;
+		} else if (div < 64) {
+			pre_div = DIV_ROUND_UP(div, 2);
+			div = 2;
+		} else if (div < 96) {
+			pre_div = DIV_ROUND_UP(div, 3);
+			div = 3;
+		} else {
+			pre_div = DIV_ROUND_UP(div, 4);
+			div = 4;
+		}
+	}
+
+	/* we were asked to pass back divider values */
+	if (divp) {
+		*divp = div - 1;
+		*pre_divp = pre_div - 1;
+	}
+
+	return parent_rate / pre_div / div;
+}
+
+static long sun9i_a80_cpus_clk_determine_rate(struct clk_hw *hw,
+					      unsigned long rate,
+					      unsigned long min_rate,
+					      unsigned long max_rate,
+					      unsigned long *best_parent_rate,
+					      struct clk_hw **best_parent_clk)
+{
+	struct clk *clk = hw->clk, *parent, *best_parent = NULL;
+	int i, num_parents;
+	unsigned long parent_rate, best = 0, child_rate, best_child_rate = 0;
+
+	/* find the parent that can help provide the fastest rate <= rate */
+	num_parents = __clk_get_num_parents(clk);
+	for (i = 0; i < num_parents; i++) {
+		parent = clk_get_parent_by_index(clk, i);
+		if (!parent)
+			continue;
+		if (__clk_get_flags(clk) & CLK_SET_RATE_PARENT)
+			parent_rate = __clk_round_rate(parent, rate);
+		else
+			parent_rate = __clk_get_rate(parent);
+
+		child_rate = sun9i_a80_cpus_clk_round(rate, NULL, NULL, i,
+						  parent_rate);
+
+		if (child_rate <= rate && child_rate > best_child_rate) {
+			best_parent = parent;
+			best = parent_rate;
+			best_child_rate = child_rate;
+		}
+	}
+
+	if (best_parent)
+		*best_parent_clk = __clk_get_hw(best_parent);
+	*best_parent_rate = best;
+
+	return best_child_rate;
+}
+
+static int sun9i_a80_cpus_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+				   unsigned long parent_rate)
+{
+	struct sun9i_a80_cpus_clk *cpus = to_sun9i_a80_cpus_clk(hw);
+	unsigned long flags;
+	u8 div, pre_div, parent;
+	u32 reg;
+
+	spin_lock_irqsave(&sun9i_a80_cpus_lock, flags);
+
+	reg = readl(cpus->reg);
+
+	/* need to know which parent is used to apply pre-divider */
+	parent = SUN9I_CPUS_MUX_GET_PARENT(reg);
+	sun9i_a80_cpus_clk_round(rate, &div, &pre_div, parent, parent_rate);
+
+	reg = SUN9I_CPUS_DIV_SET(reg, div);
+	reg = SUN9I_CPUS_PLL4_DIV_SET(reg, pre_div);
+	writel(reg, cpus->reg);
+
+	spin_unlock_irqrestore(&sun9i_a80_cpus_lock, flags);
+
+	return 0;
+}
+
+static const struct clk_ops sun9i_a80_cpus_clk_ops = {
+	.determine_rate	= sun9i_a80_cpus_clk_determine_rate,
+	.recalc_rate	= sun9i_a80_cpus_clk_recalc_rate,
+	.set_rate	= sun9i_a80_cpus_clk_set_rate,
+};
+
+static int sun9i_a80_cpus_clk_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	const char *clk_name = np->name;
+	const char *parents[SUN9I_CPUS_MAX_PARENTS];
+	struct resource *r;
+	struct sun9i_a80_cpus_clk *cpus;
+	struct clk_mux *mux;
+	struct clk *clk;
+	int i = 0;
+
+	cpus = devm_kzalloc(&pdev->dev, sizeof(*cpus), GFP_KERNEL);
+	if (!cpus)
+		return -ENOMEM;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	cpus->reg = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(cpus->reg))
+		return PTR_ERR(cpus->reg);
+
+	/* we have a mux, we will have >1 parents */
+	while (i < SUN9I_CPUS_MAX_PARENTS &&
+	       (parents[i] = of_clk_get_parent_name(np, i)) != NULL)
+		i++;
+
+	of_property_read_string(np, "clock-output-names", &clk_name);
+
+	mux = devm_kzalloc(&pdev->dev, sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		return -ENOMEM;
+
+	/* set up clock properties */
+	mux->reg = cpus->reg;
+	mux->shift = SUN9I_CPUS_MUX_SHIFT;
+	mux->mask = SUN9I_CPUS_MUX_MASK;
+	mux->lock = &sun9i_a80_cpus_lock;
+
+	clk = clk_register_composite(NULL, clk_name, parents, i,
+				     &mux->hw, &clk_mux_ops,
+				     &cpus->hw, &sun9i_a80_cpus_clk_ops,
+				     NULL, NULL, 0);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	return of_clk_add_provider(np, of_clk_src_simple_get, clk);
+}
+
+static const struct of_device_id sun9i_a80_cpus_clk_dt_ids[] = {
+	{ .compatible = "allwinner,sun9i-a80-cpus-clk" },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver sun9i_a80_cpus_clk_driver = {
+	.driver = {
+		.name = "sun9i-a80-cpus-clk",
+		.of_match_table = sun9i_a80_cpus_clk_dt_ids,
+	},
+	.probe = sun9i_a80_cpus_clk_probe,
+};
+module_platform_driver(sun9i_a80_cpus_clk_driver);
+
+MODULE_AUTHOR("Chen-Yu Tsai <wens@csie.org>");
+MODULE_DESCRIPTION("Allwinner A80 CPUS Clock Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

Re: [PATCH v2 2/5] clk: sunxi: support the cpus (cpu special) clock on the Allwinner A80

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 10599 bytes --]

Hi,

On Fri, May 01, 2015 at 12:10:03AM +0800, Chen-Yu Tsai wrote:
> The "cpus" clock is the clock for the embedded processor in the A80.
> It is also part of the PRCM clock tree.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  Documentation/devicetree/bindings/clock/sunxi.txt |   1 +
>  drivers/clk/sunxi/Makefile                        |   2 +-
>  drivers/clk/sunxi/clk-sun9i-cpus.c                | 243 ++++++++++++++++++++++
>  3 files changed, 245 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/clk/sunxi/clk-sun9i-cpus.c
> 
> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> index 2015b2beb957..c52735b0b924 100644
> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> @@ -27,6 +27,7 @@ Required properties:
>  	"allwinner,sun5i-a10s-ahb-gates-clk" - for the AHB gates on A10s
>  	"allwinner,sun7i-a20-ahb-gates-clk" - for the AHB gates on A20
>  	"allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
> +	"allwinner,sun9i-a80-cpus-clk" - for the CPUS on A80
>  	"allwinner,sun6i-a31-ahb1-clk" - for the AHB1 clock on A31
>  	"allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
>  	"allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> index 058f273d6154..f0f33131b048 100644
> --- a/drivers/clk/sunxi/Makefile
> +++ b/drivers/clk/sunxi/Makefile
> @@ -13,4 +13,4 @@ obj-y += clk-usb.o
>  
>  obj-$(CONFIG_MFD_SUN6I_PRCM) += \
>  	clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \
> -	clk-sun8i-apb0.o
> +	clk-sun8i-apb0.o clk-sun9i-cpus.o

I'm really not sure about that option selection.

If you only select the A31, you will get drivers that won't be
relevant at all here.

How about something like

ifeq ($(CONFIG_MFD_SUN6I_PRCM), y)
obj-$(CONFIG_MACH_SUN6I) = ....
obj-$(CONFIG_MACH_SUN8I) = ....
obj-$(CONFIG_MACH_SUN9I) = ....
endif

?

> diff --git a/drivers/clk/sunxi/clk-sun9i-cpus.c b/drivers/clk/sunxi/clk-sun9i-cpus.c
> new file mode 100644
> index 000000000000..1ec61ccf8cbf
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-sun9i-cpus.c
> @@ -0,0 +1,243 @@
> +/*
> + * Copyright (C) 2015 Chen-Yu Tsai
> + *
> + * Chen-Yu Tsai <wens@csie.org>
> + *
> + * Allwinner A80 CPUS clock driver
> + *
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +static DEFINE_SPINLOCK(sun9i_a80_cpus_lock);
> +
> +/**
> + * sun9i_a80_cpus_clk_setup() - Setup function for a80 cpus composite clk
> + */
> +
> +#define SUN9I_CPUS_MAX_PARENTS		4
> +#define SUN9I_CPUS_MUX_PARENT_PLL4	3
> +#define SUN9I_CPUS_MUX_SHIFT		16
> +/* un-shifted mask is what mux_clk expects */
> +#define SUN9I_CPUS_MUX_MASK		0x3
> +#define SUN9I_CPUS_MUX_GET_PARENT(reg)	((reg >> SUN9I_CPUS_MUX_SHIFT) & \
> +					 SUN9I_CPUS_MUX_MASK)
> +
> +#define SUN9I_CPUS_DIV_SHIFT		4
> +#define SUN9I_CPUS_DIV_MASK		(0x3 << SUN9I_CPUS_DIV_SHIFT)
> +#define SUN9I_CPUS_DIV_GET(reg)		((reg & SUN9I_CPUS_DIV_MASK) >> \
> +						SUN9I_CPUS_DIV_SHIFT)
> +#define SUN9I_CPUS_DIV_SET(reg, div)	((reg & ~SUN9I_CPUS_DIV_MASK) | \
> +						(div << SUN9I_CPUS_DIV_SHIFT))
> +#define SUN9I_CPUS_PLL4_DIV_SHIFT	8
> +#define SUN9I_CPUS_PLL4_DIV_MASK	(0x1f << SUN9I_CPUS_PLL4_DIV_SHIFT)

You have some masks that are shifted, some that are not.

I don't really have a preference, but being consistent would be great.

(and you can use GENMASK to generate your masks).

> +#define SUN9I_CPUS_PLL4_DIV_GET(reg)	((reg & SUN9I_CPUS_PLL4_DIV_MASK) >> \
> +						SUN9I_CPUS_PLL4_DIV_SHIFT)
> +#define SUN9I_CPUS_PLL4_DIV_SET(reg, div) ((reg & ~SUN9I_CPUS_PLL4_DIV_MASK) | \
> +						(div << SUN9I_CPUS_PLL4_DIV_SHIFT))
> +
> +struct sun9i_a80_cpus_clk {
> +	struct clk_hw hw;
> +	void __iomem *reg;
> +};
> +
> +#define to_sun9i_a80_cpus_clk(_hw) container_of(_hw, struct sun9i_a80_cpus_clk, hw)
> +
> +static unsigned long sun9i_a80_cpus_clk_recalc_rate(struct clk_hw *hw,
> +						unsigned long parent_rate)

These lines above generate checkpatch warnings, please fix them.

> +	struct sun9i_a80_cpus_clk *cpus = to_sun9i_a80_cpus_clk(hw);
> +	unsigned long rate;
> +	u32 reg;
> +
> +	/* Fetch the register value */
> +	reg = readl(cpus->reg);
> +
> +	/* apply pre-divider first if parent is pll4 */
> +	if (SUN9I_CPUS_MUX_GET_PARENT(reg) == SUN9I_CPUS_MUX_PARENT_PLL4)
> +		parent_rate /= SUN9I_CPUS_PLL4_DIV_GET(reg) + 1;
> +
> +	/* clk divider */
> +	rate = parent_rate / (SUN9I_CPUS_DIV_GET(reg) + 1);
> +
> +	return rate;
> +}
> +
> +static long sun9i_a80_cpus_clk_round(unsigned long rate, u8 *divp, u8 *pre_divp,
> +				 u8 parent, unsigned long parent_rate)
> +{
> +	u8 div, pre_div = 1;
> +
> +	/*
> +	 * clock can only divide, so we will never be able to achieve
> +	 * frequencies higher than the parent frequency
> +	 */
> +	if (parent_rate && rate > parent_rate)
> +		rate = parent_rate;
> +
> +	div = DIV_ROUND_UP(parent_rate, rate);
> +
> +	/* calculate pre-divider if parent is pll4 */
> +	if (parent == SUN9I_CPUS_MUX_PARENT_PLL4 && div > 4) {
> +		/* pre-divider is 1 ~ 32 */
> +		if (div < 32) {
> +			pre_div = div;
> +			div = 1;
> +		} else if (div < 64) {
> +			pre_div = DIV_ROUND_UP(div, 2);
> +			div = 2;
> +		} else if (div < 96) {
> +			pre_div = DIV_ROUND_UP(div, 3);
> +			div = 3;
> +		} else {
> +			pre_div = DIV_ROUND_UP(div, 4);
> +			div = 4;
> +		}
> +	}
> +
> +	/* we were asked to pass back divider values */
> +	if (divp) {
> +		*divp = div - 1;
> +		*pre_divp = pre_div - 1;
> +	}
> +
> +	return parent_rate / pre_div / div;
> +}
> +
> +static long sun9i_a80_cpus_clk_determine_rate(struct clk_hw *hw,
> +					      unsigned long rate,
> +					      unsigned long min_rate,
> +					      unsigned long max_rate,
> +					      unsigned long *best_parent_rate,
> +					      struct clk_hw **best_parent_clk)
> +{
> +	struct clk *clk = hw->clk, *parent, *best_parent = NULL;
> +	int i, num_parents;
> +	unsigned long parent_rate, best = 0, child_rate, best_child_rate = 0;
> +
> +	/* find the parent that can help provide the fastest rate <= rate */
> +	num_parents = __clk_get_num_parents(clk);
> +	for (i = 0; i < num_parents; i++) {
> +		parent = clk_get_parent_by_index(clk, i);
> +		if (!parent)
> +			continue;
> +		if (__clk_get_flags(clk) & CLK_SET_RATE_PARENT)
> +			parent_rate = __clk_round_rate(parent, rate);
> +		else
> +			parent_rate = __clk_get_rate(parent);
> +
> +		child_rate = sun9i_a80_cpus_clk_round(rate, NULL, NULL, i,
> +						  parent_rate);
> +
> +		if (child_rate <= rate && child_rate > best_child_rate) {
> +			best_parent = parent;
> +			best = parent_rate;
> +			best_child_rate = child_rate;
> +		}
> +	}
> +
> +	if (best_parent)
> +		*best_parent_clk = __clk_get_hw(best_parent);
> +	*best_parent_rate = best;
> +
> +	return best_child_rate;
> +}
> +
> +static int sun9i_a80_cpus_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> +				   unsigned long parent_rate)
> +{
> +	struct sun9i_a80_cpus_clk *cpus = to_sun9i_a80_cpus_clk(hw);
> +	unsigned long flags;
> +	u8 div, pre_div, parent;
> +	u32 reg;
> +
> +	spin_lock_irqsave(&sun9i_a80_cpus_lock, flags);
> +
> +	reg = readl(cpus->reg);
> +
> +	/* need to know which parent is used to apply pre-divider */
> +	parent = SUN9I_CPUS_MUX_GET_PARENT(reg);
> +	sun9i_a80_cpus_clk_round(rate, &div, &pre_div, parent, parent_rate);
> +
> +	reg = SUN9I_CPUS_DIV_SET(reg, div);
> +	reg = SUN9I_CPUS_PLL4_DIV_SET(reg, pre_div);
> +	writel(reg, cpus->reg);
> +
> +	spin_unlock_irqrestore(&sun9i_a80_cpus_lock, flags);
> +
> +	return 0;
> +}
> +
> +static const struct clk_ops sun9i_a80_cpus_clk_ops = {
> +	.determine_rate	= sun9i_a80_cpus_clk_determine_rate,
> +	.recalc_rate	= sun9i_a80_cpus_clk_recalc_rate,
> +	.set_rate	= sun9i_a80_cpus_clk_set_rate,
> +};

It all looks like you could use the factors clock for this.

The only thing that you seem to be using a custom clock for is the pre
divider on one of the parent, but that's something that could be
reused for other clocks (like the A10 pll6, or the A20 MBUS).

> +
> +static int sun9i_a80_cpus_clk_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	const char *clk_name = np->name;
> +	const char *parents[SUN9I_CPUS_MAX_PARENTS];
> +	struct resource *r;
> +	struct sun9i_a80_cpus_clk *cpus;
> +	struct clk_mux *mux;
> +	struct clk *clk;
> +	int i = 0;
> +
> +	cpus = devm_kzalloc(&pdev->dev, sizeof(*cpus), GFP_KERNEL);
> +	if (!cpus)
> +		return -ENOMEM;
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	cpus->reg = devm_ioremap_resource(&pdev->dev, r);
> +	if (IS_ERR(cpus->reg))
> +		return PTR_ERR(cpus->reg);
> +
> +	/* we have a mux, we will have >1 parents */
> +	while (i < SUN9I_CPUS_MAX_PARENTS &&
> +	       (parents[i] = of_clk_get_parent_name(np, i)) != NULL)
> +		i++;
> +
> +	of_property_read_string(np, "clock-output-names", &clk_name);
> +
> +	mux = devm_kzalloc(&pdev->dev, sizeof(*mux), GFP_KERNEL);
> +	if (!mux)
> +		return -ENOMEM;
> +
> +	/* set up clock properties */
> +	mux->reg = cpus->reg;
> +	mux->shift = SUN9I_CPUS_MUX_SHIFT;
> +	mux->mask = SUN9I_CPUS_MUX_MASK;
> +	mux->lock = &sun9i_a80_cpus_lock;
> +
> +	clk = clk_register_composite(NULL, clk_name, parents, i,
> +				     &mux->hw, &clk_mux_ops,
> +				     &cpus->hw, &sun9i_a80_cpus_clk_ops,
> +				     NULL, NULL, 0);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	return of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +}
> +
> +static const struct of_device_id sun9i_a80_cpus_clk_dt_ids[] = {
> +	{ .compatible = "allwinner,sun9i-a80-cpus-clk" },
> +	{ /* sentinel */ }
> +};
> +
> +static struct platform_driver sun9i_a80_cpus_clk_driver = {
> +	.driver = {
> +		.name = "sun9i-a80-cpus-clk",
> +		.of_match_table = sun9i_a80_cpus_clk_dt_ids,
> +	},
> +	.probe = sun9i_a80_cpus_clk_probe,
> +};
> +module_platform_driver(sun9i_a80_cpus_clk_driver);
> +
> +MODULE_AUTHOR("Chen-Yu Tsai <wens@csie.org>");
> +MODULE_DESCRIPTION("Allwinner A80 CPUS Clock Driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.1.4
> 

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/5] clk: sunxi: support the cpus (cpu special) clock on the Allwinner A80

[Chen-Yu Tsai]

Hi,

On Mon, May 4, 2015 at 8:51 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Fri, May 01, 2015 at 12:10:03AM +0800, Chen-Yu Tsai wrote:
>> The "cpus" clock is the clock for the embedded processor in the A80.
>> It is also part of the PRCM clock tree.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>  Documentation/devicetree/bindings/clock/sunxi.txt |   1 +
>>  drivers/clk/sunxi/Makefile                        |   2 +-
>>  drivers/clk/sunxi/clk-sun9i-cpus.c                | 243 ++++++++++++++++++++++
>>  3 files changed, 245 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/clk/sunxi/clk-sun9i-cpus.c
>>
>> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
>> index 2015b2beb957..c52735b0b924 100644
>> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
>> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
>> @@ -27,6 +27,7 @@ Required properties:
>>       "allwinner,sun5i-a10s-ahb-gates-clk" - for the AHB gates on A10s
>>       "allwinner,sun7i-a20-ahb-gates-clk" - for the AHB gates on A20
>>       "allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
>> +     "allwinner,sun9i-a80-cpus-clk" - for the CPUS on A80
>>       "allwinner,sun6i-a31-ahb1-clk" - for the AHB1 clock on A31
>>       "allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
>>       "allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
>> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
>> index 058f273d6154..f0f33131b048 100644
>> --- a/drivers/clk/sunxi/Makefile
>> +++ b/drivers/clk/sunxi/Makefile
>> @@ -13,4 +13,4 @@ obj-y += clk-usb.o
>>
>>  obj-$(CONFIG_MFD_SUN6I_PRCM) += \
>>       clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \
>> -     clk-sun8i-apb0.o
>> +     clk-sun8i-apb0.o clk-sun9i-cpus.o
>
> I'm really not sure about that option selection.
>
> If you only select the A31, you will get drivers that won't be
> relevant at all here.
>
> How about something like
>
> ifeq ($(CONFIG_MFD_SUN6I_PRCM), y)
> obj-$(CONFIG_MACH_SUN6I) = ....
> obj-$(CONFIG_MACH_SUN8I) = ....
> obj-$(CONFIG_MACH_SUN9I) = ....
> endif
>
> ?

I suppose that works, though sun9i shares apb0 (apbs) clock with sun8i.

>> diff --git a/drivers/clk/sunxi/clk-sun9i-cpus.c b/drivers/clk/sunxi/clk-sun9i-cpus.c
>> new file mode 100644
>> index 000000000000..1ec61ccf8cbf
>> --- /dev/null
>> +++ b/drivers/clk/sunxi/clk-sun9i-cpus.c
>> @@ -0,0 +1,243 @@
>> +/*
>> + * Copyright (C) 2015 Chen-Yu Tsai
>> + *
>> + * Chen-Yu Tsai <wens@csie.org>
>> + *
>> + * Allwinner A80 CPUS clock driver
>> + *
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +
>> +static DEFINE_SPINLOCK(sun9i_a80_cpus_lock);
>> +
>> +/**
>> + * sun9i_a80_cpus_clk_setup() - Setup function for a80 cpus composite clk
>> + */
>> +
>> +#define SUN9I_CPUS_MAX_PARENTS               4
>> +#define SUN9I_CPUS_MUX_PARENT_PLL4   3
>> +#define SUN9I_CPUS_MUX_SHIFT         16
>> +/* un-shifted mask is what mux_clk expects */
>> +#define SUN9I_CPUS_MUX_MASK          0x3
>> +#define SUN9I_CPUS_MUX_GET_PARENT(reg)       ((reg >> SUN9I_CPUS_MUX_SHIFT) & \
>> +                                      SUN9I_CPUS_MUX_MASK)
>> +
>> +#define SUN9I_CPUS_DIV_SHIFT         4
>> +#define SUN9I_CPUS_DIV_MASK          (0x3 << SUN9I_CPUS_DIV_SHIFT)
>> +#define SUN9I_CPUS_DIV_GET(reg)              ((reg & SUN9I_CPUS_DIV_MASK) >> \
>> +                                             SUN9I_CPUS_DIV_SHIFT)
>> +#define SUN9I_CPUS_DIV_SET(reg, div) ((reg & ~SUN9I_CPUS_DIV_MASK) | \
>> +                                             (div << SUN9I_CPUS_DIV_SHIFT))
>> +#define SUN9I_CPUS_PLL4_DIV_SHIFT    8
>> +#define SUN9I_CPUS_PLL4_DIV_MASK     (0x1f << SUN9I_CPUS_PLL4_DIV_SHIFT)
>
> You have some masks that are shifted, some that are not.
>
> I don't really have a preference, but being consistent would be great.
>
> (and you can use GENMASK to generate your masks).

Yeah. I think we've been through this once with the sun6i-ahb1 clock.
Though, see below.

>> +#define SUN9I_CPUS_PLL4_DIV_GET(reg) ((reg & SUN9I_CPUS_PLL4_DIV_MASK) >> \
>> +                                             SUN9I_CPUS_PLL4_DIV_SHIFT)
>> +#define SUN9I_CPUS_PLL4_DIV_SET(reg, div) ((reg & ~SUN9I_CPUS_PLL4_DIV_MASK) | \
>> +                                             (div << SUN9I_CPUS_PLL4_DIV_SHIFT))
>> +
>> +struct sun9i_a80_cpus_clk {
>> +     struct clk_hw hw;
>> +     void __iomem *reg;
>> +};
>> +
>> +#define to_sun9i_a80_cpus_clk(_hw) container_of(_hw, struct sun9i_a80_cpus_clk, hw)
>> +
>> +static unsigned long sun9i_a80_cpus_clk_recalc_rate(struct clk_hw *hw,
>> +                                             unsigned long parent_rate)
>
> These lines above generate checkpatch warnings, please fix them.

OK.

>> +     struct sun9i_a80_cpus_clk *cpus = to_sun9i_a80_cpus_clk(hw);
>> +     unsigned long rate;
>> +     u32 reg;
>> +
>> +     /* Fetch the register value */
>> +     reg = readl(cpus->reg);
>> +
>> +     /* apply pre-divider first if parent is pll4 */
>> +     if (SUN9I_CPUS_MUX_GET_PARENT(reg) == SUN9I_CPUS_MUX_PARENT_PLL4)
>> +             parent_rate /= SUN9I_CPUS_PLL4_DIV_GET(reg) + 1;
>> +
>> +     /* clk divider */
>> +     rate = parent_rate / (SUN9I_CPUS_DIV_GET(reg) + 1);
>> +
>> +     return rate;
>> +}
>> +
>> +static long sun9i_a80_cpus_clk_round(unsigned long rate, u8 *divp, u8 *pre_divp,
>> +                              u8 parent, unsigned long parent_rate)
>> +{
>> +     u8 div, pre_div = 1;
>> +
>> +     /*
>> +      * clock can only divide, so we will never be able to achieve
>> +      * frequencies higher than the parent frequency
>> +      */
>> +     if (parent_rate && rate > parent_rate)
>> +             rate = parent_rate;
>> +
>> +     div = DIV_ROUND_UP(parent_rate, rate);
>> +
>> +     /* calculate pre-divider if parent is pll4 */
>> +     if (parent == SUN9I_CPUS_MUX_PARENT_PLL4 && div > 4) {
>> +             /* pre-divider is 1 ~ 32 */
>> +             if (div < 32) {
>> +                     pre_div = div;
>> +                     div = 1;
>> +             } else if (div < 64) {
>> +                     pre_div = DIV_ROUND_UP(div, 2);
>> +                     div = 2;
>> +             } else if (div < 96) {
>> +                     pre_div = DIV_ROUND_UP(div, 3);
>> +                     div = 3;
>> +             } else {
>> +                     pre_div = DIV_ROUND_UP(div, 4);
>> +                     div = 4;
>> +             }
>> +     }
>> +
>> +     /* we were asked to pass back divider values */
>> +     if (divp) {
>> +             *divp = div - 1;
>> +             *pre_divp = pre_div - 1;
>> +     }
>> +
>> +     return parent_rate / pre_div / div;
>> +}
>> +
>> +static long sun9i_a80_cpus_clk_determine_rate(struct clk_hw *hw,
>> +                                           unsigned long rate,
>> +                                           unsigned long min_rate,
>> +                                           unsigned long max_rate,
>> +                                           unsigned long *best_parent_rate,
>> +                                           struct clk_hw **best_parent_clk)
>> +{
>> +     struct clk *clk = hw->clk, *parent, *best_parent = NULL;
>> +     int i, num_parents;
>> +     unsigned long parent_rate, best = 0, child_rate, best_child_rate = 0;
>> +
>> +     /* find the parent that can help provide the fastest rate <= rate */
>> +     num_parents = __clk_get_num_parents(clk);
>> +     for (i = 0; i < num_parents; i++) {
>> +             parent = clk_get_parent_by_index(clk, i);
>> +             if (!parent)
>> +                     continue;
>> +             if (__clk_get_flags(clk) & CLK_SET_RATE_PARENT)
>> +                     parent_rate = __clk_round_rate(parent, rate);
>> +             else
>> +                     parent_rate = __clk_get_rate(parent);
>> +
>> +             child_rate = sun9i_a80_cpus_clk_round(rate, NULL, NULL, i,
>> +                                               parent_rate);
>> +
>> +             if (child_rate <= rate && child_rate > best_child_rate) {
>> +                     best_parent = parent;
>> +                     best = parent_rate;
>> +                     best_child_rate = child_rate;
>> +             }
>> +     }
>> +
>> +     if (best_parent)
>> +             *best_parent_clk = __clk_get_hw(best_parent);
>> +     *best_parent_rate = best;
>> +
>> +     return best_child_rate;
>> +}
>> +
>> +static int sun9i_a80_cpus_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>> +                                unsigned long parent_rate)
>> +{
>> +     struct sun9i_a80_cpus_clk *cpus = to_sun9i_a80_cpus_clk(hw);
>> +     unsigned long flags;
>> +     u8 div, pre_div, parent;
>> +     u32 reg;
>> +
>> +     spin_lock_irqsave(&sun9i_a80_cpus_lock, flags);
>> +
>> +     reg = readl(cpus->reg);
>> +
>> +     /* need to know which parent is used to apply pre-divider */
>> +     parent = SUN9I_CPUS_MUX_GET_PARENT(reg);
>> +     sun9i_a80_cpus_clk_round(rate, &div, &pre_div, parent, parent_rate);
>> +
>> +     reg = SUN9I_CPUS_DIV_SET(reg, div);
>> +     reg = SUN9I_CPUS_PLL4_DIV_SET(reg, pre_div);
>> +     writel(reg, cpus->reg);
>> +
>> +     spin_unlock_irqrestore(&sun9i_a80_cpus_lock, flags);
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct clk_ops sun9i_a80_cpus_clk_ops = {
>> +     .determine_rate = sun9i_a80_cpus_clk_determine_rate,
>> +     .recalc_rate    = sun9i_a80_cpus_clk_recalc_rate,
>> +     .set_rate       = sun9i_a80_cpus_clk_set_rate,
>> +};
>
> It all looks like you could use the factors clock for this.
>
> The only thing that you seem to be using a custom clock for is the pre
> divider on one of the parent, but that's something that could be
> reused for other clocks (like the A10 pll6, or the A20 MBUS).

We can add a custom recalc_rate() callback for factors clock,
and also pass the parent index to the get_factors() callback.
That would get rid of a lot of boilerplate.

What do you think? It kind of extends factors clk beyond what it was
designed for. If you agree, I'd also want to (ab)use it for other
A80 clocks which have multiple dividers but don't fit the current
factors clock formula.

ChenYu

>> +
>> +static int sun9i_a80_cpus_clk_probe(struct platform_device *pdev)
>> +{
>> +     struct device_node *np = pdev->dev.of_node;
>> +     const char *clk_name = np->name;
>> +     const char *parents[SUN9I_CPUS_MAX_PARENTS];
>> +     struct resource *r;
>> +     struct sun9i_a80_cpus_clk *cpus;
>> +     struct clk_mux *mux;
>> +     struct clk *clk;
>> +     int i = 0;
>> +
>> +     cpus = devm_kzalloc(&pdev->dev, sizeof(*cpus), GFP_KERNEL);
>> +     if (!cpus)
>> +             return -ENOMEM;
>> +
>> +     r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +     cpus->reg = devm_ioremap_resource(&pdev->dev, r);
>> +     if (IS_ERR(cpus->reg))
>> +             return PTR_ERR(cpus->reg);
>> +
>> +     /* we have a mux, we will have >1 parents */
>> +     while (i < SUN9I_CPUS_MAX_PARENTS &&
>> +            (parents[i] = of_clk_get_parent_name(np, i)) != NULL)
>> +             i++;
>> +
>> +     of_property_read_string(np, "clock-output-names", &clk_name);
>> +
>> +     mux = devm_kzalloc(&pdev->dev, sizeof(*mux), GFP_KERNEL);
>> +     if (!mux)
>> +             return -ENOMEM;
>> +
>> +     /* set up clock properties */
>> +     mux->reg = cpus->reg;
>> +     mux->shift = SUN9I_CPUS_MUX_SHIFT;
>> +     mux->mask = SUN9I_CPUS_MUX_MASK;
>> +     mux->lock = &sun9i_a80_cpus_lock;
>> +
>> +     clk = clk_register_composite(NULL, clk_name, parents, i,
>> +                                  &mux->hw, &clk_mux_ops,
>> +                                  &cpus->hw, &sun9i_a80_cpus_clk_ops,
>> +                                  NULL, NULL, 0);
>> +     if (IS_ERR(clk))
>> +             return PTR_ERR(clk);
>> +
>> +     return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>> +}
>> +
>> +static const struct of_device_id sun9i_a80_cpus_clk_dt_ids[] = {
>> +     { .compatible = "allwinner,sun9i-a80-cpus-clk" },
>> +     { /* sentinel */ }
>> +};
>> +
>> +static struct platform_driver sun9i_a80_cpus_clk_driver = {
>> +     .driver = {
>> +             .name = "sun9i-a80-cpus-clk",
>> +             .of_match_table = sun9i_a80_cpus_clk_dt_ids,
>> +     },
>> +     .probe = sun9i_a80_cpus_clk_probe,
>> +};
>> +module_platform_driver(sun9i_a80_cpus_clk_driver);
>> +
>> +MODULE_AUTHOR("Chen-Yu Tsai <wens@csie.org>");
>> +MODULE_DESCRIPTION("Allwinner A80 CPUS Clock Driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.1.4
>>
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

Re: [PATCH v2 2/5] clk: sunxi: support the cpus (cpu special) clock on the Allwinner A80

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 11782 bytes --]

On Mon, May 04, 2015 at 11:22:33PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Mon, May 4, 2015 at 8:51 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Hi,
> >
> > On Fri, May 01, 2015 at 12:10:03AM +0800, Chen-Yu Tsai wrote:
> >> The "cpus" clock is the clock for the embedded processor in the A80.
> >> It is also part of the PRCM clock tree.
> >>
> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >> ---
> >>  Documentation/devicetree/bindings/clock/sunxi.txt |   1 +
> >>  drivers/clk/sunxi/Makefile                        |   2 +-
> >>  drivers/clk/sunxi/clk-sun9i-cpus.c                | 243 ++++++++++++++++++++++
> >>  3 files changed, 245 insertions(+), 1 deletion(-)
> >>  create mode 100644 drivers/clk/sunxi/clk-sun9i-cpus.c
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> >> index 2015b2beb957..c52735b0b924 100644
> >> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> >> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> >> @@ -27,6 +27,7 @@ Required properties:
> >>       "allwinner,sun5i-a10s-ahb-gates-clk" - for the AHB gates on A10s
> >>       "allwinner,sun7i-a20-ahb-gates-clk" - for the AHB gates on A20
> >>       "allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
> >> +     "allwinner,sun9i-a80-cpus-clk" - for the CPUS on A80
> >>       "allwinner,sun6i-a31-ahb1-clk" - for the AHB1 clock on A31
> >>       "allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
> >>       "allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
> >> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> >> index 058f273d6154..f0f33131b048 100644
> >> --- a/drivers/clk/sunxi/Makefile
> >> +++ b/drivers/clk/sunxi/Makefile
> >> @@ -13,4 +13,4 @@ obj-y += clk-usb.o
> >>
> >>  obj-$(CONFIG_MFD_SUN6I_PRCM) += \
> >>       clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \
> >> -     clk-sun8i-apb0.o
> >> +     clk-sun8i-apb0.o clk-sun9i-cpus.o
> >
> > I'm really not sure about that option selection.
> >
> > If you only select the A31, you will get drivers that won't be
> > relevant at all here.
> >
> > How about something like
> >
> > ifeq ($(CONFIG_MFD_SUN6I_PRCM), y)
> > obj-$(CONFIG_MACH_SUN6I) = ....
> > obj-$(CONFIG_MACH_SUN8I) = ....
> > obj-$(CONFIG_MACH_SUN9I) = ....
> > endif
> >
> > ?
> 
> I suppose that works, though sun9i shares apb0 (apbs) clock with
> sun8i.

I'd expect that if you set the files to build multiple time,
everything would just work, so that we could have apbs built both in
the list for sun8i and sun9i.

> >> diff --git a/drivers/clk/sunxi/clk-sun9i-cpus.c b/drivers/clk/sunxi/clk-sun9i-cpus.c
> >> new file mode 100644
> >> index 000000000000..1ec61ccf8cbf
> >> --- /dev/null
> >> +++ b/drivers/clk/sunxi/clk-sun9i-cpus.c
> >> @@ -0,0 +1,243 @@
> >> +/*
> >> + * Copyright (C) 2015 Chen-Yu Tsai
> >> + *
> >> + * Chen-Yu Tsai <wens@csie.org>
> >> + *
> >> + * Allwinner A80 CPUS clock driver
> >> + *
> >> + */
> >> +
> >> +#include <linux/clk-provider.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of.h>
> >> +#include <linux/platform_device.h>
> >> +
> >> +static DEFINE_SPINLOCK(sun9i_a80_cpus_lock);
> >> +
> >> +/**
> >> + * sun9i_a80_cpus_clk_setup() - Setup function for a80 cpus composite clk
> >> + */
> >> +
> >> +#define SUN9I_CPUS_MAX_PARENTS               4
> >> +#define SUN9I_CPUS_MUX_PARENT_PLL4   3
> >> +#define SUN9I_CPUS_MUX_SHIFT         16
> >> +/* un-shifted mask is what mux_clk expects */
> >> +#define SUN9I_CPUS_MUX_MASK          0x3
> >> +#define SUN9I_CPUS_MUX_GET_PARENT(reg)       ((reg >> SUN9I_CPUS_MUX_SHIFT) & \
> >> +                                      SUN9I_CPUS_MUX_MASK)
> >> +
> >> +#define SUN9I_CPUS_DIV_SHIFT         4
> >> +#define SUN9I_CPUS_DIV_MASK          (0x3 << SUN9I_CPUS_DIV_SHIFT)
> >> +#define SUN9I_CPUS_DIV_GET(reg)              ((reg & SUN9I_CPUS_DIV_MASK) >> \
> >> +                                             SUN9I_CPUS_DIV_SHIFT)
> >> +#define SUN9I_CPUS_DIV_SET(reg, div) ((reg & ~SUN9I_CPUS_DIV_MASK) | \
> >> +                                             (div << SUN9I_CPUS_DIV_SHIFT))
> >> +#define SUN9I_CPUS_PLL4_DIV_SHIFT    8
> >> +#define SUN9I_CPUS_PLL4_DIV_MASK     (0x1f << SUN9I_CPUS_PLL4_DIV_SHIFT)
> >
> > You have some masks that are shifted, some that are not.
> >
> > I don't really have a preference, but being consistent would be great.
> >
> > (and you can use GENMASK to generate your masks).
> 
> Yeah. I think we've been through this once with the sun6i-ahb1 clock.
> Though, see below.

Maybe we did :)

> >> +     struct sun9i_a80_cpus_clk *cpus = to_sun9i_a80_cpus_clk(hw);
> >> +     unsigned long rate;
> >> +     u32 reg;
> >> +
> >> +     /* Fetch the register value */
> >> +     reg = readl(cpus->reg);
> >> +
> >> +     /* apply pre-divider first if parent is pll4 */
> >> +     if (SUN9I_CPUS_MUX_GET_PARENT(reg) == SUN9I_CPUS_MUX_PARENT_PLL4)
> >> +             parent_rate /= SUN9I_CPUS_PLL4_DIV_GET(reg) + 1;
> >> +
> >> +     /* clk divider */
> >> +     rate = parent_rate / (SUN9I_CPUS_DIV_GET(reg) + 1);
> >> +
> >> +     return rate;
> >> +}
> >> +
> >> +static long sun9i_a80_cpus_clk_round(unsigned long rate, u8 *divp, u8 *pre_divp,
> >> +                              u8 parent, unsigned long parent_rate)
> >> +{
> >> +     u8 div, pre_div = 1;
> >> +
> >> +     /*
> >> +      * clock can only divide, so we will never be able to achieve
> >> +      * frequencies higher than the parent frequency
> >> +      */
> >> +     if (parent_rate && rate > parent_rate)
> >> +             rate = parent_rate;
> >> +
> >> +     div = DIV_ROUND_UP(parent_rate, rate);
> >> +
> >> +     /* calculate pre-divider if parent is pll4 */
> >> +     if (parent == SUN9I_CPUS_MUX_PARENT_PLL4 && div > 4) {
> >> +             /* pre-divider is 1 ~ 32 */
> >> +             if (div < 32) {
> >> +                     pre_div = div;
> >> +                     div = 1;
> >> +             } else if (div < 64) {
> >> +                     pre_div = DIV_ROUND_UP(div, 2);
> >> +                     div = 2;
> >> +             } else if (div < 96) {
> >> +                     pre_div = DIV_ROUND_UP(div, 3);
> >> +                     div = 3;
> >> +             } else {
> >> +                     pre_div = DIV_ROUND_UP(div, 4);
> >> +                     div = 4;
> >> +             }
> >> +     }
> >> +
> >> +     /* we were asked to pass back divider values */
> >> +     if (divp) {
> >> +             *divp = div - 1;
> >> +             *pre_divp = pre_div - 1;
> >> +     }
> >> +
> >> +     return parent_rate / pre_div / div;
> >> +}
> >> +
> >> +static long sun9i_a80_cpus_clk_determine_rate(struct clk_hw *hw,
> >> +                                           unsigned long rate,
> >> +                                           unsigned long min_rate,
> >> +                                           unsigned long max_rate,
> >> +                                           unsigned long *best_parent_rate,
> >> +                                           struct clk_hw **best_parent_clk)
> >> +{
> >> +     struct clk *clk = hw->clk, *parent, *best_parent = NULL;
> >> +     int i, num_parents;
> >> +     unsigned long parent_rate, best = 0, child_rate, best_child_rate = 0;
> >> +
> >> +     /* find the parent that can help provide the fastest rate <= rate */
> >> +     num_parents = __clk_get_num_parents(clk);
> >> +     for (i = 0; i < num_parents; i++) {
> >> +             parent = clk_get_parent_by_index(clk, i);
> >> +             if (!parent)
> >> +                     continue;
> >> +             if (__clk_get_flags(clk) & CLK_SET_RATE_PARENT)
> >> +                     parent_rate = __clk_round_rate(parent, rate);
> >> +             else
> >> +                     parent_rate = __clk_get_rate(parent);
> >> +
> >> +             child_rate = sun9i_a80_cpus_clk_round(rate, NULL, NULL, i,
> >> +                                               parent_rate);
> >> +
> >> +             if (child_rate <= rate && child_rate > best_child_rate) {
> >> +                     best_parent = parent;
> >> +                     best = parent_rate;
> >> +                     best_child_rate = child_rate;
> >> +             }
> >> +     }
> >> +
> >> +     if (best_parent)
> >> +             *best_parent_clk = __clk_get_hw(best_parent);
> >> +     *best_parent_rate = best;
> >> +
> >> +     return best_child_rate;
> >> +}
> >> +
> >> +static int sun9i_a80_cpus_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> >> +                                unsigned long parent_rate)
> >> +{
> >> +     struct sun9i_a80_cpus_clk *cpus = to_sun9i_a80_cpus_clk(hw);
> >> +     unsigned long flags;
> >> +     u8 div, pre_div, parent;
> >> +     u32 reg;
> >> +
> >> +     spin_lock_irqsave(&sun9i_a80_cpus_lock, flags);
> >> +
> >> +     reg = readl(cpus->reg);
> >> +
> >> +     /* need to know which parent is used to apply pre-divider */
> >> +     parent = SUN9I_CPUS_MUX_GET_PARENT(reg);
> >> +     sun9i_a80_cpus_clk_round(rate, &div, &pre_div, parent, parent_rate);
> >> +
> >> +     reg = SUN9I_CPUS_DIV_SET(reg, div);
> >> +     reg = SUN9I_CPUS_PLL4_DIV_SET(reg, pre_div);
> >> +     writel(reg, cpus->reg);
> >> +
> >> +     spin_unlock_irqrestore(&sun9i_a80_cpus_lock, flags);
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +static const struct clk_ops sun9i_a80_cpus_clk_ops = {
> >> +     .determine_rate = sun9i_a80_cpus_clk_determine_rate,
> >> +     .recalc_rate    = sun9i_a80_cpus_clk_recalc_rate,
> >> +     .set_rate       = sun9i_a80_cpus_clk_set_rate,
> >> +};
> >
> > It all looks like you could use the factors clock for this.
> >
> > The only thing that you seem to be using a custom clock for is the pre
> > divider on one of the parent, but that's something that could be
> > reused for other clocks (like the A10 pll6, or the A20 MBUS).
> 
> We can add a custom recalc_rate() callback for factors clock,
> and also pass the parent index to the get_factors() callback.
> That would get rid of a lot of boilerplate.
> 
> What do you think?

I was more thinking about adding an additional callback that would
take the parent index as an argument, and would return for that parent
the multiplier or divider to apply.

That would be quite easy to support, and would support both fixed
divider like the one found on the A20 MBUS, or the A20 AHB clock, and
dynamic factors like this one, while have most code in the core.

> It kind of extends factors clk beyond what it was designed for. If
> you agree, I'd also want to (ab)use it for other A80 clocks which
> have multiple dividers but don't fit the current factors clock
> formula.

I think we're far beyond the point where factors clock are actually to
handle clocks with factors ;)

We've stretched that notion to handle multiple cases, up to the point
where it's basically an additional layer on top of the clock framework
itself.

I'd be quite okay to extend it, but so far the assumption has always
been that the formula was based on

parent * n >> p / (k * m)

If that formula was to change, I'm pretty sure that this would require
a lot of changes, both in the factors code itself, plus to all the
users.

I'm not against it, but if it's just for a few clocks, I don't think
it's worth it. Maybe we can just have a similar layer for that other
formula.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/5] clk: sunxi: support the cpus (cpu special) clock on the Allwinner A80

[Chen-Yu Tsai]

On Tue, May 5, 2015 at 4:25 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Mon, May 04, 2015 at 11:22:33PM +0800, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Mon, May 4, 2015 at 8:51 PM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > Hi,
>> >
>> > On Fri, May 01, 2015 at 12:10:03AM +0800, Chen-Yu Tsai wrote:
>> >> The "cpus" clock is the clock for the embedded processor in the A80.
>> >> It is also part of the PRCM clock tree.
>> >>
>> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> >> ---
>> >>  Documentation/devicetree/bindings/clock/sunxi.txt |   1 +
>> >>  drivers/clk/sunxi/Makefile                        |   2 +-
>> >>  drivers/clk/sunxi/clk-sun9i-cpus.c                | 243 ++++++++++++++++++++++
>> >>  3 files changed, 245 insertions(+), 1 deletion(-)
>> >>  create mode 100644 drivers/clk/sunxi/clk-sun9i-cpus.c
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
>> >> index 2015b2beb957..c52735b0b924 100644
>> >> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
>> >> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
>> >> @@ -27,6 +27,7 @@ Required properties:
>> >>       "allwinner,sun5i-a10s-ahb-gates-clk" - for the AHB gates on A10s
>> >>       "allwinner,sun7i-a20-ahb-gates-clk" - for the AHB gates on A20
>> >>       "allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
>> >> +     "allwinner,sun9i-a80-cpus-clk" - for the CPUS on A80
>> >>       "allwinner,sun6i-a31-ahb1-clk" - for the AHB1 clock on A31
>> >>       "allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
>> >>       "allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
>> >> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
>> >> index 058f273d6154..f0f33131b048 100644
>> >> --- a/drivers/clk/sunxi/Makefile
>> >> +++ b/drivers/clk/sunxi/Makefile
>> >> @@ -13,4 +13,4 @@ obj-y += clk-usb.o
>> >>
>> >>  obj-$(CONFIG_MFD_SUN6I_PRCM) += \
>> >>       clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \
>> >> -     clk-sun8i-apb0.o
>> >> +     clk-sun8i-apb0.o clk-sun9i-cpus.o
>> >
>> > I'm really not sure about that option selection.
>> >
>> > If you only select the A31, you will get drivers that won't be
>> > relevant at all here.
>> >
>> > How about something like
>> >
>> > ifeq ($(CONFIG_MFD_SUN6I_PRCM), y)
>> > obj-$(CONFIG_MACH_SUN6I) = ....
>> > obj-$(CONFIG_MACH_SUN8I) = ....
>> > obj-$(CONFIG_MACH_SUN9I) = ....
>> > endif
>> >
>> > ?
>>
>> I suppose that works, though sun9i shares apb0 (apbs) clock with
>> sun8i.
>
> I'd expect that if you set the files to build multiple time,
> everything would just work, so that we could have apbs built both in
> the list for sun8i and sun9i.

It should, but would it be included twice? I suppose the linker
is smart enough to spot this?

>> >> diff --git a/drivers/clk/sunxi/clk-sun9i-cpus.c b/drivers/clk/sunxi/clk-sun9i-cpus.c
>> >> new file mode 100644
>> >> index 000000000000..1ec61ccf8cbf
>> >> --- /dev/null
>> >> +++ b/drivers/clk/sunxi/clk-sun9i-cpus.c
>> >> @@ -0,0 +1,243 @@
>> >> +/*
>> >> + * Copyright (C) 2015 Chen-Yu Tsai
>> >> + *
>> >> + * Chen-Yu Tsai <wens@csie.org>
>> >> + *
>> >> + * Allwinner A80 CPUS clock driver
>> >> + *
>> >> + */
>> >> +
>> >> +#include <linux/clk-provider.h>
>> >> +#include <linux/module.h>
>> >> +#include <linux/of.h>
>> >> +#include <linux/platform_device.h>
>> >> +
>> >> +static DEFINE_SPINLOCK(sun9i_a80_cpus_lock);
>> >> +
>> >> +/**
>> >> + * sun9i_a80_cpus_clk_setup() - Setup function for a80 cpus composite clk
>> >> + */
>> >> +
>> >> +#define SUN9I_CPUS_MAX_PARENTS               4
>> >> +#define SUN9I_CPUS_MUX_PARENT_PLL4   3
>> >> +#define SUN9I_CPUS_MUX_SHIFT         16
>> >> +/* un-shifted mask is what mux_clk expects */
>> >> +#define SUN9I_CPUS_MUX_MASK          0x3
>> >> +#define SUN9I_CPUS_MUX_GET_PARENT(reg)       ((reg >> SUN9I_CPUS_MUX_SHIFT) & \
>> >> +                                      SUN9I_CPUS_MUX_MASK)
>> >> +
>> >> +#define SUN9I_CPUS_DIV_SHIFT         4
>> >> +#define SUN9I_CPUS_DIV_MASK          (0x3 << SUN9I_CPUS_DIV_SHIFT)
>> >> +#define SUN9I_CPUS_DIV_GET(reg)              ((reg & SUN9I_CPUS_DIV_MASK) >> \
>> >> +                                             SUN9I_CPUS_DIV_SHIFT)
>> >> +#define SUN9I_CPUS_DIV_SET(reg, div) ((reg & ~SUN9I_CPUS_DIV_MASK) | \
>> >> +                                             (div << SUN9I_CPUS_DIV_SHIFT))
>> >> +#define SUN9I_CPUS_PLL4_DIV_SHIFT    8
>> >> +#define SUN9I_CPUS_PLL4_DIV_MASK     (0x1f << SUN9I_CPUS_PLL4_DIV_SHIFT)
>> >
>> > You have some masks that are shifted, some that are not.
>> >
>> > I don't really have a preference, but being consistent would be great.
>> >
>> > (and you can use GENMASK to generate your masks).
>>
>> Yeah. I think we've been through this once with the sun6i-ahb1 clock.
>> Though, see below.
>
> Maybe we did :)
>
>> >> +     struct sun9i_a80_cpus_clk *cpus = to_sun9i_a80_cpus_clk(hw);
>> >> +     unsigned long rate;
>> >> +     u32 reg;
>> >> +
>> >> +     /* Fetch the register value */
>> >> +     reg = readl(cpus->reg);
>> >> +
>> >> +     /* apply pre-divider first if parent is pll4 */
>> >> +     if (SUN9I_CPUS_MUX_GET_PARENT(reg) == SUN9I_CPUS_MUX_PARENT_PLL4)
>> >> +             parent_rate /= SUN9I_CPUS_PLL4_DIV_GET(reg) + 1;
>> >> +
>> >> +     /* clk divider */
>> >> +     rate = parent_rate / (SUN9I_CPUS_DIV_GET(reg) + 1);
>> >> +
>> >> +     return rate;
>> >> +}
>> >> +
>> >> +static long sun9i_a80_cpus_clk_round(unsigned long rate, u8 *divp, u8 *pre_divp,
>> >> +                              u8 parent, unsigned long parent_rate)
>> >> +{
>> >> +     u8 div, pre_div = 1;
>> >> +
>> >> +     /*
>> >> +      * clock can only divide, so we will never be able to achieve
>> >> +      * frequencies higher than the parent frequency
>> >> +      */
>> >> +     if (parent_rate && rate > parent_rate)
>> >> +             rate = parent_rate;
>> >> +
>> >> +     div = DIV_ROUND_UP(parent_rate, rate);
>> >> +
>> >> +     /* calculate pre-divider if parent is pll4 */
>> >> +     if (parent == SUN9I_CPUS_MUX_PARENT_PLL4 && div > 4) {
>> >> +             /* pre-divider is 1 ~ 32 */
>> >> +             if (div < 32) {
>> >> +                     pre_div = div;
>> >> +                     div = 1;
>> >> +             } else if (div < 64) {
>> >> +                     pre_div = DIV_ROUND_UP(div, 2);
>> >> +                     div = 2;
>> >> +             } else if (div < 96) {
>> >> +                     pre_div = DIV_ROUND_UP(div, 3);
>> >> +                     div = 3;
>> >> +             } else {
>> >> +                     pre_div = DIV_ROUND_UP(div, 4);
>> >> +                     div = 4;
>> >> +             }
>> >> +     }
>> >> +
>> >> +     /* we were asked to pass back divider values */
>> >> +     if (divp) {
>> >> +             *divp = div - 1;
>> >> +             *pre_divp = pre_div - 1;
>> >> +     }
>> >> +
>> >> +     return parent_rate / pre_div / div;
>> >> +}
>> >> +
>> >> +static long sun9i_a80_cpus_clk_determine_rate(struct clk_hw *hw,
>> >> +                                           unsigned long rate,
>> >> +                                           unsigned long min_rate,
>> >> +                                           unsigned long max_rate,
>> >> +                                           unsigned long *best_parent_rate,
>> >> +                                           struct clk_hw **best_parent_clk)
>> >> +{
>> >> +     struct clk *clk = hw->clk, *parent, *best_parent = NULL;
>> >> +     int i, num_parents;
>> >> +     unsigned long parent_rate, best = 0, child_rate, best_child_rate = 0;
>> >> +
>> >> +     /* find the parent that can help provide the fastest rate <= rate */
>> >> +     num_parents = __clk_get_num_parents(clk);
>> >> +     for (i = 0; i < num_parents; i++) {
>> >> +             parent = clk_get_parent_by_index(clk, i);
>> >> +             if (!parent)
>> >> +                     continue;
>> >> +             if (__clk_get_flags(clk) & CLK_SET_RATE_PARENT)
>> >> +                     parent_rate = __clk_round_rate(parent, rate);
>> >> +             else
>> >> +                     parent_rate = __clk_get_rate(parent);
>> >> +
>> >> +             child_rate = sun9i_a80_cpus_clk_round(rate, NULL, NULL, i,
>> >> +                                               parent_rate);
>> >> +
>> >> +             if (child_rate <= rate && child_rate > best_child_rate) {
>> >> +                     best_parent = parent;
>> >> +                     best = parent_rate;
>> >> +                     best_child_rate = child_rate;
>> >> +             }
>> >> +     }
>> >> +
>> >> +     if (best_parent)
>> >> +             *best_parent_clk = __clk_get_hw(best_parent);
>> >> +     *best_parent_rate = best;
>> >> +
>> >> +     return best_child_rate;
>> >> +}
>> >> +
>> >> +static int sun9i_a80_cpus_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>> >> +                                unsigned long parent_rate)
>> >> +{
>> >> +     struct sun9i_a80_cpus_clk *cpus = to_sun9i_a80_cpus_clk(hw);
>> >> +     unsigned long flags;
>> >> +     u8 div, pre_div, parent;
>> >> +     u32 reg;
>> >> +
>> >> +     spin_lock_irqsave(&sun9i_a80_cpus_lock, flags);
>> >> +
>> >> +     reg = readl(cpus->reg);
>> >> +
>> >> +     /* need to know which parent is used to apply pre-divider */
>> >> +     parent = SUN9I_CPUS_MUX_GET_PARENT(reg);
>> >> +     sun9i_a80_cpus_clk_round(rate, &div, &pre_div, parent, parent_rate);
>> >> +
>> >> +     reg = SUN9I_CPUS_DIV_SET(reg, div);
>> >> +     reg = SUN9I_CPUS_PLL4_DIV_SET(reg, pre_div);
>> >> +     writel(reg, cpus->reg);
>> >> +
>> >> +     spin_unlock_irqrestore(&sun9i_a80_cpus_lock, flags);
>> >> +
>> >> +     return 0;
>> >> +}
>> >> +
>> >> +static const struct clk_ops sun9i_a80_cpus_clk_ops = {
>> >> +     .determine_rate = sun9i_a80_cpus_clk_determine_rate,
>> >> +     .recalc_rate    = sun9i_a80_cpus_clk_recalc_rate,
>> >> +     .set_rate       = sun9i_a80_cpus_clk_set_rate,
>> >> +};
>> >
>> > It all looks like you could use the factors clock for this.
>> >
>> > The only thing that you seem to be using a custom clock for is the pre
>> > divider on one of the parent, but that's something that could be
>> > reused for other clocks (like the A10 pll6, or the A20 MBUS).
>>
>> We can add a custom recalc_rate() callback for factors clock,
>> and also pass the parent index to the get_factors() callback.
>> That would get rid of a lot of boilerplate.
>>
>> What do you think?
>
> I was more thinking about adding an additional callback that would
> take the parent index as an argument, and would return for that parent
> the multiplier or divider to apply.
>
> That would be quite easy to support, and would support both fixed
> divider like the one found on the A20 MBUS, or the A20 AHB clock, and
> dynamic factors like this one, while have most code in the core.

That means we would have to determine the pre-divider value first,
in the case of dynamic ones, and they calculate the common factors.
For most of the cases we just end up using the highest pre-divider
to drop that parent rate closer to the others.

There's still the problem on how to set it though. If it were one
of the factors then we'd extend .recalc_rate to cope with that factor
not being a "common" factor. Or maybe just add an extra one. :)

>> It kind of extends factors clk beyond what it was designed for. If
>> you agree, I'd also want to (ab)use it for other A80 clocks which
>> have multiple dividers but don't fit the current factors clock
>> formula.
>
> I think we're far beyond the point where factors clock are actually to
> handle clocks with factors ;)
>
> We've stretched that notion to handle multiple cases, up to the point
> where it's basically an additional layer on top of the clock framework
> itself.
>
> I'd be quite okay to extend it, but so far the assumption has always
> been that the formula was based on
>
> parent * n >> p / (k * m)

I believe it's: (parent * (n * (k+1)) >> p) / (m+1)

The one I came across was: parent * n / (p+1) / (m+1)
where "p" is not a power of two divider.

> If that formula was to change, I'm pretty sure that this would require
> a lot of changes, both in the factors code itself, plus to all the
> users.

Actually the only place that makes this assumption is the .recalc_rate
callback. The other callbacks are just standard adapter stuff, putting
together the factors into a register. Hence my suggestion for a
.recalc_rate callback inside of factors clock. This would allow the
implementation to do whatever it saw fit to do with the 4 factors.

ChenYu

> I'm not against it, but if it's just for a few clocks, I don't think
> it's worth it. Maybe we can just have a similar layer for that other
> formula.
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

Re: [PATCH v2 2/5] clk: sunxi: support the cpus (cpu special) clock on the Allwinner A80

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 12000 bytes --]

On Tue, May 05, 2015 at 06:01:16PM +0800, Chen-Yu Tsai wrote:
> On Tue, May 5, 2015 at 4:25 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Mon, May 04, 2015 at 11:22:33PM +0800, Chen-Yu Tsai wrote:
> >> Hi,
> >>
> >> On Mon, May 4, 2015 at 8:51 PM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > Hi,
> >> >
> >> > On Fri, May 01, 2015 at 12:10:03AM +0800, Chen-Yu Tsai wrote:
> >> >> The "cpus" clock is the clock for the embedded processor in the A80.
> >> >> It is also part of the PRCM clock tree.
> >> >>
> >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >> >> ---
> >> >>  Documentation/devicetree/bindings/clock/sunxi.txt |   1 +
> >> >>  drivers/clk/sunxi/Makefile                        |   2 +-
> >> >>  drivers/clk/sunxi/clk-sun9i-cpus.c                | 243 ++++++++++++++++++++++
> >> >>  3 files changed, 245 insertions(+), 1 deletion(-)
> >> >>  create mode 100644 drivers/clk/sunxi/clk-sun9i-cpus.c
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> >> >> index 2015b2beb957..c52735b0b924 100644
> >> >> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> >> >> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> >> >> @@ -27,6 +27,7 @@ Required properties:
> >> >>       "allwinner,sun5i-a10s-ahb-gates-clk" - for the AHB gates on A10s
> >> >>       "allwinner,sun7i-a20-ahb-gates-clk" - for the AHB gates on A20
> >> >>       "allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
> >> >> +     "allwinner,sun9i-a80-cpus-clk" - for the CPUS on A80
> >> >>       "allwinner,sun6i-a31-ahb1-clk" - for the AHB1 clock on A31
> >> >>       "allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
> >> >>       "allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
> >> >> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> >> >> index 058f273d6154..f0f33131b048 100644
> >> >> --- a/drivers/clk/sunxi/Makefile
> >> >> +++ b/drivers/clk/sunxi/Makefile
> >> >> @@ -13,4 +13,4 @@ obj-y += clk-usb.o
> >> >>
> >> >>  obj-$(CONFIG_MFD_SUN6I_PRCM) += \
> >> >>       clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \
> >> >> -     clk-sun8i-apb0.o
> >> >> +     clk-sun8i-apb0.o clk-sun9i-cpus.o
> >> >
> >> > I'm really not sure about that option selection.
> >> >
> >> > If you only select the A31, you will get drivers that won't be
> >> > relevant at all here.
> >> >
> >> > How about something like
> >> >
> >> > ifeq ($(CONFIG_MFD_SUN6I_PRCM), y)
> >> > obj-$(CONFIG_MACH_SUN6I) = ....
> >> > obj-$(CONFIG_MACH_SUN8I) = ....
> >> > obj-$(CONFIG_MACH_SUN9I) = ....
> >> > endif
> >> >
> >> > ?
> >>
> >> I suppose that works, though sun9i shares apb0 (apbs) clock with
> >> sun8i.
> >
> > I'd expect that if you set the files to build multiple time,
> > everything would just work, so that we could have apbs built both in
> > the list for sun8i and sun9i.
> 
> It should, but would it be included twice? I suppose the linker
> is smart enough to spot this?

It looks like it is:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/ata/Makefile

Maybe not the linker itself, but the build system seems to handle that
just fine.

> >> >> +     struct sun9i_a80_cpus_clk *cpus = to_sun9i_a80_cpus_clk(hw);
> >> >> +     unsigned long rate;
> >> >> +     u32 reg;
> >> >> +
> >> >> +     /* Fetch the register value */
> >> >> +     reg = readl(cpus->reg);
> >> >> +
> >> >> +     /* apply pre-divider first if parent is pll4 */
> >> >> +     if (SUN9I_CPUS_MUX_GET_PARENT(reg) == SUN9I_CPUS_MUX_PARENT_PLL4)
> >> >> +             parent_rate /= SUN9I_CPUS_PLL4_DIV_GET(reg) + 1;
> >> >> +
> >> >> +     /* clk divider */
> >> >> +     rate = parent_rate / (SUN9I_CPUS_DIV_GET(reg) + 1);
> >> >> +
> >> >> +     return rate;
> >> >> +}
> >> >> +
> >> >> +static long sun9i_a80_cpus_clk_round(unsigned long rate, u8 *divp, u8 *pre_divp,
> >> >> +                              u8 parent, unsigned long parent_rate)
> >> >> +{
> >> >> +     u8 div, pre_div = 1;
> >> >> +
> >> >> +     /*
> >> >> +      * clock can only divide, so we will never be able to achieve
> >> >> +      * frequencies higher than the parent frequency
> >> >> +      */
> >> >> +     if (parent_rate && rate > parent_rate)
> >> >> +             rate = parent_rate;
> >> >> +
> >> >> +     div = DIV_ROUND_UP(parent_rate, rate);
> >> >> +
> >> >> +     /* calculate pre-divider if parent is pll4 */
> >> >> +     if (parent == SUN9I_CPUS_MUX_PARENT_PLL4 && div > 4) {
> >> >> +             /* pre-divider is 1 ~ 32 */
> >> >> +             if (div < 32) {
> >> >> +                     pre_div = div;
> >> >> +                     div = 1;
> >> >> +             } else if (div < 64) {
> >> >> +                     pre_div = DIV_ROUND_UP(div, 2);
> >> >> +                     div = 2;
> >> >> +             } else if (div < 96) {
> >> >> +                     pre_div = DIV_ROUND_UP(div, 3);
> >> >> +                     div = 3;
> >> >> +             } else {
> >> >> +                     pre_div = DIV_ROUND_UP(div, 4);
> >> >> +                     div = 4;
> >> >> +             }
> >> >> +     }
> >> >> +
> >> >> +     /* we were asked to pass back divider values */
> >> >> +     if (divp) {
> >> >> +             *divp = div - 1;
> >> >> +             *pre_divp = pre_div - 1;
> >> >> +     }
> >> >> +
> >> >> +     return parent_rate / pre_div / div;
> >> >> +}
> >> >> +
> >> >> +static long sun9i_a80_cpus_clk_determine_rate(struct clk_hw *hw,
> >> >> +                                           unsigned long rate,
> >> >> +                                           unsigned long min_rate,
> >> >> +                                           unsigned long max_rate,
> >> >> +                                           unsigned long *best_parent_rate,
> >> >> +                                           struct clk_hw **best_parent_clk)
> >> >> +{
> >> >> +     struct clk *clk = hw->clk, *parent, *best_parent = NULL;
> >> >> +     int i, num_parents;
> >> >> +     unsigned long parent_rate, best = 0, child_rate, best_child_rate = 0;
> >> >> +
> >> >> +     /* find the parent that can help provide the fastest rate <= rate */
> >> >> +     num_parents = __clk_get_num_parents(clk);
> >> >> +     for (i = 0; i < num_parents; i++) {
> >> >> +             parent = clk_get_parent_by_index(clk, i);
> >> >> +             if (!parent)
> >> >> +                     continue;
> >> >> +             if (__clk_get_flags(clk) & CLK_SET_RATE_PARENT)
> >> >> +                     parent_rate = __clk_round_rate(parent, rate);
> >> >> +             else
> >> >> +                     parent_rate = __clk_get_rate(parent);
> >> >> +
> >> >> +             child_rate = sun9i_a80_cpus_clk_round(rate, NULL, NULL, i,
> >> >> +                                               parent_rate);
> >> >> +
> >> >> +             if (child_rate <= rate && child_rate > best_child_rate) {
> >> >> +                     best_parent = parent;
> >> >> +                     best = parent_rate;
> >> >> +                     best_child_rate = child_rate;
> >> >> +             }
> >> >> +     }
> >> >> +
> >> >> +     if (best_parent)
> >> >> +             *best_parent_clk = __clk_get_hw(best_parent);
> >> >> +     *best_parent_rate = best;
> >> >> +
> >> >> +     return best_child_rate;
> >> >> +}
> >> >> +
> >> >> +static int sun9i_a80_cpus_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> >> >> +                                unsigned long parent_rate)
> >> >> +{
> >> >> +     struct sun9i_a80_cpus_clk *cpus = to_sun9i_a80_cpus_clk(hw);
> >> >> +     unsigned long flags;
> >> >> +     u8 div, pre_div, parent;
> >> >> +     u32 reg;
> >> >> +
> >> >> +     spin_lock_irqsave(&sun9i_a80_cpus_lock, flags);
> >> >> +
> >> >> +     reg = readl(cpus->reg);
> >> >> +
> >> >> +     /* need to know which parent is used to apply pre-divider */
> >> >> +     parent = SUN9I_CPUS_MUX_GET_PARENT(reg);
> >> >> +     sun9i_a80_cpus_clk_round(rate, &div, &pre_div, parent, parent_rate);
> >> >> +
> >> >> +     reg = SUN9I_CPUS_DIV_SET(reg, div);
> >> >> +     reg = SUN9I_CPUS_PLL4_DIV_SET(reg, pre_div);
> >> >> +     writel(reg, cpus->reg);
> >> >> +
> >> >> +     spin_unlock_irqrestore(&sun9i_a80_cpus_lock, flags);
> >> >> +
> >> >> +     return 0;
> >> >> +}
> >> >> +
> >> >> +static const struct clk_ops sun9i_a80_cpus_clk_ops = {
> >> >> +     .determine_rate = sun9i_a80_cpus_clk_determine_rate,
> >> >> +     .recalc_rate    = sun9i_a80_cpus_clk_recalc_rate,
> >> >> +     .set_rate       = sun9i_a80_cpus_clk_set_rate,
> >> >> +};
> >> >
> >> > It all looks like you could use the factors clock for this.
> >> >
> >> > The only thing that you seem to be using a custom clock for is the pre
> >> > divider on one of the parent, but that's something that could be
> >> > reused for other clocks (like the A10 pll6, or the A20 MBUS).
> >>
> >> We can add a custom recalc_rate() callback for factors clock,
> >> and also pass the parent index to the get_factors() callback.
> >> That would get rid of a lot of boilerplate.
> >>
> >> What do you think?
> >
> > I was more thinking about adding an additional callback that would
> > take the parent index as an argument, and would return for that parent
> > the multiplier or divider to apply.
> >
> > That would be quite easy to support, and would support both fixed
> > divider like the one found on the A20 MBUS, or the A20 AHB clock, and
> > dynamic factors like this one, while have most code in the core.
> 
> That means we would have to determine the pre-divider value first,
> in the case of dynamic ones, and they calculate the common factors.
>
> For most of the cases we just end up using the highest pre-divider
> to drop that parent rate closer to the others.
> 
> There's still the problem on how to set it though. If it were one
> of the factors then we'd extend .recalc_rate to cope with that factor
> not being a "common" factor. Or maybe just add an extra one. :)

Yeah, maybe it would be the easiest solution.

> >> It kind of extends factors clk beyond what it was designed for. If
> >> you agree, I'd also want to (ab)use it for other A80 clocks which
> >> have multiple dividers but don't fit the current factors clock
> >> formula.
> >
> > I think we're far beyond the point where factors clock are actually to
> > handle clocks with factors ;)
> >
> > We've stretched that notion to handle multiple cases, up to the point
> > where it's basically an additional layer on top of the clock framework
> > itself.
> >
> > I'd be quite okay to extend it, but so far the assumption has always
> > been that the formula was based on
> >
> > parent * n >> p / (k * m)
> 
> I believe it's: (parent * (n * (k+1)) >> p) / (m+1)
> 
> The one I came across was: parent * n / (p+1) / (m+1)
> where "p" is not a power of two divider.

Hmmm, ok.

Then maybe we can just have a flag to say wether p is a power of two
or not, just like the clock framework itself.

> > If that formula was to change, I'm pretty sure that this would require
> > a lot of changes, both in the factors code itself, plus to all the
> > users.
> 
> Actually the only place that makes this assumption is the .recalc_rate
> callback. The other callbacks are just standard adapter stuff, putting
> together the factors into a register. Hence my suggestion for a
> .recalc_rate callback inside of factors clock. This would allow the
> implementation to do whatever it saw fit to do with the 4 factors.

Ok, feel free to post some patches doing this, and we will see then :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v2 3/5] mfd: sun6i-prcm: Add support for PRCM found on Allwinner A80 SoC

[Chen-Yu Tsai]

This patch adds support for the PRCM on the A80 SoC. There is little
to no document for this at the moment. Only register offsets are
available. However with some testing, the clock and reset controls
seem to be the similar to the ones on the A31.

One thing that needs verifying is whether the apbs divider is the same
as the A31 or the A23. The lowest divider is different between those
2 implementations.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 .../devicetree/bindings/mfd/sun6i-prcm.txt         |  3 +-
 drivers/mfd/sun6i-prcm.c                           | 58 ++++++++++++++++++++++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/sun6i-prcm.txt b/Documentation/devicetree/bindings/mfd/sun6i-prcm.txt
index 03c5a551da55..c100abfcb2a4 100644
--- a/Documentation/devicetree/bindings/mfd/sun6i-prcm.txt
+++ b/Documentation/devicetree/bindings/mfd/sun6i-prcm.txt
@@ -4,7 +4,8 @@ PRCM is an MFD device exposing several Power Management related devices
 (like clks and reset controllers).
 
 Required properties:
- - compatible: "allwinner,sun6i-a31-prcm" or "allwinner,sun8i-a23-prcm"
+ - compatible: "allwinner,sun6i-a31-prcm", "allwinner,sun8i-a23-prcm",
+	       "allwinner,sun9i-a80-prcm"
  - reg: The PRCM registers range
 
 The prcm node may contain several subdevices definitions:
diff --git a/drivers/mfd/sun6i-prcm.c b/drivers/mfd/sun6i-prcm.c
index 191173166d65..e56344f04066 100644
--- a/drivers/mfd/sun6i-prcm.c
+++ b/drivers/mfd/sun6i-prcm.c
@@ -33,6 +33,22 @@ static const struct resource sun6i_a31_apb0_clk_res[] = {
 	},
 };
 
+static const struct resource sun9i_a80_cpus_clk_res[] = {
+	{
+		.start = 0x10,
+		.end = 0x13,
+		.flags = IORESOURCE_MEM,
+	},
+};
+
+static const struct resource sun9i_a80_apbs_clk_res[] = {
+	{
+		.start = 0x1c,
+		.end = 0x1f,
+		.flags = IORESOURCE_MEM,
+	},
+};
+
 static const struct resource sun6i_a31_apb0_gates_clk_res[] = {
 	{
 		.start = 0x28,
@@ -111,6 +127,39 @@ static const struct mfd_cell sun8i_a23_prcm_subdevs[] = {
 	},
 };
 
+static const struct mfd_cell sun9i_a80_prcm_subdevs[] = {
+	{
+		.name = "sun9i-a80-cpus-clk",
+		.of_compatible = "allwinner,sun9i-a80-cpus-clk",
+		.num_resources = ARRAY_SIZE(sun9i_a80_cpus_clk_res),
+		.resources = sun9i_a80_cpus_clk_res,
+	},
+	{
+		.name = "sun9i-a80-apbs-clk",
+		.of_compatible = "allwinner,sun8i-a23-apb0-clk",
+		.num_resources = ARRAY_SIZE(sun9i_a80_apbs_clk_res),
+		.resources = sun9i_a80_apbs_clk_res,
+	},
+	{
+		.name = "sun9i-a80-apbs-gates-clk",
+		.of_compatible = "allwinner,sun9i-a80-apbs-gates-clk",
+		.num_resources = ARRAY_SIZE(sun6i_a31_apb0_gates_clk_res),
+		.resources = sun6i_a31_apb0_gates_clk_res,
+	},
+	{
+		.name = "sun9i-a80-r-ir-clk",
+		.of_compatible = "allwinner,sun4i-a10-mod0-clk",
+		.num_resources = ARRAY_SIZE(sun6i_a31_ir_clk_res),
+		.resources = sun6i_a31_ir_clk_res,
+	},
+	{
+		.name = "sun9i-a80-apbs-clock-reset",
+		.of_compatible = "allwinner,sun6i-a31-clock-reset",
+		.num_resources = ARRAY_SIZE(sun6i_a31_apb0_rstc_res),
+		.resources = sun6i_a31_apb0_rstc_res,
+	},
+};
+
 static const struct prcm_data sun6i_a31_prcm_data = {
 	.nsubdevs = ARRAY_SIZE(sun6i_a31_prcm_subdevs),
 	.subdevs = sun6i_a31_prcm_subdevs,
@@ -121,6 +170,11 @@ static const struct prcm_data sun8i_a23_prcm_data = {
 	.subdevs = sun8i_a23_prcm_subdevs,
 };
 
+static const struct prcm_data sun9i_a80_prcm_data = {
+	.nsubdevs = ARRAY_SIZE(sun9i_a80_prcm_subdevs),
+	.subdevs = sun9i_a80_prcm_subdevs,
+};
+
 static const struct of_device_id sun6i_prcm_dt_ids[] = {
 	{
 		.compatible = "allwinner,sun6i-a31-prcm",
@@ -130,6 +184,10 @@ static const struct of_device_id sun6i_prcm_dt_ids[] = {
 		.compatible = "allwinner,sun8i-a23-prcm",
 		.data = &sun8i_a23_prcm_data,
 	},
+	{
+		.compatible = "allwinner,sun9i-a80-prcm",
+		.data = &sun9i_a80_prcm_data,
+	},
 	{ /* sentinel */ },
 };
 
-- 
2.1.4

Re: [PATCH v2 3/5] mfd: sun6i-prcm: Add support for PRCM found on Allwinner A80 SoC

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 712 bytes --]

On Fri, May 01, 2015 at 12:10:04AM +0800, Chen-Yu Tsai wrote:
> This patch adds support for the PRCM on the A80 SoC. There is little
> to no document for this at the moment. Only register offsets are
> available. However with some testing, the clock and reset controls
> seem to be the similar to the ones on the A31.
> 
> One thing that needs verifying is whether the apbs divider is the same
> as the A31 or the A23. The lowest divider is different between those
> 2 implementations.

Is this commit log still true now that we have access to documentation
the PRCM?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 3/5] mfd: sun6i-prcm: Add support for PRCM found on Allwinner A80 SoC

[Chen-Yu Tsai]

On Mon, May 4, 2015 at 8:53 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Fri, May 01, 2015 at 12:10:04AM +0800, Chen-Yu Tsai wrote:
>> This patch adds support for the PRCM on the A80 SoC. There is little
>> to no document for this at the moment. Only register offsets are
>> available. However with some testing, the clock and reset controls
>> seem to be the similar to the ones on the A31.
>>
>> One thing that needs verifying is whether the apbs divider is the same
>> as the A31 or the A23. The lowest divider is different between those
>> 2 implementations.
>
> Is this commit log still true now that we have access to documentation
> the PRCM?

Oops, forgot to update this one....

We have all documentation concerning the clocks.

ChenYu

[PATCH v2 4/5] ARM: dts: sun9i: Add A80 PRCM clocks and reset control nodes

[Chen-Yu Tsai]

This adds the PRCM clocks and reset controls to the A80 dtsi.

The list of apbs clock gates is incomplete. Tests show that bits 0~20
are mutable. We will need documents from Allwinner to complete the
support.

Also update clock and reset phandles for r_uart.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun9i-a80.dtsi | 64 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi
index d3dece2eea72..f0869ff8006f 100644
--- a/arch/arm/boot/dts/sun9i-a80.dtsi
+++ b/arch/arm/boot/dts/sun9i-a80.dtsi
@@ -169,6 +169,14 @@
 					     "usb_phy2", "usb_hsic_12M";
 		};
 
+		pll3: clk@06000008 {
+			/* placeholder until implemented */
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-rate = <0>;
+			clock-output-names = "pll3";
+		};
+
 		pll4: clk@0600000c {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun9i-a80-pll4-clk";
@@ -751,13 +759,67 @@
 			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
+		prcm@08001400 {
+			compatible = "allwinner,sun9i-a80-prcm";
+			reg = <0x08001400 0x200>;
+
+			cpus_clk: cpus_clk {
+				compatible = "allwinner,sun9i-a80-cpus-clk";
+				#clock-cells = <0>;
+				clocks = <&osc32k>, <&osc24M>, <&pll4>, <&pll3>;
+				clock-output-names = "cpus";
+			};
+
+			ahbs: ahbs_clk {
+				compatible = "fixed-factor-clock";
+				#clock-cells = <0>;
+				clock-div = <1>;
+				clock-mult = <1>;
+				clocks = <&cpus_clk>;
+				clock-output-names = "ahbs";
+			};
+
+			apbs: apbs_clk {
+				compatible = "allwinner,sun8i-a23-apb0-clk";
+				#clock-cells = <0>;
+				clocks = <&ahbs>;
+				clock-output-names = "apbs";
+			};
+
+			apbs_gates: apbs_gates_clk {
+				compatible = "allwinner,sun9i-a80-apbs-gates-clk";
+				#clock-cells = <1>;
+				clocks = <&apbs>;
+				clock-output-names = "apbs_pio", "apbs_ir",
+						"apbs_timer", "apbs_rsb",
+						"apbs_uart", "apbs_1wire",
+						"apbs_i2c0", "apbs_i2c1",
+						"apbs_ps2_0", "apbs_ps2_1",
+						"apbs_dma", "apbs_i2s0",
+						"apbs_i2s1", "apbs_twd";
+			};
+
+			r_ir_clk: r_ir_clk {
+				#clock-cells = <0>;
+				compatible = "allwinner,sun4i-a10-mod0-clk";
+				clocks = <&osc32k>, <&osc24M>;
+				clock-output-names = "r_ir";
+			};
+
+			apbs_rst: apbs_rst {
+				compatible = "allwinner,sun6i-a31-clock-reset";
+				#reset-cells = <1>;
+			};
+		};
+
 		r_uart: serial@08002800 {
 			compatible = "snps,dw-apb-uart";
 			reg = <0x08002800 0x400>;
 			interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
-			clocks = <&osc24M>;
+			clocks = <&apbs_gates 4>;
+			resets = <&apbs_rst 4>;
 			status = "disabled";
 		};
 	};
-- 
2.1.4

Re: [PATCH v2 4/5] ARM: dts: sun9i: Add A80 PRCM clocks and reset control nodes

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1614 bytes --]

On Fri, May 01, 2015 at 12:10:05AM +0800, Chen-Yu Tsai wrote:
> This adds the PRCM clocks and reset controls to the A80 dtsi.
> 
> The list of apbs clock gates is incomplete. Tests show that bits 0~20
> are mutable. We will need documents from Allwinner to complete the
> support.
> 
> Also update clock and reset phandles for r_uart.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  arch/arm/boot/dts/sun9i-a80.dtsi | 64 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi
> index d3dece2eea72..f0869ff8006f 100644
> --- a/arch/arm/boot/dts/sun9i-a80.dtsi
> +++ b/arch/arm/boot/dts/sun9i-a80.dtsi
> @@ -169,6 +169,14 @@
>  					     "usb_phy2", "usb_hsic_12M";
>  		};
>  
> +		pll3: clk@06000008 {
> +			/* placeholder until implemented */
> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +			clock-rate = <0>;
> +			clock-output-names = "pll3";
> +		};
> +
>  		pll4: clk@0600000c {
>  			#clock-cells = <0>;
>  			compatible = "allwinner,sun9i-a80-pll4-clk";
> @@ -751,13 +759,67 @@
>  			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>  		};
>  
> +		prcm@08001400 {
> +			compatible = "allwinner,sun9i-a80-prcm";
> +			reg = <0x08001400 0x200>;
> +
> +			cpus_clk: cpus_clk {

I wonder whether it would not be more readable to have this as
clk@<prcm_offset>, just like all the other clocks?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 4/5] ARM: dts: sun9i: Add A80 PRCM clocks and reset control nodes

[Chen-Yu Tsai]

On Mon, May 4, 2015 at 9:05 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Fri, May 01, 2015 at 12:10:05AM +0800, Chen-Yu Tsai wrote:
>> This adds the PRCM clocks and reset controls to the A80 dtsi.
>>
>> The list of apbs clock gates is incomplete. Tests show that bits 0~20
>> are mutable. We will need documents from Allwinner to complete the
>> support.
>>
>> Also update clock and reset phandles for r_uart.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>  arch/arm/boot/dts/sun9i-a80.dtsi | 64 +++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 63 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi
>> index d3dece2eea72..f0869ff8006f 100644
>> --- a/arch/arm/boot/dts/sun9i-a80.dtsi
>> +++ b/arch/arm/boot/dts/sun9i-a80.dtsi
>> @@ -169,6 +169,14 @@
>>                                            "usb_phy2", "usb_hsic_12M";
>>               };
>>
>> +             pll3: clk@06000008 {
>> +                     /* placeholder until implemented */
>> +                     #clock-cells = <0>;
>> +                     compatible = "fixed-clock";
>> +                     clock-rate = <0>;
>> +                     clock-output-names = "pll3";
>> +             };
>> +
>>               pll4: clk@0600000c {
>>                       #clock-cells = <0>;
>>                       compatible = "allwinner,sun9i-a80-pll4-clk";
>> @@ -751,13 +759,67 @@
>>                       interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>>               };
>>
>> +             prcm@08001400 {
>> +                     compatible = "allwinner,sun9i-a80-prcm";
>> +                     reg = <0x08001400 0x200>;
>> +
>> +                     cpus_clk: cpus_clk {
>
> I wonder whether it would not be more readable to have this as
> clk@<prcm_offset>, just like all the other clocks?

I agree it is more readable, though it is purely aesthetics. What offset
did you have in mind? Offset from PRCM base (0x08001400)? Or the complete
address?

Since I'm sending a new version anyway, I can make this change.

ChenYu

Re: [PATCH v2 4/5] ARM: dts: sun9i: Add A80 PRCM clocks and reset control nodes

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2405 bytes --]

On Mon, May 04, 2015 at 11:25:18PM +0800, Chen-Yu Tsai wrote:
> On Mon, May 4, 2015 at 9:05 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Fri, May 01, 2015 at 12:10:05AM +0800, Chen-Yu Tsai wrote:
> >> This adds the PRCM clocks and reset controls to the A80 dtsi.
> >>
> >> The list of apbs clock gates is incomplete. Tests show that bits 0~20
> >> are mutable. We will need documents from Allwinner to complete the
> >> support.
> >>
> >> Also update clock and reset phandles for r_uart.
> >>
> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >> ---
> >>  arch/arm/boot/dts/sun9i-a80.dtsi | 64 +++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 63 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi
> >> index d3dece2eea72..f0869ff8006f 100644
> >> --- a/arch/arm/boot/dts/sun9i-a80.dtsi
> >> +++ b/arch/arm/boot/dts/sun9i-a80.dtsi
> >> @@ -169,6 +169,14 @@
> >>                                            "usb_phy2", "usb_hsic_12M";
> >>               };
> >>
> >> +             pll3: clk@06000008 {
> >> +                     /* placeholder until implemented */
> >> +                     #clock-cells = <0>;
> >> +                     compatible = "fixed-clock";
> >> +                     clock-rate = <0>;
> >> +                     clock-output-names = "pll3";
> >> +             };
> >> +
> >>               pll4: clk@0600000c {
> >>                       #clock-cells = <0>;
> >>                       compatible = "allwinner,sun9i-a80-pll4-clk";
> >> @@ -751,13 +759,67 @@
> >>                       interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
> >>               };
> >>
> >> +             prcm@08001400 {
> >> +                     compatible = "allwinner,sun9i-a80-prcm";
> >> +                     reg = <0x08001400 0x200>;
> >> +
> >> +                     cpus_clk: cpus_clk {
> >
> > I wonder whether it would not be more readable to have this as
> > clk@<prcm_offset>, just like all the other clocks?
> 
> I agree it is more readable, though it is purely aesthetics. What offset
> did you have in mind? Offset from PRCM base (0x08001400)? Or the complete
> address?

I'd say the offset within the PRCM.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v2 5/5] ARM: dts: sun9i: Add TODO comments for the main and low power clocks

[Chen-Yu Tsai]

The main (24MHz) clock on the A80 is configurable via the PRCM address
space. The low power/speed (32kHz) clock is from an external chip, the
AC100.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun9i-a80.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi
index f0869ff8006f..517d5a8c7b59 100644
--- a/arch/arm/boot/dts/sun9i-a80.dtsi
+++ b/arch/arm/boot/dts/sun9i-a80.dtsi
@@ -133,6 +133,14 @@
 		 */
 		ranges = <0 0 0 0x20000000>;
 
+		/*
+		 * This clock is actually configurable from the PRCM address
+		 * space. The external 24M oscillator can be turned off, and
+		 * the clock switched to an internal 16M RC oscillator. Under
+		 * normal operation there's no reason to do this, and the
+		 * default is to use the external good one, so just model this
+		 * as a fixed clock.
+		 */
 		osc24M: osc24M_clk {
 			#clock-cells = <0>;
 			compatible = "fixed-clock";
@@ -140,6 +148,13 @@
 			clock-output-names = "osc24M";
 		};
 
+		/*
+		 * The 32k clock is from an external source, normally the
+		 * AC100 codec/RTC chip. This clock is by default enabled
+		 * and clocked at 32768 Hz, from the oscillator connected
+		 * to the AC100. It is configurable, but no such driver or
+		 * bindings exist yet.
+		 */
 		osc32k: osc32k_clk {
 			#clock-cells = <0>;
 			compatible = "fixed-clock";
-- 
2.1.4