| Message ID | 1456686105-6885-1-git-send-email-KCHSU0@nuvoton.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 29, 2016 at 03:01:45AM +0800, John Hsu wrote: > +static int nau8825_resume_setup(struct nau8825 *nau8825) > +{ I'd expect to see this shared with initial power on? > regcache_cache_only(nau8825->regmap, false); > - regcache_sync(nau8825->regmap); > - enable_irq(client->irq); > + enable_irq(nau8825->irq); We're removing the register cache sync here but I don't see us adding it anywhere else.
Hi, On 3/1/2016 11:26 AM, Mark Brown wrote: > On Mon, Feb 29, 2016 at 03:01:45AM +0800, John Hsu wrote: > > >> +static int nau8825_resume_setup(struct nau8825 *nau8825) >> +{ >> > > I'd expect to see this shared with initial power on? > > There is a little things different between resume and initiation. But I think the function could be reused. >> regcache_cache_only(nau8825->regmap, false); >> - regcache_sync(nau8825->regmap); >> - enable_irq(client->irq); >> + enable_irq(nau8825->irq); >> > > We're removing the register cache sync here but I don't see us adding it > anywhere else. > A part of suspend and resume action moves to set bias function, nau8825_set_bias_level. We make register cache dirty in bias off; and make register cache sync after resume in bias standby.
Hi, On 3/10/2016 2:12 PM, John Hsu wrote: >>> regcache_cache_only(nau8825->regmap, false); >>> - regcache_sync(nau8825->regmap); >>> - enable_irq(client->irq); >>> + enable_irq(nau8825->irq); >>> >> >> We're removing the register cache sync here but I don't see us adding it >> anywhere else. >> > A part of suspend and resume action moves to set bias function, > nau8825_set_bias_level. > We make register cache dirty in bias off; and make register cache sync > after resume in bias standby. > Just confirm it. Could you accept the explain about the register cache sync? We need to change it or not? Very appreciate.
On Thu, Mar 10, 2016 at 02:12:46PM +0800, John Hsu wrote: > On 3/1/2016 11:26 AM, Mark Brown wrote: > >On Mon, Feb 29, 2016 at 03:01:45AM +0800, John Hsu wrote: > >>+static int nau8825_resume_setup(struct nau8825 *nau8825) > >>+{ > >I'd expect to see this shared with initial power on? > There is a little things different between resume and initiation. > But I think the function could be reused. Really? Bear in mind the device might have lost power over suspend... > >> regcache_cache_only(nau8825->regmap, false); > >>- regcache_sync(nau8825->regmap); > >>- enable_irq(client->irq); > >>+ enable_irq(nau8825->irq); > >We're removing the register cache sync here but I don't see us adding it > >anywhere else. > A part of suspend and resume action moves to set bias function, > nau8825_set_bias_level. > We make register cache dirty in bias off; and make register cache sync after > resume in bias standby. OK.
On Tue, Mar 15, 2016 at 03:53:03PM +0800, John Hsu wrote: > On 3/10/2016 2:12 PM, John Hsu wrote: > >A part of suspend and resume action moves to set bias function, > >nau8825_set_bias_level. > >We make register cache dirty in bias off; and make register cache sync > >after resume in bias standby. > Just confirm it. Could you accept the explain about the register cache sync? > We need to change it or not? Very appreciate. Please don't send content free pings and please allow a reasonable time for review. People get busy, go on holiday, attend conferences and so on so unless there is some reason for urgency (like critical bug fixes) please allow at least a couple of weeks for review. Sending content free pings just adds to the mail volume (if they are seen at all) and if something has gone wrong you'll have to resend the patches anyway.
Hi, On 3/15/2016 5:27 PM, Mark Brown wrote: > On Thu, Mar 10, 2016 at 02:12:46PM +0800, John Hsu wrote: > >> On 3/1/2016 11:26 AM, Mark Brown wrote: >> >>> On Mon, Feb 29, 2016 at 03:01:45AM +0800, John Hsu wrote: >>> > > >>>> +static int nau8825_resume_setup(struct nau8825 *nau8825) >>>> +{ >>>> > > >>> I'd expect to see this shared with initial power on? >>> > > >> There is a little things different between resume and initiation. >> But I think the function could be reused. >> > > Really? Bear in mind the device might have lost power over suspend... > Thanks for your remind. I'll design this function based on the concept.
diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c index 504c969..2e2e11a 100644 --- a/sound/soc/codecs/nau8825.c +++ b/sound/soc/codecs/nau8825.c @@ -1092,6 +1092,36 @@ static int nau8825_set_sysclk(struct snd_soc_codec *codec, int clk_id, return nau8825_configure_sysclk(nau8825, clk_id, freq); } +static int nau8825_resume_setup(struct nau8825 *nau8825) +{ + struct regmap *regmap = nau8825->regmap; + + /* IRQ Output Enable */ + regmap_update_bits(regmap, NAU8825_REG_INTERRUPT_MASK, + NAU8825_IRQ_OUTPUT_EN, NAU8825_IRQ_OUTPUT_EN); + + /* Enable internal VCO needed for interruptions */ + nau8825_configure_sysclk(nau8825, NAU8825_CLK_INTERNAL, 0); + + /* Enable DDACR needed for interrupts */ + regmap_update_bits(regmap, NAU8825_REG_ENA_CTRL, + NAU8825_ENABLE_DACR, NAU8825_ENABLE_DACR); + + /* Chip needs one FSCLK cycle in order to generate interrupts, + * as we cannot guarantee one will be provided by the system. Turning + * master mode on then off enables us to generate that FSCLK cycle + * with a minimum of contention on the clock bus. + */ + regmap_update_bits(regmap, NAU8825_REG_I2S_PCM_CTRL2, + NAU8825_I2S_MS_MASK, NAU8825_I2S_MS_MASTER); + regmap_update_bits(regmap, NAU8825_REG_I2S_PCM_CTRL2, + NAU8825_I2S_MS_MASK, NAU8825_I2S_MS_SLAVE); + + nau8825_restart_jack_detection(regmap); + + return 0; +} + static int nau8825_set_bias_level(struct snd_soc_codec *codec, enum snd_soc_bias_level level) { @@ -1121,6 +1151,8 @@ static int nau8825_set_bias_level(struct snd_soc_codec *codec, "Failed to sync cache: %d\n", ret); return ret; } + if (nau8825->irq) + nau8825_resume_setup(nau8825); } break; @@ -1330,24 +1362,22 @@ static int nau8825_i2c_remove(struct i2c_client *client) #ifdef CONFIG_PM_SLEEP static int nau8825_suspend(struct device *dev) { - struct i2c_client *client = to_i2c_client(dev); struct nau8825 *nau8825 = dev_get_drvdata(dev); + struct snd_soc_codec *codec = snd_soc_dapm_to_codec(nau8825->dapm); - disable_irq(client->irq); + disable_irq(nau8825->irq); + snd_soc_codec_force_bias_level(codec, SND_SOC_BIAS_OFF); regcache_cache_only(nau8825->regmap, true); - regcache_mark_dirty(nau8825->regmap); return 0; } static int nau8825_resume(struct device *dev) { - struct i2c_client *client = to_i2c_client(dev); struct nau8825 *nau8825 = dev_get_drvdata(dev); regcache_cache_only(nau8825->regmap, false); - regcache_sync(nau8825->regmap); - enable_irq(client->irq); + enable_irq(nau8825->irq); return 0; }
This patch is to fix interrupt fails when resume back. In solution, we add IRQ re-initiation, and reconstruct suspend and resume with set_bias_level function. Signed-off-by: John Hsu <KCHSU0@nuvoton.com> --- sound/soc/codecs/nau8825.c | 42 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-)