thesofproject / linux

Linux kernel source tree
Other
89 stars 129 forks source link

Unconditionally dev_dbg firmware and tplg paths #3819

Open afq984 opened 2 years ago

afq984 commented 2 years ago

There are multiple ways to override the paths but it is only logged sometimes. For example in sof-pci-dev.c there are multiple conditionals to change them, but only some of the branches log them. Also we do not log them at all in sof-of-dev.c. I propose we unconditionally dev_dbg the the paths in snd_sof_device_probe(). This will give us a lot of confidence that we're testing the correct firmware.

plbossart commented 2 years ago

@afq984 can you be more specific on what cases you saw where the information is not provided for PCI "but only some of the branches log them"

I am not sure if printing the logs is a silver bullet, there might be additional rules in the firmware selection where the kernel fetches the file from /lib/firmware/updates or other directories. If there is any higher-level rule that kicks in, the SOF driver would only report the local path and filename, but not the absolute path.

afq984 commented 2 years ago

@afq984 can you be more specific on what cases you saw where the information is not provided for PCI "but only some of the branches log them"

Below are some examples

https://github.com/thesofproject/linux/blob/b41e86dd683bc0e224c2a1383ffc5b2cd3238403/sound/soc/sof/sof-pci-dev.c#L290-L294

https://github.com/thesofproject/linux/blob/b41e86dd683bc0e224c2a1383ffc5b2cd3238403/sound/soc/sof/sof-pci-dev.c#L275-L279

there might be additional rules in the firmware selection where the kernel fetches the file from /lib/firmware/updates or other directories

Ah, I'm not aware about it.

I'm mainly concerned about us overriding the paths with modprobe in udev rules on ChromeOS, but we don't have good ways to know if we wrote the udev rules correctly. Failures could be from:

But I just learned that I can inspect /sys/module/snd_sof_*/parameters/*, maybe we don't need the log for this case then? I imagine this only works if the module is loaded successfully though.

marc-hb commented 1 year ago

I just found this by chance... is newer (and longer) #3867 a duplicate of this?

afq984 commented 1 year ago

@marc-hb I gave #3867 a quick look. Indeed having the fw/tplg checksums on /sys helps us a lot

marc-hb commented 1 year ago

@marc-hb I gave https://github.com/thesofproject/linux/issues/3867 a quick look. Indeed having the fw/tplg checksums on /sys helps us a lot

Thanks! could you please summarize why in #3867 while linking back to here?

there might be additional rules in the firmware selection where the kernel fetches the file from /lib/firmware/updates or other directories.

Indeed, the request_firmware() framework has that information and should ideally provide it to either each driver or to /sys

If there is any higher-level rule that kicks in, the SOF driver would only report the local path and filename, but not the absolute path.

For the vast majority of people who have a single firmware directory this is enough. Even for people who have multiple /lib/fimware/___ directories it is still useful.