linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] DRAM devfreq support for Allwinner A64/H5
@ 2021-09-29  4:42 Samuel Holland
  2021-09-29  4:42 ` [PATCH 01/10] PM / devfreq: strengthen check for freq_table Samuel Holland
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Samuel Holland @ 2021-09-29  4:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Samuel Holland

This series adds a new devfreq driver for the MBUS/DRAM controller in
some Allwinner SoCs, and enables it for the A64 and H5.

The first four patches make some improvements to the devfreq core to
support drivers without any OPPs in the DT. They could be merged
independently. (Since this driver controls only a divider, and can only
slow down the DRAM clock, it works with most any base DRAM frequency.)

Then the binding and DTs are updated in patches 5-9. The MBUS nodes
already existed, but were not bound to any driver before; they were only
used for their dma-ranges property.

Finally, the driver is added in patch 10.

I am not 100% sure the best way to handle DRAM register access -- as a
separate reg property, a separate node, or simply enlarging the MBUS
register range. While the DRAM controller is a separate IP block, the
MBUS hardware has the ability to double-buffer certain DRAM controller
registers, and the hardware MDFS process writes to some DRAM controller
registers as well. So they are rather tightly integrated.

Like the patch 10 description says, this driver could support additional
SoCs: at least A33, A83T, and H3. I can send follow-up patches for
these, but I cannot test A33 or A83T.

Samuel Holland (10):
  PM / devfreq: strengthen check for freq_table
  PM / devfreq: Do not require devices to have OPPs
  PM / devfreq: Drop code for descending freq_table
  PM / devfreq: Add a recommended frequency helper
  dt-bindings: clock: sunxi: Export CLK_DRAM for devfreq
  dt-bindings: arm: sunxi: Expand MBUS binding
  dt-bindings: arm: sunxi: Add H5 MBUS compatible
  ARM: dts: sunxi: h3/h5: Update MBUS node
  arm64: dts: allwinner: a64: Update MBUS node
  PM / devfreq: Add a driver for the sun8i/sun50i MBUS

 .../arm/sunxi/allwinner,sun4i-a10-mbus.yaml   |  77 ++-
 arch/arm/boot/dts/sun8i-h3.dtsi               |   4 +
 arch/arm/boot/dts/sunxi-h3-h5.dtsi            |  11 +-
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  10 +-
 arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |   4 +
 drivers/clk/sunxi-ng/ccu-sun50i-a64.h         |   2 -
 drivers/clk/sunxi-ng/ccu-sun8i-h3.h           |   2 -
 drivers/devfreq/Kconfig                       |   8 +
 drivers/devfreq/Makefile                      |   1 +
 drivers/devfreq/devfreq.c                     |  77 ++-
 drivers/devfreq/sun8i-a33-mbus.c              | 482 ++++++++++++++++++
 include/dt-bindings/clock/sun50i-a64-ccu.h    |   2 +-
 include/dt-bindings/clock/sun8i-h3-ccu.h      |   2 +-
 include/linux/devfreq.h                       |   2 +
 14 files changed, 644 insertions(+), 40 deletions(-)
 create mode 100644 drivers/devfreq/sun8i-a33-mbus.c

-- 
2.31.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 01/10] PM / devfreq: strengthen check for freq_table
  2021-09-29  4:42 [PATCH 00/10] DRAM devfreq support for Allwinner A64/H5 Samuel Holland
@ 2021-09-29  4:42 ` Samuel Holland
  2021-09-30  3:58   ` Chanwoo Choi
  2021-09-29  4:42 ` [PATCH 02/10] PM / devfreq: Do not require devices to have OPPs Samuel Holland
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Samuel Holland @ 2021-09-29  4:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Samuel Holland

Since commit ea572f816032 ("PM / devfreq: Change return type of
devfreq_set_freq_table()"), all devfreq devices are expected to have a
valid freq_table. The devfreq core unconditionally dereferences
freq_table in the sysfs code and in get_freq_range().

Therefore, we need to ensure that freq_table is both non-null and
non-empty (length is > 0). If either check fails, replace the table
using set_freq_table() or return the error.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/devfreq/devfreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 28f3e0ba6cdd..7a8b022ba456 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -827,7 +827,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		goto err_dev;
 	}
 
-	if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
+	if (!devfreq->profile->max_state || !devfreq->profile->freq_table) {
 		mutex_unlock(&devfreq->lock);
 		err = set_freq_table(devfreq);
 		if (err < 0)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 02/10] PM / devfreq: Do not require devices to have OPPs
  2021-09-29  4:42 [PATCH 00/10] DRAM devfreq support for Allwinner A64/H5 Samuel Holland
  2021-09-29  4:42 ` [PATCH 01/10] PM / devfreq: strengthen check for freq_table Samuel Holland
@ 2021-09-29  4:42 ` Samuel Holland
  2021-09-30  4:19   ` Chanwoo Choi
  2021-09-29  4:42 ` [PATCH 03/10] PM / devfreq: Drop code for descending freq_table Samuel Holland
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Samuel Holland @ 2021-09-29  4:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Samuel Holland

Since commit ea572f816032 ("PM / devfreq: Change return type of
devfreq_set_freq_table()"), all devfreq devices are required to have a
valid freq_table. If freq_table is not provided by the driver, it will
be filled in by set_freq_table() from the OPPs; if that fails,
devfreq_add_device() will return an error.

However, since commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when
adding the devfreq device"), devfreq devices are _also_ required to have
an OPP table, even if they provide freq_table. devfreq_add_device()
requires dev_pm_opp_find_freq_ceil() and dev_pm_opp_find_freq_floor() to
return successfully, specifically to initialize scaling_min/max_freq.

Not all drivers need an OPP table. For example, a driver where all
frequencies are determined dynamically could work by filling out only
freq_table. But with the current code it must call dev_pm_opp_add() on
every freq_table entry to probe successfully.

The offending properties, scaling_min/max_freq, are only necessary if a
device has OPPs; if no OPPs exist at all, OPPs cannot be dynamically
enabled or disabled, so those values have no effect. Thus it is trivial
to restore support for devices with freq_table only and not OPPs -- move
those initializations behind the check for a valid OPP table.

Since get_freq_range() uses scaling_max_freq in a min() expression, it
must be initialized to the maximum possible value. scaling_min_freq is
initialized as well for consistency.

Fixes: ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when adding the devfreq device")
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/devfreq/devfreq.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 7a8b022ba456..426e31e6c448 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -835,24 +835,28 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		mutex_lock(&devfreq->lock);
 	}
 
-	devfreq->scaling_min_freq = find_available_min_freq(devfreq);
-	if (!devfreq->scaling_min_freq) {
-		mutex_unlock(&devfreq->lock);
-		err = -EINVAL;
-		goto err_dev;
-	}
-
-	devfreq->scaling_max_freq = find_available_max_freq(devfreq);
-	if (!devfreq->scaling_max_freq) {
-		mutex_unlock(&devfreq->lock);
-		err = -EINVAL;
-		goto err_dev;
-	}
-
-	devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
 	devfreq->opp_table = dev_pm_opp_get_opp_table(dev);
-	if (IS_ERR(devfreq->opp_table))
+	if (IS_ERR(devfreq->opp_table)) {
 		devfreq->opp_table = NULL;
+		devfreq->scaling_min_freq = 0;
+		devfreq->scaling_max_freq = ULONG_MAX;
+	} else {
+		devfreq->scaling_min_freq = find_available_min_freq(devfreq);
+		if (!devfreq->scaling_min_freq) {
+			mutex_unlock(&devfreq->lock);
+			err = -EINVAL;
+			goto err_dev;
+		}
+
+		devfreq->scaling_max_freq = find_available_max_freq(devfreq);
+		if (!devfreq->scaling_max_freq) {
+			mutex_unlock(&devfreq->lock);
+			err = -EINVAL;
+			goto err_dev;
+		}
+
+		devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
+	}
 
 	atomic_set(&devfreq->suspend_count, 0);
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 03/10] PM / devfreq: Drop code for descending freq_table
  2021-09-29  4:42 [PATCH 00/10] DRAM devfreq support for Allwinner A64/H5 Samuel Holland
  2021-09-29  4:42 ` [PATCH 01/10] PM / devfreq: strengthen check for freq_table Samuel Holland
  2021-09-29  4:42 ` [PATCH 02/10] PM / devfreq: Do not require devices to have OPPs Samuel Holland
@ 2021-09-29  4:42 ` Samuel Holland
  2021-09-29  4:42 ` [PATCH 04/10] PM / devfreq: Add a recommended frequency helper Samuel Holland
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Samuel Holland @ 2021-09-29  4:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Samuel Holland

Since commit 416b46a2627a ("PM / devfreq: Show the all available
frequencies"), freq_table's documentation requires it to be sorted in
ascending order. That commit modified available_frequencies_show() to
assume that order. This is also the order used by all existing drivers
and by set_freq_table().

However, there is still some code left over for compatibility with a
freq_table sorted descending. To avoid confusion, let's remove it.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/devfreq/devfreq.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 426e31e6c448..f5d27f5285db 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -130,16 +130,10 @@ static void get_freq_range(struct devfreq *devfreq,
 
 	/*
 	 * Initialize minimum/maximum frequency from freq table.
-	 * The devfreq drivers can initialize this in either ascending or
-	 * descending order and devfreq core supports both.
+	 * The devfreq drivers should initialize this in ascending order.
 	 */
-	if (freq_table[0] < freq_table[devfreq->profile->max_state - 1]) {
-		*min_freq = freq_table[0];
-		*max_freq = freq_table[devfreq->profile->max_state - 1];
-	} else {
-		*min_freq = freq_table[devfreq->profile->max_state - 1];
-		*max_freq = freq_table[0];
-	}
+	*min_freq = freq_table[0];
+	*max_freq = freq_table[devfreq->profile->max_state - 1];
 
 	/* Apply constraints from PM QoS */
 	qos_min_freq = dev_pm_qos_read_value(devfreq->dev.parent,
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 04/10] PM / devfreq: Add a recommended frequency helper
  2021-09-29  4:42 [PATCH 00/10] DRAM devfreq support for Allwinner A64/H5 Samuel Holland
                   ` (2 preceding siblings ...)
  2021-09-29  4:42 ` [PATCH 03/10] PM / devfreq: Drop code for descending freq_table Samuel Holland
@ 2021-09-29  4:42 ` Samuel Holland
  2021-09-29  4:42 ` [PATCH 05/10] dt-bindings: clock: sunxi: Export CLK_DRAM for devfreq Samuel Holland
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Samuel Holland @ 2021-09-29  4:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Samuel Holland

This helper peforms the same function as devfreq_recommended_opp().
However, it works on devices without OPPs by iterating over freq_table.
Since freq_table is assumed to be sorted in ascending order, the
algorithm is relatively simple.

Devices with OPPs should continue using devfreq_recommended_opp(), as
that function respects disabled OPPs.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/devfreq/devfreq.c | 27 +++++++++++++++++++++++++++
 include/linux/devfreq.h   |  2 ++
 2 files changed, 29 insertions(+)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index f5d27f5285db..fd46792297ad 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1984,6 +1984,33 @@ subsys_initcall(devfreq_init);
  * OPP framework.
  */
 
+/**
+ * devfreq_recommended_freq() - Helper function to get the proper frequency from
+ *			        freq_table for the value given to target callback.
+ * @devfreq:	The devfreq device.
+ * @freq:	The frequency given to target function.
+ * @flags:	Flags handed from devfreq framework.
+ */
+void devfreq_recommended_freq(struct devfreq *devfreq,
+			      unsigned long *freq, u32 flags)
+{
+	const unsigned long *min = devfreq->profile->freq_table;
+	const unsigned long *max = min + devfreq->profile->max_state - 1;
+	const unsigned long *f;
+
+	if (flags & DEVFREQ_FLAG_LEAST_UPPER_BOUND) {
+		/* Find the first item lower than freq, or else min. */
+		for (f = max; f > min && *f > *freq; --f)
+			;
+	} else {
+		/* Find the first item higher than freq, or else max. */
+		for (f = min; f < max && *f < *freq; ++f)
+			;
+	}
+	*freq = *f;
+}
+EXPORT_SYMBOL(devfreq_recommended_freq);
+
 /**
  * devfreq_recommended_opp() - Helper function to get proper OPP for the
  *			     freq value given to target callback.
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 142474b4af96..4d324fea8a78 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -239,6 +239,8 @@ void devfreq_resume(void);
 int update_devfreq(struct devfreq *devfreq);
 
 /* Helper functions for devfreq user device driver with OPP. */
+void devfreq_recommended_freq(struct devfreq *devfreq,
+			      unsigned long *freq, u32 flags);
 struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
 				unsigned long *freq, u32 flags);
 int devfreq_register_opp_notifier(struct device *dev,
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 05/10] dt-bindings: clock: sunxi: Export CLK_DRAM for devfreq
  2021-09-29  4:42 [PATCH 00/10] DRAM devfreq support for Allwinner A64/H5 Samuel Holland
                   ` (3 preceding siblings ...)
  2021-09-29  4:42 ` [PATCH 04/10] PM / devfreq: Add a recommended frequency helper Samuel Holland
@ 2021-09-29  4:42 ` Samuel Holland
  2021-09-29  4:42 ` [PATCH 06/10] dt-bindings: arm: sunxi: Expand MBUS binding Samuel Holland
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Samuel Holland @ 2021-09-29  4:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Samuel Holland

The MBUS node needs to reference the CLK_DRAM clock, as the MBUS
hardware implements memory dynamic frequency scaling using this clock.

Export this clock for SoCs which will be getting a devfreq driver.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/clk/sunxi-ng/ccu-sun50i-a64.h      | 2 --
 drivers/clk/sunxi-ng/ccu-sun8i-h3.h        | 2 --
 include/dt-bindings/clock/sun50i-a64-ccu.h | 2 +-
 include/dt-bindings/clock/sun8i-h3-ccu.h   | 2 +-
 4 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.h b/drivers/clk/sunxi-ng/ccu-sun50i-a64.h
index 54d1f96f4b68..a8c11c0b4e06 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.h
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.h
@@ -51,8 +51,6 @@
 
 #define CLK_USB_OHCI1_12M		92
 
-#define CLK_DRAM			94
-
 /* All the DRAM gates are exported */
 
 /* And the DSI and GPU module clock is exported */
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-h3.h b/drivers/clk/sunxi-ng/ccu-sun8i-h3.h
index d8c38447e11b..e13f3c4b57d0 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-h3.h
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-h3.h
@@ -42,8 +42,6 @@
 
 /* The first bunch of module clocks are exported */
 
-#define CLK_DRAM		96
-
 /* All the DRAM gates are exported */
 
 /* Some more module clocks are exported */
diff --git a/include/dt-bindings/clock/sun50i-a64-ccu.h b/include/dt-bindings/clock/sun50i-a64-ccu.h
index 318eb15c414c..175892189e9d 100644
--- a/include/dt-bindings/clock/sun50i-a64-ccu.h
+++ b/include/dt-bindings/clock/sun50i-a64-ccu.h
@@ -113,7 +113,7 @@
 #define CLK_USB_OHCI0		91
 
 #define CLK_USB_OHCI1		93
-
+#define CLK_DRAM		94
 #define CLK_DRAM_VE		95
 #define CLK_DRAM_CSI		96
 #define CLK_DRAM_DEINTERLACE	97
diff --git a/include/dt-bindings/clock/sun8i-h3-ccu.h b/include/dt-bindings/clock/sun8i-h3-ccu.h
index 30d2d15373a2..5d4ada2c22e6 100644
--- a/include/dt-bindings/clock/sun8i-h3-ccu.h
+++ b/include/dt-bindings/clock/sun8i-h3-ccu.h
@@ -126,7 +126,7 @@
 #define CLK_USB_OHCI1		93
 #define CLK_USB_OHCI2		94
 #define CLK_USB_OHCI3		95
-
+#define CLK_DRAM		96
 #define CLK_DRAM_VE		97
 #define CLK_DRAM_CSI		98
 #define CLK_DRAM_DEINTERLACE	99
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 06/10] dt-bindings: arm: sunxi: Expand MBUS binding
  2021-09-29  4:42 [PATCH 00/10] DRAM devfreq support for Allwinner A64/H5 Samuel Holland
                   ` (4 preceding siblings ...)
  2021-09-29  4:42 ` [PATCH 05/10] dt-bindings: clock: sunxi: Export CLK_DRAM for devfreq Samuel Holland
@ 2021-09-29  4:42 ` Samuel Holland
  2021-09-29 13:46   ` Rob Herring
  2021-09-29  4:42 ` [PATCH 07/10] dt-bindings: arm: sunxi: Add H5 MBUS compatible Samuel Holland
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Samuel Holland @ 2021-09-29  4:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Samuel Holland

The MBUS provides more than address translation and bandwidth control.
It also provides a PMU to measure bandwidth usage by certain masters,
and it provides notification via IRQ when they are active or idle.

The MBUS is also tightly integrated with the DRAM controller to provide
a Memory Dynamic Frequency Scaling (MDFS) feature. In view of this, the
MBUS binding needs to represent the hardware resources needed for MDFS,
which include the clocks and MMIO range of the adjacent DRAM controller.

Add the additional resources for the H3 and A64 compatibles, and a new
example showing how they are used.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 .../arm/sunxi/allwinner,sun4i-a10-mbus.yaml   | 75 ++++++++++++++++++-
 1 file changed, 72 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/sunxi/allwinner,sun4i-a10-mbus.yaml b/Documentation/devicetree/bindings/arm/sunxi/allwinner,sun4i-a10-mbus.yaml
index e713a6fe4cf7..c1fb404d2fb3 100644
--- a/Documentation/devicetree/bindings/arm/sunxi/allwinner,sun4i-a10-mbus.yaml
+++ b/Documentation/devicetree/bindings/arm/sunxi/allwinner,sun4i-a10-mbus.yaml
@@ -33,10 +33,33 @@ properties:
       - allwinner,sun50i-a64-mbus
 
   reg:
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: MBUS interconnect/bandwidth/PMU registers
+      - description: DRAM controller/PHY registers
+
+  reg-names:
+    items:
+      - const: "mbus"
+      - const: "dram"
 
   clocks:
+    minItems: 1
+    items:
+      - description: MBUS interconnect module clock
+      - description: DRAM controller/PHY module clock
+      - description: Register bus clock, shared by MBUS and DRAM
+
+  clock-names:
+    items:
+      - const: "mbus"
+      - const: "dram"
+      - const: "bus"
+
+  interrupts:
     maxItems: 1
+    description:
+      MBUS PMU activity interrupt.
 
   dma-ranges:
     description:
@@ -53,13 +76,42 @@ required:
   - clocks
   - dma-ranges
 
+if:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - allwinner,sun8i-h3-mbus
+          - allwinner,sun50i-a64-mbus
+
+then:
+  properties:
+    reg:
+      minItems: 2
+
+    clocks:
+      minItems: 3
+
+  required:
+    - reg-names
+    - clock-names
+
+else:
+  properties:
+    reg:
+      maxItems: 1
+
+    clocks:
+      maxItems: 1
+
 additionalProperties: false
 
 examples:
   - |
-    #include <dt-bindings/clock/sun5i-ccu.h>
+    #include <dt-bindings/clock/sun50i-a64-ccu.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
 
-    mbus: dram-controller@1c01000 {
+    dram-controller@1c01000 {
         compatible = "allwinner,sun5i-a13-mbus";
         reg = <0x01c01000 0x1000>;
         clocks = <&ccu CLK_MBUS>;
@@ -69,4 +121,21 @@ examples:
         #interconnect-cells = <1>;
     };
 
+  - |
+    dram-controller@1c62000 {
+        compatible = "allwinner,sun50i-a64-mbus";
+        reg = <0x01c62000 0x1000>,
+              <0x01c63000 0x1000>;
+        reg-names = "mbus", "dram";
+        clocks = <&ccu CLK_MBUS>,
+                 <&ccu CLK_DRAM>,
+                 <&ccu CLK_BUS_DRAM>;
+        clock-names = "mbus", "dram", "bus";
+        interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+        dma-ranges = <0x00000000 0x40000000 0xc0000000>;
+        #interconnect-cells = <1>;
+    };
+
 ...
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 07/10] dt-bindings: arm: sunxi: Add H5 MBUS compatible
  2021-09-29  4:42 [PATCH 00/10] DRAM devfreq support for Allwinner A64/H5 Samuel Holland
                   ` (5 preceding siblings ...)
  2021-09-29  4:42 ` [PATCH 06/10] dt-bindings: arm: sunxi: Expand MBUS binding Samuel Holland
@ 2021-09-29  4:42 ` Samuel Holland
  2021-09-29  4:42 ` [PATCH 08/10] ARM: dts: sunxi: h3/h5: Update MBUS node Samuel Holland
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Samuel Holland @ 2021-09-29  4:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Samuel Holland

The H5 SoC has a MBUS very similar to the H3 SoC, but it has a smaller
MDFS divider range (1-4 instead of 1-16). Add a separate compatible for
this variant.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 .../devicetree/bindings/arm/sunxi/allwinner,sun4i-a10-mbus.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/sunxi/allwinner,sun4i-a10-mbus.yaml b/Documentation/devicetree/bindings/arm/sunxi/allwinner,sun4i-a10-mbus.yaml
index c1fb404d2fb3..c070f99e0bb7 100644
--- a/Documentation/devicetree/bindings/arm/sunxi/allwinner,sun4i-a10-mbus.yaml
+++ b/Documentation/devicetree/bindings/arm/sunxi/allwinner,sun4i-a10-mbus.yaml
@@ -31,6 +31,7 @@ properties:
       - allwinner,sun5i-a13-mbus
       - allwinner,sun8i-h3-mbus
       - allwinner,sun50i-a64-mbus
+      - allwinner,sun50i-h5-mbus
 
   reg:
     minItems: 1
@@ -83,6 +84,7 @@ if:
         enum:
           - allwinner,sun8i-h3-mbus
           - allwinner,sun50i-a64-mbus
+          - allwinner,sun50i-h5-mbus
 
 then:
   properties:
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 08/10] ARM: dts: sunxi: h3/h5: Update MBUS node
  2021-09-29  4:42 [PATCH 00/10] DRAM devfreq support for Allwinner A64/H5 Samuel Holland
                   ` (6 preceding siblings ...)
  2021-09-29  4:42 ` [PATCH 07/10] dt-bindings: arm: sunxi: Add H5 MBUS compatible Samuel Holland
@ 2021-09-29  4:42 ` Samuel Holland
  2021-09-29  4:42 ` [PATCH 09/10] arm64: dts: allwinner: a64: " Samuel Holland
  2021-09-29  4:42 ` [PATCH 10/10] PM / devfreq: Add a driver for the sun8i/sun50i MBUS Samuel Holland
  9 siblings, 0 replies; 19+ messages in thread
From: Samuel Holland @ 2021-09-29  4:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Samuel Holland

In order to support memory dynamic frequency scaling (MDFS), the MBUS
binding now requires enumerating more resources. Provide them in the
device tree.

Since the H3 and H5 have different clock divider limits, they need
separate compatibles.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 arch/arm/boot/dts/sun8i-h3.dtsi              |  4 ++++
 arch/arm/boot/dts/sunxi-h3-h5.dtsi           | 11 ++++++++---
 arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi |  4 ++++
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 4e89701df91f..43acb98cf390 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -282,6 +282,10 @@ &display_clocks {
 	compatible = "allwinner,sun8i-h3-de2-clk";
 };
 
