On Mon, Apr 01, 2024 at 08:50:24AM -0500, Pierre-Louis Bossart wrote:
+static int tasdevice_comp_probe(struct snd_soc_component *comp) +{
- ret = request_firmware(&fw_entry, dspfw_binaryname, tas_dev->dev);
- if (ret) {
dev_err(tas_dev->dev,"%s: request_firmware %x open status: %d.\n", __func__,tas_dev->sdw_peripheral->id.unique_id, ret);goto out;- }
- tasdevice_dspfw_ready(fw_entry, tas_dev);
Question: is there a specific reason why all this functionality needs to be done in a component probe instead of when the device reports as ATTACHED?
Since you have an interaction with userspace for the firmware, and firmware download takes time, you would want to do this as early as possible.
Usually the component probe is part of the card creation, so you could add card-related or control related things. Downloading firmware does not strike me as a card-related activity?
This does have the effect of ensuring that the card won't instantiate without firmware. Given how utterly dependent on firmware this device seems to be I can see a case for blocking card registration until the firmware is ready, there's a usability thing there but it feels like there's a usability issue with any error handling for missing firmware on these devices and not registering the card does seem like a valid choice. That said it would still be nicer to initiate the firmware process earlier in order to minimise latency and then either defer registration of the component until we've managed to load firmware or have a check at component probe to make sure that the firmware is ready.
Another question is whether the firmware needs to be downloaded again during system/pm_runtime resume? That may depend on how power is managed at the hardware level, but in theory an SDCA device should report to the host whether the firmware is still valid or needs to be downloaded.
That does seem like a concern too.