[OpenWrt-Devel] [PATCH 3/3] ath79: add support for Teltonika RUT955

Daniel Golle daniel at makrotopia.org
Tue Feb 4 09:20:33 EST 2020


On Mon, Feb 03, 2020 at 11:51:29PM +0100, Piotr Dymacz wrote:
> Hi Daniel, Adrian,
> 
> See my comments inline.
> 
> On 02.02.2020 17:41, Daniel Golle wrote:
> > The Teltonika RUT955 is an industrial 2G/3G/4G WiFi router with
> > various additional inputs and outputs.
> > 
> > Specification:
> > 
> > - 550/400/200 MHz (CPU/DDR/AHB)
> > - 128 MB of RAM (DDR2)
> > - 16 MB of FLASH (SPI NOR)
> > - 4x 10/100 Mbps Ethernet, with passive PoE support on LAN1
> > - 2T2R 2,4 GHz (AR9344)
> 
> Have you tested RF sensitivity? I remember external GPIO-driven LNA in
> RUT900 has to be defined, otherwise the radio was almost deaf.
> 
> > - built-in 3G module (example: Qutectel EC-25EU)
> 
> Typo: s/Qutectel/Quectel
> 
> I think the only 3G-based one in whole series is RUT900.
> EC25E is a 4G one.
> 
> > - RS232 on D-Sub9 port (Cypress ACM via USB, /dev/ttyACM0)
> > - RS422/RS485 (AR934x high speed UART, /dev/ttyATH1)
> > - analog 0-9V input (MCP3221)
> > - various digital inputs and outputs incl. a relay
> > - 2x miniSIM slot (can be swapped via GPIO)
> > - 2x RP-SMA/F (Wi-Fi), 2x SMA/F (3G), 1x GPS
> > - 2x 74HC595 shift registers providing 16 GPOs
> > - 12x LED (4 are driven by AR9344, 7 by 74HC595)
> > - 1x button (reset)
> > - DC jack for main power input (9-30 V)
> > - debugging UART available on PCB edge connector
> > 
> > Serial console (/dev/ttyS0) pinout:
> > 
> > - RX: pin1 (square) on top side of the main PCB (AR9344 is on top)
> > - TX: pin1 (square) on bottom side
> > 
> > Flash instruction:
> > 
> > Vendor firmware is based on OpenWrt CC release. Use the "factory" image
> > directly in GUI (make sure to uncheck "keep settings") or in U-Boot web
> > based recovery. To avoid any problems, make sure to first update vendor
> > firmware to latest version - "factory" image was successfully tested on
> > device running "RUT9XX_R_00.06.051" firmware and U-Boot "3.0.2".
> > 
> > Signed-off-by: Daniel Golle <daniel at makrotopia.org>
> > ---
> >   target/linux/ath79/dts/ar9344_tlt_rut955.dts  | 301 ++++++++++++++++++
> >   .../generic/base-files/etc/board.d/02_network |   5 +
> >   target/linux/ath79/image/generic.mk           |  37 +++
> >   3 files changed, 343 insertions(+)
> >   create mode 100644 target/linux/ath79/dts/ar9344_tlt_rut955.dts
> > 
> > diff --git a/target/linux/ath79/dts/ar9344_tlt_rut955.dts b/target/linux/ath79/dts/ar9344_tlt_rut955.dts
> > new file mode 100644
> > index 0000000000..06d18f8d26
> > --- /dev/null
> > +++ b/target/linux/ath79/dts/ar9344_tlt_rut955.dts
> > @@ -0,0 +1,301 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> > +/dts-v1/;
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/input/input.h>
> > +
> > +#include "ar9344.dtsi"
> > +
> > +/ {
> > +	model = "Teltonika RUT955";
> > +	compatible = "tlt,rut955", "qca,ar9344";
> 
> I know that Teltonika uses this prefix in their software but maybe as Adrian
> suggested, full name would fit better here? I'm fine with both.
> 
> At least, until it's in [0], we can use whatever fits.
> 
> > +
> > +	aliases {
> > +		serial0 = &uart;
> > +		serial1 = &hs_uart;
> > +		led-boot = &led_system_green;
> > +		led-failsafe = &led_system_red;
> > +		led-running = &led_system_green;
> > +		led-upgrade = &led_system_red;
> > +		label-mac-device = &eth1;
> > +	};
> > +
> > +	i2c {
> > +		compatible = "i2c-gpio";
> > +		scl-gpios = <&gpio 16 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> > +		sda-gpios = <&gpio 17 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		hwmon at 4d {
> > +			compatible = "microchip,mcp3221";
> > +			reg = <0x4d>;
> > +		};
> > +	};
> > +
> > +	gpio_ext_spi {
> > +		compatible = "spi-gpio";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pmx_led_spi_gpio>;
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		sck-gpios = <&gpio 4 GPIO_ACTIVE_HIGH>;     // 74HC595 SRCLK (Serial Clock)
> > +		mosi-gpios = <&gpio 12 GPIO_ACTIVE_HIGH>;   // 74HC595 SER (Serial)
> > +		cs-gpios = <&gpio 20 GPIO_ACTIVE_HIGH>;     // 74HC595 RCLK (Register Clock)
> > +		num-chipselects = <1>;
> > +
> > +		gpio_ext: gpio_ext at 0 {
> > +			compatible = "fairchild,74hc595";
> > +			reg = <0>;
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +			registers-number = <2>;
> > +			spi-max-frequency = <10000000>;
> > +			gpio-line-names = "signal_bar0", "signal_bar1", "signal_bar2", "signal_bar3",
> > +				"signal_bar4", "status_red", "status_green", "sim_sel",
> > +				"", "relay", "modem_vbus", "modem_rst",
> > +				"", "", "", "";
> 
> I don't think we use this property anywhere else in ath79 (or even other
> DTS-based targets?) so far and AFAIK it doesn't have any usage before we
> start using new char dev for GPIOs (libgpiod).

My intention is to avoid deprecated kernel interfaces in favor of
using the 
> 
> Anyway, wouldn't it be more readable to at least add "led" to names of lines
> driving LEDs?

The gpio-line-names is the suggested interface of the kernel to
apply labels to GPIO lines the user can see in /sys/kernel/debug/gpio.
I agree we should use libgpiod or any other way to later on refer to
such label in UCI.

> 
> Also, maybe it would make sense to provide access to SIM select and relay
> lines in user-space (with gpio-export in DTS or in UCI)?

I'll add uci-defaults for gpio_switches as a temporary fix until we
have proper handling allowing to identify GPIO lines by name rather
than a (potentilly non-constant) number.

> 
> > +		};
> > +	};
> > +
> > +	reg_usb_modem_vbus: reg_usb_modem_vbus {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "usb_modem_vbus";
> > +		regulator-min-microvolt = <5000000>;
> > +		regulator-max-microvolt = <5000000>;
> > +		gpio = <&gpio_ext 10 GPIO_ACTIVE_HIGH>;
> > +		enable-active-high;
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +	};
> 
> IIRC, with this approach, user would need to unload USB host driver to be
> able to disable VBUS (and thus 'hard-reset' modem). I would consider using
> "gpio-export" to give user control over that, especially if it controls
> power _only_ for the modem.

