[PATCH v2 0/3] Meson8/Meson8b: introduce a HHI syscon node

[PATCH v2 0/3] Meson8/Meson8b: introduce a HHI syscon node

[Martin Blumenstingl]

The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem
as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
There is a register area called "HHI" which is used for multiple IP
blocks of the SoC:
- the system clock controller
- a few reset lines (there is a separate reset controller, these reset
  lines are not part of the other reset controller). this reset
  controller is currently implemented in the clock controller driver
- a HDMI controller
- temperature sensor calibration data (by "data" I really mean data,
  the ADC driver has four bits for the TSC data in it's own register
  space, however on Meson8b and Meson8m2 there is a fifth TSC bit which
  is stored in the HHI register area)

The first three could be implemented with a single node (either in one
big driver, or using a MFD driver which would register function-
specific drivers).
However, the TSC data is a big problem, because the ADC has it's own
set of registers but needs to write one bit in the HHI register area.

NOTE: this series has multiple dependencies:
- the clock controller changes depend "meson8b: add the CPU_DIV16 clock
  for the ARM TWD" as well as "meson8b: register the clock controller
  early" [2]
- the dts changes depend on "fix clock controller register size on
  Meson8/Meson8b" [3]


Changes since v1 at [4]:
- added a "amlogic,meson-hhi-sysctrl" compatible to the syscon node
  (which is the parent of the clock controller) in patch #1 and #3
- added Neil's Acked-by
- rebased on top of clk-meson's meson-clk-4.20-1 tag and my other
  series "Meson8b: fixes for the cpu_scale_div clock" from [5]


[0] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006733.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007890.html
[2] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007900.html
[3] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007897.html
[4] https://patchwork.kernel.org/cover/10539051/
[5] https://patchwork.kernel.org/cover/10617617/


Martin Blumenstingl (3):
  dt-bindings: clock: meson8b: use the registers from the HHI syscon
  clk: meson: meson8b: use the HHI syscon if available
  ARM: dts: meson: switch the clock controller to the HHI register area

 .../bindings/clock/amlogic,meson8b-clkc.txt   | 13 ++++------
 arch/arm/boot/dts/meson.dtsi                  |  7 ++++++
 arch/arm/boot/dts/meson8.dtsi                 | 15 ++++++------
 arch/arm/boot/dts/meson8b.dtsi                | 15 ++++++------
 drivers/clk/meson/meson8b.c                   | 24 ++++++++++++-------
 5 files changed, 43 insertions(+), 31 deletions(-)

-- 
2.19.1

[PATCH v2 0/3] Meson8/Meson8b: introduce a HHI syscon node

[Martin Blumenstingl]

The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem
as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
There is a register area called "HHI" which is used for multiple IP
blocks of the SoC:
- the system clock controller
- a few reset lines (there is a separate reset controller, these reset
  lines are not part of the other reset controller). this reset
  controller is currently implemented in the clock controller driver
- a HDMI controller
- temperature sensor calibration data (by "data" I really mean data,
  the ADC driver has four bits for the TSC data in it's own register
  space, however on Meson8b and Meson8m2 there is a fifth TSC bit which
  is stored in the HHI register area)

The first three could be implemented with a single node (either in one
big driver, or using a MFD driver which would register function-
specific drivers).
However, the TSC data is a big problem, because the ADC has it's own
set of registers but needs to write one bit in the HHI register area.

NOTE: this series has multiple dependencies:
- the clock controller changes depend "meson8b: add the CPU_DIV16 clock
  for the ARM TWD" as well as "meson8b: register the clock controller
  early" [2]
- the dts changes depend on "fix clock controller register size on
  Meson8/Meson8b" [3]


Changes since v1 at [4]:
- added a "amlogic,meson-hhi-sysctrl" compatible to the syscon node
  (which is the parent of the clock controller) in patch #1 and #3
- added Neil's Acked-by
- rebased on top of clk-meson's meson-clk-4.20-1 tag and my other
  series "Meson8b: fixes for the cpu_scale_div clock" from [5]


[0] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006733.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007890.html
[2] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007900.html
[3] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007897.html
[4] https://patchwork.kernel.org/cover/10539051/
[5] https://patchwork.kernel.org/cover/10617617/


Martin Blumenstingl (3):
  dt-bindings: clock: meson8b: use the registers from the HHI syscon
  clk: meson: meson8b: use the HHI syscon if available
  ARM: dts: meson: switch the clock controller to the HHI register area

 .../bindings/clock/amlogic,meson8b-clkc.txt   | 13 ++++------
 arch/arm/boot/dts/meson.dtsi                  |  7 ++++++
 arch/arm/boot/dts/meson8.dtsi                 | 15 ++++++------
 arch/arm/boot/dts/meson8b.dtsi                | 15 ++++++------
 drivers/clk/meson/meson8b.c                   | 24 ++++++++++++-------
 5 files changed, 43 insertions(+), 31 deletions(-)

-- 
2.19.1

[PATCH v2 1/3] dt-bindings: clock: meson8b: use the registers from the HHI syscon

[Martin Blumenstingl]

The clock controller on Meson8/Meson8m2 and Meson8b is part of a
register region called "HHI". This register area contains more
functionality than just a clock controller:
- the clock controller
- some reset controller bits
- temperature sensor calibration coefficient (only on Meson8b and
  Meson8m2 - one one out of five TSC bits is stored in the HHI
  registers)
- HDMI controller

The HHI register area may be accessed concurrently. Allow this by using
a "system controller" as parent node.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Acked-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../bindings/clock/amlogic,meson8b-clkc.txt         | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
index b455c5aa9139..4d94091c1d2d 100644
--- a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
+++ b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
@@ -9,15 +9,13 @@ Required Properties:
 	- "amlogic,meson8-clkc" for Meson8 (S802) SoCs
 	- "amlogic,meson8b-clkc" for Meson8 (S805) SoCs
 	- "amlogic,meson8m2-clkc" for Meson8m2 (S812) SoCs
-- reg: it must be composed by two tuples:
-	0) physical base address of the xtal register and length of memory
-	   mapped region.
-	1) physical base address of the clock controller and length of memory
-	   mapped region.
-
 - #clock-cells: should be 1.
 - #reset-cells: should be 1.
 
+Parent node should have the following properties :
+- compatible: "amlogic,meson-hhi-sysctrl", "simple-mfd", "syscon"
+- reg: base address and size of the HHI system control register space.
+
 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
@@ -30,9 +28,8 @@ device tree sources).
 
 Example: Clock controller node:
 
-	clkc: clock-controller@c1104000 {
+	clkc: clock-controller {
 		compatible = "amlogic,meson8b-clkc";
-		reg = <0xc1108000 0x4>, <0xc1104000 0x460>;
 		#clock-cells = <1>;
 		#reset-cells = <1>;
 	};
-- 
2.19.1

[PATCH v2 1/3] dt-bindings: clock: meson8b: use the registers from the HHI syscon

[Martin Blumenstingl]

The clock controller on Meson8/Meson8m2 and Meson8b is part of a
register region called "HHI". This register area contains more
functionality than just a clock controller:
- the clock controller
- some reset controller bits
- temperature sensor calibration coefficient (only on Meson8b and
  Meson8m2 - one one out of five TSC bits is stored in the HHI
  registers)
- HDMI controller

The HHI register area may be accessed concurrently. Allow this by using
a "system controller" as parent node.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Acked-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../bindings/clock/amlogic,meson8b-clkc.txt         | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
index b455c5aa9139..4d94091c1d2d 100644
--- a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
+++ b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
@@ -9,15 +9,13 @@ Required Properties:
 	- "amlogic,meson8-clkc" for Meson8 (S802) SoCs
 	- "amlogic,meson8b-clkc" for Meson8 (S805) SoCs
 	- "amlogic,meson8m2-clkc" for Meson8m2 (S812) SoCs
-- reg: it must be composed by two tuples:
-	0) physical base address of the xtal register and length of memory
-	   mapped region.
-	1) physical base address of the clock controller and length of memory
-	   mapped region.
-
 - #clock-cells: should be 1.
 - #reset-cells: should be 1.
 
+Parent node should have the following properties :
+- compatible: "amlogic,meson-hhi-sysctrl", "simple-mfd", "syscon"
+- reg: base address and size of the HHI system control register space.
+
 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
@@ -30,9 +28,8 @@ device tree sources).
 
 Example: Clock controller node:
 
