RE: [PATCH 2/8] Thermal: Create zone level APIs

["R, Durgadoss" <durgadoss.r@intel.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2976 bytes --]

> -----Original Message-----
> From: Zhang, Rui
> Sent: Friday, February 08, 2013 3:24 PM
> To: R, Durgadoss
> Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org;
> eduardo.valentin@ti.com; hongbo.zhang@linaro.org; wni@nvidia.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@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > eduardo.valentin@ti.com; hongbo.zhang@linaro.org; wni@nvidia.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
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