[PATCH 0/2] clk: Add Artpec-6 SoC support

[PATCH 0/2] clk: Add Artpec-6 SoC support

[Lars Persson]

Add clock support for the Artpec-6 SoC port. The ARM parts are in the series
"arm: Add Artpec-6 SoC" and it goes through the arm-soc tree.

Lars Persson (2):
  clk: add device tree binding for artpec-6 pll1 clock
  clk: add artpec-6 pll1 clock driver

 .../devicetree/bindings/clock/artpec6.txt          | 16 +++++
 drivers/clk/Makefile                               |  1 +
 drivers/clk/clk-artpec6.c                          | 70 ++++++++++++++++++++++
 3 files changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/artpec6.txt
 create mode 100644 drivers/clk/clk-artpec6.c

-- 
2.1.4

[PATCH 1/2] clk: add device tree binding for artpec-6 pll1 clock

[Lars Persson]

Add device tree documentation for the main PLL in the Artpec-6 SoC.

Signed-off-by: Lars Persson <larper@axis.com>
---
 Documentation/devicetree/bindings/clock/artpec6.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/artpec6.txt

diff --git a/Documentation/devicetree/bindings/clock/artpec6.txt b/Documentation/devicetree/bindings/clock/artpec6.txt
new file mode 100644
index 0000000..521fec8
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/artpec6.txt
@@ -0,0 +1,16 @@
+* Clock bindings for Axis ARTPEC-6 chip
+
+Required properties:
+- #clock-cells: Should be <0>
+- compatible: Should be "axis,artpec6-pll1-clock"
+- reg: Address and length of the DEVSTAT register.
+- clocks: The PLL's input clock.
+
+Examples:
+
+pll1_clk: pll1_clk {
+	#clock-cells = <0>;
+	compatible = "axis,artpec6-pll1-clock";
+	reg = <0xf8000000 4>;
+	clocks = <&ext_clk>;
+};
-- 
2.1.4

[PATCH 2/2] clk: add artpec-6 pll1 clock driver

[Lars Persson]

The PLL1 clock is a fixed-factor clock with factors derived from boot
mode pins. This driver is a simple wrapper to register the fixed
factor clock according to the pin settings.

Signed-off-by: Lars Persson <larper@axis.com>
---
 drivers/clk/Makefile      |  1 +
 drivers/clk/clk-artpec6.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)
 create mode 100644 drivers/clk/clk-artpec6.c

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index b038e36..388f0cf 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -17,6 +17,7 @@ endif
 
 # hardware specific clock types
 # please keep this section sorted lexicographically by file/directory path name
+obj-$(CONFIG_MACH_ARTPEC6)		+= clk-artpec6.o
 obj-$(CONFIG_MACH_ASM9260)		+= clk-asm9260.o
 obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN)	+= clk-axi-clkgen.o
 obj-$(CONFIG_ARCH_AXXIA)		+= clk-axm5516.o
diff --git a/drivers/clk/clk-artpec6.c b/drivers/clk/clk-artpec6.c
new file mode 100644
index 0000000..3664c44
--- /dev/null
+++ b/drivers/clk/clk-artpec6.c
@@ -0,0 +1,70 @@
+/*
+ * ARTPEC-6 clock initialization
+ *
+ * Copyright 2015 Axis Comunications AB.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+static void __init of_artpec6_pll1_setup(struct device_node *np)
+{
+	void __iomem *devstat;
+	struct clk *clk;
+	const char *clk_name = np->name;
+	const char *parent_name;
+	u32 pll_mode, pll_m, pll_n;
+
+	parent_name = of_clk_get_parent_name(np, 0);
+
+	devstat = of_iomap(np, 0);
+	if (devstat == NULL) {
+		pr_err("error to ioremap DEVSTAT\n");
+		return;
+	}
+
+	/* DEVSTAT register contains PLL settings */
+	pll_mode = (readl(devstat) >> 6) & 3;
+	iounmap(devstat);
+
+	/*
+	 * pll1 settings are designed for different DDR speeds using a fixed
+	 * 50MHz external clock. However, a different external clock could be
+	 * used on different boards.
+	 * CPU clock is half the DDR clock.
+	 */
+	switch (pll_mode) {
+	case 0: /* DDR3-2133 mode */
+		pll_m = 4;
+		pll_n = 85;
+		break;
+	case 1: /* DDR3-1866 mode */
+		pll_m = 6;
+		pll_n = 112;
+		break;
+	case 2: /* DDR3-1600 mode */
+		pll_m = 4;
+		pll_n = 64;
+		break;
+	case 3: /* DDR3-1333 mode */
+		pll_m = 8;
+		pll_n = 106;
+		break;
+	}
+	/* ext_clk is defined in device tree */
+	clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0,
+			pll_n, pll_m);
+	if (IS_ERR(clk)) {
+		pr_err("%s not registered\n", clk_name);
+		return;
+	}
+	of_clk_add_provider(np, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(artpec6_pll1, "axis,artpec6-pll1-clock", of_artpec6_pll1_setup);
-- 
2.1.4

Re: [PATCH 1/2] clk: add device tree binding for artpec-6 pll1 clock

[Rob Herring]

On Thu, Feb 11, 2016 at 05:01:03PM +0100, Lars Persson wrote:
> Add device tree documentation for the main PLL in the Artpec-6 SoC.

Roughly how many clocks does this SoC have?

> 
> Signed-off-by: Lars Persson <larper@axis.com>
> ---
>  Documentation/devicetree/bindings/clock/artpec6.txt | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/artpec6.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/artpec6.txt b/Documentation/devicetree/bindings/clock/artpec6.txt
> new file mode 100644
> index 0000000..521fec8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/artpec6.txt
> @@ -0,0 +1,16 @@
> +* Clock bindings for Axis ARTPEC-6 chip
> +
> +Required properties:
> +- #clock-cells: Should be <0>
> +- compatible: Should be "axis,artpec6-pll1-clock"
> +- reg: Address and length of the DEVSTAT register.
> +- clocks: The PLL's input clock.
> +
> +Examples:
> +
> +pll1_clk: pll1_clk {
> +	#clock-cells = <0>;
> +	compatible = "axis,artpec6-pll1-clock";
> +	reg = <0xf8000000 4>;
> +	clocks = <&ext_clk>;
> +};
> -- 
> 2.1.4
> 

Re: [PATCH 1/2] clk: add device tree binding for artpec-6 pll1 clock

[Lars Persson]

On 02/12/2016 05:39 PM, Rob Herring wrote:
> On Thu, Feb 11, 2016 at 05:01:03PM +0100, Lars Persson wrote:
>> Add device tree documentation for the main PLL in the Artpec-6 SoC.
> Roughly how many clocks does this SoC have?
It will have 17 clocks declared in the device tree and three 
SoC-specific clock drivers.

>
>> Signed-off-by: Lars Persson <larper@axis.com>
>> ---
>>   Documentation/devicetree/bindings/clock/artpec6.txt | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/clock/artpec6.txt
>>
>> diff --git a/Documentation/devicetree/bindings/clock/artpec6.txt b/Documentation/devicetree/bindings/clock/artpec6.txt
>> new file mode 100644
>> index 0000000..521fec8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/artpec6.txt
>> @@ -0,0 +1,16 @@
>> +* Clock bindings for Axis ARTPEC-6 chip
>> +
>> +Required properties:
>> +- #clock-cells: Should be <0>
>> +- compatible: Should be "axis,artpec6-pll1-clock"
>> +- reg: Address and length of the DEVSTAT register.
>> +- clocks: The PLL's input clock.
>> +
>> +Examples:
>> +
>> +pll1_clk: pll1_clk {
>> +	#clock-cells = <0>;
>> +	compatible = "axis,artpec6-pll1-clock";
>> +	reg = <0xf8000000 4>;
>> +	clocks = <&ext_clk>;
>> +};
>> -- 
>> 2.1.4
>>

Re: [PATCH 1/2] clk: add device tree binding for artpec-6 pll1 clock

[Michael Turquette]

Quoting Lars Persson (2016-02-14 00:03:06)
> 
> 
> On 02/12/2016 05:39 PM, Rob Herring wrote:
> > On Thu, Feb 11, 2016 at 05:01:03PM +0100, Lars Persson wrote:
> >> Add device tree documentation for the main PLL in the Artpec-6 SoC.
> > Roughly how many clocks does this SoC have?
> It will have 17 clocks declared in the device tree and three 
> SoC-specific clock drivers.

Are all of those clks going to individual DT nodes with clock-cells = 0?

If so, I wonder if you should be targeting a clock-controller style
binding description, with a node that represents the clock-controller IP
block, with clock-cells >= 1. It really comes down to whether or not
these clock controls exist in the same IP block.

You mentioned three distinct clock drivers. So possibly three clock
controller nodes in DT then?

Is there a reference manual/register map available for this SoC?

> 
> >
> >> Signed-off-by: Lars Persson <larper@axis.com>
> >> ---
> >>   Documentation/devicetree/bindings/clock/artpec6.txt | 16 ++++++++++++++++
> >>   1 file changed, 16 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/clock/artpec6.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/artpec6.txt b/Documentation/devicetree/bindings/clock/artpec6.txt
> >> new file mode 100644
> >> index 0000000..521fec8
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/clock/artpec6.txt
> >> @@ -0,0 +1,16 @@
> >> +* Clock bindings for Axis ARTPEC-6 chip

Please specify that this is based on the clock provider binding in
Documentation/devicetree/bindings/clock/clock-bindings.txt

Regards,
Mike

> >> +
> >> +Required properties:
> >> +- #clock-cells: Should be <0>
> >> +- compatible: Should be "axis,artpec6-pll1-clock"
> >> +- reg: Address and length of the DEVSTAT register.
> >> +- clocks: The PLL's input clock.
> >> +
> >> +Examples:
> >> +
> >> +pll1_clk: pll1_clk {
> >> +    #clock-cells = <0>;
> >> +    compatible = "axis,artpec6-pll1-clock";
> >> +    reg = <0xf8000000 4>;
> >> +    clocks = <&ext_clk>;
> >> +};
> >> -- 
> >> 2.1.4
> >>
> 

Re: [PATCH 2/2] clk: add artpec-6 pll1 clock driver

[Michael Turquette]

Hi Lars,

Quoting Lars Persson (2016-02-11 08:01:04)
> The PLL1 clock is a fixed-factor clock with factors derived from boot
> mode pins. This driver is a simple wrapper to register the fixed
> factor clock according to the pin settings.
> 
> Signed-off-by: Lars Persson <larper@axis.com>
> ---
>  drivers/clk/Makefile      |  1 +
>  drivers/clk/clk-artpec6.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 71 insertions(+)
>  create mode 100644 drivers/clk/clk-artpec6.c
> 
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index b038e36..388f0cf 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -17,6 +17,7 @@ endif
>  
>  # hardware specific clock types
>  # please keep this section sorted lexicographically by file/directory path name
> +obj-$(CONFIG_MACH_ARTPEC6)             += clk-artpec6.o
>  obj-$(CONFIG_MACH_ASM9260)             += clk-asm9260.o
>  obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN)    += clk-axi-clkgen.o
>  obj-$(CONFIG_ARCH_AXXIA)               += clk-axm5516.o
> diff --git a/drivers/clk/clk-artpec6.c b/drivers/clk/clk-artpec6.c
> new file mode 100644
> index 0000000..3664c44
> --- /dev/null
> +++ b/drivers/clk/clk-artpec6.c

You mentioned having three drivers. Maybe consider putting this in
drivers/clk/axis?

Continuing my questions from patch #1, should this clock be coupled with
other artpec6 clocks sharing the same clock controller?

> @@ -0,0 +1,70 @@
> +/*
> + * ARTPEC-6 clock initialization
> + *
> + * Copyright 2015 Axis Comunications AB.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +
> +static void __init of_artpec6_pll1_setup(struct device_node *np)
> +{
> +       void __iomem *devstat;
> +       struct clk *clk;
> +       const char *clk_name = np->name;
> +       const char *parent_name;
> +       u32 pll_mode, pll_m, pll_n;
> +
> +       parent_name = of_clk_get_parent_name(np, 0);
> +
> +       devstat = of_iomap(np, 0);
> +       if (devstat == NULL) {
> +               pr_err("error to ioremap DEVSTAT\n");
> +               return;
> +       }
> +
> +       /* DEVSTAT register contains PLL settings */
> +       pll_mode = (readl(devstat) >> 6) & 3;
> +       iounmap(devstat);
> +
> +       /*
> +        * pll1 settings are designed for different DDR speeds using a fixed
> +        * 50MHz external clock. However, a different external clock could be
> +        * used on different boards.
> +        * CPU clock is half the DDR clock.
> +        */
> +       switch (pll_mode) {
> +       case 0: /* DDR3-2133 mode */
> +               pll_m = 4;
> +               pll_n = 85;
> +               break;
> +       case 1: /* DDR3-1866 mode */
> +               pll_m = 6;
> +               pll_n = 112;
> +               break;
> +       case 2: /* DDR3-1600 mode */
> +               pll_m = 4;
> +               pll_n = 64;
> +               break;
> +       case 3: /* DDR3-1333 mode */
> +               pll_m = 8;
> +               pll_n = 106;
> +               break;
> +       }
> +       /* ext_clk is defined in device tree */
> +       clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0,
> +                       pll_n, pll_m);
> +       if (IS_ERR(clk)) {
> +               pr_err("%s not registered\n", clk_name);
> +               return;
> +       }
> +       of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +}
> +CLK_OF_DECLARE(artpec6_pll1, "axis,artpec6-pll1-clock", of_artpec6_pll1_setup);