-	clkc: clock-controller at c1104000 {
+	clkc: clock-controller {
 		compatible = "amlogic,meson8b-clkc";
-		reg = <0xc1108000 0x4>, <0xc1104000 0x460>;
 		#clock-cells = <1>;
 		#reset-cells = <1>;
 	};
-- 
2.19.1

[PATCH v2 2/3] clk: meson: meson8b: use the HHI syscon if available

[Martin Blumenstingl]

The clock controller is located in a register range (called "HHI") which
contains more than just registers for the clock controller. Known
consumers of the HHI register range are:
- the clock controller
- a reset controller
- temperature sensor calibration coefficient (TSC) (only on Meson8b and
  Meson8m2)
- HDMI controller

The main reason for using a syscon is the "temperature sensor
calibration coefficient" which has to be set for the built-in temperature
sensor to work correctly. Four TSC bits are located in the SAR ADC's
register space. However on Meson8b and Meson8m2 there is a fifth TSC bit
which is unfortunately located in the HHI register space. To be more
precise, bit 9 of the HHI_DPLL_TOP_0 register (which sits right between
the HHI_SYS_PLL and HHI_VID_PLL registers).

Get the regmap from the parent (HHI syscon) node to support all
functionality of the HHI register range. Backwards compatibility with
old .dtbs is ensured by falling back to parsing the registers just like
before this change.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Acked-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/clk/meson/meson8b.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index 1e02ca4be2fc..9bd5920da0ff 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -10,6 +10,7 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/init.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of_address.h>
 #include <linux/reset-controller.h>
 #include <linux/slab.h>
@@ -1115,16 +1116,21 @@ static void __init meson8b_clkc_init(struct device_node *np)
 	struct regmap *map;
 	int i, 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;
-	}
+	map = syscon_node_to_regmap(of_get_parent(np));
+	if (IS_ERR(map)) {
+		pr_info("failed to get HHI regmap - Trying obsolete regs\n");
 
-	map = regmap_init_mmio(NULL, clk_base, &clkc_regmap_config);
-	if (IS_ERR(map))
-		return;
+		/* 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;
+		}
+
+		map = regmap_init_mmio(NULL, clk_base, &clkc_regmap_config);
+		if (IS_ERR(map))
+			return;
+	}
 
 	rstc = kzalloc(sizeof(*rstc), GFP_KERNEL);
 	if (!rstc)
-- 
2.19.1

[PATCH v2 2/3] clk: meson: meson8b: use the HHI syscon if available

[Martin Blumenstingl]

The clock controller is located in a register range (called "HHI") which
contains more than just registers for the clock controller. Known
consumers of the HHI register range are:
- the clock controller
- a reset controller
- temperature sensor calibration coefficient (TSC) (only on Meson8b and
  Meson8m2)
- HDMI controller

The main reason for using a syscon is the "temperature sensor
calibration coefficient" which has to be set for the built-in temperature
sensor to work correctly. Four TSC bits are located in the SAR ADC's
register space. However on Meson8b and Meson8m2 there is a fifth TSC bit
which is unfortunately located in the HHI register space. To be more
precise, bit 9 of the HHI_DPLL_TOP_0 register (which sits right between
the HHI_SYS_PLL and HHI_VID_PLL registers).

Get the regmap from the parent (HHI syscon) node to support all
functionality of the HHI register range. Backwards compatibility with
old .dtbs is ensured by falling back to parsing the registers just like
before this change.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Acked-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/clk/meson/meson8b.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index 1e02ca4be2fc..9bd5920da0ff 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -10,6 +10,7 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/init.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of_address.h>
 #include <linux/reset-controller.h>
 #include <linux/slab.h>
@@ -1115,16 +1116,21 @@ static void __init meson8b_clkc_init(struct device_node *np)
 	struct regmap *map;
 	int i, 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;
-	}
+	map = syscon_node_to_regmap(of_get_parent(np));
+	if (IS_ERR(map)) {
+		pr_info("failed to get HHI regmap - Trying obsolete regs\n");
 
-	map = regmap_init_mmio(NULL, clk_base, &clkc_regmap_config);
-	if (IS_ERR(map))
-		return;
+		/* 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;
+		}
+
+		map = regmap_init_mmio(NULL, clk_base, &clkc_regmap_config);
+		if (IS_ERR(map))
+			return;
+	}
 
 	rstc = kzalloc(sizeof(*rstc), GFP_KERNEL);
 	if (!rstc)
-- 
2.19.1

[PATCH v2 3/3] ARM: dts: meson: switch the clock controller to the HHI register area

[Martin Blumenstingl]

The clock controller on Meson8/Meson8m2 and Meson8b is part of a
register region called "HHI". This register area contains more
functionality than just a clock controller:
- the clock controller
- some reset controller bits
- temperature sensor calibration data (on Meson8b and Meson8m2 only)
- HDMI controller

Allow access to this HHI register area as "system controller". Also
migrate the Meson8 and Meson8b clock controllers to this new node.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Acked-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm/boot/dts/meson.dtsi   |  7 +++++++
 arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
 arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
index 0d9faf1a51ea..ad903d1a2cb6 100644
--- a/arch/arm/boot/dts/meson.dtsi
+++ b/arch/arm/boot/dts/meson.dtsi
@@ -80,6 +80,13 @@
 			#size-cells = <1>;
 			ranges = <0x0 0xc1100000 0x200000>;
 
+			hhi: system-controller@4000 {
+				compatible = "amlogic,meson-hhi-sysctrl",
+					     "simple-mfd",
+					     "syscon";
+				reg = <0x4000 0x400>;
+			};
+
 			assist: assist@7c00 {
 				compatible = "amlogic,meson-mx-assist", "syscon";
 				reg = <0x7c00 0x200>;
diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
index 7162e0ca05b0..3e3d9c54cddc 100644
--- a/arch/arm/boot/dts/meson8.dtsi
+++ b/arch/arm/boot/dts/meson8.dtsi
@@ -190,13 +190,6 @@
 };
 
 &cbus {
-	clkc: clock-controller@4000 {
-		#clock-cells = <1>;
-		#reset-cells = <1>;
-		compatible = "amlogic,meson8-clkc";
-		reg = <0x8000 0x4>, <0x4000 0x400>;
-	};
-
 	reset: reset-controller@4404 {
 		compatible = "amlogic,meson8b-reset";
 		reg = <0x4404 0x9c>;
@@ -323,6 +316,14 @@
 	status = "okay";
 };
 
+&hhi {
+	clkc: clock-controller {
+		compatible = "amlogic,meson8-clkc";
+		#clock-cells = <1>;
+		#reset-cells = <1>;
+	};
+};
+
 &hwrng {
 	compatible = "amlogic,meson8-rng", "amlogic,meson-rng";
 	clocks = <&clkc CLKID_RNG0>;
diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index cd1ca9dda126..f92aab0aa247 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -159,13 +159,6 @@
 };
 
 &cbus {
-	clkc: clock-controller@4000 {
-		#clock-cells = <1>;
-		#reset-cells = <1>;
-		compatible = "amlogic,meson8b-clkc";
-		reg = <0x8000 0x4>, <0x4000 0x400>;
-	};
-
 	reset: reset-controller@4404 {
 		compatible = "amlogic,meson8b-reset";
 		reg = <0x4404 0x9c>;
@@ -313,6 +306,14 @@
 	status = "okay";
 };
 
+&hhi {
+	clkc: clock-controller {
+		compatible = "amlogic,meson8-clkc";
+		#clock-cells = <1>;
+		#reset-cells = <1>;
+	};
+};
+
 &hwrng {
 	compatible = "amlogic,meson8b-rng", "amlogic,meson-rng";
 	clocks = <&clkc CLKID_RNG0>;
-- 
2.19.1

[PATCH v2 3/3] ARM: dts: meson: switch the clock controller to the HHI register area

[Martin Blumenstingl]

The clock controller on Meson8/Meson8m2 and Meson8b is part of a
register region called "HHI". This register area contains more
functionality than just a clock controller:
- the clock controller
- some reset controller bits
- temperature sensor calibration data (on Meson8b and Meson8m2 only)
- HDMI controller

Allow access to this HHI register area as "system controller". Also
migrate the Meson8 and Meson8b clock controllers to this new node.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Acked-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm/boot/dts/meson.dtsi   |  7 +++++++
 arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
 arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
index 0d9faf1a51ea..ad903d1a2cb6 100644
--- a/arch/arm/boot/dts/meson.dtsi
+++ b/arch/arm/boot/dts/meson.dtsi
@@ -80,6 +80,13 @@
 			#size-cells = <1>;
 			ranges = <0x0 0xc1100000 0x200000>;
 
+			hhi: system-controller at 4000 {
+				compatible = "amlogic,meson-hhi-sysctrl",
+					     "simple-mfd",
+					     "syscon";
+				reg = <0x4000 0x400>;
+			};
+
 			assist: assist at 7c00 {
 				compatible = "amlogic,meson-mx-assist", "syscon";
 				reg = <0x7c00 0x200>;
diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
index 7162e0ca05b0..3e3d9c54cddc 100644
--- a/arch/arm/boot/dts/meson8.dtsi
+++ b/arch/arm/boot/dts/meson8.dtsi
@@ -190,13 +190,6 @@
 };
 
 &cbus {
-	clkc: clock-controller at 4000 {
-		#clock-cells = <1>;
-		#reset-cells = <1>;
-		compatible = "amlogic,meson8-clkc";
-		reg = <0x8000 0x4>, <0x4000 0x400>;
-	};
-
 	reset: reset-controller at 4404 {
 		compatible = "amlogic,meson8b-reset";
 		reg = <0x4404 0x9c>;
@@ -323,6 +316,14 @@
 	status = "okay";
 };
 
+&hhi {
+	clkc: clock-controller {
+		compatible = "amlogic,meson8-clkc";
+		#clock-cells = <1>;
+		#reset-cells = <1>;
+	};
+};
+
 &hwrng {
 	compatible = "amlogic,meson8-rng", "amlogic,meson-rng";
 	clocks = <&clkc CLKID_RNG0>;
diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index cd1ca9dda126..f92aab0aa247 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -159,13 +159,6 @@
 };
 
 &cbus {
-	clkc: clock-controller at 4000 {
-		#clock-cells = <1>;
-		#reset-cells = <1>;
-		compatible = "amlogic,meson8b-clkc";
-		reg = <0x8000 0x4>, <0x4000 0x400>;
-	};
-
 	reset: reset-controller at 4404 {
 		compatible = "amlogic,meson8b-reset";
 		reg = <0x4404 0x9c>;
@@ -313,6 +306,14 @@
 	status = "okay";
 };
 
+&hhi {
+	clkc: clock-controller {
+		compatible = "amlogic,meson8-clkc";
+		#clock-cells = <1>;
+		#reset-cells = <1>;
+	};
+};
+
 &hwrng {
 	compatible = "amlogic,meson8b-rng", "amlogic,meson-rng";
 	clocks = <&clkc CLKID_RNG0>;
-- 
2.19.1

Re: [PATCH v2 3/3] ARM: dts: meson: switch the clock controller to the HHI register area

[Neil Armstrong]

On 28/10/2018 13:08, Martin Blumenstingl wrote:
> The clock controller on Meson8/Meson8m2 and Meson8b is part of a
> register region called "HHI". This register area contains more
> functionality than just a clock controller:
> - the clock controller
> - some reset controller bits
> - temperature sensor calibration data (on Meson8b and Meson8m2 only)
> - HDMI controller
> 
> Allow access to this HHI register area as "system controller". Also
> migrate the Meson8 and Meson8b clock controllers to this new node.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  arch/arm/boot/dts/meson.dtsi   |  7 +++++++
>  arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
>  arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
>  3 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
> index 0d9faf1a51ea..ad903d1a2cb6 100644
> --- a/arch/arm/boot/dts/meson.dtsi
> +++ b/arch/arm/boot/dts/meson.dtsi

[...]

>  
> +&hhi {
> +	clkc: clock-controller {
> +		compatible = "amlogic,meson8-clkc";
> +		#clock-cells = <1>;
> +		#reset-cells = <1>;
> +	};
> +};
> +
>  &hwrng {
>  	compatible = "amlogic,meson8b-rng", "amlogic,meson-rng";
>  	clocks = <&clkc CLKID_RNG0>;
> 

Kevin,

I didn't apply this one, but it can wait another cycle I think.

Neil

[PATCH v2 3/3] ARM: dts: meson: switch the clock controller to the HHI register area

[Neil Armstrong]

On 28/10/2018 13:08, Martin Blumenstingl wrote:
> The clock controller on Meson8/Meson8m2 and Meson8b is part of a
> register region called "HHI". This register area contains more
> functionality than just a clock controller:
> - the clock controller
> - some reset controller bits
> - temperature sensor calibration data (on Meson8b and Meson8m2 only)
> - HDMI controller
> 
> Allow access to this HHI register area as "system controller". Also
> migrate the Meson8 and Meson8b clock controllers to this new node.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  arch/arm/boot/dts/meson.dtsi   |  7 +++++++
>  arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
>  arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
>  3 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
> index 0d9faf1a51ea..ad903d1a2cb6 100644
> --- a/arch/arm/boot/dts/meson.dtsi
> +++ b/arch/arm/boot/dts/meson.dtsi

[...]

>  
> +&hhi {
> +	clkc: clock-controller {
> +		compatible = "amlogic,meson8-clkc";
> +		#clock-cells = <1>;
> +		#reset-cells = <1>;
> +	};
> +};
> +
>  &hwrng {
>  	compatible = "amlogic,meson8b-rng", "amlogic,meson-rng";
>  	clocks = <&clkc CLKID_RNG0>;
> 

Kevin,

I didn't apply this one, but it can wait another cycle I think.

Neil

Re: [PATCH v2 3/3] ARM: dts: meson: switch the clock controller to the HHI register area

[Kevin Hilman]

Neil Armstrong <narmstrong@baylibre.com> writes:

> On 28/10/2018 13:08, Martin Blumenstingl wrote:
>> The clock controller on Meson8/Meson8m2 and Meson8b is part of a
>> register region called "HHI". This register area contains more
>> functionality than just a clock controller:
>> - the clock controller
>> - some reset controller bits
>> - temperature sensor calibration data (on Meson8b and Meson8m2 only)
>> - HDMI controller
>> 
>> Allow access to this HHI register area as "system controller". Also
>> migrate the Meson8 and Meson8b clock controllers to this new node.
>> 
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  arch/arm/boot/dts/meson.dtsi   |  7 +++++++
>>  arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
>>  arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
>>  3 files changed, 23 insertions(+), 14 deletions(-)
>> 
>> diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
>> index 0d9faf1a51ea..ad903d1a2cb6 100644
>> --- a/arch/arm/boot/dts/meson.dtsi
>> +++ b/arch/arm/boot/dts/meson.dtsi
>
> [...]
>
>>  
>> +&hhi {
>> +	clkc: clock-controller {
>> +		compatible = "amlogic,meson8-clkc";
>> +		#clock-cells = <1>;
>> +		#reset-cells = <1>;
>> +	};
>> +};
>> +
>>  &hwrng {
>>  	compatible = "amlogic,meson8b-rng", "amlogic,meson-rng";
>>  	clocks = <&clkc CLKID_RNG0>;
>> 
>
> Kevin,
>
> I didn't apply this one, but it can wait another cycle I think.

OK, I'll take that one through my dt branch.

I don't see any obvious dependency on the clock patches, so I can apply
it now and include it with my next round of PRs, so...

Queued for v4.21 (branch: v4.21/dt)

Thanks,

Kevin

Re: [PATCH v2 3/3] ARM: dts: meson: switch the clock controller to the HHI register area

[Kevin Hilman]

Neil Armstrong <narmstrong@baylibre.com> writes:

> On 28/10/2018 13:08, Martin Blumenstingl wrote:
>> The clock controller on Meson8/Meson8m2 and Meson8b is part of a
>> register region called "HHI". This register area contains more
>> functionality than just a clock controller:
>> - the clock controller
>> - some reset controller bits
>> - temperature sensor calibration data (on Meson8b and Meson8m2 only)
>> - HDMI controller
>> 
>> Allow access to this HHI register area as "system controller". Also
>> migrate the Meson8 and Meson8b clock controllers to this new node.
>> 
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  arch/arm/boot/dts/meson.dtsi   |  7 +++++++
>>  arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
>>  arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
>>  3 files changed, 23 insertions(+), 14 deletions(-)
>> 
>> diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
>> index 0d9faf1a51ea..ad903d1a2cb6 100644
>> --- a/arch/arm/boot/dts/meson.dtsi
>> +++ b/arch/arm/boot/dts/meson.dtsi
>
> [...]
>
>>  
>> +&hhi {
>> +	clkc: clock-controller {
>> +		compatible = "amlogic,meson8-clkc";
>> +		#clock-cells = <1>;
>> +		#reset-cells = <1>;
>> +	};
>> +};
>> +
>>  &hwrng {
>>  	compatible = "amlogic,meson8b-rng", "amlogic,meson-rng";
>>  	clocks = <&clkc CLKID_RNG0>;
>> 
>
> Kevin,
>
> I didn't apply this one, but it can wait another cycle I think.

OK, I'll take that one through my dt branch.

I don't see any obvious dependency on the clock patches, so I can apply
it now and include it with my next round of PRs, so...

Queued for v4.21 (branch: v4.21/dt)

Thanks,

Kevin


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2 3/3] ARM: dts: meson: switch the clock controller to the HHI register area

[Kevin Hilman]

Kevin Hilman <khilman@baylibre.com> writes:

> Neil Armstrong <narmstrong@baylibre.com> writes:
>
>> On 28/10/2018 13:08, Martin Blumenstingl wrote:
>>> The clock controller on Meson8/Meson8m2 and Meson8b is part of a
>>> register region called "HHI". This register area contains more
>>> functionality than just a clock controller:
>>> - the clock controller
>>> - some reset controller bits
>>> - temperature sensor calibration data (on Meson8b and Meson8m2 only)
>>> - HDMI controller
>>> 
>>> Allow access to this HHI register area as "system controller". Also
>>> migrate the Meson8 and Meson8b clock controllers to this new node.
>>> 
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>>  arch/arm/boot/dts/meson.dtsi   |  7 +++++++
>>>  arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
>>>  arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
>>>  3 files changed, 23 insertions(+), 14 deletions(-)
>>> 
>>> diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
>>> index 0d9faf1a51ea..ad903d1a2cb6 100644
>>> --- a/arch/arm/boot/dts/meson.dtsi
>>> +++ b/arch/arm/boot/dts/meson.dtsi
>>
>> [...]
>>
>>>  
>>> +&hhi {
>>> +	clkc: clock-controller {
>>> +		compatible = "amlogic,meson8-clkc";
>>> +		#clock-cells = <1>;
>>> +		#reset-cells = <1>;
>>> +	};
>>> +};
>>> +
>>>  &hwrng {
>>>  	compatible = "amlogic,meson8b-rng", "amlogic,meson-rng";
>>>  	clocks = <&clkc CLKID_RNG0>;
>>> 
>>
>> Kevin,
>>
>> I didn't apply this one, but it can wait another cycle I think.
>
> OK, I'll take that one through my dt branch.
>
> I don't see any obvious dependency on the clock patches,

... and a quick boot test shows I was wrong, and should've (re)read
Martin's cover letter a couple more times. :)

I think it's best to leave this one until the next cycle when all the
deps are merged.

Kevin

Re: [PATCH v2 3/3] ARM: dts: meson: switch the clock controller to the HHI register area

[Kevin Hilman]

Kevin Hilman <khilman@baylibre.com> writes:

> Neil Armstrong <narmstrong@baylibre.com> writes:
>
>> On 28/10/2018 13:08, Martin Blumenstingl wrote:
>>> The clock controller on Meson8/Meson8m2 and Meson8b is part of a
>>> register region called "HHI". This register area contains more
>>> functionality than just a clock controller:
>>> - the clock controller
>>> - some reset controller bits
>>> - temperature sensor calibration data (on Meson8b and Meson8m2 only)
>>> - HDMI controller
>>> 
>>> Allow access to this HHI register area as "system controller". Also
>>> migrate the Meson8 and Meson8b clock controllers to this new node.
>>> 
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>>  arch/arm/boot/dts/meson.dtsi   |  7 +++++++
>>>  arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
>>>  arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
>>>  3 files changed, 23 insertions(+), 14 deletions(-)
>>> 
>>> diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
>>> index 0d9faf1a51ea..ad903d1a2cb6 100644
>>> --- a/arch/arm/boot/dts/meson.dtsi
>>> +++ b/arch/arm/boot/dts/meson.dtsi
>>
>> [...]
>>
>>>  
>>> +&hhi {
>>> +	clkc: clock-controller {
>>> +		compatible = "amlogic,meson8-clkc";
>>> +		#clock-cells = <1>;
>>> +		#reset-cells = <1>;
>>> +	};
>>> +};
>>> +
>>>  &hwrng {
>>>  	compatible = "amlogic,meson8b-rng", "amlogic,meson-rng";
>>>  	clocks = <&clkc CLKID_RNG0>;
>>> 
>>
>> Kevin,
>>
>> I didn't apply this one, but it can wait another cycle I think.
>
> OK, I'll take that one through my dt branch.
>
> I don't see any obvious dependency on the clock patches,

... and a quick boot test shows I was wrong, and should've (re)read
Martin's cover letter a couple more times. :)

I think it's best to leave this one until the next cycle when all the
deps are merged.

Kevin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2 3/3] ARM: dts: meson: switch the clock controller to the HHI register area

[Martin Blumenstingl]

Hi Kevin,

On Fri, Nov 30, 2018 at 9:15 PM Kevin Hilman <khilman@baylibre.com> wrote:
>
> Kevin Hilman <khilman@baylibre.com> writes:
>
> > Neil Armstrong <narmstrong@baylibre.com> writes:
> >
> >> On 28/10/2018 13:08, Martin Blumenstingl wrote:
> >>> The clock controller on Meson8/Meson8m2 and Meson8b is part of a
> >>> register region called "HHI". This register area contains more
> >>> functionality than just a clock controller:
> >>> - the clock controller
> >>> - some reset controller bits
> >>> - temperature sensor calibration data (on Meson8b and Meson8m2 only)
> >>> - HDMI controller
> >>>
> >>> Allow access to this HHI register area as "system controller". Also
> >>> migrate the Meson8 and Meson8b clock controllers to this new node.
> >>>
> >>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> >>> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> >>> ---
> >>>  arch/arm/boot/dts/meson.dtsi   |  7 +++++++
> >>>  arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
> >>>  arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
> >>>  3 files changed, 23 insertions(+), 14 deletions(-)
> >>>
> >>> diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
> >>> index 0d9faf1a51ea..ad903d1a2cb6 100644
> >>> --- a/arch/arm/boot/dts/meson.dtsi
> >>> +++ b/arch/arm/boot/dts/meson.dtsi
> >>
> >> [...]
> >>
> >>>
> >>> +&hhi {
> >>> +   clkc: clock-controller {
> >>> +           compatible = "amlogic,meson8-clkc";
> >>> +           #clock-cells = <1>;
> >>> +           #reset-cells = <1>;
> >>> +   };
> >>> +};
> >>> +
> >>>  &hwrng {
> >>>     compatible = "amlogic,meson8b-rng", "amlogic,meson-rng";
> >>>     clocks = <&clkc CLKID_RNG0>;
> >>>
> >>
> >> Kevin,
> >>
> >> I didn't apply this one, but it can wait another cycle I think.
> >
> > OK, I'll take that one through my dt branch.
> >
> > I don't see any obvious dependency on the clock patches,
>
> ... and a quick boot test shows I was wrong, and should've (re)read
> Martin's cover letter a couple more times. :)
yes, there's a hard dependency on the clock patches

> I think it's best to leave this one until the next cycle when all the
> deps are merged.
I have this on my "resend this for the next development cycle" list


Regards
Martin

Re: [PATCH v2 3/3] ARM: dts: meson: switch the clock controller to the HHI register area

[Martin Blumenstingl]

Hi Kevin,

On Fri, Nov 30, 2018 at 9:15 PM Kevin Hilman <khilman@baylibre.com> wrote:
>
> Kevin Hilman <khilman@baylibre.com> writes:
>
> > Neil Armstrong <narmstrong@baylibre.com> writes:
> >
> >> On 28/10/2018 13:08, Martin Blumenstingl wrote:
> >>> The clock controller on Meson8/Meson8m2 and Meson8b is part of a
> >>> register region called "HHI". This register area contains more
> >>> functionality than just a clock controller:
> >>> - the clock controller
> >>> - some reset controller bits
> >>> - temperature sensor calibration data (on Meson8b and Meson8m2 only)
> >>> - HDMI controller
> >>>
> >>> Allow access to this HHI register area as "system controller". Also
> >>> migrate the Meson8 and Meson8b clock controllers to this new node.
> >>>
> >>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> >>> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> >>> ---
> >>>  arch/arm/boot/dts/meson.dtsi   |  7 +++++++
> >>>  arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
> >>>  arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
> >>>  3 files changed, 23 insertions(+), 14 deletions(-)
> >>>
> >>> diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
> >>> index 0d9faf1a51ea..ad903d1a2cb6 100644
> >>> --- a/arch/arm/boot/dts/meson.dtsi
> >>> +++ b/arch/arm/boot/dts/meson.dtsi
> >>
> >> [...]
> >>
> >>>
> >>> +&hhi {
> >>> +   clkc: clock-controller {
> >>> +           compatible = "amlogic,meson8-clkc";
> >>> +           #clock-cells = <1>;
> >>> +           #reset-cells = <1>;
> >>> +   };
> >>> +};
> >>> +
> >>>  &hwrng {
> >>>     compatible = "amlogic,meson8b-rng", "amlogic,meson-rng";
> >>>     clocks = <&clkc CLKID_RNG0>;
> >>>
> >>
> >> Kevin,
> >>
> >> I didn't apply this one, but it can wait another cycle I think.
> >
> > OK, I'll take that one through my dt branch.
> >
> > I don't see any obvious dependency on the clock patches,
>
> ... and a quick boot test shows I was wrong, and should've (re)read
> Martin's cover letter a couple more times. :)
yes, there's a hard dependency on the clock patches

> I think it's best to leave this one until the next cycle when all the
> deps are merged.
I have this on my "resend this for the next development cycle" list


Regards
Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2 0/3] Meson8/Meson8b: introduce a HHI syscon node

[Rob Herring]

On Sun, Oct 28, 2018 at 01:08:56PM +0100, Martin Blumenstingl wrote:
> The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem
> as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
> There is a register area called "HHI" which is used for multiple IP
> blocks of the SoC:
> - the system clock controller
> - a few reset lines (there is a separate reset controller, these reset
>   lines are not part of the other reset controller). this reset
>   controller is currently implemented in the clock controller driver
> - a HDMI controller

Does this have it's own clocks, resets or other resources?

> - temperature sensor calibration data (by "data" I really mean data,
>   the ADC driver has four bits for the TSC data in it's own register
>   space, however on Meson8b and Meson8m2 there is a fifth TSC bit which
>   is stored in the HHI register area)
> 
> The first three could be implemented with a single node (either in one
> big driver, or using a MFD driver which would register function-
> specific drivers).
> However, the TSC data is a big problem, because the ADC has it's own
> set of registers but needs to write one bit in the HHI register area.

Generally, that would be solved with a phandle to the HHI and maybe an 
offset cell in the ADC node. I don't see why that's a big problem.

> 
> NOTE: this series has multiple dependencies:
> - the clock controller changes depend "meson8b: add the CPU_DIV16 clock
>   for the ARM TWD" as well as "meson8b: register the clock controller
>   early" [2]
> - the dts changes depend on "fix clock controller register size on
>   Meson8/Meson8b" [3]
> 
> 
> Changes since v1 at [4]:
> - added a "amlogic,meson-hhi-sysctrl" compatible to the syscon node
>   (which is the parent of the clock controller) in patch #1 and #3
> - added Neil's Acked-by
> - rebased on top of clk-meson's meson-clk-4.20-1 tag and my other
>   series "Meson8b: fixes for the cpu_scale_div clock" from [5]
> 
> 
> [0] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006733.html
> [1] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007890.html
> [2] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007900.html
> [3] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007897.html
> [4] https://patchwork.kernel.org/cover/10539051/
> [5] https://patchwork.kernel.org/cover/10617617/
> 
> 
> Martin Blumenstingl (3):
>   dt-bindings: clock: meson8b: use the registers from the HHI syscon
>   clk: meson: meson8b: use the HHI syscon if available
>   ARM: dts: meson: switch the clock controller to the HHI register area
> 
>  .../bindings/clock/amlogic,meson8b-clkc.txt   | 13 ++++------
>  arch/arm/boot/dts/meson.dtsi                  |  7 ++++++
>  arch/arm/boot/dts/meson8.dtsi                 | 15 ++++++------
>  arch/arm/boot/dts/meson8b.dtsi                | 15 ++++++------
>  drivers/clk/meson/meson8b.c                   | 24 ++++++++++++-------
>  5 files changed, 43 insertions(+), 31 deletions(-)
> 
> -- 
> 2.19.1
> 

[PATCH v2 0/3] Meson8/Meson8b: introduce a HHI syscon node

[Rob Herring]

On Sun, Oct 28, 2018 at 01:08:56PM +0100, Martin Blumenstingl wrote:
> The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem
> as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
> There is a register area called "HHI" which is used for multiple IP
> blocks of the SoC:
> - the system clock controller
> - a few reset lines (there is a separate reset controller, these reset
>   lines are not part of the other reset controller). this reset
>   controller is currently implemented in the clock controller driver
> - a HDMI controller

Does this have it's own clocks, resets or other resources?

> - temperature sensor calibration data (by "data" I really mean data,
>   the ADC driver has four bits for the TSC data in it's own register
>   space, however on Meson8b and Meson8m2 there is a fifth TSC bit which
>   is stored in the HHI register area)
> 
> The first three could be implemented with a single node (either in one
> big driver, or using a MFD driver which would register function-
> specific drivers).
> However, the TSC data is a big problem, because the ADC has it's own
> set of registers but needs to write one bit in the HHI register area.

Generally, that would be solved with a phandle to the HHI and maybe an 
offset cell in the ADC node. I don't see why that's a big problem.

> 
> NOTE: this series has multiple dependencies:
> - the clock controller changes depend "meson8b: add the CPU_DIV16 clock
>   for the ARM TWD" as well as "meson8b: register the clock controller
>   early" [2]
> - the dts changes depend on "fix clock controller register size on
>   Meson8/Meson8b" [3]
> 
> 
> Changes since v1 at [4]:
> - added a "amlogic,meson-hhi-sysctrl" compatible to the syscon node
>   (which is the parent of the clock controller) in patch #1 and #3
> - added Neil's Acked-by
> - rebased on top of clk-meson's meson-clk-4.20-1 tag and my other
>   series "Meson8b: fixes for the cpu_scale_div clock" from [5]
> 
> 
> [0] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006733.html
> [1] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007890.html
> [2] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007900.html
> [3] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007897.html
> [4] https://patchwork.kernel.org/cover/10539051/
> [5] https://patchwork.kernel.org/cover/10617617/
> 
> 
> Martin Blumenstingl (3):
>   dt-bindings: clock: meson8b: use the registers from the HHI syscon
>   clk: meson: meson8b: use the HHI syscon if available
>   ARM: dts: meson: switch the clock controller to the HHI register area
> 
>  .../bindings/clock/amlogic,meson8b-clkc.txt   | 13 ++++------
>  arch/arm/boot/dts/meson.dtsi                  |  7 ++++++
>  arch/arm/boot/dts/meson8.dtsi                 | 15 ++++++------
>  arch/arm/boot/dts/meson8b.dtsi                | 15 ++++++------
>  drivers/clk/meson/meson8b.c                   | 24 ++++++++++++-------
>  5 files changed, 43 insertions(+), 31 deletions(-)
> 
> -- 
> 2.19.1
> 

Re: [PATCH v2 0/3] Meson8/Meson8b: introduce a HHI syscon node

[Martin Blumenstingl]

Hi Rob,

On Tue, Oct 30, 2018 at 11:13 PM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Oct 28, 2018 at 01:08:56PM +0100, Martin Blumenstingl wrote:
> > The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem
> > as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
> > There is a register area called "HHI" which is used for multiple IP
> > blocks of the SoC:
> > - the system clock controller
> > - a few reset lines (there is a separate reset controller, these reset
> >   lines are not part of the other reset controller). this reset
> >   controller is currently implemented in the clock controller driver
> > - a HDMI controller
>
> Does this have it's own clocks, resets or other resources?
I'm not sure what's the best way to answer this as the HDMI part is
not part of the public S805 datasheet. however, I went ahead and read
Amlogic's BSP code (see [1], [2] and [3] - beware: these files will
not make checkpatch happy ;)).
first I have to make a correction here: while looking deeper into this
the HDMI controller is *not* inside the HHI register area (but instead
on the APB bus). sorry for getting this wrong

here's how would make the .dts to look like (assuming we support all
peripherals that I know of in the HHI area):

&hhi {
    compatible = "amlogic,meson-hhi-sysctrl",
                           "simple-mfd",
                           "syscon";

    clkc: clock-controller {
        ...
    };

    hhi_power: power-controller {
        compatible = "amlogic,meson8-hhi-power-controller";
        #power-domain-cells = <1>;
    };

    hdmi_phy: phy {
        compatible = "amlogic,meson8-hdmi-phy";
        #phy-cells = <0>;
        /*
         * HHI_HDMI_PHY_CNTL0 needs a magic value based on
         * power on/off status. HHI_HDMI_PHY_CNTL1 has
         * bit[1]: enable clock, bit[0]: soft reset (maybe
         * these two should be provided by the clkc?)
         */
    };
};

