[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z4_XQQ0tkD1EkOJ4@shell.armlinux.org.uk>
Date: Tue, 21 Jan 2025 17:20:01 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Armin Wolf <W_Armin@....de>, Huisong Li <lihuisong@...wei.com>,
linux-hwmon@...r.kernel.org, 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, 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
On Tue, Jan 21, 2025 at 06:50:00AM -0800, Guenter Roeck wrote:
> 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.
I think something needs to budge here, because I think what you're
asking is actually impossible!
One can't change the type of struct hwmon_channel_info.config to be a
u64 without also updating every hwmon driver that assigns to that
member.
This is not possible:
struct hwmon_channel_info {
enum hwmon_sensor_types type;
- const u32 *config;
+ const u64 *config;
};
static u32 marvell_hwmon_chip_config[] = {
...
};
static const struct hwmon_channel_info marvell_hwmon_chip = {
.type = hwmon_chip,
.config = marvell_hwmon_chip_config,
};
This assignment to .config will cause a warning/error with the above
change. If instead we do:
- .config = marvell_hwmon_chip_config,
+ .config = (u64 *)marvell_hwmon_chip_config,
which would have to happen to every driver, then no, this also doesn't
work, because config[1] now points beyond the bounds of
marvell_hwmon_chip_config, which only has two u32 entries.
You can't just change the type of struct hwmon_channel_info.config
without patching every driver that assigns to
struct hwmon_channel_info.config as things currently stand.
The only way out of that would be:
1. convert *all* drivers that defines a config array to be defined by
their own macro in hwmon.h, and then switch that macro to make the
definitions be a u64 array at the same time as switching struct
hwmon_channel_info.config
2. convert *all* drivers to use HWMON_CHANNEL_INFO() unconditionally,
and switch that along with struct hwmon_channel_info.config.
3. add a new member to struct hwmon_channel_info such as
"const u64 *config64" and then gradually convert drivers to use it.
Once everyone is converted over, then remove "const u32 *config",
optionally rename "config64" back to "config" and then re-patch all
drivers. That'll be joyful, with multiple patches to drivers that
need to be merged in sync with hwmon changes - and last over several
kernel release cycles.
This is not going to be an easy change!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists