linux-kernel - Re: [PATCH 6/6] arm64: dts: cix: add initial CIX P1(SKY1) dts support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <24003082-d1ca-43c6-ae96-3705e0f964f0@kernel.org>
Date: Mon, 24 Feb 2025 13:07:45 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Peter Chen <peter.chen@...tech.com>
Cc: Arnd Bergmann <arnd@...db.de>, Rob Herring <robh@...nel.org>,
 krzk+dt@...nel.org, Conor Dooley <conor+dt@...nel.org>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, cix-kernel-upstream@...tech.com,
 "Fugang . duan" <fugang.duan@...tech.com>
Subject: Re: [PATCH 6/6] arm64: dts: cix: add initial CIX P1(SKY1) dts support

On 24/02/2025 11:39, Peter Chen wrote:
>>>>>>
>>>>>>> +       sky1_fixed_clocks: fixed-clocks {
>>>>>>> +               uartclk: uartclk {
>>>>>>> +                       compatible = "fixed-clock";
>>>>>>> +                       #clock-cells = <0>;
>>>>>>> +                       clock-frequency = <100000000>;
>>>>>>> +                       clock-output-names = "uartclk";
>>>>>>
>>>>>>> +               uart_apb_pclk: uart_apb_pclk {
>>>>>>> +                       compatible = "fixed-clock";
>>>>>>> +                       #clock-cells = <0>;
>>>>>>> +                       clock-frequency = <200000000>;
>>>>>>> +                       clock-output-names = "apb_pclk";
>>>>>>
>>>>>>
>>>>>> Clock names don't need "clk" in them, and there should
>>>>>> be no underscore -- use '-' instead of '_' when separating
>>>>>> strings in DT.
>>>>>
>>>>> Will change to:
>>>>> uart_apb: clock-uart-apb {
>>>>
>>>> No, instead explain why this is part of SoC - or what are you missing
>>>> here - and use preferred naming.
>>>
>>> It is in SoC part, APB clock uses to visit register, and the function
>>> amba_get_enable_pclk at file drivers/amba/bus.c needs it during uart
>>> device probes. It uses common Arm uart pl011 IP, the binding doc
>>> described at: Documentation/devicetree/bindings/serial/pl011.yaml
>>
>> So you added fake clock? Everything you wrote is not the reason to add
>> such clock.
> 
> Not a fake clock, it is the real clocks, but depends on firmware open
> their parents and configure their rate. It could let others do their

In one place you speak about UART, which is the consumer and not
relevant. Here you mention it is real clock. That's all confusing, so to
clarify:

We talk about clock which is generated/output by something. Something
which controls way it is generated is clock controller. Either you have
here crystal or have here clock controller. If first, fixed clock is for
that. If second, you need proper clock controller binding. You can add
stubs for missing pieces, but this requires explanation and TODO/FIXME
comment.

> upstream work based on workable console.
> 
> Which option you would like to accept?

You did not describe the hardware, so I have no clue what is there.

Best regards,
Krzysztof