Skip to content
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

Add support for FritzBox 5491 (GPON fiber) #145

Merged
merged 2 commits into from Apr 11, 2019
Merged

Add support for FritzBox 5491 (GPON fiber) #145

merged 2 commits into from Apr 11, 2019

Conversation

cawidtu
Copy link
Contributor

@cawidtu cawidtu commented Mar 30, 2019

I own a FritzBox 5491 and found it to be almost identical to the 7490 model. My commits to support Freetz for the 5491 are based on the functionality already available for the 7490. Has been working stably for several weeks. Only requurement: The kernel sources have to be extracted from the 5491 tarball and uploaded to the servers usually accessed by Freetz (only relevant for "replace kernel").

@er13
Copy link
Member

er13 commented Mar 31, 2019

  • Missing FREETZ_TYPE_FIBER changes - the box is neither DSL nor CABLE nor LTE nor WLAN_REPEATER (s. config/avm/features.in)
  • What are the differencies between 7490.07.01 and 5491.07.01 kernel sources? Is it really necessary to use a separate kernel-sources-package? Could delta-to-7490 method be used?
  • Unnecessary permission changes
  • Wrong/unnecessary FRITZ.Box_6840_LTE.105.06.*.image changes in ce5f152 (probably wrong merge)
  • Separate download locations for DE / EN but just one file / checksum => tools/verify-dls.sh won't work. Please use the same scheme like e.g. 4040 [Edit: correction 2 identical default "FRITZ.Box_5491.de-en-es-it-fr-pl.171.07.01.image" entries, what for]
  • Unnecessary whitespace changes

Please fix and resubmit.

@cawidtu
Copy link
Contributor Author

cawidtu commented Apr 1, 2019

Thank you for your feedback. I will take care of all issues a.s.a.p. Regarding item 2: I made a diff -ur on the linux-3.10 directories from sources of the 7490 and 5491, version 07.01, and obtained a patch file being 768 kilobytes in size. Would you prefer applying this patch instead of hosting the individual sources of 5491?

@er13
Copy link
Member

er13 commented Apr 3, 2019

As already asked in the 1st response - what are the differences between 7490.07.01 and 5491.07.01 kernel sources? (Try to) describe them with your own words.

There are a lot of differences which are completely negligible, e.g. (generation) timestamps in AVM module sources or even code changes in AVM modules. Freetz doesn't replace AVM modules, so all the differences within the corresponding sources are of no matter.

If the diffs are however not negligible then a separate tarball is the preferred way.

@cawidtu
Copy link
Contributor Author

cawidtu commented Apr 4, 2019

You challenge me since I am not really confident with Freetz' internals. But looking into the changes, only AVM-related files seem to be affected. The list of all differing files looks as follows:

linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_gen_debug.c
linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_gen_endian.h
linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_gen_enum_range.c
linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_gen_enum_range.h
linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_gen_flow.dot
linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_gen_types.h
linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_perl_convert.pm
linux-3.10/drivers/char/avm_new/avm_event.h
linux-3.10/include/linux/avm_event.h
linux-3.10/include/uapi/linux/avm_event.h

The complete diff is also attached.
diff-firm-07.01-7490-5491-sources.zip

So it seems we just take the 7490 kernel sources if "replace kernel" is selected?

@er13
Copy link
Member

er13 commented Apr 4, 2019

