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@xxxxxxx> >> --- >> 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); >> +}