[PATCH v3 1/2] ALSA: ASoC: add DT bindings for cs4270

[PATCH v3 1/2] ALSA: ASoC: add DT bindings for cs4270

[Daniel Mack]

Signed-off-by: Daniel Mack <zonque@gmail.com>
Cc: Timur Tabi <timur@freescale.com>
---
 Documentation/devicetree/bindings/sound/cs4270.txt |   16 ++++++++++++++++
 sound/soc/codecs/cs4270.c                          |   11 +++++++++++
 2 files changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/cs4270.txt

diff --git a/Documentation/devicetree/bindings/sound/cs4270.txt b/Documentation/devicetree/bindings/sound/cs4270.txt
new file mode 100644
index 0000000..7f0bfd8
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/cs4270.txt
@@ -0,0 +1,16 @@
+CS4270 audio CODEC
+
+The driver for this device currently only supports I2C.
+
+Required properties:
+
+  - compatible : "cirrus,cs4270"
+
+  - reg : the I2C address of the device for I2C
+
+Example:
+
+codec: cs4270@48 {
+	compatible = "cirrus,cs4270";
+	reg = <0x48>;
+};
diff --git a/sound/soc/codecs/cs4270.c b/sound/soc/codecs/cs4270.c
index 047917f..4b71b01 100644
--- a/sound/soc/codecs/cs4270.c
+++ b/sound/soc/codecs/cs4270.c
@@ -29,6 +29,7 @@
 #include <linux/i2c.h>
 #include <linux/delay.h>
 #include <linux/regulator/consumer.h>
