On 01/08/2014 08:31 PM, Eduardo Valentin wrote: > * PGP Signed by an unknown key > > On 08-01-2014 07:10, R, Durgadoss wrote: >> Hi Wei, >> >>> -----Original Message----- >>> From: linux-pm-owner@xxxxxxxxxxxxxxx [mailto:linux-pm- >>> owner@xxxxxxxxxxxxxxx] On Behalf Of Wei Ni >>> Sent: Wednesday, January 08, 2014 2:37 PM >>> To: eduardo.valentin@xxxxxx; Zhang, Rui; mark.rutland@xxxxxxx >>> Cc: MLongnecker@xxxxxxxxxx; swarren@xxxxxxxxxxxxx; linux- >>> pm@xxxxxxxxxxxxxxx; linux-tegra@xxxxxxxxxxxxxxx; Wei Ni >>> Subject: [PATCH] thermal: use device node to get thermal zone >>> >>> If use name to get the thermal zone, sometimes it can't >>> get the unique thermal zone, because the thermal fw support >>> same name for different thermal zone. >>> So we can change to use device node to get thermal zone. > > Please check the documentation for .*by_name function. There is an error > code for the case of multiple instances. In fact, it is designed to be > used by users who know that in the system there is going to exist only > one zone with that name. > > I don't think it is correct to simply remove the existing API and > enforce DT usage. What if the system does not use DT? > >> >> Eduardo introduced this API because we wanted to get the >> tzd pointer by using the name. So, for your case, can you >> introduce a new API *get_by_node instead of changing this >> API ? > > Agreed, if you need a search by DT node, then you need to provide the > use cases and propose a new one. Yes, you are right, I should not remove it simply, I will add a new API .*by_node. On the tegra board, it will use two or more sensors to estimate the skin temperature by reading temps from these sensors and calculate them. For example, we have two sensors: sensor1 and sensor2. We can register them to thermal framework by using DT, something like: thermal-zones { sensor1: lm90-local { ... thermal-sensors = <&lm90 0>; }; sensor2: lm90-remote { ... thermal-sensors = <&lm90 1>; }; } Then I will add a device node for my skin temperature driver, something like: skin_temp { ... #thermal-sensor-cells = <0>; sub-devs { dev@0 { dev = <&sensor1>; }; dev@1 { dev = <&sensor2>; }; }; }; So I can parse the DT in the skin temperature driver to get the nodes of the sensor1 and sensor2, and can use .*get_by_node to get thermal zone device, then use .get_temp() and other callbacks to get temperature and other information. If use the *.get_by_name, it may not get the uniqu one, because I don't know if there has the same name thermal zone, because some other driver may not use DT to register thermal zone, it can define any name by itself. > >> >> Thanks, >> Durga >> >>> >>> Signed-off-by: Wei Ni <wni@xxxxxxxxxx> >>> --- >>> drivers/thermal/of-thermal.c | 6 ++++-- >>> drivers/thermal/thermal_core.c | 37 ++++++++++++++++--------------------- >>> include/linux/thermal.h | 4 +++- >>> 3 files changed, 23 insertions(+), 24 deletions(-) >>> >>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c >>> index 04b1be7..97c12cf 100644 >>> --- a/drivers/thermal/of-thermal.c >>> +++ b/drivers/thermal/of-thermal.c >>> @@ -330,7 +330,7 @@ thermal_zone_of_add_sensor(struct device_node *zone, >>> struct thermal_zone_device *tzd; >>> struct __thermal_zone *tz; >>> >>> - tzd = thermal_zone_get_zone_by_name(zone->name); >>> + tzd = thermal_zone_get_zone_by_node(zone); > > In DT, nodes have unique names. Are you seen a problem with the above code? this file is used for DT parse, I think it's better to use .*by_node instead. > >>> if (IS_ERR(tzd)) >>> return ERR_PTR(-EPROBE_DEFER); >>> >>> @@ -804,6 +804,8 @@ int __init of_parse_thermal_zones(void) >>> of_thermal_free_zone(tz); >>> /* attempting to build remaining zones still */ >>> } >>> + >>> + zone->np = child; >>> } >>> >>> return 0; >>> @@ -837,7 +839,7 @@ void of_thermal_destroy_zones(void) >>> for_each_child_of_node(np, child) { >>> struct thermal_zone_device *zone; >>> >>> - zone = thermal_zone_get_zone_by_name(child->name); >>> + zone = thermal_zone_get_zone_by_node(child); >>> if (IS_ERR(zone)) >>> continue; >>> >>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c >>> index 338a88b..89e0637 100644 >>> --- a/drivers/thermal/thermal_core.c >>> +++ b/drivers/thermal/thermal_core.c >>> @@ -1635,42 +1635,37 @@ void thermal_zone_device_unregister(struct >>> thermal_zone_device *tz) >>> EXPORT_SYMBOL_GPL(thermal_zone_device_unregister); >>> >>> /** >>> - * thermal_zone_get_zone_by_name() - search for a zone and returns its ref >>> - * @name: thermal zone name to fetch the temperature >>> + * thermal_zone_get_zone_by_node() - search for a zone and returns its ref >>> + * @node: device node of the thermal zone >>> * >>> - * When only one zone is found with the passed name, returns a reference to it. >>> + * When thermal zone is found with the passed device node, returns a >>> reference >>> + * to it. >>> * >>> * Return: On success returns a reference to an unique thermal zone with >>> - * matching name equals to @name, an ERR_PTR otherwise (-EINVAL for invalid >>> - * paramenters, -ENODEV for not found and -EEXIST for multiple matches). >>> + * matching device node, an ERR_PTR otherwise (-EINVAL for invalid >>> + * paramenters, -ENODEV for not found). >>> */ >>> -struct thermal_zone_device *thermal_zone_get_zone_by_name(const char >>> *name) >>> +struct thermal_zone_device * >>> +thermal_zone_get_zone_by_node(struct device_node *node) >>> { > > You are removing an API, are you checking that all existing users of > this API are changed accordingly? I don't think so. Sorry, it's my mistake, I will not remove it in my next pactch. > >>> - struct thermal_zone_device *pos = NULL, *ref = ERR_PTR(-EINVAL); >>> - unsigned int found = 0; >>> + struct thermal_zone_device *pos = NULL, *ref = ERR_PTR(-ENODEV); >>> + bool found = false; >>> >>> - if (!name) >>> - goto exit; >>> + if (!node) >>> + return ERR_PTR(-EINVAL); >>> >>> mutex_lock(&thermal_list_lock); >>> list_for_each_entry(pos, &thermal_tz_list, node) >>> - if (!strnicmp(name, pos->type, THERMAL_NAME_LENGTH)) { >>> - found++; >>> + if (node == pos->np) { >>> ref = pos; >>> + found = true; >>> + break; >>> } >>> mutex_unlock(&thermal_list_lock); >>> >>> - /* nothing has been found, thus an error code for it */ >>> - if (found == 0) >>> - ref = ERR_PTR(-ENODEV); >>> - else if (found > 1) >>> - /* Success only when an unique zone is found */ >>> - ref = ERR_PTR(-EEXIST); >>> - >>> -exit: >>> return ref; >>> } >>> -EXPORT_SYMBOL_GPL(thermal_zone_get_zone_by_name); >>> +EXPORT_SYMBOL_GPL(thermal_zone_get_zone_by_node); >>> >>> #ifdef CONFIG_NET >>> static const struct genl_multicast_group thermal_event_mcgrps[] = { >>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h >>> index f7e11c7..a94de8c 100644 >>> --- a/include/linux/thermal.h >>> +++ b/include/linux/thermal.h >>> @@ -162,6 +162,7 @@ struct thermal_zone_device { >>> int id; >>> char type[THERMAL_NAME_LENGTH]; >>> struct device device; >>> + struct device_node *np; > > Please describe better if you want to add this node in here. Better to > add it in a separated patch. > >>> struct thermal_attr *trip_temp_attrs; >>> struct thermal_attr *trip_type_attrs; >>> struct thermal_attr *trip_hyst_attrs; >>> @@ -285,7 +286,8 @@ struct thermal_cooling_device * >>> thermal_of_cooling_device_register(struct device_node *np, char *, void *, >>> const struct thermal_cooling_device_ops *); >>> void thermal_cooling_device_unregister(struct thermal_cooling_device *); >>> -struct thermal_zone_device *thermal_zone_get_zone_by_name(const char >>> *name); >>> +struct thermal_zone_device * >>> +thermal_zone_get_zone_by_node(struct device_node *node); >>> int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long >>> *temp); >>> >>> int get_tz_trend(struct thermal_zone_device *, int); >>> -- >>> 1.7.9.5 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in >>> the body of a message to majordomo@xxxxxxxxxxxxxxx >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-pm" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
