Messages in this thread | | | Date | Thu, 12 Apr 2007 09:27:57 +0200 | From | Hans de Goede <> | Subject | Re: [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/
| |