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: <Z70b1x6ephA1FyEy@nchen-desktop>
Date: Tue, 25 Feb 2025 09:24:39 +0800
From: Peter Chen <peter.chen@...tech.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
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 25-02-24 13:07:45, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL
> 
> 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.
> 

Thanks for clarify, it is the second case, we have clock controller for
that, but now it is not ready to upstream due to some dependency, like
mailbox device driver, I will drop UART node at v2 patch set.

-- 

Best regards,
Peter