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!
