Re: [PATCH 2/3] thermal: of: consolidate sensor callbacks as ops
Hi Eduardo, On 11/27/2014 06:28 AM, Eduardo Valentin wrote: > * PGP Signed by an unknown key > > > Hello Navneet, > > On Wed, Nov 26, 2014 at 05:16:28PM -0800, Navneet Kumar wrote: >> From: navneet kumar >> >> Consolidate all the sensor callbacks (get_temp/get_trend) >> into a 'thermal_of_sensor_ops' struct. >> >> As a part of this, introduce a 'thermal_zone_of_sensor_register2' >> sensor API that accepts sensor_ops instead of the two callbacks >> as separate arguments to the register function. > > This is usually not a good thing to do. Specially about the suffix > '.*2', sounds really broken :-(. Best thing to do is to update the API > with the improvement, and update all the users of that old API. > > This is one of the key Linux design decision. Please, have a look at: > Documentation/stable_api_nonsense.txt > > To understand why doing such thing is a bad thing to do. > Thanks for correcting me. I was thinking on the lines of staging this patch as- 1. release a *register2 2. fixup rest of the drivers ( as a separate patch) to use *register2 3. rename all the references of register2 as register and eventually terminate the use of the old signature'd API. Either ways, i see your patch now, and will do the needful rebase too! thanks again. >> >> Modify the older version of register function to call register2. >> >> Adjust all the references to get_temp/get_trend callbacks. >> >> Signed-off-by: navneet kumar >> --- >> drivers/thermal/of-thermal.c | 78 >> >> include/linux/thermal.h | 14 >> 2 files changed, 64 insertions(+), 28 deletions(-) >> >> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c >> index cf9ee3e82fee..3d47a0cf3825 100644 >> --- a/drivers/thermal/of-thermal.c >> +++ b/drivers/thermal/of-thermal.c >> @@ -96,8 +96,7 @@ struct __thermal_zone { >> >> /* sensor interface */ >> void *sensor_data; >> -int (*get_temp)(void *, long *); >> -int (*get_trend)(void *, long *); >> +struct thermal_of_sensor_ops sops; > > Have you seen this patch: > https://git.kernel.org/cgit/linux/kernel/git/evalenti/linux-soc-thermal.git/commit/?h=linus&id=2251aef64a38db60f4ae7a4a83f9203c6791f196 > > ? > > Please rebase your changes on top of my -linus branch: > git://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git > linus > > > BR, > > Eduardo Valentin >> }; >> >> /*** DT thermal zone device callbacks ***/ >> @@ -107,10 +106,10 @@ static int of_thermal_get_temp(struct >> thermal_zone_device *tz, >> { >> struct __thermal_zone *data = tz->devdata; >> >> -if (!data->get_temp) >> +if (!data->sops.get_temp) >> return -EINVAL; >> >> -return data->get_temp(data->sensor_data, temp); >> +return data->sops.get_temp(data->sensor_data, temp); >> } >> >> static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, >> @@ -120,10 +119,10 @@ static int of_thermal_get_trend(struct >> thermal_zone_device *tz, int trip, >> long dev_trend; >> int r; >> >> -if (!data->get_trend) >> +if (!data->sops.get_trend) >> return -EINVAL; >> >> -r = data->get_trend(data->sensor_data, &dev_trend); >> +r = data->sops.get_trend(data->sensor_data, &dev_trend); >> if (r) >> return r; >> >> @@ -324,8 +323,7 @@ static struct thermal_zone_device_ops of_thermal_ops = { >> 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_of_sensor_ops *sops) >> { >> struct thermal_zone_device *tzd; >> struct __thermal_zone *tz; >> @@ -337,8 +335,9 @@ thermal_zone_of_add_sensor(struct device_node *zone, >> tz = tzd->devdata; >> >> mutex_lock(&tzd->lock); >> -tz->get_temp = get_temp; >> -tz->get_trend = get_trend; >> +if (sops) >> +memcpy(&(tz->sops), sops, sizeof(*sops)); >> + >> tz->sensor_data = data; >> >> tzd->ops->get_temp = of_thermal_get_temp; >> @@ -349,15 +348
Re: [PATCH 3/3] thermal: of: notify sensor driver on trip updates
Hi Eduardo, On 11/27/2014 06:32 AM, Eduardo Valentin wrote: > * PGP Signed by an unknown key > > Hello Navneet, > > On Wed, Nov 26, 2014 at 05:16:29PM -0800, Navneet Kumar wrote: >> From: navneet kumar >> >> some thermal sensor hardwares include logic which >> can raise interrupts at certain programmed temperature >> thresholds. >> >> Drivers for such sensors should be able to learn the >> appropriate threshold temperatures for interrupts by querying >> the thermal framework. >> >> This change provides a mechanism to allow a sensor driver to >> update it's thresholds when userspace changes a trip point >> temperature. >> >> While this behavior may not make sense in thermal zones >> with more than one sensor, no such examples exist in >> the kernel. >> >> Signed-off-by: navneet kumar >> --- >> drivers/thermal/of-thermal.c | 7 +++ >> include/linux/thermal.h | 1 + >> 2 files changed, 8 insertions(+) >> >> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c >> index 3d47a0cf3825..3568e4a586dc 100644 >> --- a/drivers/thermal/of-thermal.c >> +++ b/drivers/thermal/of-thermal.c >> @@ -258,6 +258,9 @@ static int of_thermal_set_trip_temp(struct >> thermal_zone_device *tz, int trip, >> /* thermal framework should take care of data->mask & (1 << trip) */ >> data->trips[trip].temperature = temp; >> >> +if (data->sops.trip_update) >> +data->sops.trip_update(data->sensor_data, trip); >> + >> return 0; >> } >> >> @@ -285,6 +288,9 @@ static int of_thermal_set_trip_hyst(struct >> thermal_zone_device *tz, int trip, >> /* thermal framework should take care of data->mask & (1 << trip) */ >> data->trips[trip].hysteresis = hyst; >> >> +if (data->sops.trip_update) >> +data->sops.trip_update(data->sensor_data, trip); >> + >> return 0; >> } >> >> @@ -500,6 +506,7 @@ void thermal_zone_of_sensor_unregister(struct device >> *dev, >> >> tz->sops.get_temp = NULL; >> tz->sops.get_trend = NULL; >> +tz->sops.trip_update = NULL; >> tz->sensor_data = NULL; >> mutex_unlock(&tzd->lock); >> } >> diff --git a/include/linux/thermal.h b/include/linux/thermal.h >> index 58341c56a01f..b93e65815175 100644 >> --- a/include/linux/thermal.h >> +++ b/include/linux/thermal.h >> @@ -292,6 +292,7 @@ struct thermal_genl_event { >> struct thermal_of_sensor_ops { >> int (*get_temp)(void *, long *); >> int (*get_trend)(void *, long *); >> +int (*trip_update)(void *, int); > > First thing I ask you is to update your work on top of my -linus branch, > as I already mentioned. Reasoning is that part of the changes you are > sending is already there. will do. > > As for this new callback, I am fine with it as long as it is also > available for drivers that do not use of-thermal. Once again, of-thermal > is not a competitor of thermal core. It will never be. It is not a new > thermal API. I agree that this callback is not a part of the thermal_core functionality. However, when a driver registers directly with the thermal_core (doesn't use of-thermal), it 'owns' the set_trip_XX callbacks in the first place; which is the sole purpose of using the 'trip_update' callback in of-thermal. Adding an additional 'update' to the thermal_core ops would be a no-op. right? > > That said, it does not make sense to have functionality in of-thermal that > do not belong to thermal core. Exceptions are, of course, for helping > doing the same operations we already have in thermal core. > > All the best, > > Eduardo Valentin > >> }; >> >> /* Function declarations */ >> -- >> 1.8.1.5 >> > > * Unknown Key > * 0x7DA4E256 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] thermal: of: notify sensor driver on trip updates
On 12/01/2014 01:23 PM, Eduardo Valentin wrote: > * PGP Signed by an unknown key > > On Mon, Dec 01, 2014 at 12:45:52PM -0800, navneet kumar wrote: >> Hi Eduardo, >> >> On 11/27/2014 06:32 AM, Eduardo Valentin wrote: >>>> Old Signed by an unknown key >>> >>> Hello Navneet, >>> >>> On Wed, Nov 26, 2014 at 05:16:29PM -0800, Navneet Kumar wrote: >>>> From: navneet kumar >>>> >>>> some thermal sensor hardwares include logic which >>>> can raise interrupts at certain programmed temperature >>>> thresholds. >>>> >>>> Drivers for such sensors should be able to learn the >>>> appropriate threshold temperatures for interrupts by querying >>>> the thermal framework. >>>> >>>> This change provides a mechanism to allow a sensor driver to >>>> update it's thresholds when userspace changes a trip point >>>> temperature. >>>> >>>> While this behavior may not make sense in thermal zones >>>> with more than one sensor, no such examples exist in >>>> the kernel. >>>> >>>> Signed-off-by: navneet kumar >>>> --- >>>> drivers/thermal/of-thermal.c | 7 +++ >>>> include/linux/thermal.h | 1 + >>>> 2 files changed, 8 insertions(+) >>>> >>>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c >>>> index 3d47a0cf3825..3568e4a586dc 100644 >>>> --- a/drivers/thermal/of-thermal.c >>>> +++ b/drivers/thermal/of-thermal.c >>>> @@ -258,6 +258,9 @@ static int of_thermal_set_trip_temp(struct >>>> thermal_zone_device *tz, int trip, >>>>/* thermal framework should take care of data->mask & (1 << trip) */ >>>>data->trips[trip].temperature = temp; >>>> >>>> + if (data->sops.trip_update) >>>> + data->sops.trip_update(data->sensor_data, trip); >>>> + >>>>return 0; >>>> } >>>> >>>> @@ -285,6 +288,9 @@ static int of_thermal_set_trip_hyst(struct >>>> thermal_zone_device *tz, int trip, >>>>/* thermal framework should take care of data->mask & (1 << trip) */ >>>>data->trips[trip].hysteresis = hyst; >>>> >>>> + if (data->sops.trip_update) >>>> + data->sops.trip_update(data->sensor_data, trip); >>>> + >>>>return 0; >>>> } >>>> >>>> @@ -500,6 +506,7 @@ void thermal_zone_of_sensor_unregister(struct device >>>> *dev, >>>> >>>>tz->sops.get_temp = NULL; >>>>tz->sops.get_trend = NULL; >>>> + tz->sops.trip_update = NULL; >>>>tz->sensor_data = NULL; >>>>mutex_unlock(&tzd->lock); >>>> } >>>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h >>>> index 58341c56a01f..b93e65815175 100644 >>>> --- a/include/linux/thermal.h >>>> +++ b/include/linux/thermal.h >>>> @@ -292,6 +292,7 @@ struct thermal_genl_event { >>>> struct thermal_of_sensor_ops { >>>>int (*get_temp)(void *, long *); >>>>int (*get_trend)(void *, long *); >>>> + int (*trip_update)(void *, int); >>> >>> First thing I ask you is to update your work on top of my -linus branch, >>> as I already mentioned. Reasoning is that part of the changes you are >>> sending is already there. >> will do. >>> >>> As for this new callback, I am fine with it as long as it is also >>> available for drivers that do not use of-thermal. Once again, of-thermal >>> is not a competitor of thermal core. It will never be. It is not a new >>> thermal API. >> I agree that this callback is not a part of the thermal_core functionality. >> However, when a driver registers directly with the thermal_core (doesn't use >> of-thermal), it 'owns' the set_trip_XX callbacks in the first place; which is >> the sole purpose of using the 'trip_update' callback in of-thermal. >> >> Adding an additional 'update' to the thermal_core ops would be a no-op. >> right? > > Yes, you are right. Now I understand your point. > > Can we then re-use the .set_trips nomenclature? Sorry, I fail to understand. Are you suggesting to re-use the interface for set_trip 'temp' as well as 'hyst'? If so, is it just to maintain the commonality across thermal_core and of-thermal interfaces? The way i see it, the driver just needs to get some kind of 'update' that 'something' changed with a trip point; and can later query the trips from of-thermal. (Lukasz's patch helps with that). Functionality-wise, using two callbacks seems excessive. But i may be wrong :-) > > Cheers, > >>> >>> That said, it does not make sense to have functionality in of-thermal that >>> do not belong to thermal core. Exceptions are, of course, for helping >>> doing the same operations we already have in thermal core. >>> >>> All the best, >>> >>> Eduardo Valentin >>> >>>> }; >>>> >>>> /* Function declarations */ >>>> -- >>>> 1.8.1.5 >>>> >>> >>> * Unknown Key >>> * 0x7DA4E256 >>> > > * Unknown Key > * 0x7DA4E256 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/4] thermal: of: Extend of-thermal to export table of trip points
Hi Eduardo, Lukasz, [Combining my concerns as a single text blob here] I. Firstly, with the current patch 1. is it really needed to duplicate the struct thermal_trip? Why don’t we get rid of the __thermal_trip and stay with the 'thermal_trip' ? it is not a big change. 2. gtrips is not updated on "set_trip_temp" etc. actions via sysfs. (am I missing something?). II. The other concern is more of a design question 1. Do we intend to keep the of-thermal as a middle layer between the thermal_core and the sensor device? OR, is the role of of-thermal just to parse the DT and opt out ? currently of-thermal is somewhat a hybrid of these as, in addition to parsing the dt, it holds on to the data related to trip points etc. etc. 2. assuming latter is true (OF is just a dt parser helper): we should not be adding more intelligence and dependencies linked to the OF. 3. assuming former is true (OF is a well-defined middle layer): All is good till the point OF maintains the trip points (which is a good thing since, OF caches on to the data); BUT, if we expose this information to the sensor device too (as this patch is doing), 3a. we violate the layering principle :-) 3b. more importantly, this is all just excessive logic that we put in which *could be useful* only if we intend to extend the role of OF as a trip point management layer that does more than just holding on to the data. This may include - -> The sensor devices to know nothing about the trip_points, instead the sensor devices would work on "temperature thresholds" and OF would map sensor thresholds to the actual trip points as needed (configured from DT); while the sensor devices stick to using "thresholds". -> Queuing from above, sensors, most of the time, only need to know a high and a low temp threshold; which essentially is a subset of the active/passive etc. trip points. Calculation of that based on the current temp, as of today is replicated across all the sensor drivers and can be hoisted up to the of-thermal. Seems like this is the opportune time to make a call about the role of of-thermal? On 11/26/2014 07:18 AM, Eduardo Valentin wrote: > * PGP Signed by an unknown key > > Hello, > > On Wed, Nov 26, 2014 at 09:35:10AM +0100, Lukasz Majewski wrote: >> Hi Eduardo, >> >>> Hello Lukasz, >>> >>> On Thu, Nov 20, 2014 at 05:21:27PM +0100, Lukasz Majewski wrote: This patch extends the of-thermal.c to export copy of trip points for a given thermal zone. Thermal drivers should use of_thermal_get_trip_points() method to get pointer to table of thermal trip points. Signed-off-by: Lukasz Majewski --- Changes for v2: - New patch - as suggested by Eduardo Valentin --- drivers/thermal/of-thermal.c | 33 + drivers/thermal/thermal_core.h | 7 +++ include/linux/thermal.h| 14 ++ 3 files changed, 54 insertions(+) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index 336af7f..33921c5 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -89,6 +89,7 @@ struct __thermal_zone { /* trip data */ int ntrips; struct __thermal_trip *trips; + struct thermal_trip *gtrips; Do we really need this duplication ? /* cooling binding data */ int num_tbps; @@ -152,6 +153,27 @@ bool of_thermal_is_trip_en(struct thermal_zone_device *tz, int trip) return true; } +/** + * of_thermal_get_trip_points - function to get access to a globally exported + *trip points + * + * @tz: pointer to a thermal zone + * + * This function provides a pointer to the copy of trip points table + * + * Return: pointer to trip points table, NULL otherwise + */ +const struct thermal_trip * const +of_thermal_get_trip_points(struct thermal_zone_device *tz) +{ + struct __thermal_zone *data = tz->devdata; + + if (!data) + return NULL; + + return data->gtrips; +} + >>> >>> EXPORT_SYMBOL_GPL(of_thermal_get_trip_points); >> >> Ok. >> >>> static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, enum thermal_trend *trend) { @@ -767,6 +789,16 @@ thermal_of_build_thermal_zone(struct device_node *np) goto free_tbps; } + tz-
Re: [PATCH v2 3/4] thermal: of: Extend of-thermal to export table of trip points
On 11/26/2014 01:12 PM, Guenter Roeck wrote: > On 11/26/2014 12:43 PM, navneet kumar wrote: >> >> Hi Eduardo, Lukasz, >> >> [Combining my concerns as a single text blob here] >> >> I. Firstly, with the current patch >> 1. is it really needed to duplicate the struct thermal_trip? Why >> don’t >> we get rid of the __thermal_trip and stay with the 'thermal_trip' >> ? it >> is not a big change. >> >> 2. gtrips is not updated on "set_trip_temp" etc. actions via >> sysfs. (am >> I missing something?). >> >> II. The other concern is more of a design question >> 1. Do we intend to keep the of-thermal as a middle layer between the >> thermal_core and the sensor device? OR, is the role of of-thermal >> just >> to parse the DT and opt out ? currently of-thermal is somewhat a >> hybrid >> of these as, in addition to parsing the dt, it holds on to the data >> related to trip points etc. etc. >> >> 2. assuming latter is true (OF is just a dt parser helper): we should >> not be adding more intelligence and dependencies linked to the OF. >> >> 3. assuming former is true (OF is a well-defined middle layer): >> All is >> good till the point OF maintains the trip points (which is a good >> thing >> since, OF caches on to the data); BUT, if we expose this >> information to >> the sensor device too (as this patch is doing), >> >> 3a. we violate the layering principle :-) >> >> 3b. more importantly, this is all just excessive logic that we >> put in which *could be useful* only if we intend to extend the >> role of OF as a trip point management layer that does more than >> just holding on to the data. This may include - >> >> -> The sensor devices to know nothing about the >> trip_points, instead the sensor devices would work on >> "temperature thresholds" and OF would map sensor >> thresholds to the actual trip points as needed >> (configured from DT); while the sensor devices stick to >> using "thresholds". >> >> -> Queuing from above, sensors, most of the time, only >> need to know a high and a low temp threshold; which >> essentially is a subset of the active/passive etc. trip >> points. Calculation of that based on the current temp, >> as of today is replicated across all the sensor drivers >> and can be hoisted up to the of-thermal. >> > > Sorry, lost you here. What replicated calculation do you refer to ? > > Thanks, > Guenter > Some sensors have an ability to generate interrupts based upon a configured high and low temp thresholds/values. Once any of these limits is crossed, the sensor hardware has to be reconfigured with a new set of such values. I'll try to explain with an example - consider trip points TP1, TP2, TP3, TP4 sorted as low to high; and the current temp T1 such that - TP2> Seems like this is the opportune time to make a call about the role of >> of-thermal? >> >> On 11/26/2014 07:18 AM, Eduardo Valentin wrote: >>> * PGP Signed by an unknown key >>> >>> Hello, >>> >>> On Wed, Nov 26, 2014 at 09:35:10AM +0100, Lukasz Majewski wrote: >>>> Hi Eduardo, >>>> >>>>> Hello Lukasz, >>>>> >>>>> On Thu, Nov 20, 2014 at 05:21:27PM +0100, Lukasz Majewski wrote: >>>>>> This patch extends the of-thermal.c to export copy of trip points >>>>>> for a given thermal zone. >>>>>> >>>>>> Thermal drivers should use of_thermal_get_trip_points() method to >>>>>> get pointer to table of thermal trip points. >>>>>> >>>>>> Signed-off-by: Lukasz Majewski >>>>>> --- >>>>>> Changes for v2: >>>>>> - New patch - as suggested by Eduardo Valentin >>>>>> --- >>>>>> drivers/thermal/of-thermal.c | 33 >>>>>> + drivers/thermal/thermal_core.h | >>>>>> 7 +++ include/linux/thermal.h| 14 ++ >>>>>> 3 files changed, 54 insertions(+) >>>>>> >>>>>> diff --git a/drivers/thermal/of-thermal.c >>>>>> b/drivers/thermal/of-thermal.c index 336af7f..33921c5 100644 >>>>
[PATCH 1/3] thermal: of: support writable trips via dt
From: navneet kumar Support writable trip points configuration from the device tree. 'OF' reads this configuration and adjusts the 'trips' mask accordingly to allow the 'set_trip_xxx' calls to be effective. Signed-off-by: Diwakar Tundlam --- drivers/thermal/of-thermal.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index 62143ba31001..cf9ee3e82fee 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -604,7 +604,8 @@ static int thermal_of_get_trip_type(struct device_node *np, * Return: 0 on success, proper error code otherwise */ static int thermal_of_populate_trip(struct device_node *np, - struct __thermal_trip *trip) + struct __thermal_trip *trip, + bool *trip_writable) { int prop; int ret; @@ -629,6 +630,8 @@ static int thermal_of_populate_trip(struct device_node *np, return ret; } + *trip_writable = of_property_read_bool(np, "writable"); + /* Required for cooling map matching */ trip->np = np; of_node_get(np); @@ -657,6 +660,8 @@ thermal_of_build_thermal_zone(struct device_node *np) struct __thermal_zone *tz; int ret, i; u32 prop; + bool trip_writable; + u64 m = 0; if (!np) { pr_err("no thermal zone np\n"); @@ -700,9 +705,14 @@ thermal_of_build_thermal_zone(struct device_node *np) i = 0; for_each_child_of_node(child, gchild) { - ret = thermal_of_populate_trip(gchild, &tz->trips[i++]); + trip_writable = false; + ret = thermal_of_populate_trip(gchild, &tz->trips[i], + &trip_writable); if (ret) goto free_trips; + if (trip_writable) + m |= 1ULL << i; + i++; } of_node_put(child); -- 1.8.1.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] thermal: of: consolidate sensor callbacks as ops
From: navneet kumar Consolidate all the sensor callbacks (get_temp/get_trend) into a 'thermal_of_sensor_ops' struct. As a part of this, introduce a 'thermal_zone_of_sensor_register2' sensor API that accepts sensor_ops instead of the two callbacks as separate arguments to the register function. Modify the older version of register function to call register2. Adjust all the references to get_temp/get_trend callbacks. Signed-off-by: navneet kumar --- drivers/thermal/of-thermal.c | 78 include/linux/thermal.h | 14 2 files changed, 64 insertions(+), 28 deletions(-) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index cf9ee3e82fee..3d47a0cf3825 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -96,8 +96,7 @@ struct __thermal_zone { /* sensor interface */ void *sensor_data; - int (*get_temp)(void *, long *); - int (*get_trend)(void *, long *); + struct thermal_of_sensor_ops sops; }; /*** DT thermal zone device callbacks ***/ @@ -107,10 +106,10 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, { struct __thermal_zone *data = tz->devdata; - if (!data->get_temp) + if (!data->sops.get_temp) return -EINVAL; - return data->get_temp(data->sensor_data, temp); + return data->sops.get_temp(data->sensor_data, temp); } static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, @@ -120,10 +119,10 @@ static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip, long dev_trend; int r; - if (!data->get_trend) + if (!data->sops.get_trend) return -EINVAL; - r = data->get_trend(data->sensor_data, &dev_trend); + r = data->sops.get_trend(data->sensor_data, &dev_trend); if (r) return r; @@ -324,8 +323,7 @@ static struct thermal_zone_device_ops of_thermal_ops = { 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_of_sensor_ops *sops) { struct thermal_zone_device *tzd; struct __thermal_zone *tz; @@ -337,8 +335,9 @@ thermal_zone_of_add_sensor(struct device_node *zone, tz = tzd->devdata; mutex_lock(&tzd->lock); - tz->get_temp = get_temp; - tz->get_trend = get_trend; + if (sops) + memcpy(&(tz->sops), sops, sizeof(*sops)); + tz->sensor_data = data; tzd->ops->get_temp = of_thermal_get_temp; @@ -349,15 +348,15 @@ thermal_zone_of_add_sensor(struct device_node *zone, } /** - * thermal_zone_of_sensor_register - registers a sensor to a DT thermal zone + * thermal_zone_of_sensor_register2 - registers a sensor to a DT thermal zone * @dev: a valid struct device pointer of a sensor device. Must contain * a valid .of_node, for the sensor node. * @sensor_id: a sensor identifier, in case the sensor IP has more * than one sensors * @data: a private pointer (owned by the caller) that will be passed *back, when a temperature reading is needed. - * @get_temp: a pointer to a function that reads the sensor temperature. - * @get_trend: a pointer to a function that reads the sensor temperature trend. + * @sops: handle to the sensor ops (get_temp/get_trend etc.) provided by the + * sensor to OF. * * This function will search the list of thermal zones described in device * tree and look for the zone that refer to the sensor device pointed by @@ -370,21 +369,13 @@ thermal_zone_of_add_sensor(struct device_node *zone, * The thermal zone temperature trend is provided by the @get_trend function * pointer. When called, it will have the private pointer @data back. * - * TODO: - * 01 - This function must enqueue the new sensor instead of using - * it as the only source of temperature values. - * - * 02 - There must be a way to match the sensor with all thermal zones - * that refer to it. - * * Return: On success returns a valid struct thermal_zone_device, * otherwise, it returns a corresponding ERR_PTR(). Caller must * check the return value with help of IS_ERR() helper. */ struct thermal_zone_device * -thermal_zone_of_sensor_register(struct device *dev, int sensor_id, - void *data, int (*get_temp)(void *, long *), - int (*get_trend)(void *, long *)) +thermal_zone_of_sensor_register2(struct device *dev, int sensor_id, + void *data, struct thermal_of_sensor_ops *sops) { struct device_node *n
[PATCH 3/3] thermal: of: notify sensor driver on trip updates
From: navneet kumar some thermal sensor hardwares include logic which can raise interrupts at certain programmed temperature thresholds. Drivers for such sensors should be able to learn the appropriate threshold temperatures for interrupts by querying the thermal framework. This change provides a mechanism to allow a sensor driver to update it's thresholds when userspace changes a trip point temperature. While this behavior may not make sense in thermal zones with more than one sensor, no such examples exist in the kernel. Signed-off-by: navneet kumar --- drivers/thermal/of-thermal.c | 7 +++ include/linux/thermal.h | 1 + 2 files changed, 8 insertions(+) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index 3d47a0cf3825..3568e4a586dc 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -258,6 +258,9 @@ static int of_thermal_set_trip_temp(struct thermal_zone_device *tz, int trip, /* thermal framework should take care of data->mask & (1 << trip) */ data->trips[trip].temperature = temp; + if (data->sops.trip_update) + data->sops.trip_update(data->sensor_data, trip); + return 0; } @@ -285,6 +288,9 @@ static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip, /* thermal framework should take care of data->mask & (1 << trip) */ data->trips[trip].hysteresis = hyst; + if (data->sops.trip_update) + data->sops.trip_update(data->sensor_data, trip); + return 0; } @@ -500,6 +506,7 @@ void thermal_zone_of_sensor_unregister(struct device *dev, tz->sops.get_temp = NULL; tz->sops.get_trend = NULL; + tz->sops.trip_update = NULL; tz->sensor_data = NULL; mutex_unlock(&tzd->lock); } diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 58341c56a01f..b93e65815175 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -292,6 +292,7 @@ struct thermal_genl_event { struct thermal_of_sensor_ops { int (*get_temp)(void *, long *); int (*get_trend)(void *, long *); + int (*trip_update)(void *, int); }; /* Function declarations */ -- 1.8.1.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/