Re: [PATCH V2 3/7] soundwire: amd: add conditional for check for device resume

[Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxx>]

On 1/20/25 4:13 AM, Vijendar Mukunda wrote:
> Add a conditional check to resume SoundWire manager device, when the
> SoundWire manager instance is in the suspended state.
> 
> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
> ---
>  drivers/soundwire/amd_manager.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
> index 60be5805715e..6831b3729db5 100644
> --- a/drivers/soundwire/amd_manager.c
> +++ b/drivers/soundwire/amd_manager.c
> @@ -850,7 +850,10 @@ static void amd_sdw_update_slave_status(u32 status_change_0to7, u32 status_chang
>  static void amd_sdw_process_wake_event(struct amd_sdw_manager *amd_manager)
>  {
>  	dev_dbg(amd_manager->dev, "SoundWire Wake event reported\n");
> -	pm_request_resume(amd_manager->dev);
> +	if (pm_runtime_suspended(amd_manager->dev)) {
> +		dev_dbg(amd_manager->dev, "Device is in suspend state\n");
> +		pm_request_resume(amd_manager->dev);
> +	}

Is this test actually doing something useful?

If the device is already active, presumably doing a pm_request_resume() is a no-op. If it's already suspended it does something.
Testing the device state is risky with all the asynchronous behavior in SoundWire, best to leave the state checks to be handled inside the pm_runtime core, no?

IIRC the only time when such a test in needed is in the system suspend callbacks where specific action needs to be taken if the device is in pm_runtime suspended mode with the clock_stop mode engaged.