| Message ID | 20191112130237.10141-2-pawel.harlozinski@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: rt274: Disable jack report IRQ with disabling jack | expand |
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.
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
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.
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,
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(-)