Re: [PATCH 02/10] riscv: dts: sophgo: cv18xx: Split into CPU core and peripheral parts

[Krzysztof Kozlowski <krzk@kernel.org>]

On 10/02/2025 14:45, Alexander Sverdlin wrote:
> Thanks for quick review Krzysztof!
> 
> On Mon, 2025-02-10 at 09:43 +0100, Krzysztof Kozlowski wrote:
>> On 09/02/2025 23:06, Alexander Sverdlin wrote:
>>> Make the peripheral device tree re-usable on ARM64 platform by splitting it
>>> into CPU-core specific and peripheral parts.
>>>
>>> Add SOC_PERIPHERAL_IRQ() macro which explicitly maps peripheral nubering
>>> into "plic" interrupt-controller numbering.
>>>
>>> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
>>> ---
>>>   arch/riscv/boot/dts/sophgo/cv181x.dtsi        |   2 +-
>>>   arch/riscv/boot/dts/sophgo/cv18xx-periph.dtsi | 313 ++++++++++++++++++
>>>   arch/riscv/boot/dts/sophgo/cv18xx.dtsi        | 305 +----------------
>                                  ^^^^^^^^^^^
> [1]
> 
>>> --- /dev/null
>>> +++ b/arch/riscv/boot/dts/sophgo/cv18xx-periph.dtsi
>>> @@ -0,0 +1,313 @@
>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>> +/*
>>> + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org>
>>> + * Copyright (C) 2023 Inochi Amaoto <inochiama@outlook.com>
>>> + */
>>> +
>>> +#include <dt-bindings/clock/sophgo,cv1800.h>
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>> +
>>> +/ {
>>> +	osc: oscillator {
>>> +		compatible = "fixed-clock";
>>
>> I really doubt that external oscillator is a peripheral. This is either
>> part of board or the SoC.
> 
> This is actually a problem of the original cv18xx.dtsi [1]. Do you think
> I need to fix it as part of my series? This would touch all the pure
> RiscV boards (using CV18xx SoCs, not SG200x SoCs), which I could avoid
> otherwise.

You are moving the node out of cv18xx.dtsi, so you can move it to final
place for example. But I do not insist, because I also do not know the
final (truly correct) place - don't know the hardware here.

Best regards,
Krzysztof