Thanks for the reviews Mark, much appreciated. +int snd_sof_load_firmware_memcpy(struct snd_sof_dev *sdev)

...

+{ + struct snd_sof_pdata *plat_data = dev_get_platdata(sdev->dev); + const char *fw_filename; + int ret; This never actually calls the load_firmware() operation for the DSP AFAICT?

it's actually the implementation of the load_firmware callback, see e.g. for Baytrail/Broadwell /*Firmware loading */     .load_firmware    = snd_sof_load_firmware_memcpy,

...

+ ret = request_firmware(&plat_data->fw, fw_filename, sdev->dev); + + if (ret < 0) { + dev_err(sdev->dev, "error: request firmware failed err: %d\n", + ret); + return ret; + } I'd suggest logging the name of the firmware we tried to load, users will thank you.

yes indeed, thanks for the suggestion, will add this right away.

...

+ /* create fw_version debugfs to store boot version info */ + if (sdev->first_boot) { + ret = snd_sof_debugfs_buf_create_item(sdev, &sdev->fw_version, + sizeof(sdev->fw_version), + "fw_version"); + + if (ret < 0) { + dev_err(sdev->dev, "error: cannot create debugfs for fw_version\n"); + return ret; + } + } As Andy said elsewhere debugfs stuff like that probably shouldn't be fatal.

yes, we've fixed this already.