[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