Re: [PATCH 02/19] soundwire: amd: Add support for AMD Master driver

On 1/11/23 03:02, Vijendar Mukunda wrote:

AMD ACP IP block has two soundwire controller devices.

s/controller/manager?

Add support for

• Master driver probe & remove sequence
• Helper functions to enable/disable interrupts, Initialize sdw controller, enable sdw pads
• Master driver sdw_master_ops & port_ops callbacks

Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com

drivers/soundwire/amd_master.c | 1075 +++++++++++++++++++++++++++++ drivers/soundwire/amd_master.h | 279 ++++++++ include/linux/soundwire/sdw_amd.h | 21 + 3 files changed, 1375 insertions(+) create mode 100644 drivers/soundwire/amd_master.c create mode 100644 drivers/soundwire/amd_master.h

diff --git a/drivers/soundwire/amd_master.c b/drivers/soundwire/amd_master.c new file mode 100644 index 000000000000..7e1f618254ac --- /dev/null +++ b/drivers/soundwire/amd_master.c @@ -0,0 +1,1075 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • SoundWire AMD Master driver
• • Copyright 2023 Advanced Micro Devices, Inc.
• */

+#include <linux/completion.h> +#include <linux/device.h> +#include <linux/io.h> +#include <linux/jiffies.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/soundwire/sdw.h> +#include <linux/soundwire/sdw_registers.h> +#include <linux/soundwire/sdw_amd.h> +#include <linux/wait.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include "bus.h" +#include "amd_master.h"

+#define DRV_NAME "amd_sdw_controller"

+#define to_amd_sdw(b) container_of(b, struct amd_sdwc_ctrl, bus)

+static int amd_enable_sdw_pads(struct amd_sdwc_ctrl *ctrl) +{

• u32 sw_pad_enable_mask;
• u32 sw_pad_pulldown_mask;
• u32 sw_pad_pulldown_val;
• u32 val = 0;
• switch (ctrl->instance) {

Goodness no. A controller has one or more masters. It cannot have pins as described in the SoundWire master specification.

• case ACP_SDW0:

• sw_pad_enable_mask = AMD_SDW0_PAD_KEEPER_EN_MASK;
  

• sw_pad_pulldown_mask = AMD_SDW0_PAD_PULLDOWN_CTRL_ENABLE_MASK;
  

• break;
  

• case ACP_SDW1:

• sw_pad_enable_mask = AMD_SDW1_PAD_KEEPER_EN_MASK;
  

• sw_pad_pulldown_mask = AMD_SDW1_PAD_PULLDOWN_CTRL_ENABLE_MASK;
  

• break;
  

• default:

• return -EINVAL;
  

• }
• mutex_lock(ctrl->sdw_lock);
• val = acp_reg_readl(ctrl->mmio + ACP_SW_PAD_KEEPER_EN);
• val |= sw_pad_enable_mask;
• acp_reg_writel(val, ctrl->mmio + ACP_SW_PAD_KEEPER_EN);
• mutex_unlock(ctrl->sdw_lock);
• usleep_range(1000, 1500);
• mutex_lock(ctrl->sdw_lock);
• sw_pad_pulldown_val = acp_reg_readl(ctrl->mmio + ACP_PAD_PULLDOWN_CTRL);
• sw_pad_pulldown_val &= sw_pad_pulldown_mask;
• acp_reg_writel(sw_pad_pulldown_val, ctrl->mmio + ACP_PAD_PULLDOWN_CTRL);
• mutex_unlock(ctrl->sdw_lock);
• return 0;

+}

+static int amd_init_sdw_controller(struct amd_sdwc_ctrl *ctrl) +{

• u32 acp_sw_en_reg, acp_sw_en_stat_reg, sw_bus_reset_reg;
• u32 val = 0;
• u32 timeout = 0;
• u32 retry_count = 0;
• switch (ctrl->instance) {
• case ACP_SDW0:

• acp_sw_en_reg = ACP_SW_EN;
  

• acp_sw_en_stat_reg = ACP_SW_EN_STATUS;
  

• sw_bus_reset_reg = ACP_SW_BUS_RESET_CTRL;
  

• break;
  

• case ACP_SDW1:

• acp_sw_en_reg = ACP_P1_SW_EN;
  

• acp_sw_en_stat_reg = ACP_P1_SW_EN_STATUS;
  

• sw_bus_reset_reg = ACP_P1_SW_BUS_RESET_CTRL;
  

• break;
  

• default:

• return -EINVAL;
  

• }
• acp_reg_writel(AMD_SDW_ENABLE, ctrl->mmio + acp_sw_en_reg);
• do {

• val = acp_reg_readl(ctrl->mmio + acp_sw_en_stat_reg);
  

• if (val)
  

• 	break;
  

• usleep_range(10, 50);
  

• } while (retry_count++ < AMD_SDW_STAT_MAX_RETRY_COUNT);
• if (retry_count > AMD_SDW_STAT_MAX_RETRY_COUNT)

• return -ETIMEDOUT;
  

• /* Sdw Controller reset */
• acp_reg_writel(AMD_SDW_BUS_RESET_REQ, ctrl->mmio + sw_bus_reset_reg);
• val = acp_reg_readl(ctrl->mmio + sw_bus_reset_reg);
• while (!(val & AMD_SDW_BUS_RESET_DONE)) {

• val = acp_reg_readl(ctrl->mmio + sw_bus_reset_reg);
  

• if (timeout > AMD_DELAY_LOOP_ITERATION)
  

• 	break;
  

• usleep_range(1, 5);
  

• timeout++;
  

• }

no test on timeout here to check if the bus was indeed reset?

If you are talking about bus_reset you are referring to a master btw. The terms bus/master/link are interchangeable. A controller is not defined in the SoundWire specification, this is part of the DisCo spec to deal with enumeration when multiple bus/master/link are supported in the platform.

• timeout = 0;
• acp_reg_writel(AMD_SDW_BUS_RESET_CLEAR_REQ, ctrl->mmio + sw_bus_reset_reg);
• val = acp_reg_readl(ctrl->mmio + sw_bus_reset_reg);
• while (val) {

• val = acp_reg_readl(ctrl->mmio + sw_bus_reset_reg);
  

• if (timeout > AMD_DELAY_LOOP_ITERATION)
  

• 	break;
  

• usleep_range(1, 5);
  

• timeout++;
  

• }
• if (timeout == AMD_DELAY_LOOP_ITERATION) {

• dev_err(ctrl->dev, "Failed to reset SW%x Soundwire Controller\n", ctrl->instance);
  

• return -ETIMEDOUT;
  

• }
• retry_count = 0;
• acp_reg_writel(AMD_SDW_DISABLE, ctrl->mmio + acp_sw_en_reg);
• do {

• val = acp_reg_readl(ctrl->mmio + acp_sw_en_stat_reg);
  

• if (!val)
  

• 	break;
  

• usleep_range(10, 50);
  

• } while (retry_count++ < AMD_SDW_STAT_MAX_RETRY_COUNT);
• if (retry_count > AMD_SDW_STAT_MAX_RETRY_COUNT)

• return -ETIMEDOUT;
  

• return 0;

+}

+static int amd_enable_sdw_controller(struct amd_sdwc_ctrl *ctrl) +{

• u32 acp_sw_en_reg;
• u32 acp_sw_en_stat_reg;
• u32 val = 0;
• u32 retry_count = 0;
• switch (ctrl->instance) {
• case ACP_SDW0:

• acp_sw_en_reg = ACP_SW_EN;
  

• acp_sw_en_stat_reg = ACP_SW_EN_STATUS;
  

• break;
  

• case ACP_SDW1:

• acp_sw_en_reg = ACP_P1_SW_EN;
  

• acp_sw_en_stat_reg = ACP_P1_SW_EN_STATUS;
  

• break;
  

• default:

• return -EINVAL;
  

• }
• acp_reg_writel(AMD_SDW_ENABLE, ctrl->mmio + acp_sw_en_reg);
• do {

• val = acp_reg_readl(ctrl->mmio + acp_sw_en_stat_reg);
  

• if (val)
  

• 	break;
  

• usleep_range(10, 50);
  

• } while (retry_count++ < AMD_SDW_STAT_MAX_RETRY_COUNT);
• if (retry_count > AMD_SDW_STAT_MAX_RETRY_COUNT)

• return -ETIMEDOUT;
  

• return 0;

+}

+static int amd_disable_sdw_controller(struct amd_sdwc_ctrl *ctrl) +{

• u32 clk_resume_ctrl_reg;
• u32 acp_sw_en_reg;
• u32 acp_sw_en_stat_reg;
• u32 val = 0;
• u32 retry_count = 0;
• switch (ctrl->instance) {
• case ACP_SDW0:

• acp_sw_en_reg = ACP_SW_EN;
  

• acp_sw_en_stat_reg = ACP_SW_EN_STATUS;
  

• clk_resume_ctrl_reg = ACP_SW_CLK_RESUME_CTRL;
  

• break;
  

• case ACP_SDW1:

• acp_sw_en_reg = ACP_P1_SW_EN;
  

• acp_sw_en_stat_reg = ACP_P1_SW_EN_STATUS;
  

• clk_resume_ctrl_reg = ACP_P1_SW_CLK_RESUME_CTRL;
  

• break;
  

• default:

• return -EINVAL;
  

• }
• acp_reg_writel(AMD_SDW_DISABLE, ctrl->mmio + acp_sw_en_reg);
• /*

• * After invoking controller disable sequence, check whether
  

• * controller has executed clock stop sequence. In this case,
  

• * controller should ignore checking enable status register.
  

again clock stop is a sequence at the master/link/bus level, not the controller.

• */
  

• val = acp_reg_readl(ctrl->mmio + clk_resume_ctrl_reg);
• if (val)

• return 0;
  

• do {

• val = acp_reg_readl(ctrl->mmio + acp_sw_en_stat_reg);
  

• if (!val)
  

• 	break;
  

• usleep_range(10, 50);
  

• } while (retry_count++ < AMD_SDW_STAT_MAX_RETRY_COUNT);
• if (retry_count > AMD_SDW_STAT_MAX_RETRY_COUNT)

• return -ETIMEDOUT;
  

• return 0;

+}

+static int amd_enable_sdw_interrupts(struct amd_sdwc_ctrl *ctrl) +{

• u32 val;
• u32 acp_ext_intr_stat, acp_ext_intr_ctrl, acp_sdw_intr_mask;
• u32 sw_stat_mask_0to7, sw_stat_mask_8to11, sw_err_intr_mask;
• switch (ctrl->instance) {
• case ACP_SDW0:

• acp_ext_intr_ctrl = ACP_EXTERNAL_INTR_CNTL;
  

should be renamed and end in CNTL0 if the other is CNTL1

And it's manager anyways, not controller.

• acp_sdw_intr_mask = AMD_SDW0_EXT_INTR_MASK;
  

• acp_ext_intr_stat = ACP_EXTERNAL_INTR_STAT;
  

• sw_stat_mask_0to7 = SW_STATE_CHANGE_STATUS_MASK_0TO7;
  

• sw_stat_mask_8to11 = SW_STATE_CHANGE_STATUS_MASK_8TO11;
  

• sw_err_intr_mask = SW_ERROR_INTR_MASK;
  

• break;
  

• case ACP_SDW1:

• acp_ext_intr_ctrl = ACP_EXTERNAL_INTR_CNTL1;
  

• acp_sdw_intr_mask = AMD_SDW1_EXT_INTR_MASK;
  

• acp_ext_intr_stat = ACP_EXTERNAL_INTR_STAT1;
  

• sw_stat_mask_0to7 = P1_SW_STATE_CHANGE_STATUS_MASK_0TO7;
  

• sw_stat_mask_8to11 = P1_SW_STATE_CHANGE_STATUS_MASK_8TO11;
  

• sw_err_intr_mask = P1_SW_ERROR_INTR_MASK;
  

• break;
  

• default:

• return -EINVAL;
  

• }
• mutex_lock(ctrl->sdw_lock);
• val = acp_reg_readl(ctrl->mmio + acp_ext_intr_ctrl);
• val |= acp_sdw_intr_mask;
• acp_reg_writel(val, ctrl->mmio + acp_ext_intr_ctrl);
• val = acp_reg_readl(ctrl->mmio + acp_ext_intr_ctrl);
• mutex_unlock(ctrl->sdw_lock);
• dev_dbg(ctrl->dev, "%s: acp_ext_intr_ctrl[0x%x]:0x%x\n", __func__, acp_ext_intr_ctrl, val);
• val = acp_reg_readl(ctrl->mmio + acp_ext_intr_stat);
• if (val)

• acp_reg_writel(val, ctrl->mmio + acp_ext_intr_stat);
  

• acp_reg_writel(AMD_SDW_IRQ_MASK_0TO7, ctrl->mmio + sw_stat_mask_0to7);
• acp_reg_writel(AMD_SDW_IRQ_MASK_8TO11, ctrl->mmio + sw_stat_mask_8to11);
• acp_reg_writel(AMD_SDW_IRQ_ERROR_MASK, ctrl->mmio + sw_err_intr_mask);
• return 0;

+}

+static u64 amd_sdwc_send_cmd_get_resp(struct amd_sdwc_ctrl *ctrl, u32 lword, u32 uword) +{

• u64 resp = 0;
• u32 imm_cmd_stat_reg, imm_cmd_uword_reg, imm_cmd_lword_reg;
• u32 imm_resp_uword_reg, imm_resp_lword_reg;
• u32 resp_lower, resp_high;
• u32 sts = 0;
• u32 timeout = 0;
• switch (ctrl->instance) {
• case ACP_SDW0:

• imm_cmd_stat_reg = SW_IMM_CMD_STS;
  

• imm_cmd_uword_reg = SW_IMM_CMD_UPPER_WORD;
  

• imm_cmd_lword_reg = SW_IMM_CMD_LOWER_QWORD;
  

• imm_resp_uword_reg = SW_IMM_RESP_UPPER_WORD;
  

• imm_resp_lword_reg = SW_IMM_RESP_LOWER_QWORD;
  

• break;
  

• case ACP_SDW1:

• imm_cmd_stat_reg = P1_SW_IMM_CMD_STS;
  

• imm_cmd_uword_reg = P1_SW_IMM_CMD_UPPER_WORD;
  

• imm_cmd_lword_reg = P1_SW_IMM_CMD_LOWER_QWORD;
  

• imm_resp_uword_reg = P1_SW_IMM_RESP_UPPER_WORD;
  

• imm_resp_lword_reg = P1_SW_IMM_RESP_LOWER_QWORD;
  

naming consistency would be good, the P1 is sometimes a prefix, sometimes a suffix, sometimes in the middle. Pick one.

• break;
  

• default:

• return -EINVAL;
  

• }
• sts = acp_reg_readl(ctrl->mmio + imm_cmd_stat_reg);
• while (sts & AMD_SDW_IMM_CMD_BUSY) {

• sts = acp_reg_readl(ctrl->mmio + imm_cmd_stat_reg);
  

• if (timeout > AMD_SDW_RETRY_COUNT) {
  

• 	dev_err(ctrl->dev, "SDW%x previous cmd status clear failed\n",
  

• 		ctrl->instance);
  

• 	return -ETIMEDOUT;
  

• }
  

• timeout++;
  

• }
• timeout = 0;
• if (sts & AMD_SDW_IMM_RES_VALID) {

• dev_err(ctrl->dev, "SDW%x controller is in bad state\n", ctrl->instance);
  

• acp_reg_writel(0x00, ctrl->mmio + imm_cmd_stat_reg);
  

• }
• acp_reg_writel(uword, ctrl->mmio + imm_cmd_uword_reg);
• acp_reg_writel(lword, ctrl->mmio + imm_cmd_lword_reg);
• sts = acp_reg_readl(ctrl->mmio + imm_cmd_stat_reg);
• while (!(sts & AMD_SDW_IMM_RES_VALID)) {

• sts = acp_reg_readl(ctrl->mmio + imm_cmd_stat_reg);
  

• if (timeout > AMD_SDW_RETRY_COUNT) {
  

• 	dev_err(ctrl->dev, "SDW%x cmd response timeout occurred\n", ctrl->instance);
  

• 	return -ETIMEDOUT;
  

• }
  

• timeout++;
  

• }
• resp_high = acp_reg_readl(ctrl->mmio + imm_resp_uword_reg);
• resp_lower = acp_reg_readl(ctrl->mmio + imm_resp_lword_reg);
• timeout = 0;
• acp_reg_writel(AMD_SDW_IMM_RES_VALID, ctrl->mmio + imm_cmd_stat_reg);
• while ((sts & AMD_SDW_IMM_RES_VALID)) {

• sts = acp_reg_readl(ctrl->mmio + imm_cmd_stat_reg);
  

• if (timeout > AMD_SDW_RETRY_COUNT) {
  

• 	dev_err(ctrl->dev, "SDW%x cmd status retry failed\n", ctrl->instance);
  

• 	return -ETIMEDOUT;
  

• }
  

• timeout++;
  

• }
• resp = resp_high;
• resp = (resp << 32) | resp_lower;
• return resp;

+}

+static enum sdw_command_response +amd_program_scp_addr(struct amd_sdwc_ctrl *ctrl, struct sdw_msg *msg) +{

• struct sdw_msg scp_msg = {0};
• u64 response_buf[2] = {0};
• u32 uword = 0, lword = 0;
• int nack = 0, no_ack = 0;
• int index, timeout = 0;
• scp_msg.dev_num = msg->dev_num;
• scp_msg.addr = SDW_SCP_ADDRPAGE1;
• scp_msg.buf = &msg->addr_page1;
• amd_sdwc_ctl_word_prep(&lword, &uword, AMD_SDW_CMD_WRITE, &scp_msg, 0);
• response_buf[0] = amd_sdwc_send_cmd_get_resp(ctrl, lword, uword);
• scp_msg.addr = SDW_SCP_ADDRPAGE2;
• scp_msg.buf = &msg->addr_page2;
• amd_sdwc_ctl_word_prep(&lword, &uword, AMD_SDW_CMD_WRITE, &scp_msg, 0);
• response_buf[1] = amd_sdwc_send_cmd_get_resp(ctrl, lword, uword);
• /* check response the writes */

response to the writes? after the writes?

• for (index = 0; index < 2; index++) {

• if (response_buf[index] == -ETIMEDOUT) {
  

• 	dev_err(ctrl->dev, "Program SCP cmd timeout\n");
  

• 	timeout = 1;
  

• } else if (!(response_buf[index] & AMD_SDW_MCP_RESP_ACK)) {
  

• 	no_ack = 1;
  

• 	if (response_buf[index] & AMD_SDW_MCP_RESP_NACK) {
  

• 		nack = 1;
  

• 		dev_err(ctrl->dev, "Program SCP NACK received\n");
  

• 	}
  

this is a copy of the cadence_master.c code... With the error added that this is not for a controller but for a master...

• }
  

• }
• if (timeout) {

• dev_err_ratelimited(ctrl->dev,
  

• 		    "SCP_addrpage command timeout for Slave %d\n", msg->dev_num);
  

• return SDW_CMD_TIMEOUT;
  

• }
• if (nack) {

• dev_err_ratelimited(ctrl->dev,
  

• 		    "SCP_addrpage NACKed for Slave %d\n", msg->dev_num);
  

• return SDW_CMD_FAIL;
  

• }
• if (no_ack) {

• dev_dbg_ratelimited(ctrl->dev,
  

• 		    "SCP_addrpage ignored for Slave %d\n", msg->dev_num);
  

• return SDW_CMD_IGNORED;
  

• }
• return SDW_CMD_OK;

this should probably become a helper since the response is really the same as in cadence_master.c

There's really room for optimization and reuse here.

+}

