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.ro...@ericsson.com> 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-keer...@ti.com> > >> Cc: Jean Delvare <kh...@linux-fr.org> > >> Cc: Guenter Roeck <guenter.ro...@ericsson.com> > >> Cc: lm-sens...@lm-sensors.org > > > > 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.
> 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. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
