[PATCH 0/8] hwmon: (pmbus/core) Various fixes and improvements

[PATCH 0/8] hwmon: (pmbus/core) Various fixes and improvements

[Guenter Roeck]

Improve debugfs code by improving its scalability and making it
unconditional. Use the new i2c_client debugfs directory to reduce
local code. To retain backward compatibility, link the old pmbus
debugfs directory to the i2c debugfs directory.

Report the CAPABILITY register content in debugfs if it is available.

Declare regulator notification function as void since its return value
is not used.

Last but not least, fix various checkpatch issues.

----------------------------------------------------------------
Guenter Roeck (8):
      hwmon: (pmbus/core) Fix various coding style issues
      hwmon: (pmbus/core) Use local debugfs variable in debugfs initialization
      hwmon: (pmbus/core) Use the new i2c_client debugfs directory
      hwmon: (pmbus/core) Make debugfs code unconditional
      hwmon: (pmbus/core) Declare regulator notification function as void
      hwmon: (pmbus/core) Optimize debugfs block data attribute initialization
      hwmon: (pmbus/core) Optimize debugfs status attribute initialization
      hwmon: (pmbus/core) Report content of CAPABILITY register in debugfs

 drivers/hwmon/pmbus/pmbus_core.c | 363 +++++++++++++++------------------------
 1 file changed, 142 insertions(+), 221 deletions(-)

[PATCH 1/8] hwmon: (pmbus/core) Fix various coding style issues

[Guenter Roeck]

Checkpatch reports bad multi-line comments, bad multi-line alignments,
missing blank lines after variable declarations, unnecessary empty lines,
unnecessary spaces, and unnecessary braces. Fix most of the reported
problems except for some multi-line alignment problems.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/pmbus/pmbus_core.c | 41 +++++++++++++-------------------
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 787683e83db6..e38d69d723c0 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -44,8 +44,7 @@ struct pmbus_sensor {
 	enum pmbus_sensor_classes class;	/* sensor class */
 	bool update;		/* runtime sensor update needed */
 	bool convert;		/* Whether or not to apply linear/vid/direct */
-	int data;		/* Sensor data.
-				   Negative if there was a read error */
+	int data;		/* Sensor data; negative if there was a read error */
 };
 #define to_pmbus_sensor(_attr) \
 	container_of(_attr, struct pmbus_sensor, attribute)
@@ -192,11 +191,10 @@ static void pmbus_update_ts(struct i2c_client *client, bool write_op)
 	struct pmbus_data *data = i2c_get_clientdata(client);
 	const struct pmbus_driver_info *info = data->info;
 
-	if (info->access_delay) {
+	if (info->access_delay)
 		data->access_time = ktime_get();
-	} else if (info->write_delay && write_op) {
+	else if (info->write_delay && write_op)
 		data->write_time = ktime_get();
-	}
 }
 
 int pmbus_set_page(struct i2c_client *client, int page, int phase)
@@ -292,7 +290,6 @@ int pmbus_write_word_data(struct i2c_client *client, int page, u8 reg,
 }
 EXPORT_SYMBOL_NS_GPL(pmbus_write_word_data, "PMBUS");
 
-
 static int pmbus_write_virt_reg(struct i2c_client *client, int page, int reg,
 				u16 word)
 {
@@ -381,14 +378,14 @@ int pmbus_update_fan(struct i2c_client *client, int page, int id,
 	u8 to;
 
 	from = _pmbus_read_byte_data(client, page,
-				    pmbus_fan_config_registers[id]);
+				     pmbus_fan_config_registers[id]);
 	if (from < 0)
 		return from;
 
 	to = (from & ~mask) | (config & mask);
 	if (to != from) {
 		rv = _pmbus_write_byte_data(client, page,
-					   pmbus_fan_config_registers[id], to);
+					    pmbus_fan_config_registers[id], to);
 		if (rv < 0)
 			return rv;
 	}
@@ -563,7 +560,7 @@ static int pmbus_get_fan_rate(struct i2c_client *client, int page, int id,
 	}
 
 	config = _pmbus_read_byte_data(client, page,
-				      pmbus_fan_config_registers[id]);
+				       pmbus_fan_config_registers[id]);
 	if (config < 0)
 		return config;
 
