RE: [PATCH 07/13] Thermal: Update binding logic based on platform data

[Zhang Rui <rui.zhang@intel.com>]

On 三, 2012-08-15 at 03:17 -0600, R, Durgadoss wrote:
> Hi Rui,
> 
> > > > > +static void update_bind_info(struct thermal_cooling_device *cdev)
> > > > > +{
> > > > > +	int i, ret;
> > > > > +	struct thermal_zone_params *tzp;
> > > > > +	struct thermal_zone_device *pos = NULL;
> > > > > +
> > > > > +	mutex_lock(&thermal_list_lock);
> > > > > +
> > > > > +	list_for_each_entry(pos, &thermal_tz_list, node) {
> > > > > +		if (!pos->tzp && !pos->ops->bind)
> > > > > +			continue;
> > > > > +
> > > > > +		if (!pos->tzp && pos->ops->bind) {
> > > > > +			ret = pos->ops->bind(pos, cdev);
> > > > > +			if (ret)
> > > > > +				print_bind_err_msg(pos, cdev, ret);
> > > > > +		}
> > > > > +
> > > > > +		tzp = pos->tzp;
> > > > > +		for (i = 0; i < tzp->num_cdevs; i++) {
> > > > > +			if (!strcmp(tzp->cdevs_name[i], cdev->type)) {
> > > > > +				__bind(pos, tzp->trip_mask[i], cdev);
> > > > > +				break;
> > > > > +			}
> > > > > +		}
> > > > > +	}
> > > > > +	mutex_unlock(&thermal_list_lock);
> > > > > +}
> > > >
> > > > I still do not understand why we need this kind of bind.
> > > > Say, the platform thermal driver knows the platform data, i.e. it knows
> > > > which cooling devices should be bound to which trip points.
> > > > why we can not move this kind of logic to the .bind() callback, offered
> > > > by the platform thermal driver?
> > > > say, in .bind() callback,
> > > > the platform thermal driver has the pointer of the platform data, right?
> > > > the .cdev parameter can be used to find the cooling device name,
> > > > and we can make the comparison there. instead of introducing new binding
> > > > functions in the generic thermal layer.
> > >
> > > For once, I got little confused between the generic platform thermal sensor
> > > drivers (the chip drivers) and the platform level driver (not specific for chip,
> > > but for a platform). So, yes we can put this in the platform level driver.
> > >
> > Hmm,
> > I'm not clear about the difference between these two drivers.
> > what is supposed to be done in the platform thermal sensor drivers and
> > what is supposed to be done in the platform level driver?
> 
> A sensor driver can be a generic chip driver like emc1403 (this is the one
> that I have worked on..) or coretemp (the CPU DTS driver for x86). They sit
> in different sub systems (these two in hwmon). We might not be allowed to
> add any thermal framework specific code in these drivers. The same driver
> works on all platforms.

does the sensor know anything about the "policy"?
Say, does it have any trip points? does it know which device can be
throttled to cool itself?
I think the answer is "no", right?

> 
> A platform level thermal driver knows information about the thermal sensors,
> and their zones on the platform; and is specific to the platform.
> For x86, this will be in drivers/x86/platform/ whereas might be in some other
> place for other architectures. An example is intel_mid_thermal.c which sits
> in drivers/x86/platform. We can add our thermal framework specific code
> to this driver.
> 
but I think intel_mide_thermal driver is also a platform thermal sensor
driver at the same time.

> > 
> > At least for now, all the thermal drivers are both thermal sensor driver
> > and platform level driver, right?
>  
> Not all the times, although there are some instances where both are same.
> We use coretemp.c and intel_mid_thermal.c (which are different), for the
> x86 mid platforms.
> 
so you want to use coretemp.c as a temperature sensor, and then bind
your own cooling devices to it in your platform level thermal driver?

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html