Confirmed ... MAC address (WD-03, OEM install) is at 0x28, not 0x4000 => one more common item. LOL!
Will update, amend push 
Missed it.
And here are my additions! 
Did you think about my proposals?
More examples about DTS and image definition consolidation:
No worries! Just made the ethernet mac address common also.
So a single recipe then, right? I like it! Will incorporate it.
No big issue here. Should I also change HooToo to ht-tm05 (https://openwrt.org/toh/hootoo/hootoo_ht-tm05)? If so, will do this on this new branch (and coming PR) ... rather than mess with the existing one, trying to get that one closed out, it's been going a long time 
In the commit message you mean?
On the check marks, I think you mean that you agree, those ones are done and look good ... do I have that right?
Thanks!
Single recipe is another, but yeah.
I meant the compatible, model and others - see my quoted comment! 
It would be nice to check how does sysupgrade from older OpenWrt releases (19.xx, 18.xx) to this PR work.
If it soft-bricks, then it would be nice to re-test with changed compatible strings: does it refuse to upgrade?
Keep it as is, for now ...
There is another HooToo page on OpenWrt Wiki which also need a rework: HooToo TM02 
I hope not just that PR but this PR lands before 20.xx branches off.
I mean changing the model in the DTS file.
Those check marks are done with your latest commit.
What's wrong with those pages?
All these devices think that the mtd2, Config partition is the u-boot-env partition, but it isn't.
And if someone got a serial connection and do a saveenv it will destroy the OpenWrt installation.
Because the real u-boot-env partition is the params partition:
dev: size erasesize name
mtd0: 00800000 00010000 "ALL"
mtd1: 00030000 00010000 "Bootloader"
mtd2: 00010000 00010000 "Config"
mtd3: 00010000 00010000 "Factory"
mtd4: 00180000 00010000 "Kernel_RootFS"
mtd5: 00010000 00010000 "params"
mtd6: 00010000 00010000 "user_backup"
mtd7: 00010000 00010000 "user"
mtd8: 00600000 00010000 "Rootfs"
See discussion at the HooToo TM05 PR!
Another bad thing that these HooToo / RAVPower devices are limited to 1536 K kernel size: the u-boot reads only 0x180000 bytes at start and bad crc occurs when OpenWrt kernel exceeds this.
For example: Ravpower wd03 does not start with openwrt master - #9 by jeff
But we have a fix for these issues now 
1 Like
Right, so the recipe update (consolidation), and the name change - did get that. Let me make those changes, push a new commit for you to look at.
I can check that - but I know you can force, to ignore the results of that check. That's how I put HooToo firmware on RAVPower (and vice versa).
Will do.
Me too! No idea how to get the HooToo one in place 
. Then this one on top of course, but it should be easier.
OK, understood!
1 Like
OK, @xabolcs, I think your proposed changes are all incorporated, and pushed to my branch. Please look at let me know if you agree! And FYI, I had to force the sysupgrade, the metadata check worked, and complained about the new name ... 
.
The .patch trick in GitHub is very cool, thanks!
I meant check sysupgrade-ing from the latest bootable OpenWrt release, one of 19.07.x or 18.06.x to your image from this pull request.
Your sysupgrade.bin doesn't contain the LZMA Loader just the OKLI magic, I'm expecting soft-brick after forcing with sysupgrade -F.
Yep, agreed! And I have seen that happen before, so is there a reason to do it again? . Asking because after that cycle I have go to all the way back to OEM flash (programmer) again, so there is some pain involved .
Thanks!
If you confirm the following, then no need to re-test:
- simple
sysupgradestops from upgrading sysupgrade -Fsoft-bricks the device:- some sort of missing or bad magic
- it can be recovered with the TFTP recovery with
kernel(rootfsisn't exercised by the current OpenWrt releases)
Are you saying that using for TFTP recovery the kernel and rootfs files extracted from OEM firmware binary (fw-7620-WiFiDGRJ-RAVPower-RP-WD03-2.000.074) hard-bricks the device?
Review incoming.
In target/linux/ramips/dts/mt7620n_sunvalley.dtsi:
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
/dts-v1/;
#include "mt7620n.dtsi"
From the diff it looks like that you "re-licenced" the mt7620n_ravpower_wd03.dts file.
In target/linux/ramips/dts/mt7620n_sunvalley.dtsi:
leds {
compatible = "gpio-leds";
- green-wifi {
- label = "wd03:green:wifi";
- gpios = <&gpio2 0 GPIO_ACTIVE_HIGH>;
+ led_power: power {
+ label = "tm05:blue:power";
+ gpios = <&gpio3 0 GPIO_ACTIVE_LOW>;
+ default-state = "on";
};
- blue-wifi {
- label = "wd03:blue:wifi";
- gpios = <&gpio3 0 GPIO_ACTIVE_HIGH>;
+ wifi {
+ label = "tm05:green:wifi";
+ gpios = <&gpio2 0 GPIO_ACTIVE_HIGH>;
+ linux,default-trigger = "phy0tpt";
};
};
Those tm05 names in the wifi labels look weird in a DTSI. 
How about rename the dtsi to mt7620n_sunvalley_filehub.dtsi and use filehub:blue:power, filehub:green:wifi, ... as LED alias?
That would be non-sense, as the ongoing HooToo PR uses tm05 as label. 
Are you able to define here the LEDs without labels and define with labels only int dts?
I know nothing about DTS include. 
In file target/linux/ramips/image/lzma-loader/Makefile:
@@ -8,7 +8,7 @@
include $(TOPDIR)/rules.mk
-LZMA_TEXT_START := 0x80a00000
+LZMA_TEXT_START :=
LOADER := loader.bin
LOADER_NAME := $(basename $(notdir $(LOADER)))
LOADER_DATA :=
This change is needless, just confuses the reviewer.
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/dts-v1/;
+
+#include "mt7620n_sunvalley.dtsi"
+
+/ {
+ compatible = "ravpower,rp-wd03", "ralink,mt7620n-soc";
+ model = "Ravpower RP-WD03 FileHub N300";
+};
Please use RAVPower in model!
Including the "N300" prefix is a nice addition!
In file target/linux/ramips/image/mt7620.mk:
@@ -908,14 +897,14 @@ define Device/ralink_mt7620a-v22sg-evb
endef
TARGET_DEVICES += ralink_mt7620a-v22sg-evb
-define Device/ravpower_wd03
+define Device/ravpower_rp-wd03
+ $(Device/sunvalley_filehub)
SOC := mt7620n
- IMAGE_SIZE := 7872k
DEVICE_VENDOR := Ravpower
- DEVICE_MODEL := WD03
- DEVICE_PACKAGES := kmod-usb2 kmod-usb-ohci
+ DEVICE_MODEL := RP-WD03
+ LZMA_TEXT_START := 0x81800000
endef
-TARGET_DEVICES += ravpower_wd03
+TARGET_DEVICES += ravpower_rp-wd03
define Device/sanlinking_d240
SOC := mt7620a
Please update DEVICE_VENDOR too!
Sorry, not following that one . Can you clarify? Thanks!
Sure, will do! Agree with you.
[quote="xabolcs, post:181, topic:68420"]
This change is needless, just confuses the reviewer.
If I don't do this, it overtakes my other parameters, no? Thinking it should be removed here anyways, this value is not used => leaving it is more confusing (not used), no?
Good catch, thanks!
Yes!
Nope - and sorry if I confused you. I just meant that using v19.07 overwrites the u-boot-env partition ... agreed? And trying from u-boot to then save env variables will break (OpenWrt) rootfs ... again, with v19.07. We (you!) fixed all that fortunately!
Updated and amended commit - with most of what you noted above. Just need clarification on a couple. Thanks!
Look at the current DTS! It doesn't have any SPDX line!
Introducing HooToo TM05 with tm05 LED labels, and renaming a few weeks later to filehub doesn't make sense, IMHO.
It would be nice to check the DTSI includes, or if it doesn't work, just differentiate the leds section!
Just try it! It will work!
This is a needless and an unrelated change. It confuses only the beginners, not the reviewer!
This change is a "Let refactor that LZMA Loader code" ... not a "Harmonize HooToo and RAVPower devices in mt720 subtarget:!
I thought about it, again.
How about just keeping it as "RAVPower RP-WD03"?
It would be line with the newly added RP-WD009, and would be closer tho the current/old name.
Yes, I'm aware of that - but Adrian commented, said this had to be added (it was blocking not to have it). And yes, some files are missing it - I think they are trying to get this fixed 
Ahh, so go back and change this on the HooToo TM-05 you mean? I can, no issue here.
Sort of 
. I struggled for a while, thinking this code was actually used (it's not) ... so why leave in something that is confusing and not adding value? In can put it back it, but I actually think that's more confusing .