| Message ID | 20210515001130.20654-2-avalentin@marcant.net |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for ZyXEL LTE3301-Plus | expand |
André Valentin <avalentin@marcant.net> writes: > This adds a new device id to the image tools. > > Signed-off-by: André Valentin <avalentin@marcant.net> > --- > tools/firmware-utils/src/zytrx.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/firmware-utils/src/zytrx.c b/tools/firmware-utils/src/zytrx.c > index 302efc6010..b3bbbe3c69 100644 > --- a/tools/firmware-utils/src/zytrx.c > +++ b/tools/firmware-utils/src/zytrx.c > @@ -96,6 +96,7 @@ static struct board_t { > uint32_t modelid; > } boards[] = { > { "MT7621A", "NR7101", 0x07010001 }, > + { "MT7621A", "LTE3301-PLUS", 0x03030001 }, > {} > }; Great! But I winder if my initial implementation was a bit on the stupid side. We might want to move those two fields out of the code and just provide them directly as input parameters instead of this indirect mapping. What do you think? Anyway, good to know that the remaining fields are constant. In particular the magic code "3 6035 122 0\n". This was identical in your stock firmware image? I wonder what it means... As you might have noticed, I guessed a lot when writing this tool :-) I got the GPL source drop for the NR7101 from ZyXEL a while later. But that didn't help much as the image header tool was provided only as binary. Pretty much expected, and nothing to complain about. They are still among the best by providing code that will build a complete working image, and including source for all the GPL parts AFAICS. Bjørn
Am 15.05.21 um 12:00 schrieb Bjørn Mork: > André Valentin <avalentin@marcant.net> writes: > >> This adds a new device id to the image tools. >> >> Signed-off-by: André Valentin <avalentin@marcant.net> >> --- >> tools/firmware-utils/src/zytrx.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/tools/firmware-utils/src/zytrx.c b/tools/firmware-utils/src/zytrx.c >> index 302efc6010..b3bbbe3c69 100644 >> --- a/tools/firmware-utils/src/zytrx.c >> +++ b/tools/firmware-utils/src/zytrx.c >> @@ -96,6 +96,7 @@ static struct board_t { >> uint32_t modelid; >> } boards[] = { >> { "MT7621A", "NR7101", 0x07010001 }, >> + { "MT7621A", "LTE3301-PLUS", 0x03030001 }, >> {} >> }; > > > Great! > > But I winder if my initial implementation was a bit on the stupid > side. We might want to move those two fields out of the code and just > provide them directly as input parameters instead of this indirect > mapping. What do you think? It think we should move it outside. I expect ZyXEL to produce more of this stuff. > > Anyway, good to know that the remaining fields are constant. In > particular the magic code "3 6035 122 0\n". This was identical in your > stock firmware image? I wonder what it means... It is. But a real test with a transition from original firmware has to be done yet. Verified is replacement with update via bootloader. > > As you might have noticed, I guessed a lot when writing this tool :-)But the right guess :-) Thank you for your effort in this. > > I got the GPL source drop for the NR7101 from ZyXEL a while later. But > that didn't help much as the image header tool was provided only as > binary. Pretty much expected, and nothing to complain about. They are > still among the best by providing code that will build a complete > working image, and including source for all the GPL parts AFAICS. Yes, that's the reason why we use ZyXEL devices. The always provide full source, but with a locked image builder binary. That's why I could release this so quick after your commit. I just waited for someone able to implement the image builder. Kind regards, André
Andre Valentin <avalentin@marcant.net> writes: > Am 15.05.21 um 12:00 schrieb Bjørn Mork: > >> But I winder if my initial implementation was a bit on the stupid >> side. We might want to move those two fields out of the code and just >> provide them directly as input parameters instead of this indirect >> mapping. What do you think? > > It think we should move it outside. I expect ZyXEL to produce more of this stuff. Yes. I do have an LTE5398 flashed with LTE7490 firmware and it is almost identical to the NR7101. I'll change the zytrx tool when adding support for that. And I just got a Coverity report pointing out some bad error handling. Will fix that too. But I'll wait until your patches go in so you don't have to rebase. >> Anyway, good to know that the remaining fields are constant. In >> particular the magic code "3 6035 122 0\n". This was identical in your >> stock firmware image? I wonder what it means... > > It is. But a real test with a transition from original firmware has to > be done yet. Verified is replacement with update via bootloader. From what I could tell on the NR7101, the only difference in validation was related to the ZyXEL version string. And that seemed almost unintentional: My first attempts with a free form string made the web update tool go into an infinite loop after loading the image. That's why I made the initramfs version string slighly different. But it's still far from the original so it could cause problems.. There wasn't any other differences in the validation between stock and bootloader. In particular, neither verifies the strange "kernelChksum" field. It's just reported by the bootloader. I still have no idea how to compute it. Could not make any reasonable checksum match the samples I have. Not that another checksum would be reasonable at all - there are too many as it is:-) Just wondering: Did your stock firmware also come with that DebugFlag lock-down thing? Seems quite risky to me. Definitely not something we can have along with a writable rootfs. Bjørn
diff --git a/tools/firmware-utils/src/zytrx.c b/tools/firmware-utils/src/zytrx.c index 302efc6010..b3bbbe3c69 100644 --- a/tools/firmware-utils/src/zytrx.c +++ b/tools/firmware-utils/src/zytrx.c @@ -96,6 +96,7 @@ static struct board_t { uint32_t modelid; } boards[] = { { "MT7621A", "NR7101", 0x07010001 }, + { "MT7621A", "LTE3301-PLUS", 0x03030001 }, {} };
This adds a new device id to the image tools. Signed-off-by: André Valentin <avalentin@marcant.net> --- tools/firmware-utils/src/zytrx.c | 1 + 1 file changed, 1 insertion(+)