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] [day] [month] [year] [list]
Message-ID: <CAGHj7OJaSvSsr0z43phjs1YvL17xfJWesJFj0iQpdvi8=VXZhA@mail.gmail.com>
Date: Fri, 26 Dec 2025 21:54:44 +0100
From: Benoit Masson <yahoo@...enite.com>
To: jdelvare@...e.com, linux@...ck-us.net
Cc: linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hwmon: it87: describe per-chip temperature resources

This patch is intended to be part of a series to help with support of
newer IT chipp which have new more dynamic options, and make the
review easier.

This patch is about making the number of:
 - temperature limit registers,
 - temperature offset registers, and
 - temperature-to-PWM mapping slots (per PWM bank)
configurable per chip, instead of hard-coded constants.

for all existing chips in the in-tree driver this patch does not
change behavior because you:
 - keep the old defaults (3 temp limits, 3 offsets, “3+3” PWM temp
mapping banks),
 - and only prepare per-chip fields without actually changing any
it87_devices[] initialisers yet.

Thanks
Benoit

On Fri, Dec 26, 2025 at 9:30 PM benoit.masson <yahoo@...enite.com> wrote:
>
> Signed-off-by: benoit.masson <yahoo@...enite.com>
> ---
>  drivers/hwmon/it87.c | 50 ++++++++++++++++++++++++++++++++------------
>  1 file changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index e233aafa8856..f9eca0bc02bc 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -54,6 +54,7 @@
>  #include <linux/hwmon-vid.h>
>  #include <linux/err.h>
>  #include <linux/mutex.h>
> +#include <linux/minmax.h>
>  #include <linux/sysfs.h>
>  #include <linux/string.h>
>  #include <linux/dmi.h>
> @@ -279,8 +280,9 @@ static const u8 IT87_REG_AUTO_BASE[] = { 0x60, 0x68, 0x70, 0x78, 0xa0, 0xa8 };
>  #define NUM_VIN                        ARRAY_SIZE(IT87_REG_VIN)
>  #define NUM_VIN_LIMIT          8
>  #define NUM_TEMP               6
> -#define NUM_TEMP_OFFSET                ARRAY_SIZE(IT87_REG_TEMP_OFFSET)
> -#define NUM_TEMP_LIMIT         3
> +#define IT87_TEMP_OFFSET_MAX   ARRAY_SIZE(IT87_REG_TEMP_OFFSET)
> +#define IT87_TEMP_LIMIT_DEFAULT        3
> +#define IT87_TEMP_MAP_DEFAULT  3
>  #define NUM_FAN                        ARRAY_SIZE(IT87_REG_FAN)
>  #define NUM_FAN_DIV            3
>  #define NUM_PWM                        ARRAY_SIZE(IT87_REG_PWM)
> @@ -290,6 +292,9 @@ struct it87_devices {
>         const char *name;
>         const char * const model;
>         u32 features;
> +       u8 num_temp_limit;
> +       u8 num_temp_offset;
> +       u8 num_temp_map;
>         u8 peci_mask;
>         u8 old_peci_mask;
>         u8 smbus_bitmap;        /* SMBus enable bits in extra config register */
> @@ -578,6 +583,9 @@ struct it87_data {
>         int sioaddr;
>         enum chips type;
>         u32 features;
> +       u8 num_temp_limit;
> +       u8 num_temp_offset;
> +       u8 num_temp_map;
>         u8 peci_mask;
>         u8 old_peci_mask;
>
> @@ -926,12 +934,13 @@ static struct it87_data *it87_update_device(struct device *dev)
>                         data->temp[i][0] =
>                                 it87_read_value(data, IT87_REG_TEMP(i));
>
> -                       if (has_temp_offset(data) && i < NUM_TEMP_OFFSET)
> +                       if (has_temp_offset(data) &&
> +                           i < data->num_temp_offset)
>                                 data->temp[i][3] =
>                                   it87_read_value(data,
>                                                   IT87_REG_TEMP_OFFSET[i]);
>
> -                       if (i >= NUM_TEMP_LIMIT)
> +                       if (i >= data->num_temp_limit)
>                                 continue;
>
>                         data->temp[i][1] =
> @@ -1679,16 +1688,17 @@ static ssize_t show_pwm_temp_map(struct device *dev,
>         struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
>         struct it87_data *data = it87_update_device(dev);
>         int nr = sensor_attr->index;
> +       u8 num_map = data->num_temp_map ?: IT87_TEMP_MAP_DEFAULT;
>         int map;
>
>         if (IS_ERR(data))
>                 return PTR_ERR(data);
>
>         map = data->pwm_temp_map[nr];
> -       if (map >= 3)
> +       if (map >= num_map)
>                 map = 0;        /* Should never happen */
> -       if (nr >= 3)            /* pwm channels 3..6 map to temp4..6 */
> -               map += 3;
> +       if (nr >= num_map)      /* pwm channels 3..6 map to temp4..6 */
> +               map += num_map;
>
>         return sprintf(buf, "%d\n", (int)BIT(map));
>  }
> @@ -1700,6 +1710,7 @@ static ssize_t set_pwm_temp_map(struct device *dev,
>         struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
>         struct it87_data *data = dev_get_drvdata(dev);
>         int nr = sensor_attr->index;
> +       u8 num_map = data->num_temp_map ?: IT87_TEMP_MAP_DEFAULT;
>         long val;
>         int err;
>         u8 reg;
> @@ -1707,8 +1718,8 @@ static ssize_t set_pwm_temp_map(struct device *dev,
>         if (kstrtol(buf, 10, &val) < 0)
>                 return -EINVAL;
>
> -       if (nr >= 3)
> -               val -= 3;
> +       if (nr >= num_map)
> +               val -= num_map;
>
>         switch (val) {
>         case BIT(0):
> @@ -3206,7 +3217,7 @@ static void it87_check_limit_regs(struct it87_data *data)
>                 if (reg == 0xff)
>                         it87_write_value(data, IT87_REG_VIN_MIN(i), 0);
>         }
> -       for (i = 0; i < NUM_TEMP_LIMIT; i++) {
> +       for (i = 0; i < data->num_temp_limit; i++) {
>                 reg = it87_read_value(data, IT87_REG_TEMP_HIGH(i));
>                 if (reg == 0xff)
>                         it87_write_value(data, IT87_REG_TEMP_HIGH(i), 127);
> @@ -3399,6 +3410,7 @@ static int it87_probe(struct platform_device *pdev)
>         struct resource *res;
>         struct device *dev = &pdev->dev;
>         struct it87_sio_data *sio_data = dev_get_platdata(dev);
> +       const struct it87_devices *chip;
>         int enable_pwm_interface;
>         struct device *hwmon_dev;
>         int err;
> @@ -3421,9 +3433,21 @@ static int it87_probe(struct platform_device *pdev)
>         data->type = sio_data->type;
>         data->smbus_bitmap = sio_data->smbus_bitmap;
>         data->ec_special_config = sio_data->ec_special_config;
> -       data->features = it87_devices[sio_data->type].features;
> -       data->peci_mask = it87_devices[sio_data->type].peci_mask;
> -       data->old_peci_mask = it87_devices[sio_data->type].old_peci_mask;
> +       chip = &it87_devices[sio_data->type];
> +       data->features = chip->features;
> +       data->peci_mask = chip->peci_mask;
> +       data->old_peci_mask = chip->old_peci_mask;
> +       data->num_temp_limit = chip->num_temp_limit ?
> +                              chip->num_temp_limit : IT87_TEMP_LIMIT_DEFAULT;
> +       if (chip->num_temp_offset)
> +               data->num_temp_offset = min(chip->num_temp_offset,
> +                                           (u8)IT87_TEMP_OFFSET_MAX);
> +       else if (has_temp_offset(data))
> +               data->num_temp_offset = IT87_TEMP_OFFSET_MAX;
> +       else
> +               data->num_temp_offset = 0;
> +       data->num_temp_map = chip->num_temp_map ?
> +                            chip->num_temp_map : IT87_TEMP_MAP_DEFAULT;
>         /*
>          * IT8705F Datasheet 0.4.1, 3h == Version G.
>          * IT8712F Datasheet 0.9.1, section 8.3.5 indicates 8h == Version J.
> --
> 2.50.1 (Apple Git-155)
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