Re: [lm-sensors] THMC50 review

Re: [lm-sensors] THMC50 review

[Goede, J.W.R. de]

On Mon, 21 May 2007 11:29:42 -0700
 "Juerg Haefliger" <juergh@gmail.com> wrote:
> I'm about ready to do some reviewing but I need to check
> with Mark
> Hofmann to figure out who comes next.  Don't know where
> your driver is
> in the queue.
> 

Actually keeping a queue that can be used with people which
have time to review sounds like a plan. I think we should
create a page for this at the trac wiki. Mark AFAIK you've
been collecting reviews, can you create such a page. (I can
do it myself but I don't have a list of everything that
needs reviewing).

Then we can add links to reviews from the mailinglist
archive there too :)

Talking about reviews, Mark can you please take a look at
and respond to the discussion about the table in the
abituguru3 driver?

Regards,

Hans



> ...juerg
> 
> 
> On 5/22/07, Krzysztof Helt <krzysztof.h1@wp.pl> wrote:
> > Good afternoon,
> >
> > After Jean's suggestion, I polite request any of you to
> do review my port of the THMC50 driver for the 2.6
> kernel. I understand that you are busy, but please queue
> it to your task list.
> >
> > I hope it will open the door to the -mm tree.
> >
> > If the task get queued I warn you that I am stubborn
> and I will ask how long it will take about once per
> month.
> >
> > The driver is here:
> >
>
http://lists.lm-sensors.org/pipermail/lm-sensors/2007-March/019094.html
> >
> > Kind regards,
> > Krzysztof Helt
> >
> 



_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] THMC50 review

[Jean Delvare]

On Mon, 21 May 2007 21:49:26 +0200, Goede, J.W.R. de wrote:
> Actually keeping a queue that can be used with people which
> have time to review sounds like a plan. I think we should
> create a page for this at the trac wiki. Mark AFAIK you've
> been collecting reviews, can you create such a page. (I can
> do it myself but I don't have a list of everything that
> needs reviewing).

This certainly doesn't prevent you from creating such a page. If the
list is incomplete, it can be completed later, by Mark or others.

Note that I am already trying to maintain
http://www.lm-sensors.org/wiki/Devices
with this information. If you search for "review" on this page, you
should find most drivers waiting for a review, with a link to the
latest version that was posted.

Drivers being ported from lm-sensors SVN to Linux 2.6 are also listed on
http://www.lm-sensors.org/wiki/Kernel2.6

Not to say that a dedicated page wouldn't give us a better visibility,
but OTOH I would like to avoid having yet another place were this
information needs to be updated.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] THMC50 review

[Krzysztof Helt]

Jean Delvare wrote:
> On Mon, 21 May 2007 21:49:26 +0200, Goede, J.W.R. de wrote:
>> Actually keeping a queue that can be used with people which
>> have time to review sounds like a plan. I think we should
>> create a page for this at the trac wiki. Mark AFAIK you've
>> been collecting reviews, can you create such a page. (I can
>> do it myself but I don't have a list of everything that
>> needs reviewing).
> 
> This certainly doesn't prevent you from creating such a page. If the
> list is incomplete, it can be completed later, by Mark or others.
> 

The great thing would be to mark who is going to review the driver (after it is decided), so a contributor of the patch can see how far in the queue the driver is.

> Note that I am already trying to maintain
> http://www.lm-sensors.org/wiki/Devices
> with this information. If you search for "review" on this page, you
> should find most drivers waiting for a review, with a link to the
> latest version that was posted.

The information about the ADM1022 is not accurate. I have not tested the 2.4 kernel, but the thmc50 driver do not handle the adm1022 mode. It means that it works if the adm1022 is working as the pure thmc50 (which is default). This is not true for my motherboard. The adm1022 works in an extended mode (two external temperatures) and the 2.4 I used to create the 2.6 patch did not handle this. The result of incorrect mode wss the fan forced to full speed.

I may fix it.

Regards,
Krzysztof

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] THMC50 review

[Juerg Haefliger]

On 5/22/07, Jean Delvare <khali@linux-fr.org> wrote:
> On Mon, 21 May 2007 21:49:26 +0200, Goede, J.W.R. de wrote:
> > Actually keeping a queue that can be used with people which
> > have time to review sounds like a plan. I think we should
> > create a page for this at the trac wiki. Mark AFAIK you've
> > been collecting reviews, can you create such a page. (I can
> > do it myself but I don't have a list of everything that
> > needs reviewing).
>
> This certainly doesn't prevent you from creating such a page. If the
> list is incomplete, it can be completed later, by Mark or others.
>
> Note that I am already trying to maintain
> http://www.lm-sensors.org/wiki/Devices
> with this information. If you search for "review" on this page, you
> should find most drivers waiting for a review, with a link to the
> latest version that was posted.

This is actually already pretty close to perfect. What would further
help is another column that shows the assigned reviewer and some way
to indicate the priority/status. Or should we just use the date as the
priority and a link to a ticket for status updates?



> Drivers being ported from lm-sensors SVN to Linux 2.6 are also listed on
> http://www.lm-sensors.org/wiki/Kernel2.6
>
> Not to say that a dedicated page wouldn't give us a better visibility,
> but OTOH I would like to avoid having yet another place were this
> information needs to be updated.
>
> --
> Jean Delvare
>

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] THMC50 review