Instead of using CLK_OF_DECLARE can you make this a platform_driver?

Best regards,
Mike

> -- 
> 2.1.4
> 

Re: [PATCH 1/2] clk: add device tree binding for artpec-6 pll1 clock

[Lars Persson]

On 02/17/2016 12:59 AM, Michael Turquette wrote:
> Quoting Lars Persson (2016-02-14 00:03:06)
>>
>> On 02/12/2016 05:39 PM, Rob Herring wrote:
>>> On Thu, Feb 11, 2016 at 05:01:03PM +0100, Lars Persson wrote:
>>>> Add device tree documentation for the main PLL in the Artpec-6 SoC.
>>> Roughly how many clocks does this SoC have?
>> It will have 17 clocks declared in the device tree and three
>> SoC-specific clock drivers.
> Are all of those clks going to individual DT nodes with clock-cells = 0?
>
> If so, I wonder if you should be targeting a clock-controller style
> binding description, with a node that represents the clock-controller IP
> block, with clock-cells >= 1. It really comes down to whether or not
> these clock controls exist in the same IP block.

Indeed most clocks come from the same clock controller IP. However there 
was nothing to control there because this clock tree is completely 
static. This is why we chose to describe it in the device tree using the 
standard clock components.

The two audio related clock drivers that we will submit for a later 
merge window will have control knobs. There will be one mux belonging to 
the common clock controller and fractional dividers as separate IPs.

I guess we could create a clock controller that outputs the pll1 clock 
and in the future also the muxed audio clock. Then describe the rest of 
the clock tree with standard components in the DT.

>
> You mentioned three distinct clock drivers. So possibly three clock
> controller nodes in DT then?
We still prefer to fully describe the static parts of the the clock tree 
in the DT. Will you accept this ?
>
> Is there a reference manual/register map available for this SoC?
No.
>>>> Signed-off-by: Lars Persson <larper@axis.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/clock/artpec6.txt | 16 ++++++++++++++++
>>>>    1 file changed, 16 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/clock/artpec6.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/clock/artpec6.txt b/Documentation/devicetree/bindings/clock/artpec6.txt
>>>> new file mode 100644
>>>> index 0000000..521fec8
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/artpec6.txt
>>>> @@ -0,0 +1,16 @@
>>>> +* Clock bindings for Axis ARTPEC-6 chip
> Please specify that this is based on the clock provider binding in
> Documentation/devicetree/bindings/clock/clock-bindings.txt
Will be fixed in v2.
>
> Regards,
> Mike
>
>>>> +
>>>> +Required properties:
>>>> +- #clock-cells: Should be <0>
>>>> +- compatible: Should be "axis,artpec6-pll1-clock"
>>>> +- reg: Address and length of the DEVSTAT register.
>>>> +- clocks: The PLL's input clock.
>>>> +
>>>> +Examples:
>>>> +
>>>> +pll1_clk: pll1_clk {
>>>> +    #clock-cells = <0>;
>>>> +    compatible = "axis,artpec6-pll1-clock";
>>>> +    reg = <0xf8000000 4>;
>>>> +    clocks = <&ext_clk>;
>>>> +};
>>>> -- 
>>>> 2.1.4
>>>>

Re: [PATCH 2/2] clk: add artpec-6 pll1 clock driver

[Lars Persson]

On 02/17/2016 01:02 AM, Michael Turquette wrote:
> Hi Lars,
>
> Quoting Lars Persson (2016-02-11 08:01:04)
>> The PLL1 clock is a fixed-factor clock with factors derived from boot
>> mode pins. This driver is a simple wrapper to register the fixed
>> factor clock according to the pin settings.
>>
>> Signed-off-by: Lars Persson <larper@axis.com>
>> ---
>>   drivers/clk/Makefile      |  1 +
>>   drivers/clk/clk-artpec6.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 71 insertions(+)
>>   create mode 100644 drivers/clk/clk-artpec6.c
>>
>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>> index b038e36..388f0cf 100644
>> --- a/drivers/clk/Makefile
>> +++ b/drivers/clk/Makefile
>> @@ -17,6 +17,7 @@ endif
>>   
>>   # hardware specific clock types
>>   # please keep this section sorted lexicographically by file/directory path name
>> +obj-$(CONFIG_MACH_ARTPEC6)             += clk-artpec6.o
>>   obj-$(CONFIG_MACH_ASM9260)             += clk-asm9260.o
>>   obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN)    += clk-axi-clkgen.o
>>   obj-$(CONFIG_ARCH_AXXIA)               += clk-axm5516.o
>> diff --git a/drivers/clk/clk-artpec6.c b/drivers/clk/clk-artpec6.c
>> new file mode 100644
>> index 0000000..3664c44
>> --- /dev/null
>> +++ b/drivers/clk/clk-artpec6.c
> You mentioned having three drivers. Maybe consider putting this in
> drivers/clk/axis?
Will be fixed in v2.
>
> Continuing my questions from patch #1, should this clock be coupled with
> other artpec6 clocks sharing the same clock controller?
>
>> @@ -0,0 +1,70 @@
>> +/*
>> + * ARTPEC-6 clock initialization
>> + *
>> + * Copyright 2015 Axis Comunications AB.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +
>> +static void __init of_artpec6_pll1_setup(struct device_node *np)
>> +{
>> +       void __iomem *devstat;
>> +       struct clk *clk;
>> +       const char *clk_name = np->name;
>> +       const char *parent_name;
>> +       u32 pll_mode, pll_m, pll_n;
>> +
>> +       parent_name = of_clk_get_parent_name(np, 0);
>> +
>> +       devstat = of_iomap(np, 0);
>> +       if (devstat == NULL) {
>> +               pr_err("error to ioremap DEVSTAT\n");
>> +               return;
>> +       }
>> +
>> +       /* DEVSTAT register contains PLL settings */
>> +       pll_mode = (readl(devstat) >> 6) & 3;
>> +       iounmap(devstat);
>> +
>> +       /*
>> +        * pll1 settings are designed for different DDR speeds using a fixed
>> +        * 50MHz external clock. However, a different external clock could be
>> +        * used on different boards.
>> +        * CPU clock is half the DDR clock.
>> +        */
>> +       switch (pll_mode) {
>> +       case 0: /* DDR3-2133 mode */
>> +               pll_m = 4;
>> +               pll_n = 85;
>> +               break;
>> +       case 1: /* DDR3-1866 mode */
>> +               pll_m = 6;
>> +               pll_n = 112;
>> +               break;
>> +       case 2: /* DDR3-1600 mode */
>> +               pll_m = 4;
>> +               pll_n = 64;
>> +               break;
>> +       case 3: /* DDR3-1333 mode */
>> +               pll_m = 8;
>> +               pll_n = 106;
>> +               break;
>> +       }
>> +       /* ext_clk is defined in device tree */
>> +       clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0,
>> +                       pll_n, pll_m);
>> +       if (IS_ERR(clk)) {
>> +               pr_err("%s not registered\n", clk_name);
>> +               return;
>> +       }
>> +       of_clk_add_provider(np, of_clk_src_simple_get, clk);
>> +}
>> +CLK_OF_DECLARE(artpec6_pll1, "axis,artpec6-pll1-clock", of_artpec6_pll1_setup);
> Instead of using CLK_OF_DECLARE can you make this a platform_driver?
Will be fixed in v2.
>
> Best regards,
> Mike
>
>> -- 
>> 2.1.4
>>

Re: [PATCH 1/2] clk: add device tree binding for artpec-6 pll1 clock

