Hi, just some comments on your comments ;-)
> -----Original Message----- > From: Yousong Zhou [mailto:[email protected]] > Sent: Dienstag, 20. August 2019 17:58 > To: [email protected] > Cc: John Crispin <[email protected]>; OpenWrt Development List > <[email protected]>; Paul Zanna > <[email protected]> > Subject: Re: [OpenWrt-Devel] [PATCH] ramips: add support for Northbound > Networks Zodiac GX > > On Tue, 20 Aug 2019 at 23:38, <[email protected]> wrote: > > > > Hi, > > > > > -----Original Message----- > > > From: openwrt-devel [mailto:openwrt-devel- > [email protected]] > > > On Behalf Of Yousong Zhou > > > Sent: Dienstag, 20. August 2019 15:52 > > > To: [email protected] > > > Cc: Yousong Zhou <[email protected]>; openwrt- > > > [email protected]; [email protected] > > > 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 <[email protected]> > > > --- > > > .../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@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@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@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@0 { > > > + label = "u-boot"; > > > + reg = <0x0 0x30000>; > > > + read-only; > > > + }; > > > + > > > + partition@30000 { > > > + label = "u-boot-env"; > > > + reg = <0x30000 0x10000>; > > > + read-only; > > > + }; > > > + > > > + factory: partition@40000 { > > > + label = "factory"; > > > + reg = <0x40000 0x10000>; > > > + read-only; > > > + }; > > > + > > > + partition@50000 { > > > + compatible = "denx,uimage"; > > > + label = "firmware"; > > > + reg = <0x50000 0xfb0000>; > > > + }; > > > + }; > > > + }; > > > +}; > > > + > > > +ðernet { > > > + 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
openpgp-digital-signature.asc
Description: PGP signature
_______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
