Re: [PATCH 04/15] hwmon: (it87) Introduce feature flag to reflect internal in7 sensor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 29 Mar 2015 23:33:44 -0700, Guenter Roeck wrote:
> On some chips, in7 is always an internal voltage sensor. Introduce
> feature flag to reflect this condition to simplify adding support
> for new chips.
> 
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> ---
>  drivers/hwmon/it87.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index bdd6b33a3b25..2b41a598cee7 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -258,6 +258,7 @@ struct it87_devices {
>  #define FEAT_FAN16_CONFIG	(1 << 7)	/* Need to enable 16-bit fans */
>  #define FEAT_FIVE_FANS		(1 << 8)	/* Supports five fans */
>  #define FEAT_VID		(1 << 9)	/* Set if chip supports VID */
> +#define FEAT_IN7_INTERNAL	(1 << 10)	/* Set if in7 is internal */
>  
>  static const struct it87_devices it87_devices[] = {
>  	[it87] = {
> @@ -295,7 +296,7 @@ static const struct it87_devices it87_devices[] = {
>  		.name = "it8721",
>  		.features = FEAT_NEWER_AUTOPWM | FEAT_12MV_ADC | FEAT_16BIT_FANS
>  		  | FEAT_TEMP_OFFSET | FEAT_TEMP_OLD_PECI | FEAT_TEMP_PECI
> -		  | FEAT_FAN16_CONFIG | FEAT_FIVE_FANS,
> +		  | FEAT_FAN16_CONFIG | FEAT_FIVE_FANS | FEAT_IN7_INTERNAL,
>  		.peci_mask = 0x05,
>  		.old_peci_mask = 0x02,	/* Actually reports PCH */
>  		.suffix = "F",
> @@ -303,14 +304,15 @@ static const struct it87_devices it87_devices[] = {
>  	[it8728] = {
>  		.name = "it8728",
>  		.features = FEAT_NEWER_AUTOPWM | FEAT_12MV_ADC | FEAT_16BIT_FANS
> -		  | FEAT_TEMP_OFFSET | FEAT_TEMP_PECI | FEAT_FIVE_FANS,
> +		  | FEAT_TEMP_OFFSET | FEAT_TEMP_PECI | FEAT_FIVE_FANS
> +		  | FEAT_IN7_INTERNAL,
>  		.peci_mask = 0x07,
>  		.suffix = "F",
>  	},
>  	[it8771] = {
>  		.name = "it8771",
>  		.features = FEAT_NEWER_AUTOPWM | FEAT_12MV_ADC | FEAT_16BIT_FANS
> -		  | FEAT_TEMP_OFFSET | FEAT_TEMP_PECI,
> +		  | FEAT_TEMP_OFFSET | FEAT_TEMP_PECI | FEAT_IN7_INTERNAL,
>  				/* PECI: guesswork */
>  				/* 12mV ADC (OHM) */
>  				/* 16 bit fans (OHM) */
> @@ -321,7 +323,7 @@ static const struct it87_devices it87_devices[] = {
>  	[it8772] = {
>  		.name = "it8772",
>  		.features = FEAT_NEWER_AUTOPWM | FEAT_12MV_ADC | FEAT_16BIT_FANS
> -		  | FEAT_TEMP_OFFSET | FEAT_TEMP_PECI,
> +		  | FEAT_TEMP_OFFSET | FEAT_TEMP_PECI | FEAT_IN7_INTERNAL,
>  				/* PECI (coreboot) */
>  				/* 12mV ADC (HWSensors4, OHM) */
>  				/* 16 bit fans (HWSensors4, OHM) */
> @@ -353,14 +355,14 @@ static const struct it87_devices it87_devices[] = {
>  	[it8786] = {
>  		.name = "it8786",
>  		.features = FEAT_NEWER_AUTOPWM | FEAT_12MV_ADC | FEAT_16BIT_FANS
> -		  | FEAT_TEMP_OFFSET | FEAT_TEMP_PECI,
> +		  | FEAT_TEMP_OFFSET | FEAT_TEMP_PECI | FEAT_IN7_INTERNAL,
>  		.peci_mask = 0x07,
>  		.suffix = "E",
>  	},
>  	[it8603] = {
>  		.name = "it8603",
>  		.features = FEAT_NEWER_AUTOPWM | FEAT_12MV_ADC | FEAT_16BIT_FANS
> -		  | FEAT_TEMP_OFFSET | FEAT_TEMP_PECI,
> +		  | FEAT_TEMP_OFFSET | FEAT_TEMP_PECI | FEAT_IN7_INTERNAL,
>  		.peci_mask = 0x07,
>  		.suffix = "E",
>  	},
> @@ -379,6 +381,7 @@ static const struct it87_devices it87_devices[] = {
>  #define has_fan16_config(data)	((data)->features & FEAT_FAN16_CONFIG)
>  #define has_five_fans(data)	((data)->features & FEAT_FIVE_FANS)
>  #define has_vid(data)		((data)->features & FEAT_VID)
> +#define has_in7_internal(data)	((data)->features & FEAT_IN7_INTERNAL)
>  
>  struct it87_sio_data {
>  	enum chips type;
> @@ -1862,6 +1865,11 @@ static int __init it87_find(unsigned short *address,
>  
>  	/* in8 (Vbat) is always internal */
>  	sio_data->internal = (1 << 2);
> +
> +	/* in7 (VSB or VCCH5V) is always internal on some chips */
> +	if (it87_devices[sio_data->type].features & FEAT_IN7_INTERNAL)
> +		sio_data->internal |= (1 << 1);
> +
>  	/* Only the IT8603E has in9 */
>  	if (sio_data->type != it8603)
>  		sio_data->skip_in |= (1 << 9);

Nitpicking, but wouldn't it make more sense to deal with in7 first,
then in8, then in9?

> @@ -1962,7 +1970,6 @@ static int __init it87_find(unsigned short *address,
>  		sio_data->skip_in |= (1 << 5); /* No VIN5 */
>  		sio_data->skip_in |= (1 << 6); /* No VIN6 */
>  
> -		sio_data->internal |= (1 << 1); /* in7 is VSB */
>  		sio_data->internal |= (1 << 3); /* in9 is AVCC */
>  
>  		sio_data->beep_pin = superio_inb(IT87_SIO_BEEP_PIN_REG) & 0x3f;
> @@ -2023,9 +2030,7 @@ static int __init it87_find(unsigned short *address,
>  		}
>  		if (reg & (1 << 0))
>  			sio_data->internal |= (1 << 0);
> -		if ((reg & (1 << 1)) || sio_data->type == it8721 ||
> -		    sio_data->type == it8728 || sio_data->type == it8771 ||
> -		    sio_data->type == it8772 || sio_data->type == it8786)
> +		if (reg & (1 << 1))
>  			sio_data->internal |= (1 << 1);
>  
>  		/*

Other than that, no objection.

Reviewed-by: Jean Delvare <jdelvare@xxxxxxx>

-- 
Jean Delvare
SUSE L3 Support

_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors




[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux