Patchwork [13/20] ASoC: pxa: pxa-ssp: add DT bindings

login
register
mail settings
Submitter Daniel Mack
Date Aug. 7, 2013, 3:34 p.m.
Message ID <1375889649-14638-14-git-send-email-zonque@gmail.com>
Download mbox | patch
Permalink /patch/265534/
State New
Headers show

Comments

Daniel Mack - Aug. 7, 2013, 3:34 p.m.
This patch contains a cruel hack for looking up the the DMA request
number. The problem here is that the implementation as it stands will
allocate the DMA channel from the user of the ssp port, and hence we
cannot allocate a real channel here.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 Documentation/devicetree/bindings/sound/mrvl,pxa-ssp.txt |  7 +++++++
 sound/soc/pxa/pxa-ssp.c                                  | 12 ++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/mrvl,pxa-ssp.txt
Mark Brown - Aug. 7, 2013, 4:40 p.m.
On Wed, Aug 07, 2013 at 05:34:02PM +0200, Daniel Mack wrote:
> This patch contains a cruel hack for looking up the the DMA request
> number. The problem here is that the implementation as it stands will
> allocate the DMA channel from the user of the ssp port, and hence we
> cannot allocate a real channel here.
> 
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
>  Documentation/devicetree/bindings/sound/mrvl,pxa-ssp.txt |  7 +++++++
>  sound/soc/pxa/pxa-ssp.c                                  | 12 ++++++++++--
>  2 files changed, 17 insertions(+), 2 deletions(-)

So, really this should be part of a generic binding for the SSP device
but obviously this is just a virtual device that binds to a generic bit
of hardware that can be used for other applications like SPI.  For the
ASoC case we could instantiate the DAI from the machine driver but that
isn't going to work for things like SPI.  I'm not 100% sure what the
best way to deal with that is.

One way of doing this is to have two compatible strings for the same bit
of hardware describing the application.  I'm not sure that's nice but it
works...
Daniel Mack - Aug. 8, 2013, 9:39 a.m.
On 07.08.2013 18:40, Mark Brown wrote:
> On Wed, Aug 07, 2013 at 05:34:02PM +0200, Daniel Mack wrote:
>> This patch contains a cruel hack for looking up the the DMA request
>> number. The problem here is that the implementation as it stands will
>> allocate the DMA channel from the user of the ssp port, and hence we
>> cannot allocate a real channel here.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/sound/mrvl,pxa-ssp.txt |  7 +++++++
>>  sound/soc/pxa/pxa-ssp.c                                  | 12 ++++++++++--
>>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> So, really this should be part of a generic binding for the SSP device
> but obviously this is just a virtual device that binds to a generic bit
> of hardware that can be used for other applications like SPI.  For the
> ASoC case we could instantiate the DAI from the machine driver but that
> isn't going to work for things like SPI.  I'm not 100% sure what the
> best way to deal with that is.

Yes, I pondered over this for a while as well. I thought about having
the ssp port instantiated as real device just like we have it now, and
then just add a phandle reference in DT from the DAI to the upstream device.

Then, we should also allocate the DMA channel from the upstream device
and pass it down to the DAI. But then again, the pxa-pcm-lib needs some
rework, as it's currently in charge of obtaining and releasing the channel.

I wanted to postpone that issue until the rest of the series has settled
and limit changes to the bare minimum for now. Does that sound feasible?


Daniel
Mark Brown - Aug. 8, 2013, 1:20 p.m.
On Thu, Aug 08, 2013 at 11:39:59AM +0200, Daniel Mack wrote:

> Then, we should also allocate the DMA channel from the upstream device
> and pass it down to the DAI. But then again, the pxa-pcm-lib needs some
> rework, as it's currently in charge of obtaining and releasing the channel.

> I wanted to postpone that issue until the rest of the series has settled
> and limit changes to the bare minimum for now. Does that sound feasible?

Yup, the big problem here is the whole DT as ABI thing - what happens in
the code isn't so important but the DT is supposed to be stable.  Or we
could decide that this is one of the cases that's unstable in which case
we should probably flag it up in the DT binding document if nothing else.
Daniel Mack - Aug. 9, 2013, 1:03 p.m.
On 08.08.2013 15:20, Mark Brown wrote:
> On Thu, Aug 08, 2013 at 11:39:59AM +0200, Daniel Mack wrote:
> 
>> Then, we should also allocate the DMA channel from the upstream device
>> and pass it down to the DAI. But then again, the pxa-pcm-lib needs some
>> rework, as it's currently in charge of obtaining and releasing the channel.
> 
>> I wanted to postpone that issue until the rest of the series has settled
>> and limit changes to the bare minimum for now. Does that sound feasible?
> 
> Yup, the big problem here is the whole DT as ABI thing - what happens in
> the code isn't so important but the DT is supposed to be stable.  Or we
> could decide that this is one of the cases that's unstable in which case
> we should probably flag it up in the DT binding document if nothing else.
> 

Alright, I agreee. It's not much code that was needed to add support for
what I proposed, so I added a patch for that to my queue. Thanks for the
heads-up.


Daniel

Patch

diff --git a/Documentation/devicetree/bindings/sound/mrvl,pxa-ssp.txt b/Documentation/devicetree/bindings/sound/mrvl,pxa-ssp.txt
new file mode 100644
index 0000000..19d5488
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/mrvl,pxa-ssp.txt
@@ -0,0 +1,7 @@ 
+Marvell PXA SSP CPU DAI bindings
+
+Required properties:
+
+	compatible	Must be "mrvl,pxa-ssp-dai"
+
+
diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
index 6f4dd75..82b9985 100644
--- a/sound/soc/pxa/pxa-ssp.c
+++ b/sound/soc/pxa/pxa-ssp.c
@@ -21,6 +21,7 @@ 
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/pxa2xx_ssp.h>
+#include <linux/of.h>
 
 #include <asm/irq.h>
 
@@ -798,6 +799,12 @@  static const struct snd_soc_component_driver pxa_ssp_component = {
 	.name		= "pxa-ssp",
 };
 
+#ifdef CONFIG_OF
+static const struct of_device_id pxa_ssp_of_ids[] = {
+	{ .compatible = "mrvl,pxa-ssp-dai" },
+};
+#endif
+
 static int asoc_ssp_probe(struct platform_device *pdev)
 {
 	return snd_soc_register_component(&pdev->dev, &pxa_ssp_component,
@@ -812,8 +819,9 @@  static int asoc_ssp_remove(struct platform_device *pdev)
 
 static struct platform_driver asoc_ssp_driver = {
 	.driver = {
-			.name = "pxa-ssp-dai",
-			.owner = THIS_MODULE,
+		.name = "pxa-ssp-dai",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(pxa_ssp_of_ids),
 	},
 
 	.probe = asoc_ssp_probe,