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

Hu Yaohui
2014-01-08T03:24:56Z

I am new here. How can I could not mail a new message to this mail list?
TIA

On Tue, Jan 7, 2014 at 10:19 PM, Wei Ni <wni@nvidia.com> wrote:

> On 01/07/2014 07:17 PM, Eduardo Valentin wrote:
> > * PGP Signed by an unknown key
> >
> > On 06-01-2014 22:48, Wei Ni wrote:
> >> Hi, Eduardo
> >> Will you consider my comments :)
> >
> > By now Wei, it is better if you start a new thread, by sending a patch
> > on top of it, as this thread has been already merged by Rui.
>
> Ok, I will send it.
>
> Thanks.
> Wei.
> >
> >>
> >> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org