On 11/01/23 21:24, Pierre-Louis Bossart wrote: > > On 1/11/23 03:02, Vijendar Mukunda wrote: >> Add wake enable interrupt support for both the soundwire controller > SoundWire. > >> instances. >> >> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx> >> Signed-off-by: Mastan Katragadda <Mastan.Katragadda@xxxxxxx> >> --- >> drivers/soundwire/amd_master.c | 9 +++++++++ >> drivers/soundwire/amd_master.h | 1 + >> include/linux/soundwire/sdw_amd.h | 1 + >> 3 files changed, 11 insertions(+) >> >> diff --git a/drivers/soundwire/amd_master.c b/drivers/soundwire/amd_master.c >> index 290c59ab7760..2fd77a673c22 100644 >> --- a/drivers/soundwire/amd_master.c >> +++ b/drivers/soundwire/amd_master.c >> @@ -1219,6 +1219,13 @@ static void amd_sdwc_update_slave_status_work(struct work_struct *work) >> u32 sw_status_change_mask_0to7_reg; >> u32 sw_status_change_mask_8to11_reg; >> >> + if (ctrl->wake_event) { >> + pm_runtime_resume(ctrl->dev); >> + acp_reg_writel(0x00, ctrl->mmio + ACP_SW_WAKE_EN); >> + ctrl->wake_event = false; >> + return; >> + } > this is surprising. > > A wake event typically happens when the bus is in clock-stop mode. > You cannot deal with wake events while dealing with the peripheral > status update, because you can only get that status when the manager is > up-and-running. There's a conceptual miss here, no? > > If the wake comes from the PCI side, then it's the same comment: why > would the wake be handled while updating the peripheral status. > > What am I missing? > It's a miss. This should be moved out of slave_status_work() even though when wake enable irq is received we are just returning from API. will move wake interrupt handling in to a separate helper function.