[PATCH v2 0/3] add the reset controller to the Meson8b clkc

[PATCH v2 0/3] add the reset controller to the Meson8b clkc

[Martin Blumenstingl]

This registers the known (soft) reset lines provided by the clock
controller's registers.

This is the first preparation step for SMP and CPU hotplug support on
Meson8/Meson8b/Meson8m2. Booting the secondary cores on these SoCs
requires asserting and de-asserting a reset line (one for each CPU
core). These reset lines are provided by the clock controller.

The reset controller part of the meson8b clock controller has to be
registered early (which I did through CLK_OF_DECLARE_DRIVER), because
the secondary cores are started *very* early in the boot process (and
meson8b_clkc_probe is invoked long after we need the reset controller
to be available for booting the secondary CPU cores).

The user of the reset-controller (= the patches which enable SMP and
CPU hotplug support) will follow in the next days. I decided to split
this because the SMP series will probably consist of 6 patches alone
(and may need to go through two separate trees).


Changes since v1 at [0]:
- updated cover letter description as we are now registering more than
  four reset lines
- split patch #1 into a dt-binding and clk driver patch
- slightly reworded the dt-binding documentation so it's now clear that
  the reset identifiers are preprocessor macros in
  dt-bindings/clock/meson8b-clkc.h (v1 of this series didn't have these
  macros at all)
- patch #2 (previously part of patch #1) now registers all known reset
  lines (see [1] and [2] for the results of my detective work)
- patch #3 is untouched


[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-July/004283.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2017-July/004330.html
[2] http://lists.infradead.org/pipermail/linux-amlogic/2017-July/004347.html


Martin Blumenstingl (3):
  dt-bindings: clock: meson8b: describe the embedded reset controller
  clk: meson: meson8b: register the built-in reset controller
  ARM: dts: meson: mark the clock controller also as reset controller

 .../bindings/clock/amlogic,meson8b-clkc.txt        |   8 +-
 arch/arm/boot/dts/meson8.dtsi                      |   1 +
 arch/arm/boot/dts/meson8b.dtsi                     |   1 +
 drivers/clk/meson/Kconfig                          |   1 +
 drivers/clk/meson/meson8b.c                        | 159 +++++++++++++++++++--
 drivers/clk/meson/meson8b.h                        |  25 +++-
 6 files changed, 181 insertions(+), 14 deletions(-)

-- 
2.13.3

[PATCH v2 1/3] dt-bindings: clock: meson8b: describe the embedded reset controller

[Martin Blumenstingl]

The Amlogic Meson8/Meson8b/Meson8m2 clock controller provides some reset
lines. These are used for example to boot the secondary CPU cores.

This patch describes the reset controller which is embedded into the
clock controller on these SoCs.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
index 606da38c0959..e1af4764114d 100644
--- a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
+++ b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
@@ -16,18 +16,24 @@ Required Properties:
 	   mapped region.
 
 - #clock-cells: should be 1.
+- #reset-cells: should be 1.
 
 Each clock is assigned an identifier and client nodes can use this identifier
 to specify the clock which they consume. All available clocks are defined as
 preprocessor macros in the dt-bindings/clock/meson8b-clkc.h header and can be
 used in device tree sources.
+This is also valid for the reset lines provided by the clock controller: a
+preprocessor macro for each reset line is defined (in the same header file as
+the clock identifiers).
+
 
 Example: Clock controller node:
 
 	clkc: clock-controller@c1104000 {
-		#clock-cells = <1>;
 		compatible = "amlogic,meson8b-clkc";
 		reg = <0xc1108000 0x4>, <0xc1104000 0x460>;
+		#clock-cells = <1>;
+		#reset-cells = <1>;
 	};
 
 
-- 
2.13.3

Re: [PATCH v2 1/3] dt-bindings: clock: meson8b: describe the embedded reset controller

[Neil Armstrong]

Le 22/07/2017 20:58, Martin Blumenstingl a écrit :
> The Amlogic Meson8/Meson8b/Meson8m2 clock controller provides some reset
> lines. These are used for example to boot the secondary CPU cores.
> 
> This patch describes the reset controller which is embedded into the
> clock controller on these SoCs.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
> index 606da38c0959..e1af4764114d 100644
> --- a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
> +++ b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
> @@ -16,18 +16,24 @@ Required Properties:
>  	   mapped region.
>  
>  - #clock-cells: should be 1.
> +- #reset-cells: should be 1.
>  
>  Each clock is assigned an identifier and client nodes can use this identifier
>  to specify the clock which they consume. All available clocks are defined as
>  preprocessor macros in the dt-bindings/clock/meson8b-clkc.h header and can be
>  used in device tree sources.
> +This is also valid for the reset lines provided by the clock controller: a
> +preprocessor macro for each reset line is defined (in the same header file as
> +the clock identifiers).
> +
>  
>  Example: Clock controller node:
>  
>  	clkc: clock-controller@c1104000 {
> -		#clock-cells = <1>;
>  		compatible = "amlogic,meson8b-clkc";
>  		reg = <0xc1108000 0x4>, <0xc1104000 0x460>;
> +		#clock-cells = <1>;
> +		#reset-cells = <1>;
>  	};
>  
>  
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

[PATCH v2 2/3] clk: meson: meson8b: register the built-in reset controller

[Martin Blumenstingl]

The clock controller also includes some reset lines. This patch
implements a reset controller to assert and de-assert these resets.
The reset controller itself is registered early (through
CLK_OF_DECLARE_DRIVER) because it is needed very early in the boot
process (to start the secondary CPU cores).

According to the public S805 datasheet there are two more reset bits
in the HHI_SYS_CPU_CLK_CNTL0 register, which are not implemented by
this patch (as these seem to be unused in Amlogic's vendor Linux kernel
sources and their u-boot tree):
- bit 15: GEN_DIV_SOFT_RESET
- bit 14: SOFT_RESET

All information was taken from the public S805 Datasheet and Amlogic's
vendor GPL kernel sources. This patch is based on an earlier version
submitted by Carlo Caione.

Suggested-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 drivers/clk/meson/Kconfig   |   1 +
 drivers/clk/meson/meson8b.c | 159 ++++++++++++++++++++++++++++++++++++++++----
 drivers/clk/meson/meson8b.h |  25 ++++++-
 3 files changed, 172 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 5588f75a8414..d2d0174a6eca 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -6,6 +6,7 @@ config COMMON_CLK_AMLOGIC
 config COMMON_CLK_MESON8B
 	bool
 	depends on COMMON_CLK_AMLOGIC
+	select RESET_CONTROLLER
 	help
 	  Support for the clock controller on AmLogic S802 (Meson8),
 	  S805 (Meson8b) and S812 (Meson8m2) devices. Say Y if you
diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index bb3f1de876b1..0f853057885c 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -25,6 +25,8 @@
 #include <linux/clk-provider.h>
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
 #include <linux/init.h>
 
 #include "clkc.h"
@@ -32,6 +34,13 @@
 
 static DEFINE_SPINLOCK(clk_lock);
 
+static void __iomem *clk_base;
+
+struct meson8b_clk_reset {
+	struct reset_controller_dev reset;
+	void __iomem *base;
+};
+
 static const struct pll_rate_table sys_pll_rate_table[] = {
 	PLL_RATE(312000000, 52, 1, 2),
 	PLL_RATE(336000000, 56, 1, 2),
@@ -690,20 +699,114 @@ static struct clk_divider *const meson8b_clk_dividers[] = {
 	&meson8b_mpeg_clk_div,
 };
 
+static const struct meson8b_clk_reset_line {
+	u32 reg;
+	u8 bit_idx;
+} meson8b_clk_reset_bits[] = {
+	[RESETID_L2_CACHE_SOFT_RESET] = {
+		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 30
+	},
+	[RESETID_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET] = {
+		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 29
+	},
+	[RESETID_SCU_SOFT_RESET] = {
+		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 28
+	},
+	[RESETID_CPU3_SOFT_RESET] = {
+		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 27
+	},
+	[RESETID_CPU2_SOFT_RESET] = {
+		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 26
+	},
+	[RESETID_CPU1_SOFT_RESET] = {
+		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 25
+	},
+	[RESETID_CPU0_SOFT_RESET] = {
+		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 24
+	},
+	[RESETID_A5_GLOBAL_RESET] = {
+		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 18
+	},
+	[RESETID_A5_AXI_SOFT_RESET] = {
+		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 17
+	},
+	[RESETID_A5_ABP_SOFT_RESET] = {
+		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 16
+	},
+	[RESETID_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET] = {
+		.reg = HHI_SYS_CPU_CLK_CNTL1, .bit_idx = 30
+	},
+	[RESETID_VID_CLK_CNTL_SOFT_RESET] = {
+		.reg = HHI_VID_CLK_CNTL, .bit_idx = 15
+	},
+	[RESETID_VID_DIVIDER_CNTL_SOFT_RESET_POST] = {
+		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 7
+	},
+	[RESETID_VID_DIVIDER_CNTL_SOFT_RESET_PRE] = {
+		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 3
+	},
+	[RESETID_VID_DIVIDER_CNTL_RESET_N_POST] = {
+		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 1
+	},
+	[RESETID_VID_DIVIDER_CNTL_RESET_N_PRE] = {
+		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 0
+	},
+};
+
+static int meson8b_clk_reset_update(struct reset_controller_dev *rcdev,
+				    unsigned long id, bool assert)
+{
+	struct meson8b_clk_reset *meson8b_clk_reset =
+		container_of(rcdev, struct meson8b_clk_reset, reset);
+	unsigned long flags;
+	const struct meson8b_clk_reset_line *reset;
+	u32 val;
+
+	if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
+		return -EINVAL;
+
+	reset = &meson8b_clk_reset_bits[id];
+
+	spin_lock_irqsave(&clk_lock, flags);
+
+	val = readl(meson8b_clk_reset->base + reset->reg);
+	if (assert)
+		val |= BIT(reset->bit_idx);
+	else
+		val &= ~BIT(reset->bit_idx);
+	writel(val, meson8b_clk_reset->base + reset->reg);
+
+	spin_unlock_irqrestore(&clk_lock, flags);
+
+	return 0;
+}
+
+static int meson8b_clk_reset_assert(struct reset_controller_dev *rcdev,
+				     unsigned long id)
+{
+	return meson8b_clk_reset_update(rcdev, id, true);
+}
+
+static int meson8b_clk_reset_deassert(struct reset_controller_dev *rcdev,
+				       unsigned long id)
+{
+	return meson8b_clk_reset_update(rcdev, id, false);
+}
+
+static const struct reset_control_ops meson8b_clk_reset_ops = {
+	.assert = meson8b_clk_reset_assert,
+	.deassert = meson8b_clk_reset_deassert,
+};
+
 static int meson8b_clkc_probe(struct platform_device *pdev)
 {
-	void __iomem *clk_base;
 	int ret, clkid, i;
 	struct clk_hw *parent_hw;
 	struct clk *parent_clk;
 	struct device *dev = &pdev->dev;
 
-	/*  Generic clocks and PLLs */
-	clk_base = of_iomap(dev->of_node, 1);
-	if (!clk_base) {
-		pr_err("%s: Unable to map clk base\n", __func__);
+	if (!clk_base)
 		return -ENXIO;
-	}
 
 	/* Populate base address for PLLs */
 	for (i = 0; i < ARRAY_SIZE(meson8b_clk_plls); i++)
@@ -743,7 +846,7 @@ static int meson8b_clkc_probe(struct platform_device *pdev)
 		/* FIXME convert to devm_clk_register */
 		ret = devm_clk_hw_register(dev, meson8b_hw_onecell_data.hws[clkid]);
 		if (ret)
-			goto iounmap;
+			return ret;
 	}
 
 	/*
@@ -766,15 +869,11 @@ static int meson8b_clkc_probe(struct platform_device *pdev)
 	if (ret) {
 		pr_err("%s: failed to register clock notifier for cpu_clk\n",
 				__func__);
-		goto iounmap;
+		return ret;
 	}
 
 	return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
 			&meson8b_hw_onecell_data);
-
-iounmap:
-	iounmap(clk_base);
-	return ret;
 }
 
 static const struct of_device_id meson8b_clkc_match_table[] = {
@@ -793,3 +892,39 @@ static struct platform_driver meson8b_driver = {
 };
 
 builtin_platform_driver(meson8b_driver);
+
+static void __init meson8b_clkc_reset_init(struct device_node *np)
+{
+	struct meson8b_clk_reset *rstc;
+	int ret;
+
+	/* Generic clocks, PLLs and some of the reset-bits */
+	clk_base = of_iomap(np, 1);
+	if (!clk_base) {
+		pr_err("%s: Unable to map clk base\n", __func__);
+		return;
+	}
+
+	rstc = kzalloc(sizeof(*rstc), GFP_KERNEL);
+	if (!rstc)
+		return;
+
+	/* Reset Controller */
+	rstc->base = clk_base;
+	rstc->reset.ops = &meson8b_clk_reset_ops;
+	rstc->reset.nr_resets = ARRAY_SIZE(meson8b_clk_reset_bits);
+	rstc->reset.of_node = np;
+	ret = reset_controller_register(&rstc->reset);
+	if (ret) {
+		pr_err("%s: Failed to register clkc reset controller: %d\n",
+		       __func__, ret);
+		return;
+	}
+}
+
+CLK_OF_DECLARE_DRIVER(meson8_clkc, "amlogic,meson8-clkc",
+		      meson8b_clkc_reset_init);
+CLK_OF_DECLARE_DRIVER(meson8b_clkc, "amlogic,meson8b-clkc",
+		      meson8b_clkc_reset_init);
+CLK_OF_DECLARE_DRIVER(meson8m2_clkc, "amlogic,meson8m2-clkc",
+		      meson8b_clkc_reset_init);
diff --git a/drivers/clk/meson/meson8b.h b/drivers/clk/meson/meson8b.h
index a687e02547dc..5f4d8e49dd4d 100644
--- a/drivers/clk/meson/meson8b.h
+++ b/drivers/clk/meson/meson8b.h
@@ -37,6 +37,9 @@
 #define HHI_GCLK_AO			0x154 /* 0x55 offset in data sheet */
 #define HHI_SYS_CPU_CLK_CNTL1		0x15c /* 0x57 offset in data sheet */
 #define HHI_MPEG_CLK_CNTL		0x174 /* 0x5d offset in data sheet */
+#define HHI_VID_CLK_CNTL		0x17c /* 0x5f offset in data sheet */
+#define HHI_VID_DIVIDER_CNTL		0x198 /* 0x66 offset in data sheet */
+#define HHI_SYS_CPU_CLK_CNTL0		0x19c /* 0x67 offset in data sheet */
 #define HHI_MPLL_CNTL			0x280 /* 0xa0 offset in data sheet */
 #define HHI_SYS_PLL_CNTL		0x300 /* 0xc0 offset in data sheet */
 #define HHI_VID_PLL_CNTL		0x320 /* 0xc8 offset in data sheet */
@@ -163,7 +166,27 @@
 
 #define CLK_NR_CLKS		96
 
-/* include the CLKIDs that have been made part of the stable DT binding */
+#define RESETID_L2_CACHE_SOFT_RESET				0
+#define RESETID_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET		1
+#define RESETID_SCU_SOFT_RESET					2
+#define RESETID_CPU0_SOFT_RESET					3
+#define RESETID_CPU1_SOFT_RESET					4
+#define RESETID_CPU2_SOFT_RESET					5
+#define RESETID_CPU3_SOFT_RESET					6
+#define RESETID_A5_GLOBAL_RESET					7
+#define RESETID_A5_AXI_SOFT_RESET				8
+#define RESETID_A5_ABP_SOFT_RESET				9
+#define RESETID_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET		10
+#define RESETID_VID_CLK_CNTL_SOFT_RESET				11
+#define RESETID_VID_DIVIDER_CNTL_SOFT_RESET_POST		12
+#define RESETID_VID_DIVIDER_CNTL_SOFT_RESET_PRE			13
+#define RESETID_VID_DIVIDER_CNTL_RESET_N_POST			14
+#define RESETID_VID_DIVIDER_CNTL_RESET_N_PRE			15
+
+/*
+ * include the CLKID and RESETID that have
+ * been made part of the stable DT binding
+ */
 #include <dt-bindings/clock/meson8b-clkc.h>
 
 #endif /* __MESON8B_H */
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] clk: meson: meson8b: register the built-in reset controller

