Re: [PATCH 2/2] soundwire: Intel: stop sdw clock in system suspend

[Pierre-Louis Bossart]

On 6/29/26 16:44, Bard Liao wrote:
> There is no need to keep the SoundWire clock active in system suspend.
> 
> Signed-off-by: Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx>
> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx>
> ---
>  drivers/soundwire/intel_auxdevice.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c
> index 0b8107bec9ab..10fd27f4fe39 100644
> --- a/drivers/soundwire/intel_auxdevice.c
> +++ b/drivers/soundwire/intel_auxdevice.c
> @@ -669,7 +669,8 @@ static int __maybe_unused intel_suspend(struct device *dev)
>  		return 0;
>  	}
>  
> -	ret = sdw_intel_stop_bus(sdw, false);
> +	/* No need to keep the SoundWire clock active in system suspend */
> +	ret = sdw_intel_stop_bus(sdw, true);

erm, are you sure about this change?

What this does is stop the SoundWire clock before entering system suspend. 
Is this needed? The whole point of system suspend is that the host will stop operating completely.
There is in theory no need to stop the clock because the clock restart capability will not be used.
Think for example of jack detection, it's supported in pm_runtime suspend with the SoundWire interrupt capability, but not in system suspend.
By changing the clock-stop argument to true, you are requesting the clock stop mechanism to be enabled even though you don't need it.

I do recall that in some generations it was actually not supported to turn off power to the IP while the wake detector was enabled.

>  	if (ret < 0) {
>  		dev_err(dev, "%s: cannot stop bus: %d\n", __func__, ret);
>  		return ret;