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 17:42:40 +0800
From:   Inochi Amaoto <inochiama@...look.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        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:     Inochi Amaoto <inochiama@...look.com>,
        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 09:37, Inochi Amaoto wrote:
>> Add clock generator node for CV1800B and CV1812H.
>>
>> Signed-off-by: Inochi Amaoto <inochiama@...look.com>
>> Link: https://github.com/milkv-duo/duo-files/blob/main/hardware/CV1800B/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf
>> ---
>>  arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 4 ++++
>>  arch/riscv/boot/dts/sophgo/cv1812h.dtsi | 4 ++++
>>  arch/riscv/boot/dts/sophgo/cv18xx.dtsi  | 6 ++++++
>>  3 files changed, 14 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
>> index 165e9e320a8c..baf641829e72 100644
>> --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
>> +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
>> @@ -16,3 +16,7 @@ &plic {
>>  &clint {
>>  	compatible = "sophgo,cv1800b-clint", "thead,c900-clint";
>>  };
>> +
>> +&clk {
>> +	compatible = "sophgo,cv1800-clk";
>> +};
>> diff --git a/arch/riscv/boot/dts/sophgo/cv1812h.dtsi b/arch/riscv/boot/dts/sophgo/cv1812h.dtsi
>> index 9a375935b00c..83243c918204 100644
>> --- a/arch/riscv/boot/dts/sophgo/cv1812h.dtsi
>> +++ b/arch/riscv/boot/dts/sophgo/cv1812h.dtsi
>> @@ -21,3 +21,7 @@ &plic {
>>  &clint {
>>  	compatible = "sophgo,cv1812h-clint", "thead,c900-clint";
>>  };
>> +
>> +&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.

Thanks,
Inochi

>Best regards,
>Krzysztof
>
>