All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Abel Vesa <abel.vesa@linaro.org>
Cc: Andy Gross <agross@kernel.org>,
	Konrad Dybcio <konrad.dybcio@somainline.org>,
	Mike Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [RFC 1/9] clk: qcom: qcc-sdm845: Collapse gdsc structs into macros
Date: Tue, 9 Aug 2022 15:25:39 -0500	[thread overview]
Message-ID: <YvLCwyB9rBWXmfZt@baldur> (raw)
In-Reply-To: <20220726142303.4126434-2-abel.vesa@linaro.org>

On Tue 26 Jul 09:22 CDT 2022, Abel Vesa wrote:

> Collapse gdsc structs definitions into macros to make them
> more compact visually.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>  drivers/clk/qcom/gcc-sdm845.c | 129 ++++------------------------------
>  drivers/clk/qcom/gdsc.h       |  10 +++
>  2 files changed, 23 insertions(+), 116 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
> index 58aa3ec9a7fc..8529e9c8c90c 100644
> --- a/drivers/clk/qcom/gcc-sdm845.c
> +++ b/drivers/clk/qcom/gcc-sdm845.c
> @@ -3191,122 +3191,19 @@ static struct clk_branch gcc_lpass_sway_clk = {
>  };
>  #endif
>  
> -static struct gdsc pcie_0_gdsc = {
> -	.gdscr = 0x6b004,
> -	.pd = {
> -		.name = "pcie_0_gdsc",
> -	},
> -	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = POLL_CFG_GDSCR,
> -};
> -
> -static struct gdsc pcie_1_gdsc = {
> -	.gdscr = 0x8d004,
> -	.pd = {
> -		.name = "pcie_1_gdsc",
> -	},
> -	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = POLL_CFG_GDSCR,
> -};
> -
> -static struct gdsc ufs_card_gdsc = {
> -	.gdscr = 0x75004,
> -	.pd = {
> -		.name = "ufs_card_gdsc",
> -	},
> -	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = POLL_CFG_GDSCR,
> -};
> -
> -static struct gdsc ufs_phy_gdsc = {
> -	.gdscr = 0x77004,
> -	.pd = {
> -		.name = "ufs_phy_gdsc",
> -	},
> -	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = POLL_CFG_GDSCR,
> -};
> -
> -static struct gdsc usb30_prim_gdsc = {
> -	.gdscr = 0xf004,
> -	.pd = {
> -		.name = "usb30_prim_gdsc",
> -	},
> -	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = POLL_CFG_GDSCR,
> -};
> -
> -static struct gdsc usb30_sec_gdsc = {
> -	.gdscr = 0x10004,
> -	.pd = {
> -		.name = "usb30_sec_gdsc",
> -	},
> -	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = POLL_CFG_GDSCR,
> -};
> -
> -static struct gdsc hlos1_vote_aggre_noc_mmu_audio_tbu_gdsc = {
> -	.gdscr = 0x7d030,
> -	.pd = {
> -		.name = "hlos1_vote_aggre_noc_mmu_audio_tbu_gdsc",
> -	},
> -	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = VOTABLE,
> -};
> -
> -static struct gdsc hlos1_vote_aggre_noc_mmu_pcie_tbu_gdsc = {
> -	.gdscr = 0x7d03c,
> -	.pd = {
> -		.name = "hlos1_vote_aggre_noc_mmu_pcie_tbu_gdsc",
> -	},
> -	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = VOTABLE,
> -};
> -
> -static struct gdsc hlos1_vote_aggre_noc_mmu_tbu1_gdsc = {
> -	.gdscr = 0x7d034,
> -	.pd = {
> -		.name = "hlos1_vote_aggre_noc_mmu_tbu1_gdsc",
> -	},
> -	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = VOTABLE,
> -};
> -
> -static struct gdsc hlos1_vote_aggre_noc_mmu_tbu2_gdsc = {
> -	.gdscr = 0x7d038,
> -	.pd = {
> -		.name = "hlos1_vote_aggre_noc_mmu_tbu2_gdsc",
> -	},
> -	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = VOTABLE,
> -};
> -
> -static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc = {
> -	.gdscr = 0x7d040,
> -	.pd = {
> -		.name = "hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc",
> -	},
> -	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = VOTABLE,
> -};
> -
> -static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf1_gdsc = {
> -	.gdscr = 0x7d048,
> -	.pd = {
> -		.name = "hlos1_vote_mmnoc_mmu_tbu_hf1_gdsc",
> -	},
> -	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = VOTABLE,
> -};
> -
> -static struct gdsc hlos1_vote_mmnoc_mmu_tbu_sf_gdsc = {
> -	.gdscr = 0x7d044,
> -	.pd = {
> -		.name = "hlos1_vote_mmnoc_mmu_tbu_sf_gdsc",
> -	},
> -	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = VOTABLE,
> -};
> +DEFINE_QCOM_CC_GDSC(pcie_0_gdsc, 0x6b004, "pcie_0_gdsc", PWRSTS_OFF_ON, POLL_CFG_GDSCR);
> +DEFINE_QCOM_CC_GDSC(pcie_1_gdsc, 0x8d004, "pcie_1_gdsc", PWRSTS_OFF_ON, POLL_CFG_GDSCR);
> +DEFINE_QCOM_CC_GDSC(ufs_card_gdsc, 0x75004, "ufs_card_gdsc", PWRSTS_OFF_ON, POLL_CFG_GDSCR);
> +DEFINE_QCOM_CC_GDSC(ufs_phy_gdsc, 0x77004, "ufs_phy_gdsc", PWRSTS_OFF_ON, POLL_CFG_GDSCR);
> +DEFINE_QCOM_CC_GDSC(usb30_prim_gdsc, 0xf004, "usb30_prim_gdsc", PWRSTS_OFF_ON, POLL_CFG_GDSCR);
> +DEFINE_QCOM_CC_GDSC(usb30_sec_gdsc, 0x10004, "usb30_sec_gdsc", PWRSTS_OFF_ON, POLL_CFG_GDSCR);
> +DEFINE_QCOM_CC_GDSC(hlos1_vote_aggre_noc_mmu_audio_tbu_gdsc, 0x7d030, "hlos1_vote_aggre_noc_mmu_audio_tbu_gdsc", PWRSTS_OFF_ON, VOTABLE);
> +DEFINE_QCOM_CC_GDSC(hlos1_vote_aggre_noc_mmu_pcie_tbu_gdsc, 0x7d03c, "hlos1_vote_aggre_noc_mmu_pcie_tbu_gdsc", PWRSTS_OFF_ON, VOTABLE);
> +DEFINE_QCOM_CC_GDSC(hlos1_vote_aggre_noc_mmu_tbu1_gdsc, 0x7d034, "hlos1_vote_aggre_noc_mmu_tbu1_gdsc", PWRSTS_OFF_ON, VOTABLE);
> +DEFINE_QCOM_CC_GDSC(hlos1_vote_aggre_noc_mmu_tbu2_gdsc, 0x7d038, "hlos1_vote_aggre_noc_mmu_tbu2_gdsc", PWRSTS_OFF_ON, VOTABLE);
> +DEFINE_QCOM_CC_GDSC(hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc, 0x7d040, "hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc", PWRSTS_OFF_ON, VOTABLE);
> +DEFINE_QCOM_CC_GDSC(hlos1_vote_mmnoc_mmu_tbu_hf1_gdsc, 0x7d048, "hlos1_vote_mmnoc_mmu_tbu_hf1_gdsc", PWRSTS_OFF_ON, VOTABLE);
> +DEFINE_QCOM_CC_GDSC(hlos1_vote_mmnoc_mmu_tbu_sf_gdsc, 0x7d044, "hlos1_vote_mmnoc_mmu_tbu_sf_gdsc", PWRSTS_OFF_ON, VOTABLE);

