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

Use firmware load path from sysfs #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

minlexx
Copy link

@minlexx minlexx commented Jan 8, 2024

Some linux distributions override firmware loading path using /sys/module/firmware_class/parameters/path.
Kernel supports it, teach pd-mapper to support it too.

Kernel supports only single firmware path specified in firmware_class path. In postmarketOS we created a service script msm-firmware-loader that runs in early boot stage and symlinks all firmware it can find on various device partitions into a single directory, which is then written into /sys/module/firmware_class/parameters/path.

With this patch (and some extra logging output enabled) I can see that pd-mapper is able to successfully find all JSON files at startup and replies to messages when modem remoteproc is started:

Got firmware path from sysfs: /lib/firmware/msm-firmware-loader/target
 - found: /lib/firmware/msm-firmware-loader/target/./adspr.jsn
 - found: /lib/firmware/msm-firmware-loader/target/./adsps.jsn
 - found: /lib/firmware/msm-firmware-loader/target/./adspua.jsn
 - found: /lib/firmware/msm-firmware-loader/target/./cdspr.jsn
 - found: /lib/firmware/msm-firmware-loader/target/./modemr.jsn
 - found: /lib/firmware/msm-firmware-loader/target/./modemuw.jsn
...
Sent reply to request name: tms/pddump_disabled
Sent reply to request name: tms/pdr_enabled
Sent reply to request name: avs/audio
Sent reply to request name: kernel/elf_loader
Sent reply to request name: avs/audio

@minlexx minlexx force-pushed the respect-firmware-sysfs-path branch from be0daaa to f00f4d7 Compare January 8, 2024 21:49
Some distributions override firmware loading path
using /sys/module/firmware_class/parameters/path.
Kernel supports it, teach pd-mapper to support it too.

Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org>
@minlexx minlexx force-pushed the respect-firmware-sysfs-path branch from f00f4d7 to 96cb6f4 Compare April 13, 2024 20:01
@minlexx
Copy link
Author

minlexx commented Apr 13, 2024

This PR suffered from the same issue as tqftpserv one for using path from sysfs: dirname() function modifies its argument by inserting NUL-chars to separate directory from filename. Original code only called dirname() once, in initial version of this MR it was called twice on the same string, possibly corrupting the path 😢

Now it is fixed in the same way as in linux-msm/tqftpserv@98d162c - dirname is only called once on a copy of argument, and firmware_path is used instead in places where dirname() was used before.

@@ -220,6 +237,8 @@ static int pd_enumerate_jsons(struct assoc *json_set)
size_t len;
size_t n;

read_fw_path_from_sysfs(fw_sysfs_path, sizeof(fw_sysfs_path));
Copy link

Choose a reason for hiding this comment

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

Could you please rework the code so that this function always return a valid path. It should be either a path from sysfs or a FIRMWARE_BASE. Rename the function accordingly.

Copy link
Author

Choose a reason for hiding this comment

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

The code for this function was taken from related tqftpserv PR, I though it would be easier to review if they are identical... But this can be done, yeah.. I'll think about it

Copy link
Author

Choose a reason for hiding this comment

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

The idea is to get rid of complicated "if" below, where we first try this, then try that..?

Copy link

Choose a reason for hiding this comment

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

yes

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.

2 participants