linux-kernel - Re: [PATCHv9 02/20] thermal: introduce device tree parser

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <52CB6AE2.2090002@nvidia.com>
Date:	Tue, 7 Jan 2014 10:48:02 +0800
From:	Wei Ni <wni@...dia.com>
To:	Wei Ni <wni@...dia.com>,
	Eduardo Valentin <eduardo.valentin@...com>,
	"swarren@...dotorg.org" <swarren@...dotorg.org>,
	"pawel.moll@....com" <pawel.moll@....com>,
	"mark.rutland@....com" <mark.rutland@....com>,
	"ian.campbell@...rix.com" <ian.campbell@...rix.com>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	"linux@...ck-us.net" <linux@...ck-us.net>,
	"rui.zhang@...el.com" <rui.zhang@...el.com>
CC:	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	"durgadoss.r@...el.com" <durgadoss.r@...el.com>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"lm-sensors@...sensors.org" <lm-sensors@...sensors.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv9 02/20] thermal: introduce device tree parser

Hi, Eduardo
Will you consider my comments :)

Thanks.
Wei.

On 12/31/2013 06:17 PM, Wei Ni wrote:
> On 11/13/2013 03:46 AM, Eduardo Valentin wrote:
>> This patch introduces a device tree bindings for
>> describing the hardware thermal behavior and limits.
>> Also a parser to read and interpret the data and feed
>> it in the thermal framework is presented.
>>
>> This patch introduces a thermal data parser for device
>> tree. The parsed data is used to build thermal zones
>> and thermal binding parameters. The output data
>> can then be used to deploy thermal policies.
>>
>> This patch adds also documentation regarding this
>> API and how to define tree nodes to use
>> this infrastructure.
>>
>> Note that, in order to be able to have control
>> on the sensor registration on the DT thermal zone,
>> it was required to allow changing the thermal zone
>> .get_temp callback. For this reason, this patch
>> also removes the 'const' modifier from the .ops
>> field of thermal zone devices.
>>
>> ...
>> +
>> +static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip,
>> +				enum thermal_trend *trend)
>> +{
>> +	struct __thermal_zone *data = tz->devdata;
>> +	long dev_trend;
>> +	int r;
>> +
>> +	if (!data->get_trend)
>> +		return -EINVAL;
>> +
>> +	r = data->get_trend(data->sensor_data, &dev_trend);
> I think the ->get_trend should be defined as:
> .get_trend(*dev, int, *long)
> so that the "trip" can be passed into it, otherwise the "trip" can't be
> used.
> And the dev_trend should be returned as THERMAL_TREND_XXX directly.
> because the THERM_TREND_xx is more than three states.
> 
> The code may be something like:
> r = data->get_trend(data->sensor_data, trip, &dev_trend);
> if (r)
>     return r;
> *trend = dev_trend;
> return 0;
>> +	if (r)
>> +		return r;
>> +
>> +	/* TODO: These intervals might have some thresholds, but in core code */
>> +	if (dev_trend > 0)
>> +		*trend = THERMAL_TREND_RAISING;
>> +	else if (dev_trend < 0)
>> +		*trend = THERMAL_TREND_DROPPING;
>> +	else
>> +		*trend = THERMAL_TREND_STABLE;
>> +
>> +	return 0;
>> +}
>> +
>> .....
>> +
>> +/***   sensor API   ***/
>> +
>> +static struct thermal_zone_device *
>> +thermal_zone_of_add_sensor(struct device_node *zone,
>> +			   struct device_node *sensor, void *data,
>> +			   int (*get_temp)(void *, long *),
>> +			   int (*get_trend)(void *, long *))
>> +{
>> +	struct thermal_zone_device *tzd;
>> +	struct __thermal_zone *tz;
>> +
>> +	tzd = thermal_zone_get_zone_by_name(zone->name);
> I think we could get the thermal zone by node,
> something like:
> thermal_zone_get_zone_by_node(zone);
> so that it can get unique zone.
> 
> I think we can add a member "struct device_node *np" in the
> thermal_zone_device,
> and set it after you call thermal_zone_device_register successfully.
> Then add following codes:
> thermal_zone_get_zone_by_node(struct device_node *node)
> {
>         struct thermal_zone_device *pos = NULL, *ref = ERR_PTR(-ENODEV);
>         bool found = false;
> 
>         if (!node)
>                 return ERR_PTR(-EINVAL);
> 
>         mutex_lock(&thermal_list_lock);
>         list_for_each_entry(pos, &thermal_tz_list, node)
>                 if (node == pos->np) {
>                         ref = pos;
>                         found = true;
>                         break;
>                 }
>         mutex_unlock(&thermal_list_lock);
> 
>         return ref;
> }
> 
> Thanks.
> Wei.
>> +	if (IS_ERR(tzd))
>> +		return ERR_PTR(-EPROBE_DEFER);
>> +
>> +	tz = tzd->devdata;
>> +
>> +	mutex_lock(&tzd->lock);
>> +	tz->get_temp = get_temp;
>> +	tz->get_trend = get_trend;
>> +	tz->sensor_data = data;
>> +
>> +	tzd->ops->get_temp = of_thermal_get_temp;
>> +	tzd->ops->get_trend = of_thermal_get_trend;
>> +	mutex_unlock(&tzd->lock);
>> +
>> +	return tzd;
>> +}
>> +
>>
> 

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