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

[Inochi Amaoto]

>
>On 07/12/2023 09:37, Inochi Amaoto wrote:
>> Add clock generator node for CV1800B and CV1812H.
>>
>> Signed-off-by: Inochi Amaoto <inochiama@xxxxxxxxxxx>
>> 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@3002000 {
>> +			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
>
>