Message-ID: <38ef7ee0-1879-42d5-a7fd-d32b96d01367@kernel.org>
Date: Thu, 11 Dec 2025 07:16:25 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Jerome Brunet <jbrunet@...libre.com>
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 09/12/2025 11:16, Jerome Brunet wrote:
> 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.

I see, thanks for explanation, it's fine.

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

There is no such requirement, but "has never been case before" is
clearly not accurate, because I raised this question multiple times last
two-three years.

There is no reasonable way to hold publishing of DTS, therefore if
someone uses arguments like above with waiting for driver, I usually got
suspicious.

Also note, that many contributions from various people (not saying that
this one here is) were bad quality and badly designed but without seeing
DTS it takes me significantly more time to understand that design. So
yes, publish your DTS solving all of the questions and making reviewing
easier. Or don't and receive questions...


Best regards,
Krzysztof

Powered by blists - more mailing lists