On Wed, Dec 08, 2021 at 12:12:41PM +0100, Cezary Rojewski wrote:

> +int avs_dsp_get_core(struct avs_dev *adev, u32 core_id)
> +{

...

> +	if (atomic_add_return(1, ref) == 1) {
> +		ret = avs_dsp_enable(adev, mask);
> +		if (ret)
> +			goto err_enable_dsp;
> +	}

> +int avs_dsp_put_core(struct avs_dev *adev, u32 core_id)
> +{

...

> +	ref = &adev->core_refs[core_id];
> +	if (atomic_dec_and_test(ref)) {
> +		ret = avs_dsp_disable(adev, mask);

This looks wrong - there's nothing that ensures that we don't get
a sequence like:

	CPU0		CPU1
	decrement
			increment
			enable DSP
	disable DSP

that I can see here?  Either there's a lock missing which ensures
that the actual DSP management is in sync with the refcount or
there's no need for the use of atomics since the wider lock will
ensure that only one thing could be updating at once.  In general
I'd expect something heavier weight than atomics.