[OpenWrt-Devel] [PATCH] ramips: add support for Northbound Networks Zodiac GX

mail at adrianschmutzler.de mail at adrianschmutzler.de
Tue Aug 20 12:14:09 EDT 2019


Hi,

just some comments on your comments ;-)

> -----Original Message-----
> From: Yousong Zhou [mailto:yszhou4tech at gmail.com]
> Sent: Dienstag, 20. August 2019 17:58
> To: mail at adrianschmutzler.de
> Cc: John Crispin <john at phrozen.org>; OpenWrt Development List
> <openwrt-devel at lists.openwrt.org>; Paul Zanna
> <paul at northboundnetworks.com>
> Subject: Re: [OpenWrt-Devel] [PATCH] ramips: add support for Northbound
> Networks Zodiac GX
> 
> On Tue, 20 Aug 2019 at 23:38, <mail at adrianschmutzler.de> wrote:
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: openwrt-devel [mailto:openwrt-devel-
> bounces at lists.openwrt.org]
> > > On Behalf Of Yousong Zhou
> > > Sent: Dienstag, 20. August 2019 15:52
> > > To: john at phrozen.org
> > > Cc: Yousong Zhou <yszhou4tech at gmail.com>; openwrt-
> > > devel at lists.openwrt.org; paul at northboundnetworks.com
> > > Subject: [OpenWrt-Devel] [PATCH] ramips: add support for Northbound
> > > Networks Zodiac GX
> > >
> > > Hardware spec
> > >
> > >  - MT7621A dual-core 880MHz
> > >  - 16MB Flash
> > >  - 256MB RAM
> > >  - 5 GbE ports
> > >
> > > Vendor device page:
> > > https://northboundnetworks.com/products/zodiac-gx
> > >
> > > Signed-off-by: Yousong Zhou <yszhou4tech at gmail.com>
> > > ---
> > >  .../ramips/base-files/etc/board.d/02_network  |  1 +
> > >  .../dts/mt7621_northbound_zodiac-gx.dts       | 97
> +++++++++++++++++++
> > >  target/linux/ramips/image/mt7621.mk           |  9 ++
> > >  3 files changed, 107 insertions(+)
> > >  create mode 100644
> > > target/linux/ramips/dts/mt7621_northbound_zodiac-
> > > gx.dts
> > >
> > > diff --git a/target/linux/ramips/base-files/etc/board.d/02_network
> > > b/target/linux/ramips/base-files/etc/board.d/02_network
> > > index c0de9d4e50..2e3e5fbba7 100755
> > > --- a/target/linux/ramips/base-files/etc/board.d/02_network
> > > +++ b/target/linux/ramips/base-files/etc/board.d/02_network
> > > @@ -392,6 +392,7 @@ ramips_setup_interfaces()
> > >                       "0:lan" "1:lan" "2:lan" "3:lan" "4:wan" "6 at eth0"
> > >               ;;
> > >       linksys,re6500)
> >
> > should be "|\" instead of ")" here?
> 
> Good catch.
> 
> >
> > > +     northbound,zodiac-gx)
> > >               ucidef_add_switch "switch0" \
> > >                       "0:lan:1" "1:lan:2" "2:lan:3" "3:lan:4" "6 at eth0"
> > >               ;;
> >
> > Above you say "5 ports", these are only four ...?
> 
> Indeed.  Will dig deeper to find better fit.
> 
> The device was designed to be used as a switch.  So I was searching for the
> line containing only "lan"

Ah, I see. With ports relabeling as you do here, you might not find one and have to create your own at the end.