&apb {
    hdmi_tx: hdmi-tx@42000 {
        compatible = "amlogic,meson8-hdmi";
        reg = <0x42000 0xc>;
        interrupts = <GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
        #address-cells = <1>;
        #size-cells = <0>;

        /*
         * internally the actual HDMI controller registers
         * are not accessed directly but through two special
         * (address and data) registers: HDMI_ADDR_PORT,
         * HDMI_DATA_PORT.
         */

        /*
         * there are five internal reset registers:
         * TX_SYS5_TX_SOFT_RESET_1
         * TX_SYS5_TX_SOFT_RESET_2
         * TX_SYS5_RX_SOFT_RESET_1
         * TX_SYS5_RX_SOFT_RESET_2
         * TX_SYS5_RX_SOFT_RESET_3
         */

        /*
         * TODO: this has to know a few details about the
         * audio stream as well to route the audio correctly
         * (vendor code differentiates between 2ch / 8ch and
         * programs the HDMI controller accordingly)
         */

        pinctrl-0 = <&hdmi_hpd_pins>, <&hdmi_i2c_pins>;
        pinctrl-names = "default";

        /* TODO: what is VCLK1_HDMI exactly? */
        clocks = <&clkc CLKID_HDMI_PCLK>,
                 <&clkc CLKID_HDMI_SYS>,
                 <&clkc CLKID_VCLK1_HDMI>;
        clock-names = "pclk", "sys", "vclk1";

        power-domains = <&hhi_power 0>, <&hhi_power 1> ;
        power-domain-names = "edid", "hdcp";

        phys = <&hdmi_phy>;
        phy-names = "hdmi";

        /* VPU VENC Input */
        hdmi_tx_venc_port: port@0 {
            reg = <0>;

            hdmi_tx_in: endpoint {
                /*
                 * this is provided by the VPU which has
                 * many (>10) clock inputs - all connected
                 * to "clkc" above
                 */
                remote-endpoint = <&hdmi_tx_out>;
            };
        };

        /* TMDS Output */
        hdmi_tx_tmds_port: port@1 {
            reg = <1>;
        };
    };
};