In general the code changes under drivers/char/avm_new and drivers/char/avm_new/avm_dist_event are not negligible. The code there gets built into the kernel (controlled by CONFIG_AVM_SAMMEL, which is y in all AVM kernel .config's).

In some cases one however can ignore the changes there as these are just the result of a reorder of function declarations/definitions, i.e. the same functions but in some other order, don't ask me why AVM does it.

Feel free to go for the patch-based alternative. The compressed version of the patch is quite small and could be added to the repository. The only thing we have to do before that is to teach tools/freetz_patch to support the compressed patches.

@er13
Copy link
Member

er13 commented Apr 4, 2019

Btw: I see make/linux/configs/avm/config-vr9-5491_07.01 in your commits, but I don't see make/linux/configs/freetz/config-vr9-5491_07.01. Why is it missing?

@cawidtu
Copy link
Contributor Author

cawidtu commented Apr 4, 2019

Probably a mistake. I use a file make/linux/configs/freetz/config-vr9-5491_07.01 that is identical to the config-vr9-7490_07.01 in the same directory. Gonna add it to my list for the revised PR.

@cawidtu
Copy link
Contributor Author

cawidtu commented Apr 6, 2019

Still work in progress

@PeterPawn
Copy link
Contributor

PeterPawn commented Apr 6, 2019

Still work in progress

Kleiner Tipp von mir ... die Änderungen am Freetz-Master nicht als "merge commit" in den eigenen Branch übernehmen, sondern immer mittels "rebase". Das erspart solche "leeren" Commits, von denen man oben zwei sehen kann.

Anders als bei der Übernahme eines PR in den Master braucht es auch die (Zusatz-)Information aus diesem "merge commit", wann die Änderungen übernommen wurden, nicht wirklich - damit ist der eigentlich komplett überflüssig. Und wenn das noch eine Weile so geht, bis der PR "ready for merge" ist und Du noch einige Male den Master-Stand nachziehen willst, werden das immer mehr dieser "merge commits".

Am besten macht man es so (oder zumindest so ähnlich) - "upstream" ist das Freetz-Master-Repo, "origin" bei mir ein eigenes lokales Repo und "github" mein Fork auf GitHub:

vidar:/home/GitHub/YourFreetz $ git fetch upstream
remote: Enumerating objects: 15, done.
remote: Counting objects: 100% (15/15), done.
remote: Compressing objects: 100% (8/8), done.
remote: Total 15 (delta 7), reused 13 (delta 7), pack-reused 0
Unpacking objects: 100% (15/15), done.
From https://github.com/Freetz/freetz
   96328a7e5..539a24715  master     -> upstream/master
vidar:/home/GitHub/YourFreetz $ git checkout master
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
vidar:/home/GitHub/YourFreetz $ git pull --rebase
Already up to date.
Current branch master is up to date.
vidar:/home/GitHub/YourFreetz # git pull --rebase upstream master
From https://github.com/Freetz/freetz
 * branch                master     -> FETCH_HEAD
Updating 96328a7e5..539a24715
Fast-forward
 FIRMWARES                                                           | 10 +++++++++-
 config/avm/availability.in                                          |  8 ++++++++
 config/mod/download.in                                              | 36 ++++++++++++++++++++++++++++++------
 make/linux/patches/2.6.39.3.x86/989-no-error-unused-parameter.patch | 11 +++++++++++
 4 files changed, 58 insertions(+), 7 deletions(-)
 create mode 100644 make/linux/patches/2.6.39.3.x86/989-no-error-unused-parameter.patch
Current branch master is up to date.
vidar:/home/GitHub/YourFreetz $ git log -n 3 | cat
commit 539a24715f05f21b825863eb5e44b98b69ff97c8
Author: Stefan Mischke <survivorx@gmx.de>
Date:   Sat Apr 6 11:13:24 2019 +0200

    PUMA6/kernel: Add a patch for drivers/net/avm_cpu_connect/Makefile which adds the parameter "-Wno-error=unused-parameter" to fix the compile error reported in #147. (#147, #149)

commit 18ee5bb9888a326c767be43f2c48444ceb17786c
Author: PrinzVonBillAir <PrinzVonBillAir@users.noreply.github.com>
Date:   Sat Apr 6 11:01:23 2019 +0200

    Version bump for 7590 labor and adding labor firmware support for 1750E/4040/6490 Cable/6590 Cable/6890 LTE/7530/7560/7580 (#144)

    1750, 4040, 6490, 6590, 6890, 7530, 7560, 7580: add support for labor firmware

    7590: bump labor firmware version to the latest one

commit 96328a7e50851976aab004e00e28e1ff655cb5e2
Author: Eugene Rudoy <gene.devel@gmail.com>
Date:   Thu Apr 4 21:29:05 2019 +0200

    autossh: bump version to 1.4g

    https://www.harding.motd.ca/autossh/CHANGES.txt
vidar:/home/GitHub/YourFreetz $ git push
Enumerating objects: 26, done.
Counting objects: 100% (26/26), done.
Delta compression using up to 2 threads
Compressing objects: 100% (15/15), done.
Writing objects: 100% (15/15), 2.64 KiB | 676.00 KiB/s, done.
Total 15 (delta 10), reused 0 (delta 0)
To git:/srv/git/YourFreetz.git
   96328a7e5..539a24715  master -> master
vidar:/home/GitHub/YourFreetz $ git push github
Enumerating objects: 26, done.
Counting objects: 100% (26/26), done.
Delta compression using up to 2 threads
Compressing objects: 100% (15/15), done.
Writing objects: 100% (15/15), 2.64 KiB | 676.00 KiB/s, done.
Total 15 (delta 10), reused 0 (delta 0)
remote: Resolving deltas: 100% (10/10), completed with 9 local objects.
To github.com:PeterPawn/YourFreetz.git
   96328a7e5..539a24715  master -> master
vidar:/home/GitHub/YourFreetz $

Damit sind die Änderungen am Freetz-Master übernommen und es gibt trotzdem keinen zusätzlichen "merge commit" in der History ... diese sind (da sie eben auch den Hash eines nachfolgenden Commits verändern) an dieser Stelle nicht nur "häßlich" - nein, sie sorgen auch für zusätzliche "Verwirrung" und irgendwann wirst Du den gesamten PR ohnehin noch einmal "rebasen" müssen, wenn er denn fertig ist.

Man kann auch die beiden derzeitigen "merge commits" noch entfernen, indem man einfach ein git rebase 539a24715 macht und damit die (echten) Commits erneut auf die History ab 539a247 (dem Commit vor dem ersten zu entfernenden "merge") anwendet. Allerdings muß man dann den lokalen Stand mittels "git push -f" mit dem jeweiligen Repo synchronisieren, weil sich die History ja auf getrennten Wegen befindet, wenn die "merge commits" rausfallen und man die zuvor schon "veröffentlicht" hatte.

@er13
Copy link
Member

er13 commented Apr 6, 2019

@cawidtu:
Could you please rebase your branch and squash all the commits done so far. It's hard to review the code with errors and corrections of the errors. Thanks.

@er13
Copy link
Member

er13 commented Apr 6, 2019

config/avm/hardware.in

forgot to remove || \ after 7490

FREETZ_TYPE_FIBER

make sure FREETZ_TYPE_DSL is not selected

md5-checksum for the source package

FREETZ_AVM_SOURCE_7490_07_01 => FREETZ_AVM_SOURCE_7490_07_01 || FREETZ_AVM_SOURCE_5491_07_01 is missing

@cawidtu
Copy link
Contributor Author

cawidtu commented Apr 6, 2019

Danke. Kümmere mich schnellstmöglich darum, kann aber gerade nicht.

@cawidtu
Copy link
Contributor Author

cawidtu commented Apr 6, 2019

Thank you for your patience. I was not aware of the fact that we need to duplicate the checksum of the 7490 sources under FREETZ_AVM_SOURCE_5491_07_01. But in hindsight it makes sense. I unified all my changes into one commit. Hope it is useful now. We are still dealing with a quite big kernel delta patch ( 000-delta-from-7490.07.01.patch is about 680 k) but I am not sure whether compressed patch files are supported.

@er13
Copy link
Member

er13 commented Apr 7, 2019

Could you please as already asked in #145 (comment) change the line https://github.com/Freetz/freetz/blob/master/config/mod/download.in#L154 from

default "9dc643cfd52afb67a4ea169cfe82551d"  if FREETZ_AVM_SOURCE_7490_07_01

to

default "9dc643cfd52afb67a4ea169cfe82551d"  if FREETZ_AVM_SOURCE_7490_07_01 || FREETZ_AVM_SOURCE_5491_07_01

instead of adding the checksum for the 2nd time. This way it's easier to see that multiple source packages share the same basis.


Could you please while adding FREETZ_FOO_5491_BAR take care of the proper sorting, i.e. a 5491-entry should always come before the 7490 one.


Empty line with comment sign before default "FRITZ.Box_5491.de-en-es-it-fr-pl.171.07.01.image" is missing.

@cawidtu
Copy link
Contributor Author

cawidtu commented Apr 8, 2019

Fixed these issues. Sorry I misunderstood the instruction regarding the checksum.

@er13
Copy link
Member

er13 commented Apr 9, 2019

Thanks!

@cawidtu
Copy link
Contributor Author

cawidtu commented Apr 10, 2019

Great that compressed patches are supported now. I followed the conventions suggested above and created a patch file named 000-7490.07.01-5491.07.01-delta.patch.xz (001 prefix was already taken). Kernel is successfully built and patched, see the excerpt from the log below. Hope it is fine.

...
#firmware version specific patches
applying patch file make/linux/patches/3.10.107/5491_07.01/000-7490.07.01-5491.07.01-delta.patch.xz
patching file linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_gen_debug.c
patching file linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_gen_endian.h
patching file linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_gen_enum_range.c
patching file linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_gen_enum_range.h
patching file linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_gen_flow.dot
patching file linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_gen_types.h
patching file linux-3.10/drivers/char/avm_new/avm_dist_event/avm_event_perl_convert.pm
patching file linux-3.10/drivers/char/avm_new/avm_event.h
patching file linux-3.10/drivers/char/Piglet_noemif/init_Piglet
patching file linux-3.10/drivers/char/Piglet_noemif/init_Piglet_noemif
patching file linux-3.10/drivers/char/Piglet_noemif/Kbuild
patching file linux-3.10/drivers/char/Piglet_noemif/Makefile.24
patching file linux-3.10/drivers/char/Piglet_noemif/Makefile.26
----------------------------------------------------------------------
applying patch file make/linux/patches/3.10.107/5491_07.01/001-avm-menuconfig-symbols.patch
...

@er13
Copy link
Member

er13 commented Apr 10, 2019

Great! Thanks a lot!

Now the very last comment ;-)

Could you please add 5491 to FIRMWARES.

@cawidtu
Copy link
Contributor Author

cawidtu commented Apr 11, 2019

Done. Please check whether my change to the preceding section about 5xxx (all allocated to Fritz!Box Fon) makes sense.

@er13 er13 merged commit 4ecd7af into Freetz:master Apr 11, 2019
@er13
Copy link
Member

er13 commented Apr 11, 2019

Everything looks good. Thanks a lot!

@jschwartzenberg
Copy link

I have the 5490 (AON fiber) here. Indeed very similar to the 7490. I hope I can look at this PR and add support for it as well.

er13 added a commit that referenced this pull request May 3, 2019
er13 added a commit that referenced this pull request May 3, 2019
PeterPawn pushed a commit to PeterPawn/YourFreetz that referenced this pull request May 7, 2019
PeterPawn pushed a commit to PeterPawn/YourFreetz that referenced this pull request May 7, 2019
fda77 pushed a commit to Freetz-NG/freetz-ng that referenced this pull request Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants