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

[Zhang Rui]

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@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > eduardo.valentin@xxxxxx; hongbo.zhang@xxxxxxxxxx; wni@xxxxxxxxxx
> > 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?

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/