+static int amd_prep_msg(struct amd_sdwc_ctrl *ctrl, struct sdw_msg *msg, int *cmd) +{

• int ret;
• if (msg->page) {

• ret = amd_program_scp_addr(ctrl, msg);
  

• if (ret) {
  

• 	msg->len = 0;
  

• 	return ret;
  

• }
  

• }
• switch (msg->flags) {
• case SDW_MSG_FLAG_READ:

• *cmd = AMD_SDW_CMD_READ;
  

• break;
  

• case SDW_MSG_FLAG_WRITE:

• *cmd = AMD_SDW_CMD_WRITE;
  

• break;
  

• default:

• dev_err(ctrl->dev, "Invalid msg cmd: %d\n", msg->flags);
  

• return -EINVAL;
  

• }
• return 0;

+}

this is the same code as in cadence_master.c

you just replaced sdw_cnds by amd_sdw_ctrl (which is a mistake) and cdns->dev by ctrl->dev.

+static unsigned int _amd_sdwc_xfer_msg(struct amd_sdwc_ctrl *ctrl, struct sdw_msg *msg,

• 		       int cmd, int cmd_offset)
  

+{

• u64 response = 0;
• u32 uword = 0, lword = 0;
• int nack = 0, no_ack = 0;
• int timeout = 0;
• amd_sdwc_ctl_word_prep(&lword, &uword, cmd, msg, cmd_offset);
• response = amd_sdwc_send_cmd_get_resp(ctrl, lword, uword);
• if (response & AMD_SDW_MCP_RESP_ACK) {

• if (cmd == AMD_SDW_CMD_READ)
  

• 	msg->buf[cmd_offset] = FIELD_GET(AMD_SDW_MCP_RESP_RDATA, response);
  

• } else {

• no_ack = 1;
  

• if (response == -ETIMEDOUT) {
  

• 	timeout = 1;
  

• } else if (response & AMD_SDW_MCP_RESP_NACK) {
  

• 	nack = 1;
  

• 	dev_err(ctrl->dev, "Program SCP NACK received\n");
  

• }
  

• }
• if (timeout) {

• dev_err_ratelimited(ctrl->dev, "command timeout for Slave %d\n", msg->dev_num);
  

• return SDW_CMD_TIMEOUT;
  

• }
• if (nack) {

• dev_err_ratelimited(ctrl->dev,
  

• 		    "command response NACK received for Slave %d\n", msg->dev_num);
  

• return SDW_CMD_FAIL;
  

• }
• if (no_ack) {

• dev_err_ratelimited(ctrl->dev, "command is ignored for Slave %d\n", msg->dev_num);
  

• return SDW_CMD_IGNORED;
  

• }
• return SDW_CMD_OK;

+}

+static enum sdw_command_response amd_sdwc_xfer_msg(struct sdw_bus *bus, struct sdw_msg *msg) +{

• struct amd_sdwc_ctrl *ctrl = to_amd_sdw(bus);
• int ret, i;
• int cmd = 0;
• ret = amd_prep_msg(ctrl, msg, &cmd);
• if (ret)

• return SDW_CMD_FAIL_OTHER;
  

• for (i = 0; i < msg->len; i++) {

• ret = _amd_sdwc_xfer_msg(ctrl, msg, cmd, i);
  

• if (ret)
  

• 	return ret;
  

• }
• return SDW_CMD_OK;

+}

+static enum sdw_command_response +amd_reset_page_addr(struct sdw_bus *bus, unsigned int dev_num) +{

• struct amd_sdwc_ctrl *ctrl = to_amd_sdw(bus);
• struct sdw_msg msg;
• /* Create dummy message with valid device number */
• memset(&msg, 0, sizeof(msg));
• msg.dev_num = dev_num;
• return amd_program_scp_addr(ctrl, &msg);

+}

+static u32 amd_sdwc_read_ping_status(struct sdw_bus *bus) +{

• struct amd_sdwc_ctrl *ctrl = to_amd_sdw(bus);
• u64 response;
• u32 slave_stat = 0;
• response = amd_sdwc_send_cmd_get_resp(ctrl, 0, 0);
• /* slave status from ping response*/
• slave_stat = FIELD_GET(AMD_SDW_MCP_SLAVE_STAT_0_3, response);
• slave_stat |= FIELD_GET(AMD_SDW_MCP_SLAVE_STAT_4_11, response) << 8;
• dev_dbg(ctrl->dev, "%s: slave_stat:0x%x\n", __func__, slave_stat);
• return slave_stat;

+}

+static void amd_sdwc_compute_slave_ports(struct sdw_master_runtime *m_rt,

• 			 struct sdw_transport_data *t_data)
  

