Re: [PATCH 03/10] soc: amlogic: clk-measure: Add support for more SoCs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The `clk_msr_a9` array is missing the `const` qualifier, causing it to be placed in the modifiable `.data` section instead of the read-only `.rodata` section.
- [Low] Possible typo in the clock name "exit_Tsin_B_CLK_IN" for ID 26 in the `clk_msr_a9` array.
--

commit f03ec42ca0d7092f88d23ba06d4bfb38fc33ede1
Author: Chuan Liu <chuan.liu@amlogic.com>

soc: amlogic: clk-measure: Add support for more SoCs

This commit adds clk-measure driver support for several Amlogic SoCs
including A5, A4, S7, S7D, S6, and A9.

> diff --git a/drivers/soc/amlogic/meson-clk-measure.c b/drivers/soc/amlogic/meson-clk-measure.c
> index 0d4ba2555d175..671e0c3753dec 100644
> --- a/drivers/soc/amlogic/meson-clk-measure.c
> +++ b/drivers/soc/amlogic/meson-clk-measure.c
[ ... ]
> @@ -1041,6 +1041,843 @@ static const struct meson_msr_id clk_msr_t7[] = {
[ ... ]
> +static struct meson_msr_id clk_msr_a9[] = {

[Severity: Low]
Is the missing const qualifier here intentional? Without const, this array is
placed in the modifiable data section instead of the read-only rodata section,
increasing the mutable memory footprint.

Other clock ID tables introduced in this patch, such as clk_msr_a5 and
clk_msr_s7, are properly defined as static const struct meson_msr_id. Because
the driver only copies the elements from these arrays during device probe,
the original template array does not need to be mutable.

> +	CLK_MSR_ID(0,  "sys_clk"),
> +	CLK_MSR_ID(1,  "axi_clk"),
[ ... ]
> +	CLK_MSR_ID(24, "ddr_dpll_pt_clk"),
> +	CLK_MSR_ID(25, "mod_Tsin_A_CLK_IN"),
> +	CLK_MSR_ID(26, "exit_Tsin_B_CLK_IN"),

[Severity: Low]
Does this string contain a typo? The name "exit_Tsin_B_CLK_IN" is exported to
userspace via debugfs.

In similar SoC definitions like clk_msr_s6, the corresponding ID 26 is named
"ext_Tsin_B_CLK_IN", likely standing for external. Using "exit" here instead
of "ext" could confuse users or break userspace scripts expecting consistent
naming across SoCs.

> +	CLK_MSR_ID(27, "u3p2pll1_0_clk_div16"),
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-more-socs-to-support-clk_measure-v1-0-940e269aa52f@amlogic.com?part=3

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic