On Fri, Aug 19, 2011 at 11:47 AM, Guenter Roeck
<[email protected]> 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
>> <[email protected]> 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 <[email protected]>
>> >> Cc: Jean Delvare <[email protected]>
>> >> Cc: Guenter Roeck <[email protected]>
>> >> Cc: [email protected]
>> >
>> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to