+{

• struct sdw_slave_runtime *s_rt = NULL;
• struct sdw_port_runtime *p_rt;
• int port_bo, sample_int;
• unsigned int rate, bps, ch = 0;
• unsigned int slave_total_ch;
• struct sdw_bus_params *b_params = &m_rt->bus->params;
• port_bo = t_data->block_offset;
• list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {

• rate = m_rt->stream->params.rate;
  

• bps = m_rt->stream->params.bps;
  

• sample_int = (m_rt->bus->params.curr_dr_freq / rate);
  

• slave_total_ch = 0;
  

• list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
  

• 	ch = sdw_ch_mask_to_ch(p_rt->ch_mask);
  

• 	sdw_fill_xport_params(&p_rt->transport_params,
  

• 			      p_rt->num, false,
  

• 			      SDW_BLK_GRP_CNT_1,
  

• 			      sample_int, port_bo, port_bo >> 8,
  

• 			      t_data->hstart,
  

• 			      t_data->hstop,
  

• 			      SDW_BLK_PKG_PER_PORT, 0x0);
  

• 	sdw_fill_port_params(&p_rt->port_params,
  

• 			     p_rt->num, bps,
  

• 			     SDW_PORT_FLOW_MODE_ISOCH,
  

• 			     b_params->s_data_mode);
  

• 	port_bo += bps * ch;
  

• 	slave_total_ch += ch;
  

• }
  

• if (m_rt->direction == SDW_DATA_DIR_TX &&
  

•     m_rt->ch_count == slave_total_ch) {
  

• 	port_bo = t_data->block_offset;
  

• }
  

• }

+}

ok, this is really bad.

This is a verbatim copy of the same function in generic_bandwidth_allocation.c

see https://elixir.bootlin.com/linux/latest/source/drivers/soundwire/generic_ban...

You only removed the comments and renamed the function.

Seriously? Why would you do that?

And in addition, this has *NOTHING* to do with the master support.

Programming the ports on peripheral side is something that happens at the stream level.

I am afraid it's a double NAK, or rather NAK^2 from me here.

+static int amd_sdwc_compute_params(struct sdw_bus *bus) +{

• struct sdw_transport_data t_data = {0};
• struct sdw_master_runtime *m_rt;
• struct sdw_port_runtime *p_rt;
• struct sdw_bus_params *b_params = &bus->params;
• int port_bo, hstart, hstop, sample_int;
• unsigned int rate, bps;
• port_bo = 0;
• hstart = 1;
• hstop = bus->params.col - 1;
• t_data.hstop = hstop;
• t_data.hstart = hstart;
• list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {

• rate = m_rt->stream->params.rate;
  

• bps = m_rt->stream->params.bps;
  

• sample_int = (bus->params.curr_dr_freq / rate);
  

• list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
  

• 	port_bo = (p_rt->num * 64) + 1;
  

• 	dev_dbg(bus->dev, "p_rt->num=%d hstart=%d hstop=%d port_bo=%d\n",
  

• 		p_rt->num, hstart, hstop, port_bo);
  

• 	sdw_fill_xport_params(&p_rt->transport_params, p_rt->num,
  

• 			      false, SDW_BLK_GRP_CNT_1, sample_int,
  

• 			      port_bo, port_bo >> 8, hstart, hstop,
  

• 			      SDW_BLK_PKG_PER_PORT, 0x0);
  

• 	sdw_fill_port_params(&p_rt->port_params,
  

• 			     p_rt->num, bps,
  

• 			     SDW_PORT_FLOW_MODE_ISOCH,
  

• 			     b_params->m_data_mode);
  

• 	t_data.hstart = hstart;
  

• 	t_data.hstop = hstop;
  

• 	t_data.block_offset = port_bo;
  

• 	t_data.sub_block_offset = 0;
  

• }
  

• amd_sdwc_compute_slave_ports(m_rt, &t_data);
  

• }
• return 0;

+}

this is a variation on sdw_compute_master_ports() in generic_allocation.c

You would need a lot more comments to convince me that this is intentional and needed.

+static int amd_sdwc_port_params(struct sdw_bus *bus, struct sdw_port_params *p_params,

• 		unsigned int bank)
  

+{

• struct amd_sdwc_ctrl *ctrl = to_amd_sdw(bus);
• u32 channel_type, frame_fmt_reg, dpn_frame_fmt;
• dev_dbg(ctrl->dev, "%s: p_params->num:0x%x\n", __func__, p_params->num);
• switch (ctrl->instance) {
• case ACP_SDW0:

• channel_type = p_params->num;
  

• break;
  

• case ACP_SDW1:

• channel_type = p_params->num + ACP_SDW0_MAX_DAI;
  

• break;
  

• default:

• return -EINVAL;
  

• }
• switch (channel_type) {
• case ACP_SDW0_AUDIO_TX:

you'll have to explain what you mean by 'channel_type'

This looks like the streams that can be supported by this master implementation, with dailinks for each.

• frame_fmt_reg = ACP_SW_AUDIO_TX_FRAME_FORMAT;
  

• break;
  

• case ACP_SDW0_HS_TX:

• frame_fmt_reg = ACP_SW_HEADSET_TX_FRAME_FORMAT;
  

• break;
  

• case ACP_SDW0_BT_TX:

• frame_fmt_reg = ACP_SW_BT_TX_FRAME_FORMAT;
  

• break;
  

• case ACP_SDW1_BT_TX:

• frame_fmt_reg = ACP_P1_SW_BT_TX_FRAME_FORMAT;
  

• break;
  

• case ACP_SDW0_AUDIO_RX:

• frame_fmt_reg = ACP_SW_AUDIO_RX_FRAME_FORMAT;
  

• break;
  

• case ACP_SDW0_HS_RX:

• frame_fmt_reg = ACP_SW_HEADSET_RX_FRAME_FORMAT;
  

• break;
  

• case ACP_SDW0_BT_RX:

• frame_fmt_reg = ACP_SW_BT_RX_FRAME_FORMAT;
  

• break;
  

• case ACP_SDW1_BT_RX:

• frame_fmt_reg = ACP_P1_SW_BT_RX_FRAME_FORMAT;
  

• break;
  

• default:

• dev_err(bus->dev, "%s:Invalid channel:%d\n", __func__, channel_type);
  

• return -EINVAL;
  

• }
• dpn_frame_fmt = acp_reg_readl(ctrl->mmio + frame_fmt_reg);
• u32p_replace_bits(&dpn_frame_fmt, p_params->flow_mode, AMD_DPN_FRAME_FMT_PFM);
• u32p_replace_bits(&dpn_frame_fmt, p_params->data_mode, AMD_DPN_FRAME_FMT_PDM);
• u32p_replace_bits(&dpn_frame_fmt, p_params->bps - 1, AMD_DPN_FRAME_FMT_WORD_LEN);
• acp_reg_writel(dpn_frame_fmt, ctrl->mmio + frame_fmt_reg);
• return 0;

+}

