[OpenWrt-Devel] [PATCH] ramips: add support to JS76x8 development board

[Mathias Kresin]

Hey Robinson,

find my comments inline.

Mathias

05/12/2018 06:25, robinson:
> This commit adds support for the ZhuoTK JS76x8 development board,
> The device has the following specifications:
> 
> - SOC:MT7628AN/NN or MT7688AN (580MHz)
> - RAM:64/128/256 MB (DDR2)
> - FLASH:8/16/32 MB (SPI NOR)
> - 3x 10/100 Mbps ethernet ports (MT7628/MT7688 built-in switch)
> - 1x 2T2R 2.4 GHz Wi-Fi (MT7628), 1T1R 2.4 GHz Wi-Fi (MT7688)
> - 1x microSD slot
> - 4x USB 2.0 port
> - 4x red/green/blue/yellow color LEDs (GPIO-controlled)
> - 5x green LED (GPIO-controlled)
> - 4x button
> - 1x mini-usb debug UART
> - 1x DC jack for main power (DC 5V)
> - 1x TTL/RS232 UART
> - 1x TLL/RS485 UART
> - 13x GPIO header
> - 1x audio codec(wm8960)
> - 1x IR 1T1R
> 
> Other notes:
> 
> 1. The board is available with different amounts of RAM and flash. Chose
> the right configuration in make menuconfig to fit the board you have.

You only add support for one configuration, which option should be there 
for chosing the right image?

> 
> For more information vist www.zhuotk.com
> 
> Signed-off-by: robinson <wurobinson at qq.com>

We need your full name in the Signed-off-by. Furthermore, the install 
instructions are missing in the commit message. Have a look at 
https://openwrt.org/submitting-patches for further details.

> ---
>   target/linux/ramips/base-files/etc/board.d/01_leds |  4 ++
>   .../linux/ramips/base-files/etc/board.d/02_network |  1 +
>   target/linux/ramips/base-files/lib/ramips.sh       |  3 ++
>   .../ramips/base-files/lib/upgrade/platform.sh      |  1 +
>   target/linux/ramips/dts/JS76x8-16M-128M.dts        | 57 +++++++++++++++++++++
>   target/linux/ramips/dts/JS76x8.dtsi                | 59 ++++++++++++++++++++++
>   target/linux/ramips/image/mt76x8.mk                |  9 ++++
>   7 files changed, 134 insertions(+)
>   create mode 100644 target/linux/ramips/dts/JS76x8-16M-128M.dts
>   create mode 100644 target/linux/ramips/dts/JS76x8.dtsi
> 
> diff --git a/target/linux/ramips/base-files/etc/board.d/01_leds b/target/linux/ramips/base-files/etc/board.d/01_leds
> index b2c69d4..ef404d3 100755
> --- a/target/linux/ramips/base-files/etc/board.d/01_leds
> +++ b/target/linux/ramips/base-files/etc/board.d/01_leds
> @@ -184,6 +184,10 @@ ht-tm02)
>   	ucidef_set_led_netdev "eth" "Ethernet" "$boardname:green:lan" "eth0"
>   	set_wifi_led "$boardname:blue:wlan"
>   	;;
> +js76x8-16m-128m)

Use the first compatible from the dts file ("zhuotk,js76x8-16m-128m") as 
userspace boardname, as all recently added board to the ramips target do.

> +	ucidef_set_led_timer "system" "system" "zhuotk:green:system" "1000" "1000"
> +	set_wifi_led "zhuotk:green:wifi"
> +	;;
>   kimax,u35wf)
>   	set_wifi_led "$boardname:blue:wifi"
>   	ucidef_set_led_netdev "eth" "ETH" "$boardname:green:eth" "eth0"
> 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 3af836a..ba8c702 100755
> --- a/target/linux/ramips/base-files/etc/board.d/02_network
> +++ b/target/linux/ramips/base-files/etc/board.d/02_network
> @@ -94,6 +94,7 @@ ramips_setup_interfaces()
>   	hc5661a|\
>   	hc5962|\
>   	hlk-rm04|\
> +	js76x8-16m-128m|\
>   	k2p|\
>   	kn|\
>   	kn_rc|\
> diff --git a/target/linux/ramips/base-files/lib/ramips.sh b/target/linux/ramips/base-files/lib/ramips.sh
> index 307ab35..80fd0f6 100755
> --- a/target/linux/ramips/base-files/lib/ramips.sh
> +++ b/target/linux/ramips/base-files/lib/ramips.sh
> @@ -259,6 +259,9 @@ ramips_board_detect() {
>   	*"JHR-N926R")
>   		name="jhr-n926r"
>   		;;
> +	*"JS76x8 (16MB flash/128MB RAM)")
> +		name="js76x8-16m-128m"
> +		;;

Can be dropped as soon as you use the compatible from the dts as 
userspace boardname.

