On Wed, Sep 7, 2011 at 12:11 AM, J, KEERTHY <j-keerthy@xxxxxx> wrote: > On Tue, Sep 6, 2011 at 11:42 PM, Guenter Roeck > <guenter.roeck@xxxxxxxxxxxx> wrote: >> On Tue, 2011-09-06 at 14:02 -0400, J, KEERTHY wrote: >>> On Thu, Sep 1, 2011 at 10:10 AM, Guenter Roeck >>> <guenter.roeck@xxxxxxxxxxxx> wrote: >>> > On Thu, Sep 01, 2011 at 12:09:14AM -0400, Paul Walmsley wrote: >>> >> On Wed, 31 Aug 2011, Guenter Roeck wrote: >>> >> >>> >> > On Wed, Aug 31, 2011 at 08:36:43PM -0400, Paul Walmsley wrote: >>> >> > > Hi >>> >> > > >>> >> > > Some comments. >>> >> > > >>> >> > > On Wed, 31 Aug 2011, Keerthy wrote: >>> >> > > >>> >> > [ ... ] >>> >> > > >>> >> > > > +} >>> >> > > > + >>> >> > > > +/* Sysfs hook functions */ >>> >> > > >>> >> > > These should be conditionally compiled out if sysfs isn't compiled in. >>> >> > > >>> >> > The whole point of the hwmon subsystem is to expose hardware monitoring information >>> >> > to userland using sysfs. hwmon without sysfs doesn't make sense. >>> >> > >>> >> > So, if anything, it might make sense to disable the entire hwmon tree if sysfs is disabled. >>> >> > But please no conditionals in the code. >>> >> >>> >> Hmm. This IP block is more than just a sensor. It also can interrupt the >>> >> CPU and/or trigger a GPIO line (to shut down the chip) if the chip >>> >> temperature crosses some thresholds. On some OMAPs, the thresholds are >>> >> fixed; on others, they are software-programmable. That functionality >>> >> shouldn't require sysfs; it's almost closer to an x86 MCE. >>> >> >>> >> So based on your comments, it sounds like we should move that part of the >>> >> code to a different driver, and just leave the basic software thermal >>> >> monitoring here? >>> >> >>> > Good point. This definitely requires some thought. hwmon is meant to be hw monitoring, >>> > as the name says, not thermal management. Maybe this entire driver should be a thermal driver >>> > instead ? >>> >>> This driver is not taking any action on THSUT. This is not doing the thermal >>> management. It is a driver exposing configurable temperature thresholds. >> >> What sense would it make, then, to keep the driver around even if SYSFS >> is not defined ? > > SYSFS nodes are defined for t_hot and t_cold thresholds. SYSFS nodes > are not defined > for TSHUT thresholds which are different. The point i am trying to make here is that this is a hardware monitoring driver with configurable thresholds. The driver reads the on-die temperature. The thermal framework as such is being discussed in a separate thread, which dictates the actions to be taken once the thresholds are crossed. This driver does not take any actions but just monitors the temperatures. TSHUT or the thermal shutdown implementation requires the driver to perform a shutdown of the device on reaching a certain temperature and hence is not part of this driver. > >> >> Note that I am not looking at the code right now, but at the suggestion >> that the driver would do something useful if SYSFS is not defined. >> Question is what that is, and if that part of it should reside in a >> hwmon driver. >> >> Thanks, >> Guenter >> >> >> > > > > -- > Regards and Thanks, > Keerthy > -- Regards and Thanks, Keerthy _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
