Re: [PATCH v2 4/4] arm64: dts: qcom: qcs9100: Add support for the QCS9100 Ride and Ride Rev3 boards

[Caleb Connolly <caleb.connolly@linaro.org>]

On 17/09/2024 01:32, Konrad Dybcio wrote:
> On 11.09.2024 1:10 PM, Tengfei Fan wrote:
>> Add device tree support for the QCS9100 Ride and Ride Rev3 boards. The
>> QCS9100 is a variant of the SA8775p, and they are fully compatible with
>> each other. The QCS9100 Ride/Ride Rev3 board is essentially the same as
>> the SA8775p Ride/Ride Rev3 board, with the QCS9100 SoC mounted instead
>> of the SA8775p.
>>
>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>> ---
> 
> Reviewed-by: Konrad Dybcio <konradybcio@kernel.org>

I don't understand this, if both boards are identical except for the
name of the SoC then why do we have two devicetree files?

You can surely detect which SoC is in use at runtime if necessary, and
maybe pick a name which doesn't have the SoC in it if you really want to
avoid confusion.

If there are differentiating features which will be added later, then I
think this at least deserves a comment stating as such.

Additionally, the files should be shuffled around to better represent
that there's two very similar boards with just some minor differences,
this is a common case already and there is a standard way to handle it
(see e.g. sdm845-oneplus-common.dtsi and
sdm845-oneplus-enchilada/fajita.dts)

#include'ing a .dts file just seems like a mess here.

Kind regards,

> 
> Konrad

-- 
// Caleb (they/them)