Hi Christian,

> -----Original Message-----
> From: openwrt-devel [mailto:[email protected]]
> On Behalf Of Christian Lamparter
> Sent: Sonntag, 30. August 2020 05:04
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: [RFC PATCH v1 8/8] bcm53xx: add Cisco Meraki MR32

though this is RFC, a few cosmetic remarks I just noticed when reading:

> +     meraki,mr32)
> +             # The MAC is stored on an AT24C64 eeprom and not on the
> nvram
> +             et2macaddr=$(get_mac_binary "/sys/bus/i2c/devices/0-
> 0050/eeprom" 102)

I'd prefer hex notation for the location (0x66) like we've done it for most 
other targets recently, as this is easier to read for bigger numbers.

> +++ b/target/linux/bcm53xx/base-files/lib/preinit/07_set_preinit_iface_b
> +++ cm53xx
> @@ -0,0 +1,16 @@
> +#!/bin/sh
> +

I recently dropped the shebangs in all preinit scripts, as they are 
non-executable and sourced. (Won't hurt to keep it, though.)

> --- a/target/linux/bcm53xx/image/Makefile
> +++ b/target/linux/bcm53xx/image/Makefile
> @@ -320,6 +320,32 @@ define Device/luxul_xwr-3150  endef
> TARGET_DEVICES += luxul_xwr-3150
> 
> +define Device/meraki-mr32

I recently changed this for most targets in order to achieve a consistent 
vendor_model naming scheme for OpenWrt, so I'd be happy it the name was changed 
to meraki_mr32 here.

> +  DEVICE_TITLE := Meraki MR32

Please use to the following scheme here:

DEVICE_VENDOR := Meraki
DEVICE_MODEL := MR32

Best

Adrian 

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

_______________________________________________
openwrt-devel mailing list
[email protected]
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to