Re: [PATCH v3 1/3] arm64: dts: cn913x: add device trees for COM Express boards

[Krzysztof Kozlowski]

On 26/10/2023 19:32, Andrew Lunn wrote:
>> +#include "cn9131-db-comexpress.dtsi"
>> +
>> +/ {
>> +	model = "Marvell Armada AC5X RD COM EXPRESS type 7 carrier board";
>> +	compatible = "marvell,ac5x_rd_carrier", "marvell,cn9131", "marvell,cn9130",

Except wrong naming for compatible, I really do not understand what you
want to add here. If AC5X is the carrier, what is the model name of
entire product? If AC5X is not the carrier, where it the carrier?

>> +		     "marvell,armada-ap807-quad", "marvell,armada-ap807";
> 
> This is really a question to the DT Maintainers. This is a carrier
> board for a standardised Com express type 7 board. In theory, you
> should be able to plug any Com Express module into it, not just
> Marvells. So should the compatible list just have a compatible for the
> carrier itself? Not the module which would normally be mounted in it?

Yes, because there are some common parts of the carrier board.

> 
> Should the carrier have a .dtsi file describing it? And then we have a
> .dts file which combines the module .dtsi and the carrier .dtsi?

Depends, how this is organized depends on possible re-usage etc. Usually
answer is: yes, carrier board should have DTSI.

> 
> Sorry i did not ask this earlier, i was thinking more about SolidRuns
> systems, which tend to have custom SOMs and customs carriers, so you
> can only really mount one particular SOM into one particular
> carrier. But that is not true here.
Best regards,
Krzysztof