[PATCH 00/10] Add clock support for Armada 37xx SoCs

[PATCH 00/10] Add clock support for Armada 37xx SoCs

[Gregory CLEMENT]

Hi,

this series add clock support for the Armada 37xx SoCs.

The design of the drivers is as close as possible as the hardware is,
with some clocks made of several layers: muxing, divider and gating.

The device tree binding was written in a way that even if we discover
some change inside the clocks, the binding should not be
affected. Especially, there are some holes in the clocks, but we
should be able to add them seamless.

I tried to follow the last update made in the clock framework, I hope
theses drivers will comply the new guidelines.

Thanks,

Gregory

Gregory CLEMENT (10):
  arm64: marvell: enable Armada 3700 clock drivers
  arm64: dts: marvell: Add xtal clock support for Armada 3700
  arm64: dts: marvell: add tbg clocks for Armada 37xx
  arm64: dts: marvell: add peripherals clocks for Armada 37xx
  dt-bindings: clock: add DT binding for the Xtal clock on Armada 3700
  clk: mvebu: Add the xtal clock for Armada 3700 SoC
  dt-bindings: clock: add DT binding for the TBG clocks on Armada 3700
  clk: mvebu Add the time base generator clocks for Armada 3700
  dt-bindings: clock: add DT binding for the peripheral clocks on Armada
    3700
  clk: mvebu: Add the peripheral clock driver for Armada 3700

 .../bindings/clock/armada3700-periph-clock.txt     |  70 ++++
 .../bindings/clock/armada3700-tbg-clock.txt        |  27 ++
 .../bindings/clock/armada3700-xtal-clock.txt       |  28 ++
 arch/arm64/Kconfig.platforms                       |   1 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi       |  35 ++
 drivers/clk/mvebu/Kconfig                          |   3 +
 drivers/clk/mvebu/Makefile                         |   3 +
 drivers/clk/mvebu/armada-37xx-periph.c             | 462 +++++++++++++++++++++
 drivers/clk/mvebu/armada-37xx-tbg.c                | 172 ++++++++
 drivers/clk/mvebu/armada-37xx-xtal.c               |  93 +++++
 10 files changed, 894 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/armada3700-tbg-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
 create mode 100644 drivers/clk/mvebu/armada-37xx-periph.c
 create mode 100644 drivers/clk/mvebu/armada-37xx-tbg.c
 create mode 100644 drivers/clk/mvebu/armada-37xx-xtal.c

-- 
2.5.0

[PATCH 01/10] arm64: marvell: enable Armada 3700 clock drivers

[Gregory CLEMENT]

This patch enables the support for the clocks drivers used on the
Armada 3700.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm64/Kconfig.platforms | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 7ef1d05859ae..2fdadff9143d 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -73,6 +73,7 @@ config ARCH_MVEBU
 	bool "Marvell EBU SoC Family"
 	select ARMADA_AP806_SYSCON
 	select ARMADA_CP110_SYSCON
+	select ARMADA_37XX_CLK
 	select MVEBU_ODMI
 	help
 	  This enables support for Marvell EBU familly, including:
-- 
2.5.0

[PATCH 02/10] arm64: dts: marvell: Add xtal clock support for Armada 3700

[Gregory CLEMENT]

The configuration of the clock depend of the gpio latch. This information
is stored in the gpio block registers. That's why the block is shared
using a syscon node.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 9e2efb882983..7c61e13a16f3 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -105,6 +105,18 @@
 				status = "disabled";
 			};
 
+			gpio1: gpio@13800 {
+				compatible = "marvell,mvebu-gpio-3700",
+				"syscon", "simple-mfd";
+				reg = <0x13800 0x500>;
+
+				xtalclk: xtal-clk {
+					compatible = "marvell,armada-3700-xtal-clock";
+					clock-output-names = "xtal";
+					#clock-cells = <0>;
+				};
+			};
+
 			usb3: usb@58000 {
 				compatible = "marvell,armada3700-xhci",
 				"generic-xhci";
-- 
2.5.0

[PATCH 03/10] arm64: dts: marvell: add tbg clocks for Armada 37xx

[Gregory CLEMENT]

Add a new block of clocks. The Time Base Generators clocks can be the
parent of the peripheral clocks.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 7c61e13a16f3..e9f0b97c63d5 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -105,6 +105,13 @@
 				status = "disabled";
 			};
 
+			tbg: tbg@13200 {
+				compatible = "marvell,armada-3700-tbg-clock";
+				reg = <0x13200 0x100>;
+				clocks = <&xtalclk>;
+				#clock-cells = <1>;
+			};
+
 			gpio1: gpio@13800 {
 				compatible = "marvell,mvebu-gpio-3700",
 				"syscon", "simple-mfd";
-- 
2.5.0

[PATCH 04/10] arm64: dts: marvell: add peripherals clocks for Armada 37xx

[Gregory CLEMENT]

Add two new blocks of clocks. The peripheral clocks are the source clocks
of the peripheral of the Armada 3700 SoC.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index e9f0b97c63d5..c55542ba0c54 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -105,6 +105,22 @@
 				status = "disabled";
 			};
 
+			nb_perih_clk: nb-periph-clk@13000{
+				compatible = "marvell,armada-3700-periph-clock-nb";
+				reg = <0x13000 0x100>;
+				clocks = <&tbg 0>, <&tbg 1>, <&tbg 2>,
+				<&tbg 3>, <&xtalclk>;
+				#clock-cells = <1>;
+			};
+
+			sb_perih_clk: sb-periph-clk@13000{
+				compatible = "marvell,armada-3700-periph-clock-sb";
+				reg = <0x18000 0x100>;
+				clocks = <&tbg 0>, <&tbg 1>, <&tbg 2>,
+				<&tbg 3>, <&xtalclk>;
+				#clock-cells = <1>;
+			};
+
 			tbg: tbg@13200 {
 				compatible = "marvell,armada-3700-tbg-clock";
 				reg = <0x13200 0x100>;
-- 
2.5.0

[PATCH 05/10] dt-bindings: clock: add DT binding for the Xtal clock on Armada 3700

[Gregory CLEMENT]

This commit adds the DT binding documentation for the the Xtal clock on
Armada 3700 used in the Marvell Armada 3700 SoCs.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 .../bindings/clock/armada3700-xtal-clock.txt       | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt

diff --git a/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt b/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
new file mode 100644
index 000000000000..176c4fe6f0e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
@@ -0,0 +1,28 @@
+* Xtal Clock bindings for Marvell Armada 37xx SoCs
+
+Marvell Armada 37xx SoCs allow to determine the xtal clock frequencies by
+reading the gpio latch register.
+
+This node must be a subnode of the node exposing the register address
+of the GPIO block where the gpio latch is located.
+
+Required properties:
+- compatible : shall be one of the following:
+	"marvell,armada-3700-xtal-clock"
+- #clock-cells : from common clock binding; shall be set to 0
+
+Optional properties:
+- clock-output-names : from common clock binding; allows overwrite default clock
+	output names ("xtal")
+
+Example:
+gpio1: gpio@13800 {
+	compatible = "marvell,mvebu-gpio-3700",	"syscon", "simple-mfd";
+	reg = <0x13800 0x1000>;
+
+	xtalclk: xtal-clk {
+		compatible = "marvell,armada-3700-xtal-clock";
+		clock-output-names = "xtal";
+		#clock-cells = <0>;
+	};
+};
-- 
2.5.0

Re: [PATCH 05/10] dt-bindings: clock: add DT binding for the Xtal clock on Armada 3700

[Rob Herring]

On Fri, Jun 10, 2016 at 03:23:33PM +0200, Gregory CLEMENT wrote:
> This commit adds the DT binding documentation for the the Xtal clock on
> Armada 3700 used in the Marvell Armada 3700 SoCs.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  .../bindings/clock/armada3700-xtal-clock.txt       | 28 ++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 06/10] clk: mvebu: Add the xtal clock for Armada 3700 SoC

[Gregory CLEMENT]

This clock is the parent of all the Armada 3700 clocks. It is a fixed
rate clock which depends on the gpio configuration read when resetting
the SoC.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/clk/mvebu/Kconfig            |  3 ++
 drivers/clk/mvebu/Makefile           |  1 +
 drivers/clk/mvebu/armada-37xx-xtal.c | 93 ++++++++++++++++++++++++++++++++++++
 3 files changed, 97 insertions(+)
 create mode 100644 drivers/clk/mvebu/armada-37xx-xtal.c

diff --git a/drivers/clk/mvebu/Kconfig b/drivers/clk/mvebu/Kconfig
index 3165da77d525..fddc8ac5faff 100644
--- a/drivers/clk/mvebu/Kconfig
+++ b/drivers/clk/mvebu/Kconfig
@@ -24,6 +24,9 @@ config ARMADA_39X_CLK
 	bool
 	select MVEBU_CLK_COMMON
 
+config ARMADA_37XX_CLK
+       bool
+
 config ARMADA_XP_CLK
 	bool
 	select MVEBU_CLK_COMMON
