[PATCH 1/1] TARGET: Add new device support under target IPQ806x for Linksys E8350
mail at adrianschmutzler.de
mail at adrianschmutzler.de
Sun Jul 19 16:02:43 EDT 2020
Hi,
> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
> On Behalf Of Todor Colov
> Sent: Sonntag, 19. Juli 2020 11:05
> To: openwrt-devel at lists.openwrt.org
> Cc: Todor Colov <todorcolov at abv.bg>
> Subject: [PATCH 1/1] TARGET: Add new device support under target IPQ806x
> for Linksys E8350
>
> Signed-off-by: Todor Colov <todorcolov at abv.bg>
this patch has several formal issues, please have another look at https://openwrt.org/submitting-patches
What I see right away:
- commit title prefix
- title too long (e.g. just use "ipq806x: add support for Linksys E8350")
- missing commit description (Specifications, Flashing instructions, MAC address assignment, further notes)
> ---
> .../ipq806x/base-files/etc/board.d/01_leds | 3 +
> .../ipq806x/base-files/etc/board.d/02_network | 1 +
> .../ipq806x/base-files/etc/rc.button/reset | 44 +++
> .../ipq806x/base-files/lib/upgrade/fwtool.sh | 67 +++++
> .../base-files/lib/upgrade/platform.sh | 5 +
> .../arch/arm/boot/dts/qcom-ipq8064-e8350.dts | 265
> ++++++++++++++++++
> target/linux/ipq806x/image/Makefile | 31 ++
> .../0069-arm-boot-add-dts-files.patch | 3 +-
> 8 files changed, 418 insertions(+), 1 deletion(-) create mode 100644
> target/linux/ipq806x/base-files/etc/rc.button/reset
> create mode 100644 target/linux/ipq806x/base-files/lib/upgrade/fwtool.sh
> create mode 100644 target/linux/ipq806x/files-
> 5.4/arch/arm/boot/dts/qcom-ipq8064-e8350.dts
>
> diff --git a/target/linux/ipq806x/base-files/etc/board.d/01_leds
> b/target/linux/ipq806x/base-files/etc/board.d/01_leds
> index f8b6c32358..892dab88c8 100755
> --- a/target/linux/ipq806x/base-files/etc/board.d/01_leds
> +++ b/target/linux/ipq806x/base-files/etc/board.d/01_leds
> @@ -11,6 +11,9 @@ board=$(board_name)
> boardname="${board##*,}"
>
> case "$board" in
> +linksys,e8350)
Please take care of alphabetic sorting. Check whether this can be merged with other cases (can't see this from the mere patch).
> + ucidef_set_led_wlan "wlan" "WLAN" "${boardname}:green:wifi"
> "phy0tpt"
> + ;;
> buffalo,wxr-2533dhp)
> ucidef_set_led_wlan "wlan" "WLAN" "${boardname}:white:wireless"
> "phy0tpt"
> ucidef_set_led_switch "wan" "WAN" "${boardname}:white:internet"
> "switch0" "0x20"
> diff --git a/target/linux/ipq806x/base-files/etc/board.d/02_network
> b/target/linux/ipq806x/base-files/etc/board.d/02_network
> index 529a8d9f39..60c68da020 100755
> --- a/target/linux/ipq806x/base-files/etc/board.d/02_network
> +++ b/target/linux/ipq806x/base-files/etc/board.d/02_network
> @@ -18,6 +18,7 @@ netgear,d7800 |\
> netgear,r7500 |\
> netgear,r7500v2 |\
> qcom,ipq8064-ap148 |\
> +linksys,e8350 |\
sorting.
> tplink,vr2600v)
> ucidef_add_switch "switch0" \
> "1:lan" "2:lan" "3:lan" "4:lan" "6 at eth1" "5:wan" "0 at eth0"
> diff --git a/target/linux/ipq806x/base-files/etc/rc.button/reset
> b/target/linux/ipq806x/base-files/etc/rc.button/reset
> new file mode 100644
> index 0000000000..e6bdcb4cdc
> --- /dev/null
> +++ b/target/linux/ipq806x/base-files/etc/rc.button/reset
What's that for? Implementation of reset button is available with KEY_RESTART OOTB.
> @@ -0,0 +1,44 @@
> +#!/bin/sh
> +
> +. /lib/functions.sh
> +. /lib/functions/uci-defaults.sh
> +. /lib/functions/system.sh
> +
> +board_config_update
> +board=$(board_name)
> +
> +
> +OVERLAY="$( grep ' /overlay ' /proc/mounts )"
> +
> +case "$ACTION" in
> +pressed)
> + [ -z "$OVERLAY" ] && return 0
> +
> + return 5
> +;;
> +timeout)
> + . /etc/diag.sh
> + set_state failsafe
> +;;
> +released)
> + if [ "$SEEN" -lt 1 ]
> + then
> + echo "REBOOT" > /dev/console
> + sync
> + reboot
> + elif [ "$SEEN" -ge 5 -a -n "$OVERLAY" ]
> + then
> + echo "FACTORY RESET" > /dev/console
> + case "$board" in
> + linksys,e8350)
> + rm -f /etc/config/* ; cp -a /rom/etc/* /etc/. ;
> sync ; reboot
> + ;;
> + *)
> + jffs2reset -y && reboot &
> + ;;
> + esac
> + fi
> +;;
> +esac
> +
> +return 0
> diff --git a/target/linux/ipq806x/base-files/lib/upgrade/fwtool.sh
> b/target/linux/ipq806x/base-files/lib/upgrade/fwtool.sh
> new file mode 100644
> index 0000000000..6929518204
> --- /dev/null
> +++ b/target/linux/ipq806x/base-files/lib/upgrade/fwtool.sh
Same here. Why do you copy that stuff over?
> @@ -0,0 +1,67 @@
> +fwtool_check_signature() {
> + [ $# -gt 1 ] && return 1
> +
> + [ ! -x /usr/bin/ucert ] && {
> + if [ "$REQUIRE_IMAGE_SIGNATURE" = 1 ]; then
> + return 1
> + else
> + return 0
> + fi
> + }
> +
> + if ! fwtool -q -s /tmp/sysupgrade.ucert "$1"; then
> + echo "Image signature not found"
> + [ "$REQUIRE_IMAGE_SIGNATURE" = 1 -a "$FORCE" != 1 ] && {
> + echo "Use sysupgrade -F to override this check when
> downgrading or flashing to vendor firmware"
> + }
> + [ "$REQUIRE_IMAGE_SIGNATURE" = 1 ] && return 1
> + return 0
> + fi
> +
> + fwtool -q -T -s /dev/null "$1" | \
> + ucert -V -m - -c "/tmp/sysupgrade.ucert" -P /etc/opkg/keys
> +
> + return $?
> +}
> +
> +fwtool_check_image() {
> + [ $# -gt 1 ] && return 1
> +
> + . /usr/share/libubox/jshn.sh
> +
> + if ! fwtool -q -i /tmp/sysupgrade.meta "$1"; then
> + echo "Image metadata not found"
> + [ "$REQUIRE_IMAGE_METADATA" = 1 -a "$FORCE" != 1 ] && {
> + echo "Use sysupgrade -F to override this check when
> downgrading or flashing to vendor firmware"
> + }
> + [ "$REQUIRE_IMAGE_METADATA" = 1 ] && return 1
> + return 0
> + fi
> + # For UBI volumes we need fwtool on later stage to remove the
> metadata - via platform.sh!!!
> + cp /usr/bin/fwtool /tmp/.
> +
> + json_load "$(cat /tmp/sysupgrade.meta)" || {
> + echo "Invalid image metadata"
> + return 1
> + }
> +
> + device="$(cat /tmp/sysinfo/board_name)"
> +
> + json_select supported_devices || return 1
> +
> + json_get_keys dev_keys
> + for k in $dev_keys; do
> + json_get_var dev "$k"
> + [ "$dev" = "$device" ] && return 0
> + done
> +
> + echo "Device $device not supported by this image"
> + echo -n "Supported devices:"
> + for k in $dev_keys; do
> + json_get_var dev "$k"
> + echo -n " $dev"
> + done
> + echo
> +
> + return 1
> +}
> diff --git a/target/linux/ipq806x/base-files/lib/upgrade/platform.sh
> b/target/linux/ipq806x/base-files/lib/upgrade/platform.sh
> index 560e64af3a..50a7ade058 100644
> --- a/target/linux/ipq806x/base-files/lib/upgrade/platform.sh
> +++ b/target/linux/ipq806x/base-files/lib/upgrade/platform.sh
> @@ -27,6 +27,11 @@ platform_do_upgrade() {
> zyxel,nbg6817)
> zyxel_do_upgrade "$1"
> ;;
> + linksys,e8350)
> + # first remove metadata trailer from the UBI volume
> + /tmp/fwtool -q -t -i /dev/null "$1"
This looks strange.
> + nand_do_upgrade "$1"
> + ;;
> linksys,ea7500-v1 |\
> linksys,ea8500)
> platform_do_upgrade_linksys "$1"
> diff --git a/target/linux/ipq806x/files-5.4/arch/arm/boot/dts/qcom-ipq8064-
> e8350.dts b/target/linux/ipq806x/files-5.4/arch/arm/boot/dts/qcom-
> ipq8064-e8350.dts
> new file mode 100644
> index 0000000000..0af1d39880
> --- /dev/null
> +++ b/target/linux/ipq806x/files-5.4/arch/arm/boot/dts/qcom-ipq8064-e835
> +++ 0.dts
> @@ -0,0 +1,265 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT #include
> +"qcom-ipq8064-v2.0.dtsi"
> +
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/soc/qcom,tcsr.h>
> +
> +/ {
> + model = "Linksys EA8300 V1 WiFi Router";
So, is it V1? Then add that to the compatible etc.: linksys,e8350-v1
> + compatible = "linksys,e8350", "qcom,ipq8064";
> +
> + memory at 0 {
> + reg = <0x42000000 0x1e000000>;
> + device_type = "memory";
> + };
> +
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + rsvd at 41200000 {
> + reg = <0x41200000 0x300000>;
> + no-map;
> + };
> + };
> +
> + aliases {
> + serial0 = &gsbi4_serial;
This line can be dropped, it has been moved to the DTSI recently.
> +
> + led-boot = &led_power;
> + led-failsafe = &led_power;
> + led-running = &led_power;
> + led-upgrade = &led_power;
> + };
> +
> + keys {
> + compatible = "gpio-keys";
> + pinctrl-0 = <&button_pins>;
> + pinctrl-names = "default";
> +
> + reset {
Your indent is broken/mixed, same goes for empty lines between nodes.
DTS should have tab indent only, and nodes should be separated by empty lines.
> + label = "reset";
> + gpios = <&qcom_pinmux 68 GPIO_ACTIVE_LOW>;
> + linux,code = <KEY_RESTART>;
> + };
> +
> + wps {
> + label = "wps";
> + gpios = <&qcom_pinmux 65 GPIO_ACTIVE_LOW>;
> + linux,code = <KEY_WPS_BUTTON>;
> + };
> + wifi {
> + label = "wifi";
> + gpios = <&qcom_pinmux 67 GPIO_ACTIVE_LOW>;
> + linux,code = <KEY_RFKILL>;
> + };
> + };
> +
> + nand-controller at 1ac00000 {
> + compatible = "qcom,ipq806x-nand";
> + reg = <0x1ac00000 0x800>;
> +
> + clocks = <&gcc EBI2_CLK>,
> + <&gcc EBI2_AON_CLK>;
> + clock-names = "core", "aon";
> +
> + dmas = <&adm_dma 3>;
> + dma-names = "rxtx";
> + qcom,cmd-crci = <15>;
> + qcom,data-crci = <3>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + nand at 0 {
> + reg = <0>;
> +
> + nand-ecc-strength = <4>;
> + nand-bus-width = <8>;
> +
> + partitions {
> + compatible = "fixed-partitions";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + partition at 0 {
> + label = "ubi";
> + reg = <0 0x4000000>;
> + };
> + partition at 4000000 {
> + label = "extra";
> + reg = <0x4000000 0x4000000>;
> + };
> + };
> + };
> + };
> +
> +
> + leds {
> + compatible = "gpio-leds";
> + pinctrl-0 = <&led_pins>;
> + pinctrl-names = "default";
> +
> + led_power: power {
> + label = "e8350:green:power";
> + gpios = <&qcom_pinmux 26 GPIO_ACTIVE_HIGH>;
> + default-state = "keep";
> + };
> +
> + wps {
> + label = "e8350:green:wps";
> + gpios = <&qcom_pinmux 53 GPIO_ACTIVE_HIGH>;
> + };
> +
> + wifi {
> + label = "e8350:green:wifi";
> + gpios = <&qcom_pinmux 54 GPIO_ACTIVE_HIGH>;
> + };
> +
drop this empty line.
> + };
> +};
> +
> +&qcom_pinmux {
> + button_pins: button_pins {
> + mux {
> + pins = "gpio68","gpio65", "gpio67";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> + };
> +
> + led_pins: led_pins {
> + mux {
> + pins = "gpio26","gpio53", "gpio54";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> + };
> +};
> +
> +&gsbi4 {
> + qcom,mode = <GSBI_PROT_I2C_UART>;
> + status = "okay";
> + serial at 16340000 {
> + status = "okay";
> + };
> + /*
> + * The i2c device on gsbi4 should not be enabled.
> + * On ipq806x designs gsbi4 i2c is meant for exclusive
> + * RPM usage. Turning this on in kernel manifests as
> + * i2c failure for the RPM.
> + */
> +};
> +&gsbi5 {
> + qcom,mode = <GSBI_PROT_SPI>;
> + status = "okay";
> +
> + spi5: spi at 1a280000 {
> + status = "okay";
> +
> + pinctrl-0 = <&spi_pins>;
> + pinctrl-names = "default";
> +
> + cs-gpios = <&qcom_pinmux 20 GPIO_ACTIVE_HIGH>;
> +
> + m25p80 at 0 {
> + compatible = "jedec,spi-nor";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + spi-max-frequency = <51200000>;
> + reg = <0>;
> +
> + partitions {
> + compatible = "qcom,smem";
> + };
> + };
> + };
> +};
> +
> +&sata_phy {
> + status = "okay";
> +};
> +
> +&sata {
> + status = "okay";
> +};
> +
> +&usb3_0 {
> + clocks = <&gcc USB30_1_MASTER_CLK>;
> + status = "okay";
> +};
> +
> +&usb3_1 {
> + clocks = <&gcc USB30_0_MASTER_CLK>;
> + status = "okay";
> +};
> +
> +&pcie0 {
> + status = "okay";
> + force_gen1 = <1>;
> +};
> +
> +&pcie1 {
> + status = "okay";
> +};
> +
> +&pcie2 {
> + status = "okay";
> +};
> +
> +&mdio0 {
> + status = "okay";
> +
> + pinctrl-0 = <&mdio0_pins>;
> + pinctrl-names = "default";
> +
> + phy0: ethernet-phy at 0 {
> + reg = <0>;
> + qca,ar8327-initvals = <
> + 0x00004 0x7600000 /* PAD0_MODE */
> + 0x00008 0x1000000 /* PAD5_MODE */
> + 0x0000c 0x80 /* PAD6_MODE */
> + 0x00010 0x2613a0 /* PWS_REG */
> + 0x000e4 0x6a545 /* MAC_POWER_SEL */
> + 0x000e0 0xc74164de /* SGMII_CTRL */
> + 0x0007c 0x4e /* PORT0_STATUS */
> + 0x00094 0x4e /* PORT6_STATUS */
> + >;
> + };
> +};
> +
> +&gmac1 {
> + status = "okay";
> + phy-mode = "rgmii";
> + qcom,id = <1>;
> +
> + pinctrl-0 = <&rgmii2_pins>;
> + pinctrl-names = "default";
> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> +};
> +
> +&gmac2 {
> + status = "okay";
> + phy-mode = "sgmii";
> + qcom,id = <2>;
> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> +};
> +
> +&tcsr {
> + qcom,usb-ctrl-select = <TCSR_USB_SELECT_USB3_DUAL>;
> + compatible = "qcom,tcsr";
> +};
> +
> +&adm_dma {
> + status = "okay";
> +};
> diff --git a/target/linux/ipq806x/image/Makefile
> b/target/linux/ipq806x/image/Makefile
> index 5cc66daa21..42512ed423 100644
> --- a/target/linux/ipq806x/image/Makefile
> +++ b/target/linux/ipq806x/image/Makefile
> @@ -13,6 +13,17 @@ define Build/buffalo-rootfs-cksum
> ) >> $@
> endef
>
> +# tune addpattern for Linksys E8350 fw pattern generation define
> +Build/linksys-bin
> + $(STAGING_DIR_HOST)/bin/addpattern -p $(FW_DEVICE_ID) -v
> $(FW_VERSION) $(if $(SERIAL),-s $(SERIAL)) -i $@ -o $@.new
> + mv $@.new $@
> +endef
> +# Add Linksys E8350 fw header generator from the linksys gpl code: to
> +upgrade openwrt factory image over the native Linksys WEB interface
> define Build/addfwhdr
> + -$(STAGING_DIR_HOST)/bin/linksys/e8350/addfwhdr -i $@ -o $@.new
I didn't find addfwhdr anywhere?
> \
> + ;mv "$@.new" "$@"
> +endef
Since this is the first Linksys device needing that stuff after several others were added to the target, one should name them more specificly as the current names imply that those apply to all Linksys devices which is just not the case.
> +
> define Device/Default
> PROFILES := Default
> KERNEL_DEPENDS = $$(wildcard $(DTS_DIR)/$$(DEVICE_DTS).dts)
> @@ -109,6 +120,26 @@ define Device/compex_wpq864 endef
> TARGET_DEVICES += compex_wpq864
>
> +define Device/linksys_e8350
> + $(call Device/LegacyImage)
> + BOARD_NAME = e8350
> + SUPPORTED_DEVICES += e8350
Drop BOARD_NAME and SUPPORTED_DEVICES.
> + DEVICE_VENDOR := Linksys
> + DEVICE_MODEL := E8350
> + DEVICE_VARIANT := v1
Okay, so this should definitely be called linksys_e8350-v1 and Linksys,e8350-v1.
> + SOC := qcom-ipq8064
> + FW_VERSION := v1.0.03.003
> + FW_DEVICE_ID := 8350
> + PAGESIZE := 2048
> + BLOCKSIZE := 128k
> + KERNEL_IN_UBI := 1
> + IMAGES = factory.bin sysupgrade.ubi
> + IMAGE/sysupgrade.ubi := append-ubi | check-size 0x04000000 |
> append-metadata
> + IMAGE/factory.bin := append-ubi | check-size 0x04000000 | addfwhdr |
> linksys-bin
> + DEVICE_PACKAGES := ath10k-firmware-qca988x-ct endef
> +TARGET_DEVICES += linksys_e8350
> +
> define Device/linksys_ea7500-v1
> $(call Device/LegacyImage)
> DEVICE_VENDOR := Linksys
> diff --git a/target/linux/ipq806x/patches-5.4/0069-arm-boot-add-dts-
> files.patch b/target/linux/ipq806x/patches-5.4/0069-arm-boot-add-dts-
> files.patch
> index 8cdd198c29..8e4a3fd876 100644
> --- a/target/linux/ipq806x/patches-5.4/0069-arm-boot-add-dts-files.patch
> +++ b/target/linux/ipq806x/patches-5.4/0069-arm-boot-add-dts-files.patch
> @@ -10,7 +10,7 @@ Signed-off-by: John Crispin <john at phrozen.org>
>
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> -@@ -843,6 +843,19 @@ dtb-$(CONFIG_ARCH_QCOM) += \
> +@@ -843,6 +843,20 @@ dtb-$(CONFIG_ARCH_QCOM) += \
> qcom-ipq4019-ap.dk07.1-c1.dtb \
> qcom-ipq4019-ap.dk07.1-c2.dtb \
> qcom-ipq8064-ap148.dtb \
> @@ -19,6 +19,7 @@ Signed-off-by: John Crispin <john at phrozen.org>
> + qcom-ipq8064-db149.dtb \
> + qcom-ipq8064-ap161.dtb \
> + qcom-ipq8064-ea7500-v1.dtb \
> ++ qcom-ipq8064-e8350.dtb \
Sorting.
Best
Adrian
> + qcom-ipq8064-ea8500.dtb \
> + qcom-ipq8064-r7500.dtb \
> + qcom-ipq8064-r7500v2.dtb \
> --
> 2.27.0
>
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
-------------- 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/20200719/e80dfd16/attachment.sig>
More information about the openwrt-devel
mailing list