[PATCH v2 0/6] ASoC: davinci-mcbsp: add binding for McBSP

[PATCH v2 0/6] ASoC: davinci-mcbsp: add binding for McBSP

[Petr Kulhavy]

This series of patches adds devicetree support for the davinci McBSP audio
interface.

Changes in version 2 of the patch:

* add missing TC channel in dmas properties (for compatibility with the new EDMA3 binding)
* remove "-audio" postfix from the compatible string
* remove "channel-combine" property
* of_match_table renamed consistently with the rest of the file (davinci_i2s_match)
* devicetree DMA configuration in probe simplified

[PATCH v2 1/6] ASoC: davinci-mcbsp: add binding for McBSP

[Petr Kulhavy]

Add devicetree binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx
MultiChannel Buffered Serial Port (McBSP)

The optional register range "dat" is not implemented at the moment.
The current driver supports only DMA into RX/TX registers but no FIFO.
Once the FIFO is implemented in the driver the "dat" range will be used.

Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
---
v1: initial
v2: add missing TC channel in dmas properties (for compatibility with the new EDMA3 binding)
    remove "-audio" postfix from the compatible string
    remove "channel-combine" property

 .../devicetree/bindings/sound/davinci-mcbsp.txt    | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/davinci-mcbsp.txt

diff --git a/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
new file mode 100644
index 000000000000..de45865c3863
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
@@ -0,0 +1,51 @@
+Texas Instruments DaVinci McBSP module
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+This binding describes the "Multi-channel Buffered Serial Port" (McBSP)
+audio interface found in some TI DaVinci processors like the OMAP-L138 or AM180x.
+
+
+Required properties:
+~~~~~~~~~~~~~~~~~~~~
+- compatible : "ti,da850-mcbsp"
+
+- reg : physical base address and length of the controller memory mapped
+        region(s).
+- reg-names : Should contain:
+        * "mpu" for the main registers (required). For compatibility with
+          existing software, it is recommended this is the first entry.
+        * "dat" for the data FIFO (optional).
+
+- dmas: three element list of DMA controller phandles, DMA request line and
+	TC channel ordered triplets.
+- dma-names: identifier string for each DMA request line in the dmas property.
+	These strings correspond 1:1 with the ordered pairs in dmas. The dma
+	identifiers must be "rx" and "tx".
+
+Optional properties:
+~~~~~~~~~~~~~~~~~~~~
+- interrupts : Interrupt numbers for McBSP
+- interrupt-names : Known interrupt names are "rx" and "tx"
+
+- pinctrl-0: Should specify pin control group used for this controller.
+- pinctrl-names: Should contain only one value - "default", for more details
+        please refer to pinctrl-bindings.txt
+
+Example (AM1808):
+~~~~~~~~~~~~~~~~~
+
+mcbsp0: mcbsp@1d10000 {
+	compatible = "ti,da850-mcbsp";
+	pinctrl-names = "default";
+	pinctrl-0 = <&mcbsp0_pins>;
+
+	reg = 	<0x00110000 0x1000>,
+		<0x00310000 0x1000>;
+	reg-names = "mpu", "dat";
+	interrupts = <97 98>;
+	interrupts-names = "rx", "tx";
+	dmas = <&edma0 3 1
+		&edma0 2 1>;
+	dma-names = "tx", "rx";
+	status = "okay";
+};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/6] ASoC: Davinci: McBSP: add device tree support for McBSP

[Petr Kulhavy]

This adds DT support for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx McBSP driver.

Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
---
v1: initial
v2: remove "-audio" postfix from the compatible string
    of_match_table renamed consistently with the rest of the file (davinci_i2s_match)
    "channel-combine" property removed
    devicetree DMA configuration in probe simplified

 sound/soc/davinci/Kconfig       |   6 ++-
 sound/soc/davinci/davinci-i2s.c | 116 +++++++++++++++++++++++++++++++---------
 2 files changed, 95 insertions(+), 27 deletions(-)

diff --git a/sound/soc/davinci/Kconfig b/sound/soc/davinci/Kconfig
index 50ca291cc225..6b732d8e5896 100644
--- a/sound/soc/davinci/Kconfig
+++ b/sound/soc/davinci/Kconfig
@@ -16,7 +16,11 @@ config SND_EDMA_SOC
 	  - DRA7xx family
 
 config SND_DAVINCI_SOC_I2S
-	tristate
+	tristate "DaVinci Multichannel Buffered Serial Port (McBSP) support"
+	depends on SND_EDMA_SOC
+	help
+	  Say Y or M here if you want to have support for McBSP IP found in
+	  Texas Instruments DaVinci DA850 SoCs.
 
 config SND_DAVINCI_SOC_MCASP
 	tristate "Multichannel Audio Serial Port (McASP) support"
diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index ec98548a5fc9..a3f82d1a73dc 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -4,9 +4,15 @@
  * Author:      Vladimir Barinov, <vbarinov-L1vi/lXTdtvkgf6YlCu6wwC/G2K4zDHf@public.gmane.org>
  * Copyright:   (C) 2007 MontaVista Software, Inc., <source-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org>
  *
+ * DT support	(c) 2016 Petr Kulhavy, Barix AG <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
+ *		based on davinci-mcasp.c DT support
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
+ *
+ * TODO:
+ * on DA850 implement HW FIFOs instead of DMA into DXR and DRR registers
  */
 
 #include <linux/init.h>
@@ -648,15 +654,62 @@ static const struct snd_soc_component_driver davinci_i2s_component = {
 	.name		= "davinci-i2s",
 };
 
