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

[Wei Ni <wni@nvidia.com>]

On 01/08/2014 11:24 AM, Hu Yaohui wrote:
> I am new here. How can I could not mail a new message to this mail list?
> TIA

I use Thunderbird, it's a pretty good mail client :)

> 
> 
> On Tue, Jan 7, 2014 at 10:19 PM, Wei Ni <wni@nvidia.com
> <mailto: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
>     <mailto: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/
> 
>