linux icon indicating copy to clipboard operation
linux copied to clipboard

Unconditionally dev_dbg firmware and tplg paths

Open afq984 opened this issue 3 years ago • 5 comments

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.

afq984 avatar Aug 11 '22 20:08 afq984

@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.

plbossart avatar Aug 12 '22 13:08 plbossart

@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:

  • The udev rule did not match
  • The udev rule kicked in too late, after the catch-all rule, thus the default was already loaded

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.

afq984 avatar Aug 12 '22 19:08 afq984

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

marc-hb avatar Oct 20 '22 17:10 marc-hb

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

afq984 avatar Oct 20 '22 21:10 afq984

@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.

marc-hb avatar Oct 20 '22 22:10 marc-hb