[OpenWrt-Devel] [PATCH] ramips: add support for GL.iNet microuter-N300

[David Bauer]

Hello Adrian,

On 1/28/20 5:34 PM, Adrian Schmutzler wrote:
>> The GL.iNet microuter-N300 (internally referred as MT300N-v4) is a
>> pocket-size travel router. It is essentially identical to the VIXMINI
>> (internally referred as MT300N-v3) but with double the RAM and
>> SPI-flash.
> 
> One could consider adding the v3/v4 names with the DEVICE_ALT0 syntax.

The naming for these devices is only used in their OpenWrt fork. I wouldn't
include this in OpenWrt, as it's too abstract.

> Typically we have a space in-between here "/{" -> "/ {", but looks like it's
> missing for vixmini as well.

You are right, will fix this.

> 
>> +	compatible = "glinet,microuter-n300", "mediatek,mt7628an-soc";
>> +	model = "GL.iNet microuter-N300";
>> +};
>> +
>> +&led_power_blue {
> 
> If there is only one color per type, I'd personally remove the color from the
> label (-> led_power for this case).

This is one LED on the casing with two colors and two independent functions,
therefore including full information here.

>> diff --git a/target/linux/ramips/dts/mt7628an_glinet_vixmini_microuter.dtsi
>> b/target/linux/ramips/dts/mt7628an_glinet_vixmini_microuter.dtsi
>> new file mode 100644
>> index 0000000000..b5611ac11e
>> --- /dev/null
>> +++ b/target/linux/ramips/dts/mt7628an_glinet_vixmini_microuter.dtsi
>> @@ -0,0 +1,104 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
>> +
>> +#include "mt7628an.dtsi"
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/input/input.h>
>> +
>> +/{
> 
> Space missing here again (see above).
> 
>> +	aliases {
>> +		led-boot = &led_power_blue;
>> +		led-failsafe = &led_power_blue;
>> +		led-running = &led_power_blue;
>> +		led-upgrade = &led_power_blue;
>> +
>> +		label-mac-device = ðernet;
> 
> I'd have done this in a separate commit, or at least added a comment to the
> commit message.

Will add this to the commit message.

>> +	};
>> +
>> +	chosen {
>> +		bootargs = "console=ttyS0,115200";
>> +	};
>> +
>> +	leds {
>> +		compatible = "gpio-leds";
>> +
>> +		led_power_blue: power {
>> +			/* Name is set device-specific */
>> +			gpios = <&gpio1 10 GPIO_ACTIVE_LOW>;
>> +		};
>> +
>> +		led_wlan_white: wlan {
>> +			/* Name is set device-specific */
>> +			gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
>> +			linux,default-trigger = "phy0tpt";
>> +		};
>> +	};
>> +
>> +	keys {
>> +		compatible = "gpio-keys";
>> +
>> +		reset {
>> +			label = "reset";
>> +			gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
>> +			linux,code = <KEY_RESTART>;
>> +		};
>> +	};
>> +};
>> +
>> +&state_default {
>> +	gpio {
>> +		ralink,group = "wdt", "wled_an", "p1led_an";
>> +		ralink,function = "gpio";
>> +	};
>> +};
>> +
>> +&ethernet {
>> +	mtd-mac-address = <&factory 0x4>;
>> +};
>> +
>> +&wmac {
>> +	status = "okay";
>> +};
>> +
>> +&spi0 {
>> +	status = "okay";
>> +
>> +	flash at 0 {
>> +		compatible = "jedec,spi-nor";
>> +		reg = <0>;
>> +		spi-max-frequency = <10000000>;
>> +
>> +		partitions: partitions {
>> +			compatible = "fixed-partitions";
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +
>> +			partition at 0 {
>> +				label = "u-boot";
>> +				reg = <0x0 0x30000>;
>> +				read-only;
>> +			};
>> +
>> +			partition at 30000 {
>> +				label = "u-boot-env";
>> +				reg = <0x30000 0x10000>;
>> +				read-only;
>> +			};
>> +
>> +			factory: partition at 40000 {
>> +				label = "factory";
>> +				reg = <0x40000 0x10000>;
>> +				read-only;
>> +			};
>> +
>> +			/*
>> +			 * Firmware-partition size is model-specific
>> +			 * due to different flash sizes.
>> +			 */
>> +			firmware_part: partition at 50000 {
> 
> Why not just call it "firmware" corresponding to the label and how we do it with
> factory?

For me, adding contextual information to a label makes it easier to
understand it's origin. As neither the specification nor upstream does
specify a naming convention for labels, this should be fine.

>> diff --git a/target/linux/ramips/image/mt76x8.mk
>> b/target/linux/ramips/image/mt76x8.mk
>> index 5ddeec14d5..37cac84229 100644
>> --- a/target/linux/ramips/image/mt76x8.mk
>> +++ b/target/linux/ramips/image/mt76x8.mk
>> @@ -70,6 +70,14 @@ define Device/glinet_gl-mt300n-v2
>>  endef
>>  TARGET_DEVICES += glinet_gl-mt300n-v2
>>
>> +define Device/glinet_microuter-n300
>> +  IMAGE_SIZE := 16064k
>> +  DEVICE_VENDOR := GL.iNet
>> +  DEVICE_MODEL := microuter-N300
>> +  SUPPORTED_DEVICES += microuter-n300
> 
> That's needed for GLinet's OpenWrt?

Yes, otherwise the upgrade GUI rejects the image.

Best wishes
David

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