Received-SPF: pass (google.com: domain of linux-kernel+bounces-14252-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 2 Jan 2024 11:35:46 +0100
From: Johan Hovold <johan@kernel.org>
To: Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: Bjorn Andersson <andersson@kernel.org>, Andy Gross <agross@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Marijn Suijten <marijn.suijten@somainline.org>,
	linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Subject: Re: [PATCH v4 02/12] clk: qcom: Use qcom_branch_set_clk_en()
Message-ID: <ZZPnAvXB8oqds4KM@hovoldconsulting.com>
References: <20230717-topic-branch_aon_cleanup-v4-0-32c293ded915@linaro.org>
 <20230717-topic-branch_aon_cleanup-v4-2-32c293ded915@linaro.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230717-topic-branch_aon_cleanup-v4-2-32c293ded915@linaro.org>

On Sat, Dec 30, 2023 at 02:04:04PM +0100, Konrad Dybcio wrote:
> Instead of magically poking at the bit0 of branch clocks' CBCR, use
> the newly introduced helper.
> 
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---

> diff --git a/drivers/clk/qcom/camcc-sc8280xp.c b/drivers/clk/qcom/camcc-sc8280xp.c
> index 3dcd79b01515..94db130b85e2 100644
> --- a/drivers/clk/qcom/camcc-sc8280xp.c
> +++ b/drivers/clk/qcom/camcc-sc8280xp.c
> @@ -3010,10 +3010,8 @@ static int camcc_sc8280xp_probe(struct platform_device *pdev)
>  	clk_lucid_pll_configure(&camcc_pll6, regmap, &camcc_pll6_config);
>  	clk_lucid_pll_configure(&camcc_pll7, regmap, &camcc_pll7_config);
>  
> -	/*
> -	 * Keep camcc_gdsc_clk always enabled:
> -	 */
> -	regmap_update_bits(regmap, 0xc1e4, BIT(0), 1);
> +	/* Keep the critical clocks always-on */
> +	qcom_branch_set_clk_en(regmap, 0xc1e4); /* CAMCC_GDSC_CLK */

I still think something along the lines of

	/* Keep some clocks always on */

is preferred as it is far from obvious why a camera clock would be
considered "critical".

Or perhaps you can come up with a better description of why we've
decided not to model these clocks and just leave them ungated.

>  
>  	ret = qcom_cc_really_probe(pdev, &camcc_sc8280xp_desc, regmap);
>  	if (ret)

Johan