linux-kernel - Re: [PATCH v5 07/24] clk: mediatek: Fix asymmetrical PLL enable and disable control

Open Source and information security mailing list archives

Message-ID: <1605676900.25459.2.camel@mtksdaap41>
Date:   Wed, 18 Nov 2020 13:21:40 +0800
From:   Weiyi Lu <weiyi.lu@...iatek.com>
To:     Ikjoon Jang <ikjn@...omium.org>
CC:     Matthias Brugger <matthias.bgg@...il.com>,
        Rob Herring <robh@...nel.org>, Stephen Boyd <sboyd@...nel.org>,
        Nicolas Boichat <drinkcat@...omium.org>,
        <srv_heupstream@...iatek.com>, <linux-kernel@...r.kernel.org>,
        <linux-mediatek@...ts.infradead.org>,
        "Yingjoe Chen" <yingjoe.chen@...iatek.com>,
        <linux-clk@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v5 07/24] clk: mediatek: Fix asymmetrical PLL enable and
 disable control

On Wed, 2020-11-18 at 11:55 +0800, Ikjoon Jang wrote:
> On Mon, Nov 09, 2020 at 10:03:32AM +0800, Weiyi Lu wrote:
> > In fact, the en_mask is a combination of divider enable mask
> > and pll enable bit(bit0).
> > Before this patch, we enabled both divider mask and bit0 in prepare(),
> > but only cleared the bit0 in unprepare().
> > In the future, we hope en_mask will only be used as divider enable mask.
> > The enable register(CON0) will be set in 2 steps:
> > first is divider mask, and then bit0 during prepare(), and vice versa.
> > But considering backward compatibility, at this stage we allow en_mask
> > to be a combination or a pure divider enable mask.
> > And then we will make en_mask a pure divider enable mask in another
> > following patch series.
> 
> I have a question on this: Are there any possible problems on controlling
> divider_en and bit0 at the same time? Or is this only for cleanups?
> 

Yes, this is only for cleanups and controlling divider_en and bit0 at
the same time will not cause any problem.

> If mtk_pll_data::en_mask is not allowed to control with bit0 together,
> I guess register_pll() also needs to check en_mask::bit0 is cleared?
> 
> > 
> > Signed-off-by: Weiyi Lu <weiyi.lu@...iatek.com>
> > ---
> >  drivers/clk/mediatek/clk-pll.c | 20 ++++++++++++++++----
> >  1 file changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c
> > index f440f2cd..11ed5d1 100644
> > --- a/drivers/clk/mediatek/clk-pll.c
> > +++ b/drivers/clk/mediatek/clk-pll.c
> > @@ -238,6 +238,7 @@ static int mtk_pll_prepare(struct clk_hw *hw)
> >  {
> >  	struct mtk_clk_pll *pll = to_mtk_clk_pll(hw);
> >  	u32 r;
> > +	u32 div_en_mask;
> >  
> >  	r = readl(pll->pwr_addr) | CON0_PWR_ON;
> >  	writel(r, pll->pwr_addr);
> > @@ -247,10 +248,15 @@ static int mtk_pll_prepare(struct clk_hw *hw)
> >  	writel(r, pll->pwr_addr);
> >  	udelay(1);
> >  
> > -	r = readl(pll->base_addr + REG_CON0);
> > -	r |= pll->data->en_mask;
> > +	r = readl(pll->base_addr + REG_CON0) | CON0_BASE_EN;
> >  	writel(r, pll->base_addr + REG_CON0);
> >  
> > +	div_en_mask = pll->data->en_mask & ~CON0_BASE_EN;
> > +	if (div_en_mask) {
> > +		r = readl(pll->base_addr + REG_CON0) | div_en_mask;
> > +		writel(r, pll->base_addr + REG_CON0);
> > +	}
> > +
> >  	__mtk_pll_tuner_enable(pll);
> >  
> >  	udelay(20);
> > @@ -268,6 +274,7 @@ static void mtk_pll_unprepare(struct clk_hw *hw)
> >  {
> >  	struct mtk_clk_pll *pll = to_mtk_clk_pll(hw);
> >  	u32 r;
> > +	u32 div_en_mask;
> >  
> >  	if (pll->data->flags & HAVE_RST_BAR) {
> >  		r = readl(pll->base_addr + REG_CON0);
> > @@ -277,8 +284,13 @@ static void mtk_pll_unprepare(struct clk_hw *hw)
> >  
> >  	__mtk_pll_tuner_disable(pll);
> >  
> > -	r = readl(pll->base_addr + REG_CON0);
> > -	r &= ~CON0_BASE_EN;
> > +	div_en_mask = pll->data->en_mask & ~CON0_BASE_EN;
> > +	if (div_en_mask) {
> > +		r = readl(pll->base_addr + REG_CON0) & ~div_en_mask;
> > +		writel(r, pll->base_addr + REG_CON0);
> > +	}
> > +
> > +	r = readl(pll->base_addr + REG_CON0) & ~CON0_BASE_EN;
> >  	writel(r, pll->base_addr + REG_CON0);
> >  
> >  	r = readl(pll->pwr_addr) | CON0_ISO_EN;
> > -- 
> > 1.8.1.1.dirty
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@...ts.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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