diff --git a/drivers/clk/mvebu/Makefile b/drivers/clk/mvebu/Makefile
index 7172ef65693d..4257a36d0219 100644
--- a/drivers/clk/mvebu/Makefile
+++ b/drivers/clk/mvebu/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_ARMADA_370_CLK)	+= armada-370.o
 obj-$(CONFIG_ARMADA_375_CLK)	+= armada-375.o
 obj-$(CONFIG_ARMADA_38X_CLK)	+= armada-38x.o
 obj-$(CONFIG_ARMADA_39X_CLK)	+= armada-39x.o
+obj-$(CONFIG_ARMADA_37XX_CLK)	+= armada-37xx-xtal.o
 obj-$(CONFIG_ARMADA_XP_CLK)	+= armada-xp.o
 obj-$(CONFIG_ARMADA_AP806_SYSCON) += ap806-system-controller.o
 obj-$(CONFIG_ARMADA_CP110_SYSCON) += cp110-system-controller.o
diff --git a/drivers/clk/mvebu/armada-37xx-xtal.c b/drivers/clk/mvebu/armada-37xx-xtal.c
new file mode 100644
index 000000000000..fb051a75b2c2
--- /dev/null
+++ b/drivers/clk/mvebu/armada-37xx-xtal.c
@@ -0,0 +1,93 @@
+/*
+ * Marvell Armada 37xx SoC xtal clocks
+ *
+ * Copyright (C) 2016 Marvell
+ *
+ * Gregory CLEMENT <gregory.clement@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define NB_GPIO1_LATCH	0xC
+#define XTAL_MODE	    BIT(31)
+
+static struct clk *xtal_clk;
+
+static int armada_3700_xtal_clock_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	const char *xtal_name = "xtal";
+	struct device_node *parent;
+	struct regmap *regmap;
+	unsigned int rate;
+	u32 reg;
+	int ret;
+
+	parent = np->parent;
+	if (!parent) {
+		dev_err(&pdev->dev, "no parrent\n");
+		return -ENODEV;
+	}
+
+	regmap = syscon_node_to_regmap(parent);
+	if (IS_ERR(regmap)) {
+		dev_err(&pdev->dev, "cannot get regmap\n");
+		return PTR_ERR(regmap);
+	}
+
+	ret = regmap_read(regmap, NB_GPIO1_LATCH, &reg);
+	if (ret) {
+		dev_err(&pdev->dev, "cannot read from regmap\n");
+		return ret;
+	}
+
+	if (reg & XTAL_MODE)
+		rate = 40000000;
+	else
+		rate = 25000000;
+
+	of_property_read_string_index(np, "clock-output-names", 0, &xtal_name);
+	xtal_clk = clk_register_fixed_rate(NULL, xtal_name, NULL, 0, rate);
+	if (IS_ERR(xtal_clk))
+		return PTR_ERR(xtal_clk);
+	of_clk_add_provider(np, of_clk_src_simple_get, xtal_clk);
+
+	return 0;
+}
+
+static int armada_3700_xtal_clock_remove(struct platform_device *pdev)
+{
+	of_clk_del_provider(pdev->dev.of_node);
+	clk_unregister_fixed_rate(xtal_clk);
+
+	return 0;
+}
+
+static const struct of_device_id armada_3700_xtal_clock_of_match[] = {
+	{ .compatible = "marvell,armada-3700-xtal-clock", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, armada_3700_xtal_clock_of_match);
+
+static struct platform_driver armada_3700_xtal_clock_driver = {
+	.probe = armada_3700_xtal_clock_probe,
+	.remove = armada_3700_xtal_clock_remove,
+	.driver		= {
+		.name	= "marvell-armada-3700-xtal-clock",
+		.of_match_table = armada_3700_xtal_clock_of_match,
+	},
+};
+
+module_platform_driver(armada_3700_xtal_clock_driver);
+
+MODULE_AUTHOR("Gregory CLEMENT <gregory.clement@free-electrons.com>");
+MODULE_DESCRIPTION("Marvell Armada 37xx SoC xtal clocks driver");
+MODULE_LICENSE("GPL v2");
-- 
2.5.0

Re: [PATCH 06/10] clk: mvebu: Add the xtal clock for Armada 3700 SoC

[Stephen Boyd]

On 06/10, Gregory CLEMENT wrote:
> diff --git a/drivers/clk/mvebu/armada-37xx-xtal.c b/drivers/clk/mvebu/armada-37xx-xtal.c
> new file mode 100644
> index 000000000000..fb051a75b2c2
> --- /dev/null
> +++ b/drivers/clk/mvebu/armada-37xx-xtal.c
> @@ -0,0 +1,93 @@
> +/*
> + * Marvell Armada 37xx SoC xtal clocks
> + *
> + * Copyright (C) 2016 Marvell
> + *
> + * Gregory CLEMENT <gregory.clement@free-electrons.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define NB_GPIO1_LATCH	0xC
> +#define XTAL_MODE	    BIT(31)
> +
> +static struct clk *xtal_clk;

Please allocate this at driver probe time instead, or even just
assign it to be the platform driver's drvdata.

> +
> +static int armada_3700_xtal_clock_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	const char *xtal_name = "xtal";
> +	struct device_node *parent;
> +	struct regmap *regmap;
> +	unsigned int rate;
> +	u32 reg;
> +	int ret;
> +
> +	parent = np->parent;
> +	if (!parent) {
> +		dev_err(&pdev->dev, "no parrent\n");

s/parrent/parent/

> +		return -ENODEV;
> +	}
> +
> +	regmap = syscon_node_to_regmap(parent);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&pdev->dev, "cannot get regmap\n");
> +		return PTR_ERR(regmap);
> +	}
> +
> +	ret = regmap_read(regmap, NB_GPIO1_LATCH, &reg);
> +	if (ret) {
> +		dev_err(&pdev->dev, "cannot read from regmap\n");
> +		return ret;
> +	}
> +
> +	if (reg & XTAL_MODE)
> +		rate = 40000000;
> +	else
> +		rate = 25000000;
> +
> +	of_property_read_string_index(np, "clock-output-names", 0, &xtal_name);
> +	xtal_clk = clk_register_fixed_rate(NULL, xtal_name, NULL, 0, rate);

Please move this to use the clk_hw_register_fixed_rate() function
instead and update of_clk_add_provider to register a clk_hw
instead of clk pointer.

> +	if (IS_ERR(xtal_clk))
> +		return PTR_ERR(xtal_clk);
> +	of_clk_add_provider(np, of_clk_src_simple_get, xtal_clk);

What if this fails?

> +
> +	return 0;
> +}

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 07/10] dt-bindings: clock: add DT binding for the TBG clocks on Armada 3700

[Gregory CLEMENT]

This commit adds the DT binding documentation for the Time Base Generator
clock used in the Marvell Armada 3700 SoCs.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 .../bindings/clock/armada3700-tbg-clock.txt        | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/armada3700-tbg-clock.txt

diff --git a/Documentation/devicetree/bindings/clock/armada3700-tbg-clock.txt b/Documentation/devicetree/bindings/clock/armada3700-tbg-clock.txt
new file mode 100644
index 000000000000..0ba1d83ff363
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/armada3700-tbg-clock.txt
@@ -0,0 +1,27 @@
+* Time Base Generator Clock bindings for Marvell Armada 37xx SoCs
+
+Marvell Armada 37xx SoCs provde Time Base Generator clocks which are
+used as parent clocks for the peripheral clocks.
+
+The TBG clock consumer should specify the desired clock by having the
+clock ID in its "clocks" phandle cell.
+
+The following is a list of provided IDs and clock names on Armada 3700:
+ 0 = TBG A P
+ 1 = TBG B P
+ 2 = TBG A S
+ 3 = TBG B S
+
+Required properties:
+- compatible : shall be "marvell,armada-3700-tbg-clock"
+- reg : must be the register address of North Bridge PLL register
+- #clock-cells : from common clock binding; shall be set to 1
+
+Example:
+
+tbg: tbg@13200 {
+	compatible = "marvell,armada-3700-tbg-clock";
+	reg = <0x13200 0x1000>;
+	clocks = <&xtalclk>;
+	#clock-cells = <1>;
+};
-- 
2.5.0

Re: [PATCH 07/10] dt-bindings: clock: add DT binding for the TBG clocks on Armada 3700

[Rob Herring]

On Fri, Jun 10, 2016 at 03:23:35PM +0200, Gregory CLEMENT wrote:
> This commit adds the DT binding documentation for the Time Base Generator
> clock used in the Marvell Armada 3700 SoCs.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  .../bindings/clock/armada3700-tbg-clock.txt        | 27 ++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/armada3700-tbg-clock.txt

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 08/10] clk: mvebu Add the time base generator clocks for Armada 3700

[Gregory CLEMENT]

These clocks are children of the xtal clock and each one can be selected
as a source for the peripheral clocks.

According to the datasheet it should be possible to modify their rate,
but currently it is not supported.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/clk/mvebu/Makefile          |   1 +
 drivers/clk/mvebu/armada-37xx-tbg.c | 172 ++++++++++++++++++++++++++++++++++++
 2 files changed, 173 insertions(+)
 create mode 100644 drivers/clk/mvebu/armada-37xx-tbg.c

diff --git a/drivers/clk/mvebu/Makefile b/drivers/clk/mvebu/Makefile
index 4257a36d0219..72e3512a9d4a 100644
--- a/drivers/clk/mvebu/Makefile
+++ b/drivers/clk/mvebu/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_ARMADA_375_CLK)	+= armada-375.o
 obj-$(CONFIG_ARMADA_38X_CLK)	+= armada-38x.o
 obj-$(CONFIG_ARMADA_39X_CLK)	+= armada-39x.o
 obj-$(CONFIG_ARMADA_37XX_CLK)	+= armada-37xx-xtal.o
+obj-$(CONFIG_ARMADA_37XX_CLK)	+= armada-37xx-tbg.o
 obj-$(CONFIG_ARMADA_XP_CLK)	+= armada-xp.o
 obj-$(CONFIG_ARMADA_AP806_SYSCON) += ap806-system-controller.o
 obj-$(CONFIG_ARMADA_CP110_SYSCON) += cp110-system-controller.o
diff --git a/drivers/clk/mvebu/armada-37xx-tbg.c b/drivers/clk/mvebu/armada-37xx-tbg.c
new file mode 100644
index 000000000000..e9a331b85a78
--- /dev/null
+++ b/drivers/clk/mvebu/armada-37xx-tbg.c
@@ -0,0 +1,172 @@
+/*
+ * Marvell Armada 37xx SoC Time Base Generator clocks
+ *
+ * Copyright (C) 2016 Marvell
+ *
+ * Gregory CLEMENT <gregory.clement@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define NUM_TBG	    4
+
+#define TBG_CTRL0		0x4
+#define TBG_CTRL1		0x8
+#define TBG_CTRL7		0x20
+#define TBG_CTRL8		0x30
+
+#define TBG_DIV_MASK		0x1FF
+
+#define TBG_A_REFDIV		0
+#define TBG_B_REFDIV		16
+
+#define TBG_A_FBDIV		2
+#define TBG_B_FBDIV		18
+
+#define TBG_A_VCODIV_SE		0
+#define TBG_B_VCODIV_SE		16
+
+#define TBG_A_VCODIV_DIFF	1
+#define TBG_B_VCODIV_DIFF	17
+
+struct tbg_def {
+	char *name;
+	u32 refdiv_offset;
+	u32 fbdiv_offset;
+	u32 vcodiv_reg;
+	u32 vcodiv_offset;
+};
+
+struct tbg_def tbg[NUM_TBG] = {
+	{"TBG-A-P", TBG_A_REFDIV, TBG_A_FBDIV, TBG_CTRL8, TBG_A_VCODIV_DIFF},
+	{"TBG-B-P", TBG_B_REFDIV, TBG_B_FBDIV, TBG_CTRL8, TBG_B_VCODIV_DIFF},
+	{"TBG-A-S", TBG_A_REFDIV, TBG_A_FBDIV, TBG_CTRL1, TBG_A_VCODIV_SE},
+	{"TBG-B-S", TBG_B_REFDIV, TBG_B_FBDIV, TBG_CTRL1, TBG_B_VCODIV_SE},
+};
+
+static struct clk_onecell_data clk_tbg_data;
+
+unsigned int tbg_get_mult(void __iomem *reg, struct tbg_def *ptbg)
+{
+	u32 val;
+
+	val = readl(reg + TBG_CTRL0);
+
+	return ((val >> ptbg->fbdiv_offset) & TBG_DIV_MASK) << 2;
+}
+
+unsigned int tbg_get_div(void __iomem *reg, struct tbg_def *ptbg)
+{
+	u32 val;
+	unsigned int div;
+
+	val = readl(reg + TBG_CTRL7);
+
+	div = (val >> ptbg->refdiv_offset) & TBG_DIV_MASK;
+	if (div == 0)
+		div = 1;
+	val = readl(reg + ptbg->vcodiv_reg);
+
+	div *= 1 << ((val >>  ptbg->vcodiv_offset) & TBG_DIV_MASK);
+
+	return div;
+}
+
+
+static int armada_3700_tbg_clock_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	const char *parent_name;
+	struct resource *res;
+	struct clk *parent;
+	void __iomem *reg;
+	int i, ret;
+
+	parent = of_clk_get(np, 0);
+	if (IS_ERR(parent)) {
+		dev_err(dev, "Could get the clock parent\n");
+		return -EINVAL;
+	}
+	parent_name = __clk_get_name(parent);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	reg = devm_ioremap_resource(dev, res);
+	if (IS_ERR(reg)) {
+		dev_err(dev, "Could not map the tbg clock registers\n");
+		ret = PTR_ERR(reg);
+		goto put_clk;
+	}
+
+	clk_tbg_data.clk_num = NUM_TBG;
+	clk_tbg_data.clks = devm_kcalloc(dev, clk_tbg_data.clk_num,
+					 sizeof(struct clk *), GFP_KERNEL);
+	if (!clk_tbg_data.clks) {
+		ret = -ENOMEM;
+		goto put_clk;
+	}
+
+	for (i = 0; i < NUM_TBG; i++) {
+		const char *name;
+		unsigned int mult, div;
+
+		name = tbg[i].name;
+		mult = tbg_get_mult(reg, &tbg[i]);
+		div = tbg_get_div(reg, &tbg[i]);
+		clk_tbg_data.clks[i] = clk_register_fixed_factor(NULL, name,
+						 parent_name, 0, mult, div);
+		if (IS_ERR(clk_tbg_data.clks[i]))
+			dev_err(dev, "Can't register TBG clock %s\n", name);
+	}
+
+	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_tbg_data);
+
+	return 0;
+
+put_clk:
+	clk_put(parent);
+	return ret;
+}
+
+static int armada_3700_tbg_clock_remove(struct platform_device *pdev)
+{
+	int i;
+
+	of_clk_del_provider(pdev->dev.of_node);
+	for (i = 0; i < clk_tbg_data.clk_num; i++)
+		clk_unregister_fixed_factor(clk_tbg_data.clks[i]);
+
+	return 0;
+}
+
+static const struct of_device_id armada_3700_tbg_clock_of_match[] = {
+	{ .compatible = "marvell,armada-3700-tbg-clock", },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, armada_3700_tbg_clock_of_match);
+
+static struct platform_driver armada_3700_tbg_clock_driver = {
+	.probe = armada_3700_tbg_clock_probe,
+	.remove = armada_3700_tbg_clock_remove,
+	.driver		= {
+		.name	= "marvell-armada-3700-tbg-clock",
+		.of_match_table = armada_3700_tbg_clock_of_match,
+	},
+};
+
+module_platform_driver(armada_3700_tbg_clock_driver);
+
+MODULE_AUTHOR("Gregory CLEMENT <gregory.clement@free-electrons.com>");
+MODULE_DESCRIPTION("Marvell Armada 37xx SoC Time Base Generator driver");
+MODULE_LICENSE("GPL v2");
-- 
2.5.0

Re: [PATCH 08/10] clk: mvebu Add the time base generator clocks for Armada 3700

[Stephen Boyd]

On 06/10, Gregory CLEMENT wrote:
> +
> +struct tbg_def tbg[NUM_TBG] = {

const?

> +	{"TBG-A-P", TBG_A_REFDIV, TBG_A_FBDIV, TBG_CTRL8, TBG_A_VCODIV_DIFF},
> +	{"TBG-B-P", TBG_B_REFDIV, TBG_B_FBDIV, TBG_CTRL8, TBG_B_VCODIV_DIFF},
> +	{"TBG-A-S", TBG_A_REFDIV, TBG_A_FBDIV, TBG_CTRL1, TBG_A_VCODIV_SE},
> +	{"TBG-B-S", TBG_B_REFDIV, TBG_B_FBDIV, TBG_CTRL1, TBG_B_VCODIV_SE},
> +};
> +
> +static struct clk_onecell_data clk_tbg_data;

Please allocate this in the driver probe.

> +
> +unsigned int tbg_get_mult(void __iomem *reg, struct tbg_def *ptbg)
> +{
> +	u32 val;
> +
> +	val = readl(reg + TBG_CTRL0);
> +
> +	return ((val >> ptbg->fbdiv_offset) & TBG_DIV_MASK) << 2;
> +}
> +
> +unsigned int tbg_get_div(void __iomem *reg, struct tbg_def *ptbg)
> +{
> +	u32 val;
> +	unsigned int div;
> +
> +	val = readl(reg + TBG_CTRL7);
> +
> +	div = (val >> ptbg->refdiv_offset) & TBG_DIV_MASK;
> +	if (div == 0)
> +		div = 1;
> +	val = readl(reg + ptbg->vcodiv_reg);
> +
> +	div *= 1 << ((val >>  ptbg->vcodiv_offset) & TBG_DIV_MASK);
> +
> +	return div;
> +}
> +
> +
> +static int armada_3700_tbg_clock_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device *dev = &pdev->dev;
> +	const char *parent_name;
> +	struct resource *res;
> +	struct clk *parent;
> +	void __iomem *reg;
> +	int i, ret;
> +
> +	parent = of_clk_get(np, 0);

Why not devm_clk_get() instead? Don't make things dependent on DT
APIs unnecessarily please.

> +	if (IS_ERR(parent)) {
> +		dev_err(dev, "Could get the clock parent\n");
> +		return -EINVAL;
> +	}
> +	parent_name = __clk_get_name(parent);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	reg = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(reg)) {
> +		dev_err(dev, "Could not map the tbg clock registers\n");

devm_ioremap_resource() already spits out an error on failure so
please drop this print.

> +		ret = PTR_ERR(reg);
> +		goto put_clk;
> +	}
> +
> +	clk_tbg_data.clk_num = NUM_TBG;
> +	clk_tbg_data.clks = devm_kcalloc(dev, clk_tbg_data.clk_num,
> +					 sizeof(struct clk *), GFP_KERNEL);

Please move to clk_hw based registration.

> +	if (!clk_tbg_data.clks) {
> +		ret = -ENOMEM;
> +		goto put_clk;
> +	}
> +
> +	for (i = 0; i < NUM_TBG; i++) {
> +		const char *name;
> +		unsigned int mult, div;
> +
> +		name = tbg[i].name;
> +		mult = tbg_get_mult(reg, &tbg[i]);
> +		div = tbg_get_div(reg, &tbg[i]);
> +		clk_tbg_data.clks[i] = clk_register_fixed_factor(NULL, name,
> +						 parent_name, 0, mult, div);
> +		if (IS_ERR(clk_tbg_data.clks[i]))
> +			dev_err(dev, "Can't register TBG clock %s\n", name);
> +	}
> +
> +	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_tbg_data);

What if this fails?

> +
> +	return 0;
> +
> +put_clk:
> +	clk_put(parent);
> +	return ret;
> +}

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 09/10] dt-bindings: clock: add DT binding for the peripheral clocks on Armada 3700

[Gregory CLEMENT]

This commit adds the DT binding documentation for the peripheral clocks
used in the Marvell Armada 3700 SoCs.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 .../bindings/clock/armada3700-periph-clock.txt     | 70 ++++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt

diff --git a/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt b/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt
new file mode 100644
index 000000000000..549a5ad14031
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt
@@ -0,0 +1,70 @@
+* Peripheral Clock bindings for Marvell Armada 37xx SoCs
+
+Marvell Armada 37xx SoCs provide peripheral clocks which are
+used as clock source for the peripheral of the SoC.
+
+There are two different block associated to north bridge and south
+bridge.
+
+The peripheral clock consumer should specify the desired clock by
+having the clock ID in its "clocks" phandle cell.
+
+The following is a list of provided IDs for Armada 370 North bridge clocks:
+ID	Clock name	Description
+-----------------------------------
+0	mmc		MMC controller
+1	sata_host	Sata Host
+2	sec_at		Security AT
+3	sac_dap		Security DAP
+4	tsecm		Security Engine
+5	setm_tmx	Serial Embedded Trace Module
+6	avs		Adaptive Voltage Scaling
+7	sqf		SPI
+8	pwm		PWM
+9	i2c_2		I2C 2
+10	i2c_1		I2C 1
+11	ddr_phy		DDR PHY
+12	ddr_fclk	DDR F clock
+13	trace		Trace
+14	counter		Counter
+15	eip97		EIP 97
+16	cpu		CPU
+
+The following is a list of provided IDs for Armada 370 South bridge clocks:
+ID	Clock name	Description
+-----------------------------------
+0	gbe-50		50 MHz parent clock for Gigabit Ethernet
+1	gbe-core	parent clock for Gigabit Ethernet core
+2	gbe-125		125 MHz parent clock for Gigabit Ethernet
+3	gbe1-50		50 MHz clock for Gigabit Ethernet port 1
+4	gbe0-50		50 MHz clock for Gigabit Ethernet port 0
+5	gbe1-125	125 MHz clock for Gigabit Ethernet port 1
+6	gbe0-125	125 MHz clock for Gigabit Ethernet port 0
+7	gbe1-core	Gigabit Ethernet core port 1
+8	gbe0-core	Gigabit Ethernet core port 0
+9	gbe-bm		Gigabit Ethernet Buffer Manager
+10	sdio		SDIO
+11	usb32-sub2-sys	USB 2 clock
+12	usb32-ss-sys	USB 3 clock
+
+Required properties:
+
+- compatible : shall be "marvell,armada-3700-periph-clock-nb" for the
+  north bridge block, or
+  "marvell,armada-3700-periph-clock-sb" for the south bridge block
+- reg : must be the register address of North/South Bridge Clock register
+- #clock-cells : from common clock binding; shall be set to 1
+
+- clocks : list of the parent clock phandle in the following order:
+  TBG-A P, TBG-B P, TBG-A S, TBG-B S and finally the xtal clock.
+
+
+Example:
+
+nb_perih_clk: nb-periph-clk@13000{
+	compatible = "marvell,armada-3700-periph-clock-nb";
+	reg = <0x13000 0x1000>;
+	clocks = <&tbg 0>, <&tbg 1>, <&tbg 2>,
+	<&tbg 3>, <&xtalclk>;
+	#clock-cells = <1>;
+};
-- 
2.5.0

Re: [PATCH 09/10] dt-bindings: clock: add DT binding for the peripheral clocks on Armada 3700

[Rob Herring]

On Fri, Jun 10, 2016 at 03:23:37PM +0200, Gregory CLEMENT wrote:
> This commit adds the DT binding documentation for the peripheral clocks
> used in the Marvell Armada 3700 SoCs.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  .../bindings/clock/armada3700-periph-clock.txt     | 70 ++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt b/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt
> new file mode 100644
> index 000000000000..549a5ad14031
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt
> @@ -0,0 +1,70 @@
> +* Peripheral Clock bindings for Marvell Armada 37xx SoCs
> +
> +Marvell Armada 37xx SoCs provide peripheral clocks which are
> +used as clock source for the peripheral of the SoC.
> +
> +There are two different block associated to north bridge and south

s/block/blocks/

Otherwise,

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 10/10] clk: mvebu: Add the peripheral clock driver for Armada 3700

[Gregory CLEMENT]

These clocks are the ones which will be used as source for the
peripherals of the Armada 3700 SoC. On this SoC there is two blocks of
clocks: the North bridge one and the South bridge one.

Most of them are gatable. Most of the time their rate are their parent
rated divided by a ratio depending of two registers. Their parent can be
choose between the TBG clocks for most of them.

However, some of them can't choose their parent or directly depend of the
xtal clocks. Other ones do not use exactly the same pattern to find the
ratio between their parent rate and their rate.

For theses reason each clock is a composite clock and the operations they
use are different depending of the clock.

According to the dataheet it would be possible to select the parent clock
and the ratio, however currently the driver does not support it.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/clk/mvebu/Makefile             |   1 +
 drivers/clk/mvebu/armada-37xx-periph.c | 462 +++++++++++++++++++++++++++++++++
 2 files changed, 463 insertions(+)
 create mode 100644 drivers/clk/mvebu/armada-37xx-periph.c

diff --git a/drivers/clk/mvebu/Makefile b/drivers/clk/mvebu/Makefile
index 72e3512a9d4a..d9ae97fb43c4 100644
--- a/drivers/clk/mvebu/Makefile
+++ b/drivers/clk/mvebu/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_ARMADA_38X_CLK)	+= armada-38x.o
 obj-$(CONFIG_ARMADA_39X_CLK)	+= armada-39x.o
 obj-$(CONFIG_ARMADA_37XX_CLK)	+= armada-37xx-xtal.o
 obj-$(CONFIG_ARMADA_37XX_CLK)	+= armada-37xx-tbg.o
+obj-$(CONFIG_ARMADA_37XX_CLK)	+= armada-37xx-periph.o
 obj-$(CONFIG_ARMADA_XP_CLK)	+= armada-xp.o
 obj-$(CONFIG_ARMADA_AP806_SYSCON) += ap806-system-controller.o
 obj-$(CONFIG_ARMADA_CP110_SYSCON) += cp110-system-controller.o
diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
new file mode 100644
index 000000000000..cd5150035426
--- /dev/null
+++ b/drivers/clk/mvebu/armada-37xx-periph.c
@@ -0,0 +1,462 @@
+/*
+ * Marvell Armada 37xx SoC Peripheral clocks
+ *
+ * Copyright (C) 2016 Marvell
+ *
+ * Gregory CLEMENT <gregory.clement@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ *
+ * Most of the peripheral clocks can be modelled like this:
+ *             _____    _______    _______
+ * TBG-A-P  --|     |  |       |  |       |   ______
+ * TBG-B-P  --| Mux |--| /div1 |--| /div2 |--| Gate |--> perip_clk
+ * TBG-A-S  --|     |  |       |  |       |  |______|
+ * TBG-B-S  --|_____|  |_______|  |_______|
+ *
+ * However some clocks may use only one or two block or and use the
+ * xtal clock as parent.
+ *
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
+#include <linux/log2.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define PARENT_NUM	5
+
+#define TBG_SEL		0x0
+#define DIV_SEL0	0x4
+#define DIV_SEL1	0x8
+#define DIV_SEL2	0xC
+#define CLK_SEL		0x10
+#define CLK_DIS		0x14
+
+
+#define UNUSED	0xFFFF
+#define XTAL_CHILD	0x1 /* Xtal is the only parent of the clock */
+#define TBGA_S_CHILD	0x2 /* TBG A S is the only parent of the clock */
+#define GBE_CORE_CHILD	0x3 /* GBE core is the only parent of the clock */
+#define GBE_50_CHILD	0x4 /* GBE 50 is the only parent of the clock */
+#define GBE_125_CHILD	0x5 /* GBE 125 is the only parent of the clock */
+
+struct clk_double_div {
+	struct clk_hw hw;
+	void __iomem *reg1;
+	int shift1;
+	void __iomem *reg2;
+	int shift2;
+};
+
+#define to_clk_double_div(_hw) container_of(_hw, struct clk_double_div, hw)
+
+struct clk_periph_data {
+	char *name;
+	int gate_shift;
+	int mux_shift;
+	u32 div_reg1;
+	int div_shift1;
+	u32 div_reg2;
+	int div_shift2;
+	struct clk_div_table *table;
+	int flags;
+};
+
+static struct clk_div_table clk_table6[] = {
+	{ .val = 1, .div = 1, },
+	{ .val = 2, .div = 2, },
+	{ .val = 3, .div = 3, },
+	{ .val = 4, .div = 4, },
+	{ .val = 5, .div = 5, },
+	{ .val = 6, .div = 6, },
+	{ .val = 0, .div = 0, }, /* laste entry */
+};
+
+static struct clk_div_table clk_table1[] = {
+	{ .val = 0, .div = 1, },
+	{ .val = 1, .div = 2, },
+	{ .val = 0, .div = 0, }, /* laste entry */
+};
+
+static struct clk_div_table clk_table2[] = {
+	{ .val = 0, .div = 2, },
+	{ .val = 1, .div = 4, },
+	{ .val = 0, .div = 0, }, /* laste entry */
+};
+
+struct clk_periph_data data_nb[] = {
+	{ .name = "mmc",	.gate_shift =  2, .mux_shift = 0,
+	  .div_reg1 = DIV_SEL2, .div_shift1 = 16, .div_reg2 = DIV_SEL2,
+	  .div_shift2 = 13, .table = NULL, .flags = 0 },
+	{ .name = "sata_host",	.gate_shift =  3, .mux_shift = 2,
+	  .div_reg1 = DIV_SEL2, .div_shift1 = 10, .div_reg2 = DIV_SEL2,
+	  .div_shift2 = 7, .table = NULL, .flags = 0 },
+	{ .name = "sec at",	.gate_shift =  6, .mux_shift = 4,
+	  .div_reg1 = DIV_SEL1, .div_shift1 = 3, .div_reg2 = DIV_SEL1,
+	  .div_shift2 = 0, .table = NULL, .flags = 0 },
+	{ .name = "sec_dap",	.gate_shift =  7, .mux_shift = 6,
+	  .div_reg1 = DIV_SEL1, .div_shift1 = 9, .div_reg2 = DIV_SEL1,
+	  .div_shift2 = 6, .table = NULL, .flags = 0 },
+	{ .name = "tsecm",	.gate_shift =  8, .mux_shift = 8,
+	  .div_reg1 = DIV_SEL1, .div_shift1 = 15, .div_reg2 = DIV_SEL1,
+	  .div_shift2 = 12, .table = NULL, .flags = 0 },
+	{ .name = "setm_tmx",	.gate_shift =  10, .mux_shift = 10,
+	  .div_reg1 = DIV_SEL1, .div_shift1 = 18, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = clk_table6, .flags = 0 },
+	{ .name = "avs",	.gate_shift =  11, .mux_shift = UNUSED,
+	  .div_reg1 = UNUSED, .div_shift1 = 0, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = NULL, .flags = XTAL_CHILD},
+	{ .name = "sqf",	.gate_shift =  12, .mux_shift = 12,
+	  .div_reg1 = DIV_SEL1, .div_shift1 = 27, .div_reg2 = DIV_SEL1,
+	  .div_shift2 = 24, .table = NULL, .flags = 0 },
+	{ .name = "pwm",	.gate_shift =  13, .mux_shift = 14,
+	  .div_reg1 = DIV_SEL0, .div_shift1 = 3, .div_reg2 = DIV_SEL0,
+	  .div_shift2 = 0, .table = NULL, .flags = 0 },
+	{ .name = "i2c_2",	.gate_shift =  16, .mux_shift = UNUSED,
+	  .div_reg1 = UNUSED, .div_shift1 = 0, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = NULL, .flags = XTAL_CHILD },
+	{ .name = "i2c_1",	.gate_shift =  17, .mux_shift = UNUSED,
+	  .div_reg1 = UNUSED, .div_shift1 = 0, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = NULL, .flags = XTAL_CHILD },
+	{ .name = "ddr_phy",	.gate_shift =  19, .mux_shift = UNUSED,
+	  .div_reg1 = DIV_SEL0, .div_shift1 = 18, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = clk_table2, .flags = TBGA_S_CHILD },
+	{ .name = "ddr_fclk",	.gate_shift =  21, .mux_shift = 16,
+	  .div_reg1 = DIV_SEL0, .div_shift1 = 15, .div_reg2 = DIV_SEL0,
+	  .div_shift2 = 12, .table = NULL, .flags = 0 },
+	{ .name = "trace",	.gate_shift =  22, .mux_shift = 18,
+	  .div_reg1 = DIV_SEL0, .div_shift1 = 20, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = clk_table6, .flags = 0 },
+	{ .name = "counter",	.gate_shift =  23, .mux_shift = 20,
+	  .div_reg1 = DIV_SEL0, .div_shift1 = 23, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = clk_table6, .flags = 0 },
+	{ .name = "eip97",	.gate_shift =  24, .mux_shift = 24,
+	  .div_reg1 = DIV_SEL2, .div_shift1 = 22, .div_reg2 = DIV_SEL2,
+	  .div_shift2 = 19, .table = NULL, .flags = 0 },
+	{ .name = "cpu",	.gate_shift =  UNUSED, .mux_shift = 22,
+	  .div_reg1 = DIV_SEL0, .div_shift1 = 28, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = clk_table6, .flags = 0 },
+	{ },
+};
+
+struct clk_periph_data data_sb[] = {
+	{ .name = "gbe-50",	.gate_shift =  UNUSED, .mux_shift = 6,
+	  .div_reg1 = DIV_SEL2, .div_shift1 = 6, .div_reg2 = DIV_SEL2,
+	  .div_shift2 = 9, .table = NULL, .flags = 0 },
+	{ .name = "gbe-core",	.gate_shift =  UNUSED, .mux_shift = 8,
+	  .div_reg1 = DIV_SEL1, .div_shift1 = 18, .div_reg2 = DIV_SEL1,
+	  .div_shift2 = 21, .table = NULL, .flags = 0 },
+	{ .name = "gbe-125",	.gate_shift =  UNUSED, .mux_shift = 10,
+	  .div_reg1 = DIV_SEL1, .div_shift1 = 6, .div_reg2 = DIV_SEL1,
+	  .div_shift2 = 9, .table = NULL, .flags = 0 },
+	{ .name = "gbe1-50",	.gate_shift =  0, .mux_shift = 0,
+	  .div_reg1 = UNUSED, .div_shift1 = 0, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = NULL, .flags = GBE_50_CHILD },
+	{ .name = "gbe0-50",	.gate_shift =  1, .mux_shift = 2,
+	  .div_reg1 = UNUSED, .div_shift1 = 0, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = NULL, .flags = GBE_50_CHILD },
+	{ .name = "gbe1-125",	.gate_shift =  2, .mux_shift = 4,
+	  .div_reg1 = UNUSED, .div_shift1 = 0, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = NULL, .flags = GBE_125_CHILD },
+	{ .name = "gbe0-125",	.gate_shift =  3, .mux_shift = 6,
+	  .div_reg1 = UNUSED, .div_shift1 = 0, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = NULL, .flags = GBE_125_CHILD },
+	{ .name = "gbe1-core",	.gate_shift =  4, .mux_shift = 8,
+	  .div_reg1 = DIV_SEL1, .div_shift1 = 13, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = clk_table1, .flags = GBE_CORE_CHILD },
+	{ .name = "gbe0-core",	.gate_shift =  5, .mux_shift = 10,
+	  .div_reg1 = DIV_SEL1, .div_shift1 = 14, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = clk_table1, .flags = GBE_CORE_CHILD },
+	{ .name = "gbe-bm",	.gate_shift =  12, .mux_shift = UNUSED,
+	  .div_reg1 = DIV_SEL1, .div_shift1 = 0, .div_reg2 = UNUSED,
+	  .div_shift2 = 0, .table = clk_table1, .flags = GBE_CORE_CHILD },
+	{ .name = "sdio",	.gate_shift =  11, .mux_shift = 14,
+	  .div_reg1 = DIV_SEL0, .div_shift1 = 3, .div_reg2 = DIV_SEL0,
+	  .div_shift2 = 6, .table = NULL, .flags = 0 },
+	{ .name = "usb32-usb2-sys",	.gate_shift =  16, .mux_shift = 16,
+	  .div_reg1 = DIV_SEL0, .div_shift1 = 9, .div_reg2 = DIV_SEL0,
+	  .div_shift2 = 12, .table = NULL, .flags = 0 },
+	{ .name = "usb32-ss-sys",	.gate_shift =  17, .mux_shift = 18,
+	  .div_reg1 = DIV_SEL0, .div_shift1 = 15, .div_reg2 = DIV_SEL0,
+	  .div_shift2 = 18, .table = NULL, .flags = 0 },
+	{  },
+};
+
+const char *gbe_name[] = {
+	"gbe-50", "gbe-core", "gbe-125",
+};
+
+struct clk_periph_driver_data {
+	struct clk_onecell_data clk_data;
+	spinlock_t lock;
+};
+
+static int get_div(void __iomem *reg, int shift)
+{
+	u32 val;
+
+	val = (readl(reg) >> shift) & 0x7;
+	if (val > 6)
+		return 0;
+	return (unsigned int)val;
+}
+
+static unsigned long clk_double_div_recalc_rate(struct clk_hw *hw,
+		unsigned long parent_rate)
+{
+	struct clk_double_div *double_div = to_clk_double_div(hw);
+	unsigned int div;
+
+	div = get_div(double_div->reg1, double_div->shift1);
+	div *= get_div(double_div->reg2, double_div->shift2);
+
+	return DIV_ROUND_UP_ULL((u64)parent_rate, div);
+}
+
+const struct clk_ops clk_double_div_ops = {
+	.recalc_rate = clk_double_div_recalc_rate,
+};
+
+static int armada_3700_add_composite_clk(const struct clk_periph_data *data,
+					 const char * const *parent_name,
+					 void __iomem *reg, spinlock_t *lock,
+					 struct device *dev, struct clk *clk)
+{
+	const struct clk_ops *mux_ops = NULL, *gate_ops = NULL,
+		*div_ops = NULL;
+	struct clk_hw *mux_hw = NULL, *gate_hw = NULL, *div_hw = NULL;
+	const char * const *names;
+	struct clk_mux *mux = NULL;
+	struct clk_gate *gate = NULL;
+	struct clk_divider *div = NULL;
+	struct clk_double_div *double_div = NULL;
+	int num_parent;
+	int ret = 0;
+
+	if (data->gate_shift != UNUSED) {
+		gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
+
+		if (!gate)
+			return -ENOMEM;
+
+		gate->reg = reg + CLK_DIS;
+		gate->bit_idx = data->gate_shift;
+		gate->lock = lock;
+		gate_ops = &clk_gate_ops;
+		gate_hw = &gate->hw;
+	}
+
+	if (data->mux_shift != UNUSED) {
+		mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
+
+		if (!mux) {
+			ret = -ENOMEM;
+			goto free_gate;
+		}
+
+		mux->reg = reg + TBG_SEL;
+		mux->shift = data->mux_shift;
+		mux->mask = 0x3;
+		mux->lock = lock;
+		mux_ops = &clk_mux_ro_ops;
+		mux_hw = &mux->hw;
+	}
+
+	if (data->div_reg1 != UNUSED) {
+		if (data->div_reg2 == UNUSED) {
+			const struct clk_div_table *clkt;
+			int table_size = 0;
+
+			div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL);
+			if (!div) {
+				ret = -ENOMEM;
+				goto free_mux;
+			}
+
+			div->reg = reg + data->div_reg1;
+			div->table = data->table;
+			for (clkt = div->table; clkt->div; clkt++)
+				table_size++;
+			div->width = order_base_2(table_size);
+			div->lock = lock;
+			div_ops = &clk_divider_ro_ops;
+			div_hw = &div->hw;
+		} else {
+			double_div = devm_kzalloc(dev, sizeof(*double_div),
+						  GFP_KERNEL);
+			if (!double_div) {
+				ret = -ENOMEM;
+				goto free_mux;
+			}
+
+			double_div->reg1 = reg + data->div_reg1;
+			double_div->shift1 = data->div_shift1;
+			double_div->reg2 = reg + data->div_reg1;
+			double_div->shift2 = data->div_shift2;
+			div_ops = &clk_double_div_ops;
+			div_hw = &double_div->hw;
+		}
+	}
+
+	switch (data->flags) {
+	case XTAL_CHILD:
+		/* the xtal clock is the 5th clock */
+		names = &parent_name[4];
+		num_parent = 1;
+		break;
+	case TBGA_S_CHILD:
+		/* the TBG A S clock is the 3rd clock */
+		names = &parent_name[2];
+		num_parent = 1;
+		break;
+	case GBE_CORE_CHILD:
+		names = &gbe_name[1];
+		num_parent = 1;
+		break;
+	case  GBE_50_CHILD:
+		names = &gbe_name[0];
+		num_parent = 1;
+		break;
+	case  GBE_125_CHILD:
+		names = &gbe_name[2];
+		num_parent = 1;
+		break;
+	default:
+		names = parent_name;
+		num_parent = 4;
+	}
+	clk = clk_register_composite(NULL, data->name,
+				     names, num_parent,
+				     mux_hw, mux_ops,
+				     div_hw, div_ops,
+				     gate_hw, gate_ops,
+				     CLK_IGNORE_UNUSED);
+	if (IS_ERR(clk)) {
+		ret = -EBUSY;
+		goto free_div;
+	}
+
+	return 0;
+free_div:
+	devm_kfree(dev, div);
+	devm_kfree(dev, double_div);
+free_mux:
+	devm_kfree(dev, mux);
+free_gate:
+	devm_kfree(dev, gate);
+	return ret;
+}
+
+static const struct of_device_id armada_3700_periph_clock_of_match[] = {
+	{ .compatible = "marvell,armada-3700-periph-clock-nb",
+	  .data = data_nb, },
+	{ .compatible = "marvell,armada-3700-periph-clock-sb",
+	  .data = data_sb, },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, armada_3700_periph_clock_of_match);
+
+static int armada_3700_periph_clock_probe(struct platform_device *pdev)
+{
+	struct clk_periph_driver_data *driver_data;
+	struct device_node *np = pdev->dev.of_node;
+	const char *parent_name[PARENT_NUM];
+	const struct clk_periph_data *data;
+	const struct of_device_id *device;
+	struct device *dev = &pdev->dev;
+	int num_periph = 0, i, ret;
+	struct resource *res;
+	void __iomem *reg;
+
+	device = of_match_device(armada_3700_periph_clock_of_match, dev);
+	if (!device)
+		return -ENODEV;
+	data = device->data;
+
+	while (data[num_periph].name)
+		num_periph++;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	reg = devm_ioremap_resource(dev, res);
+	if (IS_ERR(reg)) {
+		dev_err(dev, "Could not map the periph clock registers\n");
+		return PTR_ERR(reg);
+	}
+
+	ret = of_clk_parent_fill(np, parent_name, PARENT_NUM);
+	if (ret != PARENT_NUM) {
+		dev_err(dev, "Could not retrieve the parents\n");
+		return -EINVAL;
+	}
+
+	driver_data = devm_kzalloc(dev, sizeof(struct clk_periph_driver_data),
+				   GFP_KERNEL);
+	if (!driver_data)
+		return -ENOMEM;
+
+	driver_data->clk_data.clk_num = num_periph;
+	driver_data->clk_data.clks = devm_kcalloc(dev, num_periph,
+				       sizeof(struct clk *), GFP_KERNEL);
+	if (!driver_data->clk_data.clks)
+		return -ENOMEM;
+
+	spin_lock_init(&driver_data->lock);
+
+	for (i = 0; i < num_periph; i++) {
+		struct clk *clk = driver_data->clk_data.clks[i];
+
+		if (armada_3700_add_composite_clk(&data[i], parent_name, reg,
+						  &driver_data->lock, dev, clk))
+			dev_err(dev, "Can't register periph clock %s\n",
+			       data[i].name);
+	}
+
+	ret = of_clk_add_provider(np, of_clk_src_onecell_get,
+				  &driver_data->clk_data);
+	if (ret) {
+		for (i = 0; i < num_periph; i++)
+			clk_unregister(driver_data->clk_data.clks[i]);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, driver_data);
+	return 0;
+}
+
+static int armada_3700_periph_clock_remove(struct platform_device *pdev)
+{
+	struct clk_periph_driver_data *data = platform_get_drvdata(pdev);
+	struct clk_onecell_data *clk_data = &data->clk_data;
+	int i;
+
+	of_clk_del_provider(pdev->dev.of_node);
+
+	for (i = 0; i < clk_data->clk_num; i++)
+		clk_unregister(clk_data->clks[i]);
+
+	return 0;
+}
+
+static struct platform_driver armada_3700_periph_clock_driver = {
+	.probe = armada_3700_periph_clock_probe,
+	.remove = armada_3700_periph_clock_remove,
+	.driver		= {
+		.name	= "marvell-armada-3700-periph-clock",
+		.of_match_table = armada_3700_periph_clock_of_match,
+	},
+};
+
+module_platform_driver(armada_3700_periph_clock_driver);
+
+MODULE_AUTHOR("Gregory CLEMENT <gregory.clement@free-electrons.com>");
+MODULE_DESCRIPTION("Marvell Armada 37xx SoC Peripheral clocks driver");
+MODULE_LICENSE("GPL v2");
-- 
2.5.0

