Re: [PATCH 2/3] hwmon: (nct6775) Use custom scale for ASUS motherboards.

[Guenter Roeck]

On Sun, Oct 03, 2021 at 12:08:55AM +0300, Denis Pauk wrote:
> Use custom scaling factor for:
> * TUF GAMING Z490-PLUS
> * TUF GAMING Z490-PLUS (WI-FI)
> 
> Voltage scaling factors are based on Asus software on Windows.
> 

Scaling is never correct for any SuperIO chips, and is notoriously different
for each and every PC board. I don't want to add per-board scaling data to
the kernel drivers for those chips. This would just create an unsupportable
mess.

Guenter

> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=204807
> Signed-off-by: Denis Pauk <pauk.denis@xxxxxxxxx>
> Tested-by: Kamil Pietrzak <kpietrzak@xxxxxxxxxxx>
> Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> ---
>  drivers/hwmon/nct6775.c | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c
> index 8eaf86ea2433..ba18c1cbf572 100644
> --- a/drivers/hwmon/nct6775.c
> +++ b/drivers/hwmon/nct6775.c
> @@ -140,6 +140,7 @@ struct nct6775_sio_data {
>  	int ld;
>  	enum kinds kind;
>  	enum sensor_access access;
> +	bool custom_scale;
>  
>  	/* superio_() callbacks  */
>  	void (*sio_outb)(struct nct6775_sio_data *sio_data, int reg, int val);
> @@ -1159,14 +1160,19 @@ static const u16 scale_in[15] = {
>  	800, 800
>  };
>  
> -static inline long in_from_reg(u8 reg, u8 nr)
> +static const u16 scale_in_z490[15] = {
> +	888, 4000, 1600, 1600, 9600, 800, 800, 1600, 1600, 1600, 1600, 1600, 800,
> +	800, 800
> +};
> +
> +static inline long in_from_reg(u8 reg, u8 nr, const u16 *scale)
>  {
> -	return DIV_ROUND_CLOSEST(reg * scale_in[nr], 100);
> +	return DIV_ROUND_CLOSEST(reg * scale[nr], 100);
>  }
>  
> -static inline u8 in_to_reg(u32 val, u8 nr)
> +static inline u8 in_to_reg(u32 val, u8 nr, const u16 *scale)
>  {
> -	return clamp_val(DIV_ROUND_CLOSEST(val * 100, scale_in[nr]), 0, 255);
> +	return clamp_val(DIV_ROUND_CLOSEST(val * 100, scale[nr]), 0, 255);
>  }
>  
>  /*
> @@ -1323,6 +1329,9 @@ struct nct6775_data {
>  	u8 fandiv2;
>  	u8 sio_reg_enable;
>  
> +	/* voltage scaling factors */
> +	const u16 *scale;
> +
>  	/* nct6775_*() callbacks  */
>  	u16 (*read_value)(struct nct6775_data *data, u16 reg);
>  	int (*write_value)(struct nct6775_data *data, u16 reg, u16 value);
> @@ -2026,7 +2035,7 @@ show_in_reg(struct device *dev, struct device_attribute *attr, char *buf)
>  	int index = sattr->index;
>  	int nr = sattr->nr;
>  
> -	return sprintf(buf, "%ld\n", in_from_reg(data->in[nr][index], nr));
> +	return sprintf(buf, "%ld\n", in_from_reg(data->in[nr][index], nr, data->scale));
>  }
>  
>  static ssize_t
> @@ -2044,7 +2053,7 @@ store_in_reg(struct device *dev, struct device_attribute *attr, const char *buf,
>  	if (err < 0)
>  		return err;
>  	mutex_lock(&data->update_lock);
> -	data->in[nr][index] = in_to_reg(val, nr);
> +	data->in[nr][index] = in_to_reg(val, nr, data->scale);
>  	data->write_value(data, data->REG_IN_MINMAX[index - 1][nr],
>  			  data->in[nr][index]);
>  	mutex_unlock(&data->update_lock);
> @@ -3980,6 +3989,11 @@ static int nct6775_probe(struct platform_device *pdev)
>  		data->write_value = nct6775_wmi_write_value;
>  	}
>  
> +	if (sio_data->custom_scale)
> +		data->scale = scale_in_z490;
> +	else
> +		data->scale = scale_in;
> +
>  	mutex_init(&data->update_lock);
>  	data->name = nct6775_device_names[data->kind];
>  	data->bank = 0xff;		/* Force initial bank selection */
> @@ -5020,6 +5034,7 @@ static int __init sensors_nct6775_init(void)
>  	struct nct6775_sio_data sio_data;
>  	int sioaddr[2] = { 0x2e, 0x4e };
>  	enum sensor_access access = access_direct;
> +	bool custom_scale = false;
>  	const char *board_vendor, *board_name;
>  	u8 tmp;
>  
> @@ -5043,6 +5058,10 @@ static int __init sensors_nct6775_init(void)
>  				pr_err("Can't read ChipID by Asus WMI.\n");
>  			}
>  		}
> +
> +		if (strcmp(board_name, "TUF GAMING Z490-PLUS") == 0 ||
> +		    strcmp(board_name, "TUF GAMING Z490-PLUS (WI-FI)") == 0)
> +			custom_scale = true;
>  	}
>  
>  	/*
> @@ -5066,6 +5085,7 @@ static int __init sensors_nct6775_init(void)
>  		found = true;
>  
>  		sio_data.access = access;
> +		sio_data.custom_scale = custom_scale;
>  
>  		if (access == access_asuswmi) {
>  			sio_data.sio_outb = superio_wmi_outb;
> -- 
> 2.33.0
>