&saradc {
...
    /*
     * patches for this are already sent: TSC bit[4] is
     * stored in HHI_DPLL_TOP_0 (which is right between the
     * SYS_PLL and VID_PLL registers. the PLL clocks from
     * these other registers are implemented in the driver
     * for the "clkc" node above)
     */
    amlogic,hhi-sysctrl = <&hhi>;
};


> > - temperature sensor calibration data (by "data" I really mean data,
> >   the ADC driver has four bits for the TSC data in it's own register
> >   space, however on Meson8b and Meson8m2 there is a fifth TSC bit which
> >   is stored in the HHI register area)
> >
> > The first three could be implemented with a single node (either in one
> > big driver, or using a MFD driver which would register function-
> > specific drivers).
> > However, the TSC data is a big problem, because the ADC has it's own
> > set of registers but needs to write one bit in the HHI register area.
>
> Generally, that would be solved with a phandle to the HHI and maybe an
> offset cell in the ADC node. I don't see why that's a big problem.
this is what I'm trying to do, see my dt-bindings patch for the SAR
ADC (which is used to read the temperature sensor): [0]
my understanding was that I should go through syscon to avoid double
ioremap() of the same register set (the clock controller and SAR ADC
would have to do it).
which APIs (other than syscon) would you suggest for this case?


Regards
Martin


[0] https://patchwork.kernel.org/patch/10658557/
[1] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_hw.c
[2] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_reg.c
[3] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/include/mach/register.h

[PATCH v2 0/3] Meson8/Meson8b: introduce a HHI syscon node

[Martin Blumenstingl]

Hi Rob,

On Tue, Oct 30, 2018 at 11:13 PM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Oct 28, 2018 at 01:08:56PM +0100, Martin Blumenstingl wrote:
> > The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem
> > as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
> > There is a register area called "HHI" which is used for multiple IP
> > blocks of the SoC:
> > - the system clock controller
> > - a few reset lines (there is a separate reset controller, these reset
> >   lines are not part of the other reset controller). this reset
> >   controller is currently implemented in the clock controller driver
> > - a HDMI controller
>
> Does this have it's own clocks, resets or other resources?
I'm not sure what's the best way to answer this as the HDMI part is
not part of the public S805 datasheet. however, I went ahead and read
Amlogic's BSP code (see [1], [2] and [3] - beware: these files will
not make checkpatch happy ;)).
first I have to make a correction here: while looking deeper into this
the HDMI controller is *not* inside the HHI register area (but instead
on the APB bus). sorry for getting this wrong

here's how would make the .dts to look like (assuming we support all
peripherals that I know of in the HHI area):

&hhi {
    compatible = "amlogic,meson-hhi-sysctrl",
                           "simple-mfd",
                           "syscon";

    clkc: clock-controller {
        ...
    };

    hhi_power: power-controller {
        compatible = "amlogic,meson8-hhi-power-controller";
        #power-domain-cells = <1>;
    };

    hdmi_phy: phy {
        compatible = "amlogic,meson8-hdmi-phy";
        #phy-cells = <0>;
        /*
         * HHI_HDMI_PHY_CNTL0 needs a magic value based on
         * power on/off status. HHI_HDMI_PHY_CNTL1 has
         * bit[1]: enable clock, bit[0]: soft reset (maybe
         * these two should be provided by the clkc?)
         */
    };
};

