Re: [v4,0/7] Add Mediatek thermal dirver and dtsi

From: Michael Kao <michael.kao@mediatek.com>
To: Matthias Brugger <matthias.bgg@gmail.com>
Cc: Zhang Rui <rui.zhang@intel.com>,
	Eduardo Valentin <edubezval@gmail.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>, <hsinyi@chromium.org>,
	<linux-pm@vger.kernel.org>, <srv_heupstream@mediatek.com>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-mediatek@lists.infradead.org>
Subject: Re: [v4,0/7] Add Mediatek thermal dirver and dtsi
Date: Fri, 22 May 2020 12:07:38 +0800	[thread overview]
Message-ID: <1590120458.22249.24.camel@mtksdccf07> (raw)
In-Reply-To: <7e205390-c7a7-b8c9-3ba2-344a04dc6696@gmail.com>

On Thu, 2020-05-21 at 14:51 +0200, Matthias Brugger wrote:
> Hi Michael,
> 
> On 23/03/2020 13:15, Michael Kao wrote:
> > This patchset supports for MT8183 chip to mtk_thermal.c.
> > Add thermal zone of all the thermal sensor in SoC for
> > another get temperatrue. They don't need to thermal throttle.
> > And we bind coolers for thermal zone nodes of cpu_thermal.
> > 
> > Rebase to kernel-5.6-rc1.
> > 
> > Update content:
> > 
> > [1/7]
> >     - Squash thermal zone settings in the dtsi from [v3,5/8]
> >       arm64: dts: mt8183: Increase polling frequency for CPU thermal zone
> > 
> >     - Remove the property of interrupts and mediatek,hw-reset-temp
> > 
> > [2/7]
> >     - Correct commit message
> > 
> > [4/7]
> >     - Change the target temperature to the 80C and change the commit message
> > 
> > [6/7]
> >     - Adjust newline alignment
> > 
> >     - Fix the judgement on the return value of registering thermal zone
> > 
> > This patch series base on these patches [1].
> > 
> > [v7,3/3] PM / AVS: SVS: Introduce SVS engine (https://patchwork.kernel.org/patch/11439829/)
> > 
> > Matthias Kaehlcke (1):
> >   arm64: dts: mt8183: Configure CPU cooling
> > 
> > Michael Kao (6):
> >   arm64: dts: mt8183: add thermal zone node
> >   arm64: dts: mt8183: add dynamic power coefficients
> >   arm64: dts: mt8183: Add #cooling-cells to CPU nodes
> >   thermal: mediatek: mt8183: fix bank number settings
> 
> Do I understand correctly that we need to fix the bank number before we can add
> the device tree changes. And that the last two patches are enhancements for the
> driver but needed to get a working version?
> 
> Regards,
> Matthias

Hi Matthias,

There is one bank setting of mt8183 config.
If the device tree merged first. I worry that it will crash when the
thermal zone read temperature.
It will access the invalid index of bank.
So please add the patch "fix bank number settings "
first.222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222

> 
> >   thermal: mediatek: add another get_temp ops for thermal sensors
> >   thermal: mediatek: use spinlock to protect PTPCORESEL
> > 
> >  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 156 +++++++++++++++++++++++
> >  drivers/thermal/mtk_thermal.c            |  88 +++++++++++--
> >  2 files changed, 231 insertions(+), 13 deletions(-)
> > 