Re: [PATCH v12 2/6] clk: qcom: Add Global Clock Controller (GCC) driver for SM7250

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] clk: qcom: gcc-sm7250: wrong halt_check on gcc_ddrss_gpu_axi_clk
--

commit a24e620aee1e1a738b8a01486a74fa9dab88eef4
Author: Sreeshankar K <sreeshankar0910@gmail.com>

clk: qcom: Add Global Clock Controller (GCC) driver for SM7250

This commit adds the Global Clock Controller (GCC) driver for Qualcomm SM7250
SoCs, registering PLLs, RCGs, branch clocks, GDSCs, and resets.

> diff --git a/drivers/clk/qcom/gcc-sm7250.c b/drivers/clk/qcom/gcc-sm7250.c
> new file mode 100644
> index 0000000000000..949a12ae93c99
> --- /dev/null
> +++ b/drivers/clk/qcom/gcc-sm7250.c
> @@ -0,0 +1,2278 @@

[ ... ]

> +static struct clk_branch gcc_ddrss_gpu_axi_clk = {
> +	.halt_reg = 0x71154,
> +	.halt_check = BRANCH_HALT,
> +	.clkr = {
> +		.enable_reg = 0x71154,
> +		.enable_mask = BIT(0),
> +		.hw.init = &(const struct clk_init_data){
> +			.name = "gcc_ddrss_gpu_axi_clk",
> +			.ops = &clk_branch2_ops,
> +		},
> +	},
> +};

[Severity: Medium]
As reported by Taniya Das on v9 [1], should gcc_ddrss_gpu_axi_clk use
BRANCH_VOTED instead of BRANCH_HALT for .halt_check?

When gcc_ddrss_gpu_axi_clk is disabled via
clk_branch2_disable() -> clk_branch_toggle() -> clk_branch_wait() in
drivers/clk/qcom/clk-branch.c:

	bool voted = br->halt_check & BRANCH_VOTED;
	...
	if (br->halt_check == BRANCH_HALT_DELAY || (!enabling && voted)) {
		udelay(10);
	} else if (br->halt_check == BRANCH_HALT_ENABLE ||
		   br->halt_check == BRANCH_HALT ||
		   (enabling && voted)) {
		int count = 200;

		while (count-- > 0) {
			if (check_halt(br, enabling))
				return 0;
			udelay(1);
		}
		WARN(1, "%s status stuck at 'o%s'", name,
		     enabling ? "ff" : "n");
		return -EBUSY;
	}

Having .halt_check set to BRANCH_HALT instead of BRANCH_VOTED causes
clk_branch_wait() to poll the CBCR_CLK_OFF bit on disable rather than
delaying 10us, which can trigger a WARN() splat if the voted DDRSS GPU AXI
clock remains active.

[1] https://lore.kernel.org/r/1ea96c7d-102a-4261-a70e-ca0813490a02@oss.qualcomm.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-master-v12-0-57fbe9eef978@gmail.com?part=2