Hi,

some format nitpicking again and some other comments below:

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-boun...@lists.openwrt.org]
> On Behalf Of Rafal Milecki
> Sent: Freitag, 15. Januar 2021 10:44
> To: openwrt-devel@lists.openwrt.org
> Cc: Rafał Miłecki <ra...@milecki.pl>
> Subject: [PATCH 2/2] bcm4908: build flashable & bootable firmware images
> 
> From: Rafał Miłecki <ra...@milecki.pl>
> 
> BCM4908 bootloader requires firmware with JFFS2 image containing:
> 1. cferam.000
> 2. 94908.dtb
> 3. vmlinux.lz
> 4. device custom files
> 
> cferam.000 can be obtained from the bcm63xx-cfe repository. It requires
> specifying directory path that is defined using COMPATIBLE variable.
> 
> For convenience directories with device custom files are named the same
> way.
> 
> Signed-off-by: Rafał Miłecki <ra...@milecki.pl>
> ---
>  target/linux/bcm4908/image/Makefile           | 19 +++++++++++++++++++
>  .../image/asus,gt-ac5300/rom/etc/image_ident  |  2 ++
>  .../asus,gt-ac5300/rom/etc/image_version      |  1 +
>  .../image/netgear,r8000p/etc/image_ident      |  4 ++++
>  .../image/netgear,r8000p/etc/image_version    |  1 +
>  5 files changed, 27 insertions(+)
>  create mode 100644 target/linux/bcm4908/image/asus,gt-
> ac5300/rom/etc/image_ident
>  create mode 100644 target/linux/bcm4908/image/asus,gt-
> ac5300/rom/etc/image_version
>  create mode 100644
> target/linux/bcm4908/image/netgear,r8000p/etc/image_ident
>  create mode 100644
> target/linux/bcm4908/image/netgear,r8000p/etc/image_version
> 
> diff --git a/target/linux/bcm4908/image/Makefile
> b/target/linux/bcm4908/image/Makefile
> index f5db38915d..eed4779d92 100644
> --- a/target/linux/bcm4908/image/Makefile
> +++ b/target/linux/bcm4908/image/Makefile
> @@ -13,6 +13,21 @@ define Build/bcm4908kernel
>       mv $@.new $@
>  endef
> 
> +define Build/bcm4908img
> +     rm -fr $@-bootfs
> +     mkdir -p $@-bootfs
> +     cp -r $(COMPATIBLE)/* $@-bootfs/
> +     touch $@-bootfs/1-dummy
> +     cp $(DTS_DIR)/$(firstword $(DEVICE_DTS)).dtb $@-bootfs/94908.dtb
> +     cp $(KDIR)/bcm63xx-cfe/$(COMPATIBLE)/cferam.000 $@-bootfs/
> +     cp $(IMAGE_KERNEL) $@-bootfs/vmlinux.lz
> +
> +     $(STAGING_DIR_HOST)/bin/mkfs.jffs2 --pad --little-endian --squash-
> uids -v -e 128KiB -o $@-bootfs.jffs2 -d $@-bootfs -m none -n
> +     $(STAGING_DIR_HOST)/bin/bcm4908img create $@ -f $@-
> bootfs.jffs2 endef
> +
> +DEVICE_VARS += COMPATIBLE
> +
>  define Device/Default
>    KERNEL := kernel-bin | bcm4908lzma | bcm4908kernel
>    KERNEL_DEPENDS = $$(wildcard $(DTS_DIR)/$$(DEVICE_DTS).dts) @@ -
> 29,7 +44,9 @@ define Device/asus_gt-ac5300
>    DEVICE_VENDOR := Asus
>    DEVICE_MODEL := GT-AC5300
>    DEVICE_DTS := broadcom/bcm4908/bcm4908-asus-gt-ac5300
> +  COMPATIBLE := asus,gt-ac5300

Consider using just the SUPPORTED_DEVICES variable name here instead. Even if 
it's not used here for updating, it's essentially where the current compatible 
is stored on almost all of the other targets, and it's in DEVICE_VARS by 
default. (One can use $(firstword $(SUPPORTED_DEVICES)) to be sure in 
Build/bcm4908img then)

Apart from that (and independent of whether you change the name or not), we 
should define a default value for this in Device/Default:

SUPPORTED_DEVICES := $(subst _,$(comma),$(1))
or
COMPATIBLE := $(subst _,$(comma),$(1))

This will cover all current cases, and it can easily be overwritten by 
device-specific definitions where necessary.

>    IMAGES := bin
> +  IMAGE/bin := bcm4908img
>  endef
>  TARGET_DEVICES += asus_gt-ac5300
> 
> @@ -37,7 +54,9 @@ define Device/netgear_r8000p
>    DEVICE_VENDOR := Netgear
>    DEVICE_MODEL := R8000P
>    DEVICE_DTS := broadcom/bcm4908/bcm4906-netgear-r8000p
> +  COMPATIBLE := netgear,r8000p
>    IMAGES := bin
> +  IMAGE/bin := bcm4908img
>  endef
>  TARGET_DEVICES += netgear_r8000p
> 
> diff --git a/target/linux/bcm4908/image/asus,gt-
> ac5300/rom/etc/image_ident b/target/linux/bcm4908/image/asus,gt-
> ac5300/rom/etc/image_ident
> new file mode 100644
> index 0000000000..9e73fe74a7
> --- /dev/null
> +++ b/target/linux/bcm4908/image/asus,gt-ac5300/rom/etc/image_ident

Can you point me at where these files are picked up (in code)?

I wonder whether we can replace the commas here something less strange for file 
names (preferably underscores so we simply use DEVICE_NAME variable).

Best

Adrian

> @@ -0,0 +1,2 @@
> +@(#) $imageversion: HND1731918 $
> +@(#) $imageversion: HND1731918 $
> diff --git a/target/linux/bcm4908/image/asus,gt-
> ac5300/rom/etc/image_version b/target/linux/bcm4908/image/asus,gt-
> ac5300/rom/etc/image_version
> new file mode 100644
> index 0000000000..fd67746f78
> --- /dev/null
> +++ b/target/linux/bcm4908/image/asus,gt-ac5300/rom/etc/image_version
> @@ -0,0 +1 @@
> +HND1731918
> diff --git a/target/linux/bcm4908/image/netgear,r8000p/etc/image_ident
> b/target/linux/bcm4908/image/netgear,r8000p/etc/image_ident
> new file mode 100644
> index 0000000000..15cb69d1af
> --- /dev/null
> +++ b/target/linux/bcm4908/image/netgear,r8000p/etc/image_ident
> @@ -0,0 +1,4 @@
> +@(#) $imageversion: 5024HNDrc11R8000P2602103 $
> +@(#) $imageversion: 5024HNDrc11R8000P2602103 $
> +@(#) $changelist: Changelist:
> REL_502HND04rc11_BISON04T_REL_7_14_170_34Revision:   765096 $
> +@(#) $changelist: Changelist:
> REL_502HND04rc11_BISON04T_REL_7_14_170_34Revision:   765096 $
> diff --git a/target/linux/bcm4908/image/netgear,r8000p/etc/image_version
> b/target/linux/bcm4908/image/netgear,r8000p/etc/image_version
> new file mode 100644
> index 0000000000..f469dfed31
> --- /dev/null
> +++ b/target/linux/bcm4908/image/netgear,r8000p/etc/image_version
> @@ -0,0 +1 @@
> +5024HNDrc11R8000P2602103
> --
> 2.26.2
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Attachment: openpgp-digital-signature.asc
Description: PGP signature

_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to