Date:   Wed, 6 Jan 2021 18:35:50 +0800
From:   Ikjoon Jang <ikjn@...omium.org>
To:     Weiyi Lu <weiyi.lu@...iatek.com>
Cc:     Matthias Brugger <matthias.bgg@...il.com>,
        Rob Herring <robh@...nel.org>, Stephen Boyd <sboyd@...nel.org>,
        Nicolas Boichat <drinkcat@...omium.org>,
        srv_heupstream <srv_heupstream@...iatek.com>,
        open list <linux-kernel@...r.kernel.org>,
        Project_Global_Chrome_Upstream_Group@...iatek.com,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>, linux-clk@...r.kernel.org,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v6 07/22] clk: mediatek: Fix asymmetrical PLL enable and
 disable control

On Tue, Dec 22, 2020 at 9:12 PM Weiyi Lu <weiyi.lu@...iatek.com> 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.
>
> Signed-off-by: Weiyi Lu <weiyi.lu@...iatek.com>

Reviewed-by: Ikjoon Jang <ikjn@...omium.org>

> ---
>  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-mediatek mailing list
> Linux-mediatek@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

Powered by blists - more mailing lists