+#include <linux/of_device.h>
 
 /*
  * The codec isn't really big-endian or little-endian, since the I2S
@@ -639,6 +640,15 @@ static const struct snd_soc_codec_driver soc_codec_device_cs4270 = {
 	.reg_cache_default =	cs4270_default_reg_cache,
 };
 
+/*
+ * cs4270_of_match - the device tree bindings
+ */
+static const struct of_device_id cs4270_of_match[] = {
+	{ .compatible = "cirrus,cs4270", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, cs4270_of_match);
+
 /**
  * cs4270_i2c_probe - initialize the I2C interface of the CS4270
  * @i2c_client: the I2C client object
@@ -718,6 +728,7 @@ static struct i2c_driver cs4270_i2c_driver = {
 	.driver = {
 		.name = "cs4270",
 		.owner = THIS_MODULE,
+		.of_match_table = cs4270_of_match,
 	},
 	.id_table = cs4270_id,
 	.probe = cs4270_i2c_probe,
-- 
1.7.10.4

[PATCH v3 2/2] ALSA: ASoC: Add reset-gpio DT property to cs4270 driver

[Daniel Mack]

In the process of moving over from static board files to the device
tree, reset pins of peripheral reset pins should be handled by their
corresponding drivers.

Add a reset-gpio DT property to the cs4270 driver, and de-assert it
before probing the chip. The logic could be augmented some day to
re-assert it when codec is put to suspend.

Signed-off-by: Daniel Mack <zonque@gmail.com>
Cc: Timur Tabi <timur@freescale.com>
---
 v2: cleanups as requested by Timur
 v3: return error code in case devm_gpio_request_one() fails

 Documentation/devicetree/bindings/sound/cs4270.txt |    5 +++++
 sound/soc/codecs/cs4270.c                          |   17 +++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/cs4270.txt b/Documentation/devicetree/bindings/sound/cs4270.txt
index 7f0bfd8..6b222f9 100644
--- a/Documentation/devicetree/bindings/sound/cs4270.txt
+++ b/Documentation/devicetree/bindings/sound/cs4270.txt
@@ -8,6 +8,11 @@ Required properties:
 
   - reg : the I2C address of the device for I2C
 
+Optional properties:
+
+  - reset-gpio : a GPIO spec for the reset pin. If specified, it will be
+		 deasserted before communication to the codec starts.
+
 Example:
 
 codec: cs4270@48 {
diff --git a/sound/soc/codecs/cs4270.c b/sound/soc/codecs/cs4270.c
index 4b71b01..fd11bb6 100644
--- a/sound/soc/codecs/cs4270.c
+++ b/sound/soc/codecs/cs4270.c
@@ -30,6 +30,7 @@
 #include <linux/delay.h>
 #include <linux/regulator/consumer.h>
 #include <linux/of_device.h>
+#include <linux/of_gpio.h>
 
 /*
  * The codec isn't really big-endian or little-endian, since the I2S
@@ -660,9 +661,25 @@ MODULE_DEVICE_TABLE(of, cs4270_of_match);
 static int cs4270_i2c_probe(struct i2c_client *i2c_client,
 	const struct i2c_device_id *id)
 {
+	struct device_node *np = i2c_client->dev.of_node;
 	struct cs4270_private *cs4270;
 	int ret;
 
+	/* See if we have a way to bring the codec out of reset */
+	if (np) {
+		enum of_gpio_flags flags;
+		int gpio = of_get_named_gpio_flags(np, "reset-gpio", 0, &flags);
+
+		if (gpio_is_valid(gpio)) {
+			ret = devm_gpio_request_one(&i2c_client->dev, gpio,
+				     flags & OF_GPIO_ACTIVE_LOW ?
+					GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH,
+				     "cs4270 reset");
+			if (ret < 0)
+				return ret;
+		}
+	}
+
 	/* Verify that we have a CS4270 */
 
 	ret = i2c_smbus_read_byte_data(i2c_client, CS4270_CHIPID);
-- 
1.7.10.4

Re: [PATCH v3 1/2] ALSA: ASoC: add DT bindings for cs4270

[Timur Tabi]

Daniel Mack wrote:
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> Cc: Timur Tabi <timur@freescale.com>

Please give me a couple days to get around to testing these patches.  They
look okay, but I need to make sure they don't break PowerPC.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH v3 1/2] ALSA: ASoC: add DT bindings for cs4270

[Timur Tabi]

Daniel Mack wrote:
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> Cc: Timur Tabi <timur@freescale.com>
> ---

Both patches:

Acked-by: Timur Tabi <timur@freescale.com>

I am seeing something a little weird, though, although I doubt it's
related to these patches.  Audio works, even though I see this on the boot
log:

cs4270 0-004f: found device at i2c address 4F
cs4270 0-004f: hardware revision 3
Freescale MPC8610 HPCD ALSA SoC machine driver
soc-audio soc-audio: ASoC machine snd-soc-mpc8610hpcd should use
snd_soc_register_card()
soc-audio soc-audio: Failed to register card
platform soc-audio: Driver soc-audio requests probe deferral
...
ALSA device list:
  No soundcards found.
soc-audio soc-audio: ASoC machine snd-soc-mpc8610hpcd should use
snd_soc_register_card()
soc-audio soc-audio:  cs4270-hifi <-> e0016000.ssi mapping ok
soc-audio soc-audio:  cs4270-hifi <-> e0016000.ssi mapping ok

Is there a connection between the message "ASoC machine
snd-soc-mpc8610hpcd should use snd_soc_register_card()", the requests for
probe deferral, and the "No soundcards found." message?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH v3 1/2] ALSA: ASoC: add DT bindings for cs4270

[Daniel Mack]

On 30.07.2012 22:42, Timur Tabi wrote:
> Daniel Mack wrote:
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> Cc: Timur Tabi <timur@freescale.com>
>> ---
> 
> Both patches:
> 
> Acked-by: Timur Tabi <timur@freescale.com>

Thanks.

> I am seeing something a little weird, though, although I doubt it's
> related to these patches.  Audio works, even though I see this on the boot
> log:

I also fail to see a relation here. Does it happen without the two
patches as well?


Daniel


> 
> cs4270 0-004f: found device at i2c address 4F
> cs4270 0-004f: hardware revision 3
> Freescale MPC8610 HPCD ALSA SoC machine driver
> soc-audio soc-audio: ASoC machine snd-soc-mpc8610hpcd should use
> snd_soc_register_card()
> soc-audio soc-audio: Failed to register card
> platform soc-audio: Driver soc-audio requests probe deferral
> ...
> ALSA device list:
>   No soundcards found.
> soc-audio soc-audio: ASoC machine snd-soc-mpc8610hpcd should use
> snd_soc_register_card()
> soc-audio soc-audio:  cs4270-hifi <-> e0016000.ssi mapping ok
> soc-audio soc-audio:  cs4270-hifi <-> e0016000.ssi mapping ok
> 
> Is there a connection between the message "ASoC machine
> snd-soc-mpc8610hpcd should use snd_soc_register_card()", the requests for
> probe deferral, and the "No soundcards found." message?
> 

Re: [PATCH v3 1/2] ALSA: ASoC: add DT bindings for cs4270

[Timur Tabi]

Daniel Mack wrote:
> I also fail to see a relation here. Does it happen without the two
> patches as well?

Yes, it does happen without the patches, but I was wondering if changing
the way the codec driver is probed would be relevant somehow.  I know it's
a stretch.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH v3 1/2] ALSA: ASoC: add DT bindings for cs4270

[Hebbar, Gururaja]

On Tue, Jul 31, 2012 at 02:12:31, Timur Tabi wrote:
> Daniel Mack wrote:
> > Signed-off-by: Daniel Mack <zonque@gmail.com>
> > Cc: Timur Tabi <timur@freescale.com>
> > ---
> 
> Both patches:
> 
> Acked-by: Timur Tabi <timur@freescale.com>
> 
> I am seeing something a little weird, though, although I doubt it's
> related to these patches.  Audio works, even though I see this on the boot
> log:
> 
> cs4270 0-004f: found device at i2c address 4F
> cs4270 0-004f: hardware revision 3
> Freescale MPC8610 HPCD ALSA SoC machine driver
> soc-audio soc-audio: ASoC machine snd-soc-mpc8610hpcd should use
> snd_soc_register_card()
> soc-audio soc-audio: Failed to register card
> platform soc-audio: Driver soc-audio requests probe deferral
> ...
> ALSA device list:
>   No soundcards found.
> soc-audio soc-audio: ASoC machine snd-soc-mpc8610hpcd should use
> snd_soc_register_card()

I believe you are registering "soc-audio" as a device.
 
In the "soc-audio" driver probe (souns/soc/soc-core.c - soc_probe()), 
this message is printed.

This is as per the commit below

commit fe4085e84f17a57a533a210a626e0cc9ead381f9
Author:     Mark Brown <broonie@opensource.wolfsonmicro.com>
AuthorDate: Fri Mar 2 13:07:41 2012 +0000
Commit:     Mark Brown <broonie@opensource.wolfsonmicro.com>
CommitDate: Fri Mar 2 16:21:25 2012 +0000

    ASoC: core: Log a warning when machines use soc-audio

    snd_soc_register_card() has been available and strongly preferred since
    2.6.38 but we're still seeing new drivers using it and the conversion rate
    for older machines has been low. Help address both issues by logging a
    warning when the soc-audio device probes.

    Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
    Acked-by: Liam Girdwood <lrg@ti.com>


> soc-audio soc-audio:  cs4270-hifi <-> e0016000.ssi mapping ok
> soc-audio soc-audio:  cs4270-hifi <-> e0016000.ssi mapping ok
> 
> Is there a connection between the message "ASoC machine
> snd-soc-mpc8610hpcd should use snd_soc_register_card()", the requests for
> probe deferral, and the "No soundcards found." message?
> 
> -- 
> Timur Tabi
> Linux kernel developer at Freescale
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 


Regards, 
Gururaja

