Re: [PATCH 0/9] hwmon: Add and use helper hwmon_visible_0444

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu, 10 Oct 2024 07:44:31 +0200
Heiner Kallweit <hkallweit1@gmail.com> wrote:

> On 10.10.2024 00:19, Guenter Roeck wrote:
> > On 10/9/24 13:02, Heiner Kallweit wrote:  
> >> Several drivers simply return 0444 in their is_visible callback.
> >> Add a helper in hwmon core for this use case to avoid code duplication.
> >>
> >> There are more drivers outside drivers/hwmon which would benefit
> >> from this helper as well.
> >>
> >> Heiner Kallweit (9):
> >>    hwmon: Add helper hwmon_visible_0444
> >>    hwmon: i5500_temp: Use new helper hwmon_visible_0444
> >>    hwmon: surface_fan: Use new helper hwmon_visible_0444
> >>    hwmon: sl28cpld: Use new helper hwmon_visible_0444
> >>    hwmon: gsc: Use new helper hwmon_visible_0444
> >>    hwmon: powerz: Use new helper hwmon_visible_0444
> >>    hwmon: raspberrypi: Use new helper hwmon_visible_0444
> >>    hwmon: intel-m10-bmc: Use new helper hwmon_visible_0444
> >>    hwmon: nzxt-kraken2: Use new helper hwmon_visible_0444
> >>
> >>   drivers/hwmon/gsc-hwmon.c           |  9 +--------
> >>   drivers/hwmon/hwmon.c               |  7 +++++++
> >>   drivers/hwmon/i5500_temp.c          |  8 +-------
> >>   drivers/hwmon/intel-m10-bmc-hwmon.c |  9 +--------
> >>   drivers/hwmon/nzxt-kraken2.c        |  9 +--------
> >>   drivers/hwmon/powerz.c              |  8 +-------
> >>   drivers/hwmon/raspberrypi-hwmon.c   |  8 +-------
> >>   drivers/hwmon/sl28cpld-hwmon.c      |  9 +--------
> >>   drivers/hwmon/surface_fan.c         | 10 +---------
> >>   include/linux/hwmon.h               |  2 ++
> >>   10 files changed, 17 insertions(+), 62 deletions(-)
> >>  
> > 
> > I really don't want to add such hwmon-specific but at the same time
> > generic helpers. If such a helper is made available in the core kernel,
> > I'll be happy to accept patches using it, but otherwise please refrain
> > from submitting such patch series.
> >   
> What would you consider a suited place, drivers/base or fs/sysfs or lib or .. ?
> For enum hwmon_sensor_types we have to include linux/hwmon.h. None of these
> places has any hwmon code, and I would expect concerns if generic core code
> includes subsystem headers.

I don't see this as particularly generic. 

For normal attribute handling with fixed permissions you'd just not provide
an is_visible function and hardcode the permissions in the attribute.

Maybe there are other users that need an is_visible callback?

Jonathan
> 
> > Thanks,
> > Guenter
> >   
> 
>