diff mbox series

[1/5] ASoC: amd: Registering device endpoints using MFD framework

Message ID 1569539290-756-1-git-send-email-Vishnuvardhanrao.Ravulapati@amd.com (mailing list archive)
State New, archived
Headers show
Series [1/5] ASoC: amd: Registering device endpoints using MFD framework | expand

Commit Message

RAVULAPATI, VISHNU VARDHAN RAO Sept. 26, 2019, 11:07 p.m. UTC 
Removed platform based endpoint registering.
Now Registering DMA and multiple I2S BT playback
endpoint devices automatically by using MFD framework.

Signed-off-by: Ravulapati Vishnu vardhan rao <Vishnuvardhanrao.Ravulapati@amd.com>
Reviewed-by: Vijendar Mukunda <vijendar.mukunda@amd.com>
---
 sound/soc/amd/raven/acp3x.h     |  54 ++++++++-------
 sound/soc/amd/raven/pci-acp3x.c | 148 +++++++++++++++++++++++++++++-----------
 2 files changed, 139 insertions(+), 63 deletions(-)

Comments

Mark Brown Sept. 26, 2019, 6:23 p.m. UTC | #1 
On Fri, Sep 27, 2019 at 04:37:35AM +0530, Ravulapati Vishnu vardhan rao wrote:

> -#define ACP3x_PHY_BASE_ADDRESS 0x1240000
> -#define	ACP3x_I2S_MODE	0
> -#define	ACP3x_REG_START	0x1240000
> -#define	ACP3x_REG_END	0x1250200
> -#define I2S_MODE	0x04
> -#define	BT_TX_THRESHOLD 26
> -#define	BT_RX_THRESHOLD 25
> -#define ACP3x_POWER_ON 0x00
> -#define ACP3x_POWER_ON_IN_PROGRESS 0x01
> -#define ACP3x_POWER_OFF 0x02
> -#define ACP3x_POWER_OFF_IN_PROGRESS 0x03
> +#define ACP3x_DEVS		3
> +#define ACP3x_PHY_BASE_ADDRESS	0x1240000
> +#define	ACP3x_I2S_MODE		0
> +#define	ACP3x_REG_START		0x1240000
> +#define	ACP3x_REG_END		0x1250200

A large part of this appears to be unrelated indentation changes,
these should be split out into a separate patch.

