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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/9/24 22:44, Heiner Kallweit 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.


"There are more drivers outside drivers/hwmon which would benefit
from this helper as well" very clearly suggests that the function
is not hwmon specific. Yet, obviously it is if it requires a hwmon
include file.

If this is hwmon specific, I'll want to see a different solution.
Instead of calling an exported function, hwmon drivers should set the
is_visible callback to a defined value, such as HWMON_VISIBLE_0444.
This could be an ERR_PTR() which is converted by the hwmon core.
This lets us add, for example, HWMON_VISIBLE_0644, and it avoids the
notion that the function could be called by non-hwmon drivers
(and the notion that it is a function in the first place). Instead
of calling the callback directly, the hwmon core would then have a
helper function which evaluates the pointer and either returns
a constant or calls the callback.

Guenter





[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux