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

From: Chuan Liu <chuan.liu@amlogic.com>
To: sashiko-reviews@lists.linux.dev
Cc: devicetree@vger.kernel.org, neil.armstrong@linaro.org,
	conor+dt@kernel.org, robh@kernel.org,
	linux-amlogic@lists.infradead.org
Subject: Re: [PATCH 03/10] soc: amlogic: clk-measure: Add support for more SoCs
Date: Mon, 14 Sep 2026 11:00:13 +0800	[thread overview]
Message-ID: <86ae1163-1507-4b4f-8d21-87fd2492a833@amlogic.com> (raw)
In-Reply-To: <20260911083742.9C6621F000FF@smtp.kernel.org>

On 9/11/2026 4:37 PM, sashiko-bot@kernel.org wrote:
> [你通常不会收到来自 sashiko-bot@kernel.org 的电子邮件。请访问 https://aka.ms/LearnAboutSenderIdentification，以了解这一点为什么很重要]
> 
> [ EXTERNAL EMAIL ]
> 
> 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.
> 

I'll add "const" in V2.

>> +     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.
> 

and rename "exit_Tsin_B_CLK_IN" to "ext_Tsin_B_CLK_IN".

>> +     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

-- 
Best regards,
Chuan

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