+static int amd_sdwc_transport_params(struct sdw_bus *bus,

• 		     struct sdw_transport_params *params,
  

• 		     enum sdw_reg_bank bank)
  

+{

• struct amd_sdwc_ctrl *ctrl = to_amd_sdw(bus);
• u32 ssp_counter_reg;
• u32 dpn_frame_fmt;
• u32 dpn_sampleinterval;
• u32 dpn_hctrl;
• u32 dpn_offsetctrl;
• u32 dpn_lanectrl;
• u32 channel_type;
• u32 frame_fmt_reg, sample_int_reg, hctrl_dp0_reg;
• u32 offset_reg, lane_ctrl_reg;
• switch (ctrl->instance) {
• case ACP_SDW0:

• ssp_counter_reg = ACP_SW_SSP_COUNTER;
  

• channel_type = params->port_num;
  

• break;
  

• case ACP_SDW1:

• ssp_counter_reg = ACP_P1_SW_SSP_COUNTER;
  

• channel_type = params->port_num + ACP_SDW0_MAX_DAI;
  

There's obviously a dependency between SDW0 and SDW1 managers that you haven't described?

• break;
  

• default:

• return -EINVAL;
  

• }
• acp_reg_writel(AMD_SDW_SSP_COUNTER_VAL, ctrl->mmio + ssp_counter_reg);
• dev_dbg(bus->dev, "%s: p_params->num:0x%x entry channel_type:0x%x\n",

• __func__, params->port_num, channel_type);
  

• switch (channel_type) {
• case ACP_SDW0_AUDIO_TX:
• {

• frame_fmt_reg = ACP_SW_AUDIO_TX_FRAME_FORMAT;
  

• sample_int_reg = ACP_SW_AUDIO_TX_SAMPLEINTERVAL;
  

• hctrl_dp0_reg = ACP_SW_AUDIO_TX_HCTRL_DP0;
  

• offset_reg = ACP_SW_AUDIO_TX_OFFSET_DP0;
  

• lane_ctrl_reg = ACP_SW_AUDIO_TX_CHANNEL_ENABLE_DP0;
  

This is confusing. Is this about enabling a stream or selecting the lane for this port? Same for all cases.

is this saying that the two cases are handled by the same register - unlike what is normative for the peripherals where the two concepts are handeld in DPN_ChannelEn and DPN_LaneCtrl registers?

• break;
  

• }
• case ACP_SDW0_HS_TX:
• {

• frame_fmt_reg = ACP_SW_HEADSET_TX_FRAME_FORMAT;
  

• sample_int_reg = ACP_SW_HEADSET_TX_SAMPLEINTERVAL;
  

• hctrl_dp0_reg = ACP_SW_HEADSET_TX_HCTRL;
  

• offset_reg = ACP_SW_HEADSET_TX_OFFSET;
  

• lane_ctrl_reg = ACP_SW_HEADSET_TX_CHANNEL_ENABLE_DP0;
  

• break;
  

• }
• case ACP_SDW0_BT_TX:
• {

• frame_fmt_reg = ACP_SW_BT_TX_FRAME_FORMAT;
  

• sample_int_reg = ACP_SW_BT_TX_SAMPLEINTERVAL;
  

• hctrl_dp0_reg = ACP_SW_BT_TX_HCTRL;
  

• offset_reg = ACP_SW_BT_TX_OFFSET;
  

• lane_ctrl_reg = ACP_SW_BT_TX_CHANNEL_ENABLE_DP0;
  

• break;
  

• }
• case ACP_SDW1_BT_TX:
• {

• frame_fmt_reg = ACP_P1_SW_BT_TX_FRAME_FORMAT;
  

• sample_int_reg = ACP_P1_SW_BT_TX_SAMPLEINTERVAL;
  

• hctrl_dp0_reg = ACP_P1_SW_BT_TX_HCTRL;
  

• offset_reg = ACP_P1_SW_BT_TX_OFFSET;
  

• lane_ctrl_reg = ACP_P1_SW_BT_TX_CHANNEL_ENABLE_DP0;
  

• break;
  

• }
• case ACP_SDW0_AUDIO_RX:
• {

• frame_fmt_reg = ACP_SW_AUDIO_RX_FRAME_FORMAT;
  

• sample_int_reg = ACP_SW_AUDIO_RX_SAMPLEINTERVAL;
  

• hctrl_dp0_reg = ACP_SW_AUDIO_RX_HCTRL_DP0;
  

• offset_reg = ACP_SW_AUDIO_RX_OFFSET_DP0;
  

• lane_ctrl_reg = ACP_SW_AUDIO_RX_CHANNEL_ENABLE_DP0;
  

• break;
  

• }
• case ACP_SDW0_HS_RX:
• {

• frame_fmt_reg = ACP_SW_HEADSET_RX_FRAME_FORMAT;
  

• sample_int_reg = ACP_SW_HEADSET_RX_SAMPLEINTERVAL;
  

• hctrl_dp0_reg = ACP_SW_HEADSET_RX_HCTRL;
  

• offset_reg = ACP_SW_HEADSET_RX_OFFSET;
  

• lane_ctrl_reg = ACP_SW_HEADSET_RX_CHANNEL_ENABLE_DP0;
  

• break;
  

• }
• case ACP_SDW0_BT_RX:
• {

• frame_fmt_reg = ACP_SW_BT_RX_FRAME_FORMAT;
  

• sample_int_reg = ACP_SW_BT_RX_SAMPLEINTERVAL;
  

• hctrl_dp0_reg = ACP_SW_BT_RX_HCTRL;
  

• offset_reg = ACP_SW_BT_RX_OFFSET;
  

• lane_ctrl_reg = ACP_SW_BT_RX_CHANNEL_ENABLE_DP0;
  

• break;
  

• }
• case ACP_SDW1_BT_RX:
• {

• frame_fmt_reg = ACP_P1_SW_BT_RX_FRAME_FORMAT;
  

• sample_int_reg = ACP_P1_SW_BT_RX_SAMPLEINTERVAL;
  

• hctrl_dp0_reg = ACP_P1_SW_BT_RX_HCTRL;
  

• offset_reg = ACP_P1_SW_BT_RX_OFFSET;
  

• lane_ctrl_reg = ACP_P1_SW_BT_RX_CHANNEL_ENABLE_DP0;
  

• break;
  

• }
• default:

• dev_err(bus->dev, "%s:Invalid channel:%d\n", __func__, channel_type);
  

• return -EINVAL;
  

• }
• dpn_frame_fmt = acp_reg_readl(ctrl->mmio + frame_fmt_reg);
• u32p_replace_bits(&dpn_frame_fmt, params->blk_pkg_mode, AMD_DPN_FRAME_FMT_BLK_PKG_MODE);
• u32p_replace_bits(&dpn_frame_fmt, params->blk_grp_ctrl, AMD_DPN_FRAME_FMT_BLK_GRP_CTRL);
• u32p_replace_bits(&dpn_frame_fmt, SDW_STREAM_PCM, AMD_DPN_FRAME_FMT_PCM_OR_PDM);
• acp_reg_writel(dpn_frame_fmt, ctrl->mmio + frame_fmt_reg);
• dpn_sampleinterval = params->sample_interval - 1;
• acp_reg_writel(dpn_sampleinterval, ctrl->mmio + sample_int_reg);
• dpn_hctrl = FIELD_PREP(AMD_DPN_HCTRL_HSTOP, params->hstop);
• dpn_hctrl |= FIELD_PREP(AMD_DPN_HCTRL_HSTART, params->hstart);
• acp_reg_writel(dpn_hctrl, ctrl->mmio + hctrl_dp0_reg);
• dpn_offsetctrl = FIELD_PREP(AMD_DPN_OFFSET_CTRL_1, params->offset1);
• dpn_offsetctrl |= FIELD_PREP(AMD_DPN_OFFSET_CTRL_2, params->offset2);
• acp_reg_writel(dpn_offsetctrl, ctrl->mmio + offset_reg);
• dpn_lanectrl = acp_reg_readl(ctrl->mmio + lane_ctrl_reg);
• u32p_replace_bits(&dpn_lanectrl, params->lane_ctrl, AMD_DPN_CH_EN_LCTRL);
• acp_reg_writel(dpn_lanectrl, ctrl->mmio + lane_ctrl_reg);
• return 0;

+}

