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

[PATCH 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 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>
---
 Documentation/devicetree/bindings/sound/cs4270.txt |    5 +++++
 sound/soc/codecs/cs4270.c                          |   16 ++++++++++++++++
 2 files changed, 21 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..7d206be 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,24 @@ 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 way to bring the codec out of reset */
+	if (np) {
+		enum of_gpio_flags reset_gpio_flags;
+		int reset_gpio = of_get_named_gpio_flags(np, "reset-gpio", 0,
+							 &reset_gpio_flags);
+
+		if (devm_gpio_request_one(&i2c_client->dev, reset_gpio,
+				     reset_gpio_flags & OF_GPIO_ACTIVE_LOW ?
+					GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH,
+				     "cs4270 reset") < 0) {
+			reset_gpio = -EINVAL;
+		}
+	}
+
 	/* Verify that we have a CS4270 */
 
 	ret = i2c_smbus_read_byte_data(i2c_client, CS4270_CHIPID);
-- 
1.7.10.4

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

[Timur Tabi]

Daniel Mack wrote:

> +/*
> + * 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);
> +

Why is this necessary?  The CS4270 driver works just fine on our
device-tree enabled boards.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Daniel Mack]

On 24.07.2012 22:22, Timur Tabi wrote:
> Daniel Mack wrote:
> 
>> +/*
>> + * 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);
>> +
> 
> Why is this necessary?  The CS4270 driver works just fine on our
> device-tree enabled boards.
> 

How do you instanciate it then? Somehow the DT core must know which
device to probe for a node, right?



Daniel

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

[Timur Tabi]

Daniel Mack wrote:
> How do you instanciate it then? Somehow the DT core must know which
> device to probe for a node, right?

drivers/of/of_i2c.c

It scans the device tree looking for I2C devices and registers them.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Daniel Mack]

On 24.07.2012 22:30, Timur Tabi wrote:
> Daniel Mack wrote:
>> How do you instanciate it then? Somehow the DT core must know which
>> device to probe for a node, right?
> 
> drivers/of/of_i2c.c
> 
> It scans the device tree looking for I2C devices and registers them.
> 

Yes, I'm aware of this, I just wonder how does your DT note looks like?

And are you ok with the 2nd patch?


Daniel

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

[Timur Tabi]

Daniel Mack wrote:
> Yes, I'm aware of this, I just wonder how does your DT note looks like?

Take a look at arch/powerpc/boot/dts/mpc8610_hpcd.dts

> And are you ok with the 2nd patch?

Isn't that something that should be done in platform code?  It seems very
platform-specific for a codec driver.  I have no problem using the CS4270
on my board, and I don't need this feature.

> +	/* See if we way to bring the codec out of reset */
> +	if (np) {
> +		enum of_gpio_flags reset_gpio_flags;

Blank line after variable declarations

> +		int reset_gpio = of_get_named_gpio_flags(np, "reset-gpio", 0,
> +							 &reset_gpio_flags);

Can you make this line shorter by using shorter variable names or something?

> +		if (devm_gpio_request_one(&i2c_client->dev, reset_gpio,
> +				     reset_gpio_flags & OF_GPIO_ACTIVE_LOW ?
> +					GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH,
> +				     "cs4270 reset") < 0) {
> +			reset_gpio = -EINVAL;
> +		}

I don't see where you test whether the reset-gpio property is present.  It
won't be present in my device tree.

> +	}
> +

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Daniel Mack]

On 24.07.2012 22:39, Timur Tabi wrote:
> Daniel Mack wrote:
>> Yes, I'm aware of this, I just wonder how does your DT note looks like?
> 
> Take a look at arch/powerpc/boot/dts/mpc8610_hpcd.dts

Interesting, I didn't know that works without enabling the specific
string inside the driver. And that makes me wonder why many of the
Wolfson codec have them (like wm8580). I might lack something here,
maybe Mark can explain?

