Re: [lm-sensors] Hardware monitoring subsystem maintainer positionisopen

[Hans de Goede]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/