+static int amd_sdwc_port_enable(struct sdw_bus *bus,

• 		struct sdw_enable_ch *enable_ch,
  

• 		unsigned int bank)
  

+{

• struct amd_sdwc_ctrl *ctrl = to_amd_sdw(bus);
• u32 dpn_ch_enable;
• u32 ch_enable_reg, channel_type;
• switch (ctrl->instance) {
• case ACP_SDW0:

• channel_type = enable_ch->port_num;
  

• break;
  

• case ACP_SDW1:

• channel_type = enable_ch->port_num + ACP_SDW0_MAX_DAI;
  

• break;
  

• default:

• return -EINVAL;
  

• }
• switch (channel_type) {
• case ACP_SDW0_AUDIO_TX:

• ch_enable_reg = ACP_SW_AUDIO_TX_CHANNEL_ENABLE_DP0;
  

in the function above, I commented on

lane_ctrl_reg = ACP_SW_AUDIO_TX_CHANNEL_ENABLE_DP0;

This looks really weird. You need to add comments is this is really intentional.

• break;
  

• case ACP_SDW0_HS_TX:

• ch_enable_reg = ACP_SW_HEADSET_TX_CHANNEL_ENABLE_DP0;
  

• break;
  

• case ACP_SDW0_BT_TX:

• ch_enable_reg = ACP_SW_BT_TX_CHANNEL_ENABLE_DP0;
  

• break;
  

• case ACP_SDW1_BT_TX:

• ch_enable_reg = ACP_P1_SW_BT_TX_CHANNEL_ENABLE_DP0;
  

• break;
  

• case ACP_SDW0_AUDIO_RX:

• ch_enable_reg = ACP_SW_AUDIO_RX_CHANNEL_ENABLE_DP0;
  

• break;
  

• case ACP_SDW0_HS_RX:

• ch_enable_reg = ACP_SW_HEADSET_RX_CHANNEL_ENABLE_DP0;
  

• break;
  

• case ACP_SDW0_BT_RX:

• ch_enable_reg = ACP_SW_BT_RX_CHANNEL_ENABLE_DP0;
  

• break;
  

• case ACP_SDW1_BT_RX:

• ch_enable_reg = ACP_P1_SW_BT_RX_CHANNEL_ENABLE_DP0;
  

• break;
  

• default:

• dev_err(bus->dev, "%s:Invalid channel:%d\n", __func__, channel_type);
  

• return -EINVAL;
  

• }
• dpn_ch_enable = acp_reg_readl(ctrl->mmio + ch_enable_reg);
• u32p_replace_bits(&dpn_ch_enable, enable_ch->ch_mask, AMD_DPN_CH_EN_CHMASK);
• if (enable_ch->enable)

• acp_reg_writel(dpn_ch_enable, ctrl->mmio + ch_enable_reg);
  

• else

• acp_reg_writel(0, ctrl->mmio + ch_enable_reg);
  

• return 0;

+}

+static int sdw_master_read_amd_prop(struct sdw_bus *bus) +{

• struct amd_sdwc_ctrl *ctrl = to_amd_sdw(bus);
• struct fwnode_handle *link;
• struct sdw_master_prop *prop;
• u32 quirk_mask = 0;
• u32 wake_en_mask = 0;
• u32 power_mode_mask = 0;
• char name[32];
• prop = &bus->prop;
• /* Find master handle */
• snprintf(name, sizeof(name), "mipi-sdw-link-%d-subproperties", bus->link_id);
• link = device_get_named_child_node(bus->dev, name);
• if (!link) {

• dev_err(bus->dev, "Master node %s not found\n", name);
  

• return -EIO;
  

• }
• fwnode_property_read_u32(link, "amd-sdw-enable", &quirk_mask);
• if (!(quirk_mask & AMD_SDW_QUIRK_MASK_BUS_ENABLE))

• prop->hw_disabled = true;
  

same quirk as Intel, nice :-)

• prop->quirks = SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH |

•        SDW_MASTER_QUIRKS_CLEAR_INITIAL_PARITY;
  

And here too. Is this really needed or just-copy-pasted?

• fwnode_property_read_u32(link, "amd-sdw-wake-enable", &wake_en_mask);
• ctrl->wake_en_mask = wake_en_mask;
• fwnode_property_read_u32(link, "amd-sdw-power-mode", &power_mode_mask);
• ctrl->power_mode_mask = power_mode_mask;
• return 0;

+}

+static int amd_sdwc_probe(struct platform_device *pdev)

why not use an auxiliary device? we've moved away from platform devices maybe two years ago.

+{

• const struct acp_sdw_pdata *pdata = pdev->dev.platform_data;
• struct resource *res;
• struct device *dev = &pdev->dev;
• struct sdw_master_prop *prop;
• struct sdw_bus_params *params;
• struct amd_sdwc_ctrl *ctrl;
• int ret;
• if (!pdev->dev.platform_data) {

• dev_err(&pdev->dev, "platform_data not retrieved\n");
  

• return -ENODEV;
  

• }
• ctrl = devm_kzalloc(&pdev->dev, sizeof(struct amd_sdwc_ctrl), GFP_KERNEL);
• if (!ctrl)

• return -ENOMEM;
  

• res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
• if (!res) {

• dev_err(&pdev->dev, "IORESOURCE_MEM FAILED\n");
  

• return -ENOMEM;
  

• }
• ctrl->mmio = devm_ioremap(&pdev->dev, res->start, resource_size(res));
• if (IS_ERR(ctrl->mmio)) {

• dev_err(&pdev->dev, "mmio not found\n");
  

• return PTR_ERR(ctrl->mmio);
  

• }
• ctrl->instance = pdata->instance;
• ctrl->sdw_lock = pdata->sdw_lock;
• ctrl->rows_index = sdw_find_row_index(50);
• ctrl->cols_index = sdw_find_col_index(10);
• ctrl->dev = dev;
• dev_set_drvdata(&pdev->dev, ctrl);
• ctrl->bus.ops = &amd_sdwc_ops;
• ctrl->bus.port_ops = &amd_sdwc_port_ops;
• ctrl->bus.compute_params = &amd_sdwc_compute_params;
• ctrl->bus.clk_stop_timeout = 1;
• switch (ctrl->instance) {
• case ACP_SDW0:

• ctrl->num_dout_ports =  AMD_SDW0_MAX_TX_PORTS;
  

• ctrl->num_din_ports = AMD_SDW0_MAX_RX_PORTS;
  

• break;
  

• case ACP_SDW1:

• ctrl->num_dout_ports = AMD_SDW1_MAX_TX_PORTS;
  

• ctrl->num_din_ports = AMD_SDW1_MAX_RX_PORTS;
  

• break;
  

• default:

• return -EINVAL;
  

• }
• params = &ctrl->bus.params;
• params->max_dr_freq = AMD_SDW_DEFAULT_CLK_FREQ * 2;
• params->curr_dr_freq = AMD_SDW_DEFAULT_CLK_FREQ * 2;
• params->col = 10;
• params->row = 50;
• prop = &ctrl->bus.prop;
• prop->clk_freq = &amd_sdwc_freq_tbl[0];
• prop->mclk_freq = AMD_SDW_BUS_BASE_FREQ;
• ctrl->bus.link_id = ctrl->instance;
• ret = sdw_bus_master_add(&ctrl->bus, dev, dev->fwnode);
• if (ret) {

• dev_err(dev, "Failed to register Soundwire controller (%d)\n",
  

master. the confusion continues.

• 	ret);
  

• return ret;
  

• }
• INIT_WORK(&ctrl->probe_work, amd_sdwc_probe_work);
• schedule_work(&ctrl->probe_work);
• return 0;

+}

+static int amd_sdwc_remove(struct platform_device *pdev) +{

• struct amd_sdwc_ctrl *ctrl = dev_get_drvdata(&pdev->dev);
• int ret;
• amd_disable_sdw_interrupts(ctrl);
• sdw_bus_master_delete(&ctrl->bus);
• ret = amd_disable_sdw_controller(ctrl);
• return ret;

+}

+static struct platform_driver amd_sdwc_driver = {

• .probe = &amd_sdwc_probe,
• .remove = &amd_sdwc_remove,
• .driver = {

• .name	= "amd_sdw_controller",
  

• }

+}; +module_platform_driver(amd_sdwc_driver);

+MODULE_AUTHOR("Vijendar.Mukunda@amd.com"); +MODULE_DESCRIPTION("AMD soundwire driver"); +MODULE_LICENSE("GPL v2");

"GPL" is enough

+enum amd_sdw_channel {

• /* SDW0 */
• ACP_SDW0_AUDIO_TX = 0,
• ACP_SDW0_BT_TX,
• ACP_SDW0_HS_TX,
• ACP_SDW0_AUDIO_RX,
• ACP_SDW0_BT_RX,
• ACP_SDW0_HS_RX,
• /* SDW1 */
• ACP_SDW1_BT_TX,
• ACP_SDW1_BT_RX,

+};

you really need to comment on this. It looks like you've special-cased manager ports for specific usages? This is perfectly fine in closed applications, but it's not clear how it might work with headset, amplifier and mic codec devices.

diff --git a/include/linux/soundwire/sdw_amd.h b/include/linux/soundwire/sdw_amd.h index f0123815af46..5ec39f8c2f2e 100644 --- a/include/linux/soundwire/sdw_amd.h +++ b/include/linux/soundwire/sdw_amd.h @@ -10,9 +10,30 @@

#define AMD_SDW_CLK_STOP_MODE 1 #define AMD_SDW_POWER_OFF_MODE 2 +#define ACP_SDW0 0 +#define ACP_SDW1 1 +#define ACP_SDW0_MAX_DAI 6

is this related to the definition of amd_sdw_channel or the number of ports available?

struct acp_sdw_pdata { u16 instance; struct mutex *sdw_lock; };

+struct amd_sdwc_ctrl {

• struct sdw_bus bus;
• struct device *dev;
• void __iomem *mmio;
• struct work_struct probe_work;
• struct mutex *sdw_lock;

comment please.

• int num_din_ports;
• int num_dout_ports;
• int cols_index;
• int rows_index;
• u32 instance;
• u32 quirks;
• u32 wake_en_mask;
• int num_ports;
• bool startup_done;

ah this was an Intel definition. Due to power dependencies we had to split the probe and startup step. Does AMD have a need for this? Is the SoundWire master IP dependent on DSP boot or something?

• u32 power_mode_mask;

+}; #endif