[PATCH 0/7] mfd: add basic sun6i A31 PRCM support

[PATCH 0/7] mfd: add basic sun6i A31 PRCM support

[Boris BREZILLON]

Hello,

This patch series adds support for some functions provided by the PRCM
(Power/Reset/Clock Management) unit:
- AR100, AHB0 and APB0 clocks
- APB0 reset controller

These functions are needed to get the P2WI driver working, but more
subdevices might be added later.

Best Regards,

Boris

Boris BREZILLON (7):
  reset: sunxi: document sunxi's reset controllers bindings
  reset: sunxi: allow MFD subdevices probe
  mfd: add support for sun6i PRCM (Power/Reset/Clock Management) unit
  mfd: sun6i-prcm: document DT bindings
  clk: sunxi: add PRCM (Power/Reset/Clock Management) clks support
  clk: sunxi: document PRCM clock compatible strings
  ARM: sunxi: dt: add PRCM clk and reset controller subdevices

 Documentation/devicetree/bindings/clock/sunxi.txt  |   6 +
 .../devicetree/bindings/mfd/sun6i-prcm.txt         |  71 ++++++
 .../bindings/reset/allwinner,sunxi-clock-reset.txt |  21 ++
 arch/arm/boot/dts/sun6i-a31.dtsi                   |  49 ++++
 drivers/clk/sunxi/Makefile                         |   2 +
 drivers/clk/sunxi/clk-sun6i-prcm.c                 | 253 +++++++++++++++++++++
 drivers/mfd/Kconfig                                |   8 +
 drivers/mfd/Makefile                               |   1 +
 drivers/mfd/sun6i-prcm.c                           | 163 +++++++++++++
 drivers/reset/reset-sunxi.c                        |  19 +-
 10 files changed, 592 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/sun6i-prcm.txt
 create mode 100644 Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt
 create mode 100644 drivers/clk/sunxi/clk-sun6i-prcm.c
 create mode 100644 drivers/mfd/sun6i-prcm.c

-- 
1.8.3.2

[PATCH 1/7] reset: sunxi: document sunxi's reset controllers bindings

[Boris BREZILLON]

Add DT bindings documentation for sunxi's reset controllers.

Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 .../bindings/reset/allwinner,sunxi-clock-reset.txt  | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt

diff --git a/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt b/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt
new file mode 100644
index 0000000..a582804
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt
@@ -0,0 +1,21 @@
+Allwinner sunxi Peripheral Reset Controller
+===========================================
+
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+Required properties:
+- compatible: Should be one of the following depending on your SoC:
+  "allwinner,sun6i-a31-ahb1-reset"
+  "allwinner,sun6i-a31-clock-reset"
+- reg: should be register base and length as documented in the
+  datasheet
+- #reset-cells: 1, see below
+
+example:
+
+ahb1_rst: reset@01c202c0 {
+	#reset-cells = <1>;
+	compatible = "allwinner,sun6i-a31-ahb1-reset";
+	reg = <0x01c202c0 0xc>;
+};
-- 
1.8.3.2

Re: [PATCH 1/7] reset: sunxi: document sunxi's reset controllers bindings

[Maxime Ripard]

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

Hi Boris,

On Mon, Apr 28, 2014 at 04:58:44PM +0200, Boris BREZILLON wrote:
> Add DT bindings documentation for sunxi's reset controllers.
> 
> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> ---
>  .../bindings/reset/allwinner,sunxi-clock-reset.txt  | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt
> 
> diff --git a/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt b/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt
> new file mode 100644
> index 0000000..a582804
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt
> @@ -0,0 +1,21 @@
> +Allwinner sunxi Peripheral Reset Controller
> +===========================================
> +
> +Please also refer to reset.txt in this directory for common reset
> +controller binding usage.
> +
> +Required properties:
> +- compatible: Should be one of the following depending on your SoC:

It doesn't really depend on the SoC.

Maxime

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

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

[PATCH 2/7] reset: sunxi: allow MFD subdevices probe

[Boris BREZILLON]

The current implementation uses sunxi_reset_init function for both early
init and platform device probe.

The sunxi_reset_init function uses DT to retrieve device resources, which
will be an issue if reset controllers are registered from an MFD device
that define resources from mfd_cell definition.

Moreover, we can make use of devm functions when we're in probe context.

Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/reset/reset-sunxi.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c
index 695bd34..115fbb1 100644
--- a/drivers/reset/reset-sunxi.c
+++ b/drivers/reset/reset-sunxi.c
@@ -145,7 +145,24 @@ MODULE_DEVICE_TABLE(of, sunxi_reset_dt_ids);
 
 static int sunxi_reset_probe(struct platform_device *pdev)
 {
-	return sunxi_reset_init(pdev->dev.of_node);
+	struct sunxi_reset_data *data;
+	struct resource *res;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->membase = devm_request_and_ioremap(&pdev->dev, res);
+	if (!data->membase)
+		return -ENOMEM;
+
+	data->rcdev.owner = THIS_MODULE;
+	data->rcdev.nr_resets = resource_size(res) * 32;
+	data->rcdev.ops = &sunxi_reset_ops;
+	data->rcdev.of_node = pdev->dev.of_node;
+
+	return reset_controller_register(&data->rcdev);
 }
 
 static int sunxi_reset_remove(struct platform_device *pdev)
-- 
1.8.3.2

Re: [PATCH 2/7] reset: sunxi: allow MFD subdevices probe

[Maxime Ripard]

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

On Mon, Apr 28, 2014 at 04:58:45PM +0200, Boris BREZILLON wrote:
> The current implementation uses sunxi_reset_init function for both early
> init and platform device probe.
> 
> The sunxi_reset_init function uses DT to retrieve device resources, which
> will be an issue if reset controllers are registered from an MFD device
> that define resources from mfd_cell definition.
> 
> Moreover, we can make use of devm functions when we're in probe context.
> 
> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> ---
>  drivers/reset/reset-sunxi.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c
> index 695bd34..115fbb1 100644
> --- a/drivers/reset/reset-sunxi.c
> +++ b/drivers/reset/reset-sunxi.c
> @@ -145,7 +145,24 @@ MODULE_DEVICE_TABLE(of, sunxi_reset_dt_ids);
>  
>  static int sunxi_reset_probe(struct platform_device *pdev)
>  {
> -	return sunxi_reset_init(pdev->dev.of_node);
> +	struct sunxi_reset_data *data;
> +	struct resource *res;
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	data->membase = devm_request_and_ioremap(&pdev->dev, res);
> +	if (!data->membase)
> +		return -ENOMEM;
> +
> +	data->rcdev.owner = THIS_MODULE;
> +	data->rcdev.nr_resets = resource_size(res) * 32;
> +	data->rcdev.ops = &sunxi_reset_ops;
> +	data->rcdev.of_node = pdev->dev.of_node;
> +
> +	return reset_controller_register(&data->rcdev);
>  }
>  
>  static int sunxi_reset_remove(struct platform_device *pdev)

If you're switching to devm_ functions, you'll also have to remove the
iounmap and kfree in the _remove.

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: 836 bytes --]

[PATCH 3/7] mfd: add support for sun6i PRCM (Power/Reset/Clock Management) unit

[Boris BREZILLON]

The PRCM (Power/Reset/Clock Management) block exposes several subdevices
in different subsystems (clk, reset ...)

Add basic support for the PRCM unit with clk (AR100, AHB0, and APB0 clks)
and reset controller subdevices.

Other subdevices might be added later (if needed).

Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/mfd/Kconfig      |   8 +++
 drivers/mfd/Makefile     |   1 +
 drivers/mfd/sun6i-prcm.c | 163 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 172 insertions(+)
 create mode 100644 drivers/mfd/sun6i-prcm.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 2d347c9..0a9220d 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -731,6 +731,14 @@ config MFD_STA2X11
 	select MFD_CORE
 	select REGMAP_MMIO
 
+config MFD_SUN6I_PRCM
+	bool "Allwinner PRCM controller"
+	depends on ARCH_SUNXI
+	select MFD_CORE
+	help
+	  Support for the PRCM (Power/Reset/Clock Management) unit available
+	  in A31 SoC.
+
 config MFD_SYSCON
 	bool "System Controller Register R/W Based on Regmap"
 	select REGMAP_MMIO
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 1efecf2..df7823c 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_MFD_STA2X11)	+= sta2x11-mfd.o
 obj-$(CONFIG_MFD_STMPE)		+= stmpe.o
 obj-$(CONFIG_STMPE_I2C)		+= stmpe-i2c.o
 obj-$(CONFIG_STMPE_SPI)		+= stmpe-spi.o
+obj-$(CONFIG_MFD_SUN6I_PRCM)	+= sun6i-prcm.o
 obj-$(CONFIG_MFD_TC3589X)	+= tc3589x.o
 obj-$(CONFIG_MFD_T7L66XB)	+= t7l66xb.o tmio_core.o
 obj-$(CONFIG_MFD_TC6387XB)	+= tc6387xb.o tmio_core.o
diff --git a/drivers/mfd/sun6i-prcm.c b/drivers/mfd/sun6i-prcm.c
new file mode 100644
index 0000000..e05d92d
--- /dev/null
+++ b/drivers/mfd/sun6i-prcm.c
@@ -0,0 +1,163 @@
+/*
+ * Copyright (C) 2014 Free Electrons
+ *
+ * License Terms: GNU General Public License v2
+ * Author: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ *
+ * Allwinner PRCM (Power/Reset/Clock Management) driver
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <linux/spinlock.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/completion.h>
+#include <linux/irq.h>
+#include <linux/jiffies.h>
+#include <linux/bitops.h>
+#include <linux/fs.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/uaccess.h>
+#include <linux/mfd/core.h>
+
+struct prcm_data {
+	int nsubdevs;
+	const struct mfd_cell *subdevs;
+};
+
+static const struct resource sun6i_a31_ar100_clk_res[] = {
+	{
+		.start = 0x0,
+		.end = 0x3,
+		.flags = IORESOURCE_MEM,
+	},
+};
+
+static const struct resource sun6i_a31_apb0_clk_res[] = {
+	{
+		.start = 0xc,
+		.end = 0xf,
+		.flags = IORESOURCE_MEM,
+	},
+};
+
+static const struct resource sun6i_a31_apb0_gates_clk_res[] = {
+	{
+		.start = 0x28,
+		.end = 0x2b,
+		.flags = IORESOURCE_MEM,
+	},
+};
+
+static const struct resource sun6i_a31_apb0_rstc_res[] = {
+	{
+		.start = 0xb0,
+		.end = 0xb3,
+		.flags = IORESOURCE_MEM,
+	},
+};
+
+static const struct mfd_cell sun6i_a31_prcm_subdevs[] = {
+	{
+		.name = "sun6i-a31-ar100-mux-clk",
+		.of_compatible = "allwinner,sun6i-a31-ar100-mux-clk",
+		.num_resources = ARRAY_SIZE(sun6i_a31_ar100_clk_res),
+		.resources = sun6i_a31_ar100_clk_res,
+	},
+	{
+		.name = "sun6i-a31-ar100-clk",
+		.of_compatible = "allwinner,sun6i-a31-ar100-clk",
+		.num_resources = ARRAY_SIZE(sun6i_a31_ar100_clk_res),
+		.resources = sun6i_a31_ar100_clk_res,
+	},
+	{
+		.name = "sun6i-a31-ar100-div-clk",
+		.of_compatible = "allwinner,sun6i-a31-ar100-div-clk",
+		.num_resources = ARRAY_SIZE(sun6i_a31_ar100_clk_res),
+		.resources = sun6i_a31_ar100_clk_res,
+	},
+	{
+		.name = "sun6i-a31-apb0-clk",
+		.of_compatible = "allwinner,sun6i-a31-apb0-clk",
+		.num_resources = ARRAY_SIZE(sun6i_a31_apb0_clk_res),
+		.resources = sun6i_a31_apb0_clk_res,
+	},
+	{
+		.name = "sun6i-a31-apb0-gates-clk",
+		.of_compatible = "allwinner,sun6i-a31-apb0-gates-clk",
+		.num_resources = ARRAY_SIZE(sun6i_a31_apb0_gates_clk_res),
+		.resources = sun6i_a31_apb0_gates_clk_res,
+	},
+	{
+		.name = "sun6i-a31-apb0-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,
+};
+
+static const struct of_device_id sun6i_prcm_dt_ids[] = {
+	{
+		.compatible = "allwinner,sun6i-a31-prcm",
+		.data = &sun6i_a31_prcm_data,
+	},
+	{ /* sentinel */ },
+};
+
+static int sun6i_prcm_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *match;
+	const struct prcm_data *data;
+	struct resource *res;
+	int ret;
+
+	match = of_match_node(sun6i_prcm_dt_ids, np);
+	if (!match)
+		return -EINVAL;
+
+	data = match->data;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "no prcm memory region provided\n");
+		return -ENOENT;
+	}
+
+	ret = mfd_add_devices(&pdev->dev, 0, data->subdevs, data->nsubdevs,
+			      res, -1, NULL);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add subdevices\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "PRCM initialized\n");
+
+	return 0;
+}
+
+static struct platform_driver sun6i_prcm_driver = {
+	.driver = {
+		.name = "sun6i-prcm",
+		.owner = THIS_MODULE,
+		.of_match_table = sun6i_prcm_dt_ids,
+	},
+	.probe = sun6i_prcm_probe,
+};
+module_platform_driver(sun6i_prcm_driver);
+
+MODULE_AUTHOR("Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
+MODULE_DESCRIPTION("Allwinner sun6i PRCM driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.3.2

Re: [PATCH 3/7] mfd: add support for sun6i PRCM (Power/Reset/Clock Management) unit

[Maxime Ripard]

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

Hi Boris,

On Mon, Apr 28, 2014 at 04:58:46PM +0200, Boris BREZILLON wrote:
> The PRCM (Power/Reset/Clock Management) block exposes several subdevices
> in different subsystems (clk, reset ...)
> 
> Add basic support for the PRCM unit with clk (AR100, AHB0, and APB0 clks)
> and reset controller subdevices.
> 
> Other subdevices might be added later (if needed).
> 
> Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  drivers/mfd/Kconfig      |   8 +++
>  drivers/mfd/Makefile     |   1 +
>  drivers/mfd/sun6i-prcm.c | 163 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 172 insertions(+)
>  create mode 100644 drivers/mfd/sun6i-prcm.c
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 2d347c9..0a9220d 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -731,6 +731,14 @@ config MFD_STA2X11
>  	select MFD_CORE
>  	select REGMAP_MMIO
>  
> +config MFD_SUN6I_PRCM
> +	bool "Allwinner PRCM controller"

I'd mention the A31 in the prompt

Maxime

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

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

[PATCH 4/7] mfd: sun6i-prcm: document DT bindings

[Boris BREZILLON]

Document DT bindings of the PRCM (Power/Reset/Clock Management) unit.

Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 .../devicetree/bindings/mfd/sun6i-prcm.txt         | 71 ++++++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/sun6i-prcm.txt

diff --git a/Documentation/devicetree/bindings/mfd/sun6i-prcm.txt b/Documentation/devicetree/bindings/mfd/sun6i-prcm.txt
new file mode 100644
index 0000000..de92429
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sun6i-prcm.txt
@@ -0,0 +1,71 @@
+* Allwinner PRCM (Power/Reset/Clock Management) Multi-Functional Device
+
+PRCM is an MFD device exposing several Power Management related devices
+(like clks and reset controllers).
+
+Required properties:
+ - compatible: "allwinner,sun6i-a31-prcm"
+ - reg: The PRCM registers range
+
+The prcm node may contain several subdevices definitions:
+ - see Documentation/devicetree/clk/sunxi.txt for clock devices
+ - see Documentation/devicetree/reset/allwinner,sunxi-clock-reset.txt for reset
+   controller devices
+
+
+Example:
+
+	prcm: prcm@01f01400 {
+		compatible = "allwinner,sun6i-a31-prcm";
+		reg = <0x01f01400>;
+
+		/* Put subdevices here */
+		ar100_mux: ar100_mux {
+			compatible = "allwinner,sun6i-a31-ar100-mux-clk";
+			#clock-cells = <0>;
+			clocks = <&osc32k>, <&osc24M>, <&pll6>, <&pll6>;
+		};
+
+		ar100: ar100 {
+			compatible = "allwinner,sun6i-a31-ar100-clk";
+			#clock-cells = <0>;
+			clocks = <&ar100_mux>;
+		};
+
+		ar100_div: ar100_div {
+			compatible = "allwinner,sun6i-a31-ar100-div-clk";
+			#clock-cells = <0>;
+			clocks = <&ar100>;
+		};
+
+		ahb0: ahb0 {
+			compatible = "fixed-factor-clock";
+			#clock-cells = <0>;
+			clock-div = <1>;
+			clock-mult = <1>;
+			clocks = <&ar100_div>;
+			clock-output-names = "ahb0";
+		};
+
+		apb0: apb0 {
+			compatible = "allwinner,sun6i-a31-apb0-clk";
+			#clock-cells = <0>;
+			clocks = <&ahb0>;
+			clock-output-names = "apb0";
+		};
+
+		apb0_gates: apb0_gates {
+			compatible = "allwinner,sun6i-a31-apb0-gates-clk";
+			#clock-cells = <1>;
+			clocks = <&apb0>;
+			clock-output-names = "apb0_pio", "apb0_ir",
+					"apb0_timer01", "apb0_p2wi",
+					"apb0_uart", "apb0_1wire",
+					"apb0_i2c";
+		};
+
+		apb0_rst: apb0_rst {
+			compatible = "allwinner,sun6i-a31-clock-reset";
+			#reset-cells = <1>;
+		};
+	};
-- 
1.8.3.2

Re: [PATCH 4/7] mfd: sun6i-prcm: document DT bindings

[Maxime Ripard]

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

On Mon, Apr 28, 2014 at 04:58:47PM +0200, Boris BREZILLON wrote:
> Document DT bindings of the PRCM (Power/Reset/Clock Management) unit.
> 
> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> ---
>  .../devicetree/bindings/mfd/sun6i-prcm.txt         | 71 ++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/sun6i-prcm.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/sun6i-prcm.txt b/Documentation/devicetree/bindings/mfd/sun6i-prcm.txt
> new file mode 100644
> index 0000000..de92429
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/sun6i-prcm.txt
> @@ -0,0 +1,71 @@
> +* Allwinner PRCM (Power/Reset/Clock Management) Multi-Functional Device
> +
> +PRCM is an MFD device exposing several Power Management related devices
> +(like clks and reset controllers).
> +
> +Required properties:
> + - compatible: "allwinner,sun6i-a31-prcm"
> + - reg: The PRCM registers range
> +
> +The prcm node may contain several subdevices definitions:
> + - see Documentation/devicetree/clk/sunxi.txt for clock devices
> + - see Documentation/devicetree/reset/allwinner,sunxi-clock-reset.txt for reset
> +   controller devices
> +
> +
> +Example:
> +
> +	prcm: prcm@01f01400 {
> +		compatible = "allwinner,sun6i-a31-prcm";
> +		reg = <0x01f01400>;

You're missing the size of the address range here.

Maxime

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

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

[PATCH 5/7] clk: sunxi: add PRCM (Power/Reset/Clock Management) clks support

[Boris BREZILLON]

The PRCM (Power/Reset/Clock Management) unit provides several clock
devices:
- AR100 clk: used to clock the Power Management co-processor
- AHB0 clk: used to clock the AHB0 bus
- APB0 clk and gates: used to clk

Add support for these clks in a separate driver so that they can be probed
as platform devices instead of registered during early init.
We need this to be able to probe PRCM MFD subdevices.

Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/clk/sunxi/Makefile         |   2 +
 drivers/clk/sunxi/clk-sun6i-prcm.c | 253 +++++++++++++++++++++++++++++++++++++
 2 files changed, 255 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk-sun6i-prcm.c

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index b5bac91..ef8cdc9 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -3,3 +3,5 @@
 #
 
 obj-y += clk-sunxi.o clk-factors.o
+
+obj-$(CONFIG_MFD_SUN6I_PRCM) += clk-sun6i-prcm.o
diff --git a/drivers/clk/sunxi/clk-sun6i-prcm.c b/drivers/clk/sunxi/clk-sun6i-prcm.c
new file mode 100644
index 0000000..bb7b25a
--- /dev/null
+++ b/drivers/clk/sunxi/clk-sun6i-prcm.c
@@ -0,0 +1,253 @@
+/*
+ * Copyright (C) 2014 Free Electrons
+ *
+ * License Terms: GNU General Public License v2
+ * Author: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ *
+ * Allwinner PRCM (Power/Reset/Clock Management) driver
+ *
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#define SUN6I_APB0_GATES_MAX_SIZE	32
+#define SUN6I_AR100_MAX_PARENTS		4
+
+static int sun6i_a31_ar100_mux_clk_register(struct platform_device *pdev)
+{
+	const char *parents[SUN6I_AR100_MAX_PARENTS];
+	struct device_node *np = pdev->dev.of_node;
+	const char *clk_name = np->name;
+	struct resource *r;
+	void __iomem *reg;
+	struct clk *clk;
+	int nparents;
+	int i;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	nparents = of_clk_get_parent_count(np);
+	if (nparents > SUN6I_AR100_MAX_PARENTS)
+		nparents = SUN6I_AR100_MAX_PARENTS;
+
+	for (i = 0; i < nparents; i++)
+		parents[i] = of_clk_get_parent_name(np, i);
+
+	of_property_read_string(np, "clock-output-names", &clk_name);
+
+	clk = clk_register_mux(&pdev->dev, clk_name, parents, nparents,
+			       CLK_SET_RATE_NO_REPARENT, reg,
+			       16, 2, 0, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	return of_clk_add_provider(np, of_clk_src_simple_get, clk);
+}
+
+static int sun6i_a31_ar100_clk_register(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	const char *clk_name = np->name;
+	const char *clk_parent;
+	struct resource *r;
+	void __iomem *reg;
+	struct clk *clk;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	clk_parent = of_clk_get_parent_name(np, 0);
+	if (!clk_parent)
+		return -EINVAL;
+
+	of_property_read_string(np, "clock-output-names", &clk_name);
+
+	clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
+				   0, reg, 4, 2, CLK_DIVIDER_POWER_OF_TWO,
+				   NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	return of_clk_add_provider(np, of_clk_src_simple_get, clk);
+}
+
+static int sun6i_a31_ar100_div_clk_register(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	const char *clk_name = np->name;
+	const char *clk_parent;
+	struct resource *r;
+	void __iomem *reg;
+	struct clk *clk;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	clk_parent = of_clk_get_parent_name(np, 0);
+	if (!clk_parent)
+		return -EINVAL;
+
+	of_property_read_string(np, "clock-output-names", &clk_name);
+
+	clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
+				   0, reg, 8, 5, 0, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	return of_clk_add_provider(np, of_clk_src_simple_get, clk);
+}
+
+static int sun6i_a31_apb0_clk_register(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	const char *clk_name = np->name;
+	const char *clk_parent;
+	struct resource *r;
+	void __iomem *reg;
+	struct clk *clk;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	reg = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	clk_parent = of_clk_get_parent_name(np, 0);
+	if (!clk_parent)
+		return -EINVAL;
+
+	of_property_read_string(np, "clock-output-names", &clk_name);
+
+	clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
+				   0, reg, 0, 2, CLK_DIVIDER_POWER_OF_TWO,
+				   NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	return of_clk_add_provider(np, of_clk_src_simple_get, clk);
+}
+
+static int sun6i_a31_apb0_gates_clk_register(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct clk_onecell_data *clk_data;
+	const char *clk_parent;
+	const char *clk_name;
+	struct resource *r;
+	void __iomem *reg;
+	int gate_id;
+	int ngates;
+	int i;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	reg = devm_ioremap_resource(&pdev->dev, r);
+	if (!reg)
+		return PTR_ERR(reg);
+
+	clk_parent = of_clk_get_parent_name(np, 0);
+	if (!clk_parent)
+		return -EINVAL;
+
+	ngates = of_property_count_strings(np, "clock-output-names");
+	if (ngates < 0)
+		return ngates;
+
+	if (!ngates || ngates > SUN6I_APB0_GATES_MAX_SIZE)
+		return -EINVAL;
+
+	clk_data = devm_kzalloc(&pdev->dev, sizeof(struct clk_onecell_data),
+				GFP_KERNEL);
+	if (!clk_data)
+		return -ENOMEM;
+
+	clk_data->clks = devm_kzalloc(&pdev->dev,
+				      SUN6I_APB0_GATES_MAX_SIZE *
+				      sizeof(struct clk *),
+				      GFP_KERNEL);
+	if (!clk_data->clks)
+		return -ENOMEM;
+
+	for (i = 0; i < ngates; i++) {
+		of_property_read_string_index(np, "clock-output-names",
+					      i, &clk_name);
+
+		gate_id = i;
+		of_property_read_u32_index(np, "clock-indices", i, &gate_id);
+
+		WARN_ON(gate_id >= SUN6I_APB0_GATES_MAX_SIZE);
+		if (gate_id >= SUN6I_APB0_GATES_MAX_SIZE)
+			continue;
+
+		clk_data->clks[gate_id] = clk_register_gate(&pdev->dev,
+							    clk_name,
+							    clk_parent, 0,
+							    reg, gate_id,
+							    0, NULL);
+		WARN_ON(IS_ERR(clk_data->clks[gate_id]));
+	}
+
+	clk_data->clk_num = ngates;
+
+	return of_clk_add_provider(np, of_clk_src_onecell_get, clk_data);
+}
+
+const struct of_device_id sun6i_a31_prcm_clk_dt_ids[] = {
+	{
+		.compatible = "allwinner,sun6i-a31-ar100-mux-clk",
+		.data = sun6i_a31_ar100_mux_clk_register,
+	},
+	{
+		.compatible = "allwinner,sun6i-a31-ar100-clk",
+		.data = sun6i_a31_ar100_clk_register,
+	},
+	{
+		.compatible = "allwinner,sun6i-a31-ar100-div-clk",
+		.data = sun6i_a31_ar100_div_clk_register,
+	},
+	{
+		.compatible = "allwinner,sun6i-a31-apb0-clk",
+		.data = sun6i_a31_apb0_clk_register,
+	},
+	{
+		.compatible = "allwinner,sun6i-a31-apb0-gates-clk",
+		.data = sun6i_a31_apb0_gates_clk_register,
+	},
+	{ /* sentinel */ }
+};
+
+static int sun6i_a31_prcm_clk_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	int (*register_func)(struct platform_device *pdev);
+	const struct of_device_id *match;
+
+	match = of_match_node(sun6i_a31_prcm_clk_dt_ids, np);
+	if (!match)
+		return -EINVAL;
+
+	register_func = match->data;
+	return register_func(pdev);
+}
+
+static struct platform_driver sun6i_a31_prcm_clk_driver = {
+	.driver = {
+		.name = "sun6i-a31-prcm-clk",
+		.owner = THIS_MODULE,
+		.of_match_table = sun6i_a31_prcm_clk_dt_ids,
+	},
+	.probe = sun6i_a31_prcm_clk_probe,
+};
+module_platform_driver(sun6i_a31_prcm_clk_driver);
+
+MODULE_AUTHOR("Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org");
+MODULE_DESCRIPTION("Allwinner Reset Controller Driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.3.2

Re: [PATCH 5/7] clk: sunxi: add PRCM (Power/Reset/Clock Management) clks support

[Emilio López]

Hi Boris,

El 28/04/14 11:58, Boris BREZILLON escribió:
> The PRCM (Power/Reset/Clock Management) unit provides several clock
> devices:
> - AR100 clk: used to clock the Power Management co-processor
> - AHB0 clk: used to clock the AHB0 bus
> - APB0 clk and gates: used to clk
>
> Add support for these clks in a separate driver so that they can be probed
> as platform devices instead of registered during early init.
> We need this to be able to probe PRCM MFD subdevices.
>
> Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
(..)
> +
> +MODULE_AUTHOR("Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org");

There's a missing >

> +MODULE_DESCRIPTION("Allwinner Reset Controller Driver");

Copy/paste leftover?

> +MODULE_LICENSE("GPL v2");

I just skimmed through this now, I'll have a more detailed look later. 
Thanks for working on sun6i support :)

Cheers,

Emilio

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 5/7] clk: sunxi: add PRCM (Power/Reset/Clock Management) clks support

[Boris BREZILLON]

On 28/04/2014 17:25, Emilio López wrote:
> Hi Boris,
>
> El 28/04/14 11:58, Boris BREZILLON escribió:
>> The PRCM (Power/Reset/Clock Management) unit provides several clock
>> devices:
>> - AR100 clk: used to clock the Power Management co-processor
>> - AHB0 clk: used to clock the AHB0 bus
>> - APB0 clk and gates: used to clk
>>
>> Add support for these clks in a separate driver so that they can be
>> probed
>> as platform devices instead of registered during early init.
>> We need this to be able to probe PRCM MFD subdevices.
>>
>> Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> ---
> (..)
>> +
>> +MODULE_AUTHOR("Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org");
>
> There's a missing >

>
>> +MODULE_DESCRIPTION("Allwinner Reset Controller Driver");
>
> Copy/paste leftover?

Thanks for catching those mistakes, I'll fix them.

Best Regards,

Boris
>
>
>> +MODULE_LICENSE("GPL v2");
>
> I just skimmed through this now, I'll have a more detailed look later.
> Thanks for working on sun6i support :)
>
> Cheers,
>
> Emilio

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 5/7] clk: sunxi: add PRCM (Power/Reset/Clock Management) clks support

[Chen-Yu Tsai]

Hi,

On Mon, Apr 28, 2014 at 10:58 PM, Boris BREZILLON
<boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> The PRCM (Power/Reset/Clock Management) unit provides several clock
> devices:
> - AR100 clk: used to clock the Power Management co-processor
> - AHB0 clk: used to clock the AHB0 bus
> - APB0 clk and gates: used to clk
>
> Add support for these clks in a separate driver so that they can be probed
> as platform devices instead of registered during early init.
> We need this to be able to probe PRCM MFD subdevices.
>
> Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  drivers/clk/sunxi/Makefile         |   2 +
>  drivers/clk/sunxi/clk-sun6i-prcm.c | 253 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 255 insertions(+)
>  create mode 100644 drivers/clk/sunxi/clk-sun6i-prcm.c
>
> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> index b5bac91..ef8cdc9 100644
> --- a/drivers/clk/sunxi/Makefile
> +++ b/drivers/clk/sunxi/Makefile
> @@ -3,3 +3,5 @@
>  #
>
>  obj-y += clk-sunxi.o clk-factors.o
> +
> +obj-$(CONFIG_MFD_SUN6I_PRCM) += clk-sun6i-prcm.o
> diff --git a/drivers/clk/sunxi/clk-sun6i-prcm.c b/drivers/clk/sunxi/clk-sun6i-prcm.c
> new file mode 100644
> index 0000000..bb7b25a
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-sun6i-prcm.c
> @@ -0,0 +1,253 @@
> +/*
> + * Copyright (C) 2014 Free Electrons
> + *
> + * License Terms: GNU General Public License v2
> + * Author: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + *
> + * Allwinner PRCM (Power/Reset/Clock Management) driver
> + *
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#define SUN6I_APB0_GATES_MAX_SIZE      32
> +#define SUN6I_AR100_MAX_PARENTS                4
> +
> +static int sun6i_a31_ar100_mux_clk_register(struct platform_device *pdev)
> +{
> +       const char *parents[SUN6I_AR100_MAX_PARENTS];
> +       struct device_node *np = pdev->dev.of_node;
> +       const char *clk_name = np->name;
> +       struct resource *r;
> +       void __iomem *reg;
> +       struct clk *clk;
> +       int nparents;
> +       int i;
> +
> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
> +       if (IS_ERR(reg))
> +               return PTR_ERR(reg);
> +
> +       nparents = of_clk_get_parent_count(np);
> +       if (nparents > SUN6I_AR100_MAX_PARENTS)
> +               nparents = SUN6I_AR100_MAX_PARENTS;
> +
> +       for (i = 0; i < nparents; i++)
> +               parents[i] = of_clk_get_parent_name(np, i);
> +
> +       of_property_read_string(np, "clock-output-names", &clk_name);
> +
> +       clk = clk_register_mux(&pdev->dev, clk_name, parents, nparents,
> +                              CLK_SET_RATE_NO_REPARENT, reg,
> +                              16, 2, 0, NULL);
> +       if (IS_ERR(clk))
> +               return PTR_ERR(clk);
> +
> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +}
> +
> +static int sun6i_a31_ar100_clk_register(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       const char *clk_name = np->name;
> +       const char *clk_parent;
> +       struct resource *r;
> +       void __iomem *reg;
> +       struct clk *clk;
> +
> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
> +       if (IS_ERR(reg))
> +               return PTR_ERR(reg);
> +
> +       clk_parent = of_clk_get_parent_name(np, 0);
> +       if (!clk_parent)
> +               return -EINVAL;
> +
> +       of_property_read_string(np, "clock-output-names", &clk_name);
> +
> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
> +                                  0, reg, 4, 2, CLK_DIVIDER_POWER_OF_TWO,
> +                                  NULL);
> +       if (IS_ERR(clk))
> +               return PTR_ERR(clk);
> +
> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +}
> +
> +static int sun6i_a31_ar100_div_clk_register(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       const char *clk_name = np->name;
> +       const char *clk_parent;
> +       struct resource *r;
> +       void __iomem *reg;
> +       struct clk *clk;
> +
> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
> +       if (IS_ERR(reg))
> +               return PTR_ERR(reg);
> +
> +       clk_parent = of_clk_get_parent_name(np, 0);
> +       if (!clk_parent)
> +               return -EINVAL;
> +
> +       of_property_read_string(np, "clock-output-names", &clk_name);
> +
> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
> +                                  0, reg, 8, 5, 0, NULL);
> +       if (IS_ERR(clk))
> +               return PTR_ERR(clk);
> +
> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +}

Would it be possible to merge the 3 ar100 clocks into 1 composite clock?
They do share the same register, and are related to each other.

It might be possible to re-use some code from the sunxi clock driver
for this, though I'm not sure if that's a good idea, sharing code
between modules. Emilio?

> +
> +static int sun6i_a31_apb0_clk_register(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       const char *clk_name = np->name;
> +       const char *clk_parent;
> +       struct resource *r;
> +       void __iomem *reg;
> +       struct clk *clk;
> +
> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       reg = devm_ioremap_resource(&pdev->dev, r);
> +       if (IS_ERR(reg))
> +               return PTR_ERR(reg);
> +
> +       clk_parent = of_clk_get_parent_name(np, 0);
> +       if (!clk_parent)
> +               return -EINVAL;
> +
> +       of_property_read_string(np, "clock-output-names", &clk_name);
> +
> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
> +                                  0, reg, 0, 2, CLK_DIVIDER_POWER_OF_TWO,
> +                                  NULL);

I just looked at the sun6i kernel code again.

  http://git.rhombus-tech.net/?p=linux.git;a=blob;f=arch/arm/mach-sun6i/clock/ccm_i.h;hb=refs/heads/allwinner-sunxi-a31#l376

and

  http://git.rhombus-tech.net/?p=linux.git;a=blob;f=arch/arm/mach-sun6i/clock/sys_clk.c;hb=refs/heads/allwinner-sunxi-a31#l882

The divider on the A31 is /2, /2, /4, /8. You might need a table for that.
This is different from the A23 manual I used to document most of the PRCM.
I apologize for not catching this earlier. I have updated the wiki.

> +       if (IS_ERR(clk))
> +               return PTR_ERR(clk);
> +
> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +}
> +
> +static int sun6i_a31_apb0_gates_clk_register(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       struct clk_onecell_data *clk_data;
> +       const char *clk_parent;
> +       const char *clk_name;
> +       struct resource *r;
> +       void __iomem *reg;
> +       int gate_id;
> +       int ngates;
> +       int i;
> +
> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       reg = devm_ioremap_resource(&pdev->dev, r);
> +       if (!reg)
> +               return PTR_ERR(reg);
> +
> +       clk_parent = of_clk_get_parent_name(np, 0);
> +       if (!clk_parent)
> +               return -EINVAL;
> +
> +       ngates = of_property_count_strings(np, "clock-output-names");
> +       if (ngates < 0)
> +               return ngates;
> +
> +       if (!ngates || ngates > SUN6I_APB0_GATES_MAX_SIZE)
> +               return -EINVAL;
> +
> +       clk_data = devm_kzalloc(&pdev->dev, sizeof(struct clk_onecell_data),
> +                               GFP_KERNEL);
> +       if (!clk_data)
> +               return -ENOMEM;
> +
> +       clk_data->clks = devm_kzalloc(&pdev->dev,
> +                                     SUN6I_APB0_GATES_MAX_SIZE *
> +                                     sizeof(struct clk *),
> +                                     GFP_KERNEL);
> +       if (!clk_data->clks)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < ngates; i++) {
> +               of_property_read_string_index(np, "clock-output-names",
> +                                             i, &clk_name);
> +
> +               gate_id = i;
> +               of_property_read_u32_index(np, "clock-indices", i, &gate_id);
> +
> +               WARN_ON(gate_id >= SUN6I_APB0_GATES_MAX_SIZE);
> +               if (gate_id >= SUN6I_APB0_GATES_MAX_SIZE)
> +                       continue;
> +
> +               clk_data->clks[gate_id] = clk_register_gate(&pdev->dev,
> +                                                           clk_name,
> +                                                           clk_parent, 0,
> +                                                           reg, gate_id,
> +                                                           0, NULL);
> +               WARN_ON(IS_ERR(clk_data->clks[gate_id]));
> +       }
> +
> +       clk_data->clk_num = ngates;
> +
> +       return of_clk_add_provider(np, of_clk_src_onecell_get, clk_data);
> +}
> +
> +const struct of_device_id sun6i_a31_prcm_clk_dt_ids[] = {
> +       {
> +               .compatible = "allwinner,sun6i-a31-ar100-mux-clk",
> +               .data = sun6i_a31_ar100_mux_clk_register,
> +       },
> +       {
> +               .compatible = "allwinner,sun6i-a31-ar100-clk",
> +               .data = sun6i_a31_ar100_clk_register,
> +       },
> +       {
> +               .compatible = "allwinner,sun6i-a31-ar100-div-clk",
> +               .data = sun6i_a31_ar100_div_clk_register,
> +       },
> +       {
> +               .compatible = "allwinner,sun6i-a31-apb0-clk",
> +               .data = sun6i_a31_apb0_clk_register,
> +       },
> +       {
> +               .compatible = "allwinner,sun6i-a31-apb0-gates-clk",
> +               .data = sun6i_a31_apb0_gates_clk_register,
> +       },
> +       { /* sentinel */ }
> +};
> +
> +static int sun6i_a31_prcm_clk_probe(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       int (*register_func)(struct platform_device *pdev);
> +       const struct of_device_id *match;
> +
> +       match = of_match_node(sun6i_a31_prcm_clk_dt_ids, np);
> +       if (!match)
> +               return -EINVAL;
> +
> +       register_func = match->data;
> +       return register_func(pdev);
> +}
> +
> +static struct platform_driver sun6i_a31_prcm_clk_driver = {
> +       .driver = {
> +               .name = "sun6i-a31-prcm-clk",
> +               .owner = THIS_MODULE,
> +               .of_match_table = sun6i_a31_prcm_clk_dt_ids,
> +       },
> +       .probe = sun6i_a31_prcm_clk_probe,
> +};
> +module_platform_driver(sun6i_a31_prcm_clk_driver);
> +
> +MODULE_AUTHOR("Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org");
> +MODULE_DESCRIPTION("Allwinner Reset Controller Driver");
> +MODULE_LICENSE("GPL v2");

The rest looks good to me, but Emilio should be able to give you more feedback.

Thanks for all the sun6i support you've done!


Cheers
ChenYu

Re: [PATCH 5/7] clk: sunxi: add PRCM (Power/Reset/Clock Management) clks support

[Boris BREZILLON]

Hi Chen-Yu,

On 28/04/2014 17:59, Chen-Yu Tsai wrote:
> Hi,
>
> On Mon, Apr 28, 2014 at 10:58 PM, Boris BREZILLON
> <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> The PRCM (Power/Reset/Clock Management) unit provides several clock
>> devices:
>> - AR100 clk: used to clock the Power Management co-processor
>> - AHB0 clk: used to clock the AHB0 bus
>> - APB0 clk and gates: used to clk
>>
>> Add support for these clks in a separate driver so that they can be probed
>> as platform devices instead of registered during early init.
>> We need this to be able to probe PRCM MFD subdevices.
>>
>> Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> ---
>>  drivers/clk/sunxi/Makefile         |   2 +
>>  drivers/clk/sunxi/clk-sun6i-prcm.c | 253 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 255 insertions(+)
>>  create mode 100644 drivers/clk/sunxi/clk-sun6i-prcm.c
>>
>> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
>> index b5bac91..ef8cdc9 100644
>> --- a/drivers/clk/sunxi/Makefile
>> +++ b/drivers/clk/sunxi/Makefile
>> @@ -3,3 +3,5 @@
>>  #
>>
>>  obj-y += clk-sunxi.o clk-factors.o
>> +
>> +obj-$(CONFIG_MFD_SUN6I_PRCM) += clk-sun6i-prcm.o
>> diff --git a/drivers/clk/sunxi/clk-sun6i-prcm.c b/drivers/clk/sunxi/clk-sun6i-prcm.c
>> new file mode 100644
>> index 0000000..bb7b25a
>> --- /dev/null
>> +++ b/drivers/clk/sunxi/clk-sun6i-prcm.c
>> @@ -0,0 +1,253 @@
>> +/*
>> + * Copyright (C) 2014 Free Electrons
>> + *
>> + * License Terms: GNU General Public License v2
>> + * Author: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> + *
>> + * Allwinner PRCM (Power/Reset/Clock Management) driver
>> + *
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define SUN6I_APB0_GATES_MAX_SIZE      32
>> +#define SUN6I_AR100_MAX_PARENTS                4
>> +
>> +static int sun6i_a31_ar100_mux_clk_register(struct platform_device *pdev)
>> +{
>> +       const char *parents[SUN6I_AR100_MAX_PARENTS];
>> +       struct device_node *np = pdev->dev.of_node;
>> +       const char *clk_name = np->name;
>> +       struct resource *r;
>> +       void __iomem *reg;
>> +       struct clk *clk;
>> +       int nparents;
>> +       int i;
>> +
>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>> +       if (IS_ERR(reg))
>> +               return PTR_ERR(reg);
>> +
>> +       nparents = of_clk_get_parent_count(np);
>> +       if (nparents > SUN6I_AR100_MAX_PARENTS)
>> +               nparents = SUN6I_AR100_MAX_PARENTS;
>> +
>> +       for (i = 0; i < nparents; i++)
>> +               parents[i] = of_clk_get_parent_name(np, i);
>> +
>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>> +
>> +       clk = clk_register_mux(&pdev->dev, clk_name, parents, nparents,
>> +                              CLK_SET_RATE_NO_REPARENT, reg,
>> +                              16, 2, 0, NULL);
>> +       if (IS_ERR(clk))
>> +               return PTR_ERR(clk);
>> +
>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>> +}
>> +
>> +static int sun6i_a31_ar100_clk_register(struct platform_device *pdev)
>> +{
>> +       struct device_node *np = pdev->dev.of_node;
>> +       const char *clk_name = np->name;
>> +       const char *clk_parent;
>> +       struct resource *r;
>> +       void __iomem *reg;
>> +       struct clk *clk;
>> +
>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>> +       if (IS_ERR(reg))
>> +               return PTR_ERR(reg);
>> +
>> +       clk_parent = of_clk_get_parent_name(np, 0);
>> +       if (!clk_parent)
>> +               return -EINVAL;
>> +
>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>> +
>> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
>> +                                  0, reg, 4, 2, CLK_DIVIDER_POWER_OF_TWO,
>> +                                  NULL);
>> +       if (IS_ERR(clk))
>> +               return PTR_ERR(clk);
>> +
>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>> +}
>> +
>> +static int sun6i_a31_ar100_div_clk_register(struct platform_device *pdev)
>> +{
>> +       struct device_node *np = pdev->dev.of_node;
>> +       const char *clk_name = np->name;
>> +       const char *clk_parent;
>> +       struct resource *r;
>> +       void __iomem *reg;
>> +       struct clk *clk;
>> +
>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>> +       if (IS_ERR(reg))
>> +               return PTR_ERR(reg);
>> +
>> +       clk_parent = of_clk_get_parent_name(np, 0);
>> +       if (!clk_parent)
>> +               return -EINVAL;
>> +
>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>> +
>> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
>> +                                  0, reg, 8, 5, 0, NULL);
>> +       if (IS_ERR(clk))
>> +               return PTR_ERR(clk);
>> +
>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>> +}
> Would it be possible to merge the 3 ar100 clocks into 1 composite clock?
> They do share the same register, and are related to each other.

Okay, I'll take a look at composite clocks.

> It might be possible to re-use some code from the sunxi clock driver
> for this, though I'm not sure if that's a good idea, sharing code
> between modules. Emilio?

Actually, this is what I tried in the first place, but then I realized
it would require exporting some functions declared as static in clk-sunxi.c.
Moreover, these functions do not make use of devm functions (because all
the clks declared in clk-sunxi.c are initiliazed during early), and I'd
like to uses devm function when clks are declared as platform devices.

Anyway, I'll wait for Emilio's review ;-).

>
>> +
>> +static int sun6i_a31_apb0_clk_register(struct platform_device *pdev)
>> +{
>> +       struct device_node *np = pdev->dev.of_node;
>> +       const char *clk_name = np->name;
>> +       const char *clk_parent;
>> +       struct resource *r;
>> +       void __iomem *reg;
>> +       struct clk *clk;
>> +
>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       reg = devm_ioremap_resource(&pdev->dev, r);
>> +       if (IS_ERR(reg))
>> +               return PTR_ERR(reg);
>> +
>> +       clk_parent = of_clk_get_parent_name(np, 0);
>> +       if (!clk_parent)
>> +               return -EINVAL;
>> +
>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>> +
>> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
>> +                                  0, reg, 0, 2, CLK_DIVIDER_POWER_OF_TWO,
>> +                                  NULL);
> I just looked at the sun6i kernel code again.
>
>   http://git.rhombus-tech.net/?p=linux.git;a=blob;f=arch/arm/mach-sun6i/clock/ccm_i.h;hb=refs/heads/allwinner-sunxi-a31#l376
>
> and
>
>   http://git.rhombus-tech.net/?p=linux.git;a=blob;f=arch/arm/mach-sun6i/clock/sys_clk.c;hb=refs/heads/allwinner-sunxi-a31#l882
>
> The divider on the A31 is /2, /2, /4, /8. You might need a table for that.

I saw that in the datasheet, but I just thought it was a mistake (that's
weird to have 2 times the same divisor).

> This is different from the A23 manual I used to document most of the PRCM.
> I apologize for not catching this earlier. I have updated the wiki.

No problem.

>
>> +       if (IS_ERR(clk))
>> +               return PTR_ERR(clk);
>> +
>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>> +}
>> +
>> +static int sun6i_a31_apb0_gates_clk_register(struct platform_device *pdev)
>> +{
>> +       struct device_node *np = pdev->dev.of_node;
>> +       struct clk_onecell_data *clk_data;
>> +       const char *clk_parent;
>> +       const char *clk_name;
>> +       struct resource *r;
>> +       void __iomem *reg;
>> +       int gate_id;
>> +       int ngates;
>> +       int i;
>> +
>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       reg = devm_ioremap_resource(&pdev->dev, r);
>> +       if (!reg)
>> +               return PTR_ERR(reg);
>> +
>> +       clk_parent = of_clk_get_parent_name(np, 0);
>> +       if (!clk_parent)
>> +               return -EINVAL;
>> +
>> +       ngates = of_property_count_strings(np, "clock-output-names");
>> +       if (ngates < 0)
>> +               return ngates;
>> +
>> +       if (!ngates || ngates > SUN6I_APB0_GATES_MAX_SIZE)
>> +               return -EINVAL;
>> +
>> +       clk_data = devm_kzalloc(&pdev->dev, sizeof(struct clk_onecell_data),
>> +                               GFP_KERNEL);
>> +       if (!clk_data)
>> +               return -ENOMEM;
>> +
>> +       clk_data->clks = devm_kzalloc(&pdev->dev,
>> +                                     SUN6I_APB0_GATES_MAX_SIZE *
>> +                                     sizeof(struct clk *),
>> +                                     GFP_KERNEL);
>> +       if (!clk_data->clks)
>> +               return -ENOMEM;
>> +
>> +       for (i = 0; i < ngates; i++) {
>> +               of_property_read_string_index(np, "clock-output-names",
>> +                                             i, &clk_name);
>> +
>> +               gate_id = i;
>> +               of_property_read_u32_index(np, "clock-indices", i, &gate_id);
>> +
>> +               WARN_ON(gate_id >= SUN6I_APB0_GATES_MAX_SIZE);
>> +               if (gate_id >= SUN6I_APB0_GATES_MAX_SIZE)
>> +                       continue;
>> +
>> +               clk_data->clks[gate_id] = clk_register_gate(&pdev->dev,
>> +                                                           clk_name,
>> +                                                           clk_parent, 0,
>> +                                                           reg, gate_id,
>> +                                                           0, NULL);
>> +               WARN_ON(IS_ERR(clk_data->clks[gate_id]));
>> +       }
>> +
>> +       clk_data->clk_num = ngates;
>> +
>> +       return of_clk_add_provider(np, of_clk_src_onecell_get, clk_data);
>> +}
>> +
>> +const struct of_device_id sun6i_a31_prcm_clk_dt_ids[] = {
>> +       {
>> +               .compatible = "allwinner,sun6i-a31-ar100-mux-clk",
>> +               .data = sun6i_a31_ar100_mux_clk_register,
>> +       },
>> +       {
>> +               .compatible = "allwinner,sun6i-a31-ar100-clk",
>> +               .data = sun6i_a31_ar100_clk_register,
>> +       },
>> +       {
>> +               .compatible = "allwinner,sun6i-a31-ar100-div-clk",
>> +               .data = sun6i_a31_ar100_div_clk_register,
>> +       },
>> +       {
>> +               .compatible = "allwinner,sun6i-a31-apb0-clk",
>> +               .data = sun6i_a31_apb0_clk_register,
>> +       },
>> +       {
>> +               .compatible = "allwinner,sun6i-a31-apb0-gates-clk",
>> +               .data = sun6i_a31_apb0_gates_clk_register,
>> +       },
>> +       { /* sentinel */ }
>> +};
>> +
>> +static int sun6i_a31_prcm_clk_probe(struct platform_device *pdev)
>> +{
>> +       struct device_node *np = pdev->dev.of_node;
>> +       int (*register_func)(struct platform_device *pdev);
>> +       const struct of_device_id *match;
>> +
>> +       match = of_match_node(sun6i_a31_prcm_clk_dt_ids, np);
>> +       if (!match)
>> +               return -EINVAL;
>> +
>> +       register_func = match->data;
>> +       return register_func(pdev);
>> +}
>> +
>> +static struct platform_driver sun6i_a31_prcm_clk_driver = {
>> +       .driver = {
>> +               .name = "sun6i-a31-prcm-clk",
>> +               .owner = THIS_MODULE,
>> +               .of_match_table = sun6i_a31_prcm_clk_dt_ids,
>> +       },
>> +       .probe = sun6i_a31_prcm_clk_probe,
>> +};
>> +module_platform_driver(sun6i_a31_prcm_clk_driver);
>> +
>> +MODULE_AUTHOR("Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org");
>> +MODULE_DESCRIPTION("Allwinner Reset Controller Driver");
>> +MODULE_LICENSE("GPL v2");
> The rest looks good to me, but Emilio should be able to give you more feedback.
>
> Thanks for all the sun6i support you've done!

Thanks for your review.

Best Regards,

Boris
>
> Cheers
> ChenYu

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 5/7] clk: sunxi: add PRCM (Power/Reset/Clock Management) clks support

[Chen-Yu Tsai]

