On Fri, Aug 19, 2011 at 11:47 AM, Guenter Roeck <guenter.roeck@xxxxxxxxxxxx> wrote: > On Fri, Aug 19, 2011 at 02:04:58AM -0400, J, KEERTHY wrote: >> On Fri, Aug 19, 2011 at 7:43 AM, Guenter Roeck >> <guenter.roeck@xxxxxxxxxxxx> wrote: >> > On Thu, Aug 18, 2011 at 06:52:15AM -0400, Keerthy wrote: >> >> On chip temperature sensor driver. The driver monitors the temperature of >> >> the MPU subsystem of the OMAP4. It sends notifications to the user space if >> >> the temperature crosses user defined thresholds via kobject_uevent interface. >> >> The user is allowed to configure the temperature thresholds vis sysfs nodes >> >> exposed using hwmon interface. >> >> >> >> Signed-off-by: Keerthy <j-keerthy@xxxxxx> >> >> Cc: Jean Delvare <khali@xxxxxxxxxxxx> >> >> Cc: Guenter Roeck <guenter.roeck@xxxxxxxxxxxx> >> >> Cc: lm-sensors@xxxxxxxxxxxxxx >> > >> > High level review: >> > >> > - too much and too broad mutex locking. show functions should not need locks at all, >> > set functions only while data is written into registers and into platform data. >> >> Ok. I will clean this. >> >> > - driver is quite noisy. There should definitely not be any log messages >> > if a set parameter is wrong. Show functions already return an error value >> > to the user; a log message indicating the error again just creates noise. >> > For one boolean set during probe (is_efuse_valid), each subsequent show results >> > in a log message if it is false. Some errors result in multiple log messages. >> >> A user tries to set an invalid temperature threshold. The user should >> be notified about this. The invalid temperature will not be set. The user >> should not be allowed to set an invalid temperature. It is to inform >> the user about precisely the problem with the parameter. >> > User is notified with -EINVAL. Unless on the console, which is unlikely, > the user will likely not notice a message in the kernel log. These messages on console are useful for debug purpose so can i place it under dev_dbg? Only on explicit enabling these messages can be seen. > >> In some of the samples the bandgap is not trimmed and hence >> temperature reported will be wrong. So every time a user tries to read >> he is alerted that the temperatures are not accurate. >> > In the kernel log ? Sorry, that doesn't make sense. You alert the system administrator, > not the user. Ok. Can this be a dev_dbg message? Only for debug purpose this can be useful. > > Guenter > -- Regards and Thanks, Keerthy _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
