-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
mpc85xx: add support for WatchGuard T30-W #4697
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it's work in progress.
Anyway, please split this up into three commits:
- Refresh of kernel config (without the device-related changes)
- Changes in scripts/sysupgrade-tar.sh
- Add support for device
I'm not sure whether we should put Build/sysupgrade-dtb-tar into image-commands.mk yet if it only has one user; one could consider to leave it in the target Makefile for now.
Hi Adrian, I'll flag it as WIP for now, as I have just noticed the MAC addresses are not defined properly. They are in /dev/mtd0 at different offsets, is there an nvmem way to read 17 byte ASCII based mac addresses with ':' from an offset ? In terms of the Build/sysupgrade-dtb-tar, similar is also needed for Aerohive AP370 which doesn't allow boot command modifications, and boots with bootm 0x 0x 0x. I'll work on the rest of the comments and unflag WIP and request a re-review once done. Thanks again for your comments, Damien |
Note that I'm not generally against the global build recipe, I just wanted to trigger some thought about it. I'm also not really qualified to judge the approach in general, so this will probably have to wait for some deeper review anyway. |
Nice to see your work. I have both a T30 and a T30-W. Will try to find time this coming weekend to pull your work and test it on my devices! I also have a T10 and T50 which should be able to leverage some of your work for the T30-W. |
139e758
to
49ddf8f
Compare
I have resolved the formatting issues in the PR, and added in the MAC locations, but it looks like non nvmem mac address setting is no longer functional: ucidef_set_interface_macaddr "wan" "$(mtd_get_mac_text nor-cfg0 0x1830)" |
49ddf8f
to
0dc7f6a
Compare
if (ppc_md.progress) | ||
ppc_md.progress("wg_t30w_setup_arch()", 0); | ||
|
||
//mpc85xx_smp_init(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we fixed the SMP issue with dd7d470, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me test it with master, and then remove the comment if it works properly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chunkeey P1011 is single-core anyways :^)
bank-width = <0x02>; | ||
device-width = <0x01>; | ||
|
||
partition@0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if you put the partition@0...partition@380000 into a "partitions" subnode (a new one with the fixed-partition compatible) the "nvmem-cells" for mac-addresses and friends will start to work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me give that a try and will force push another version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is correct, see this PR.
Hopefully it worked out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's no uboot involvement it should work. Thing is, I've seen u-boots that need to parse the tree and has "fixed" paths.
In these case, please take the time to document what's happening. i.e: include uboot errors messages and what is broken.
legerity@1 { | ||
compatible = "zarlink,le88266"; | ||
reg = <0x01>; | ||
spi-max-frequency = <0x7a120>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, this tells me that you are probably using the "get a working fdt with all the uboot changes" and export it as the dts :) .
This is because "500000" rolls much better off the keyboard (same as with the 40MHz from the node above).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, will change this
reg = <0x00 0xffe09000 0x00 0x1000>; | ||
bus-range = <0x00 0xff>; | ||
ranges = <0x2000000 0x0 0xa0000000 0x0 0xa0000000 0x0 0x20000000 | ||
0x1000000 0x00 0x00 0x00 0xffc10000 0x00 0x10000>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cleaning up huge ranges, reg, or interrupt-maps array is tricky. Especially if you don't have the board or a similar device. I do think this is supposed to contain two entries like this:
ranges = <0x2000000 0x0 0xa0000000 0x0 0xa0000000 0x0 0x2000000>,
<0x1000000 0x0 0x0 0x0 0xffc10000 0x0 0x10000>;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind formatting it that way, but functionally it should be the same right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is confusing (they work sort-of-like c-arrays). But there's something written about this in
https://elinux.org/Device_Tree_Mysteries Example 3.1:
Example 3.1
Two tuples, one element per tuple:
interrupts = <17>, <4>;
One tuple, two elements per tuple:
interrupts = <17 4>;
Three tuples, with formats of:
1) phandle, interrupt
2) phandle, interrupt, flags
2) phandle, interrupt
interrupts-extended = < &INTC_1 17 >, < &INTC_2 23 8 >, < &INTC_4 27 >;
There are no type declarations for arrays and tuples, so the
dtc compiler can not enforce conformance with tuple size in the value
declarations.
Thus the above three tuples would properly compile if specified as:
interrupts-extended = < &INTC_1 17 &INTC_2 >, < 23 8 >, < &INTC_4 27 >;
even though it appears not to match the formats of the three tuples.
If you compile a device tree source file containing each of the two above
definitions of interrupts, then decompile the resulting .dtb file, you will
see the dtc simply converts both forms to be:
<0x2 0x11 0x3 0x17 0x8 0x4 0x1b>
(In the test .dts file I created, &INTC_1 has the value 0x2, &INTC_2 has
the value 0x3, and &INTC_4 has the value 0x4.)
0dc7f6a
to
dcb26be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, you could put the changes into a config-default file in the target/linux/mpc85xx/p1020 directory. This has the advantage that only the p1020 subtargets (I assume that the T30-W is a P1020) kernel will grow due to the inclusion of DSA, MMC.
EDIT: Forgot to add: If you need a pointer look at the apm821xx target. The "sata" (aka NAS) has some incompatible kernel parameters with NAND-target. (But the common symbols are in the apm821xx's config-X.XX)
target/linux/mpc85xx/config-5.10
Outdated
@@ -35,20 +35,14 @@ CONFIG_COMPAT_32BIT_TIME=y | |||
# CONFIG_CORENET_GENERIC is not set | |||
# CONFIG_CPM2 is not set | |||
CONFIG_CPU_BIG_ENDIAN=y | |||
CONFIG_CRYPTO_AEAD=y | |||
CONFIG_CRYPTO_AEAD2=y | |||
CONFIG_CRC16=m |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
=m?
This is strange. It's either "not set" or "=y" in the target's kernel configurations. It's not that this doesn't "work", but maybe not as intended (since without including the CRC16 module in some way, this can backfire.)
Refresh the kernel config to add necessary modules for normal operation of mpc85xx devices, such as the WatchGuard T30-W Signed-off-by: Damien Mascord <tusker@tusker.org>
Some devices need the DTB written to a specific destination seperate from kernel partition. This patch allows for the dtb binary to be added alongside the kernel and rootfs inside the sysupgrade.bin Signed-off-by: Damien Mascord <tusker@tusker.org>
dcb26be
to
d99cc3f
Compare
This patch adds support for the WatchGuard T30-W. Specifications: * SoC: 800Mhz Freescale P1011 e500v2 * RAM: 1 GiB DDR3L, 32-bit, ECC off * Flash: 4MB SPI NOR + 8GB SD Card * WiFi: QCA9880 802.11AC 1750 * Ethernet: 5 x 1GbE on Marvell 88E6171R * UART: 115200,8,N,1 RJ45 console MAC address: ASCII text found at /dev/mtd0 at 0x1830 (wan), 0x1844 (lan1), 0x1858 (lan2), 0x186C (lan3), 0x1880 (lan4), 0x1894 (wifi) Installation procedure: * Power off, open case, and remove SD Card * Power on, and let it fail to boot SYSA * Put back in SD card (original or pre-partitioned card) * It will drop to uboot prompt * setenv ipaddr 192.168.1.1 * setenv serverip 192.168.1.2 * tftpboot openwrt-mpc85xx-p1020-watchguard_t30-w-initramfs-kernel.bin * bootm * scp openwrt-mpc85xx-p1020-watchguard_t30-w-squashfs-sysupgrade.bin \ root@192.168.1.1:/tmp * sysupgrade /tmp/openwrt-mpc85xx-p1020-watchguard_t30-w-squashfs-\ sysupgrade.bin Signed-off-by: Damien Mascord <tusker@tusker.org>
d99cc3f
to
a8881a1
Compare
Hi, Ive been running my home network off a T30 for a year now (it was three but one blew up and the other I bricked. Happy to test if this helps, though I know I'm late to this. |
@@ -45,6 +45,7 @@ CONFIG_SWCONFIG_B53_PHY_DRIVER=y | |||
# CONFIG_SWCONFIG_B53_PHY_FIXUP is not set | |||
# CONFIG_SWCONFIG_B53_SPI_DRIVER is not set | |||
# CONFIG_SWCONFIG_B53_SRAB_DRIVER is not set | |||
CONFIG_TARGET_ROOTFS_EXT4FS=y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, this is an OpenWrt Config symbol in a Linux Kernel config? I don't think you can force it that way.
What you can do is to specify the FILESYSTEMS variable in the image/p1020.mk.
However, from what I can see this setup with ext4 looks odd. Wouldn't it be possible to do a setup like the MyBook Live does? It too needs a (special v1) ext2 boot partition for the linux image + dtb since the uboot can't boot of anything else. The main rootfs is configurable as either ext4 or that squashfs + f2fs (like the RPI). This should also cut down on your special sysupgrade script. Or is there something that prevents doing this?
0x0 0x20000000>, | ||
<0x1000000 0x0 0x0 0x0 0xffc00000 | ||
0x0 0x10000>; | ||
clock-frequency = <0x1fca055>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
33 1/3 MHz?
(there's also another one above)
0x0 0x10000>; | ||
clock-frequency = <0x1fca055>; | ||
interrupt-parent = <&mpic>; | ||
interrupts = <0x10 0x02>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That 0x02 is likely a Interrupt flag. Could it be IRQ_TYPE_EDGE_FALLING?
(there seems to be more. I'm not that familiar with mpc85xx but these flags are present on all architectures.)
spi-max-frequency = <40000000>; | ||
m25p,fast-read; | ||
|
||
partition@spi_mtd_0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm, I wonder if dtc didn't throw a warning here. Since there is a reg, it should have the unit-address: partition@0
(I guess since you copy it from the manufacturer, they did you dirty. Thing is, I can't say if this would pass upstream or not.)
KERNEL = kernel-bin | gzip | fit gzip $(KDIR)/image-$$(DEVICE_DTS).dtb | ||
IMAGES := fdt.bin sysupgrade.bin sysupgrade-dtb.bin | ||
IMAGE/fdt.bin := append-dtb | ||
IMAGE/sysupgrade.bin := sysupgrade-dtb-tar \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If that one single ext2 has to stick around, maybe you could do the integration like x86 does and integrate the dtb right there. You could also make a something like the rpi does with their sd-card factory images for the (prepartitioned?) sd-card.
This patch adds support for the WatchGuard T30-W.
Specifications:
Installation procedure:
Signed-off-by: Damien Mascord tusker@tusker.org