[PATCH] ASoC: dmic codec: Support for DT

[PATCH] ASoC: dmic codec: Support for DT

[Peter Ujfalusi]

Add bindings to be able to probe the dmic codec driver via device tree.
The device tree entry of a machine driver can use phandle to provide
the device node for it's dai link structure.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 .../devicetree/bindings/sound/dmic-codec.txt       |   18 ++++++++++++++++++
 sound/soc/codecs/dmic.c                            |    6 ++++++
 2 files changed, 24 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/dmic-codec.txt

diff --git a/Documentation/devicetree/bindings/sound/dmic-codec.txt b/Documentation/devicetree/bindings/sound/dmic-codec.txt
new file mode 100644
index 0000000..f23f2e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/dmic-codec.txt
@@ -0,0 +1,18 @@
+Generic platform independent codec driver to represent digital microphones
+
+The dmic codec driver basically a dummy driver which exports the needed hooks
+to be used with digital microphone interfaces.
+
+Required properties:
+- compatible : Must be "generic,dmic-codec";
+
+Example:
+
+dmic_codec: dmic_codec@0 { /* generic DMIC codec */
+	compatible = "generic,dmic-codec";
+};
+
+sound { /* ASoC machine dirver */
+	...
+	dai-link,codec = <&dmic_codec>;
+};
diff --git a/sound/soc/codecs/dmic.c b/sound/soc/codecs/dmic.c
index 3e929f0..90e92f5 100644
--- a/sound/soc/codecs/dmic.c
+++ b/sound/soc/codecs/dmic.c
@@ -78,12 +78,18 @@ static int __devexit dmic_dev_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id dmic_of_match[] = {
+	{.compatible = "generic,dmic-codec", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, dmic_of_match);
 MODULE_ALIAS("platform:dmic-codec");
 
 static struct platform_driver dmic_driver = {
 	.driver = {
 		.name = "dmic-codec",
 		.owner = THIS_MODULE,
+		.of_match_table = dmic_of_match,
 	},
 	.probe = dmic_dev_probe,
 	.remove = __devexit_p(dmic_dev_remove),
-- 
1.7.8.6

Re: [PATCH] ASoC: dmic codec: Support for DT

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 482 bytes --]

On Tue, May 08, 2012 at 02:55:57PM +0300, Peter Ujfalusi wrote:

> +dmic_codec: dmic_codec@0 { /* generic DMIC codec */
> +	compatible = "generic,dmic-codec";
> +};

Why is this called "dmic-codec" and not "dmic"?  We probably need to
have a think about this in the context of the handling of random
on-board passive components in general - things like jacks and so on - 
is the most sensible thing to have a platform device for everything, or
is that getting too noisy?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: dmic codec: Support for DT

[Peter Ujfalusi]

On 05/08/2012 03:58 PM, Mark Brown wrote:
> On Tue, May 08, 2012 at 02:55:57PM +0300, Peter Ujfalusi wrote:
> 
>> +dmic_codec: dmic_codec@0 { /* generic DMIC codec */
>> +	compatible = "generic,dmic-codec";
>> +};
> 
> Why is this called "dmic-codec" and not "dmic"?

This is a really good question. Should have been "dmic".

> We probably need to have a think about this in the context of the
> handling of random on-board passive components in general - things
> like jacks and so on - is the most sensible thing to have a
> platform device for everything, or is that getting too noisy?

It is true that there's no real device (in a sense that it is not
programmable at least via the current dmic-codec.c driver) behind of the
dmic codec as such.
However in our setup it is a vital part of the audio setup since the CPU
dai (omap4-dmic) need to be connected to _somthing_ in order to have a link.
It could be argued that if the SW has no control over a component there
should not be dts section for it either. However when we move to the
audio machine driver level (or audio setup description) we are going to
have quiet different looking (I would say confusing) implementation
among devices, platforms.

For example tegra-seaboard have this for the link:

nvidia,i2s-controller = <&tegra_i2s1>;
nvidia,audio-codec = <&wm8903>;

It is clearly tells me that i2s1 is connected to wm8903 codec.

Now without dts entry for dmic codec on omap4:

dai-link,dai = <&dmic>;

In the background we create the device for the dmic at runtime but it
does not help the readability of the dts file. Especially if someone is
looking at the tegra version at the same type.

I would prefer to have something like this in our dts for audio support:

dai-link,dai = <&dmic>;
dai-link,codec = <&dmic_codec>;


I might be as well just plain wrong here.

-- 
Péter

Re: [PATCH] ASoC: dmic codec: Support for DT

[Peter Ujfalusi]

On 05/08/2012 03:58 PM, Mark Brown wrote:
> On Tue, May 08, 2012 at 02:55:57PM +0300, Peter Ujfalusi wrote:
> 
>> +dmic_codec: dmic_codec@0 { /* generic DMIC codec */
>> +	compatible = "generic,dmic-codec";
>> +};
> 
> Why is this called "dmic-codec" and not "dmic"?  We probably need to
> have a think about this in the context of the handling of random
> on-board passive components in general - things like jacks and so on - 
> is the most sensible thing to have a platform device for everything, or
> is that getting too noisy?

I don't think that we would need platform devices for most of the
passive components you have listed.
AFAIK (Liam can correct me if I'm wrong) the reason that we have this
(dmic codec) is to be able to use the OMAP4+ DMIC in a card.
In most cases the digital microphones are passive components connected
to a codec and the codec is then interfaced with the CPU dai.
With the OMAP4 DMIC we have the microphones connected directly to the
CPU dai. In some sense the OMAP4+ DMIC block is a CPU dai and a codec at
the same time. Or we can view it as we have PDM link betweent he CPU dai
and the codec (dmic).

Not entirely sure if we will need to have dts section for the dmic, I
can just create the platform device in the abe-twl6040 machine driver if
the setup includes digital microphones.

-- 
Péter

Re: [PATCH] ASoC: dmic codec: Support for DT

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1719 bytes --]

On Tue, May 15, 2012 at 02:44:35PM +0300, Peter Ujfalusi wrote:

> I don't think that we would need platform devices for most of the
> passive components you have listed.

I don't think we do either, but I think we should handle DMICs in a
similar way - we probably want to be able to get to a place where basic
devices that don't need complex clocking arrangements can be described
with the device tree in a generic, cross-device manner.  I think that's
going to mean that we need to be able to enumerate things like which
pins on the CODEC are actually in use and where the headset is which is
going to need at least some level of description of these passives.

For internal Linux purposes we might want to do something noticeably
different with the different devices but that is an orthogonal issue to
representing the hardware in a suitably abstract format.  I would be a
bit worried if a DMIC interface was substantially different to an AMIC
interface in DT since in terms of how they interface it's really not
that big a difference.

> AFAIK (Liam can correct me if I'm wrong) the reason that we have this
> (dmic codec) is to be able to use the OMAP4+ DMIC in a card.

Oh, I understand why it's there but this is more of a Linux internal
implementation issue than anything else.

> Not entirely sure if we will need to have dts section for the dmic, I
> can just create the platform device in the abe-twl6040 machine driver if
> the setup includes digital microphones.

That's another option and certainly seems like a good stopgap when we're
currently doing the same thing for analogue interfaces - I'm much more
comfortable with adding stuff later than with putting something that
I've got concerns about in.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]