On Thu, Jun 19, 2014 at 5:28 PM, Maxime Ripard
[off-list ref] wrote:

On Thu, Jun 19, 2014 at 12:33:41PM +0800, Chen-Yu Tsai wrote:

quoted

On Wed, Jun 18, 2014 at 6:26 PM, Maxime Ripard
[off-list ref] wrote:

quoted

On Tue, Jun 17, 2014 at 10:52:49PM +0800, Chen-Yu Tsai wrote:

quoted

The A23 has an almost identical PRCM clock tree. The difference in
the APB0 clock is the smallest divisor is 1, instead of 2.

This patch extends the sun6i-a31-apb0-clk driver to take divider
tables associated to compatibles, and adds a compatible for the A23
variant.

Signed-off-by: Chen-Yu Tsai <redacted>
---
 Documentation/devicetree/bindings/clock/sunxi.txt |  1 +
 drivers/clk/sunxi/clk-sun6i-apb0.c                | 28 ++++++++++++++++++-----
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
index af9e47d..e3a47ec 100644
--- a/Documentation/devicetree/bindings/clock/sunxi.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi.txt

@@ -28,6 +28,7 @@ Required properties:
      "allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
      "allwinner,sun4i-a10-apb0-clk" - for the APB0 clock
      "allwinner,sun6i-a31-apb0-clk" - for the APB0 clock on A31
+     "allwinner,sun8i-a23-apb0-clk" - for the APB0 clock on A23
      "allwinner,sun4i-a10-apb0-gates-clk" - for the APB0 gates on A10
      "allwinner,sun5i-a13-apb0-gates-clk" - for the APB0 gates on A13
      "allwinner,sun5i-a10s-apb0-gates-clk" - for the APB0 gates on A10s

diff --git a/drivers/clk/sunxi/clk-sun6i-apb0.c b/drivers/clk/sunxi/clk-sun6i-apb0.c
index 11f17c3..2197ac7 100644
--- a/drivers/clk/sunxi/clk-sun6i-apb0.c
+++ b/drivers/clk/sunxi/clk-sun6i-apb0.c

@@ -11,6 +11,7 @@
 #include <linux/clk-provider.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>

 /*

@@ -28,6 +29,21 @@ static const struct clk_div_table sun6i_a31_apb0_divs[] = {

For reference:

static const struct clk_div_table sun6i_a31_apb0_divs[] = {
        { .val = 0, .div = 2, },
        { .val = 1, .div = 2, },
        { .val = 2, .div = 4, },
        { .val = 3, .div = 8, },
        { /* sentinel */ },
};

quoted

quoted

      { /* sentinel */ },
 };

+/* The A23 APB0 clock has a standard power of 2 divisor */

Why not just pass CLK_DIVIDER_POWER_OF_TWO then, instead of a table?

The A31 APB0 clock uses a table for the odd /2 divisor for value 0.
The standard table I'm using for the A23 is just to keep it simple
and alike.

quoted

quoted

+static const struct clk_div_table sun8i_a23_apb0_divs[] = {
+     { .val = 0, .div = 1, },
+     { .val = 1, .div = 2, },
+     { .val = 2, .div = 4, },
+     { .val = 3, .div = 8, },
+     { /* sentinel */ },
+};
+
+const struct of_device_id sun6i_a31_apb0_clk_dt_ids[] = {
+     { .compatible = "allwinner,sun6i-a31-apb0-clk", .data = &sun6i_a31_apb0_divs },
+     { .compatible = "allwinner,sun8i-a23-apb0-clk", .data = &sun8i_a23_apb0_divs },
+     { /* sentinel */ }
+};
+
 static int sun6i_a31_apb0_clk_probe(struct platform_device *pdev)
 {
      struct device_node *np = pdev->dev.of_node;

@@ -36,12 +52,17 @@ static int sun6i_a31_apb0_clk_probe(struct platform_device *pdev)
      struct resource *r;
      void __iomem *reg;
      struct clk *clk;
+     const struct of_device_id *device;

      r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
      reg = devm_ioremap_resource(&pdev->dev, r);
      if (IS_ERR(reg))
              return PTR_ERR(reg);

+     device = of_match_device(sun6i_a31_apb0_clk_dt_ids, &pdev->dev);
+     if (!device)
+             return -EINVAL;
+
      clk_parent = of_clk_get_parent_name(np, 0);
      if (!clk_parent)
              return -EINVAL;

@@ -49,7 +70,7 @@ static int sun6i_a31_apb0_clk_probe(struct platform_device *pdev)
      of_property_read_string(np, "clock-output-names", &clk_name);

      clk = clk_register_divider_table(&pdev->dev, clk_name, clk_parent,
-                                      0, reg, 0, 2, 0, sun6i_a31_apb0_divs,
+                                      0, reg, 0, 2, 0, device->data,

I'm not sure that it will actually work for the A31, since it does
define some dividers anyway, and the divider table is !NULL, even
though there's actually no dividers defined.

I'm not following. The A31 does have a table defined. It's just cut
off in this patch. I asked Boris to add the table when he was working
on the A31 PRCM clocks. See above.

Ah right. My bad.

If these two clocks are these different though, maybe it would just be
easier to add a new driver. These are trivial enough anyway.

Yes it is. But it adds more of the same boilerplate code than actual
differences of the hardware.

I'm inclined to keep it as it is for now, unless someone strongly
objects.


ChenYu