[OpenWrt-Devel] [PATCH v2 2/2] ramips: Add support for ZBT WE1026-H

[Kristian Evensen]

Hi,

On Tue, Sep 24, 2019 at 1:28 PM Adrian Schmutzler
<mail at adrianschmutzler.de> wrote:
>
> Hi,
>
> first of all:
> Naming scheme for ZBT devices is mixed in ramips. Some include the ZBT in model name, some don't. In your case, this means the following options:
> zbtlink,zbt-we1026-h    corresponding to model    ZBT-WE1026-H
> or
> zbtlink,we1026-h    corresponding to model    WE1026-H
>
> I do not know what's correct here (if there is right/wrong for this at all), but just want you to decide about this intentionally. Even if the existing WE1026-5G proves to have the wrong scheme then, I'd use the correct one for the WE1026-H.

I prefer consistency, so my preference would be staying with the
initial naming scheme used for this "family" of devices.

> > diff --git a/target/linux/ramips/base-files/etc/board.d/01_leds b/target/linux/ramips/base-files/etc/board.d/01_leds
> > index 46202b4117..3e12c2a947 100755
> > --- a/target/linux/ramips/base-files/etc/board.d/01_leds
> > +++ b/target/linux/ramips/base-files/etc/board.d/01_leds
> > @@ -461,6 +461,11 @@ zbtlink,zbt-we826-16m|\
> >  zbtlink,zbt-we826-32m)
> >       set_wifi_led "zbt-we826:green:wifi"
> >       ;;
> > +zbtlink,we1026-h-32m)
>
> If you keep this name (without zbt), then you should sort it appropriately, i.e. "we" before "zbt" ...

True, thanks for spotting that.

> > @@ -721,6 +722,9 @@ ramips_setup_macs()
> >               wan_mac=$(mtd_get_mac_binary factory 0xe006)
> >               label_mac=$(cat /sys/class/ieee80211/phy0/macaddress)
> >               ;;
> > +     zbtlink,we1026-h-32m)
> > +             wan_mac=$(mtd_get_mac_binary factory 0x2e)
> > +             ;;
>
> Depending on how the label MAC address discussion below ends, you might merge this with the node for cudy,wr1000.
>
> >       *)
> >               wan_mac=$(macaddr_add "$(cat /sys/class/net/eth0/address)" 1)
> >               ;;
> > diff --git a/target/linux/ramips/dts/mt7620a_zbtlink_we1026-h-32m.dts b/target/linux/ramips/dts/mt7620a_zbtlink_we1026-h-
> > 32m.dts
> > new file mode 100644
> > index 0000000000..ca62ccfc84
> > --- /dev/null
> > +++ b/target/linux/ramips/dts/mt7620a_zbtlink_we1026-h-32m.dts
> > @@ -0,0 +1,14 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> > +/dts-v1/;
> > +
> > +#include "mt7620a_zbtlink_we1026-h.dtsi"
> > +
> > +/ {
> > +     compatible = "zbtlink,we1026-h-32m", "zbtlink,we1026-h",
> > +                  "zbtlink,we1026","ralink,mt7620a-soc";
> > +     model = "ZBT WE1026-H (32M)";
> > +};
> > +
> > +&firmware {
> > +     reg = <0x50000 0x1fb0000>;
> > +};
> > diff --git a/target/linux/ramips/dts/mt7620a_zbtlink_we1026-h.dtsi b/target/linux/ramips/dts/mt7620a_zbtlink_we1026-h.dtsi
> > new file mode 100644
> > index 0000000000..fed79c2809
> > --- /dev/null
> > +++ b/target/linux/ramips/dts/mt7620a_zbtlink_we1026-h.dtsi
> > @@ -0,0 +1,42 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> > +/dts-v1/;
> > +
> > +#include "mt7620a_zbtlink_we1026.dtsi"
> > +
> > +/ {
> > +     compatible = "zbtlink,we1026-h", "zbtlink,we1026",
> > +                  "ralink,mt7620a-soc";
> > +
> > +     aliases {
> > +             led-boot = &led_power;
> > +             led-failsafe = &led_power;
> > +             led-running = &led_power;
> > +             led-upgrade = &led_power;
> > +             label-mac-device = &wmac;
>
> This won't work, as wmac uses ralink,mtd-eeprom without mtd-mac-address (so you do not have access to the mac address via /proc/device-tree).

Ok, thanks for letting me know. I got confused because I saw other
devices assigning the same value to label-mac-device. I will remove
label-mac-device from the DTS then, as I do not see the point of
having something around that may or may not be used in the future. The
assignment can rather be added when label-mac-device is properly
supported.

> Current policy is to keep label-mac-device here anyway (for future use), but to include a line like the following in the mac setup section of 02_network to actually have label MAC address set:
> label_mac=$(cat /sys/class/ieee80211/phy0/macaddress)
> You have to evaluate whether phy0 or phy1 is correct for your device.
> If phy0 is, you can just add the device(s) to the cudy,wr1000 case.

Ok, thanks.

> If you have access to the WE1026-5G, too, the cleanest way would be to check label MAC address on it and then add label MAC address for all WE1026-* in 02_network, and move the label_mac_device alias to the parent DTSI where wmac is set up.

I will that a look.

> > +define Device/zbtlink_we1026-h-32m
> > +  MTK_SOC := mt7620a
> > +  DTS := WE1026-H-32M
>
> This line (DTS) can be dropped. DTS name is constructed automatically from node name and mtk_soc ...

Thanks for spotting this.

>
> > +  IMAGE_SIZE := 32448k
> > +  DEVICE_VENDOR := Zbtlink
> > +  DEVICE_MODEL := ZBT-WE1026-H
>
> If you choose that name, then you have to use zbtlink_zbt-we1026-h-32m.
> Otherwise, put "WE1026-H" here.

The DEVICE_MODEL for the other 1026-devices all start with
"ZBT-WE1026", so I prefer to remain consistent.

BR,
Kristian

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