Re: [RFC PATCH 6/6 V2] hwmon: OMAP4: On die temperature sensor driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux