[PATCH 1/2] ath79: add support for Mojo Networks AirTight C-75
Adrian Schmutzler
mail at adrianschmutzler.de
Mon Dec 14 11:28:31 EST 2020
Hi,
only have time for a quick subset of answers:
> -----Original Message-----
> From: Tomasz Maciej Nowak [mailto:tmn505 at gmail.com]
> Sent: Dienstag, 8. Dezember 2020 17:53
> To: Adrian Schmutzler <mail at adrianschmutzler.de>; openwrt-
> devel at lists.openwrt.org
> Cc: 'Vladimir Georgievsky' <vladimir.georgievsky at yahoo.com>
> Subject: Re: [PATCH 1/2] ath79: add support for Mojo Networks AirTight C-75
>
> Hi.
>
> W dniu 01.12.2020 o 20:47, Adrian Schmutzler pisze:
> > Hi,
> >
> >> -----Original Message-----
> >> From: openwrt-devel [mailto:openwrt-devel-
> bounces at lists.openwrt.org]
> >> On Behalf Of Tomasz Maciej Nowak
> >> Sent: Dienstag, 1. Dezember 2020 18:36
> >> To: openwrt-devel at lists.openwrt.org
> >> Cc: Vladimir Georgievsky <vladimir.georgievsky at yahoo.com>
> >> Subject: [PATCH 1/2] ath79: add support for Mojo Networks AirTight
> >> C-75
> >>
> >> Mojo Networks AirTight C-75 is a dual-band access point, also sold by
> >> WatchGuard under name AP320.
> >>
> >> Specification
> >> SoC: Qualcomm Atheros QCA9550
> >> RAM: 128 MiB DDR2
> >> Flash: 2x 16 MiB SPI NOR
> >> WIFI: 2.4 GHz 3T3R integrated
> >> 5 GHz 3T3R QCA9890 oversized Mini PCIe card
> >> Ethernet: 2x 10/100/1000 Mbps QCA8334
> >> port labeled LAN1 is PoE capable (802.3at)
> >> USB: 1x 2.0
> >> LEDs: 7x which two are GPIO controlled, four switch controlled, one
> >> controlled by wireless driver
> >> Buttons: 1x GPIO controlled
> >> Serial: RJ-45 port, Cisco pinout
> >> baud: 115200, parity: none, flow control: none
> >> JTAG: Yes, pins marked J1 on PCB
> >>
> >> Installation
> >> 1. Prepare TFTP server with OpenWrt initramfs-kernel image.
> >> 2. Connect to one of LAN ports.
> >> 3. Connect to serial port.
> >> 4. Power on the device and when prompted to stop autoboot, hit any
> key.
> >> 5. Adjust "ipaddr" and "serverip" addresses in U-Boot environment, use
> >> 'setenv' to do that, then run following commands:
> >> tftpboot 0x81000000 <openwrt_initramfs-kernel_image_name>
> >> bootm 0x81000000
> >> 6. Wait about 1 minute for OpenWrt to boot.
> >> 7. Transfer OpenWrt sysupgrade image to /tmp directory and flash it
> >> with:
> >> sysupgrade -n /tmp/<openwrt_sysupgrade_image_name>
> >> 8. After flashing, the access point will reboot to OpenWrt. Wait few
> >> minutes, until the Power LED stops blinking, then it's ready for
> >> configuration.
> >>
> >> Known issues
> >> Green power LED does not work.
> >>
> >> Additional information
> >> The U-Boot fails to initialise ethernet ports correctly when a UART
> >> adapter is attached to UART pins (marked J3 on PCB).
> >>
> >> Cc: Vladimir Georgievsky <vladimir.georgievsky at yahoo.com>
> >> Signed-of-by: Tomasz Maciej Nowak <tmn505 at gmail.com>
> >> ---
> >> package/boot/uboot-envtools/files/ath79 | 1 +
> >> target/linux/ath79/dts/qca9550_mojo_c-75.dts | 173
> >> ++++++++++++++++++
> >> .../generic/base-files/etc/board.d/02_network | 4 +
> >> target/linux/ath79/image/generic.mk | 11 ++
> >> 4 files changed, 189 insertions(+)
> >> create mode 100644 target/linux/ath79/dts/qca9550_mojo_c-75.dts
> >>
> >> diff --git a/package/boot/uboot-envtools/files/ath79
> >> b/package/boot/uboot-envtools/files/ath79
> >> index 259c52c77034..a6b52b370530 100644
> >> --- a/package/boot/uboot-envtools/files/ath79
> >> +++ b/package/boot/uboot-envtools/files/ath79
> >> @@ -20,6 +20,7 @@ alfa-network,n5q|\
> >> alfa-network,pi-wifi4|\
> >> alfa-network,r36a|\
> >> allnet,all-wap02860ac|\
> >> +airtight,c-75|\
> >
> > This is inconsistent with the other names. See more detailed comment
> below.
>
> Oops, that chunk should have been removed. We don't alter U-Boot
> environment on OpenWrt installation, so users don't need access to that.
>
> >
> >> arduino,yun|\
> >> buffalo,bhr-4grv2|\
> >> devolo,magic-2-wifi|\
> >> diff --git a/target/linux/ath79/dts/qca9550_mojo_c-75.dts
> >> b/target/linux/ath79/dts/qca9550_mojo_c-75.dts
> >> new file mode 100644
> >> index 000000000000..51046a740a02
> >> --- /dev/null
> >> +++ b/target/linux/ath79/dts/qca9550_mojo_c-75.dts
> >> @@ -0,0 +1,173 @@
> >> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> >> +
> >> +#include <dt-bindings/gpio/gpio.h>
> >> +#include <dt-bindings/input/input.h>
> >> +
> >> +#include "qca955x.dtsi"
> >
> > This include needs to be first (after the license).
>
> I followed Linux kernel style in which first we add global includes in
> alphabetical order then local includes. Will change that.
This is just because dts-v1 is introduced from ath79.dtsi via qca955x.dtsi and dts-v1 has to be before anything else. This is also done like this in kernel (but not consistently), e.g. ipq4019/ipq806x.
>
> >
> >> +
> >> +/ {
> >> + model = "Mojo Networks AirTight C-75";
> >> + compatible = "mojo,c-75", "qca,qca9550", "qca,qca9558";
> >
> > Please decide for one name and then use that consistently. It appears to
> me that both Mojo Networks and Airtight Networks are different _vendor_
> names (due to a rename)?
>
> The FCC filling got me confused: Requested by: Mojo Networks, Inc.; For:
> AirTight Access Point C75. Which I thought would mean AirTight is an Access
> Point class from their portfolio. But yeah, the company just renamed itself.
>
> >
> > Then please decide for one and use that one consistently, i.e.
> > either
> > model = "Mojo Networks C-75";
> > compatible = "mojo,c-75", "qca,qca9550", "qca,qca9558"; or
> > model = "Airtight Networks C-75";
> > compatible = "airtight,c-75", "qca,qca9550", "qca,qca9558";
>
> Will change that to AirTight and add DEVICE_ALT variables.
>
> >
> >> +
> >> + aliases {
> >> + label-mac-device = ð0;
> >> + led-boot = &led_power;
> >> + led-failsafe = &led_power;
> >> + led-upgrade = &led_power;
> >
> > No led-running?
>
> The power LED is by default on, so it makes led-running redundant. Is there
> something else also using that alias?
default-on is first, then diag.sh starts doing stuff due to led-boot, i.e. will have the LED blinking. I'm not sure what happens after that without led-running, i.e. is the LED turned off or on. Anyway, that will be the job of diag.sh then, not of default-on anymore.
>
> >
> >> + };
> >> +
> >> + keys {
> >> + compatible = "gpio-keys";
> >> +
> >> + reset {
> >> + linux,code = <KEY_RESTART>;
> >> + gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
> >
> > missing label?
>
> The label is set from the node name by default.
Hmm, so essentially we could drop label for about 50 % of our keys (in contrast to the more complex situation with LEDs)?
>
> >
> >> + };
> >> + };
> >> +
> >> + leds {
> >> + compatible = "gpio-leds";
> >> +
> >> + led_power: power {
> >> + label = "amber:power";
> >> + gpios = <&gpio 14 GPIO_ACTIVE_HIGH>;
> >> + default-state = "on";
> >> + };
> >> +
> >> + wlan2g {
> >> + label = "green:wlan2g";
> >> + gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
> >> + linux,default-trigger = "phy1tpt";
> >> + };
> >> + };
> >> +};
> >> +
> >> +ð0 {
> >> + status = "okay";
> >> +
> >> + mtd-mac-address = <&art 0x0>;
> >> + phy-handle = <&phy0>;
> >> + phy-mode = "rgmii";
> >> + pll-data = <0xa6000000 0x00000101 0x00001616>; };
> >> +
> >> +ð1 {
> >> + status = "okay";
> >> +
> >> + mtd-mac-address = <&art 0x6>;
> >> + phy-mode = "sgmii";
> >> + pll-data = <0x03000101 0x00000101 0x00001616>;
> >> +
> >> + fixed-link {
> >> + speed = <1000>;
> >> + full-duplex;
> >> + };
> >> +};
> >> +
> >> +&mdio0 {
> >> + status = "okay";
> >> +
> >> + phy0: ethernet-phy at 0 {
> >> + reg = <0>;
> >> + qca,ar8327-initvals = <
> >> + 0x0c 0x00080080
> >> + 0x04 0x07600000
> >> + 0x58 0xc935c935
> >> + 0x5c 0x03ffff00
> >> + 0x7c 0x0000007e
> >> + 0x94 0x0000007e
> >> + >;
> >> + };
> >> +};
> >
> > Please move mdio0 up so it's either directly before or after eth0.
>
> The nodes are sorted alphabetically. There are no includes specified
> between nodes, which would make following applied sorting not possible.
> What are the criteria saying that this node should be above ð nodes?
Well, mdioX is a subnode of ethX. Thus, I tend to see them as related, and that's probably also the reason why they are typically grouped together in the ar****/qca**** DTSI files.
It just makes reading the config easier for me when they are grouped this way. I won't force you to change that if you think alphabetic sorting is superior.
>
> >
> >> +
> >> +&pcie0 {
> >> + status = "okay";
> >> +
> >> + wifi at 0,0 {
> >> + compatible = "qcom,ath10k";
> >> + reg = <0x0000 0 0 0 0>;
> >> + };
> >> +};
> >> +
> >> +&spi {
> >> + status = "okay";
> >> +
> >> + num-cs = <2>;
> >
> > num-cs can be dropped for ath79.
>
> Ok.
>
> >
> >> +
> >> + flash at 0 {
> >> + compatible = "jedec,spi-nor";
> >> + reg = <0>;
> >> + spi-max-frequency = <25000000>;
> >> +
> >> + partitions {
> >> + compatible = "fixed-partitions";
> >> + #address-cells = <1>;
> >> + #size-cells = <1>;
> >> +
> >> + partition at 0 {
> >> + label = "u-boot";
> >> + reg = <0x000000 0x040000>;
> >> + read-only;
> >> + };
> >> +
> >> + partition at 40000 {
> >> + label = "u-boot-env";
> >> + reg = <0x040000 0x010000>;
> >> + };
> >> +
> >> + partition at 50000 {
> >> + label = "wlandrv";
> >> + reg = <0x050000 0x010000>;
> >> + read-only;
> >> + };
> >> +
> >> + partition at 60000 {
> >> + label = "firmware";
> >> + reg = <0x060000 0xf90000>;
> >> + compatible = "denx,uimage";
> >> + };
> >> +
> >> + art: partition at ff0000 {
> >> + label = "art";
> >> + reg = <0xff0000 0x010000>;
> >> + read-only;
> >> + };
> >> + };
> >> + };
> >> +
> >> + flash at 1 {
> >> + compatible = "jedec,spi-nor";
> >> + reg = <1>;
> >> + spi-max-frequency = <25000000>;
> >> +
> >> + partitions {
> >> + compatible = "fixed-partitions";
> >> + #address-cells = <1>;
> >> + #size-cells = <1>;
> >> +
> >> + partition at 0 {
> >> + label = "opt";
> >> + reg = <0x0 0x1000000>;
> >
> > read-only? or is that intended for user data?
>
> This stores the applications from OEM and their settings which are mounted
> as /opt. There is little value in keeping this if the OEM firmware on first flash
> is overwritten, so users are free to re-purpose it. Also originally this partition
> won't be mountable since it uses JFFS2 with ZLIB compression, so only full
> overwrite can take place.
>
> >
> >> + };
> >> + };
> >> + };
> >> +};
> >> +
> >> +&uart {
> >> + status = "okay";
> >> +};
> >> +
> >> +&usb_phy0 {
> >> + status = "okay";
> >> +};
> >> +
> >> +&usb0 {
> >> + status = "okay";
> >> +};
> >> +
> >> +&wmac {
> >> + status = "okay";
> >> +
> >> + mtd-cal-data = <&art 0x1000>;
> >> + mtd-mac-address = <&art 0x1002>;
> >
> > mtd-mac-address can be dropped, this will get read from caldata
> automatically.
>
> Ok.
>
> >
> >> +};
> >> diff --git
> >> a/target/linux/ath79/generic/base-files/etc/board.d/02_network
> >> b/target/linux/ath79/generic/base-files/etc/board.d/02_network
> >> index 5c0195f6ffc9..02e4d6b78dd4 100755
> >> --- a/target/linux/ath79/generic/base-files/etc/board.d/02_network
> >> +++ b/target/linux/ath79/generic/base-files/etc/board.d/02_network
> >> @@ -248,6 +248,10 @@ ath79_setup_interfaces()
> >> ucidef_add_switch "switch0" \
> >> "0 at eth0" "2:lan:1" "3:lan:2" "4:lan:3" "5:lan:4"
> >> "1:wan"
> >> ;;
> >> + mojo,c-75)
> >> + ucidef_add_switch "switch0" \
> >> + "0 at eth0" "2:wan" "3:lan" "6 at eth1"
> >> + ;;
> >
> > Interesting setup. Is this necessary or can one use
> > ucidef_set_interfaces_lan_wan "eth1" "eth0"
> > instead? (Have to think about that myself ...)
>
> Traffic needs to be properly tagged inside the switch, otherwise it will go
> between eth0 and eth1, and won't go to proper ports (2 and 3).
Okay, accepted.
Best
Adrian
>
> >
> > Best
> >
> > Adrian
>
> Regards.
>
> --
> TMN
-------------- 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/20201214/d24be46a/attachment.sig>
More information about the openwrt-devel
mailing list