| Message ID | 20230111090222.2016499-17-Vijendar.Mukunda@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add soundwire support for Pink Sardine platform | expand |
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@amd.com> > Signed-off-by: Mastan Katragadda <Mastan.Katragadda@amd.com> > --- > 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?
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@amd.com> >> Signed-off-by: Mastan Katragadda <Mastan.Katragadda@amd.com> >> --- >> 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.
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; + } + switch (ctrl->instance) { case ACP_SDW0: sw_status_change_mask_0to7_reg = SW_STATE_CHANGE_STATUS_MASK_0TO7; @@ -1258,6 +1265,8 @@ static void amd_sdwc_update_slave_status(u32 status_change_0to7, u32 status_chan if (status_change_0to7 == AMD_SDW_SLAVE_0_ATTACHED) memset(ctrl->status, 0, sizeof(ctrl->status)); + if (status_change_8to11 & AMD_SDW_WAKE_STAT_MASK) + ctrl->wake_event = true; slave_stat = status_change_0to7; slave_stat |= FIELD_GET(AMD_SDW_MCP_SLAVE_STATUS_8TO_11, status_change_8to11) << 32; dev_dbg(ctrl->dev, "%s: status_change_0to7:0x%x status_change_8to11:0x%x\n", diff --git a/drivers/soundwire/amd_master.h b/drivers/soundwire/amd_master.h index cc254255512b..e32884e3a45b 100644 --- a/drivers/soundwire/amd_master.h +++ b/drivers/soundwire/amd_master.h @@ -240,6 +240,7 @@ #define AMD_SDW_CLK_STOP_DONE 1 #define AMD_SDW_CLK_RESUME_REQ 2 #define AMD_SDW_CLK_RESUME_DONE 3 +#define AMD_SDW_WAKE_STAT_MASK BIT(16) enum amd_sdw_channel { /* SDW0 */ diff --git a/include/linux/soundwire/sdw_amd.h b/include/linux/soundwire/sdw_amd.h index f362f195b887..e3e539b868be 100644 --- a/include/linux/soundwire/sdw_amd.h +++ b/include/linux/soundwire/sdw_amd.h @@ -40,6 +40,7 @@ struct amd_sdwc_ctrl { int num_ports; bool clk_stopped; bool startup_done; + bool wake_event; u32 power_mode_mask; };