lkml.org 
[lkml]   [2007]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [lm-sensors] Hardware monitoring subsystem maintainer positionis open
Krzysztof Helt wrote:
> This is comments from someone who is newbie to this list.
>
>> 1a) We need a set of review guidelines / a review checklist.
> Here is a start:
>
> Maybe these guidelines can be described in more details and with
> links or names of documents with more description.
>

Yes they should, this was just a quick list. Feel free to help writing a better
list. And I guess we should put this list in the wiki somewhere.

>> * Must follow kernel coding style guidelines
>
> Is there any tool to check this? If there is one, a basic
> instruction how to use it would be great.
>

No tool.

>> * sysfs interface must be in accordance with
> Documentation/hwmon/sysfs
>
> The documentation is still confusing to me. Can someone of you
> update it to answer these questions directly?
>
> A. What is meaning of 0 and 1 values in pwmX_enable ?
> B. How to stop the fan (pwmX_enable = 1 and pwmX = 0 was
> suggested to someone on the list)?
> C. How t o handle situation if the pwm register minimum value (0)
> does not stop the fan, but there is a separate bit to do it? (do
> stop emulate with the bit when 0 is written to pwmX?)
>

I think this is best answered by Jean and/or Mark

>> * proper locking to avoid 2 simultaneous attempts to
> communicate with the
>> device when for example multiple sysfs files get read at once.
>
> An example or two common errors would be great help.
>

Erm, if someone doesn't know what this means / how to look for this he
shouldn't be reviewing a driver.

>> * check the code for any obvious programming errors, like:
>> -not freeing resources (in error paths and in general)
>> -overrunning an array
>> -not checking return values of calls to other parts of the
> kernel
>> -of by one errors / bad loops
>> -etc.
>
> List them, so a newbie can check the code against it.
>

The etc. was because I'm sure there are more but I couldn't come up with any
right there and then. And again a newbie shouldn't be reviewing, he should
start reading some book in device drivers writing a driver or two himself get
those reviewed and then he should no longer be a newbie :)

>> 1b) We need to decide somehow who can do reviews. For now I say
> anyone who
>> already maintains atleast one hwmon driver. But thats just a
> wild shot better
>> ideas are welcome.
>>
>
> There are volunteers already. In order to lessen their work you
> can require to follow the guidelines (if they got described) by
> authors of patches .

Yes ofcourse authors should follow the guidelines, and check they did
themselves before submitting. Also its great that there are volunteers, but
reviewing does require a certain level of competence. There are many other ways
to help out for those without the necessary skills todo the reviewing.

For example you could check out the 3.0.0 branch:
svn checkout http://lm-sensors.org/svn/lm-sensors/branches/lm-sensors-3.0.0

Edit lib/chips.c, goto the long array at the end and comment any entries for
chips you have. Optionally edit prog/sensors/main.c goto the array near the end
and again comment entries for chips you have.

Build and install lm-sensors-3.0.0 and let us know how it works, despite the
commenting it should still recognize your cips and print the same info as usual
(it can now dynamicly recognize new/unknown chips as long as the kernel
supports them). When you skipped the optional step run the sensors command as
'sensors -g', the normal sensors command will be borked if you skipped this step.

Thanks & Regards,

Hans

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-04-12 09:33    [W:0.076 / U:0.668 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site