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: linux-acpi-ow...@vger.kernel.org <linux-acpi-ow...@vger.kernel.org> > On Behalf Of Andrzej Pietrasiewicz > Sent: Tuesday, April 28, 2020 2:35 AM > To: Zhang, Rui <rui.zh...@intel.com>; 'linux...@vger.kernel.org' <linux- > p...@vger.kernel.org> > Cc: 'Rafael J . Wysocki' <r...@rjwysocki.net>; 'Len Brown' <l...@kernel.org>; > 'Jiri Pirko' <j...@mellanox.com>; 'Ido Schimmel' <ido...@mellanox.com>; > 'David S . Miller' <da...@davemloft.net>; 'Peter Kaestle' <pe...@piie.net>; > 'Darren Hart' <dvh...@infradead.org>; 'Andy Shevchenko' > <a...@infradead.org>; 'Support Opensource' > <support.opensou...@diasemi.com>; 'Daniel Lezcano' > <daniel.lezc...@linaro.org>; 'Amit Kucheria' > <amit.kuche...@verdurent.com>; 'Shawn Guo' <shawn...@kernel.org>; > 'Sascha Hauer' <s.ha...@pengutronix.de>; 'Pengutronix Kernel Team' > <ker...@pengutronix.de>; 'Fabio Estevam' <feste...@gmail.com>; 'NXP > Linux Team' <linux-...@nxp.com>; 'Heiko Stuebner' <he...@sntech.de>; > 'Orson Zhai' <orsonz...@gmail.com>; 'Baolin Wang' > <baolin.wa...@gmail.com>; 'Chunyan Zhang' <zhang.l...@gmail.com>; > 'linux-a...@vger.kernel.org' <linux-a...@vger.kernel.org>; > 'netdev@vger.kernel.org' <netdev@vger.kernel.org>; 'platform-driver- > x...@vger.kernel.org' <platform-driver-...@vger.kernel.org>; 'linux-arm- > ker...@lists.infradead.org' <linux-arm-ker...@lists.infradead.org>; > 'ker...@collabora.com' <ker...@collabora.com>; 'Barlomiej Zolnierkiewicz' > <b.zolnier...@samsung.com> > 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 <andrze...@collabora.com>; linux- > >> p...@vger.kernel.org > >> Cc: Rafael J . Wysocki <r...@rjwysocki.net>; Len Brown > >> <l...@kernel.org>; Jiri Pirko <j...@mellanox.com>; Ido Schimmel > >> <ido...@mellanox.com>; David S . Miller <da...@davemloft.net>; > Peter > >> Kaestle <pe...@piie.net>; Darren Hart <dvh...@infradead.org>; Andy > >> Shevchenko <a...@infradead.org>; Support Opensource > >> <support.opensou...@diasemi.com>; Daniel Lezcano > >> <daniel.lezc...@linaro.org>; Amit Kucheria > >> <amit.kuche...@verdurent.com>; Shawn Guo <shawn...@kernel.org>; > >> Sascha Hauer <s.ha...@pengutronix.de>; Pengutronix Kernel Team > >> <ker...@pengutronix.de>; Fabio Estevam <feste...@gmail.com>; NXP > >> Linux Team <linux-...@nxp.com>; Heiko Stuebner <he...@sntech.de>; > >> Orson Zhai <orsonz...@gmail.com>; Baolin Wang > >> <baolin.wa...@gmail.com>; Chunyan Zhang <zhang.l...@gmail.com>; > >> linux- a...@vger.kernel.org; netdev@vger.kernel.org; platform-driver- > >> x...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > >> ker...@collabora.com; Barlomiej Zolnierkiewicz > >> <b.zolnier...@samsung.com> > >> 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 <andrze...@collabora.com> > >>> Sent: Friday, April 24, 2020 12:57 AM > >>> To: linux...@vger.kernel.org > >>> Cc: Zhang, Rui <rui.zh...@intel.com>; Rafael J . Wysocki > >>> <r...@rjwysocki.net>; Len Brown <l...@kernel.org>; Jiri Pirko > >>> <j...@mellanox.com>; Ido Schimmel <ido...@mellanox.com>; David S . > >>> Miller <da...@davemloft.net>; Peter Kaestle <pe...@piie.net>; > Darren > >>> Hart <dvh...@infradead.org>; Andy Shevchenko <a...@infradead.org>; > >>> Support Opensource <support.opensou...@diasemi.com>; Daniel > Lezcano > >>> <daniel.lezc...@linaro.org>; Amit Kucheria > >>> <amit.kuche...@verdurent.com>; Shawn Guo <shawn...@kernel.org>; > >> Sascha > >>> Hauer <s.ha...@pengutronix.de>; Pengutronix Kernel Team > >>> <ker...@pengutronix.de>; Fabio Estevam <feste...@gmail.com>; NXP > >> Linux > >>> Team <linux-...@nxp.com>; Heiko Stuebner <he...@sntech.de>; > Orson > >> Zhai > >>> <orsonz...@gmail.com>; Baolin Wang <baolin.wa...@gmail.com>; > >> Chunyan > >>> Zhang <zhang.l...@gmail.com>; linux- a...@vger.kernel.org; > >>> netdev@vger.kernel.org; platform-driver- x...@vger.kernel.org; > >>> linux-arm-ker...@lists.infradead.org; > >>> ker...@collabora.com; Andrzej Pietrasiewicz > >>> <andrze...@collabora.com>; Barlomiej Zolnierkiewicz > >>> <b.zolnier...@samsung.com> > >>> 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
