Message-ID: <1jy0ncvu23.fsf@starbuckisacylon.baylibre.com>
Date: Tue, 09 Dec 2025 11:16:52 +0100
From: Jerome Brunet <jbrunet@...libre.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Jian Hu <jian.hu@...ogic.com>,  Xianwei Zhao <xianwei.zhao@...ogic.com>,
  Chuan Liu <chuan.liu@...ogic.com>,  Neil Armstrong
 <neil.armstrong@...aro.org>,  Kevin Hilman <khilman@...libre.com>,
  Stephen Boyd <sboyd@...nel.org>,  Michael Turquette
 <mturquette@...libre.com>,  robh+dt <robh+dt@...nel.org>,  Rob Herring
 <robh@...nel.org>,  devicetree <devicetree@...r.kernel.org>,  linux-clk
 <linux-clk@...r.kernel.org>,  linux-amlogic
 <linux-amlogic@...ts.infradead.org>,  linux-kernel
 <linux-kernel@...r.kernel.org>,  linux-arm-kernel
 <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v6 2/5] dt-bindings: clock: add Amlogic T7 SCMI clock
 controller

On Tue 09 Dec 2025 at 07:01, Krzysztof Kozlowski <krzk@...nel.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@...ogic.com>
>>>> Acked-by: Rob Herring (Arm) <robh@...nel.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

Powered by blists - more mailing lists