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

kernel-firmware: use copy-firmware.sh #4329

Merged
merged 4 commits into from
May 31, 2020
Merged

kernel-firmware: use copy-firmware.sh #4329

merged 4 commits into from
May 31, 2020

Conversation

MilhouseVH
Copy link
Contributor

Since this commit, the symbolic links for latest Intel DSP firmware are no longer included in the repository, but will instead be created by running copy-firmware.sh (which processes the WHENCE file).

This PR avoids the following issue:

find: 'intel/dsp_fw_bxtn.bin': No such file or directory
find: 'intel/dsp_fw_cnl.bin': No such file or directory
find: 'intel/dsp_fw_glk.bin': No such file or directory
find: 'intel/dsp_fw_kbl.bin': No such file or directory
find: 'intel/dsp_fw_release.bin': No such file or directory

The copy-firmware.sh script does a good job, however it is dependent on the WHENCE file being correct. For this reason we need to add the WHENCE entries for the Broadcom nvram configuration files that are added as a patch (which is why the call to copy-firmware.sh takes place in post_patch() and not post_unpack()). I've left the WHENCE change as a separate patch in case it needs updating in future.

When comparing the contents of the latest linux-firmware (see #4327) against a directory created by copy-firmware.sh I didn't find any issues ie. no missing files that were of any significance - LICENCE files, NOTICE.txt, authentication signatures and other non-firmware files are not copied by the script, which might save a smidgen of space.

@MilhouseVH
Copy link
Contributor Author

This could be considered for back porting.

@HiassofT
Copy link
Member

As commented on Slack the brcmfmac oatch and as a consequence also the WHENCE patch in this PR should be dropped and the brcmfmac nvram files should be moved to another patckage that can then be included by the projects that need/want it.

This allows us to have a clean, patch free kernel-firmware package without having to worry about patch rebasing on bumps

@MilhouseVH
Copy link
Contributor Author

That sounds good. Can we merge this first, and handle that in a follow up PR?

@HiassofT
Copy link
Member

Is this PR that urgent? I'd rather avoid adding yet another patch that then needs to be removed in a subsequent PR. Remember: every patch added is bad :-)

@MilhouseVH
Copy link
Contributor Author

I'd view this PR as a bug fix. If we decide to bump linux-firmware in libreelec-9.2 then we'll have the same bug there. I'd like to get this tested and merged sooner than later so that we have the option of backporting it (with #4327).

@HiassofT
Copy link
Member

HiassofT commented Apr 16, 2020

Does this fix an urgent bug like some user reported breakage or a build breakage? If not there's no need to hurry and it's better to do it properly. Then we won't have to backport 2 PRs.

@MilhouseVH
Copy link
Contributor Author

OK. I don't have any plans to expand this PR beyond what it currently does. If you'd like to push commits to this PR you're most welcome.

@HiassofT
Copy link
Member

Sorry, don't have time to dig into this ATM.

@HiassofT
Copy link
Member

I now had a closer look at the brcmfmac firmware loader and the solution is quite easy: drop patches/kernel-firmware-02-add-brcmfmac-43xxx-configs.patch and also the WHENCE patch.

The nvram files are board specific and included in the kernel firmware repo. The fallback to some non-board-specific nvram file won't work in general so there's not much point in adding these.

See also this kernel commit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=eae8e50669e15002b195177212a6e25afbe7cf4d

@MilhouseVH
Copy link
Contributor Author

That isn't how I understand the original problem, to be honest.

This is the original forum post that led us to add the "generic" nvram files as the user has this nvram-related error on an x86_64 system:

[   12.074660] brcmfmac: brcmf_fw_map_chip_to_name: using brcm/brcmfmac43455-sdio.bin for chip 0x004345(17221) rev 0x000006
[   12.075734] usbcore: registered new interface driver brcmfmac
[   12.114266] brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac43455-sdio.txt failed with error -2

Granted this is with 4.19.4 which pre-dates the board-specific commit (added 5.0-rc1) however based on the wording of the commit if we don't include brcm/brcmfmac43455-sdio.txt somehow (either from this package, or a different package) then non-RPi systems such as x86_64 will continue to fail to find a usable brcmfmac43455-sdio nvram config, and be unable to use WiFi.

The forum user subsequently confirms that the addition of brcm/brcmfmac43455-sdio.txt allows WiFi to work on his system, so if we remove the patch(es) then we're going to break non-RPi systems.

At least, this is all my understanding of the kernel commit - yes, there's a generic nvram fallback but that will only work if a generic nvram file exists (which it doesn't, by default, hence the patch in this package).

@HiassofT
Copy link
Member

HiassofT commented Apr 17, 2020

No, this isn't quite how this works. The reason why no brcmfmac nvram files were in the kernel firmware repo back then is that there's no generic nvram file that will work with all boards (as the file is board specific).

This was subsequently fixed in the kernel and nvram files were added to the kernel firmware repo.

Back then we had no possibility to add firmware files via .config/firmware, which has been added, too.

So what we basically did is add a random nvram file, that worked for this one specific user, and it probably will fail for everyone else.

The correct solution now is that if there's no firmware available from the kernel repo the user will have to extract his nvram file from the windows driver and put it into .config/firmware - or nag the vendor to submit it to the firmware repo.

TL;DR: there's a reason why there are not fallback nvram files in the repo, and we shouldn't add them either

@MilhouseVH
Copy link
Contributor Author

OK, thanks for bearing with me. 😄

Patches dropped.

@HiassofT
Copy link
Member

Thanks! Can you squash the WHENCE patch commit and the brcmfmac drop commit? Then it should be good to go IMO

@MilhouseVH
Copy link
Contributor Author

Squashed.

Copy link
Member

@HiassofT HiassofT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@MilhouseVH
Copy link
Contributor Author

I've added an additional commit so that the original problem of non-existent firmware files does not go unnoticed in future. Now, the package will fail:

find: 'intel/dsp_fw_bxtn.bin': No such file or directory
find: 'intel/dsp_fw_cnl.bin': No such file or directory
find: 'intel/dsp_fw_glk.bin': No such file or directory
find: 'intel/dsp_fw_kbl.bin': No such file or directory
find: 'intel/dsp_fw_release.bin': No such file or directory
ERROR: Firmware pattern does not exist: intel/dsp_fw_{bxtn,cnl,glk,kbl,release}.bin
FAILURE: scripts/build kernel-firmware during makeinstall_target (package.mk)
*********** FAILED COMMAND ***********
${SCRIPTS}/build "${1}" "${PARENT_PKG}"
**************************************

@CvH CvH merged commit e9c689d into LibreELEC:master May 31, 2020
Copy link

@geeebse geeebse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brcm/brcmfmac43455-sdio

Copy link

@geeebse geeebse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants