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

[Chuanhong Guo]

Hi!

some minor comments below:
On Thu, Feb 6, 2020 at 1:07 AM Daniel Golle <daniel at makrotopia.org> wrote:
...
> +
> +       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 = "led_signal_bar1", "led_signal_bar2", "led_signal_bar3", "led_signal_bar4",
> +                               "led_signal_bar5", "led_status_red", "led_status_green", "sim_sel",
> +                               "DOUT1", "DOUT2", "modem_vbus", "modem_rst",
> +                               "DOUT3", "RS485_R", "SDCS", "HWRST";
> +               };
> +       };

Would you mind to test whether it's possible to move this 74hc595
under hardware spi instead? I believe using hw spi has a tiny bit less
cpu overhead than gpio spi.
It's possible to hook one output signal to multiple gpio pins
simultaneously on ath79. Try setting gpio 4/12/20 to clk/mosi/cs2 in
gpio function register respectively, move gpio_ext node under spi and
change reg to 2.

> +
> +       leds {
> +               compatible = "gpio-leds";
> +
> +               signal0 {
> +                       label = "rut955:green:signal1";
> +                       gpios = <&gpio_ext 0 GPIO_ACTIVE_HIGH>;
> +               };
...
> +&spi {
> +       status = "okay";
> +
> +       cs-gpios = <0>, <0>;

I pushed a spi driver replacement yesterday and we don't need this
cs-gpios to fix num_chipselect in driver now :)

> +       num-cs = <2>;
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&jtag_disable_pins>, <&pmx_spi_cs1>;
> +
...
> +&eth1 {
> +       status = "okay";
> +
> +       mtd-mac-address = <&config 0x0>;
> +
> +       gmac-config {
> +               device = <&gmac>;
> +       };

This gmac-config can be removed if you don't need it.

> +};
> +
> +&builtin_switch {
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pmx_leds_switch>;
> +};
...
> +  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) |\

Do we need this "pad-to $$$$(BLOCKSIZE)" in
factory.bin/sysupgrade.bin? kernel/rootfs can be split anywhere and we
may squeeze out several kilobytes by removing this image padding.
Also I'm wondering whether "tplink-v1-image sysupgrade" recipe could
replace "append-kernel | append-rootfs | pad-rootfs" here.
("tplink-v1-header" needs to be removed from KERNEL or there'll be
duplicated headers)

> +       append-rootfs | pad-rootfs | append-metadata |\
> +       check-size $$$$(IMAGE_SIZE)
> +endef
> +TARGET_DEVICES += teltonika_rut955
> +
>  define Device/trendnet_tew-823dru
>    SOC := qca9558
>    DEVICE_VENDOR := Trendnet
> --

Regards,
Chuanhong Guo

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