linux-kernel - Re: [PATCH v3 3/4] riscv: dts: sophgo: add clock generator for Sophgo CV1800 series SoC

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]

Date:   Thu, 7 Dec 2023 13:52:16 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Inochi Amaoto <inochiama@...look.com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Chao Wei <chao.wei@...hgo.com>,
        Chen Wang <unicorn_wang@...look.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>
Cc:     Jisheng Zhang <jszhang@...nel.org>,
        Liu Gui <kenneth.liu@...hgo.com>,
        Emil Renner Berthing <emil.renner.berthing@...onical.com>,
        qiujingbao.dlmu@...il.com, dlan@...too.org,
        linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v3 3/4] riscv: dts: sophgo: add clock generator for Sophgo
 CV1800 series SoC

On 07/12/2023 10:42, Inochi Amaoto wrote:
>>> +&clk {
>>> +	compatible = "sophgo,cv1810-clk";
>>> +};
>>> diff --git a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
>>> index 2d6f4a4b1e58..6ea1b2784db9 100644
>>> --- a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
>>> +++ b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
>>> @@ -53,6 +53,12 @@ soc {
>>>  		dma-noncoherent;
>>>  		ranges;
>>>
>>> +		clk: clock-controller@...2000 {
>>> +			reg = <0x03002000 0x1000>;
>>> +			clocks = <&osc>;
>>> +			#clock-cells = <1>;
>>
>> I don't find such layout readable and maintainable. I did some parts
>> like this long, long time ago for one of my SoCs (Exynos54xx), but I
>> find it over time unmaintainable approach. I strongly suggest to have
>> compatible and other properties in one place, so cv1800 and cv1812, even
>> if it duplicates the code.
>>
> 
> Hi Krzysztof:
> 
> Thanks for your advice, but I have a question about this: when I should
> use the DT override? The memory mapping of the CV1800 and CV1810 are
> almost the same (the CV1810 have more peripheral and the future SG200X
> have the same layout). IIRC, this is why conor suggested using DT override
> to make modification easier. But duplicating node seems to break thiS, so
> I's pretty confused.

Go with whatever your subarchitecture and architecture maintainers
prefer, I just shared my opinion that I find such code difficult to read
and maintain.

Extending node with supplies, pinctrl or even clocks would be readable.
But the compatible: no. The same applies when you need to delete
property or subnode: not readable/maintainable IMHO.

Best regards,
Krzysztof