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>
Subject: Re: [PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached"
Date: Fri, 15 Dec 2017 19:17:39 -0800	[thread overview]
Message-ID: <20171216031738.GA220575@google.com> (raw)
In-Reply-To: <20171216011230.107527-1-briannorris@chromium.org>

+ others

On Fri, Dec 15, 2017 at 05:12:30PM -0800, Brian Norris wrote:
> I've found that on Google's "Kevin" Chromebook, the rt5514 codec might
> not be set up completely, yet its device is still present, and therefore
> its PM suspend/resume is called. This hits a NULL pointer exception,
> since we never had the chance to set our drvdata pointer.
> 
> This resolves crashes seen when trying to resume my system.
> 
> Fixes: e9c50aa6bd39 ("ASoC: rt5514-spi: check irq status to schedule data copy in resume function")
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> This is a v4.15-rc1 regression

I see that this is probably a bug on its own, which should be fixed on
its own (e.g., with the $subject patch), but FYI I was also pointed at
this patch, which also "resolves" this problem by fixing the rt5514 DAI
setup:

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

Would be nice to see it get handled too, possibly in 4.15 as well (I
think that's a regression too?).

Brian

> 
>  sound/soc/codecs/rt5514-spi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/rt5514-spi.c b/sound/soc/codecs/rt5514-spi.c
> index 2df91db765ac..9255afcf2c3a 100644
> --- a/sound/soc/codecs/rt5514-spi.c
> +++ b/sound/soc/codecs/rt5514-spi.c
> @@ -482,7 +482,7 @@ static int __maybe_unused rt5514_resume(struct device *dev)
>  	if (device_may_wakeup(dev))
>  		disable_irq_wake(irq);
>  
> -	if (rt5514_dsp->substream) {
> +	if (rt5514_dsp && rt5514_dsp->substream) {
>  		rt5514_spi_burst_read(RT5514_IRQ_CTRL, (u8 *)&buf, sizeof(buf));
>  		if (buf[0] & RT5514_IRQ_STATUS_BIT)
>  			rt5514_schedule_copy(rt5514_dsp);
> -- 
> 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,
	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: Re: [PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached"
Date: Fri, 15 Dec 2017 19:17:39 -0800	[thread overview]
Message-ID: <20171216031738.GA220575@google.com> (raw)
In-Reply-To: <20171216011230.107527-1-briannorris@chromium.org>

+ others

On Fri, Dec 15, 2017 at 05:12:30PM -0800, Brian Norris wrote:
> I've found that on Google's "Kevin" Chromebook, the rt5514 codec might
> not be set up completely, yet its device is still present, and therefore
> its PM suspend/resume is called. This hits a NULL pointer exception,
> since we never had the chance to set our drvdata pointer.
> 
> This resolves crashes seen when trying to resume my system.
> 
> Fixes: e9c50aa6bd39 ("ASoC: rt5514-spi: check irq status to schedule data copy in resume function")
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> This is a v4.15-rc1 regression

I see that this is probably a bug on its own, which should be fixed on
its own (e.g., with the $subject patch), but FYI I was also pointed at
this patch, which also "resolves" this problem by fixing the rt5514 DAI
setup:

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

Would be nice to see it get handled too, possibly in 4.15 as well (I
think that's a regression too?).

Brian

> 
>  sound/soc/codecs/rt5514-spi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/rt5514-spi.c b/sound/soc/codecs/rt5514-spi.c
> index 2df91db765ac..9255afcf2c3a 100644
> --- a/sound/soc/codecs/rt5514-spi.c
> +++ b/sound/soc/codecs/rt5514-spi.c
> @@ -482,7 +482,7 @@ static int __maybe_unused rt5514_resume(struct device *dev)
>  	if (device_may_wakeup(dev))
>  		disable_irq_wake(irq);
>  
> -	if (rt5514_dsp->substream) {
> +	if (rt5514_dsp && rt5514_dsp->substream) {
>  		rt5514_spi_burst_read(RT5514_IRQ_CTRL, (u8 *)&buf, sizeof(buf));
>  		if (buf[0] & RT5514_IRQ_STATUS_BIT)
>  			rt5514_schedule_copy(rt5514_dsp);
> -- 
> 2.15.1.504.g5279b80103-goog
> 

  reply	other threads:[~2017-12-16  3:17 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-16  1:12 [PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached" Brian Norris
2017-12-16  1:12 ` Brian Norris
2017-12-16  3:17 ` Brian Norris [this message]
2017-12-16  3:17   ` Brian Norris
2017-12-18  4:23   ` Cheng-yi Chiang
2017-12-18 17:42     ` Brian Norris
2017-12-18 17:42       ` Brian Norris
2017-12-19  7:38       ` Cheng-yi Chiang
2017-12-19  7:38         ` Cheng-yi Chiang
2017-12-19 10:58       ` Mark Brown
2017-12-19 10:58         ` Mark Brown
2017-12-19 17:31         ` Brian Norris
2018-01-24 15:36 ` Applied "ASoC: rt5514-spi: Check the validity of drvdata pointer on resume" to the asoc tree Mark Brown
2018-01-24 15:36   ` Mark Brown
2018-01-24 15:47   ` Mark Brown
2018-01-24 17:07   ` Brian Norris

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=20171216031738.GA220575@google.com \
    --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.