> 
> >
> > > diff --git a/target/linux/ramips/dts/mt7621_northbound_zodiac-gx.dts
> > > b/target/linux/ramips/dts/mt7621_northbound_zodiac-gx.dts
> > > new file mode 100644
> > > index 0000000000..51f2298d06
> > > --- /dev/null
> > > +++ b/target/linux/ramips/dts/mt7621_northbound_zodiac-gx.dts
> > > @@ -0,0 +1,97 @@
> > > +/dts-v1/;
> > > +
> > > +#include "mt7621.dtsi"
> > > +
> > > +#include <dt-bindings/gpio/gpio.h>
> > > +#include <dt-bindings/input/input.h>
> > > +
> > > +/ {
> > > +     compatible = "northbound,zodiac-gx", "mediatek,mt7621-soc";
> > > +     model = "Zodiac GX";
> >
> > Maybe include "Northbound" here, too.
> 
> Maybe I should just use the full name "Northbound Networks".  It's a bit long
> but should be fine.

Indeed, I would just use the same as in DEVICE_TITLE (the concatenation of DEVICE_VENDOR and DEVICE_MODEL from Makefile).

However, I think it is a good idea to remove the "networks" for the compatible (as you did).

> 
> >
> > > +
> > > +     aliases {
> > > +             led-boot = &led_status;
> > > +             led-failsafe = &led_status;
> > > +             led-running = &led_status;
> > > +             led-upgrade = &led_status;
> > > +     };
> > > +
> > > +     chosen {
> > > +             bootargs = "console=ttyS0,57600";
> > > +     };
> > > +
> > > +     leds {
> > > +             compatible = "gpio-leds";
> > > +
> > > +             led_status: status {
> > > +                     label = "zodiac:green:status";
> > > +                     gpios = <&gpio0 15 1>;
> > > +             };
> > > +     };
> > > +
> > > +     gpio-keys-polled {
> > > +             compatible = "gpio-keys-polled";
> > > +             #address-cells = <1>;
> > > +             #size-cells = <0>;
> > > +             poll-interval = <20>;
> >
> > Maybe try "gpio-keys" here, or is there a reason for the polled keys?
> 
> This line I just copied from other dts files.  Will change and test.

Note that you can also drop the line with "poll-interval" then. There should be enough gpio-keys examples around already, just grep for them.

> 
> >
> > > +
> > > +             reset {
> > > +                     label = "reset";
> > > +                     gpios = <&gpio0 18 1>;
> > > +                     linux,code = <KEY_RESTART>;
> > > +             };
> > > +     };
> > > +};
> > > +
> > > +&spi0 {
> > > +     status = "okay";
> > > +
> > > +     m25p80 at 0 {
> > > +             compatible = "jedec,spi-nor";
> > > +             reg = <0>;
> > > +             spi-max-frequency = <10000000>;
> >
> > Maybe try to increase frequency here.
> 
> Ditto.  Can you share some instructions on how to decide a higher value?  Or
> which high values are available and safe to try?

Can't give you anything specific, you might try to find a supposed SPI frequency in a technical datasheet for the device or have a look at "similar" devices with faster flash already (which depends on whether you can derive which device are "similar"...) ;-)

> 
> >
> > > +
> > > +             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;
> > > +                     };
> > > +
> > > +                     partition at 50000 {
> > > +                             compatible = "denx,uimage";
> > > +                             label = "firmware";
> > > +                             reg = <0x50000 0xfb0000>;
> > > +                     };
> > > +             };
> > > +     };
> > > +};
> > > +
> > > +&ethernet {
> > > +     mtd-mac-address = <&factory 0xe000>;
> >
> > If this really is a five port device, it would be nice to check for a WAN MAC
> address in 0xe006 and then read it from flash in 02_network (there should a
> node for that already).
> 
> All ports are supposed to be ordinary switch ports.  No wan/lan distinction by
> default.

I see. Would be nice if you had a look anyway to satisfy my curiosity, I have even seen one-port devices having two addresses recently ...

. /lib/functions.sh
. /lib/functions/system.sh
mtd_get_mac_binary factory 0xe006

> 
> >
> > > +};
> > > +
> > > +&pinctrl {
> > > +     state_default: pinctrl0 {
> > > +             gpio {
> > > +                     ralink,group = "wdt", "rgmii2", "jtag", "mdio";
> > > +                     ralink,function = "gpio";
> > > +             };
> > > +     };
> > > +};
> > > diff --git a/target/linux/ramips/image/mt7621.mk
> > > b/target/linux/ramips/image/mt7621.mk
> > > index d32feb7eab..e52b1eba19 100644
> > > --- a/target/linux/ramips/image/mt7621.mk
> > > +++ b/target/linux/ramips/image/mt7621.mk
> > > @@ -464,6 +464,15 @@ define Device/netis_wf-2881  endef
> > > TARGET_DEVICES += netis_wf-2881
> > >
> > > +define Device/northbound_zodiac-gx
> > > +  MTK_SOC := mt7621
> > > +  IMAGE_SIZE := 16064k
> > > +  DEVICE_VENDOR := Northbound Networks
> > > +  DEVICE_MODEL := Zodiac GX
> > > +  SUPPORTED_DEVICES += zodiac-gx
> >
> > SUPPORTED_DEVICES can be dropped.
> 
> I remembered OEM firmware sysupgrade checks for "zodiac-gx" and erred
> when this line was absent.

Ah, so this for flashing over factory firmware?

My statement was only valid for OpenWrt to OpenWrt sysupgrade. If you actually can exploit the SUPPORTED_DEVICES when flashing from vendor OS, then this might be different.

Best

Adrian

> 
> >
> > Best
> >
> > Adrian
> 
> Thanks for the review, Adrian
> 
> Regards,
>                 yousong
-------------- 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.infradead.org/pipermail/openwrt-devel/attachments/20190820/4ecec51d/attachment.sig>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


More information about the openwrt-devel mailing list