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

[Eduardo Valentin <eduardo.valentin@ti.com>]

Hello,

On Thu, Aug 16, 2012 at 03:31:55AM +0000, R, Durgadoss wrote:
> Hi Rui,
> 
> 
> > -----Original Message-----
> > From: Zhang, Rui
> > Sent: Thursday, August 16, 2012 9:00 AM
> > To: R, Durgadoss
> > Cc: lenb@kernel.org; rjw@sisk.pl; linux-acpi@vger.kernel.org; linux-
> > pm@vger.kernel.org; eduardo.valentin@ti.com; amit.kachhap@linaro.org;
> > wni@nvidia.com
> > Subject: RE: [PATCH 07/13] Thermal: Update binding logic based on platform
> > data
> > 
> > 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?
> 
> Yes. You are right :-)
> The answer is 'No'.
> 
> > >
> > > 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.
> 
> Yes today it is both..

So, what is the conclusion from above? I think we need to have a call here as
it will drive how driver code is going to look like. So far, the way I am
designing the OMAP thermal support is that the temp sensor drivers would
know about how to cool the zones, at SoC level. But the cooling of a platform/end-product
level would require another driver, which would require knowledge of availability
of sensors and cooling devices, in order to define the board policy.

> 
> > > >
> > > > 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?
> 
> Yes, coretemp is one fine example.
> At least I would like to get the same thing done for emc1403.c
> (and few hwmon drivers)..
> 
> Thanks,
> Durga
--
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