Re: [PATCH v3 07/14 RESEND] clk: samsung: exynos8855: add block top and peric clock support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] clk: samsung: exynos8855: wrong parent clock for
  CLK_GOUT_UART_DBG_PERIC
- [Low] clk: samsung: exynos8855: duplicate cmu-top entry in
  exynos8855_cmu_of_match
--

commit 2beaee0a6e639b45324bd65007b59d1e589e9da3
Author: Raghav Sharma <raghav.s@samsung.com>

clk: samsung: exynos8855: add block top and peric clock support

This commit adds Common Clock Framework support for the CMU_TOP and
CMU_PERIC clock management units on the Samsung Exynos 8855 SoC.

> diff --git a/drivers/clk/samsung/clk-exynos8855.c b/drivers/clk/samsung/clk-exynos8855.c
> new file mode 100644
> index 0000000000000..dcf58f2459b0f
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-exynos8855.c

[ ... ]

> +static void __init exynos8855_cmu_top_init(struct device_node *np)
> +{
> +	exynos_arm64_register_cmu(NULL, np, &top_cmu_info);
> +}
> +
> +/* Register CMU_TOP early, as it's a dependency for other early domains */
> +CLK_OF_DECLARE(exynos8855_cmu_top, "samsung,exynos8855-cmu-top",
> +	       exynos8855_cmu_top_init);

[ ... ]

> +static const struct samsung_gate_clock peric_gate_clks[] __initconst = {
> +	GATE(CLK_GOUT_CMU_PERIC, "gout_cmu_peric",
> +	     "dout_clkcmu_peric_nocp",
> +	     CMU_PERIC_CLK_CON_GAT_BLK_PERIC_UID_CMU_PERIC_IPCLKPORT_PCLK,
> +	     21, 0, 0),
> +	GATE(CLK_GOUT_SYSREG_PERIC, "gout_sysreg_peric",
> +	     "dout_clkcmu_peric_nocp",
> +	     CMU_PERIC_CLK_CON_GAT_BLK_PERIC_UID_SYSREG_PERIC_IPCLKPORT_PCLK,
> +	     21, 0, 0),
> +	GATE(CLK_GOUT_UART_DBG_PERIC, "gout_uart_dbg_peric",
> +	     "dout_clkcmu_peric_uart_dbg",
> +	     CMU_PERIC_CLK_CON_GAT_BLK_PERIC_UID_UART_DBG_IPCLKPORT_PCLK,
> +	     21, 0, 0),
> +	GATE(CLK_GOUT_USI_PERIC, "gout_usi_peric",
> +	     "dout_clkcmu_peric_nocp",
> +	     CMU_PERIC_CLK_CON_GAT_BLK_PERIC_UID_USI00_USI_IPCLKPORT_PCLK,
> +	     21, 0, 0),
> +};

[Severity: Medium]
Should CLK_GOUT_UART_DBG_PERIC have dout_clkcmu_peric_nocp as its parent
instead of dout_clkcmu_peric_uart_dbg?

CMU_PERIC_CLK_CON_GAT_BLK_PERIC_UID_UART_DBG_IPCLKPORT_PCLK is the APB
PCLK gate register, and all other IPCLKPORT_PCLK gates in peric_gate_clks
(including gout_usi_peric) are parented to the APB bus clock
dout_clkcmu_peric_nocp.  Parenting this PCLK gate to the UART baud clock
divider reports the wrong PCLK rate and does not propagate enable counts
to dout_clkcmu_peric_nocp.

[ ... ]

> +static const struct of_device_id exynos8855_cmu_of_match[] = {
> +	{
> +		.compatible = "samsung,exynos8855-cmu-peric",
> +		.data = &peric_cmu_info,
> +	}, {
> +		.compatible = "samsung,exynos8855-cmu-top",
> +		.data = &top_cmu_info,
> +	},
> +	{ }
> +};

[Severity: Low]
Should "samsung,exynos8855-cmu-top" be dropped from
exynos8855_cmu_of_match?

CMU_TOP is already registered early via CLK_OF_DECLARE(), which marks the
device node OF_POPULATED so the platform driver match entry is unused
during normal boot, and if a matching platform device were ever probed via
exynos8855_cmu_probe(), samsung_cmu_register_one() would attempt to
register top_cmu_info a second time and panic in
samsung_clk_of_add_provider().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113110612.3265841-1-raghav.s@samsung.com?part=7