@@ -788,7 +785,7 @@ static s64 pmbus_reg2data_linear(struct pmbus_data *data,
 
 	if (sensor->class == PSC_VOLTAGE_OUT) {	/* LINEAR16 */
 		exponent = data->exponent[sensor->page];
-		mantissa = (u16) sensor->data;
+		mantissa = (u16)sensor->data;
 	} else {				/* LINEAR11 */
 		exponent = ((s16)sensor->data) >> 11;
 		mantissa = ((s16)((sensor->data & 0x7ff) << 5)) >> 5;
@@ -1173,7 +1170,6 @@ static int pmbus_get_boolean(struct i2c_client *client, struct pmbus_boolean *b,
 		} else {
 			pmbus_clear_fault_page(client, page);
 		}
-
 	}
 	if (s1 && s2) {
 		s64 v1, v2;
@@ -1500,8 +1496,7 @@ struct pmbus_limit_attr {
 	u16 reg;		/* Limit register */
 	u16 sbit;		/* Alarm attribute status bit */
 	bool update;		/* True if register needs updates */
-	bool low;		/* True if low limit; for limits with compare
-				   functions only */
+	bool low;		/* True if low limit; for limits with compare functions only */
 	const char *attr;	/* Attribute name */
 	const char *alarm;	/* Alarm attribute name */
 };
@@ -2212,8 +2207,8 @@ static const u32 pmbus_fan_status_flags[] = {
 
 /* Precondition: FAN_CONFIG_x_y and FAN_COMMAND_x must exist for the fan ID */
 static int pmbus_add_fan_ctrl(struct i2c_client *client,
-		struct pmbus_data *data, int index, int page, int id,
-		u8 config)
+			      struct pmbus_data *data, int index, int page,
+			      int id, u8 config)
 {
 	struct pmbus_sensor *sensor;
 
@@ -2225,7 +2220,7 @@ static int pmbus_add_fan_ctrl(struct i2c_client *client,
 		return -ENOMEM;
 
 	if (!((data->info->func[page] & PMBUS_HAVE_PWM12) ||
-			(data->info->func[page] & PMBUS_HAVE_PWM34)))
+	      (data->info->func[page] & PMBUS_HAVE_PWM34)))
 		return 0;
 
 	sensor = pmbus_add_sensor(data, "pwm", NULL, index, page,
@@ -2935,7 +2930,7 @@ static void pmbus_notify(struct pmbus_data *data, int page, int reg, int flags)
 }
 
 static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags,
-			   unsigned int *event, bool notify)
+			    unsigned int *event, bool notify)
 {
 	int i, status;
 	const struct pmbus_status_category *cat;
@@ -2964,7 +2959,6 @@ static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flag
 
 		if (notify && status)
 			pmbus_notify(data, page, cat->reg, status);
-
 	}
 
 	/*
@@ -3015,7 +3009,6 @@ static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flag
 		*event |= REGULATOR_EVENT_OVER_TEMP_WARN;
 	}
 
-
 	return 0;
 }
 
@@ -3228,7 +3221,7 @@ static int pmbus_regulator_set_voltage(struct regulator_dev *rdev, int min_uv,
 }
 
 static int pmbus_regulator_list_voltage(struct regulator_dev *rdev,
-					 unsigned int selector)
+					unsigned int selector)
 {
 	struct device *dev = rdev_get_dev(rdev);
 	struct i2c_client *client = to_i2c_client(dev->parent);
@@ -3363,8 +3356,8 @@ static irqreturn_t pmbus_fault_handler(int irq, void *pdata)
 {
 	struct pmbus_data *data = pdata;
 	struct i2c_client *client = to_i2c_client(data->dev);
-
 	int i, status, event;
+
 	mutex_lock(&data->update_lock);
 	for (i = 0; i < data->info->pages; i++) {
 		_pmbus_get_flags(data, i, &status, &event, true);
@@ -3472,7 +3465,7 @@ DEFINE_DEBUGFS_ATTRIBUTE(pmbus_debugfs_ops_status, pmbus_debugfs_get_status,
 			 NULL, "0x%04llx\n");
 
 static ssize_t pmbus_debugfs_mfr_read(struct file *file, char __user *buf,
-				       size_t count, loff_t *ppos)
+				      size_t count, loff_t *ppos)
 {
 	int rc;
 	struct pmbus_debugfs_entry *entry = file->private_data;
@@ -3800,8 +3793,8 @@ int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info *info)
 
 	data->groups[0] = &data->group;
 	memcpy(data->groups + 1, info->groups, sizeof(void *) * groups_num);
-	data->hwmon_dev = devm_hwmon_device_register_with_groups(dev,
-					name, data, data->groups);
+	data->hwmon_dev = devm_hwmon_device_register_with_groups(dev, name,
+								 data, data->groups);
 	if (IS_ERR(data->hwmon_dev)) {
 		dev_err(dev, "Failed to register hwmon device\n");
 		return PTR_ERR(data->hwmon_dev);
-- 
2.45.2

Re: [PATCH 1/8] hwmon: (pmbus/core) Fix various coding style issues

[Tzung-Bi Shih]

On Sat, Feb 08, 2025 at 05:26:10PM -0800, Guenter Roeck wrote:
> Checkpatch reports bad multi-line comments, bad multi-line alignments,
> missing blank lines after variable declarations, unnecessary empty lines,
> unnecessary spaces, and unnecessary braces. Fix most of the reported
> problems except for some multi-line alignment problems.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

[PATCH 2/8] hwmon: (pmbus/core) Use local debugfs variable in debugfs initialization

[Guenter Roeck]

In preparation for the next patch in the series, use a local debugfs
variable during debugfs initialization.

No functional change intended.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/pmbus/pmbus_core.c | 49 ++++++++++++++++----------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index e38d69d723c0..472375b62379 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -3507,6 +3507,7 @@ static void pmbus_remove_debugfs(void *data)
 static int pmbus_init_debugfs(struct i2c_client *client,
 			      struct pmbus_data *data)
 {
+	struct dentry *debugfs;
 	int i, idx = 0;
 	char name[PMBUS_NAME_SIZE];
 	struct pmbus_debugfs_entry *entries;
@@ -3518,12 +3519,12 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 	 * Create the debugfs directory for this device. Use the hwmon device
 	 * name to avoid conflicts (hwmon numbers are globally unique).
 	 */
