Devicetree
 help / color / mirror / Atom feed
* [PATCH v4 4/5] clk: renesas: Extract RZ/V2H PLL calculation helpers into shared library
From: Prabhakar @ 2026-06-18 18:19 UTC (permalink / raw)
  To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Brian Masney,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm
  Cc: linux-kernel, linux-renesas-soc, linux-clk, devicetree, Prabhakar,
	Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260618181949.3036280-1-prabhakar.mahadev-lad.rj@bp.renesas.com>

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Move the RZ/V2H PLL and divider parameter calculation helpers from
rzv2h-cpg.c into a new reusable library.

Introduce the CLK_RZV2H_CPG_LIB Kconfig symbol and add
rzv2h-cpg-lib.c to host the PLL parameter search algorithms currently
implemented by rzv2h_get_pll_pars() and rzv2h_get_pll_divs_pars().
Export the helpers as rzv2h_cpg_get_pll_pars() and
rzv2h_cpg_get_pll_divs_pars() for use by other drivers.

Update the public clock header to expose the new interfaces and provide
compatibility aliases for the existing helper names, avoiding build
breakage for current users while allowing future conversions to the new
API.

This prepares for reuse of the PLL and divider calculation logic by
other Renesas clock drivers, including upcoming RZ/T2H and RZ/N2H CPG
support, without duplicating the implementation.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v3->v4:
- Added macros for rzv2h_get_pll_pars and rzv2h_get_pll_divs_pars

v2->v3:
- Added export.h include in rzv2h-cpg-lib.c.

v1->v2:
- New patch
---
 drivers/clk/renesas/Kconfig         |   4 +
 drivers/clk/renesas/Makefile        |   1 +
 drivers/clk/renesas/rzv2h-cpg-lib.c | 217 ++++++++++++++++++++++++++++
 drivers/clk/renesas/rzv2h-cpg.c     | 203 --------------------------
 include/linux/clk/renesas.h         |  29 ++--
 5 files changed, 238 insertions(+), 216 deletions(-)
 create mode 100644 drivers/clk/renesas/rzv2h-cpg-lib.c

diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig
index 0203ecbb3882..7659550b8566 100644
--- a/drivers/clk/renesas/Kconfig
+++ b/drivers/clk/renesas/Kconfig
@@ -260,8 +260,12 @@ config CLK_RZG2L
 
 config CLK_RZV2H
 	bool "RZ/{G3E,V2H(P)} family clock support" if COMPILE_TEST
+	select CLK_RZV2H_CPG_LIB
 	select RESET_CONTROLLER
 
+config CLK_RZV2H_CPG_LIB
+	bool "RZV2H CPG library functions" if COMPILE_TEST
+
 config CLK_RENESAS_VBATTB
 	tristate "Renesas VBATTB clock controller"
 	depends on ARCH_RZG2L || COMPILE_TEST
diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile
index bd2bed91ab29..ac790e56034b 100644
--- a/drivers/clk/renesas/Makefile
+++ b/drivers/clk/renesas/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_CLK_RCAR_GEN3_CPG)		+= rcar-gen3-cpg.o
 obj-$(CONFIG_CLK_RCAR_GEN4_CPG)		+= rcar-gen4-cpg.o
 obj-$(CONFIG_CLK_RCAR_USB2_CLOCK_SEL)	+= rcar-usb2-clock-sel.o
 obj-$(CONFIG_CLK_RZG2L)			+= rzg2l-cpg.o
+obj-$(CONFIG_CLK_RZV2H_CPG_LIB)		+= rzv2h-cpg-lib.o
 obj-$(CONFIG_CLK_RZV2H)			+= rzv2h-cpg.o
 
 # Generic
diff --git a/drivers/clk/renesas/rzv2h-cpg-lib.c b/drivers/clk/renesas/rzv2h-cpg-lib.c
new file mode 100644
index 000000000000..124239c7327e
--- /dev/null
+++ b/drivers/clk/renesas/rzv2h-cpg-lib.c
@@ -0,0 +1,217 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RZV2H CPG Library. This library provides common functions to calculate
+ * PLL parameters for the RZV2H SoC.
+ *
+ * Copyright (C) 2026 Renesas Electronics Corp.
+ *
+ */
+
+#include <linux/clk/renesas.h>
+#include <linux/export.h>
+#include <linux/math.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+/**
+ * rzv2h_cpg_get_pll_pars - Finds the best combination of PLL parameters
+ * for a given frequency.
+ *
+ * @limits: Pointer to the structure containing the limits for the PLL parameters
+ * @pars: Pointer to the structure where the best calculated PLL parameters values
+ * will be stored
+ * @freq_millihz: Target output frequency in millihertz
+ *
+ * This function calculates the best set of PLL parameters (M, K, P, S) to achieve
+ * the desired frequency.
+ * There is no direct formula to calculate the PLL parameters, as it's an open
+ * system of equations, therefore this function uses an iterative approach to
+ * determine the best solution. The best solution is one that minimizes the error
+ * (desired frequency - actual frequency).
+ *
+ * Return: true if a valid set of parameters values is found, false otherwise.
+ */
+bool rzv2h_cpg_get_pll_pars(const struct rzv2h_pll_limits *limits,
+			    struct rzv2h_pll_pars *pars, u64 freq_millihz)
+{
+	unsigned long input_fref = limits->input_fref ?: (24 * MEGA);
+	u64 fout_min_millihz = mul_u32_u32(limits->fout.min, MILLI);
+	u64 fout_max_millihz = mul_u32_u32(limits->fout.max, MILLI);
+	struct rzv2h_pll_pars p, best;
+
+	if (freq_millihz > fout_max_millihz ||
+	    freq_millihz < fout_min_millihz)
+		return false;
+
+	/* Initialize best error to maximum possible value */
+	best.error_millihz = S64_MAX;
+
+	for (p.p = limits->p.min; p.p <= limits->p.max; p.p++) {
+		u32 fref = input_fref / p.p;
+		u16 divider;
+
+		for (divider = 1 << limits->s.min, p.s = limits->s.min;
+			p.s <= limits->s.max; p.s++, divider <<= 1) {
+			for (p.m = limits->m.min; p.m <= limits->m.max; p.m++) {
+				u64 output_m, output_k_range;
+				s64 pll_k, output_k;
+				u64 fvco, output;
+
+				/*
+				 * The frequency generated by the PLL + divider
+				 * is calculated as follows:
+				 *
+				 * With:
+				 * Freq = Ffout = Ffvco / 2^(pll_s)
+				 * Ffvco = (pll_m + (pll_k / 65536)) * Ffref
+				 * Ffref = 24MHz / pll_p
+				 *
+				 * Freq can also be rewritten as:
+				 * Freq = Ffvco / 2^(pll_s)
+				 *      = ((pll_m + (pll_k / 65536)) * Ffref) / 2^(pll_s)
+				 *      = (pll_m * Ffref) / 2^(pll_s) + ((pll_k / 65536) * Ffref) / 2^(pll_s)
+				 *      = output_m + output_k
+				 *
+				 * Every parameter has been determined at this
+				 * point, but pll_k.
+				 *
+				 * Considering that:
+				 * limits->k.min <= pll_k <= limits->k.max
+				 * Then:
+				 * -0.5 <= (pll_k / 65536) < 0.5
+				 * Therefore:
+				 * -Ffref / (2 * 2^(pll_s)) <= output_k < Ffref / (2 * 2^(pll_s))
+				 */
+
+				/* Compute output M component (in mHz) */
+				output_m = DIV_ROUND_CLOSEST_ULL(mul_u32_u32(p.m, fref) * MILLI,
+								 divider);
+				/* Compute range for output K (in mHz) */
+				output_k_range = DIV_ROUND_CLOSEST_ULL(mul_u32_u32(fref, MILLI),
+								       2 * divider);
+				/*
+				 * No point in continuing if we can't achieve
+				 * the desired frequency
+				 */
+				if (freq_millihz <  (output_m - output_k_range) ||
+				    freq_millihz >= (output_m + output_k_range)) {
+					continue;
+				}
+
+				/*
+				 * Compute the K component
+				 *
+				 * Since:
+				 * Freq = output_m + output_k
+				 * Then:
+				 * output_k = Freq - output_m
+				 *          = ((pll_k / 65536) * Ffref) / 2^(pll_s)
+				 * Therefore:
+				 * pll_k = (output_k * 65536 * 2^(pll_s)) / Ffref
+				 */
+				output_k = freq_millihz - output_m;
+				pll_k = div_s64(output_k * 65536ULL * divider,
+						fref);
+				pll_k = DIV_S64_ROUND_CLOSEST(pll_k, MILLI);
+
+				/* Validate K value within allowed limits */
+				if (pll_k < limits->k.min ||
+				    pll_k > limits->k.max)
+					continue;
+
+				p.k = pll_k;
+
+				/* Compute (Ffvco * 65536) */
+				fvco = mul_u32_u32(p.m * 65536 + p.k, fref);
+				if (fvco < mul_u32_u32(limits->fvco.min, 65536) ||
+				    fvco > mul_u32_u32(limits->fvco.max, 65536))
+					continue;
+
+				/* PLL_M component of (output * 65536 * PLL_P) */
+				output = mul_u32_u32(p.m * 65536, input_fref);
+				/* PLL_K component of (output * 65536 * PLL_P) */
+				output += p.k * input_fref;
+				/* Make it in mHz */
+				output *= MILLI;
+				output = DIV_U64_ROUND_CLOSEST(output, 65536 * p.p * divider);
+
+				/* Check output frequency against limits */
+				if (output < fout_min_millihz ||
+				    output > fout_max_millihz)
+					continue;
+
+				p.error_millihz = freq_millihz - output;
+				p.freq_millihz = output;
+
+				/* If an exact match is found, return immediately */
+				if (p.error_millihz == 0) {
+					*pars = p;
+					return true;
+				}
+
+				/* Update best match if error is smaller */
+				if (abs(best.error_millihz) > abs(p.error_millihz))
+					best = p;
+			}
+		}
+	}
+
+	/* If no valid parameters were found, return false */
+	if (best.error_millihz == S64_MAX)
+		return false;
+
+	*pars = best;
+	return true;
+}
+EXPORT_SYMBOL_NS_GPL(rzv2h_cpg_get_pll_pars, "RZV2H_CPG");
+
+/*
+ * rzv2h_cpg_get_pll_divs_pars - Finds the best combination of PLL parameters
+ * and divider value for a given frequency.
+ *
+ * @limits: Pointer to the structure containing the limits for the PLL parameters
+ * @pars: Pointer to the structure where the best calculated PLL parameters and
+ * divider values will be stored
+ * @table: Pointer to the array of valid divider values
+ * @table_size: Size of the divider values array
+ * @freq_millihz: Target output frequency in millihertz
+ *
+ * This function calculates the best set of PLL parameters (M, K, P, S) and divider
+ * value to achieve the desired frequency. See rzv2h_cpg_get_pll_pars() for more
+ * details on how the PLL parameters are calculated.
+ *
+ * freq_millihz is the desired frequency generated by the PLL followed by a
+ * a gear.
+ */
+bool rzv2h_cpg_get_pll_divs_pars(const struct rzv2h_pll_limits *limits,
+				 struct rzv2h_pll_div_pars *pars,
+				 const u8 *table, u8 table_size, u64 freq_millihz)
+{
+	struct rzv2h_pll_div_pars p, best;
+
+	best.div.error_millihz = S64_MAX;
+	p.div.error_millihz = S64_MAX;
+	for (unsigned int i = 0; i < table_size; i++) {
+		if (!rzv2h_cpg_get_pll_pars(limits, &p.pll, freq_millihz * table[i]))
+			continue;
+
+		p.div.divider_value = table[i];
+		p.div.freq_millihz = DIV_U64_ROUND_CLOSEST(p.pll.freq_millihz, table[i]);
+		p.div.error_millihz = freq_millihz - p.div.freq_millihz;
+
+		if (p.div.error_millihz == 0) {
+			*pars = p;
+			return true;
+		}
+
+		if (abs(best.div.error_millihz) > abs(p.div.error_millihz))
+			best = p;
+	}
+
+	if (best.div.error_millihz == S64_MAX)
+		return false;
+
+	*pars = best;
+	return true;
+}
+EXPORT_SYMBOL_NS_GPL(rzv2h_cpg_get_pll_divs_pars, "RZV2H_CPG");
diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
index fff89f2bdc0b..738dfafc6d9c 100644
--- a/drivers/clk/renesas/rzv2h-cpg.c
+++ b/drivers/clk/renesas/rzv2h-cpg.c
@@ -220,209 +220,6 @@ struct rzv2h_plldsi_div_clk {
 
 #define RZV2H_MAX_DIV_TABLES		(16)
 
-/**
- * rzv2h_get_pll_pars - Finds the best combination of PLL parameters
- * for a given frequency.
- *
- * @limits: Pointer to the structure containing the limits for the PLL parameters
- * @pars: Pointer to the structure where the best calculated PLL parameters values
- * will be stored
- * @freq_millihz: Target output frequency in millihertz
- *
- * This function calculates the best set of PLL parameters (M, K, P, S) to achieve
- * the desired frequency.
- * There is no direct formula to calculate the PLL parameters, as it's an open
- * system of equations, therefore this function uses an iterative approach to
- * determine the best solution. The best solution is one that minimizes the error
- * (desired frequency - actual frequency).
- *
- * Return: true if a valid set of parameters values is found, false otherwise.
- */
-bool rzv2h_get_pll_pars(const struct rzv2h_pll_limits *limits,
-			struct rzv2h_pll_pars *pars, u64 freq_millihz)
-{
-	unsigned long input_fref = limits->input_fref ?: (24 * MEGA);
-	u64 fout_min_millihz = mul_u32_u32(limits->fout.min, MILLI);
-	u64 fout_max_millihz = mul_u32_u32(limits->fout.max, MILLI);
-	struct rzv2h_pll_pars p, best;
-
-	if (freq_millihz > fout_max_millihz ||
-	    freq_millihz < fout_min_millihz)
-		return false;
-
-	/* Initialize best error to maximum possible value */
-	best.error_millihz = S64_MAX;
-
-	for (p.p = limits->p.min; p.p <= limits->p.max; p.p++) {
-		u32 fref = input_fref / p.p;
-		u16 divider;
-
-		for (divider = 1 << limits->s.min, p.s = limits->s.min;
-			p.s <= limits->s.max; p.s++, divider <<= 1) {
-			for (p.m = limits->m.min; p.m <= limits->m.max; p.m++) {
-				u64 output_m, output_k_range;
-				s64 pll_k, output_k;
-				u64 fvco, output;
-
-				/*
-				 * The frequency generated by the PLL + divider
-				 * is calculated as follows:
-				 *
-				 * With:
-				 * Freq = Ffout = Ffvco / 2^(pll_s)
-				 * Ffvco = (pll_m + (pll_k / 65536)) * Ffref
-				 * Ffref = 24MHz / pll_p
-				 *
-				 * Freq can also be rewritten as:
-				 * Freq = Ffvco / 2^(pll_s)
-				 *      = ((pll_m + (pll_k / 65536)) * Ffref) / 2^(pll_s)
-				 *      = (pll_m * Ffref) / 2^(pll_s) + ((pll_k / 65536) * Ffref) / 2^(pll_s)
-				 *      = output_m + output_k
-				 *
-				 * Every parameter has been determined at this
-				 * point, but pll_k.
-				 *
-				 * Considering that:
-				 * limits->k.min <= pll_k <= limits->k.max
-				 * Then:
-				 * -0.5 <= (pll_k / 65536) < 0.5
-				 * Therefore:
-				 * -Ffref / (2 * 2^(pll_s)) <= output_k < Ffref / (2 * 2^(pll_s))
-				 */
-
-				/* Compute output M component (in mHz) */
-				output_m = DIV_ROUND_CLOSEST_ULL(mul_u32_u32(p.m, fref) * MILLI,
-								 divider);
-				/* Compute range for output K (in mHz) */
-				output_k_range = DIV_ROUND_CLOSEST_ULL(mul_u32_u32(fref, MILLI),
-								       2 * divider);
-				/*
-				 * No point in continuing if we can't achieve
-				 * the desired frequency
-				 */
-				if (freq_millihz <  (output_m - output_k_range) ||
-				    freq_millihz >= (output_m + output_k_range)) {
-					continue;
-				}
-
-				/*
-				 * Compute the K component
-				 *
-				 * Since:
-				 * Freq = output_m + output_k
-				 * Then:
-				 * output_k = Freq - output_m
-				 *          = ((pll_k / 65536) * Ffref) / 2^(pll_s)
-				 * Therefore:
-				 * pll_k = (output_k * 65536 * 2^(pll_s)) / Ffref
-				 */
-				output_k = freq_millihz - output_m;
-				pll_k = div_s64(output_k * 65536ULL * divider,
-						fref);
-				pll_k = DIV_S64_ROUND_CLOSEST(pll_k, MILLI);
-
-				/* Validate K value within allowed limits */
-				if (pll_k < limits->k.min ||
-				    pll_k > limits->k.max)
-					continue;
-
-				p.k = pll_k;
-
-				/* Compute (Ffvco * 65536) */
-				fvco = mul_u32_u32(p.m * 65536 + p.k, fref);
-				if (fvco < mul_u32_u32(limits->fvco.min, 65536) ||
-				    fvco > mul_u32_u32(limits->fvco.max, 65536))
-					continue;
-
-				/* PLL_M component of (output * 65536 * PLL_P) */
-				output = mul_u32_u32(p.m * 65536, input_fref);
-				/* PLL_K component of (output * 65536 * PLL_P) */
-				output += p.k * input_fref;
-				/* Make it in mHz */
-				output *= MILLI;
-				output = DIV_U64_ROUND_CLOSEST(output, 65536 * p.p * divider);
-
-				/* Check output frequency against limits */
-				if (output < fout_min_millihz ||
-				    output > fout_max_millihz)
-					continue;
-
-				p.error_millihz = freq_millihz - output;
-				p.freq_millihz = output;
-
-				/* If an exact match is found, return immediately */
-				if (p.error_millihz == 0) {
-					*pars = p;
-					return true;
-				}
-
-				/* Update best match if error is smaller */
-				if (abs(best.error_millihz) > abs(p.error_millihz))
-					best = p;
-			}
-		}
-	}
-
-	/* If no valid parameters were found, return false */
-	if (best.error_millihz == S64_MAX)
-		return false;
-
-	*pars = best;
-	return true;
-}
-EXPORT_SYMBOL_NS_GPL(rzv2h_get_pll_pars, "RZV2H_CPG");
-
-/*
- * rzv2h_get_pll_divs_pars - Finds the best combination of PLL parameters
- * and divider value for a given frequency.
- *
- * @limits: Pointer to the structure containing the limits for the PLL parameters
- * @pars: Pointer to the structure where the best calculated PLL parameters and
- * divider values will be stored
- * @table: Pointer to the array of valid divider values
- * @table_size: Size of the divider values array
- * @freq_millihz: Target output frequency in millihertz
- *
- * This function calculates the best set of PLL parameters (M, K, P, S) and divider
- * value to achieve the desired frequency. See rzv2h_get_pll_pars() for more details
- * on how the PLL parameters are calculated.
- *
- * freq_millihz is the desired frequency generated by the PLL followed by a
- * a gear.
- */
-bool rzv2h_get_pll_divs_pars(const struct rzv2h_pll_limits *limits,
-			     struct rzv2h_pll_div_pars *pars,
-			     const u8 *table, u8 table_size, u64 freq_millihz)
-{
-	struct rzv2h_pll_div_pars p, best;
-
-	best.div.error_millihz = S64_MAX;
-	p.div.error_millihz = S64_MAX;
-	for (unsigned int i = 0; i < table_size; i++) {
-		if (!rzv2h_get_pll_pars(limits, &p.pll, freq_millihz * table[i]))
-			continue;
-
-		p.div.divider_value = table[i];
-		p.div.freq_millihz = DIV_U64_ROUND_CLOSEST(p.pll.freq_millihz, table[i]);
-		p.div.error_millihz = freq_millihz - p.div.freq_millihz;
-
-		if (p.div.error_millihz == 0) {
-			*pars = p;
-			return true;
-		}
-
-		if (abs(best.div.error_millihz) > abs(p.div.error_millihz))
-			best = p;
-	}
-
-	if (best.div.error_millihz == S64_MAX)
-		return false;
-
-	*pars = best;
-	return true;
-}
-EXPORT_SYMBOL_NS_GPL(rzv2h_get_pll_divs_pars, "RZV2H_CPG");
-
 /**
  * struct rzv2h_plldsi_mux_clk - PLL DSI MUX clock
  *
diff --git a/include/linux/clk/renesas.h b/include/linux/clk/renesas.h
index 798bb0b54bab..c9495558cd5c 100644
--- a/include/linux/clk/renesas.h
+++ b/include/linux/clk/renesas.h
@@ -189,28 +189,31 @@ struct rzv2h_pll_div_pars {
 		.k = { .min = -32768, .max = 32767 },			\
 	}								\
 
-#ifdef CONFIG_CLK_RZV2H
-bool rzv2h_get_pll_pars(const struct rzv2h_pll_limits *limits,
-			struct rzv2h_pll_pars *pars, u64 freq_millihz);
+#ifdef CONFIG_CLK_RZV2H_CPG_LIB
+bool rzv2h_cpg_get_pll_pars(const struct rzv2h_pll_limits *limits,
+			    struct rzv2h_pll_pars *pars, u64 freq_millihz);
 
-bool rzv2h_get_pll_divs_pars(const struct rzv2h_pll_limits *limits,
-			     struct rzv2h_pll_div_pars *pars,
-			     const u8 *table, u8 table_size, u64 freq_millihz);
+bool rzv2h_cpg_get_pll_divs_pars(const struct rzv2h_pll_limits *limits,
+				 struct rzv2h_pll_div_pars *pars,
+				 const u8 *table, u8 table_size, u64 freq_millihz);
 #else
-static inline bool rzv2h_get_pll_pars(const struct rzv2h_pll_limits *limits,
-				      struct rzv2h_pll_pars *pars,
-				      u64 freq_millihz)
+static inline bool rzv2h_cpg_get_pll_pars(const struct rzv2h_pll_limits *limits,
+					  struct rzv2h_pll_pars *pars,
+					  u64 freq_millihz)
 {
 	return false;
 }
 
-static inline bool rzv2h_get_pll_divs_pars(const struct rzv2h_pll_limits *limits,
-					   struct rzv2h_pll_div_pars *pars,
-					   const u8 *table, u8 table_size,
-					   u64 freq_millihz)
+static inline bool rzv2h_cpg_get_pll_divs_pars(const struct rzv2h_pll_limits *limits,
+					       struct rzv2h_pll_div_pars *pars,
+					       const u8 *table, u8 table_size,
+					       u64 freq_millihz)
 {
 	return false;
 }
 #endif
 
+#define rzv2h_get_pll_pars	rzv2h_cpg_get_pll_pars
+#define rzv2h_get_pll_divs_pars	rzv2h_cpg_get_pll_divs_pars
+
 #endif
-- 
2.54.0


^ permalink raw reply related

* [PATCH v4 5/5] clk: renesas: r9a09g077: Add LCDC and PLL3 clock support for RZ/T2H display pipeline
From: Prabhakar @ 2026-06-18 18:19 UTC (permalink / raw)
  To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Brian Masney,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm
  Cc: linux-kernel, linux-renesas-soc, linux-clk, devicetree, Prabhakar,
	Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260618181949.3036280-1-prabhakar.mahadev-lad.rj@bp.renesas.com>

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add the clock definitions and PLL logic required to supply the LCDC
(VSPD/FCPVD/DU) blocks on the RZ/T2H (R9A09G077) SoC. The RZ/T2H display
subsystem depends on a dedicated PLL (PLL3) and a set of new derived
clocks.

Introduce a new PLL clock type and implement rate recalculation,
programming and locking sequences for PLL3 using the RZ/T2H specific
divider and VCO limits. Add the corresponding muxes and divider entries,
expose the LCDC core clock, and register the LCDC module clock using the
correct PCLK parent.

This enables the RZ/T2H clock driver to generate the display pipeline
clocking tree needed by the DU and VSP-based composition engines, allowing
upcoming display support to be integrated without duplicating CPG logic.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v3->v4:
- Added RB tag from Geert.

v2->v3:
- In r9a09g077_cpg_lcdc_div_determine_rate() made use of 
  clk_hw_get_parent_by_index() to ensure we retrieve pll3 as the parent.

v1->v2:
- Switched to use the new library
- Kconfig now selects CLK_RZV2H_CPG_LIB
- Renamed CPG_PLLEN to CPG_PLL_EN_EN
- Renamed LCDCDIV to LCDC_CLKD
- Changed ctr0/1 in r9a09g077_cpg_pll3_clk_recalc_rate() to use u32
---
 drivers/clk/renesas/Kconfig         |   2 +
 drivers/clk/renesas/r9a09g077-cpg.c | 373 +++++++++++++++++++++++++++-
 2 files changed, 374 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig
index 7659550b8566..5c0238e878b7 100644
--- a/drivers/clk/renesas/Kconfig
+++ b/drivers/clk/renesas/Kconfig
@@ -218,10 +218,12 @@ config CLK_R9A09G057
 config CLK_R9A09G077
 	bool "RZ/T2H clock support" if COMPILE_TEST
 	select CLK_RENESAS_CPG_MSSR
+	select CLK_RZV2H_CPG_LIB
 
 config CLK_R9A09G087
 	bool "RZ/N2H clock support" if COMPILE_TEST
 	select CLK_RENESAS_CPG_MSSR
+	select CLK_RZV2H_CPG_LIB
 
 config CLK_SH73A0
 	bool "SH-Mobile AG5 clock support" if COMPILE_TEST
diff --git a/drivers/clk/renesas/r9a09g077-cpg.c b/drivers/clk/renesas/r9a09g077-cpg.c
index f777601a23b9..873c41ae5606 100644
--- a/drivers/clk/renesas/r9a09g077-cpg.c
+++ b/drivers/clk/renesas/r9a09g077-cpg.c
@@ -8,16 +8,23 @@
 
 #include <linux/bitfield.h>
 #include <linux/clk-provider.h>
+#include <linux/clk/renesas.h>
 #include <linux/device.h>
 #include <linux/init.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/kernel.h>
 #include <linux/math.h>
+#include <linux/module.h>
 #include <linux/types.h>
+#include <linux/units.h>
 
 #include <dt-bindings/clock/renesas,r9a09g077-cpg-mssr.h>
 #include <dt-bindings/clock/renesas,r9a09g087-cpg-mssr.h>
 #include "renesas-cpg-mssr.h"
 
+MODULE_IMPORT_NS("RZV2H_CPG");
+
 #define RZT2H_REG_BLOCK_SHIFT	11
 #define RZT2H_REG_OFFSET_MASK	GENMASK(10, 0)
 #define RZT2H_REG_CONF(block, offset)	(((block) << RZT2H_REG_BLOCK_SHIFT) | \
@@ -66,11 +73,26 @@
 #define DIVSCI2ASYNC	CONF_PACK(SCKCR3, 10, 2)
 #define DIVSCI3ASYNC	CONF_PACK(SCKCR3, 12, 2)
 #define DIVSCI4ASYNC	CONF_PACK(SCKCR3, 14, 2)
+#define LCDCDIVSEL	CONF_PACK(SCKCR3, 20, 4)
+
+#define PLL3EN		FIELD_PREP_CONST(OFFSET_MASK, (0xc0))
+
+#define CPG_PLL_EN_EN		BIT(0)
+#define CPG_PLL3_VCO_CTR0(x)	((x) + 0x4)
+#define CPG_PLL3_VCO_CTR0_PDIV	GENMASK(21, 16)
+#define CPG_PLL3_VCO_CTR0_MDIV	GENMASK(9, 0)
+#define CPG_PLL3_VCO_CTR1(x)	((x) + 0x8)
+#define CPG_PLL3_VCO_CTR1_KDIV	GENMASK(31, 16)
+#define CPG_PLL3_VCO_CTR1_SDIV	GENMASK(2, 0)
+#define CPG_PLL_MON(x)		((x) - 0x10)
+#define CPG_PLL_MON_LOCK	BIT(0)
 
 enum rzt2h_clk_types {
 	CLK_TYPE_RZT2H_DIV = CLK_TYPE_CUSTOM,	/* Clock with divider */
 	CLK_TYPE_RZT2H_MUX,			/* Clock with clock source selector */
 	CLK_TYPE_RZT2H_FSELXSPI,		/* Clock with FSELXSPIn source selector */
+	CLK_TYPE_RZT2H_PLL3,			/* PLL3 Clock */
+	CLK_TYPE_RZT2H_LCDCDIV,			/* LCDC divider clock */
 };
 
 #define DEF_DIV(_name, _id, _parent, _conf, _dtable) \
@@ -83,10 +105,51 @@ enum rzt2h_clk_types {
 #define DEF_DIV_FSELXSPI(_name, _id, _parent, _conf, _dtable) \
 	DEF_TYPE(_name, _id, CLK_TYPE_RZT2H_FSELXSPI, .conf = _conf, \
 		 .parent = _parent, .dtable = _dtable, .flag = 0)
+#define DEF_PLL3(_name, _id, _parent, _conf) \
+	DEF_TYPE(_name, _id, CLK_TYPE_RZT2H_PLL3, .conf = _conf, \
+		 .parent = _parent)
+#define DEF_DIV_LCDC(_name, _id, _parent, _conf, _dtable) \
+	DEF_TYPE(_name, _id, CLK_TYPE_RZT2H_LCDCDIV, .conf = _conf, \
+		 .parent = _parent, .dtable = _dtable, .flag = CLK_SET_RATE_PARENT)
+
+struct pll_clk {
+	void __iomem *reg;
+	const struct rzv2h_pll_limits *limits;
+	struct device *dev;
+	struct rzv2h_pll_pars pll_parameters;
+	struct clk_hw hw;
+	unsigned long cur_rate;
+};
+
+#define to_pll(_hw)	container_of(_hw, struct pll_clk, hw)
+
+struct r9a09g077_lcdc_div_clk {
+	const struct clk_div_table *dtable;
+	void __iomem *reg;
+	struct device *dev;
+	struct clk_hw hw;
+	u32 conf;
+	u8 divider;
+};
+
+#define to_lcdc_div_clk(_hw) \
+	container_of(_hw, struct r9a09g077_lcdc_div_clk, hw)
+
+#define RZT2H_MAX_LCDC_DIV_TABLES	16
+
+static const struct rzv2h_pll_limits r9a09g077_cpg_pll3_limits = {
+	.input_fref = 48 * MEGA,
+	.fout = { .min = 25 * MEGA, .max = 430 * MEGA },
+	.fvco = { .min = 1600 * MEGA, .max = 3200 * MEGA },
+	.m = { .min = 0x40, .max = 0x3ff },
+	.p = { .min = 0x2, .max = 0x8 },
+	.s = { .min = 0x0, .max = 0x6 },
+	.k = { .min = -32768, .max = 32767 },
+};
 
 enum clk_ids {
 	/* Core Clock Outputs exported to DT */
-	LAST_DT_CORE_CLK = R9A09G077_PCLKCAN,
+	LAST_DT_CORE_CLK = R9A09G077_LCDC_CLKD,
 
 	/* External Input Clocks */
 	CLK_EXTAL,
@@ -96,10 +159,12 @@ enum clk_ids {
 	CLK_PLL0,
 	CLK_PLL1,
 	CLK_PLL2,
+	CLK_PLL3,
 	CLK_PLL4,
 	CLK_SEL_CLK_PLL0,
 	CLK_SEL_CLK_PLL1,
 	CLK_SEL_CLK_PLL2,
+	CLK_SEL_CLK_PLL3,
 	CLK_SEL_CLK_PLL4,
 	CLK_PLL4D1,
 	CLK_PLL4D1_DIV3,
@@ -107,6 +172,7 @@ enum clk_ids {
 	CLK_PLL4D3,
 	CLK_PLL4D3_DIV10,
 	CLK_PLL4D3_DIV20,
+	CLK_PLL4D50,
 	CLK_SCI0ASYNC,
 	CLK_SCI1ASYNC,
 	CLK_SCI2ASYNC,
@@ -119,6 +185,7 @@ enum clk_ids {
 	CLK_SPI3ASYNC,
 	CLK_DIVSELXSPI0_SCKCR,
 	CLK_DIVSELXSPI1_SCKCR,
+	CLK_LCDDIVSEL,
 
 	/* Module Clocks */
 	MOD_CLK_BASE,
@@ -130,6 +197,26 @@ static const struct clk_div_table dtable_1_2[] = {
 	{0, 0},
 };
 
+static const struct clk_div_table dtable_2_32[] = {
+	{0, 2},
+	{1, 4},
+	{2, 6},
+	{3, 8},
+	{4, 10},
+	{5, 12},
+	{6, 14},
+	{7, 16},
+	{8, 18},
+	{9, 20},
+	{10, 22},
+	{11, 24},
+	{12, 26},
+	{13, 28},
+	{14, 30},
+	{15, 32},
+	{0, 0},
+};
+
 static const struct clk_div_table dtable_6_8_16_32_64[] = {
 	{6, 64},
 	{5, 32},
@@ -152,6 +239,7 @@ static const struct clk_div_table dtable_24_25_30_32[] = {
 static const char * const sel_clk_pll0[] = { ".loco", ".pll0" };
 static const char * const sel_clk_pll1[] = { ".loco", ".pll1" };
 static const char * const sel_clk_pll2[] = { ".loco", ".pll2" };
+static const char * const sel_clk_pll3[] = { ".loco", ".pll3" };
 static const char * const sel_clk_pll4[] = { ".loco", ".pll4" };
 static const char * const sel_clk_pll4d1_div3_div4[] = { ".pll4d1_div3", ".pll4d1_div4" };
 static const char * const sel_clk_pll4d3_div10_div20[] = { ".pll4d3_div10", ".pll4d3_div20" };
@@ -173,10 +261,14 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = {
 		sel_clk_pll1, ARRAY_SIZE(sel_clk_pll1), CLK_MUX_READ_ONLY),
 	DEF_MUX(".sel_clk_pll2", CLK_SEL_CLK_PLL2, SEL_PLL,
 		sel_clk_pll2, ARRAY_SIZE(sel_clk_pll2), CLK_MUX_READ_ONLY),
+	DEF_MUX(".sel_clk_pll3", CLK_SEL_CLK_PLL3, SEL_PLL,
+		sel_clk_pll3, ARRAY_SIZE(sel_clk_pll3), CLK_MUX_READ_ONLY),
 	DEF_MUX(".sel_clk_pll4", CLK_SEL_CLK_PLL4, SEL_PLL,
 		sel_clk_pll4, ARRAY_SIZE(sel_clk_pll4), CLK_MUX_READ_ONLY),
 
 	DEF_FIXED(".pll4d1", CLK_PLL4D1, CLK_SEL_CLK_PLL4, 1, 1),
+	DEF_FIXED(".pll4d50", CLK_PLL4D50, CLK_SEL_CLK_PLL4, 50, 1),
+	DEF_PLL3(".pll3", CLK_PLL3, CLK_PLL4D50, PLL3EN),
 	DEF_FIXED(".pll4d1_div3", CLK_PLL4D1_DIV3, CLK_PLL4D1, 3, 1),
 	DEF_FIXED(".pll4d1_div4", CLK_PLL4D1_DIV4, CLK_PLL4D1, 4, 1),
 	DEF_FIXED(".pll4d3", CLK_PLL4D3, CLK_SEL_CLK_PLL4, 3, 1),
@@ -229,6 +321,7 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = {
 	DEF_FIXED("PCLKL", R9A09G077_CLK_PCLKL, CLK_SEL_CLK_PLL1, 16, 1),
 	DEF_FIXED("PCLKAH", R9A09G077_CLK_PCLKAH, CLK_PLL4D1, 6, 1),
 	DEF_FIXED("PCLKAM", R9A09G077_CLK_PCLKAM, CLK_PLL4D1, 12, 1),
+	DEF_FIXED("PCLKAL", R9A09G077_CLK_PCLKAL, CLK_PLL4D1, 24, 1),
 	DEF_FIXED("SDHI_CLKHS", R9A09G077_SDHI_CLKHS, CLK_SEL_CLK_PLL2, 1, 1),
 	DEF_FIXED("USB_CLK", R9A09G077_USB_CLK, CLK_PLL4D1, 48, 1),
 	DEF_FIXED("ETCLKA", R9A09G077_ETCLKA, CLK_SEL_CLK_PLL1, 5, 1),
@@ -242,6 +335,8 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = {
 			 FSELXSPI1, dtable_6_8_16_32_64),
 	DEF_MUX("PCLKCAN", R9A09G077_PCLKCAN, FSELCANFD,
 		sel_clk_pll4d3_div10_div20, ARRAY_SIZE(sel_clk_pll4d3_div10_div20), 0),
+	DEF_DIV_LCDC("LCDC_CLKD", R9A09G077_LCDC_CLKD, CLK_SEL_CLK_PLL3, LCDCDIVSEL,
+		     dtable_2_32),
 };
 
 static const struct mssr_mod_clk r9a09g077_mod_clks[] __initconst = {
@@ -272,6 +367,7 @@ static const struct mssr_mod_clk r9a09g077_mod_clks[] __initconst = {
 	DEF_MOD("sci5fck", 600, CLK_SCI5ASYNC),
 	DEF_MOD("iic2", 601, R9A09G077_CLK_PCLKL),
 	DEF_MOD("spi3", 602, CLK_SPI3ASYNC),
+	DEF_MOD("lcdc", 1204, R9A09G077_CLK_PCLKAL),
 	DEF_MOD("sdhi0", 1212, R9A09G077_CLK_PCLKAM),
 	DEF_MOD("sdhi1", 1213, R9A09G077_CLK_PCLKAM),
 };
@@ -481,6 +577,276 @@ r9a09g077_cpg_fselxspi_div_clk_register(struct device *dev,
 	return hw->clk;
 }
 
+static unsigned long r9a09g077_cpg_pll3_clk_recalc_rate(struct clk_hw *hw,
+							unsigned long parent_rate)
+{
+	struct pll_clk *pll_clk = to_pll(hw);
+	u32 ctr0, ctr1;
+	u8 pdiv, sdiv;
+	u64 rate;
+	u16 mdiv;
+	s16 kdiv;
+
+	ctr0 = readl(CPG_PLL3_VCO_CTR0(pll_clk->reg));
+	ctr1 = readl(CPG_PLL3_VCO_CTR1(pll_clk->reg));
+
+	pdiv = FIELD_GET(CPG_PLL3_VCO_CTR0_PDIV, ctr0);
+	mdiv = FIELD_GET(CPG_PLL3_VCO_CTR0_MDIV, ctr0);
+	kdiv = (s16)FIELD_GET(CPG_PLL3_VCO_CTR1_KDIV, ctr1);
+	sdiv = FIELD_GET(CPG_PLL3_VCO_CTR1_SDIV, ctr1);
+
+	rate = mul_u64_u32_shr(parent_rate, (mdiv << 16) + kdiv, 16 + sdiv);
+
+	return DIV_ROUND_CLOSEST_ULL(rate, pdiv);
+}
+
+static int r9a09g077_cpg_pll3_determine_rate(struct clk_hw *hw,
+					     struct clk_rate_request *req)
+{
+	struct pll_clk *pll_clk = to_pll(hw);
+	u64 rate_millihz;
+
+	if (req->rate == pll_clk->cur_rate)
+		return 0;
+
+	rate_millihz = mul_u32_u32(req->rate, MILLI);
+	if (!rzv2h_cpg_get_pll_pars(pll_clk->limits, &pll_clk->pll_parameters,
+				    rate_millihz)) {
+		dev_dbg(pll_clk->dev,
+			"failed to determine rate for req->rate: %lu\n",
+			req->rate);
+		return -EINVAL;
+	}
+	req->rate = DIV_ROUND_CLOSEST_ULL(pll_clk->pll_parameters.freq_millihz, MILLI);
+	pll_clk->cur_rate = req->rate;
+
+	return 0;
+}
+
+static int r9a09g077_cpg_pll3_set_rate(struct clk_hw *hw, unsigned long rate,
+				       unsigned long parent_rate)
+{
+	struct pll_clk *pll_clk = to_pll(hw);
+	struct rzv2h_pll_pars *params = &pll_clk->pll_parameters;
+	void __iomem *offset = pll_clk->reg;
+	u32 val;
+	int ret;
+
+	/* Put PLL into standby mode */
+	writel(0, offset);
+	ret = readl_poll_timeout_atomic(CPG_PLL_MON(offset),
+					val, !(val & CPG_PLL_MON_LOCK),
+					100, 2000);
+	if (ret) {
+		dev_err(pll_clk->dev, "Failed to put PLL into standby mode");
+		return ret;
+	}
+
+	/* Output clock setting 1 */
+	val = readl(CPG_PLL3_VCO_CTR0(offset));
+	FIELD_MODIFY(CPG_PLL3_VCO_CTR0_MDIV, &val, params->m);
+	FIELD_MODIFY(CPG_PLL3_VCO_CTR0_PDIV, &val, params->p);
+	writel(val, CPG_PLL3_VCO_CTR0(offset));
+
+	/* Output clock setting 2 */
+	val = readl(CPG_PLL3_VCO_CTR1(offset));
+	FIELD_MODIFY(CPG_PLL3_VCO_CTR1_KDIV, &val, params->k);
+	FIELD_MODIFY(CPG_PLL3_VCO_CTR1_SDIV, &val, params->s);
+	writel(val, CPG_PLL3_VCO_CTR1(offset));
+
+	writel(CPG_PLL_EN_EN, offset);
+
+	/* PLL normal mode transition, output clock stability check */
+	ret = readl_poll_timeout_atomic(CPG_PLL_MON(offset),
+					val, (val & CPG_PLL_MON_LOCK),
+					100, 2000);
+	if (ret) {
+		writel(0, offset);
+		dev_err(pll_clk->dev, "Failed to put PLL into normal mode");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct clk_ops r9a09g077_cpg_pll3_ops = {
+	.recalc_rate = r9a09g077_cpg_pll3_clk_recalc_rate,
+	.determine_rate = r9a09g077_cpg_pll3_determine_rate,
+	.set_rate = r9a09g077_cpg_pll3_set_rate,
+};
+
+static struct clk * __init
+r9a09g077_cpg_pll3_clk_register(struct device *dev,
+				const struct cpg_core_clk *core,
+				void __iomem *addr,
+				struct cpg_mssr_pub *pub,
+				const struct rzv2h_pll_limits *limits)
+{
+	struct clk_init_data init = {};
+	const struct clk *parent;
+	const char *parent_name;
+	struct pll_clk *pll_clk;
+	int ret;
+
+	parent = pub->clks[core->parent];
+	if (IS_ERR(parent))
+		return ERR_CAST(parent);
+
+	pll_clk = devm_kzalloc(dev, sizeof(*pll_clk), GFP_KERNEL);
+	if (!pll_clk)
+		return ERR_PTR(-ENOMEM);
+
+	parent_name = __clk_get_name(parent);
+	init.name = core->name;
+	init.ops = &r9a09g077_cpg_pll3_ops;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	pll_clk->dev = dev;
+	pll_clk->hw.init = &init;
+	pll_clk->reg = addr;
+	pll_clk->limits = limits;
+
+	ret = devm_clk_hw_register(dev, &pll_clk->hw);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return pll_clk->hw.clk;
+}
+
+static int r9a09g077_cpg_lcdc_div_determine_rate(struct clk_hw *hw,
+						 struct clk_rate_request *req)
+{
+	struct r9a09g077_lcdc_div_clk *dsi_div = to_lcdc_div_clk(hw);
+	struct clk_hw *mux_hw = clk_hw_get_parent(hw);
+	u8 table[RZT2H_MAX_LCDC_DIV_TABLES] = { 0 };
+	struct rzv2h_pll_div_pars dsi_params;
+	const struct clk_div_table *div;
+	struct pll_clk *pll_clk;
+	unsigned int i = 0;
+	u64 freq_millihz;
+
+	/* index 1 is always .pll3 in sel_clk_pll3[] */
+	pll_clk = to_pll(clk_hw_get_parent_by_index(mux_hw, 1));
+
+	for (div = dsi_div->dtable; div->div; div++) {
+		if (i >= RZT2H_MAX_LCDC_DIV_TABLES)
+			return -EINVAL;
+		table[i++] = div->div;
+	}
+
+	freq_millihz = mul_u32_u32(req->rate, MILLI);
+
+	if (!rzv2h_cpg_get_pll_divs_pars(pll_clk->limits, &dsi_params, table,
+					 i, freq_millihz)) {
+		dev_err(dsi_div->dev,
+			"LCDC divider failed to determine rate for req->rate: %lu\n",
+			req->rate);
+		return -EINVAL;
+	}
+
+	req->rate = DIV_ROUND_CLOSEST_ULL(dsi_params.div.freq_millihz, MILLI);
+	req->best_parent_rate = req->rate * dsi_params.div.divider_value;
+	dsi_div->divider = dsi_params.div.divider_value;
+	pll_clk->cur_rate = req->best_parent_rate;
+	pll_clk->pll_parameters = dsi_params.pll;
+
+	return 0;
+}
+
+static int r9a09g077_cpg_lcdc_div_set_rate(struct clk_hw *hw,
+					   unsigned long rate,
+					   unsigned long parent_rate)
+{
+	struct r9a09g077_lcdc_div_clk *dsi_div = to_lcdc_div_clk(hw);
+	const struct clk_div_table *clkt;
+	bool divider_found = false;
+	u32 val, shift;
+
+	for (clkt = dsi_div->dtable; clkt->div; clkt++) {
+		if (clkt->div == dsi_div->divider) {
+			divider_found = true;
+			break;
+		}
+	}
+
+	if (!divider_found)
+		return -EINVAL;
+
+	shift = GET_SHIFT(dsi_div->conf);
+	val = readl(dsi_div->reg);
+	val &= ~(clk_div_mask(GET_WIDTH(dsi_div->conf)) << shift);
+	val |= clkt->val << shift;
+	writel(val, dsi_div->reg);
+
+	return 0;
+}
+
+static unsigned long
+r9a09g077_cpg_lcdc_div_recalc_rate(struct clk_hw *hw,
+				   unsigned long parent_rate)
+{
+	struct r9a09g077_lcdc_div_clk *dsi_div = to_lcdc_div_clk(hw);
+	u32 div;
+
+	div = readl(dsi_div->reg);
+	div >>= GET_SHIFT(dsi_div->conf);
+	div &= clk_div_mask(GET_WIDTH(dsi_div->conf));
+	div = dsi_div->dtable[div].div;
+
+	return DIV_ROUND_CLOSEST_ULL(parent_rate, div);
+}
+
+static const struct clk_ops r9a09g077_cpg_lcdc_div_ops = {
+	.recalc_rate = r9a09g077_cpg_lcdc_div_recalc_rate,
+	.determine_rate = r9a09g077_cpg_lcdc_div_determine_rate,
+	.set_rate = r9a09g077_cpg_lcdc_div_set_rate,
+};
+
+static struct clk * __init
+r9a09g077_cpg_lcdc_div_clk_register(struct device *dev,
+				    const struct cpg_core_clk *core,
+				    void __iomem *addr,
+				    struct cpg_mssr_pub *pub)
+{
+	struct r9a09g077_lcdc_div_clk *clk_hw_data;
+	struct clk_init_data init = {};
+	struct clk **clks = pub->clks;
+	const struct clk *parent;
+	const char *parent_name;
+	struct clk_hw *hw;
+	int ret;
+
+	parent = clks[core->parent];
+	if (IS_ERR(parent))
+		return ERR_CAST(parent);
+
+	clk_hw_data = devm_kzalloc(dev, sizeof(*clk_hw_data), GFP_KERNEL);
+	if (!clk_hw_data)
+		return ERR_PTR(-ENOMEM);
+
+	clk_hw_data->dtable = core->dtable;
+	clk_hw_data->reg = addr;
+	clk_hw_data->conf = core->conf;
+	clk_hw_data->dev = dev;
+	clk_hw_data->divider = 32; /* Initialize divider for LCDC */
+
+	parent_name = __clk_get_name(parent);
+	init.name = core->name;
+	init.ops = &r9a09g077_cpg_lcdc_div_ops;
+	init.flags = core->flag;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	hw = &clk_hw_data->hw;
+	hw->init = &init;
+	ret = devm_clk_hw_register(dev, hw);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return hw->clk;
+}
+
 static struct clk * __init
 r9a09g077_cpg_clk_register(struct device *dev, const struct cpg_core_clk *core,
 			   const struct cpg_mssr_info *info,
@@ -497,6 +863,11 @@ r9a09g077_cpg_clk_register(struct device *dev, const struct cpg_core_clk *core,
 		return r9a09g077_cpg_mux_clk_register(dev, core, addr, pub);
 	case CLK_TYPE_RZT2H_FSELXSPI:
 		return r9a09g077_cpg_fselxspi_div_clk_register(dev, core, addr, pub);
+	case CLK_TYPE_RZT2H_PLL3:
+		return r9a09g077_cpg_pll3_clk_register(dev, core, pub->base1 + offset,
+						       pub, &r9a09g077_cpg_pll3_limits);
+	case CLK_TYPE_RZT2H_LCDCDIV:
+		return r9a09g077_cpg_lcdc_div_clk_register(dev, core, addr, pub);
 	default:
 		return ERR_PTR(-EINVAL);
 	}
-- 
2.54.0


^ permalink raw reply related

* Re: [PATCH 08/11] dmaengine: ste_dma40: Use power domain for LCLA SRAM
From: Frank Li @ 2026-06-18 18:23 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
	Mark Brown, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, Vinod Koul, Frank Li, Lee Jones,
	linux-arm-kernel, devicetree, linux-pm, dri-devel, dmaengine
In-Reply-To: <20260618-ux500-power-domains-v7-1-v1-8-eb5e50b1a588@kernel.org>

On Thu, Jun 18, 2026 at 07:00:54AM +0200, Linus Walleij wrote:
> Replace the LCLA ESRAM regulator with runtime PM.
>
> Use the SRAM device that owns the ESRAM34 power domain.
>
> Hold that domain while DMA transfers are active.
>
> Assisted-by: Codex:gpt-5-5
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
>  drivers/dma/ste_dma40.c | 97 ++++++++++++++++++++++++++++---------------------
>  1 file changed, 55 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 9b803c0aec25..6ca67ec446dc 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -21,8 +21,8 @@
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/of_dma.h>
> +#include <linux/of_platform.h>
>  #include <linux/amba/bus.h>
> -#include <linux/regulator/consumer.h>
>
>  #include "dmaengine.h"
>  #include "ste_dma40.h"
> @@ -571,7 +571,8 @@ struct d40_gen_dmac {
>   * to phy_chans entries.
>   * @plat_data: Pointer to provided platform_data which is the driver
>   * configuration.
> - * @lcpa_regulator: Pointer to hold the regulator for the esram bank for lcla.
> + * @lcla_dev: SRAM device for the ESRAM bank used by LCLA.
> + * @lcla_pm_enabled: Whether runtime PM was enabled for LCLA by this driver.
>   * @phy_res: Vector containing all physical channels.
>   * @lcla_pool: lcla pool settings and data.
>   * @lcpa_base: The virtual mapped address of LCPA.
> @@ -607,7 +608,8 @@ struct d40_base {
>  	struct d40_chan			**lookup_log_chans;
>  	struct d40_chan			**lookup_phy_chans;
>  	struct stedma40_platform_data	 *plat_data;
> -	struct regulator		 *lcpa_regulator;
> +	struct device			 *lcla_dev;
> +	bool				  lcla_pm_enabled;
>  	/* Physical half channels */
>  	struct d40_phy_res		 *phy_res;
>  	struct d40_lcla_pool		  lcla_pool;
> @@ -628,6 +630,22 @@ static struct device *chan2dev(struct d40_chan *d40c)
>  	return &d40c->chan.dev->device;
>  }
>
> +static void d40_transfer_runtime_get(struct d40_base *base)
> +{
> +	if (base->lcla_dev)
> +		pm_runtime_get_sync(base->lcla_dev);
> +
> +	pm_runtime_get_sync(base->dev);

Suggest create device link between base->dev and base->lcla_dev, so run
time pm framework will auto do it for you

Ref: https://lore.kernel.org/imx/20260513-b4-b4-edma-runtime-opt-v5-4-1e595bfb8423@nxp.com/

Frank

^ permalink raw reply

* Re: [PATCH 3/8] Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq
From: Frank Li @ 2026-06-18 18:27 UTC (permalink / raw)
  To: Sherry Sun (OSS)
  Cc: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
	amitkumar.karwar, neeraj.sanjaykale, marcel, luiz.dentz,
	hongxing.zhu, l.stach, lpieralisi, kwilczynski, mani, bhelgaas,
	brgl, imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
	linux-bluetooth, linux-pm, sherry.sun
In-Reply-To: <20260618101047.4185497-4-sherry.sun@oss.nxp.com>

On Thu, Jun 18, 2026 at 06:10:42PM +0800, Sherry Sun (OSS) wrote:
> From: Sherry Sun <sherry.sun@nxp.com>
>
> Power supply to the M.2 Bluetooth device attached to the host using M.2
> connector is controlled using the 'uart' pwrseq device. So add support for
> getting the pwrseq device if the OF graph link is present. Once obtained,
> the existing pwrseq APIs can be used to control the power supplies of the
> M.2 card.
>
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---
>  drivers/bluetooth/btnxpuart.c | 33 ++++++++++++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
> index e7036a48ce48..1aa8972f0dab 100644
> --- a/drivers/bluetooth/btnxpuart.c
> +++ b/drivers/bluetooth/btnxpuart.c
> @@ -9,6 +9,8 @@
>
>  #include <linux/serdev.h>
>  #include <linux/of.h>
> +#include <linux/of_graph.h>
> +#include <linux/pwrseq/consumer.h>
>  #include <linux/skbuff.h>
>  #include <linux/unaligned.h>
>  #include <linux/firmware.h>
> @@ -211,6 +213,7 @@ struct btnxpuart_dev {
>
>  	struct ps_data psdata;
>  	struct btnxpuart_data *nxp_data;
> +	struct pwrseq_desc *pwrseq;
>  	struct reset_control *pdn;
>  	struct hci_uart hu;
>  };
> @@ -1866,11 +1869,27 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
>  		return err;
>  	}
>
> +	if (of_graph_is_present(dev_of_node(&serdev->ctrl->dev))) {
> +		struct pwrseq_desc *pwrseq;
> +
> +		pwrseq = devm_pwrseq_get(&serdev->ctrl->dev, "uart");
> +		if (IS_ERR(pwrseq))
> +			return PTR_ERR(pwrseq);
> +
> +		nxpdev->pwrseq = pwrseq;
> +		err = pwrseq_power_on(pwrseq);
> +		if (err) {
> +			dev_err(&serdev->dev, "Failed to power on pwrseq\n");
> +			return err;
> +		}

Can you provide helper function like devm clk get and enabled?
like devm_pwrsq_get_on()

So simple below error handle.

Frank

> +	}
> +
>  	/* Initialize and register HCI device */
>  	hdev = hci_alloc_dev();
>  	if (!hdev) {
>  		dev_err(&serdev->dev, "Can't allocate HCI device\n");
> -		return -ENOMEM;
> +		err = -ENOMEM;
> +		goto err_pwrseq_power_off;
>  	}
>
>  	reset_control_deassert(nxpdev->pdn);
> @@ -1903,11 +1922,14 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
>
>  	if (hci_register_dev(hdev) < 0) {
>  		dev_err(&serdev->dev, "Can't register HCI device\n");
> +		err = -ENODEV;
>  		goto probe_fail;
>  	}
>
> -	if (ps_setup(hdev))
> +	if (ps_setup(hdev)) {
> +		err = -ENODEV;
>  		goto probe_fail;
> +	}
>
>  	hci_devcd_register(hdev, nxp_coredump, nxp_coredump_hdr,
>  			   nxp_coredump_notify);
> @@ -1917,7 +1939,10 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
>  probe_fail:
>  	reset_control_assert(nxpdev->pdn);
>  	hci_free_dev(hdev);
> -	return -ENODEV;
> +err_pwrseq_power_off:
> +	if (nxpdev->pwrseq)
> +		pwrseq_power_off(nxpdev->pwrseq);
> +	return err;
>  }
>
>  static void nxp_serdev_remove(struct serdev_device *serdev)
> @@ -1944,6 +1969,8 @@ static void nxp_serdev_remove(struct serdev_device *serdev)
>  	ps_cleanup(nxpdev);
>  	hci_unregister_dev(hdev);
>  	reset_control_assert(nxpdev->pdn);
> +	if (nxpdev->pwrseq)
> +		pwrseq_power_off(nxpdev->pwrseq);
>  	hci_free_dev(hdev);
>  }
>
> --
> 2.50.1
>
>

^ permalink raw reply

* Re: [PATCH 2/8] power: sequencing: pcie-m2: Add PCI ID for NXP 88W9098 and AW693 Bluetooth
From: Frank Li @ 2026-06-18 18:29 UTC (permalink / raw)
  To: Sherry Sun (OSS)
  Cc: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
	amitkumar.karwar, neeraj.sanjaykale, marcel, luiz.dentz,
	hongxing.zhu, l.stach, lpieralisi, kwilczynski, mani, bhelgaas,
	brgl, imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
	linux-bluetooth, linux-pm, sherry.sun
In-Reply-To: <20260618101047.4185497-3-sherry.sun@oss.nxp.com>

On Thu, Jun 18, 2026 at 06:10:41PM +0800, Sherry Sun (OSS) wrote:
> From: Sherry Sun <sherry.sun@nxp.com>
>
> 88W9098 is a NXP Wi-Fi/BT combo chip with PCI device ID 0x2b43 under
> Marvell Extended vendor ID. AW693 is a NXP Wi-Fi/BT combo chip with
> PCI device ID 0x3003 under NXP/Philips vendor ID.
>
> Add both chips to pwrseq_m2_pci_ids[] so that the pwrseq-pcie-m2 driver
> can create the Bluetooth serdev device when these cards are inserted into
> a PCIe M.2 Key E connector.
>
> Both chips use "nxp,88w8987-bt" as the serdev compatible string, which
> is the entry point for the btnxpuart driver. The driver identifies the
> actual chip variant at runtime via chip ID auto-detection and loads the
> appropriate firmware accordingly.
>
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/power/sequencing/pwrseq-pcie-m2.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
> index 94c3f4b7ee36..9217ffcfa6e5 100644
> --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> @@ -186,6 +186,10 @@ static int pwrseq_pcie_m2_match(struct pwrseq_device *pwrseq,
>  }
>
>  static const struct pci_device_id pwrseq_m2_pci_ids[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x2b43),
> +	  .driver_data = (kernel_ulong_t)"nxp,88w8987-bt" },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_PHILIPS, 0x3003),
> +	  .driver_data = (kernel_ulong_t)"nxp,88w8987-bt" },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x1107),
>  	  .driver_data = (kernel_ulong_t)"qcom,wcn7850-bt" },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x1103),
> --
> 2.50.1
>
>

^ permalink raw reply

* Re: [PATCH v4 5/5] clk: renesas: r9a09g077: Add LCDC and PLL3 clock support for RZ/T2H display pipeline
From: sashiko-bot @ 2026-06-18 18:32 UTC (permalink / raw)
  To: Prabhakar; +Cc: robh, wsa+renesas, devicetree, conor+dt
In-Reply-To: <20260618181949.3036280-6-prabhakar.mahadev-lad.rj@bp.renesas.com>

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The `.determine_rate` callbacks unlawfully mutate internal driver state and break clock encapsulation, violating Common Clock Framework (CCF) semantics.
- [High] Kernel panic due to divide-by-zero in `r9a09g077_cpg_pll3_clk_recalc_rate` if the bootloader leaves the PLL uninitialized.
- [High] Unchecked return values from CCF parent resolution functions lead to a kernel panic via NULL pointer dereference.
- [Medium] Passing `unsigned long req->rate` to `mul_u32_u32()` causes silent 32-bit truncation on 64-bit platforms.
--

commit 1caebb13201d322aab2b2dd055d26412e4129ce0
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

clk: renesas: r9a09g077: Add LCDC and PLL3 clock support for RZ/T2H display pipeline

This commit adds clock definitions and PLL logic for the LCDC blocks on the
RZ/T2H SoC. It introduces a new PLL clock type, implementing rate
recalculation, programming, and locking sequences for PLL3. It also exposes
the LCDC core and module clocks to enable display pipeline integration.

diff --git a/drivers/clk/renesas/r9a09g077-cpg.c b/drivers/clk/renesas/r9a09g077-cpg.c
--- a/drivers/clk/renesas/r9a09g077-cpg.c
+++ b/drivers/clk/renesas/r9a09g077-cpg.c
@@ -577,6 +577,276 @@ r9a09g077_cpg_fselxspi_div_clk_register(struct device *dev,
> +static unsigned long r9a09g077_cpg_pll3_clk_recalc_rate(struct clk_hw *hw,
> +							unsigned long parent_rate)
> +{
[ ... ]
> +	ctr0 = readl(CPG_PLL3_VCO_CTR0(pll_clk->reg));
> +	ctr1 = readl(CPG_PLL3_VCO_CTR1(pll_clk->reg));
> +
> +	pdiv = FIELD_GET(CPG_PLL3_VCO_CTR0_PDIV, ctr0);
[ ... ]
> +	return DIV_ROUND_CLOSEST_ULL(rate, pdiv);

[Severity: High]
Does this code risk a divide-by-zero if the bootloader leaves the PLL
uninitialized?

If the clock controller block is in a standby state, the register might read
as 0, making pdiv 0. Unconditionally dividing by pdiv via
DIV_ROUND_CLOSEST_ULL() without validation would trigger a fatal kernel
divide-by-zero exception during the clock probe phase.

> +}
> +
> +static int r9a09g077_cpg_pll3_determine_rate(struct clk_hw *hw,
> +					     struct clk_rate_request *req)
> +{
> +	struct pll_clk *pll_clk = to_pll(hw);
> +	u64 rate_millihz;
> +
> +	if (req->rate == pll_clk->cur_rate)
> +		return 0;
> +
> +	rate_millihz = mul_u32_u32(req->rate, MILLI);

[Severity: Medium]
Will this silently truncate req->rate to 32 bits on 64-bit platforms?

mul_u32_u32() accepts 32-bit integers, but req->rate is an unsigned long
(which is 64-bit on architectures like ARM64). If a consumer requests a very
high rate, such as using clk_round_rate(clk, ULONG_MAX) to find the maximum
possible supported rate, the argument will be silently truncated to its
lower 32 bits before multiplication.

> +	if (!rzv2h_cpg_get_pll_pars(pll_clk->limits, &pll_clk->pll_parameters,
> +				    rate_millihz)) {
[ ... ]
> +	}
> +	req->rate = DIV_ROUND_CLOSEST_ULL(pll_clk->pll_parameters.freq_millihz, MILLI);
> +	pll_clk->cur_rate = req->rate;

[Severity: High]
Is it safe to mutate internal driver state like pll_clk->cur_rate and
pll_clk->pll_parameters during a read-only rate query?

Common Clock Framework semantics strictly require .determine_rate() callbacks
to be side-effect free, as they are used by the framework for speculative
evaluations (e.g., clk_round_rate()).

> +
> +	return 0;
> +}
> +
> +static int r9a09g077_cpg_pll3_set_rate(struct clk_hw *hw, unsigned long rate,
> +				       unsigned long parent_rate)
> +{
> +	struct pll_clk *pll_clk = to_pll(hw);
> +	struct rzv2h_pll_pars *params = &pll_clk->pll_parameters;

[Severity: High]
Does this code ignore the passed rate argument and program the hardware
using the cached pll_parameters?

Because .determine_rate() mutates driver-internal state, if the framework
evaluates multiple rates before applying one, the hardware will be programmed
with incorrect values from previous speculative checks.

[ ... ]
> +static int r9a09g077_cpg_lcdc_div_determine_rate(struct clk_hw *hw,
> +						 struct clk_rate_request *req)
> +{
> +	struct r9a09g077_lcdc_div_clk *dsi_div = to_lcdc_div_clk(hw);
> +	struct clk_hw *mux_hw = clk_hw_get_parent(hw);
[ ... ]
> +	/* index 1 is always .pll3 in sel_clk_pll3[] */
> +	pll_clk = to_pll(clk_hw_get_parent_by_index(mux_hw, 1));

[Severity: High]
Can mux_hw be NULL if the current clock is an orphan?

clk_hw_get_parent(hw) can return NULL during intermediate setup phases or
if a parent fails to register. Passing a potentially NULL mux_hw to
clk_hw_get_parent_by_index() unconditionally dereferences hw->core internally,
causing a kernel panic.

Furthermore, if clk_hw_get_parent_by_index itself returns NULL, to_pll()
performs unsafe pointer arithmetic yielding an invalid address that leads
to a crash later.

[ ... ]
> +	freq_millihz = mul_u32_u32(req->rate, MILLI);

[Severity: Medium]
Will this silently truncate the 64-bit req->rate to 32 bits on 64-bit
platforms, similar to r9a09g077_cpg_pll3_determine_rate()?

[ ... ]
> +	req->rate = DIV_ROUND_CLOSEST_ULL(dsi_params.div.freq_millihz, MILLI);
> +	req->best_parent_rate = req->rate * dsi_params.div.divider_value;
> +	dsi_div->divider = dsi_params.div.divider_value;
> +	pll_clk->cur_rate = req->best_parent_rate;
> +	pll_clk->pll_parameters = dsi_params.pll;

[Severity: High]
Is it safe to mutate internal driver state during this read-only query?

This function mutates both dsi_div->divider and its grandparent PLL3's cached
parameters (pll_clk->cur_rate and pll_clk->pll_parameters) during a
speculative rate evaluation.

> +
> +	return 0;
> +}
> +
> +static int r9a09g077_cpg_lcdc_div_set_rate(struct clk_hw *hw,
> +					   unsigned long rate,
> +					   unsigned long parent_rate)
> +{
> +	struct r9a09g077_lcdc_div_clk *dsi_div = to_lcdc_div_clk(hw);
> +	const struct clk_div_table *clkt;
> +	bool divider_found = false;
> +	u32 val, shift;
> +
> +	for (clkt = dsi_div->dtable; clkt->div; clkt++) {
> +		if (clkt->div == dsi_div->divider) {

[Severity: High]
Does this code rely on the mutated dsi_div->divider state instead of
recalculating the divider from the passed rate argument?

If the Common Clock Framework evaluates multiple rates before applying one,
the divider will hold an incorrect value from the last speculative check.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618181949.3036280-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=5

^ permalink raw reply

* Re: [PATCH v4 1/5] clk: renesas: rzv2h-cpg: Use per-SoC PLL reference frequency for calculations
From: sashiko-bot @ 2026-06-18 18:33 UTC (permalink / raw)
  To: Prabhakar; +Cc: conor+dt, wsa+renesas, robh, devicetree
In-Reply-To: <20260618181949.3036280-2-prabhakar.mahadev-lad.rj@bp.renesas.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Silent 32-bit unsigned integer overflow and zero-extension corrupts PLL calculations when 'p.k' is negative on 32-bit architectures.
--

commit 68e2983ee9a6e8d870b293872442cdc2bc7b3e74
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

clk: renesas: rzv2h-cpg: Use per-SoC PLL reference frequency for calculations

Introduce a per-SoC PLL reference input frequency parameter to avoid
relying on a hardcoded 24MHz constant during PLL configuration math.

> diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
> index e271c04cee34f..fff89f2bdc0b2 100644
> --- a/drivers/clk/renesas/rzv2h-cpg.c
> +++ b/drivers/clk/renesas/rzv2h-cpg.c

[ ... ]

> @@ -335,9 +335,9 @@ bool rzv2h_get_pll_pars(const struct rzv2h_pll_limits *limits,
>  					continue;
>  
>  				/* PLL_M component of (output * 65536 * PLL_P) */
> -				output = mul_u32_u32(p.m * 65536, RZ_V2H_OSC_CLK_IN_MEGA);
> +				output = mul_u32_u32(p.m * 65536, input_fref);
>  				/* PLL_K component of (output * 65536 * PLL_P) */
> -				output += p.k * RZ_V2H_OSC_CLK_IN_MEGA;
> +				output += p.k * input_fref;
[Severity: High]
This is a pre-existing issue, but does this multiplication safely handle
negative values of p.k on 32-bit architectures?

Since p.k is a signed 16-bit integer (with limits->k.min allowed to be -32768)
and input_fref is a 32-bit unsigned long, C promotion rules on 32-bit builds
will convert p.k to a 32-bit unsigned integer before the multiplication.

If p.k is negative, the resulting 32-bit unsigned modulo value will be
zero-extended when added to the 64-bit output variable. This would incorrectly
add over 4 billion to the output frequency rather than subtracting the
intended fraction, which could severely break the PLL parameter search.

Could p.k be cast to s64 before the multiplication to prevent this unsigned
overflow and ensure proper sign-extension into the 64-bit output?

>  				/* Make it in mHz */
>  				output *= MILLI;
>  				output = DIV_U64_ROUND_CLOSEST(output, 65536 * p.p * divider);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618181949.3036280-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=1

^ permalink raw reply

* Re: [PATCH v4 4/5] clk: renesas: Extract RZ/V2H PLL calculation helpers into shared library
From: sashiko-bot @ 2026-06-18 18:34 UTC (permalink / raw)
  To: Prabhakar; +Cc: wsa+renesas, conor+dt, devicetree, robh
In-Reply-To: <20260618181949.3036280-5-prabhakar.mahadev-lad.rj@bp.renesas.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The new library file `rzv2h-cpg-lib.c` uses 64-bit math macros and functions but fails to explicitly include `<linux/math64.h>`.

Pre-existing issues:
- [High] Incorrect signed/unsigned promotion on 32-bit architectures causes frequency calculation to erroneously increase instead of decrease when the K component is negative.
--

commit 70a8fa3652af11a63021ff758a114c890594d96c
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

clk: renesas: Extract RZ/V2H PLL calculation helpers into shared library

This patch extracts the RZ/V2H PLL and divider parameter calculation
helpers into a new shared library, introducing the CLK_RZV2H_CPG_LIB
Kconfig symbol. It adds rzv2h-cpg-lib.c to host the algorithms and
updates the header to expose compatibility aliases for existing users.

> diff --git a/drivers/clk/renesas/rzv2h-cpg-lib.c b/drivers/clk/renesas/rzv2h-cpg-lib.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/clk/renesas/rzv2h-cpg-lib.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0
[ ... ]
> +#include <linux/clk/renesas.h>
> +#include <linux/export.h>
> +#include <linux/math.h>
> +#include <linux/types.h>
> +#include <linux/units.h>

[Severity: Low]
Does this new file need to explicitly include linux/math64.h?

It heavily relies on 64-bit math operations like mul_u32_u32(), div_s64(),
DIV_S64_ROUND_CLOSEST(), and DIV_U64_ROUND_CLOSEST(), which are defined in
linux/math64.h. The original implementation in rzv2h-cpg.c included it,
but it appears to be missing here, which could lead to implicit function
declaration errors on certain architectures.

[ ... ]
> +bool rzv2h_cpg_get_pll_pars(const struct rzv2h_pll_limits *limits,
> +			    struct rzv2h_pll_pars *pars, u64 freq_millihz)
> +{
[ ... ]
> +				/* PLL_M component of (output * 65536 * PLL_P) */
> +				output = mul_u32_u32(p.m * 65536, input_fref);
> +				/* PLL_K component of (output * 65536 * PLL_P) */
> +				output += p.k * input_fref;

[Severity: High]
This isn't a bug introduced by this patch, but could this integer promotion
cause incorrect frequency calculations on 32-bit architectures?

The variable p.k is declared as an s16 (signed) and input_fref is an
unsigned long. On 32-bit systems, unsigned long is a 32-bit unsigned
integer. When p.k is negative, does the C integer promotion rule cause it
to be cast to a 32-bit unsigned integer before multiplication, resulting
in a massive positive number wrapping around?

If so, when added to the u64 output variable, it would be zero-extended
rather than sign-extended, effectively increasing the frequency massively
rather than subtracting the proportional frequency.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618181949.3036280-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=4

^ permalink raw reply

* Re: [PATCH v2 2/2] software node: Fix software_node_get_reference_args() with index -1
From: Andy Shevchenko @ 2026-06-18 18:35 UTC (permalink / raw)
  To: Alban Bedel
  Cc: driver-core, devicetree, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, Rob Herring, Saravana Kannan, Zijun Hu,
	linux-kernel, Sashiko
In-Reply-To: <20260618152035.1600436-2-alban.bedel@lht.dlh.de>

On Thu, Jun 18, 2026 at 05:20:35PM +0200, Alban Bedel wrote:
> The bounds check for the index passed to
> software_node_get_reference_args() was failing when passed UINT_MAX,
> this in turn would lead to an out of bound access in the property
> array. Fix the bound check to also cover the UINT_MAX case.

...

> -	if ((index + 1) * sizeof(*ref) > prop->length)
> +	if (index >= prop->length / sizeof(*ref))

It trades multiplication for division (which might be not always
power-of-two).

>  		return -ENOENT;

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH 1/8] PCI: imx6: Add skip_pwrctrl_off flag support
From: Frank Li @ 2026-06-18 18:37 UTC (permalink / raw)
  To: Sherry Sun (OSS)
  Cc: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
	amitkumar.karwar, neeraj.sanjaykale, marcel, luiz.dentz,
	hongxing.zhu, l.stach, lpieralisi, kwilczynski, mani, bhelgaas,
	brgl, imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
	linux-bluetooth, linux-pm, sherry.sun
In-Reply-To: <20260618101047.4185497-2-sherry.sun@oss.nxp.com>

On Thu, Jun 18, 2026 at 06:10:40PM +0800, Sherry Sun (OSS) wrote:
> From: Sherry Sun <sherry.sun@nxp.com>
>
> Use dw_pcie::skip_pwrctrl_off to avoid powering off devices during suspend
> to preserve wakeup capability of the devices and also not to power on the
> devices in the init path.
> This allows controller power-off to be skipped when some devices(e.g. M.2
> cards key E without auxiliary power) required to support PCIe L2 link state
> and wake-up mechanisms.
>
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 36 +++++++++++++++++----------
>  1 file changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 0fa716d1ed75..ff5a9565dbbf 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1382,16 +1382,20 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
>  		}
>  	}
>
> -	ret = pci_pwrctrl_create_devices(dev);
> -	if (ret) {
> -		dev_err(dev, "failed to create pwrctrl devices\n");
> -		goto err_reg_disable;
> +	if (!pci->suspended) {
> +		ret = pci_pwrctrl_create_devices(dev);
> +		if (ret) {
> +			dev_err(dev, "failed to create pwrctrl devices\n");
> +			goto err_reg_disable;
> +		}

supposed create_devices only do once.

pci_pwrctrl_power_on_devices() controller on and off for difference case.

Frank
>  	}
>
> -	ret = pci_pwrctrl_power_on_devices(dev);
> -	if (ret) {
> -		dev_err(dev, "failed to power on pwrctrl devices\n");
> -		goto err_pwrctrl_destroy;
> +	if (!pp->skip_pwrctrl_off) {
> +		ret = pci_pwrctrl_power_on_devices(dev);
> +		if (ret) {
> +			dev_err(dev, "failed to power on pwrctrl devices\n");
> +			goto err_pwrctrl_destroy;
> +		}
>  	}
>
>  	ret = imx_pcie_clk_enable(imx_pcie);
> @@ -1460,9 +1464,10 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
>  err_clk_disable:
>  	imx_pcie_clk_disable(imx_pcie);
>  err_pwrctrl_power_off:
> -	pci_pwrctrl_power_off_devices(dev);
> +	if (!pp->skip_pwrctrl_off)
> +		pci_pwrctrl_power_off_devices(dev);
>  err_pwrctrl_destroy:
> -	if (ret != -EPROBE_DEFER)
> +	if (ret != -EPROBE_DEFER && !pci->suspended)
>  		pci_pwrctrl_destroy_devices(dev);
>  err_reg_disable:
>  	if (imx_pcie->vpcie)
> @@ -1482,7 +1487,8 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
>  	}
>  	imx_pcie_clk_disable(imx_pcie);
>
> -	pci_pwrctrl_power_off_devices(pci->dev);
> +	if (!pci->pp.skip_pwrctrl_off)
> +		pci_pwrctrl_power_off_devices(pci->dev);
>  	if (imx_pcie->vpcie)
>  		regulator_disable(imx_pcie->vpcie);
>  }
> @@ -1990,12 +1996,16 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  static void imx_pcie_shutdown(struct platform_device *pdev)
>  {
>  	struct imx_pcie *imx_pcie = platform_get_drvdata(pdev);
> +	struct dw_pcie *pci = imx_pcie->pci;
> +	struct dw_pcie_rp *pp = &pci->pp;
>
>  	/* bring down link, so bootloader gets clean state in case of reboot */
>  	imx_pcie_assert_core_reset(imx_pcie);
>  	imx_pcie_assert_perst(imx_pcie, true);
> -	pci_pwrctrl_power_off_devices(&pdev->dev);
> -	pci_pwrctrl_destroy_devices(&pdev->dev);
> +	if (!pp->skip_pwrctrl_off)
> +		pci_pwrctrl_power_off_devices(&pdev->dev);
> +	if (!pci->suspended)
> +		pci_pwrctrl_destroy_devices(&pdev->dev);
>  }
>
>  static const struct imx_pcie_drvdata drvdata[] = {
> --
> 2.50.1
>
>

^ permalink raw reply

* Re: [PATCH v10 1/4] dt-bindings: clock: imx95-blk-ctl: Use single quotes consistently
From: Frank Li @ 2026-06-18 18:40 UTC (permalink / raw)
  To: guoniu.zhou
  Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Laurent Pinchart, Frank Li, Abel Vesa, Peng Fan,
	Michael Turquette, Stephen Boyd, imx, linux-media, devicetree,
	linux-arm-kernel, linux-kernel, linux-clk, Guoniu Zhou
In-Reply-To: <20260618-csi_formatter-v10-1-f23830312ba5@oss.nxp.com>

On Thu, Jun 18, 2026 at 05:41:35PM +0800, guoniu.zhou@oss.nxp.com wrote:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
>
> Change "clocks" to 'clocks' in the description to match the quote style
> used for property names like '#clock-cells' throughout the file.
>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v10:
> - New patch to fix inconsistent quote usage (Krzysztof Kozlowski)
> ---
>  Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> index 27403b4c52d6..534fa219d9f9 100644
> --- a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> +++ b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> @@ -36,7 +36,7 @@ properties:
>      const: 1
>      description:
>        The clock consumer should specify the desired clock by having the clock
> -      ID in its "clocks" phandle cell. See
> +      ID in its 'clocks' phandle cell. See
>        include/dt-bindings/clock/nxp,imx95-clock.h
>
>  required:
>
> --
> 2.34.1
>
>

^ permalink raw reply

* Re: [PATCH v10 2/4] media: dt-bindings: Add CSI Pixel Formatter DT bindings
From: Frank Li @ 2026-06-18 18:41 UTC (permalink / raw)
  To: guoniu.zhou
  Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Laurent Pinchart, Frank Li, Abel Vesa, Peng Fan,
	Michael Turquette, Stephen Boyd, imx, linux-media, devicetree,
	linux-arm-kernel, linux-kernel, linux-clk, Guoniu Zhou
In-Reply-To: <20260618-csi_formatter-v10-2-f23830312ba5@oss.nxp.com>

On Thu, Jun 18, 2026 at 05:41:36PM +0800, guoniu.zhou@oss.nxp.com wrote:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
>
> The i.MX95 CSI pixel formatting module uses packet info, pixel and
> non-pixel data from the CSI-2 host controller and reformat them to
> match Pixel Link(PL) definition.
>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
> Changes in v10:
> - Drop syscon parent node from example
> - Drop Reviewed-by tags from Frank and Krzysztof due to binding changes
> - Add description for reg property
> - Add space after formatter@20 before opening brace in example
> - Enhance the port description with more detailed information
> - Delete the blank line immediately following the endpoint in example
>
> Changes in v9:
> - Use direct node instead of syscon wrapper in example
>
> Changes in v8:
> - Use standard port reference instead of video-interfaces.yaml
> - Add parent syscon node in example to show device integration
> - Add required constraints for port@0 and port@1 in ports node
>
> Changes in v7:
> - Change compatible to imx95-csi-formatter as IP is i.MX95 specific per Marco's suggestion
>   Link: https://lore.kernel.org/linux-media/20260511-csi_formatter-v6-0-01028e312e2b@oss.nxp.com/T/#mcd135b3de179b3cb69daa1fd6e0e8e27c85b3332
> ---
>  .../bindings/media/fsl,imx95-csi-formatter.yaml    | 88 ++++++++++++++++++++++
>  1 file changed, 88 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/fsl,imx95-csi-formatter.yaml b/Documentation/devicetree/bindings/media/fsl,imx95-csi-formatter.yaml
> new file mode 100644
> index 000000000000..58c4e1cc056b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/fsl,imx95-csi-formatter.yaml
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/fsl,imx95-csi-formatter.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: i.MX95 CSI Pixel Formatter
> +
> +maintainers:
> +  - Guoniu Zhou <guoniu.zhou@nxp.com>
> +
> +description:
> +  The CSI pixel formatting module found on i.MX95 uses packet info, pixel
> +  and non-pixel data from the CSI-2 host controller and reformat them to
> +  match Pixel Link(PL) definition.
> +
> +properties:
> +  compatible:
> +    const: fsl,imx95-csi-formatter
> +
> +  reg:
> +    maxItems: 1
> +    description: Register offset and size within the parent syscon
> +
> +  clocks:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          Input port, connects to MIPI CSI-2 receiver output (IDI interface)
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          Output port, connects to ISI input via Pixel Link (PL)
> +
> +    required:
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - power-domains
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/nxp,imx95-clock.h>
> +
> +    formatter@20 {
> +        compatible = "fsl,imx95-csi-formatter";
> +        reg = <0x20 0x100>;
> +        clocks = <&cameramix_csr IMX95_CLK_CAMBLK_CSI2_FOR0>;
> +        power-domains = <&scmi_devpd 3>;
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +
> +                endpoint {
> +                    remote-endpoint = <&mipi_csi_0_out>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +
> +                endpoint {
> +                    remote-endpoint = <&isi_in_2>;
> +                };
> +            };
> +        };
> +    };
>
> --
> 2.34.1
>
>

^ permalink raw reply

* Re: [PATCH v10 3/4] dt-bindings: clock: imx95-blk-ctl: Define formatter child node schema
From: Frank Li @ 2026-06-18 18:45 UTC (permalink / raw)
  To: guoniu.zhou
  Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Laurent Pinchart, Frank Li, Abel Vesa, Peng Fan,
	Michael Turquette, Stephen Boyd, imx, linux-media, devicetree,
	linux-arm-kernel, linux-kernel, linux-clk, Guoniu Zhou
In-Reply-To: <20260618-csi_formatter-v10-3-f23830312ba5@oss.nxp.com>

On Thu, Jun 18, 2026 at 05:41:37PM +0800, guoniu.zhou@oss.nxp.com wrote:
> From: Guoniu Zhou <guoniu.zhou@nxp.com>
>
> The Camera CSR contains control registers for multiple CSI formatter IPs
> at different register offsets. Each formatter is an independent hardware
> block with its own clock input and media pipeline connection.
>
> Define schema to allow formatter child nodes under nxp,imx95-camera-csr,
> with 'reg' property specifying the formatter's register offset within the
> CSR address space.
>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> ---
> Changes in v10:
> - Use single quotes for regex pattern to be consistent (Krzysztof Kozlowski)
> - Add formatter subnode binding and camera-csr syscon example
> - Update commit title and message
>
> Changes in v9:
> - New patch to address the issue of formatter acting as a child node of syscon
> ---
>  .../bindings/clock/nxp,imx95-blk-ctl.yaml          | 64 +++++++++++++++++++++-
>  1 file changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> index 534fa219d9f9..b4d0a7670fac 100644
> --- a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> +++ b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> @@ -46,7 +46,27 @@ required:
>    - power-domains
>    - clocks
>
> -additionalProperties: false
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: nxp,imx95-camera-csr
> +    then:
> +      properties:
> +        '#address-cells':
> +          const: 1
> +        '#size-cells':
> +          const: 1
> +      required:
> +        - '#address-cells'
> +        - '#size-cells'
> +      patternProperties:
> +        '^formatter@[0-9a-f]+$':
> +          type: object
> +          $ref: /schemas/media/fsl,imx95-csi-formatter.yaml#

suppose      "unevaluatedProperties:" false should under '^formatter@[0-9a-f]+$':

> +
> +unevaluatedProperties: false

here should keep original additionalProperties: false

Frank

>
>  examples:
>    - |
> @@ -57,4 +77,46 @@ examples:
>        clocks = <&scmi_clk 114>;
>        power-domains = <&scmi_devpd 21>;
>      };
> +
> +  - |
> +    #include <dt-bindings/clock/nxp,imx95-clock.h>
> +
> +    syscon@4ac10000 {
> +      compatible = "nxp,imx95-camera-csr", "syscon";
> +      reg = <0x4ac10000 0x10000>;
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +      #clock-cells = <1>;
> +      clocks = <&scmi_clk 62>;
> +      power-domains = <&scmi_devpd 3>;
> +
> +      formatter@20 {
> +        compatible = "fsl,imx95-csi-formatter";
> +        reg = <0x20 0x100>;
> +        clocks = <&cameramix_csr IMX95_CLK_CAMBLK_CSI2_FOR0>;
> +        power-domains = <&scmi_devpd 3>;
> +
> +        ports {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          port@0 {
> +            reg = <0>;
> +
> +            endpoint {
> +              remote-endpoint = <&mipi_csi_0_out>;
> +            };
> +
> +          };
> +
> +          port@1 {
> +            reg = <1>;
> +
> +            endpoint {
> +              remote-endpoint = <&isi_in_2>;
> +            };
> +          };
> +        };
> +      };
> +    };
>  ...
>
> --
> 2.34.1
>
>

^ permalink raw reply

* Re: [PATCH V2 2/3] dmaengine: zynqmp_dma: Add per-channel reset support
From: Frank Li @ 2026-06-18 18:52 UTC (permalink / raw)
  To: Golla Nagendra
  Cc: vkoul, Frank.Li, michal.simek, robh, krzk+dt, conor+dt,
	jay.buddhabhatti, harini.katakam, m.tretter, radhey.shyam.pandey,
	abin.joseph, kees, sakari.ailus, git, dmaengine, devicetree,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20260618071056.2024286-3-nagendra.golla@amd.com>

On Thu, Jun 18, 2026 at 12:40:55PM +0530, Golla Nagendra wrote:
> [You don't often get email from nagendra.golla@amd.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Versal Gen 2 and Versal Net SoCs expose a dedicated reset line per
> ZDMA channel, replacing the earlier approach where a single reset
> was shared across all channels. Add reset handling in the channel
> probe path using device_reset_optional() to trigger a reset pulse
> on the channel during initialization.
>
> Platforms without per-channel reset continue to work unaffected
> since device_reset_optional() returns 0 when no reset is specified.
>
> add pm_runtime_put_noidle() in the probe error path before
> pm_runtime_disable() to balance the usage counter incremented by
> pm_runtime_resume_and_get(). This is particularly important since
> device_reset_optional() can return -EPROBE_DEFER, causing the
> kernel to retry probe() and leak one PM usage count per retry
> without the put.

Use sperate patch to fix this problem

Frank
>
> Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
> ---
>  drivers/dma/xilinx/zynqmp_dma.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index f6a812e49ddc..a9dfec3c0ca3 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
> @@ -18,6 +18,7 @@
>  #include <linux/clk.h>
>  #include <linux/io-64-nonatomic-lo-hi.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/reset.h>
>
>  #include "../dmaengine.h"
>
> @@ -916,6 +917,11 @@ static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
>         if (IS_ERR(chan->regs))
>                 return PTR_ERR(chan->regs);
>
> +       err = device_reset_optional(&pdev->dev);
> +       if (err)
> +               return dev_err_probe(&pdev->dev, err,
> +                                    "failed to reset channel\n");
> +
>         chan->bus_width = ZYNQMP_DMA_BUS_WIDTH_64;
>         chan->dst_burst_len = ZYNQMP_DMA_MAX_DST_BURST_LEN;
>         chan->src_burst_len = ZYNQMP_DMA_MAX_SRC_BURST_LEN;
> @@ -1152,6 +1158,7 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
>  err_disable_pm:
>         if (!pm_runtime_enabled(zdev->dev))
>                 zynqmp_dma_runtime_suspend(zdev->dev);
> +       pm_runtime_put_noidle(zdev->dev);
>         pm_runtime_disable(zdev->dev);
>         return ret;
>  }
> --
> 2.34.1
>

^ permalink raw reply

* Re: [PATCH v4 2/2] clk: amlogic: Add A9 AO clock controller driver
From: Julian Braha @ 2026-06-18 18:56 UTC (permalink / raw)
  To: jian.hu, Neil Armstrong, Jerome Brunet, Michael Turquette,
	Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Xianwei Zhao, Kevin Hilman, Martin Blumenstingl
  Cc: linux-amlogic, linux-clk, devicetree, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20260618-a9_aoclk-v4-2-569d0425e50c@amlogic.com>

Hi Jian,

On 6/18/26 10:49, Jian Hu via B4 Relay wrote:

> +config COMMON_CLK_A9_AO
> +	tristate "Amlogic A9 SoC AO clock controller support"
> +	depends on ARM64 || COMPILE_TEST
> +	default ARCH_MESON
> +	select COMMON_CLK_MESON_REGMAP
> +	select COMMON_CLK_MESON_CLKC_UTILS
> +	select COMMON_CLK_MESON_DUALDIV

Selecting COMMON_CLK_MESON_REGMAP is unnecessary since you're already
selecting COMMON_CLK_MESON_DUALDIV here.

- Julian Braha


^ permalink raw reply

* Re: [PATCH RFC v4 01/12] dt-bindings: clk: zte: Add zx297520v3 top clock and reset bindings
From: Stefan Dösinger @ 2026-06-18 18:59 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Philipp Zabel, Brian Masney, linux-clk, devicetree,
	linux-kernel, linux-arm-kernel
In-Reply-To: <20260617-deed-snap-4649ffae0e27@spud>

[-- Attachment #1: Type: text/plain, Size: 862 bytes --]

Am Donnerstag, 18. Juni 2026, 00:23:56 Ostafrikanische Zeit schrieben Sie:

> Do you actually need an aux bus here though? Since you have to add
> simple-mfd for your the syscon-reboot and simple-mfd is a real bus, can you
> set the reset controller up with an mfd_cell + devm_mfd_add_devices()
> instead?

I'll have to read up on devm_mfd_add_devices; The aux bus was the suggestion 
of Philipp Zabel. At first sight it sounds to me like they do fairly similar 
things. I don't see any precedence for [devm_]mfd_add_devices in drivers/clk/.

Whatever way I go I'd like to use the same for all 3 clock/reset controllers. 
So far I only made topclk a simple-mfd. I recently stumbled upon spinlock 
registers in matrixclk, so I guess I can justify a simple-mfd there too. For 
lspclk all I can see is clocks and resets and I ran out of unknown registers 
in there.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

^ permalink raw reply

* Re: [PATCH v2 0/2] iio: magnetometer: add support for Melexis MLX90393
From: Andy Shevchenko @ 2026-06-18 18:59 UTC (permalink / raw)
  To: Nikhil Gautam
  Cc: linux-iio, jic23, dlechner, nuno.sa, andy, robh, krzk+dt,
	conor+dt, devicetree, linux-kernel
In-Reply-To: <20260618160141.11409-1-nikhilgtr@gmail.com>

On Thu, Jun 18, 2026 at 09:31:39PM +0530, Nikhil Gautam wrote:
> Hi,
> 
> This series adds initial Industrial I/O subsystem support for the
> Melexis MLX90393 3-axis magnetometer and temperature sensor.
> 
> The MLX90393 supports both I2C and SPI interfaces. This series
> implements support for the I2C interface while keeping the driver
> structure transport-independent to simplify future SPI support.
> 
> Currently supported features:
> 
> * Raw magnetic field measurements for X/Y/Z axes
> * Raw temperature measurements
> * Configurable gain/scale selection
> * Configurable oversampling ratio
> * Direct mode operation through the IIO subsystem
> * I2C interface support
> 
> The driver has been tested on Raspberry Pi 5 hardware using an
> MLX90393 sensor connected over I2C. Magnetic field and temperature
> measurements were verified through the IIO sysfs interface.

This doesn't answer to two important questions:
- why do we need a brand new driver?
  Can't one of the existing be updated to cover this HW?

- where to find the datasheet? Any Links or other means to get it?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH V2 3/3] dmaengine: zynqmp_dma: Guard IRQ handler against spurious interrupts
From: Frank Li @ 2026-06-18 19:15 UTC (permalink / raw)
  To: Golla Nagendra
  Cc: vkoul, Frank.Li, michal.simek, robh, krzk+dt, conor+dt,
	jay.buddhabhatti, harini.katakam, m.tretter, radhey.shyam.pandey,
	abin.joseph, kees, sakari.ailus, git, dmaengine, devicetree,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20260618071056.2024286-4-nagendra.golla@amd.com>

On Thu, Jun 18, 2026 at 12:40:56PM +0530, Golla Nagendra wrote:
> [You don't often get email from nagendra.golla@amd.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Add pm_runtime_get_if_active() check in zynqmp_dma_irq_handler() to
> safely handle spurious interrupts that may arrive while the device is
> runtime-suspended. Without this guard, a spurious interrupt could cause
> the handler to access hardware registers (ISR, IMR) with clocks gated,
> potentially leading to a synchronous external abort and kernel crash.
>
> When the device is not runtime-active, pm_runtime_get_if_active()
> returns false without incrementing the usage counter, and the handler
> returns IRQ_NONE immediately. When the device is active, it increments
> the usage counter to prevent a concurrent runtime suspend during
> register access, and pm_runtime_put() releases the reference afterward.
>
> Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
> ---
>  drivers/dma/xilinx/zynqmp_dma.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index a9dfec3c0ca3..ce9163138be7 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
> @@ -730,6 +730,9 @@ static irqreturn_t zynqmp_dma_irq_handler(int irq, void *data)
>         u32 isr, imr, status;
>         irqreturn_t ret = IRQ_NONE;
>
> +       if (pm_runtime_get_if_active(chan->dev) <= 0)
> +               return IRQ_NONE;
> +

Can you add AQUIRE macro in include/linux/pm_runtime.h
so here can use PM_RUNTIME_ACQUIRE_IF_ACITVE

Other person can get benefit for auto clean up especially if there are some
difference return path.

Frank

>         isr = readl(chan->regs + ZYNQMP_DMA_ISR);
>         imr = readl(chan->regs + ZYNQMP_DMA_IMR);
>         status = isr & ~imr;
> @@ -756,6 +759,8 @@ static irqreturn_t zynqmp_dma_irq_handler(int irq, void *data)
>                 ret = IRQ_HANDLED;
>         }
>
> +       pm_runtime_put(chan->dev);
> +
>         return ret;
>  }
>
> --
> 2.34.1
>

^ permalink raw reply

* Re: [PATCH v2 02/10] libfdt: Don't assume that a FDT_BEGIN_NODE tag is available at offset 0
From: Herve Codina @ 2026-06-18 19:17 UTC (permalink / raw)
  To: David Gibson
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ayush Singh,
	Geert Uytterhoeven, devicetree-compiler, devicetree, linux-kernel,
	devicetree-spec, Hui Pu, Ian Ray, Luca Ceresoli, Thomas Petazzoni
In-Reply-To: <ajPE5eGcOwWMAeiN@zatzit>

Hi David,

On Thu, 18 Jun 2026 20:13:57 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Apr 09, 2026 at 01:54:18PM +0200, Herve Codina wrote:
> > In several places, libfdt assumes that a FDT_BEGIN_NODE tag is present
> > at the offset 0 of the structure block.
> > 
> > This assumption is not correct. Indeed, a FDT_NOP can be present at the
> > offset 0 and this is a legit case.
> > 
> > fdt_first_node() has been introduced recently to get the offset of the
> > first node (first FDT_BEGIN_NODE) in a fdt blob.
> > 
> > Use this function to get the first node offset instead of looking for
> > this node at offset 0.
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>  
> 
> The problem is real, of course.  But this approach to solving it with
> a special case just for the root node is really ugly.
> 
> Granted, it's a problem of my own making - I chose not to create an
> fdt_root_offset() function in the first place, instead making it part
> of the API that offset 0 means the root node.  Nonetheless, here we
> are and the question is whether we can do better.
> 
> # Straightforward things first
> 
>  - This patch should be folded with 1/10, they're both harder to
>    understand without the context of the other.

Ok, I will squash, no problem.

> 
>  - If it must exist, the function should be fdt_root_offset(), not
>    fdt_first_node(), for at least three reasons:
>     * "first" in what sense?
>     * "first" amongst what set of nodes?
>     * We have a strong convention to always explicitly say "offset",
>       not just referring to offset values as "node" or "property".
>       This is deliberate: it's an attempt to discourage the otherwise
>       likely misunderstanding that a function getting a "node" gives
>       you some sort of persistent handle.  "offset" makes it clearer
>       that the value will no longer be valid after a modification to
>       the tree.

Make sense. I will rename to fdt_root_offset()

> 
>  - The situation described is subtle enough that this *really* needs a
>    testcase.  It shouldn't be that hard: change the existing
>    'nopulate' test tool to add an FDT_NOP before the first tag, not
>    just after

Yes, will add a test.


> 
> # Is FDT_NOP before the root node actually legitimate?
> 
> Arguably the simplest solution here would be to explicitly ban this.
> Yes, it would be a slightly odd restriction in the spec.  However,
> avoiding the mess in the library might be worth it.  Note that this
> situation can never arise from fdt_nop_node(), unless you apply it to
> the root node, in which case there's no tree left.

We tried to have something robust for future addition (structured tags).
Maybe a future tag will be nopified by some future tools before being
processed by libfdt.

IMHO, we should have support for FDT_NOP before the root node.

> 
> # Less special casery
> 
> Even if we accept the need for FDT_NOP before the root node, I think
> we can do better.  The below implements this as a special case, just
> for offset 0.  Instead, we could allow all node operations on a
> FDT_NOP offset, automatically advancing to the next FDT_BEGIN_NODE
> tag.  We may be able to do that in check_node_offset_() minimising
> code duplication.
> 

IHMO, check_node_offset_() should only check that the given offset is a
node and not trying to find the next node available after possible FDT_NOP.
Got the feeling that having this kind of search in check_node_offset_() is
error prone.

I am not sure that a lot of code duplication will be present. On some entry
points, we have this kind of code:
   --- 8< ---
   if (offset == 0) {
	offset = fdt_root_offset(fdt);
	if (offset < 0)
		return offset;
   }
   --- 8< ---

It has the benefit to keep things clear and is needed only on some entry
points (API function). Internal function should receive an offset pointing
to a node. For those internal function check_node_offset_() should not
automatically skip FDT_NOP tags but should really return an error if such a
tag is encountered.

For offsets other than offset 0, FDT_NOP is handled without any extra cost in
current code implementation.

Best regards,
Hervé

^ permalink raw reply

* Re: [PATCH v2 2/2] iio: magnetometer: add support for Melexis MLX90393
From: Andy Shevchenko @ 2026-06-18 19:26 UTC (permalink / raw)
  To: Nikhil Gautam
  Cc: linux-iio, jic23, dlechner, nuno.sa, andy, robh, krzk+dt,
	conor+dt, devicetree, linux-kernel
In-Reply-To: <20260618160141.11409-3-nikhilgtr@gmail.com>

On Thu, Jun 18, 2026 at 09:31:41PM +0530, Nikhil Gautam wrote:
> Add Industrial I/O subsystem support for the Melexis
> MLX90393 3-axis magnetometer and temperature sensor.
> 
> The driver currently supports:
> 
> raw magnetic field measurements
> raw temperature measurements
> configurable gain/scale selection
> configurable oversampling ratio
> direct mode operation
> 
> The MLX90393 supports both I2C and SPI interfaces. This
> initial implementation adds support for the I2C interface.
> 
> The driver is structured around a shared sensor core with
> a small transport abstraction layer to simplify future SPI
> support without duplicating sensor logic.

...

> +config MLX90393
> +	tristate "MELEXIS MLX90393 3-axis magnetometer sensor"
> +	depends on I2C

Why not a regmap?

> +	help
> +	  Say yes here to build support for the MELEXIS MLX90393 3-axis
> +	  magnetometer.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mlx90393.
> +
>  config MMC35240
>  	tristate "MEMSIC MMC35240 3-axis magnetic sensor"
>  	select REGMAP_I2C
> diff --git a/drivers/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile
> index 9297723a97d8..542c89d38a59 100644
> --- a/drivers/iio/magnetometer/Makefile
> +++ b/drivers/iio/magnetometer/Makefile
> @@ -14,6 +14,8 @@ obj-$(CONFIG_BMC150_MAGN_SPI) += bmc150_magn_spi.o
>  
>  obj-$(CONFIG_MAG3110)	+= mag3110.o
>  obj-$(CONFIG_HID_SENSOR_MAGNETOMETER_3D) += hid-sensor-magn-3d.o
> +obj-$(CONFIG_MLX90393)		+= mlx90393_core.o
> +obj-$(CONFIG_MLX90393)		+= mlx90393_i2c.o
>  obj-$(CONFIG_MMC35240)	+= mmc35240.o

...

> +#ifndef MLX90393_H
> +#define MLX90393_H

> +#include <linux/bitops.h>

> +#include <linux/bits.h>

Not required, it's covered by bitops.h.

> +#include <linux/types.h>

...

> +#define MLX90393_MEASURE_ALL \
> +	(MLX90393_MEASURE_TEMP | MLX90393_MEASURE_X | \
> +	MLX90393_MEASURE_Y | MLX90393_MEASURE_Z)

Split logically.

#define MLX90393_MEASURE_ALL \
	(MLX90393_MEASURE_TEMP | \
	 MLX90393_MEASURE_X | MLX90393_MEASURE_Y | MLX90393_MEASURE_Z)

Or just a (long) single line.

...

> +	int (*xfer)(void *context, const u8 *tx, int tx_len,
> +		    u8 *rx, int rx_len);

One line, it's only 81 characters.

> +};
> +
> +int mlx90393_core_probe(struct device *dev,

You want forward declaration for struct device.

> +			const struct mlx90393_transfer_ops *ops,
> +			void *context);
> +

...

+ array_size.h
+ bitfield.h // FIELD_GET()

> +#include <linux/delay.h>

+ errno.h // -Exxx

> +#include <linux/module.h>
> +#include <linux/mutex.h>

+ types.h // uXX

> +#include <linux/unaligned.h>
> +#include <linux/units.h>

IWYU, please (just pointed out a few missing above, there are more).

...

> +/* Datasheet: Table no.17 */
> +static const int mlx90393_scale_table[MLX90393_AXIS_MAX]
> +				[MLX90393_GAIN_MAX]
> +				[MLX90393_RES_MAX] = {

This is broken indentation.

> +	/* XY axis */
> +	{
> +		{ 751, 1502, 3004, 6009},
> +		{ 601, 1202, 2403, 4840},
> +		{ 451, 901, 1803, 3605},
> +		{ 376, 751, 1502, 3004},
> +		{ 300, 601, 1202, 2403},
> +		{ 250, 501, 1001, 2003},
> +		{ 200, 401, 801, 1602},
> +		{ 150, 300, 601, 1202},
> +	},
> +	/* Z axis */
> +	{
> +		{ 1210, 2420, 4840, 9680},
> +		{ 968, 1936, 3872, 7744},
> +		{ 726, 1452, 2904, 5808},
> +		{ 605, 1210, 2420, 4840},
> +		{ 484, 968, 1936, 3872},
> +		{ 403, 807, 1613, 3227},
> +		{ 323, 645, 1291, 2581},
> +		{ 242, 484, 968, 1936},
> +	}
> +};

...

> +/*
> + * Calculate total conversion time in microseconds.
> + *
> + * Formula derived from datasheet timing equations.

Which formula? Where is datasheet? What if I have no access to it?
Always repeat the important details in the comment in the code.

> + */

> +

Unneeded blank line.

> +static int mlx90393_get_tconv_us(struct mlx90393_data *data)
> +{
> +	const int osr = data->osr;
> +	const int osr2 = data->osr2;
> +	const int df = data->dig_filt;
> +
> +	int tconvm;
> +	int tconvt;
> +
> +	int m = 3; /* X,Y,Z */
> +
> +	/*
> +	 * Datasheet:

What chapter/section/table name? Page number?

> +	 * TCONVM = 67 + 64 * 2^OSR * (2 + 2^DIG_FILT)

What does this cryptic message mean? Please, accompany this with more English
plain text.

> +	 */
> +	tconvm = 67 + (64 * BIT(osr) * (2 + BIT(df)));
> +
> +	/*
> +	 * Datasheet:
> +	 * TCONVT = 67 + 192 * 2^OSR2
> +	 */
> +	tconvt = 67 + (192 * BIT(osr2));
> +	/*
> +	 * Total conversion time:
> +	 * TSTBY + TACTIVE + m * TCONVM + TCONVT + TCONV_END
> +	 */
> +	return 220 + 360 + (m * tconvm) + tconvt + 1100;
> +}

...

> +static int mlx90393_xfer(struct mlx90393_data *data,
> +			 const u8 *tx, int tx_len,
> +			 u8 *rx, int rx_len)
> +{
> +	return data->ops->xfer(data->bus_context,
> +			tx, tx_len,
> +			rx, rx_len);

It's perfectly one line.

Also you might want to have

	if (!...->xfer)
		return -E...;

> +}

...

> +static int mlx90393_check_status(u8 cmd, u8 status)
> +{
> +	/* Always validate error bit */
> +	if (status & MLX90393_STATUS_ERROR)
> +		return -EIO;
> +
> +	switch (cmd & MLX90393_CMD_MASK) {
> +	case MLX90393_CMD_RM:
> +		/*
> +		 * D1:D0 indicates response availability
> +		 * 00 means invalid/no measurement
> +		 */
> +		if ((status & MLX90393_STATUS_RESP) == 0)
> +			return -EIO;
> +		return 0;
> +	case MLX90393_CMD_RT:
> +		/* Reset acknowledge */
> +		if (!(status & MLX90393_STATUS_RT))

For sake of consistency you might want to also compare to 0 here.

> +			return -EIO;
> +		return 0;
> +	default:
> +		return 0;
> +	}
> +}

...

> +static int mlx90393_write_reg(struct mlx90393_data *data, u8 reg, u16 val)

Here the variable is named 'reg' there is 'reg_addr'. As I can see the code is
full of inconsistencies (like 2+ people with different style guidelines wrote
it). Please. take your time and check the code and make it consistent.

> +{
> +	u8 tx[4];
> +	u8 status;
> +	int ret;
> +
> +	tx[0] = MLX90393_CMD_WR;
> +	put_unaligned_be16(val, &tx[1]);
> +	/* Register address is encoded in bits [7:2] */
> +	tx[3] = reg << 2;
> +
> +	ret = mlx90393_xfer(data, tx, sizeof(tx), &status, 1);
> +	if (ret)
> +		return ret;
> +
> +	return mlx90393_check_status(tx[0], status);
> +}
> +
> +static int mlx90393_update_bits(struct mlx90393_data *data, u8 reg_addr,
> +				u16 mask, u16 val)
> +{
> +	u16 reg;
> +	int ret;
> +
> +	ret = mlx90393_read_reg(data, reg_addr, &reg);
> +	if (ret)
> +		return ret;

> +	reg &= ~mask;
> +	reg |= (val << __ffs(mask)) & mask;

bitfield.h has macros for this.


> +	return mlx90393_write_reg(data, reg_addr, reg);
> +}

...

> +static int mlx90393_find_osr(int val, int *osr)
> +{
> +	for (unsigned int i = 0; i < MLX90393_OSR_MAX;  i++)
> +		if (mlx90393_osr_avail[i] == val) {
> +			*osr = i;
> +			return 0;
> +		}

Missing {}.

> +	return -EINVAL;
> +}

...

> +static int mlx90393_get_temp_osr2(struct mlx90393_data *data, int *val)
> +{
> +	*val = mlx90393_osr2_avail[data->osr2];

Missing blank line.

> +	return IIO_VAL_INT;
> +}

...

> +static int mlx90393_write_raw(struct iio_dev *indio_dev,
> +			      const struct iio_chan_spec *chan,
> +			      int val, int val2,
> +			      long mask)
> +{
> +	struct mlx90393_data *data = iio_priv(indio_dev);

> +	int ret;

Not needed, return directly.

> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE: {
> +		guard(mutex)(&data->lock);
> +		ret = mlx90393_set_scale(data, chan, val, val2);
> +		return ret;
> +	}
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO: {
> +		guard(mutex)(&data->lock);
> +		switch (chan->type) {
> +		case IIO_TEMP:
> +			return mlx90393_set_temp_osr2(data, val);
> +
> +		case IIO_MAGN:
> +			return mlx90393_set_osr(data, val);
> +
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +}

...

> +			/* Datasheet: 22124 millidegC/LSB */
> +		/* Datasheet: temperature offset */

Again, at least put a page, better to have Section/Table/et cetera title.

...

> +static int mlx90393_read_avail(struct iio_dev *indio_dev,
> +			       const struct iio_chan_spec *chan,
> +			       const int **vals,
> +			       int *type,
> +			       int *length,
> +			       long mask)
> +{
> +	struct mlx90393_data *data = iio_priv(indio_dev);
> +	static int scale_avail[MLX90393_GAIN_MAX][MLX90393_AXIS_MAX];
> +	enum mlx90393_axis_type axis;
> +	u8 res;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE: {
> +		guard(mutex)(&data->lock);
> +		axis = chan->channel2 == IIO_MOD_Z;
> +		res = axis ? data->res_z : data->res_xy;
> +
> +		for (unsigned int i = 0; i < MLX90393_GAIN_MAX; i++) {
> +			scale_avail[i][0] = 0;
> +			scale_avail[i][1] = mlx90393_scale_table[axis][i][res];
> +		}
> +
> +		*vals = &scale_avail[0][0];
> +		*type = IIO_VAL_INT_PLUS_NANO;
> +		*length = MLX90393_GAIN_MAX * MLX90393_AXIS_MAX;
> +		return IIO_AVAIL_LIST;
> +	}
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		if (chan->type == IIO_TEMP) {
> +			*vals = mlx90393_osr2_avail;
> +			*type = IIO_VAL_INT;
> +			*length = MLX90393_OSR2_MAX;
> +		} else {
> +			*vals = mlx90393_osr_avail;
> +			*type = IIO_VAL_INT;
> +			*length = MLX90393_OSR_MAX;
> +		}
> +		return IIO_AVAIL_LIST;
> +
> +	default:
> +		return -EINVAL;
> +	}

> +	return -EINVAL;

Besides missing blank line, this is actually a dead code.

> +}

...

> +static int mlx90393_init(struct mlx90393_data *data)
> +{
> +	int ret;
> +	u16 reg;
> +
> +	/* Exit mode */
> +	ret = mlx90393_write_cmd(data, MLX90393_CMD_EX);
> +	if (ret)
> +		return ret;
> +
> +	/* Wait for device comes out of reset */

Datasheet? Empirical?

> +	fsleep(1000);

1 * USEC_PER_MSEC
(will require time.h to be included).

> +	/* Reset device */
> +	ret = mlx90393_write_cmd(data, MLX90393_CMD_RT);
> +	if (ret)
> +		return ret;
> +
> +	/* Wait for device to reset */
> +	fsleep(6000);

As per above.

> +	ret = mlx90393_read_reg(data, MLX90393_REG_CONF1, &reg);
> +	if (ret)
> +		return ret;
> +
> +	data->gain_sel = FIELD_GET(MLX90393_CONF1_GAIN_SEL, reg);
> +	data->hallconf = FIELD_GET(MLX90393_CONF1_HALLCONF, reg);
> +
> +	ret = mlx90393_read_reg(data, MLX90393_REG_CONF3, &reg);
> +	if (ret)
> +		return ret;
> +
> +	data->res_xy = FIELD_GET(MLX90393_CONF3_RES_X, reg);
> +	data->res_z = FIELD_GET(MLX90393_CONF3_RES_Z, reg);
> +	data->dig_filt = FIELD_GET(MLX90393_CONF3_DIG_FILT, reg);
> +	data->osr = FIELD_GET(MLX90393_CONF3_OSR, reg);
> +	data->osr2 = FIELD_GET(MLX90393_CONF3_OSR2, reg);
> +
> +	return 0;
> +}

...

> +int mlx90393_core_probe(struct device *dev,
> +			const struct mlx90393_transfer_ops *ops,
> +			void *context)
> +{
> +	struct iio_dev *indio_dev;
> +	struct mlx90393_data *data;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	devm_mutex_init(dev, &data->lock);

Nonsense. If we don't check the return code of devm_*(), there is a little
reason to use it in the first place. But then one should not use devm further.
Easy fix: check for returned errors.

> +	data->dev = dev;
> +	data->ops = ops;
> +	data->bus_context = context;
> +
> +	indio_dev->name = "mlx90393";
> +	indio_dev->info = &mlx90393_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = mlx90393_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(mlx90393_channels);
> +
> +	ret = mlx90393_init(data);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to initialize device\n");
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}
> +EXPORT_SYMBOL_GPL(mlx90393_core_probe);

Make it namespaces.

...

+ array_size.h
+ errno.h

> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/mod_devicetable.h>

...and so on.

Same, follow the IWYU principle.

...

> +/*
> + * MLX90393 commands use repeated-start transfers where
> + * every command is followed by a status/data response.
> + */
> +static int mlx90393_i2c_xfer(void *context,
> +			     const u8 *tx, int tx_len,
> +			     u8 *rx, int rx_len)
> +{
> +	struct i2c_client *client = context;
> +	int ret;
> +	struct i2c_msg msgs[2] = {
> +		[0] = {
> +			.addr = client->addr,
> +			.len = tx_len,
> +			.buf = (u8 *)tx,
> +		},
> +		[1] = {
> +			.addr = client->addr,
> +			.flags = I2C_M_RD,
> +			.len = rx_len,
> +			.buf = rx,
> +		},
> +	};
> +
> +	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> +	if (ret != ARRAY_SIZE(msgs))
> +		return ret < 0 ? ret : -EIO;

Please, make this to be the regular pattern

	if (ret < 0)
		return ret;
	if (ret != ARRAY_SIZE(msgs))
		return -EIO;

> +	return 0;
> +}

...

> +static struct i2c_driver mlx90393_i2c_driver = {
> +	.driver = {
> +		.name = "mlx90393",
> +		.of_match_table = mlx90393_of_match,
> +	},
> +	.probe = mlx90393_i2c_probe,
> +};

> +

Remove this blank line.

> +module_i2c_driver(mlx90393_i2c_driver);

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH RFC v4 10/12] reset: zte: Add a zx297520v3 reset driver
From: Stefan Dösinger @ 2026-06-18 19:28 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Brian Masney, Philipp Zabel
  Cc: linux-clk, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <90c4f50eb23dec06497d46f9c0f522a6b90a918b.camel@pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 3121 bytes --]

Am Donnerstag, 18. Juni 2026, 12:24:26 Ostafrikanische Zeit schrieb Philipp 
Zabel:
> On Di, 2026-06-16 at 23:26 +0300, Stefan Dösinger wrote:
> > This drives the auxiliary devices created by the clock driver.
> 
> Which auxiliary devices? Which clock driver?

The ones from patches 7-10. But I guess you're telling me to spell this out 
for readers who see my patch in the kernel commit log rather than the 
submission series.

> > +	[ZX297520V3_UART0_RESET]     = { .reg = 0x78,  .mask = BIT(6)  | 
BIT(7) 
> > },
> Is this a single reset line controlled by two bits (do you know what
> they are)? Or might these actually be two different reset controls that
> are just always set together?

Most devices on this SoC have two reset lines. In most cases asserting one is 
enough to reset the device, and consequently both need to be deasserted to 
bring the device out of reset. In some cases both need to be asserted to reset 
the device and it keeps operating fine with only one asserted. I believe I 
documented some of that in comments, but maybe I forgot to move it from the 
clock part of the driver.

Exceptions apply - some devices have only one reset bit and for some I haven't 
found any. USB as you noticed has 3.

I don't really know the difference between the two lines. I don't have a 
datasheet and ZTE's downstream kernel only operates on the USB resets. The old 
upstream zx2967xx code had no reset driver at all. So I found the resets by 
setting the registers and then single bits to 0 and seeing what disappears 
from mmio space. Everything on this board except USB comes with resets 
deasserted on boot.

I'm pretty sure that what I identified as resets are actually resets and not 
extra clocks because previous device configuration gets lost after asserting a 
reset, whereas it is retained when disabling pclk.

I absolutely expect to run into surprises and epiphanies in the future and 
problems on as of yet untested types of zx297520v3 devices.

> > +	[ZX297520V3_USB_RESET]      =  { .reg = 0x80,   .mask = BIT(3) | 
BIT(4)
> > | BIT(5), +		.wait_mask = BIT(1)},
> 
> Same as above, are these actually three separate reset lines?

It is likely a combination of the same 2 indistinguishable lines (4, 5) and a 
separate USB PHY line (3) - this is what ZTE's code suggests, but it is a mess 
of #ifdefs and redirection, so I don't know if I isolated the correct 
codepath. (No, a kernel built from that ugly code doesn't boot, so I can't add 
printks).

The PHY reset line does not do anything observable on my device if I recall 
correctly. It might be nonexistent, it might be a leftover from older 
revisions or some devices might have different PHYs, similarly to how Ethernet 
PHYs differ.

I'll double check those USB resets before I resend. It's been months since I 
looked at this particular part of the board, and maybe I missed something. On 
the booted ZTE kernel and in the bootloader, when booted via USB, all 3 are 
deasserted. When booted via the NAND boot chain they are asserted when the 
kernel takes over.



[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

^ permalink raw reply

* Re: [PATCH v4 01/12] dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA
From: Frank Li @ 2026-06-18 19:35 UTC (permalink / raw)
  To: Akhil R
  Cc: Alexandre Belloni, Frank Li, Miquel Raynal, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Guenter Roeck, Philipp Zabel,
	Jon Hunter, Thierry Reding, linux-i3c, devicetree, linux-hwmon,
	linux-tegra, linux-kernel
In-Reply-To: <20260616095429.3947205-2-akhilrajeev@nvidia.com>

On Tue, Jun 16, 2026 at 09:54:15AM +0000, Akhil R wrote:
> Add the 'mipi-i3c-static-method' property mentioned in the MIPI I3C
> Discovery and Configuration Specification [1] to specify which discovery
> method an I3C device supports during bus initialization. The property is
> a bitmap, where a bit value of 1 indicates support for that method, and 0
> indicates lack of support.
>
> Bit 0: SETDASA CCC (Direct)
> Bit 1: SETAASA CCC (Broadcast)
> Bit 2: Other CCC (vendor / standards extension)
> All other bits are reserved.
>
> It is specifically needed when an I3C device requires SETAASA for the
> address assignment. SETDASA will be supported by default if this property
> is absent, which means for now the property just serves as a flag to
> enable SETAASA, but keep the property as a bitmap to align with the
> specifications.
>
> [1] https://www.mipi.org/mipi-disco-for-i3c-download
>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  .../devicetree/bindings/i3c/i3c.yaml          | 36 ++++++++++++++++---
>  include/dt-bindings/i3c/i3c.h                 |  4 +++
>  2 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/i3c/i3c.yaml b/Documentation/devicetree/bindings/i3c/i3c.yaml
> index e25fa72fd785..5603f2e7807d 100644
> --- a/Documentation/devicetree/bindings/i3c/i3c.yaml
> +++ b/Documentation/devicetree/bindings/i3c/i3c.yaml
> @@ -31,10 +31,12 @@ properties:
>        described in the device tree, which in turn means we have to describe
>        I3C devices.
>
> -      Another use case for describing an I3C device in the device tree is when
> -      this I3C device has a static I2C address and we want to assign it a
> -      specific I3C dynamic address before the DAA takes place (so that other
> -      devices on the bus can't take this dynamic address).
> +      Other use-cases for describing an I3C device in the device tree are:
> +      - When the I3C device has a static I2C address and we want to assign
> +        it a specific I3C dynamic address before the DAA takes place (so
> +        that other devices on the bus can't take this dynamic address).
> +      - When the I3C device requires SETAASA for its discovery and uses a
> +        pre-defined static address.
>
>    "#size-cells":
>      const: 0
> @@ -145,7 +147,31 @@ patternProperties:
>            Dynamic address to be assigned to this device. In case static address is
>            present (first cell of the reg property != 0), this address is assigned
>            through SETDASA. If static address is not present, this address is assigned
> -          through SETNEWDA after assigning a temporary address via ENTDAA.
> +          through SETNEWDA after assigning a temporary address via ENTDAA. If
> +          SETAASA is used, this property is not used, and the static address itself
> +          becomes the dynamic address.
> +
> +      mipi-i3c-static-method:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0x1
> +        maximum: 0x7
> +        default: 1
> +        description: |
> +          Bitmap describing which methods of Dynamic Address Assignment from a
> +          static address are supported by this I3C Target. For each defined bit
> +          position, a set bit indicates support for that method and a cleared
> +          bit indicates lack of support.
> +
> +            Bit 0: SETDASA CCC (Direct)
> +            Bit 1: SETAASA CCC (Broadcast)
> +            Bit 2: Other CCC (vendor / standards extension)
> +            All other bits are reserved.
> +
> +          This property follows the MIPI I3C specification. The primary use
> +          of this property is to indicate support for SETAASA, i.e Bit 1, but
> +          will allow other values mentioned in the specification so that it
> +          mirrors the specification. SETDASA will remain as the default method
> +          even if this property is not present.
>
>      required:
>        - reg
> diff --git a/include/dt-bindings/i3c/i3c.h b/include/dt-bindings/i3c/i3c.h
> index 373439218bba..78b8c634aad8 100644
> --- a/include/dt-bindings/i3c/i3c.h
> +++ b/include/dt-bindings/i3c/i3c.h
> @@ -13,4 +13,8 @@
>  #define I2C_NO_FILTER_HIGH_FREQUENCY    (1 << 5)
>  #define I2C_NO_FILTER_LOW_FREQUENCY     (2 << 5)
>
> +#define I3C_ADDR_METHOD_SETDASA     (1 << 0)
> +#define I3C_ADDR_METHOD_SETAASA     (1 << 1)
> +#define I3C_ADDR_METHOD_VENDOR      (1 << 2)
> +
>  #endif
> --
> 2.43.0
>

^ permalink raw reply

* Re: [PATCH v4 03/12] i3c: master: Support ACPI enumeration of child devices
From: Frank Li @ 2026-06-18 19:41 UTC (permalink / raw)
  To: Akhil R
  Cc: Alexandre Belloni, Frank Li, Miquel Raynal, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Guenter Roeck, Philipp Zabel,
	Jon Hunter, Thierry Reding, linux-i3c, devicetree, linux-hwmon,
	linux-tegra, linux-kernel
In-Reply-To: <20260616095429.3947205-4-akhilrajeev@nvidia.com>

On Tue, Jun 16, 2026 at 09:54:17AM +0000, Akhil R wrote:
> Although the existing subsystem allows host controllers to register
> through the ACPI table, it was not possible to describe I3C or I2C
> devices when using ACPI. This is because the driver relied on the reg
> property to retrieve the PID, static address, etc., whereas ACPI uses
> _ADR or serial resources to describe such devices.
>
> Read _ADR and LVR from ACPI resources and extract the data as per the
> ACPI specification for an I3C bus. Also read mipi-i3c-static-address as
> per the MIPI DISCO specifications [1] to get the static address to be
> used.
>
> Enable describing I3C or I2C devices in the ACPI table. This is required
> if the device uses a static address or if it needs device-specific
> properties.
>
> [1] https://www.mipi.org/mipi-disco-for-i3c-download
>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/i3c/master.c | 149 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 141 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 3b19a5e8f46d..f0e05bcac26d 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -5,6 +5,7 @@
>   * Author: Boris Brezillon <boris.brezillon@bootlin.com>
>   */
>
> +#include <linux/acpi.h>
>  #include <linux/atomic.h>
>  #include <linux/bitmap.h>
>  #include <linux/bug.h>
> @@ -2596,6 +2597,55 @@ EXPORT_SYMBOL_GPL(i3c_master_do_daa);
>
>  #define OF_I3C_REG1_IS_I2C_DEV			BIT(31)
>
> +#ifdef CONFIG_ACPI
> +static int i3c_acpi_get_i2c_resource(struct acpi_resource *ares, void *data)
> +{
> +	struct i2c_dev_boardinfo *boardinfo = data;
> +	struct acpi_resource_i2c_serialbus *sb;
> +
> +	if (boardinfo->base.addr || !i2c_acpi_get_i2c_resource(ares, &sb))
> +		return 1;
> +
> +	boardinfo->base.addr = sb->slave_address;
> +	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
> +		boardinfo->base.flags |= I2C_CLIENT_TEN;
> +
> +	boardinfo->lvr = sb->lvr;
> +
> +	return 1;
> +}
> +
> +static int i3c_acpi_add_i2c_boardinfo(struct i2c_dev_boardinfo *boardinfo,
> +				      struct fwnode_handle *fwnode)
> +{
> +	struct acpi_device *adev = to_acpi_device_node(fwnode);
> +	LIST_HEAD(resources);
> +	int ret;
> +
> +	boardinfo->base.fwnode = acpi_fwnode_handle(adev);
> +	acpi_set_modalias(adev, dev_name(&adev->dev), boardinfo->base.type,
> +			  sizeof(boardinfo->base.type));
> +
> +	ret = acpi_dev_get_resources(adev, &resources,
> +				     i3c_acpi_get_i2c_resource, boardinfo);
> +	if (ret < 0)
> +		return ret;
> +
> +	acpi_dev_free_resource_list(&resources);
> +
> +	if (!boardinfo->base.addr)
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +#else
> +static inline int i3c_acpi_add_i2c_boardinfo(struct i2c_dev_boardinfo *boardinfo,
> +					     struct fwnode_handle *fwnode)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +
>  static int
>  i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
>  			     struct fwnode_handle *fwnode, u32 *reg)
> @@ -2612,6 +2662,13 @@ i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
>  		ret = of_i2c_get_board_info(dev, to_of_node(fwnode), &boardinfo->base);
>  		if (ret)
>  			return ret;
> +
> +		/* LVR is encoded in reg[2] for Device Tree. */
> +		boardinfo->lvr = reg[2];
> +	} else if (is_acpi_device_node(fwnode)) {
> +		ret = i3c_acpi_add_i2c_boardinfo(boardinfo, fwnode);
> +		if (ret)
> +			return ret;
>  	} else {
>  		return -EINVAL;
>  	}
> @@ -2626,9 +2683,6 @@ i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
>  		return -EOPNOTSUPP;
>  	}
>
> -	/* LVR is encoded in reg[2]. */
> -	boardinfo->lvr = reg[2];
> -
>  	list_add_tail(&boardinfo->node, &master->boardinfo.i2c);
>  	fwnode_handle_get(fwnode);
>
> @@ -2683,8 +2737,8 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
>  	return 0;
>  }
>
> -static int i3c_master_add_dev(struct i3c_master_controller *master,
> -			      struct fwnode_handle *fwnode)
> +static int i3c_master_add_of_dev(struct i3c_master_controller *master,
> +				 struct fwnode_handle *fwnode)
>  {
>  	u32 reg[3];
>  	int ret;
> @@ -2708,6 +2762,74 @@ static int i3c_master_add_dev(struct i3c_master_controller *master,
>  	return ret;
>  }
>
> +#ifdef CONFIG_ACPI
> +static int i3c_master_add_acpi_dev(struct i3c_master_controller *master,
> +				   struct fwnode_handle *fwnode)
> +{
> +	struct acpi_device *adev = to_acpi_device_node(fwnode);
> +	acpi_bus_address adr;
> +	u32 reg[3] = { 0 };
> +	int ret;
> +
> +	/*
> +	 * If the ACPI table entry has _ADR method, it's an I3C device.
> +	 * Otherwise it may be an I2C device described by an I2cSerialBus
> +	 * resource. If no I2cSerialBus resource is found, ignore the entry.
> +	 */
> +	if (!acpi_has_method(adev->handle, "_ADR")) {
> +		ret = i3c_master_add_i2c_boardinfo(master, fwnode, reg);
> +		if (ret == -ENODEV)
> +			return 0;
> +
> +		return ret;
> +	}
> +
> +	adr = acpi_device_adr(adev);
> +
> +	/* For I3C devices, _ADR will have the 48 bit PID of the device  */
> +	reg[1] = upper_32_bits(adr);
> +	reg[2] = lower_32_bits(adr);
> +
> +	fwnode_property_read_u32(fwnode, "mipi-i3c-static-address", &reg[0]);
> +
> +	return i3c_master_add_i3c_boardinfo(master, fwnode, reg);
> +}
> +
> +static u8 i3c_acpi_i2c_get_lvr(struct i2c_client *client)
> +{
> +	struct acpi_device *adev = to_acpi_device_node(client->dev.fwnode);
> +	struct i2c_dev_boardinfo boardinfo = {};
> +	LIST_HEAD(resources);
> +	int ret;
> +	u8 lvr;
> +
> +	lvr = I3C_LVR_I2C_INDEX(2) | I3C_LVR_I2C_FM_MODE;
> +
> +	ret = acpi_dev_get_resources(adev, &resources,
> +				     i3c_acpi_get_i2c_resource, &boardinfo);
> +	if (ret < 0)
> +		return lvr;
> +
> +	if (boardinfo.base.addr)
> +		lvr = boardinfo.lvr;
> +
> +	acpi_dev_free_resource_list(&resources);
> +
> +	return lvr;
> +}
> +#else
> +static inline int i3c_master_add_acpi_dev(struct i3c_master_controller *master,
> +					  struct fwnode_handle *fwnode)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline u8 i3c_acpi_i2c_get_lvr(struct i2c_client *client)
> +{
> +	return I3C_LVR_I2C_INDEX(2) | I3C_LVR_I2C_FM_MODE;
> +}
> +#endif
> +
>  static int fwnode_populate_i3c_bus(struct i3c_master_controller *master)
>  {
>  	struct device *dev = &master->dev;
> @@ -2719,7 +2841,13 @@ static int fwnode_populate_i3c_bus(struct i3c_master_controller *master)
>  		return 0;
>
>  	fwnode_for_each_available_child_node_scoped(fwnode, child) {
> -		ret = i3c_master_add_dev(master, child);
> +		if (is_of_node(child))
> +			ret = i3c_master_add_of_dev(master, child);
> +		else if (is_acpi_device_node(child))
> +			ret = i3c_master_add_acpi_dev(master, child);
> +		else
> +			continue;
> +
>  		if (ret)
>  			return ret;
>  	}
> @@ -2787,8 +2915,13 @@ static u8 i3c_master_i2c_get_lvr(struct i2c_client *client)
>  	u8 lvr = I3C_LVR_I2C_INDEX(2) | I3C_LVR_I2C_FM_MODE;
>  	u32 reg[3];
>
> -	if (!fwnode_property_read_u32_array(client->dev.fwnode, "reg", reg, ARRAY_SIZE(reg)))
> -		lvr = reg[2];
> +	if (is_of_node(client->dev.fwnode)) {
> +		if (!fwnode_property_read_u32_array(client->dev.fwnode, "reg",
> +						    reg, ARRAY_SIZE(reg)))
> +			lvr = reg[2];
> +	} else if (is_acpi_device_node(client->dev.fwnode)) {
> +		lvr = i3c_acpi_i2c_get_lvr(client);
> +	}
>
>  	return lvr;
>  }
> --
> 2.43.0
>

^ permalink raw reply

* Re: [PATCH v4 07/12] i3c: dw-i3c-master: Add SETAASA as supported CCC
From: Frank Li @ 2026-06-18 19:43 UTC (permalink / raw)
  To: Akhil R
  Cc: Alexandre Belloni, Frank Li, Miquel Raynal, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Guenter Roeck, Philipp Zabel,
	Jon Hunter, Thierry Reding, linux-i3c, devicetree, linux-hwmon,
	linux-tegra, linux-kernel
In-Reply-To: <20260616095429.3947205-8-akhilrajeev@nvidia.com>

On Tue, Jun 16, 2026 at 09:54:21AM +0000, Akhil R wrote:
> Add SETAASA and SETHID to the supported list of CCC commands for
> DesignWare I3C host controller.
>
> SETAASA is a broadcast command that assigns predefined static addresses
> to all I3C devices on the bus.
>
> SETHID is to stop HID bit flipping by the SPD Hub to which the SPD devices
> are connected. It is a prerequisite command to be sent before SETAASA as
> recommended by JESD300-5 and JESD403 sideband bus specifications.
>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/i3c/master/dw-i3c-master.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 971b429b76bc..3e510fddf06c 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -309,6 +309,8 @@ static bool dw_i3c_master_supports_ccc_cmd(struct i3c_master_controller *m,
>  	case I3C_CCC_GETSTATUS:
>  	case I3C_CCC_GETMXDS:
>  	case I3C_CCC_GETHDRCAP:
> +	case I3C_CCC_SETAASA:
> +	case I3C_CCC_VENDOR(0, true): /* SETHID */
>  		return true;
>  	default:
>  		return false;
> --
> 2.43.0
>

^ permalink raw reply

* Re: [PATCH v4 03/12] i3c: master: Support ACPI enumeration of child devices
From: Frank Li @ 2026-06-18 19:53 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Akhil R, linux-hwmon, conor+dt, linux-i3c, Frank.Li, devicetree,
	robh
In-Reply-To: <20260616101515.34CA61F000E9@smtp.kernel.org>

On Tue, Jun 16, 2026 at 10:15:14AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] devm_kzalloc allocated 'boardinfo' is leaked for every ACPI child node that is ignored because it lacks I2C/I3C resources.
> --
>
> commit 9a388c2056a553d628ce4b8b623a812d4aa08951
> Author: Akhil R <akhilrajeev@nvidia.com>
>
> i3c: master: Support ACPI enumeration of child devices
>
> This commit adds support for describing I3C and I2C devices via ACPI tables,
> rather than relying solely on the device tree reg property. It implements
> reading _ADR and LVR properties from ACPI resources to retrieve the static
> address and other device properties.
>
> > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > index 3b19a5e8f46d0..f0e05bcac26da 100644
> > --- a/drivers/i3c/master.c
> > +++ b/drivers/i3c/master.c
>
> [ ... ]
>
> > @@ -2708,6 +2762,74 @@ static int i3c_master_add_dev(struct i3c_master_controller *master,
> >  	return ret;
> >  }
> >
> > +#ifdef CONFIG_ACPI
> > +static int i3c_master_add_acpi_dev(struct i3c_master_controller *master,
> > +				   struct fwnode_handle *fwnode)
> > +{
> > +	struct acpi_device *adev = to_acpi_device_node(fwnode);
> > +	acpi_bus_address adr;
> > +	u32 reg[3] = { 0 };
> > +	int ret;
> > +
> > +	/*
> > +	 * If the ACPI table entry has _ADR method, it's an I3C device.
> > +	 * Otherwise it may be an I2C device described by an I2cSerialBus
> > +	 * resource. If no I2cSerialBus resource is found, ignore the entry.
> > +	 */
> > +	if (!acpi_has_method(adev->handle, "_ADR")) {
> > +		ret = i3c_master_add_i2c_boardinfo(master, fwnode, reg);
> > +		if (ret == -ENODEV)
> > +			return 0;
>
> [Severity: Medium]
> Does this leak the boardinfo allocated in i3c_master_add_i2c_boardinfo()?
>
> If an ACPI child lacks an _ADR method, i3c_master_add_i2c_boardinfo()
> unconditionally allocates boardinfo using devm_kzalloc().
>
> If the node also lacks an I2cSerialBus resource, i3c_acpi_add_i2c_boardinfo()
> returns -ENODEV. This causes i3c_master_add_i2c_boardinfo() to return -ENODEV
> without freeing the allocated boardinfo.
>
> Because we catch -ENODEV here and return 0, the overall bus probe continues
> and ultimately succeeds. This means the devres-allocated memory stays tied
> to the master device, leaking sizeof(struct i2c_dev_boardinfo) for every
> ignored ACPI child node.

Is it true?

Frank

>
> > +
> > +		return ret;
> > +	}
> > +
> > +	adr = acpi_device_adr(adev);
>
> [ ... ]
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260616095429.3947205-1-akhilrajeev@nvidia.com?part=3

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox