Re: [PATCH v3 2/4] arm64: dts: morello: Add support for common functionalities

[Krzysztof Kozlowski <krzk@kernel.org>]

On 03/01/2025 16:32, Vincenzo Frascino wrote:
> 
> 
> On 03/01/2025 07:53, Krzysztof Kozlowski wrote:
>> On Thu, Jan 02, 2025 at 03:54:14PM +0000, Vincenzo Frascino wrote:
>>> The Morello architecture is an experimental extension to Armv8.2-A,
>>> which extends the AArch64 state with the principles proposed in
>>> version 7 of the Capability Hardware Enhanced RISC Instructions
>>> (CHERI) ISA.
>>>
>>> The Morello Platform (soc) and the Fixed Virtual Platfom (fvp) share
>>> some functionalities that have conveniently been included in
>>> morello.dtsi to avoid duplication.
>>>
>>> Introduce morello.dtsi.
>>>
>>> Note: Morello fvp will be introduced with a future patch series.
>>>
>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>> ---
>>>  arch/arm64/boot/dts/arm/morello.dtsi | 341 +++++++++++++++++++++++++++
>>>  1 file changed, 341 insertions(+)
>>>  create mode 100644 arch/arm64/boot/dts/arm/morello.dtsi
>>>
>>> diff --git a/arch/arm64/boot/dts/arm/morello.dtsi b/arch/arm64/boot/dts/arm/morello.dtsi
>>> new file mode 100644
>>> index 000000000000..67bc960f4596
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/arm/morello.dtsi
>>> @@ -0,0 +1,341 @@
>>> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
>>> +/*
>>> + * Copyright (c) 2020-2024, Arm Limited. All rights reserved.
>>> + */
>>> +
>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +
>>> +/ {
>>> +	interrupt-parent = <&gic>;
>>> +
>>> +	#address-cells = <2>;
>>> +	#size-cells = <2>;
>>> +
>>> +	chosen { };
>>
>> Drop
> 
> Fine, I will do it in the next iteration. The file you asked me to see
> (sm8650.dtsi) has the same expression. 


Because it did exactly the same as you here - copied it.

> Why this is incorrect? What's the rationale?


Redundant, makes this code unnecessary bigger and raises question: what
if something relies on it but the actual dependency is not
expressed/documented?

...

> 
>>> +			interrupts = <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
>>> +			clocks = <&dpu_aclk>;
>>> +
>>> +			hdmi_tx: hdmi-transmitter@70 {
>>> +				compatible = "nxp,tda998x";
>>> +				reg = <0x70>;
>>> +				video-ports = <0x234501>;
>>> +				port {
>>> +					tda998x_0_input: endpoint {
>>> +						remote-endpoint = <&dp_pl0_out0>;
>>> +					};
>>> +				};
>>> +			};
>>> +		};
>>> +
>>> +		gic: interrupt-controller@2c010000 {
>>> +			compatible = "arm,gic-v3";
>>> +			#address-cells = <2>;
>>> +			#interrupt-cells = <3>;
>>> +			#size-cells = <2>;
>>> +			ranges;
>>
>>
>> Missing reg.... and why do you have ranges here? No children.
>>
> 
> I will remove it in the next iteration.
> 


Next patch brought some answers here. I just don't get why entire GIC is
not part of the DTSI.


Best regards,
Krzysztof