Unverified Commit db38d86d authored by Paul Olaru's avatar Paul Olaru Committed by Mark Brown
Browse files

ASoC: sof: Improve sof_ipc4_bytes_ext_put function 



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: default avatarDaniel Baluta <daniel.baluta@nxp.com>
Reviewed-by: default avatarPéter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: default avatarPaul Olaru <paul.olaru@nxp.com>
Signed-off-by: default avatarDaniel Baluta <daniel.baluta@nxp.com>
Link: https://lore.kernel.org/r/20230503081049.73847-3-daniel.baluta@oss.nxp.com


Signed-off-by: default avatarMark Brown <broonie@kernel.org>
parent 299f6c75

sound/soc/sof/ipc4-control.c

+35 −4
Original line number Diff line number Diff line
@@ -54,6 +54,26 @@ static int sof_ipc4_set_get_kcontrol_data(struct snd_sof_control *scontrol, 
	msg->primary |= SOF_IPC4_MOD_INSTANCE(swidget->instance_id);



	ret = iops->set_get_data(sdev, msg, msg->data_size, set);

	if (!set)

		goto unlock;



	/* It is a set-data operation, and we have a valid 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, msg, msg->data_size, set);

		if (ret < 0)

			goto unlock;

	}



unlock:

	if (lock)
@@ -327,13 +347,24 @@ static int sof_ipc4_bytes_ext_put(struct snd_sof_control *scontrol, 
		return -EINVAL;

	}



	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;

	}



	/* Copy the whole binary data which includes the ABI header and the payload */

	if (copy_from_user(data, tlvd->tlv, header.length))

	if (copy_from_user(data, tlvd->tlv, header.length)) {

		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;

		return -EFAULT;

	}



	sof_ipc4_set_get_bytes_data(sdev, scontrol, true, true);



	return 0;

	return sof_ipc4_set_get_bytes_data(sdev, scontrol, true, true);

}



static int _sof_ipc4_bytes_ext_get(struct snd_sof_control *scontrol,