[Neil Armstrong]

Le 22/07/2017 20:58, Martin Blumenstingl a écrit :
> The clock controller also includes some reset lines. This patch
> implements a reset controller to assert and de-assert these resets.
> The reset controller itself is registered early (through
> CLK_OF_DECLARE_DRIVER) because it is needed very early in the boot
> process (to start the secondary CPU cores).
> 
> According to the public S805 datasheet there are two more reset bits
> in the HHI_SYS_CPU_CLK_CNTL0 register, which are not implemented by
> this patch (as these seem to be unused in Amlogic's vendor Linux kernel
> sources and their u-boot tree):
> - bit 15: GEN_DIV_SOFT_RESET
> - bit 14: SOFT_RESET
> 
> All information was taken from the public S805 Datasheet and Amlogic's
> vendor GPL kernel sources. This patch is based on an earlier version
> submitted by Carlo Caione.
> 
> Suggested-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  drivers/clk/meson/Kconfig   |   1 +
>  drivers/clk/meson/meson8b.c | 159 ++++++++++++++++++++++++++++++++++++++++----
>  drivers/clk/meson/meson8b.h |  25 ++++++-
>  3 files changed, 172 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index 5588f75a8414..d2d0174a6eca 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -6,6 +6,7 @@ config COMMON_CLK_AMLOGIC
>  config COMMON_CLK_MESON8B
>  	bool
>  	depends on COMMON_CLK_AMLOGIC
> +	select RESET_CONTROLLER
>  	help
>  	  Support for the clock controller on AmLogic S802 (Meson8),
>  	  S805 (Meson8b) and S812 (Meson8m2) devices. Say Y if you
> diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
> index bb3f1de876b1..0f853057885c 100644
> --- a/drivers/clk/meson/meson8b.c
> +++ b/drivers/clk/meson/meson8b.c
> @@ -25,6 +25,8 @@
>  #include <linux/clk-provider.h>
>  #include <linux/of_address.h>
>  #include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
>  #include <linux/init.h>
>  
>  #include "clkc.h"
> @@ -32,6 +34,13 @@
>  
>  static DEFINE_SPINLOCK(clk_lock);
>  
> +static void __iomem *clk_base;
> +
> +struct meson8b_clk_reset {
> +	struct reset_controller_dev reset;
> +	void __iomem *base;
> +};
> +
>  static const struct pll_rate_table sys_pll_rate_table[] = {
>  	PLL_RATE(312000000, 52, 1, 2),
>  	PLL_RATE(336000000, 56, 1, 2),
> @@ -690,20 +699,114 @@ static struct clk_divider *const meson8b_clk_dividers[] = {
>  	&meson8b_mpeg_clk_div,
>  };
>  
> +static const struct meson8b_clk_reset_line {
> +	u32 reg;
> +	u8 bit_idx;
> +} meson8b_clk_reset_bits[] = {
> +	[RESETID_L2_CACHE_SOFT_RESET] = {
> +		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 30
> +	},
> +	[RESETID_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET] = {
> +		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 29
> +	},
> +	[RESETID_SCU_SOFT_RESET] = {
> +		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 28
> +	},
> +	[RESETID_CPU3_SOFT_RESET] = {
> +		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 27
> +	},
> +	[RESETID_CPU2_SOFT_RESET] = {
> +		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 26
> +	},
> +	[RESETID_CPU1_SOFT_RESET] = {
> +		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 25
> +	},
> +	[RESETID_CPU0_SOFT_RESET] = {
> +		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 24
> +	},
> +	[RESETID_A5_GLOBAL_RESET] = {
> +		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 18
> +	},
> +	[RESETID_A5_AXI_SOFT_RESET] = {
> +		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 17
> +	},
> +	[RESETID_A5_ABP_SOFT_RESET] = {
> +		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 16
> +	},
> +	[RESETID_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET] = {
> +		.reg = HHI_SYS_CPU_CLK_CNTL1, .bit_idx = 30
> +	},
> +	[RESETID_VID_CLK_CNTL_SOFT_RESET] = {
> +		.reg = HHI_VID_CLK_CNTL, .bit_idx = 15
> +	},
> +	[RESETID_VID_DIVIDER_CNTL_SOFT_RESET_POST] = {
> +		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 7
> +	},
> +	[RESETID_VID_DIVIDER_CNTL_SOFT_RESET_PRE] = {
> +		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 3
> +	},
> +	[RESETID_VID_DIVIDER_CNTL_RESET_N_POST] = {
> +		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 1
> +	},
> +	[RESETID_VID_DIVIDER_CNTL_RESET_N_PRE] = {
> +		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 0
> +	},
> +};
> +
> +static int meson8b_clk_reset_update(struct reset_controller_dev *rcdev,
> +				    unsigned long id, bool assert)
> +{
> +	struct meson8b_clk_reset *meson8b_clk_reset =
> +		container_of(rcdev, struct meson8b_clk_reset, reset);
> +	unsigned long flags;
> +	const struct meson8b_clk_reset_line *reset;
> +	u32 val;
> +
> +	if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
> +		return -EINVAL;
> +
> +	reset = &meson8b_clk_reset_bits[id];
> +
> +	spin_lock_irqsave(&clk_lock, flags);
> +
> +	val = readl(meson8b_clk_reset->base + reset->reg);
> +	if (assert)
> +		val |= BIT(reset->bit_idx);
> +	else
> +		val &= ~BIT(reset->bit_idx);
> +	writel(val, meson8b_clk_reset->base + reset->reg);
> +
> +	spin_unlock_irqrestore(&clk_lock, flags);
> +
> +	return 0;
> +}
> +
> +static int meson8b_clk_reset_assert(struct reset_controller_dev *rcdev,
> +				     unsigned long id)
> +{
> +	return meson8b_clk_reset_update(rcdev, id, true);
> +}
> +
> +static int meson8b_clk_reset_deassert(struct reset_controller_dev *rcdev,
> +				       unsigned long id)
> +{
> +	return meson8b_clk_reset_update(rcdev, id, false);
> +}
> +
> +static const struct reset_control_ops meson8b_clk_reset_ops = {
> +	.assert = meson8b_clk_reset_assert,
> +	.deassert = meson8b_clk_reset_deassert,
> +};
> +
>  static int meson8b_clkc_probe(struct platform_device *pdev)
>  {
> -	void __iomem *clk_base;
>  	int ret, clkid, i;
>  	struct clk_hw *parent_hw;
>  	struct clk *parent_clk;
>  	struct device *dev = &pdev->dev;
>  
> -	/*  Generic clocks and PLLs */
> -	clk_base = of_iomap(dev->of_node, 1);
> -	if (!clk_base) {
> -		pr_err("%s: Unable to map clk base\n", __func__);
> +	if (!clk_base)
>  		return -ENXIO;
> -	}
>  
>  	/* Populate base address for PLLs */
>  	for (i = 0; i < ARRAY_SIZE(meson8b_clk_plls); i++)
> @@ -743,7 +846,7 @@ static int meson8b_clkc_probe(struct platform_device *pdev)
>  		/* FIXME convert to devm_clk_register */
>  		ret = devm_clk_hw_register(dev, meson8b_hw_onecell_data.hws[clkid]);
>  		if (ret)
> -			goto iounmap;
> +			return ret;
>  	}
>  
>  	/*
> @@ -766,15 +869,11 @@ static int meson8b_clkc_probe(struct platform_device *pdev)
>  	if (ret) {
>  		pr_err("%s: failed to register clock notifier for cpu_clk\n",
>  				__func__);
> -		goto iounmap;
> +		return ret;
>  	}
>  
>  	return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
>  			&meson8b_hw_onecell_data);
> -
> -iounmap:
> -	iounmap(clk_base);
> -	return ret;
>  }
>  
>  static const struct of_device_id meson8b_clkc_match_table[] = {
> @@ -793,3 +892,39 @@ static struct platform_driver meson8b_driver = {
>  };
>  
>  builtin_platform_driver(meson8b_driver);
> +
> +static void __init meson8b_clkc_reset_init(struct device_node *np)
> +{
> +	struct meson8b_clk_reset *rstc;
> +	int ret;
> +
> +	/* Generic clocks, PLLs and some of the reset-bits */
> +	clk_base = of_iomap(np, 1);
> +	if (!clk_base) {
> +		pr_err("%s: Unable to map clk base\n", __func__);
> +		return;
> +	}
> +
> +	rstc = kzalloc(sizeof(*rstc), GFP_KERNEL);
> +	if (!rstc)
> +		return;
> +
> +	/* Reset Controller */
> +	rstc->base = clk_base;
> +	rstc->reset.ops = &meson8b_clk_reset_ops;
> +	rstc->reset.nr_resets = ARRAY_SIZE(meson8b_clk_reset_bits);
> +	rstc->reset.of_node = np;
> +	ret = reset_controller_register(&rstc->reset);
> +	if (ret) {
> +		pr_err("%s: Failed to register clkc reset controller: %d\n",
> +		       __func__, ret);
> +		return;
> +	}
> +}
> +
> +CLK_OF_DECLARE_DRIVER(meson8_clkc, "amlogic,meson8-clkc",
> +		      meson8b_clkc_reset_init);
> +CLK_OF_DECLARE_DRIVER(meson8b_clkc, "amlogic,meson8b-clkc",
> +		      meson8b_clkc_reset_init);
> +CLK_OF_DECLARE_DRIVER(meson8m2_clkc, "amlogic,meson8m2-clkc",
> +		      meson8b_clkc_reset_init);
> diff --git a/drivers/clk/meson/meson8b.h b/drivers/clk/meson/meson8b.h
> index a687e02547dc..5f4d8e49dd4d 100644
> --- a/drivers/clk/meson/meson8b.h
> +++ b/drivers/clk/meson/meson8b.h
> @@ -37,6 +37,9 @@
>  #define HHI_GCLK_AO			0x154 /* 0x55 offset in data sheet */
>  #define HHI_SYS_CPU_CLK_CNTL1		0x15c /* 0x57 offset in data sheet */
>  #define HHI_MPEG_CLK_CNTL		0x174 /* 0x5d offset in data sheet */
> +#define HHI_VID_CLK_CNTL		0x17c /* 0x5f offset in data sheet */
> +#define HHI_VID_DIVIDER_CNTL		0x198 /* 0x66 offset in data sheet */
> +#define HHI_SYS_CPU_CLK_CNTL0		0x19c /* 0x67 offset in data sheet */
>  #define HHI_MPLL_CNTL			0x280 /* 0xa0 offset in data sheet */
>  #define HHI_SYS_PLL_CNTL		0x300 /* 0xc0 offset in data sheet */
>  #define HHI_VID_PLL_CNTL		0x320 /* 0xc8 offset in data sheet */
> @@ -163,7 +166,27 @@
>  
>  #define CLK_NR_CLKS		96
>  
> -/* include the CLKIDs that have been made part of the stable DT binding */
> +#define RESETID_L2_CACHE_SOFT_RESET				0
> +#define RESETID_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET		1
> +#define RESETID_SCU_SOFT_RESET					2
> +#define RESETID_CPU0_SOFT_RESET					3
> +#define RESETID_CPU1_SOFT_RESET					4
> +#define RESETID_CPU2_SOFT_RESET					5
> +#define RESETID_CPU3_SOFT_RESET					6
> +#define RESETID_A5_GLOBAL_RESET					7
> +#define RESETID_A5_AXI_SOFT_RESET				8
> +#define RESETID_A5_ABP_SOFT_RESET				9
> +#define RESETID_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET		10
> +#define RESETID_VID_CLK_CNTL_SOFT_RESET				11
> +#define RESETID_VID_DIVIDER_CNTL_SOFT_RESET_POST		12
> +#define RESETID_VID_DIVIDER_CNTL_SOFT_RESET_PRE			13
> +#define RESETID_VID_DIVIDER_CNTL_RESET_N_POST			14
> +#define RESETID_VID_DIVIDER_CNTL_RESET_N_PRE			15

