The patch is on top of this patch set. Run into an issue during test today, will send out after the issue resolved.
Thanks, rui > -----Original Message----- > From: [email protected] <[email protected]> > On Behalf Of Andrzej Pietrasiewicz > Sent: Tuesday, April 28, 2020 2:35 AM > To: Zhang, Rui <[email protected]>; '[email protected]' <linux- > [email protected]> > Cc: 'Rafael J . Wysocki' <[email protected]>; 'Len Brown' <[email protected]>; > 'Jiri Pirko' <[email protected]>; 'Ido Schimmel' <[email protected]>; > 'David S . Miller' <[email protected]>; 'Peter Kaestle' <[email protected]>; > 'Darren Hart' <[email protected]>; 'Andy Shevchenko' > <[email protected]>; 'Support Opensource' > <[email protected]>; 'Daniel Lezcano' > <[email protected]>; 'Amit Kucheria' > <[email protected]>; 'Shawn Guo' <[email protected]>; > 'Sascha Hauer' <[email protected]>; 'Pengutronix Kernel Team' > <[email protected]>; 'Fabio Estevam' <[email protected]>; 'NXP > Linux Team' <[email protected]>; 'Heiko Stuebner' <[email protected]>; > 'Orson Zhai' <[email protected]>; 'Baolin Wang' > <[email protected]>; 'Chunyan Zhang' <[email protected]>; > '[email protected]' <[email protected]>; > '[email protected]' <[email protected]>; 'platform-driver- > [email protected]' <[email protected]>; 'linux-arm- > [email protected]' <[email protected]>; > '[email protected]' <[email protected]>; 'Barlomiej Zolnierkiewicz' > <[email protected]> > Subject: Re: [PATCH v3 2/2] thermal: core: Stop polling DISABLED thermal > devices > Importance: High > > Hi, > > W dniu 27.04.2020 o 16:20, Zhang, Rui pisze: > > > > > >> -----Original Message----- > >> From: Zhang, Rui > >> Sent: Friday, April 24, 2020 5:03 PM > >> To: Andrzej Pietrasiewicz <[email protected]>; linux- > >> [email protected] > >> Cc: Rafael J . Wysocki <[email protected]>; Len Brown > >> <[email protected]>; Jiri Pirko <[email protected]>; Ido Schimmel > >> <[email protected]>; David S . Miller <[email protected]>; > Peter > >> Kaestle <[email protected]>; Darren Hart <[email protected]>; Andy > >> Shevchenko <[email protected]>; Support Opensource > >> <[email protected]>; Daniel Lezcano > >> <[email protected]>; Amit Kucheria > >> <[email protected]>; Shawn Guo <[email protected]>; > >> Sascha Hauer <[email protected]>; Pengutronix Kernel Team > >> <[email protected]>; Fabio Estevam <[email protected]>; NXP > >> Linux Team <[email protected]>; Heiko Stuebner <[email protected]>; > >> Orson Zhai <[email protected]>; Baolin Wang > >> <[email protected]>; Chunyan Zhang <[email protected]>; > >> linux- [email protected]; [email protected]; platform-driver- > >> [email protected]; [email protected]; > >> [email protected]; Barlomiej Zolnierkiewicz > >> <[email protected]> > >> Subject: RE: [PATCH v3 2/2] thermal: core: Stop polling DISABLED > >> thermal devices > >> > >> Hi, Andrzej, > >> > >> Thanks for the patches. My Linux laptop was broken and won't get > >> fixed till next week, so I may lost some of the discussions previously. > >> > >>> -----Original Message----- > >>> From: Andrzej Pietrasiewicz <[email protected]> > >>> Sent: Friday, April 24, 2020 12:57 AM > >>> To: [email protected] > >>> Cc: Zhang, Rui <[email protected]>; Rafael J . Wysocki > >>> <[email protected]>; Len Brown <[email protected]>; Jiri Pirko > >>> <[email protected]>; Ido Schimmel <[email protected]>; David S . > >>> Miller <[email protected]>; Peter Kaestle <[email protected]>; > Darren > >>> Hart <[email protected]>; Andy Shevchenko <[email protected]>; > >>> Support Opensource <[email protected]>; Daniel > Lezcano > >>> <[email protected]>; Amit Kucheria > >>> <[email protected]>; Shawn Guo <[email protected]>; > >> Sascha > >>> Hauer <[email protected]>; Pengutronix Kernel Team > >>> <[email protected]>; Fabio Estevam <[email protected]>; NXP > >> Linux > >>> Team <[email protected]>; Heiko Stuebner <[email protected]>; > Orson > >> Zhai > >>> <[email protected]>; Baolin Wang <[email protected]>; > >> Chunyan > >>> Zhang <[email protected]>; linux- [email protected]; > >>> [email protected]; platform-driver- [email protected]; > >>> [email protected]; > >>> [email protected]; Andrzej Pietrasiewicz > >>> <[email protected]>; Barlomiej Zolnierkiewicz > >>> <[email protected]> > >>> Subject: [PATCH v3 2/2] thermal: core: Stop polling DISABLED thermal > >>> devices > >>> Importance: High > >>> > >>> Polling DISABLED devices is not desired, as all such "disabled" > >>> devices are meant to be handled by userspace. This patch introduces > >>> and uses > >>> should_stop_polling() to decide whether the device should be polled > >>> or > >> not. > >>> > >> Thanks for the fix, and IMO, this reveal some more problems. > >> Say, we need to define "DISABLED" thermal zone. > >> Can we read the temperature? Can we trust the trip point value? > >> > >> IMO, a disabled thermal zone does not mean it is handled by > >> userspace, because that is what the userspace governor designed for. > >> Instead, if a thermal zone is disabled, in > >> thermal_zone_device_update(), we should basically skip all the other > operations as well. > >> > > I overlooked the last line of the patch. So > > thermal_zone_device_update() returns immediately if the thermal zone is > disabled, right? > > > > But how can we stop polling in this case? > > It does stop. However, I indeed observe an extra call to > thermal_zone_device_update() before it fully stops. > I think what happens is this: > > - storing "disabled" in mode ends up in thermal_zone_device_set_mode(), > which calls driver's ->set_mode() and then calls > thermal_zone_device_update(), which returns immediately and does not > touch the tz->poll_queue delayed work > > - thermal_zone_device_update() is called from the delayed work when its > time comes and this time it also returns immediately, not modifying the said > delayed work, so polling effectively stops now. > > > There is no chance to call into monitor_thermal_zone() in > > thermal_zone_device_update(), or do I miss something? > > Without the last "if" statement in this patch polling stops with the first > call to > thermal_zone_device_update() because it indeed disables the delayed work. > > So you are probably right - that last "if" should not be introduced. > > > > >> I'll try your patches and probably make an incremental patch. > > > > I have finished a small patch set to improve this based on my > > understanding, and will post it tomorrow after testing. > > > > Is your small patchset based on top of this series or is it a completely > rewritten version? > > Andrzej
