All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: Oder Chiou <oder_chiou@realtek.com>,
	Bard Liao <bardliao@realtek.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
	Matthias Kaehlcke <mka@chromium.org>,
	Jeffy Chen <jeffy.chen@rock-chips.com>,
	linux-rockchip@lists.infradead.org,
	Jimmy Cheng-Yi Chiang <cychiang@google.com>,
	Brian Norris <briannorris@chromium.org>
Subject: [PATCH for-4.15] ASoC: rt5514-spi: only enable wakeup when fully initialized
Date: Fri, 15 Dec 2017 20:07:23 -0800	[thread overview]
Message-ID: <20171216040723.11710-1-briannorris@chromium.org> (raw)

If an rt5514-spi device is probed but the platform hasn't linked it in,
we might never fully request the SPI IRQ, nor configure the rt5514 DSP,
but we still might try to enable the SPI IRQ (enable_irq_wake()). This
is bad, and among other things, can cause the interrupt to trigger every
time we try to suspend the system (e.g., because the interrupt trigger
setting was never set properly).

Instead of setting our wakeup capabilities in the SPI driver probe
routine, let's wait until we've actually requested the IRQ.

Fixes issues seen on the "kevin" Chromebook (Samsung Chromebook Plus).

Fixes: 58f1c07d23cd ("ASoC: rt5514: Voice wakeup support.")
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
The patch that introduced the wakeup was in 4.15-rc1. I think this qualifies as
a bugfix, and should probably go into 4.15.

Other relevant patches that are useful, and fix similar (or the same) issues:

https://patchwork.kernel.org/patch/10067725/
[PATCH] ASoC: rockchip: Use dummy_dai for rt5514 dsp dailink

https://patchwork.kernel.org/patch/10116761/
[PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached"

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

diff --git a/sound/soc/codecs/rt5514-spi.c b/sound/soc/codecs/rt5514-spi.c
index 9255afcf2c3a..48e4b9002caf 100644
--- a/sound/soc/codecs/rt5514-spi.c
+++ b/sound/soc/codecs/rt5514-spi.c
@@ -289,6 +289,8 @@ static int rt5514_spi_pcm_probe(struct snd_soc_platform *platform)
 			dev_err(&rt5514_spi->dev,
 				"%s Failed to reguest IRQ: %d\n", __func__,
 				ret);
+		else
+			device_init_wakeup(rt5514_dsp->dev, true);
 	}
 
 	return 0;
@@ -456,8 +458,6 @@ static int rt5514_spi_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	device_init_wakeup(&spi->dev, true);
-
 	return 0;
 }
 
-- 
2.15.1.504.g5279b80103-goog

WARNING: multiple messages have this Message-ID (diff)
From: Brian Norris <briannorris@chromium.org>
To: Oder Chiou <oder_chiou@realtek.com>,
	Bard Liao <bardliao@realtek.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>
Cc: alsa-devel@alsa-project.org,
	Brian Norris <briannorris@chromium.org>,
	Jimmy Cheng-Yi Chiang <cychiang@google.com>,
	Jeffy Chen <jeffy.chen@rock-chips.com>,
	linux-kernel@vger.kernel.org, Takashi Iwai <tiwai@suse.com>,
	linux-rockchip@lists.infradead.org,
	Matthias Kaehlcke <mka@chromium.org>
Subject: [PATCH for-4.15] ASoC: rt5514-spi: only enable wakeup when fully initialized
Date: Fri, 15 Dec 2017 20:07:23 -0800	[thread overview]
Message-ID: <20171216040723.11710-1-briannorris@chromium.org> (raw)

If an rt5514-spi device is probed but the platform hasn't linked it in,
we might never fully request the SPI IRQ, nor configure the rt5514 DSP,
but we still might try to enable the SPI IRQ (enable_irq_wake()). This
is bad, and among other things, can cause the interrupt to trigger every
time we try to suspend the system (e.g., because the interrupt trigger
setting was never set properly).

Instead of setting our wakeup capabilities in the SPI driver probe
routine, let's wait until we've actually requested the IRQ.

Fixes issues seen on the "kevin" Chromebook (Samsung Chromebook Plus).

Fixes: 58f1c07d23cd ("ASoC: rt5514: Voice wakeup support.")
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
The patch that introduced the wakeup was in 4.15-rc1. I think this qualifies as
a bugfix, and should probably go into 4.15.

Other relevant patches that are useful, and fix similar (or the same) issues:

https://patchwork.kernel.org/patch/10067725/
[PATCH] ASoC: rockchip: Use dummy_dai for rt5514 dsp dailink

https://patchwork.kernel.org/patch/10116761/
[PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached"

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

diff --git a/sound/soc/codecs/rt5514-spi.c b/sound/soc/codecs/rt5514-spi.c
index 9255afcf2c3a..48e4b9002caf 100644
--- a/sound/soc/codecs/rt5514-spi.c
+++ b/sound/soc/codecs/rt5514-spi.c
@@ -289,6 +289,8 @@ static int rt5514_spi_pcm_probe(struct snd_soc_platform *platform)
 			dev_err(&rt5514_spi->dev,
 				"%s Failed to reguest IRQ: %d\n", __func__,
 				ret);
+		else
+			device_init_wakeup(rt5514_dsp->dev, true);
 	}
 
 	return 0;
@@ -456,8 +458,6 @@ static int rt5514_spi_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	device_init_wakeup(&spi->dev, true);
-
 	return 0;
 }
 
-- 
2.15.1.504.g5279b80103-goog

             reply	other threads:[~2017-12-16  4:07 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-16  4:07 Brian Norris [this message]
2017-12-16  4:07 ` [PATCH for-4.15] ASoC: rt5514-spi: only enable wakeup when fully initialized Brian Norris
2017-12-19 11:09 ` Applied "ASoC: rt5514-spi: only enable wakeup when fully initialized" to the asoc tree Mark Brown
2017-12-19 11:09   ` Mark Brown
2017-12-19 11:09   ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171216040723.11710-1-briannorris@chromium.org \
    --to=briannorris@chromium.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=bardliao@realtek.com \
    --cc=broonie@kernel.org \
    --cc=cychiang@google.com \
    --cc=jeffy.chen@rock-chips.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mka@chromium.org \
    --cc=oder_chiou@realtek.com \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.