Re: [PATCH v5 1/3] ASoC: tac5xx2-sdw: add soundwire based codec driver

[Mark Brown]

On Tue, Apr 07, 2026 at 03:18:27PM +0530, Niranjan H Y wrote:

> +static s32 tac_load_and_cache_firmware(struct tac5xx2_prv *tac_dev)
> +{
> +	const struct firmware *fmw = NULL;
> +	const char *fw_name_used = NULL;
> +	s32 ret = 0;
> +	u8 *cached_data = NULL;
> +	u32 fw_hdr_size;
> +
> +	mutex_lock(&tac_dev->pde_lock);
> +	ret = request_firmware(&fmw, tac_dev->fw_binaryname, tac_dev->dev);

This is happening in the SoundWire bus callback - if request_firmware()
ends up being slow (eg, a userspace helper faffing) we might block other
things on the bus.

> +static s32 tac_download_fw_to_hw(struct tac5xx2_prv *tac_dev)
> +{

> +	while (offset < img_sz && num_files < TAC_MAX_FW_CHUNKS) {

> +		if (file_length > img_sz || offset > img_sz - TAC_FW_FILE_HDR ||
> +		    file_length > img_sz - offset - TAC_FW_FILE_HDR) {
> +			dev_warn(tac_dev->dev, "File at offset %d exceeds buffer: length=%u, available=%zu\n",
> +				 offset, file_length, img_sz - offset - TAC_FW_FILE_HDR);
> +			break;
> +		}

> +	if (num_files == 0) {
> +		dev_err(tac_dev->dev, "firmware with no files\n");
> +		ret = -EINVAL;
> +		goto out;
> +	}

If we've parsed any files then an oversized file will just break out of
the loop and download what we got already, is that desirable?

> +	if (tac_dev->sm_func_data) {
> +		ret = sdca_regmap_write_init(dev, tac_dev->regmap,
> +					     tac_dev->sm_func_data);
> +		if (ret) {
> +			dev_err(dev, "smartmic init table update failed\n");
> +			goto io_init_err;
> +		}
> +		dev_dbg(dev, "smartmic init done\n");
> +
> +		if (first) {
> +			/* Set default value to CS:11 */
> +			tac_dev->cx11_value = 1;
> +			regmap_write(tac_dev->regmap,
> +				     SDW_SDCA_CTL(TAC_FUNCTION_ID_SM,
> +						  TAC_SDCA_ENT_CX11,
> +							TAC_SDCA_CTL_CX_CLK_SEL, 0),
> +					tac_dev->cx11_value);

This looks like it could race with tac_cx11_put() - they both write
cx11_value but the control could be modifying it.  I can imagine that
happening if userspace starts and is setting config while the SoundWire
bus is coming on line.

Attachment: signature.asc
Description: PGP signature