FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Ubuntu > Ubuntu Kernel Team

 
 
LinkBack Thread Tools
 
Old 02-26-2010, 04:40 PM
David Partington
 
Default hwmon: (it87) Add support for the ITE IT8720F Bug: #357766

Kernel team:


I have a Biostar motherboard with the it8716 superio device. When I
execute the sensors hardware monitor the VID values is incorrect, In
reviewing the it87.c source I fine the following code burst:

126 /* Logical device 7 registers (IT8712F and later) */
127 #define IT87_SIO_GPIO3_REG 0x27
128 #define IT87_SIO_GPIO5_REG 0x29
129 #define IT87_SIO_PINX2_REG 0x2c /* Pin selection */
130 #define IT87_SIO_VID_REG 0xfc /* VID value */



161 /* The IT8718F and IT8720F have the VID value in a different
register, in
162 Super-I/O configuration space. */
163 #define IT87_REG_VID 0x0a


The current logic is as follows:


1061
1062 if ((sio_data->type == it8718 || sio_data->type ==
it8720)
1063 && !(sio_data->skip_vid))
1064 sio_data->vid_value =
superio_inb(IT87_SIO_VID_REG);
1065



1559 /* The 8705 does not have VID capability.
1560 The 8718 and the 8720 don't use IT87_REG_VID for the
1561 same purpose. */
1562 if (data->type == it8712 || data->type == it8716) {
1563 data->vid = it87_read_value(data,
IT87_REG_VID);
1564 /* The older IT8712F revisions had only 5
VID pins,
1565 but we assume it is always safe to read
6 bits. */
1566 data->vid &= 0x3f;
1567 }


Based upon the definitions, the two sections of code above are incorrect
and should be:


1061
1062 if ((sio_data->type == it8718 || sio_data->type ==
it8720)
1063 && !(sio_data->skip_vid))
1064 sio_data->vid_value =
superio_inb(IT87_REG_VID);
1065



1559 /* The 8705 does not have VID capability.
1560 The 8718 and the 8720 don't use IT87_REG_VID for the
1561 same purpose. */
1562 if (data->type == it8712 || data->type == it8716) {
1563 data->vid = it87_read_value(data,
IT87_SIO_VID_REG);
1564 /* The older IT8712F revisions had only 5
VID pins,
1565 but we assume it is always safe to read
6 bits. */
1566 data->vid &= 0x3f;
1567 }


Regards, David Partington



--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 03-01-2010, 12:42 PM
Tim Gardner
 
Default hwmon: (it87) Add support for the ITE IT8720F Bug: #357766

David Partington wrote:
> Kernel team:
>
>
> I have a Biostar motherboard with the it8716 superio device. When I
> execute the sensors hardware monitor the VID values is incorrect, In
> reviewing the it87.c source I fine the following code burst:
>

<snip>

Well, you've gotten this far. How about a proper patch?

--
Tim Gardner tim.gardner@canonical.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 03-01-2010, 01:39 PM
David Partington
 
Default hwmon: (it87) Add support for the ITE IT8720F Bug: #357766

Tim Gardner wrote:

David Partington wrote:


Kernel team:


I have a Biostar motherboard with the it8716 superio device. When I
execute the sensors hardware monitor the VID values is incorrect, In
reviewing the it87.c source I fine the following code burst:




<snip>

Well, you've gotten this far. How about a proper patch?



Tim,



Thanks for your reply.* Actually in the code submitted at the bottom of
my email was the suggested change.* However, I did submit an actual
defect report with the suggested fix as well.* Please see new defect
#528741.



Regards, David



--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 03-01-2010, 01:49 PM
Tim Gardner
 
Default hwmon: (it87) Add support for the ITE IT8720F Bug: #357766

David Partington wrote:
> Tim Gardner wrote:
>> David Partington wrote:
>>
>>> Kernel team:
>>>
>>>
>>> I have a Biostar motherboard with the it8716 superio device. When I
>>> execute the sensors hardware monitor the VID values is incorrect, In
>>> reviewing the it87.c source I fine the following code burst:
>>>
>>>
>>
>> <snip>
>>
>> Well, you've gotten this far. How about a proper patch?
>>
>>
> Tim,
>
> Thanks for your reply. Actually in the code submitted at the bottom of
> my email was the suggested change. However, I did submit an actual
> defect report with the suggested fix as well. Please see new defect
> #528741.
>
> Regards, David
>

The code at the bottom of
https://lists.ubuntu.com/archives/kernel-team/2010-February/009082.html
is not a proper patch wrt the Linux kernel ecosystem. See
Documentation/SubmittingPatches in Linus' kernel tree at
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git.

The advantage of using diff and patch is the avoidance of transcription
errors when applying patches.

rtg
--
Tim Gardner tim.gardner@canonical.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 03-01-2010, 02:26 PM
David Partington
 
Default hwmon: (it87) Add support for the ITE IT8720F Bug: #357766

Tim Gardner wrote:

David Partington wrote:


Tim Gardner wrote:


David Partington wrote:



Kernel team:


I have a Biostar motherboard with the it8716 superio device. When I
execute the sensors hardware monitor the VID values is incorrect, In
reviewing the it87.c source I fine the following code burst:




<snip>

Well, you've gotten this far. How about a proper patch?




Tim,

Thanks for your reply. Actually in the code submitted at the bottom of
my email was the suggested change. However, I did submit an actual
defect report with the suggested fix as well. Please see new defect
#528741.

Regards, David




The code at the bottom of
https://lists.ubuntu.com/archives/kernel-team/2010-February/009082.html
is not a proper patch wrt the Linux kernel ecosystem. See
Documentation/SubmittingPatches in Linus' kernel tree at
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git.

The advantage of using diff and patch is the avoidance of transcription
errors when applying patches.

rtg


Okay.



I have never submitted a kernel patch before - so you are correct, I
probably did not follow a recommended procedure.* I thought that rather
than complaining about something that does not work, I endeavored to
show how to fix the issue.



Regards, David



--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 03-02-2010, 12:58 PM
Stefan Bader
 
Default hwmon: (it87) Add support for the ITE IT8720F Bug: #357766

David Partington wrote:
> Tim Gardner wrote:
>> David Partington wrote:
>>
>>> Tim Gardner wrote:
>>>
>>>> David Partington wrote:
>>>>
>>>>
>>>>> Kernel team:
>>>>>
>>>>>
>>>>> I have a Biostar motherboard with the it8716 superio device. When I
>>>>> execute the sensors hardware monitor the VID values is incorrect, In
>>>>> reviewing the it87.c source I fine the following code burst:
>>>>>
>>>>>
>>>> <snip>
>>>>
>>>> Well, you've gotten this far. How about a proper patch?
>>>>
>>>>
>>> Tim,
>>>
>>> Thanks for your reply. Actually in the code submitted at the bottom of
>>> my email was the suggested change. However, I did submit an actual
>>> defect report with the suggested fix as well. Please see new defect
>>> #528741.
>>>
>>> Regards, David
>>>
>>>
>>
>> The code at the bottom of
>> https://lists.ubuntu.com/archives/kernel-team/2010-February/009082.html
>> is not a proper patch wrt the Linux kernel ecosystem. See
>> Documentation/SubmittingPatches in Linus' kernel tree at
>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git.
>>
>> The advantage of using diff and patch is the avoidance of transcription
>> errors when applying patches.
>>
>> rtg
>>
> Okay.
>
> I have never submitted a kernel patch before - so you are correct, I
> probably did not follow a recommended procedure. I thought that rather
> than complaining about something that does not work, I endeavored to
> show how to fix the issue.
>
> Regards, David
>

Hi David,

the issue is not so much that you did not follow a procedure, but that you can
help greatly by doing it to get the fix taken forward without needing too much
time and being prone to errors.
If you for the approach have a copy of the file as it is now and one copy of it
as it should be, then a diff -up <old file> <new file> >mypatch.patch gives you
a file which can be taken an used to make the exact same changes as you did.
Following a textual description, it is just too easy to get things wrong.

Cheers,
Stefan

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 03-02-2010, 05:19 PM
David Partington
 
Default hwmon: (it87) Add support for the ITE IT8720F Bug: #357766

On 03/02/2010 07:58 AM, Stefan Bader wrote:

David Partington wrote:


Tim Gardner wrote:


David Partington wrote:



Tim Gardner wrote:



David Partington wrote:




Kernel team:


I have a Biostar motherboard with the it8716 superio device. When I
execute the sensors hardware monitor the VID values is incorrect, In
reviewing the it87.c source I fine the following code burst:




<snip>

Well, you've gotten this far. How about a proper patch?




Tim,

Thanks for your reply. Actually in the code submitted at the bottom of
my email was the suggested change. However, I did submit an actual
defect report with the suggested fix as well. Please see new defect
#528741.

Regards, David




The code at the bottom of
https://lists.ubuntu.com/archives/kernel-team/2010-February/009082.html
is not a proper patch wrt the Linux kernel ecosystem. See
Documentation/SubmittingPatches in Linus' kernel tree at
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git.

The advantage of using diff and patch is the avoidance of transcription
errors when applying patches.

rtg



Okay.

I have never submitted a kernel patch before - so you are correct, I
probably did not follow a recommended procedure. I thought that rather
than complaining about something that does not work, I endeavored to
show how to fix the issue.

Regards, David



Hi David,

the issue is not so much that you did not follow a procedure, but that you can
help greatly by doing it to get the fix taken forward without needing too much
time and being prone to errors.
If you for the approach have a copy of the file as it is now and one copy of it
as it should be, then a diff -up<old file> <new file> >mypatch.patch gives you
a file which can be taken an used to make the exact same changes as you did.
Following a textual description, it is just too easy to get things wrong.

Cheers,
Stefan



Stephan,

Thanks for your reply. I certainly do want to help as much as possible
and save time for the team.


Per your suggestion I created a diff file to show the suggested
changes. This diff file is attached.


Regards, David
--- it87.c-old 2010-03-02 11:56:04.095882165 -0600
+++ it87.c-new 2010-03-02 12:04:07.837135796 -0600
@@ -1061,7 +1061,7 @@
1061
1062 if ((sio_data->type == it8718 || sio_data->type == it8720)
1063 && !(sio_data->skip_vid))
-1064 sio_data->vid_value = superio_inb(IT87_SIO_VID_REG);
+1064 sio_data->vid_value = superio_inb(IT87_REG_VID);
1065
1066 reg = superio_inb(IT87_SIO_PINX2_REG);
1067 if (reg & (1 << 0))
@@ -1557,10 +1557,10 @@
1557
1558 data->sensor = it87_read_value(data, IT87_REG_TEMP_ENABLE);
1559 /* The 8705 does not have VID capability.
-1560 The 8718 and the 8720 don't use IT87_REG_VID for the
+1560 The 8718 and the 8720 don't use IT87_SIO_VID_REG for the
1561 same purpose. */
1562 if (data->type == it8712 || data->type == it8716) {
-1563 data->vid = it87_read_value(data, IT87_REG_VID);
+1563 data->vid = it87_read_value(data, IT87_SIO_VID_REG);
1564 /* The older IT8712F revisions had only 5 VID pins,
1565 but we assume it is always safe to read 6 bits. */
1566 data->vid &= 0x3f;
--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 03-02-2010, 06:24 PM
Stefan Bader
 
Default hwmon: (it87) Add support for the ITE IT8720F Bug: #357766

This is a multi-part message in MIME format.<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
 
Old 03-02-2010, 06:37 PM
David Partington
 
Default hwmon: (it87) Add support for the ITE IT8720F Bug: #357766

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

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 03-02-2010, 07:17 PM
Stefan Bader
 
Default hwmon: (it87) Add support for the ITE IT8720F Bug: #357766

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

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 

Thread Tools




All times are GMT. The time now is 05:19 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org