Re: [PATCH v5 1/2] arm64: dts: marvell: Add Armada 98DX2530 SoC and RD-AC5X board

From: Chris Packham
Date: Wed May 11 2022 - 18:59:46 EST



On 12/05/22 04:20, Andrew Lunn wrote:
> On Wed, May 11, 2022 at 07:10:03PM +0300, Vadym Kochan wrote:
>> Hi Chris,
>>
>>> arch/arm64/boot/dts/marvell/Makefile | 1 +
>>> .../boot/dts/marvell/armada-98dx2530.dtsi | 310 ++++++++++++++++++
>>> arch/arm64/boot/dts/marvell/rd-ac5x.dts | 90 +++++
>>> 3 files changed, 401 insertions(+)
>> Marvell is going to start the upstreaming of AC5X boards support,
That's good news. I'm probably the customer that's been nagging the
Marvell support team. But I'm also impatient hence I started working on
this series. The pinctrl and mvneta changes have already been accepted.
>> we have also patches with similar .dts(i) files
>> but with different naming:
>>
>> ac5.dtsi
>> ac5_rd.dts
>> ac5_db.dts
>> ac5x_db.dts
>>
>> What do you think about to use these naming scheme ?

Personally I thought they'd be rejected upstream as being too vague and
generic. I settled on armada-98dx2530 as I saw the 98dx2530 name used on
the Marvell Portal to refer to the CnM block for the AC5/AC5X. I was
going to call the board file "rd-ac5x-32g16hvg6hlg.dts" as that's what
the silkscreen on the board I have says but I shortened it to "rd-ac5x"
as the switch port configuration is largely irrelevant to the board
support I'm trying to get landed.

> Chris has done all the hard work, he gets to pick the naming. And get
> his files merged first.

I'm not against changing if there is a consensus. On another thread the
idea of armada-98dx25xx/armada-98dx35xx was mentioned. That might be a
reasonable compromise (although technically there's no difference in the
CPU block between the 25xx and 35xx).

> However, now that i come to look in arch/arm64/boot/dts/marvell, i
> think most of the current filenames proposed don't match the current names.
>
> armada-98dx2530.dtsi fits the current pattern.
>
> However, Chris's board files should probably be
>
> armada-98dx2530-rd.dts
>
> and the other files should be
>
> armada-98dx2530-db.dts
>
> armada-98dx2530-x-db.dts
>
> What does the x in x_db mean? Does that refer to the board or the SoC?

The x is from AC5X so we'd actually have armada-98dx25xx-db.dts and
armada-98dx35xx-db.dts. My board would be called armada-98dx35xx-rd.dts
or perhaps armada-98dx3550-rd.dts.