I think you should directly expose them in a dt-bindings/reset/meson8b-clkc-reset.h file and include it here.

> +
> +/*
> + * include the CLKID and RESETID that have
> + * been made part of the stable DT binding
> + */
>  #include <dt-bindings/clock/meson8b-clkc.h>
>  
>  #endif /* __MESON8B_H */
> 

Apart from that :
Reviewed-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] clk: meson: meson8b: register the built-in reset controller

[Martin Blumenstingl]

Hi Neil,

On Tue, Jul 25, 2017 at 9:58 AM, Neil Armstrong <narmstrong@baylibre.com> wrote:
>
>
> Le 22/07/2017 20:58, Martin Blumenstingl a écrit :
>> The clock controller also includes some reset lines. This patch
>> implements a reset controller to assert and de-assert these resets.
>> The reset controller itself is registered early (through
>> CLK_OF_DECLARE_DRIVER) because it is needed very early in the boot
>> process (to start the secondary CPU cores).
>>
>> According to the public S805 datasheet there are two more reset bits
>> in the HHI_SYS_CPU_CLK_CNTL0 register, which are not implemented by
>> this patch (as these seem to be unused in Amlogic's vendor Linux kernel
>> sources and their u-boot tree):
>> - bit 15: GEN_DIV_SOFT_RESET
>> - bit 14: SOFT_RESET
>>
>> All information was taken from the public S805 Datasheet and Amlogic's
>> vendor GPL kernel sources. This patch is based on an earlier version
>> submitted by Carlo Caione.
>>
>> Suggested-by: Carlo Caione <carlo@endlessm.com>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> ---
>>  drivers/clk/meson/Kconfig   |   1 +
>>  drivers/clk/meson/meson8b.c | 159 ++++++++++++++++++++++++++++++++++++++++----
>>  drivers/clk/meson/meson8b.h |  25 ++++++-
>>  3 files changed, 172 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index 5588f75a8414..d2d0174a6eca 100644
>> --- a/drivers/clk/meson/Kconfig
>> +++ b/drivers/clk/meson/Kconfig
>> @@ -6,6 +6,7 @@ config COMMON_CLK_AMLOGIC
>>  config COMMON_CLK_MESON8B
>>       bool
>>       depends on COMMON_CLK_AMLOGIC
>> +     select RESET_CONTROLLER
>>       help
>>         Support for the clock controller on AmLogic S802 (Meson8),
>>         S805 (Meson8b) and S812 (Meson8m2) devices. Say Y if you
>> diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
>> index bb3f1de876b1..0f853057885c 100644
>> --- a/drivers/clk/meson/meson8b.c
>> +++ b/drivers/clk/meson/meson8b.c
>> @@ -25,6 +25,8 @@
>>  #include <linux/clk-provider.h>
>>  #include <linux/of_address.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/reset-controller.h>
>> +#include <linux/slab.h>
>>  #include <linux/init.h>
>>
>>  #include "clkc.h"
>> @@ -32,6 +34,13 @@
>>
>>  static DEFINE_SPINLOCK(clk_lock);
>>
>> +static void __iomem *clk_base;
>> +
>> +struct meson8b_clk_reset {
>> +     struct reset_controller_dev reset;
>> +     void __iomem *base;
>> +};
>> +
>>  static const struct pll_rate_table sys_pll_rate_table[] = {
>>       PLL_RATE(312000000, 52, 1, 2),
>>       PLL_RATE(336000000, 56, 1, 2),
>> @@ -690,20 +699,114 @@ static struct clk_divider *const meson8b_clk_dividers[] = {
>>       &meson8b_mpeg_clk_div,
>>  };
>>
>> +static const struct meson8b_clk_reset_line {
>> +     u32 reg;
>> +     u8 bit_idx;
>> +} meson8b_clk_reset_bits[] = {
>> +     [RESETID_L2_CACHE_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 30
>> +     },
>> +     [RESETID_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 29
>> +     },
>> +     [RESETID_SCU_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 28
>> +     },
>> +     [RESETID_CPU3_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 27
>> +     },
>> +     [RESETID_CPU2_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 26
>> +     },
>> +     [RESETID_CPU1_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 25
>> +     },
>> +     [RESETID_CPU0_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 24
>> +     },
>> +     [RESETID_A5_GLOBAL_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 18
>> +     },
>> +     [RESETID_A5_AXI_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 17
>> +     },
>> +     [RESETID_A5_ABP_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 16
>> +     },
>> +     [RESETID_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL1, .bit_idx = 30
>> +     },
>> +     [RESETID_VID_CLK_CNTL_SOFT_RESET] = {
>> +             .reg = HHI_VID_CLK_CNTL, .bit_idx = 15
>> +     },
>> +     [RESETID_VID_DIVIDER_CNTL_SOFT_RESET_POST] = {
>> +             .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 7
>> +     },
>> +     [RESETID_VID_DIVIDER_CNTL_SOFT_RESET_PRE] = {
>> +             .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 3
>> +     },
>> +     [RESETID_VID_DIVIDER_CNTL_RESET_N_POST] = {
>> +             .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 1
>> +     },
>> +     [RESETID_VID_DIVIDER_CNTL_RESET_N_PRE] = {
>> +             .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 0
>> +     },
>> +};
>> +
>> +static int meson8b_clk_reset_update(struct reset_controller_dev *rcdev,
>> +                                 unsigned long id, bool assert)
>> +{
>> +     struct meson8b_clk_reset *meson8b_clk_reset =
>> +             container_of(rcdev, struct meson8b_clk_reset, reset);
>> +     unsigned long flags;
>> +     const struct meson8b_clk_reset_line *reset;
>> +     u32 val;
>> +
>> +     if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
>> +             return -EINVAL;
>> +
>> +     reset = &meson8b_clk_reset_bits[id];
>> +
>> +     spin_lock_irqsave(&clk_lock, flags);
>> +
>> +     val = readl(meson8b_clk_reset->base + reset->reg);
>> +     if (assert)
>> +             val |= BIT(reset->bit_idx);
>> +     else
>> +             val &= ~BIT(reset->bit_idx);
>> +     writel(val, meson8b_clk_reset->base + reset->reg);
>> +
>> +     spin_unlock_irqrestore(&clk_lock, flags);
>> +
>> +     return 0;
>> +}
>> +
>> +static int meson8b_clk_reset_assert(struct reset_controller_dev *rcdev,
>> +                                  unsigned long id)
>> +{
>> +     return meson8b_clk_reset_update(rcdev, id, true);
>> +}
>> +
>> +static int meson8b_clk_reset_deassert(struct reset_controller_dev *rcdev,
>> +                                    unsigned long id)
>> +{
>> +     return meson8b_clk_reset_update(rcdev, id, false);
>> +}
>> +
>> +static const struct reset_control_ops meson8b_clk_reset_ops = {
>> +     .assert = meson8b_clk_reset_assert,
>> +     .deassert = meson8b_clk_reset_deassert,
>> +};
>> +
>>  static int meson8b_clkc_probe(struct platform_device *pdev)
>>  {
>> -     void __iomem *clk_base;
>>       int ret, clkid, i;
>>       struct clk_hw *parent_hw;
>>       struct clk *parent_clk;
>>       struct device *dev = &pdev->dev;
>>
>> -     /*  Generic clocks and PLLs */
>> -     clk_base = of_iomap(dev->of_node, 1);
>> -     if (!clk_base) {
>> -             pr_err("%s: Unable to map clk base\n", __func__);
>> +     if (!clk_base)
>>               return -ENXIO;
>> -     }
>>
>>       /* Populate base address for PLLs */
>>       for (i = 0; i < ARRAY_SIZE(meson8b_clk_plls); i++)
>> @@ -743,7 +846,7 @@ static int meson8b_clkc_probe(struct platform_device *pdev)
>>               /* FIXME convert to devm_clk_register */
>>               ret = devm_clk_hw_register(dev, meson8b_hw_onecell_data.hws[clkid]);
>>               if (ret)
>> -                     goto iounmap;
>> +                     return ret;
>>       }
>>
>>       /*
>> @@ -766,15 +869,11 @@ static int meson8b_clkc_probe(struct platform_device *pdev)
>>       if (ret) {
>>               pr_err("%s: failed to register clock notifier for cpu_clk\n",
>>                               __func__);
>> -             goto iounmap;
>> +             return ret;
>>       }
>>
>>       return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
>>                       &meson8b_hw_onecell_data);
>> -
>> -iounmap:
>> -     iounmap(clk_base);
>> -     return ret;
>>  }
>>
>>  static const struct of_device_id meson8b_clkc_match_table[] = {
>> @@ -793,3 +892,39 @@ static struct platform_driver meson8b_driver = {
>>  };
>>
>>  builtin_platform_driver(meson8b_driver);
>> +
>> +static void __init meson8b_clkc_reset_init(struct device_node *np)
>> +{
>> +     struct meson8b_clk_reset *rstc;
>> +     int ret;
>> +
>> +     /* Generic clocks, PLLs and some of the reset-bits */
>> +     clk_base = of_iomap(np, 1);
>> +     if (!clk_base) {
>> +             pr_err("%s: Unable to map clk base\n", __func__);
>> +             return;
>> +     }
>> +
>> +     rstc = kzalloc(sizeof(*rstc), GFP_KERNEL);
>> +     if (!rstc)
>> +             return;
>> +
>> +     /* Reset Controller */
>> +     rstc->base = clk_base;
>> +     rstc->reset.ops = &meson8b_clk_reset_ops;
>> +     rstc->reset.nr_resets = ARRAY_SIZE(meson8b_clk_reset_bits);
>> +     rstc->reset.of_node = np;
>> +     ret = reset_controller_register(&rstc->reset);
>> +     if (ret) {
>> +             pr_err("%s: Failed to register clkc reset controller: %d\n",
>> +                    __func__, ret);
>> +             return;
>> +     }
>> +}
>> +
>> +CLK_OF_DECLARE_DRIVER(meson8_clkc, "amlogic,meson8-clkc",
>> +                   meson8b_clkc_reset_init);
>> +CLK_OF_DECLARE_DRIVER(meson8b_clkc, "amlogic,meson8b-clkc",
>> +                   meson8b_clkc_reset_init);
>> +CLK_OF_DECLARE_DRIVER(meson8m2_clkc, "amlogic,meson8m2-clkc",
>> +                   meson8b_clkc_reset_init);
>> diff --git a/drivers/clk/meson/meson8b.h b/drivers/clk/meson/meson8b.h
>> index a687e02547dc..5f4d8e49dd4d 100644
>> --- a/drivers/clk/meson/meson8b.h
>> +++ b/drivers/clk/meson/meson8b.h
>> @@ -37,6 +37,9 @@
>>  #define HHI_GCLK_AO                  0x154 /* 0x55 offset in data sheet */
>>  #define HHI_SYS_CPU_CLK_CNTL1                0x15c /* 0x57 offset in data sheet */
>>  #define HHI_MPEG_CLK_CNTL            0x174 /* 0x5d offset in data sheet */
>> +#define HHI_VID_CLK_CNTL             0x17c /* 0x5f offset in data sheet */
>> +#define HHI_VID_DIVIDER_CNTL         0x198 /* 0x66 offset in data sheet */
>> +#define HHI_SYS_CPU_CLK_CNTL0                0x19c /* 0x67 offset in data sheet */
>>  #define HHI_MPLL_CNTL                        0x280 /* 0xa0 offset in data sheet */
>>  #define HHI_SYS_PLL_CNTL             0x300 /* 0xc0 offset in data sheet */
>>  #define HHI_VID_PLL_CNTL             0x320 /* 0xc8 offset in data sheet */
>> @@ -163,7 +166,27 @@
>>
>>  #define CLK_NR_CLKS          96
>>
>> -/* include the CLKIDs that have been made part of the stable DT binding */
>> +#define RESETID_L2_CACHE_SOFT_RESET                          0
>> +#define RESETID_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET           1
>> +#define RESETID_SCU_SOFT_RESET                                       2
>> +#define RESETID_CPU0_SOFT_RESET                                      3
>> +#define RESETID_CPU1_SOFT_RESET                                      4
>> +#define RESETID_CPU2_SOFT_RESET                                      5
>> +#define RESETID_CPU3_SOFT_RESET                                      6
>> +#define RESETID_A5_GLOBAL_RESET                                      7
>> +#define RESETID_A5_AXI_SOFT_RESET                            8
>> +#define RESETID_A5_ABP_SOFT_RESET                            9
>> +#define RESETID_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET          10
>> +#define RESETID_VID_CLK_CNTL_SOFT_RESET                              11
>> +#define RESETID_VID_DIVIDER_CNTL_SOFT_RESET_POST             12
>> +#define RESETID_VID_DIVIDER_CNTL_SOFT_RESET_PRE                      13
>> +#define RESETID_VID_DIVIDER_CNTL_RESET_N_POST                        14
>> +#define RESETID_VID_DIVIDER_CNTL_RESET_N_PRE                 15
>
> I think you should directly expose them in a dt-bindings/reset/meson8b-clkc-reset.h file and include it here.
just to confirm: do you want to move these to separate (new) header or
would meson8b-clkc.h be fine as well?
in either case: I suspect that changes to these will be very rare (as
I've re-checked that Amlogic's GPL kernel or u-boot definitely uses
these), so exposing them directly probably makes sense