&apb {
    hdmi_tx: hdmi-tx at 42000 {
        compatible = "amlogic,meson8-hdmi";
        reg = <0x42000 0xc>;
        interrupts = <GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
        #address-cells = <1>;
        #size-cells = <0>;

        /*
         * internally the actual HDMI controller registers
         * are not accessed directly but through two special
         * (address and data) registers: HDMI_ADDR_PORT,
         * HDMI_DATA_PORT.
         */

        /*
         * there are five internal reset registers:
         * TX_SYS5_TX_SOFT_RESET_1
         * TX_SYS5_TX_SOFT_RESET_2
         * TX_SYS5_RX_SOFT_RESET_1
         * TX_SYS5_RX_SOFT_RESET_2
         * TX_SYS5_RX_SOFT_RESET_3
         */

        /*
         * TODO: this has to know a few details about the
         * audio stream as well to route the audio correctly
         * (vendor code differentiates between 2ch / 8ch and
         * programs the HDMI controller accordingly)
         */

        pinctrl-0 = <&hdmi_hpd_pins>, <&hdmi_i2c_pins>;
        pinctrl-names = "default";

        /* TODO: what is VCLK1_HDMI exactly? */
        clocks = <&clkc CLKID_HDMI_PCLK>,
                 <&clkc CLKID_HDMI_SYS>,
                 <&clkc CLKID_VCLK1_HDMI>;
        clock-names = "pclk", "sys", "vclk1";

        power-domains = <&hhi_power 0>, <&hhi_power 1> ;
        power-domain-names = "edid", "hdcp";

        phys = <&hdmi_phy>;
        phy-names = "hdmi";

        /* VPU VENC Input */
        hdmi_tx_venc_port: port at 0 {
            reg = <0>;

            hdmi_tx_in: endpoint {
                /*
                 * this is provided by the VPU which has
                 * many (>10) clock inputs - all connected
                 * to "clkc" above
                 */
                remote-endpoint = <&hdmi_tx_out>;
            };
        };

        /* TMDS Output */
        hdmi_tx_tmds_port: port at 1 {
            reg = <1>;
        };
    };
};

&saradc {
...
    /*
     * patches for this are already sent: TSC bit[4] is
     * stored in HHI_DPLL_TOP_0 (which is right between the
     * SYS_PLL and VID_PLL registers. the PLL clocks from
     * these other registers are implemented in the driver
     * for the "clkc" node above)
     */
    amlogic,hhi-sysctrl = <&hhi>;
};


> > - temperature sensor calibration data (by "data" I really mean data,
> >   the ADC driver has four bits for the TSC data in it's own register
> >   space, however on Meson8b and Meson8m2 there is a fifth TSC bit which
> >   is stored in the HHI register area)
> >
> > The first three could be implemented with a single node (either in one
> > big driver, or using a MFD driver which would register function-
> > specific drivers).
> > However, the TSC data is a big problem, because the ADC has it's own
> > set of registers but needs to write one bit in the HHI register area.
>
> Generally, that would be solved with a phandle to the HHI and maybe an
> offset cell in the ADC node. I don't see why that's a big problem.
this is what I'm trying to do, see my dt-bindings patch for the SAR
ADC (which is used to read the temperature sensor): [0]
my understanding was that I should go through syscon to avoid double
ioremap() of the same register set (the clock controller and SAR ADC
would have to do it).
which APIs (other than syscon) would you suggest for this case?


Regards
Martin


[0] https://patchwork.kernel.org/patch/10658557/
[1] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_hw.c
[2] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_reg.c
[3] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/include/mach/register.h

Re: [PATCH v2 0/3] Meson8/Meson8b: introduce a HHI syscon node

[Chen-Yu Tsai]

On Thu, Nov 1, 2018 at 6:10 PM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Rob,
>
> On Tue, Oct 30, 2018 at 11:13 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Sun, Oct 28, 2018 at 01:08:56PM +0100, Martin Blumenstingl wrote:
> > > The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem
> > > as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
> > > There is a register area called "HHI" which is used for multiple IP
> > > blocks of the SoC:
> > > - the system clock controller
> > > - a few reset lines (there is a separate reset controller, these reset
> > >   lines are not part of the other reset controller). this reset
> > >   controller is currently implemented in the clock controller driver
> > > - a HDMI controller
> >
> > Does this have it's own clocks, resets or other resources?
> I'm not sure what's the best way to answer this as the HDMI part is
> not part of the public S805 datasheet. however, I went ahead and read
> Amlogic's BSP code (see [1], [2] and [3] - beware: these files will
> not make checkpatch happy ;)).
> first I have to make a correction here: while looking deeper into this
> the HDMI controller is *not* inside the HHI register area (but instead
> on the APB bus). sorry for getting this wrong
>
> here's how would make the .dts to look like (assuming we support all
> peripherals that I know of in the HHI area):
>
> &hhi {
>     compatible = "amlogic,meson-hhi-sysctrl",
>                            "simple-mfd",
>                            "syscon";
>
>     clkc: clock-controller {
>         ...
>     };
>
>     hhi_power: power-controller {
>         compatible = "amlogic,meson8-hhi-power-controller";
>         #power-domain-cells = <1>;
>     };
>
>     hdmi_phy: phy {
>         compatible = "amlogic,meson8-hdmi-phy";
>         #phy-cells = <0>;
>         /*
>          * HHI_HDMI_PHY_CNTL0 needs a magic value based on
>          * power on/off status. HHI_HDMI_PHY_CNTL1 has
>          * bit[1]: enable clock, bit[0]: soft reset (maybe
>          * these two should be provided by the clkc?)
>          */
>     };
> };
>
> &apb {
>     hdmi_tx: hdmi-tx@42000 {
>         compatible = "amlogic,meson8-hdmi";
>         reg = <0x42000 0xc>;
>         interrupts = <GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
>         #address-cells = <1>;
>         #size-cells = <0>;
>
>         /*
>          * internally the actual HDMI controller registers
>          * are not accessed directly but through two special
>          * (address and data) registers: HDMI_ADDR_PORT,
>          * HDMI_DATA_PORT.
>          */
>
>         /*
>          * there are five internal reset registers:
>          * TX_SYS5_TX_SOFT_RESET_1
>          * TX_SYS5_TX_SOFT_RESET_2
>          * TX_SYS5_RX_SOFT_RESET_1
>          * TX_SYS5_RX_SOFT_RESET_2
>          * TX_SYS5_RX_SOFT_RESET_3
>          */
>
>         /*
>          * TODO: this has to know a few details about the
>          * audio stream as well to route the audio correctly
>          * (vendor code differentiates between 2ch / 8ch and
>          * programs the HDMI controller accordingly)
>          */
>
>         pinctrl-0 = <&hdmi_hpd_pins>, <&hdmi_i2c_pins>;
>         pinctrl-names = "default";
>
>         /* TODO: what is VCLK1_HDMI exactly? */
>         clocks = <&clkc CLKID_HDMI_PCLK>,
>                  <&clkc CLKID_HDMI_SYS>,
>                  <&clkc CLKID_VCLK1_HDMI>;
>         clock-names = "pclk", "sys", "vclk1";
>
>         power-domains = <&hhi_power 0>, <&hhi_power 1> ;
>         power-domain-names = "edid", "hdcp";
>
>         phys = <&hdmi_phy>;
>         phy-names = "hdmi";
>
>         /* VPU VENC Input */
>         hdmi_tx_venc_port: port@0 {
>             reg = <0>;
>
>             hdmi_tx_in: endpoint {
>                 /*
>                  * this is provided by the VPU which has
>                  * many (>10) clock inputs - all connected
>                  * to "clkc" above
>                  */
>                 remote-endpoint = <&hdmi_tx_out>;
>             };
>         };
>
>         /* TMDS Output */
>         hdmi_tx_tmds_port: port@1 {
>             reg = <1>;
>         };
>     };
> };
>
> &saradc {
> ...
>     /*
>      * patches for this are already sent: TSC bit[4] is
>      * stored in HHI_DPLL_TOP_0 (which is right between the
>      * SYS_PLL and VID_PLL registers. the PLL clocks from
>      * these other registers are implemented in the driver
>      * for the "clkc" node above)
>      */
>     amlogic,hhi-sysctrl = <&hhi>;
> };
>
>
> > > - temperature sensor calibration data (by "data" I really mean data,
> > >   the ADC driver has four bits for the TSC data in it's own register
> > >   space, however on Meson8b and Meson8m2 there is a fifth TSC bit which
> > >   is stored in the HHI register area)
> > >
> > > The first three could be implemented with a single node (either in one
> > > big driver, or using a MFD driver which would register function-
> > > specific drivers).
> > > However, the TSC data is a big problem, because the ADC has it's own
> > > set of registers but needs to write one bit in the HHI register area.
> >
> > Generally, that would be solved with a phandle to the HHI and maybe an
> > offset cell in the ADC node. I don't see why that's a big problem.
> this is what I'm trying to do, see my dt-bindings patch for the SAR
> ADC (which is used to read the temperature sensor): [0]
> my understanding was that I should go through syscon to avoid double
> ioremap() of the same register set (the clock controller and SAR ADC
> would have to do it).
> which APIs (other than syscon) would you suggest for this case?

I would suggest avoiding the syscon API when you have other drivers going
through the same register space. You can do a syscon like approach by
having the HHI driver register one or multiple regmaps, with appropriate
access controls, which other drivers can fetch using dev_get_regmap() (via
a phandle to the HHI device). I don't know much about simple-mfd though.
If the registers are neatly grouped together by function it would be easier
to deal with. Or you could just have one device node with appropriate cell
properties and have a big driver register all the functions. That driver
would likely live under drivers/soc/.

ChenYu

>
> Regards
> Martin
>
>
> [0] https://patchwork.kernel.org/patch/10658557/
> [1] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_hw.c
> [2] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_reg.c
> [3] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/include/mach/register.h

[PATCH v2 0/3] Meson8/Meson8b: introduce a HHI syscon node

[Chen-Yu Tsai]

On Thu, Nov 1, 2018 at 6:10 PM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Rob,
>
> On Tue, Oct 30, 2018 at 11:13 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Sun, Oct 28, 2018 at 01:08:56PM +0100, Martin Blumenstingl wrote:
> > > The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem
> > > as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
> > > There is a register area called "HHI" which is used for multiple IP
> > > blocks of the SoC:
> > > - the system clock controller
> > > - a few reset lines (there is a separate reset controller, these reset
> > >   lines are not part of the other reset controller). this reset
> > >   controller is currently implemented in the clock controller driver
> > > - a HDMI controller
> >
> > Does this have it's own clocks, resets or other resources?
> I'm not sure what's the best way to answer this as the HDMI part is
> not part of the public S805 datasheet. however, I went ahead and read
> Amlogic's BSP code (see [1], [2] and [3] - beware: these files will
> not make checkpatch happy ;)).
> first I have to make a correction here: while looking deeper into this
> the HDMI controller is *not* inside the HHI register area (but instead
> on the APB bus). sorry for getting this wrong
>
> here's how would make the .dts to look like (assuming we support all
> peripherals that I know of in the HHI area):
>
> &hhi {
>     compatible = "amlogic,meson-hhi-sysctrl",
>                            "simple-mfd",
>                            "syscon";
>
>     clkc: clock-controller {
>         ...
>     };
>
>     hhi_power: power-controller {
>         compatible = "amlogic,meson8-hhi-power-controller";
>         #power-domain-cells = <1>;
>     };
>
>     hdmi_phy: phy {
>         compatible = "amlogic,meson8-hdmi-phy";
>         #phy-cells = <0>;
>         /*
>          * HHI_HDMI_PHY_CNTL0 needs a magic value based on
>          * power on/off status. HHI_HDMI_PHY_CNTL1 has
>          * bit[1]: enable clock, bit[0]: soft reset (maybe
>          * these two should be provided by the clkc?)
>          */
>     };
> };
>
> &apb {
>     hdmi_tx: hdmi-tx at 42000 {
>         compatible = "amlogic,meson8-hdmi";
>         reg = <0x42000 0xc>;
>         interrupts = <GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
>         #address-cells = <1>;
>         #size-cells = <0>;
>
>         /*
>          * internally the actual HDMI controller registers
>          * are not accessed directly but through two special
>          * (address and data) registers: HDMI_ADDR_PORT,
>          * HDMI_DATA_PORT.
>          */
>
>         /*
>          * there are five internal reset registers:
>          * TX_SYS5_TX_SOFT_RESET_1
>          * TX_SYS5_TX_SOFT_RESET_2
>          * TX_SYS5_RX_SOFT_RESET_1
>          * TX_SYS5_RX_SOFT_RESET_2
>          * TX_SYS5_RX_SOFT_RESET_3
>          */
>
>         /*
>          * TODO: this has to know a few details about the
>          * audio stream as well to route the audio correctly
>          * (vendor code differentiates between 2ch / 8ch and
>          * programs the HDMI controller accordingly)
>          */
>
>         pinctrl-0 = <&hdmi_hpd_pins>, <&hdmi_i2c_pins>;
>         pinctrl-names = "default";
>
>         /* TODO: what is VCLK1_HDMI exactly? */
>         clocks = <&clkc CLKID_HDMI_PCLK>,
>                  <&clkc CLKID_HDMI_SYS>,
>                  <&clkc CLKID_VCLK1_HDMI>;
>         clock-names = "pclk", "sys", "vclk1";
>
>         power-domains = <&hhi_power 0>, <&hhi_power 1> ;
>         power-domain-names = "edid", "hdcp";
>
>         phys = <&hdmi_phy>;
>         phy-names = "hdmi";
>
>         /* VPU VENC Input */
>         hdmi_tx_venc_port: port at 0 {
>             reg = <0>;
>
>             hdmi_tx_in: endpoint {
>                 /*
>                  * this is provided by the VPU which has
>                  * many (>10) clock inputs - all connected
>                  * to "clkc" above
>                  */
>                 remote-endpoint = <&hdmi_tx_out>;
>             };
>         };
>
>         /* TMDS Output */
>         hdmi_tx_tmds_port: port at 1 {
>             reg = <1>;
>         };
>     };
> };
>
> &saradc {
> ...
>     /*
>      * patches for this are already sent: TSC bit[4] is
>      * stored in HHI_DPLL_TOP_0 (which is right between the
>      * SYS_PLL and VID_PLL registers. the PLL clocks from
>      * these other registers are implemented in the driver
>      * for the "clkc" node above)
>      */
>     amlogic,hhi-sysctrl = <&hhi>;
> };
>
>
> > > - temperature sensor calibration data (by "data" I really mean data,
> > >   the ADC driver has four bits for the TSC data in it's own register
> > >   space, however on Meson8b and Meson8m2 there is a fifth TSC bit which
> > >   is stored in the HHI register area)
> > >
> > > The first three could be implemented with a single node (either in one
> > > big driver, or using a MFD driver which would register function-
> > > specific drivers).
> > > However, the TSC data is a big problem, because the ADC has it's own
> > > set of registers but needs to write one bit in the HHI register area.
> >
> > Generally, that would be solved with a phandle to the HHI and maybe an
> > offset cell in the ADC node. I don't see why that's a big problem.
> this is what I'm trying to do, see my dt-bindings patch for the SAR
> ADC (which is used to read the temperature sensor): [0]
> my understanding was that I should go through syscon to avoid double
> ioremap() of the same register set (the clock controller and SAR ADC
> would have to do it).
> which APIs (other than syscon) would you suggest for this case?

I would suggest avoiding the syscon API when you have other drivers going
through the same register space. You can do a syscon like approach by
having the HHI driver register one or multiple regmaps, with appropriate
access controls, which other drivers can fetch using dev_get_regmap() (via
a phandle to the HHI device). I don't know much about simple-mfd though.
If the registers are neatly grouped together by function it would be easier
to deal with. Or you could just have one device node with appropriate cell
properties and have a big driver register all the functions. That driver
would likely live under drivers/soc/.

ChenYu

>
> Regards
> Martin
>
>
> [0] https://patchwork.kernel.org/patch/10658557/
> [1] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_hw.c
> [2] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_reg.c
> [3] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/include/mach/register.h

Re: [PATCH v2 0/3] Meson8/Meson8b: introduce a HHI syscon node

[Neil Armstrong]

On 01/11/2018 11:20, Chen-Yu Tsai wrote:
> On Thu, Nov 1, 2018 at 6:10 PM Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
>>
>> Hi Rob,
>>
>> On Tue, Oct 30, 2018 at 11:13 PM Rob Herring <robh@kernel.org> wrote:
>>>
>>> On Sun, Oct 28, 2018 at 01:08:56PM +0100, Martin Blumenstingl wrote:
>>>> The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem
>>>> as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
>>>> There is a register area called "HHI" which is used for multiple IP
>>>> blocks of the SoC:
>>>> - the system clock controller
>>>> - a few reset lines (there is a separate reset controller, these reset
>>>>   lines are not part of the other reset controller). this reset
>>>>   controller is currently implemented in the clock controller driver
>>>> - a HDMI controller
>>>
>>> Does this have it's own clocks, resets or other resources?

[...]

>>>>
>>>> The first three could be implemented with a single node (either in one
>>>> big driver, or using a MFD driver which would register function-
>>>> specific drivers).
>>>> However, the TSC data is a big problem, because the ADC has it's own
>>>> set of registers but needs to write one bit in the HHI register area.
>>>
>>> Generally, that would be solved with a phandle to the HHI and maybe an
>>> offset cell in the ADC node. I don't see why that's a big problem.
>> this is what I'm trying to do, see my dt-bindings patch for the SAR
>> ADC (which is used to read the temperature sensor): [0]
>> my understanding was that I should go through syscon to avoid double
>> ioremap() of the same register set (the clock controller and SAR ADC
>> would have to do it).
>> which APIs (other than syscon) would you suggest for this case?
> 
> I would suggest avoiding the syscon API when you have other drivers going
> through the same register space. You can do a syscon like approach by
> having the HHI driver register one or multiple regmaps, with appropriate
> access controls, which other drivers can fetch using dev_get_regmap() (via
> a phandle to the HHI device). I don't know much about simple-mfd though.
> If the registers are neatly grouped together by function it would be easier
> to deal with. Or you could just have one device node with appropriate cell
> properties and have a big driver register all the functions. That driver
> would likely live under drivers/soc/.

Well, it's the role of simple-mfd, when you only have plaform-tied sub drivers,
and it's over-engineered to write an MFD driver for this.

At some point, we should move forward with this since it's a "system controller"
specific mapping, and "syscon" was designed specifically to solve this kind
of mapping in DT.

The other SoCs (GX, AXG and G12A) uses this exact DT architecture to solve this
issue, and we should use this same schema for other Amlogic SoCs even if it's
not the most perfect way to solve it.

Neil

> 
> ChenYu
> 
>>
>> Regards
>> Martin
>>
>>
>> [0] https://patchwork.kernel.org/patch/10658557/
>> [1] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_hw.c
>> [2] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_reg.c
>> [3] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/include/mach/register.h

[PATCH v2 0/3] Meson8/Meson8b: introduce a HHI syscon node

[Neil Armstrong]

