Re: [PATCH v6 2/5] dt-bindings: clock: add Amlogic T7 SCMI clock controller

[Jerome Brunet <jbrunet@baylibre.com>]

On Tue 09 Dec 2025 at 07:01, Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 08/12/2025 09:40, Jian Hu wrote:
>> Hi, Krzysztof
>> 
>> 
>> Thans for your review.
>> 
>> On 12/8/2025 2:17 PM, Krzysztof Kozlowski wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> On Thu, Dec 04, 2025 at 01:36:31PM +0800, Jian Hu wrote:
>>>> Add DT bindings for the SCMI clock controller of the Amlogic T7 SoC family.
>>>>
>>>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>>>> Acked-by: Rob Herring (Arm) <robh@kernel.org>
>>>> ---
>>>>   include/dt-bindings/clock/amlogic,t7-scmi.h | 47 +++++++++++++++++++++
>>>>   1 file changed, 47 insertions(+)
>>>>   create mode 100644 include/dt-bindings/clock/amlogic,t7-scmi.h
>>>>
>>> Where is any binding doc for this? Why is this a separate patch?
>> 
>> 
>> The ARM SCMI device tree binding specification is located at 
>> ./Documentation/devicetree/bindings/firmware/arm,scmi.yaml.
>
> Then git grep for the file name - there is no such compatible. Are you
> sure you follow writing bindings doc?
>
> Think how are you going to use these values. You will have phandle, yes?
> To some controller, yes? Which one?

For the C3 (I believe the T7 is the same), the compatible being used is
"arm,scmi-smc". It is a generic one documented here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/firmware/arm,scmi.yaml?h=v6.18#n202

The phandle used is a subnode of that, to clock protocol:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi?h=v6.18#n116

Same things is done on imx, stm and rockchip platforms from what I can
see.

Jian is just adding the arbitrary IDs used to identify the clocks in the
FW. I don't think there is anything out of the ordirnary here.

Is there something else Rob and I missed reviewing this ?

>
>> 
>> Certain secure clocks on the T7 rely on the ARM SCMI driver stack, which 
>> is officially supported by ARM.
>> 
>> The kernel-side SCMI client implementation resides in 
>> ./drivers/firmware/arm_scmi/.
>> 
>> To enable ARM SCMI on T7, three components are needed:
>> 
>> - Kernel-side definition of ARM SCMI clock indices (this patch addresses 
>> this component);
>> - SCMI server implementation in the ARM Trusted Firmware (ATF) running 
>> at Exception Level 3 (EL3), which has been integrated into the bootloader;
>> - Device Tree Source (DTS) configuration for ARM SCMI clock nodes (the 
>> DTS changes will be submitted after the T7 clock driver patches are 
>> merged upstream).
>
> So silently you keep the users hidden? No, I want to see the users.
>

Is there a new requirement to submit the DTS file changes along with the
driver changes now ?

This has never been case before, especially since the changes are merged
through different trees.

>
> Best regards,
> Krzysztof

-- 
Jerome