Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 10 May 2019 15:42:19 +0530
From:   Viresh Kumar <viresh.kumar@linaro.org>
To:     Andy Tang <andy.tang@nxp.com>
Cc:     Daniel Lezcano <daniel.lezcano@linaro.org>,
        Shawn Guo <shawnguo@kernel.org>, Leo Li <leoyang.li@nxp.com>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
        "rui.zhang@intel.com" <rui.zhang@intel.com>,
        "edubezval@gmail.com" <edubezval@gmail.com>
Subject: Re: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal
 zone node
Message-ID: <20190510101219.oruzvzlk7mm6iahw@vireshk-i7>
References: <20190423022507.34969-1-andy.tang@nxp.com>
 <20190510031335.GD15856@dragon>
 <VI1PR04MB4333D24227603D1497BA3CB9F30C0@VI1PR04MB4333.eurprd04.prod.outlook.com>
 <9fb2e306-38c7-2af7-5470-ff5bc4e23370@linaro.org>
 <VI1PR04MB4333372C1DABD0E4C9DD7FE8F30C0@VI1PR04MB4333.eurprd04.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <VI1PR04MB4333372C1DABD0E4C9DD7FE8F30C0@VI1PR04MB4333.eurprd04.prod.outlook.com>
User-Agent: NeoMutt/20180716-391-311a52
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 10-05-19, 08:47, Andy Tang wrote:
> + Viresh for help.
> 
> > -----Original Message-----
> > From: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Sent: 2019年5月10日 15:17
> > To: Andy Tang <andy.tang@nxp.com>; Shawn Guo <shawnguo@kernel.org>
> > Cc: Leo Li <leoyang.li@nxp.com>; robh+dt@kernel.org;
> > mark.rutland@arm.com; linux-arm-kernel@lists.infradead.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux-pm@vger.kernel.org; rui.zhang@intel.com; edubezval@gmail.com
> > Subject: Re: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more thermal
> > zone node
> > 
> > Caution: EXT Email
> > 
> > On 10/05/2019 05:40, Andy Tang wrote:
> > >> -----Original Message-----
> > >> From: Shawn Guo <shawnguo@kernel.org>
> > >> Sent: 2019年5月10日 11:14
> > >> To: Andy Tang <andy.tang@nxp.com>
> > >> Cc: Leo Li <leoyang.li@nxp.com>; robh+dt@kernel.org;
> > >> mark.rutland@arm.com; linux-arm-kernel@lists.infradead.org;
> > >> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > >> linux-pm@vger.kernel.org; daniel.lezcano@linaro.org;
> > >> rui.zhang@intel.com; edubezval@gmail.com
> > >> Subject: [EXT] Re: [PATCH v6] arm64: dts: ls1088a: add one more
> > >> thermal zone node
> > >>
> > >> Caution: EXT Email
> > >>
> > >> On Tue, Apr 23, 2019 at 10:25:07AM +0800, Yuantian Tang wrote:
> > >>> Ls1088a has 2 thermal sensors, core cluster and SoC platform. Core
> > >>> cluster sensor is used to monitor the temperature of core and SoC
> > >>> platform is for platform. The current dts only support the first sensor.
> > >>> This patch adds the second sensor node to dts to enable it.
> > >>>
> > >>> Signed-off-by: Yuantian Tang <andy.tang@nxp.com>
> > >>> ---
> > >>> v6:
> > >>>         - add cooling device map to cpu0-7 in platform node.
> > > I like to explain a little. I think it makes sense that multiple thermal zone
> > map to same cooling device.
> > > In this way, no matter which thermal zone raises a temp alarm, it can call
> > cooling device to chill out.
> > > I also asked cpufreq maintainer about the cooling map issue, he think it
> > would be fine.

Yes, you asked me and I said it should be okay.

> > > I have tested and no issue found.
> > >
> > > Daniel, what's your thought?
> > 
> > If there are multiple thermal zones, they will be managed by different
> > instances of a thermal governor. Each instances will act on the shared cooling
> > device and will collide in their decisions:
> > 
> >  - If the sensors are closed, their behavior will be similar regarding the
> > temperature. The governors may take the same decision for the cooling
> > device. But in such case having just one thermal zone managed is enough.
> > 
> >  - If the sensors are not closed, their behavior will be different regarding the
> > temperature. The governors will take different decision regarding the cooling
> > device (one will decrease the freq, other will increase the freq).
> > 
> > As the thermal governors are not able to manage several thermal zones and
> > there is one cooling device (the cpu cooling device), this setup won't work as
> > expected IMO.
> > 
> > The setup making sense is having a thermal zone per 'cluster' and a cooling
> > device per 'cluster'. That means the platform has one clock line per 'cluster'.
> > The thermal management happens in a self-contained thermal zone (one
> > cooling device - one governor - one thermal zone).
> > 
> > In the case of HMP, other combinations are possible to be optimal.

But not sure how I missed the obvious, though I do remember thinking about this.

So the problem is that the cpu_cooling driver will get requests in parallel to
set different max frequencies and the last call will always win and may result
in undesired outcome.

Sorry about creating the confusion.

-- 
viresh