From: Paul Olaru paul.olaru@nxp.com

The function is improved in the way that if the firmware returns a validation error on the newly sent bytes, then the kernel will automatically restore to the old bytes value for a given kcontrol.

This way, if the firmware rejects a data blob then the kernel will also reject it, instead of saving it for the next suspend/resume cycle. The old behaviour is that the kernel would save it anyway and on next firmware boot it would apply the previously-rejected configuration, leading to errors during playback.

Additionally, the function also saves previously validated configurations, so that if the firmware does end up rejecting a new bytes value the kernel can send an old, previously-valid configuration.

Reviewed-by: Daniel Baluta daniel.baluta@nxp.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Paul Olaru paul.olaru@nxp.com Signed-off-by: Daniel Baluta daniel.baluta@nxp.com --- sound/soc/sof/ipc3-control.c | 54 ++++++++++++++++++++++++++++++++---- sound/soc/sof/sof-audio.h | 1 + 2 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sof/ipc3-control.c b/sound/soc/sof/ipc3-control.c index ad040e7bb850..a8deec7dc021 100644 --- a/sound/soc/sof/ipc3-control.c +++ b/sound/soc/sof/ipc3-control.c @@ -96,6 +96,26 @@ static int sof_ipc3_set_get_kcontrol_data(struct snd_sof_control *scontrol, cdata->elems_remaining = 0;

ret = iops->set_get_data(sdev, cdata, cdata->rhdr.hdr.size, set); + if (!set) + goto unlock; + + /* It is a set-data operation, and we have a backup that we can restore */ + if (ret < 0) { + if (!scontrol->old_ipc_control_data) + goto unlock; + /* + * Current ipc_control_data is not valid, we use the last known good + * configuration + */ + memcpy(scontrol->ipc_control_data, scontrol->old_ipc_control_data, + scontrol->max_size); + kfree(scontrol->old_ipc_control_data); + scontrol->old_ipc_control_data = NULL; + /* Send the last known good configuration to firmware */ + ret = iops->set_get_data(sdev, cdata, cdata->rhdr.hdr.size, set); + if (ret < 0) + goto unlock; + }

unlock: if (lock) @@ -351,6 +371,7 @@ static int sof_ipc3_bytes_ext_put(struct snd_sof_control *scontrol, struct sof_ipc_ctrl_data *cdata = scontrol->ipc_control_data; struct snd_soc_component *scomp = scontrol->scomp; struct snd_ctl_tlv header; + int ret = -EINVAL;

/* * The beginning of bytes data contains a header from where @@ -381,31 +402,52 @@ static int sof_ipc3_bytes_ext_put(struct snd_sof_control *scontrol, return -EINVAL; }

- if (copy_from_user(cdata->data, tlvd->tlv, header.length)) - return -EFAULT; + if (!scontrol->old_ipc_control_data) { + /* Create a backup of the current, valid bytes control */ + scontrol->old_ipc_control_data = kmemdup(scontrol->ipc_control_data, + scontrol->max_size, GFP_KERNEL); + if (!scontrol->old_ipc_control_data) + return -ENOMEM; + } + + if (copy_from_user(cdata->data, tlvd->tlv, header.length)) { + ret = -EFAULT; + goto err_restore; + }

if (cdata->data->magic != SOF_ABI_MAGIC) { dev_err_ratelimited(scomp->dev, "Wrong ABI magic 0x%08x\n", cdata->data->magic); - return -EINVAL; + goto err_restore; }

if (SOF_ABI_VERSION_INCOMPATIBLE(SOF_ABI_VERSION, cdata->data->abi)) { dev_err_ratelimited(scomp->dev, "Incompatible ABI version 0x%08x\n", cdata->data->abi); - return -EINVAL; + goto err_restore; }

/* be->max has been verified to be >= sizeof(struct sof_abi_hdr) */ if (cdata->data->size > scontrol->max_size - sizeof(struct sof_abi_hdr)) { dev_err_ratelimited(scomp->dev, "Mismatch in ABI data size (truncated?)\n"); - return -EINVAL; + goto err_restore; }

/* notify DSP of byte control updates */ - if (pm_runtime_active(scomp->dev)) + if (pm_runtime_active(scomp->dev)) { + /* Actually send the data to the DSP; this is an opportunity to validate the data */ return sof_ipc3_set_get_kcontrol_data(scontrol, true, true); + }

return 0; + +err_restore: + /* If we have an issue, we restore the old, valid bytes control data */ + if (scontrol->old_ipc_control_data) { + memcpy(cdata->data, scontrol->old_ipc_control_data, scontrol->max_size); + kfree(scontrol->old_ipc_control_data); + scontrol->old_ipc_control_data = NULL; + } + return ret; }

static int _sof_ipc3_bytes_ext_get(struct snd_sof_control *scontrol, diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index a090a9eb4828..5d5eeb1a1a6f 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -362,6 +362,7 @@ struct snd_sof_control { size_t priv_size; /* size of private data */ size_t max_size; void *ipc_control_data; + void *old_ipc_control_data; int max; /* applicable to volume controls */ u32 size; /* cdata size */ u32 *volume_table; /* volume table computed from tlv data*/