Re: [PATCH 1/3] pmbus: support for custom sysfs attributes
On 4/11/19 12:53 AM, Adamski, Krzysztof (Nokia - PL/Wroclaw) wrote:
On Wed, Apr 10, 2019 at 05:35:21PM -0700, Guenter Roeck wrote:
On 4/10/19 3:38 PM, Adamski, Krzysztof (Nokia - PL/Wroclaw) wrote:
This patch makes it possible to pass custom struct attribute_group array
via the pmbus_driver_info struct so that those can be added to the
attribute groups passed to hwmon_device_register_with_groups().
This makes it possible to register custom sysfs attributes by PMBUS
drivers similar to how you can do this with most other busses/classes.
Signed-off-by: Krzysztof Adamski
---
drivers/hwmon/pmbus/pmbus.h | 3 +++
drivers/hwmon/pmbus/pmbus_core.c | 13 -
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 1d24397d36ec..fb267ec11623 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -417,6 +417,9 @@ struct pmbus_driver_info {
/* Regulator functionality, if supported by this chip driver. */
int num_regulators;
const struct regulator_desc *reg_desc;
+
+ /* custom attributes */
+ const struct attribute_group **groups;
I can understand the need and desire for one additional group. More than one
is highly questionable. Please explain why you think that more than one extra
attribute would ever be needed. It does add substantial complexity, so
there should be a good reason.
The only situation I could come up is if the driver would want to group
attributes in different directories by setting different name for each
of them. One other reason I choose to use this approach is that this
seems to be standard way for passing this information on other
layers/frameworks. For example, this is the same "format" you would
pass this kind of data when creating a class, a bus, a driver or when
you use any of the *_register_with_groups().
This approach is simply more generic with (to my opinion) low cost of
implementation. But if we don't want to support that, I'm fine to change
this to single custom group.
I think we can postpone this discussion for now, as we'll go with standard
attributes to be implemented in the pmbus core.
Thanks,
Guenter
Re: [PATCH 1/3] pmbus: support for custom sysfs attributes
On Wed, Apr 10, 2019 at 05:35:21PM -0700, Guenter Roeck wrote:
>On 4/10/19 3:38 PM, Adamski, Krzysztof (Nokia - PL/Wroclaw) wrote:
>>This patch makes it possible to pass custom struct attribute_group array
>>via the pmbus_driver_info struct so that those can be added to the
>>attribute groups passed to hwmon_device_register_with_groups().
>>
>>This makes it possible to register custom sysfs attributes by PMBUS
>>drivers similar to how you can do this with most other busses/classes.
>>
>>Signed-off-by: Krzysztof Adamski
>>---
>> drivers/hwmon/pmbus/pmbus.h | 3 +++
>> drivers/hwmon/pmbus/pmbus_core.c | 13 -
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>>diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
>>index 1d24397d36ec..fb267ec11623 100644
>>--- a/drivers/hwmon/pmbus/pmbus.h
>>+++ b/drivers/hwmon/pmbus/pmbus.h
>>@@ -417,6 +417,9 @@ struct pmbus_driver_info {
>> /* Regulator functionality, if supported by this chip driver. */
>> int num_regulators;
>> const struct regulator_desc *reg_desc;
>>+
>>+ /* custom attributes */
>>+ const struct attribute_group **groups;
>
>I can understand the need and desire for one additional group. More than one
>is highly questionable. Please explain why you think that more than one extra
>attribute would ever be needed. It does add substantial complexity, so
>there should be a good reason.
The only situation I could come up is if the driver would want to group
attributes in different directories by setting different name for each
of them. One other reason I choose to use this approach is that this
seems to be standard way for passing this information on other
layers/frameworks. For example, this is the same "format" you would
pass this kind of data when creating a class, a bus, a driver or when
you use any of the *_register_with_groups().
This approach is simply more generic with (to my opinion) low cost of
implementation. But if we don't want to support that, I'm fine to change
this to single custom group.
Krzysztof
Re: [PATCH 1/3] pmbus: support for custom sysfs attributes
On 4/10/19 3:38 PM, Adamski, Krzysztof (Nokia - PL/Wroclaw) wrote:
This patch makes it possible to pass custom struct attribute_group array
via the pmbus_driver_info struct so that those can be added to the
attribute groups passed to hwmon_device_register_with_groups().
This makes it possible to register custom sysfs attributes by PMBUS
drivers similar to how you can do this with most other busses/classes.
Signed-off-by: Krzysztof Adamski
---
drivers/hwmon/pmbus/pmbus.h | 3 +++
drivers/hwmon/pmbus/pmbus_core.c | 13 -
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 1d24397d36ec..fb267ec11623 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -417,6 +417,9 @@ struct pmbus_driver_info {
/* Regulator functionality, if supported by this chip driver. */
int num_regulators;
const struct regulator_desc *reg_desc;
+
+ /* custom attributes */
+ const struct attribute_group **groups;
I can understand the need and desire for one additional group. More than one
is highly questionable. Please explain why you think that more than one extra
attribute would ever be needed. It does add substantial complexity, so
there should be a good reason.
Thanks,
Guenter
};
/* Regulator ops */
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 2e2b5851139c..7a7dcdc8acc9 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -103,7 +103,7 @@ struct pmbus_data {
int max_attributes;
int num_attributes;
struct attribute_group group;
- const struct attribute_group *groups[2];
+ const struct attribute_group **groups;
struct dentry *debugfs; /* debugfs device directory */
struct pmbus_sensor *sensors;
@@ -2305,6 +2305,7 @@ int pmbus_do_probe(struct i2c_client *client, const
struct i2c_device_id *id,
struct device *dev = >dev;
const struct pmbus_platform_data *pdata = dev_get_platdata(dev);
struct pmbus_data *data;
+ size_t groups_num = 0;
int ret;
if (!info)
@@ -2319,6 +2320,15 @@ int pmbus_do_probe(struct i2c_client *client, const
struct i2c_device_id *id,
if (!data)
return -ENOMEM;
+ if (info->groups)
+ while (info->groups[groups_num])
+ groups_num++;
+
+ data->groups = devm_kcalloc(dev, groups_num + 2, sizeof(void *),
+ GFP_KERNEL);
+ if (!data->groups)
+ return -ENOMEM;
+
i2c_set_clientdata(client, data);
mutex_init(>update_lock);
data->dev = dev;
@@ -2346,6 +2356,7 @@ int pmbus_do_probe(struct i2c_client *client, const
struct i2c_device_id *id,
}
data->groups[0] = >group;
+ memcpy(data->groups + 1, info->groups, sizeof(void *) * groups_num);
data->hwmon_dev = hwmon_device_register_with_groups(dev, client->name,
data, data->groups);
if (IS_ERR(data->hwmon_dev)) {
[PATCH 1/3] pmbus: support for custom sysfs attributes
This patch makes it possible to pass custom struct attribute_group array
via the pmbus_driver_info struct so that those can be added to the
attribute groups passed to hwmon_device_register_with_groups().
This makes it possible to register custom sysfs attributes by PMBUS
drivers similar to how you can do this with most other busses/classes.
Signed-off-by: Krzysztof Adamski
---
drivers/hwmon/pmbus/pmbus.h | 3 +++
drivers/hwmon/pmbus/pmbus_core.c | 13 -
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 1d24397d36ec..fb267ec11623 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -417,6 +417,9 @@ struct pmbus_driver_info {
/* Regulator functionality, if supported by this chip driver. */
int num_regulators;
const struct regulator_desc *reg_desc;
+
+ /* custom attributes */
+ const struct attribute_group **groups;
};
/* Regulator ops */
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 2e2b5851139c..7a7dcdc8acc9 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -103,7 +103,7 @@ struct pmbus_data {
int max_attributes;
int num_attributes;
struct attribute_group group;
- const struct attribute_group *groups[2];
+ const struct attribute_group **groups;
struct dentry *debugfs; /* debugfs device directory */
struct pmbus_sensor *sensors;
@@ -2305,6 +2305,7 @@ int pmbus_do_probe(struct i2c_client *client, const
struct i2c_device_id *id,
struct device *dev = >dev;
const struct pmbus_platform_data *pdata = dev_get_platdata(dev);
struct pmbus_data *data;
+ size_t groups_num = 0;
int ret;
if (!info)
@@ -2319,6 +2320,15 @@ int pmbus_do_probe(struct i2c_client *client, const
struct i2c_device_id *id,
if (!data)
return -ENOMEM;
+ if (info->groups)
+ while (info->groups[groups_num])
+ groups_num++;
+
+ data->groups = devm_kcalloc(dev, groups_num + 2, sizeof(void *),
+ GFP_KERNEL);
+ if (!data->groups)
+ return -ENOMEM;
+
i2c_set_clientdata(client, data);
mutex_init(>update_lock);
data->dev = dev;
@@ -2346,6 +2356,7 @@ int pmbus_do_probe(struct i2c_client *client, const
struct i2c_device_id *id,
}
data->groups[0] = >group;
+ memcpy(data->groups + 1, info->groups, sizeof(void *) * groups_num);
data->hwmon_dev = hwmon_device_register_with_groups(dev, client->name,
data, data->groups);
if (IS_ERR(data->hwmon_dev)) {
--
2.20.1