Re: [PATCH v3 1/2] ALSA: ASoC: add DT bindings for cs4270

[Mark Brown]

On Mon, Jul 30, 2012 at 03:42:31PM -0500, Timur Tabi wrote:

> Is there a connection between the message "ASoC machine
> snd-soc-mpc8610hpcd should use snd_soc_register_card()", the requests for
> probe deferral, and the "No soundcards found." message?

Probe deferral is related to the "No soundcards found" since the
deferred probe runs start after the init completes.  The lack of an
update to snd_soc_register_card() isn't related.

Re: [PATCH v3 1/2] ALSA: ASoC: add DT bindings for cs4270

[Timur Tabi]

Mark Brown wrote:
> Probe deferral is related to the "No soundcards found" since the
> deferred probe runs start after the init completes. 

Is there anything I can do to fix this?

> The lack of an
> update to snd_soc_register_card() isn't related.

Are you planning on removing support for soc-audio (thereby forcing me to
update to snd_soc_register_card), or can I just ignore the warning?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH v3 1/2] ALSA: ASoC: add DT bindings for cs4270

[Mark Brown]

On Tue, Jul 31, 2012 at 09:49:48AM -0500, Timur Tabi wrote:
> Mark Brown wrote:

> > Probe deferral is related to the "No soundcards found" since the
> > deferred probe runs start after the init completes. 

> Is there anything I can do to fix this?

It's not a problem.  If you ensure that all the dependencies for the
card are registered before the card it won't happen but that'll depend
on the order people have in their DT.

> > The lack of an
> > update to snd_soc_register_card() isn't related.

> Are you planning on removing support for soc-audio (thereby forcing me to
> update to snd_soc_register_card), or can I just ignore the warning?

You should update.

Re: [PATCH v3 1/2] ALSA: ASoC: add DT bindings for cs4270

[Timur Tabi]

Mark Brown wrote:

> It's not a problem.  If you ensure that all the dependencies for the
> card are registered before the card it won't happen but that'll depend
> on the order people have in their DT.

Well, it may not be a real problem, but it is disconcerting.  I can
imagine a bunch of bogus bug reports coming my way because people think
the drivers have not loaded properly.

>>> The lack of an
>>> update to snd_soc_register_card() isn't related.
> 
>> Are you planning on removing support for soc-audio (thereby forcing me to
>> update to snd_soc_register_card), or can I just ignore the warning?
> 
> You should update.

I know, but is it urgent that I update?  How much time do I have?  If
you're telling me that you're going to yank it in 3.7, then I need to
re-prioritize my work.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH v3 1/2] ALSA: ASoC: add DT bindings for cs4270

[Mark Brown]

On Wed, Jul 25, 2012 at 03:28:34PM +0200, Daniel Mack wrote:
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> Cc: Timur Tabi <timur@freescale.com>

Applied both, thanks.

Re: [PATCH v3 1/2] ALSA: ASoC: add DT bindings for cs4270

[Mark Brown]

On Tue, Jul 31, 2012 at 11:02:48AM -0500, Timur Tabi wrote:
> Mark Brown wrote:

> > It's not a problem.  If you ensure that all the dependencies for the
> > card are registered before the card it won't happen but that'll depend
> > on the order people have in their DT.

> Well, it may not be a real problem, but it is disconcerting.  I can
> imagine a bunch of bogus bug reports coming my way because people think
> the drivers have not loaded properly.

Yeah.  TBH I think the best fix is to remove the logging of the cards
rather than anything else, make the core announce each card as it
appears if it's useful.

> >> Are you planning on removing support for soc-audio (thereby forcing me to
> >> update to snd_soc_register_card), or can I just ignore the warning?

> > You should update.

> I know, but is it urgent that I update?  How much time do I have?  If
> you're telling me that you're going to yank it in 3.7, then I need to
> re-prioritize my work.

Not that urgent, no - conversion won't be forced until it's blocking
something in the framework and that's not likely to happen any time
soon.