Hi, > -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-boun...@lists.openwrt.org] > On Behalf Of eveans2...@gmail.com > Sent: Sonntag, 6. Juni 2021 23:56 > To: openwrt-devel@lists.openwrt.org > Cc: Ian Chang <ianch...@ieiworld.com> > Subject: [PATCH] cn913x: add support for iEi Puzzle-M901/Puzzle-M902
Looks like you sent in an early alpha-state patch.
Please remove all the unfinished/debugging stuff from the DTS and/or label this
as RFC.
A few selected comments below.
>
> From: Ian Chang <ianch...@ieiworld.com>
>
> Hardware specification
> ----------------------
> * CN9130 SoC, Quad-core ARMv8 Cortex-72 @ 2200 MHz
> * 4 GB DDR
> * 4 GB eMMC
> mmcblk0
> ├─mmcblk0p1 64M kernel_1
> ├─mmcblk0p2 64M kernel_2
> ├─mmcblk0p3 512M rootfs_1
> ├─mmcblk0p4 512M rootfs_2
> ├─mmcblk0p5 512M Reserved
> ├─mmcblk0p6 64M Reserved
> └─mmcblk0p7 1.8G rootfs_data
>
> * 4 MB (SPI Flash)
> * 6 x 2.5 Gigabit ports (Puzzle-M901)
> - External PHY with 6 ports (AQR112R)
> * 6 x 2.5 Gigabit ports (Puzzle-M902)
> - External PHY with 6 ports (AQR112R)
> 3 x 10 Gigabit ports (Puzzle-M902)
> - External PHY with 3 ports (AQR113R)
>
> * 4 x Front panel LED
> * 1 x USB 3.0
> * Reset button on Rear panel
> * UART (115200 8N1,header on PCB)
Commit message lacks flashing instructions.
[...]
> --- /dev/null
> +++ b/target/linux/mvebu/files/arch/arm64/boot/dts/marvell/puzzle-
> armada
> +++ -common.dtsi
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
It's "GPL-2.0-or-later OR MIT".
> + cp0_sfp_eth0: sfp-eth@0 {
> +
> + /*
> + * SFP cages are unconnected on early PCBs because of an
> the I2C
> + * lanes not being connected. Prevent the port for being
> + * unusable by disabling the SFP node.
> + */
> + /* status = "disabled"; */
Remove debugging stuff like this.
> + };
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> +
> +&cp0_uart0 {
> + status = "okay";
Indent should be only tabs in DTS.
> + cp0_spi0_pins: cp0-spi-pins-0 {
> + marvell,pins = "mpp13", "mpp14", "mpp15", "mpp16";
> + marvell,function = "spi1";
> + };
> + };
> +};
> \ No newline at end of file
Please add the newlines ...
> diff --git a/target/linux/mvebu/files/arch/arm64/boot/dts/marvell/puzzle-
> m901-cn9131-db.dts
> b/target/linux/mvebu/files/arch/arm64/boot/dts/marvell/puzzle-m901-
> cn9131-db.dts
> new file mode 100644
> index 0000000000..d6879613b6
> --- /dev/null
> +++ b/target/linux/mvebu/files/arch/arm64/boot/dts/marvell/puzzle-m901-
> c
> +++ n9131-db.dts
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (C) 2019 Marvell International Ltd.
> + *
> + * Device tree for the CN9131-DB board.
> + */
> +
> +#include "puzzle-m901-cn9130-db.dts"
Deriving one dts from another is bad practice. That's what DTSI files are for.
> +
> +/ {
> + model = "Puzzle-M901";
> + compatible = "marvell,cn9131", "marvell,cn9130",
> + "marvell,armada-ap807-quad", "marvell,armada-ap807";
Why is model and compatible completely different. Please decide for one name
and then use it consistently.
> +
> + aliases {
> + i2c0 = &cp1_i2c0;
> + ethernet3 = &cp1_eth0;
> + ethernet4 = &cp1_eth1;
> + ethernet5 = &cp1_eth2;
> + gpio3 = &cp1_gpio1;
> + gpio4 = &cp1_gpio2;
Indent .............
> diff --git a/target/linux/mvebu/image/cortexa72.mk
> b/target/linux/mvebu/image/cortexa72.mk
> index 1440c07a0b..c04ad2ae9e 100644
> --- a/target/linux/mvebu/image/cortexa72.mk
> +++ b/target/linux/mvebu/image/cortexa72.mk
> @@ -43,3 +43,27 @@ define Device/marvell_macchiatobin-singleshot
> SUPPORTED_DEVICES := marvell,armada8040-mcbin-singleshot
> endef
> TARGET_DEVICES += marvell_macchiatobin-singleshot
> +
> +define Device/marvell_puzzle-m901-cn9131-db
> + $(call Device/Default-arm64)
> + DEVICE_VENDOR := iEi
> + DEVICE_MODEL := Puzzle-M901
> + DEVICE_DTS := puzzle-m901-cn9131-db
Please name the DTS file properly so the default DEVICE_DTS can be used (and
the one in device definition dropped).
> + IMAGE/sdcard.img.gz := boot-img-ext4 | sdcard-img-ext4 | gzip |
> +append-metadata
> + IMAGES += factory.bin
> + IMAGE/factory.bin := append-kernel | pad-to 64k
> + SUPPORTED_DEVICES :=marvell,cn9131 marvell,puzzle-m901-cn9131-db
One should be enough. It should be the one autoassigned, i.e. the device node
name with "," instead of "_".
Best
Adrian
openpgp-digital-signature.asc
Description: PGP signature
_______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
