From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 795BFCA0FF0 for ; Mon, 25 Aug 2025 14:59:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc: To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=W4oY2oiTfCUt1+MOP5G5IqQvEA4mJfEjXMDK2BOiY2g=; b=38+l1DqcpvZh/DQLPYXYP2zVgD rwRXw+MDFCdbVAZUoWvv9RYxHk+oVchtYo6Le0sX5zWD+asHePuM6YbNVlBJQgpzFxQKg4eRjDVIy IZUyKsY66jSgoZ5477w37k+XKW3s5gOFWPPpOnymdBipXeuO0PWB00BmtAmhZj4fFbxtmJKGONITU Kn5SU1HtPcnkg7weIXtYNkEKIX59SD52nbHBbHzKcmAALgbb/SmhBNs2N4c4j2ViRv4QEIFIn80Sx 4SadQNbMGBGHG+M8Jb2s/e+M7cI91MzJIDRW15IiVjOIitUYEpZyjN83+JW3ik59iD954PO5oEkKe 1WVcE81Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uqYew-00000008L9J-289k; Mon, 25 Aug 2025 14:58:58 +0000 Received: from bali.collaboradmins.com ([2a01:4f8:201:9162::2]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uqWVS-00000007wqz-29a7; Mon, 25 Aug 2025 12:41:06 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1756125659; bh=DTT7025o6QZOA9whxQI3H5F2CGpujugULOrA7+jZGEc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XxWTc/E5VW0OQ92f92pKw17Q3jfIUceMh07Qfy80KNq0z9U9rRY1PmnQ7RvWJ8QOA k/RLPcz22lYDsI/+tpqx7W4JOpRl6l+svWld0mQk7h8Bi9Cd0C94j7oaHtkW3ISMBg mBF3rYU1axmMRpzsHkyR0+y8vFIQB691g0lBYWEOVS8H0WUjFGmZ9Dvn9vwb1AfqoQ OojGbnLeI1c+s0W7M+8HL2aJKr1rRVxttQILAwcPMJdnnyt9hC1S1JAutElkWWwOXt aKagE4e71XnMYGrxdGDsvIf5M9Ipi3ty3FgTXuviUmeYsQKMqwmb19wsj8R/eM0yaa D+/x4sLR478eA== Received: from laura.lan (unknown [IPv6:2001:b07:646b:e2:b1df:895a:e67b:5cd4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: laura.nao) by bali.collaboradmins.com (Postfix) with ESMTPSA id 6BDEC17E0523; Mon, 25 Aug 2025 14:40:58 +0200 (CEST) From: Laura Nao To: wenst@chromium.org Cc: angelogioacchino.delregno@collabora.com, conor+dt@kernel.org, devicetree@vger.kernel.org, guangjie.song@mediatek.com, kernel@collabora.com, krzk+dt@kernel.org, laura.nao@collabora.com, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, matthias.bgg@gmail.com, mturquette@baylibre.com, netdev@vger.kernel.org, nfraprado@collabora.com, p.zabel@pengutronix.de, richardcochran@gmail.com, robh@kernel.org, sboyd@kernel.org Subject: Re: [PATCH v4 02/27] clk: mediatek: clk-pll: Add ops for PLLs using set/clr regs and FENC Date: Mon, 25 Aug 2025 14:39:52 +0200 Message-Id: <20250825123952.208448-1-laura.nao@collabora.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250825_054102_744923_FC38A405 X-CRM114-Status: GOOD ( 25.20 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org Hi Chen-Yu, On 8/15/25 05:18, Chen-Yu Tsai wrote: > On Tue, Aug 5, 2025 at 10:55 PM Laura Nao wrote: >> >> MT8196 uses a combination of set/clr registers to control the PLL >> enable state, along with a FENC bit to check the preparation status. >> Add new set of PLL clock operations with support for set/clr enable and >> FENC status logic. >> >> Reviewed-by: Nícolas F. R. A. Prado >> Reviewed-by: AngeloGioacchino Del Regno >> Signed-off-by: Laura Nao >> --- >> drivers/clk/mediatek/clk-pll.c | 42 +++++++++++++++++++++++++++++++++- >> drivers/clk/mediatek/clk-pll.h | 5 ++++ >> 2 files changed, 46 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c >> index 49ca25dd5418..8f46de77f42d 100644 >> --- a/drivers/clk/mediatek/clk-pll.c >> +++ b/drivers/clk/mediatek/clk-pll.c >> @@ -37,6 +37,13 @@ int mtk_pll_is_prepared(struct clk_hw *hw) >> return (readl(pll->en_addr) & BIT(pll->data->pll_en_bit)) != 0; >> } >> >> +static int mtk_pll_fenc_is_prepared(struct clk_hw *hw) >> +{ >> + struct mtk_clk_pll *pll = to_mtk_clk_pll(hw); >> + >> + return readl(pll->fenc_addr) & pll->fenc_mask; > > Nits: > > I'd do a double-negate (!!) just to indicate that we only care about > true or false. > > Also, why do we need to store fenc_mask instead of just shifting the bit > here? Same goes for the register address. |pll| has the base address. > Why do we need to pre-calculate it? > > The code is OK; it just seems a bit wasteful on memory. > Thanks for the heads up - since these are only used here for now, I agree they’re not really adding value. I’ll drop fenc_mask and fenc_addr in the next revision. Best, Laura > Either way, this is > > Reviewed-by: Chen-Yu Tsai > >> +} >> + >> static unsigned long __mtk_pll_recalc_rate(struct mtk_clk_pll *pll, u32 fin, >> u32 pcw, int postdiv) >> { >> @@ -274,6 +281,25 @@ void mtk_pll_unprepare(struct clk_hw *hw) >> writel(r, pll->pwr_addr); >> } >> >> +static int mtk_pll_prepare_setclr(struct clk_hw *hw) >> +{ >> + struct mtk_clk_pll *pll = to_mtk_clk_pll(hw); >> + >> + writel(BIT(pll->data->pll_en_bit), pll->en_set_addr); >> + >> + /* Wait 20us after enable for the PLL to stabilize */ >> + udelay(20); >> + >> + return 0; >> +} >> + >> +static void mtk_pll_unprepare_setclr(struct clk_hw *hw) >> +{ >> + struct mtk_clk_pll *pll = to_mtk_clk_pll(hw); >> + >> + writel(BIT(pll->data->pll_en_bit), pll->en_clr_addr); >> +} >> + >> const struct clk_ops mtk_pll_ops = { >> .is_prepared = mtk_pll_is_prepared, >> .prepare = mtk_pll_prepare, >> @@ -283,6 +309,16 @@ const struct clk_ops mtk_pll_ops = { >> .set_rate = mtk_pll_set_rate, >> }; >> >> +const struct clk_ops mtk_pll_fenc_clr_set_ops = { >> + .is_prepared = mtk_pll_fenc_is_prepared, >> + .prepare = mtk_pll_prepare_setclr, >> + .unprepare = mtk_pll_unprepare_setclr, >> + .recalc_rate = mtk_pll_recalc_rate, >> + .round_rate = mtk_pll_round_rate, >> + .set_rate = mtk_pll_set_rate, >> +}; >> +EXPORT_SYMBOL_GPL(mtk_pll_fenc_clr_set_ops); >> + >> struct clk_hw *mtk_clk_register_pll_ops(struct mtk_clk_pll *pll, >> const struct mtk_pll_data *data, >> void __iomem *base, >> @@ -315,6 +351,9 @@ struct clk_hw *mtk_clk_register_pll_ops(struct mtk_clk_pll *pll, >> pll->hw.init = &init; >> pll->data = data; >> >> + pll->fenc_addr = base + data->fenc_sta_ofs; >> + pll->fenc_mask = BIT(data->fenc_sta_bit); >> + >> init.name = data->name; >> init.flags = (data->flags & PLL_AO) ? CLK_IS_CRITICAL : 0; >> init.ops = pll_ops; >> @@ -337,12 +376,13 @@ struct clk_hw *mtk_clk_register_pll(const struct mtk_pll_data *data, >> { >> struct mtk_clk_pll *pll; >> struct clk_hw *hw; >> + const struct clk_ops *pll_ops = data->ops ? data->ops : &mtk_pll_ops; >> >> pll = kzalloc(sizeof(*pll), GFP_KERNEL); >> if (!pll) >> return ERR_PTR(-ENOMEM); >> >> - hw = mtk_clk_register_pll_ops(pll, data, base, &mtk_pll_ops); >> + hw = mtk_clk_register_pll_ops(pll, data, base, pll_ops); >> if (IS_ERR(hw)) >> kfree(pll); >> >> diff --git a/drivers/clk/mediatek/clk-pll.h b/drivers/clk/mediatek/clk-pll.h >> index c4d06bb11516..7fdc5267a2b5 100644 >> --- a/drivers/clk/mediatek/clk-pll.h >> +++ b/drivers/clk/mediatek/clk-pll.h >> @@ -29,6 +29,7 @@ struct mtk_pll_data { >> u32 reg; >> u32 pwr_reg; >> u32 en_mask; >> + u32 fenc_sta_ofs; >> u32 pd_reg; >> u32 tuner_reg; >> u32 tuner_en_reg; >> @@ -51,6 +52,7 @@ struct mtk_pll_data { >> u32 en_clr_reg; >> u8 pll_en_bit; /* Assume 0, indicates BIT(0) by default */ >> u8 pcw_chg_bit; >> + u8 fenc_sta_bit; >> }; >> >> /* >> @@ -72,6 +74,8 @@ struct mtk_clk_pll { >> void __iomem *en_addr; >> void __iomem *en_set_addr; >> void __iomem *en_clr_addr; >> + void __iomem *fenc_addr; >> + u32 fenc_mask; >> const struct mtk_pll_data *data; >> }; >> >> @@ -82,6 +86,7 @@ void mtk_clk_unregister_plls(const struct mtk_pll_data *plls, int num_plls, >> struct clk_hw_onecell_data *clk_data); >> >> extern const struct clk_ops mtk_pll_ops; >> +extern const struct clk_ops mtk_pll_fenc_clr_set_ops; >> >> static inline struct mtk_clk_pll *to_mtk_clk_pll(struct clk_hw *hw) >> { >> -- >> 2.39.5 >>