[RFC PATCH v1 8/8] bcm53xx: add Cisco Meraki MR32

[Adrian Schmutzler]

Hi Christian,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
> On Behalf Of Christian Lamparter
> Sent: Sonntag, 30. August 2020 05:04
> To: openwrt-devel at lists.openwrt.org
> Cc: rafal at milecki.pl; hauke at hauke-m.de; f.fainelli at gmail.com;
> chrisrblake93 at gmail.com
> 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 
-------------- 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/20200830/2950bea2/attachment.sig>