On Tue, Apr 29, 2014 at 1:14 AM, Boris BREZILLON
<boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hi Chen-Yu,
>
> On 28/04/2014 17:59, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Mon, Apr 28, 2014 at 10:58 PM, Boris BREZILLON
>> <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>> The PRCM (Power/Reset/Clock Management) unit provides several clock
>>> devices:
>>> - AR100 clk: used to clock the Power Management co-processor
>>> - AHB0 clk: used to clock the AHB0 bus
>>> - APB0 clk and gates: used to clk
>>>
>>> Add support for these clks in a separate driver so that they can be probed
>>> as platform devices instead of registered during early init.
>>> We need this to be able to probe PRCM MFD subdevices.
>>>
>>> Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>> ---
>>>  drivers/clk/sunxi/Makefile         |   2 +
>>>  drivers/clk/sunxi/clk-sun6i-prcm.c | 253 +++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 255 insertions(+)
>>>  create mode 100644 drivers/clk/sunxi/clk-sun6i-prcm.c
>>>
>>> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
>>> index b5bac91..ef8cdc9 100644
>>> --- a/drivers/clk/sunxi/Makefile
>>> +++ b/drivers/clk/sunxi/Makefile
>>> @@ -3,3 +3,5 @@
>>>  #
>>>
>>>  obj-y += clk-sunxi.o clk-factors.o
>>> +
>>> +obj-$(CONFIG_MFD_SUN6I_PRCM) += clk-sun6i-prcm.o
>>> diff --git a/drivers/clk/sunxi/clk-sun6i-prcm.c b/drivers/clk/sunxi/clk-sun6i-prcm.c
>>> new file mode 100644
>>> index 0000000..bb7b25a
>>> --- /dev/null
>>> +++ b/drivers/clk/sunxi/clk-sun6i-prcm.c
>>> @@ -0,0 +1,253 @@
>>> +/*
>>> + * Copyright (C) 2014 Free Electrons
>>> + *
>>> + * License Terms: GNU General Public License v2
>>> + * Author: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>> + *
>>> + * Allwinner PRCM (Power/Reset/Clock Management) driver
>>> + *
>>> + */
>>> +
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#define SUN6I_APB0_GATES_MAX_SIZE      32
>>> +#define SUN6I_AR100_MAX_PARENTS                4
>>> +
>>> +static int sun6i_a31_ar100_mux_clk_register(struct platform_device *pdev)
>>> +{
>>> +       const char *parents[SUN6I_AR100_MAX_PARENTS];
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +       const char *clk_name = np->name;
>>> +       struct resource *r;
>>> +       void __iomem *reg;
>>> +       struct clk *clk;
>>> +       int nparents;
>>> +       int i;
>>> +
>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>>> +       if (IS_ERR(reg))
>>> +               return PTR_ERR(reg);
>>> +
>>> +       nparents = of_clk_get_parent_count(np);
>>> +       if (nparents > SUN6I_AR100_MAX_PARENTS)
>>> +               nparents = SUN6I_AR100_MAX_PARENTS;
>>> +
>>> +       for (i = 0; i < nparents; i++)
>>> +               parents[i] = of_clk_get_parent_name(np, i);
>>> +
>>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>>> +
>>> +       clk = clk_register_mux(&pdev->dev, clk_name, parents, nparents,
>>> +                              CLK_SET_RATE_NO_REPARENT, reg,
>>> +                              16, 2, 0, NULL);
>>> +       if (IS_ERR(clk))
>>> +               return PTR_ERR(clk);
>>> +
>>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>>> +}
>>> +
>>> +static int sun6i_a31_ar100_clk_register(struct platform_device *pdev)
>>> +{
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +       const char *clk_name = np->name;
>>> +       const char *clk_parent;
>>> +       struct resource *r;
>>> +       void __iomem *reg;
>>> +       struct clk *clk;
>>> +
>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>>> +       if (IS_ERR(reg))
>>> +               return PTR_ERR(reg);
>>> +
>>> +       clk_parent = of_clk_get_parent_name(np, 0);
>>> +       if (!clk_parent)
>>> +               return -EINVAL;
>>> +
>>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>>> +
>>> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
>>> +                                  0, reg, 4, 2, CLK_DIVIDER_POWER_OF_TWO,
>>> +                                  NULL);
>>> +       if (IS_ERR(clk))
>>> +               return PTR_ERR(clk);
>>> +
>>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>>> +}
>>> +
>>> +static int sun6i_a31_ar100_div_clk_register(struct platform_device *pdev)
>>> +{
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +       const char *clk_name = np->name;
>>> +       const char *clk_parent;
>>> +       struct resource *r;
>>> +       void __iomem *reg;
>>> +       struct clk *clk;
>>> +
>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>>> +       if (IS_ERR(reg))
>>> +               return PTR_ERR(reg);
>>> +
>>> +       clk_parent = of_clk_get_parent_name(np, 0);
>>> +       if (!clk_parent)
>>> +               return -EINVAL;
>>> +
>>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>>> +
>>> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
>>> +                                  0, reg, 8, 5, 0, NULL);
>>> +       if (IS_ERR(clk))
>>> +               return PTR_ERR(clk);
>>> +
>>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>>> +}
>> Would it be possible to merge the 3 ar100 clocks into 1 composite clock?
>> They do share the same register, and are related to each other.
>
> Okay, I'll take a look at composite clocks.
>
>> It might be possible to re-use some code from the sunxi clock driver
>> for this, though I'm not sure if that's a good idea, sharing code
>> between modules. Emilio?
>
> Actually, this is what I tried in the first place, but then I realized
> it would require exporting some functions declared as static in clk-sunxi.c.
> Moreover, these functions do not make use of devm functions (because all
> the clks declared in clk-sunxi.c are initiliazed during early), and I'd
> like to uses devm function when clks are declared as platform devices.

Yeah, that's why I was uncertain about it.

> Anyway, I'll wait for Emilio's review ;-).
>
>>
>>> +
>>> +static int sun6i_a31_apb0_clk_register(struct platform_device *pdev)
>>> +{
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +       const char *clk_name = np->name;
>>> +       const char *clk_parent;
>>> +       struct resource *r;
>>> +       void __iomem *reg;
>>> +       struct clk *clk;
>>> +
>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +       reg = devm_ioremap_resource(&pdev->dev, r);
>>> +       if (IS_ERR(reg))
>>> +               return PTR_ERR(reg);
>>> +
>>> +       clk_parent = of_clk_get_parent_name(np, 0);
>>> +       if (!clk_parent)
>>> +               return -EINVAL;
>>> +
>>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>>> +
>>> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
>>> +                                  0, reg, 0, 2, CLK_DIVIDER_POWER_OF_TWO,
>>> +                                  NULL);
>> I just looked at the sun6i kernel code again.
>>
>>   http://git.rhombus-tech.net/?p=linux.git;a=blob;f=arch/arm/mach-sun6i/clock/ccm_i.h;hb=refs/heads/allwinner-sunxi-a31#l376
>>
>> and
>>
>>   http://git.rhombus-tech.net/?p=linux.git;a=blob;f=arch/arm/mach-sun6i/clock/sys_clk.c;hb=refs/heads/allwinner-sunxi-a31#l882
>>
>> The divider on the A31 is /2, /2, /4, /8. You might need a table for that.
>
> I saw that in the datasheet, but I just thought it was a mistake (that's
> weird to have 2 times the same divisor).

Oh, you have the datasheet?

We also have muxes with 2 PLL6's. And Allwinner's u-boot p2wi driver also
assumes 12 MHz, instead of 24 MHz, as the source clock rate. Either they
messed up twice, or they had some reason to do it like this. The p2wi
driver in u-boot-sunxi also assumes 12 MHz, and I assume it works for
everyone with A31 hardware.

>> This is different from the A23 manual I used to document most of the PRCM.
>> I apologize for not catching this earlier. I have updated the wiki.
>
> No problem.
>
>>
>>> +       if (IS_ERR(clk))
>>> +               return PTR_ERR(clk);
>>> +
>>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>>> +}
>>> +
>>> +static int sun6i_a31_apb0_gates_clk_register(struct platform_device *pdev)
>>> +{
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +       struct clk_onecell_data *clk_data;
>>> +       const char *clk_parent;
>>> +       const char *clk_name;
>>> +       struct resource *r;
>>> +       void __iomem *reg;
>>> +       int gate_id;
>>> +       int ngates;
>>> +       int i;
>>> +
>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +       reg = devm_ioremap_resource(&pdev->dev, r);
>>> +       if (!reg)
>>> +               return PTR_ERR(reg);
>>> +
>>> +       clk_parent = of_clk_get_parent_name(np, 0);
>>> +       if (!clk_parent)
>>> +               return -EINVAL;
>>> +
>>> +       ngates = of_property_count_strings(np, "clock-output-names");
>>> +       if (ngates < 0)
>>> +               return ngates;
>>> +
>>> +       if (!ngates || ngates > SUN6I_APB0_GATES_MAX_SIZE)
>>> +               return -EINVAL;
>>> +
>>> +       clk_data = devm_kzalloc(&pdev->dev, sizeof(struct clk_onecell_data),
>>> +                               GFP_KERNEL);
>>> +       if (!clk_data)
>>> +               return -ENOMEM;
>>> +
>>> +       clk_data->clks = devm_kzalloc(&pdev->dev,
>>> +                                     SUN6I_APB0_GATES_MAX_SIZE *
>>> +                                     sizeof(struct clk *),
>>> +                                     GFP_KERNEL);
>>> +       if (!clk_data->clks)
>>> +               return -ENOMEM;
>>> +
>>> +       for (i = 0; i < ngates; i++) {
>>> +               of_property_read_string_index(np, "clock-output-names",
>>> +                                             i, &clk_name);
>>> +
>>> +               gate_id = i;
>>> +               of_property_read_u32_index(np, "clock-indices", i, &gate_id);
>>> +
>>> +               WARN_ON(gate_id >= SUN6I_APB0_GATES_MAX_SIZE);
>>> +               if (gate_id >= SUN6I_APB0_GATES_MAX_SIZE)
>>> +                       continue;
>>> +
>>> +               clk_data->clks[gate_id] = clk_register_gate(&pdev->dev,
>>> +                                                           clk_name,
>>> +                                                           clk_parent, 0,
>>> +                                                           reg, gate_id,
>>> +                                                           0, NULL);
>>> +               WARN_ON(IS_ERR(clk_data->clks[gate_id]));
>>> +       }
>>> +
>>> +       clk_data->clk_num = ngates;
>>> +
>>> +       return of_clk_add_provider(np, of_clk_src_onecell_get, clk_data);
>>> +}
>>> +
>>> +const struct of_device_id sun6i_a31_prcm_clk_dt_ids[] = {
>>> +       {
>>> +               .compatible = "allwinner,sun6i-a31-ar100-mux-clk",
>>> +               .data = sun6i_a31_ar100_mux_clk_register,
>>> +       },
>>> +       {
>>> +               .compatible = "allwinner,sun6i-a31-ar100-clk",
>>> +               .data = sun6i_a31_ar100_clk_register,
>>> +       },
>>> +       {
>>> +               .compatible = "allwinner,sun6i-a31-ar100-div-clk",
>>> +               .data = sun6i_a31_ar100_div_clk_register,
>>> +       },
>>> +       {
>>> +               .compatible = "allwinner,sun6i-a31-apb0-clk",
>>> +               .data = sun6i_a31_apb0_clk_register,
>>> +       },
>>> +       {
>>> +               .compatible = "allwinner,sun6i-a31-apb0-gates-clk",
>>> +               .data = sun6i_a31_apb0_gates_clk_register,
>>> +       },
>>> +       { /* sentinel */ }
>>> +};
>>> +
>>> +static int sun6i_a31_prcm_clk_probe(struct platform_device *pdev)
>>> +{
>>> +       struct device_node *np = pdev->dev.of_node;
>>> +       int (*register_func)(struct platform_device *pdev);
>>> +       const struct of_device_id *match;
>>> +
>>> +       match = of_match_node(sun6i_a31_prcm_clk_dt_ids, np);
>>> +       if (!match)
>>> +               return -EINVAL;
>>> +
>>> +       register_func = match->data;
>>> +       return register_func(pdev);
>>> +}
>>> +
>>> +static struct platform_driver sun6i_a31_prcm_clk_driver = {
>>> +       .driver = {
>>> +               .name = "sun6i-a31-prcm-clk",
>>> +               .owner = THIS_MODULE,
>>> +               .of_match_table = sun6i_a31_prcm_clk_dt_ids,
>>> +       },
>>> +       .probe = sun6i_a31_prcm_clk_probe,
>>> +};
>>> +module_platform_driver(sun6i_a31_prcm_clk_driver);
>>> +
>>> +MODULE_AUTHOR("Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org");
>>> +MODULE_DESCRIPTION("Allwinner Reset Controller Driver");
>>> +MODULE_LICENSE("GPL v2");
>> The rest looks good to me, but Emilio should be able to give you more feedback.
>>
>> Thanks for all the sun6i support you've done!
>
> Thanks for your review.
>
> Best Regards,
>
> Boris
>>
>> Cheers
>> ChenYu
>
> --
> Boris Brezillon, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
>

Re: [PATCH 5/7] clk: sunxi: add PRCM (Power/Reset/Clock Management) clks support

[Boris BREZILLON]

