diff mbox series

[16/19] soundwire: amd: handle wake enable interrupt

Message ID 20230111090222.2016499-17-Vijendar.Mukunda@amd.com
State New
Headers
Series [01/19] ASoC: amd: ps: create platform devices based on acp config |

Commit Message

Mukunda,Vijendar Jan. 11, 2023, 9:02 a.m. UTC 
  Add wake enable interrupt support for both the soundwire controller
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(+)

Comments

Pierre-Louis Bossart Jan. 11, 2023, 3:54 p.m. UTC | #1 
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?
Mukunda,Vijendar Jan. 12, 2023, 10:21 a.m. UTC | #2 
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 mbox series

Patch

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;
 };