[PATCH 0/3] Fixes for stop monitoring disabled devices series

[PATCH 0/3] Fixes for stop monitoring disabled devices series

[Andrzej Pietrasiewicz]

This short series contains fixes for "Stop monitoring disabled devices"
series https://www.spinics.net/lists/arm-kernel/msg817861.html

Invocation of thermal_zone_device_is_enabled() in acpi/thermal is now
redundant, because thermal_zone_device_update() now is capable of
handling disabled devices.

In imx's ->get_temp() the lock must not be taken, otherwise a deadlock
happens. The decision whether explicitly running a measurement cycle
is needed is taken based on driver's local irq_enabled variable.

Finally, thermal_zone_device_is_enabled() is made available to the
core only, as there are no driver users of it.

Andrzej Pietrasiewicz (3):
  acpi: thermal: Don't call thermal_zone_device_is_enabled()
  thermal: imx: Use driver's local data to decide whether to run a
    measurement
  thermal: Make thermal_zone_device_is_enabled() available to core only

 drivers/acpi/thermal.c         | 3 ---
 drivers/thermal/imx_thermal.c  | 7 ++++---
 drivers/thermal/thermal_core.c | 1 -
 drivers/thermal/thermal_core.h | 2 ++
 include/linux/thermal.h        | 5 -----
 5 files changed, 6 insertions(+), 12 deletions(-)

-- 
2.17.1

[PATCH 1/3] acpi: thermal: Don't call thermal_zone_device_is_enabled()

[Andrzej Pietrasiewicz]

thermal_zone_device_update() can now handle disabled thermal zones, so
the check here is not needed.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/acpi/thermal.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 29a2b73fe035..12c0ece746f0 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -499,9 +499,6 @@ static void acpi_thermal_check(void *data)
 {
 	struct acpi_thermal *tz = data;
 
-	if (!thermal_zone_device_is_enabled(tz->thermal_zone))
-		return;
-
 	thermal_zone_device_update(tz->thermal_zone,
 				   THERMAL_EVENT_UNSPECIFIED);
 }
-- 
2.17.1

[PATCH 2/3] thermal: imx: Use driver's local data to decide whether to run a measurement

[Andrzej Pietrasiewicz]

Use driver's local data to evaluate the need to run or not to run a
measurement.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/thermal/imx_thermal.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 9700ae39feb7..c0efa7bc48e2 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -252,10 +252,11 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
 	const struct thermal_soc_data *soc_data = data->socdata;
 	struct regmap *map = data->tempmon;
 	unsigned int n_meas;
-	bool wait;
+	bool wait, run_measurement;
 	u32 val;
 
-	if (thermal_zone_device_is_enabled(tz)) {
+	run_measurement = !data->irq_enabled;
+	if (!run_measurement) {
 		/* Check if a measurement is currently in progress */
 		regmap_read(map, soc_data->temp_data, &val);
 		wait = !(val & soc_data->temp_valid_mask);
@@ -282,7 +283,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
 
 	regmap_read(map, soc_data->temp_data, &val);
 
-	if (!thermal_zone_device_is_enabled(tz)) {
+	if (run_measurement) {
 		regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
 			     soc_data->measure_temp_mask);
 		regmap_write(map, soc_data->sensor_ctrl + REG_SET,
-- 
2.17.1

[PATCH 3/3] thermal: Make thermal_zone_device_is_enabled() available to core only

[Andrzej Pietrasiewicz]

This function is not needed by drivers.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/thermal/thermal_core.c | 1 -
 drivers/thermal/thermal_core.h | 2 ++
 include/linux/thermal.h        | 5 -----
 3 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index a61e91513584..052343c59b0a 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -519,7 +519,6 @@ int thermal_zone_device_is_enabled(struct thermal_zone_device *tz)
 
 	return mode == THERMAL_DEVICE_ENABLED;
 }
-EXPORT_SYMBOL_GPL(thermal_zone_device_is_enabled);
 
 void thermal_zone_device_update(struct thermal_zone_device *tz,
 				enum thermal_notify_event event)
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index c95689586e19..b1464b3a21e2 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -166,4 +166,6 @@ of_thermal_get_trip_points(struct thermal_zone_device *tz)
 }
 #endif
 
+int thermal_zone_device_is_enabled(struct thermal_zone_device *tz);
+
 #endif /* __THERMAL_CORE_H__ */
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index b9efaa780d88..108251f23e5c 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -418,7 +418,6 @@ void thermal_cdev_update(struct thermal_cooling_device *);
 void thermal_notify_framework(struct thermal_zone_device *, int);
 int thermal_zone_device_enable(struct thermal_zone_device *tz);
 int thermal_zone_device_disable(struct thermal_zone_device *tz);
-int thermal_zone_device_is_enabled(struct thermal_zone_device *tz);
 #else
 static inline struct thermal_zone_device *thermal_zone_device_register(
 	const char *type, int trips, int mask, void *devdata,
@@ -472,10 +471,6 @@ static inline int thermal_zone_device_enable(struct thermal_zone_device *tz)
 
 static inline int thermal_zone_device_disable(struct thermal_zone_device *tz)
 { return -ENODEV; }
-
-static inline int
-thermal_zone_device_is_enabled(struct thermal_zone_device *tz)
-{ return -ENODEV; }
 #endif /* CONFIG_THERMAL */
 
 #endif /* __THERMAL_H__ */
-- 
2.17.1

Re: [PATCH 0/3] Fixes for stop monitoring disabled devices series

[Daniel Lezcano]

On 03/07/2020 12:43, Andrzej Pietrasiewicz wrote:
> This short series contains fixes for "Stop monitoring disabled devices"
> series https://www.spinics.net/lists/arm-kernel/msg817861.html
> 
> Invocation of thermal_zone_device_is_enabled() in acpi/thermal is now
> redundant, because thermal_zone_device_update() now is capable of
> handling disabled devices.
> 
> In imx's ->get_temp() the lock must not be taken, otherwise a deadlock
> happens. The decision whether explicitly running a measurement cycle
> is needed is taken based on driver's local irq_enabled variable.
> 
> Finally, thermal_zone_device_is_enabled() is made available to the
> core only, as there are no driver users of it.
> 
> Andrzej Pietrasiewicz (3):
>   acpi: thermal: Don't call thermal_zone_device_is_enabled()
>   thermal: imx: Use driver's local data to decide whether to run a
>     measurement
>   thermal: Make thermal_zone_device_is_enabled() available to core only
> 
>  drivers/acpi/thermal.c         | 3 ---
>  drivers/thermal/imx_thermal.c  | 7 ++++---
>  drivers/thermal/thermal_core.c | 1 -
>  drivers/thermal/thermal_core.h | 2 ++
>  include/linux/thermal.h        | 5 -----
>  5 files changed, 6 insertions(+), 12 deletions(-)

Is this series easily merge-able with the other series?




-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 0/3] Fixes for stop monitoring disabled devices series

[Andrzej Pietrasiewicz]

Hi,

W dniu 03.07.2020 o 13:06, Daniel Lezcano pisze:
> On 03/07/2020 12:43, Andrzej Pietrasiewicz wrote:
>> This short series contains fixes for "Stop monitoring disabled devices"
>> series https://www.spinics.net/lists/arm-kernel/msg817861.html
>>
>> Invocation of thermal_zone_device_is_enabled() in acpi/thermal is now
>> redundant, because thermal_zone_device_update() now is capable of
>> handling disabled devices.
>>
>> In imx's ->get_temp() the lock must not be taken, otherwise a deadlock
>> happens. The decision whether explicitly running a measurement cycle
>> is needed is taken based on driver's local irq_enabled variable.
>>
>> Finally, thermal_zone_device_is_enabled() is made available to the
>> core only, as there are no driver users of it.
>>
>> Andrzej Pietrasiewicz (3):
>>    acpi: thermal: Don't call thermal_zone_device_is_enabled()
>>    thermal: imx: Use driver's local data to decide whether to run a
>>      measurement
>>    thermal: Make thermal_zone_device_is_enabled() available to core only
>>
>>   drivers/acpi/thermal.c         | 3 ---
>>   drivers/thermal/imx_thermal.c  | 7 ++++---
>>   drivers/thermal/thermal_core.c | 1 -
>>   drivers/thermal/thermal_core.h | 2 ++
>>   include/linux/thermal.h        | 5 -----
>>   5 files changed, 6 insertions(+), 12 deletions(-)
> 
> Is this series easily merge-able with the other series?
> 

So-so.

Some simple conflicts needed to be resolved.

I have created a branch for you to look at and decide
how far off it is from the original and whether the
original Acked-by/Reviewed-by can be retained.

Note that I might have lost some portions of code
during conflict resolution. It seems to me I haven't
but you know.

The branch:

https://gitlab.collabora.com/andrzej.p/kernel-tests/-/tree/thermal-dont-poll-disabled-for-daniel

The code compiles at each stage on x86_64 + COMPILE_TEST.

Andrzej

Re: [PATCH 0/3] Fixes for stop monitoring disabled devices series

[Daniel Lezcano]

On 03/07/2020 13:57, Andrzej Pietrasiewicz wrote:
> Hi,
> 
> W dniu 03.07.2020 o 13:06, Daniel Lezcano pisze:
>> On 03/07/2020 12:43, Andrzej Pietrasiewicz wrote:
>>> This short series contains fixes for "Stop monitoring disabled devices"
>>> series https://www.spinics.net/lists/arm-kernel/msg817861.html
>>>
>>> Invocation of thermal_zone_device_is_enabled() in acpi/thermal is now
>>> redundant, because thermal_zone_device_update() now is capable of
>>> handling disabled devices.
>>>
>>> In imx's ->get_temp() the lock must not be taken, otherwise a deadlock
>>> happens. The decision whether explicitly running a measurement cycle
>>> is needed is taken based on driver's local irq_enabled variable.
>>>
>>> Finally, thermal_zone_device_is_enabled() is made available to the
>>> core only, as there are no driver users of it.
>>>
>>> Andrzej Pietrasiewicz (3):
>>>    acpi: thermal: Don't call thermal_zone_device_is_enabled()
>>>    thermal: imx: Use driver's local data to decide whether to run a
>>>      measurement
>>>    thermal: Make thermal_zone_device_is_enabled() available to core only
>>>
>>>   drivers/acpi/thermal.c         | 3 ---
>>>   drivers/thermal/imx_thermal.c  | 7 ++++---
>>>   drivers/thermal/thermal_core.c | 1 -
>>>   drivers/thermal/thermal_core.h | 2 ++
>>>   include/linux/thermal.h        | 5 -----
>>>   5 files changed, 6 insertions(+), 12 deletions(-)
>>
>> Is this series easily merge-able with the other series?
>>
> 
> So-so.
> 
> Some simple conflicts needed to be resolved.
> 
> I have created a branch for you to look at and decide
> how far off it is from the original and whether the
> original Acked-by/Reviewed-by can be retained.
> 
> Note that I might have lost some portions of code
> during conflict resolution. It seems to me I haven't
> but you know.
> 
> The branch:
> 
> https://gitlab.collabora.com/andrzej.p/kernel-tests/-/tree/thermal-dont-poll-disabled-for-daniel

Ok, I propose to keep the these three patches on top of V7.

Rui are you fine with that ?



-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 0/3] Fixes for stop monitoring disabled devices series

[Zhang Rui]

On Fri, 2020-07-03 at 14:05 +0200, Daniel Lezcano wrote:
> On 03/07/2020 13:57, Andrzej Pietrasiewicz wrote:
> > Hi,
> > 
> > W dniu 03.07.2020 o 13:06, Daniel Lezcano pisze:
> > > On 03/07/2020 12:43, Andrzej Pietrasiewicz wrote:
> > > > This short series contains fixes for "Stop monitoring disabled
> > > > devices"
> > > > series https://www.spinics.net/lists/arm-kernel/msg817861.html
> > > > 
> > > > Invocation of thermal_zone_device_is_enabled() in acpi/thermal
> > > > is now
> > > > redundant, because thermal_zone_device_update() now is capable
> > > > of
> > > > handling disabled devices.
> > > > 
> > > > In imx's ->get_temp() the lock must not be taken, otherwise a
> > > > deadlock
> > > > happens. The decision whether explicitly running a measurement
> > > > cycle
> > > > is needed is taken based on driver's local irq_enabled
> > > > variable.
> > > > 
> > > > Finally, thermal_zone_device_is_enabled() is made available to
> > > > the
> > > > core only, as there are no driver users of it.
> > > > 
> > > > Andrzej Pietrasiewicz (3):
> > > >    acpi: thermal: Don't call thermal_zone_device_is_enabled()
> > > >    thermal: imx: Use driver's local data to decide whether to
> > > > run a
> > > >      measurement
> > > >    thermal: Make thermal_zone_device_is_enabled() available to
> > > > core only
> > > > 
> > > >   drivers/acpi/thermal.c         | 3 ---
> > > >   drivers/thermal/imx_thermal.c  | 7 ++++---
> > > >   drivers/thermal/thermal_core.c | 1 -
> > > >   drivers/thermal/thermal_core.h | 2 ++
> > > >   include/linux/thermal.h        | 5 -----
> > > >   5 files changed, 6 insertions(+), 12 deletions(-)
> > > 
> > > Is this series easily merge-able with the other series?
> > > 
> > 
> > So-so.
> > 
> > Some simple conflicts needed to be resolved.
> > 
> > I have created a branch for you to look at and decide
> > how far off it is from the original and whether the
> > original Acked-by/Reviewed-by can be retained.
> > 
> > Note that I might have lost some portions of code
> > during conflict resolution. It seems to me I haven't
> > but you know.
> > 
> > The branch:
> > 
> > 
https://gitlab.collabora.com/andrzej.p/kernel-tests/-/tree/thermal-dont-poll-disabled-for-daniel
> 
> Ok, I propose to keep the these three patches on top of V7.
> 
> Rui are you fine with that ?

Yes, that works for me.

thanks,
rui
> 
> 
>