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 <krzysztof.adamski@xxxxxxxxx>
>>---
>> 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
