[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1410044e-554d-4009-9248-ce063b352659@roeck-us.net>
Date: Tue, 13 Jan 2026 09:54:41 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: "benoit.masson" <yahoo@...enite.com>
Cc: Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/5] hwmon: it87: prepare for extended PWM temp maps
On Sat, Jan 10, 2026 at 02:26:10AM +0100, benoit.masson wrote:
> Introduce helper logic for PWM-to-temperature mappings so newer
> register layouts can be supported without affecting legacy chips.
>
> Signed-off-by: benoit.masson <yahoo@...enite.com>
> ---
> drivers/hwmon/it87.c | 178 ++++++++++++++++++++++++++++++++-----------
> 1 file changed, 133 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index ec5b1668dd7b..dfd1e896c1ab 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -251,6 +251,7 @@ static const u8 IT87_REG_TEMP_OFFSET[] = { 0x56, 0x57, 0x59 };
> #define IT87_REG_FAN_MAIN_CTRL 0x13
> #define IT87_REG_FAN_CTL 0x14
> static const u8 IT87_REG_PWM[] = { 0x15, 0x16, 0x17, 0x7f, 0xa7, 0xaf };
> +static const u8 IT87_REG_PWM_8665[] = { 0x15, 0x16, 0x17, 0x1e, 0x1f, 0x92 };
> static const u8 IT87_REG_PWM_DUTY[] = { 0x63, 0x6b, 0x73, 0x7b, 0xa3, 0xab };
>
> static const u8 IT87_REG_VIN[] = { 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26,
> @@ -283,6 +284,7 @@ static const u8 IT87_REG_AUTO_BASE[] = { 0x60, 0x68, 0x70, 0x78, 0xa0, 0xa8 };
> #define IT87_TEMP_OFFSET_MAX ARRAY_SIZE(IT87_REG_TEMP_OFFSET)
> #define IT87_TEMP_LIMIT_DEFAULT 3
> #define IT87_TEMP_MAP_DEFAULT 3
> +#define IT87_PWM_OLD_NUM_TEMP 3
> #define NUM_FAN ARRAY_SIZE(IT87_REG_FAN)
> #define NUM_FAN_DIV 3
> #define NUM_PWM ARRAY_SIZE(IT87_REG_PWM)
> @@ -331,6 +333,7 @@ struct it87_devices {
> #define FEAT_FOUR_PWM BIT(21) /* Supports four fan controls */
> #define FEAT_FOUR_TEMP BIT(22)
> #define FEAT_FANCTL_ONOFF BIT(23) /* chip has FAN_CTL ON/OFF */
> +#define FEAT_NEW_TEMPMAP BIT(24) /* PWM uses extended temp map */
>
> static const struct it87_devices it87_devices[] = {
> [it87] = {
> @@ -554,6 +557,7 @@ static const struct it87_devices it87_devices[] = {
> #define has_scaling(data) ((data)->features & (FEAT_12MV_ADC | \
> FEAT_10_9MV_ADC))
> #define has_fanctl_onoff(data) ((data)->features & FEAT_FANCTL_ONOFF)
> +#define has_new_tempmap(data) ((data)->features & FEAT_NEW_TEMPMAP)
>
> struct it87_sio_data {
> int sioaddr;
> @@ -632,7 +636,9 @@ struct it87_data {
> u8 has_pwm; /* Bitfield, pwm control enabled */
> u8 pwm_ctrl[NUM_PWM]; /* Register value */
> u8 pwm_duty[NUM_PWM]; /* Manual PWM value set by user */
> - u8 pwm_temp_map[NUM_PWM];/* PWM to temp. chan. mapping (bits 1-0) */
> + u8 pwm_temp_map[NUM_PWM];/* PWM to temp. chan. mapping */
> + u8 pwm_temp_map_mask;
> + u8 pwm_temp_map_shift;
>
> /* Automatic fan speed control registers */
> u8 auto_pwm[NUM_AUTO_PWM][4]; /* [nr][3] is hard-coded */
> @@ -714,6 +720,72 @@ static int pwm_from_reg(const struct it87_data *data, u8 reg)
> return (reg & 0x7f) << 1;
> }
>
> +static inline u8 pwm_temp_map_get(const struct it87_data *data, u8 ctrl)
> +{
> + return (ctrl >> data->pwm_temp_map_shift) &
> + data->pwm_temp_map_mask;
> +}
> +
> +static inline u8 pwm_temp_map_set(const struct it87_data *data, u8 ctrl,
> + u8 map)
> +{
> + ctrl &= ~(data->pwm_temp_map_mask << data->pwm_temp_map_shift);
> + return ctrl | ((map & data->pwm_temp_map_mask)
> + << data->pwm_temp_map_shift);
> +}
> +
> +static inline u8 pwm_num_temp_map(const struct it87_data *data)
> +{
> + return data->num_temp_map ? data->num_temp_map :
> + IT87_TEMP_MAP_DEFAULT;
> +}
> +
> +static unsigned int pwm_temp_channel(const struct it87_data *data,
> + int nr, u8 map)
> +{
> + if (has_new_tempmap(data)) {
> + u8 num = pwm_num_temp_map(data);
> +
> + if (map >= num)
> + map = 0;
> + return map;
> + }
> +
> + if (map >= IT87_PWM_OLD_NUM_TEMP)
> + map = 0;
> +
> + if (nr >= IT87_PWM_OLD_NUM_TEMP)
> + map += IT87_PWM_OLD_NUM_TEMP;
> +
> + return map;
> +}
> +
> +static int pwm_temp_map_from_channel(const struct it87_data *data, int nr,
> + unsigned int channel, u8 *map)
> +{
> + if (has_new_tempmap(data)) {
> + u8 num = pwm_num_temp_map(data);
> +
> + if (channel >= num)
> + return -EINVAL;
> + *map = channel;
> + return 0;
> + }
> +
> + if (nr >= IT87_PWM_OLD_NUM_TEMP) {
> + if (channel < IT87_PWM_OLD_NUM_TEMP ||
> + channel >= 2 * IT87_PWM_OLD_NUM_TEMP)
> + return -EINVAL;
> + channel -= IT87_PWM_OLD_NUM_TEMP;
> + } else {
> + if (channel >= IT87_PWM_OLD_NUM_TEMP)
> + return -EINVAL;
> + }
> +
> + *map = channel;
> + return 0;
> +}
> +
> static int DIV_TO_REG(int val)
> {
> int answer = 0;
> @@ -725,6 +797,14 @@ static int DIV_TO_REG(int val)
>
> #define DIV_FROM_REG(val) BIT(val)
>
> +static inline u16 it87_reg_pwm(const struct it87_data *data, int nr)
> +{
> + if (data->type == it8613 || data->type == it8622)
Did you try to compile this patch ? I guess not, because I get
drivers/hwmon/it87.c: In function ‘it87_reg_pwm’:
drivers/hwmon/it87.c:802:27: error: ‘it8613’ undeclared (first use in this function); did you mean ‘it8603’?
802 | if (data->type == it8613 || data->type == it8622)
| ^~~~~~
| it8603
drivers/hwmon/it87.c:802:27: note: each undeclared identifier is reported only once for each function it appears in
The code should use register pointers in struct it87_data to avoid such
problems and the need for runtime type checks.
Guenter
Powered by blists - more mailing lists