On 28/04/2014 20:03, Chen-Yu Tsai wrote:
> On Tue, Apr 29, 2014 at 1:14 AM, Boris BREZILLON
> <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> Hi Chen-Yu,
>>
>> On 28/04/2014 17:59, Chen-Yu Tsai wrote:
>>> Hi,
>>>
>>> On Mon, Apr 28, 2014 at 10:58 PM, Boris BREZILLON
>>> <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>>> The PRCM (Power/Reset/Clock Management) unit provides several clock
>>>> devices:
>>>> - AR100 clk: used to clock the Power Management co-processor
>>>> - AHB0 clk: used to clock the AHB0 bus
>>>> - APB0 clk and gates: used to clk
>>>>
>>>> Add support for these clks in a separate driver so that they can be probed
>>>> as platform devices instead of registered during early init.
>>>> We need this to be able to probe PRCM MFD subdevices.
>>>>
>>>> Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>> ---
>>>>  drivers/clk/sunxi/Makefile         |   2 +
>>>>  drivers/clk/sunxi/clk-sun6i-prcm.c | 253 +++++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 255 insertions(+)
>>>>  create mode 100644 drivers/clk/sunxi/clk-sun6i-prcm.c
>>>>
>>>> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
>>>> index b5bac91..ef8cdc9 100644
>>>> --- a/drivers/clk/sunxi/Makefile
>>>> +++ b/drivers/clk/sunxi/Makefile
>>>> @@ -3,3 +3,5 @@
>>>>  #
>>>>
>>>>  obj-y += clk-sunxi.o clk-factors.o
>>>> +
>>>> +obj-$(CONFIG_MFD_SUN6I_PRCM) += clk-sun6i-prcm.o
>>>> diff --git a/drivers/clk/sunxi/clk-sun6i-prcm.c b/drivers/clk/sunxi/clk-sun6i-prcm.c
>>>> new file mode 100644
>>>> index 0000000..bb7b25a
>>>> --- /dev/null
>>>> +++ b/drivers/clk/sunxi/clk-sun6i-prcm.c
>>>> @@ -0,0 +1,253 @@
>>>> +/*
>>>> + * Copyright (C) 2014 Free Electrons
>>>> + *
>>>> + * License Terms: GNU General Public License v2
>>>> + * Author: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>> + *
>>>> + * Allwinner PRCM (Power/Reset/Clock Management) driver
>>>> + *
>>>> + */
>>>> +
>>>> +#include <linux/clk-provider.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/platform_device.h>
>>>> +
>>>> +#define SUN6I_APB0_GATES_MAX_SIZE      32
>>>> +#define SUN6I_AR100_MAX_PARENTS                4
>>>> +
>>>> +static int sun6i_a31_ar100_mux_clk_register(struct platform_device *pdev)
>>>> +{
>>>> +       const char *parents[SUN6I_AR100_MAX_PARENTS];
>>>> +       struct device_node *np = pdev->dev.of_node;
>>>> +       const char *clk_name = np->name;
>>>> +       struct resource *r;
>>>> +       void __iomem *reg;
>>>> +       struct clk *clk;
>>>> +       int nparents;
>>>> +       int i;
>>>> +
>>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>>>> +       if (IS_ERR(reg))
>>>> +               return PTR_ERR(reg);
>>>> +
>>>> +       nparents = of_clk_get_parent_count(np);
>>>> +       if (nparents > SUN6I_AR100_MAX_PARENTS)
>>>> +               nparents = SUN6I_AR100_MAX_PARENTS;
>>>> +
>>>> +       for (i = 0; i < nparents; i++)
>>>> +               parents[i] = of_clk_get_parent_name(np, i);
>>>> +
>>>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>>>> +
>>>> +       clk = clk_register_mux(&pdev->dev, clk_name, parents, nparents,
>>>> +                              CLK_SET_RATE_NO_REPARENT, reg,
>>>> +                              16, 2, 0, NULL);
>>>> +       if (IS_ERR(clk))
>>>> +               return PTR_ERR(clk);
>>>> +
>>>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>>>> +}
>>>> +
>>>> +static int sun6i_a31_ar100_clk_register(struct platform_device *pdev)
>>>> +{
>>>> +       struct device_node *np = pdev->dev.of_node;
>>>> +       const char *clk_name = np->name;
>>>> +       const char *clk_parent;
>>>> +       struct resource *r;
>>>> +       void __iomem *reg;
>>>> +       struct clk *clk;
>>>> +
>>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>>>> +       if (IS_ERR(reg))
>>>> +               return PTR_ERR(reg);
>>>> +
>>>> +       clk_parent = of_clk_get_parent_name(np, 0);
>>>> +       if (!clk_parent)
>>>> +               return -EINVAL;
>>>> +
>>>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>>>> +
>>>> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
>>>> +                                  0, reg, 4, 2, CLK_DIVIDER_POWER_OF_TWO,
>>>> +                                  NULL);
>>>> +       if (IS_ERR(clk))
>>>> +               return PTR_ERR(clk);
>>>> +
>>>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>>>> +}
>>>> +
>>>> +static int sun6i_a31_ar100_div_clk_register(struct platform_device *pdev)
>>>> +{
>>>> +       struct device_node *np = pdev->dev.of_node;
>>>> +       const char *clk_name = np->name;
>>>> +       const char *clk_parent;
>>>> +       struct resource *r;
>>>> +       void __iomem *reg;
>>>> +       struct clk *clk;
>>>> +
>>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +       reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>>>> +       if (IS_ERR(reg))
>>>> +               return PTR_ERR(reg);
>>>> +
>>>> +       clk_parent = of_clk_get_parent_name(np, 0);
>>>> +       if (!clk_parent)
>>>> +               return -EINVAL;
>>>> +
>>>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>>>> +
>>>> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
>>>> +                                  0, reg, 8, 5, 0, NULL);
>>>> +       if (IS_ERR(clk))
>>>> +               return PTR_ERR(clk);
>>>> +
>>>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>>>> +}
>>> Would it be possible to merge the 3 ar100 clocks into 1 composite clock?
>>> They do share the same register, and are related to each other.
>> Okay, I'll take a look at composite clocks.
>>
>>> It might be possible to re-use some code from the sunxi clock driver
>>> for this, though I'm not sure if that's a good idea, sharing code
>>> between modules. Emilio?
>> Actually, this is what I tried in the first place, but then I realized
>> it would require exporting some functions declared as static in clk-sunxi.c.
>> Moreover, these functions do not make use of devm functions (because all
>> the clks declared in clk-sunxi.c are initiliazed during early), and I'd
>> like to uses devm function when clks are declared as platform devices.
> Yeah, that's why I was uncertain about it.
>
>> Anyway, I'll wait for Emilio's review ;-).
>>
>>>> +
>>>> +static int sun6i_a31_apb0_clk_register(struct platform_device *pdev)
>>>> +{
>>>> +       struct device_node *np = pdev->dev.of_node;
>>>> +       const char *clk_name = np->name;
>>>> +       const char *clk_parent;
>>>> +       struct resource *r;
>>>> +       void __iomem *reg;
>>>> +       struct clk *clk;
>>>> +
>>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +       reg = devm_ioremap_resource(&pdev->dev, r);
>>>> +       if (IS_ERR(reg))
>>>> +               return PTR_ERR(reg);
>>>> +
>>>> +       clk_parent = of_clk_get_parent_name(np, 0);
>>>> +       if (!clk_parent)
>>>> +               return -EINVAL;
>>>> +
>>>> +       of_property_read_string(np, "clock-output-names", &clk_name);
>>>> +
>>>> +       clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
>>>> +                                  0, reg, 0, 2, CLK_DIVIDER_POWER_OF_TWO,
>>>> +                                  NULL);
>>> I just looked at the sun6i kernel code again.
>>>
>>>   http://git.rhombus-tech.net/?p=linux.git;a=blob;f=arch/arm/mach-sun6i/clock/ccm_i.h;hb=refs/heads/allwinner-sunxi-a31#l376
>>>
>>> and
>>>
>>>   http://git.rhombus-tech.net/?p=linux.git;a=blob;f=arch/arm/mach-sun6i/clock/sys_clk.c;hb=refs/heads/allwinner-sunxi-a31#l882
>>>
>>> The divider on the A31 is /2, /2, /4, /8. You might need a table for that.
>> I saw that in the datasheet, but I just thought it was a mistake (that's
>> weird to have 2 times the same divisor).
> Oh, you have the datasheet?

No, I said datasheet, but I meant the code you point out last time
(otherwise I would have no problem to do the clk tree implementation) :-(

>
> We also have muxes with 2 PLL6's. And Allwinner's u-boot p2wi driver also
> assumes 12 MHz, instead of 24 MHz, as the source clock rate. Either they
> messed up twice, or they had some reason to do it like this. The p2wi
> driver in u-boot-sunxi also assumes 12 MHz, and I assume it works for
> everyone with A31 hardware.

Okay, I'll take a look at the output signal (on the P2WI clk pin) and
see if it matches your assumptions ;-).

>
>>> This is different from the A23 manual I used to document most of the PRCM.
>>> I apologize for not catching this earlier. I have updated the wiki.
>> No problem.
>>
>>>> +       if (IS_ERR(clk))
>>>> +               return PTR_ERR(clk);
>>>> +
>>>> +       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>>>> +}
>>>> +
>>>> +static int sun6i_a31_apb0_gates_clk_register(struct platform_device *pdev)
>>>> +{
>>>> +       struct device_node *np = pdev->dev.of_node;
>>>> +       struct clk_onecell_data *clk_data;
>>>> +       const char *clk_parent;
>>>> +       const char *clk_name;
>>>> +       struct resource *r;
>>>> +       void __iomem *reg;
>>>> +       int gate_id;
>>>> +       int ngates;
>>>> +       int i;
>>>> +
>>>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +       reg = devm_ioremap_resource(&pdev->dev, r);
>>>> +       if (!reg)
>>>> +               return PTR_ERR(reg);
>>>> +
>>>> +       clk_parent = of_clk_get_parent_name(np, 0);
>>>> +       if (!clk_parent)
>>>> +               return -EINVAL;
>>>> +
>>>> +       ngates = of_property_count_strings(np, "clock-output-names");
>>>> +       if (ngates < 0)
>>>> +               return ngates;
>>>> +
>>>> +       if (!ngates || ngates > SUN6I_APB0_GATES_MAX_SIZE)
>>>> +               return -EINVAL;
>>>> +
>>>> +       clk_data = devm_kzalloc(&pdev->dev, sizeof(struct clk_onecell_data),
>>>> +                               GFP_KERNEL);
>>>> +       if (!clk_data)
>>>> +               return -ENOMEM;
>>>> +
>>>> +       clk_data->clks = devm_kzalloc(&pdev->dev,
>>>> +                                     SUN6I_APB0_GATES_MAX_SIZE *
>>>> +                                     sizeof(struct clk *),
>>>> +                                     GFP_KERNEL);
>>>> +       if (!clk_data->clks)
>>>> +               return -ENOMEM;
>>>> +
>>>> +       for (i = 0; i < ngates; i++) {
>>>> +               of_property_read_string_index(np, "clock-output-names",
>>>> +                                             i, &clk_name);
>>>> +
>>>> +               gate_id = i;
>>>> +               of_property_read_u32_index(np, "clock-indices", i, &gate_id);
>>>> +
>>>> +               WARN_ON(gate_id >= SUN6I_APB0_GATES_MAX_SIZE);
>>>> +               if (gate_id >= SUN6I_APB0_GATES_MAX_SIZE)
>>>> +                       continue;
>>>> +
>>>> +               clk_data->clks[gate_id] = clk_register_gate(&pdev->dev,
>>>> +                                                           clk_name,
>>>> +                                                           clk_parent, 0,
>>>> +                                                           reg, gate_id,
>>>> +                                                           0, NULL);
>>>> +               WARN_ON(IS_ERR(clk_data->clks[gate_id]));
>>>> +       }
>>>> +
>>>> +       clk_data->clk_num = ngates;
>>>> +
>>>> +       return of_clk_add_provider(np, of_clk_src_onecell_get, clk_data);
>>>> +}
>>>> +
>>>> +const struct of_device_id sun6i_a31_prcm_clk_dt_ids[] = {
>>>> +       {
>>>> +               .compatible = "allwinner,sun6i-a31-ar100-mux-clk",
>>>> +               .data = sun6i_a31_ar100_mux_clk_register,
>>>> +       },
>>>> +       {
>>>> +               .compatible = "allwinner,sun6i-a31-ar100-clk",
>>>> +               .data = sun6i_a31_ar100_clk_register,
>>>> +       },
>>>> +       {
>>>> +               .compatible = "allwinner,sun6i-a31-ar100-div-clk",
>>>> +               .data = sun6i_a31_ar100_div_clk_register,
>>>> +       },
>>>> +       {
>>>> +               .compatible = "allwinner,sun6i-a31-apb0-clk",
>>>> +               .data = sun6i_a31_apb0_clk_register,
>>>> +       },
>>>> +       {
>>>> +               .compatible = "allwinner,sun6i-a31-apb0-gates-clk",
>>>> +               .data = sun6i_a31_apb0_gates_clk_register,
>>>> +       },
>>>> +       { /* sentinel */ }
>>>> +};
>>>> +
>>>> +static int sun6i_a31_prcm_clk_probe(struct platform_device *pdev)
>>>> +{
>>>> +       struct device_node *np = pdev->dev.of_node;
>>>> +       int (*register_func)(struct platform_device *pdev);
>>>> +       const struct of_device_id *match;
>>>> +
>>>> +       match = of_match_node(sun6i_a31_prcm_clk_dt_ids, np);
>>>> +       if (!match)
>>>> +               return -EINVAL;
>>>> +
>>>> +       register_func = match->data;
>>>> +       return register_func(pdev);
>>>> +}
>>>> +
>>>> +static struct platform_driver sun6i_a31_prcm_clk_driver = {
>>>> +       .driver = {
>>>> +               .name = "sun6i-a31-prcm-clk",
>>>> +               .owner = THIS_MODULE,
>>>> +               .of_match_table = sun6i_a31_prcm_clk_dt_ids,
>>>> +       },
>>>> +       .probe = sun6i_a31_prcm_clk_probe,
>>>> +};
>>>> +module_platform_driver(sun6i_a31_prcm_clk_driver);
>>>> +
>>>> +MODULE_AUTHOR("Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org");
>>>> +MODULE_DESCRIPTION("Allwinner Reset Controller Driver");
>>>> +MODULE_LICENSE("GPL v2");
>>> The rest looks good to me, but Emilio should be able to give you more feedback.
>>>
>>> Thanks for all the sun6i support you've done!
>> Thanks for your review.
>>
>> Best Regards,
>>
>> Boris
>>> Cheers
>>> ChenYu
>> --
>> Boris Brezillon, Free Electrons
>> Embedded Linux and Kernel engineering
>> http://free-electrons.com
>>

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 5/7] clk: sunxi: add PRCM (Power/Reset/Clock Management) clks support

[Maxime Ripard]

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

On Mon, Apr 28, 2014 at 04:58:48PM +0200, Boris BREZILLON wrote:
> The PRCM (Power/Reset/Clock Management) unit provides several clock
> devices:
> - AR100 clk: used to clock the Power Management co-processor
> - AHB0 clk: used to clock the AHB0 bus
> - APB0 clk and gates: used to clk

Used to clk?

> 
> Add support for these clks in a separate driver so that they can be probed
> as platform devices instead of registered during early init.
> We need this to be able to probe PRCM MFD subdevices.
> 
> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> ---
>  drivers/clk/sunxi/Makefile         |   2 +
>  drivers/clk/sunxi/clk-sun6i-prcm.c | 253 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 255 insertions(+)
>  create mode 100644 drivers/clk/sunxi/clk-sun6i-prcm.c
> 
> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> index b5bac91..ef8cdc9 100644
> --- a/drivers/clk/sunxi/Makefile
> +++ b/drivers/clk/sunxi/Makefile
> @@ -3,3 +3,5 @@
>  #
>  
>  obj-y += clk-sunxi.o clk-factors.o
> +
> +obj-$(CONFIG_MFD_SUN6I_PRCM) += clk-sun6i-prcm.o
> diff --git a/drivers/clk/sunxi/clk-sun6i-prcm.c b/drivers/clk/sunxi/clk-sun6i-prcm.c
> new file mode 100644
> index 0000000..bb7b25a
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-sun6i-prcm.c
> @@ -0,0 +1,253 @@
> +/*
> + * Copyright (C) 2014 Free Electrons
> + *
> + * License Terms: GNU General Public License v2
> + * Author: Boris BREZILLON <boris.brezillon@free-electrons.com>
> + *
> + * Allwinner PRCM (Power/Reset/Clock Management) driver
> + *
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#define SUN6I_APB0_GATES_MAX_SIZE	32
> +#define SUN6I_AR100_MAX_PARENTS		4
> +
> +static int sun6i_a31_ar100_mux_clk_register(struct platform_device *pdev)
> +{
> +	const char *parents[SUN6I_AR100_MAX_PARENTS];
> +	struct device_node *np = pdev->dev.of_node;
> +	const char *clk_name = np->name;
> +	struct resource *r;
> +	void __iomem *reg;
> +	struct clk *clk;
> +	int nparents;
> +	int i;
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
> +	if (IS_ERR(reg))
> +		return PTR_ERR(reg);

devm_ioremap returns NULL on error, and not an error pointer.

> +
> +	nparents = of_clk_get_parent_count(np);
> +	if (nparents > SUN6I_AR100_MAX_PARENTS)
> +		nparents = SUN6I_AR100_MAX_PARENTS;
> +
> +	for (i = 0; i < nparents; i++)
> +		parents[i] = of_clk_get_parent_name(np, i);
> +
> +	of_property_read_string(np, "clock-output-names", &clk_name);
> +
> +	clk = clk_register_mux(&pdev->dev, clk_name, parents, nparents,
> +			       CLK_SET_RATE_NO_REPARENT, reg,
> +			       16, 2, 0, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	return of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +}
> +
> +static int sun6i_a31_ar100_clk_register(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	const char *clk_name = np->name;
> +	const char *clk_parent;
> +	struct resource *r;
> +	void __iomem *reg;
> +	struct clk *clk;
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
> +	if (IS_ERR(reg))
> +		return PTR_ERR(reg);

Ditto.

And you'll probably want to use devm_ioremap_resource when you'll have
a single clock for the AR100.

> +
> +	clk_parent = of_clk_get_parent_name(np, 0);
> +	if (!clk_parent)
> +		return -EINVAL;
> +
> +	of_property_read_string(np, "clock-output-names", &clk_name);
> +
> +	clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
> +				   0, reg, 4, 2, CLK_DIVIDER_POWER_OF_TWO,
> +				   NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	return of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +}
> +
> +static int sun6i_a31_ar100_div_clk_register(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	const char *clk_name = np->name;
> +	const char *clk_parent;
> +	struct resource *r;
> +	void __iomem *reg;
> +	struct clk *clk;
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	reg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
> +	if (IS_ERR(reg))
> +		return PTR_ERR(reg);
> +
> +	clk_parent = of_clk_get_parent_name(np, 0);
> +	if (!clk_parent)
> +		return -EINVAL;
> +
> +	of_property_read_string(np, "clock-output-names", &clk_name);
> +
> +	clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
> +				   0, reg, 8, 5, 0, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	return of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +}
> +
> +static int sun6i_a31_apb0_clk_register(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	const char *clk_name = np->name;
> +	const char *clk_parent;
> +	struct resource *r;
> +	void __iomem *reg;
> +	struct clk *clk;
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	reg = devm_ioremap_resource(&pdev->dev, r);
> +	if (IS_ERR(reg))
> +		return PTR_ERR(reg);
> +
> +	clk_parent = of_clk_get_parent_name(np, 0);
> +	if (!clk_parent)
> +		return -EINVAL;
> +
> +	of_property_read_string(np, "clock-output-names", &clk_name);
> +
> +	clk = clk_register_divider(&pdev->dev, clk_name, clk_parent,
> +				   0, reg, 0, 2, CLK_DIVIDER_POWER_OF_TWO,
> +				   NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	return of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +}
> +
> +static int sun6i_a31_apb0_gates_clk_register(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct clk_onecell_data *clk_data;
> +	const char *clk_parent;
> +	const char *clk_name;
> +	struct resource *r;
> +	void __iomem *reg;
> +	int gate_id;
> +	int ngates;
> +	int i;
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	reg = devm_ioremap_resource(&pdev->dev, r);
> +	if (!reg)
> +		return PTR_ERR(reg);
> +
> +	clk_parent = of_clk_get_parent_name(np, 0);
> +	if (!clk_parent)
> +		return -EINVAL;
> +
> +	ngates = of_property_count_strings(np, "clock-output-names");
> +	if (ngates < 0)
> +		return ngates;
> +
> +	if (!ngates || ngates > SUN6I_APB0_GATES_MAX_SIZE)
> +		return -EINVAL;
> +
> +	clk_data = devm_kzalloc(&pdev->dev, sizeof(struct clk_onecell_data),
> +				GFP_KERNEL);
> +	if (!clk_data)
> +		return -ENOMEM;
> +
> +	clk_data->clks = devm_kzalloc(&pdev->dev,
> +				      SUN6I_APB0_GATES_MAX_SIZE *
> +				      sizeof(struct clk *),
> +				      GFP_KERNEL);
> +	if (!clk_data->clks)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < ngates; i++) {
> +		of_property_read_string_index(np, "clock-output-names",
> +					      i, &clk_name);
> +
> +		gate_id = i;
> +		of_property_read_u32_index(np, "clock-indices", i, &gate_id);
> +
> +		WARN_ON(gate_id >= SUN6I_APB0_GATES_MAX_SIZE);
> +		if (gate_id >= SUN6I_APB0_GATES_MAX_SIZE)
> +			continue;
> +
> +		clk_data->clks[gate_id] = clk_register_gate(&pdev->dev,
> +							    clk_name,
> +							    clk_parent, 0,
> +							    reg, gate_id,
> +							    0, NULL);
> +		WARN_ON(IS_ERR(clk_data->clks[gate_id]));
> +	}
> +
> +	clk_data->clk_num = ngates;
> +
> +	return of_clk_add_provider(np, of_clk_src_onecell_get, clk_data);
> +}
> +
> +const struct of_device_id sun6i_a31_prcm_clk_dt_ids[] = {
> +	{
> +		.compatible = "allwinner,sun6i-a31-ar100-mux-clk",
> +		.data = sun6i_a31_ar100_mux_clk_register,
> +	},
> +	{
> +		.compatible = "allwinner,sun6i-a31-ar100-clk",
> +		.data = sun6i_a31_ar100_clk_register,
> +	},
> +	{
> +		.compatible = "allwinner,sun6i-a31-ar100-div-clk",
> +		.data = sun6i_a31_ar100_div_clk_register,
> +	},
> +	{
> +		.compatible = "allwinner,sun6i-a31-apb0-clk",
> +		.data = sun6i_a31_apb0_clk_register,
> +	},
> +	{
> +		.compatible = "allwinner,sun6i-a31-apb0-gates-clk",
> +		.data = sun6i_a31_apb0_gates_clk_register,
> +	},
> +	{ /* sentinel */ }
> +};
> +
> +static int sun6i_a31_prcm_clk_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	int (*register_func)(struct platform_device *pdev);
> +	const struct of_device_id *match;
> +
> +	match = of_match_node(sun6i_a31_prcm_clk_dt_ids, np);
> +	if (!match)
> +		return -EINVAL;
> +
> +	register_func = match->data;
> +	return register_func(pdev);
> +}
> +
> +static struct platform_driver sun6i_a31_prcm_clk_driver = {
> +	.driver = {
> +		.name = "sun6i-a31-prcm-clk",
> +		.owner = THIS_MODULE,
> +		.of_match_table = sun6i_a31_prcm_clk_dt_ids,
> +	},
> +	.probe = sun6i_a31_prcm_clk_probe,

You're not calling the of_clk_del_provider, and you should probably
unregister your clocks too.

Maxime

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

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

Re: [PATCH 5/7] clk: sunxi: add PRCM (Power/Reset/Clock Management) clks support

[Boris BREZILLON]

On 29/04/2014 01:40, Maxime Ripard wrote:
> On Mon, Apr 28, 2014 at 04:58:48PM +0200, Boris BREZILLON wrote:
>> The PRCM (Power/Reset/Clock Management) unit provides several clock
>> devices:
>> - AR100 clk: used to clock the Power Management co-processor
>> - AHB0 clk: used to clock the AHB0 bus
>> - APB0 clk and gates: used to clk
> Used to clk?
"Used to clk peripherals connected on the APB0 bus"

I'll add the missing words in the next version :-).

>
[...]
> Ditto.
>
> And you'll probably want to use devm_ioremap_resource when you'll have
> a single clock for the AR100.

Absolutely.

>
>> +
>> +	clk_parent = of_clk_get_parent_name(np, 0);
>> +	if (!clk_parent)
>> +		return -EINVAL;
[...]
>> +
>> +static struct platform_driver sun6i_a31_prcm_clk_driver = {
>> +	.driver = {
>> +		.name = "sun6i-a31-prcm-clk",
>> +		.owner = THIS_MODULE,
>> +		.of_match_table = sun6i_a31_prcm_clk_dt_ids,
>> +	},
>> +	.probe = sun6i_a31_prcm_clk_probe,
> You're not calling the of_clk_del_provider, and you should probably
> unregister your clocks too.

This driver cannot be compiled as a module, and as a result the probed
clks will never be removed.

Do you really want to support clk removal for this HW block ?

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 5/7] clk: sunxi: add PRCM (Power/Reset/Clock Management) clks support

[Maxime Ripard]

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

On Wed, May 07, 2014 at 07:12:30PM +0200, Boris BREZILLON wrote:
> 
> On 29/04/2014 01:40, Maxime Ripard wrote:
> > On Mon, Apr 28, 2014 at 04:58:48PM +0200, Boris BREZILLON wrote:
> >> The PRCM (Power/Reset/Clock Management) unit provides several clock
> >> devices:
> >> - AR100 clk: used to clock the Power Management co-processor
> >> - AHB0 clk: used to clock the AHB0 bus
> >> - APB0 clk and gates: used to clk
> > Used to clk?
> "Used to clk peripherals connected on the APB0 bus"
> 
> I'll add the missing words in the next version :-).
> 
> >
> [...]
> > Ditto.
> >
> > And you'll probably want to use devm_ioremap_resource when you'll have
> > a single clock for the AR100.
> 
> Absolutely.
> 
> >
> >> +
> >> +	clk_parent = of_clk_get_parent_name(np, 0);
> >> +	if (!clk_parent)
> >> +		return -EINVAL;
> [...]
> >> +
> >> +static struct platform_driver sun6i_a31_prcm_clk_driver = {
> >> +	.driver = {
> >> +		.name = "sun6i-a31-prcm-clk",
> >> +		.owner = THIS_MODULE,
> >> +		.of_match_table = sun6i_a31_prcm_clk_dt_ids,
> >> +	},
> >> +	.probe = sun6i_a31_prcm_clk_probe,
> > You're not calling the of_clk_del_provider, and you should probably
> > unregister your clocks too.
> 
> This driver cannot be compiled as a module, and as a result the probed
> clks will never be removed.
> 
> Do you really want to support clk removal for this HW block ?

Hmm, no, then it's fine.

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: 836 bytes --]

[PATCH 6/7] clk: sunxi: document PRCM clock compatible strings

[Boris BREZILLON]

Document new compatible strings for clock provided by the PRCM
(Power/Reset/Clock Management) unit.

Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 Documentation/devicetree/bindings/clock/sunxi.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
index a5160d8..0c9dd6a 100644
--- a/Documentation/devicetree/bindings/clock/sunxi.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi.txt
@@ -20,12 +20,18 @@ Required properties:
 	"allwinner,sun5i-a13-ahb-gates-clk" - for the AHB gates on A13
 	"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-mux-clk" - for the AR100 multiplexer on A31
+	"allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
+	"allwinner,sun6i-a31-ar100-div-clk" - for the AR100 divided output clk on A31
+	"allwinner,sun6i-a31-ahb0-clk" - for the AHB0 multiplexer on A31
 	"allwinner,sun6i-a31-ahb1-mux-clk" - for the AHB1 multiplexer on A31
 	"allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
 	"allwinner,sun4i-a10-apb0-clk" - for the APB0 clock
+	"allwinner,sun6i-a31-apb0-clk" - for the APB0 clock on A31
 	"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
+	"allwinner,sun6i-a31-apb0-gates-clk" - for the APB0 gates on A31
 	"allwinner,sun7i-a20-apb0-gates-clk" - for the APB0 gates on A20
 	"allwinner,sun4i-a10-apb1-clk" - for the APB1 clock
 	"allwinner,sun4i-a10-apb1-mux-clk" - for the APB1 clock muxing
-- 
1.8.3.2

[PATCH 7/7] ARM: sunxi: dt: add PRCM clk and reset controller subdevices

[Boris BREZILLON]

Add DT definitions for PRCM (Power/Reset/Clock Management) clock and reset
controller subdevices.

Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/sun6i-a31.dtsi | 49 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index ec3253a..83a1634 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -501,6 +501,55 @@
 		prcm@01f01c00 {
 			compatible = "allwinner,sun6i-a31-prcm";
 			reg = <0x01f01400 0x200>;
+
+			ar100_mux: ar100_mux {
+				compatible = "allwinner,sun6i-a31-ar100-mux-clk";
+				#clock-cells = <0>;
+				clocks = <&osc32k>, <&osc24M>, <&pll6>, <&pll6>;
+			};
+
+			ar100: ar100 {
+				compatible = "allwinner,sun6i-a31-ar100-clk";
+				#clock-cells = <0>;
+				clocks = <&ar100_mux>;
+			};
+
+			ar100_div: ar100_div {
+				compatible = "allwinner,sun6i-a31-ar100-div-clk";
+				#clock-cells = <0>;
+				clocks = <&ar100>;
+			};
+
+			ahb0: ahb0 {
+				compatible = "fixed-factor-clock";
+				#clock-cells = <0>;
+				clock-div = <1>;
+				clock-mult = <1>;
+				clocks = <&ar100_div>;
+				clock-output-names = "ahb0";
+			};
+
+			apb0: apb0 {
+				compatible = "allwinner,sun6i-a31-apb0-clk";
+				#clock-cells = <0>;
+				clocks = <&ahb0>;
+				clock-output-names = "apb0";
+			};
+
+			apb0_gates: apb0_gates {
+				compatible = "allwinner,sun6i-a31-apb0-gates-clk";
+				#clock-cells = <1>;
+				clocks = <&apb0>;
+				clock-output-names = "apb0_pio", "apb0_ir",
+						"apb0_timer01", "apb0_p2wi",
+						"apb0_uart", "apb0_1wire",
+						"apb0_i2c";
+			};
+
+			apb0_rst: apb0_rst {
+				compatible = "allwinner,sun6i-a31-clock-reset";
+				#reset-cells = <1>;
+			};
 		};
 	};
 };