Re: [PATCH 10/10] clk: mvebu: Add the peripheral clock driver for Armada 3700

[Stephen Boyd]

On 06/10, Gregory CLEMENT wrote:
> These clocks are the ones which will be used as source for the
> peripherals of the Armada 3700 SoC. On this SoC there is two blocks of
> clocks: the North bridge one and the South bridge one.
> 
> Most of them are gatable. Most of the time their rate are their parent
> rated divided by a ratio depending of two registers. Their parent can be
> choose between the TBG clocks for most of them.
> 
> However, some of them can't choose their parent or directly depend of the
> xtal clocks. Other ones do not use exactly the same pattern to find the
> ratio between their parent rate and their rate.
> 
> For theses reason each clock is a composite clock and the operations they

s/theses/these/

> use are different depending of the clock.
> 
> According to the dataheet it would be possible to select the parent clock

s/dataheet/datasheet/

> and the ratio, however currently the driver does not support it.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
> new file mode 100644
> index 000000000000..cd5150035426
> --- /dev/null
> +++ b/drivers/clk/mvebu/armada-37xx-periph.c
> @@ -0,0 +1,462 @@
> +/*
> + * Marvell Armada 37xx SoC Peripheral clocks
> + *
> + * Copyright (C) 2016 Marvell
> + *
> + * Gregory CLEMENT <gregory.clement@free-electrons.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + *
> + * Most of the peripheral clocks can be modelled like this:
> + *             _____    _______    _______
> + * TBG-A-P  --|     |  |       |  |       |   ______
> + * TBG-B-P  --| Mux |--| /div1 |--| /div2 |--| Gate |--> perip_clk
> + * TBG-A-S  --|     |  |       |  |       |  |______|
> + * TBG-B-S  --|_____|  |_______|  |_______|
> + *
> + * However some clocks may use only one or two block or and use the
> + * xtal clock as parent.
> + *
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/clk.h>
> +#include <linux/log2.h>

Is this include used?

> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>

Is this include used?

> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#define PARENT_NUM	5
> +
> +#define TBG_SEL		0x0
> +#define DIV_SEL0	0x4
> +#define DIV_SEL1	0x8
> +#define DIV_SEL2	0xC
> +#define CLK_SEL		0x10
> +#define CLK_DIS		0x14
> +
> +
> +#define UNUSED	0xFFFF
> +#define XTAL_CHILD	0x1 /* Xtal is the only parent of the clock */
> +#define TBGA_S_CHILD	0x2 /* TBG A S is the only parent of the clock */
> +#define GBE_CORE_CHILD	0x3 /* GBE core is the only parent of the clock */
> +#define GBE_50_CHILD	0x4 /* GBE 50 is the only parent of the clock */
> +#define GBE_125_CHILD	0x5 /* GBE 125 is the only parent of the clock */
> +
> +struct clk_double_div {
> +	struct clk_hw hw;
> +	void __iomem *reg1;
> +	int shift1;
> +	void __iomem *reg2;
> +	int shift2;
> +};
> +
> +#define to_clk_double_div(_hw) container_of(_hw, struct clk_double_div, hw)
> +
> +struct clk_periph_data {
> +	char *name;
> +	int gate_shift;
> +	int mux_shift;
> +	u32 div_reg1;
> +	int div_shift1;
> +	u32 div_reg2;
> +	int div_shift2;
> +	struct clk_div_table *table;
> +	int flags;
> +};
> +
> +static struct clk_div_table clk_table6[] = {

const?

> +	{ .val = 1, .div = 1, },
> +	{ .val = 2, .div = 2, },
> +	{ .val = 3, .div = 3, },
> +	{ .val = 4, .div = 4, },
> +	{ .val = 5, .div = 5, },
> +	{ .val = 6, .div = 6, },
> +	{ .val = 0, .div = 0, }, /* laste entry */
> +};
> +
> +static struct clk_div_table clk_table1[] = {

const?

> +	{ .val = 0, .div = 1, },
> +	{ .val = 1, .div = 2, },
> +	{ .val = 0, .div = 0, }, /* laste entry */
> +};
> +
> +static struct clk_div_table clk_table2[] = {

const?

> +	{ .val = 0, .div = 2, },
> +	{ .val = 1, .div = 4, },
> +	{ .val = 0, .div = 0, }, /* laste entry */

s/laste/last/ 3 times

> +};
> +
> +struct clk_periph_data data_nb[] = {

const?

> +
> +struct clk_periph_data data_sb[] = {

const?

> +
> +const char *gbe_name[] = {

const char * const?

> +	"gbe-50", "gbe-core", "gbe-125",
> +};
> +
> +struct clk_periph_driver_data {
> +	struct clk_onecell_data clk_data;
> +	spinlock_t lock;
> +};
> +
> +static int get_div(void __iomem *reg, int shift)

Should this return unsigned instead?

> +{
> +	u32 val;
> +
> +	val = (readl(reg) >> shift) & 0x7;
> +	if (val > 6)
> +		return 0;
> +	return (unsigned int)val;

What is this cast for?

> +}
> +
> +static unsigned long clk_double_div_recalc_rate(struct clk_hw *hw,
> +		unsigned long parent_rate)
> +{
> +	struct clk_double_div *double_div = to_clk_double_div(hw);
> +	unsigned int div;
> +
> +	div = get_div(double_div->reg1, double_div->shift1);
> +	div *= get_div(double_div->reg2, double_div->shift2);
> +
> +	return DIV_ROUND_UP_ULL((u64)parent_rate, div);
> +}
> +
> +const struct clk_ops clk_double_div_ops = {

static?

> +	.recalc_rate = clk_double_div_recalc_rate,
> +};
> +
> +static int armada_3700_add_composite_clk(const struct clk_periph_data *data,
> +					 const char * const *parent_name,
> +					 void __iomem *reg, spinlock_t *lock,
> +					 struct device *dev, struct clk *clk)
> +{
> +	const struct clk_ops *mux_ops = NULL, *gate_ops = NULL,
> +		*div_ops = NULL;
> +	struct clk_hw *mux_hw = NULL, *gate_hw = NULL, *div_hw = NULL;
> +	const char * const *names;
> +	struct clk_mux *mux = NULL;
> +	struct clk_gate *gate = NULL;
> +	struct clk_divider *div = NULL;
> +	struct clk_double_div *double_div = NULL;
> +	int num_parent;
> +	int ret = 0;
> +
> +	if (data->gate_shift != UNUSED) {
> +		gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
> +
> +		if (!gate)
> +			return -ENOMEM;
> +
> +		gate->reg = reg + CLK_DIS;
> +		gate->bit_idx = data->gate_shift;
> +		gate->lock = lock;
> +		gate_ops = &clk_gate_ops;
> +		gate_hw = &gate->hw;
> +	}
> +
> +	if (data->mux_shift != UNUSED) {
> +		mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
> +
> +		if (!mux) {
> +			ret = -ENOMEM;
> +			goto free_gate;
> +		}
> +
> +		mux->reg = reg + TBG_SEL;
> +		mux->shift = data->mux_shift;
> +		mux->mask = 0x3;
> +		mux->lock = lock;
> +		mux_ops = &clk_mux_ro_ops;
> +		mux_hw = &mux->hw;
> +	}
> +
> +	if (data->div_reg1 != UNUSED) {
> +		if (data->div_reg2 == UNUSED) {
> +			const struct clk_div_table *clkt;
> +			int table_size = 0;
> +
> +			div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL);
> +			if (!div) {
> +				ret = -ENOMEM;
> +				goto free_mux;
> +			}
> +
> +			div->reg = reg + data->div_reg1;
> +			div->table = data->table;
> +			for (clkt = div->table; clkt->div; clkt++)
> +				table_size++;
> +			div->width = order_base_2(table_size);
> +			div->lock = lock;
> +			div_ops = &clk_divider_ro_ops;
> +			div_hw = &div->hw;
> +		} else {
> +			double_div = devm_kzalloc(dev, sizeof(*double_div),
> +						  GFP_KERNEL);
> +			if (!double_div) {
> +				ret = -ENOMEM;
> +				goto free_mux;
> +			}
> +
> +			double_div->reg1 = reg + data->div_reg1;
> +			double_div->shift1 = data->div_shift1;
> +			double_div->reg2 = reg + data->div_reg1;
> +			double_div->shift2 = data->div_shift2;
> +			div_ops = &clk_double_div_ops;
> +			div_hw = &double_div->hw;
> +		}
> +	}
> +
> +	switch (data->flags) {
> +	case XTAL_CHILD:
> +		/* the xtal clock is the 5th clock */
> +		names = &parent_name[4];
> +		num_parent = 1;
> +		break;
> +	case TBGA_S_CHILD:
> +		/* the TBG A S clock is the 3rd clock */
> +		names = &parent_name[2];
> +		num_parent = 1;
> +		break;
> +	case GBE_CORE_CHILD:
> +		names = &gbe_name[1];
> +		num_parent = 1;
> +		break;
> +	case  GBE_50_CHILD:
> +		names = &gbe_name[0];
> +		num_parent = 1;
> +		break;
> +	case  GBE_125_CHILD:
> +		names = &gbe_name[2];
> +		num_parent = 1;
> +		break;
> +	default:
> +		names = parent_name;
> +		num_parent = 4;
> +	}
> +	clk = clk_register_composite(NULL, data->name,

Please pass a dev here, it may be useful one day.

> +				     names, num_parent,
> +				     mux_hw, mux_ops,
> +				     div_hw, div_ops,
> +				     gate_hw, gate_ops,
> +				     CLK_IGNORE_UNUSED);
> +	if (IS_ERR(clk)) {
> +		ret = -EBUSY;

Why not ret = PTR_ERR(clk)?

> +		goto free_div;
> +	}
> +
> +	return 0;
> +free_div:
> +	devm_kfree(dev, div);
> +	devm_kfree(dev, double_div);
> +free_mux:
> +	devm_kfree(dev, mux);
> +free_gate:
> +	devm_kfree(dev, gate);
> +	return ret;
> +}
> +
> +static const struct of_device_id armada_3700_periph_clock_of_match[] = {
> +	{ .compatible = "marvell,armada-3700-periph-clock-nb",
> +	  .data = data_nb, },
> +	{ .compatible = "marvell,armada-3700-periph-clock-sb",
> +	  .data = data_sb, },
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, armada_3700_periph_clock_of_match);
> +
> +static int armada_3700_periph_clock_probe(struct platform_device *pdev)
> +{
> +	struct clk_periph_driver_data *driver_data;
> +	struct device_node *np = pdev->dev.of_node;
> +	const char *parent_name[PARENT_NUM];
> +	const struct clk_periph_data *data;
> +	const struct of_device_id *device;
> +	struct device *dev = &pdev->dev;
> +	int num_periph = 0, i, ret;
> +	struct resource *res;
> +	void __iomem *reg;
> +
> +	device = of_match_device(armada_3700_periph_clock_of_match, dev);
> +	if (!device)
> +		return -ENODEV;
> +	data = device->data;

We have of_device_get_match_data() to simplify this.

> +
> +	while (data[num_periph].name)
> +		num_periph++;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	reg = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(reg)) {
> +		dev_err(dev, "Could not map the periph clock registers\n");
> +		return PTR_ERR(reg);
> +	}
> +
> +	ret = of_clk_parent_fill(np, parent_name, PARENT_NUM);
> +	if (ret != PARENT_NUM) {
> +		dev_err(dev, "Could not retrieve the parents\n");
> +		return -EINVAL;
> +	}
> +
> +	driver_data = devm_kzalloc(dev, sizeof(struct clk_periph_driver_data),

sizeof(*driver_data) please

> +				   GFP_KERNEL);
> +	if (!driver_data)
> +		return -ENOMEM;
> +
> +	driver_data->clk_data.clk_num = num_periph;
> +	driver_data->clk_data.clks = devm_kcalloc(dev, num_periph,
> +				       sizeof(struct clk *), GFP_KERNEL);
> +	if (!driver_data->clk_data.clks)
> +		return -ENOMEM;

Again, move to clk_hw based registration.

> +
> +	spin_lock_init(&driver_data->lock);
> +
> +	for (i = 0; i < num_periph; i++) {
> +		struct clk *clk = driver_data->clk_data.clks[i];
> +
> +		if (armada_3700_add_composite_clk(&data[i], parent_name, reg,
> +						  &driver_data->lock, dev, clk))
> +			dev_err(dev, "Can't register periph clock %s\n",
> +			       data[i].name);
> +	}
> +
> +	ret = of_clk_add_provider(np, of_clk_src_onecell_get,
> +				  &driver_data->clk_data);
> +	if (ret) {
> +		for (i = 0; i < num_periph; i++)
> +			clk_unregister(driver_data->clk_data.clks[i]);

It would be nice if we had devm_* registration in composite
clks

> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, driver_data);
> +	return 0;
> +}

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 10/10] clk: mvebu: Add the peripheral clock driver for Armada 3700

[Gregory CLEMENT]

Hi Stephen,
 
 On jeu., juin 30 2016, Stephen Boyd <sboyd@codeaurora.org> wrote:

>> +
>> +	spin_lock_init(&driver_data->lock);
>> +
>> +	for (i = 0; i < num_periph; i++) {
>> +		struct clk *clk = driver_data->clk_data.clks[i];
>> +
>> +		if (armada_3700_add_composite_clk(&data[i], parent_name, reg,
>> +						  &driver_data->lock, dev, clk))
>> +			dev_err(dev, "Can't register periph clock %s\n",
>> +			       data[i].name);
>> +	}
>> +
>> +	ret = of_clk_add_provider(np, of_clk_src_onecell_get,
>> +				  &driver_data->clk_data);
>> +	if (ret) {
>> +		for (i = 0; i < num_periph; i++)
>> +			clk_unregister(driver_data->clk_data.clks[i]);
>
> It would be nice if we had devm_* registration in composite
> clks
>

This comment is the only one I didn't take into account in the second
version I've just sent. I agree it would be nice and I am willing to add
it. However these last days I was very busy with the birth of my last
child: I managed to find time to write this second version in time to be
merged for 4.8 but not enough to add these new function. I will try to
send a patch for it next week.

Thanks for your review,

Gregory


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 00/10] Add clock support for Armada 37xx SoCs

[Gregory CLEMENT]

Hi,
 
 On ven., juin 10 2016, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote:

> Hi,
>
> this series add clock support for the Armada 37xx SoCs.
>
> The design of the drivers is as close as possible as the hardware is,
> with some clocks made of several layers: muxing, divider and gating.
>
> The device tree binding was written in a way that even if we discover
> some change inside the clocks, the binding should not be
> affected. Especially, there are some holes in the clocks, but we
> should be able to add them seamless.
>
> I tried to follow the last update made in the clock framework, I hope
> theses drivers will comply the new guidelines.

I applied the first 4 patches on the mvebu branches, the first one on
mvebu/arm64 and the 3 other on mvebu/dt64. I did this in order to not
miss the arm-soc merge window.

The binding was acked by the dt maintainer and the review of the clk
maintainer was abut the internal of the driver. The only change I did
was fixing a typo in the address part of the name of the sb-periph-clk
node as pointed by Victor Gu. I will submit a second version soon
addressing these concerns.

Gregory

>
> Gregory CLEMENT (10):
>   arm64: marvell: enable Armada 3700 clock drivers
>   arm64: dts: marvell: Add xtal clock support for Armada 3700
>   arm64: dts: marvell: add tbg clocks for Armada 37xx
>   arm64: dts: marvell: add peripherals clocks for Armada 37xx
>   dt-bindings: clock: add DT binding for the Xtal clock on Armada 3700
>   clk: mvebu: Add the xtal clock for Armada 3700 SoC
>   dt-bindings: clock: add DT binding for the TBG clocks on Armada 3700
>   clk: mvebu Add the time base generator clocks for Armada 3700
>   dt-bindings: clock: add DT binding for the peripheral clocks on Armada
>     3700
>   clk: mvebu: Add the peripheral clock driver for Armada 3700
>
>  .../bindings/clock/armada3700-periph-clock.txt     |  70 ++++
>  .../bindings/clock/armada3700-tbg-clock.txt        |  27 ++
>  .../bindings/clock/armada3700-xtal-clock.txt       |  28 ++
>  arch/arm64/Kconfig.platforms                       |   1 +
>  arch/arm64/boot/dts/marvell/armada-37xx.dtsi       |  35 ++
>  drivers/clk/mvebu/Kconfig                          |   3 +
>  drivers/clk/mvebu/Makefile                         |   3 +
>  drivers/clk/mvebu/armada-37xx-periph.c             | 462 +++++++++++++++++++++
>  drivers/clk/mvebu/armada-37xx-tbg.c                | 172 ++++++++
>  drivers/clk/mvebu/armada-37xx-xtal.c               |  93 +++++
>  10 files changed, 894 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt
>  create mode 100644 Documentation/devicetree/bindings/clock/armada3700-tbg-clock.txt
>  create mode 100644 Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
>  create mode 100644 drivers/clk/mvebu/armada-37xx-periph.c
>  create mode 100644 drivers/clk/mvebu/armada-37xx-tbg.c
>  create mode 100644 drivers/clk/mvebu/armada-37xx-xtal.c
>
> -- 
> 2.5.0
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com