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. > > 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 -- 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