[Mark M. Hoffman]

Hi all:

* Goede, J.W.R. de <j.w.r.degoede@hhs.nl> [2007-05-21 21:49:26 +0200]:
> On Mon, 21 May 2007 11:29:42 -0700
>  "Juerg Haefliger" <juergh@gmail.com> wrote:
> > I'm about ready to do some reviewing but I need to check
> > with Mark
> > Hofmann to figure out who comes next.  Don't know where
> > your driver is
> > in the queue.
> > 
> 
> Actually keeping a queue that can be used with people which
> have time to review sounds like a plan. I think we should
> create a page for this at the trac wiki. Mark AFAIK you've
> been collecting reviews, can you create such a page. (I can
> do it myself but I don't have a list of everything that
> needs reviewing).

Given the later messages in this thread, it seems the consensus is to update
the existing Devices page in the wiki to track this info.  I've updated the
lm93 entry as an example.

Maybe this does warrant adding a column to the table: "Reviewer".  Thoughts?

> Then we can add links to reviews from the mailinglist
> archive there too :)
> 
> Talking about reviews, Mark can you please take a look at
> and respond to the discussion about the table in the
> abituguru3 driver?

Eventually, yes.

-- 
Mark M. Hoffman
mhoffman@lightlink.com


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] THMC50 review

[Jean Delvare]

Hi Mark,

On Sun, 27 May 2007 14:14:53 -0400, Mark M. Hoffman wrote:
> Hi all:
> 
> * Goede, J.W.R. de <j.w.r.degoede@hhs.nl> [2007-05-21 21:49:26 +0200]:
> > On Mon, 21 May 2007 11:29:42 -0700
> >  "Juerg Haefliger" <juergh@gmail.com> wrote:
> > > I'm about ready to do some reviewing but I need to check
> > > with Mark
> > > Hofmann to figure out who comes next.  Don't know where
> > > your driver is
> > > in the queue.
> > > 
> > 
> > Actually keeping a queue that can be used with people which
> > have time to review sounds like a plan. I think we should
> > create a page for this at the trac wiki. Mark AFAIK you've
> > been collecting reviews, can you create such a page. (I can
> > do it myself but I don't have a list of everything that
> > needs reviewing).
> 
> Given the later messages in this thread, it seems the consensus is to update
> the existing Devices page in the wiki to track this info.  I've updated the
> lm93 entry as an example.
> 
> Maybe this does warrant adding a column to the table: "Reviewer".  Thoughts?

I don't think this is a good idea. That column would be only used for a
very small number of entries, so it would essentially waste space on the
page. This page is already difficult to see when viewed in 1024x768
resolution, this would only make it worse.

So I'd rather simply mention the reviewer in the comments column. If
you think it's not visible enough, then we can move the information
somewhere else: either a dedicated page on the wiki, or in the ticket
system (one ticket per new driver) as was done for the dme1737 driver.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] THMC50 review

[Jean Delvare]

On Tue, 22 May 2007 11:34:37 -0700, Juerg Haefliger wrote:
> On 5/22/07, Jean Delvare <khali@linux-fr.org> wrote:
> > On Mon, 21 May 2007 21:49:26 +0200, Goede, J.W.R. de wrote:
> > > Actually keeping a queue that can be used with people which
> > > have time to review sounds like a plan. I think we should
> > > create a page for this at the trac wiki. Mark AFAIK you've
> > > been collecting reviews, can you create such a page. (I can
> > > do it myself but I don't have a list of everything that
> > > needs reviewing).
> >
> > This certainly doesn't prevent you from creating such a page. If the
> > list is incomplete, it can be completed later, by Mark or others.
> >
> > Note that I am already trying to maintain
> > http://www.lm-sensors.org/wiki/Devices
> > with this information. If you search for "review" on this page, you
> > should find most drivers waiting for a review, with a link to the
> > latest version that was posted.
> 
> This is actually already pretty close to perfect. What would further
> help is another column that shows the assigned reviewer and some way
> to indicate the priority/status. Or should we just use the date as the
> priority and a link to a ticket for status updates?

The concept of priority doesn't hold in our context. Reviews are done
on a voluntary basis. There is no central authority to decide and
impose which driver review has higher priority.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] THMC50 review

[Jean Delvare]

Hi Krzysztof,

On Wed, 23 May 2007 20:08:43 +0200, Krzysztof Helt wrote:
> The information about the ADM1022 is not accurate. I have not
> tested the 2.4 kernel, but the thmc50 driver do not handle the
> adm1022 mode. It means that it works if the adm1022 is working as
> the pure thmc50 (which is default). This is not true for my
> motherboard. The adm1022 works in an extended mode (two external
> temperatures) and the 2.4 I used to create the 2.6 patch did not
> handle this.

When we list a device as supported in the wiki, it means that we have a
driver with useful functionality, not that 100% of the features of the
device are supported. When drivers have known limitations, we document
them in Documentation/hwmon/$driver. If we start giving details in the
Devices page, it will grow so fast that it will quickly become unusable.

> The result of incorrect mode wss the fan forced to full speed.

This is a bug.

> I may fix it.

This would be great, yes.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors