On 7/19/21 11:51 AM, Vijendar Mukunda wrote:
> ACP5.x IP has multiple I2S controllers and DMA controller.
> Create platform devices for I2S HS controller instance, I2S SP controller
> instance and DMA contrller.
typo: controller
> Pass PCI resources like MMIO, irq to these platform devices.
>
> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
> ---
> sound/soc/amd/vangogh/acp5x.h | 10 ++++
> sound/soc/amd/vangogh/pci-acp5x.c | 95 ++++++++++++++++++++++++++++++-
> 2 files changed, 102 insertions(+), 3 deletions(-)
>
> diff --git a/sound/soc/amd/vangogh/acp5x.h b/sound/soc/amd/vangogh/acp5x.h
> index 708586109315..bbf29fd2b12f 100644
> --- a/sound/soc/amd/vangogh/acp5x.h
> +++ b/sound/soc/amd/vangogh/acp5x.h
> @@ -22,6 +22,16 @@
> #define ACP_ERR_INTR_MASK 0x20000000
> #define ACP_EXT_INTR_STAT_CLEAR_MASK 0xFFFFFFFF
>
> +#define ACP5x_DEVS 0x03
3?
> +#define ACP5x_REG_START 0x1240000
> +#define ACP5x_REG_END 0x1250200
> +#define ACP5x_I2STDM_REG_START 0x1242400
> +#define ACP5x_I2STDM_REG_END 0x1242410
> +#define ACP5x_HS_TDM_REG_START 0x1242814
> +#define ACP5x_HS_TDM_REG_END 0x1242824
> +#define I2S_MODE 0x00
> +#define ACP5x_I2S_MODE 0x00
> +
> /* common header file uses exact offset rather than relative
> * offset which requires substraction logic from base_addr
typo: subtraction
> * for accessing ACP5x MMIO space registers
> diff --git a/sound/soc/amd/vangogh/pci-acp5x.c b/sound/soc/amd/vangogh/pci-acp5x.c
> index 523b962fe35e..3cc15a15b745 100644
> --- a/sound/soc/amd/vangogh/pci-acp5x.c
> +++ b/sound/soc/amd/vangogh/pci-acp5x.c
> @@ -8,11 +8,16 @@
> #include <linux/module.h>
> #include <linux/io.h>
> #include <linux/delay.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
>
> #include "acp5x.h"
>
> struct acp5x_dev_data {
> void __iomem *acp5x_base;
> + bool acp5x_audio_mode;
> + struct resource *res;
> + struct platform_device *pdev[3];
pdev[ACP5x_DEVS] ?
> };
>
> static int acp5x_power_on(void __iomem *acp5x_base)
> @@ -114,9 +119,12 @@ static int snd_acp5x_probe(struct pci_dev *pci,
> const struct pci_device_id *pci_id)
> {
> struct acp5x_dev_data *adata;
> - int ret;
> - u32 addr;
> + struct platform_device_info pdevinfo[3];
> + unsigned int irqflags;
> + int ret, i;
> + u32 addr, val;
>
> + irqflags = IRQF_SHARED;
> if (pci->revision != 0x50)
> return -ENODEV;
>
> @@ -150,6 +158,83 @@ static int snd_acp5x_probe(struct pci_dev *pci,
> if (ret)
> goto release_regions;
>
> + val = acp_readl(adata->acp5x_base + ACP_PIN_CONFIG);
> + switch (val) {
> + case I2S_MODE:
> + adata->res = devm_kzalloc(&pci->dev,
> + sizeof(struct resource) * 4,
what is this 4 value?
> + GFP_KERNEL);
> + if (!adata->res) {
> + ret = -ENOMEM;
> + goto de_init;
> + }
> +
> + adata->res[0].name = "acp5x_i2s_iomem";
> + adata->res[0].flags = IORESOURCE_MEM;
> + adata->res[0].start = addr;
> + adata->res[0].end = addr + (ACP5x_REG_END - ACP5x_REG_START);
> +
> + adata->res[1].name = "acp5x_i2s_sp";
> + adata->res[1].flags = IORESOURCE_MEM;
> + adata->res[1].start = addr + ACP5x_I2STDM_REG_START;
> + adata->res[1].end = addr + ACP5x_I2STDM_REG_END;
> +
> + adata->res[2].name = "acp5x_i2s_hs";
> + adata->res[2].flags = IORESOURCE_MEM;
> + adata->res[2].start = addr + ACP5x_HS_TDM_REG_START;
> + adata->res[2].end = addr + ACP5x_HS_TDM_REG_END;
> +
> + adata->res[3].name = "acp5x_i2s_irq";
> + adata->res[3].flags = IORESOURCE_IRQ;
> + adata->res[3].start = pci->irq;
> + adata->res[3].end = adata->res[3].start;
> +
> + adata->acp5x_audio_mode = ACP5x_I2S_MODE;
> +
> + memset(&pdevinfo, 0, sizeof(pdevinfo));
> + pdevinfo[0].name = "acp5x_i2s_dma";
> + pdevinfo[0].id = 0;
> + pdevinfo[0].parent = &pci->dev;
> + pdevinfo[0].num_res = 4;
> + pdevinfo[0].res = &adata->res[0];
> + pdevinfo[0].data = &irqflags;
> + pdevinfo[0].size_data = sizeof(irqflags);
> +
> + pdevinfo[1].name = "acp5x_i2s_playcap";
> + pdevinfo[1].id = 0;
> + pdevinfo[1].parent = &pci->dev;
> + pdevinfo[1].num_res = 1;
> + pdevinfo[1].res = &adata->res[1];
> +
> + pdevinfo[2].name = "acp5x_i2s_playcap";
> + pdevinfo[2].id = 1;
> + pdevinfo[2].parent = &pci->dev;
> + pdevinfo[2].num_res = 1;
> + pdevinfo[2].res = &adata->res[2];
> +
> + for (i = 0; i < ACP5x_DEVS; i++) {
> + adata->pdev[i] =
> + platform_device_register_full(&pdevinfo[i]);
> + if (IS_ERR(adata->pdev[i])) {
> + dev_err(&pci->dev, "cannot register %s device\n",
> + pdevinfo[i].name);
> + ret = PTR_ERR(adata->pdev[i]);
> + goto unregister_devs;
> + }
> + }
> + break;
> + default:
> + dev_info(&pci->dev, "ACP audio mode : %d\n", val);
> + }
> + return 0;
> +
> +unregister_devs:
> + if (val == I2S_MODE)
nit-pick: you can't reach this point outside of the I2S_MODE, so this test is not useful
> + for (i = 0; i < ACP5x_DEVS; i++)
> + platform_device_unregister(adata->pdev[i]);
only unregister what you registered?
for (--i; i > 0; i--)
> +de_init:
> + if (acp5x_deinit(adata->acp5x_base))
> + dev_err(&pci->dev, "ACP de-init failed\n");
> release_regions:
> pci_release_regions(pci);
> disable_pci:
> @@ -161,9 +246,13 @@ static int snd_acp5x_probe(struct pci_dev *pci,
> static void snd_acp5x_remove(struct pci_dev *pci)
> {
> struct acp5x_dev_data *adata;
> - int ret;
> + int i, ret;
>
> adata = pci_get_drvdata(pci);
> + if (adata->acp5x_audio_mode == ACP5x_I2S_MODE) {
> + for (i = 0; i < ACP5x_DEVS; i++)
> + platform_device_unregister(adata->pdev[i]);
> + }
> ret = acp5x_deinit(adata->acp5x_base);
> if (ret)
> dev_err(&pci->dev, "ACP de-init failed\n");
>