-- 
1.8.3.2

Re: [PATCH 7/7] ARM: sunxi: dt: add PRCM clk and reset controller subdevices

[Chen-Yu Tsai]

Hi,

On Mon, Apr 28, 2014 at 10:58 PM, Boris BREZILLON
<boris.brezillon@free-electrons.com> wrote:
> Add DT definitions for PRCM (Power/Reset/Clock Management) clock and reset
> controller subdevices.
>
> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> ---
>  arch/arm/boot/dts/sun6i-a31.dtsi | 49 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
> index ec3253a..83a1634 100644
> --- a/arch/arm/boot/dts/sun6i-a31.dtsi
> +++ b/arch/arm/boot/dts/sun6i-a31.dtsi
> @@ -501,6 +501,55 @@
>                 prcm@01f01c00 {

Seems the address here was wrong to start with.

>                         compatible = "allwinner,sun6i-a31-prcm";
>                         reg = <0x01f01400 0x200>;
> +
> +                       ar100_mux: ar100_mux {

Might we use clk@01f01XXX for the names of the clock nodes?

> +                               compatible = "allwinner,sun6i-a31-ar100-mux-clk";
> +                               #clock-cells = <0>;
> +                               clocks = <&osc32k>, <&osc24M>, <&pll6>, <&pll6>;
> +                       };
> +
> +                       ar100: ar100 {
> +                               compatible = "allwinner,sun6i-a31-ar100-clk";
> +                               #clock-cells = <0>;
> +                               clocks = <&ar100_mux>;
> +                       };
> +
> +                       ar100_div: ar100_div {
> +                               compatible = "allwinner,sun6i-a31-ar100-div-clk";
> +                               #clock-cells = <0>;
> +                               clocks = <&ar100>;
> +                       };
> +
> +                       ahb0: ahb0 {
> +                               compatible = "fixed-factor-clock";
> +                               #clock-cells = <0>;
> +                               clock-div = <1>;
> +                               clock-mult = <1>;
> +                               clocks = <&ar100_div>;
> +                               clock-output-names = "ahb0";
> +                       };
> +
> +                       apb0: apb0 {
> +                               compatible = "allwinner,sun6i-a31-apb0-clk";
> +                               #clock-cells = <0>;
> +                               clocks = <&ahb0>;
> +                               clock-output-names = "apb0";
> +                       };
> +
> +                       apb0_gates: apb0_gates {
> +                               compatible = "allwinner,sun6i-a31-apb0-gates-clk";
> +                               #clock-cells = <1>;
> +                               clocks = <&apb0>;
> +                               clock-output-names = "apb0_pio", "apb0_ir",
> +                                               "apb0_timer01", "apb0_p2wi",
> +                                               "apb0_uart", "apb0_1wire",
> +                                               "apb0_i2c";
> +                       };
> +
> +                       apb0_rst: apb0_rst {

Also use reset@01f01XXX here?

> +                               compatible = "allwinner,sun6i-a31-clock-reset";
> +                               #reset-cells = <1>;
> +                       };
>                 };
>         };
>  };

Thanks!

ChenYu

Re: [PATCH 7/7] ARM: sunxi: dt: add PRCM clk and reset controller subdevices

[Boris BREZILLON]

On 28/04/2014 18:02, Chen-Yu Tsai wrote:
> Hi,
>
> On Mon, Apr 28, 2014 at 10:58 PM, Boris BREZILLON
> <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> Add DT definitions for PRCM (Power/Reset/Clock Management) clock and reset
>> controller subdevices.
>>
>> Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> ---
>>  arch/arm/boot/dts/sun6i-a31.dtsi | 49 ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
>> index ec3253a..83a1634 100644
>> --- a/arch/arm/boot/dts/sun6i-a31.dtsi
>> +++ b/arch/arm/boot/dts/sun6i-a31.dtsi
>> @@ -501,6 +501,55 @@
>>                 prcm@01f01c00 {
> Seems the address here was wrong to start with.

Absolutely, I'll fix it.

>
>>                         compatible = "allwinner,sun6i-a31-prcm";
>>                         reg = <0x01f01400 0x200>;
>> +
>> +                       ar100_mux: ar100_mux {
> Might we use clk@01f01XXX for the names of the clock nodes?

Actually, I had a discussion with Maxime, and we decided to remove the
address suffix because the PRCM block is not a bus, and thus should not
have child node with addresses.
But I'm not a DT binding expert (it might be acceptable to define child
nodes with addresses even when the parent is not a bus :-)).
Advices from DT maintainers on that specific point would be great.

Best Regards,

Boris

>> +                               compatible = "allwinner,sun6i-a31-ar100-mux-clk";
>> +                               #clock-cells = <0>;
>> +                               clocks = <&osc32k>, <&osc24M>, <&pll6>, <&pll6>;
>> +                       };
>> +
>> +                       ar100: ar100 {
>> +                               compatible = "allwinner,sun6i-a31-ar100-clk";
>> +                               #clock-cells = <0>;
>> +                               clocks = <&ar100_mux>;
>> +                       };
>> +
>> +                       ar100_div: ar100_div {
>> +                               compatible = "allwinner,sun6i-a31-ar100-div-clk";
>> +                               #clock-cells = <0>;
>> +                               clocks = <&ar100>;
>> +                       };
>> +
>> +                       ahb0: ahb0 {
>> +                               compatible = "fixed-factor-clock";
>> +                               #clock-cells = <0>;
>> +                               clock-div = <1>;
>> +                               clock-mult = <1>;
>> +                               clocks = <&ar100_div>;
>> +                               clock-output-names = "ahb0";
>> +                       };
>> +
>> +                       apb0: apb0 {
>> +                               compatible = "allwinner,sun6i-a31-apb0-clk";
>> +                               #clock-cells = <0>;
>> +                               clocks = <&ahb0>;
>> +                               clock-output-names = "apb0";
>> +                       };
>> +
>> +                       apb0_gates: apb0_gates {
>> +                               compatible = "allwinner,sun6i-a31-apb0-gates-clk";
>> +                               #clock-cells = <1>;
>> +                               clocks = <&apb0>;
>> +                               clock-output-names = "apb0_pio", "apb0_ir",
>> +                                               "apb0_timer01", "apb0_p2wi",
>> +                                               "apb0_uart", "apb0_1wire",
>> +                                               "apb0_i2c";
>> +                       };
>> +
>> +                       apb0_rst: apb0_rst {
> Also use reset@01f01XXX here?
>
>> +                               compatible = "allwinner,sun6i-a31-clock-reset";
>> +                               #reset-cells = <1>;
>> +                       };
>>                 };
>>         };
>>  };
> Thanks!
>
> ChenYu

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 7/7] ARM: sunxi: dt: add PRCM clk and reset controller subdevices

[Chen-Yu Tsai]

On Tue, Apr 29, 2014 at 1:27 AM, Boris BREZILLON
<boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>
> On 28/04/2014 18:02, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Mon, Apr 28, 2014 at 10:58 PM, Boris BREZILLON
>> <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>> Add DT definitions for PRCM (Power/Reset/Clock Management) clock and reset
>>> controller subdevices.
>>>
>>> Signed-off-by: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>> ---
>>>  arch/arm/boot/dts/sun6i-a31.dtsi | 49 ++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 49 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
>>> index ec3253a..83a1634 100644
>>> --- a/arch/arm/boot/dts/sun6i-a31.dtsi
>>> +++ b/arch/arm/boot/dts/sun6i-a31.dtsi
>>> @@ -501,6 +501,55 @@
>>>                 prcm@01f01c00 {
>> Seems the address here was wrong to start with.
>
> Absolutely, I'll fix it.
>
>>
>>>                         compatible = "allwinner,sun6i-a31-prcm";
>>>                         reg = <0x01f01400 0x200>;
>>> +
>>> +                       ar100_mux: ar100_mux {
>> Might we use clk@01f01XXX for the names of the clock nodes?
>
> Actually, I had a discussion with Maxime, and we decided to remove the
> address suffix because the PRCM block is not a bus, and thus should not
> have child node with addresses.
> But I'm not a DT binding expert (it might be acceptable to define child
> nodes with addresses even when the parent is not a bus :-)).
> Advices from DT maintainers on that specific point would be great.

Then I would suggest using a _clk suffix in the name, so at least we can
tell what type of device it is. That might be enough to satisfy ePAPR.
At least socfpga, omap54xx, omap44xx are doing it this way.

>>> +                               compatible = "allwinner,sun6i-a31-ar100-mux-clk";
>>> +                               #clock-cells = <0>;
>>> +                               clocks = <&osc32k>, <&osc24M>, <&pll6>, <&pll6>;
>>> +                       };
>>> +
>>> +                       ar100: ar100 {
>>> +                               compatible = "allwinner,sun6i-a31-ar100-clk";
>>> +                               #clock-cells = <0>;
>>> +                               clocks = <&ar100_mux>;
>>> +                       };
>>> +
>>> +                       ar100_div: ar100_div {
>>> +                               compatible = "allwinner,sun6i-a31-ar100-div-clk";
>>> +                               #clock-cells = <0>;
>>> +                               clocks = <&ar100>;
>>> +                       };
>>> +
>>> +                       ahb0: ahb0 {
>>> +                               compatible = "fixed-factor-clock";
>>> +                               #clock-cells = <0>;
>>> +                               clock-div = <1>;
>>> +                               clock-mult = <1>;
>>> +                               clocks = <&ar100_div>;
>>> +                               clock-output-names = "ahb0";
>>> +                       };
>>> +
>>> +                       apb0: apb0 {
>>> +                               compatible = "allwinner,sun6i-a31-apb0-clk";
>>> +                               #clock-cells = <0>;
>>> +                               clocks = <&ahb0>;
>>> +                               clock-output-names = "apb0";
>>> +                       };
>>> +
>>> +                       apb0_gates: apb0_gates {
>>> +                               compatible = "allwinner,sun6i-a31-apb0-gates-clk";
>>> +                               #clock-cells = <1>;
>>> +                               clocks = <&apb0>;
>>> +                               clock-output-names = "apb0_pio", "apb0_ir",
>>> +                                               "apb0_timer01", "apb0_p2wi",
>>> +                                               "apb0_uart", "apb0_1wire",
>>> +                                               "apb0_i2c";
>>> +                       };
>>> +
>>> +                       apb0_rst: apb0_rst {
>> Also use reset@01f01XXX here?
>>
>>> +                               compatible = "allwinner,sun6i-a31-clock-reset";
>>> +                               #reset-cells = <1>;
>>> +                       };
>>>                 };
>>>         };
>>>  };
>> Thanks!
>>
>> ChenYu
>
> --
> Boris Brezillon, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
>