+static struct snd_platform_data*
+davinci_i2s_set_pdata_from_of(struct platform_device *pdev)
+{
+	struct snd_platform_data *pdata = NULL;
+	struct device_node *np;
+
+	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
+		return dev_get_platdata(&pdev->dev);
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	np = pdev->dev.of_node;
+
+	/* parse properties here */
+
+	/*
+	 * pdata->clk_input_pin is deliberately not exported to DT
+	 * and the default value of the clk_input_pin is MCBSP_CLKR.
+	 * The value MCBSP_CLKS makes no sense as it turns the CPU
+	 * to a bit clock master in the SND_SOC_DAIFMT_CBM_CFS mode
+	 * where it should be bit clock slave!
+	 */
+
+	return pdata;
+}
+
 static int davinci_i2s_probe(struct platform_device *pdev)
 {
+	struct snd_dmaengine_dai_dma_data *dma_data;
+	struct snd_platform_data *pdata;
 	struct davinci_mcbsp_dev *dev;
 	struct resource *mem, *res;
 	void __iomem *io_base;
 	int *dma;
 	int ret;
 
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pdata = davinci_i2s_set_pdata_from_of(pdev);
+	if (IS_ERR(pdata)) {
+		dev_err(&pdev->dev, "Error populating platform data, err %ld\n",
+			PTR_ERR(pdata));
+		return PTR_ERR(pdata);
+	}
+
+	mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mpu");
+	if (!mem) {
+		dev_warn(&pdev->dev,
+			 "\"mpu\" mem resource not found, using index 0\n");
+		mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+		if (!mem) {
+			dev_err(&pdev->dev, "no mem resource?\n");
+			return -ENODEV;
+		}
+	}
+
 	io_base = devm_ioremap_resource(&pdev->dev, mem);
 	if (IS_ERR(io_base))
 		return PTR_ERR(io_base);
@@ -666,39 +719,43 @@ static int davinci_i2s_probe(struct platform_device *pdev)
 	if (!dev)
 		return -ENOMEM;
 
-	dev->clk = clk_get(&pdev->dev, NULL);
-	if (IS_ERR(dev->clk))
-		return -ENODEV;
-	clk_enable(dev->clk);
-
 	dev->base = io_base;
 
-	dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK].addr =
-	    (dma_addr_t)(mem->start + DAVINCI_MCBSP_DXR_REG);
-
-	dev->dma_data[SNDRV_PCM_STREAM_CAPTURE].addr =
-	    (dma_addr_t)(mem->start + DAVINCI_MCBSP_DRR_REG);
+	/* setup DMA, first TX, then RX */
+	dma_data = &dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK];
+	dma_data->addr = (dma_addr_t)(mem->start + DAVINCI_MCBSP_DXR_REG);
 
-	/* first TX, then RX */
 	res = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "no DMA resource\n");
-		ret = -ENXIO;
-		goto err_release_clk;
+	if (res) {
+		dma = &dev->dma_request[SNDRV_PCM_STREAM_PLAYBACK];
+		*dma = res->start;
+		dma_data->filter_data = dma;
+	} else if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
+		dma_data->filter_data = "tx";
+	} else {
+		dev_err(&pdev->dev, "Missing DMA tx resource\n");
+		return -ENODEV;
 	}
-	dma = &dev->dma_request[SNDRV_PCM_STREAM_PLAYBACK];
-	*dma = res->start;
-	dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK].filter_data = dma;
+
+	dma_data = &dev->dma_data[SNDRV_PCM_STREAM_CAPTURE];
+	dma_data->addr = (dma_addr_t)(mem->start + DAVINCI_MCBSP_DRR_REG);
 
 	res = platform_get_resource(pdev, IORESOURCE_DMA, 1);
-	if (!res) {
-		dev_err(&pdev->dev, "no DMA resource\n");
-		ret = -ENXIO;
-		goto err_release_clk;
+	if (res) {
+		dma = &dev->dma_request[SNDRV_PCM_STREAM_CAPTURE];
+		*dma = res->start;
+		dma_data->filter_data = dma;
+	} else if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
+		dma_data->filter_data = "rx";
+	} else {
+		dev_err(&pdev->dev, "Missing DMA rx resource\n");
+		return -ENODEV;
 	}
-	dma = &dev->dma_request[SNDRV_PCM_STREAM_CAPTURE];
-	*dma = res->start;
-	dev->dma_data[SNDRV_PCM_STREAM_CAPTURE].filter_data = dma;
+
+	dev->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(dev->clk))
+		return -ENODEV;
+	clk_enable(dev->clk);
 
 	dev->dev = &pdev->dev;
 	dev_set_drvdata(&pdev->dev, dev);
@@ -737,11 +794,18 @@ static int davinci_i2s_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id davinci_i2s_match[] = {
+	{ .compatible = "ti,da850-mcbsp" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, davinci_i2s_match);
+
 static struct platform_driver davinci_mcbsp_driver = {
 	.probe		= davinci_i2s_probe,
 	.remove		= davinci_i2s_remove,
 	.driver		= {
 		.name	= "davinci-mcbsp",
+		.of_match_table = of_match_ptr(davinci_i2s_match),
 	},
 };
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 3/6] ARM: davinci: da850: add clocks for mcbsp0 and 1

[Petr Kulhavy]

Add clock definitions "davinci-mcbsp.0" and "davinci-mcbsp.1" in order
to make McBSP driver work on the DA850 platform.

The McBSP 0 and 1 interfaces were not defined for the DA850 platform.
Neither were the related clocks. In order to make the use of McBSP via
devicetree the clocks need to be defined.

Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
---
v1: initial
v2: no change

 arch/arm/mach-davinci/da850.c | 16 ++++++++++++++++
 arch/arm/mach-davinci/psc.h   |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 97d8779a9a65..2b9d972e30ac 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -306,6 +306,20 @@ static struct clk mcasp_clk = {
 	.flags		= DA850_CLK_ASYNC3,
 };
 
