Re: [PATCH v3 09/14] ASoC: SOF: Add firmware loader support

Mark Brown
2019-01-09T21:02:33Z

[-- Attachment #1.1: Type: text/plain, Size: 1043 bytes --]

On Tue, Dec 11, 2018 at 03:23:13PM -0600, Pierre-Louis Bossart wrote:

> +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?

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

> + /* 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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]