+&mbus {
+	compatible = "allwinner,sun8i-h3-mbus";
+};
+
 &mmc0 {
 	compatible = "allwinner,sun7i-a20-mmc";
 	clocks = <&ccu CLK_BUS_MMC0>,
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index c7428df9469e..3a683e190dab 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -568,9 +568,14 @@ external_mdio: mdio@2 {
 		};
 
 		mbus: dram-controller@1c62000 {
-			compatible = "allwinner,sun8i-h3-mbus";
-			reg = <0x01c62000 0x1000>;
-			clocks = <&ccu CLK_MBUS>;
+			/* compatible is in per SoC .dtsi file */
+			reg = <0x01c62000 0x1000>,
+			      <0x01c63000 0x1000>;
+			reg-names = "mbus", "dram";
+			clocks = <&ccu CLK_MBUS>,
+				 <&ccu CLK_DRAM>,
+				 <&ccu CLK_BUS_DRAM>;
+			clock-names = "mbus", "dram", "bus";
 			#address-cells = <1>;
 			#size-cells = <1>;
 			dma-ranges = <0x00000000 0x40000000 0xc0000000>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
index 578a63dedf46..35d5d238e313 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
@@ -233,6 +233,10 @@ &display_clocks {
 	compatible = "allwinner,sun50i-h5-de2-clk";
 };
 
+&mbus {
+	compatible = "allwinner,sun50i-h5-mbus";
+};
+
 &mmc0 {
 	compatible = "allwinner,sun50i-h5-mmc",
 		     "allwinner,sun50i-a64-mmc";
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 09/10] arm64: dts: allwinner: a64: Update MBUS node
  2021-09-29  4:42 [PATCH 00/10] DRAM devfreq support for Allwinner A64/H5 Samuel Holland
                   ` (7 preceding siblings ...)
  2021-09-29  4:42 ` [PATCH 08/10] ARM: dts: sunxi: h3/h5: Update MBUS node Samuel Holland
@ 2021-09-29  4:42 ` Samuel Holland
  2021-09-29  4:42 ` [PATCH 10/10] PM / devfreq: Add a driver for the sun8i/sun50i MBUS Samuel Holland
  9 siblings, 0 replies; 19+ messages in thread
From: Samuel Holland @ 2021-09-29  4:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Samuel Holland

In order to support memory dynamic frequency scaling (MDFS), the MBUS
binding now requires enumerating more resources. Provide them in the
device tree.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 6ddb717f2f98..609a59c6c778 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -1129,8 +1129,14 @@ pwm: pwm@1c21400 {
 
 		mbus: dram-controller@1c62000 {
 			compatible = "allwinner,sun50i-a64-mbus";
-			reg = <0x01c62000 0x1000>;
-			clocks = <&ccu 112>;
+			reg = <0x01c62000 0x1000>,
+			      <0x01c63000 0x1000>;
+			reg-names = "mbus", "dram";
+			clocks = <&ccu CLK_MBUS>,
+				 <&ccu CLK_DRAM>,
+				 <&ccu CLK_BUS_DRAM>;
+			clock-names = "mbus", "dram", "bus";
+			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
 			#address-cells = <1>;
 			#size-cells = <1>;
 			dma-ranges = <0x00000000 0x40000000 0xc0000000>;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 10/10] PM / devfreq: Add a driver for the sun8i/sun50i MBUS
  2021-09-29  4:42 [PATCH 00/10] DRAM devfreq support for Allwinner A64/H5 Samuel Holland
                   ` (8 preceding siblings ...)
  2021-09-29  4:42 ` [PATCH 09/10] arm64: dts: allwinner: a64: " Samuel Holland
@ 2021-09-29  4:42 ` Samuel Holland
  2021-09-30  4:35   ` Chanwoo Choi
  9 siblings, 1 reply; 19+ messages in thread
From: Samuel Holland @ 2021-09-29  4:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Samuel Holland

This driver works by adjusting the divider on the DRAM controller's
module clock. Thus there is no fixed set of OPPs, only "full speed" down
to "quarter speed" (or whatever the maximum divider is on that variant).

It makes use of the MDFS hardware in the MBUS, in "DFS" mode, which
takes care of updating registers during the critical section while DRAM
is inaccessible.

This driver should support several sunxi SoCs, starting with the A33,
which have a DesignWare DDR3 controller with merged PHY register space
and the matching MBUS register layout (so not A63 or later). However,
the driver has only been tested on the A64/H5, so those are the only
compatibles enabled for now.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/devfreq/Kconfig          |   8 +
 drivers/devfreq/Makefile         |   1 +
 drivers/devfreq/sun8i-a33-mbus.c | 482 +++++++++++++++++++++++++++++++
 3 files changed, 491 insertions(+)
 create mode 100644 drivers/devfreq/sun8i-a33-mbus.c

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index e87d01c0b76a..b94eb04761f6 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -132,6 +132,14 @@ config ARM_RK3399_DMC_DEVFREQ
 	  It sets the frequency for the memory controller and reads the usage counts
 	  from hardware.
 
+config ARM_SUN8I_A33_MBUS_DEVFREQ
+	tristate "sun8i/sun50i MBUS DEVFREQ Driver"
+	depends on ARCH_SUNXI || COMPILE_TEST
+	select DEVFREQ_GOV_SIMPLE_ONDEMAND
+	help
+	  This adds the DEVFREQ driver for the MBUS controller in some
+	  Allwinner sun8i (A33 through H3) and sun50i (A64 and H5) SoCs.
+
 source "drivers/devfreq/event/Kconfig"
 
 endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
index a16333ea7034..0b6be92a25d9 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ)	+= exynos-bus.o
 obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ)	+= imx-bus.o
 obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ)	+= imx8m-ddrc.o
 obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ)	+= rk3399_dmc.o
+obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ)	+= sun8i-a33-mbus.o
 obj-$(CONFIG_ARM_TEGRA_DEVFREQ)		+= tegra30-devfreq.o
 
 # DEVFREQ Event Drivers
