[PATCH] bcm4908: initial work on the Broadcom BCM4908 target

[Adrian Schmutzler]

Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
> On Behalf Of Rafal Milecki
> Sent: Freitag, 8. Januar 2021 14:53
> To: openwrt-devel at lists.openwrt.org
> Cc: Rafał Miłecki <rafal at milecki.pl>
> Subject: [PATCH] bcm4908: initial work on the Broadcom BCM4908 target
> 
> From: Rafał Miłecki <rafal at milecki.pl>
> 
> BCM4906, BCM4908 and BCM49408 are SoCs with 64 bit ARMv8 B53 CPUs.
> Upstream Linux is slowly getting support for that SoCs family so it makes
> sense to add target for it.
> 
> It isn't usable yet (it only produces a bootable kernel) so "source-only"
> is used.
> 

some nitpicks below, as usual.

[...]

> --- /dev/null
> +++ b/target/linux/bcm4908/generic/target.mk
> @@ -0,0 +1 @@
> +BOARDNAME:=Generic
> diff --git a/target/linux/bcm4908/image/Makefile
> b/target/linux/bcm4908/image/Makefile
> new file mode 100644
> index 0000000000..672cdf40e6
> --- /dev/null
> +++ b/target/linux/bcm4908/image/Makefile
> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +include $(TOPDIR)/rules.mk
> +include $(INCLUDE_DIR)/image.mk
> +
> +define Build/bcm4908lzma
> +	$(STAGING_DIR_HOST)/bin/lzma e -lc1 -lp2 -pb2 -d22 $@ $@.new
> +	mv $@.new $@
> +endef
> +
> +define Device/Default
> +  KERNEL := kernel-bin | bcm4908lzma
> +  KERNEL_DEPENDS = $$(wildcard $(DTS_DIR)/$$(DEVICE_DTS).dts)
> +  KERNEL_INITRAMFS_SUFFIX := .bin
> +  KERNEL_INITRAMFS := kernel-bin | bcm4908lzma
> +  FILESYSTEMS := squashfs
> +  KERNEL_NAME := Image
> +  IMAGE_NAME = $$(IMAGE_PREFIX)-$$(1).$$(2)
> +  BLOCKSIZE := 128k
> +  PAGESIZE := 2048
> +endef
> +
> +define Device/asus-gt-ac5300

It would be nice if we could use the vendor_model scheme with underscore here, like for most of the other targets (i.e. asus_gt-ac5300 in this case).

> +  DEVICE_VENDOR := Asus
> +  DEVICE_MODEL := GT-AC5300
> +  DEVICE_DTS := broadcom/bcm4908/bcm4908-asus-gt-ac5300

This could be auto-assembled based on SOC variable and DEVICE_NAME/$1 eventually.

> +  IMAGES := bin
> +endef
> +TARGET_DEVICES += asus-gt-ac5300
> +

[...]

> +--- /dev/null
> ++++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908.dtsi
> +@@ -0,0 +1,187 @@
> ++// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> ++
> ++#include <dt-bindings/interrupt-controller/irq.h>
> ++#include <dt-bindings/interrupt-controller/arm-gic.h>
> ++
> ++/dts-v1/;

dts-v1 needs to be placed before any definitions to be valid. I assume the includes only contain macros (so don't count in that context), but one might still consider to move the dts-v1 in front of them, directly after the license. (Same argument applies indirectly to the includes in the DTS file above.

Best

Adrian 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openpgp-digital-signature.asc
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.openwrt.org/pipermail/openwrt-devel/attachments/20210108/5d69384a/attachment.sig>