I tied the regulator to the corresponding port of the USB hub as that
seemed like the most clean thing to do.

> 
> Also, see (long) discussion about this subject here: [1]

You are right that this limits user control and I've also been involved
in that debate and 
> 
> > +
> > +	leds {
> > +		compatible = "gpio-leds";
> 
> Any reason to use h/w control on WAN/LANs LEDs instead of defining them as
> gpio-leds and thus allow users to have control on them? I suppose we don't
> have a general rule here, though.

It's what the vendor-firmware does as well and as those LEDs are right
next to the corresponding RJ-45 socket and there are plenty of
user-controlled LEDs, I thought I'd just keep it like that.

> 
> > +
> > +		signal0 {
> 
> Could you keep it the same as in RUT900? Count from 1, not 0?

Ack.

> 
> > +			label = "rut955:green:signal1";
> > +			gpios = <&gpio_ext 0 GPIO_ACTIVE_HIGH>;
> > +			default-state = "off";
> > +		};
> > +
> > +		signal1 {
> > +			label = "rut955:green:signal2";
> > +			gpios = <&gpio_ext 1 GPIO_ACTIVE_HIGH>;
> > +			default-state = "off";
> > +		};
> > +
> > +		signal2 {
> > +			label = "rut955:green:signal3";
> > +			gpios = <&gpio_ext 2 GPIO_ACTIVE_HIGH>;
> > +			default-state = "off";
> > +		};
> > +
> > +		signal3 {
> > +			label = "rut955:green:signal4";
> > +			gpios = <&gpio_ext 3 GPIO_ACTIVE_HIGH>;
> > +			default-state = "off";
> > +		};
> > +
> > +		signal4 {
> > +			label = "rut955:green:signal5";
> > +			gpios = <&gpio_ext 4 GPIO_ACTIVE_HIGH>;
> > +			default-state = "off";
> > +		};
> > +
> > +		led_system_red: systemred {
> > +			label = "rut955:green:red";
> > +			gpios = <&gpio_ext 5 GPIO_ACTIVE_HIGH>;
> > +			default-state = "off";
> > +		};
> > +
> > +		led_system_green: systemgreen {
> > +			label = "rut955:green:system";
> > +			gpios = <&gpio_ext 6 GPIO_ACTIVE_HIGH>;
> > +			default-state = "on";
> > +		};
> > +	};
> > +
> > +	keys {
> > +		compatible = "gpio-keys";
> > +
> > +		reset {
> 
> Please, add here also label = "reset". I think this property is optional but
> without it, at least in debugfs (/sys/kernel/debug/gpio), it will get
> meaningless name from parent node's compat string:
> 
> gpio-15  (                    |gpio-keys           ) in  lo IRQ
> 
> > +			linux,code = <KEY_RESTART>;
> > +			gpios = <&gpio 15 GPIO_ACTIVE_HIGH>;
> 
> Are you sure it's active high? In RUT900 it's set to active low: [2].

It was working, but that doesn't mean it was right.
I'll do another round of testing to figure it out.

> 
> > +			debounce-interval = <60>;
> > +		};
> > +	};
> > +};
> > +
> > +&gpio {
> > +	gpio-line-names = "", "wan_led", "input", "mmc_cs",
> > +		"leds_sck", "", "", "",
> > +		"", "", "", "",
> > +		"leds_mosi", "lan2_led", "lan1_led", "",
> > +		"i2c_scl", "i2c_sda", "", "DIN2",
> > +		"spi?", "DIN1", "lan3_led";
> 
> "spi?"?
> 
> I'm wondering if we really need this 'gpio-line-names' now?

They shows up in /sys/kernel/debug/gpio, so it is useful imho.


> 
> > +};
> > +
> > +&ref {
> > +	clock-frequency = <40000000>;
> > +};
> > +
> > +&uart {
> > +	status = "okay";
> > +};
> > +
> > +&hs_uart {
> > +	status = "okay";
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pmx_uart2>;
> > +};
> > +
> > +&spi {
> > +	cs-gpios = <0>, <0>;
> > +	num-cs = <2>;
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&jtag_disable_pins>, <&pmx_spi_cs1>;
> > +
> > +	status = "okay";
> > +
> > +	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 = <0x0 0x20000>;
> > +				read-only;
> > +			};
> > +
> > +			config: partition at 20000 {
> > +				label = "config";
> > +				reg = <0x20000 0x10000>;
> > +				read-only;
> > +			};
> > +
> > +			art: partition at 30000 {
> > +				label = "art";
> > +				reg = <0x30000 0x10000>;
> > +				read-only;
> > +			};
> > +
> > +			partition at 40000 {
> > +				label = "firmware";
> > +				reg = <0x40000 0xf30000>;
> > +				compatible = "tplink,firmware";
> > +			};
> > +
> > +			partition at f70000 {
> > +				label = "event-log";
> > +				reg = <0xf70000 0x80000>;
> 
> As Adrian wrote, this should be 0x90000 (576k, same as in RUT900).

Right, my bad.

> 
> > +			};
> > +		};
> > +	};
> > +
> > +	microsd at 1 {
> > +		compatible = "mmc-spi-slot";
> > +		spi-max-frequency = <25000000>;
> > +		reg = <1>;
> > +		voltage-ranges = <3200 3400>;
> > +		broken-cd;
> > +		status = "disabled";
> > +	};
> > +};
> > +
> > +&usb {
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +	status = "okay";
> > +
> > +	port at 1 {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		reg = <1>;
> > +
> > +		hub_port1: port at 1 { // user USB port
> 
> Maybe a "label" property would be better here instead of a comment?

Ok.

> 
> > +			compatible = "usb-a-connector";
> > +			reg = <1>;
> > +		};
> > +
> > +		hub_port2: port at 2 { // N/C
> > +			reg = <2>;
> > +		};
> 
> I'm not sure if not populated connectors should be mentioned in DTS?
> 
> > +
> > +		hub_port3: port at 3 { // Cypress CDC-ACM serial (RS-232 D-Sub9)
> 
> Label?
> 
> > +			reg = <3>;
> > +		};
> > +
> > +		hub_port4: port at 4 { // Quectel EC-25 modem
> > +			reg = <4>;
> > +			vbus-supply = <&reg_usb_modem_vbus>;
> > +		};
> 
> Also "hub_portX" aren't referenced anywhere, you can drop these labels.
> 
> > +	};
> > +};
> > +
> > +&usb_phy {
> > +	status = "okay";
> > +};
> > +
> > +&wmac {
> > +	status = "okay";
> > +
> > +	mtd-cal-data = <&art 0x1000>;
> > +	mtd-mac-address = <&config 0x0>;
> > +	mtd-mac-address-increment = <2>;
> > +};
> > +
> > +&eth1 {
> 
> Why did you change nodes order here (eth1 before eth0)?
> 
> > +	status = "okay";
> > +
> > +	mtd-mac-address = <&config 0x0>;
> > +
> > +	gmac-config {
> > +		device = <&gmac>;
> > +		switch-phy-swap = <0>;
> > +		switch-only-mode = <1>;
> 
> Have you tested that network configuration? Enabling 'switch-only-mode'
> doesn't make sense as it will make all internal switch PHYs connect to GMAC1
> only.

I copied this from similar (TP-Link) devices, all ports (incl. WAN)
were working. I'll give it a try with those attributes removed to see
if they are actually needed.

> 
> > +	};
> > +};
> > +
> > +&eth0 {
> > +	status = "okay";
> > +
> > +	phy-handle = <&swphy4>;
> > +
> > +	mtd-mac-address = <&config 0x0>;
> > +	mtd-mac-address-increment = <1>;
> > +};
> > +
> > +&builtin_switch {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pmx_leds_switch>;
> > +};
> > +
> > +&pinmux {
> > +	pmx_spi_cs1: pinmux_spi_cs1 {
> > +		pinctrl-single,bits = <0x0 0x07000000 0xff000000>;
> > +	};
> > +
> > +	pmx_led_spi_gpio: pinmux_led_spi_gpio {
> > +		pinctrl-single,bits = <0x4 0x0 0xff>,
> > +					<0xc 0x0 0xff>,
> > +					<0x14 0x0 0xff>;
> > +	};
> > +
> > +	pmx_leds_switch: pinmux_leds_switch {
> > +		pinctrl-single,bits =  <0x0 0x00002d00 0x0000ff00>,
> > +					<0xc 0x002c2b00 0x00ffff00>,
> > +					<0x14 0x002a0000 0x00ff0000>;
> > +	};
> > +
> > +	pmx_uart2: pinmux_uart2 {
> > +		pinctrl-single,bits = <0x10 0x4f000000 0xff000000>,
> > +				<0x3c 0x000b0000 0x00ff0000>;
> 
> I'm wondering, are these pins also configured to output and input in GPIO_OE
> and GPIO_IN registers?

GPIO_OE and GPIO_IN are not range of the pinmux which starts at
0x1804002c. Hence it cannot be set using that pinmux, but it's possible
by gpio-hogs. As the RS485 serial right now still doesn't work
(supposedly for lack-of-clock-signal-reasons) it's hard to test if
anything additional is needed at this point.

> 
> > +	};
> > +};
> > 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 4630cf8447..4b75dc0359 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
> > @@ -210,6 +210,11 @@ ath79_setup_interfaces()
> >   		ucidef_add_switch "switch0" \
> >   			"0 at eth0" "1:lan" "2:lan" "3:wan"
> >   		;;
> > +	tlt,rut955)
> > +		ucidef_set_interface_wan "eth1"
> > +		ucidef_add_switch "switch0" \
> > +			"0 at eth0" "2:lan:3" "3:lan:2" "4:lan:1"
> > +		;;
> >   	tplink,archer-a7-v5|\
> >   	tplink,archer-c6-v2|\
> >   	tplink,archer-c6-v2-us|\
> > diff --git a/target/linux/ath79/image/generic.mk b/target/linux/ath79/image/generic.mk
> > index 1bc7b2d68e..ab11120da8 100644
> > --- a/target/linux/ath79/image/generic.mk
> > +++ b/target/linux/ath79/image/generic.mk
> > @@ -36,6 +36,11 @@ define Build/addpattern
> >   	-mv "$@.new" "$@"
> >   endef
> > +define Build/append-md5sum-bin
> > +	$(STAGING_DIR_HOST)/bin/mkhash md5 $@ | sed 's/../\\\\x&/g' |\
> > +		xargs echo -ne >> $@
> > +endef
> > +
> >   define Build/cybertan-trx
> >   	@echo -n '' > $@-empty.bin
> >   	-$(STAGING_DIR_HOST)/bin/trx -o $@.new \
> > @@ -73,6 +78,17 @@ define Build/pisen_wmb001n-factory
> >     rm -rf "$@.tmp"
> >   endef
> > +define Build/teltonika-fw-fake-checksum
> > +	# Teltonika U-Boot web based firmware upgrade/recovery routine compares
> > +	# 16 bytes from md5sum1[16] field in TP-Link v1 header (offset: 76 bytes
> > +	# from begin of the firmware file) with 16 bytes stored just before
> > +	# 0xdeadc0de marker. Values are only compared, MD5 sum is not verified.
> > +	let \
> > +		offs="$$(stat -c%s $@) - 20"; \
> > +		dd if=$@ bs=1 count=16 skip=76 |\
> > +		dd of=$@ bs=1 count=16 seek=$$offs conv=notrunc
> > +endef
> > +
> >   define Device/seama
> >     KERNEL := kernel-bin | append-dtb | relocate-kernel | lzma
> >     KERNEL_INITRAMFS := $$(KERNEL) | seama
> > @@ -1044,6 +1060,27 @@ define Device/sitecom_wlr-7100
> >   endef
> >   TARGET_DEVICES += sitecom_wlr-7100
> > +define Device/tlt_rut955
> > +  SOC := ar9344
> > +  DEVICE_TITLE := Teltonika RUT955
> > +  DEVICE_PACKAGES := kmod-usb2 kmod-usb-acm  kmod-usb-net-qmi-wwan kmod-usb-serial-option kmod-hwmon-mcp3021 uqmi -uboot-envtools
> > +  IMAGE_SIZE := 15552k
> > +  TPLINK_HWID := 0x35000001
> > +  TPLINK_HWREV := 0x1
> > +  TPLINK_HEADER_VERSION := 1
> > +  KERNEL := kernel-bin | append-dtb | lzma | tplink-v1-header
> > +  KERNEL_INITRAMFS := kernel-bin | append-dtb | lzma | uImage lzma
> > +  IMAGES := sysupgrade.bin factory.bin
> > +  IMAGE/factory.bin := append-kernel | pad-to $$$$(BLOCKSIZE) | append-rootfs |\
> > +	pad-rootfs | teltonika-fw-fake-checksum | append-string master |\
> > +	append-md5sum-bin | check-size $$$$(IMAGE_SIZE)
> > +  IMAGE/sysupgrade.bin := append-kernel | pad-to $$$$(BLOCKSIZE) |\
> > +	append-rootfs | pad-rootfs | append-metadata |\
> > +	check-size $$$$(IMAGE_SIZE)
> > +  SUPPORTED_DEVICES += rut900
> 
> Please, drop this line.
> I will add support for RUT900 with a separate patch.

Ack.

> 
> > +endef
> > +TARGET_DEVICES += tlt_rut955
> > +
> >   define Device/trendnet_tew-823dru
> >     SOC := qca9558
> >     DEVICE_VENDOR := Trendnet
> > 
> 
> [0] https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/vendor-prefixes.yaml
> 
> [1]
> https://www.mail-archive.com/openwrt-devel@lists.openwrt.org/msg49220.html
> 
> [2] https://github.com/openwrt/openwrt/blob/master/target/linux/ar71xx/files/arch/mips/ath79/mach-rut9xx.c#L111
> 
> -- 
> Cheers,
> Piotr
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

_______________________________________________
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