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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7e363634-b80f-4ae7-8d09-1bb07225eecd@linux.dev>
Date: Mon, 20 Jan 2025 09:30:30 -0600
From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.dev>
To: Vijendar Mukunda <Vijendar.Mukunda@....com>, vkoul@...nel.org
Cc: alsa-devel@...a-project.org, yung-chuan.liao@...ux.intel.com,
 sanyog.r.kale@...el.com, Basavaraj.Hiregoudar@....com,
 Sunil-kumar.Dommati@....com, venkataprasad.potturu@....com,
 Mario.Limonciello@....com, linux-sound@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2 3/7] soundwire: amd: add conditional for check for
 device resume

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@....com>
> ---
>  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.