* Re: [PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached" [not found] <20171216011230.107527-1-briannorris@chromium.org> @ 2017-12-16 3:17 ` Brian Norris 2017-12-18 4:23 ` Cheng-yi Chiang 0 siblings, 1 reply; 6+ messages in thread From: Brian Norris @ 2017-12-16 3:17 UTC (permalink / raw) To: Oder Chiou, Bard Liao, Liam Girdwood, Mark Brown Cc: alsa-devel, Jimmy Cheng-Yi Chiang, Jeffy Chen, linux-kernel, Takashi Iwai, linux-rockchip, Matthias Kaehlcke + 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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached" 2017-12-16 3:17 ` [PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached" Brian Norris @ 2017-12-18 4:23 ` Cheng-yi Chiang 2017-12-18 17:42 ` Brian Norris 0 siblings, 1 reply; 6+ messages in thread From: Cheng-yi Chiang @ 2017-12-18 4:23 UTC (permalink / raw) To: Brian Norris Cc: Oder Chiou, alsa-devel, linux-kernel, Jeffy Chen, Takashi Iwai, Liam Girdwood, linux-rockchip, Mark Brown, Bard Liao, Matthias Kaehlcke Hi Brian, Oder has posted the same fix : https://patchwork.kernel.org/ patch/10066257/ and it has been applied. Perhaps you can cherry-pick it to v4.15 ? Thanks! On Sat, Dec 16, 2017 at 11:17 AM, Brian Norris <briannorris@chromium.org> wrote: > + 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 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached" 2017-12-18 4:23 ` Cheng-yi Chiang @ 2017-12-18 17:42 ` Brian Norris 2017-12-19 7:38 ` Cheng-yi Chiang 2017-12-19 10:58 ` Mark Brown 0 siblings, 2 replies; 6+ messages in thread From: Brian Norris @ 2017-12-18 17:42 UTC (permalink / raw) To: Cheng-yi Chiang Cc: Oder Chiou, alsa-devel, linux-kernel, Jeffy Chen, Takashi Iwai, Liam Girdwood, linux-rockchip, Mark Brown, Bard Liao, Matthias Kaehlcke Hi! (By the way, your mail is HTML and likely will get rejected by many mailing lists and/or people reading these mailing lists.) On Mon, Dec 18, 2017 at 12:23:18PM +0800, Cheng-yi Chiang wrote: > Hi Brian, > Oder has posted the same fix : [1]https://patchwork.kernel.org/ > patch/10066257/ and it has been applied. OK cool. Obviously I'm biased, but I prefer mine, as it has less needless whitespace, and is appropriately documented ;) But it should be a fine replacement. > Perhaps you can cherry-pick it to v4.15 ? I have no say in that; that would be up to Mark, I think. It's most certainly a regression in 4.15-rc1, so IMO it should be given a proper 'Fixes: e9c50aa6bd39 ("ASoC: rt5514-spi: check irq status to schedule data copy in resume function")' tag and sent for 4.15, not 4.16. Brian ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached" 2017-12-18 17:42 ` Brian Norris @ 2017-12-19 7:38 ` Cheng-yi Chiang 2017-12-19 10:58 ` Mark Brown 1 sibling, 0 replies; 6+ messages in thread From: Cheng-yi Chiang @ 2017-12-19 7:38 UTC (permalink / raw) To: Brian Norris Cc: Oder Chiou, alsa-devel, linux-kernel, Jeffy Chen, Takashi Iwai, Liam Girdwood, linux-rockchip, Mark Brown, Bard Liao, Matthias Kaehlcke Hi Brian, I am sorry for not using the plain text mode in the previous mail. I agree with you on other points. On Tue, Dec 19, 2017 at 1:42 AM, Brian Norris <briannorris@chromium.org> wrote: > Hi! > > (By the way, your mail is HTML and likely will get rejected by many > mailing lists and/or people reading these mailing lists.) > > On Mon, Dec 18, 2017 at 12:23:18PM +0800, Cheng-yi Chiang wrote: >> Hi Brian, >> Oder has posted the same fix : [1]https://patchwork.kernel.org/ >> patch/10066257/ and it has been applied. > > OK cool. Obviously I'm biased, but I prefer mine, as it has less > needless whitespace, and is appropriately documented ;) But it should be > a fine replacement. > >> Perhaps you can cherry-pick it to v4.15 ? > > I have no say in that; that would be up to Mark, I think. It's most > certainly a regression in 4.15-rc1, so IMO it should be given a proper > 'Fixes: e9c50aa6bd39 ("ASoC: rt5514-spi: check irq status to schedule > data copy in resume function")' tag and sent for 4.15, not 4.16. > > Brian ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached" 2017-12-18 17:42 ` Brian Norris 2017-12-19 7:38 ` Cheng-yi Chiang @ 2017-12-19 10:58 ` Mark Brown 2017-12-19 17:31 ` Brian Norris 1 sibling, 1 reply; 6+ messages in thread From: Mark Brown @ 2017-12-19 10:58 UTC (permalink / raw) To: Brian Norris Cc: Oder Chiou, alsa-devel, Liam Girdwood, linux-kernel, Jeffy Chen, Takashi Iwai, linux-rockchip, Matthias Kaehlcke, Bard Liao, Cheng-yi Chiang [-- Attachment #1.1: Type: text/plain, Size: 644 bytes --] On Mon, Dec 18, 2017 at 09:42:36AM -0800, Brian Norris wrote: > On Mon, Dec 18, 2017 at 12:23:18PM +0800, Cheng-yi Chiang wrote: > > Hi Brian, > > Oder has posted the same fix : [1]https://patchwork.kernel.org/ > > patch/10066257/ and it has been applied. > > Perhaps you can cherry-pick it to v4.15 ? > I have no say in that; that would be up to Mark, I think. It's most > certainly a regression in 4.15-rc1, so IMO it should be given a proper > 'Fixes: e9c50aa6bd39 ("ASoC: rt5514-spi: check irq status to schedule > data copy in resume function")' tag and sent for 4.15, not 4.16. It's been applied as a fix for some time. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached" 2017-12-19 10:58 ` Mark Brown @ 2017-12-19 17:31 ` Brian Norris 0 siblings, 0 replies; 6+ messages in thread From: Brian Norris @ 2017-12-19 17:31 UTC (permalink / raw) To: Mark Brown Cc: Cheng-yi Chiang, Oder Chiou, Bard Liao, Liam Girdwood, Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-kernel, Matthias Kaehlcke, Jeffy Chen, linux-rockchip On Tue, Dec 19, 2017 at 10:58:18AM +0000, Mark Brown wrote: > It's been applied as a fix for some time. Indeed it has. Sorry for missing that. I look forward to seeing it in a release candidate, so my system will again work on mainline :) Brian ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-19 17:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20171216011230.107527-1-briannorris@chromium.org>
2017-12-16 3:17 ` [PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached" Brian Norris
2017-12-18 4:23 ` Cheng-yi Chiang
2017-12-18 17:42 ` Brian Norris
2017-12-19 7:38 ` Cheng-yi Chiang
2017-12-19 10:58 ` Mark Brown
2017-12-19 17:31 ` Brian Norris
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox