Re: [PATCH 21/21] soundwire: amd: Disable interrupts if enabling manager fails

[Mukunda,Vijendar]

On 10/30/26 16:17, Lee Jones wrote:
> In amd_sdw_manager_start() and amd_resume_runtime(),
> amd_enable_sdw_interrupts() is called before amd_enable_sdw_manager().
> If amd_enable_sdw_manager() fails, both functions return immediately
> with SoundWire interrupts left unmasked.  In addition,
> amd_resume_runtime() ignores the return value of
> amd_init_sdw_manager().
>
> Check the return value of amd_init_sdw_manager() in
> amd_resume_runtime(), and disable interrupts if amd_enable_sdw_manager()
> fails in amd_sdw_manager_start() and amd_resume_runtime().
++
The unchecked return value of amd_init_sdw_manager() in
amd_resume_runtime() is already addressed in our v3 patch series:

https://lore.kernel.org/alsa-devel/20261013085111.2009061-6-Vijendar.Mukunda@xxxxxxx/T/#m7741edcedb1264764edc7f8559e39af8b23433fb

The missing amd_disable_sdw_interrupts() on amd_enable_sdw_manager()
failure is a valid observation and will be addressed in our upcoming
v4 patch series. Please hold this patch until v4 is on-list to
avoid conflicts.
> Fixes: 81ff58ff71ad ("soundwire: amd: add runtime pm ops for AMD SoundWire manager driver")
> Closes: https://sashiko.dev/#/bug/linux-309820e5-f754-4d3d-80aa-7f70d59f3e8e
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Lee Jones <lee@xxxxxxxxxx>
> ---
>   drivers/soundwire/amd_manager.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
> index faff3e9eefad..f64e8fea7e16 100644
> --- a/drivers/soundwire/amd_manager.c
> +++ b/drivers/soundwire/amd_manager.c
> @@ -1109,8 +1109,10 @@ int amd_sdw_manager_start(struct amd_sdw_manager *amd_manager)
>   			return ret;
>   		amd_enable_sdw_interrupts(amd_manager);
>   		ret = amd_enable_sdw_manager(amd_manager);
> -		if (ret)
> +		if (ret) {
> +			amd_disable_sdw_interrupts(amd_manager);
>   			return ret;
> +		}
>   		amd_sdw_set_frameshape(amd_manager);
>   	}
>   	/* Enable runtime PM */
> @@ -1503,11 +1505,15 @@ static int __maybe_unused amd_resume_runtime(struct device *dev)
>   		ret = amd_sdw_clk_init_ctrl(amd_manager);
>   		if (ret)
>   			return ret;
> -		amd_init_sdw_manager(amd_manager);
> +		ret = amd_init_sdw_manager(amd_manager);
> +		if (ret)
> +			return ret;
>   		amd_enable_sdw_interrupts(amd_manager);
>   		ret = amd_enable_sdw_manager(amd_manager);
> -		if (ret)
> +		if (ret) {
> +			amd_disable_sdw_interrupts(amd_manager);
>   			return ret;
> +		}
>   		amd_sdw_set_frameshape(amd_manager);
>   	}
>   	if (amd_manager->acp_rev >= ACP70_PCI_REV_ID) {