[PATCH RFT/RFC] hwmon: (pmbus) Do not set regulators_node for single-channel chips

[PATCH RFT/RFC] hwmon: (pmbus) Do not set regulators_node for single-channel chips

[Guenter Roeck]

Single-channel regulators do not need and should not have a "regulators"
node. We can not entirely remove it due to existing bindings. To solve the
problem for new drivers, provide additional macros PMBUS_REGULATOR_ONE_NODE
and PMBUS_REGULATOR_STEP_ONE_NODE and convert existing drivers to use those
macros. The exception is the ir38064 driver because its devicetree files
and its description do not require or use the nested regulators node.

Modify PMBUS_REGULATOR_STEP_ONE and PMBUS_REGULATOR_ONE to set the
regulators_node pointer to NULL.

Cc: Cedricjustine.Encarnacion@analog.com
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
RFT: Untested. Open question is if not setting .regulators_node is
     sufficient to solve the problem.
RFC: Open question is if existing bindings (if unused in-tree) should
     be converted to not require the nested "regulators" node.

 drivers/hwmon/pmbus/lm25066.c  |  2 +-
 drivers/hwmon/pmbus/mpq7932.c  |  4 ++--
 drivers/hwmon/pmbus/pmbus.h    | 18 +++++++++++++++---
 drivers/hwmon/pmbus/tda38640.c |  2 +-
 drivers/hwmon/pmbus/tps25990.c |  2 +-
 5 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/pmbus/lm25066.c b/drivers/hwmon/pmbus/lm25066.c
index 40b0dda32ea6..dd7275a67a0a 100644
--- a/drivers/hwmon/pmbus/lm25066.c
+++ b/drivers/hwmon/pmbus/lm25066.c
@@ -437,7 +437,7 @@ static int lm25066_write_word_data(struct i2c_client *client, int page, int reg,
 
 #if IS_ENABLED(CONFIG_SENSORS_LM25066_REGULATOR)
 static const struct regulator_desc lm25066_reg_desc[] = {
-	PMBUS_REGULATOR_ONE("vout"),
+	PMBUS_REGULATOR_ONE_NODE("vout"),
 };
 #endif
 
diff --git a/drivers/hwmon/pmbus/mpq7932.c b/drivers/hwmon/pmbus/mpq7932.c
index c1e2d0cb2fd0..8f10e37a7a76 100644
--- a/drivers/hwmon/pmbus/mpq7932.c
+++ b/drivers/hwmon/pmbus/mpq7932.c
@@ -51,8 +51,8 @@ static const struct regulator_desc mpq7932_regulators_desc[] = {
 };
 
 static const struct regulator_desc mpq7932_regulators_desc_one[] = {
-	PMBUS_REGULATOR_STEP_ONE("buck", MPQ7932_N_VOLTAGES,
-				 MPQ7932_UV_STEP, MPQ7932_BUCK_UV_MIN),
+	PMBUS_REGULATOR_STEP_ONE_NODE("buck", MPQ7932_N_VOLTAGES,
+				      MPQ7932_UV_STEP, MPQ7932_BUCK_UV_MIN),
 };
 #endif
 
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index ddb19c9726d6..ccc38e367195 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -508,11 +508,11 @@ int pmbus_regulator_init_cb(struct regulator_dev *rdev,
 
 #define PMBUS_REGULATOR(_name, _id)   PMBUS_REGULATOR_STEP(_name, _id, 0, 0, 0)
 
-#define PMBUS_REGULATOR_STEP_ONE(_name, _voltages, _step, _min_uV)  \
+#define __PMBUS_REGULATOR_STEP_ONE(_name, _node, _voltages, _step, _min_uV)  \
 	{							\
 		.name = (_name),				\
 		.of_match = of_match_ptr(_name),		\
-		.regulators_node = of_match_ptr("regulators"),	\
+		.regulators_node = of_match_ptr(_node),		\
 		.ops = &pmbus_regulator_ops,			\
 		.type = REGULATOR_VOLTAGE,			\
 		.owner = THIS_MODULE,				\
@@ -522,7 +522,19 @@ int pmbus_regulator_init_cb(struct regulator_dev *rdev,
 		.init_cb = pmbus_regulator_init_cb,		\
 	}
 
-#define PMBUS_REGULATOR_ONE(_name)   PMBUS_REGULATOR_STEP_ONE(_name, 0, 0, 0)
+/*
+ * _NODE macros are defined for historic reasons and MUST NOT be used in new
+ * drivers.
+ */
+#define PMBUS_REGULATOR_STEP_ONE_NODE(_name, _voltages, _step, _min_uV)  \
+	__PMBUS_REGULATOR_STEP_ONE(_name, "regulators", _voltages, _step, _min_uV)
+
+#define PMBUS_REGULATOR_ONE_NODE(_name)	PMBUS_REGULATOR_STEP_ONE_NODE(_name, 0, 0, 0)
+
+#define PMBUS_REGULATOR_STEP_ONE(_name, _voltages, _step, _min_uV)  \
+	__PMBUS_REGULATOR_STEP_ONE(_name, NULL, _voltages, _step, _min_uV)
+
+#define PMBUS_REGULATOR_ONE(_name)	PMBUS_REGULATOR_STEP_ONE(_name, 0, 0, 0)
 
 /* Function declarations */
 
diff --git a/drivers/hwmon/pmbus/tda38640.c b/drivers/hwmon/pmbus/tda38640.c
index 07fe58c24485..d902d39f49f4 100644
--- a/drivers/hwmon/pmbus/tda38640.c
+++ b/drivers/hwmon/pmbus/tda38640.c
@@ -15,7 +15,7 @@
 #include "pmbus.h"
 
 static const struct regulator_desc __maybe_unused tda38640_reg_desc[] = {
-	PMBUS_REGULATOR_ONE("vout"),
+	PMBUS_REGULATOR_ONE_NODE("vout"),
 };
 
 struct tda38640_data {
diff --git a/drivers/hwmon/pmbus/tps25990.c b/drivers/hwmon/pmbus/tps25990.c
index 0d2655e69549..c13edd7e1abf 100644
--- a/drivers/hwmon/pmbus/tps25990.c
+++ b/drivers/hwmon/pmbus/tps25990.c
@@ -333,7 +333,7 @@ static int tps25990_write_byte_data(struct i2c_client *client,
 
 #if IS_ENABLED(CONFIG_SENSORS_TPS25990_REGULATOR)
 static const struct regulator_desc tps25990_reg_desc[] = {
-	PMBUS_REGULATOR_ONE("vout"),
+	PMBUS_REGULATOR_ONE_NODE("vout"),
 };
 #endif
 
-- 
2.45.2

Re: [PATCH RFT/RFC] hwmon: (pmbus) Do not set regulators_node for single-channel chips

[Krzysztof Kozlowski]

On 22/03/2025 15:26, Guenter Roeck wrote:
> Single-channel regulators do not need and should not have a "regulators"
> node. We can not entirely remove it due to existing bindings. To solve the
> problem for new drivers, provide additional macros PMBUS_REGULATOR_ONE_NODE
> and PMBUS_REGULATOR_STEP_ONE_NODE and convert existing drivers to use those
> macros. The exception is the ir38064 driver because its devicetree files
> and its description do not require or use the nested regulators node.
> 
> Modify PMBUS_REGULATOR_STEP_ONE and PMBUS_REGULATOR_ONE to set the
> regulators_node pointer to NULL.
> 
> Cc: Cedricjustine.Encarnacion@analog.com
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> RFT: Untested. Open question is if not setting .regulators_node is
>      sufficient to solve the problem.
> RFC: Open question is if existing bindings (if unused in-tree) should
>      be converted to not require the nested "regulators" node.

Looks correct.


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH RFT/RFC] hwmon: (pmbus) Do not set regulators_node for single-channel chips

[Guenter Roeck]

On Sat, Mar 22, 2025 at 07:26:02AM -0700, Guenter Roeck wrote:
> Single-channel regulators do not need and should not have a "regulators"
> node. We can not entirely remove it due to existing bindings. To solve the
> problem for new drivers, provide additional macros PMBUS_REGULATOR_ONE_NODE
> and PMBUS_REGULATOR_STEP_ONE_NODE and convert existing drivers to use those
> macros. The exception is the ir38064 driver because its devicetree files
> and its description do not require or use the nested regulators node.
> 
> Modify PMBUS_REGULATOR_STEP_ONE and PMBUS_REGULATOR_ONE to set the
> regulators_node pointer to NULL.
> 
> Cc: Cedricjustine.Encarnacion@analog.com
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> RFT: Untested. Open question is if not setting .regulators_node is
>      sufficient to solve the problem.
> RFC: Open question is if existing bindings (if unused in-tree) should
>      be converted to not require the nested "regulators" node.

I did not get any test feedback. Given that we are at -rc1, I decided to apply
the patch. Let's see if there is any fall-out.

Guenter