it87 causes VIA hardware to lockup

it87 causes VIA hardware to lockup

[Russell King - ARM Linux]

Hi,

I recently upgraded a machine that backs up data at a club from a
2.6.30 based distribution to 4.8.0 based distribution, and I find
that modprobing the it87 sensor driver on 4.8.0 causes the machine
to lockup.

Unfortunately, as far as I'm aware, there is no kernel oops or
kernel messages, it just locks the machine up such that it requires
a power cycle.

With 2.6.30, it was possible to modprobe the driver and lm_sensors
would then read out the fan and temperature information (which is
necessary to monitor the head-less machine that is otherwise buried
in an area that hardly anyone goes.)

it87-isa-0290
Adapter: ISA adapter
in0:         +1.60 V  (min =  +0.00 V, max =  +4.08 V)
in1:         +2.50 V  (min =  +0.00 V, max =  +4.08 V)
in2:         +3.34 V  (min =  +0.00 V, max =  +4.08 V)
in3:         +2.29 V  (min =  +0.00 V, max =  +4.08 V)
in4:         +3.14 V  (min =  +0.00 V, max =  +4.08 V)
in5:         +2.72 V  (min =  +0.00 V, max =  +4.08 V)
in6:         +2.11 V  (min =  +0.00 V, max =  +4.08 V)
in7:         +2.10 V  (min =  +0.00 V, max =  +4.08 V)
Vbat:        +0.00 V
fan1:       3245 RPM  (min =    0 RPM, div = 8)
fan2:          0 RPM  (min =    0 RPM, div = 8)
temp1:        +9.0 C  (low  = +127.0 C, high = +127.0 C)  sensor = thermistor
temp2:        +5.0 C  (low  = +127.0 C, high = +127.0 C)  sensor = thermistor
temp3:       +27.0 C  (low  = +127.0 C, high = +127.0 C)  sensor = thermal diode

Looking at the changes between the two kernels, I'd imagine that
unconditionally touching the registers at 0x4e/0x4f (which the
driver never used to do) is upsetting the bridges, causing the
machine to lock up.

Unfortuantely, I'm unable to "play" with the machine, as my only
access to it is remotely, and locking the machine up at this stage
would mean (a) that it's no longer able to do its job, and (b) it
would require visiting the machine physically to power cycle it.

Obviously, bisecting over such a large range of kernels is also
out as that would require causing the machine to lockup in order
to test failure, something I can't risk remotely.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: it87 causes VIA hardware to lockup

[Guenter Roeck]

On Sun, Mar 12, 2017 at 10:34:13AM +0000, Russell King - ARM Linux wrote:
> Hi,
> 
> I recently upgraded a machine that backs up data at a club from a
> 2.6.30 based distribution to 4.8.0 based distribution, and I find
> that modprobing the it87 sensor driver on 4.8.0 causes the machine
> to lockup.
> 
> Unfortunately, as far as I'm aware, there is no kernel oops or
> kernel messages, it just locks the machine up such that it requires
> a power cycle.
> 
> With 2.6.30, it was possible to modprobe the driver and lm_sensors
> would then read out the fan and temperature information (which is
> necessary to monitor the head-less machine that is otherwise buried
> in an area that hardly anyone goes.)
> 
> it87-isa-0290
> Adapter: ISA adapter
> in0:         +1.60 V  (min =  +0.00 V, max =  +4.08 V)
> in1:         +2.50 V  (min =  +0.00 V, max =  +4.08 V)
> in2:         +3.34 V  (min =  +0.00 V, max =  +4.08 V)
> in3:         +2.29 V  (min =  +0.00 V, max =  +4.08 V)
> in4:         +3.14 V  (min =  +0.00 V, max =  +4.08 V)
> in5:         +2.72 V  (min =  +0.00 V, max =  +4.08 V)
> in6:         +2.11 V  (min =  +0.00 V, max =  +4.08 V)
> in7:         +2.10 V  (min =  +0.00 V, max =  +4.08 V)
> Vbat:        +0.00 V
> fan1:       3245 RPM  (min =    0 RPM, div = 8)
> fan2:          0 RPM  (min =    0 RPM, div = 8)
> temp1:        +9.0 C  (low  = +127.0 C, high = +127.0 C)  sensor = thermistor
> temp2:        +5.0 C  (low  = +127.0 C, high = +127.0 C)  sensor = thermistor
> temp3:       +27.0 C  (low  = +127.0 C, high = +127.0 C)  sensor = thermal diode
> 
> Looking at the changes between the two kernels, I'd imagine that
> unconditionally touching the registers at 0x4e/0x4f (which the
> driver never used to do) is upsetting the bridges, causing the
> machine to lock up.
> 
I found some reference suggesting that The IT8705F may respond on
both SIO addresses. Can you try the following patch ?

Guenter

---
>From 97ec1bb3274914b07ffb01a2c5d3501fee9af8c4 Mon Sep 17 00:00:00 2001
From: Guenter Roeck <linux@roeck-us.net>
Date: Sun, 12 Mar 2017 06:18:58 -0700
Subject: [PATCH] hwmon: (it87) Don't check second SIO address for older chips

IT8705F is known to respond on both SIO addresses. Stop scanning for
chips after finding one.

Fixes: e84bd9535e2b ("hwmon: (it87) Add support for second Super-IO chip")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/it87.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
index 3e57a6120551..cce0683cf2fd 100644
--- a/drivers/hwmon/it87.c
+++ b/drivers/hwmon/it87.c
@@ -3215,7 +3215,15 @@ static int __init sm_it87_init(void)
 		err = it87_device_add(i, isa_address, &sio_data);
 		if (err)
 			goto exit_dev_unregister;
+
 		found = true;
+
+		/*
+		 * Don't continue searching after finding an older chip
+		 * which may respond on both SIO addresses.
+		 */
+		if (sio_data.type == it87)
+			break;
 	}
 
 	if (!found) {
-- 
2.7.4

Re: it87 causes VIA hardware to lockup

[Russell King - ARM Linux]

On Sun, Mar 12, 2017 at 06:25:44AM -0700, Guenter Roeck wrote:
> I found some reference suggesting that The IT8705F may respond on
> both SIO addresses. Can you try the following patch ?

Thanks for the patch - it'll take about a week or so for me to test,
as I can only risk testing it when I'm physically at the machine.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: it87 causes VIA hardware to lockup

[Guenter Roeck]

On Sun, Mar 12, 2017 at 02:15:22PM +0000, Russell King - ARM Linux wrote:
> On Sun, Mar 12, 2017 at 06:25:44AM -0700, Guenter Roeck wrote:
> > I found some reference suggesting that The IT8705F may respond on
> > both SIO addresses. Can you try the following patch ?
> 
> Thanks for the patch - it'll take about a week or so for me to test,
> as I can only risk testing it when I'm physically at the machine.
> 

My pleasure. Sorry for the trouble. I attached a more comprehensive version
of the patch.

Thanks,
Guenter

---
>From 5b3f92a9e0df49a111a96f43130d2cee370e4c7a Mon Sep 17 00:00:00 2001
From: Guenter Roeck <linux@roeck-us.net>
Date: Sun, 12 Mar 2017 06:18:58 -0700
Subject: [PATCH] hwmon: (it87) Avoid registering the same chip on both SIO
 addresses

IT8705F is known to respond on both SIO addresses. Other chips may have
the same behavior. Add checks to avoid registering the same chip twice.

Fixes: e84bd9535e2b ("hwmon: (it87) Add support for second Super-IO chip")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/it87.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
index 3e57a6120551..7053968fe0d3 100644
--- a/drivers/hwmon/it87.c
+++ b/drivers/hwmon/it87.c
@@ -3197,7 +3197,7 @@ static int __init sm_it87_init(void)
 {
 	int sioaddr[2] = { REG_2E, REG_4E };
 	struct it87_sio_data sio_data;
-	unsigned short isa_address;
+	unsigned short isa_address[2];
 	bool found = false;
 	int i, err;
 
@@ -3207,15 +3207,29 @@ static int __init sm_it87_init(void)
 
 	for (i = 0; i < ARRAY_SIZE(sioaddr); i++) {
 		memset(&sio_data, 0, sizeof(struct it87_sio_data));
-		isa_address = 0;
-		err = it87_find(sioaddr[i], &isa_address, &sio_data);
-		if (err || isa_address == 0)
+		isa_address[i] = 0;
+		err = it87_find(sioaddr[i], &isa_address[i], &sio_data);
+		if (err || isa_address[i] == 0)
 			continue;
+		/*
+		 * Don't register second chip if its ISA address matches
+		 * the first chip's ISA address.
+		 */
+		if (i && isa_address[i] == isa_address[0])
+			break;
 
-		err = it87_device_add(i, isa_address, &sio_data);
+		err = it87_device_add(i, isa_address[i], &sio_data);
 		if (err)
 			goto exit_dev_unregister;
+
 		found = true;
+
+		/*
+		 * IT8705F may respond on both SIO addresses.
+		 * Stop probing after finding one.
+		 */
+		if (sio_data.type == it87)
+			break;
 	}
 
 	if (!found) {
-- 
2.7.4

Re: it87 causes VIA hardware to lockup

[Guenter Roeck]

On Sun, Mar 12, 2017 at 07:26:34AM -0700, Guenter Roeck wrote:
> On Sun, Mar 12, 2017 at 02:15:22PM +0000, Russell King - ARM Linux wrote:
> > On Sun, Mar 12, 2017 at 06:25:44AM -0700, Guenter Roeck wrote:
> > > I found some reference suggesting that The IT8705F may respond on
> > > both SIO addresses. Can you try the following patch ?
> > 
> > Thanks for the patch - it'll take about a week or so for me to test,
> > as I can only risk testing it when I'm physically at the machine.
> > 
> 
> My pleasure. Sorry for the trouble. I attached a more comprehensive version
> of the patch.
> 
Hi Russell,

did you by any chance have time to test the patch ? I would like to send
the fix upstream, but I held it back in the hope to get feedback if it
actually solves the problem.

Thanks,
Guenter

> Thanks,
> Guenter
> 
> ---
> From 5b3f92a9e0df49a111a96f43130d2cee370e4c7a Mon Sep 17 00:00:00 2001
> From: Guenter Roeck <linux@roeck-us.net>
> Date: Sun, 12 Mar 2017 06:18:58 -0700
> Subject: [PATCH] hwmon: (it87) Avoid registering the same chip on both SIO
>  addresses
> 
> IT8705F is known to respond on both SIO addresses. Other chips may have
> the same behavior. Add checks to avoid registering the same chip twice.
> 
> Fixes: e84bd9535e2b ("hwmon: (it87) Add support for second Super-IO chip")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/hwmon/it87.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index 3e57a6120551..7053968fe0d3 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -3197,7 +3197,7 @@ static int __init sm_it87_init(void)
>  {
>  	int sioaddr[2] = { REG_2E, REG_4E };
>  	struct it87_sio_data sio_data;
> -	unsigned short isa_address;
> +	unsigned short isa_address[2];
>  	bool found = false;
>  	int i, err;
>  
> @@ -3207,15 +3207,29 @@ static int __init sm_it87_init(void)
>  
>  	for (i = 0; i < ARRAY_SIZE(sioaddr); i++) {
>  		memset(&sio_data, 0, sizeof(struct it87_sio_data));
> -		isa_address = 0;
> -		err = it87_find(sioaddr[i], &isa_address, &sio_data);
> -		if (err || isa_address == 0)
> +		isa_address[i] = 0;
> +		err = it87_find(sioaddr[i], &isa_address[i], &sio_data);
> +		if (err || isa_address[i] == 0)
>  			continue;
> +		/*
> +		 * Don't register second chip if its ISA address matches
> +		 * the first chip's ISA address.
> +		 */
> +		if (i && isa_address[i] == isa_address[0])
> +			break;
>  
> -		err = it87_device_add(i, isa_address, &sio_data);
> +		err = it87_device_add(i, isa_address[i], &sio_data);
>  		if (err)
>  			goto exit_dev_unregister;
> +
>  		found = true;
> +
> +		/*
> +		 * IT8705F may respond on both SIO addresses.
> +		 * Stop probing after finding one.
> +		 */
> +		if (sio_data.type == it87)
> +			break;
>  	}
>  
>  	if (!found) {
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: it87 causes VIA hardware to lockup

[Russell King - ARM Linux]

On Tue, Mar 21, 2017 at 06:44:30AM -0700, Guenter Roeck wrote:
> On Sun, Mar 12, 2017 at 07:26:34AM -0700, Guenter Roeck wrote:
> > On Sun, Mar 12, 2017 at 02:15:22PM +0000, Russell King - ARM Linux wrote:
> > > On Sun, Mar 12, 2017 at 06:25:44AM -0700, Guenter Roeck wrote:
> > > > I found some reference suggesting that The IT8705F may respond on
> > > > both SIO addresses. Can you try the following patch ?
> > > 
> > > Thanks for the patch - it'll take about a week or so for me to test,
> > > as I can only risk testing it when I'm physically at the machine.
> > > 
> > 
> > My pleasure. Sorry for the trouble. I attached a more comprehensive version
> > of the patch.
> > 
> Hi Russell,
> 
> did you by any chance have time to test the patch ? I would like to send
> the fix upstream, but I held it back in the hope to get feedback if it
> actually solves the problem.

Sorry, I haven't, and given my current situation, I'm unlikely to be
able to for quite some time (medical issues).

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: it87 causes VIA hardware to lockup

[Guenter Roeck]

On Tue, Mar 21, 2017 at 02:08:12PM +0000, Russell King - ARM Linux wrote:
> On Tue, Mar 21, 2017 at 06:44:30AM -0700, Guenter Roeck wrote:
> > On Sun, Mar 12, 2017 at 07:26:34AM -0700, Guenter Roeck wrote:
> > > On Sun, Mar 12, 2017 at 02:15:22PM +0000, Russell King - ARM Linux wrote:
> > > > On Sun, Mar 12, 2017 at 06:25:44AM -0700, Guenter Roeck wrote:
> > > > > I found some reference suggesting that The IT8705F may respond on
> > > > > both SIO addresses. Can you try the following patch ?
> > > > 
> > > > Thanks for the patch - it'll take about a week or so for me to test,
> > > > as I can only risk testing it when I'm physically at the machine.
> > > > 
> > > 
> > > My pleasure. Sorry for the trouble. I attached a more comprehensive version
> > > of the patch.
> > > 
> > Hi Russell,
> > 
> > did you by any chance have time to test the patch ? I would like to send
> > the fix upstream, but I held it back in the hope to get feedback if it
> > actually solves the problem.
> 
> Sorry, I haven't, and given my current situation, I'm unlikely to be
> able to for quite some time (medical issues).
> 
Sorry to hear that. Hope you'll get better soon.

I'll submit the patch as-is upstream; at least it doesn't break anything.
If it doesn't fix your problem, we'll have to look at it again at a later
point.

Thanks,
Guenter

Re: it87 causes VIA hardware to lockup

[Jean Delvare]

Hi Guenter,

On Tue, 21 Mar 2017 10:05:03 -0700, Guenter Roeck wrote:
> I'll submit the patch as-is upstream; at least it doesn't break anything.
> If it doesn't fix your problem, we'll have to look at it again at a later
> point.

Given that this patch fixes a regression in kernels v4.7 to v.4.10,
shouldn't it go to stable@?

As a side note, I think the second half of the patch is redundant, it
only makes registration slightly faster on IT8705F, and could have bad
side effects at least in theory. The first half seems sufficient to
me...

-- 
Jean Delvare
SUSE L3 Support

Re: it87 causes VIA hardware to lockup

[Guenter Roeck]

On Sun, Apr 09, 2017 at 03:38:06PM +0200, Jean Delvare wrote:
> Hi Guenter,
> 
> On Tue, 21 Mar 2017 10:05:03 -0700, Guenter Roeck wrote:
> > I'll submit the patch as-is upstream; at least it doesn't break anything.
> > If it doesn't fix your problem, we'll have to look at it again at a later
> > point.
> 
> Given that this patch fixes a regression in kernels v4.7 to v.4.10,
> shouldn't it go to stable@?
> 
The patch has a Fixes: tag, so that should happen automatically. 
I'll have to check if it applies cleanly to earlier kernels and if
necessary send backport(s) to Greg.

> As a side note, I think the second half of the patch is redundant, it
> only makes registration slightly faster on IT8705F, and could have bad
> side effects at least in theory. The first half seems sufficient to
> me...
> 
It only affects systems with two Super-IO chips, and I wanted to play safe. The
worst side effect I can imagine would be that a second chip in a system with
IT8705 as first chip would not be accepted, which is not worse than before
when only one chip was supported.

Thanks,
Guenter

Re: it87 causes VIA hardware to lockup

[Jean Delvare]

Hi Guenter,

On Sun, 9 Apr 2017 08:24:07 -0700, Guenter Roeck wrote:
> On Sun, Apr 09, 2017 at 03:38:06PM +0200, Jean Delvare wrote:
> > On Tue, 21 Mar 2017 10:05:03 -0700, Guenter Roeck wrote:
> > > I'll submit the patch as-is upstream; at least it doesn't break anything.
> > > If it doesn't fix your problem, we'll have to look at it again at a later
> > > point.
> > 
> > Given that this patch fixes a regression in kernels v4.7 to v.4.10,
> > shouldn't it go to stable@?
>
> The patch has a Fixes: tag, so that should happen automatically. 
> I'll have to check if it applies cleanly to earlier kernels and if
> necessary send backport(s) to Greg.

I took a look at stable branches v4.9 and v4.10 and I can't find this
fix. Do you still plan to check if the fix applies and poke Greg about
it? Or do you want me to do it?

> > As a side note, I think the second half of the patch is redundant, it
> > only makes registration slightly faster on IT8705F, and could have bad
> > side effects at least in theory. The first half seems sufficient to
> > me...
>
> It only affects systems with two Super-IO chips, and I wanted to play safe. The
> worst side effect I can imagine would be that a second chip in a system with
> IT8705 as first chip would not be accepted, which is not worse than before
> when only one chip was supported.

But isn't as good as doing the right thing, which would require less
code. So I don't really follow your logic.

-- 
Jean Delvare
SUSE L3 Support

Re: it87 causes VIA hardware to lockup

[Guenter Roeck]

Hi Jean,

On 04/25/2017 06:30 AM, Jean Delvare wrote:
> Hi Guenter,
>
> On Sun, 9 Apr 2017 08:24:07 -0700, Guenter Roeck wrote:
>> On Sun, Apr 09, 2017 at 03:38:06PM +0200, Jean Delvare wrote:
>>> On Tue, 21 Mar 2017 10:05:03 -0700, Guenter Roeck wrote:
>>>> I'll submit the patch as-is upstream; at least it doesn't break anything.
>>>> If it doesn't fix your problem, we'll have to look at it again at a later
>>>> point.
>>>
>>> Given that this patch fixes a regression in kernels v4.7 to v.4.10,
>>> shouldn't it go to stable@?
>>
>> The patch has a Fixes: tag, so that should happen automatically.
>> I'll have to check if it applies cleanly to earlier kernels and if
>> necessary send backport(s) to Greg.
>
> I took a look at stable branches v4.9 and v4.10 and I can't find this
> fix. Do you still plan to check if the fix applies and poke Greg about
> it? Or do you want me to do it?
>

Severe lack of time on my side, sorry. Feel free to do it if you have some slack.

>>> As a side note, I think the second half of the patch is redundant, it
>>> only makes registration slightly faster on IT8705F, and could have bad
>>> side effects at least in theory. The first half seems sufficient to
>>> me...
>>
>> It only affects systems with two Super-IO chips, and I wanted to play safe. The
>> worst side effect I can imagine would be that a second chip in a system with
>> IT8705 as first chip would not be accepted, which is not worse than before
>> when only one chip was supported.
>
> But isn't as good as doing the right thing, which would require less
> code. So I don't really follow your logic.
>

Call it paranoia. I have not received feedback if the patch is actually working,
and I am unable to test it myself. Feel free to send a patch to remove the redundant
part if you think it should be dropped.

Thanks,
Guenter

Re: it87 causes VIA hardware to lockup

[Jean Delvare]

Hi Guenter,

On Tue, 25 Apr 2017 06:49:27 -0700, Guenter Roeck wrote:
> On 04/25/2017 06:30 AM, Jean Delvare wrote:
> > I took a look at stable branches v4.9 and v4.10 and I can't find this
> > fix. Do you still plan to check if the fix applies and poke Greg about
> > it? Or do you want me to do it?
> 
> Severe lack of time on my side, sorry. Feel free to do it if you have some slack.

I finally found some time to look into this. The fix applies just fine
to kernel branches 4.9 and 4.10, so I asked stable@ to pick it.

> > (...)
> > But isn't as good as doing the right thing, which would require less
> > code. So I don't really follow your logic.
> 
> Call it paranoia. I have not received feedback if the patch is actually working,
> and I am unable to test it myself. Feel free to send a patch to remove the redundant
> part if you think it should be dropped.

Yes I think, and yes I will.

-- 
Jean Delvare
SUSE L3 Support