>> +
>> +/*
>> + * include the CLKID and RESETID that have
>> + * been made part of the stable DT binding
>> + */
>>  #include <dt-bindings/clock/meson8b-clkc.h>
>>
>>  #endif /* __MESON8B_H */
>>
>
> Apart from that :
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
thank you for reviewing my patches from this and the SMP/hotplug series!


Martin

[PATCH v2 3/3] ARM: dts: meson: mark the clock controller also as reset controller

[Martin Blumenstingl]

The clock controller provides a few reset lines as well. Add the
#reset-cells property so we can pass the CPU soft reset lines to their
corresponding CPU cores.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 arch/arm/boot/dts/meson8.dtsi  | 1 +
 arch/arm/boot/dts/meson8b.dtsi | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
index 6fe6a159e960..b98d44fde6b6 100644
--- a/arch/arm/boot/dts/meson8.dtsi
+++ b/arch/arm/boot/dts/meson8.dtsi
@@ -168,6 +168,7 @@
 &cbus {
 	clkc: clock-controller@4000 {
 		#clock-cells = <1>;
+		#reset-cells = <1>;
 		compatible = "amlogic,meson8-clkc";
 		reg = <0x8000 0x4>, <0x4000 0x460>;
 	};
diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index 8fce13844b0c..bc278da7df0d 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -119,6 +119,7 @@
 &cbus {
 	clkc: clock-controller@4000 {
 		#clock-cells = <1>;
+		#reset-cells = <1>;
 		compatible = "amlogic,meson8b-clkc";
 		reg = <0x8000 0x4>, <0x4000 0x460>;
 	};
-- 
2.13.3

Re: [PATCH v2 3/3] ARM: dts: meson: mark the clock controller also as reset controller

[Neil Armstrong]

Le 22/07/2017 20:58, Martin Blumenstingl a écrit :
> The clock controller provides a few reset lines as well. Add the
> #reset-cells property so we can pass the CPU soft reset lines to their
> corresponding CPU cores.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  arch/arm/boot/dts/meson8.dtsi  | 1 +
>  arch/arm/boot/dts/meson8b.dtsi | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
> index 6fe6a159e960..b98d44fde6b6 100644
> --- a/arch/arm/boot/dts/meson8.dtsi
> +++ b/arch/arm/boot/dts/meson8.dtsi
> @@ -168,6 +168,7 @@
>  &cbus {
>  	clkc: clock-controller@4000 {
>  		#clock-cells = <1>;
> +		#reset-cells = <1>;
>  		compatible = "amlogic,meson8-clkc";
>  		reg = <0x8000 0x4>, <0x4000 0x460>;
>  	};
> diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
> index 8fce13844b0c..bc278da7df0d 100644
> --- a/arch/arm/boot/dts/meson8b.dtsi
> +++ b/arch/arm/boot/dts/meson8b.dtsi
> @@ -119,6 +119,7 @@
>  &cbus {
>  	clkc: clock-controller@4000 {
>  		#clock-cells = <1>;
> +		#reset-cells = <1>;
>  		compatible = "amlogic,meson8b-clkc";
>  		reg = <0x8000 0x4>, <0x4000 0x460>;
>  	};
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>