| Message ID | 20231221073558.3181911-1-Vijendar.Mukunda@amd.com |
|---|---|
| Headers | show |
| Series | soundwire/SOF: add SoundWire Interface support for AMD SOF stack | expand |
On 21/12/23 22:08, Vinod Koul wrote: > On 21-12-23, 13:05, Vijendar Mukunda wrote: >> As sdw pads enable sequence is executed only once, invoke it >> from probe sequence. >> >> Program required pads for both manager instances >> based on link_mask during probe sequence. This will avoid >> acquiring mutex lock. > something wrong with your editor to have this formatting, you can use > upto 80 chars here! Will check. >> Remove unnecessary delay after programming ACP_SW_PAD_KEEPER_EN >> register. >> >> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com> >> --- >> drivers/soundwire/amd_init.c | 45 +++++++++++++++++++++++++++++++++ >> drivers/soundwire/amd_manager.c | 18 ------------- >> 2 files changed, 45 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/soundwire/amd_init.c b/drivers/soundwire/amd_init.c >> index 5c9569d9ad01..b3b3c7266384 100644 >> --- a/drivers/soundwire/amd_init.c >> +++ b/drivers/soundwire/amd_init.c >> @@ -15,6 +15,47 @@ >> >> #include "amd_init.h" >> >> +#define ACP_PAD_PULLDOWN_CTRL 0x0001448 >> +#define ACP_SW_PAD_KEEPER_EN 0x0001454 >> +#define AMD_SDW_PAD_PULLDOWN_CTRL_ENABLE_MASK 0x7f9a >> +#define AMD_SDW0_PAD_PULLDOWN_CTRL_ENABLE_MASK 0x7f9f >> +#define AMD_SDW1_PAD_PULLDOWN_CTRL_ENABLE_MASK 0x7ffa >> +#define AMD_SDW0_PAD_EN_MASK 1 >> +#define AMD_SDW1_PAD_EN_MASK 0x10 >> +#define AMD_SDW_PAD_EN_MASK (AMD_SDW0_PAD_EN_MASK | AMD_SDW1_PAD_EN_MASK) >> + >> +static int amd_enable_sdw_pads(void __iomem *mmio, u32 link_mask, struct device *dev) >> +{ >> + u32 val; >> + u32 pad_keeper_en_mask, pad_pulldown_ctrl_mask; >> + >> + switch (link_mask) { >> + case 1: >> + pad_keeper_en_mask = AMD_SDW0_PAD_EN_MASK; >> + pad_pulldown_ctrl_mask = AMD_SDW0_PAD_PULLDOWN_CTRL_ENABLE_MASK; >> + break; >> + case 2: >> + pad_keeper_en_mask = AMD_SDW1_PAD_EN_MASK; >> + pad_pulldown_ctrl_mask = AMD_SDW1_PAD_PULLDOWN_CTRL_ENABLE_MASK; >> + break; >> + case 3: >> + pad_keeper_en_mask = AMD_SDW_PAD_EN_MASK; >> + pad_pulldown_ctrl_mask = AMD_SDW_PAD_PULLDOWN_CTRL_ENABLE_MASK; >> + break; >> + default: >> + dev_err(dev, "No SDW Links are enabled\n"); >> + return -ENODEV; >> + } >> + >> + val = readl(mmio + ACP_SW_PAD_KEEPER_EN); >> + val |= pad_keeper_en_mask; >> + writel(val, mmio + ACP_SW_PAD_KEEPER_EN); >> + val = readl(mmio + ACP_PAD_PULLDOWN_CTRL); >> + val &= pad_pulldown_ctrl_mask; >> + writel(val, mmio + ACP_PAD_PULLDOWN_CTRL); > updatel() local macro? We have identified similar update() inline function should be used in other parts of the code with different mmio base address. We will push it as a separate patch. >> + return 0; >> +} >> + >> static int sdw_amd_cleanup(struct sdw_amd_ctx *ctx) >> { >> int i; >> @@ -37,6 +78,7 @@ static struct sdw_amd_ctx *sdw_amd_probe_controller(struct sdw_amd_res *res) >> struct platform_device_info pdevinfo[2]; >> u32 link_mask; >> int count, index; >> + int ret; >> >> if (!res) >> return NULL; >> @@ -50,6 +92,9 @@ static struct sdw_amd_ctx *sdw_amd_probe_controller(struct sdw_amd_res *res) >> >> count = res->count; >> dev_dbg(&adev->dev, "Creating %d SDW Link devices\n", count); >> + ret = amd_enable_sdw_pads(res->mmio_base, res->link_mask, res->parent); >> + if (ret) >> + return NULL; >> >> /* >> * we need to alloc/free memory manually and can't use devm: >> diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c >> index c27b0b0f33a6..1427cccfc309 100644 >> --- a/drivers/soundwire/amd_manager.c >> +++ b/drivers/soundwire/amd_manager.c >> @@ -26,23 +26,6 @@ >> >> #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 = readl(amd_manager->acp_mmio + ACP_SW_PAD_KEEPER_EN); >> - val |= amd_manager->reg_mask->sw_pad_enable_mask; >> - writel(val, amd_manager->acp_mmio + ACP_SW_PAD_KEEPER_EN); >> - usleep_range(1000, 1500); >> - >> - sw_pad_pulldown_val = readl(amd_manager->acp_mmio + ACP_PAD_PULLDOWN_CTRL); >> - sw_pad_pulldown_val &= amd_manager->reg_mask->sw_pad_pulldown_mask; >> - writel(sw_pad_pulldown_val, amd_manager->acp_mmio + ACP_PAD_PULLDOWN_CTRL); >> - mutex_unlock(amd_manager->acp_sdw_lock); > so the code is copied from a GPL declared file to now and GPL + BSD one! > Have you had lawyers look into this... why change one file license ? As per recommendations from our legal team, we have updated the license as dual one for amd_init.c file. We have also observed that license terms should be updated for other files as well (amd_manager.c, amd_manager.h & sdw_amd.h) as dual one, which we have planned to submit as a supplement patch. >> -} >> - >> static int amd_init_sdw_manager(struct amd_sdw_manager *amd_manager) >> { >> u32 val; >> @@ -872,7 +855,6 @@ int amd_sdw_manager_start(struct amd_sdw_manager *amd_manager) >> >> prop = &amd_manager->bus.prop; >> if (!prop->hw_disabled) { >> - amd_enable_sdw_pads(amd_manager); >> ret = amd_init_sdw_manager(amd_manager); >> if (ret) >> return ret; >> -- >> 2.34.1
On 22/12/23 14:51, Vinod Koul wrote: > On 22-12-23, 12:45, Mukunda,Vijendar wrote: >> On 21/12/23 22:08, Vinod Koul wrote: >>> so the code is copied from a GPL declared file to now and GPL + BSD one! >>> Have you had lawyers look into this... why change one file license ? >> As per recommendations from our legal team, we have updated the license as dual >> one for amd_init.c file. >> We have also observed that license terms should be updated for other files as >> well (amd_manager.c, amd_manager.h & sdw_amd.h) as dual one, which we have >> planned to submit as a supplement patch. > Lets change that first before we move code from one license file to > another Will push the license update patch first. > > Btw why would you want to do the change of license form GPL to dual? As this code being used by AMD SOF stack which uses dual license, So we want to maintain the same license terms. >
On 21/12/23 13:05, Vijendar Mukunda wrote: > This patch series is to redesign existing platform device > creation logic for SoundWire managers and Implement generic > functions for SoundWire manager probe, start and exit sequence > which are common for both Legacy(NO DSP enabled) and SOF stack, > and add SoundWire Interface support for AMD SOF stack > (ACP 6.3 based platforms). > > Vijendar Mukunda (12): > ASoC/soundwire: implement generic api for scanning amd soundwire > controller > > drivers: soundwire: implement function to extract slave information > drivers: soundwire: refactor soundwire pads enable > drivers: soundwire: refactor register mask structure > ASoC: SOF: amd: add code for invoking soundwire manager helper > functions > ASoC: SOF: amd: add interrupt handling for SoundWire manager devices > ASoC: SOF: amd: Add Soundwire DAI configuration support for AMD > platforms > ASoC: SOF: amd: add machine select logic for soundwire based platforms > ASoC: SOF: amd: update descriptor fields for acp6.3 based platform > ASoC: SOF: amd: select soundwire dependency flag for acp6.3 based > platform > ASoC: SOF: amd: refactor acp driver pm ops We need to post v2 version patch set. There are few patch dependencies. "drivers: soundwire: refactor amd soundwire manager device node creation" patch has dependency on "ASoC/soundwire: implement generic api for scanning amd soundwire controller" SOF patch " ASoC: SOF: amd: add code for invoking soundwire manager helper functions" has dependency on SoundWire patch set. As this patch series has to go in two different sub systems, Please suggest how can we push the patch series to get it reviewed and merged at one go. > > drivers/soundwire/Makefile | 2 +- > drivers/soundwire/amd_init.c | 235 +++++++++++++++++++++++++++++ > drivers/soundwire/amd_init.h | 13 ++ > drivers/soundwire/amd_manager.c | 41 +---- > drivers/soundwire/amd_manager.h | 12 +- > include/linux/soundwire/sdw_amd.h | 79 ++++++++-- > include/sound/sof/dai-amd.h | 7 + > include/sound/sof/dai.h | 2 + > include/uapi/sound/sof/tokens.h | 4 + > sound/soc/amd/acp/Kconfig | 7 + > sound/soc/amd/acp/Makefile | 2 + > sound/soc/amd/acp/amd-sdw-acpi.c | 62 ++++++++ > sound/soc/sof/amd/Kconfig | 18 +++ > sound/soc/sof/amd/acp-common.c | 65 +++++++- > sound/soc/sof/amd/acp-dsp-offset.h | 10 ++ > sound/soc/sof/amd/acp.c | 202 ++++++++++++++++++++++++- > sound/soc/sof/amd/acp.h | 26 +++- > sound/soc/sof/amd/pci-acp63.c | 7 + > sound/soc/sof/ipc3-pcm.c | 25 +++ > sound/soc/sof/ipc3-topology.c | 40 +++++ > sound/soc/sof/sof-audio.h | 1 + > sound/soc/sof/topology.c | 5 + > 22 files changed, 798 insertions(+), 67 deletions(-) > create mode 100644 drivers/soundwire/amd_init.c > create mode 100644 drivers/soundwire/amd_init.h > create mode 100644 sound/soc/amd/acp/amd-sdw-acpi.c >
On Tue, Jan 09, 2024 at 06:11:18PM +0530, Mukunda,Vijendar wrote: > SOF patch " ASoC: SOF: amd: add code for invoking soundwire manager > helper functions" has dependency on SoundWire patch set. > As this patch series has to go in two different sub systems, > Please suggest how can we push the patch series to get it reviewed and merged > at one go. I'd expect Vinod to review the Soundwire bits and either ack them or provide a pull request with them that I can fetch into the ASoC tree.
This patch series is to redesign existing platform device creation logic for SoundWire managers and Implement generic functions for SoundWire manager probe, start and exit sequence which are common for both Legacy(NO DSP enabled) and SOF stack, and add SoundWire Interface support for AMD SOF stack (ACP 6.3 based platforms). Vijendar Mukunda (12): ASoC/soundwire: implement generic api for scanning amd soundwire controller drivers: soundwire: refactor amd soundwire manager device node creation drivers: soundwire: implement function to extract slave information drivers: soundwire: refactor soundwire pads enable drivers: soundwire: refactor register mask structure ASoC: SOF: amd: add code for invoking soundwire manager helper functions ASoC: SOF: amd: add interrupt handling for SoundWire manager devices ASoC: SOF: amd: Add Soundwire DAI configuration support for AMD platforms ASoC: SOF: amd: add machine select logic for soundwire based platforms ASoC: SOF: amd: update descriptor fields for acp6.3 based platform ASoC: SOF: amd: select soundwire dependency flag for acp6.3 based platform ASoC: SOF: amd: refactor acp driver pm ops drivers/soundwire/Makefile | 2 +- drivers/soundwire/amd_init.c | 235 +++++++++++++++++++++++++++++ drivers/soundwire/amd_init.h | 13 ++ drivers/soundwire/amd_manager.c | 41 +---- drivers/soundwire/amd_manager.h | 12 +- include/linux/soundwire/sdw_amd.h | 79 ++++++++-- include/sound/sof/dai-amd.h | 7 + include/sound/sof/dai.h | 2 + include/uapi/sound/sof/tokens.h | 4 + sound/soc/amd/acp/Kconfig | 7 + sound/soc/amd/acp/Makefile | 2 + sound/soc/amd/acp/amd-sdw-acpi.c | 62 ++++++++ sound/soc/sof/amd/Kconfig | 18 +++ sound/soc/sof/amd/acp-common.c | 65 +++++++- sound/soc/sof/amd/acp-dsp-offset.h | 10 ++ sound/soc/sof/amd/acp.c | 202 ++++++++++++++++++++++++- sound/soc/sof/amd/acp.h | 26 +++- sound/soc/sof/amd/pci-acp63.c | 7 + sound/soc/sof/ipc3-pcm.c | 25 +++ sound/soc/sof/ipc3-topology.c | 40 +++++ sound/soc/sof/sof-audio.h | 1 + sound/soc/sof/topology.c | 5 + 22 files changed, 798 insertions(+), 67 deletions(-) create mode 100644 drivers/soundwire/amd_init.c create mode 100644 drivers/soundwire/amd_init.h create mode 100644 sound/soc/amd/acp/amd-sdw-acpi.c