+static struct clk mcbsp0_clk = {
+	.name		= "mcbsp0",
+	.parent		= &pll1_sysclk2,
+	.lpsc		= DA850_LPSC1_McBSP0,
+	.gpsc		= 1,
+};
+
+static struct clk mcbsp1_clk = {
+	.name		= "mcbsp1",
+	.parent		= &pll1_sysclk2,
+	.lpsc		= DA850_LPSC1_McBSP1,
+	.gpsc		= 1,
+};
+
 static struct clk lcdc_clk = {
 	.name		= "lcdc",
 	.parent		= &pll0_sysclk2,
@@ -464,6 +478,8 @@ static struct clk_lookup da850_clks[] = {
 	CLK("davinci_emac.1",	NULL,		&emac_clk),
 	CLK("davinci_mdio.0",	"fck",		&emac_clk),
 	CLK("davinci-mcasp.0",	NULL,		&mcasp_clk),
+	CLK("davinci-mcbsp.0",	NULL,		&mcbsp0_clk),
+	CLK("davinci-mcbsp.1",	NULL,		&mcbsp1_clk),
 	CLK("da8xx_lcdc.0",	"fck",		&lcdc_clk),
 	CLK("da830-mmc.0",	NULL,		&mmcsd0_clk),
 	CLK("da830-mmc.1",	NULL,		&mmcsd1_clk),
diff --git a/arch/arm/mach-davinci/psc.h b/arch/arm/mach-davinci/psc.h
index 99d47cfa301f..8af9f09fc10c 100644
--- a/arch/arm/mach-davinci/psc.h
+++ b/arch/arm/mach-davinci/psc.h
@@ -171,6 +171,8 @@
 #define DA8XX_LPSC1_I2C			11
 #define DA8XX_LPSC1_UART1		12
 #define DA8XX_LPSC1_UART2		13
+#define DA850_LPSC1_McBSP0		14
+#define DA850_LPSC1_McBSP1		15
 #define DA8XX_LPSC1_LCDC		16
 #define DA8XX_LPSC1_PWM			17
 #define DA850_LPSC1_MMC_SD1		18
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/6] ARM: davinci: da8xx-dt: add OF_DEV_AUXDATA entries for mcbsp0 and mcbsp1

[Petr Kulhavy]

Add OF_DEV_AUXDATA for mcbsp to be able to use clocks.

Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
---
v1: initial
v2: remove "-audio" postfix from the compatible string

 arch/arm/mach-davinci/da8xx-dt.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index c4b5808ca7c1..d2f5bfbba629 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -45,8 +45,11 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("ns16550a", 0x01d0d000, "serial8250.2", NULL),
 	OF_DEV_AUXDATA("ti,davinci_mdio", 0x01e24000, "davinci_mdio.0", NULL),
 	OF_DEV_AUXDATA("ti,davinci-dm6467-emac", 0x01e20000, "davinci_emac.1",
-		       NULL),
-	OF_DEV_AUXDATA("ti,da830-mcasp-audio", 0x01d00000, "davinci-mcasp.0", NULL),
+			NULL),
+	OF_DEV_AUXDATA("ti,da830-mcasp-audio", 0x01d00000, "davinci-mcasp.0",
+			NULL),
+	OF_DEV_AUXDATA("ti,da850-mcbsp", 0x01d10000, "davinci-mcbsp.0", NULL),
+	OF_DEV_AUXDATA("ti,da850-mcbsp", 0x01d11000, "davinci-mcbsp.1", NULL),
 	{}
 };
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 5/6] ARM: DTS: da850: Fix wrong number of interrupts

[Petr Kulhavy]

The DA850 supports interrupts 0 to 100. The number of interrupts
property "ti,intc-size" was wrongly set to 100. Corrected to 101.

Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
---
v1: initial
v2: no change

 arch/arm/boot/dts/da850.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 226cda76e77c..5996e765e59c 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -19,7 +19,7 @@
 			compatible = "ti,cp-intc";
 			interrupt-controller;
 			#interrupt-cells = <1>;
-			ti,intc-size = <100>;
+			ti,intc-size = <101>;
 			reg = <0xfffee000 0x2000>;
 		};
 	};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 6/6] ARM: DTS: da850: Add McBSP0 and McBSP1

[Petr Kulhavy]

Add SoC nodes for McBSP0 and McBSP1 as well as the corresponding pinmux
configurations.

Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
---
v1: initial
v2: add missing TC channel in dmas properties (for compatibility with the new EDMA3 binding)
    remove "-audio" postfix from the compatible string 

 arch/arm/boot/dts/da850.dtsi | 45 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 5996e765e59c..708d48e1b6d1 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -148,7 +148,24 @@
 					0xc 0x88888888 0xffffffff
 				>;
 			};
-
+			mcbsp0_pins: pinmux_mcbsp0_pins {
+				pinctrl-single,bits = <
+					/* PINMUX2:
+					* CLKS0, DX0, DR0, FSX0
+					* FSR0, CLKX0, CLKR0
+					*/
+					0x8 0x02222220 0xfffffff0
+				>;
+			};
+			mcbsp1_pins: pinmux_mcbsp1_pins {
+				pinctrl-single,bits = <
+					/* PINMUX1:
+					* CLKS1, DX1, DR1, FSX1,
+					* FSR1, CLKX1, CLKR1
+					*/
+					0x4 0x22222220 0xfffffff0
+				>;
+			};
 		};
 		edma0: edma@01c00000 {
 			compatible = "ti,edma3-tpcc";
@@ -335,6 +352,32 @@
 				<&edma0 0 1>;
 			dma-names = "tx", "rx";
 		};
