Re: [PATCH v3 1/7] hwmon: (it87) Allow disabling exiting of configuration mode

[Guenter Roeck <linux@roeck-us.net>]

On 1/29/23 12:56, Guenter Roeck wrote:
> On Sat, Jan 28, 2023 at 05:03:02PM +1100, Frank Crawford wrote:
>> Disabling configuration mode on some chips can result in system
>> hang-ups and access failures to the Super-IO chip at the
>> second SIO address. Never exit configuration mode on these
>> chips to avoid the problem.
>>
>> This patch should be applied in conjunction with a previous one to
>> initialise the second chip for certain mother boards.
>>
>> Signed-off-by: Frank Crawford <frank@crawford.emu.id.au>
>> ---
>>
>> v3:
>>   * Correct possible uninitialised pointer issue.
>>
>> v2:
>>   * Convert to use feature flag and related macros rather than a separate
>>     field, as suggested in review.
>>   * Reverse sense of flag in superio_exit to simplify feature macro.
>>
>> ---
>>   drivers/hwmon/it87.c | 26 ++++++++++++++++++--------
>>   1 file changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
>> index a8a6a0ffee82..923a9563be92 100644
>> --- a/drivers/hwmon/it87.c
>> +++ b/drivers/hwmon/it87.c
>> @@ -128,10 +128,12 @@ static inline int superio_enter(int ioreg)
>>   	return 0;
>>   }
>>   
>> -static inline void superio_exit(int ioreg)
>> +static inline void superio_exit(int ioreg, bool noexit)
>>   {
>> -	outb(0x02, ioreg);
>> -	outb(0x02, ioreg + 1);
>> +	if (!noexit) {
>> +		outb(0x02, ioreg);
>> +		outb(0x02, ioreg + 1);
>> +	}
>>   	release_region(ioreg, 2);
>>   }
>>   
>> @@ -300,6 +302,13 @@ struct it87_devices {
>>   #define FEAT_PWM_FREQ2		BIT(16)	/* Separate pwm freq 2 */
>>   #define FEAT_SIX_TEMP		BIT(17)	/* Up to 6 temp sensors */
>>   #define FEAT_VIN3_5V		BIT(18)	/* VIN3 connected to +5V */
>> +/*
>> + * Disabling configuration mode on some chips can result in system
>> + * hang-ups and access failures to the Super-IO chip at the
>> + * second SIO address. Never exit configuration mode on these
>> + * chips to avoid the problem.
>> + */
>> +#define FEAT_CONF_NOEXIT	BIT(28)	/* Chip should not exit conf mode */
> 
> Feature bits are supposed to be numbered sequentially, not randomly
> assigned. Please use bit 19.
> 

Given that the commit window is coming up, and since I don't have
other issues with your series, I am inclined to apply it with the
bit changed. Please let me know in the next day or two if this is
acceptable for you. If not, the series will have to wait until v6.4.

Thanks,
Guenter