From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com> To: "Rafael J. Wysocki" <rafael@kernel.org> Cc: "Daniel Lezcano" <daniel.lezcano@linexp.org>, "Daniel Lezcano" <daniel.lezcano@linaro.org>, "Kevin Hilman" <khilman@baylibre.com>, "Alexandre Bailon" <abailon@baylibre.com>, "Linux PM" <linux-pm@vger.kernel.org>, "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>, "Amit Kucheria" <amitk@kernel.org>, "Zhang Rui" <rui.zhang@intel.com>, "Jonathan Corbet" <corbet@lwn.net>, "Len Brown" <lenb@kernel.org>, "Raju Rangoju" <rajur@chelsio.com>, "David S. Miller" <davem@davemloft.net>, "Jakub Kicinski" <kuba@kernel.org>, "Paolo Abeni" <pabeni@redhat.com>, "Ido Schimmel" <idosch@nvidia.com>, "Petr Machata" <petrm@nvidia.com>, "Luca Coelho" <luciano.coelho@intel.com>, "Kalle Valo" <kvalo@kernel.org>, "Peter Kaestle" <peter@piie.net>, "Hans de Goede" <hdegoede@redhat.com>, "Mark Gross" <markgross@kernel.org>, "Sebastian Reichel" <sre@kernel.org>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "Support Opensource" <support.opensource@diasemi.com>, "Shawn Guo" <shawnguo@kernel.org>, "Sascha Hauer" <s.hauer@pengutronix.de>, "Pengutronix Kernel Team" <kernel@pengutronix.de>, "Fabio Estevam" <festevam@gmail.com>, "NXP Linux Team" <linux-imx@nxp.com>, "Niklas Söderlund" <niklas.soderlund@ragnatech.se>, "Miri Korenblit" <miriam.rachel.korenblit@intel.com>, "Johannes Berg" <johannes.berg@intel.com>, "Sumeet Pawnikar" <sumeet.r.pawnikar@intel.com>, "Dan Carpenter" <dan.carpenter@oracle.com>, "Chuansheng Liu" <chuansheng.liu@intel.com>, "Jiasheng Jiang" <jiasheng@iscas.ac.cn>, "Antoine Tenart" <atenart@kernel.org>, "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>, "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>, "open list:ACPI THERMAL DRIVER" <linux-acpi@vger.kernel.org>, "open list:CXGB4 ETHERNET DRIVER (CXGB4)" <netdev@vger.kernel.org>, "open list:INTEL WIRELESS WIFI LINK (iwlwifi)" <linux-wireless@vger.kernel.org>, "open list:ACER ASPIRE ONE TEMPERATURE AND FAN DRIVER" <platform-driver-x86@vger.kernel.org>, "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org>, "open list:RENESAS R-CAR THERMAL DRIVERS" <linux-renesas-soc@vger.kernel.org> Subject: Re: [PATCH v2 01/14] thermal/core: Change thermal_zone_ops to thermal_sensor_ops Date: Tue, 17 May 2022 12:02:13 -0700 [thread overview] Message-ID: <afcc7b08ebe2578d32e6595d258afeec3e73512e.camel@linux.intel.com> (raw) In-Reply-To: <CAJZ5v0hqN-zKZvWTNPzW2P22Dirmyh99qyycf+US4Z9Yxw9mhA@mail.gmail.com> On Tue, 2022-05-17 at 20:53 +0200, Rafael J. Wysocki wrote: > On Tue, May 17, 2022 at 6:51 PM srinivas pandruvada > <srinivas.pandruvada@linux.intel.com> wrote: > > > > On Tue, 2022-05-17 at 17:42 +0200, Rafael J. Wysocki wrote: > > > On Sat, May 7, 2022 at 2:55 PM Daniel Lezcano > > > <daniel.lezcano@linexp.org> wrote: > > > > > > > > A thermal zone is software abstraction of a sensor associated > > > > with > > > > properties and cooling devices if any. > > > > > > > > The fact that we have thermal_zone and thermal_zone_ops mixed > > > > is > > > > confusing and does not clearly identify the different > > > > components > > > > entering in the thermal management process. A thermal zone > > > > appears > > > > to > > > > be a sensor while it is not. > > > > > > Well, the majority of the operations in thermal_zone_ops don't > > > apply > > > to thermal sensors. For example, ->set_trips(), - > > > >get_trip_type(), > > > ->get_trip_temp(). > > > > > In past we discussed adding thermal sensor sysfs with threshold to > > notify temperature. > > > > So sensor can have set/get_threshold() functions instead of the > > set/get_trip for zones. > > > > Like we have /sys/class/thermal_zone* we can have > > /sys/class/thermal_sensor*. > > Exactly, so renaming thermal_zone_ops as thermal_sensor_ops isn't > quite helpful in this respect. > > IMO there should be operations for sensors and there should be > operations for thermal zones and those two sets of operations should > be different. > > > Thermal sensor(s) are bound to thermal zones. > > So I think that this binding should be analogous to the binding > between thermal zones and cooling devices. > > > This can also include multiple sensors in a zone and can create a > > virtual sensor also. > > It can. > > However, what's the difference between a thermal zone with multiple > sensors and a thermal zone with one virtual sensor being an aggregate > of multiple physical sensors? > Either way is fine. A thermal sensor can be aggregate of other sensors. > Both involve some type of aggregation of temperature values measured > by the physical sensors. > > > > > In order to set the scene for multiple thermal sensors > > > > aggregated > > > > into > > > > a single thermal zone. Rename the thermal_zone_ops to > > > > thermal_sensor_ops, that will appear clearyl the thermal zone > > > > is > > > > not a > > > > sensor but an abstraction of one [or multiple] sensor(s). > > > > > > So I'm not convinced that the renaming mentioned above is > > > particularly > > > clean either. > > > > > > IMV the way to go would be to split the thermal sensor > > > operations, > > > like ->get_temp(), out of thermal_zone_ops. > > > > > > But then it is not clear what a thermal zone with multiple > > > sensors in > > > it really means. I guess it would require an aggregation > > > function to > > > combine the thermal sensors in it that would produce an effective > > > temperature to check against the trip points. > > > > > > Honestly, I don't think that setting a separate set of trips for > > > each > > > sensor in a thermal zone would make a lot of sense. > >
WARNING: multiple messages have this Message-ID (diff)
From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com> To: "Rafael J. Wysocki" <rafael@kernel.org> Cc: "Daniel Lezcano" <daniel.lezcano@linexp.org>, "Daniel Lezcano" <daniel.lezcano@linaro.org>, "Kevin Hilman" <khilman@baylibre.com>, "Alexandre Bailon" <abailon@baylibre.com>, "Linux PM" <linux-pm@vger.kernel.org>, "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>, "Amit Kucheria" <amitk@kernel.org>, "Zhang Rui" <rui.zhang@intel.com>, "Jonathan Corbet" <corbet@lwn.net>, "Len Brown" <lenb@kernel.org>, "Raju Rangoju" <rajur@chelsio.com>, "David S. Miller" <davem@davemloft.net>, "Jakub Kicinski" <kuba@kernel.org>, "Paolo Abeni" <pabeni@redhat.com>, "Ido Schimmel" <idosch@nvidia.com>, "Petr Machata" <petrm@nvidia.com>, "Luca Coelho" <luciano.coelho@intel.com>, "Kalle Valo" <kvalo@kernel.org>, "Peter Kaestle" <peter@piie.net>, "Hans de Goede" <hdegoede@redhat.com>, "Mark Gross" <markgross@kernel.org>, "Sebastian Reichel" <sre@kernel.org>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "Support Opensource" <support.opensource@diasemi.com>, "Shawn Guo" <shawnguo@kernel.org>, "Sascha Hauer" <s.hauer@pengutronix.de>, "Pengutronix Kernel Team" <kernel@pengutronix.de>, "Fabio Estevam" <festevam@gmail.com>, "NXP Linux Team" <linux-imx@nxp.com>, "Niklas Söderlund" <niklas.soderlund@ragnatech.se>, "Miri Korenblit" <miriam.rachel.korenblit@intel.com>, "Johannes Berg" <johannes.berg@intel.com>, "Sumeet Pawnikar" <sumeet.r.pawnikar@intel.com>, "Dan Carpenter" <dan.carpenter@oracle.com>, "Chuansheng Liu" <chuansheng.liu@intel.com>, "Jiasheng Jiang" <jiasheng@iscas.ac.cn>, "Antoine Tenart" <atenart@kernel.org>, "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>, "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>, "open list:ACPI THERMAL DRIVER" <linux-acpi@vger.kernel.org>, "open list:CXGB4 ETHERNET DRIVER (CXGB4)" <netdev@vger.kernel.org>, "open list:INTEL WIRELESS WIFI LINK (iwlwifi)" <linux-wireless@vger.kernel.org>, "open list:ACER ASPIRE ONE TEMPERATURE AND FAN DRIVER" <platform-driver-x86@vger.kernel.org>, "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org>, "open list:RENESAS R-CAR THERMAL DRIVERS" <linux-renesas-soc@vger.kernel.org> Subject: Re: [PATCH v2 01/14] thermal/core: Change thermal_zone_ops to thermal_sensor_ops Date: Tue, 17 May 2022 12:02:13 -0700 [thread overview] Message-ID: <afcc7b08ebe2578d32e6595d258afeec3e73512e.camel@linux.intel.com> (raw) In-Reply-To: <CAJZ5v0hqN-zKZvWTNPzW2P22Dirmyh99qyycf+US4Z9Yxw9mhA@mail.gmail.com> On Tue, 2022-05-17 at 20:53 +0200, Rafael J. Wysocki wrote: > On Tue, May 17, 2022 at 6:51 PM srinivas pandruvada > <srinivas.pandruvada@linux.intel.com> wrote: > > > > On Tue, 2022-05-17 at 17:42 +0200, Rafael J. Wysocki wrote: > > > On Sat, May 7, 2022 at 2:55 PM Daniel Lezcano > > > <daniel.lezcano@linexp.org> wrote: > > > > > > > > A thermal zone is software abstraction of a sensor associated > > > > with > > > > properties and cooling devices if any. > > > > > > > > The fact that we have thermal_zone and thermal_zone_ops mixed > > > > is > > > > confusing and does not clearly identify the different > > > > components > > > > entering in the thermal management process. A thermal zone > > > > appears > > > > to > > > > be a sensor while it is not. > > > > > > Well, the majority of the operations in thermal_zone_ops don't > > > apply > > > to thermal sensors. For example, ->set_trips(), - > > > >get_trip_type(), > > > ->get_trip_temp(). > > > > > In past we discussed adding thermal sensor sysfs with threshold to > > notify temperature. > > > > So sensor can have set/get_threshold() functions instead of the > > set/get_trip for zones. > > > > Like we have /sys/class/thermal_zone* we can have > > /sys/class/thermal_sensor*. > > Exactly, so renaming thermal_zone_ops as thermal_sensor_ops isn't > quite helpful in this respect. > > IMO there should be operations for sensors and there should be > operations for thermal zones and those two sets of operations should > be different. > > > Thermal sensor(s) are bound to thermal zones. > > So I think that this binding should be analogous to the binding > between thermal zones and cooling devices. > > > This can also include multiple sensors in a zone and can create a > > virtual sensor also. > > It can. > > However, what's the difference between a thermal zone with multiple > sensors and a thermal zone with one virtual sensor being an aggregate > of multiple physical sensors? > Either way is fine. A thermal sensor can be aggregate of other sensors. > Both involve some type of aggregation of temperature values measured > by the physical sensors. > > > > > In order to set the scene for multiple thermal sensors > > > > aggregated > > > > into > > > > a single thermal zone. Rename the thermal_zone_ops to > > > > thermal_sensor_ops, that will appear clearyl the thermal zone > > > > is > > > > not a > > > > sensor but an abstraction of one [or multiple] sensor(s). > > > > > > So I'm not convinced that the renaming mentioned above is > > > particularly > > > clean either. > > > > > > IMV the way to go would be to split the thermal sensor > > > operations, > > > like ->get_temp(), out of thermal_zone_ops. > > > > > > But then it is not clear what a thermal zone with multiple > > > sensors in > > > it really means. I guess it would require an aggregation > > > function to > > > combine the thermal sensors in it that would produce an effective > > > temperature to check against the trip points. > > > > > > Honestly, I don't think that setting a separate set of trips for > > > each > > > sensor in a thermal zone would make a lot of sense. > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-05-17 19:02 UTC|newest] Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-07 12:54 [PATCH v2 00/14] thermal OF rework Daniel Lezcano 2022-05-07 12:54 ` [PATCH v2 01/14] thermal/core: Change thermal_zone_ops to thermal_sensor_ops Daniel Lezcano 2022-05-07 12:54 ` Daniel Lezcano 2022-05-08 10:26 ` Andy Shevchenko 2022-05-08 10:26 ` Andy Shevchenko 2022-05-09 9:20 ` Geert Uytterhoeven 2022-05-09 9:20 ` Geert Uytterhoeven 2022-05-17 15:42 ` Rafael J. Wysocki 2022-05-17 15:42 ` Rafael J. Wysocki 2022-05-17 16:50 ` srinivas pandruvada 2022-05-17 16:50 ` srinivas pandruvada 2022-05-17 18:53 ` Rafael J. Wysocki 2022-05-17 18:53 ` Rafael J. Wysocki 2022-05-17 19:02 ` srinivas pandruvada [this message] 2022-05-17 19:02 ` srinivas pandruvada 2022-05-17 19:07 ` Rafael J. Wysocki 2022-05-17 19:07 ` Rafael J. Wysocki 2022-06-26 17:33 ` Daniel Lezcano 2022-06-26 17:33 ` Daniel Lezcano 2022-05-07 12:54 ` [PATCH v2 02/14] thermal/core: Add a thermal sensor structure in the thermal zone Daniel Lezcano 2022-05-07 12:54 ` Daniel Lezcano 2022-05-09 7:00 ` Krzysztof Kozlowski 2022-05-09 7:00 ` Krzysztof Kozlowski 2022-05-09 9:28 ` Geert Uytterhoeven 2022-05-09 9:28 ` Geert Uytterhoeven 2022-05-07 12:54 ` [PATCH v2 03/14] thermal/core: Remove duplicate information when an error occurs Daniel Lezcano 2022-05-17 15:45 ` Rafael J. Wysocki 2022-05-07 12:54 ` [PATCH v2 04/14] thermal/of: Replace device node match with device node search Daniel Lezcano 2022-05-07 12:54 ` [PATCH v2 05/14] thermal/of: Remove the device node pointer for thermal_trip Daniel Lezcano 2022-05-07 12:54 ` [PATCH v2 06/14] thermal/of: Move thermal_trip structure to thermal.h Daniel Lezcano 2022-05-07 12:54 ` [PATCH v2 07/14] thermal/core: Remove unneeded EXPORT_SYMBOLS Daniel Lezcano 2022-05-07 12:54 ` [PATCH v2 08/14] thermal/core: Move thermal_set_delay_jiffies to static Daniel Lezcano 2022-05-07 12:54 ` [PATCH v2 09/14] thermal/core: Rename trips to ntrips Daniel Lezcano 2022-05-07 12:54 ` [PATCH v2 10/14] thermal/core: Add thermal_trip in thermal_zone Daniel Lezcano 2022-05-07 12:54 ` [PATCH v2 11/14] thermal/core: Register with the trip points Daniel Lezcano 2022-05-07 12:54 ` [PATCH v2 12/14] thermal/of: Store the trips in the thermal zone Daniel Lezcano 2022-05-07 12:54 ` [PATCH v2 13/14] thermal/of: Use thermal trips stored " Daniel Lezcano 2022-05-07 12:54 ` [PATCH v2 14/14] thermal/of: Initialize trip points separately Daniel Lezcano 2022-05-13 17:23 ` [PATCH v2 00/14] thermal OF rework Daniel Lezcano 2022-05-13 17:56 ` Rafael J. Wysocki 2022-05-13 19:18 ` Daniel Lezcano
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=afcc7b08ebe2578d32e6595d258afeec3e73512e.camel@linux.intel.com \ --to=srinivas.pandruvada@linux.intel.com \ --cc=abailon@baylibre.com \ --cc=amitk@kernel.org \ --cc=andriy.shevchenko@linux.intel.com \ --cc=atenart@kernel.org \ --cc=chuansheng.liu@intel.com \ --cc=corbet@lwn.net \ --cc=dan.carpenter@oracle.com \ --cc=daniel.lezcano@linaro.org \ --cc=daniel.lezcano@linexp.org \ --cc=davem@davemloft.net \ --cc=festevam@gmail.com \ --cc=hdegoede@redhat.com \ --cc=idosch@nvidia.com \ --cc=jiasheng@iscas.ac.cn \ --cc=johannes.berg@intel.com \ --cc=kernel@pengutronix.de \ --cc=khilman@baylibre.com \ --cc=kuba@kernel.org \ --cc=kvalo@kernel.org \ --cc=lenb@kernel.org \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-doc@vger.kernel.org \ --cc=linux-imx@nxp.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux-renesas-soc@vger.kernel.org \ --cc=linux-wireless@vger.kernel.org \ --cc=luciano.coelho@intel.com \ --cc=markgross@kernel.org \ --cc=miquel.raynal@bootlin.com \ --cc=miriam.rachel.korenblit@intel.com \ --cc=netdev@vger.kernel.org \ --cc=niklas.soderlund@ragnatech.se \ --cc=pabeni@redhat.com \ --cc=peter@piie.net \ --cc=petrm@nvidia.com \ --cc=platform-driver-x86@vger.kernel.org \ --cc=rafael@kernel.org \ --cc=rajur@chelsio.com \ --cc=rui.zhang@intel.com \ --cc=s.hauer@pengutronix.de \ --cc=shawnguo@kernel.org \ --cc=sre@kernel.org \ --cc=sumeet.r.pawnikar@intel.com \ --cc=support.opensource@diasemi.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.