[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4D68720C2E767A4AA6A8796D42C8EB5927AC1F@BGSMSX101.gar.corp.intel.com>
Date: Fri, 8 Feb 2013 10:27:41 +0000
From: "R, Durgadoss" <durgadoss.r@...el.com>
To: "Zhang, Rui" <rui.zhang@...el.com>
CC: "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"eduardo.valentin@...com" <eduardo.valentin@...com>,
"hongbo.zhang@...aro.org" <hongbo.zhang@...aro.org>,
"wni@...dia.com" <wni@...dia.com>
Subject: RE: [PATCH 2/8] Thermal: Create zone level APIs
> -----Original Message-----
> From: Zhang, Rui
> Sent: Friday, February 08, 2013 3:24 PM
> To: R, Durgadoss
> Cc: linux-pm@...r.kernel.org; linux-kernel@...r.kernel.org;
> eduardo.valentin@...com; hongbo.zhang@...aro.org; wni@...dia.com
> Subject: RE: [PATCH 2/8] Thermal: Create zone level APIs
>
> On Fri, 2013-02-08 at 01:54 -0700, R, Durgadoss wrote:
> > Hi Rui,
> >
> > > -----Original Message-----
> > > From: Zhang, Rui
> > > Sent: Friday, February 08, 2013 1:42 PM
> > > To: R, Durgadoss
> > > Cc: linux-pm@...r.kernel.org; linux-kernel@...r.kernel.org;
> > > eduardo.valentin@...com; hongbo.zhang@...aro.org; wni@...dia.com
> > > Subject: Re: [PATCH 2/8] Thermal: Create zone level APIs
> > >
> > > On Tue, 2013-02-05 at 16:16 +0530, Durgadoss R wrote:
> > > > This patch adds a new thermal_zone structure to
> > > > thermal.h. Also, adds zone level APIs to the thermal
> > > > framework.
> > > >
> >
> > [snip.]
> >
> > > > +
> > > > +struct thermal_sensor *get_sensor_by_name(const char *name)
> > > > +{
> > > > + struct thermal_sensor *pos;
> > > > + struct thermal_sensor *ts = NULL;
> > > > +
> > > > + mutex_lock(&sensor_list_lock);
> > > > + for_each_thermal_sensor(pos) {
> > > > + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH))
> > > {
> > > > + ts = pos;
> > > > + break;
> > >
> > > this function depends on the assumption that all the sensor names are
> > > unique.
> > > thus I prefer to go through all the list and return -EINVAL if duplicate
> > > names found, because in this case, the pointer returned may be not the
> > > sensor we want to get.
> >
> > Yes, I agree with you. But I prefer having this check in the register API
> > itself, which then will not allow duplicates.
> >
> No, I do not think so.
>
> Unique cdev/sensor name is not a hard rule for generic thermal layer,
> and will not be.
> Because any cooling device driver does not have the technology that if
> its name is platform unique or not.
>
> Say, your platform thermal driver wants to use FAN cooling device, what
> if another FAN cooling device has been registered before the FAN your
> platform thermal driver wants to use get registered?
> If the platform thermal driver wants to use get_cdev/sensor_by_name(),
> it has already made the assumption that all the cooling devices have
> unique names. Thus duplicate names are a big issue, we should abort the
> platform thermal driver, rather than aborting the cooling device driver
> with duplicate names.
>
> > The reason being, we use this get* API (comparatively) a lot more than
> > the register APIs.
>
> why?
> why can not invoke get_sensor/cdev_by_name once and cache the pointer
> in
> the platform thermal driver?
Okay, I did not think of this caching part ..
Then, I am fine with this change. Will fix it in next version.
Thanks,
Durga
Powered by blists - more mailing lists