-	data->debugfs = debugfs_create_dir(dev_name(data->hwmon_dev),
-					   pmbus_debugfs_dir);
-	if (IS_ERR_OR_NULL(data->debugfs)) {
-		data->debugfs = NULL;
+	debugfs = debugfs_create_dir(dev_name(data->hwmon_dev),
+				     pmbus_debugfs_dir);
+	if (IS_ERR_OR_NULL(debugfs))
 		return -ENODEV;
-	}
+
+	data->debugfs = debugfs;
 
 	/*
 	 * Allocate the max possible entries we need.
@@ -3548,7 +3549,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 		entries[idx].client = client;
 		entries[idx].page = 0;
 		entries[idx].reg = PMBUS_REVISION;
-		debugfs_create_file("revision", 0444, data->debugfs,
+		debugfs_create_file("revision", 0444, debugfs,
 				    &entries[idx++],
 				    &pmbus_debugfs_ops);
 	}
@@ -3557,7 +3558,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 		entries[idx].client = client;
 		entries[idx].page = 0;
 		entries[idx].reg = PMBUS_MFR_ID;
-		debugfs_create_file("mfr_id", 0444, data->debugfs,
+		debugfs_create_file("mfr_id", 0444, debugfs,
 				    &entries[idx++],
 				    &pmbus_debugfs_ops_mfr);
 	}
@@ -3566,7 +3567,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 		entries[idx].client = client;
 		entries[idx].page = 0;
 		entries[idx].reg = PMBUS_MFR_MODEL;
-		debugfs_create_file("mfr_model", 0444, data->debugfs,
+		debugfs_create_file("mfr_model", 0444, debugfs,
 				    &entries[idx++],
 				    &pmbus_debugfs_ops_mfr);
 	}
@@ -3575,7 +3576,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 		entries[idx].client = client;
 		entries[idx].page = 0;
 		entries[idx].reg = PMBUS_MFR_REVISION;
-		debugfs_create_file("mfr_revision", 0444, data->debugfs,
+		debugfs_create_file("mfr_revision", 0444, debugfs,
 				    &entries[idx++],
 				    &pmbus_debugfs_ops_mfr);
 	}
@@ -3584,7 +3585,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 		entries[idx].client = client;
 		entries[idx].page = 0;
 		entries[idx].reg = PMBUS_MFR_LOCATION;
-		debugfs_create_file("mfr_location", 0444, data->debugfs,
+		debugfs_create_file("mfr_location", 0444, debugfs,
 				    &entries[idx++],
 				    &pmbus_debugfs_ops_mfr);
 	}
@@ -3593,7 +3594,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 		entries[idx].client = client;
 		entries[idx].page = 0;
 		entries[idx].reg = PMBUS_MFR_DATE;
-		debugfs_create_file("mfr_date", 0444, data->debugfs,
+		debugfs_create_file("mfr_date", 0444, debugfs,
 				    &entries[idx++],
 				    &pmbus_debugfs_ops_mfr);
 	}
@@ -3602,7 +3603,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 		entries[idx].client = client;
 		entries[idx].page = 0;
 		entries[idx].reg = PMBUS_MFR_SERIAL;
-		debugfs_create_file("mfr_serial", 0444, data->debugfs,
+		debugfs_create_file("mfr_serial", 0444, debugfs,
 				    &entries[idx++],
 				    &pmbus_debugfs_ops_mfr);
 	}
@@ -3615,7 +3616,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 			entries[idx].client = client;
 			entries[idx].page = i;
 			scnprintf(name, PMBUS_NAME_SIZE, "status%d", i);
-			debugfs_create_file(name, 0444, data->debugfs,
+			debugfs_create_file(name, 0444, debugfs,
 					    &entries[idx++],
 					    &pmbus_debugfs_ops_status);
 		}
@@ -3625,7 +3626,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 			entries[idx].page = i;
 			entries[idx].reg = PMBUS_STATUS_VOUT;
 			scnprintf(name, PMBUS_NAME_SIZE, "status%d_vout", i);
-			debugfs_create_file(name, 0444, data->debugfs,
+			debugfs_create_file(name, 0444, debugfs,
 					    &entries[idx++],
 					    &pmbus_debugfs_ops);
 		}
@@ -3635,7 +3636,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 			entries[idx].page = i;
 			entries[idx].reg = PMBUS_STATUS_IOUT;
 			scnprintf(name, PMBUS_NAME_SIZE, "status%d_iout", i);
-			debugfs_create_file(name, 0444, data->debugfs,
+			debugfs_create_file(name, 0444, debugfs,
 					    &entries[idx++],
 					    &pmbus_debugfs_ops);
 		}
@@ -3645,7 +3646,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 			entries[idx].page = i;
 			entries[idx].reg = PMBUS_STATUS_INPUT;
 			scnprintf(name, PMBUS_NAME_SIZE, "status%d_input", i);
-			debugfs_create_file(name, 0444, data->debugfs,
+			debugfs_create_file(name, 0444, debugfs,
 					    &entries[idx++],
 					    &pmbus_debugfs_ops);
 		}
@@ -3655,7 +3656,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 			entries[idx].page = i;
 			entries[idx].reg = PMBUS_STATUS_TEMPERATURE;
 			scnprintf(name, PMBUS_NAME_SIZE, "status%d_temp", i);
-			debugfs_create_file(name, 0444, data->debugfs,
+			debugfs_create_file(name, 0444, debugfs,
 					    &entries[idx++],
 					    &pmbus_debugfs_ops);
 		}
@@ -3665,7 +3666,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 			entries[idx].page = i;
 			entries[idx].reg = PMBUS_STATUS_CML;
 			scnprintf(name, PMBUS_NAME_SIZE, "status%d_cml", i);
-			debugfs_create_file(name, 0444, data->debugfs,
+			debugfs_create_file(name, 0444, debugfs,
 					    &entries[idx++],
 					    &pmbus_debugfs_ops);
 		}
@@ -3675,7 +3676,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 			entries[idx].page = i;
 			entries[idx].reg = PMBUS_STATUS_OTHER;
 			scnprintf(name, PMBUS_NAME_SIZE, "status%d_other", i);
-			debugfs_create_file(name, 0444, data->debugfs,
+			debugfs_create_file(name, 0444, debugfs,
 					    &entries[idx++],
 					    &pmbus_debugfs_ops);
 		}
@@ -3686,7 +3687,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 			entries[idx].page = i;
 			entries[idx].reg = PMBUS_STATUS_MFR_SPECIFIC;
 			scnprintf(name, PMBUS_NAME_SIZE, "status%d_mfr", i);
-			debugfs_create_file(name, 0444, data->debugfs,
+			debugfs_create_file(name, 0444, debugfs,
 					    &entries[idx++],
 					    &pmbus_debugfs_ops);
 		}
@@ -3696,7 +3697,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 			entries[idx].page = i;
 			entries[idx].reg = PMBUS_STATUS_FAN_12;
 			scnprintf(name, PMBUS_NAME_SIZE, "status%d_fan12", i);
-			debugfs_create_file(name, 0444, data->debugfs,
+			debugfs_create_file(name, 0444, debugfs,
 					    &entries[idx++],
 					    &pmbus_debugfs_ops);
 		}
@@ -3706,14 +3707,14 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 			entries[idx].page = i;
 			entries[idx].reg = PMBUS_STATUS_FAN_34;
 			scnprintf(name, PMBUS_NAME_SIZE, "status%d_fan34", i);
-			debugfs_create_file(name, 0444, data->debugfs,
+			debugfs_create_file(name, 0444, debugfs,
 					    &entries[idx++],
 					    &pmbus_debugfs_ops);
 		}
 	}
 
-	return devm_add_action_or_reset(data->dev,
-					pmbus_remove_debugfs, data->debugfs);
+	return devm_add_action_or_reset(data->dev, pmbus_remove_debugfs,
+					debugfs);
 }
 #else
 static int pmbus_init_debugfs(struct i2c_client *client,
-- 
2.45.2

Re: [PATCH 2/8] hwmon: (pmbus/core) Use local debugfs variable in debugfs initialization

[Tzung-Bi Shih]

On Sat, Feb 08, 2025 at 05:26:11PM -0800, Guenter Roeck wrote:
> In preparation for the next patch in the series, use a local debugfs
> variable during debugfs initialization.
> 
> No functional change intended.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

[PATCH 3/8] hwmon: (pmbus/core) Use the new i2c_client debugfs directory

[Guenter Roeck]

The I2C core now manages a debugfs directory per I2C client. PMBus has
its own debugfs hierarchy. Link the two, so a user will be pointed to
the I2C domain from the PMBus domain.

Suggested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/pmbus/pmbus_core.c | 63 +++++++++++++++++++++-----------
 1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 472375b62379..be794ba993b8 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -8,6 +8,7 @@
 
 #include <linux/debugfs.h>
 #include <linux/delay.h>
+#include <linux/dcache.h>
 #include <linux/kernel.h>
 #include <linux/math64.h>
 #include <linux/module.h>
@@ -99,7 +100,6 @@ struct pmbus_data {
 	int num_attributes;
 	struct attribute_group group;
 	const struct attribute_group **groups;
-	struct dentry *debugfs;		/* debugfs device directory */
 
 	struct pmbus_sensor *sensors;
 
@@ -3497,34 +3497,49 @@ static const struct file_operations pmbus_debugfs_ops_mfr = {
 	.open = simple_open,
 };
 
-static void pmbus_remove_debugfs(void *data)
+static void pmbus_remove_symlink(void *symlink)
 {
-	struct dentry *entry = data;
-
-	debugfs_remove_recursive(entry);
+	debugfs_remove(symlink);
 }
 
 static int pmbus_init_debugfs(struct i2c_client *client,
 			      struct pmbus_data *data)
 {
-	struct dentry *debugfs;
-	int i, idx = 0;
-	char name[PMBUS_NAME_SIZE];
+	struct dentry *symlink_d, *debugfs = client->debugfs;
 	struct pmbus_debugfs_entry *entries;
+	const char *pathname, *symlink;
+	char name[PMBUS_NAME_SIZE];
+	int i, idx = 0;
 
-	if (!pmbus_debugfs_dir)
+	/*
+	 * client->debugfs may be NULL or an ERR_PTR(). dentry_path_raw()
+	 * does not check if its parameters are valid, so validate
+	 * client->debugfs before using it.
+	 */
+	if (!pmbus_debugfs_dir || IS_ERR_OR_NULL(debugfs))
 		return -ENODEV;
 
 	/*
-	 * Create the debugfs directory for this device. Use the hwmon device
-	 * name to avoid conflicts (hwmon numbers are globally unique).
+	 * Backwards compatibility: Create symlink from /pmbus/<hwmon_device>
+	 * to i2c debugfs directory.
 	 */
-	debugfs = debugfs_create_dir(dev_name(data->hwmon_dev),
-				     pmbus_debugfs_dir);
-	if (IS_ERR_OR_NULL(debugfs))
-		return -ENODEV;
+	pathname = dentry_path_raw(debugfs, name, sizeof(name));
+	if (IS_ERR(pathname))
+		return PTR_ERR(pathname);
 
-	data->debugfs = debugfs;
+	/*
+	 * The path returned by dentry_path_raw() starts with '/'. Prepend it
+	 * with ".." to get the symlink relative to the pmbus root directory.
+	 */
+	symlink = kasprintf(GFP_KERNEL, "..%s", pathname);
+	if (!symlink)
+		return -ENOMEM;
+
+	symlink_d = debugfs_create_symlink(dev_name(data->hwmon_dev),
+					   pmbus_debugfs_dir, symlink);
+	kfree(symlink);
+
+	devm_add_action_or_reset(data->dev, pmbus_remove_symlink, symlink_d);
 
 	/*
 	 * Allocate the max possible entries we need.
@@ -3712,9 +3727,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 					    &pmbus_debugfs_ops);
 		}
 	}
-
-	return devm_add_action_or_reset(data->dev, pmbus_remove_debugfs,
-					debugfs);
+	return 0;
 }
 #else
 static int pmbus_init_debugfs(struct i2c_client *client,
@@ -3819,9 +3832,15 @@ EXPORT_SYMBOL_NS_GPL(pmbus_do_probe, "PMBUS");
 
 struct dentry *pmbus_get_debugfs_dir(struct i2c_client *client)
 {
-	struct pmbus_data *data = i2c_get_clientdata(client);
-
-	return data->debugfs;
+	/*
+	 * client->debugfs may be an ERR_PTR(). Returning that to
+	 * the calling code would potentially require additional
+	 * complexity in the calling code and otherwise add no
+	 * value. Return NULL in that case.
+	 */
+	if (IS_ERR_OR_NULL(client->debugfs))
+		return NULL;
+	return client->debugfs;
 }
 EXPORT_SYMBOL_NS_GPL(pmbus_get_debugfs_dir, "PMBUS");
 
-- 
2.45.2

Re: [PATCH 3/8] hwmon: (pmbus/core) Use the new i2c_client debugfs directory

[Tzung-Bi Shih]

On Sat, Feb 08, 2025 at 05:26:12PM -0800, Guenter Roeck wrote:
> The I2C core now manages a debugfs directory per I2C client. PMBus has
> its own debugfs hierarchy. Link the two, so a user will be pointed to
> the I2C domain from the PMBus domain.
> 
> Suggested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

[PATCH 4/8] hwmon: (pmbus/core) Make debugfs code unconditional

[Guenter Roeck]

Drop contitionals around debugfs code to compile it unconditionally.
In practice it will be optimized away by the compiler if CONFIG_DEBUG_FS
is not enabled, so the code size is not affected by this change.

Also silently ignore errors if debugfs initialization fails.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/pmbus/pmbus_core.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index be794ba993b8..39cdcbb96215 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -3421,7 +3421,6 @@ static int pmbus_irq_setup(struct i2c_client *client, struct pmbus_data *data)
 
 static struct dentry *pmbus_debugfs_dir;	/* pmbus debugfs directory */
 
-#if IS_ENABLED(CONFIG_DEBUG_FS)
 static int pmbus_debugfs_get(void *data, u64 *val)
 {
 	int rc;
@@ -3502,8 +3501,8 @@ static void pmbus_remove_symlink(void *symlink)
 	debugfs_remove(symlink);
 }
 
-static int pmbus_init_debugfs(struct i2c_client *client,
-			      struct pmbus_data *data)
+static void pmbus_init_debugfs(struct i2c_client *client,
+			       struct pmbus_data *data)
 {
 	struct dentry *symlink_d, *debugfs = client->debugfs;
 	struct pmbus_debugfs_entry *entries;
@@ -3517,7 +3516,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 	 * client->debugfs before using it.
 	 */
 	if (!pmbus_debugfs_dir || IS_ERR_OR_NULL(debugfs))
-		return -ENODEV;
+		return;
 
 	/*
 	 * Backwards compatibility: Create symlink from /pmbus/<hwmon_device>
@@ -3525,7 +3524,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 	 */
 	pathname = dentry_path_raw(debugfs, name, sizeof(name));
 	if (IS_ERR(pathname))
-		return PTR_ERR(pathname);
+		return;
 
 	/*
 	 * The path returned by dentry_path_raw() starts with '/'. Prepend it
@@ -3533,7 +3532,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 	 */
 	symlink = kasprintf(GFP_KERNEL, "..%s", pathname);
 	if (!symlink)
-		return -ENOMEM;
+		return;
 
 	symlink_d = debugfs_create_symlink(dev_name(data->hwmon_dev),
 					   pmbus_debugfs_dir, symlink);
@@ -3550,7 +3549,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 			       7 + data->info->pages * 10, sizeof(*entries),
 			       GFP_KERNEL);
 	if (!entries)
-		return -ENOMEM;
+		return;
 
 	/*
 	 * Add device-specific entries.
@@ -3727,15 +3726,7 @@ static int pmbus_init_debugfs(struct i2c_client *client,
 					    &pmbus_debugfs_ops);
 		}
 	}
-	return 0;
 }
-#else
-static int pmbus_init_debugfs(struct i2c_client *client,
-			      struct pmbus_data *data)
-{
-	return 0;
-}
-#endif	/* IS_ENABLED(CONFIG_DEBUG_FS) */
 
 int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info *info)
 {
@@ -3822,9 +3813,7 @@ int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info *info)
 	if (ret)
 		return ret;
 
-	ret = pmbus_init_debugfs(client, data);
-	if (ret)
-		dev_warn(dev, "Failed to register debugfs\n");
+	pmbus_init_debugfs(client, data);
 
 	return 0;
 }
-- 
2.45.2

Re: [PATCH 4/8] hwmon: (pmbus/core) Make debugfs code unconditional

[Tzung-Bi Shih]

On Sat, Feb 08, 2025 at 05:26:13PM -0800, Guenter Roeck wrote:
> Drop contitionals around debugfs code to compile it unconditionally.
> In practice it will be optimized away by the compiler if CONFIG_DEBUG_FS
> is not enabled, so the code size is not affected by this change.
> 
> Also silently ignore errors if debugfs initialization fails.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

[PATCH 5/8] hwmon: (pmbus/core) Declare regulator notification function as void

[Guenter Roeck]

The regulator notification function never returns an error.
Declare it as void.

While at it, fix its indentation.

No functional change.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/pmbus/pmbus_core.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 39cdcbb96215..3085afc9c1ed 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -3313,17 +3313,16 @@ static int pmbus_regulator_register(struct pmbus_data *data)
 	return 0;
 }
 
-static int pmbus_regulator_notify(struct pmbus_data *data, int page, int event)
+static void pmbus_regulator_notify(struct pmbus_data *data, int page, int event)
 {
-		int j;
+	int j;
 
-		for (j = 0; j < data->info->num_regulators; j++) {
-			if (page == rdev_get_id(data->rdevs[j])) {
-				regulator_notifier_call_chain(data->rdevs[j], event, NULL);
-				break;
-			}
+	for (j = 0; j < data->info->num_regulators; j++) {
+		if (page == rdev_get_id(data->rdevs[j])) {
+			regulator_notifier_call_chain(data->rdevs[j], event, NULL);
+			break;
 		}
-		return 0;
+	}
 }
 #else
 static int pmbus_regulator_register(struct pmbus_data *data)
@@ -3331,9 +3330,8 @@ static int pmbus_regulator_register(struct pmbus_data *data)
 	return 0;
 }
 
-static int pmbus_regulator_notify(struct pmbus_data *data, int page, int event)
+static void pmbus_regulator_notify(struct pmbus_data *data, int page, int event)
 {
-		return 0;
 }
 #endif
 
-- 
2.45.2

Re: [PATCH 5/8] hwmon: (pmbus/core) Declare regulator notification function as void

[Tzung-Bi Shih]

On Sat, Feb 08, 2025 at 05:26:14PM -0800, Guenter Roeck wrote:
> The regulator notification function never returns an error.
> Declare it as void.
> 
> While at it, fix its indentation.
> 
> No functional change.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

[PATCH 6/8] hwmon: (pmbus/core) Optimize debugfs block data attribute initialization

[Guenter Roeck]

Define debugfs attributes which need block data access in a data
structure and loop through it instead of creating debugfs files
one by one. This reduces code size and simplifies adding additional
attributes if needed.

While this is currently only used for manufacturer specific attributes,
the access code is generic and also works for other block attributes,
so rename operation functions from _mfg to _block.

While at it, rename the "revison" file to "pmbus_revision" to make its
meaning more obvious and to create a clear distinction against the
"mfg_revision" file.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/pmbus/pmbus_core.c | 85 +++++++++++---------------------
 1 file changed, 29 insertions(+), 56 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 3085afc9c1ed..91dfb9ec9223 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -3461,8 +3461,8 @@ static int pmbus_debugfs_get_status(void *data, u64 *val)
 DEFINE_DEBUGFS_ATTRIBUTE(pmbus_debugfs_ops_status, pmbus_debugfs_get_status,
 			 NULL, "0x%04llx\n");
 
-static ssize_t pmbus_debugfs_mfr_read(struct file *file, char __user *buf,
-				      size_t count, loff_t *ppos)
+static ssize_t pmbus_debugfs_block_read(struct file *file, char __user *buf,
+					size_t count, loff_t *ppos)
 {
 	int rc;
 	struct pmbus_debugfs_entry *entry = file->private_data;
@@ -3487,9 +3487,9 @@ static ssize_t pmbus_debugfs_mfr_read(struct file *file, char __user *buf,
 	return simple_read_from_buffer(buf, count, ppos, data, rc);
 }
 
-static const struct file_operations pmbus_debugfs_ops_mfr = {
+static const struct file_operations pmbus_debugfs_block_ops = {
 	.llseek = noop_llseek,
-	.read = pmbus_debugfs_mfr_read,
+	.read = pmbus_debugfs_block_read,
 	.write = NULL,
 	.open = simple_open,
 };
@@ -3499,6 +3499,20 @@ static void pmbus_remove_symlink(void *symlink)
 	debugfs_remove(symlink);
 }
 
+struct pmbus_debugfs_data {
+	u8 reg;
+	const char *name;
+};
+
+static const struct pmbus_debugfs_data pmbus_debugfs_block_data[] = {
+	{ .reg = PMBUS_MFR_ID, .name = "mfr_id" },
+	{ .reg = PMBUS_MFR_MODEL, .name = "mfr_model" },
+	{ .reg = PMBUS_MFR_REVISION, .name = "mfr_revision" },
+	{ .reg = PMBUS_MFR_LOCATION, .name = "mfr_location" },
+	{ .reg = PMBUS_MFR_DATE, .name = "mfr_date" },
+	{ .reg = PMBUS_MFR_SERIAL, .name = "mfr_serial" },
+};
+
 static void pmbus_init_debugfs(struct i2c_client *client,
 			       struct pmbus_data *data)
 {
@@ -3561,63 +3575,22 @@ static void pmbus_init_debugfs(struct i2c_client *client,
 		entries[idx].client = client;
 		entries[idx].page = 0;
 		entries[idx].reg = PMBUS_REVISION;
-		debugfs_create_file("revision", 0444, debugfs,
+		debugfs_create_file("pmbus_revision", 0444, debugfs,
 				    &entries[idx++],
 				    &pmbus_debugfs_ops);
 	}
 
-	if (pmbus_check_block_register(client, 0, PMBUS_MFR_ID)) {
-		entries[idx].client = client;
-		entries[idx].page = 0;
-		entries[idx].reg = PMBUS_MFR_ID;
-		debugfs_create_file("mfr_id", 0444, debugfs,
-				    &entries[idx++],
-				    &pmbus_debugfs_ops_mfr);
-	}
+	for (i = 0; i < ARRAY_SIZE(pmbus_debugfs_block_data); i++) {
+		const struct pmbus_debugfs_data *d = &pmbus_debugfs_block_data[i];
 
-	if (pmbus_check_block_register(client, 0, PMBUS_MFR_MODEL)) {
-		entries[idx].client = client;
-		entries[idx].page = 0;
-		entries[idx].reg = PMBUS_MFR_MODEL;
-		debugfs_create_file("mfr_model", 0444, debugfs,
-				    &entries[idx++],
-				    &pmbus_debugfs_ops_mfr);
-	}
-
-	if (pmbus_check_block_register(client, 0, PMBUS_MFR_REVISION)) {
-		entries[idx].client = client;
-		entries[idx].page = 0;
-		entries[idx].reg = PMBUS_MFR_REVISION;
-		debugfs_create_file("mfr_revision", 0444, debugfs,
-				    &entries[idx++],
-				    &pmbus_debugfs_ops_mfr);
-	}
-
-	if (pmbus_check_block_register(client, 0, PMBUS_MFR_LOCATION)) {
-		entries[idx].client = client;
-		entries[idx].page = 0;
-		entries[idx].reg = PMBUS_MFR_LOCATION;
-		debugfs_create_file("mfr_location", 0444, debugfs,
-				    &entries[idx++],
-				    &pmbus_debugfs_ops_mfr);
-	}
-
-	if (pmbus_check_block_register(client, 0, PMBUS_MFR_DATE)) {
-		entries[idx].client = client;
-		entries[idx].page = 0;
-		entries[idx].reg = PMBUS_MFR_DATE;
-		debugfs_create_file("mfr_date", 0444, debugfs,
-				    &entries[idx++],
-				    &pmbus_debugfs_ops_mfr);
-	}
-
-	if (pmbus_check_block_register(client, 0, PMBUS_MFR_SERIAL)) {
-		entries[idx].client = client;
-		entries[idx].page = 0;
-		entries[idx].reg = PMBUS_MFR_SERIAL;
-		debugfs_create_file("mfr_serial", 0444, debugfs,
-				    &entries[idx++],
-				    &pmbus_debugfs_ops_mfr);
+		if (pmbus_check_block_register(client, 0, d->reg)) {
+			entries[idx].client = client;
+			entries[idx].page = 0;
+			entries[idx].reg = d->reg;
+			debugfs_create_file(d->name, 0444, debugfs,
+					    &entries[idx++],
+					    &pmbus_debugfs_block_ops);
+		}
 	}
 
 	/* Add page specific entries */
-- 
2.45.2

Re: [PATCH 6/8] hwmon: (pmbus/core) Optimize debugfs block data attribute initialization

[Tzung-Bi Shih]

On Sat, Feb 08, 2025 at 05:26:15PM -0800, Guenter Roeck wrote:
> Define debugfs attributes which need block data access in a data
> structure and loop through it instead of creating debugfs files
> one by one. This reduces code size and simplifies adding additional
> attributes if needed.
> 
> While this is currently only used for manufacturer specific attributes,
> the access code is generic and also works for other block attributes,
> so rename operation functions from _mfg to _block.
> 
> While at it, rename the "revison" file to "pmbus_revision" to make its
> meaning more obvious and to create a clear distinction against the
> "mfg_revision" file.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Re: [PATCH 0/8] hwmon: (pmbus/core) Various fixes and improvements

[Tzung-Bi Shih]

On Sat, Feb 08, 2025 at 05:26:09PM -0800, Guenter Roeck wrote:
>       hwmon: (pmbus/core) Optimize debugfs status attribute initialization
>       hwmon: (pmbus/core) Report content of CAPABILITY register in debugfs

It looks like the last 2 patches aren't in the mailing list.

Re: [PATCH 0/8] hwmon: (pmbus/core) Various fixes and improvements

[Guenter Roeck]

On 2/9/25 18:21, Tzung-Bi Shih wrote:
> On Sat, Feb 08, 2025 at 05:26:09PM -0800, Guenter Roeck wrote:
>>        hwmon: (pmbus/core) Optimize debugfs status attribute initialization
>>        hwmon: (pmbus/core) Report content of CAPABILITY register in debugfs
> 
> It looks like the last 2 patches aren't in the mailing list.


Yes, I noticed. Looks like gmail decided that I sent too many e-mails today
and blocked the last two :-(. I just sent them separately.

Guenter