ramips: RAVPower RP-WD03 updates, using OKLI loader, and other image improvements #3386
Conversation
|
@mpratt14 raised a question on this - is a single commit OK, or rather 2 ... correcting / aligning the device name first, then updating the device support. FYI, to hopefully make this one easier to review, it's very closely aligned to the recently added HooToo HT-TM05. Thanks! |
|
I guess the main reason for splitting into 2 commits is so that the DTSI is seen as a new file instead of a rename Also maybe rename the commit / PR to "add lzma loader to..." since the device was already in the makefile? |
target/linux/ramips/image/mt7620.mk
Outdated
| SOC := mt7620n | ||
| IMAGE_SIZE := 6144k | ||
| DEVICE_PACKAGES := kmod-usb2 kmod-usb-ohci kmod-i2c-ralink |
There was a problem hiding this comment.
move this right after "DEVICE_MODEL" so that it's not seen as a line change
There was a problem hiding this comment.
Good point, done - thanks!
| @@ -1,30 +1,16 @@ | |||
| /dts-v1/; | |||
|
|
|||
| // SPDX-License-Identifier: GPL-2.0-or-later OR MIT | |||
There was a problem hiding this comment.
an empty line between license comment and #include
target/linux/ramips/image/mt7620.mk
Outdated
| DEVICE_PACKAGES := kmod-usb2 kmod-usb-ohci kmod-i2c-ralink | ||
| DEVICE_VENDOR := RAVPower | ||
| DEVICE_MODEL := RP-WD03 | ||
| LZMA_TEXT_START := 0x81800000 |
There was a problem hiding this comment.
if LZMA_TEXT_START applies to both devices it should be in the common section
However I thought this was set recently as the default in master...so maybe just delete the line?
There was a problem hiding this comment.
Ummm ... so leave this then? Thanks!
There was a problem hiding this comment.
nah, its definitely the same value now as the default, so remove line
read the commit I relinked it
arrmo
force-pushed
the
ravpower-rp-wd03
branch
from
64600ea
to
12ac0cf
Compare
|
Commit (and PR) title is confusing, as this device is supported already by OpenWrt. |
| @@ -924,14 +915,16 @@ define Device/ralink_mt7620a-v22sg-evb | |||
| endef | |||
| TARGET_DEVICES += ralink_mt7620a-v22sg-evb | |||
|
|
|||
| define Device/ravpower_wd03 | |||
There was a problem hiding this comment.
You should touch this original recipe only when you touch the original .dts!
There was a problem hiding this comment.
Sorry, not quite understanding this one 😞. It seemed like folks wanted it changed / corrected, to match the vendor name better?
There was a problem hiding this comment.
@xabolcs can you rephrase what you mean? This commit changes both recipe and DTS at same time, unless you mean something else...
There was a problem hiding this comment.
@mpratt14 I meant exactly that.
Somehow I saw that the HooToo dts was renamed to dtsi.
There was a problem hiding this comment.
Yeah this is a feature of git where if you have a deleted file and a new file but they are more than 50% similar it counts it as a renamed file instead
I mentioned earlier and in forum that this makes it hard to review and understand what this commit does in the future. Solution would be to do all the renaming in a separate commit
There was a problem hiding this comment.
It's OK to me to see one of the dts as a rename, but I missed the other here.
I was looking this PR through app.
| @@ -953,6 +946,19 @@ define Device/sercomm_na930 | |||
| endef | |||
| TARGET_DEVICES += sercomm_na930 | |||
|
|
|||
| define Device/sunvalley_loader_okli | |||
There was a problem hiding this comment.
I prefer sunvalley_filehub as recipe name, and would include SOC, IMAGE_SIZE and DEVICE_PACKAGES recipe variables too.
There was a problem hiding this comment.
LOL - I changed this because @mpratt14 preferred this name. Easy for me to change it back, just need folks to agree 🤣. Thanks!
There was a problem hiding this comment.
Just let Adrian decide, I was following the pattern that he and I decided for my most recent PR
There was a problem hiding this comment.
Yep, no issue at all - and agreed. Easy change, if needed. Thanks!
There was a problem hiding this comment.
And my points are to save as many lines as possible, because they differ only in memory size.
|
@adschm the context for this PR is the following. This PR is for renewing support for the already supported "Ravpower WD03". As you can see in the description ("The implementation is based heavily off the HooToo HT-TM05, as the hardware The files for HooToo can be crossflashed to this RAVPower and it works as intended. And while it shares the same hardware, it shares the same issues too:
(And there is a HT-TM02 too which also shares these issues) Sure, this renewing could be done with In this PR @arrmo tries to create shared recipes and dtsi like in PR #3235 or like in EnGenius 202 commits (6decbf3 and 22caf30). |
|
Renaming the device (and it's compatible) from "Ravpower WD03" to "RAVPower RP-WD03" has the following purposes
|
Good point. Perhaps rename to something like update or improve (vs. add)? |
arrmo
force-pushed
the
ravpower-rp-wd03
branch
from
12ac0cf
to
c6c18a6
Compare
The RP-WD03 images (both sysupgrade and factory) are broken by the 1.5 MByte kernel size limit. If you have no clue about commit message, then try to get inspiration from below! One paragraph about the kernel size limitation. The second important thing is the misplaced You could write a half paragraph about it's content, why does it have Another paragraph about renaming the device: what does it buy us? Why does it worth it? After those you could write a few paragraph about the similarity between TM05 and WD03: common recipe, dtsi and the modified installation steps. |
| @@ -0,0 +1,26 @@ | |||
| // SPDX-License-Identifier: GPL-2.0-or-later OR MIT | |||
There was a problem hiding this comment.
This (and/or the dtsi) is a re-license, IMHO.
You have to ask all the contributors do they agree with the change!
There was a problem hiding this comment.
I have no issue removing this, to keep it as it was before - but I was told that this was missed in some cases before (an error, it happens), and now it has to be included. So added it based on that.
But I get your point, no issue. Let me add the folks who it seems contributed to this file before (if I have it right!). Any concerns with this addition ... @adschm, @mans0n, @mwarning, @981213?
Thanks!
There was a problem hiding this comment.
@mwarning aye (although, I do not remember contributing here..)
There was a problem hiding this comment.
I thought all files in the folder need license identifier, but DTSI does not have DTS tag, so DTS has both, DTSI only has license
There was a problem hiding this comment.
Grep for Signed-off-by: in the commit history!
Here is how GitHub sees them:
And here is git cli:
mt7620n_ravpower_wd03.dts
git log -- target/linux/ramips/dts/mt7620n_ravpower_wd03.dts | grep "Signed-off-by:" | sort | uniq
Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
Signed-off-by: Moritz Warning <moritzwarning@web.de>
Signed-off-by: Sungbo Eo <mans0n@gorani.run>
WD03.dts
git log -- target/linux/ramips/dts/WD03.dts | grep "Signed-off-by:" | sort | uniq
Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
Signed-off-by: Alex Maclean <monkeh@monkeh.net>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com> [split up]
Signed-off-by: INAGAKI Hiroshi <musashino.open@gmail.com>
Signed-off-by: Mathias Kresin <dev@kresin.me>
Signed-off-by: Matthias Badaire <mbadaire@gmail.com>
Signed-off-by: Petr Štetiar <ynezz@true.cz>
There was a problem hiding this comment.
@arrmo , yup, please keep the header and do the re-license work!
There was a problem hiding this comment.
But I get your point, no issue. Let me add the folks who it seems contributed to this file before (if I have it right!). Any concerns with this addition ... @adschm, @mans0n, @mwarning, @981213?
Acked-by: Adrian Schmutzler freifunk@adrianschmutzler.de
There was a problem hiding this comment.
Sorry, to understand - "re-license" work. Do you mean to get everyone on the lists above to approve this? Easier would be to remove it, go back the way it was 🤣.
|
I don't get the point here. Is this really adding a device or just altering an existing one? |
|
Sorry! That's on me - have been tied up at work lately, haven't gotten back to this. I was thinking folks would want the full outline in the commit, like the HooToo (as this is aligning the two somewhat, given very similar HW). But this is an update, not new HW. Let me try to clarify. Thanks! |
|
well, if this is not adding a device, but changing it, you should group the changes into commits based on what they change (i.e. partitioning, introduction of a DTSI, ...) |
arrmo
force-pushed
the
ravpower-rp-wd03
branch
from
c6c18a6
to
a3472ab
Compare
|
OK, updated - sorry for the delay. I tried to add in the information that @xabolcs mentioned above, and overall clean up the commit message. Please do comment if you still see issues. Thanks! |
|
@adschm There is 3 basic changes for this PR
so how many commits should there be? 1 or 2 or 3 or.... @arrmo btw rename the PR itself at the top right of the page (pencil button) |
👍
How about the following?
Last step can be split into two, if it helps. |
|
I don't see anything wrong with the license tag... |
|
What's the reason for the introduction of the new loader here? I doesn't seem to be broken? If you actually introduce the new loader, it makes sense to do the rename in the same commit or immediately afterwards, as that will prevent (probably broken) sysupgrade; however, the fact that this would break sysupgrade is actually a reason against introducing this change |
Current kernel exceeds 1.5 MB, vendor u-boot is unable to boot, as it reads only 1.5 MB: Bootlogs from forum: See my comment above! |
arrmo
changed the title
I had missed that - thanks! |
|
FYI, to the comments above - I had reported in the forum (and others had as well) => the current build (from master, in OpenWrt downloads) doesn't work. It soft-bricks the device. This PR addresses that problem, and aligns with the HooToo as well (very similar HW). Thanks! |
|
I've just disabled the device for buildbots. So, the remaining job seems to wrap this up properly into separate commits with adequate descriptions. |
That make sense - of course hits the area where I struggle the most, git-idiot (me!). 🤣 Thanks! |
|
Well, you will need to learn it at some point if you want to continue contributing. |
|
@arrmo dont be afraid to mess it up because you can always delete your commit and pull it from this github branch to start over, or you can copy your branch the way it is now to a new branch with |
Agreed! My challenge is when I split to multiple commits, then someone wants a change to a middle one - that's where I get messed up. But that aside ... what split is desired here? Just trying to understand before starting over 😄. Thanks! |
The key is "git rebase --interactive ..."
Sometimes I have the feeling that you don't actually read what we are posting here. There are at least two comments above that quite explicitly state what to do. That BTW was also the reason of the enormous number of comments on the Hootoo commit. If you just had read the stuff carefully, we would have finished much quicker and without the pain of explaining everything three times. |
I actually do read them very carefully, it's unfortunate that you think I don't. I admit, a lot of my confusion comes from different answers from different folks - and I admit, then I'm not sure what the right answer is (and what to do). Sorry if I'm not following the internal discussion fully, but I really am reading it (and trying to digest). I even had some above where I made a change to address specific comments from one person, only to have another want it done differently. Even above I see different thoughts on the split, so now I'm stuck - different opinions, no idea which one is right 😞. Thanks. |
So, you have to decide for one of the following options:
Apart from that, I don't really see a difference between the comments on the commit splitting conceptually. |
|
LOL - yes, agreed ... I have struggled with 1, I admit. I'll roll with 2, take my best guess. Won't keep everyone happy, which will mean a few comments I'm guessing. But will deal with it - trying to do the right thing here (my device is blown as part of the assembly / disassembly / debug, so this is to help others out, not me). Thanks. |
…improvements
The RAVPower RP-WD03 is a battery powered router, with an Ethernet and USB port.
Due due a limitation in the vendor supplied U-Boot bootloader, we cannot exceed
a 1.5 MB kernel size, as is the case with recent releases (i.e. post v19.07). This
breaks both factory and sysupgrade images. To address this, the updates here include
usage of the lzma loader (loader-okli) - to work around this limitation.
The improvements here also address the "misplaced" U-Boot environment partition,
which is located between the kernel and rootfs in the stock image / implementation.
This is addressed by making use of mtd-concat, to "piece together" sections of flash,
maximizing space available in the booted image.
Hardware support here is based heavily off the HooToo HT-TM05, as the hardware
is almost identical (except for RAM size), and is from the same vendor (SunValley).
Given the hardware commonalities, the RAVPower RP-WD03 and HooToo HT-TM05 common
elements in the dts files are also combined in a shared dtsi file.
Specifications:
SOC: MediaTek MT7620N
BATTERY: 6700mAh
WLAN: 802.11bgn
LAN: 1x 10/100 Mbps Ethernet
USB: 1x USB 2.0 (Type-A)
RAM: 32 MB
FLASH: GigaDevice GD25Q64, Serial 8 MB Flash, clocked at 50 MHz
Flash itself specified to 80 MHz, but speed limited by mt7620 SPI
fast-read enabled (m25p)
LED: Status LED (blue after boot, green with WiFi traffic
4 leds to indicate power level of the battery (unable to control)
INPUT: Power, reset button
MAC assignment based on vendor firmware:
2.4 GHz *:b4 (factory 0x04)
LAN/label *:b4 (factory 0x28)
WAN *:b5 (factory 0x2e)
Tested and working:
- Ethernet
- 2.4 GHz WiFi (Correct MAC-address)
- Installation from TFTP (recovery)
- OpenWRT sysupgrade (Preserving and non-preserving), through the usual
ways: command line and LuCI
- LEDs (except as noted above)
- Button (reset)
- I2C, which is needed for reading battery charge status and level
- U-Boot environment / variables (from U-Boot, and OpenWrt)
Installation:
- Download the needed OpenWrt install files, place them in the root
of a clean TFTP server running on your computer. Rename the files as,
- openwrt-ramips-mt7620-ravpower_rp-wd03-squashfs-kernel.bin => kernel
- openwrt-ramips-mt7620-ravpower_rp-wd03-squashfs-rootfs.bin => rootfs
- Plug the router into your computer via Ethernet
- Set your computer to use 10.10.10.254 as its IP address
- With your router shut down, hold down the power button until the first
white LED lights up.
- Push and hold the reset button and release the power button. Continue
holding the reset button for 30 seconds or until it begins searching
for files on your TFTP server, whichever comes first.
- The router (10.10.10.128) will look for your computer at 10.10.10.254
and install the two files. Once it has finished installation, it will
automatically reboot and start up OpenWrt.
- Set your computer to use DHCP for its IP address
Notes:
- U-Boot environment can be modified, u-boot-env is preserved on initial
install or sysupgrade
- mtd-concat functionality is included, to leave a "hole" for u-boot-env,
combining the OEM kernel and rootfs partitions
I would like to thank @mpratt14 and @xabolcs for their help getting the
lzma loader to work!
Signed-off-by: Russell Morris <rmorris@rkmorris.us>
arrmo
force-pushed
the
ravpower-rp-wd03
branch
from
a3472ab
to
8c0e3ed
Compare
|
Since I think it will be more efficient after all I have separated the changes and wrapped them up properly myself. The result can be found here: https://git.openwrt.org/?p=openwrt/staging/adrian.git;a=shortlog;h=refs/heads/ravpower Please critically and thoroughly review and finally test. A few questions that popped up:
|
It looks good at first sight! Thank you for wrapping up! For the second commit it would be nice to add
I don't know how OpenWrt could initialize it or it is a problem at all. I hope @arrmo could test it. But the u-boot is with |
You are specifically referring to overwritten uboot-env and mislabelled config here? |
Exactly. From Device page: More details in the discussion! |
Thanks very much! I was working on this in parallel, but obviously much slower at it. And this does help me understand how you like to see these, learning for the future.
Absolutely! FYI, I pulled down your ravpower branch, built it (all fine) ... but 1 issue noted so far => serial port speed is not right (which I can sort of work around). I did check, and it's correct in mt7620n.dtsi ... huh?!?! Will dig here, seems very odd. But re-confirmed, part way through boot it changes from 57600 (u-boot, startup), to 115200. Thanks again! |
|
BTW, if I can get the serial port working, I can capture serial port logs, just so folks have something to check over as well.
Not making sense to me yet. Hmmm. |
|
Found it! It's also in target/linux/ramips/dts/mt7620n_ravpower_rp-wd03.dts => need to remove it there, agreed. Thanks! |
|
That minor update (remove console from ravpower dts), and then it all looks good to me ... boots, okli-loader found, mtd-concat in place. Other things you can think of to check? And the serial log, Thanks! |
|
So, the console value added for rp-wd03 initially (here: 5ef79af) is wrong and needs to be changed to the default 57600 ...? |
|
Correct! Just like HooToo, it just needs to be removed in the device level dts (i.e. WD03 file), and let it default to the top level mt7620n.dtsi. I tried it here, and it's correct (that's how I got the serial log above ... if not I start to get garbled data part way through the boot). Make sense? If it helps, here is what I changed locally (from your branch), |
|
And factory 0x4000 indeed does not contain a valid MAC address? |
|
Correct - at least best I can tell 😆. The same as the HooToo, and here is a dump of the factory partition (rp-wd03), To make sure, I did a MAC lookup, and 00 1c c2 is in fact SunValley (Part II Research, Inc.). Nothing up at 0x4000, agreed? |
|
I couldn't agree more - thanks to all 3 of you, very much appreciated. And apologies for any grief I caused, a bit new to this, learning as I go. |
The RAVPower RP-WD03 is a battery powered router, with an Ethernet and USB port.
Vendor U-Boot limited to 1.5 MB kernel size, so use lzma loader (loader-okli).
The implementation is based heavily off the HooToo HT-TM05, as the hardware
is almost identical (except for RAM size), and is from the same vendor (SunValley).
Given the hardware commonalities, the RAVPower RP-WD03 and HooToo HT-TM05 common
elements in the dts files are also combined in a shared dtsi file.
Specifications:
SOC: MediaTek MT7620N
BATTERY: 6700mAh
WLAN: 802.11bgn
LAN: 1x 10/100 Mbps Ethernet
USB: 1x USB 2.0 (Type-A)
RAM: 32 MB
FLASH: GigaDevice GD25Q64, Serial 8 MB Flash, clocked at 50 MHz
Flash itself specified to 80 MHz, but speed limited by mt7620 SPI
fast-read enabled (m25p)
LED: Status LED (blue after boot, green with WiFi traffic
4 leds to indicate power level of the battery (unable to control)
INPUT: Power, reset button
MAC assignment based on vendor firmware:
2.4 GHz *:b4 (factory 0x04)
LAN/label *:b4 (factory 0x28)
WAN *:b5 (factory 0x2e)
Tested and working:
ways: command line and LuCI
Installation:
of a clean TFTP server running on your computer. Rename the files as,
white LED lights up.
holding the reset button for 30 seconds or until it begins searching
for files on your TFTP server, whichever comes first.
and install the two files. Once it has finished installation, it will
automatically reboot and start up OpenWrt.
Notes:
install or sysupgrade
combining the OEM kernel and rootfs partitions
I would like to thank @mpratt14 and @xabolcs for their help getting the
lzma loader to work!
Signed-off-by: Russell Morris rmorris@rkmorris.us