>> And are you ok with the 2nd patch?
> 
> Isn't that something that should be done in platform code?  It seems very
> platform-specific for a codec driver.  

Sure, the code I have at the moment does it that way, but the idea is to
have little to no platform specific code. And also, if anything in the
system needs to know when and how to drive the reset line, it should be
the driver, right?

> I have no problem using the CS4270
> on my board, and I don't need this feature.

Because you care for that either in the bootloader or the platform code
I believe?

> 
>> +	/* See if we way to bring the codec out of reset */
>> +	if (np) {
>> +		enum of_gpio_flags reset_gpio_flags;
> 
> Blank line after variable declarations
> 
>> +		int reset_gpio = of_get_named_gpio_flags(np, "reset-gpio", 0,
>> +							 &reset_gpio_flags);
> 
> Can you make this line shorter by using shorter variable names or something?

Right, fixed.

>> +		if (devm_gpio_request_one(&i2c_client->dev, reset_gpio,
>> +				     reset_gpio_flags & OF_GPIO_ACTIVE_LOW ?
>> +					GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH,
>> +				     "cs4270 reset") < 0) {
>> +			reset_gpio = -EINVAL;
>> +		}
> 
> I don't see where you test whether the reset-gpio property is present.  It
> won't be present in my device tree.

The idea is that reset_gpio will be < 0 in that case, and so
devm_gpio_request_one() fails. So your platform should be ok and no
further checks are necessary.


Thanks for the review,
Daniel

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

[Timur Tabi]

Daniel Mack wrote:

> Sure, the code I have at the moment does it that way, but the idea is to
> have little to no platform specific code. And also, if anything in the
> system needs to know when and how to drive the reset line, it should be
> the driver, right?

Maybe.  We do have a lot of similar code in the boot loader, but of course
that means that it's a static configuration.

>> I have no problem using the CS4270
>> on my board, and I don't need this feature.
> 
> Because you care for that either in the bootloader or the platform code
> I believe?

I don't have any platform code that initializes the codec.

Of course, that doesn't mean that you shouldn't need any.  I'm not
fundamentally opposed to your patch, I just don't have any context to go by.

Also, I have a gut feeling that if someone else needs to do the same
thing, then this code:

devm_gpio_request_one(&i2c_client->dev, reset_gpio,
	reset_gpio_flags & OF_GPIO_ACTIVE_LOW ?
	GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH,
	"cs4270 reset")

won't work for him, because it's not generic enough.

>> I don't see where you test whether the reset-gpio property is present.  It
>> won't be present in my device tree.
> 
> The idea is that reset_gpio will be < 0 in that case, and so
> devm_gpio_request_one() fails. So your platform should be ok and no
> further checks are necessary.

Well, I would prefer that you check for the property before making any
calls that use it.

Also, I just noticed a typo here:

	/* See if we way to bring the codec out of reset */

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Stephen Warren]

On 07/24/2012 02:30 PM, Timur Tabi wrote:
> Daniel Mack wrote:
>> How do you instanciate it then? Somehow the DT core must know which
>> device to probe for a node, right?
> 
> drivers/of/of_i2c.c
> 
> It scans the device tree looking for I2C devices and registers them.

The I2C code matches the compatible values from the device tree against
two tables in the kernel. First, the of_match_table that this patch
adds, and then if there's no match it falls back to the I2C device ID
table, and matches that against the DT compatible value with the vendor
prefix stripped off.

So while the second method does work as a fall-back, in the past I've
received guidance (I think from Grant Likely) that we should still add
the of_match_table to drivers in order to be explicit.

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

[Stephen Warren]

On 07/24/2012 02:39 PM, Timur Tabi wrote:
> Daniel Mack wrote:
>> Yes, I'm aware of this, I just wonder how does your DT note looks like?
> 
> Take a look at arch/powerpc/boot/dts/mpc8610_hpcd.dts
> 
>> And are you ok with the 2nd patch?
> 
> Isn't that something that should be done in platform code?  It seems very
> platform-specific for a codec driver.  I have no problem using the CS4270
> on my board, and I don't need this feature.