> +static struct device *get_mfd_cell_dev(const char *device_name, int r)
> +{
> +	char auto_dev_name[25];
> +	struct device *dev;
> +
> +	snprintf(auto_dev_name, sizeof(auto_dev_name),
> +		 "%s.%d.auto", device_name, r);
> +	dev = bus_find_device_by_name(&platform_bus_type,
> +					NULL, auto_dev_name);
> +	dev_info(dev, "device %s added\n", auto_dev_name);

Remove this log message, it's going to be very noisy.

> +		r = mfd_add_hotplug_devices(adata->parent, adata->cell,	3);
> +		for (i = 0; i < 3 ; i++)
> +			dev = get_mfd_cell_dev(adata->cell[i].name, i);

What is this doing?  We never look at the result of this
get_mfd_cell_dev() and having a function like this suggests that
there's some abstraction issue here.

> +	kfree(adata->cell);
>  	iounmap(adata->acp3x_base);
> +	/*ignore device status and return driver probe error*/
> +	return -ENODEV;
>  release_regions:

This looks broken, as well as discarding error codes (making
things harder to diagnose) it means we stop unwinding things and
leave the rest of the resources lying around.
diff mbox series

Patch

diff --git a/sound/soc/amd/raven/acp3x.h b/sound/soc/amd/raven/acp3x.h
index 4f2cadd..3a27fc8 100644
--- a/sound/soc/amd/raven/acp3x.h
+++ b/sound/soc/amd/raven/acp3x.h
@@ -7,33 +7,41 @@ 
 
 #include "chip_offset_byte.h"
 
-#define ACP3x_PHY_BASE_ADDRESS 0x1240000
-#define	ACP3x_I2S_MODE	0
-#define	ACP3x_REG_START	0x1240000
-#define	ACP3x_REG_END	0x1250200
-#define I2S_MODE	0x04
-#define	BT_TX_THRESHOLD 26
-#define	BT_RX_THRESHOLD 25
-#define ACP3x_POWER_ON 0x00
-#define ACP3x_POWER_ON_IN_PROGRESS 0x01
-#define ACP3x_POWER_OFF 0x02
-#define ACP3x_POWER_OFF_IN_PROGRESS 0x03
+#define ACP3x_DEVS		3
+#define ACP3x_PHY_BASE_ADDRESS	0x1240000
+#define	ACP3x_I2S_MODE		0
+#define	ACP3x_REG_START		0x1240000
+#define	ACP3x_REG_END		0x1250200
+#define ACP3x_I2STDM_REG_START	0x1242400
+#define ACP3x_I2STDM_REG_END	0x1242410
+#define ACP3x_BT_TDM_REG_START	0x1242800
+#define ACP3x_BT_TDM_REG_END	0x1242810
+#define I2S_MODE		0x04
+#define	I2S_RX_THRESHOLD	27
+#define	I2S_TX_THRESHOLD	28
+#define	BT_TX_THRESHOLD		26
+#define	BT_RX_THRESHOLD		25
+#define ACP_ERR_INTR_MASK	29
+#define ACP3x_POWER_ON		0x00
+#define ACP3x_POWER_ON_IN_PROGRESS	0x01
+#define ACP3x_POWER_OFF			0x02
+#define ACP3x_POWER_OFF_IN_PROGRESS	0x03
 #define ACP3x_SOFT_RESET__SoftResetAudDone_MASK	0x00010001
 
 #define ACP_SRAM_PTE_OFFSET	0x02050000
-#define PAGE_SIZE_4K_ENABLE 0x2
+#define PAGE_SIZE_4K_ENABLE	0x2
 #define MEM_WINDOW_START	0x4000000
-#define PLAYBACK_FIFO_ADDR_OFFSET 0x400
-#define CAPTURE_FIFO_ADDR_OFFSET  0x500
-
-#define PLAYBACK_MIN_NUM_PERIODS    2
-#define PLAYBACK_MAX_NUM_PERIODS    8
-#define PLAYBACK_MAX_PERIOD_SIZE    16384
-#define PLAYBACK_MIN_PERIOD_SIZE    4096
-#define CAPTURE_MIN_NUM_PERIODS     2
-#define CAPTURE_MAX_NUM_PERIODS     8
-#define CAPTURE_MAX_PERIOD_SIZE     16384
-#define CAPTURE_MIN_PERIOD_SIZE     4096
+#define PLAYBACK_FIFO_ADDR_OFFSET	0x400
+#define CAPTURE_FIFO_ADDR_OFFSET	0x500
+
+#define PLAYBACK_MIN_NUM_PERIODS	2
+#define PLAYBACK_MAX_NUM_PERIODS	8
+#define PLAYBACK_MAX_PERIOD_SIZE	16384
+#define PLAYBACK_MIN_PERIOD_SIZE	4096
+#define CAPTURE_MIN_NUM_PERIODS		2
+#define CAPTURE_MAX_NUM_PERIODS		8
+#define CAPTURE_MAX_PERIOD_SIZE		16384
+#define CAPTURE_MIN_PERIOD_SIZE		4096
 
 #define MAX_BUFFER (PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS)
 #define MIN_BUFFER MAX_BUFFER
diff --git a/sound/soc/amd/raven/pci-acp3x.c b/sound/soc/amd/raven/pci-acp3x.c
index facec24..c0b8fad 100644
--- a/sound/soc/amd/raven/pci-acp3x.c
+++ b/sound/soc/amd/raven/pci-acp3x.c
@@ -9,23 +9,49 @@ 
 #include <linux/io.h>
 #include <linux/platform_device.h>
 #include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/pm_runtime.h>
+#include <sound/pcm.h>
 
 #include "acp3x.h"
 
+struct i2s_platform_data {
+	unsigned int cap;
+	int channel;
+	u32 snd_rates;
+};
 struct acp3x_dev_data {
+	struct device *parent;
+	struct mfd_cell *cell;
+	struct resource *res;
 	void __iomem *acp3x_base;
 	bool acp3x_audio_mode;
-	struct resource *res;
 	struct platform_device *pdev;
 };
 
+
+static struct device *get_mfd_cell_dev(const char *device_name, int r)
+{
+	char auto_dev_name[25];
+	struct device *dev;
+
+	snprintf(auto_dev_name, sizeof(auto_dev_name),
+		 "%s.%d.auto", device_name, r);
+	dev = bus_find_device_by_name(&platform_bus_type,
+					NULL, auto_dev_name);
+	dev_info(dev, "device %s added\n", auto_dev_name);
+	return dev;
+}
+
 static int snd_acp3x_probe(struct pci_dev *pci,
 			   const struct pci_device_id *pci_id)
 {
 	int ret;
-	u32 addr, val;
+	uint64_t addr;
+	int val, i, r;
 	struct acp3x_dev_data *adata;
-	struct platform_device_info pdevinfo;
+	struct device *dev;
+	struct i2s_platform_data *i2s_pdata;
 	unsigned int irqflags;
 
 	if (pci_enable_device(pci)) {
@@ -63,47 +89,82 @@  static int snd_acp3x_probe(struct pci_dev *pci,
 	}
 	pci_set_master(pci);
 	pci_set_drvdata(pci, adata);
-
+	adata->parent = &pci->dev;
 	val = rv_readl(adata->acp3x_base + mmACP_I2S_PIN_CONFIG);
 	switch (val) {
 	case I2S_MODE:
 		adata->res = devm_kzalloc(&pci->dev,
-					  sizeof(struct resource) * 2,
-					  GFP_KERNEL);
-		if (!adata->res) {
+				sizeof(struct resource) * 4,
+						GFP_KERNEL);
+		adata->cell = devm_kzalloc(&pci->dev,
+				sizeof(struct mfd_cell) * 3,
+						GFP_KERNEL);
+		if (!adata->cell) {
 			ret = -ENOMEM;
 			goto unmap_mmio;
 		}
 
-		adata->res[0].name = "acp3x_i2s_iomem";
-		adata->res[0].flags = IORESOURCE_MEM;
-		adata->res[0].start = addr;
-		adata->res[0].end = addr + (ACP3x_REG_END - ACP3x_REG_START);
-
-		adata->res[1].name = "acp3x_i2s_irq";
-		adata->res[1].flags = IORESOURCE_IRQ;
-		adata->res[1].start = pci->irq;
-		adata->res[1].end = pci->irq;
-
-		adata->acp3x_audio_mode = ACP3x_I2S_MODE;
-
-		memset(&pdevinfo, 0, sizeof(pdevinfo));
-		pdevinfo.name = "acp3x_rv_i2s";
-		pdevinfo.id = 0;
-		pdevinfo.parent = &pci->dev;
-		pdevinfo.num_res = 2;
-		pdevinfo.res = adata->res;
-		pdevinfo.data = &irqflags;
-		pdevinfo.size_data = sizeof(irqflags);
-
-		adata->pdev = platform_device_register_full(&pdevinfo);
-		if (IS_ERR(adata->pdev)) {
-			dev_err(&pci->dev, "cannot register %s device\n",
-				pdevinfo.name);
-			ret = PTR_ERR(adata->pdev);
-			goto unmap_mmio;
+		i2s_pdata = devm_kzalloc(&pci->dev,
+				sizeof(struct i2s_platform_data) * 3,
+						GFP_KERNEL);
+		if (i2s_pdata == NULL) {
+			kfree(adata->res);
+			kfree(adata->cell);
+			return -ENOMEM;
 		}
+		adata->res[0].name	= "acp3x_i2s_iomem";
+		adata->res[0].flags	= IORESOURCE_MEM;
+		adata->res[0].start	= addr;
+		adata->res[0].end	= addr +
+			(ACP3x_REG_END - ACP3x_REG_START);
+		i2s_pdata[0].cap	= 0;
+		i2s_pdata[0].snd_rates	= SNDRV_PCM_RATE_8000_96000;
+
+		adata->res[1].name	= "acp3x_i2s_sp_play_cap";
+		adata->res[1].flags	= IORESOURCE_MEM;
+		adata->res[1].start	= addr + ACP3x_I2STDM_REG_START;
+		adata->res[1].end	= addr + ACP3x_I2STDM_REG_END;
+		i2s_pdata[1].cap	= 0;
+		i2s_pdata[1].snd_rates	= SNDRV_PCM_RATE_8000_96000;
+
+		adata->res[2].name	= "acp3x_i2s_bt_play_cap";
+		adata->res[2].flags	= IORESOURCE_MEM;
+		adata->res[2].start	= addr + ACP3x_BT_TDM_REG_START;
+		adata->res[2].end	= addr + ACP3x_BT_TDM_REG_END;
+		i2s_pdata[2].cap	= 0;
+		i2s_pdata[2].snd_rates	= SNDRV_PCM_RATE_8000_96000;
+
+		adata->res[3].name	= "acp3x_i2s_irq";
+		adata->res[3].flags	= IORESOURCE_IRQ;
+		adata->res[3].start	= pci->irq;
+		adata->res[3].end	= adata->res[3].start;
+
+		adata->acp3x_audio_mode	= ACP3x_I2S_MODE;
+
+		adata->cell[0].name	=	"acp3x_rv_i2s_dma";
+		adata->cell[0].num_resources	= 4;
+		adata->cell[0].resources	= &adata->res[0];
+		adata->cell[0].platform_data	= &irqflags;
+		adata->cell[0].pdata_size	= sizeof(irqflags);
+
+		adata->cell[1].name		= "acp3x_i2s_playcap";
+		adata->cell[1].num_resources	= 1;
+		adata->cell[1].resources	= &adata->res[1];
+		adata->cell[1].platform_data	= &i2s_pdata[0];
+		adata->cell[1].pdata_size	=
+				sizeof(struct i2s_platform_data);
+
+		adata->cell[2].name		= "acp3x_i2s_playcap";
+		adata->cell[2].num_resources	= 1;
+		adata->cell[2].resources	= &adata->res[2];
+		adata->cell[2].platform_data	= &i2s_pdata[1];
+		adata->cell[2].pdata_size	=
+				sizeof(struct i2s_platform_data);
+		r = mfd_add_hotplug_devices(adata->parent, adata->cell,	3);
+		for (i = 0; i < 3 ; i++)
+			dev = get_mfd_cell_dev(adata->cell[i].name, i);
 		break;
+
 	default:
 		dev_err(&pci->dev, "Invalid ACP audio mode : %d\n", val);
 		ret = -ENODEV;
@@ -112,23 +173,29 @@  static int snd_acp3x_probe(struct pci_dev *pci,
 	return 0;
 
 unmap_mmio:
-	pci_disable_msi(pci);
+	mfd_remove_devices(adata->parent);
+	kfree(adata->res);
+	kfree(adata->cell);
 	iounmap(adata->acp3x_base);
+	/*ignore device status and return driver probe error*/
+	return -ENODEV;
 release_regions:
 	pci_release_regions(pci);
+	/*ignore device status and return driver probe error*/
+	return -ENODEV;
 disable_pci:
 	pci_disable_device(pci);
-
-	return ret;
+	/*ignore device status and return driver probe error*/
+	return -ENODEV;
 }
 
 static void snd_acp3x_remove(struct pci_dev *pci)
 {
 	struct acp3x_dev_data *adata = pci_get_drvdata(pci);
-
-	platform_device_unregister(adata->pdev);
+	mfd_remove_devices(adata->parent);
+	kfree(adata->res);
+	kfree(adata->cell);
 	iounmap(adata->acp3x_base);
-
 	pci_disable_msi(pci);
 	pci_release_regions(pci);
 	pci_disable_device(pci);
@@ -151,6 +218,7 @@  static struct pci_driver acp3x_driver  = {
 
 module_pci_driver(acp3x_driver);
 
+MODULE_AUTHOR("Vishnuvardhanrao.Ravulapati@amd.com");
 MODULE_AUTHOR("Maruthi.Bayyavarapu@amd.com");
 MODULE_DESCRIPTION("AMD ACP3x PCI driver");
 MODULE_LICENSE("GPL v2");