All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ASoC: ts3a227e: do not report jack status when there is i2c read err
@ 2015-07-01  4:18 yang.a.fang
  2015-07-01 17:12 ` Dylan Reid
  2015-07-01 23:24 ` [PATCH v2] " yang.a.fang
  0 siblings, 2 replies; 9+ messages in thread
From: yang.a.fang @ 2015-07-01  4:18 UTC (permalink / raw)
  To: broonie, lgirdwood
  Cc: alsa-devel, srinivas.sripathi, Fang, Yang A, praveen.k.jain,
	denny.iriawan, sathyanarayana.nujella, kevin.strasser, dgreid

From: "Fang, Yang A" <yang.a.fang@intel.com>

After suspend -> resume the ts3a227e_interrupt sometimes comes before i2c
controller resume is called .regmap_read will return incorrect status
and report a wrong jack status.We should return if there is read err,the
interrupt will come again since it is level triggered and we are not yet
clear the interrupt. In addtion,cht_bsw_max98090_ti machine driver
registered additional notifier base on jack event which will program
the audio codec.there will be codec timeout err if such event occurs
prior to i2c controller is resumed.

Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
---
 sound/soc/codecs/ts3a227e.c |   18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/ts3a227e.c b/sound/soc/codecs/ts3a227e.c
index 12232d7..12d0f2a 100644
--- a/sound/soc/codecs/ts3a227e.c
+++ b/sound/soc/codecs/ts3a227e.c
@@ -23,6 +23,7 @@
 #include "ts3a227e.h"
 
 struct ts3a227e {
+	struct device *dev;
 	struct regmap *regmap;
 	struct snd_soc_jack *jack;
 	bool plugged;
@@ -189,16 +190,28 @@ static irqreturn_t ts3a227e_interrupt(int irq, void *data)
 	struct ts3a227e *ts3a227e = (struct ts3a227e *)data;
 	struct regmap *regmap = ts3a227e->regmap;
 	unsigned int int_reg, kp_int_reg, acc_reg, i;
+	struct device *dev = ts3a227e->dev;
+	int ret;
 
 	/* Check for plug/unplug. */
-	regmap_read(regmap, TS3A227E_REG_INTERRUPT, &int_reg);
+	ret = regmap_read(regmap, TS3A227E_REG_INTERRUPT, &int_reg);
+	if (ret) {
+		dev_err(dev, "failed to clear interrupt ret=%d\n", ret);
+		return IRQ_HANDLED;
+	}
+
 	if (int_reg & (DETECTION_COMPLETE_EVENT | INS_REM_EVENT)) {
 		regmap_read(regmap, TS3A227E_REG_ACCESSORY_STATUS, &acc_reg);
 		ts3a227e_new_jack_state(ts3a227e, acc_reg);
 	}
 
 	/* Report any key events. */
-	regmap_read(regmap, TS3A227E_REG_KP_INTERRUPT, &kp_int_reg);
+	ret = regmap_read(regmap, TS3A227E_REG_KP_INTERRUPT, &kp_int_reg);
+	if (ret) {
+		dev_err(dev, "failed to clear key interrupt ret=%d\n", ret);
+		return IRQ_HANDLED;
+	}
+
 	for (i = 0; i < TS3A227E_NUM_BUTTONS; i++) {
 		if (kp_int_reg & PRESS_MASK(i))
 			ts3a227e->buttons_held |= (1 << i);
@@ -283,6 +296,7 @@ static int ts3a227e_i2c_probe(struct i2c_client *i2c,
 		return -ENOMEM;
 
 	i2c_set_clientdata(i2c, ts3a227e);
+	ts3a227e->dev = dev;
 
 	ts3a227e->regmap = devm_regmap_init_i2c(i2c, &ts3a227e_regmap_config);
 	if (IS_ERR(ts3a227e->regmap))
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] ASoC: ts3a227e: do not report jack status when there is i2c read err
  2015-07-01  4:18 [PATCH] ASoC: ts3a227e: do not report jack status when there is i2c read err yang.a.fang
@ 2015-07-01 17:12 ` Dylan Reid
  2015-07-01 17:34   ` Yang Fang
  2015-07-01 23:24 ` [PATCH v2] " yang.a.fang
  1 sibling, 1 reply; 9+ messages in thread
From: Dylan Reid @ 2015-07-01 17:12 UTC (permalink / raw)
  To: Yang Fang
  Cc: alsa-devel@alsa-project.org, Sripathi, Srinivas, Praveen K Jain,
	Liam Girdwood, Denny Iriawan, Mark Brown, Nujella, Sathyanarayana,
	kevin.strasser

On Tue, Jun 30, 2015 at 9:18 PM,  <yang.a.fang@intel.com> wrote:
> From: "Fang, Yang A" <yang.a.fang@intel.com>
>
> After suspend -> resume the ts3a227e_interrupt sometimes comes before i2c
> controller resume is called .regmap_read will return incorrect status
> and report a wrong jack status.We should return if there is read err,the
> interrupt will come again since it is level triggered and we are not yet
> clear the interrupt. In addtion,cht_bsw_max98090_ti machine driver
> registered additional notifier base on jack event which will program
> the audio codec.there will be codec timeout err if such event occurs
> prior to i2c controller is resumed.

Thanks, I think the error checking is good to have anyway, but should
the interrupt also be disabled across suspend/resume?  I'd hope this
device's resume callback wouldn't happen until after the parent i2c
bus is ready.

>
> Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
> ---
>  sound/soc/codecs/ts3a227e.c |   18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/codecs/ts3a227e.c b/sound/soc/codecs/ts3a227e.c
> index 12232d7..12d0f2a 100644
> --- a/sound/soc/codecs/ts3a227e.c
> +++ b/sound/soc/codecs/ts3a227e.c
> @@ -23,6 +23,7 @@
>  #include "ts3a227e.h"
>
>  struct ts3a227e {
> +       struct device *dev;
>         struct regmap *regmap;
>         struct snd_soc_jack *jack;
>         bool plugged;
> @@ -189,16 +190,28 @@ static irqreturn_t ts3a227e_interrupt(int irq, void *data)
>         struct ts3a227e *ts3a227e = (struct ts3a227e *)data;
>         struct regmap *regmap = ts3a227e->regmap;
>         unsigned int int_reg, kp_int_reg, acc_reg, i;
> +       struct device *dev = ts3a227e->dev;
> +       int ret;
>
>         /* Check for plug/unplug. */
> -       regmap_read(regmap, TS3A227E_REG_INTERRUPT, &int_reg);
> +       ret = regmap_read(regmap, TS3A227E_REG_INTERRUPT, &int_reg);
> +       if (ret) {
> +               dev_err(dev, "failed to clear interrupt ret=%d\n", ret);
> +               return IRQ_HANDLED;
> +       }
> +
>         if (int_reg & (DETECTION_COMPLETE_EVENT | INS_REM_EVENT)) {
>                 regmap_read(regmap, TS3A227E_REG_ACCESSORY_STATUS, &acc_reg);
>                 ts3a227e_new_jack_state(ts3a227e, acc_reg);
>         }
>
>         /* Report any key events. */
> -       regmap_read(regmap, TS3A227E_REG_KP_INTERRUPT, &kp_int_reg);
> +       ret = regmap_read(regmap, TS3A227E_REG_KP_INTERRUPT, &kp_int_reg);
> +       if (ret) {
> +               dev_err(dev, "failed to clear key interrupt ret=%d\n", ret);
> +               return IRQ_HANDLED;
> +       }
> +
>         for (i = 0; i < TS3A227E_NUM_BUTTONS; i++) {
>                 if (kp_int_reg & PRESS_MASK(i))
>                         ts3a227e->buttons_held |= (1 << i);
> @@ -283,6 +296,7 @@ static int ts3a227e_i2c_probe(struct i2c_client *i2c,
>                 return -ENOMEM;
>
>         i2c_set_clientdata(i2c, ts3a227e);
> +       ts3a227e->dev = dev;
>
>         ts3a227e->regmap = devm_regmap_init_i2c(i2c, &ts3a227e_regmap_config);
>         if (IS_ERR(ts3a227e->regmap))
> --
> 1.7.9.5
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ASoC: ts3a227e: do not report jack status when there is i2c read err
  2015-07-01 17:12 ` Dylan Reid
@ 2015-07-01 17:34   ` Yang Fang
  2015-07-01 18:05     ` Mark Brown
  0 siblings, 1 reply; 9+ messages in thread
From: Yang Fang @ 2015-07-01 17:34 UTC (permalink / raw)
  To: Dylan Reid
  Cc: alsa-devel@alsa-project.org, Sripathi, Srinivas, yang.a.fang,
	Praveen K Jain, Liam Girdwood, Denny Iriawan, Mark Brown,
	Nujella, Sathyanarayana, kevin.strasser, mika.westerberg

On Wed, Jul 01, 2015 at 10:12:37AM -0700, Dylan Reid wrote:
> On Tue, Jun 30, 2015 at 9:18 PM,  <yang.a.fang@intel.com> wrote:
> > From: "Fang, Yang A" <yang.a.fang@intel.com>
> >
> > After suspend -> resume the ts3a227e_interrupt sometimes comes before i2c
> > controller resume is called .regmap_read will return incorrect status
> > and report a wrong jack status.We should return if there is read err,the
> > interrupt will come again since it is level triggered and we are not yet
> > clear the interrupt. In addtion,cht_bsw_max98090_ti machine driver
> > registered additional notifier base on jack event which will program
> > the audio codec.there will be codec timeout err if such event occurs
> > prior to i2c controller is resumed.
> 
> Thanks, I think the error checking is good to have anyway, but should
> the interrupt also be disabled across suspend/resume?  I'd hope this
> device's resume callback wouldn't happen until after the parent i2c
> bus is ready.
I am looping Mika. I was expecting that interrupt would come after i2c
bus is ready. but with current pinctrl-cherryview driver the interrupt
comes in random order after resume.
> 
> >
> > Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
> > ---
> >  sound/soc/codecs/ts3a227e.c |   18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/soc/codecs/ts3a227e.c b/sound/soc/codecs/ts3a227e.c
> > index 12232d7..12d0f2a 100644
> > --- a/sound/soc/codecs/ts3a227e.c
> > +++ b/sound/soc/codecs/ts3a227e.c
> > @@ -23,6 +23,7 @@
> >  #include "ts3a227e.h"
> >
> >  struct ts3a227e {
> > +       struct device *dev;
> >         struct regmap *regmap;
> >         struct snd_soc_jack *jack;
> >         bool plugged;
> > @@ -189,16 +190,28 @@ static irqreturn_t ts3a227e_interrupt(int irq, void *data)
> >         struct ts3a227e *ts3a227e = (struct ts3a227e *)data;
> >         struct regmap *regmap = ts3a227e->regmap;
> >         unsigned int int_reg, kp_int_reg, acc_reg, i;
> > +       struct device *dev = ts3a227e->dev;
> > +       int ret;
> >
> >         /* Check for plug/unplug. */
> > -       regmap_read(regmap, TS3A227E_REG_INTERRUPT, &int_reg);
> > +       ret = regmap_read(regmap, TS3A227E_REG_INTERRUPT, &int_reg);
> > +       if (ret) {
> > +               dev_err(dev, "failed to clear interrupt ret=%d\n", ret);
> > +               return IRQ_HANDLED;
> > +       }
> > +
> >         if (int_reg & (DETECTION_COMPLETE_EVENT | INS_REM_EVENT)) {
> >                 regmap_read(regmap, TS3A227E_REG_ACCESSORY_STATUS, &acc_reg);
> >                 ts3a227e_new_jack_state(ts3a227e, acc_reg);
> >         }
> >
> >         /* Report any key events. */
> > -       regmap_read(regmap, TS3A227E_REG_KP_INTERRUPT, &kp_int_reg);
> > +       ret = regmap_read(regmap, TS3A227E_REG_KP_INTERRUPT, &kp_int_reg);
> > +       if (ret) {
> > +               dev_err(dev, "failed to clear key interrupt ret=%d\n", ret);
> > +               return IRQ_HANDLED;
> > +       }
> > +
> >         for (i = 0; i < TS3A227E_NUM_BUTTONS; i++) {
> >                 if (kp_int_reg & PRESS_MASK(i))
> >                         ts3a227e->buttons_held |= (1 << i);
> > @@ -283,6 +296,7 @@ static int ts3a227e_i2c_probe(struct i2c_client *i2c,
> >                 return -ENOMEM;
> >
> >         i2c_set_clientdata(i2c, ts3a227e);
> > +       ts3a227e->dev = dev;
> >
> >         ts3a227e->regmap = devm_regmap_init_i2c(i2c, &ts3a227e_regmap_config);
> >         if (IS_ERR(ts3a227e->regmap))
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ASoC: ts3a227e: do not report jack status when there is i2c read err
  2015-07-01 17:34   ` Yang Fang
@ 2015-07-01 18:05     ` Mark Brown
  2015-07-01 18:24       ` Fang, Yang A
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Brown @ 2015-07-01 18:05 UTC (permalink / raw)
  To: Yang Fang
  Cc: alsa-devel@alsa-project.org, Sripathi, Srinivas, Praveen K Jain,
	Liam Girdwood, Denny Iriawan, Nujella, Sathyanarayana,
	kevin.strasser, Dylan Reid, mika.westerberg


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

On Wed, Jul 01, 2015 at 10:34:09AM -0700, Yang Fang wrote:
> On Wed, Jul 01, 2015 at 10:12:37AM -0700, Dylan Reid wrote:

> > Thanks, I think the error checking is good to have anyway, but should
> > the interrupt also be disabled across suspend/resume?  I'd hope this
> > device's resume callback wouldn't happen until after the parent i2c
> > bus is ready.

> I am looping Mika. I was expecting that interrupt would come after i2c
> bus is ready. but with current pinctrl-cherryview driver the interrupt
> comes in random order after resume.

I wouldn't rely on this, it's not going to be true in general - the
interrupt could flag at any time after the interrupt controller is
resumed so unless the I2C controller takes steps to ensure it is resumed
before interrupts (which has its own complications) you could get an
interrupt delivered prior to the I2C controller being ready.  Look at
how arizona handles this for one example of dealing with this problem,
though it's not ideal.

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

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ASoC: ts3a227e: do not report jack status when there is i2c read err
  2015-07-01 18:05     ` Mark Brown
@ 2015-07-01 18:24       ` Fang, Yang A
  2015-07-01 23:20         ` Fang, Yang A
  0 siblings, 1 reply; 9+ messages in thread
From: Fang, Yang A @ 2015-07-01 18:24 UTC (permalink / raw)
  To: 'Mark Brown'
  Cc: alsa-devel@alsa-project.org, Sripathi, Srinivas, Jain, Praveen K,
	Liam Girdwood, Iriawan, Denny, Nujella, Sathyanarayana,
	kevin.strasser@linux.intel.com, Dylan Reid,
	mika.westerberg@linux.intel.com



> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Wednesday, July 01, 2015 11:05 AM
> To: Fang, Yang A
> Cc: Dylan Reid; Liam Girdwood; alsa-devel@alsa-project.org; Sripathi, Srinivas;
> Jain, Praveen K; Iriawan, Denny; Nujella, Sathyanarayana;
> kevin.strasser@linux.intel.com; mika.westerberg@linux.intel.com
> Subject: Re: [alsa-devel] [PATCH] ASoC: ts3a227e: do not report jack status
> when there is i2c read err
> 
> On Wed, Jul 01, 2015 at 10:34:09AM -0700, Yang Fang wrote:
> > On Wed, Jul 01, 2015 at 10:12:37AM -0700, Dylan Reid wrote:
> 
> > > Thanks, I think the error checking is good to have anyway, but
> > > should the interrupt also be disabled across suspend/resume?  I'd
> > > hope this device's resume callback wouldn't happen until after the
> > > parent i2c bus is ready.
> 
> > I am looping Mika. I was expecting that interrupt would come after i2c
> > bus is ready. but with current pinctrl-cherryview driver the interrupt
> > comes in random order after resume.
> 
> I wouldn't rely on this, it's not going to be true in general - the interrupt could
> flag at any time after the interrupt controller is resumed so unless the I2C
> controller takes steps to ensure it is resumed before interrupts (which has its
> own complications) you could get an interrupt delivered prior to the I2C
> controller being ready.  Look at how arizona handles this for one example of
> dealing with this problem, though it's not ideal.

Thanks. i did a quick look at Arizona-core.c it disabled irq on suspend and enable irq
on resume.  I will give a try if this is what you referred to. 

In addition, current patch can address the issue even if the interrupt came prior to 
I2c bus is reumed . can I keep the err checking ?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ASoC: ts3a227e: do not report jack status when there is i2c read err
  2015-07-01 18:24       ` Fang, Yang A
@ 2015-07-01 23:20         ` Fang, Yang A
  0 siblings, 0 replies; 9+ messages in thread
From: Fang, Yang A @ 2015-07-01 23:20 UTC (permalink / raw)
  To: 'Mark Brown'
  Cc: 'alsa-devel@alsa-project.org', Sripathi, Srinivas,
	Jain, Praveen K, 'Liam Girdwood', Iriawan, Denny,
	Nujella, Sathyanarayana, 'kevin.strasser@linux.intel.com',
	'Dylan Reid', 'mika.westerberg@linux.intel.com'



> -----Original Message-----
> From: Fang, Yang A
> Sent: Wednesday, July 01, 2015 11:25 AM
> To: 'Mark Brown'
> Cc: Dylan Reid; Liam Girdwood; alsa-devel@alsa-project.org; Sripathi, Srinivas;
> Jain, Praveen K; Iriawan, Denny; Nujella, Sathyanarayana;
> kevin.strasser@linux.intel.com; mika.westerberg@linux.intel.com
> Subject: RE: [alsa-devel] [PATCH] ASoC: ts3a227e: do not report jack status
> when there is i2c read err
> 
> 
> 
> > -----Original Message-----
> > From: Mark Brown [mailto:broonie@kernel.org]
> > Sent: Wednesday, July 01, 2015 11:05 AM
> > To: Fang, Yang A
> > Cc: Dylan Reid; Liam Girdwood; alsa-devel@alsa-project.org; Sripathi,
> > Srinivas; Jain, Praveen K; Iriawan, Denny; Nujella, Sathyanarayana;
> > kevin.strasser@linux.intel.com; mika.westerberg@linux.intel.com
> > Subject: Re: [alsa-devel] [PATCH] ASoC: ts3a227e: do not report jack
> > status when there is i2c read err
> >
> > On Wed, Jul 01, 2015 at 10:34:09AM -0700, Yang Fang wrote:
> > > On Wed, Jul 01, 2015 at 10:12:37AM -0700, Dylan Reid wrote:
> >
> > > > Thanks, I think the error checking is good to have anyway, but
> > > > should the interrupt also be disabled across suspend/resume?  I'd
> > > > hope this device's resume callback wouldn't happen until after the
> > > > parent i2c bus is ready.
> >
> > > I am looping Mika. I was expecting that interrupt would come after
> > > i2c bus is ready. but with current pinctrl-cherryview driver the
> > > interrupt comes in random order after resume.
> >
> > I wouldn't rely on this, it's not going to be true in general - the
> > interrupt could flag at any time after the interrupt controller is
> > resumed so unless the I2C controller takes steps to ensure it is
> > resumed before interrupts (which has its own complications) you could
> > get an interrupt delivered prior to the I2C controller being ready.
> > Look at how arizona handles this for one example of dealing with this
> problem, though it's not ideal.
> 
> Thanks. i did a quick look at Arizona-core.c it disabled irq on suspend and
> enable irq on resume.  I will give a try if this is what you referred to.
> 
> In addition, current patch can address the issue even if the interrupt came
> prior to I2c bus is reumed . can I keep the err checking ?

Disable irq on suspend and enable irq on resume works well. I saw interrupt
 occurs after TI resume. I will post V2 patch

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] ASoC: ts3a227e: do not report jack status when there is i2c read err
  2015-07-01  4:18 [PATCH] ASoC: ts3a227e: do not report jack status when there is i2c read err yang.a.fang
  2015-07-01 17:12 ` Dylan Reid
@ 2015-07-01 23:24 ` yang.a.fang
  2015-07-07 13:47   ` Mark Brown
  2015-07-07 21:21   ` [PATCH v3] " yang.a.fang
  1 sibling, 2 replies; 9+ messages in thread
From: yang.a.fang @ 2015-07-01 23:24 UTC (permalink / raw)
  To: broonie, lgirdwood
  Cc: alsa-devel, srinivas.sripathi, Fang, Yang A, praveen.k.jain,
	denny.iriawan, sathyanarayana.nujella, kevin.strasser, dgreid,
	mika.westerberg

From: "Fang, Yang A" <yang.a.fang@intel.com>

After suspend -> resume the ts3a227e_interrupt sometimes comes before i2c
controller resume is called .regmap_read will return incorrect status
and report a wrong jack status.This patch will disable irq on suspend
and enable irq again on the resume to make sure interrupt is coming
after TI resumes. Also We should return if there is read err,the
interrupt will come again since it is level triggered and we are not yet
clear the interrupt.

In addtion,cht_bsw_max98090_ti machine driver registered additional
notifier base on jack event which will program the audio codec.there
will be codec timeout err if such event occurs prior to i2c controller
is resumed.

Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
---
 sound/soc/codecs/ts3a227e.c |   47 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/ts3a227e.c b/sound/soc/codecs/ts3a227e.c
index 12232d7..611c09c 100644
--- a/sound/soc/codecs/ts3a227e.c
+++ b/sound/soc/codecs/ts3a227e.c
@@ -23,11 +23,13 @@
 #include "ts3a227e.h"
 
 struct ts3a227e {
+	struct device *dev;
 	struct regmap *regmap;
 	struct snd_soc_jack *jack;
 	bool plugged;
 	bool mic_present;
 	unsigned int buttons_held;
+	int irq;
 };
 
 /* Button values to be reported on the jack */
@@ -189,16 +191,28 @@ static irqreturn_t ts3a227e_interrupt(int irq, void *data)
 	struct ts3a227e *ts3a227e = (struct ts3a227e *)data;
 	struct regmap *regmap = ts3a227e->regmap;
 	unsigned int int_reg, kp_int_reg, acc_reg, i;
+	struct device *dev = ts3a227e->dev;
+	int ret;
 
 	/* Check for plug/unplug. */
-	regmap_read(regmap, TS3A227E_REG_INTERRUPT, &int_reg);
+	ret = regmap_read(regmap, TS3A227E_REG_INTERRUPT, &int_reg);
+	if (ret) {
+		dev_err(dev, "failed to clear interrupt ret=%d\n", ret);
+		return IRQ_HANDLED;
+	}
+
 	if (int_reg & (DETECTION_COMPLETE_EVENT | INS_REM_EVENT)) {
 		regmap_read(regmap, TS3A227E_REG_ACCESSORY_STATUS, &acc_reg);
 		ts3a227e_new_jack_state(ts3a227e, acc_reg);
 	}
 
 	/* Report any key events. */
-	regmap_read(regmap, TS3A227E_REG_KP_INTERRUPT, &kp_int_reg);
+	ret = regmap_read(regmap, TS3A227E_REG_KP_INTERRUPT, &kp_int_reg);
+	if (ret) {
+		dev_err(dev, "failed to clear key interrupt ret=%d\n", ret);
+		return IRQ_HANDLED;
+	}
+
 	for (i = 0; i < TS3A227E_NUM_BUTTONS; i++) {
 		if (kp_int_reg & PRESS_MASK(i))
 			ts3a227e->buttons_held |= (1 << i);
@@ -283,6 +297,8 @@ static int ts3a227e_i2c_probe(struct i2c_client *i2c,
 		return -ENOMEM;
 
 	i2c_set_clientdata(i2c, ts3a227e);
+	ts3a227e->dev = dev;
+	ts3a227e->irq = i2c->irq;
 
 	ts3a227e->regmap = devm_regmap_init_i2c(i2c, &ts3a227e_regmap_config);
 	if (IS_ERR(ts3a227e->regmap))
@@ -320,6 +336,32 @@ static int ts3a227e_i2c_probe(struct i2c_client *i2c,
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int ts3a227e_suspend(struct device *dev)
+{
+	struct ts3a227e *ts3a227e = dev_get_drvdata(dev);
+
+	dev_dbg(ts3a227e->dev, "suspend disable irq\n");
+	disable_irq(ts3a227e->irq);
+
+	return 0;
+}
+
+static int ts3a227e_resume(struct device *dev)
+{
+	struct ts3a227e *ts3a227e = dev_get_drvdata(dev);
+
+	dev_dbg(ts3a227e->dev, "resume enable irq\n");
+	enable_irq(ts3a227e->irq);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops ts3a227e_pm = {
+	SET_SYSTEM_SLEEP_PM_OPS(ts3a227e_suspend, ts3a227e_resume)
+};
+
 static const struct i2c_device_id ts3a227e_i2c_ids[] = {
 	{ "ts3a227e", 0 },
 	{ }
@@ -336,6 +378,7 @@ static struct i2c_driver ts3a227e_driver = {
 	.driver = {
 		.name = "ts3a227e",
 		.owner = THIS_MODULE,
+		.pm = &ts3a227e_pm,
 		.of_match_table = of_match_ptr(ts3a227e_of_match),
 	},
 	.probe = ts3a227e_i2c_probe,
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] ASoC: ts3a227e: do not report jack status when there is i2c read err
  2015-07-01 23:24 ` [PATCH v2] " yang.a.fang
@ 2015-07-07 13:47   ` Mark Brown
  2015-07-07 21:21   ` [PATCH v3] " yang.a.fang
  1 sibling, 0 replies; 9+ messages in thread
From: Mark Brown @ 2015-07-07 13:47 UTC (permalink / raw)
  To: yang.a.fang
  Cc: alsa-devel, srinivas.sripathi, praveen.k.jain, lgirdwood,
	denny.iriawan, sathyanarayana.nujella, kevin.strasser, dgreid,
	mika.westerberg


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

On Wed, Jul 01, 2015 at 04:24:48PM -0700, yang.a.fang@intel.com wrote:

> after TI resumes. Also We should return if there is read err,the
> interrupt will come again since it is level triggered and we are not yet
> clear the interrupt.

> +	ret = regmap_read(regmap, TS3A227E_REG_INTERRUPT, &int_reg);
> +	if (ret) {
> +		dev_err(dev, "failed to clear interrupt ret=%d\n", ret);
> +		return IRQ_HANDLED;
> +	}

This should return IRQ_NONE not IRQ_HANDLED - we didn't actually
successfully handle the interrupt.  This lets the genirq code error
handling kick in if for some reason things are really broken and we just
can't interact with the device any more, it will eventually disable the
interrupt in that case.

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

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3] ASoC: ts3a227e: do not report jack status when there is i2c read err
  2015-07-01 23:24 ` [PATCH v2] " yang.a.fang
  2015-07-07 13:47   ` Mark Brown
@ 2015-07-07 21:21   ` yang.a.fang
  1 sibling, 0 replies; 9+ messages in thread
From: yang.a.fang @ 2015-07-07 21:21 UTC (permalink / raw)
  To: broonie, lgirdwood
  Cc: alsa-devel, srinivas.sripathi, Fang, Yang A, praveen.k.jain,
	denny.iriawan, sathyanarayana.nujella, kevin.strasser, dgreid

From: "Fang, Yang A" <yang.a.fang@intel.com>

After suspend -> resume the ts3a227e_interrupt sometimes comes before i2c
controller resume is called .regmap_read will return incorrect status
and report a wrong jack status.This patch will disable irq on suspend
and enable irq again on the resume to make sure interrupt is coming
after TI resumes. Also We should return if there is read err,the
interrupt will come again since it is level triggered and we are not yet
clear the interrupt.

In addtion,cht_bsw_max98090_ti machine driver registered additional
notifier base on jack event which will program the audio codec.there
will be codec timeout err if such event occurs prior to i2c controller
is resumed.

Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
---
 sound/soc/codecs/ts3a227e.c |   47 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/ts3a227e.c b/sound/soc/codecs/ts3a227e.c
index 12232d7..ffc6f30 100644
--- a/sound/soc/codecs/ts3a227e.c
+++ b/sound/soc/codecs/ts3a227e.c
@@ -23,11 +23,13 @@
 #include "ts3a227e.h"
 
 struct ts3a227e {
+	struct device *dev;
 	struct regmap *regmap;
 	struct snd_soc_jack *jack;
 	bool plugged;
 	bool mic_present;
 	unsigned int buttons_held;
+	int irq;
 };
 
 /* Button values to be reported on the jack */
@@ -189,16 +191,28 @@ static irqreturn_t ts3a227e_interrupt(int irq, void *data)
 	struct ts3a227e *ts3a227e = (struct ts3a227e *)data;
 	struct regmap *regmap = ts3a227e->regmap;
 	unsigned int int_reg, kp_int_reg, acc_reg, i;
+	struct device *dev = ts3a227e->dev;
+	int ret;
 
 	/* Check for plug/unplug. */
-	regmap_read(regmap, TS3A227E_REG_INTERRUPT, &int_reg);
+	ret = regmap_read(regmap, TS3A227E_REG_INTERRUPT, &int_reg);
+	if (ret) {
+		dev_err(dev, "failed to clear interrupt ret=%d\n", ret);
+		return IRQ_NONE;
+	}
+
 	if (int_reg & (DETECTION_COMPLETE_EVENT | INS_REM_EVENT)) {
 		regmap_read(regmap, TS3A227E_REG_ACCESSORY_STATUS, &acc_reg);
 		ts3a227e_new_jack_state(ts3a227e, acc_reg);
 	}
 
 	/* Report any key events. */
-	regmap_read(regmap, TS3A227E_REG_KP_INTERRUPT, &kp_int_reg);
+	ret = regmap_read(regmap, TS3A227E_REG_KP_INTERRUPT, &kp_int_reg);
+	if (ret) {
+		dev_err(dev, "failed to clear key interrupt ret=%d\n", ret);
+		return IRQ_NONE;
+	}
+
 	for (i = 0; i < TS3A227E_NUM_BUTTONS; i++) {
 		if (kp_int_reg & PRESS_MASK(i))
 			ts3a227e->buttons_held |= (1 << i);
@@ -283,6 +297,8 @@ static int ts3a227e_i2c_probe(struct i2c_client *i2c,
 		return -ENOMEM;
 
 	i2c_set_clientdata(i2c, ts3a227e);
+	ts3a227e->dev = dev;
+	ts3a227e->irq = i2c->irq;
 
 	ts3a227e->regmap = devm_regmap_init_i2c(i2c, &ts3a227e_regmap_config);
 	if (IS_ERR(ts3a227e->regmap))
@@ -320,6 +336,32 @@ static int ts3a227e_i2c_probe(struct i2c_client *i2c,
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int ts3a227e_suspend(struct device *dev)
+{
+	struct ts3a227e *ts3a227e = dev_get_drvdata(dev);
+
+	dev_dbg(ts3a227e->dev, "suspend disable irq\n");
+	disable_irq(ts3a227e->irq);
+
+	return 0;
+}
+
+static int ts3a227e_resume(struct device *dev)
+{
+	struct ts3a227e *ts3a227e = dev_get_drvdata(dev);
+
+	dev_dbg(ts3a227e->dev, "resume enable irq\n");
+	enable_irq(ts3a227e->irq);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops ts3a227e_pm = {
+	SET_SYSTEM_SLEEP_PM_OPS(ts3a227e_suspend, ts3a227e_resume)
+};
+
 static const struct i2c_device_id ts3a227e_i2c_ids[] = {
 	{ "ts3a227e", 0 },
 	{ }
@@ -336,6 +378,7 @@ static struct i2c_driver ts3a227e_driver = {
 	.driver = {
 		.name = "ts3a227e",
 		.owner = THIS_MODULE,
+		.pm = &ts3a227e_pm,
 		.of_match_table = of_match_ptr(ts3a227e_of_match),
 	},
 	.probe = ts3a227e_i2c_probe,
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-07-07 21:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-01  4:18 [PATCH] ASoC: ts3a227e: do not report jack status when there is i2c read err yang.a.fang
2015-07-01 17:12 ` Dylan Reid
2015-07-01 17:34   ` Yang Fang
2015-07-01 18:05     ` Mark Brown
2015-07-01 18:24       ` Fang, Yang A
2015-07-01 23:20         ` Fang, Yang A
2015-07-01 23:24 ` [PATCH v2] " yang.a.fang
2015-07-07 13:47   ` Mark Brown
2015-07-07 21:21   ` [PATCH v3] " yang.a.fang

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.