Message ID | 40981d0bb722eb509628bcf82c31f632e4cf747a.1701823757.git.daniel@makrotopia.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp3804859vqy; Tue, 5 Dec 2023 16:57:50 -0800 (PST) X-Google-Smtp-Source: AGHT+IF1XBfLYUAdA6rNgXL7R8tdKf+2n9xJZyEqpQbxs0bxQ/kqqfrui24I88q+hU/cpbnZ6PHa X-Received: by 2002:a17:90b:1e07:b0:285:b6be:d89c with SMTP id pg7-20020a17090b1e0700b00285b6bed89cmr156324pjb.7.1701824269847; Tue, 05 Dec 2023 16:57:49 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701824269; cv=none; d=google.com; s=arc-20160816; b=ObIxmSYVy4TYMaQAtr8472GvDGZPEIxsfl3Lr07ZQr2p+TxYTS/dlyfADqouZ/sue7 a7GtEoHNgxinWFH/RTWSeM5LiNcQGPC0TzOrdGU6nPUC2hBgQy/QY8eDS7zsZN2HhUIr 55evNPI28UQdidef4ypSig8q8epCfwf5k5oxCXwmyAx8YZv50GpbHDKlamO9+UxiYyTr IRWdDWfwdYcySFVFGYaHPzy1Usk+tB8c0LiVMGNddD3byuRtuDmx5TGZof9QOclh6pwe 14eNyBZmnNGz2FfrTJkXOGUFcR44n3Y3RSRah2TSKY5ni2pLjbHJ5HsigvaZPYaKMjeR 1+SA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:to:from:date; bh=YnQkDBSBMeJbc9YYrbQWZJ/dmCkXIqy05IXoTh7gku0=; fh=c42LVhuUkr+o7mWDW8QvYc6yvOZbRLq3bfcFbTPxJEU=; b=fhjlboKP4P6t7L14Vlq7QcYELEQAvQ1xren0hGrd6x1uo9VE5rS8PaWRdCCXqr7d4o r/IMH48dkubG5QV9irORZVwznZnwY4HdfPJ4i4j3FWtxObtPdVc547kWSe3Pcd7/iRUP fya04/fevs1+FcwPe0IEX2VUPZlObQRPqFEeuSLLoeBWCwnUnZl8kacXhVfHQ0RWxQBG y3umjnFJ88ojnuf/QNG6wu5K6Se/zgvf1fKpqEHdu7t4J32j4xYE+O1/g0rFaA834zGQ jPoHYxPWJqIPhYsl3mD/uJavvGuJo4ytAxkk3QBfRATv7kxTrc1fXPLF8jfqLTf9ykHQ mo2w== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from fry.vger.email (fry.vger.email. [2620:137:e000::3:8]) by mx.google.com with ESMTPS id x20-20020a17090aca1400b00286ec2f2bdfsi1300361pjt.120.2023.12.05.16.57.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Dec 2023 16:57:49 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by fry.vger.email (Postfix) with ESMTP id 4C5AD804A4F8; Tue, 5 Dec 2023 16:57:47 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at fry.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1376387AbjLFA5b (ORCPT <rfc822;chrisfriedt@gmail.com> + 99 others); Tue, 5 Dec 2023 19:57:31 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50930 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229802AbjLFA5a (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 5 Dec 2023 19:57:30 -0500 Received: from pidgin.makrotopia.org (pidgin.makrotopia.org [185.142.180.65]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 99F27D62; Tue, 5 Dec 2023 16:57:33 -0800 (PST) Received: from local by pidgin.makrotopia.org with esmtpsa (TLS1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.96.2) (envelope-from <daniel@makrotopia.org>) id 1rAgE7-0002TW-12; Wed, 06 Dec 2023 00:57:24 +0000 Date: Wed, 6 Dec 2023 00:57:20 +0000 From: Daniel Golle <daniel@makrotopia.org> To: Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, Matthias Brugger <matthias.bgg@gmail.com>, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Sabrina Dubroca <sd@queasysnail.net>, Daniel Golle <daniel@makrotopia.org>, Jianhui Zhao <zhaojh329@gmail.com>, Chen-Yu Tsai <wenst@chromium.org>, "Garmin.Chang" <Garmin.Chang@mediatek.com>, Sam Shih <sam.shih@mediatek.com>, Frank Wunderlich <frank-w@public-files.de>, Dan Carpenter <dan.carpenter@linaro.org>, James Liao <jamesjj.liao@mediatek.com>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, netdev@vger.kernel.org Subject: [PATCH v3 3/4] clk: mediatek: Add pcw_chg_shift control Message-ID: <40981d0bb722eb509628bcf82c31f632e4cf747a.1701823757.git.daniel@makrotopia.org> References: <23bc89d407e7797e97b703fa939b43bfe79296ce.1701823757.git.daniel@makrotopia.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <23bc89d407e7797e97b703fa939b43bfe79296ce.1701823757.git.daniel@makrotopia.org> X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Tue, 05 Dec 2023 16:57:47 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784492085597349772 X-GMAIL-MSGID: 1784492085597349772 |
Series |
[v3,1/4] dt-bindings: clock: mediatek: add MT7988 clock IDs
|
|
Commit Message
Daniel Golle
Dec. 6, 2023, 12:57 a.m. UTC
From: Sam Shih <sam.shih@mediatek.com> Introduce pcw_chg_shfit control to optionally use that instead of the hardcoded PCW_CHG_MASK macro. This will needed for clocks on the MT7988 SoC. Signed-off-by: Sam Shih <sam.shih@mediatek.com> Signed-off-by: Daniel Golle <daniel@makrotopia.org> --- v3: use git --from ... v2: no changes drivers/clk/mediatek/clk-pll.c | 5 ++++- drivers/clk/mediatek/clk-pll.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)
Comments
Il 06/12/23 01:57, Daniel Golle ha scritto: > From: Sam Shih <sam.shih@mediatek.com> > > Introduce pcw_chg_shfit control to optionally use that instead of the > hardcoded PCW_CHG_MASK macro. > This will needed for clocks on the MT7988 SoC. > > Signed-off-by: Sam Shih <sam.shih@mediatek.com> > Signed-off-by: Daniel Golle <daniel@makrotopia.org> > --- > v3: use git --from ... > v2: no changes > > drivers/clk/mediatek/clk-pll.c | 5 ++++- > drivers/clk/mediatek/clk-pll.h | 1 + > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c > index 513ab6b1b3229..9f08bc5d2a8a2 100644 > --- a/drivers/clk/mediatek/clk-pll.c > +++ b/drivers/clk/mediatek/clk-pll.c > @@ -114,7 +114,10 @@ static void mtk_pll_set_rate_regs(struct mtk_clk_pll *pll, u32 pcw, > pll->data->pcw_shift); > val |= pcw << pll->data->pcw_shift; > writel(val, pll->pcw_addr); > - chg = readl(pll->pcw_chg_addr) | PCW_CHG_MASK; > + if (pll->data->pcw_chg_shift) > + chg = readl(pll->pcw_chg_addr) | BIT(pll->data->pcw_chg_shift); > + else > + chg = readl(pll->pcw_chg_addr) | PCW_CHG_MASK; > writel(chg, pll->pcw_chg_addr); > if (pll->tuner_addr) > writel(val + 1, pll->tuner_addr); > diff --git a/drivers/clk/mediatek/clk-pll.h b/drivers/clk/mediatek/clk-pll.h > index f17278ff15d78..d28d317e84377 100644 > --- a/drivers/clk/mediatek/clk-pll.h > +++ b/drivers/clk/mediatek/clk-pll.h > @@ -44,6 +44,7 @@ struct mtk_pll_data { > u32 pcw_reg; > int pcw_shift; > u32 pcw_chg_reg; > + int pcw_chg_shift; > const struct mtk_pll_div_table *div_table; > const char *parent_name; > u32 en_reg; Hmm... no, this is not the best at all and can be improved. Okay, so, the situation here is that one or some PLL(s) on MT7988 have a different PCW_CHG_MASK as far as I understand. Situation here is: - Each PLL must be registered to clk-pll - Each driver declaring a PLL does exactly so - There's a function to register the PLL You definitely don't want to add a conditional in pll_set_rate(): even though this is technically not a performance path on the current SoCs (and will probably never be), it's simply useless to have this (very small) overhead there. The solution is to: - Change that pcw_chg_shift to an unsigned short int type (or u8, your call): you don't need 32 bits for this number, as the expected range of this member is [0-31], and this can be expressed in just 4 bits (u8 is the smallest though) - Add that to function mtk_clk_register_pll_ops() - Change mtk_pll_set_rate_regs() to always do chg = readl(pll->pcw_chg_addr) | BIT(pll->data->pcw_chg_shift); Cheers, Angelo
Hi Angelo, thank you for taking the time to review and for the helpful comments. On Wed, Dec 06, 2023 at 11:38:36AM +0100, AngeloGioacchino Del Regno wrote: > Il 06/12/23 01:57, Daniel Golle ha scritto: > > From: Sam Shih <sam.shih@mediatek.com> > > > > Introduce pcw_chg_shfit control to optionally use that instead of the > > hardcoded PCW_CHG_MASK macro. > > This will needed for clocks on the MT7988 SoC. > > > > Signed-off-by: Sam Shih <sam.shih@mediatek.com> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org> > > --- > > v3: use git --from ... > > v2: no changes > > > > drivers/clk/mediatek/clk-pll.c | 5 ++++- > > drivers/clk/mediatek/clk-pll.h | 1 + > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c > > index 513ab6b1b3229..9f08bc5d2a8a2 100644 > > --- a/drivers/clk/mediatek/clk-pll.c > > +++ b/drivers/clk/mediatek/clk-pll.c > > @@ -114,7 +114,10 @@ static void mtk_pll_set_rate_regs(struct mtk_clk_pll *pll, u32 pcw, > > pll->data->pcw_shift); > > val |= pcw << pll->data->pcw_shift; > > writel(val, pll->pcw_addr); > > - chg = readl(pll->pcw_chg_addr) | PCW_CHG_MASK; > > + if (pll->data->pcw_chg_shift) > > + chg = readl(pll->pcw_chg_addr) | BIT(pll->data->pcw_chg_shift); > > + else > > + chg = readl(pll->pcw_chg_addr) | PCW_CHG_MASK; > > writel(chg, pll->pcw_chg_addr); > > if (pll->tuner_addr) > > writel(val + 1, pll->tuner_addr); > > diff --git a/drivers/clk/mediatek/clk-pll.h b/drivers/clk/mediatek/clk-pll.h > > index f17278ff15d78..d28d317e84377 100644 > > --- a/drivers/clk/mediatek/clk-pll.h > > +++ b/drivers/clk/mediatek/clk-pll.h > > @@ -44,6 +44,7 @@ struct mtk_pll_data { > > u32 pcw_reg; > > int pcw_shift; > > u32 pcw_chg_reg; > > + int pcw_chg_shift; > > const struct mtk_pll_div_table *div_table; > > const char *parent_name; > > u32 en_reg; > > Hmm... no, this is not the best at all and can be improved. > > Okay, so, the situation here is that one or some PLL(s) on MT7988 have a different > PCW_CHG_MASK as far as I understand. Correct. *All* clocks of MT7988 have a different PCW_CHG_MASK, BIT(2) instead of BIT(31). > > Situation here is: > - Each PLL must be registered to clk-pll > - Each driver declaring a PLL does exactly so > - There's a function to register the PLL > > You definitely don't want to add a conditional in pll_set_rate(): even though > this is technically not a performance path on the current SoCs (and will probably > never be), it's simply useless to have this (very small) overhead there. > > The solution is to: > - Change that pcw_chg_shift to an unsigned short int type (or u8, your call): > you don't need 32 bits for this number, as the expected range of this member > is [0-31], and this can be expressed in just 4 bits (u8 is the smallest though) Ack will use u8 instead, despite the struct not being packed, so I wonder if it actually makes a difference. > - Add that to function mtk_clk_register_pll_ops() > - Change mtk_pll_set_rate_regs() to always do > chg = readl(pll->pcw_chg_addr) | BIT(pll->data->pcw_chg_shift); As mtk_pll_data is a read-only member of the mtk_pll struct, we can't set pcw_chg_shift to 31 in mtk_clk_register_pll_ops() in case it is set to 0. The only (much more intrusive change) would be to explicitely declare .pcw_chg_shift = 31 in all current drivers setting .pcs_chg_reg != 0. Should I do that instead?
diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c index 513ab6b1b3229..9f08bc5d2a8a2 100644 --- a/drivers/clk/mediatek/clk-pll.c +++ b/drivers/clk/mediatek/clk-pll.c @@ -114,7 +114,10 @@ static void mtk_pll_set_rate_regs(struct mtk_clk_pll *pll, u32 pcw, pll->data->pcw_shift); val |= pcw << pll->data->pcw_shift; writel(val, pll->pcw_addr); - chg = readl(pll->pcw_chg_addr) | PCW_CHG_MASK; + if (pll->data->pcw_chg_shift) + chg = readl(pll->pcw_chg_addr) | BIT(pll->data->pcw_chg_shift); + else + chg = readl(pll->pcw_chg_addr) | PCW_CHG_MASK; writel(chg, pll->pcw_chg_addr); if (pll->tuner_addr) writel(val + 1, pll->tuner_addr); diff --git a/drivers/clk/mediatek/clk-pll.h b/drivers/clk/mediatek/clk-pll.h index f17278ff15d78..d28d317e84377 100644 --- a/drivers/clk/mediatek/clk-pll.h +++ b/drivers/clk/mediatek/clk-pll.h @@ -44,6 +44,7 @@ struct mtk_pll_data { u32 pcw_reg; int pcw_shift; u32 pcw_chg_reg; + int pcw_chg_shift; const struct mtk_pll_div_table *div_table; const char *parent_name; u32 en_reg;