+
+		mcbsp0: mcbsp@1d10000 {
+			compatible = "ti,da850-mcbsp";
+			reg =	<0x00110000 0x1000>,
+				<0x00310000 0x1000>;
+			reg-names = "mpu", "dat";
+			interrupts = <97 98>;
+			interrupts-names = "rx", "tx";
+			dmas = <&edma0 3 1
+				&edma0 2 1>;
+			dma-names = "tx", "rx";
+			status = "disabled";
+		};
+		mcbsp1: mcbsp@1d11000 {
+			compatible = "ti,da850-mcbsp";
+			reg =	<0x00111000 0x1000>,
+				<0x00311000 0x1000>;
+			reg-names = "mpu", "dat";
+			interrupts = <99 100>;
+			interrupts-names = "rx", "tx";
+			dmas = <&edma0 5 1
+				&edma0 4 1>;
+			dma-names = "tx", "rx";
+			status = "disabled";
+		};
+
 	};
 	nand_cs3@62000000 {
 		compatible = "ti,davinci-nand";
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/6] ASoC: davinci-mcbsp: add binding for McBSP

[Rob Herring]

On Mon, Apr 11, 2016 at 01:45:12PM +0200, Petr Kulhavy wrote:
> Add devicetree binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx
> MultiChannel Buffered Serial Port (McBSP)
> 
> The optional register range "dat" is not implemented at the moment.
> The current driver supports only DMA into RX/TX registers but no FIFO.
> Once the FIFO is implemented in the driver the "dat" range will be used.
> 
> Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
> ---
> v1: initial
> v2: add missing TC channel in dmas properties (for compatibility with the new EDMA3 binding)
>     remove "-audio" postfix from the compatible string
>     remove "channel-combine" property
> 
>  .../devicetree/bindings/sound/davinci-mcbsp.txt    | 51 ++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
> new file mode 100644
> index 000000000000..de45865c3863
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
> @@ -0,0 +1,51 @@
> +Texas Instruments DaVinci McBSP module
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +This binding describes the "Multi-channel Buffered Serial Port" (McBSP)
> +audio interface found in some TI DaVinci processors like the OMAP-L138 or AM180x.
> +
> +
> +Required properties:
> +~~~~~~~~~~~~~~~~~~~~
> +- compatible : "ti,da850-mcbsp"

You list several SoCs above, but only one compatible string here. A 
specific compatible string per SoC please.

> +
> +- reg : physical base address and length of the controller memory mapped
> +        region(s).
> +- reg-names : Should contain:
> +        * "mpu" for the main registers (required). For compatibility with
> +          existing software, it is recommended this is the first entry.

s/recommended/required/

> +        * "dat" for the data FIFO (optional).
> +
> +- dmas: three element list of DMA controller phandles, DMA request line and
> +	TC channel ordered triplets.
> +- dma-names: identifier string for each DMA request line in the dmas property.
> +	These strings correspond 1:1 with the ordered pairs in dmas. The dma
> +	identifiers must be "rx" and "tx".
> +
> +Optional properties:
> +~~~~~~~~~~~~~~~~~~~~
> +- interrupts : Interrupt numbers for McBSP
> +- interrupt-names : Known interrupt names are "rx" and "tx"
> +
> +- pinctrl-0: Should specify pin control group used for this controller.
> +- pinctrl-names: Should contain only one value - "default", for more details
> +        please refer to pinctrl-bindings.txt
> +
> +Example (AM1808):
> +~~~~~~~~~~~~~~~~~
> +
> +mcbsp0: mcbsp@1d10000 {
> +	compatible = "ti,da850-mcbsp";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mcbsp0_pins>;
> +
> +	reg = 	<0x00110000 0x1000>,
> +		<0x00310000 0x1000>;
> +	reg-names = "mpu", "dat";
> +	interrupts = <97 98>;
> +	interrupts-names = "rx", "tx";
> +	dmas = <&edma0 3 1
> +		&edma0 2 1>;
> +	dma-names = "tx", "rx";
> +	status = "okay";
> +};
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/6] ASoC: davinci-mcbsp: add binding for McBSP

[Petr Kulhavy]

On 13.04.2016 16:30, Rob Herring wrote:
> On Mon, Apr 11, 2016 at 01:45:12PM +0200, Petr Kulhavy wrote:
>> Add devicetree binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx
>> MultiChannel Buffered Serial Port (McBSP)
>>
>> The optional register range "dat" is not implemented at the moment.
>> The current driver supports only DMA into RX/TX registers but no FIFO.
>> Once the FIFO is implemented in the driver the "dat" range will be used.
>>
>> Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
>> ---
>> v1: initial
>> v2: add missing TC channel in dmas properties (for compatibility with the new EDMA3 binding)
>>      remove "-audio" postfix from the compatible string
>>      remove "channel-combine" property
>>
>>   .../devicetree/bindings/sound/davinci-mcbsp.txt    | 51 ++++++++++++++++++++++
>>   1 file changed, 51 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
>>
>> diff --git a/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
>> new file mode 100644
>> index 000000000000..de45865c3863
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
>> @@ -0,0 +1,51 @@
>> +Texas Instruments DaVinci McBSP module
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +This binding describes the "Multi-channel Buffered Serial Port" (McBSP)
>> +audio interface found in some TI DaVinci processors like the OMAP-L138 or AM180x.
>> +
>> +
>> +Required properties:
>> +~~~~~~~~~~~~~~~~~~~~
>> +- compatible : "ti,da850-mcbsp"
> You list several SoCs above, but only one compatible string here. A
> specific compatible string per SoC please.
>
Hi Rob,

thank you for your feedback. I can test only on the AM1808 platform, 
however as far as I understand the OMAP L138 and AM1808 use the same 
McBSP hardware. The TI guys can give more insight here... Isn't it then 
redundant to define more compatible strings? Sorry for my ignorance, I 
just don't know the policy of defining the compatible strings.

>> +
>> +- reg : physical base address and length of the controller memory mapped
>> +        region(s).
>> +- reg-names : Should contain:
>> +        * "mpu" for the main registers (required). For compatibility with
>> +          existing software, it is recommended this is the first entry.
> s/recommended/required/
Recommended is correct, but I think it make sense to drop the sentence.
If the reg-names are provided then the probe() finds the resource 
regardless of the index.
If not provided it expects it at index 0.
But since we declare that the reg-names is mandatory this sentence is 
just confusing and should be removed.

