linux-kernel - Re: [PATCH v2 01/14] thermal/core: Change thermal_zone_ops to thermal_sensor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0hqN-zKZvWTNPzW2P22Dirmyh99qyycf+US4Z9Yxw9mhA@mail.gmail.com>
Date:   Tue, 17 May 2022 20:53:54 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     srinivas pandruvada <srinivas.pandruvada@...ux.intel.com>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Daniel Lezcano <daniel.lezcano@...exp.org>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Kevin Hilman <khilman@...libre.com>,
        Alexandre Bailon <abailon@...libre.com>,
        Linux PM <linux-pm@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Amit Kucheria <amitk@...nel.org>,
        Zhang Rui <rui.zhang@...el.com>,
        Jonathan Corbet <corbet@....net>, Len Brown <lenb@...nel.org>,
        Raju Rangoju <rajur@...lsio.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Ido Schimmel <idosch@...dia.com>,
        Petr Machata <petrm@...dia.com>,
        Luca Coelho <luciano.coelho@...el.com>,
        Kalle Valo <kvalo@...nel.org>, Peter Kaestle <peter@...e.net>,
        Hans de Goede <hdegoede@...hat.com>,
        Mark Gross <markgross@...nel.org>,
        Sebastian Reichel <sre@...nel.org>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Support Opensource <support.opensource@...semi.com>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        Niklas Söderlund <niklas.soderlund@...natech.se>,
        Miri Korenblit <miriam.rachel.korenblit@...el.com>,
        Johannes Berg <johannes.berg@...el.com>,
        Sumeet Pawnikar <sumeet.r.pawnikar@...el.com>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Chuansheng Liu <chuansheng.liu@...el.com>,
        Jiasheng Jiang <jiasheng@...as.ac.cn>,
        Antoine Tenart <atenart@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        "open list:ACPI THERMAL DRIVER" <linux-acpi@...r.kernel.org>,
        "open list:CXGB4 ETHERNET DRIVER (CXGB4)" <netdev@...r.kernel.org>,
        "open list:INTEL WIRELESS WIFI LINK (iwlwifi)" 
        <linux-wireless@...r.kernel.org>,
        "open list:ACER ASPIRE ONE TEMPERATURE AND FAN DRIVER" 
        <platform-driver-x86@...r.kernel.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>,
        "open list:RENESAS R-CAR THERMAL DRIVERS" 
        <linux-renesas-soc@...r.kernel.org>
Subject: Re: [PATCH v2 01/14] thermal/core: Change thermal_zone_ops to thermal_sensor_ops

On Tue, May 17, 2022 at 6:51 PM srinivas pandruvada
<srinivas.pandruvada@...ux.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@...exp.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?

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.
>