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] [day] [month] [year] [list]

Message-ID: <2ab354e0-339e-448a-957d-4226781a7621@amd.com>
Date: Tue, 21 Jan 2025 10:56:23 +0530
From: "Mukunda,Vijendar" <vijendar.mukunda@....com>
To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.dev>, 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 20/01/25 21:00, Pierre-Louis Bossart wrote:
> 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.
Soundwire device resume sequence can be invoked from multiple places
same time due to different interrupt sources. We have added this extra condition
check to safeguard the resume invoking sequence.
Will drop this code and retest our test scenarios.