Re: [PATCH V4 2/8] soundwire: amd: Add support for AMD Manager driver

On 27/02/23 22:05, Pierre-Louis Bossart wrote:

On 2/27/23 10:47, Vijendar Mukunda wrote:

...

AMD ACP(v6.x) IP block has two SoundWire manager devices. Add support for

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

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

drivers/soundwire/amd_manager.c | 749 ++++++++++++++++++++++++++++++ drivers/soundwire/amd_manager.h | 252 ++++++++++ include/linux/soundwire/sdw_amd.h | 67 +++ 3 files changed, 1068 insertions(+) create mode 100644 drivers/soundwire/amd_manager.c create mode 100644 drivers/soundwire/amd_manager.h create mode 100644 include/linux/soundwire/sdw_amd.h

diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c new file mode 100644 index 000000000000..63cd8d066602 --- /dev/null +++ b/drivers/soundwire/amd_manager.c @@ -0,0 +1,749 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • SoundWire AMD Manager 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/wait.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include "bus.h" +#include "amd_manager.h"

+#define DRV_NAME "amd_sdw_manager"

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

+static void amd_enable_sdw_pads(struct amd_sdw_manager *amd_manager) +{

• u32 sw_pad_pulldown_val;
• u32 val;
• mutex_lock(amd_manager->acp_sdw_lock);
• val = acp_reg_readl(amd_manager->acp_mmio + ACP_SW_PAD_KEEPER_EN);
• val |= amd_manager->reg_mask->sw_pad_enable_mask;
• acp_reg_writel(val, amd_manager->acp_mmio + ACP_SW_PAD_KEEPER_EN);
• usleep_range(1000, 1500);
• sw_pad_pulldown_val = acp_reg_readl(amd_manager->acp_mmio + ACP_PAD_PULLDOWN_CTRL);
• sw_pad_pulldown_val &= amd_manager->reg_mask->sw_pad_pulldown_mask;
• acp_reg_writel(sw_pad_pulldown_val, amd_manager->acp_mmio + ACP_PAD_PULLDOWN_CTRL);
• mutex_unlock(amd_manager->acp_sdw_lock);

+}

+static int amd_init_sdw_manager(struct amd_sdw_manager *amd_manager) +{

• u32 val;
• u32 timeout = 0;
• u32 retry_count = 0;
• acp_reg_writel(AMD_SDW_ENABLE, amd_manager->mmio + ACP_SW_EN);
• do {

• val = acp_reg_readl(amd_manager->mmio + ACP_SW_EN_STATUS);
  

• if (val)
  

• 	break;
  

• usleep_range(10, 20);
  

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

• return -ETIMEDOUT;
  

• /* SoundWire manager bus reset */
• acp_reg_writel(AMD_SDW_BUS_RESET_REQ, amd_manager->mmio + ACP_SW_BUS_RESET_CTRL);
• val = acp_reg_readl(amd_manager->mmio + ACP_SW_BUS_RESET_CTRL);
• while (!(val & AMD_SDW_BUS_RESET_DONE)) {

• val = acp_reg_readl(amd_manager->mmio + ACP_SW_BUS_RESET_CTRL);
  

• if (timeout > AMD_DELAY_LOOP_ITERATION)
  

• 	break;
  

• usleep_range(1, 5);
  

• timeout++;
  

• }
• if (timeout > AMD_DELAY_LOOP_ITERATION)

• return -ETIMEDOUT;
  

nit-pick: add a new line here since you're starting a different sequence.

will fix it.

...

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

• val = acp_reg_readl(amd_manager->mmio + ACP_SW_BUS_RESET_CTRL);
  

• if (timeout > AMD_DELAY_LOOP_ITERATION)
  

• 	break;
  

• usleep_range(1, 5);
  

• timeout++;
  

• }
• if (timeout > AMD_DELAY_LOOP_ITERATION) {

• dev_err(amd_manager->dev, "Failed to reset SoundWire manager instance%d\n",
  

• 	amd_manager->instance);
  

• return -ETIMEDOUT;
  

• }

and same here, this is a new sequence starting here so a newline would help

will fix it.

...

• retry_count = 0;
• acp_reg_writel(AMD_SDW_DISABLE, amd_manager->mmio + ACP_SW_EN);
• do {

• val = acp_reg_readl(amd_manager->mmio + ACP_SW_EN_STATUS);
  

• if (!val)
  

• 	break;
  

• usleep_range(10, 20);
  

• } 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_manager(struct amd_sdw_manager *amd_manager) +{

• u32 val;
• u32 retry_count = 0;
• acp_reg_writel(AMD_SDW_ENABLE, amd_manager->mmio + ACP_SW_EN);
• do {

• val = acp_reg_readl(amd_manager->mmio + ACP_SW_EN_STATUS);
  

• if (val)
  

• 	break;
  

• usleep_range(10, 20);
  

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

• return -ETIMEDOUT;
  

• return 0;

+} +static void amd_sdw_ctl_word_prep(u32 *low_word, u32 *high_word, u32 cmd_type,

• 		  struct sdw_msg *msg, int cmd_offset)
  

+{

• u32 low_data = 0, high_data;
• u16 addr;
• u8 addr_high, addr_low;
• u8 data = 0;

nit-pick: this looks a bit messy, usually it's simpler to have initialized variables on separate lines. If you keep the low and high on the same line, then at least use the same order.

will fix it.

...

• addr = msg->addr + cmd_offset;
• addr_high = (addr & 0xFF00) >> 8;
• addr_low = addr & 0xFF;
• if (cmd_type == AMD_SDW_CMD_WRITE)

• data = msg->buf[cmd_offset];
  

• high_data = FIELD_PREP(AMD_SDW_MCP_CMD_DEV_ADDR, msg->dev_num);
• high_data |= FIELD_PREP(AMD_SDW_MCP_CMD_COMMAND, cmd_type);
• high_data |= FIELD_PREP(AMD_SDW_MCP_CMD_REG_ADDR_HIGH, addr_high);
• low_data |= FIELD_PREP(AMD_SDW_MCP_CMD_REG_ADDR_LOW, addr_low);
• low_data |= FIELD_PREP(AMD_SDW_MCP_CMD_REG_DATA, data);
• *high_word = high_data;
• *low_word = low_data;

+}

+static u64 amd_sdw_send_cmd_get_resp(struct amd_sdw_manager *amd_manager, u32 lword, u32 uword) +{

• u64 resp;
• u32 resp_lower, resp_high;

nit-pick: resp_low? you seem to use this low/high pattern in a number of places.

will fix it.

...

• u32 sts;
• u32 timeout = 0;
• sts = acp_reg_readl(amd_manager->mmio + ACP_SW_IMM_CMD_STS);
• while (sts & AMD_SDW_IMM_CMD_BUSY) {

• if (timeout++ > AMD_SDW_RETRY_COUNT) {
  

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

• 		amd_manager->instance);
  

• 	return -ETIMEDOUT;
  

• }
  

• usleep_range(5, 10);
  

• sts = acp_reg_readl(amd_manager->mmio + ACP_SW_IMM_CMD_STS);
  

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

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

• acp_reg_writel(0x00, amd_manager->mmio + ACP_SW_IMM_CMD_STS);
  

• }
• acp_reg_writel(uword, amd_manager->mmio + ACP_SW_IMM_CMD_UPPER_WORD);
• acp_reg_writel(lword, amd_manager->mmio + ACP_SW_IMM_CMD_LOWER_QWORD);
• sts = acp_reg_readl(amd_manager->mmio + ACP_SW_IMM_CMD_STS);
• while (!(sts & AMD_SDW_IMM_RES_VALID)) {

• if (timeout++ > AMD_SDW_RETRY_COUNT) {
  

• 	dev_err(amd_manager->dev, "SDW%x cmd response timeout occurred\n",
  

• 		amd_manager->instance);
  

• 	return -ETIMEDOUT;
  

• }
  

• usleep_range(5, 10);
  

• sts = acp_reg_readl(amd_manager->mmio + ACP_SW_IMM_CMD_STS);
  

• }
• resp_high = acp_reg_readl(amd_manager->mmio + ACP_SW_IMM_RESP_UPPER_WORD);
• resp_lower = acp_reg_readl(amd_manager->mmio + ACP_SW_IMM_RESP_LOWER_QWORD);

nit-pick: newline here.

will fix it.

...

• timeout = 0;
• acp_reg_writel(AMD_SDW_IMM_RES_VALID, amd_manager->mmio + ACP_SW_IMM_CMD_STS);
• sts = acp_reg_readl(amd_manager->mmio + ACP_SW_IMM_CMD_STS);
• while ((sts & AMD_SDW_IMM_RES_VALID)) {

• if (timeout++ > AMD_SDW_RETRY_COUNT) {
  

• 	dev_err(amd_manager->dev, "SDW%x cmd status retry failed\n",
  

• 		amd_manager->instance);
  

• 	return -ETIMEDOUT;
  

• }
  

• usleep_range(5, 10);
  

• sts = acp_reg_readl(amd_manager->mmio + ACP_SW_IMM_CMD_STS);
  

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

+} +static unsigned int _amd_sdw_xfer_msg(struct amd_sdw_manager *amd_manager, struct sdw_msg *msg,

• 		      int cmd, int cmd_offset)
  

+{

• u64 response;
• u32 uword = 0, lword = 0;

couldn't figure out what uword and lword refer to. Is this high/low as above or something else?

its high and low. Register naming convention uses upper word and lower word. will use naming convention throughout the code.

...

• amd_sdw_ctl_word_prep(&lword, &uword, cmd, msg, cmd_offset);
• response = amd_sdw_send_cmd_get_resp(amd_manager, lword, uword);
• return amd_sdw_fill_msg_resp(amd_manager, msg, response, cmd_offset);

+}