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 devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html