linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: "R, Durgadoss" <durgadoss.r@intel.com>
Cc: "Zhang, Rui" <rui.zhang@intel.com>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"hongbo.zhang@linaro.org" <hongbo.zhang@linaro.org>,
	"wni@nvidia.com" <wni@nvidia.com>
Subject: Re: [PATCH 4/8] Thermal: Add Thermal_trip sysfs node
Date: Thu, 20 Dec 2012 08:38:39 -0800	[thread overview]
Message-ID: <20121220163839.GA30120@kroah.com> (raw)
In-Reply-To: <4D68720C2E767A4AA6A8796D42C8EB5924CF1F@BGSMSX101.gar.corp.intel.com>

On Thu, Dec 20, 2012 at 04:25:32PM +0000, R, Durgadoss wrote:
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Thursday, December 20, 2012 9:42 PM
> > To: R, Durgadoss
> > Cc: Zhang, Rui; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org;
> > hongbo.zhang@linaro.org; wni@nvidia.com
> > Subject: Re: [PATCH 4/8] Thermal: Add Thermal_trip sysfs node
> > 
> > On Thu, Dec 20, 2012 at 07:52:03AM +0000, R, Durgadoss wrote:
> > > > On Tue, Dec 18, 2012 at 02:59:33PM +0530, Durgadoss R wrote:
> > > > > This patch adds a thermal_trip directory under
> > > > > /sys/class/thermal/zoneX. This directory contains
> > > > > the trip point values for sensors bound to this
> > > > > zone.
> > > >
> > > > Eeek, you just broke userspace tools that now can no longer see these
> > > > entries :(
> > > >
> > > > Why do you need to create a subdirectory?  As you found out, doing so
> > > > isn't the easiest, right?  That is on purpose.
> > >
> > > Yes, I observed the complexity.
> > >
> > > >
> > > > I really wouldn't recommend doing this at all, please stick within the
> > > > 'struct device' framework here, don't create new kobjects and hang sysfs
> > > > files off of them.
> > >
> > > But, we cannot put all _trip directly under ZoneX directory.
> > 
> > Why not?  What is preventing this?
> > 
> > > We can remove the thermal_trip directory, and put sensorY_trip under
> > > /sys/class/thermal/zoneX/.  But this sensorY_trip needs to be a
> > > directory which has four sysfs nodes named, active, passive, crit,
> > > hot.
> > >
> > > Rui, What do you think about this ?
> > >
> > > The only other way I see, is directly put
> > sensorY_trip_[active/passive/hot/crit]
> > > which will create way too many nodes, under /sys/class/thermal/zoneX/.
> > 
> > What is "too many"?  20000?  50000?  How many are we talking about here?
> 
> Not in 1000's though..
> 
> > What is the limiting factor that is preventing this from all going into
> > one directory?
> 
> We support a MAX of 12 sensors per zone today, which will lead to
> 12 * 4, 48 nodes under this directory named
> sensorY_trip_[active/passive/hot/crit], besides the other nodes.

That's fine, we can easily support that many files, have you tried this
already?

The main point is, if you use a kobject like you are, userspace tools
can't "see" these directories and files easily, if at all.  Try it out
with libudev yourself to verify it, the attributes will not show up as
owned to that device like you need them to be.

So put them all in one directory, we can handle 10's of thousands of
files quite easily, so 48 is trivial :)

thanks,

greg k-h

  reply	other threads:[~2012-12-20 16:35 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-18  9:29 [PATCH 0/8] Thermal Framework Enhancements Durgadoss R
2012-12-18  9:29 ` [PATCH 1/8] Thermal: Create sensor level APIs Durgadoss R
2012-12-18 11:13   ` Joe Perches
2012-12-18  9:29 ` [PATCH 2/8] Thermal: Create zone " Durgadoss R
2012-12-18 11:30   ` Joe Perches
2012-12-20  6:02     ` R, Durgadoss
2012-12-18  9:29 ` [PATCH 3/8] Thermal: Add APIs to bind cdev to new zone structure Durgadoss R
2012-12-25  8:30   ` Wei Ni
2012-12-26  3:30     ` R, Durgadoss
2012-12-18  9:29 ` [PATCH 4/8] Thermal: Add Thermal_trip sysfs node Durgadoss R
2012-12-20  5:42   ` Greg KH
2012-12-20  7:52     ` R, Durgadoss
2012-12-20 16:12       ` Greg KH
2012-12-20 16:25         ` R, Durgadoss
2012-12-20 16:38           ` Greg KH [this message]
2012-12-20 16:58             ` R, Durgadoss
2012-12-20 17:51               ` Greg KH
2012-12-20 18:12                 ` R, Durgadoss
2012-12-27  7:01   ` Hongbo Zhang
2012-12-18  9:29 ` [PATCH 5/8] Thermal: Add 'thermal_map' " Durgadoss R
2012-12-18  9:29 ` [PATCH 6/8] Thermal: Add Documentation to new APIs Durgadoss R
2012-12-18  9:29 ` [PATCH 7/8] Thermal: Make PER_ZONE values configurable Durgadoss R
2012-12-18  9:29 ` [PATCH 8/8] Thermal: Dummy driver used for testing Durgadoss R
2012-12-25  8:38   ` Wei Ni
2012-12-26  3:29     ` R, Durgadoss
2012-12-20  5:37 ` [PATCH 0/8] Thermal Framework Enhancements Greg KH
2012-12-20  6:16   ` R, Durgadoss
2012-12-21  8:05 ` Wei Ni
2012-12-21  8:30   ` R, Durgadoss
2012-12-21  8:46     ` Hongbo Zhang
2012-12-21  9:17       ` R, Durgadoss

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121220163839.GA30120@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=durgadoss.r@intel.com \
    --cc=hongbo.zhang@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rui.zhang@intel.com \
    --cc=wni@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).