Re: [PATCH 2/2] arm64: dts: rockchip: Add CM3588 NAS board

From: Heiko Stübner
Date: Wed May 29 2024 - 03:58:15 EST


Hi Sebastian,

Am Dienstag, 28. Mai 2024, 17:55:55 CEST schrieb Sebastian Kropatsch:
> Am 27.05.2024 um 22:54 schrieb Heiko Stübner:
> > Am Montag, 27. Mai 2024, 21:02:02 CEST schrieb Jonas Karlman:
> >> On 2024-05-26 23:48, Sebastian Kropatsch wrote:
> >>> The CM3588 NAS by FriendlyElec pairs the CM3588 compute module, based on
> >>> the Rockchip RK3588 SoC, with the CM3588 NAS Kit carrier board.
> >>>
> >>> Hardware features:
> >>> - Rockchip RK3588 SoC
> >>> - 4GB/8GB/16GB LPDDR4x RAM
> >>> - 64GB eMMC
> >>> - MicroSD card slot
> >>> - 1x RTL8125B 2.5G Ethernet
> >>> - 4x M.2 M-Key with PCIe 3.0 x1 (via bifurcation) for NVMe SSDs
> >>> - 2x USB 3.0 (USB 3.1 Gen1) Type-A, 1x USB 2.0 Type-A
> >>> - 1x USB 3.0 Type-C with DP AltMode support
> >>> - 2x HDMI 2.1 out, 1x HDMI in
> >>> - MIPI-CSI Connector, MIPI-DSI Connector
> >>> - 40-pin GPIO header
> >>> - 4 buttons: power, reset, recovery, MASK, user button
> >>> - 3.5mm Headphone out, 2.0mm PH-2A Mic in
> >>> - 5V Fan connector, PWM buzzer, IR receiver, RTC battery connector
> >>>
> >>> PCIe bifurcation is used to handle all four M.2 sockets at PCIe 3.0 x1
> >>> speed. Data lane mapping in the DT is done like described in commit
> >>> f8020dfb311d ("phy: rockchip-snps-pcie3: fix bifurcation on rk3588").
> >>>
> >>> This device tree includes support for eMMC, SD card, ethernet, all USB2
> >>> and USB3 ports, all four M.2 slots, GPU, RTC, buzzer, UART debugging as
> >>> well as the buttons and LEDs.
> >>> The GPIOs are labeled according to the schematics.
> >>>
> >>> Signed-off-by: Sebastian Kropatsch <seb-dev@xxxxxx>
> >>> ---
> >>> arch/arm64/boot/dts/rockchip/Makefile | 1 +
> >>> .../boot/dts/rockchip/rk3588-cm3588-nas.dts | 1269 +++++++++++++++++
> >>> 2 files changed, 1270 insertions(+)
> >>> create mode 100644 arch/arm64/boot/dts/rockchip/rk3588-cm3588-nas.dts
> >>
> >> Because the CM3588 is a SoM and the NAS is a carrier board this should
> >> probably be split in two, cm3588.dtsi and cm3588-nas.dts.
> >
> > also, because of that way too generic name "cm", please incorporate the
> > company name in the filename as well. For the same reason we named
> > the rk3568-wolfvision-pf5.dts that way ;-) [Wolfvision being the company]
> >
> > So maybe:
> > rk3588-friendlyelec-cm3588.dtsi and rk3588-friendlyelec-cm3588-nas.dts
> >
>
> Yes, I agree that the name is very generic. I struggled with this as
> well, but your suggestion sounds good!
>
> In this case, is it also preferred to change the commit message to
> include the company name event though the commit message subject exceeds
> 50 characters this way?
> ("arm64: dts: rockchip: Add FriendlyElec CM3588 NAS board")

Were does a 50 character limit even come from?
In other words, the subject should be meaningful and with the needed
prefixes adhering to a 50 chars maxlen would cause pretty unreadable
subject in a lot of cases.

So the proposed subject is absolutely fine :-) .


Heiko