[PATCH 0/9] hwmon: Add and use helper hwmon_visible_0444
Heiner Kallweit
hkallweit1 at gmail.com
Thu Oct 10 08:39:32 PDT 2024
On 10.10.2024 17:14, Heiner Kallweit wrote:
> On 10.10.2024 16:31, Guenter Roeck wrote:
>> 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.
>>
> What I was referring to is device drivers in other subsystems which
> expose e.g. thermal sensor data via hwmon API.
>
>> 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.
>>
> What goes in the same direction, but may be better/cleaner:
> We could add an umode_t parameter to struct hwmon_ops, and if it's
> non-zero then hwmon core interprets it as static visibility instead
> of calling is_visible().
>
I just sent a patch with this approach to have a basis for discussion.
>> Guenter
>>
> Heiner
More information about the linux-arm-kernel
mailing list