@@ -18,6 +18,7 @@
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#include <linux/debugfs.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>
@@ -230,7 +231,28 @@ static int pmbus_probe(struct i2c_client *client,
.id_table = pmbus_id,
};
-module_i2c_driver(pmbus_driver);
+struct dentry *pmbus_debugfs_dir;
+EXPORT_SYMBOL_GPL(pmbus_debugfs_dir);
+
+static int __init pmbus_init(void)
+{
+ pmbus_debugfs_dir =
debugfs_create_dir(pmbus_driver.driver.name, NULL);
+ if (IS_ERR(pmbus_debugfs_dir))
+ /* Don't fail out if we don't have debugfs support. */
+ pmbus_debugfs_dir = NULL;
+
+ return i2c_add_driver(&pmbus_driver);
+}
+
+static void __exit pmbus_exit(void)
+{
+ debugfs_remove_recursive(pmbus_debugfs_dir);
+
+ i2c_del_driver(&pmbus_driver);
+}
+
+module_init(pmbus_init);
+module_exit(pmbus_exit);
MODULE_AUTHOR("Guenter Roeck");
MODULE_DESCRIPTION("Generic PMBus driver");
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index bfcb13b..c772b83 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -25,6 +25,8 @@
#include <linux/bitops.h>
#include <linux/regulator/driver.h>
+struct dentry;
+
/*
* Registers
*/
@@ -383,6 +385,12 @@ struct pmbus_driver_info {
/* Regulator functionality, if supported by this chip driver. */
int num_regulators;
const struct regulator_desc *reg_desc;
+
+ /*
+ * Controls whether or not to create debugfs entries for this
driver's
+ * devices.
+ */
+ bool debugfs;
};
/* Regulator ops */
@@ -401,6 +409,9 @@ struct pmbus_driver_info {
.owner = THIS_MODULE, \
}
+/* Handle for debugfs directory */
+extern struct dentry *pmbus_debugfs_dir;
+
/* Function declarations */
void pmbus_clear_cache(struct i2c_client *client);
diff --git a/drivers/hwmon/pmbus/pmbus_core.c
b/drivers/hwmon/pmbus/pmbus_core.c
index 1a51f8f..afbd364 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -19,6 +19,7 @@
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#include <linux/debugfs.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>
@@ -49,6 +50,9 @@
#define PB_STATUS_FAN34_BASE (PB_STATUS_FAN_BASE + PMBUS_PAGES)
#define PB_STATUS_TEMP_BASE (PB_STATUS_FAN34_BASE + PMBUS_PAGES)
#define PB_STATUS_INPUT_BASE (PB_STATUS_TEMP_BASE + PMBUS_PAGES)
+#define PB_STATUS_CML_BASE (PB_STATUS_INPUT_BASE + PMBUS_PAGES)
+#define PB_STATUS_OTHER_BASE (PB_STATUS_CML_BASE + PMBUS_PAGES)
+#define PB_STATUS_MFR_BASE (PB_STATUS_OTHER_BASE + PMBUS_PAGES)
We are not actively using those status registers, are we ?
I am a bit concerned that we'll continuously read those status registers
but don't do anything with it most of the time. Ultimately I'd prefer
to get rid of all caching, not more, since it gets quite expensive
on chips with many pages.
Maybe just display uncached status register values in debugfs ?
#define PB_STATUS_VMON_BASE (PB_STATUS_INPUT_BASE + 1)
#define PB_NUM_STATUS_REG (PB_STATUS_VMON_BASE + 1)
@@ -101,6 +105,7 @@ struct pmbus_data {
int num_attributes;
struct attribute_group group;
const struct attribute_group *groups[2];
+ struct dentry *debugfs; /* debugfs device directory */
struct pmbus_sensor *sensors;
@@ -119,6 +124,11 @@ struct pmbus_data {
u8 currpage;
};
+struct pmbus_debugfs_entry {
+ struct device *dev;
+ int index;
+};
+
void pmbus_clear_cache(struct i2c_client *client)
{
struct pmbus_data *data = i2c_get_clientdata(client);
@@ -422,6 +432,19 @@ static struct pmbus_data
*pmbus_update_device(struct device *dev)
= _pmbus_read_byte_data(client, i,
s->reg);
}
+
+ if (!data->debugfs)
+ continue;
+
+ data->status[PB_STATUS_CML_BASE + i]
+ = _pmbus_read_byte_data(client, i,
+ PMBUS_STATUS_CML);
+ data->status[PB_STATUS_OTHER_BASE + i]
+ = _pmbus_read_byte_data(client, i,
+ PMBUS_STATUS_OTHER);
+ data->status[PB_STATUS_MFR_BASE + i]
+ = _pmbus_read_byte_data(client, i,
+ PMBUS_STATUS_MFR_SPECIFIC);
}
if (info->func[0] & PMBUS_HAVE_STATUS_INPUT)
@@ -1891,6 +1914,151 @@ static int pmbus_regulator_register(struct
pmbus_data *data)
}
#endif
+static int pmbus_debugfs_get(void *data, u64 *val)
+{
+ struct pmbus_debugfs_entry *entry = data;
+ struct pmbus_data *pdata = pmbus_update_device(entry->dev);
+
+ *val = pdata->status[entry->index];
+
+ return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(pmbus_debugfs_ops8, pmbus_debugfs_get, NULL,
+ "0x%02llx\n");
+DEFINE_DEBUGFS_ATTRIBUTE(pmbus_debugfs_ops16, pmbus_debugfs_get, NULL,
+ "0x%04llx\n");
+
+static int pmbus_init_debugfs(struct i2c_client *client,
+ struct pmbus_data *data)
+{
+ int i, idx = 0;
+ struct dentry *dbg;
+ char name[PMBUS_NAME_SIZE];
+ struct pmbus_debugfs_entry *entries;
+
+ /*
+ * Either user didn't request debugfs or debugfs is not enabled in
+ * kernel. Exit but don't throw an error in these cases.
+ */
Here might be the place to initialize pmbus_debugfs_dir
if it is not yet initialized.
+ if (!data->info->debugfs || !pmbus_debugfs_dir)
+ return 0;
+
+ /*
+ * 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;
+ return -ENODEV;
+ }
+
+ /* Allocate the max possible entries we need. */
+ entries = devm_kzalloc(data->dev,
+ sizeof(*entries) * (data->info->pages * 10),
+ GFP_KERNEL);
+ if (!entries)
+ return -ENOMEM;
+
+ for (i = 0; i < data->info->pages; ++i) {
+ /* check accessibility of status register if it's not page
0 */
+ if (!i || pmbus_check_status_register(client, i)) {
+ entries[idx].dev = data->hwmon_dev;
+ entries[idx].index = PB_STATUS_BASE + i;
+ snprintf(name, PMBUS_NAME_SIZE, "status%d", i);
+ dbg = debugfs_create_file(name, 0444, data->debugfs,
+ &entries[idx++],
+ &pmbus_debugfs_ops16);
What is the point of the dbg variable ?
+ }
+
+ if (data->info->func[i] & PMBUS_HAVE_STATUS_VOUT) {
+ entries[idx].dev = data->hwmon_dev;
+ entries[idx].index = PB_STATUS_VOUT_BASE + i;
+ snprintf(name, PMBUS_NAME_SIZE, "status%d_vout", i);
+ dbg = debugfs_create_file(name, 0444, data->debugfs,
+ &entries[idx++],
+ &pmbus_debugfs_ops8);
+ }
+
+ if (data->info->func[i] & PMBUS_HAVE_STATUS_IOUT) {
+ entries[idx].dev = data->hwmon_dev;
+ entries[idx].index = PB_STATUS_IOUT_BASE + i;
+ snprintf(name, PMBUS_NAME_SIZE, "status%d_iout", i);
+ dbg = debugfs_create_file(name, 0444, data->debugfs,
+ &entries[idx++],
+ &pmbus_debugfs_ops8);
+ }
+
+ if (data->info->func[i] & PMBUS_HAVE_STATUS_INPUT) {
+ entries[idx].dev = data->hwmon_dev;
+ entries[idx].index = PB_STATUS_INPUT_BASE + i;
+ snprintf(name, PMBUS_NAME_SIZE, "status%d_input", i);
+ dbg = debugfs_create_file(name, 0444, data->debugfs,
+ &entries[idx++],
+ &pmbus_debugfs_ops8);
+ }
+
+ if (data->info->func[i] & PMBUS_HAVE_STATUS_TEMP) {
+ entries[idx].dev = data->hwmon_dev;
+ entries[idx].index = PB_STATUS_TEMP_BASE + i;
+ snprintf(name, PMBUS_NAME_SIZE, "status%d_temp", i);
+ dbg = debugfs_create_file(name, 0444, data->debugfs,
+ &entries[idx++],
+ &pmbus_debugfs_ops8);
+ }
+
+ if (pmbus_check_byte_register(client, i, PMBUS_STATUS_CML)) {
+ entries[idx].dev = data->hwmon_dev;
+ entries[idx].index = PB_STATUS_CML_BASE + i;
+ snprintf(name, PMBUS_NAME_SIZE, "status%d_cml", i);
+ dbg = debugfs_create_file(name, 0444, data->debugfs,
+ &entries[idx++],
+ &pmbus_debugfs_ops8);
+ }
+
+ if (pmbus_check_byte_register(client, i,
PMBUS_STATUS_OTHER)) {
+ entries[idx].dev = data->hwmon_dev;
+ entries[idx].index = PB_STATUS_OTHER_BASE + i;
+ snprintf(name, PMBUS_NAME_SIZE, "status%d_other", i);
+ dbg = debugfs_create_file(name, 0444, data->debugfs,
+ &entries[idx++],
+ &pmbus_debugfs_ops8);
+ }
+
+ if (pmbus_check_byte_register(client, i,
+ PMBUS_STATUS_MFR_SPECIFIC)) {
+ entries[idx].dev = data->hwmon_dev;
+ entries[idx].index = PB_STATUS_MFR_BASE + i;
+ snprintf(name, PMBUS_NAME_SIZE, "status%d_mfr", i);
+ dbg = debugfs_create_file(name, 0444, data->debugfs,
+ &entries[idx++],
+ &pmbus_debugfs_ops8);
+ }
+
+ if (data->info->func[i] & PMBUS_HAVE_STATUS_FAN12) {
+ entries[idx].dev = data->hwmon_dev;
+ entries[idx].index = PB_STATUS_FAN_BASE + i;
+ snprintf(name, PMBUS_NAME_SIZE, "status%d_fan12", i);
+ dbg = debugfs_create_file(name, 0444, data->debugfs,
+ &entries[idx++],
+ &pmbus_debugfs_ops8);
+ }
+
+ if (data->info->func[i] & PMBUS_HAVE_STATUS_FAN34) {
+ entries[idx].dev = data->hwmon_dev;
+ entries[idx].index = PB_STATUS_FAN34_BASE + i;
+ snprintf(name, PMBUS_NAME_SIZE, "status%d_fan34", i);
+ dbg = debugfs_create_file(name, 0444, data->debugfs,
+ &entries[idx++],
+ &pmbus_debugfs_ops8);
+ }
+ }
+
+ return 0;
+}
+
Typically debugfs code is conditional. What happens if DEBUGFS
is not enabled ? See below.
int pmbus_do_probe(struct i2c_client *client, const struct
i2c_device_id *id,
struct pmbus_driver_info *info)
{
@@ -1950,6 +2118,10 @@ int pmbus_do_probe(struct i2c_client *client,
const struct i2c_device_id *id,
if (ret)
goto out_unregister;
+ ret = pmbus_init_debugfs(client, data);
+ if (ret)
+ dev_warn(dev, "Failed to register debugfs\n");
I think this warning will be generated if debugfs is disabled.
We should not do that. Consider putting the debugfs code into
#fidef and have a dummy pmbus_init_debugfs() returning 0 if
it is disabled.