diff --git a/drivers/devfreq/sun8i-a33-mbus.c b/drivers/devfreq/sun8i-a33-mbus.c
new file mode 100644
index 000000000000..00f24850cf8d
--- /dev/null
+++ b/drivers/devfreq/sun8i-a33-mbus.c
@@ -0,0 +1,482 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Copyright (C) 2020-2021 Samuel Holland <samuel@sholland.org>
+//
+
+#include <linux/clk.h>
+#include <linux/devfreq.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+
+#define MBUS_CR				0x0000
+#define MBUS_CR_GET_DRAM_TYPE(x)	(((x) >> 16) & 0x7)
+#define MBUS_CR_DRAM_TYPE_DDR2		2
+#define MBUS_CR_DRAM_TYPE_DDR3		3
+#define MBUS_CR_DRAM_TYPE_DDR4		4
+#define MBUS_CR_DRAM_TYPE_LPDDR2	6
+#define MBUS_CR_DRAM_TYPE_LPDDR3	7
+
+#define MBUS_TMR			0x000c
+#define MBUS_TMR_PERIOD(x)		((x) - 1)
+
+#define MBUS_PMU_CFG			0x009c
+#define MBUS_PMU_CFG_PERIOD(x)		(((x) - 1) << 16)
+#define MBUS_PMU_CFG_UNIT		(0x3 << 1)
+#define MBUS_PMU_CFG_UNIT_B		(0x0 << 1)
+#define MBUS_PMU_CFG_UNIT_KB		(0x1 << 1)
+#define MBUS_PMU_CFG_UNIT_MB		(0x2 << 1)
+#define MBUS_PMU_CFG_ENABLE		(0x1 << 0)
+
+#define MBUS_PMU_BWCR(n)		(0x00a0 + (0x04 * (n)))
+
+#define MBUS_TOTAL_BWCR			MBUS_PMU_BWCR(5)
+#define MBUS_TOTAL_BWCR_H616		MBUS_PMU_BWCR(13)
+
+#define MBUS_MDFSCR			0x0100
+#define MBUS_MDFSCR_BUFFER_TIMING	(0x1 << 15)
+#define MBUS_MDFSCR_PAD_HOLD		(0x1 << 13)
+#define MBUS_MDFSCR_BYPASS		(0x1 << 4)
+#define MBUS_MDFSCR_MODE		(0x1 << 1)
+#define MBUS_MDFSCR_MODE_DFS		(0x0 << 1)
+#define MBUS_MDFSCR_MODE_CFS		(0x1 << 1)
+#define MBUS_MDFSCR_START		(0x1 << 0)
+
+#define MBUS_MDFSMRMR			0x0108
+
+#define DRAM_PWRCTL			0x0004
+#define DRAM_PWRCTL_SELFREF_EN		(0x1 << 0)
+
+#define DRAM_RFSHTMG			0x0090
+#define DRAM_RFSHTMG_TREFI(x)		((x) << 16)
+#define DRAM_RFSHTMG_TRFC(x)		((x) << 0)
+
+#define DRAM_VTFCR			0x00b8
+#define DRAM_VTFCR_VTF_ENABLE		(0x3 << 8)
+
+#define DRAM_ODTMAP			0x0120
+
+#define DRAM_DX_MAX			4
+
+#define DRAM_DXnGCR0(n)			(0x0344 + 0x80 * (n))
+#define DRAM_DXnGCR0_DXODT		(0x3 << 4)
+#define DRAM_DXnGCR0_DXODT_DYNAMIC	(0x0 << 4)
+#define DRAM_DXnGCR0_DXODT_ENABLED	(0x1 << 4)
+#define DRAM_DXnGCR0_DXODT_DISABLED	(0x2 << 4)
+#define DRAM_DXnGCR0_DXEN		(0x1 << 0)
+
+struct sun8i_a33_mbus_variant {
+	u32					min_dram_divider;
+	u32					max_dram_divider;
+	u32					odt_freq_mhz;
+};
+
+struct sun8i_a33_mbus {
+	const struct sun8i_a33_mbus_variant	*variant;
+	void __iomem				*reg_dram;
+	void __iomem				*reg_mbus;
+	struct clk				*clk_bus;
+	struct clk				*clk_dram;
+	struct clk				*clk_mbus;
+	struct devfreq				*devfreq_dram;
+	struct devfreq_simple_ondemand_data	gov_data;
+	struct devfreq_dev_profile		profile;
+	u32					data_width;
+	u32					nominal_bw;
+	u32					odtmap;
+	u32					tREFI_ns;
+	u32					tRFC_ns;
+	unsigned long				freq_table[];
+};
+
+/*
+ * The unit for this value is (MBUS clock cycles / MBUS_TMR_PERIOD). When
+ * MBUS_TMR_PERIOD is programmed to match the MBUS clock frequency in MHz, as
+ * it is during DRAM init and during probe, the resulting unit is microseconds.
+ */
+static int pmu_period = 50000;
+module_param(pmu_period, int, 0644);
+MODULE_PARM_DESC(pmu_period, "Bandwidth measurement period (microseconds)");
+
+static u32 sun8i_a33_mbus_get_peak_bw(struct sun8i_a33_mbus *priv)
+{
+	/* Returns the peak transfer (in KiB) during any single PMU period. */
+	return readl_relaxed(priv->reg_mbus + MBUS_TOTAL_BWCR);
+}
+
+static void sun8i_a33_mbus_restart_pmu_counters(struct sun8i_a33_mbus *priv)
+{
+	u32 pmu_cfg = MBUS_PMU_CFG_PERIOD(pmu_period) | MBUS_PMU_CFG_UNIT_KB;
+
+	/* All PMU counters are cleared on a disable->enable transition. */
+	writel_relaxed(pmu_cfg,
+		       priv->reg_mbus + MBUS_PMU_CFG);
+	writel_relaxed(pmu_cfg | MBUS_PMU_CFG_ENABLE,
+		       priv->reg_mbus + MBUS_PMU_CFG);
+
+}
+
+static void sun8i_a33_mbus_update_nominal_bw(struct sun8i_a33_mbus *priv,
+					     u32 ddr_freq_mhz)
+{
+	/*
+	 * Nominal bandwidth (KiB per PMU period):
+	 *
+	 *   DDR transfers   microseconds     KiB
+	 *   ------------- * ------------ * --------
+	 *    microsecond     PMU period    transfer
+	 */
+	priv->nominal_bw = ddr_freq_mhz * pmu_period * priv->data_width / 1024;
+}
+
+static int sun8i_a33_mbus_set_dram_freq(struct sun8i_a33_mbus *priv,
+					unsigned long freq)
+{
+	u32  ddr_freq_mhz = freq / USEC_PER_SEC; /* DDR */
+	u32 dram_freq_mhz =    ddr_freq_mhz / 2; /* SDR */
+	u32 mctl_freq_mhz =   dram_freq_mhz / 2; /* HDR */
+	u32 dxodt, mdfscr, pwrctl, vtfcr;
+	u32 i, tREFI_32ck, tRFC_ck;
+	int ret;
+
+	/* The rate change is not effective until the MDFS process runs. */
+	ret = clk_set_rate(priv->clk_dram, freq);
+	if (ret)
+		return ret;
+
+	/* Disable automatic self-refesh and VTF before starting MDFS. */
+	pwrctl = readl_relaxed(priv->reg_dram + DRAM_PWRCTL) &
+		 ~DRAM_PWRCTL_SELFREF_EN;
+	writel_relaxed(pwrctl, priv->reg_dram + DRAM_PWRCTL);
+	vtfcr = readl_relaxed(priv->reg_dram + DRAM_VTFCR);
+	writel_relaxed(vtfcr & ~DRAM_VTFCR_VTF_ENABLE,
+		       priv->reg_dram + DRAM_VTFCR);
+
+	/* Set up MDFS and enable double buffering for timing registers. */
+	mdfscr = MBUS_MDFSCR_MODE_DFS |
+		 MBUS_MDFSCR_BYPASS |
+		 MBUS_MDFSCR_PAD_HOLD |
+		 MBUS_MDFSCR_BUFFER_TIMING;
+	writel(mdfscr, priv->reg_mbus + MBUS_MDFSCR);
+
+	/* Update the buffered copy of RFSHTMG. */
+	tREFI_32ck = priv->tREFI_ns * mctl_freq_mhz / 1000 / 32;
+	tRFC_ck = DIV_ROUND_UP(priv->tRFC_ns * mctl_freq_mhz, 1000);
+	writel(DRAM_RFSHTMG_TREFI(tREFI_32ck) | DRAM_RFSHTMG_TRFC(tRFC_ck),
+	       priv->reg_dram + DRAM_RFSHTMG);
+
+	/* Enable ODT if needed, or disable it to save power. */
+	if (priv->odtmap && dram_freq_mhz > priv->variant->odt_freq_mhz) {
+		dxodt = DRAM_DXnGCR0_DXODT_DYNAMIC;
+		writel(priv->odtmap, priv->reg_dram + DRAM_ODTMAP);
+	} else {
+		dxodt = DRAM_DXnGCR0_DXODT_DISABLED;
+		writel(0, priv->reg_dram + DRAM_ODTMAP);
+	}
+	for (i = 0; i < DRAM_DX_MAX; ++i) {
+		void __iomem *reg = priv->reg_dram + DRAM_DXnGCR0(i);
+
+		writel((readl(reg) & ~DRAM_DXnGCR0_DXODT) | dxodt, reg);
+	}
+
+	dev_dbg(priv->devfreq_dram->dev.parent,
+		"Setting DRAM to %u MHz, tREFI=%u, tRFC=%u, ODT=%s\n",
+		dram_freq_mhz, tREFI_32ck, tRFC_ck,
+		dxodt == DRAM_DXnGCR0_DXODT_DYNAMIC ? "dynamic" : "disabled");
+
+	/* Trigger hardware MDFS. */
+	writel(mdfscr | MBUS_MDFSCR_START, priv->reg_mbus + MBUS_MDFSCR);
+	ret = readl_poll_timeout_atomic(priv->reg_mbus + MBUS_MDFSCR, mdfscr,
+					!(mdfscr & MBUS_MDFSCR_START), 10, 1000);
+	if (ret)
+		return ret;
+
+	/* Disable double buffering. */
+	writel(0, priv->reg_mbus + MBUS_MDFSCR);
+
+	/* Restore VTF configuration. */
+	writel_relaxed(vtfcr, priv->reg_dram + DRAM_VTFCR);
+
+	/* Enable automatic self-refresh at the lowest frequency only. */
+	if (freq == priv->freq_table[0])
+		pwrctl |= DRAM_PWRCTL_SELFREF_EN;
+	writel_relaxed(pwrctl, priv->reg_dram + DRAM_PWRCTL);
+
+	sun8i_a33_mbus_restart_pmu_counters(priv);
+	sun8i_a33_mbus_update_nominal_bw(priv, ddr_freq_mhz);
+
+	return 0;
+}
+
+static int sun8i_a33_mbus_set_dram_target(struct device *dev,
+					  unsigned long *freq, u32 flags)
+{
+	struct sun8i_a33_mbus *priv = dev_get_drvdata(dev);
+	struct devfreq *devfreq = priv->devfreq_dram;
+	int ret;
+
+	devfreq_recommended_freq(devfreq, freq, flags);
+
+	if (*freq == devfreq->previous_freq)
+		return 0;
+
+	ret = sun8i_a33_mbus_set_dram_freq(priv, *freq);
+	if (ret) {
+		dev_warn(dev, "failed to set DRAM frequency: %d\n", ret);
+		*freq = devfreq->previous_freq;
+	}
+
+	return ret;
+}
+
+static int sun8i_a33_mbus_get_dram_status(struct device *dev,
+					  struct devfreq_dev_status *stat)
+{
+	struct sun8i_a33_mbus *priv = dev_get_drvdata(dev);
+
+	stat->busy_time		= sun8i_a33_mbus_get_peak_bw(priv);
+	stat->total_time	= priv->nominal_bw;
+	stat->current_frequency	= priv->devfreq_dram->previous_freq;
+
+	sun8i_a33_mbus_restart_pmu_counters(priv);
+
+	dev_dbg(dev, "Using %lu/%lu (%lu%%) at %lu MHz\n",
+		stat->busy_time, stat->total_time,
+		DIV_ROUND_CLOSEST(stat->busy_time * 100, stat->total_time),
+		stat->current_frequency / USEC_PER_SEC);
+
+	return 0;
+}
+
+static int sun8i_a33_mbus_hw_init(struct device *dev,
+				  struct sun8i_a33_mbus *priv,
+				  unsigned long ddr_freq)
+{
+	u32 i, mbus_cr, mbus_freq_mhz;
+
+	/* Choose tREFI and tRFC to match the configured DRAM type. */
+	mbus_cr = readl_relaxed(priv->reg_mbus + MBUS_CR);
+	switch (MBUS_CR_GET_DRAM_TYPE(mbus_cr)) {
+	case MBUS_CR_DRAM_TYPE_DDR2:
+	case MBUS_CR_DRAM_TYPE_DDR3:
+	case MBUS_CR_DRAM_TYPE_DDR4:
+		priv->tREFI_ns = 7800;
+		priv->tRFC_ns = 350;
+		break;
+	case MBUS_CR_DRAM_TYPE_LPDDR2:
+	case MBUS_CR_DRAM_TYPE_LPDDR3:
+		priv->tREFI_ns = 3900;
+		priv->tRFC_ns = 210;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Save ODTMAP so it can be restored when raising the frequency. */
+	priv->odtmap = readl_relaxed(priv->reg_dram + DRAM_ODTMAP);
+
+	/* Compute the DRAM data bus width by counting enabled DATx8 blocks. */
+	for (i = 0; i < DRAM_DX_MAX; ++i) {
+		void __iomem *reg = priv->reg_dram + DRAM_DXnGCR0(i);
+
+		if (!(readl_relaxed(reg) & DRAM_DXnGCR0_DXEN))
+			break;
+	}
+	priv->data_width = i;
+
+	dev_dbg(dev, "Detected %u-bit %sDDRx with%s ODT\n",
+		priv->data_width * 8,
+		MBUS_CR_GET_DRAM_TYPE(mbus_cr) > 4 ? "LP" : "",
+		priv->odtmap ? "" : "out");
+
+	/* Program MBUS_TMR such that the PMU period unit is microseconds. */
+	mbus_freq_mhz = clk_get_rate(priv->clk_mbus) / USEC_PER_SEC;
+	writel_relaxed(MBUS_TMR_PERIOD(mbus_freq_mhz),
+		       priv->reg_mbus + MBUS_TMR);
+
+	/* "Master Ready Mask Register" bits must be set or MDFS will block. */
+	writel_relaxed(0xffffffff, priv->reg_mbus + MBUS_MDFSMRMR);
+
+	sun8i_a33_mbus_restart_pmu_counters(priv);
+	sun8i_a33_mbus_update_nominal_bw(priv, ddr_freq / USEC_PER_SEC);
+
+	return 0;
+}
+
+static int __maybe_unused sun8i_a33_mbus_suspend(struct device *dev)
+{
+	struct sun8i_a33_mbus *priv = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(priv->clk_bus);
+
+	return 0;
+}
+
+static int __maybe_unused sun8i_a33_mbus_resume(struct device *dev)
+{
+	struct sun8i_a33_mbus *priv = dev_get_drvdata(dev);
+
+	return clk_prepare_enable(priv->clk_bus);
+}
+
+static int sun8i_a33_mbus_probe(struct platform_device *pdev)
+{
+	const struct sun8i_a33_mbus_variant *variant;
+	struct device *dev = &pdev->dev;
+	struct sun8i_a33_mbus *priv;
+	unsigned long base_freq;
+	unsigned int max_state;
+	unsigned int div;
+	const char *err;
+	int i, ret;
+
+	variant = device_get_match_data(dev);
+	if (!variant)
+		return -EINVAL;
+
+	max_state = variant->max_dram_divider - variant->min_dram_divider + 1;
+
+	priv = devm_kzalloc(dev, struct_size(priv, freq_table, max_state), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, priv);
+
+	priv->variant = variant;
+
+	priv->reg_dram = devm_platform_ioremap_resource_byname(pdev, "dram");
+	if (IS_ERR(priv->reg_dram))
+		return PTR_ERR(priv->reg_dram);
+
+	priv->reg_mbus = devm_platform_ioremap_resource_byname(pdev, "mbus");
+	if (IS_ERR(priv->reg_mbus))
+		return PTR_ERR(priv->reg_mbus);
+
+	priv->clk_bus = devm_clk_get(dev, "bus");
+	if (IS_ERR(priv->clk_bus))
+		return dev_err_probe(dev, PTR_ERR(priv->clk_bus),
+				     "failed to get bus clock\n");
+
+	priv->clk_dram = devm_clk_get(dev, "dram");
+	if (IS_ERR(priv->clk_dram))
+		return dev_err_probe(dev, PTR_ERR(priv->clk_dram),
+				     "failed to get dram clock\n");
+
+	priv->clk_mbus = devm_clk_get(dev, "mbus");
+	if (IS_ERR(priv->clk_mbus))
+		return dev_err_probe(dev, PTR_ERR(priv->clk_mbus),
+				     "failed to get mbus clock\n");
+
+	ret = clk_prepare_enable(priv->clk_bus);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to enable bus clock\n");
+
+	/* Lock the DRAM clock rate to keep priv->nominal_bw in sync. */
+	ret = clk_rate_exclusive_get(priv->clk_dram);
+	if (ret) {
+		err = "failed to lock dram clock rate\n";
+		goto err_disable_bus;
+	}
+
+	/* Lock the MBUS clock rate to keep MBUS_TMR_PERIOD in sync. */
+	ret = clk_rate_exclusive_get(priv->clk_mbus);
+	if (ret) {
+		err = "failed to lock mbus clock rate\n";
+		goto err_unlock_dram;
+	}
+
+	priv->gov_data.upthreshold	= 10;
+	priv->gov_data.downdifferential	=  5;
+
+	priv->profile.initial_freq	= clk_get_rate(priv->clk_dram);
+	priv->profile.polling_ms	= 1000;
+	priv->profile.target		= sun8i_a33_mbus_set_dram_target;
+	priv->profile.get_dev_status	= sun8i_a33_mbus_get_dram_status;
+	priv->profile.freq_table	= priv->freq_table;
+	priv->profile.max_state		= max_state;
+
+	base_freq = clk_get_rate(clk_get_parent(priv->clk_dram));
+	for (i = 0, div = variant->max_dram_divider; i < max_state; ++i, --div)
+		priv->freq_table[i] = base_freq / div;
+
+	ret = sun8i_a33_mbus_hw_init(dev, priv, priv->profile.initial_freq);
+	if (ret) {
+		err = "failed to init hardware\n";
+		goto err_unlock_mbus;
+	}
+
+	priv->devfreq_dram = devm_devfreq_add_device(dev, &priv->profile,
+						     DEVFREQ_GOV_SIMPLE_ONDEMAND,
+						     &priv->gov_data);
+	if (IS_ERR(priv->devfreq_dram)) {
+		ret = PTR_ERR(priv->devfreq_dram);
+		err = "failed to add devfreq device\n";
+		goto err_unlock_mbus;
+	}
+
+	priv->devfreq_dram->suspend_freq = priv->freq_table[0];
+
+	return 0;
+
+err_unlock_mbus:
+	clk_rate_exclusive_put(priv->clk_mbus);
+err_unlock_dram:
+	clk_rate_exclusive_put(priv->clk_dram);
+err_disable_bus:
+	clk_disable_unprepare(priv->clk_bus);
+
+	return dev_err_probe(dev, ret, err);
+}
+
+static int sun8i_a33_mbus_remove(struct platform_device *pdev)
+{
+	struct sun8i_a33_mbus *priv = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	ret = sun8i_a33_mbus_set_dram_freq(priv, priv->profile.initial_freq);
+	if (ret)
+		dev_warn(dev, "failed to restore DRAM frequency: %d\n", ret);
+
+	clk_rate_exclusive_put(priv->clk_mbus);
+	clk_rate_exclusive_put(priv->clk_dram);
+	clk_disable_unprepare(priv->clk_bus);
+
+	return 0;
+}
+
+static const struct sun8i_a33_mbus_variant sun50i_a64_mbus = {
+	.min_dram_divider	= 1,
+	.max_dram_divider	= 4,
+	.odt_freq_mhz		= 400,
+};
+
+static const struct of_device_id sun8i_a33_mbus_of_match[] = {
+	{ .compatible = "allwinner,sun50i-a64-mbus", .data = &sun50i_a64_mbus },
+	{ .compatible = "allwinner,sun50i-h5-mbus", .data = &sun50i_a64_mbus },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, sun8i_a33_mbus_of_match);
+
+static SIMPLE_DEV_PM_OPS(sun8i_a33_mbus_pm_ops,
+			 sun8i_a33_mbus_suspend, sun8i_a33_mbus_resume);
+
+static struct platform_driver sun8i_a33_mbus_driver = {
+	.probe	= sun8i_a33_mbus_probe,
+	.remove	= sun8i_a33_mbus_remove,
+	.driver	= {
+		.name		= "sun8i-mbus",
+		.of_match_table	= sun8i_a33_mbus_of_match,
+		.pm		= pm_ptr(&sun8i_a33_mbus_pm_ops),
+	},
+};
+module_platform_driver(sun8i_a33_mbus_driver);
+
+MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
+MODULE_DESCRIPTION("Allwinner sun8i/sun50i MBUS DEVFREQ Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 06/10] dt-bindings: arm: sunxi: Expand MBUS binding
  2021-09-29  4:42 ` [PATCH 06/10] dt-bindings: arm: sunxi: Expand MBUS binding Samuel Holland
@ 2021-09-29 13:46   ` Rob Herring
  0 siblings, 0 replies; 19+ messages in thread
From: Rob Herring @ 2021-09-29 13:46 UTC (permalink / raw)
  To: Samuel Holland
  Cc: linux-pm, linux-kernel, Kyungmin Park, Rob Herring,
	Michael Turquette, Chen-Yu Tsai, Chanwoo Choi, MyungJoo Ham,
	Maxime Ripard, Stephen Boyd, devicetree, linux-sunxi,
	linux-arm-kernel, Jernej Skrabec

On Tue, 28 Sep 2021 23:42:50 -0500, Samuel Holland wrote:
> The MBUS provides more than address translation and bandwidth control.
> It also provides a PMU to measure bandwidth usage by certain masters,
> and it provides notification via IRQ when they are active or idle.
> 
> The MBUS is also tightly integrated with the DRAM controller to provide
> a Memory Dynamic Frequency Scaling (MDFS) feature. In view of this, the
> MBUS binding needs to represent the hardware resources needed for MDFS,
> which include the clocks and MMIO range of the adjacent DRAM controller.
> 
> Add the additional resources for the H3 and A64 compatibles, and a new
> example showing how they are used.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  .../arm/sunxi/allwinner,sun4i-a10-mbus.yaml   | 75 ++++++++++++++++++-
>  1 file changed, 72 insertions(+), 3 deletions(-)
> 

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/1534210


dram-controller@1c62000: 'clock-names' is a required property
	arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino-emmc.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.0.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.1.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab-early-adopter.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-bananapi-m2-plus.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-bananapi-m2-plus-v1.2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-emlid-neutis-n5-devboard.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h3-cc.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h3-it.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h5-cc.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-r1s-h5.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-prime.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-bananapi-m2-zero.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-libretech-all-h3-cc.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-orangepi-r1.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dt.yaml
	arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus-v1.2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-beelink-x2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-emlid-neutis-n5h3-devboard.dt.yaml
	arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dt.yaml
	arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-m1.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-m1-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-neo-air.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-neo.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-r1.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-lite.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-one.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-pc.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-plus2e.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-zero-plus2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-rervision-dvk.dt.yaml
	arch/arm/boot/dts/sun8i-h3-zeropi.dt.yaml

dram-controller@1c62000: clocks: [[2, 112]] is too short
	arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino-emmc.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.0.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.1.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab-early-adopter.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dt.yaml

dram-controller@1c62000: clocks: [[3, 113]] is too short
	arch/arm64/boot/dts/allwinner/sun50i-h5-bananapi-m2-plus.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-bananapi-m2-plus-v1.2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-emlid-neutis-n5-devboard.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h3-cc.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h3-it.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h5-cc.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-r1s-h5.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-prime.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-bananapi-m2-zero.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-libretech-all-h3-cc.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-orangepi-r1.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dt.yaml
	arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus-v1.2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-beelink-x2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-emlid-neutis-n5h3-devboard.dt.yaml
	arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dt.yaml
	arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-m1.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-m1-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-neo-air.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-neo.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-r1.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-lite.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-one.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-pc.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-plus2e.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-zero-plus2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-rervision-dvk.dt.yaml
	arch/arm/boot/dts/sun8i-h3-zeropi.dt.yaml

dram-controller@1c62000: reg: [[29761536, 4096]] is too short
	arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino-emmc.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.0.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.1.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab-early-adopter.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-bananapi-m2-plus.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-bananapi-m2-plus-v1.2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-emlid-neutis-n5-devboard.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h3-cc.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h3-it.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h5-cc.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-r1s-h5.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-prime.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-bananapi-m2-zero.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-libretech-all-h3-cc.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-orangepi-r1.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dt.yaml
	arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus-v1.2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-beelink-x2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-emlid-neutis-n5h3-devboard.dt.yaml
	arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dt.yaml
	arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-m1.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-m1-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-neo-air.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-neo.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-r1.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-lite.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-one.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-pc.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-plus2e.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-zero-plus2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-rervision-dvk.dt.yaml
	arch/arm/boot/dts/sun8i-h3-zeropi.dt.yaml

dram-controller@1c62000: 'reg-names' is a required property
	arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-nanopi-a64.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino-emmc.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-orangepi-win.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.0.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.1.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab-early-adopter.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-bananapi-m2-plus.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-bananapi-m2-plus-v1.2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-emlid-neutis-n5-devboard.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h3-cc.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h3-it.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h5-cc.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-r1s-h5.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-prime.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dt.yaml
	arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-bananapi-m2-zero.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-libretech-all-h3-cc.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-orangepi-r1.dt.yaml
	arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dt.yaml
	arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus-v1.2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-beelink-x2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-emlid-neutis-n5h3-devboard.dt.yaml
	arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dt.yaml
	arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-m1.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-m1-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-neo-air.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-neo.dt.yaml
	arch/arm/boot/dts/sun8i-h3-nanopi-r1.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-lite.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-one.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-pc.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-plus2e.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-plus.dt.yaml
	arch/arm/boot/dts/sun8i-h3-orangepi-zero-plus2.dt.yaml
	arch/arm/boot/dts/sun8i-h3-rervision-dvk.dt.yaml
	arch/arm/boot/dts/sun8i-h3-zeropi.dt.yaml


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 01/10] PM / devfreq: strengthen check for freq_table
  2021-09-29  4:42 ` [PATCH 01/10] PM / devfreq: strengthen check for freq_table Samuel Holland
@ 2021-09-30  3:58   ` Chanwoo Choi
  0 siblings, 0 replies; 19+ messages in thread
From: Chanwoo Choi @ 2021-09-30  3:58 UTC (permalink / raw)
  To: Samuel Holland, MyungJoo Ham, Kyungmin Park, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel

On 9/29/21 1:42 PM, Samuel Holland wrote:
> Since commit ea572f816032 ("PM / devfreq: Change return type of
> devfreq_set_freq_table()"), all devfreq devices are expected to have a
> valid freq_table. The devfreq core unconditionally dereferences
> freq_table in the sysfs code and in get_freq_range().
> 
> Therefore, we need to ensure that freq_table is both non-null and
> non-empty (length is > 0). If either check fails, replace the table
> using set_freq_table() or return the error.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  drivers/devfreq/devfreq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 28f3e0ba6cdd..7a8b022ba456 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -827,7 +827,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  		goto err_dev;
>  	}
>  
> -	if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
> +	if (!devfreq->profile->max_state || !devfreq->profile->freq_table) {
>  		mutex_unlock(&devfreq->lock);
>  		err = set_freq_table(devfreq);
>  		if (err < 0)
> 


Applied it. Thanks.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 02/10] PM / devfreq: Do not require devices to have OPPs
  2021-09-29  4:42 ` [PATCH 02/10] PM / devfreq: Do not require devices to have OPPs Samuel Holland
@ 2021-09-30  4:19   ` Chanwoo Choi
  2021-09-30 11:37     ` Samuel Holland
  0 siblings, 1 reply; 19+ messages in thread
From: Chanwoo Choi @ 2021-09-30  4:19 UTC (permalink / raw)
  To: Samuel Holland, MyungJoo Ham, Kyungmin Park, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel

Hi Samuel,


On 9/29/21 1:42 PM, Samuel Holland wrote:
> Since commit ea572f816032 ("PM / devfreq: Change return type of
> devfreq_set_freq_table()"), all devfreq devices are required to have a
> valid freq_table. If freq_table is not provided by the driver, it will
> be filled in by set_freq_table() from the OPPs; if that fails,
> devfreq_add_device() will return an error.
> 
> However, since commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when
> adding the devfreq device"), devfreq devices are _also_ required to have
> an OPP table, even if they provide freq_table. devfreq_add_device()
> requires dev_pm_opp_find_freq_ceil() and dev_pm_opp_find_freq_floor() to
> return successfully, specifically to initialize scaling_min/max_freq.
> 
> Not all drivers need an OPP table. For example, a driver where all
> frequencies are determined dynamically could work by filling out only
> freq_table. But with the current code it must call dev_pm_opp_add() on
> every freq_table entry to probe successfully.

As you commented, if device has no opp table, it should call dev_pm_opp_add().
The devfreq have to use OPP for controlling the frequency/regulator.

Actually, I want that all devfreq driver uses the OPP as default way.
Are there any reason why don't use the OPP table?

> 
> The offending properties, scaling_min/max_freq, are only necessary if a
> device has OPPs; if no OPPs exist at all, OPPs cannot be dynamically
> enabled or disabled, so those values have no effect. Thus it is trivial
> to restore support for devices with freq_table only and not OPPs -- move
> those initializations behind the check for a valid OPP table.
> 
> Since get_freq_range() uses scaling_max_freq in a min() expression, it
> must be initialized to the maximum possible value. scaling_min_freq is
> initialized as well for consistency.
> 
> Fixes: ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when adding the devfreq device")
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  drivers/devfreq/devfreq.c | 36 ++++++++++++++++++++----------------
>  1 file changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 7a8b022ba456..426e31e6c448 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -835,24 +835,28 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  		mutex_lock(&devfreq->lock);
>  	}
>  
> -	devfreq->scaling_min_freq = find_available_min_freq(devfreq);
> -	if (!devfreq->scaling_min_freq) {
> -		mutex_unlock(&devfreq->lock);
> -		err = -EINVAL;
> -		goto err_dev;
> -	}
> -
> -	devfreq->scaling_max_freq = find_available_max_freq(devfreq);
> -	if (!devfreq->scaling_max_freq) {
> -		mutex_unlock(&devfreq->lock);
> -		err = -EINVAL;
> -		goto err_dev;
> -	}
> -
> -	devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
>  	devfreq->opp_table = dev_pm_opp_get_opp_table(dev);
> -	if (IS_ERR(devfreq->opp_table))
> +	if (IS_ERR(devfreq->opp_table)) {
>  		devfreq->opp_table = NULL;
> +		devfreq->scaling_min_freq = 0;
> +		devfreq->scaling_max_freq = ULONG_MAX;
> +	} else {
> +		devfreq->scaling_min_freq = find_available_min_freq(devfreq);
> +		if (!devfreq->scaling_min_freq) {
> +			mutex_unlock(&devfreq->lock);
> +			err = -EINVAL;
> +			goto err_dev;
> +		}
> +
> +		devfreq->scaling_max_freq = find_available_max_freq(devfreq);
> +		if (!devfreq->scaling_max_freq) {
> +			mutex_unlock(&devfreq->lock);
> +			err = -EINVAL;
> +			goto err_dev;
> +		}
> +
> +		devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
> +	}
>  
>  	atomic_set(&devfreq->suspend_count, 0);
>  
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 10/10] PM / devfreq: Add a driver for the sun8i/sun50i MBUS
  2021-09-29  4:42 ` [PATCH 10/10] PM / devfreq: Add a driver for the sun8i/sun50i MBUS Samuel Holland
@ 2021-09-30  4:35   ` Chanwoo Choi
  0 siblings, 0 replies; 19+ messages in thread
From: Chanwoo Choi @ 2021-09-30  4:35 UTC (permalink / raw)
  To: Samuel Holland, MyungJoo Ham, Kyungmin Park, Maxime Ripard,
	Chen-Yu Tsai, Jernej Skrabec, Rob Herring
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel

Hi Samuel,


On 9/29/21 1:42 PM, Samuel Holland wrote:
> This driver works by adjusting the divider on the DRAM controller's
> module clock. Thus there is no fixed set of OPPs, only "full speed" down
> to "quarter speed" (or whatever the maximum divider is on that variant).
> 
> It makes use of the MDFS hardware in the MBUS, in "DFS" mode, which
> takes care of updating registers during the critical section while DRAM
> is inaccessible.
> 
> This driver should support several sunxi SoCs, starting with the A33,
> which have a DesignWare DDR3 controller with merged PHY register space
> and the matching MBUS register layout (so not A63 or later). However,
> the driver has only been tested on the A64/H5, so those are the only
> compatibles enabled for now.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  drivers/devfreq/Kconfig          |   8 +
>  drivers/devfreq/Makefile         |   1 +
>  drivers/devfreq/sun8i-a33-mbus.c | 482 +++++++++++++++++++++++++++++++
>  3 files changed, 491 insertions(+)
>  create mode 100644 drivers/devfreq/sun8i-a33-mbus.c
> 
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index e87d01c0b76a..b94eb04761f6 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -132,6 +132,14 @@ config ARM_RK3399_DMC_DEVFREQ
>  	  It sets the frequency for the memory controller and reads the usage counts
>  	  from hardware.
>  
> +config ARM_SUN8I_A33_MBUS_DEVFREQ
> +	tristate "sun8i/sun50i MBUS DEVFREQ Driver"
> +	depends on ARCH_SUNXI || COMPILE_TEST
> +	select DEVFREQ_GOV_SIMPLE_ONDEMAND
> +	help
> +	  This adds the DEVFREQ driver for the MBUS controller in some
> +	  Allwinner sun8i (A33 through H3) and sun50i (A64 and H5) SoCs.
> +
>  source "drivers/devfreq/event/Kconfig"
>  
>  endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index a16333ea7034..0b6be92a25d9 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ)	+= exynos-bus.o
>  obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ)	+= imx-bus.o
>  obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ)	+= imx8m-ddrc.o
>  obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ)	+= rk3399_dmc.o
> +obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ)	+= sun8i-a33-mbus.o
>  obj-$(CONFIG_ARM_TEGRA_DEVFREQ)		+= tegra30-devfreq.o
>  
>  # DEVFREQ Event Drivers
> diff --git a/drivers/devfreq/sun8i-a33-mbus.c b/drivers/devfreq/sun8i-a33-mbus.c
> new file mode 100644
> index 000000000000..00f24850cf8d
> --- /dev/null
> +++ b/drivers/devfreq/sun8i-a33-mbus.c
> @@ -0,0 +1,482 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (C) 2020-2021 Samuel Holland <samuel@sholland.org>
> +//
> +
> +#include <linux/clk.h>
> +#include <linux/devfreq.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +
> +#define MBUS_CR				0x0000
> +#define MBUS_CR_GET_DRAM_TYPE(x)	(((x) >> 16) & 0x7)
> +#define MBUS_CR_DRAM_TYPE_DDR2		2
> +#define MBUS_CR_DRAM_TYPE_DDR3		3
> +#define MBUS_CR_DRAM_TYPE_DDR4		4
> +#define MBUS_CR_DRAM_TYPE_LPDDR2	6
> +#define MBUS_CR_DRAM_TYPE_LPDDR3	7
> +
> +#define MBUS_TMR			0x000c
> +#define MBUS_TMR_PERIOD(x)		((x) - 1)
> +
> +#define MBUS_PMU_CFG			0x009c
> +#define MBUS_PMU_CFG_PERIOD(x)		(((x) - 1) << 16)
> +#define MBUS_PMU_CFG_UNIT		(0x3 << 1)
> +#define MBUS_PMU_CFG_UNIT_B		(0x0 << 1)
> +#define MBUS_PMU_CFG_UNIT_KB		(0x1 << 1)
> +#define MBUS_PMU_CFG_UNIT_MB		(0x2 << 1)
> +#define MBUS_PMU_CFG_ENABLE		(0x1 << 0)
> +
> +#define MBUS_PMU_BWCR(n)		(0x00a0 + (0x04 * (n)))
> +
> +#define MBUS_TOTAL_BWCR			MBUS_PMU_BWCR(5)
> +#define MBUS_TOTAL_BWCR_H616		MBUS_PMU_BWCR(13)
> +
> +#define MBUS_MDFSCR			0x0100
> +#define MBUS_MDFSCR_BUFFER_TIMING	(0x1 << 15)
> +#define MBUS_MDFSCR_PAD_HOLD		(0x1 << 13)
> +#define MBUS_MDFSCR_BYPASS		(0x1 << 4)
> +#define MBUS_MDFSCR_MODE		(0x1 << 1)
> +#define MBUS_MDFSCR_MODE_DFS		(0x0 << 1)
> +#define MBUS_MDFSCR_MODE_CFS		(0x1 << 1)
> +#define MBUS_MDFSCR_START		(0x1 << 0)
> +
> +#define MBUS_MDFSMRMR			0x0108
> +
> +#define DRAM_PWRCTL			0x0004
> +#define DRAM_PWRCTL_SELFREF_EN		(0x1 << 0)
> +
> +#define DRAM_RFSHTMG			0x0090
> +#define DRAM_RFSHTMG_TREFI(x)		((x) << 16)
> +#define DRAM_RFSHTMG_TRFC(x)		((x) << 0)
> +
> +#define DRAM_VTFCR			0x00b8
> +#define DRAM_VTFCR_VTF_ENABLE		(0x3 << 8)
> +
> +#define DRAM_ODTMAP			0x0120
> +
> +#define DRAM_DX_MAX			4
> +
> +#define DRAM_DXnGCR0(n)			(0x0344 + 0x80 * (n))
> +#define DRAM_DXnGCR0_DXODT		(0x3 << 4)
> +#define DRAM_DXnGCR0_DXODT_DYNAMIC	(0x0 << 4)
> +#define DRAM_DXnGCR0_DXODT_ENABLED	(0x1 << 4)
> +#define DRAM_DXnGCR0_DXODT_DISABLED	(0x2 << 4)
> +#define DRAM_DXnGCR0_DXEN		(0x1 << 0)
> +
> +struct sun8i_a33_mbus_variant {
> +	u32					min_dram_divider;
> +	u32					max_dram_divider;
> +	u32					odt_freq_mhz;
> +};
> +
> +struct sun8i_a33_mbus {
> +	const struct sun8i_a33_mbus_variant	*variant;
> +	void __iomem				*reg_dram;
> +	void __iomem				*reg_mbus;
> +	struct clk				*clk_bus;
> +	struct clk				*clk_dram;
> +	struct clk				*clk_mbus;
> +	struct devfreq				*devfreq_dram;
> +	struct devfreq_simple_ondemand_data	gov_data;
> +	struct devfreq_dev_profile		profile;
> +	u32					data_width;
> +	u32					nominal_bw;
> +	u32					odtmap;
> +	u32					tREFI_ns;
> +	u32					tRFC_ns;
> +	unsigned long				freq_table[];
> +};
> +
> +/*
> + * The unit for this value is (MBUS clock cycles / MBUS_TMR_PERIOD). When
> + * MBUS_TMR_PERIOD is programmed to match the MBUS clock frequency in MHz, as
> + * it is during DRAM init and during probe, the resulting unit is microseconds.
> + */
> +static int pmu_period = 50000;
> +module_param(pmu_period, int, 0644);
> +MODULE_PARM_DESC(pmu_period, "Bandwidth measurement period (microseconds)");
> +
> +static u32 sun8i_a33_mbus_get_peak_bw(struct sun8i_a33_mbus *priv)
> +{
> +	/* Returns the peak transfer (in KiB) during any single PMU period. */
> +	return readl_relaxed(priv->reg_mbus + MBUS_TOTAL_BWCR);
> +}
> +
> +static void sun8i_a33_mbus_restart_pmu_counters(struct sun8i_a33_mbus *priv)
> +{
> +	u32 pmu_cfg = MBUS_PMU_CFG_PERIOD(pmu_period) | MBUS_PMU_CFG_UNIT_KB;
> +
> +	/* All PMU counters are cleared on a disable->enable transition. */
> +	writel_relaxed(pmu_cfg,
> +		       priv->reg_mbus + MBUS_PMU_CFG);
> +	writel_relaxed(pmu_cfg | MBUS_PMU_CFG_ENABLE,
> +		       priv->reg_mbus + MBUS_PMU_CFG);
> +
> +}
> +
> +static void sun8i_a33_mbus_update_nominal_bw(struct sun8i_a33_mbus *priv,
> +					     u32 ddr_freq_mhz)
> +{
> +	/*
> +	 * Nominal bandwidth (KiB per PMU period):
> +	 *
> +	 *   DDR transfers   microseconds     KiB
> +	 *   ------------- * ------------ * --------
> +	 *    microsecond     PMU period    transfer
> +	 */
> +	priv->nominal_bw = ddr_freq_mhz * pmu_period * priv->data_width / 1024;
> +}
> +
> +static int sun8i_a33_mbus_set_dram_freq(struct sun8i_a33_mbus *priv,
> +					unsigned long freq)
> +{
> +	u32  ddr_freq_mhz = freq / USEC_PER_SEC; /* DDR */
> +	u32 dram_freq_mhz =    ddr_freq_mhz / 2; /* SDR */
> +	u32 mctl_freq_mhz =   dram_freq_mhz / 2; /* HDR */
> +	u32 dxodt, mdfscr, pwrctl, vtfcr;
> +	u32 i, tREFI_32ck, tRFC_ck;
> +	int ret;
> +
> +	/* The rate change is not effective until the MDFS process runs. */
> +	ret = clk_set_rate(priv->clk_dram, freq);
> +	if (ret)
> +		return ret;
> +
> +	/* Disable automatic self-refesh and VTF before starting MDFS. */
> +	pwrctl = readl_relaxed(priv->reg_dram + DRAM_PWRCTL) &
> +		 ~DRAM_PWRCTL_SELFREF_EN;
> +	writel_relaxed(pwrctl, priv->reg_dram + DRAM_PWRCTL);
> +	vtfcr = readl_relaxed(priv->reg_dram + DRAM_VTFCR);
> +	writel_relaxed(vtfcr & ~DRAM_VTFCR_VTF_ENABLE,
> +		       priv->reg_dram + DRAM_VTFCR);
> +
> +	/* Set up MDFS and enable double buffering for timing registers. */
> +	mdfscr = MBUS_MDFSCR_MODE_DFS |
> +		 MBUS_MDFSCR_BYPASS |
> +		 MBUS_MDFSCR_PAD_HOLD |
> +		 MBUS_MDFSCR_BUFFER_TIMING;
> +	writel(mdfscr, priv->reg_mbus + MBUS_MDFSCR);
> +
> +	/* Update the buffered copy of RFSHTMG. */
> +	tREFI_32ck = priv->tREFI_ns * mctl_freq_mhz / 1000 / 32;
> +	tRFC_ck = DIV_ROUND_UP(priv->tRFC_ns * mctl_freq_mhz, 1000);
> +	writel(DRAM_RFSHTMG_TREFI(tREFI_32ck) | DRAM_RFSHTMG_TRFC(tRFC_ck),
> +	       priv->reg_dram + DRAM_RFSHTMG);
> +
> +	/* Enable ODT if needed, or disable it to save power. */
> +	if (priv->odtmap && dram_freq_mhz > priv->variant->odt_freq_mhz) {
> +		dxodt = DRAM_DXnGCR0_DXODT_DYNAMIC;
> +		writel(priv->odtmap, priv->reg_dram + DRAM_ODTMAP);
> +	} else {
> +		dxodt = DRAM_DXnGCR0_DXODT_DISABLED;
> +		writel(0, priv->reg_dram + DRAM_ODTMAP);
> +	}
> +	for (i = 0; i < DRAM_DX_MAX; ++i) {
> +		void __iomem *reg = priv->reg_dram + DRAM_DXnGCR0(i);
> +
> +		writel((readl(reg) & ~DRAM_DXnGCR0_DXODT) | dxodt, reg);
> +	}
> +
> +	dev_dbg(priv->devfreq_dram->dev.parent,
> +		"Setting DRAM to %u MHz, tREFI=%u, tRFC=%u, ODT=%s\n",
> +		dram_freq_mhz, tREFI_32ck, tRFC_ck,
> +		dxodt == DRAM_DXnGCR0_DXODT_DYNAMIC ? "dynamic" : "disabled");
> +
> +	/* Trigger hardware MDFS. */
> +	writel(mdfscr | MBUS_MDFSCR_START, priv->reg_mbus + MBUS_MDFSCR);
> +	ret = readl_poll_timeout_atomic(priv->reg_mbus + MBUS_MDFSCR, mdfscr,
> +					!(mdfscr & MBUS_MDFSCR_START), 10, 1000);
> +	if (ret)
> +		return ret;
> +
> +	/* Disable double buffering. */
> +	writel(0, priv->reg_mbus + MBUS_MDFSCR);
> +
> +	/* Restore VTF configuration. */
> +	writel_relaxed(vtfcr, priv->reg_dram + DRAM_VTFCR);
> +
> +	/* Enable automatic self-refresh at the lowest frequency only. */
> +	if (freq == priv->freq_table[0])
> +		pwrctl |= DRAM_PWRCTL_SELFREF_EN;
> +	writel_relaxed(pwrctl, priv->reg_dram + DRAM_PWRCTL);
> +
> +	sun8i_a33_mbus_restart_pmu_counters(priv);
> +	sun8i_a33_mbus_update_nominal_bw(priv, ddr_freq_mhz);
> +
> +	return 0;
> +}
> +
> +static int sun8i_a33_mbus_set_dram_target(struct device *dev,
> +					  unsigned long *freq, u32 flags)
> +{
> +	struct sun8i_a33_mbus *priv = dev_get_drvdata(dev);
> +	struct devfreq *devfreq = priv->devfreq_dram;
> +	int ret;
> +
> +	devfreq_recommended_freq(devfreq, freq, flags);
> +
> +	if (*freq == devfreq->previous_freq)
> +		return 0;
> +
> +	ret = sun8i_a33_mbus_set_dram_freq(priv, *freq);
> +	if (ret) {
> +		dev_warn(dev, "failed to set DRAM frequency: %d\n", ret);
> +		*freq = devfreq->previous_freq;
> +	}
> +
> +	return ret;
> +}
> +
> +static int sun8i_a33_mbus_get_dram_status(struct device *dev,
> +					  struct devfreq_dev_status *stat)
> +{
> +	struct sun8i_a33_mbus *priv = dev_get_drvdata(dev);
> +
> +	stat->busy_time		= sun8i_a33_mbus_get_peak_bw(priv);
> +	stat->total_time	= priv->nominal_bw;
> +	stat->current_frequency	= priv->devfreq_dram->previous_freq;
> +
> +	sun8i_a33_mbus_restart_pmu_counters(priv);
> +
> +	dev_dbg(dev, "Using %lu/%lu (%lu%%) at %lu MHz\n",
> +		stat->busy_time, stat->total_time,
> +		DIV_ROUND_CLOSEST(stat->busy_time * 100, stat->total_time),
> +		stat->current_frequency / USEC_PER_SEC);
> +
> +	return 0;
> +}
> +
> +static int sun8i_a33_mbus_hw_init(struct device *dev,
> +				  struct sun8i_a33_mbus *priv,
> +				  unsigned long ddr_freq)
> +{
> +	u32 i, mbus_cr, mbus_freq_mhz;
> +
> +	/* Choose tREFI and tRFC to match the configured DRAM type. */
> +	mbus_cr = readl_relaxed(priv->reg_mbus + MBUS_CR);
> +	switch (MBUS_CR_GET_DRAM_TYPE(mbus_cr)) {
> +	case MBUS_CR_DRAM_TYPE_DDR2:
> +	case MBUS_CR_DRAM_TYPE_DDR3:
> +	case MBUS_CR_DRAM_TYPE_DDR4:
> +		priv->tREFI_ns = 7800;
> +		priv->tRFC_ns = 350;
> +		break;
> +	case MBUS_CR_DRAM_TYPE_LPDDR2:
> +	case MBUS_CR_DRAM_TYPE_LPDDR3:
> +		priv->tREFI_ns = 3900;
> +		priv->tRFC_ns = 210;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	/* Save ODTMAP so it can be restored when raising the frequency. */
> +	priv->odtmap = readl_relaxed(priv->reg_dram + DRAM_ODTMAP);
> +
> +	/* Compute the DRAM data bus width by counting enabled DATx8 blocks. */
> +	for (i = 0; i < DRAM_DX_MAX; ++i) {
> +		void __iomem *reg = priv->reg_dram + DRAM_DXnGCR0(i);
> +
> +		if (!(readl_relaxed(reg) & DRAM_DXnGCR0_DXEN))
> +			break;
> +	}
> +	priv->data_width = i;
> +
> +	dev_dbg(dev, "Detected %u-bit %sDDRx with%s ODT\n",
> +		priv->data_width * 8,
> +		MBUS_CR_GET_DRAM_TYPE(mbus_cr) > 4 ? "LP" : "",
> +		priv->odtmap ? "" : "out");
> +
> +	/* Program MBUS_TMR such that the PMU period unit is microseconds. */
> +	mbus_freq_mhz = clk_get_rate(priv->clk_mbus) / USEC_PER_SEC;
> +	writel_relaxed(MBUS_TMR_PERIOD(mbus_freq_mhz),
> +		       priv->reg_mbus + MBUS_TMR);
> +
> +	/* "Master Ready Mask Register" bits must be set or MDFS will block. */
> +	writel_relaxed(0xffffffff, priv->reg_mbus + MBUS_MDFSMRMR);
> +
> +	sun8i_a33_mbus_restart_pmu_counters(priv);
> +	sun8i_a33_mbus_update_nominal_bw(priv, ddr_freq / USEC_PER_SEC);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused sun8i_a33_mbus_suspend(struct device *dev)
> +{
> +	struct sun8i_a33_mbus *priv = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(priv->clk_bus);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused sun8i_a33_mbus_resume(struct device *dev)
> +{
> +	struct sun8i_a33_mbus *priv = dev_get_drvdata(dev);
> +
> +	return clk_prepare_enable(priv->clk_bus);
> +}
> +
> +static int sun8i_a33_mbus_probe(struct platform_device *pdev)
> +{
> +	const struct sun8i_a33_mbus_variant *variant;
> +	struct device *dev = &pdev->dev;
> +	struct sun8i_a33_mbus *priv;
> +	unsigned long base_freq;
> +	unsigned int max_state;
> +	unsigned int div;
> +	const char *err;
> +	int i, ret;
> +
> +	variant = device_get_match_data(dev);
> +	if (!variant)
> +		return -EINVAL;
> +
> +	max_state = variant->max_dram_divider - variant->min_dram_divider + 1;
> +
> +	priv = devm_kzalloc(dev, struct_size(priv, freq_table, max_state), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	priv->variant = variant;
> +
> +	priv->reg_dram = devm_platform_ioremap_resource_byname(pdev, "dram");
> +	if (IS_ERR(priv->reg_dram))
> +		return PTR_ERR(priv->reg_dram);
> +
> +	priv->reg_mbus = devm_platform_ioremap_resource_byname(pdev, "mbus");
> +	if (IS_ERR(priv->reg_mbus))
> +		return PTR_ERR(priv->reg_mbus);
> +
> +	priv->clk_bus = devm_clk_get(dev, "bus");
> +	if (IS_ERR(priv->clk_bus))
> +		return dev_err_probe(dev, PTR_ERR(priv->clk_bus),
> +				     "failed to get bus clock\n");
> +
> +	priv->clk_dram = devm_clk_get(dev, "dram");
> +	if (IS_ERR(priv->clk_dram))
> +		return dev_err_probe(dev, PTR_ERR(priv->clk_dram),
> +				     "failed to get dram clock\n");
> +
> +	priv->clk_mbus = devm_clk_get(dev, "mbus");
> +	if (IS_ERR(priv->clk_mbus))
> +		return dev_err_probe(dev, PTR_ERR(priv->clk_mbus),
> +				     "failed to get mbus clock\n");
> +
> +	ret = clk_prepare_enable(priv->clk_bus);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to enable bus clock\n");
> +
> +	/* Lock the DRAM clock rate to keep priv->nominal_bw in sync. */
> +	ret = clk_rate_exclusive_get(priv->clk_dram);
> +	if (ret) {
> +		err = "failed to lock dram clock rate\n";
> +		goto err_disable_bus;
> +	}
> +
> +	/* Lock the MBUS clock rate to keep MBUS_TMR_PERIOD in sync. */
> +	ret = clk_rate_exclusive_get(priv->clk_mbus);
> +	if (ret) {
> +		err = "failed to lock mbus clock rate\n";
> +		goto err_unlock_dram;
> +	}
> +
> +	priv->gov_data.upthreshold	= 10;
> +	priv->gov_data.downdifferential	=  5;
> +
> +	priv->profile.initial_freq	= clk_get_rate(priv->clk_dram);
> +	priv->profile.polling_ms	= 1000;
> +	priv->profile.target		= sun8i_a33_mbus_set_dram_target;
> +	priv->profile.get_dev_status	= sun8i_a33_mbus_get_dram_status;
> +	priv->profile.freq_table	= priv->freq_table;
> +	priv->profile.max_state		= max_state;
> +
> +	base_freq = clk_get_rate(clk_get_parent(priv->clk_dram));
> +	for (i = 0, div = variant->max_dram_divider; i < max_state; ++i, --div)
> +		priv->freq_table[i] = base_freq / div;

This patchset supports the case without OPP.
But, if you use dev_pm_opp_add() function at this point,
you can use the existing devfreq core without any modification for no OPP case.

Why don't you use dev_pm_opp_add at this point?

(snip)

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 02/10] PM / devfreq: Do not require devices to have OPPs
  2021-09-30  4:19   ` Chanwoo Choi
@ 2021-09-30 11:37     ` Samuel Holland
  2021-10-01  1:59       ` Chanwoo Choi
  0 siblings, 1 reply; 19+ messages in thread
From: Samuel Holland @ 2021-09-30 11:37 UTC (permalink / raw)
  To: Chanwoo Choi, MyungJoo Ham, Kyungmin Park
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Rob Herring

On 9/29/21 11:19 PM, Chanwoo Choi wrote:
> Hi Samuel,
> 
> 
> On 9/29/21 1:42 PM, Samuel Holland wrote:
>> Since commit ea572f816032 ("PM / devfreq: Change return type of
>> devfreq_set_freq_table()"), all devfreq devices are required to have a
>> valid freq_table. If freq_table is not provided by the driver, it will
>> be filled in by set_freq_table() from the OPPs; if that fails,
>> devfreq_add_device() will return an error.
>>
>> However, since commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when
>> adding the devfreq device"), devfreq devices are _also_ required to have
>> an OPP table, even if they provide freq_table. devfreq_add_device()
>> requires dev_pm_opp_find_freq_ceil() and dev_pm_opp_find_freq_floor() to
>> return successfully, specifically to initialize scaling_min/max_freq.
>>
>> Not all drivers need an OPP table. For example, a driver where all
>> frequencies are determined dynamically could work by filling out only
>> freq_table. But with the current code it must call dev_pm_opp_add() on
>> every freq_table entry to probe successfully.
> 
> As you commented, if device has no opp table, it should call dev_pm_opp_add().
> The devfreq have to use OPP for controlling the frequency/regulator.
> 
> Actually, I want that all devfreq driver uses the OPP as default way.

The current code/documentation implies that an OPP table is intended to
be optional. For example:

 * struct devfreq - Device devfreq structure
...
 * @opp_table:  Reference to OPP table of dev.parent, if one exists.

So this should be updated if an OPP table is no longer optional.

> Are there any reason why don't use the OPP table?

dev_pm_opp_add() takes a voltage, and assumes the existence of some
voltage regulator, but there is none involved here. The only way to have
an OPP table without regulators is to use a static table in the
devicetree. But that also doesn't make much sense, because the OPPs
aren't actually customizable; they are integer dividers from a fixed
base clock. And adding a fixed OPP table to each board would be a lot of
work to replace a trivial loop in the driver. So it seems to be the
wrong abstraction.

Using an OPP table adds extra complexity (memory allocations, error
cases), just to duplicate the list of frequencies that already has to
exist in freq_table. And the driver works fine without any of that.

Regards,
Samuel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 02/10] PM / devfreq: Do not require devices to have OPPs
  2021-10-01  1:59       ` Chanwoo Choi
@ 2021-10-01  1:45         ` Samuel Holland
  2021-10-01  2:14           ` Chanwoo Choi
  0 siblings, 1 reply; 19+ messages in thread
From: Samuel Holland @ 2021-10-01  1:45 UTC (permalink / raw)
  To: Chanwoo Choi, MyungJoo Ham, Kyungmin Park
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Rob Herring

On 9/30/21 8:59 PM, Chanwoo Choi wrote:
> On 9/30/21 8:37 PM, Samuel Holland wrote:
>> On 9/29/21 11:19 PM, Chanwoo Choi wrote:
>>> Hi Samuel,
>>>
>>>
>>> On 9/29/21 1:42 PM, Samuel Holland wrote:
>>>> Since commit ea572f816032 ("PM / devfreq: Change return type of
>>>> devfreq_set_freq_table()"), all devfreq devices are required to have a
>>>> valid freq_table. If freq_table is not provided by the driver, it will
>>>> be filled in by set_freq_table() from the OPPs; if that fails,
>>>> devfreq_add_device() will return an error.
>>>>
>>>> However, since commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when
>>>> adding the devfreq device"), devfreq devices are _also_ required to have
>>>> an OPP table, even if they provide freq_table. devfreq_add_device()
>>>> requires dev_pm_opp_find_freq_ceil() and dev_pm_opp_find_freq_floor() to
>>>> return successfully, specifically to initialize scaling_min/max_freq.
>>>>
>>>> Not all drivers need an OPP table. For example, a driver where all
>>>> frequencies are determined dynamically could work by filling out only
>>>> freq_table. But with the current code it must call dev_pm_opp_add() on
>>>> every freq_table entry to probe successfully.
>>>
>>> As you commented, if device has no opp table, it should call dev_pm_opp_add().
>>> The devfreq have to use OPP for controlling the frequency/regulator.
>>>
>>> Actually, I want that all devfreq driver uses the OPP as default way.
>>
>> The current code/documentation implies that an OPP table is intended to
>> be optional. For example:
>>
>>  * struct devfreq - Device devfreq structure
>> ...
>>  * @opp_table:  Reference to OPP table of dev.parent, if one exists.
>>
>> So this should be updated if an OPP table is no longer optional.
> 
> Right. Need to update it.
> 
>>
>>> Are there any reason why don't use the OPP table?
>>
>> dev_pm_opp_add() takes a voltage, and assumes the existence of some
>> voltage regulator, but there is none involved here. The only way to have
>> an OPP table without regulators is to use a static table in the
>> devicetree. But that also doesn't make much sense, because the OPPs
>> aren't actually customizable; they are integer dividers from a fixed
>> base clock.
> 
> You can use OPP for only clock control without regulator. OPP already
> provides them. OPP already provides the helpful function which
> implement the functions to handle the clock/regulator or power doamin.
> It is useful framework to control clock/regulator. 
> 
> If the standard framework in Linux kernel, it is best to use
> this framework in order to remove the duplicate codes on multiple
> device drivers. It is one of advantage of Linux kernel. 
> 
> Also, if OPP doesn't support the some requirement of you,
> you can contribute and update the OPP.
> 
>  And adding a fixed OPP table to each board would be a lot of
>> work to replace a trivial loop in the driver. So it seems to be the
>> wrong abstraction.
> 
> I don't understand. As I commented for patch 10, you can add
> the OPP entry of the clock without the fixed OPP table in devicetree.
> 
>>
>> Using an OPP table adds extra complexity (memory allocations, error
>> cases), just to duplicate the list of frequencies that already has to
>> exist in freq_table. And the driver works fine without any of that.
> 
> 'freq_table' of devfreq was developed before of adding OPP interface to Linux kernel as I knew. Actually, I prefer to use the OPP interface
> instead of initializing the freq_table directly by device driver.
> I just keep the 'freq_table' for preventing the build/working issue
> for older device driver. I think OPP is enough to control frequency/voltage
> and it provides the various helper funcitons for user of OPP.

Thanks for the explanation. I will convert the driver to use
dev_pm_opp_add(), and I will drop patches 2 and 4. I think patch 3 is
still worth considering.

Regards,
Samuel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 02/10] PM / devfreq: Do not require devices to have OPPs
  2021-09-30 11:37     ` Samuel Holland
@ 2021-10-01  1:59       ` Chanwoo Choi
  2021-10-01  1:45         ` Samuel Holland
  0 siblings, 1 reply; 19+ messages in thread
From: Chanwoo Choi @ 2021-10-01  1:59 UTC (permalink / raw)
  To: Samuel Holland, MyungJoo Ham, Kyungmin Park
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Rob Herring

On 9/30/21 8:37 PM, Samuel Holland wrote:
> On 9/29/21 11:19 PM, Chanwoo Choi wrote:
>> Hi Samuel,
>>
>>
>> On 9/29/21 1:42 PM, Samuel Holland wrote:
>>> Since commit ea572f816032 ("PM / devfreq: Change return type of
>>> devfreq_set_freq_table()"), all devfreq devices are required to have a
>>> valid freq_table. If freq_table is not provided by the driver, it will
>>> be filled in by set_freq_table() from the OPPs; if that fails,
>>> devfreq_add_device() will return an error.
>>>
>>> However, since commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when
>>> adding the devfreq device"), devfreq devices are _also_ required to have
>>> an OPP table, even if they provide freq_table. devfreq_add_device()
>>> requires dev_pm_opp_find_freq_ceil() and dev_pm_opp_find_freq_floor() to
>>> return successfully, specifically to initialize scaling_min/max_freq.
>>>
>>> Not all drivers need an OPP table. For example, a driver where all
>>> frequencies are determined dynamically could work by filling out only
>>> freq_table. But with the current code it must call dev_pm_opp_add() on
>>> every freq_table entry to probe successfully.
>>
>> As you commented, if device has no opp table, it should call dev_pm_opp_add().
>> The devfreq have to use OPP for controlling the frequency/regulator.
>>
>> Actually, I want that all devfreq driver uses the OPP as default way.
> 
> The current code/documentation implies that an OPP table is intended to
> be optional. For example:
> 
>  * struct devfreq - Device devfreq structure
> ...
>  * @opp_table:  Reference to OPP table of dev.parent, if one exists.
> 
> So this should be updated if an OPP table is no longer optional.

Right. Need to update it.

> 
>> Are there any reason why don't use the OPP table?
> 
> dev_pm_opp_add() takes a voltage, and assumes the existence of some
> voltage regulator, but there is none involved here. The only way to have
> an OPP table without regulators is to use a static table in the
> devicetree. But that also doesn't make much sense, because the OPPs
> aren't actually customizable; they are integer dividers from a fixed
> base clock.

You can use OPP for only clock control without regulator. OPP already
provides them. OPP already provides the helpful function which
implement the functions to handle the clock/regulator or power doamin.
It is useful framework to control clock/regulator. 

If the standard framework in Linux kernel, it is best to use
this framework in order to remove the duplicate codes on multiple
device drivers. It is one of advantage of Linux kernel. 

Also, if OPP doesn't support the some requirement of you,
you can contribute and update the OPP.

 And adding a fixed OPP table to each board would be a lot of
> work to replace a trivial loop in the driver. So it seems to be the
> wrong abstraction.

I don't understand. As I commented for patch 10, you can add
the OPP entry of the clock without the fixed OPP table in devicetree.

> 
> Using an OPP table adds extra complexity (memory allocations, error
> cases), just to duplicate the list of frequencies that already has to
> exist in freq_table. And the driver works fine without any of that.

'freq_table' of devfreq was developed before of adding OPP interface to Linux kernel as I knew. Actually, I prefer to use the OPP interface
instead of initializing the freq_table directly by device driver.
I just keep the 'freq_table' for preventing the build/working issue
for older device driver. I think OPP is enough to control frequency/voltage
and it provides the various helper funcitons for user of OPP.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 02/10] PM / devfreq: Do not require devices to have OPPs
  2021-10-01  1:45         ` Samuel Holland
@ 2021-10-01  2:14           ` Chanwoo Choi
  0 siblings, 0 replies; 19+ messages in thread
From: Chanwoo Choi @ 2021-10-01  2:14 UTC (permalink / raw)
  To: Samuel Holland, MyungJoo Ham, Kyungmin Park
  Cc: Michael Turquette, Stephen Boyd, devicetree, linux-arm-kernel,
	linux-pm, linux-sunxi, linux-kernel, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Rob Herring

On 10/1/21 10:45 AM, Samuel Holland wrote:
> On 9/30/21 8:59 PM, Chanwoo Choi wrote:
>> On 9/30/21 8:37 PM, Samuel Holland wrote:
>>> On 9/29/21 11:19 PM, Chanwoo Choi wrote:
>>>> Hi Samuel,
>>>>
>>>>
>>>> On 9/29/21 1:42 PM, Samuel Holland wrote:
>>>>> Since commit ea572f816032 ("PM / devfreq: Change return type of
>>>>> devfreq_set_freq_table()"), all devfreq devices are required to have a
>>>>> valid freq_table. If freq_table is not provided by the driver, it will
>>>>> be filled in by set_freq_table() from the OPPs; if that fails,
>>>>> devfreq_add_device() will return an error.
>>>>>
>>>>> However, since commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when
>>>>> adding the devfreq device"), devfreq devices are _also_ required to have
>>>>> an OPP table, even if they provide freq_table. devfreq_add_device()
>>>>> requires dev_pm_opp_find_freq_ceil() and dev_pm_opp_find_freq_floor() to
>>>>> return successfully, specifically to initialize scaling_min/max_freq.
>>>>>
>>>>> Not all drivers need an OPP table. For example, a driver where all
>>>>> frequencies are determined dynamically could work by filling out only
>>>>> freq_table. But with the current code it must call dev_pm_opp_add() on
>>>>> every freq_table entry to probe successfully.
>>>>
>>>> As you commented, if device has no opp table, it should call dev_pm_opp_add().
>>>> The devfreq have to use OPP for controlling the frequency/regulator.
>>>>
>>>> Actually, I want that all devfreq driver uses the OPP as default way.
>>>
>>> The current code/documentation implies that an OPP table is intended to
>>> be optional. For example:
>>>
>>>  * struct devfreq - Device devfreq structure
>>> ...
>>>  * @opp_table:  Reference to OPP table of dev.parent, if one exists.
>>>
>>> So this should be updated if an OPP table is no longer optional.
>>
>> Right. Need to update it.
>>
>>>
>>>> Are there any reason why don't use the OPP table?
>>>
>>> dev_pm_opp_add() takes a voltage, and assumes the existence of some
>>> voltage regulator, but there is none involved here. The only way to have
>>> an OPP table without regulators is to use a static table in the
>>> devicetree. But that also doesn't make much sense, because the OPPs
>>> aren't actually customizable; they are integer dividers from a fixed
>>> base clock.
>>
>> You can use OPP for only clock control without regulator. OPP already
>> provides them. OPP already provides the helpful function which
>> implement the functions to handle the clock/regulator or power doamin.
>> It is useful framework to control clock/regulator. 
>>
>> If the standard framework in Linux kernel, it is best to use
>> this framework in order to remove the duplicate codes on multiple
>> device drivers. It is one of advantage of Linux kernel. 
>>
>> Also, if OPP doesn't support the some requirement of you,
>> you can contribute and update the OPP.
>>
>>  And adding a fixed OPP table to each board would be a lot of
>>> work to replace a trivial loop in the driver. So it seems to be the
>>> wrong abstraction.
>>
>> I don't understand. As I commented for patch 10, you can add
>> the OPP entry of the clock without the fixed OPP table in devicetree.
>>
>>>
>>> Using an OPP table adds extra complexity (memory allocations, error
>>> cases), just to duplicate the list of frequencies that already has to
>>> exist in freq_table. And the driver works fine without any of that.
>>
>> 'freq_table' of devfreq was developed before of adding OPP interface to Linux kernel as I knew. Actually, I prefer to use the OPP interface
>> instead of initializing the freq_table directly by device driver.
>> I just keep the 'freq_table' for preventing the build/working issue
>> for older device driver. I think OPP is enough to control frequency/voltage
>> and it provides the various helper funcitons for user of OPP.
> 
> Thanks for the explanation. I will convert the driver to use
> dev_pm_opp_add(), and I will drop patches 2 and 4. I think patch 3 is
> still worth considering.

Thanks. Previously, there was some devfreq driver to use freq_table
with descending order. I'll check the devfreq driver in linux kernel.
If there are no any use-case of freq_table with descending order,
I'll accept the patch3.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-10-01  1:53 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29  4:42 [PATCH 00/10] DRAM devfreq support for Allwinner A64/H5 Samuel Holland
2021-09-29  4:42 ` [PATCH 01/10] PM / devfreq: strengthen check for freq_table Samuel Holland
2021-09-30  3:58   ` Chanwoo Choi
2021-09-29  4:42 ` [PATCH 02/10] PM / devfreq: Do not require devices to have OPPs Samuel Holland
2021-09-30  4:19   ` Chanwoo Choi
2021-09-30 11:37     ` Samuel Holland
2021-10-01  1:59       ` Chanwoo Choi
2021-10-01  1:45         ` Samuel Holland
2021-10-01  2:14           ` Chanwoo Choi
2021-09-29  4:42 ` [PATCH 03/10] PM / devfreq: Drop code for descending freq_table Samuel Holland
2021-09-29  4:42 ` [PATCH 04/10] PM / devfreq: Add a recommended frequency helper Samuel Holland
2021-09-29  4:42 ` [PATCH 05/10] dt-bindings: clock: sunxi: Export CLK_DRAM for devfreq Samuel Holland
2021-09-29  4:42 ` [PATCH 06/10] dt-bindings: arm: sunxi: Expand MBUS binding Samuel Holland
2021-09-29 13:46   ` Rob Herring
2021-09-29  4:42 ` [PATCH 07/10] dt-bindings: arm: sunxi: Add H5 MBUS compatible Samuel Holland
2021-09-29  4:42 ` [PATCH 08/10] ARM: dts: sunxi: h3/h5: Update MBUS node Samuel Holland
2021-09-29  4:42 ` [PATCH 09/10] arm64: dts: allwinner: a64: " Samuel Holland
2021-09-29  4:42 ` [PATCH 10/10] PM / devfreq: Add a driver for the sun8i/sun50i MBUS Samuel Holland
2021-09-30  4:35   ` Chanwoo Choi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).