[Michael Turquette]

Hi Lars,

Quoting Lars Persson (2016-02-17 02:29:14)
> 
> 
> On 02/17/2016 12:59 AM, Michael Turquette wrote:
> > Quoting Lars Persson (2016-02-14 00:03:06)
> >>
> >> On 02/12/2016 05:39 PM, Rob Herring wrote:
> >>> On Thu, Feb 11, 2016 at 05:01:03PM +0100, Lars Persson wrote:
> >>>> Add device tree documentation for the main PLL in the Artpec-6 SoC.
> >>> Roughly how many clocks does this SoC have?
> >> It will have 17 clocks declared in the device tree and three
> >> SoC-specific clock drivers.
> > Are all of those clks going to individual DT nodes with clock-cells = 0?
> >
> > If so, I wonder if you should be targeting a clock-controller style
> > binding description, with a node that represents the clock-controller IP
> > block, with clock-cells >= 1. It really comes down to whether or not
> > these clock controls exist in the same IP block.
> 
> Indeed most clocks come from the same clock controller IP. However there 
> was nothing to control there because this clock tree is completely 
> static. This is why we chose to describe it in the device tree using the 
> standard clock components.

Those components are there for board-level clocks, such as a fixed rate
crystal feeding into a more complex IC. They are not substitutes for
writing a proper clock controller driver.

> 
> The two audio related clock drivers that we will submit for a later 
> merge window will have control knobs. There will be one mux belonging to 
> the common clock controller and fractional dividers as separate IPs.

So the mux belongs to the same clock controller IP block as the pll?
Then the DT binding description should definitely not be a
one-clock-per-node style.

> 
> I guess we could create a clock controller that outputs the pll1 clock 
> and in the future also the muxed audio clock. Then describe the rest of 
> the clock tree with standard components in the DT.
> 
> >
> > You mentioned three distinct clock drivers. So possibly three clock
> > controller nodes in DT then?
> We still prefer to fully describe the static parts of the the clock tree 
> in the DT. Will you accept this ?

No, description of the clock tree in DT should not be your goal. I know
it sounds nice from a data-driven point of view but in practice it is a
bad idea.

(Note that all of the above references the clock tree internal to an
SoC, and doesn't cover off-chip clocks, like a fixed-rate xtal that
varies from board to board.)

What you should do is model each clock generator/controller as a DT
node; this fits nicely in line with the Linux device model. Any leaf
clocks that are consumed by other devices are exposed as offsets from
the phandle. Clocks internal to the clock controller/generator block,
such as intermediate dividers, should not be exposed in device tree as
they are not direct inputs to downstream devices.

Regards,
Mike

> >
> > Is there a reference manual/register map available for this SoC?
> No.
> >>>> Signed-off-by: Lars Persson <larper@axis.com>
> >>>> ---
> >>>>    Documentation/devicetree/bindings/clock/artpec6.txt | 16 ++++++++++++++++
> >>>>    1 file changed, 16 insertions(+)
> >>>>    create mode 100644 Documentation/devicetree/bindings/clock/artpec6.txt
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/clock/artpec6.txt b/Documentation/devicetree/bindings/clock/artpec6.txt
> >>>> new file mode 100644
> >>>> index 0000000..521fec8
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/clock/artpec6.txt
> >>>> @@ -0,0 +1,16 @@
> >>>> +* Clock bindings for Axis ARTPEC-6 chip
> > Please specify that this is based on the clock provider binding in
> > Documentation/devicetree/bindings/clock/clock-bindings.txt
> Will be fixed in v2.
> >
> > Regards,
> > Mike
> >
> >>>> +
> >>>> +Required properties:
> >>>> +- #clock-cells: Should be <0>
> >>>> +- compatible: Should be "axis,artpec6-pll1-clock"
> >>>> +- reg: Address and length of the DEVSTAT register.
> >>>> +- clocks: The PLL's input clock.
> >>>> +
> >>>> +Examples:
> >>>> +
> >>>> +pll1_clk: pll1_clk {
> >>>> +    #clock-cells = <0>;
> >>>> +    compatible = "axis,artpec6-pll1-clock";
> >>>> +    reg = <0xf8000000 4>;
> >>>> +    clocks = <&ext_clk>;
> >>>> +};
> >>>> -- 
> >>>> 2.1.4
> >>>>
>