Personally I have a really hard time looking at such a compact chunk of
text and hence this is harder for me to spot mistakes and differences
in.

While I like the effort of making things easier to maintain this made me
further appreciate the change we've done in the interconnect providers,
where we're doing the exact opposite - and remove magical macros.

Regards,
Bjorn

>  
>  static struct clk_regmap *gcc_sdm845_clocks[] = {
>  	[GCC_AGGRE_NOC_PCIE_TBU_CLK] = &gcc_aggre_noc_pcie_tbu_clk.clkr,
> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
> index 5de48c9439b2..c0e616b49dee 100644
> --- a/drivers/clk/qcom/gdsc.h
> +++ b/drivers/clk/qcom/gdsc.h
> @@ -78,6 +78,16 @@ struct gdsc_desc {
>  	size_t num;
>  };
>  
> +#define DEFINE_QCOM_CC_GDSC(_name, _gdscr, _pd_name, _pwrsts, _flags) \
> +	static struct gdsc _name = {			\
> +		.gdscr = _gdscr,		\
> +		.pd = {				\
> +			.name = _pd_name,	\
> +		},				\
> +		.pwrsts = _pwrsts,		\
> +		.flags = _flags,		\
> +	}
> +
>  #ifdef CONFIG_QCOM_GDSC
>  int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *,
>  		  struct regmap *);
> -- 
> 2.34.3
> 

  parent reply	other threads:[~2022-08-09 20:26 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-26 14:22 [RFC 0/9] clk: qcom: gcc-sdm845: Swicth from expanded definitions to compact macros Abel Vesa
2022-07-26 14:22 ` [RFC 1/9] clk: qcom: qcc-sdm845: Collapse gdsc structs into macros Abel Vesa
2022-07-26 16:36   ` Dmitry Baryshkov
2022-08-09 20:25   ` Bjorn Andersson [this message]
2022-08-10  7:45     ` Abel Vesa
2022-07-26 14:22 ` [RFC 2/9] clk: qcom: gcc-sdm845: Switch from parent_hws to parent_data Abel Vesa
2022-07-26 14:22 ` [RFC 3/9] clk: qcom: rcg: Add macros to collapse definition Abel Vesa
2022-07-26 16:39   ` Dmitry Baryshkov
2022-07-26 14:22 ` [RFC 4/9] clk: qcom: alpha-pll: " Abel Vesa
2022-07-26 14:22 ` [RFC 5/9] clk: qcom: branch: " Abel Vesa
2022-07-26 14:23 ` [RFC 6/9] clk: qcom: common: Add macro wrapper for all clock types Abel Vesa
2022-07-26 16:48   ` Dmitry Baryshkov
2022-07-26 14:23 ` [RFC 7/9] clk: qcom: gcc-sdm845: Switch to macros to collapse branch clocks definitions Abel Vesa
2022-07-26 14:23 ` [RFC 8/9] clk: qcom: gcc-sdm845: Switch to macros to collapse rcg2 " Abel Vesa
2022-07-26 16:47   ` Dmitry Baryshkov
2022-07-29  2:04   ` kernel test robot
2022-07-26 14:23 ` [RFC 9/9] clk: qcom: gcc-sdm845: Switch to macros to collapse alpha-pll " Abel Vesa
2022-07-26 19:16 ` [RFC 0/9] clk: qcom: gcc-sdm845: Swicth from expanded definitions to compact macros Konrad Dybcio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YvLCwyB9rBWXmfZt@baldur \
    --to=bjorn.andersson@linaro.org \
    --cc=abel.vesa@linaro.org \
    --cc=agross@kernel.org \
    --cc=konrad.dybcio@somainline.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.