Petr
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/6] ASoC: davinci-mcbsp: add binding for McBSP

[Sekhar Nori]

On Friday 15 April 2016 02:18 PM, Petr Kulhavy wrote:

> On 13.04.2016 16:30, Rob Herring wrote:

>> On Mon, Apr 11, 2016 at 01:45:12PM +0200, Petr Kulhavy wrote:

>>> a/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
>>> b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
>>> new file mode 100644
>>> index 000000000000..de45865c3863
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
>>> @@ -0,0 +1,51 @@
>>> +Texas Instruments DaVinci McBSP module
>>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +This binding describes the "Multi-channel Buffered Serial Port" (McBSP)
>>> +audio interface found in some TI DaVinci processors like the
>>> OMAP-L138 or AM180x.
>>> +
>>> +
>>> +Required properties:
>>> +~~~~~~~~~~~~~~~~~~~~
>>> +- compatible : "ti,da850-mcbsp"
>> You list several SoCs above, but only one compatible string here. A
>> specific compatible string per SoC please.
>>
> Hi Rob,
> 
> thank you for your feedback. I can test only on the AM1808 platform,
> however as far as I understand the OMAP L138 and AM1808 use the same
> McBSP hardware. The TI guys can give more insight here... Isn't it then
> redundant to define more compatible strings? Sorry for my ignorance, I
> just don't know the policy of defining the compatible strings.

DA850, OMAP-L138 and AM18x are pin-for-pin compatible devices meant for
different markets/applications. IP-wise, silicon-integration-wise or
pin-wise, there is no difference in way McASP behaves between the parts.

Thanks,
Sekhar

Re: [PATCH v2 2/6] ASoC: Davinci: McBSP: add device tree support for McBSP

[Peter Ujfalusi]

On 04/11/16 14:45, Petr Kulhavy wrote:
> This adds DT support for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx McBSP driver.
> 
> Signed-off-by: Petr Kulhavy <petr@barix.com>
> ---
> v1: initial
> v2: remove "-audio" postfix from the compatible string
>     of_match_table renamed consistently with the rest of the file (davinci_i2s_match)
>     "channel-combine" property removed
>     devicetree DMA configuration in probe simplified
> 
>  sound/soc/davinci/Kconfig       |   6 ++-
>  sound/soc/davinci/davinci-i2s.c | 116 +++++++++++++++++++++++++++++++---------
>  2 files changed, 95 insertions(+), 27 deletions(-)
> 
> diff --git a/sound/soc/davinci/Kconfig b/sound/soc/davinci/Kconfig
> index 50ca291cc225..6b732d8e5896 100644
> --- a/sound/soc/davinci/Kconfig
> +++ b/sound/soc/davinci/Kconfig
> @@ -16,7 +16,11 @@ config SND_EDMA_SOC
>  	  - DRA7xx family
>  
>  config SND_DAVINCI_SOC_I2S
> -	tristate
> +	tristate "DaVinci Multichannel Buffered Serial Port (McBSP) support"
> +	depends on SND_EDMA_SOC
> +	help
> +	  Say Y or M here if you want to have support for McBSP IP found in
> +	  Texas Instruments DaVinci DA850 SoCs.
>  
>  config SND_DAVINCI_SOC_MCASP
>  	tristate "Multichannel Audio Serial Port (McASP) support"
> diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
> index ec98548a5fc9..a3f82d1a73dc 100644
> --- a/sound/soc/davinci/davinci-i2s.c
> +++ b/sound/soc/davinci/davinci-i2s.c
> @@ -4,9 +4,15 @@
>   * Author:      Vladimir Barinov, <vbarinov@embeddedalley.com>
>   * Copyright:   (C) 2007 MontaVista Software, Inc., <source@mvista.com>
>   *
> + * DT support	(c) 2016 Petr Kulhavy, Barix AG <petr@barix.com>
> + *		based on davinci-mcasp.c DT support
> + *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
> + *
> + * TODO:
> + * on DA850 implement HW FIFOs instead of DMA into DXR and DRR registers
>   */
>  
>  #include <linux/init.h>
> @@ -648,15 +654,62 @@ static const struct snd_soc_component_driver davinci_i2s_component = {
>  	.name		= "davinci-i2s",
>  };
>  
> +static struct snd_platform_data*
> +davinci_i2s_set_pdata_from_of(struct platform_device *pdev)

It is a bit misleading function name, would be better to name it as
davinci_i2s_get_pdata()

as it will fetch the legacy pdata or craft one when booting with DT.

> +{
> +	struct snd_platform_data *pdata = NULL;
> +	struct device_node *np;
> +
> +	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
> +		return dev_get_platdata(&pdev->dev);
> +
> +	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return ERR_PTR(-ENOMEM);
> +
> +	np = pdev->dev.of_node;

???

> +
> +	/* parse properties here */
> +
> +	/*
> +	 * pdata->clk_input_pin is deliberately not exported to DT
> +	 * and the default value of the clk_input_pin is MCBSP_CLKR.
> +	 * The value MCBSP_CLKS makes no sense as it turns the CPU
> +	 * to a bit clock master in the SND_SOC_DAIFMT_CBM_CFS mode
> +	 * where it should be bit clock slave!
> +	 */
> +
> +	return pdata;
> +}
> +
>  static int davinci_i2s_probe(struct platform_device *pdev)
>  {
> +	struct snd_dmaengine_dai_dma_data *dma_data;
> +	struct snd_platform_data *pdata;
>  	struct davinci_mcbsp_dev *dev;
>  	struct resource *mem, *res;
>  	void __iomem *io_base;
>  	int *dma;
>  	int ret;
>  
> -	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	pdata = davinci_i2s_set_pdata_from_of(pdev);
> +	if (IS_ERR(pdata)) {
> +		dev_err(&pdev->dev, "Error populating platform data, err %ld\n",
> +			PTR_ERR(pdata));
> +		return PTR_ERR(pdata);
> +	}

The current driver does not use the pdata as far as I can see. Kind of
strange. So now in DT boot you will allocate memory for the pdata, which is
not used by the driver at all.
I think we should implement the pdata handling in the driver first for the
legacy mode, then probably having pdata allocated for DT case might make sense.

> +
> +	mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mpu");
> +	if (!mem) {
> +		dev_warn(&pdev->dev,
> +			 "\"mpu\" mem resource not found, using index 0\n");
> +		mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +		if (!mem) {
> +			dev_err(&pdev->dev, "no mem resource?\n");
> +			return -ENODEV;
> +		}
> +	}
> +
>  	io_base = devm_ioremap_resource(&pdev->dev, mem);
>  	if (IS_ERR(io_base))
>  		return PTR_ERR(io_base);
> @@ -666,39 +719,43 @@ static int davinci_i2s_probe(struct platform_device *pdev)
>  	if (!dev)
>  		return -ENOMEM;
>  
> -	dev->clk = clk_get(&pdev->dev, NULL);
> -	if (IS_ERR(dev->clk))
> -		return -ENODEV;
> -	clk_enable(dev->clk);
> -
>  	dev->base = io_base;
>  
> -	dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK].addr =
> -	    (dma_addr_t)(mem->start + DAVINCI_MCBSP_DXR_REG);
> -
> -	dev->dma_data[SNDRV_PCM_STREAM_CAPTURE].addr =
> -	    (dma_addr_t)(mem->start + DAVINCI_MCBSP_DRR_REG);
> +	/* setup DMA, first TX, then RX */
> +	dma_data = &dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK];
> +	dma_data->addr = (dma_addr_t)(mem->start + DAVINCI_MCBSP_DXR_REG);
>  
> -	/* first TX, then RX */
>  	res = platform_get_resource(pdev, IORESOURCE_DMA, 0);
> -	if (!res) {
> -		dev_err(&pdev->dev, "no DMA resource\n");
> -		ret = -ENXIO;
> -		goto err_release_clk;
> +	if (res) {
> +		dma = &dev->dma_request[SNDRV_PCM_STREAM_PLAYBACK];
> +		*dma = res->start;
> +		dma_data->filter_data = dma;
> +	} else if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
> +		dma_data->filter_data = "tx";
> +	} else {
> +		dev_err(&pdev->dev, "Missing DMA tx resource\n");
> +		return -ENODEV;
>  	}
> -	dma = &dev->dma_request[SNDRV_PCM_STREAM_PLAYBACK];
> -	*dma = res->start;
> -	dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK].filter_data = dma;
> +
> +	dma_data = &dev->dma_data[SNDRV_PCM_STREAM_CAPTURE];
> +	dma_data->addr = (dma_addr_t)(mem->start + DAVINCI_MCBSP_DRR_REG);
>  
>  	res = platform_get_resource(pdev, IORESOURCE_DMA, 1);
> -	if (!res) {
> -		dev_err(&pdev->dev, "no DMA resource\n");
> -		ret = -ENXIO;
> -		goto err_release_clk;
> +	if (res) {
> +		dma = &dev->dma_request[SNDRV_PCM_STREAM_CAPTURE];
> +		*dma = res->start;
> +		dma_data->filter_data = dma;
> +	} else if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
> +		dma_data->filter_data = "rx";
> +	} else {
> +		dev_err(&pdev->dev, "Missing DMA rx resource\n");
> +		return -ENODEV;
>  	}
> -	dma = &dev->dma_request[SNDRV_PCM_STREAM_CAPTURE];
> -	*dma = res->start;
> -	dev->dma_data[SNDRV_PCM_STREAM_CAPTURE].filter_data = dma;
> +
> +	dev->clk = clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(dev->clk))
> +		return -ENODEV;
> +	clk_enable(dev->clk);
>  
>  	dev->dev = &pdev->dev;
>  	dev_set_drvdata(&pdev->dev, dev);
> @@ -737,11 +794,18 @@ static int davinci_i2s_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct of_device_id davinci_i2s_match[] = {
> +	{ .compatible = "ti,da850-mcbsp" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, davinci_i2s_match);
> +
>  static struct platform_driver davinci_mcbsp_driver = {
>  	.probe		= davinci_i2s_probe,
>  	.remove		= davinci_i2s_remove,
>  	.driver		= {
>  		.name	= "davinci-mcbsp",
> +		.of_match_table = of_match_ptr(davinci_i2s_match),
>  	},
>  };

The rest looks good to me.

-- 
Péter

Re: [alsa-devel] [PATCH v2 2/6] ASoC: Davinci: McBSP: add device tree support for McBSP

[Petr Kulhavy]

On 15.04.2016 11:32, Peter Ujfalusi wrote:
> On 04/11/16 14:45, Petr Kulhavy wrote:
>> This adds DT support for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx McBSP driver.
>>
>> Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
>> ---
>> v1: initial
>> v2: remove "-audio" postfix from the compatible string
>>      of_match_table renamed consistently with the rest of the file (davinci_i2s_match)
>>      "channel-combine" property removed
>>      devicetree DMA configuration in probe simplified
>>
>>   sound/soc/davinci/Kconfig       |   6 ++-
>>   sound/soc/davinci/davinci-i2s.c | 116 +++++++++++++++++++++++++++++++---------
>>   2 files changed, 95 insertions(+), 27 deletions(-)
>>
>> diff --git a/sound/soc/davinci/Kconfig b/sound/soc/davinci/Kconfig
>> index 50ca291cc225..6b732d8e5896 100644
>> --- a/sound/soc/davinci/Kconfig
>> +++ b/sound/soc/davinci/Kconfig
>> @@ -16,7 +16,11 @@ config SND_EDMA_SOC
>>   	  - DRA7xx family
>>   
>>   config SND_DAVINCI_SOC_I2S
>> -	tristate
>> +	tristate "DaVinci Multichannel Buffered Serial Port (McBSP) support"
>> +	depends on SND_EDMA_SOC
>> +	help
>> +	  Say Y or M here if you want to have support for McBSP IP found in
>> +	  Texas Instruments DaVinci DA850 SoCs.
>>   
>>   config SND_DAVINCI_SOC_MCASP
>>   	tristate "Multichannel Audio Serial Port (McASP) support"
>> diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
>> index ec98548a5fc9..a3f82d1a73dc 100644
>> --- a/sound/soc/davinci/davinci-i2s.c
>> +++ b/sound/soc/davinci/davinci-i2s.c
>> @@ -4,9 +4,15 @@
>>    * Author:      Vladimir Barinov, <vbarinov-L1vi/lXTdtvkgf6YlCu6wwC/G2K4zDHf@public.gmane.org>
>>    * Copyright:   (C) 2007 MontaVista Software, Inc., <source-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org>
>>    *
>> + * DT support	(c) 2016 Petr Kulhavy, Barix AG <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
>> + *		based on davinci-mcasp.c DT support
>> + *
>>    * This program is free software; you can redistribute it and/or modify
>>    * it under the terms of the GNU General Public License version 2 as
>>    * published by the Free Software Foundation.
>> + *
>> + * TODO:
>> + * on DA850 implement HW FIFOs instead of DMA into DXR and DRR registers
>>    */
>>   
>>   #include <linux/init.h>
>> @@ -648,15 +654,62 @@ static const struct snd_soc_component_driver davinci_i2s_component = {
>>   	.name		= "davinci-i2s",
>>   };
>>   
>> +static struct snd_platform_data*
>> +davinci_i2s_set_pdata_from_of(struct platform_device *pdev)
> It is a bit misleading function name, would be better to name it as
> davinci_i2s_get_pdata()
>
> as it will fetch the legacy pdata or craft one when booting with DT.
Thanks, I will change the name.
> +
> +	/* parse properties here */
> +
> +	/*
> +	 * pdata->clk_input_pin is deliberately not exported to DT
> +	 * and the default value of the clk_input_pin is MCBSP_CLKR.
> +	 * The value MCBSP_CLKS makes no sense as it turns the CPU
> +	 * to a bit clock master in the SND_SOC_DAIFMT_CBM_CFS mode
> +	 * where it should be bit clock slave!
> +	 */
> +
> +	return pdata;
> +}
> +
>   static int davinci_i2s_probe(struct platform_device *pdev)
>   {
> +	struct snd_dmaengine_dai_dma_data *dma_data;
> +	struct snd_platform_data *pdata;
>   	struct davinci_mcbsp_dev *dev;
>   	struct resource *mem, *res;
>   	void __iomem *io_base;
>   	int *dma;
>   	int ret;
>   
> -	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	pdata = davinci_i2s_set_pdata_from_of(pdev);
> +	if (IS_ERR(pdata)) {
> +		dev_err(&pdev->dev, "Error populating platform data, err %ld\n",
> +			PTR_ERR(pdata));
> +		return PTR_ERR(pdata);
> +	}
> The current driver does not use the pdata as far as I can see. Kind of
> strange. So now in DT boot you will allocate memory for the pdata, which is
> not used by the driver at all.
> I think we should implement the pdata handling in the driver first for the
> legacy mode, then probably having pdata allocated for DT case might make sense.
Indeed the current driver does not use pdata. Neither the legacy board 
drivers in arch/mach-davinci do.
Well, except for the dm365_evm which sets the asp_chan_q which is later 
not used:

static struct snd_platform_data dm365_evm_snd_data __maybe_unused = {
         .asp_chan_q = EVENTQ_3,
};


If the pdata is not used at all I would completely drop it.
Or what do you think the pdata should carry?

Petr
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/6] ASoC: Davinci: McBSP: add device tree support for McBSP

[Peter Ujfalusi]

On 04/18/16 12:27, Petr Kulhavy wrote:
>>   static int davinci_i2s_probe(struct platform_device *pdev)
>>   {
>> +    struct snd_dmaengine_dai_dma_data *dma_data;
>> +    struct snd_platform_data *pdata;
>>       struct davinci_mcbsp_dev *dev;
>>       struct resource *mem, *res;
>>       void __iomem *io_base;
>>       int *dma;
>>       int ret;
>>   -    mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +    pdata = davinci_i2s_set_pdata_from_of(pdev);
>> +    if (IS_ERR(pdata)) {
>> +        dev_err(&pdev->dev, "Error populating platform data, err %ld\n",
>> +            PTR_ERR(pdata));
>> +        return PTR_ERR(pdata);
>> +    }
>> The current driver does not use the pdata as far as I can see. Kind of
>> strange. So now in DT boot you will allocate memory for the pdata, which is
>> not used by the driver at all.
>> I think we should implement the pdata handling in the driver first for the
>> legacy mode, then probably having pdata allocated for DT case might make sense.
> Indeed the current driver does not use pdata. Neither the legacy board drivers
> in arch/mach-davinci do.
> Well, except for the dm365_evm which sets the asp_chan_q which is later not used:
> 
> static struct snd_platform_data dm365_evm_snd_data __maybe_unused = {
>         .asp_chan_q = EVENTQ_3,
> };
> 
> 
> If the pdata is not used at all I would completely drop it.

the asp_chan_q was used with the old davinci_pcm which has been removed a
while ago.

> Or what do you think the pdata should carry?

Good question. Since the driver itself does not care about pdata I would
remove it. On the other hand if you implement the FIFO handling you will need
pdata for legacy mode to select the FIFO threshold.
Currently there is one pdata structure for legacy mode used by the McASP,
McBSP and the Voice Codec. It contains mixed options applicable for all and
some which applicable only for one of the drivers...
I think if we want to make things clean and neat we would aim to have separate
pdata for each of the drivers.

I would probably drop the current pdata for McBSP and when I see what data I
need for the driver later I would create one for the McBSP only.

-- 
Péter

Re: [PATCH v2 2/6] ASoC: Davinci: McBSP: add device tree support for McBSP

[Petr Kulhavy]

On 18.04.2016 12:18, Peter Ujfalusi wrote:
> On 04/18/16 12:27, Petr Kulhavy wrote:
>
>> Or what do you think the pdata should carry?
> Good question. Since the driver itself does not care about pdata I would
> remove it. On the other hand if you implement the FIFO handling you will need
> pdata for legacy mode to select the FIFO threshold.
> Currently there is one pdata structure for legacy mode used by the McASP,
> McBSP and the Voice Codec. It contains mixed options applicable for all and
> some which applicable only for one of the drivers...
> I think if we want to make things clean and neat we would aim to have separate
> pdata for each of the drivers.
>
> I would probably drop the current pdata for McBSP and when I see what data I
> need for the driver later I would create one for the McBSP only.
>

Thanks, Peter. I agree with you.
I don't have the time to implement the FIFO now, so it makes sense to 
drop the pdata for now and add a dedicated one if needed later.
Shall I remove the pdata also from the EVK legacy board drivers?

I'm going to prepare another patch set.

Cheers
Petr

Re: [PATCH v2 2/6] ASoC: Davinci: McBSP: add device tree support for McBSP

[Peter Ujfalusi]

On 04/18/16 13:24, Petr Kulhavy wrote:
> 
> 
> On 18.04.2016 12:18, Peter Ujfalusi wrote:
>> On 04/18/16 12:27, Petr Kulhavy wrote:
>>
>>> Or what do you think the pdata should carry?
>> Good question. Since the driver itself does not care about pdata I would
>> remove it. On the other hand if you implement the FIFO handling you will need
>> pdata for legacy mode to select the FIFO threshold.
>> Currently there is one pdata structure for legacy mode used by the McASP,
>> McBSP and the Voice Codec. It contains mixed options applicable for all and
>> some which applicable only for one of the drivers...
>> I think if we want to make things clean and neat we would aim to have separate
>> pdata for each of the drivers.
>>
>> I would probably drop the current pdata for McBSP and when I see what data I
>> need for the driver later I would create one for the McBSP only.
>>
> 
> Thanks, Peter. I agree with you.
> I don't have the time to implement the FIFO now, so it makes sense to drop the
> pdata for now and add a dedicated one if needed later.
> Shall I remove the pdata also from the EVK legacy board drivers?

Yes please. It is not used and not applicable either.

> I'm going to prepare another patch set.

-- 
Péter

Re: [PATCH v2 1/6] ASoC: davinci-mcbsp: add binding for McBSP

[Rob Herring]

On Fri, Apr 15, 2016 at 3:48 AM, Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org> wrote:
>
> On 13.04.2016 16:30, Rob Herring wrote:
>>
>> On Mon, Apr 11, 2016 at 01:45:12PM +0200, Petr Kulhavy wrote:
>>>
>>> Add devicetree binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx
>>> MultiChannel Buffered Serial Port (McBSP)
>>>
>>> The optional register range "dat" is not implemented at the moment.
>>> The current driver supports only DMA into RX/TX registers but no FIFO.
>>> Once the FIFO is implemented in the driver the "dat" range will be used.
>>>
>>> Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
>>> ---
>>> v1: initial
>>> v2: add missing TC channel in dmas properties (for compatibility with the
>>> new EDMA3 binding)
>>>      remove "-audio" postfix from the compatible string
>>>      remove "channel-combine" property
>>>
>>>   .../devicetree/bindings/sound/davinci-mcbsp.txt    | 51
>>> ++++++++++++++++++++++
>>>   1 file changed, 51 insertions(+)
>>>   create mode 100644
>>> Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
>>> b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
>>> new file mode 100644
>>> index 000000000000..de45865c3863
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
>>> @@ -0,0 +1,51 @@
>>> +Texas Instruments DaVinci McBSP module
>>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +This binding describes the "Multi-channel Buffered Serial Port" (McBSP)
>>> +audio interface found in some TI DaVinci processors like the OMAP-L138
>>> or AM180x.
>>> +
>>> +
>>> +Required properties:
>>> +~~~~~~~~~~~~~~~~~~~~
>>> +- compatible : "ti,da850-mcbsp"
>>
>> You list several SoCs above, but only one compatible string here. A
>> specific compatible string per SoC please.
>>
> Hi Rob,
>
> thank you for your feedback. I can test only on the AM1808 platform, however
> as far as I understand the OMAP L138 and AM1808 use the same McBSP hardware.
> The TI guys can give more insight here... Isn't it then redundant to define
> more compatible strings? Sorry for my ignorance, I just don't know the
> policy of defining the compatible strings.

Based on Sekhar's reply okay.

>>> +
>>> +- reg : physical base address and length of the controller memory mapped
>>> +        region(s).
>>> +- reg-names : Should contain:
>>> +        * "mpu" for the main registers (required). For compatibility
>>> with
>>> +          existing software, it is recommended this is the first entry.
>>
>> s/recommended/required/
>
> Recommended is correct, but I think it make sense to drop the sentence.
> If the reg-names are provided then the probe() finds the resource regardless
> of the index.
> If not provided it expects it at index 0.
> But since we declare that the reg-names is mandatory this sentence is just
> confusing and should be removed.

No, required is correct. The order of reg (or any other property)
entries must be defined regardless of the use of *-names or not.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html