Message ID | c16acf3c-6ca6-4033-a115-58f6031ed868@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | hwmon: Add static visibility member to struct hwmon_ops | expand |
On 10/10/24 08:38, Heiner Kallweit wrote: > Several drivers return the same static value in their is_visible > callback, what results in code duplication. Therefore add an option > for drivers to specify a static visibility directly. > Yes, that works. Minor comment below. With that fixed, we are good to go. Thanks, Guenter > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/hwmon/hwmon.c | 19 +++++++++++++++---- > include/linux/hwmon.h | 4 +++- > 2 files changed, 18 insertions(+), 5 deletions(-) > > diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c > index 9c35c4d03..8e9159e8d 100644 > --- a/drivers/hwmon/hwmon.c > +++ b/drivers/hwmon/hwmon.c > @@ -145,6 +145,17 @@ static const struct class hwmon_class = { > > static DEFINE_IDA(hwmon_ida); > > +static umode_t hwmon_ops_is_visible(const struct hwmon_ops *ops, > + const void *drvdata, > + enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + if (ops->visible) > + return ops->visible; > + > + return ops->is_visible(drvdata, type, attr, channel); > +} > + > /* Thermal zone handling */ > > /* > @@ -267,8 +278,8 @@ static int hwmon_thermal_register_sensors(struct device *dev) > int err; > > if (!(info[i]->config[j] & HWMON_T_INPUT) || > - !chip->ops->is_visible(drvdata, hwmon_temp, > - hwmon_temp_input, j)) > + !hwmon_ops_is_visible(chip->ops, drvdata, hwmon_temp, > + hwmon_temp_input, j)) > continue; > > err = hwmon_thermal_add_sensor(dev, j); > @@ -506,7 +517,7 @@ static struct attribute *hwmon_genattr(const void *drvdata, > const char *name; > bool is_string = is_string_attr(type, attr); > > - mode = ops->is_visible(drvdata, type, attr, index); > + mode = hwmon_ops_is_visible(ops, drvdata, type, attr, index); > if (!mode) > return ERR_PTR(-ENOENT); > > @@ -1033,7 +1044,7 @@ hwmon_device_register_with_info(struct device *dev, const char *name, > if (!dev || !name || !chip) > return ERR_PTR(-EINVAL); > > - if (!chip->ops || !chip->ops->is_visible || !chip->info) > + if (!chip->ops || !(chip->ops->visible || chip->ops->is_visible) || !chip->info) > return ERR_PTR(-EINVAL); > > return __hwmon_device_register(dev, name, drvdata, chip, extra_groups); > diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h > index 5c6a421ad..667229956 100644 > --- a/include/linux/hwmon.h > +++ b/include/linux/hwmon.h > @@ -368,7 +368,8 @@ enum hwmon_intrusion_attributes { > > /** > * struct hwmon_ops - hwmon device operations > - * @is_visible: Callback to return attribute visibility. Mandatory. > + * @visible: Static visibility. If non-zero, @is_visible is ignored. > + * @is_visible: Callback to return attribute visibility. Conditionally mandatory. "Mandatory unless 'visible' is non-zero". > * Parameters are: > * @const void *drvdata: > * Pointer to driver-private data structure passed > @@ -412,6 +413,7 @@ enum hwmon_intrusion_attributes { > * The function returns 0 on success or a negative error number. > */ > struct hwmon_ops { > + umode_t visible; > umode_t (*is_visible)(const void *drvdata, enum hwmon_sensor_types type, > u32 attr, int channel); > int (*read)(struct device *dev, enum hwmon_sensor_types type,
diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c index 9c35c4d03..8e9159e8d 100644 --- a/drivers/hwmon/hwmon.c +++ b/drivers/hwmon/hwmon.c @@ -145,6 +145,17 @@ static const struct class hwmon_class = { static DEFINE_IDA(hwmon_ida); +static umode_t hwmon_ops_is_visible(const struct hwmon_ops *ops, + const void *drvdata, + enum hwmon_sensor_types type, + u32 attr, int channel) +{ + if (ops->visible) + return ops->visible; + + return ops->is_visible(drvdata, type, attr, channel); +} + /* Thermal zone handling */ /* @@ -267,8 +278,8 @@ static int hwmon_thermal_register_sensors(struct device *dev) int err; if (!(info[i]->config[j] & HWMON_T_INPUT) || - !chip->ops->is_visible(drvdata, hwmon_temp, - hwmon_temp_input, j)) + !hwmon_ops_is_visible(chip->ops, drvdata, hwmon_temp, + hwmon_temp_input, j)) continue; err = hwmon_thermal_add_sensor(dev, j); @@ -506,7 +517,7 @@ static struct attribute *hwmon_genattr(const void *drvdata, const char *name; bool is_string = is_string_attr(type, attr); - mode = ops->is_visible(drvdata, type, attr, index); + mode = hwmon_ops_is_visible(ops, drvdata, type, attr, index); if (!mode) return ERR_PTR(-ENOENT); @@ -1033,7 +1044,7 @@ hwmon_device_register_with_info(struct device *dev, const char *name, if (!dev || !name || !chip) return ERR_PTR(-EINVAL); - if (!chip->ops || !chip->ops->is_visible || !chip->info) + if (!chip->ops || !(chip->ops->visible || chip->ops->is_visible) || !chip->info) return ERR_PTR(-EINVAL); return __hwmon_device_register(dev, name, drvdata, chip, extra_groups); diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h index 5c6a421ad..667229956 100644 --- a/include/linux/hwmon.h +++ b/include/linux/hwmon.h @@ -368,7 +368,8 @@ enum hwmon_intrusion_attributes { /** * struct hwmon_ops - hwmon device operations - * @is_visible: Callback to return attribute visibility. Mandatory. + * @visible: Static visibility. If non-zero, @is_visible is ignored. + * @is_visible: Callback to return attribute visibility. Conditionally mandatory. * Parameters are: * @const void *drvdata: * Pointer to driver-private data structure passed @@ -412,6 +413,7 @@ enum hwmon_intrusion_attributes { * The function returns 0 on success or a negative error number. */ struct hwmon_ops { + umode_t visible; umode_t (*is_visible)(const void *drvdata, enum hwmon_sensor_types type, u32 attr, int channel); int (*read)(struct device *dev, enum hwmon_sensor_types type,
Several drivers return the same static value in their is_visible callback, what results in code duplication. Therefore add an option for drivers to specify a static visibility directly. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/hwmon/hwmon.c | 19 +++++++++++++++---- include/linux/hwmon.h | 4 +++- 2 files changed, 18 insertions(+), 5 deletions(-)