Perhaps your board permanently ties the reset line to de-asserted, or
the bootloader/... deasserts the codecs reset pin before the kernel boots?

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

[Timur Tabi]

Stephen Warren wrote:
> Perhaps your board permanently ties the reset line to de-asserted, or
> the bootloader/... deasserts the codecs reset pin before the kernel boots?

Probably.  We have a pretty sophisticate FPGA on our reference boards that
handles a lot of stuff like this.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Timur Tabi]

Stephen Warren wrote:
> The I2C code matches the compatible values from the device tree against
> two tables in the kernel. First, the of_match_table that this patch
> adds, and then if there's no match it falls back to the I2C device ID
> table, and matches that against the DT compatible value with the vendor
> prefix stripped off.
> 
> So while the second method does work as a fall-back, in the past I've
> received guidance (I think from Grant Likely) that we should still add
> the of_match_table to drivers in order to be explicit.

So you're saying that all I2C device drivers should have an
of_match_table()?  If so, then we should probably fix all codec drivers in
one patch.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Mark Brown]

On Tue, Jul 24, 2012 at 03:39:21PM -0500, Timur Tabi wrote:
> Daniel Mack wrote:

> > And are you ok with the 2nd patch?

...which requires the first one anyway.

> Isn't that something that should be done in platform code?  It seems very
> platform-specific for a codec driver.  I have no problem using the CS4270
> on my board, and I don't need this feature.

Using a reset signal on the CODEC isn't at all platform specific - it's
something the device itself supports so I'd expect the driver for the
device to support it so it can be joined up with it for register I/O and
so on, besides there's not really anywhere else to put it in the DT.

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

[Mark Brown]

On Tue, Jul 24, 2012 at 05:02:35PM -0500, Timur Tabi wrote:
> Stephen Warren wrote:

> > Perhaps your board permanently ties the reset line to de-asserted, or
> > the bootloader/... deasserts the codecs reset pin before the kernel boots?

> Probably.  We have a pretty sophisticate FPGA on our reference boards that
> handles a lot of stuff like this.

More likely it's just tied to whichever of ground or a supply holds the
device out of reset, if it's the FPGA I'd expect you to have runtime
control of it.

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

[Mark Brown]

On Tue, Jul 24, 2012 at 05:03:35PM -0500, Timur Tabi wrote:
> Stephen Warren wrote:
> > The I2C code matches the compatible values from the device tree against
> > two tables in the kernel. First, the of_match_table that this patch
> > adds, and then if there's no match it falls back to the I2C device ID
> > table, and matches that against the DT compatible value with the vendor
> > prefix stripped off.

> > So while the second method does work as a fall-back, in the past I've
> > received guidance (I think from Grant Likely) that we should still add
> > the of_match_table to drivers in order to be explicit.

> So you're saying that all I2C device drivers should have an

It's certainly better to add one, it avoids any ambiguity in part
numbers (there's at least one other chip vendor (Wondermedia) using wm
for example).

> of_match_table()?  If so, then we should probably fix all codec drivers in
> one patch.

Meh, it's fine doing it piecemeal really.  It's hardly urgent anyway.

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

[Daniel Mack]

On 25.07.2012 00:44, Mark Brown wrote:
> On Tue, Jul 24, 2012 at 05:03:35PM -0500, Timur Tabi wrote:
>> Stephen Warren wrote:
>>> The I2C code matches the compatible values from the device tree against
>>> two tables in the kernel. First, the of_match_table that this patch
>>> adds, and then if there's no match it falls back to the I2C device ID
>>> table, and matches that against the DT compatible value with the vendor
>>> prefix stripped off.
> 
>>> So while the second method does work as a fall-back, in the past I've
>>> received guidance (I think from Grant Likely) that we should still add
>>> the of_match_table to drivers in order to be explicit.
> 
>> So you're saying that all I2C device drivers should have an
> 
> It's certainly better to add one, it avoids any ambiguity in part
> numbers (there's at least one other chip vendor (Wondermedia) using wm
> for example).
> 
>> of_match_table()?  If so, then we should probably fix all codec drivers in
>> one patch.
> 
> Meh, it's fine doing it piecemeal really.  It's hardly urgent anyway.

