[PATCH 6/6] qoriq: add support for WatchGuard Firebox M300

Stijn Tintel stijn at linux-ipv6.be
Sun Aug 22 05:51:14 PDT 2021


On 22/08/2021 14:35, Adrian Schmutzler wrote:
> Hi,
>
>> -----Original Message-----
>> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
>> On Behalf Of Stijn Tintel
>> Sent: Sonntag, 22. August 2021 01:15
>> To: openwrt-devel at lists.openwrt.org
>> Subject: [PATCH 6/6] qoriq: add support for WatchGuard Firebox M300
>>
>> This device is based on NXP's QorIQ T2081QDS board, with a quad-core dual-
>> threaded 1.5 GHz ppc64 CPU and 4GB ECC RAM. The board has 5 ethernet
>> interfaces, of which 3 are connected to the ethernet ports on the front
>> panel. The other 2 are internally connected to a Marvell
>> 88E6171 switch; the other 5 ports of this switch are also connected to the
>> ethernet ports on the front panel.
>>
>> Installation: write the sdcard image to an SD card. Stock U-Boot will not boot,
>> wait for it to fail then run these commands:
>>
>> setenv wgBootSysA "setenv bootargs root=/dev/mmcblk0p2 rw rootdelay=2
>> console=$consoledev,$baudrate fsl_dpaa_fman.fsl_fm_max_frm=1522;
>> ext2load mmc 0:1 $fdtaddr image-m300.dtb; ext2load mmc 0:1 $loadaddr
>> firebox_m300-kernel.bin; bootm $loadaddr - $fdtaddr"
>> saveenv
>> reset
>>
>> The default U-Boot boot entry will now boot OpenWrt from the SD card.
> a few mostly cosmetic comments below, with the only real issue at the very bottom.
>
> Target introduction in 5/6 looked fine.
Thanks!
>
>> Signed-off-by: Stijn Tintel <stijn at linux-ipv6.be>
>> ---
>>  .../base-files/lib/preinit/79_move_config     |  17 +
>>  .../qoriq/base-files/lib/upgrade/platform.sh  |  38 +++
>> .../files/arch/powerpc/boot/dts/fsl/m300.dts  | 294 ++++++++++++++++++
>>  target/linux/qoriq/image/generic.mk           |  13 +
>>  4 files changed, 362 insertions(+)
>>  create mode 100644 target/linux/qoriq/base-
>> files/lib/preinit/79_move_config
>>  create mode 100755 target/linux/qoriq/base-files/lib/upgrade/platform.sh
>>  create mode 100644
>> target/linux/qoriq/files/arch/powerpc/boot/dts/fsl/m300.dts
>>
>> diff --git a/target/linux/qoriq/base-files/lib/preinit/79_move_config
>> b/target/linux/qoriq/base-files/lib/preinit/79_move_config
>> new file mode 100644
>> index 0000000000..22ec022f6a
>> --- /dev/null
>> +++ b/target/linux/qoriq/base-files/lib/preinit/79_move_config
>> @@ -0,0 +1,17 @@
>> +# Copyright (C) 2015 OpenWrt.org
> New files should have an SPDX identifier.
Replaced with SPDX.
>
>> +
>> +. /lib/functions.sh
>> +. /lib/upgrade/common.sh
>> +
>> +move_config() {
>> +	local partdev
>> +
>> +	if export_bootdevice && export_partdevice partdev 1; then
>> +		mkdir -p /boot
>> +		mount -o rw,noatime "/dev/$partdev" /boot
>> +		[ -f "/boot/$BACKUP_FILE" ] && mv -f "/boot/$BACKUP_FILE"
>> /
>> +		umount /boot
>> +	fi
>> +}
>> +
>> +boot_hook_add preinit_mount_root move_config
>> diff --git a/target/linux/qoriq/base-files/lib/upgrade/platform.sh
>> b/target/linux/qoriq/base-files/lib/upgrade/platform.sh
>> new file mode 100755
>> index 0000000000..d88f70e3f6
>> --- /dev/null
>> +++ b/target/linux/qoriq/base-files/lib/upgrade/platform.sh
>> @@ -0,0 +1,38 @@
>> +#
>> +# Copyright (C) 2011 OpenWrt.org
>> +#
Replaced with SPDX.
>> +
>> +PART_NAME=firmware
>> +REQUIRE_IMAGE_METADATA=1
>> +
>> +platform_check_image() {
>> +	case "$(board_name)" in
>> +	watchguard,firebox-m300)
>> +		legacy_sdcard_check_image "$1"
>> +		;;
>> +	*)
>> +		return 0
>> +		;;
>> +	esac
>> +}
>> +
>> +platform_copy_config() {
>> +	case "$(board_name)" in
>> +	watchguard,firebox-m300)
>> +		legacy_sdcard_copy_config "$1"
>> +		;;
>> +	*)
>> +		return 0
>> +	esac
>> +}
>> +
>> +platform_do_upgrade() {
>> +	case "$(board_name)" in
>> +	watchguard,firebox-m300)
>> +		legacy_sdcard_do_upgrade "$1"
>> +		;;
>> +	*)
>> +		default_do_upgrade "$1"
>> +		;;
>> +	esac
>> +}
>> diff --git a/target/linux/qoriq/files/arch/powerpc/boot/dts/fsl/m300.dts
>> b/target/linux/qoriq/files/arch/powerpc/boot/dts/fsl/m300.dts
>> new file mode 100644
>> index 0000000000..71ff57dcb6
>> --- /dev/null
>> +++ b/target/linux/qoriq/files/arch/powerpc/boot/dts/fsl/m300.dts
>> @@ -0,0 +1,294 @@
>> +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0-or-later
>> +/*
>> + * WatchGuard Firebox M300 Device Tree Source
>> + * Based on t2081qds.dts from Linux 5.10
>> + *
>> + * Copyright 2013 - 2015 Freescale Semiconductor Inc.
>> + * Copyright 2020 - 2021 Stijn Tintel <stijn at linux-ipv6.be>  */
>> +
>> +/include/ "t208xsi-pre.dtsi"
>> +/include/ "t208xqds.dtsi"
>> +
>> +/ {
>> +	model = "WatchGuard Firebox M300";
>> +	compatible = "watchguard,firebox-m300", "fsl,T2081QDS";
> I'd add an empty line after these two.
Done.
>
>> +	#address-cells = <2>;
>> +	#size-cells = <2>;
> Do we need these without a direct subnode?
Don't know to be honest. Upstream still has them in t2081qds.dts.
However, the board still boots fine with these removed, so I've removed
them.
>
>> +	interrupt-parent = <&mpic>;
>> +
>> +	aliases {
>> +	};
> Consider dropping the empty node.
Done.
>
>> +};
>> +
>> +// mdio-mux under &boardctrl + its aliases removed. causes crash:
>> +// Oops: Machine check, sig: 7 [#1]
>> +
>> +/include/ "t2081si-post.dtsi"
>> +
>> +// add stuff below the include to make sure we override whatever is
>> +there
>> +
>> +&enet0 {
>> +	phy-connection-type = "sgmii";
>> +	phy-handle = <&phy1>;
>> +};
>> +
>> +&enet1 {
>> +	phy-connection-type = "sgmii";
>> +	phy-handle = <&phy2>;
>> +};
>> +
>> +&enet2 {
>> +	phy-connection-type = "rgmii";
>> +	fixed-link = <0x10 0x01 0x3e8 0x00 0x00>; };
>> +
>> +&enet3 {
>> +	phy-connection-type = "rgmii";
>> +	fixed-link = <0x04 0x01 0x3e8 0x00 0x00>; };
>> +
>> +&enet4 {
>> +	status = "disabled";
>> +};
>> +
>> +&enet5 {
>> +	status = "disabled";
>> +};
>> +
>> +&enet6 {
>> +	status = "disabled";
>> +};
>> +
>> +&enet7 {
>> +	phy-connection-type = "sgmii";
>> +	phy-handle = <&phy0>;
>> +};
>> +
>> +&ifc {
>> +	ranges = <0x00 0x00 0x0f 0xefc00000 0x400000>;
>> +
>> +	nor at 0,0 {
>> +		reg = <0x00 0x00 0x400000>;
>> +
>> +		partition at 00000000 {
> The @x number could be shortened.
I personally like them "aligned", as it improves readability imo. But as
`git grep partition@` in the kernel tree seems to disagree, I'll adapt.
>
>> +			reg = <0x00 0x10000>;
>> +			label = "NOR (RW) LANNER RCW Code";
> Labels here might need some refactoring, too.
Since we're not really touching anything on the NOR (yet), I prefer to
keep the OEM names for now. What else would you suggest?
>
>> +		};
>> +
>> +		partition at 00010000 {
>> +			reg = <0x10000 0x20000>;
>> +			label = "NOR (RW) WG CFG0";
>> +		};
>> +
>> +		partition at 00030000 {
>> +			reg = <0x30000 0x10000>;
>> +			label = "NOR (RW) WG CFG1";
>> +		};
>> +
>> +		partition at 00040000 {
>> +			reg = <0x40000 0x10000>;
>> +			label = "NOR (RW) WG MFG DATA";
>> +		};
>> +
>> +		partition at 00050000 {
>> +			reg = <0x50000 0xb0000>;
>> +			label = "NOR (RW) WG bootOpt Data & reserved";
>> +		};
>> +
>> +		partition at 00100000 {
>> +			reg = <0x100000 0xb0000>;
>> +			label = "NOR (RW) WG extra reserved 1";
>> +		};
>> +
>> +		partition at 001B0000 {
>> +			reg = <0x1b0000 0xb0000>;
>> +			label = "NOR (RW) WG extra reserved 2";
>> +		};
>> +
>> +		partition at 00260000 {
>> +			reg = <0x260000 0xc0000>;
>> +			label = "NOR (RW) WG U-Boot FAILSAFE";
>> +		};
>> +
>> +		partition at 00320000 {
>> +			reg = <0x320000 0x10000>;
>> +			label = "NOR (RW) FMAN";
>> +		};
>> +
>> +		partition at 00330000 {
>> +			reg = <0x330000 0x10000>;
>> +			label = "NOR (RW) WG U-Boot ENV";
>> +		};
>> +
>> +		partition at 00340000 {
>> +			reg = <0x340000 0xc0000>;
>> +			label = "NOR (RW) WG U-Boot Image";
>> +		};
>> +	};
>> +
>> +	nand at 2,0 {
>> +		status = "disabled";
>> +	};
>> +};
>> +
>> +&mdio0 {
>> +	// m300 ethernet port 0
>> +	phy0: ethernet-phy at 0 {
>> +		reg = <0x00>;
>> +	};
>> +
>> +	// m300 ethernet port 1
>> +	phy1: ethernet-phy at 1 {
>> +		reg = <0x01>;
>> +	};
>> +
>> +	phy2: ethernet-phy at 2 {
>> +		reg = <0x02>;
>> +	};
>> +
>> +	phy3: ethernet-phy at 3 {
>> +		reg = <0x03>;
>> +	};
>> +
>> +	// m300 mv88e6123 switch detected
>> +	switch0: switch at 10 {
>> +		compatible = "marvell,mv88e6085";
>> +		reg = <0x10>;
>> +
>> +		//dsa,member = <0 0>;
>> +		mdio {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			switch0phy0: switch0phy0 at 0 {
>> +				reg = <0x00>;
>> +				interrupt-parent = <&switch0>;
>> +			};
>> +
>> +			switch0phy1: switch0phy1 at 1 {
>> +				reg = <0x01>;
>> +				interrupt-parent = <&switch0>;
>> +			};
>> +
>> +			switch0phy2: switch0phy2 at 2 {
>> +				reg = <0x02>;
>> +				interrupt-parent = <&switch0>;
>> +			};
>> +
>> +			switch0phy3: switch0phy3 at 3 {
>> +				reg = <0x03>;
>> +				interrupt-parent = <&switch0>;
>> +			};
>> +
>> +			switch0phy4: switch0phy4 at 4 {
>> +				reg = <0x04>;
>> +				interrupt-parent = <&switch0>;
>> +			};
>> +		};
>> +
>> +		ports {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			port at 0 {
>> +				reg = <0>;
>> +				// TODO: rename to eth3 after figuring out
>> how to rename SoC eth3
>> +				label = "sw0p0";
>> +				phy-handle = <&switch0phy0>;
>> +			};
>> +
>> +			port at 1 {
>> +				// TODO: rename to eth4 after figuring out
>> how to rename SoC eth4
>> +				reg = <1>;
>> +				label = "sw0p1";
>> +				phy-handle = <&switch0phy1>;
>> +			};
>> +
>> +			port at 2 {
>> +				reg = <2>;
>> +				label = "eth5";
>> +				phy-handle = <&switch0phy2>;
>> +			};
>> +
>> +			port at 3 {
>> +				reg = <3>;
>> +				label = "eth6";
>> +				phy-handle = <&switch0phy3>;
>> +			};
>> +
>> +			port at 4 {
>> +				reg = <4>;
>> +				label = "eth7";
>> +				phy-handle = <&switch0phy4>;
>> +			};
>> +
>> +			port at 5 {
>> +				status = "disabled";
>> +
>> +				reg = "<5>";
>> +				label = "cpu1";
>> +				ethernet = <&enet2>;
>> +				phy-mode = "rgmii";
>> +
>> +				fixed-link {
>> +					speed = <1000>;
>> +					full-duplex;
>> +				};
> This is still WIP? Or why do we need to add this if it's disabled?
I believe both ports are connected to the switch, but this is currently
not supported in DSA. I prefer to keep this until support for multiple
CPU ports lands in OpenWrt.
>
>> +			};
>> +
>> +			port at 6 {
>> +				reg = <6>;
>> +				label = "cpu";
>> +				ethernet = <&enet3>;
>> +				phy-mode = "rgmii-id";
>> +
>> +				fixed-link {
>> +					speed = <1000>;
>> +					full-duplex;
>> +				};
>> +			};
>> +		};
>> +	};
>> +};
>> +
>> +&soc {
>> +	i2c at 118000 {
>> +		tpm at 29 {
>> +			compatible = "tpm,tpm_i2c_atmel";
>> +			reg = <0x29>;
>> +		};
>> +		hwmon at 2c {
>> +			compatible = "winbond,w83793";
>> +			reg = <0x2c>;
>> +		};
>> +		hwmon at 2d {
>> +			compatible = "winbond,w83793";
>> +			reg = <0x2d>;
>> +		};
>> +		rtc at 32 {
>> +			compatible = "ricoh,rs5c372a";
>> +			reg = <0x32>;
>> +		};
>> +		pca9547 at 77 {
>> +			status = "disabled";
>> +		};
>> +	};
>> +
>> +	spi at 110000 {
>> +		// DTS decompiled from OEM DTB contains flash at 0 but
>> doesn't work
>> +		// spi-nor spi0.0: unrecognized JEDEC id bytes: ff ff ff ff ff ff
>> +		// disable for now
>> +		flash at 0 {
>> +			status = "disabled";
>> +		};
>> +
>> +		flash at 1 {
>> +			status = "disabled";
>> +		};
>> +
>> +		flash at 2 {
>> +			status = "disabled";
>> +		};
>> +	};
>> +};
>> diff --git a/target/linux/qoriq/image/generic.mk
>> b/target/linux/qoriq/image/generic.mk
>> index e69de29bb2..2709811658 100644
>> --- a/target/linux/qoriq/image/generic.mk
>> +++ b/target/linux/qoriq/image/generic.mk
>> @@ -0,0 +1,13 @@
>> +define Device/firebox_m300
> This node name should be "Device/watchguard_firebox-m300", so it matches the compatible.
Renamed.
>
>> +  DEVICE_VENDOR := WatchGuard
>> +  DEVICE_MODEL := Firebox M300
>> +  DEVICE_DTS_DIR := $(DTS_DIR)/fsl
>> +  DEVICE_PACKAGES := kmod-hwmon-w83793 kmod-rtc-rs5c372a
>> +kmod-tpm-i2c-atmel
>> +  BLOCKSIZE := 128k
>> +  KERNEL := kernel-bin | gzip | uImage gzip
>> +  SUPPORTED_DEVICES := fsl,T2081QDS, watchguard,firebox-m300
> The second entry should be dropped, since it's derived from the node name after change.
> Do you really need the first one? If yes,
>
> SUPPORTED_DEVICES += fsl,T2081QDS
>
> otherwise drop the entire line.
I think this is a leftover from when sysupgrade wasn't working due to
board mismatch. Dropped.
>
> With the new node name you should either rename the DTS to firebox-m300.dts [*] or add a specific DEVICE_DTS entry here if the shorter name can't be changed for some reason.
>
> * I'd personally even prefer the long name watchguard-firebox-m300.dts, which would require to change the default DEVICE_DTS in patch 5/6. However, kernel is completely inconsistent here, so I don't know whether that would be acceptable for the particular target there.
I  think watchguard-firebox-m300.dts is the most descriptive name, so I
went for that.
>
> Best
>
> Adrian
>
>> +  IMAGES := sdcard.img.gz sysupgrade.img.gz
>> +  IMAGE/sysupgrade.img.gz :=  sdcard-img | gzip | append-metadata
>> +  IMAGE/sdcard.img.gz := sdcard-img | gzip endef TARGET_DEVICES +=
>> +firebox_m300
>> --
>> 2.31.1
>>
>>
Thanks for your quick review, Adrian, I really appreciate it.

Changes can already be reviewed in my staging tree:
https://git.openwrt.org/?p=openwrt/staging/stintel.git;a=shortlog;h=refs/heads/qoriq
They will be squashed into the relevant commits before sending out a v2
series.

Stijn




More information about the openwrt-devel mailing list