On 01/11/2018 11:20, Chen-Yu Tsai wrote:
> On Thu, Nov 1, 2018 at 6:10 PM Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
>>
>> Hi Rob,
>>
>> On Tue, Oct 30, 2018 at 11:13 PM Rob Herring <robh@kernel.org> wrote:
>>>
>>> On Sun, Oct 28, 2018 at 01:08:56PM +0100, Martin Blumenstingl wrote:
>>>> The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem
>>>> as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
>>>> There is a register area called "HHI" which is used for multiple IP
>>>> blocks of the SoC:
>>>> - the system clock controller
>>>> - a few reset lines (there is a separate reset controller, these reset
>>>>   lines are not part of the other reset controller). this reset
>>>>   controller is currently implemented in the clock controller driver
>>>> - a HDMI controller
>>>
>>> Does this have it's own clocks, resets or other resources?

[...]

>>>>
>>>> The first three could be implemented with a single node (either in one
>>>> big driver, or using a MFD driver which would register function-
>>>> specific drivers).
>>>> However, the TSC data is a big problem, because the ADC has it's own
>>>> set of registers but needs to write one bit in the HHI register area.
>>>
>>> Generally, that would be solved with a phandle to the HHI and maybe an
>>> offset cell in the ADC node. I don't see why that's a big problem.
>> this is what I'm trying to do, see my dt-bindings patch for the SAR
>> ADC (which is used to read the temperature sensor): [0]
>> my understanding was that I should go through syscon to avoid double
>> ioremap() of the same register set (the clock controller and SAR ADC
>> would have to do it).
>> which APIs (other than syscon) would you suggest for this case?
> 
> I would suggest avoiding the syscon API when you have other drivers going
> through the same register space. You can do a syscon like approach by
> having the HHI driver register one or multiple regmaps, with appropriate
> access controls, which other drivers can fetch using dev_get_regmap() (via
> a phandle to the HHI device). I don't know much about simple-mfd though.
> If the registers are neatly grouped together by function it would be easier
> to deal with. Or you could just have one device node with appropriate cell
> properties and have a big driver register all the functions. That driver
> would likely live under drivers/soc/.

Well, it's the role of simple-mfd, when you only have plaform-tied sub drivers,
and it's over-engineered to write an MFD driver for this.

At some point, we should move forward with this since it's a "system controller"
specific mapping, and "syscon" was designed specifically to solve this kind
of mapping in DT.

The other SoCs (GX, AXG and G12A) uses this exact DT architecture to solve this
issue, and we should use this same schema for other Amlogic SoCs even if it's
not the most perfect way to solve it.

Neil

> 
> ChenYu
> 
>>
>> Regards
>> Martin
>>
>>
>> [0] https://patchwork.kernel.org/patch/10658557/
>> [1] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_hw.c
>> [2] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_reg.c
>> [3] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/include/mach/register.h

Re: [PATCH v2 0/3] Meson8/Meson8b: introduce a HHI syscon node

[Neil Armstrong]

On 08/11/2018 15:42, Neil Armstrong wrote:
> On 01/11/2018 11:20, Chen-Yu Tsai wrote:
>> On Thu, Nov 1, 2018 at 6:10 PM Martin Blumenstingl
>> <martin.blumenstingl@googlemail.com> wrote:
>>>
>>> Hi Rob,
>>>
>>> On Tue, Oct 30, 2018 at 11:13 PM Rob Herring <robh@kernel.org> wrote:
>>>>
>>>> On Sun, Oct 28, 2018 at 01:08:56PM +0100, Martin Blumenstingl wrote:
>>>>> The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem
>>>>> as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
>>>>> There is a register area called "HHI" which is used for multiple IP
>>>>> blocks of the SoC:
>>>>> - the system clock controller
>>>>> - a few reset lines (there is a separate reset controller, these reset
>>>>>   lines are not part of the other reset controller). this reset
>>>>>   controller is currently implemented in the clock controller driver
>>>>> - a HDMI controller
>>>>
>>>> Does this have it's own clocks, resets or other resources?
> 
> [...]
> 
>>>>>
>>>>> The first three could be implemented with a single node (either in one
>>>>> big driver, or using a MFD driver which would register function-
>>>>> specific drivers).
>>>>> However, the TSC data is a big problem, because the ADC has it's own
>>>>> set of registers but needs to write one bit in the HHI register area.
>>>>
>>>> Generally, that would be solved with a phandle to the HHI and maybe an
>>>> offset cell in the ADC node. I don't see why that's a big problem.
>>> this is what I'm trying to do, see my dt-bindings patch for the SAR
>>> ADC (which is used to read the temperature sensor): [0]
>>> my understanding was that I should go through syscon to avoid double
>>> ioremap() of the same register set (the clock controller and SAR ADC
>>> would have to do it).
>>> which APIs (other than syscon) would you suggest for this case?
>>
>> I would suggest avoiding the syscon API when you have other drivers going
>> through the same register space. You can do a syscon like approach by
>> having the HHI driver register one or multiple regmaps, with appropriate
>> access controls, which other drivers can fetch using dev_get_regmap() (via
>> a phandle to the HHI device). I don't know much about simple-mfd though.
>> If the registers are neatly grouped together by function it would be easier
>> to deal with. Or you could just have one device node with appropriate cell
>> properties and have a big driver register all the functions. That driver
>> would likely live under drivers/soc/.
> 
> Well, it's the role of simple-mfd, when you only have plaform-tied sub drivers,
> and it's over-engineered to write an MFD driver for this.
> 
> At some point, we should move forward with this since it's a "system controller"
> specific mapping, and "syscon" was designed specifically to solve this kind
> of mapping in DT.
> 
> The other SoCs (GX, AXG and G12A) uses this exact DT architecture to solve this
> issue, and we should use this same schema for other Amlogic SoCs even if it's
> not the most perfect way to solve it.

Considering the "syscon" and "simple-mfd" bindings documentation not marked
deprecated, and that the whole Amlogic architecture (and new SoCs support)
is already using this design, I don't see any reason not aligning the
Meson8 & Meson8b clk support on the ARM64 variants scheme.

I do act we will need to avoid syscon for *new* driver and DT designs.

Applied on next/drivers for 4.21

Neil

> 
> Neil
> 
>>
>> ChenYu
>>
>>>
>>> Regards
>>> Martin
>>>
>>>
>>> [0] https://patchwork.kernel.org/patch/10658557/
>>> [1] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_hw.c
>>> [2] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_reg.c
>>> [3] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/include/mach/register.h
> 

[PATCH v2 0/3] Meson8/Meson8b: introduce a HHI syscon node

[Neil Armstrong]

On 08/11/2018 15:42, Neil Armstrong wrote:
> On 01/11/2018 11:20, Chen-Yu Tsai wrote:
>> On Thu, Nov 1, 2018 at 6:10 PM Martin Blumenstingl
>> <martin.blumenstingl@googlemail.com> wrote:
>>>
>>> Hi Rob,
>>>
>>> On Tue, Oct 30, 2018 at 11:13 PM Rob Herring <robh@kernel.org> wrote:
>>>>
>>>> On Sun, Oct 28, 2018 at 01:08:56PM +0100, Martin Blumenstingl wrote:
>>>>> The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem
>>>>> as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
>>>>> There is a register area called "HHI" which is used for multiple IP
>>>>> blocks of the SoC:
>>>>> - the system clock controller
>>>>> - a few reset lines (there is a separate reset controller, these reset
>>>>>   lines are not part of the other reset controller). this reset
>>>>>   controller is currently implemented in the clock controller driver
>>>>> - a HDMI controller
>>>>
>>>> Does this have it's own clocks, resets or other resources?
> 
> [...]
> 
>>>>>
>>>>> The first three could be implemented with a single node (either in one
>>>>> big driver, or using a MFD driver which would register function-
>>>>> specific drivers).
>>>>> However, the TSC data is a big problem, because the ADC has it's own
>>>>> set of registers but needs to write one bit in the HHI register area.
>>>>
>>>> Generally, that would be solved with a phandle to the HHI and maybe an
>>>> offset cell in the ADC node. I don't see why that's a big problem.
>>> this is what I'm trying to do, see my dt-bindings patch for the SAR
>>> ADC (which is used to read the temperature sensor): [0]
>>> my understanding was that I should go through syscon to avoid double
>>> ioremap() of the same register set (the clock controller and SAR ADC
>>> would have to do it).
>>> which APIs (other than syscon) would you suggest for this case?
>>
>> I would suggest avoiding the syscon API when you have other drivers going
>> through the same register space. You can do a syscon like approach by
>> having the HHI driver register one or multiple regmaps, with appropriate
>> access controls, which other drivers can fetch using dev_get_regmap() (via
>> a phandle to the HHI device). I don't know much about simple-mfd though.
>> If the registers are neatly grouped together by function it would be easier
>> to deal with. Or you could just have one device node with appropriate cell
>> properties and have a big driver register all the functions. That driver
>> would likely live under drivers/soc/.
> 
> Well, it's the role of simple-mfd, when you only have plaform-tied sub drivers,
> and it's over-engineered to write an MFD driver for this.
> 
> At some point, we should move forward with this since it's a "system controller"
> specific mapping, and "syscon" was designed specifically to solve this kind
> of mapping in DT.
> 
> The other SoCs (GX, AXG and G12A) uses this exact DT architecture to solve this
> issue, and we should use this same schema for other Amlogic SoCs even if it's
> not the most perfect way to solve it.

Considering the "syscon" and "simple-mfd" bindings documentation not marked
deprecated, and that the whole Amlogic architecture (and new SoCs support)
is already using this design, I don't see any reason not aligning the
Meson8 & Meson8b clk support on the ARM64 variants scheme.

I do act we will need to avoid syscon for *new* driver and DT designs.

Applied on next/drivers for 4.21

Neil

> 
> Neil
> 
>>
>> ChenYu
>>
>>>
>>> Regards
>>> Martin
>>>
>>>
>>> [0] https://patchwork.kernel.org/patch/10658557/
>>> [1] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_hw.c
>>> [2] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_reg.c
>>> [3] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/include/mach/register.h
>