Ok, thanks for all the explanation. So I'll repost a cleaned up version
of the 2nd patch then. Timur, are you ok with that?


Daniel

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

[Daniel Mack]

On 24.07.2012 23:01, Timur Tabi wrote:

> Also, I have a gut feeling that if someone else needs to do the same
> thing, then this code:
> 
> devm_gpio_request_one(&i2c_client->dev, reset_gpio,
> 	reset_gpio_flags & OF_GPIO_ACTIVE_LOW ?
> 	GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH,
> 	"cs4270 reset")
> 
> won't work for him, because it's not generic enough.

Why is that? The GPIO in the spec can be located anywhere on the board,
either directly driven by the SoC or by any arbitrary other part on on
the board that exposes itself as GPIO chip. And with the ACTIVE_LOW
flag, it's even possible to compensate external circuitry that changes
the polarity.

Which case can you imagine where that flexibility wouldn't suffice?


Daniel

[PATCH 2/2 v2] 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>
---
 Documentation/devicetree/bindings/sound/cs4270.txt |    5 +++++
 sound/soc/codecs/cs4270.c                          |   14 ++++++++++++++
 2 files changed, 19 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..1b03058 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,22 @@ 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))
+			devm_gpio_request_one(&i2c_client->dev, gpio,
+				     flags & OF_GPIO_ACTIVE_LOW ?
+					GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH,
+				     "cs4270 reset");
+	}
+
 	/* Verify that we have a CS4270 */
 
 	ret = i2c_smbus_read_byte_data(i2c_client, CS4270_CHIPID);
-- 
1.7.10.4

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

[Mark Brown]

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

On Wed, Jul 25, 2012 at 09:03:29AM +0200, Daniel Mack wrote:
> 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.

I'm missing 1/2...  Please also don't bury patches in the middle of
previous threads.

> +		enum of_gpio_flags flags;
> +		int gpio = of_get_named_gpio_flags(np, "reset-gpio", 0, &flags);
> +
> +		if (gpio_is_valid(gpio))
> +			devm_gpio_request_one(&i2c_client->dev, gpio,
> +				     flags & OF_GPIO_ACTIVE_LOW ?
> +					GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH,
> +				     "cs4270 reset");

This ignores the return code and won't work well with probe deferral, if
we manage to get a GPIO from the DT then we should fail if we're unable
request it.  Passing back the return code should get you deferral
support for free in 3.6 and onwards.

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

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

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

[Daniel Mack]

On 25.07.2012 15:18, Mark Brown wrote:
> On Wed, Jul 25, 2012 at 09:03:29AM +0200, Daniel Mack wrote:
>> 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.
> 
> I'm missing 1/2...  Please also don't bury patches in the middle of
> previous threads.
> 
>> +		enum of_gpio_flags flags;
>> +		int gpio = of_get_named_gpio_flags(np, "reset-gpio", 0, &flags);
>> +
>> +		if (gpio_is_valid(gpio))
>> +			devm_gpio_request_one(&i2c_client->dev, gpio,
>> +				     flags & OF_GPIO_ACTIVE_LOW ?
>> +					GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH,
>> +				     "cs4270 reset");
> 
> This ignores the return code and won't work well with probe deferral, if
> we manage to get a GPIO from the DT then we should fail if we're unable
> request it.  Passing back the return code should get you deferral
> support for free in 3.6 and onwards.
> 

Ok, will resent both patches.