Re: [PATCH] hwmon: (nct6775) Integrated new model nct6116

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

 



On Wed, Jul 10, 2019 at 09:29:55PM +0200, Björn Gerhart wrote:
> oh no, the patch is WRONG as I noticed right now. Sorry for the noise! I’ll re-work it and come back!
> 
Now I have to figure out what you mean ;-).
Couple of comments anyway below.

Guenter

> > Am 10.07.2019 um 21:19 schrieb Björn Gerhart <gerhart@xxxxxxxxx>:
> > 
> > The nct6116 is the successor model of nct6106. Mainly I’m unsure about my changes related to:
> > - NCT6116_BEEP_BITS
> > - int cr24 (cr23 is new, and I’m unsure if the code has to get modified therefore)
> > - NCT6116_REG_FAN_PULSES
> > - NCT6116_FAN_PULSE_SHIFT
> > - NCT6116_ALARM_BITS
> > 
> > With the nct6116 wired on my mainboard, the sensors command seems to work as I would expect.
> > 
> > Index: nct6775.c
> > ===================================================================
> > --- nct6775.c	(revision 16313)
> > +++ nct6775.c	(working copy)
> > @@ -34,6 +34,7 @@
> >  *
> >  * Chip        #vin    #fan    #pwm    #temp  chip IDs       man ID
> >  * nct6106d     9      3       3       6+3    0xc450 0xc1    0x5ca3
> > + * nct6116d     9      5       5       3+3    0xd280 0xc1    0x5ca3
> >  * nct6775f     9      4       3       6+3    0xb470 0xc1    0x5ca3
> >  * nct6776f     9      5       3       6+3    0xc330 0xc1    0x5ca3
> >  * nct6779d    15      5       5       2+6    0xc560 0xc1    0x5ca3
> > @@ -72,12 +73,13 @@
> > 
> > #define USE_ALTERNATE
> > 
> > -enum kinds { nct6106, nct6775, nct6776, nct6779, nct6791, nct6792, nct6793,
> > -	     nct6795, nct6796, nct6797, nct6798 };
> > +enum kinds { nct6106, nct6116, nct6775, nct6776, nct6779, nct6791, nct6792,
> > +	     nct6793, nct6795, nct6796, nct6797, nct6798 };
> > 
> > /* used to set data->name = nct6775_device_names[data->sio_kind] */
> > static const char * const nct6775_device_names[] = {
> > 	"nct6106",
> > +	"nct6116",
> > 	"nct6775",
> > 	"nct6776",
> > 	"nct6779",
> > @@ -92,6 +94,7 @@
> > 
> > static const char * const nct6775_sio_names[] __initconst = {
> > 	"NCT6106D",
> > +	"NCT6116D",
> > 	"NCT6775F",
> > 	"NCT6776D/F",
> > 	"NCT6779D",
> > @@ -129,6 +132,7 @@
> > #define SIO_REG_ADDR		0x60	/* Logical device address (2 bytes) */
> > 
> > #define SIO_NCT6106_ID		0xc450
> > +#define SIO_NCT6116_ID		0xd280
> > #define SIO_NCT6775_ID		0xb470
> > #define SIO_NCT6776_ID		0xc330
> > #define SIO_NCT6779_ID		0xc560
> > @@ -819,18 +823,29 @@
> > 	0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x07, 0x08, 0x09 };
> > 
> > static const u16 NCT6106_REG_TEMP[] = { 0x10, 0x11, 0x12, 0x13, 0x14, 0x15 };
> > +static const u16 NCT6116_REG_TEMP[] = { 0x10, 0x11, 0x12 };
> > static const u16 NCT6106_REG_TEMP_MON[] = { 0x18, 0x19, 0x1a };
> > static const u16 NCT6106_REG_TEMP_HYST[] = {
> > 	0xc3, 0xc7, 0xcb, 0xcf, 0xd3, 0xd7 };
> > +static const u16 NCT6116_REG_TEMP_HYST[] = {
> > +	0xc3, 0xc7, 0xcb };

Please move all NCT6116 related declarations/defines together.

> > static const u16 NCT6106_REG_TEMP_OVER[] = {
> > 	0xc2, 0xc6, 0xca, 0xce, 0xd2, 0xd6 };
> > +static const u16 NCT6116_REG_TEMP_OVER[] = {
> > +	0xc2, 0xc6, 0xca };
> > static const u16 NCT6106_REG_TEMP_CRIT_L[] = {
> > 	0xc0, 0xc4, 0xc8, 0xcc, 0xd0, 0xd4 };
> > +static const u16 NCT6116_REG_TEMP_CRIT_L[] = {
> > +	0xc0, 0xc4, 0xc8 };
> > static const u16 NCT6106_REG_TEMP_CRIT_H[] = {
> > 	0xc1, 0xc5, 0xc9, 0xcf, 0xd1, 0xd5 };
> > +static const u16 NCT6116_REG_TEMP_CRIT_H[] = {
> > +	0xc1, 0xc5, 0xc9 };
> > static const u16 NCT6106_REG_TEMP_OFFSET[] = { 0x311, 0x312, 0x313 };
> > static const u16 NCT6106_REG_TEMP_CONFIG[] = {
> > 	0xb7, 0xb8, 0xb9, 0xba, 0xbb, 0xbc };
> > +static const u16 NCT6116_REG_TEMP_CONFIG[] = {
> > +	0xb7, 0xb8, 0xb9 };
> > 
> > static const u16 NCT6106_REG_FAN[] = { 0x20, 0x22, 0x24 };
> > static const u16 NCT6106_REG_FAN_MIN[] = { 0xe0, 0xe2, 0xe4 };
> > @@ -845,6 +860,8 @@
> > static const u16 NCT6106_REG_TEMP_SEL[] = { 0x110, 0x120, 0x130 };
> > static const u16 NCT6106_REG_TEMP_SOURCE[] = {
> > 	0xb0, 0xb1, 0xb2, 0xb3, 0xb4, 0xb5 };
> > +static const u16 NCT6116_REG_TEMP_SOURCE[] = {
> > +	0xb0, 0xb1, 0xb2 };
> > 
> > static const u16 NCT6106_REG_CRITICAL_TEMP[] = { 0x11a, 0x12a, 0x13a };
> > static const u16 NCT6106_REG_CRITICAL_TEMP_TOLERANCE[] = {
> > @@ -886,6 +903,16 @@
> > 	48, -1				/* intrusion0, intrusion1 */
> > };
> > 
> > +static const s8 NCT6116_ALARM_BITS[] = {
> > +	0, 1, 2, 3, 4, 5, 7, 8,		/* in0.. in7 */
> > +	9, -1, -1, -1, -1, -1, -1,	/* in8..in14 */
> > +	-1,				/* unused */
> > +	32, 33, 34, -1, -1,		/* fan1..fan5 */
> > +	-1, -1, -1,			/* unused */
> > +	16, 17, 18,			/* temp1..temp3 */
> > +	48, -1				/* intrusion0, intrusion1 */
> > +};
> > +
> > static const u16 NCT6106_REG_BEEP[NUM_REG_BEEP] = {
> > 	0x3c0, 0x3c1, 0x3c2, 0x3c3, 0x3c4 };
> > 
> > @@ -1308,6 +1335,10 @@
> > 		return reg == 0x20 || reg == 0x22 || reg == 0x24 ||
> > 		  reg == 0xe0 || reg == 0xe2 || reg == 0xe4 ||
> > 		  reg == 0x111 || reg == 0x121 || reg == 0x131;
> > +	case nct6116:
> > +		return reg == 0x20 || reg == 0x22 || reg == 0x24 ||
> > +		  reg == 0xe0 || reg == 0xe2 || reg == 0xe4 ||
> > +		  reg == 0x111 || reg == 0x121 || reg == 0x131;

Same as case statement for nct6106; please merge.

> > 	case nct6775:
> > 		return (((reg & 0xff00) == 0x100 ||
> > 		    (reg & 0xff00) == 0x200) &&
> > @@ -1687,6 +1718,7 @@
> > 			data->auto_pwm[i][data->auto_pwm_num] = 0xff;
> > 			break;
> > 		case nct6106:
> > +		case nct6116:
> > 		case nct6779:
> > 		case nct6791:
> > 		case nct6792:
> > @@ -3123,6 +3155,7 @@
> > 		case nct6776:
> > 			break; /* always enabled, nothing to do */
> > 		case nct6106:
> > +		case nct6116:
> > 		case nct6779:
> > 		case nct6791:
> > 		case nct6792:
> > @@ -3549,6 +3582,11 @@
> > 
> > 		fan3pin = !(cr24 & 0x80);
> > 		pwm3pin = cr24 & 0x08;
> > +	} else if (data->kind == nct6116) {
> > +		int cr24 = superio_inb(sioreg, 0x24);
> > +
> > +		fan3pin = !(cr24 & 0x80);
> > +		pwm3pin = cr24 & 0x08;

This is the same as the if statement for nct6106. Please merge into a single
if statement.

> > 	} else {
> > 		/*
> > 		 * NCT6779D, NCT6791D, NCT6792D, NCT6793D, NCT6795D, NCT6796D,
> > @@ -3820,6 +3858,78 @@
> > 		reg_temp_crit_h = NCT6106_REG_TEMP_CRIT_H;
> > 
> > 		break;
> > +	case nct6116:
> > +		data->in_num = 9;
> > +		data->pwm_num = 3;
> > +		data->auto_pwm_num = 4;
> > +		data->temp_fixed_num = 3;
> > +		data->num_temp_alarms = 3;
> > +		data->num_temp_beeps = 3;
> > +
> > +		data->fan_from_reg = fan_from_reg13;
> > +		data->fan_from_reg_min = fan_from_reg13;
> > +
> > +		data->temp_label = nct6776_temp_label;
> > +		data->temp_mask = NCT6776_TEMP_MASK;
> > +		data->virt_temp_mask = NCT6776_VIRT_TEMP_MASK;
> > +
> > +		data->REG_VBAT = NCT6106_REG_VBAT;
> > +		data->REG_DIODE = NCT6106_REG_DIODE;
> > +		data->DIODE_MASK = NCT6106_DIODE_MASK;
> > +		data->REG_VIN = NCT6106_REG_IN;
> > +		data->REG_IN_MINMAX[0] = NCT6106_REG_IN_MIN;
> > +		data->REG_IN_MINMAX[1] = NCT6106_REG_IN_MAX;
> > +		data->REG_TARGET = NCT6106_REG_TARGET;
> > +		data->REG_FAN = NCT6106_REG_FAN;
> > +		data->REG_FAN_MODE = NCT6106_REG_FAN_MODE;
> > +		data->REG_FAN_MIN = NCT6106_REG_FAN_MIN;
> > +		data->REG_FAN_PULSES = NCT6106_REG_FAN_PULSES;
> > +		data->FAN_PULSE_SHIFT = NCT6106_FAN_PULSE_SHIFT;
> > +		data->REG_FAN_TIME[0] = NCT6106_REG_FAN_STOP_TIME;
> > +		data->REG_FAN_TIME[1] = NCT6106_REG_FAN_STEP_UP_TIME;
> > +		data->REG_FAN_TIME[2] = NCT6106_REG_FAN_STEP_DOWN_TIME;
> > +		data->REG_PWM[0] = NCT6106_REG_PWM;
> > +		data->REG_PWM[1] = NCT6106_REG_FAN_START_OUTPUT;
> > +		data->REG_PWM[2] = NCT6106_REG_FAN_STOP_OUTPUT;
> > +		data->REG_PWM[5] = NCT6106_REG_WEIGHT_DUTY_STEP;
> > +		data->REG_PWM[6] = NCT6106_REG_WEIGHT_DUTY_BASE;
> > +		data->REG_PWM_READ = NCT6106_REG_PWM_READ;
> > +		data->REG_PWM_MODE = NCT6106_REG_PWM_MODE;
> > +		data->PWM_MODE_MASK = NCT6106_PWM_MODE_MASK;
> > +		data->REG_AUTO_TEMP = NCT6106_REG_AUTO_TEMP;
> > +		data->REG_AUTO_PWM = NCT6106_REG_AUTO_PWM;
> > +		data->REG_CRITICAL_TEMP = NCT6106_REG_CRITICAL_TEMP;
> > +		data->REG_CRITICAL_TEMP_TOLERANCE
> > +		  = NCT6106_REG_CRITICAL_TEMP_TOLERANCE;
> > +		data->REG_CRITICAL_PWM_ENABLE = NCT6106_REG_CRITICAL_PWM_ENABLE;
> > +		data->CRITICAL_PWM_ENABLE_MASK
> > +		  = NCT6106_CRITICAL_PWM_ENABLE_MASK;
> > +		data->REG_CRITICAL_PWM = NCT6106_REG_CRITICAL_PWM;
> > +		data->REG_TEMP_OFFSET = NCT6106_REG_TEMP_OFFSET;
> > +		data->REG_TEMP_SOURCE = NCT6116_REG_TEMP_SOURCE;
> > +		data->REG_TEMP_SEL = NCT6106_REG_TEMP_SEL;
> > +		data->REG_WEIGHT_TEMP_SEL = NCT6106_REG_WEIGHT_TEMP_SEL;
> > +		data->REG_WEIGHT_TEMP[0] = NCT6106_REG_WEIGHT_TEMP_STEP;
> > +		data->REG_WEIGHT_TEMP[1] = NCT6106_REG_WEIGHT_TEMP_STEP_TOL;
> > +		data->REG_WEIGHT_TEMP[2] = NCT6106_REG_WEIGHT_TEMP_BASE;
> > +		data->REG_ALARM = NCT6106_REG_ALARM;
> > +		data->ALARM_BITS = NCT6116_ALARM_BITS;
> > +		data->REG_BEEP = NCT6106_REG_BEEP;
> > +		data->BEEP_BITS = NCT6106_BEEP_BITS;
> > +
> > +		reg_temp = NCT6116_REG_TEMP;
> > +		reg_temp_mon = NCT6106_REG_TEMP_MON;
> > +		num_reg_temp = ARRAY_SIZE(NCT6116_REG_TEMP);
> > +		num_reg_temp_mon = ARRAY_SIZE(NCT6106_REG_TEMP_MON);
> > +		reg_temp_over = NCT6116_REG_TEMP_OVER;
> > +		reg_temp_hyst = NCT6116_REG_TEMP_HYST;
> > +		reg_temp_config = NCT6116_REG_TEMP_CONFIG;
> > +		reg_temp_alternate = NCT6106_REG_TEMP_ALTERNATE;
> > +		reg_temp_crit = NCT6106_REG_TEMP_CRIT;
> > +		reg_temp_crit_l = NCT6116_REG_TEMP_CRIT_L;
> > +		reg_temp_crit_h = NCT6116_REG_TEMP_CRIT_H;
> > +
> > +		break;
> > 	case nct6775:
> > 		data->in_num = 9;
> > 		data->pwm_num = 3;
> > @@ -4365,6 +4475,7 @@
> > 		data->have_vid = (cr2a & 0x60) == 0x40;
> > 		break;
> > 	case nct6106:
> > +	case nct6116:
> > 	case nct6779:
> > 	case nct6791:
> > 	case nct6792:
> > @@ -4396,6 +4507,9 @@
> > 		case nct6106:
> > 			tmp |= 0xe0;
> > 			break;
> > +		case nct6116:
> > +			tmp |= 0xe0;
> > +			break;
Identical code; please merge with nct6106 case.

> > 		case nct6775:
> > 			tmp |= 0x1e;
> > 			break;
> > @@ -4589,6 +4703,9 @@
> > 	case SIO_NCT6106_ID:
> > 		sio_data->kind = nct6106;
> > 		break;
> > +	case SIO_NCT6116_ID:
> > +		sio_data->kind = nct6116;
> > +		break;
> > 	case SIO_NCT6775_ID:
> > 		sio_data->kind = nct6775;
> > 		break;
> > 
> 



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux