Re: [PATCH v1 2/2] soundwire: qcom: add EE-aware register layout and cpu selection

[Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>]

On 6/8/26 6:53 PM, Mohammad Rafi Shaik wrote:
> Some Qualcomm SoundWire masters expose interrupt, FIFO and status
> registers in EE-specific register windows on v2.0 and newer hardware.
> 
> Add support for selecting the SoundWire execution environment from DT
> and use it to program the correct register window for the active EE.

So this is integration details, which can be derived dynamically based
on version or compatible.


> The driver now reads the EE value from the new
> qcom,swr-master-ee-val property, with qcom,ee as a fallback for
> backward compatibility.

> 
> For v2.0+ hardware, the IRQ/FIFO/status register layout is adjusted by
Which exact version, do we have minor or step values for this?

> the EE window stride so the driver programs the correct bank for the
> selected EE. The interrupt enable path is also updated to always use
> the selected EE window.
> 
> This change allows SoundWire interrupt routing and register accesses to
> work correctly on platforms where the master is not mapped to the
> default EE1 window.
> 
> In Shikra, the soundwire execution environment is set to 0 unlike other
> Qualcomm boards.
> 

Can we get this integration details dynamically.
> Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
> ---
>  drivers/soundwire/qcom.c | 78 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 65 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 3d8f5a81e..f4b8ff77b 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -26,6 +26,7 @@
>  #define SWRM_COMP_STATUS					0x014
>  #define SWRM_LINK_MANAGER_EE					0x018
>  #define SWRM_EE_CPU						1
> +#define SWRM_MAX_EE						1

Can we not make the EE_CPU dynamic and everything should fall in place?
isn't?

Is the register layout changed?

>  #define SWRM_FRM_GEN_ENABLED					BIT(0)
>  #define SWRM_VERSION_1_3_0					0x01030000
>  #define SWRM_VERSION_1_5_1					0x01050001
> @@ -118,6 +119,7 @@
>  #define SWRM_V2_0_CLK_CTRL					0x5060
>  #define SWRM_V2_0_CLK_CTRL_CLK_START				BIT(0)
>  #define SWRM_V2_0_LINK_STATUS					0x5064
> +#define SWRM_V2_REG_EE_STRIDE					0x1000
>  
>  #define SWRM_DP_PORT_CTRL_EN_CHAN_SHFT				0x18
>  #define SWRM_DP_PORT_CTRL_OFFSET2_SHFT				0x10
> @@ -202,6 +204,7 @@ struct qcom_swrm_ctrl {
>  	struct mutex port_lock;
>  	struct clk *hclk;
>  	int irq;
> +	u32 ee;
>  	unsigned int version;
>  	int wake_irq;
>  	int num_din_ports;
> @@ -222,6 +225,7 @@ struct qcom_swrm_ctrl {
>  	u32 slave_status;
>  	u32 wr_fifo_depth;
>  	bool clock_stop_not_supported;
> +	unsigned int reg_layout_local[SWRM_OFFSET_DP_SAMPLECTRL2_BANK + 1];
>  };
>  
>  struct qcom_swrm_data {
> @@ -328,6 +332,36 @@ static const struct qcom_swrm_data swrm_v3_0_data = {
>  };
>  #define to_qcom_sdw(b)	container_of(b, struct qcom_swrm_ctrl, bus)
>  
> +static void qcom_swrm_set_ee_register_layout(struct qcom_swrm_ctrl *ctrl,
> +					     const struct qcom_swrm_data *data)
> +{
> +	int ee_offset;
> +
> +	memcpy(ctrl->reg_layout_local, data->reg_layout,
> +	       sizeof(ctrl->reg_layout_local));
> +	ctrl->reg_layout = ctrl->reg_layout_local;
> +
> +	if (ctrl->version < SWRM_VERSION_2_0_0)
> +		return;
> +
> +	/*
> +	 * Current register constants map EE1. For EE0, use the EE register
> +	 * window stride to access status/IRQ/FIFO registers.
> +	 */
> +	ee_offset = ((int)ctrl->ee - SWRM_EE_CPU) * SWRM_V2_REG_EE_STRIDE;
> +	if (!ee_offset)
> +		return;
> +
> +	ctrl->reg_layout_local[SWRM_REG_FRAME_GEN_ENABLED] += ee_offset;
> +	ctrl->reg_layout_local[SWRM_REG_INTERRUPT_STATUS] += ee_offset;
> +	ctrl->reg_layout_local[SWRM_REG_INTERRUPT_CLEAR] += ee_offset;
> +	ctrl->reg_layout_local[SWRM_REG_INTERRUPT_CPU_EN] += ee_offset;
> +	ctrl->reg_layout_local[SWRM_REG_CMD_FIFO_WR_CMD] += ee_offset;
> +	ctrl->reg_layout_local[SWRM_REG_CMD_FIFO_RD_CMD] += ee_offset;
> +	ctrl->reg_layout_local[SWRM_REG_CMD_FIFO_STATUS] += ee_offset;
> +	ctrl->reg_layout_local[SWRM_REG_CMD_FIFO_RD_FIFO_ADDR] += ee_offset;

Why not make these registers take the ee value rather then doing this way?


> +}
> +
>  static int qcom_swrm_ahb_reg_read(struct qcom_swrm_ctrl *ctrl, int reg,
>  				  u32 *val)
>  {
> @@ -904,12 +938,13 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl)
>  	ctrl->reg_write(ctrl, SWRM_MCP_CFG_ADDR, val);
>  
>  	if (ctrl->version == SWRM_VERSION_1_7_0) {
> -		ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, SWRM_EE_CPU);
> +		ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, ctrl->ee);
>  		ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL,
> -				SWRM_MCP_BUS_CLK_START << SWRM_EE_CPU);
> +				SWRM_MCP_BUS_CLK_START << ctrl->ee);
>  	} else if (ctrl->version >= SWRM_VERSION_2_0_0) {
> -		ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, SWRM_EE_CPU);
> -		ctrl->reg_write(ctrl, SWRM_V2_0_CLK_CTRL,
> +		ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, ctrl->ee);
> +		ctrl->reg_write(ctrl, SWRM_V2_0_CLK_CTRL +
> +				((int)ctrl->ee - SWRM_EE_CPU) * SWRM_V2_REG_EE_STRIDE,
>  				SWRM_V2_0_CLK_CTRL_CLK_START);
>  	} else {
>  		ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL, SWRM_MCP_BUS_CLK_START);
> @@ -935,11 +970,9 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl)
>  	ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CLEAR],
>  			0xFFFFFFFF);
>  
> -	/* enable CPU IRQs */
> -	if (ctrl->mmio) {
> -		ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
> -				SWRM_INTERRUPT_STATUS_RMSK);
> -	}
> +	/* enable CPU IRQs for the selected EE window */
> +	ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
> +			SWRM_INTERRUPT_STATUS_RMSK);
>  
>  	/* Set IRQ to PULSE */
>  	ctrl->reg_write(ctrl, SWRM_COMP_CFG_ADDR,
> @@ -1545,7 +1578,22 @@ static int qcom_swrm_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	data = of_device_get_match_data(dev);
> +	ctrl->ee = SWRM_EE_CPU;
> +	ret = of_property_read_u32(dev->of_node, "qcom,swr-master-ee-val", &ctrl->ee);
> +	if (ret)
> +		ret = of_property_read_u32(dev->of_node, "qcom,ee", &ctrl->ee);
> +	if (ret)
> +		ctrl->ee = SWRM_EE_CPU;
> +	if (ctrl->ee > SWRM_MAX_EE) {
> +		dev_warn(dev, "invalid SoundWire EE %u, using EE%u\n",
> +			 ctrl->ee, SWRM_EE_CPU);
> +		ctrl->ee = SWRM_EE_CPU;
> +	}
>  	ctrl->max_reg = data->max_reg;
> +	/*
> +	 * Defer EE register window selection until HW version is known.
> +	 * For v2.0+ the IRQ/FIFO window is EE-banked.
> +	 */
>  	ctrl->reg_layout = data->reg_layout;
>  	ctrl->rows_index = sdw_find_row_index(data->default_rows);
>  	ctrl->cols_index = sdw_find_col_index(data->default_cols);
> @@ -1623,6 +1671,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
>  	prop->default_row = data->default_rows;
>  
>  	ctrl->reg_read(ctrl, SWRM_COMP_HW_VERSION, &ctrl->version);
> +	qcom_swrm_set_ee_register_layout(ctrl, data);
>  
>  	ret = devm_request_threaded_irq(dev, ctrl->irq, NULL,
>  					qcom_swrm_irq_handler,
> @@ -1733,16 +1782,19 @@ static int __maybe_unused swrm_runtime_resume(struct device *dev)
>  		reset_control_reset(ctrl->audio_cgcr);
>  
>  		if (ctrl->version == SWRM_VERSION_1_7_0) {
> -			ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, SWRM_EE_CPU);
> +			ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, ctrl->ee);
>  			ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL,
> -					SWRM_MCP_BUS_CLK_START << SWRM_EE_CPU);
> +					SWRM_MCP_BUS_CLK_START << ctrl->ee);
>  		} else if (ctrl->version >= SWRM_VERSION_2_0_0) {
> -			ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, SWRM_EE_CPU);
> -			ctrl->reg_write(ctrl, SWRM_V2_0_CLK_CTRL,
> +			ctrl->reg_write(ctrl, SWRM_LINK_MANAGER_EE, ctrl->ee);
> +			ctrl->reg_write(ctrl, SWRM_V2_0_CLK_CTRL +
> +					((int)ctrl->ee - SWRM_EE_CPU) *
> +					SWRM_V2_REG_EE_STRIDE,
>  					SWRM_V2_0_CLK_CTRL_CLK_START);
>  		} else {
>  			ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL, SWRM_MCP_BUS_CLK_START);
>  		}
> +
>  		ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CLEAR],
>  			SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET);
>