* [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR
@ 2009-05-05 9:25 Daniel Mack
2009-05-05 9:25 ` [PATCH 2/2] ASoC: cs4270: add power management support Daniel Mack
2009-05-05 14:51 ` [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR Timur Tabi
0 siblings, 2 replies; 15+ messages in thread
From: Daniel Mack @ 2009-05-05 9:25 UTC (permalink / raw)
To: alsa-devel; +Cc: Timur Tabi, Mark Brown
Replace the magic 0x80 value with a suitable macro definition.
Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: Timur Tabi <timur@freescale.com>
Cc: Mark Brown <broonie@sirena.org.uk>
---
sound/soc/codecs/cs4270.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/cs4270.c b/sound/soc/codecs/cs4270.c
index 0f453a0..bc338df 100644
--- a/sound/soc/codecs/cs4270.c
+++ b/sound/soc/codecs/cs4270.c
@@ -56,6 +56,7 @@
#define CS4270_FIRSTREG 0x01
#define CS4270_LASTREG 0x08
#define CS4270_NUMREGS (CS4270_LASTREG - CS4270_FIRSTREG + 1)
+#define CS4270_I2C_INCR 0x80
/* Bit masks for the CS4270 registers */
#define CS4270_CHIPID_ID 0xF0
@@ -296,7 +297,7 @@ static int cs4270_fill_cache(struct snd_soc_codec *codec)
s32 length;
length = i2c_smbus_read_i2c_block_data(i2c_client,
- CS4270_FIRSTREG | 0x80, CS4270_NUMREGS, cache);
+ CS4270_FIRSTREG | CS4270_I2C_INCR, CS4270_NUMREGS, cache);
if (length != CS4270_NUMREGS) {
dev_err(codec->dev, "i2c read failure, addr=0x%x\n",
--
1.6.2.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] ASoC: cs4270: add power management support
2009-05-05 9:25 [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR Daniel Mack
@ 2009-05-05 9:25 ` Daniel Mack
2009-05-05 15:30 ` Timur Tabi
2009-05-05 14:51 ` [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR Timur Tabi
1 sibling, 1 reply; 15+ messages in thread
From: Daniel Mack @ 2009-05-05 9:25 UTC (permalink / raw)
To: alsa-devel; +Cc: Timur Tabi, Mark Brown
Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: Timur Tabi <timur@freescale.com>
Cc: Mark Brown <broonie@sirena.org.uk>
---
sound/soc/codecs/cs4270.c | 61 ++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 60 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/cs4270.c b/sound/soc/codecs/cs4270.c
index bc338df..80c7282 100644
--- a/sound/soc/codecs/cs4270.c
+++ b/sound/soc/codecs/cs4270.c
@@ -18,7 +18,7 @@
* - The machine driver's 'startup' function must call
* cs4270_set_dai_sysclk() with the value of MCLK.
* - Only I2S and left-justified modes are supported
- * - Power management is not supported
+ * - Power management is supported
*/
#include <linux/module.h>
@@ -27,6 +27,7 @@
#include <sound/soc.h>
#include <sound/initval.h>
#include <linux/i2c.h>
+#include <linux/delay.h>
#include "cs4270.h"
@@ -65,6 +66,8 @@
#define CS4270_PWRCTL_PDN_ADC 0x20
#define CS4270_PWRCTL_PDN_DAC 0x02
#define CS4270_PWRCTL_PDN 0x01
+#define CS4270_PWRCTL_PDN_ALL \
+ (CS4270_PWRCTL_PDN_ADC | CS4270_PWRCTL_PDN_DAC | CS4270_PWRCTL_PDN)
#define CS4270_MODE_SPEED_MASK 0x30
#define CS4270_MODE_1X 0x00
#define CS4270_MODE_2X 0x10
@@ -788,6 +791,60 @@ static struct i2c_device_id cs4270_id[] = {
};
MODULE_DEVICE_TABLE(i2c, cs4270_id);
+#ifdef CONFIG_PM
+
+/* This suspend/resume implementation can handle both - a simple standby
+ * where the codec remains powered, and a full suspend, where the voltage
+ * domain the codec is connected to is teared down and/or any other hardware
+ * reset condition is asserted.
+ *
+ * The codec's own power saving features are enabled in the suspend callback,
+ * and all registers are written back to the hardware when resuming.
+ */
+
+static int cs4270_i2c_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+ struct cs4270_private *cs4270 = i2c_get_clientdata(client);
+ struct snd_soc_codec *codec = &cs4270->codec;
+ int reg = snd_soc_read(codec, CS4270_PWRCTL);
+
+ if (reg < 0)
+ return reg;
+
+ reg |= CS4270_PWRCTL_PDN_ALL;
+ return snd_soc_write(codec, CS4270_PWRCTL, reg);
+}
+
+static int cs4270_i2c_resume(struct i2c_client *client)
+{
+ struct cs4270_private *cs4270 = i2c_get_clientdata(client);
+ struct snd_soc_codec *codec = &cs4270->codec;
+ int reg;
+
+ /* In case the device was put to hard reset during sleep, we need to
+ * wait 500ns here before any I2C communication. */
+ ndelay(500);
+
+ /* first restore the entire register cache ... */
+ for (reg = CS4270_FIRSTREG; reg <= CS4270_LASTREG; reg++) {
+ u8 val = snd_soc_read(codec, reg);
+
+ if (i2c_smbus_write_byte_data(client, reg, val)) {
+ dev_err(codec->dev, "i2c write failed\n");
+ return -EIO;
+ }
+ }
+
+ /* ... then disable the power-down bits */
+ reg = snd_soc_read(codec, CS4270_PWRCTL);
+ reg &= ~CS4270_PWRCTL_PDN_ALL;
+ return snd_soc_write(codec, CS4270_PWRCTL, reg);
+}
+#else
+#define cs4270_i2c_suspend NULL
+#define cs4270_i2c_resume NULL
+#endif /* CONFIG_PM */
+
/*
* cs4270_i2c_driver - I2C device identification
*
@@ -802,6 +859,8 @@ static struct i2c_driver cs4270_i2c_driver = {
.id_table = cs4270_id,
.probe = cs4270_i2c_probe,
.remove = cs4270_i2c_remove,
+ .suspend = cs4270_i2c_suspend,
+ .resume = cs4270_i2c_resume,
};
/*
--
1.6.2.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR
2009-05-05 9:25 [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR Daniel Mack
2009-05-05 9:25 ` [PATCH 2/2] ASoC: cs4270: add power management support Daniel Mack
@ 2009-05-05 14:51 ` Timur Tabi
2009-05-05 15:15 ` Daniel Mack
1 sibling, 1 reply; 15+ messages in thread
From: Timur Tabi @ 2009-05-05 14:51 UTC (permalink / raw)
To: Daniel Mack; +Cc: alsa-devel, Mark Brown
Daniel Mack wrote:
> Replace the magic 0x80 value with a suitable macro definition.
>
> Signed-off-by: Daniel Mack <daniel@caiaq.de>
> Cc: Timur Tabi <timur@freescale.com>
> Cc: Mark Brown <broonie@sirena.org.uk>
And I thought *I* was anal-retentive! :-)
Acked-by: Timur Tabi <timur@freescale.com>
(although personally, I would have called it CS4270_I2C_AUTO_INCR)
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR
2009-05-05 14:51 ` [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR Timur Tabi
@ 2009-05-05 15:15 ` Daniel Mack
2009-05-05 18:09 ` Mark Brown
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Mack @ 2009-05-05 15:15 UTC (permalink / raw)
To: Timur Tabi; +Cc: alsa-devel, Mark Brown
On Tue, May 05, 2009 at 09:51:47AM -0500, Timur Tabi wrote:
> Daniel Mack wrote:
> > Replace the magic 0x80 value with a suitable macro definition.
> >
> > Signed-off-by: Daniel Mack <daniel@caiaq.de>
> > Cc: Timur Tabi <timur@freescale.com>
> > Cc: Mark Brown <broonie@sirena.org.uk>
>
> And I thought *I* was anal-retentive! :-)
Erm, yes - need to explain that :)
I planned to use that macro a second time for the register write-back at
resume time. But unfortunately, the i2c stack is so badly broken that it
does not allow the write of a whole data block without having the length
itself as part of the message (which is wrong for the codec).
But the change was there already by then, and so I left the patch in my
queue.
Daniel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] ASoC: cs4270: add power management support
2009-05-05 9:25 ` [PATCH 2/2] ASoC: cs4270: add power management support Daniel Mack
@ 2009-05-05 15:30 ` Timur Tabi
2009-05-05 15:51 ` Daniel Mack
0 siblings, 1 reply; 15+ messages in thread
From: Timur Tabi @ 2009-05-05 15:30 UTC (permalink / raw)
To: Daniel Mack; +Cc: alsa-devel, Mark Brown
Daniel Mack wrote:
> +static int cs4270_i2c_suspend(struct i2c_client *client, pm_message_t mesg)
> +{
> + struct cs4270_private *cs4270 = i2c_get_clientdata(client);
> + struct snd_soc_codec *codec = &cs4270->codec;
> + int reg = snd_soc_read(codec, CS4270_PWRCTL);
> +
> + if (reg < 0)
> + return reg;
You check for an error code from snd_soc_read() here, but ...
> + /* ... then disable the power-down bits */
> + reg = snd_soc_read(codec, CS4270_PWRCTL);
you don't check for it here.
In cs4270_i2c_suspend(), snd_soc_read() can never return an error,
because you're passing a hard-coded register address.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] ASoC: cs4270: add power management support
2009-05-05 15:30 ` Timur Tabi
@ 2009-05-05 15:51 ` Daniel Mack
2009-05-05 17:55 ` Timur Tabi
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Mack @ 2009-05-05 15:51 UTC (permalink / raw)
To: Timur Tabi; +Cc: alsa-devel, Mark Brown
On Tue, May 05, 2009 at 10:30:48AM -0500, Timur Tabi wrote:
> > +static int cs4270_i2c_suspend(struct i2c_client *client, pm_message_t mesg)
> > +{
> > + struct cs4270_private *cs4270 = i2c_get_clientdata(client);
> > + struct snd_soc_codec *codec = &cs4270->codec;
> > + int reg = snd_soc_read(codec, CS4270_PWRCTL);
> > +
> > + if (reg < 0)
> > + return reg;
>
> You check for an error code from snd_soc_read() here, but ...
>
> > + /* ... then disable the power-down bits */
> > + reg = snd_soc_read(codec, CS4270_PWRCTL);
>
> you don't check for it here.
>
> In cs4270_i2c_suspend(), snd_soc_read() can never return an error,
> because you're passing a hard-coded register address.
Ok - see the patch below.
Thanks,
Daniel
>From d2303f37695145018e07aad85200f06b24341648 Mon Sep 17 00:00:00 2001
From: Daniel Mack <daniel@caiaq.de>
Date: Thu, 2 Apr 2009 15:43:44 +0200
Subject: [PATCH 2/2] ASoC: cs4270: add power management support
Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: Timur Tabi <timur@freescale.com>
Cc: Mark Brown <broonie@sirena.org.uk>
---
sound/soc/codecs/cs4270.c | 59 ++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 58 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/cs4270.c b/sound/soc/codecs/cs4270.c
index bc338df..65c68a0 100644
--- a/sound/soc/codecs/cs4270.c
+++ b/sound/soc/codecs/cs4270.c
@@ -18,7 +18,7 @@
* - The machine driver's 'startup' function must call
* cs4270_set_dai_sysclk() with the value of MCLK.
* - Only I2S and left-justified modes are supported
- * - Power management is not supported
+ * - Power management is supported
*/
#include <linux/module.h>
@@ -27,6 +27,7 @@
#include <sound/soc.h>
#include <sound/initval.h>
#include <linux/i2c.h>
+#include <linux/delay.h>
#include "cs4270.h"
@@ -65,6 +66,8 @@
#define CS4270_PWRCTL_PDN_ADC 0x20
#define CS4270_PWRCTL_PDN_DAC 0x02
#define CS4270_PWRCTL_PDN 0x01
+#define CS4270_PWRCTL_PDN_ALL \
+ (CS4270_PWRCTL_PDN_ADC | CS4270_PWRCTL_PDN_DAC | CS4270_PWRCTL_PDN)
#define CS4270_MODE_SPEED_MASK 0x30
#define CS4270_MODE_1X 0x00
#define CS4270_MODE_2X 0x10
@@ -788,6 +791,58 @@ static struct i2c_device_id cs4270_id[] = {
};
MODULE_DEVICE_TABLE(i2c, cs4270_id);
+#ifdef CONFIG_PM
+
+/* This suspend/resume implementation can handle both - a simple standby
+ * where the codec remains powered, and a full suspend, where the voltage
+ * domain the codec is connected to is teared down and/or any other hardware
+ * reset condition is asserted.
+ *
+ * The codec's own power saving features are enabled in the suspend callback,
+ * and all registers are written back to the hardware when resuming.
+ */
+
+static int cs4270_i2c_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+ struct cs4270_private *cs4270 = i2c_get_clientdata(client);
+ struct snd_soc_codec *codec = &cs4270->codec;
+ int reg = snd_soc_read(codec, CS4270_PWRCTL) | CS4270_PWRCTL_PDN_ALL;
+ return snd_soc_write(codec, CS4270_PWRCTL, reg);
+}
+
+static int cs4270_i2c_resume(struct i2c_client *client)
+{
+ struct cs4270_private *cs4270 = i2c_get_clientdata(client);
+ struct snd_soc_codec *codec = &cs4270->codec;
+ int reg;
+
+ /* In case the device was put to hard reset during sleep, we need to
+ * wait 500ns here before any I2C communication. */
+ ndelay(500);
+
+ /* first restore the entire register cache ... */
+ for (reg = CS4270_FIRSTREG; reg <= CS4270_LASTREG; reg++) {
+ u8 val = snd_soc_read(codec, reg);
+
+ if (i2c_smbus_write_byte_data(client, reg, val)) {
+ dev_err(codec->dev, "i2c write failed\n");
+ return -EIO;
+ }
+ }
+
+ /* ... then disable the power-down bits */
+ reg = snd_soc_read(codec, CS4270_PWRCTL);
+ if (reg < 0)
+ return reg;
+
+ reg &= ~CS4270_PWRCTL_PDN_ALL;
+ return snd_soc_write(codec, CS4270_PWRCTL, reg);
+}
+#else
+#define cs4270_i2c_suspend NULL
+#define cs4270_i2c_resume NULL
+#endif /* CONFIG_PM */
+
/*
* cs4270_i2c_driver - I2C device identification
*
@@ -802,6 +857,8 @@ static struct i2c_driver cs4270_i2c_driver = {
.id_table = cs4270_id,
.probe = cs4270_i2c_probe,
.remove = cs4270_i2c_remove,
+ .suspend = cs4270_i2c_suspend,
+ .resume = cs4270_i2c_resume,
};
/*
--
1.6.2.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] ASoC: cs4270: add power management support
2009-05-05 15:51 ` Daniel Mack
@ 2009-05-05 17:55 ` Timur Tabi
2009-05-05 23:26 ` Daniel Mack
0 siblings, 1 reply; 15+ messages in thread
From: Timur Tabi @ 2009-05-05 17:55 UTC (permalink / raw)
To: Daniel Mack; +Cc: alsa-devel, Mark Brown
Daniel Mack wrote:
> From d2303f37695145018e07aad85200f06b24341648 Mon Sep 17 00:00:00 2001
> From: Daniel Mack <daniel@caiaq.de>
> Date: Thu, 2 Apr 2009 15:43:44 +0200
> Subject: [PATCH 2/2] ASoC: cs4270: add power management support
You forgot to add "v2" to the subject line. And you'll need a v3 on the
next version of the patch, because ...
> +static int cs4270_i2c_resume(struct i2c_client *client)
> +{
> + struct cs4270_private *cs4270 = i2c_get_clientdata(client);
> + struct snd_soc_codec *codec = &cs4270->codec;
> + int reg;
> +
> + /* In case the device was put to hard reset during sleep, we need to
> + * wait 500ns here before any I2C communication. */
> + ndelay(500);
> +
> + /* first restore the entire register cache ... */
> + for (reg = CS4270_FIRSTREG; reg <= CS4270_LASTREG; reg++) {
> + u8 val = snd_soc_read(codec, reg);
> +
> + if (i2c_smbus_write_byte_data(client, reg, val)) {
> + dev_err(codec->dev, "i2c write failed\n");
> + return -EIO;
> + }
> + }
> +
> + /* ... then disable the power-down bits */
> + reg = snd_soc_read(codec, CS4270_PWRCTL);
> + if (reg < 0)
> + return reg;
... you forgot this "reg < 0" check. Also, ...
> +static int cs4270_i2c_suspend(struct i2c_client *client, pm_message_t mesg)
> +{
> + struct cs4270_private *cs4270 = i2c_get_clientdata(client);
> + struct snd_soc_codec *codec = &cs4270->codec;
> + int reg = snd_soc_read(codec, CS4270_PWRCTL) | CS4270_PWRCTL_PDN_ALL;
> + return snd_soc_write(codec, CS4270_PWRCTL, reg);
> +}
... you need a blank line above the "return"
> +
> + reg &= ~CS4270_PWRCTL_PDN_ALL;
> + return snd_soc_write(codec, CS4270_PWRCTL, reg);
> +}
And here.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR
2009-05-05 15:15 ` Daniel Mack
@ 2009-05-05 18:09 ` Mark Brown
2009-05-05 23:19 ` Daniel Mack
0 siblings, 1 reply; 15+ messages in thread
From: Mark Brown @ 2009-05-05 18:09 UTC (permalink / raw)
To: Daniel Mack; +Cc: alsa-devel, Timur Tabi
On Tue, May 05, 2009 at 05:15:38PM +0200, Daniel Mack wrote:
> I planned to use that macro a second time for the register write-back at
> resume time. But unfortunately, the i2c stack is so badly broken that it
> does not allow the write of a whole data block without having the length
> itself as part of the message (which is wrong for the codec).
Could you clarify what you mean when you say that the I2C block requires
the length be part of the message? The I2C stack needs to know the
amount of data to write but I'm not aware of any restrictions it places
on the content of the data.
I've applied the first patch.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR
2009-05-05 18:09 ` Mark Brown
@ 2009-05-05 23:19 ` Daniel Mack
2009-05-06 8:44 ` Mark Brown
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Mack @ 2009-05-05 23:19 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel, Timur Tabi
On Tue, May 05, 2009 at 07:09:01PM +0100, Mark Brown wrote:
> > I planned to use that macro a second time for the register write-back at
> > resume time. But unfortunately, the i2c stack is so badly broken that it
> > does not allow the write of a whole data block without having the length
> > itself as part of the message (which is wrong for the codec).
>
> Could you clarify what you mean when you say that the I2C block requires
> the length be part of the message? The I2C stack needs to know the
> amount of data to write but I'm not aware of any restrictions it places
> on the content of the data.
All high-level i2c functions to write block data, namely
i2c_smbus_write_block_data() and i2c_smbus_write_i2c_block_data(), put
the length of the data being sent inside the block sent on the wire. I
couldn't believe it myself, but even my hardware I2C analyzer clearly
shows that. The API seems to assume that communication to I2C devices
always wants data to be sent with a leading command, followed by the
number of data bytes attached and then the data itself. Correct me if
I'm wrong on that.
Daniel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] ASoC: cs4270: add power management support
2009-05-05 17:55 ` Timur Tabi
@ 2009-05-05 23:26 ` Daniel Mack
2009-05-06 15:27 ` Timur Tabi
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Mack @ 2009-05-05 23:26 UTC (permalink / raw)
To: Timur Tabi; +Cc: alsa-devel, Mark Brown
On Tue, May 05, 2009 at 12:55:21PM -0500, Timur Tabi wrote:
> You forgot to add "v2" to the subject line. And you'll need a v3 on the
> next version of the patch, because ...
Ok, next round ...
Daniel
>From fff7cf8e74d1529060abe5207ad9b7d39c578026 Mon Sep 17 00:00:00 2001
From: Daniel Mack <daniel@caiaq.de>
Date: Thu, 2 Apr 2009 15:43:44 +0200
Subject: [PATCH v3] ASoC: cs4270: add power management support
Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: Timur Tabi <timur@freescale.com>
Cc: Mark Brown <broonie@sirena.org.uk>
---
sound/soc/codecs/cs4270.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 57 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/cs4270.c b/sound/soc/codecs/cs4270.c
index bc338df..4d4a45e 100644
--- a/sound/soc/codecs/cs4270.c
+++ b/sound/soc/codecs/cs4270.c
@@ -18,7 +18,7 @@
* - The machine driver's 'startup' function must call
* cs4270_set_dai_sysclk() with the value of MCLK.
* - Only I2S and left-justified modes are supported
- * - Power management is not supported
+ * - Power management is supported
*/
#include <linux/module.h>
@@ -27,6 +27,7 @@
#include <sound/soc.h>
#include <sound/initval.h>
#include <linux/i2c.h>
+#include <linux/delay.h>
#include "cs4270.h"
@@ -65,6 +66,8 @@
#define CS4270_PWRCTL_PDN_ADC 0x20
#define CS4270_PWRCTL_PDN_DAC 0x02
#define CS4270_PWRCTL_PDN 0x01
+#define CS4270_PWRCTL_PDN_ALL \
+ (CS4270_PWRCTL_PDN_ADC | CS4270_PWRCTL_PDN_DAC | CS4270_PWRCTL_PDN)
#define CS4270_MODE_SPEED_MASK 0x30
#define CS4270_MODE_1X 0x00
#define CS4270_MODE_2X 0x10
@@ -788,6 +791,57 @@ static struct i2c_device_id cs4270_id[] = {
};
MODULE_DEVICE_TABLE(i2c, cs4270_id);
+#ifdef CONFIG_PM
+
+/* This suspend/resume implementation can handle both - a simple standby
+ * where the codec remains powered, and a full suspend, where the voltage
+ * domain the codec is connected to is teared down and/or any other hardware
+ * reset condition is asserted.
+ *
+ * The codec's own power saving features are enabled in the suspend callback,
+ * and all registers are written back to the hardware when resuming.
+ */
+
+static int cs4270_i2c_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+ struct cs4270_private *cs4270 = i2c_get_clientdata(client);
+ struct snd_soc_codec *codec = &cs4270->codec;
+ int reg = snd_soc_read(codec, CS4270_PWRCTL) | CS4270_PWRCTL_PDN_ALL;
+
+ return snd_soc_write(codec, CS4270_PWRCTL, reg);
+}
+
+static int cs4270_i2c_resume(struct i2c_client *client)
+{
+ struct cs4270_private *cs4270 = i2c_get_clientdata(client);
+ struct snd_soc_codec *codec = &cs4270->codec;
+ int reg;
+
+ /* In case the device was put to hard reset during sleep, we need to
+ * wait 500ns here before any I2C communication. */
+ ndelay(500);
+
+ /* first restore the entire register cache ... */
+ for (reg = CS4270_FIRSTREG; reg <= CS4270_LASTREG; reg++) {
+ u8 val = snd_soc_read(codec, reg);
+
+ if (i2c_smbus_write_byte_data(client, reg, val)) {
+ dev_err(codec->dev, "i2c write failed\n");
+ return -EIO;
+ }
+ }
+
+ /* ... then disable the power-down bits */
+ reg = snd_soc_read(codec, CS4270_PWRCTL);
+ reg &= ~CS4270_PWRCTL_PDN_ALL;
+
+ return snd_soc_write(codec, CS4270_PWRCTL, reg);
+}
+#else
+#define cs4270_i2c_suspend NULL
+#define cs4270_i2c_resume NULL
+#endif /* CONFIG_PM */
+
/*
* cs4270_i2c_driver - I2C device identification
*
@@ -802,6 +856,8 @@ static struct i2c_driver cs4270_i2c_driver = {
.id_table = cs4270_id,
.probe = cs4270_i2c_probe,
.remove = cs4270_i2c_remove,
+ .suspend = cs4270_i2c_suspend,
+ .resume = cs4270_i2c_resume,
};
/*
--
1.6.2.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR
2009-05-05 23:19 ` Daniel Mack
@ 2009-05-06 8:44 ` Mark Brown
2009-05-06 9:39 ` Daniel Mack
0 siblings, 1 reply; 15+ messages in thread
From: Mark Brown @ 2009-05-06 8:44 UTC (permalink / raw)
To: Daniel Mack; +Cc: alsa-devel, Timur Tabi
On Wed, May 06, 2009 at 01:19:24AM +0200, Daniel Mack wrote:
> All high-level i2c functions to write block data, namely
> i2c_smbus_write_block_data() and i2c_smbus_write_i2c_block_data(), put
> the length of the data being sent inside the block sent on the wire. I
> couldn't believe it myself, but even my hardware I2C analyzer clearly
> shows that. The API seems to assume that communication to I2C devices
> always wants data to be sent with a leading command, followed by the
> number of data bytes attached and then the data itself. Correct me if
> I'm wrong on that.
That's not the I2C API, that's the SMBus API. SMBus is more restricted
than I2C - it essentially defines a protocol on top of I2C. If the
device doesn't implement SMBus then you should use the I2C API directly.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR
2009-05-06 8:44 ` Mark Brown
@ 2009-05-06 9:39 ` Daniel Mack
2009-05-06 15:28 ` Timur Tabi
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Mack @ 2009-05-06 9:39 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel, Timur Tabi
On Wed, May 06, 2009 at 09:44:34AM +0100, Mark Brown wrote:
> > All high-level i2c functions to write block data, namely
> > i2c_smbus_write_block_data() and i2c_smbus_write_i2c_block_data(), put
> > the length of the data being sent inside the block sent on the wire. I
> > couldn't believe it myself, but even my hardware I2C analyzer clearly
> > shows that. The API seems to assume that communication to I2C devices
> > always wants data to be sent with a leading command, followed by the
> > number of data bytes attached and then the data itself. Correct me if
> > I'm wrong on that.
>
> That's not the I2C API, that's the SMBus API. SMBus is more restricted
> than I2C - it essentially defines a protocol on top of I2C. If the
> device doesn't implement SMBus then you should use the I2C API directly.
Hmm, ok. That makes sense. However, the driver uses the equivalent
function to read the whole register set, so I wanted to keep it in
sync.
Anyway, the patch I sent for the resume code writes the registeres one
by one. There are only eigth of them, so it doesn't really matter.
But thanks for your clarification :)
Daniel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] ASoC: cs4270: add power management support
2009-05-05 23:26 ` Daniel Mack
@ 2009-05-06 15:27 ` Timur Tabi
2009-05-07 8:02 ` Mark Brown
0 siblings, 1 reply; 15+ messages in thread
From: Timur Tabi @ 2009-05-06 15:27 UTC (permalink / raw)
To: Daniel Mack; +Cc: alsa-devel, Mark Brown
On Tue, May 5, 2009 at 6:26 PM, Daniel Mack <daniel@caiaq.de> wrote:
> From fff7cf8e74d1529060abe5207ad9b7d39c578026 Mon Sep 17 00:00:00 2001
> From: Daniel Mack <daniel@caiaq.de>
> Date: Thu, 2 Apr 2009 15:43:44 +0200
> Subject: [PATCH v3] ASoC: cs4270: add power management support
>
> Signed-off-by: Daniel Mack <daniel@caiaq.de>
> Cc: Timur Tabi <timur@freescale.com>
> Cc: Mark Brown <broonie@sirena.org.uk>
Acked-by: Timur Tabi <timur@freescale.com>
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR
2009-05-06 9:39 ` Daniel Mack
@ 2009-05-06 15:28 ` Timur Tabi
0 siblings, 0 replies; 15+ messages in thread
From: Timur Tabi @ 2009-05-06 15:28 UTC (permalink / raw)
To: Daniel Mack; +Cc: alsa-devel, Mark Brown
On Wed, May 6, 2009 at 4:39 AM, Daniel Mack <daniel@caiaq.de> wrote:
> Hmm, ok. That makes sense. However, the driver uses the equivalent
> function to read the whole register set, so I wanted to keep it in
> sync.
That's because I could never figure out which API I should use for
this. I just kept trying variants until something worked.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] ASoC: cs4270: add power management support
2009-05-06 15:27 ` Timur Tabi
@ 2009-05-07 8:02 ` Mark Brown
0 siblings, 0 replies; 15+ messages in thread
From: Mark Brown @ 2009-05-07 8:02 UTC (permalink / raw)
To: Timur Tabi; +Cc: alsa-devel
On Wed, May 06, 2009 at 10:27:19AM -0500, Timur Tabi wrote:
> Acked-by: Timur Tabi <timur@freescale.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2009-05-07 8:02 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-05 9:25 [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR Daniel Mack
2009-05-05 9:25 ` [PATCH 2/2] ASoC: cs4270: add power management support Daniel Mack
2009-05-05 15:30 ` Timur Tabi
2009-05-05 15:51 ` Daniel Mack
2009-05-05 17:55 ` Timur Tabi
2009-05-05 23:26 ` Daniel Mack
2009-05-06 15:27 ` Timur Tabi
2009-05-07 8:02 ` Mark Brown
2009-05-05 14:51 ` [PATCH 1/2] ASoC: cs4270: introduce CS4270_I2C_INCR Timur Tabi
2009-05-05 15:15 ` Daniel Mack
2009-05-05 18:09 ` Mark Brown
2009-05-05 23:19 ` Daniel Mack
2009-05-06 8:44 ` Mark Brown
2009-05-06 9:39 ` Daniel Mack
2009-05-06 15:28 ` Timur Tabi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.