[alsa-devel] [PATCH v2 0/6] ASoC: davinci-mcbsp: add binding for McBSP
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
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@barix.com --- 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"; +};
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@barix.com
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 withexisting 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
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@barix.com
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 withexisting 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
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
On Fri, Apr 15, 2016 at 3:48 AM, Petr Kulhavy petr@barix.com 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@barix.com
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 compatibilitywith
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
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) +{ + 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), }, };
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 inTexas 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.
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@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 inTexas 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.
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
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.
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
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.
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@barix.com --- 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
Add OF_DEV_AUXDATA for mcbsp to be able to use clocks.
Signed-off-by: Petr Kulhavy petr@barix.com --- 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), {} };
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@barix.com --- 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>; }; };
Add SoC nodes for McBSP0 and McBSP1 as well as the corresponding pinmux configurations.
Signed-off-by: Petr Kulhavy petr@barix.com --- 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";
participants (4)
-
Peter Ujfalusi -
Petr Kulhavy -
Rob Herring -
Sekhar Nori