Re: [PATCH 2/4] soc: amlogic: clk-measure: Add A1 and T7 support

[Neil Armstrong <neil.armstrong@linaro.org>]

On 4/13/26 11:10, Krzysztof Kozlowski wrote:
> On 13/04/2026 10:21, Jian Hu wrote:
>>
>> On 4/12/2026 5:55 PM, Krzysztof Kozlowski wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> On 10/04/2026 12:03, Jian Hu wrote:
>>>> Add support for the A1 and T7 SoC family in amlogic clk measure.
>>>>
>>>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>>>> ---
>>>>    drivers/soc/amlogic/meson-clk-measure.c | 272 ++++++++++++++++++++++++
>>>>    1 file changed, 272 insertions(+)
>>>>
>>>> diff --git a/drivers/soc/amlogic/meson-clk-measure.c b/drivers/soc/amlogic/meson-clk-measure.c
>>>> index d862e30a244e..083524671b76 100644
>>>> --- a/drivers/soc/amlogic/meson-clk-measure.c
>>>> +++ b/drivers/soc/amlogic/meson-clk-measure.c
>>>> @@ -787,6 +787,258 @@ static const struct meson_msr_id clk_msr_s4[] = {
>>>>
>>>>    };
>>>>
>>>> +static struct meson_msr_id clk_msr_a1[] = {
>>> And existing code uses what sort of array? Seems you send us obsolete or
>>> downstream code.
>>
>>
>> Thanks for your review.
>>
>>
>> I have checked the previous Amlogic SoC's commits. Such as Amlogic AXG,
>> G12A, C3, S4.
>>
>> The clk_msr_xx entry is added after last SoC's array, sorted by
>> submissin date rather than alphabetical order.
>>
>> So I place A1 and T7 after S4 accordingly.
>>
>>
>> The A1 clock controller driver was already supported in
>> https://lore.kernel.org/all/20230523135351.19133-7-ddrokosov@sberdevices.ru/
>>
>> It is also present in the mainline kernel:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/meson/Kconfig#n113
>>
>>
>> This clock measure IP is used to measure the internal clock paths
>> frequencies,  and A1 clock controller driver was supported.
>>
>> Since the corresponding clock measure driver does not support A1 yet, So
>> add A1 clk msr here.
> 
> No, what qualifiers or keywords are used for existing arrays? IOW,
> please investigate and understand why you are doing this very different
> than existing code. Maybe because you sent us downstream, so you
> replicated all other downstream issues.

I see, the existing uses "static const struct".

Jian, could to switch to that please ?

Neil

> 
> Best regards,
> Krzysztof