Re: [PATCH 20/21] soundwire: amd: Check clock resume timeout in amd_resume_runtime()

["Mukunda,Vijendar" <vijendar.mukunda@amd.com>]

On 10/30/26 18:21, Mukunda,Vijendar wrote:
>
>
> On 10/30/26 16:16, Lee Jones wrote:
>> When resuming from AMD_SDW_POWER_OFF_MODE in amd_resume_runtime(), the
>> return value of readl_poll_timeout() when polling for
>> AMD_SDW_CLK_RESUME_DONE is stored in ret but never checked, allowing
>> resume to proceed even when clock resume times out.
>>
>> Return ret if polling for AMD_SDW_CLK_RESUME_DONE fails.
>>
>> Fixes: 81ff58ff71ad ("soundwire: amd: add runtime pm ops for AMD 
>> SoundWire manager driver")
>> Closes: 
>> https://sashiko.dev/#/bug/linux-c6360b30-11b9-40a1-ab04-528462af313c
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Lee Jones <lee@kernel.org>
>> ---
>>   drivers/soundwire/amd_manager.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/soundwire/amd_manager.c 
>> b/drivers/soundwire/amd_manager.c
>> index ba8e6d42c467..faff3e9eefad 100644
>> --- a/drivers/soundwire/amd_manager.c
>> +++ b/drivers/soundwire/amd_manager.c
>> @@ -1494,10 +1494,10 @@ static int __maybe_unused 
>> amd_resume_runtime(struct device *dev)
>>               ret = readl_poll_timeout(amd_manager->mmio + 
>> ACP_SW_CLK_RESUME_CTRL, val,
>>                            (val & AMD_SDW_CLK_RESUME_DONE), 
>> ACP_DELAY_US,
>>                            AMD_SDW_TIMEOUT);
>> -            if (val & AMD_SDW_CLK_RESUME_DONE) {
>> -                writel(0, amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL);
>> -                amd_manager->clk_stopped = false;
>> -            }
>> +            if (ret)
>> +                return ret;
>> +            writel(0, amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL);
>> +            amd_manager->clk_stopped = false;
> ++
>
> The bug is valid. On clock resume timeout the code falls through to
> sdw_clear_slave_status() and full bus re-init without the clock
> being active, which is incorrect.
>
> However, we have identified additional issues in this code path that
> need to be addressed together:
>
> 1. writel(0, ACP_SW_CLK_RESUME_CTRL) is incorrect — this register
>    is not a plain zero-clear register. Only the Clk_Resume_Req bit
>    should be cleared using a read-modify-write on the success path.
Slight correction.  Ignore comment 1.

>
> 2. The same unchecked ret pattern exists in
>    amd_acp63_sdw_clock_stop_exit() and needs to be fixed together.
>
> We are planning to address all of these as part of our upcoming v4
> patch series which will be posted shortly. Could you please hold
> this patch until v4 is on-list to avoid conflicts?
>>           }
>>           sdw_clear_slave_status(bus, 
>> SDW_UNATTACH_REQUEST_MASTER_RESET);
>>           ret = amd_sdw_clk_init_ctrl(amd_manager);
>