>   	*"K2P")
>   		name="k2p"
>   		;;
> diff --git a/target/linux/ramips/base-files/lib/upgrade/platform.sh b/target/linux/ramips/base-files/lib/upgrade/platform.sh
> index 120cf83..1c686ba 100755
> --- a/target/linux/ramips/base-files/lib/upgrade/platform.sh
> +++ b/target/linux/ramips/base-files/lib/upgrade/platform.sh
> @@ -235,6 +235,7 @@ platform_check_image() {
>   	zbt-wg3526-16M|\
>   	zbt-wg3526-32M|\
>   	zbt-wr8305rt|\
> +	js76x8-16m-128m|\

You can drop this one, it isn't required anymore.

>   	zorlik,zl5900v2|\
>   	zte-q7|\
>   	zyxel,keenetic-extra-ii|\
> diff --git a/target/linux/ramips/dts/JS76x8-16M-128M.dts b/target/linux/ramips/dts/JS76x8-16M-128M.dts
> new file mode 100644
> index 0000000..7a08490
> --- /dev/null
> +++ b/target/linux/ramips/dts/JS76x8-16M-128M.dts
> @@ -0,0 +1,57 @@
> +/dts-v1/;
> +
> +
> +#include "JS76x8.dtsi"
> +
> +
> +/ {
> +	compatible = "zhuotk,js76x8", "zhuotk,js76x8-16m-128m", "mediatek,mt7628an-soc";

wrong order. From most to least specific.

> +	model = "JS76x8 (16MB flash/128MB RAM)";
> +
> +	memory at 0 {
> +		device_type = "memory";
> +                reg = <0x0 0x8000000>;
> +
> +	};
> +};
> +
> +&spi0 {
> +	status = "okay";
> +
> +	m25p80 at 0 {

flash@

> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +		spi-max-frequency = <10000000>;
> +		m25p,chunked-io = <32>;
> +
> +		partitions {
> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			partition at 0 {
> +				label = "u-boot";
> +				reg = <0x0 0x30000>;
> +				read-only;
> +			};
> +
> +			partition at 30000 {
> +				label = "u-boot-env";
> +				reg = <0x30000 0x10000>;
> +				read-only;
> +			};
> +
> +			factory: partition at 40000 {
> +				label = "factory";
> +				reg = <0x40000 0x10000>;
> +				read-only;
> +			};
> +
> +			partition at 50000 {
> +				compatible = "denx,uimage";
> +				label = "firmware";
> +                              reg = <0x50000 0xfb0000>;

At least here you have a spaces vs. tabs issues. Please check all your 
changes to make sure to use tabs only.

> +			};
> +		};
> +	};
> +};
> diff --git a/target/linux/ramips/dts/JS76x8.dtsi b/target/linux/ramips/dts/JS76x8.dtsi
> new file mode 100644
> index 0000000..e060c98
> --- /dev/null
> +++ b/target/linux/ramips/dts/JS76x8.dtsi
> @@ -0,0 +1,59 @@
> +/dts-v1/;
> +
> +#include "mt7628an.dtsi"

Please include dt-bindings/gpio/gpio.h 
(http://lxr.free-electrons.com/source/include/dt-bindings/gpio/gpio.h?v=4.4) 
here as well and keep alphabetical order.

Use the GPIO_ACTIVE_LOW and GPIO_ACTIVE_HIGH macros afterwards in stead 
of 1 and 0 in the gpio parameters.

Check the recent ramips board additions for examples.


> +
> +/ {
> +	compatible = "mediatek,mt7628an-eval-board", "mediatek,mt7628an-soc";

This isn't for sure the mt7628an eval board. Use "zhuotk,js76x8" instead.

> +
> +        aliases {
> +                led-boot = &led_system;
> +                led-failsafe = &led_system;
> +                led-running = &led_system;
> +                led-upgrade = &led_system;
> +        };
> +
> +        chosen {
> +                bootargs = "console=ttyS0,115200";
> +        };
> +
> +        gpio-leds {

commit message mentions 9 leds but only two are added.

> +                compatible = "gpio-leds";
> +
> +                led_system: system {
> +                        label = "zhuotk:green:system";

The led namings follows the follow pattern: <board>:<colour>:<function>. 
Should be "js76x8:<colour>:<function>" here.

> +                        gpios = <&gpio1 5 1>; > +                        default-state = "off";

Drop the default-state = off. It is the default.

> +                };

I somehow miss the buttons mentioned in the commit message.

> +
> +                wifi {
> +                        label = "zhuotk:green:wifi";
> +                        gpios = <&gpio1 7 1>;
> +                        default-state = "off";
> +                };
> +        };
> +};
> +
> +&pinctrl {
> +	state_default: pinctrl0 {
> +		refclk {
> +			ralink,group = "refclk";

Her is for sure something missing. refclk covers only a single pin but 
two (gpio) leds are defined. Please get the groups which need to be 
switched to gpio mode from

> +			ralink,function = "gpio";
> +		};
> +	};
> +};
> +
> +&wmac {
> +	status = "okay";
> +};
> +
> +&spi0 {
> +	status = "okay";
> +};

What is connected to the spi? I can't find anything in the commit message.

> +
> +&wmac {
> +	status = "okay";
> +};

Added twice.

> +
> +&ethernet {
> +        mtd-mac-address = <&factory 0x28>;
> +};
> diff --git a/target/linux/ramips/image/mt76x8.mk b/target/linux/ramips/image/mt76x8.mk
> index d2f402e..087f5ba 100644
> --- a/target/linux/ramips/image/mt76x8.mk
> +++ b/target/linux/ramips/image/mt76x8.mk
> @@ -76,6 +76,15 @@ define Device/hiwifi_hc5861b
>   endef
>   TARGET_DEVICES += hiwifi_hc5861b
>   
> +define Device/js76x8-16m-128m
> +  DTS := JS76x8-16M-128M
> +  IMAGE_SIZE := $(ralink_default_fw_size_16M)

ralink_default_fw_size_16M=16121856 but your firmware partition is 
0xfb0000 == 16449536.

> +  SUPPORTED_DEVICES := js76x8-16m-128m
> +  DEVICE_TITLE :=ZhuoTK JS76x8 (16M flash/128M RAM)
> +  DEVICE_PACKAGES:= kmod-usb2 kmod-usb-ohci uboot-envtools

Adding the u-boot env tools without any configuration for this board 
doesn't make much sense.

> +endef
> +TARGET_DEVICES += js76x8-16m-128m
> +
>   define Device/LinkIt7688
>     DTS := LINKIT7688
>     IMAGE_SIZE := $(ralink_default_fw_size_32M)
> 


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