Messages in this thread | | | Subject | Re: [PATCH v5 07/24] clk: mediatek: Fix asymmetrical PLL enable and disable control | From | Weiyi Lu <> | Date | Wed, 18 Nov 2020 13:21:40 +0800 |
| |
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@mediatek.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@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
| |