ASoC: rt274: Disable jack report IRQ with disabling jack
diff mbox series

Message ID 20191112130237.10141-2-pawel.harlozinski@linux.intel.com
State New
Headers show
Series
  • ASoC: rt274: Disable jack report IRQ with disabling jack
Related show

Commit Message

Harlozinski, Pawel Nov. 12, 2019, 1:02 p.m. UTC
When machine driver is unloaded there is nothing to handle
jack detect reports. This were causing flood of messages
from unhandled IRQs.

Signed-off-by: Pawel Harlozinski <pawel.harlozinski@linux.intel.com>

---
 sound/soc/codecs/rt274.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Mark Brown Nov. 12, 2019, 5:10 p.m. UTC | #1
On Tue, Nov 12, 2019 at 02:02:37PM +0100, Pawel Harlozinski wrote:

>  		/* Disable jack detection */
>  		regmap_update_bits(rt274->regmap, RT274_EAPD_GPIO_IRQ_CTRL,
>  					RT274_IRQ_EN, RT274_IRQ_DIS);
> -
> +		disable_irq(rt274->i2c->irq);
>  		return 0;

Shouldn't the register update above be suppressing interrupts?
disable_irq() is a bit of a hammer and interferes with things like
possible share use.
Harlozinski, Pawel Nov. 13, 2019, 1:55 p.m. UTC | #2
On 11/12/2019 6:10 PM, Mark Brown wrote:
> On Tue, Nov 12, 2019 at 02:02:37PM +0100, Pawel Harlozinski wrote:
>
>>   		/* Disable jack detection */
>>   		regmap_update_bits(rt274->regmap, RT274_EAPD_GPIO_IRQ_CTRL,
>>   					RT274_IRQ_EN, RT274_IRQ_DIS);
>> -
>> +		disable_irq(rt274->i2c->irq);
>>   		return 0;
> Shouldn't the register update above be suppressing interrupts?
For rt274 disable_irq is also needed, otherwise we're getting flood of 
irq's in case of not loaded machine board.
> disable_irq() is a bit of a hammer and interferes with things like
> possible share use.

This irq should be handled in codec code anyway - control of jack detect 
events from non-codec code is done with set_jack.
Similar solutions for jack report irq enable/disable flow are 
implemented in rt5640 and rt5651.

> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Mark Brown Nov. 13, 2019, 5 p.m. UTC | #3
On Wed, Nov 13, 2019 at 02:55:53PM +0100, Harlozinski, Pawel wrote:
> On 11/12/2019 6:10 PM, Mark Brown wrote:
> > On Tue, Nov 12, 2019 at 02:02:37PM +0100, Pawel Harlozinski wrote:

> > >   		/* Disable jack detection */
> > >   		regmap_update_bits(rt274->regmap, RT274_EAPD_GPIO_IRQ_CTRL,
> > >   					RT274_IRQ_EN, RT274_IRQ_DIS);
> > > -
> > > +		disable_irq(rt274->i2c->irq);
> > >   		return 0;

> > Shouldn't the register update above be suppressing interrupts?
> For rt274 disable_irq is also needed, otherwise we're getting flood of irq's
> in case of not loaded machine board.

Through what mechanism is it needed?  If your machine driver is having
an impact on this I'm rather worried.

> > disable_irq() is a bit of a hammer and interferes with things like
> > possible share use.

> This irq should be handled in codec code anyway - control of jack detect
> events from non-codec code is done with set_jack.

The issue isn't that this is in CODEC code, the issue is that it's
usually very worrying to need to explicitly disable and enable an
interrupt at the controller level when the device appears to have
controls that should stop it asserting the interrupt when it's not
wanted.

> Similar solutions for jack report irq enable/disable flow are implemented in
> rt5640 and rt5651.

This may be an indication that those drivers should be improved rather
than that they should be copied.

Patch
diff mbox series

diff --git a/sound/soc/codecs/rt274.c b/sound/soc/codecs/rt274.c
index cbb5e176d11a..993fd365524a 100644
--- a/sound/soc/codecs/rt274.c
+++ b/sound/soc/codecs/rt274.c
@@ -408,13 +408,13 @@  static int rt274_mic_detect(struct snd_soc_component *component,
 		/* Disable jack detection */
 		regmap_update_bits(rt274->regmap, RT274_EAPD_GPIO_IRQ_CTRL,
 					RT274_IRQ_EN, RT274_IRQ_DIS);
-
+		disable_irq(rt274->i2c->irq);
 		return 0;
 	}
 
 	regmap_update_bits(rt274->regmap, RT274_EAPD_GPIO_IRQ_CTRL,
 				RT274_IRQ_EN, RT274_IRQ_EN);
-
+	enable_irq(rt274->i2c->irq);
 	/* Send an initial report */
 	rt274_irq(0, rt274);
 
@@ -1197,6 +1197,8 @@  static int rt274_i2c_probe(struct i2c_client *i2c,
 				"Failed to reguest IRQ: %d\n", ret);
 			return ret;
 		}
+		/* Gets re-enabled by .set_jack = rt274_mic_detect  */
+		disable_irq(rt274->i2c->irq);
 	}
 
 	ret = devm_snd_soc_register_component(&i2c->dev,