Re: [v4,7/7] thermal: mediatek: use spinlock to protect PTPCORESEL

From: Michael Kao <michael.kao@mediatek.com>
To: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, srv_heupstream@mediatek.com,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Eduardo Valentin <edubezval@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	linux-mediatek@lists.infradead.org, hsinyi@chromium.org,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Zhang Rui <rui.zhang@intel.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [v4,7/7] thermal: mediatek: use spinlock to protect PTPCORESEL
Date: Tue, 7 Jul 2020 10:57:01 +0800	[thread overview]
Message-ID: <1594090621.20216.0.camel@mtksdccf07> (raw)
In-Reply-To: <1591329023.12739.0.camel@mtksdccf07>

On Fri, 2020-06-05 at 11:50 +0800, Michael Kao wrote:
> On Fri, 2020-05-22 at 17:36 +0200, Daniel Lezcano wrote:
> > On 23/03/2020 13:15, Michael Kao wrote:
> > > From: "michael.kao" <michael.kao@mediatek.com>
> > > 
> > > The driver of thermal and svs will use the
> > > same register for the project which should select
> > > bank before reading sensor value.
> > 
> > Here there is a design problem AFAICT. The sensor should not be using
> > external locks.
> > 
> The PTPCORESEL is a common register used by svs and thermal.
> The thermal need to ensure PTPCORESEL register will not be changed by
> svs when thermal switch bank to read raw data of temperature.
> So we use svs_lock to make sure there is no conflict between the two
> drivers.
> > 
> > > Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> > > ---
> > >  drivers/thermal/mtk_thermal.c | 9 ++++-----
> > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> > > index 9eaca432920e..594ad4f0f8cd 100644
> > > --- a/drivers/thermal/mtk_thermal.c
> > > +++ b/drivers/thermal/mtk_thermal.c
> > > @@ -22,6 +22,7 @@
> > >  #include <linux/thermal.h>
> > >  #include <linux/reset.h>
> > >  #include <linux/types.h>
> > > +#include <linux/power/mtk_svs.h>
> > >  
> > >  /* AUXADC Registers */
> > >  #define AUXADC_CON1_SET_V	0x008
> > > @@ -262,7 +263,7 @@ struct mtk_thermal {
> > >  	struct clk *clk_peri_therm;
> > >  	struct clk *clk_auxadc;
> > >  	/* lock: for getting and putting banks */
> > > -	struct mutex lock;
> > > +	unsigned long flags;
> > >  
> > >  	/* Calibration values */
> > >  	s32 adc_ge;
> > > @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank)
> > >  	u32 val;
> > >  
> > >  	if (mt->conf->need_switch_bank) {
> > > -		mutex_lock(&mt->lock);
> > > +		mt->flags = claim_mtk_svs_lock();
> > >  
> > >  		val = readl(mt->thermal_base + PTPCORESEL);
> > >  		val &= ~0xf;
> > > @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank)
> > >  	struct mtk_thermal *mt = bank->mt;
> > >  
> > >  	if (mt->conf->need_switch_bank)
> > > -		mutex_unlock(&mt->lock);
> > > +		release_mtk_svs_lock(mt->flags);
> > >  }
> > >  
> > >  /**
> > > @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev)
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > -	mutex_init(&mt->lock);
> > > -
> > >  	mt->dev = &pdev->dev;
> > >  
> > >  	auxadc = of_parse_phandle(np, "mediatek,auxadc", 0);
> > > 
> > 
> > 
> Hi Daniel,

Gently ping.
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel