Patchwork hwmon: (it87) Add support for the ITE IT8720F Bug: #357766

login
register
mail settings
Submitter Stefan Bader
Date March 2, 2010, 7:24 p.m.
Message ID <4B8D65F2.2010008@canonical.com>
Download mbox | patch
Permalink /patch/46702/
State Deferred
Delegated to: Stefan Bader
Headers show

Comments

Stefan Bader - March 2, 2010, 7:24 p.m.
<removed old conversation to clean things up>

Hi David,

ok, so this is a bug that still exists in the current upstream code. So this
should be reported there. You can do that yourself or I can forward the patch on
your behalf.

What you also should do is to add a new bug (with "ubuntu-bug linux") because
the report you reference is about adding support for newer chipsets while the
problem seems to have existed before.

One question, for clarification: did you compile yourself a kernel to test
whether the results after the change are what you would expect? If no, we might
provide you a test kernel before proceeding. If yes, this can be documented in
the bug report.

When the patch has been accepted upstream, it can be picked up by upstream
stable maintainers and finally find its way back into the Ubuntu kernel. I know
its a long way. :) But it helps to make sure things get fixed for others too.

Regards,
Stefan
David Partington - March 2, 2010, 7:37 p.m.
On 03/02/2010 01:24 PM, Stefan Bader wrote:
> <removed old conversation to clean things up>
>
> Hi David,
>
> ok, so this is a bug that still exists in the current upstream code. So this
> should be reported there. You can do that yourself or I can forward the patch on
> your behalf.
>
> What you also should do is to add a new bug (with "ubuntu-bug linux") because
> the report you reference is about adding support for newer chipsets while the
> problem seems to have existed before.
>
> One question, for clarification: did you compile yourself a kernel to test
> whether the results after the change are what you would expect? If no, we might
> provide you a test kernel before proceeding. If yes, this can be documented in
> the bug report.
>
> When the patch has been accepted upstream, it can be picked up by upstream
> stable maintainers and finally find its way back into the Ubuntu kernel. I know
> its a long way. :) But it helps to make sure things get fixed for others too.
>
> Regards,
> Stefan
>    
Stephan,

I want to test it, but do not have the necessary .h files.  I do not 
believe the whole kernel needs to be recomplied as the it87 module is a 
.ko (kernel object) file that is loaded via modprobe or via the modules 
file in /etc.  If you can point me to were to get the necessary .h files 
that would be great.  I did download the .git tool and have reviewed the 
Kernel Team Git Guide.  Not being a member of the Kernel team, I did not 
think I could use this approach.

Regards, David
Stefan Bader - March 2, 2010, 8:17 p.m.
David Partington wrote:
> On 03/02/2010 01:24 PM, Stefan Bader wrote:
>> <removed old conversation to clean things up>
>>
>> Hi David,
>>
>> ok, so this is a bug that still exists in the current upstream code.
>> So this
>> should be reported there. You can do that yourself or I can forward
>> the patch on
>> your behalf.
>>
>> What you also should do is to add a new bug (with "ubuntu-bug linux")
>> because
>> the report you reference is about adding support for newer chipsets
>> while the
>> problem seems to have existed before.
>>
>> One question, for clarification: did you compile yourself a kernel to
>> test
>> whether the results after the change are what you would expect? If no,
>> we might
>> provide you a test kernel before proceeding. If yes, this can be
>> documented in
>> the bug report.
>>
>> When the patch has been accepted upstream, it can be picked up by
>> upstream
>> stable maintainers and finally find its way back into the Ubuntu
>> kernel. I know
>> its a long way. :) But it helps to make sure things get fixed for
>> others too.
>>
>> Regards,
>> Stefan
>>    
> Stephan,
> 
> I want to test it, but do not have the necessary .h files.  I do not
> believe the whole kernel needs to be recomplied as the it87 module is a
> .ko (kernel object) file that is loaded via modprobe or via the modules
> file in /etc.  If you can point me to were to get the necessary .h files
> that would be great.  I did download the .git tool and have reviewed the
> Kernel Team Git Guide.  Not being a member of the Kernel team, I did not
> think I could use this approach.
> 
> Regards, David

David,

unfortunately it must be completely recompiled as the kernel uses versioned
function calls. If you want to spend the time and effort you can access the git
trees (you only cannot write back to them). Alternatively I could offer to
compile you a kernel when you create a bug report. ;-)

Regards,
Stefan
Andy Whitcroft - March 5, 2010, 10:58 a.m.
On Tue, Mar 02, 2010 at 01:37:48PM -0600, David Partington wrote:

> I want to test it, but do not have the necessary .h files.  I do not 
> believe the whole kernel needs to be recomplied as the it87 module is a 
> .ko (kernel object) file that is loaded via modprobe or via the modules 
> file in /etc.  If you can point me to were to get the necessary .h files 
> that would be great.  I did download the .git tool and have reviewed the 
> Kernel Team Git Guide.  Not being a member of the Kernel team, I did not 
> think I could use this approach.

Did you manage to test Stefan's patch?  It is unclear whether you needed
a test kernel made with this applied.  Please let us know how you got on.

-apw
David Partington - March 6, 2010, 6:45 p.m.

Patch

To: Jean Delvare <khali@linux-fr.org>
Cc: lm-sensors@lm-sensors.org, linux-kernel@vger.kernel.org

From a4aef2350f79db5c27c8c04c0d9a0ef0f222656e Mon Sep 17 00:00:00 2001
From: David Partington <david@partington.com>
Date: Tue, 2 Mar 2010 19:46:44 +0100
Subject: [PATCH] ihwmon: (it87) Fix VID register access for it8718 and it8720

The comments on the register definitions for the VID register indicates
that IT8718 and IT8720 use a different register (IT87_REG_VID) while the
other sensors use IT87_SIO_VID_REG.
But the code that accesses the register uses the names swaped around. So
the VID value of a it8716 device has been found incorrect.

Signed-off-by: David Partington <david@partington.com>
CC: stable@kernel.org
---
 drivers/hwmon/it87.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
index 0ffe84d..aaf3e2c 100644
--- a/drivers/hwmon/it87.c
+++ b/drivers/hwmon/it87.c
@@ -1061,7 +1061,7 @@  static int __init it87_find(unsigned short *address,
 
 		if ((sio_data->type == it8718 || sio_data->type == it8720)
 		 && !(sio_data->skip_vid))
-			sio_data->vid_value = superio_inb(IT87_SIO_VID_REG);
+			sio_data->vid_value = superio_inb(IT87_REG_VID);
 
 		reg = superio_inb(IT87_SIO_PINX2_REG);
 		if (reg & (1 << 0))
@@ -1557,10 +1557,10 @@  static struct it87_data *it87_update_device(struct device *dev)
 
 		data->sensor = it87_read_value(data, IT87_REG_TEMP_ENABLE);
 		/* The 8705 does not have VID capability.
-		   The 8718 and the 8720 don't use IT87_REG_VID for the
+		   The 8718 and the 8720 don't use IT87_SIO_VID_REG for the
 		   same purpose. */
 		if (data->type == it8712 || data->type == it8716) {
-			data->vid = it87_read_value(data, IT87_REG_VID);
+			data->vid = it87_read_value(data, IT87_SIO_VID_REG);
 			/* The older IT8712F revisions had only 5 VID pins,
 			   but we assume it is always safe to read 6 bits. */
 			data->vid &= 0x3f;
-- 
1.6.3.3