netdev - Re: [PATCH v1 00/21] hwmon: Fix the type of 'config' in struct hwmon_channel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fd412ce2-ec40-fe73-6b1b-284a99a39f12@huawei.com>
Date: Wed, 22 Jan 2025 10:52:58 +0800
From: "lihuisong (C)" <lihuisong@...wei.com>
To: Guenter Roeck <linux@...ck-us.net>, Armin Wolf <W_Armin@....de>,
	<linux-hwmon@...r.kernel.org>
CC: <linux-kernel@...r.kernel.org>, <linux-media@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <arm-scmi@...r.kernel.org>,
	<netdev@...r.kernel.org>, <linux-rtc@...r.kernel.org>,
	<oss-drivers@...igine.com>, <linux-rdma@...r.kernel.org>,
	<platform-driver-x86@...r.kernel.org>, <linuxarm@...wei.com>,
	<jdelvare@...e.com>, <kernel@...davale.org>, <pauk.denis@...il.com>,
	<james@...iv.tech>, <sudeep.holla@....com>, <cristian.marussi@....com>,
	<matt@...ostay.sg>, <mchehab@...nel.org>, <irusskikh@...vell.com>,
	<andrew+netdev@...n.ch>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<kuba@...nel.org>, <pabeni@...hat.com>, <saeedm@...dia.com>,
	<leon@...nel.org>, <tariqt@...dia.com>, <louis.peens@...igine.com>,
	<hkallweit1@...il.com>, <linux@...linux.org.uk>, <kabel@...nel.org>,
	<hdegoede@...hat.com>, <ilpo.jarvinen@...ux.intel.com>,
	<alexandre.belloni@...tlin.com>, <krzk@...nel.org>,
	<jonathan.cameron@...wei.com>, <zhanjie9@...ilicon.com>,
	<zhenglifeng1@...wei.com>, <liuyonglong@...wei.com>
Subject: Re: [PATCH v1 00/21] hwmon: Fix the type of 'config' in struct
 hwmon_channel_info to u64

Hi Guenter,

在 2025/1/21 22:50, Guenter Roeck 写道:
> On 1/21/25 06:12, Armin Wolf wrote:
>> Am 21.01.25 um 07:44 schrieb Huisong Li:
>>
>>> The hwmon_device_register() is deprecated. When I try to repace it with
>>> hwmon_device_register_with_info() for acpi_power_meter driver, I 
>>> found that
>>> the power channel attribute in linux/hwmon.h have to extend and is more
>>> than 32 after this replacement.
>>>
>>> However, the maximum number of hwmon channel attributes is 32 which is
>>> limited by current hwmon codes. This is not good to add new channel
>>> attribute for some hwmon sensor type and support more channel 
>>> attribute.
>>>
>>> This series are aimed to do this. And also make sure that 
>>> acpi_power_meter
>>> driver can successfully replace the deprecated hwmon_device_register()
>>> later.
>>
>
> This explanation completely misses the point. The series tries to make 
> space
> for additional "standard" attributes. Such a change should be independent
> of a driver conversion and be discussed on its own, not in the context
> of a new driver or a driver conversion.
Making space for new attributes later.
After all hwmon core have ability to do that.
>
>> Hi,
>>
>> what kind of new power attributes do you want to add to the hwmon API?
>>
>> AFAIK the acpi-power-meter driver supports the following attributes:
>>
>>      power1_accuracy            -> HWMON_P_ACCURACY
>>      power1_cap_min            -> HWMON_P_CAP_MIN
>>      power1_cap_max            -> HWMON_P_CAP_MAX
>>      power1_cap_hyst            -> HWMON_P_CAP_HYST
>>      power1_cap            -> HWMON_P_CAP
>>      power1_average            -> HWMON_P_AVERAGE
>>      power1_average_min        -> HWMON_P_AVERAGE_MIN
>>      power1_average_max        -> HWMON_P_AVERAGE_MAX
>>      power1_average_interval        -> HWMON_P_AVERAGE_INTERVAL
>>      power1_average_interval_min    -> HWMON_P_AVERAGE_INTERVAL_MIN
>>      power1_average_interval_max    -> HWMON_P_AVERAGE_INTERVAL_MAX
>>      power1_alarm            -> HWMON_P_ALARM
>>      power1_model_number
>>      power1_oem_info
>>      power1_serial_number
>>      power1_is_battery
>>      name                -> handled by hwmon core
>>
>> The remaining attributes are in my opinion not generic enough to add 
>> them to the generic
>> hwmon power attributes. I think you should implement them as a 
>> attribute_group which can
>> be passed to hwmon_device_register_with_info() using the 
>> "extra_groups" parameter.
>>
>
> I absolutely agree. More specifically, it looks like this is about the 
> following
> attributes.
>
> >      power1_model_number
> >      power1_oem_info
> >      power1_serial_number
> >      power1_is_battery
>
> Those are not hwmon attributes and should not be (or have been) exposed
> as sysfs attributes in the first place, but (if really wanted/needed)
> through debugfs files. Even _if_ exposed as sysfs attributes they should
> not have the power1_ prefix (except maybe for the last one).
I plan to accept the suggestion Armin proposed that acpi_power_meter can 
use the "extra_groups" parameter for these "not generic hwmon power 
attributes".
Should we drop the power1_ prefix? But this will lead to the change of 
these attributes.
Do you mean 'power1_is_battery' can be added to hwmon power attributes 
in hwmon.h?
>
> On top of that, doubling the size of configuration bits for everything
> because one sensor type needs more than 32 bits seems excessive.
> If we ever get to that point I think I'd rather introduce a second
> sensor type for power sensor attributes.
>
The bigest obstacle is that drivers which not use HWMON_CHANNEL_INFO 
hwmon provided need to be modifyed.🙁
>
> .