[PATCH] Fix kbl_rt5663_rt5514_max98927 regression
Starting with 6.0-rc1 these messages are logged and the sound card is unavailable. Adding legacy_dai_naming to the rt5514-spi causes it to function properly again.
[ 16.928454] kbl_r5514_5663_max kbl_r5514_5663_max: ASoC: CPU DAI spi-PRP0001:00 not registered [ 16.928561] platform kbl_r5514_5663_max: deferred probe pending --- sound/soc/codecs/rt5514-spi.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/sound/soc/codecs/rt5514-spi.c b/sound/soc/codecs/rt5514-spi.c index 1a25a3787935..362663abcb89 100644 --- a/sound/soc/codecs/rt5514-spi.c +++ b/sound/soc/codecs/rt5514-spi.c @@ -298,13 +298,14 @@ static int rt5514_spi_pcm_new(struct snd_soc_component *component, }
static const struct snd_soc_component_driver rt5514_spi_component = { - .name = DRV_NAME, - .probe = rt5514_spi_pcm_probe, - .open = rt5514_spi_pcm_open, - .hw_params = rt5514_spi_hw_params, - .hw_free = rt5514_spi_hw_free, - .pointer = rt5514_spi_pcm_pointer, - .pcm_construct = rt5514_spi_pcm_new, + .name = DRV_NAME, + .probe = rt5514_spi_pcm_probe, + .open = rt5514_spi_pcm_open, + .hw_params = rt5514_spi_hw_params, + .hw_free = rt5514_spi_hw_free, + .pointer = rt5514_spi_pcm_pointer, + .pcm_construct = rt5514_spi_pcm_new, + .legacy_dai_naming = 1, };
/**
On 11/2/22 16:05, Jason Montleon wrote:
Starting with 6.0-rc1 these messages are logged and the sound card is unavailable. Adding legacy_dai_naming to the rt5514-spi causes it to function properly again.
[ 16.928454] kbl_r5514_5663_max kbl_r5514_5663_max: ASoC: CPU DAI spi-PRP0001:00 not registered [ 16.928561] platform kbl_r5514_5663_max: deferred probe pending
Thanks for reporting this regression, much appreciated.
a) you need to CC: maintainers Mark Brown and Takashi Iwai b) the commit title should be something like "ASoC: rt5514: fix legacy dai naming". c) it's not clear if this is actually enough. there's no legacy_dai_naming for e.g. rt5663 and the .endianness member is not set.
Adding Charles Keepax for comments.
sound/soc/codecs/rt5514-spi.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/sound/soc/codecs/rt5514-spi.c b/sound/soc/codecs/rt5514-spi.c index 1a25a3787935..362663abcb89 100644 --- a/sound/soc/codecs/rt5514-spi.c +++ b/sound/soc/codecs/rt5514-spi.c @@ -298,13 +298,14 @@ static int rt5514_spi_pcm_new(struct snd_soc_component *component, }
static const struct snd_soc_component_driver rt5514_spi_component = {
- .name = DRV_NAME,
- .probe = rt5514_spi_pcm_probe,
- .open = rt5514_spi_pcm_open,
- .hw_params = rt5514_spi_hw_params,
- .hw_free = rt5514_spi_hw_free,
- .pointer = rt5514_spi_pcm_pointer,
- .pcm_construct = rt5514_spi_pcm_new,
- .name = DRV_NAME,
- .probe = rt5514_spi_pcm_probe,
- .open = rt5514_spi_pcm_open,
- .hw_params = rt5514_spi_hw_params,
- .hw_free = rt5514_spi_hw_free,
- .pointer = rt5514_spi_pcm_pointer,
- .pcm_construct = rt5514_spi_pcm_new,
- .legacy_dai_naming = 1,
};
/**
On Wed, 02 Nov 2022 23:05:14 +0100, Pierre-Louis Bossart wrote:
On 11/2/22 16:05, Jason Montleon wrote:
Starting with 6.0-rc1 these messages are logged and the sound card is unavailable. Adding legacy_dai_naming to the rt5514-spi causes it to function properly again.
[ 16.928454] kbl_r5514_5663_max kbl_r5514_5663_max: ASoC: CPU DAI spi-PRP0001:00 not registered [ 16.928561] platform kbl_r5514_5663_max: deferred probe pending
Thanks for reporting this regression, much appreciated.
a) you need to CC: maintainers Mark Brown and Takashi Iwai b) the commit title should be something like "ASoC: rt5514: fix legacy dai naming". c) it's not clear if this is actually enough. there's no legacy_dai_naming for e.g. rt5663 and the .endianness member is not set.
IIUC, rt5663.c should be fine; it used to have non_legacy_dai_naming flag and it was dropped after the switch.
But, through a quick glance, rt5677-spi.c seems to be the same pattern as rt5514-spi.c. The rt5677.c was covered properly but the *-spi.c wan't.
Takashi
Adding Charles Keepax for comments.
sound/soc/codecs/rt5514-spi.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/sound/soc/codecs/rt5514-spi.c b/sound/soc/codecs/rt5514-spi.c index 1a25a3787935..362663abcb89 100644 --- a/sound/soc/codecs/rt5514-spi.c +++ b/sound/soc/codecs/rt5514-spi.c @@ -298,13 +298,14 @@ static int rt5514_spi_pcm_new(struct snd_soc_component *component, }
static const struct snd_soc_component_driver rt5514_spi_component = {
- .name = DRV_NAME,
- .probe = rt5514_spi_pcm_probe,
- .open = rt5514_spi_pcm_open,
- .hw_params = rt5514_spi_hw_params,
- .hw_free = rt5514_spi_hw_free,
- .pointer = rt5514_spi_pcm_pointer,
- .pcm_construct = rt5514_spi_pcm_new,
- .name = DRV_NAME,
- .probe = rt5514_spi_pcm_probe,
- .open = rt5514_spi_pcm_open,
- .hw_params = rt5514_spi_hw_params,
- .hw_free = rt5514_spi_hw_free,
- .pointer = rt5514_spi_pcm_pointer,
- .pcm_construct = rt5514_spi_pcm_new,
- .legacy_dai_naming = 1,
};
/**
On Thu, Nov 03, 2022 at 08:59:03AM +0100, Takashi Iwai wrote:
On Wed, 02 Nov 2022 23:05:14 +0100, Pierre-Louis Bossart wrote:
On 11/2/22 16:05, Jason Montleon wrote:
Starting with 6.0-rc1 these messages are logged and the sound card is unavailable. Adding legacy_dai_naming to the rt5514-spi causes it to function properly again.
[ 16.928454] kbl_r5514_5663_max kbl_r5514_5663_max: ASoC: CPU DAI spi-PRP0001:00 not registered [ 16.928561] platform kbl_r5514_5663_max: deferred probe pending
Thanks for reporting this regression, much appreciated.
a) you need to CC: maintainers Mark Brown and Takashi Iwai b) the commit title should be something like "ASoC: rt5514: fix legacy dai naming". c) it's not clear if this is actually enough. there's no legacy_dai_naming for e.g. rt5663 and the .endianness member is not set.
IIUC, rt5663.c should be fine; it used to have non_legacy_dai_naming flag and it was dropped after the switch.
But, through a quick glance, rt5677-spi.c seems to be the same pattern as rt5514-spi.c. The rt5677.c was covered properly but the *-spi.c wan't.
Yeah I think these got missed as they are effectively CPU side devices but living in the CODEC space. Looks like it would be reasonable to add legacy_dai_naming to both of them to me.
Thanks, Charles
static const struct snd_soc_component_driver rt5514_spi_component = {
- .name = DRV_NAME,
- .probe = rt5514_spi_pcm_probe,
- .open = rt5514_spi_pcm_open,
- .hw_params = rt5514_spi_hw_params,
- .hw_free = rt5514_spi_hw_free,
- .pointer = rt5514_spi_pcm_pointer,
- .pcm_construct = rt5514_spi_pcm_new,
- .name = DRV_NAME,
- .probe = rt5514_spi_pcm_probe,
- .open = rt5514_spi_pcm_open,
- .hw_params = rt5514_spi_hw_params,
- .hw_free = rt5514_spi_hw_free,
- .pointer = rt5514_spi_pcm_pointer,
- .pcm_construct = rt5514_spi_pcm_new,
- .legacy_dai_naming = 1,
};
/**
On Thu, 03 Nov 2022 10:54:04 +0100, Charles Keepax wrote:
On Thu, Nov 03, 2022 at 08:59:03AM +0100, Takashi Iwai wrote:
On Wed, 02 Nov 2022 23:05:14 +0100, Pierre-Louis Bossart wrote:
On 11/2/22 16:05, Jason Montleon wrote:
Starting with 6.0-rc1 these messages are logged and the sound card is unavailable. Adding legacy_dai_naming to the rt5514-spi causes it to function properly again.
[ 16.928454] kbl_r5514_5663_max kbl_r5514_5663_max: ASoC: CPU DAI spi-PRP0001:00 not registered [ 16.928561] platform kbl_r5514_5663_max: deferred probe pending
Thanks for reporting this regression, much appreciated.
a) you need to CC: maintainers Mark Brown and Takashi Iwai b) the commit title should be something like "ASoC: rt5514: fix legacy dai naming". c) it's not clear if this is actually enough. there's no legacy_dai_naming for e.g. rt5663 and the .endianness member is not set.
IIUC, rt5663.c should be fine; it used to have non_legacy_dai_naming flag and it was dropped after the switch.
But, through a quick glance, rt5677-spi.c seems to be the same pattern as rt5514-spi.c. The rt5677.c was covered properly but the *-spi.c wan't.
Yeah I think these got missed as they are effectively CPU side devices but living in the CODEC space. Looks like it would be reasonable to add legacy_dai_naming to both of them to me.
BTW, the bug was reported on bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=216641
Please respond on there and add the link to the fix patch, too.
thanks,
Takashi
Thanks, Charles
static const struct snd_soc_component_driver rt5514_spi_component = {
- .name = DRV_NAME,
- .probe = rt5514_spi_pcm_probe,
- .open = rt5514_spi_pcm_open,
- .hw_params = rt5514_spi_hw_params,
- .hw_free = rt5514_spi_hw_free,
- .pointer = rt5514_spi_pcm_pointer,
- .pcm_construct = rt5514_spi_pcm_new,
- .name = DRV_NAME,
- .probe = rt5514_spi_pcm_probe,
- .open = rt5514_spi_pcm_open,
- .hw_params = rt5514_spi_hw_params,
- .hw_free = rt5514_spi_hw_free,
- .pointer = rt5514_spi_pcm_pointer,
- .pcm_construct = rt5514_spi_pcm_new,
- .legacy_dai_naming = 1,
};
/**
Starting with 6.0-rc1 these messages are logged and the sound card is unavailable. Adding legacy_dai_naming to the rt5514-spi causes it to function properly again.
[ 16.928454] kbl_r5514_5663_max kbl_r5514_5663_max: ASoC: CPU DAI spi-PRP0001:00 not registered [ 16.928561] platform kbl_r5514_5663_max: deferred probe pending --- sound/soc/codecs/rt5514-spi.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/sound/soc/codecs/rt5514-spi.c b/sound/soc/codecs/rt5514-spi.c index 1a25a3787935..362663abcb89 100644 --- a/sound/soc/codecs/rt5514-spi.c +++ b/sound/soc/codecs/rt5514-spi.c @@ -298,13 +298,14 @@ static int rt5514_spi_pcm_new(struct snd_soc_component *component, }
static const struct snd_soc_component_driver rt5514_spi_component = { - .name = DRV_NAME, - .probe = rt5514_spi_pcm_probe, - .open = rt5514_spi_pcm_open, - .hw_params = rt5514_spi_hw_params, - .hw_free = rt5514_spi_hw_free, - .pointer = rt5514_spi_pcm_pointer, - .pcm_construct = rt5514_spi_pcm_new, + .name = DRV_NAME, + .probe = rt5514_spi_pcm_probe, + .open = rt5514_spi_pcm_open, + .hw_params = rt5514_spi_hw_params, + .hw_free = rt5514_spi_hw_free, + .pointer = rt5514_spi_pcm_pointer, + .pcm_construct = rt5514_spi_pcm_new, + .legacy_dai_naming = 1, };
/**
Starting with 6.0-rc1 the CPU DAI is not registered and the sound card is unavailable. Adding legacy_dai_naming causes it to function properly again. --- sound/soc/codecs/rt5677-spi.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/sound/soc/codecs/rt5677-spi.c b/sound/soc/codecs/rt5677-spi.c index 8f3993a4c1cc..d25703dd7499 100644 --- a/sound/soc/codecs/rt5677-spi.c +++ b/sound/soc/codecs/rt5677-spi.c @@ -396,15 +396,16 @@ static int rt5677_spi_pcm_probe(struct snd_soc_component *component) }
static const struct snd_soc_component_driver rt5677_spi_dai_component = { - .name = DRV_NAME, - .probe = rt5677_spi_pcm_probe, - .open = rt5677_spi_pcm_open, - .close = rt5677_spi_pcm_close, - .hw_params = rt5677_spi_hw_params, - .hw_free = rt5677_spi_hw_free, - .prepare = rt5677_spi_prepare, - .pointer = rt5677_spi_pcm_pointer, - .pcm_construct = rt5677_spi_pcm_new, + .name = DRV_NAME, + .probe = rt5677_spi_pcm_probe, + .open = rt5677_spi_pcm_open, + .close = rt5677_spi_pcm_close, + .hw_params = rt5677_spi_hw_params, + .hw_free = rt5677_spi_hw_free, + .prepare = rt5677_spi_prepare, + .pointer = rt5677_spi_pcm_pointer, + .pcm_construct = rt5677_spi_pcm_new, + .legacy_dai_naming = 1, };
/* Select a suitable transfer command for the next transfer to ensure
On Thu, Nov 03, 2022 at 09:11:43AM -0400, Jason Montleon wrote:
Starting with 6.0-rc1 these messages are logged and the sound card is unavailable. Adding legacy_dai_naming to the rt5514-spi causes it to function properly again.
[ 16.928454] kbl_r5514_5663_max kbl_r5514_5663_max: ASoC: CPU DAI spi-PRP0001:00 not registered [ 16.928561] platform kbl_r5514_5663_max: deferred probe pending
sound/soc/codecs/rt5514-spi.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
You've not provided a Signed-off-by for this so I can't do anything with it, please see Documentation/process/submitting-patches.rst for details on what this is and why it's important.
Please don't send new patches in reply to old patches or serieses, this makes it harder for both people and tools to understand what is going on - it can bury things in mailboxes and make it difficult to keep track of what current patches are, both for the new patches and the old ones.
On Thu, 03 Nov 2022 14:11:43 +0100, Jason Montleon wrote:
Starting with 6.0-rc1 these messages are logged and the sound card is unavailable. Adding legacy_dai_naming to the rt5514-spi causes it to function properly again.
[ 16.928454] kbl_r5514_5663_max kbl_r5514_5663_max: ASoC: CPU DAI spi-PRP0001:00 not registered [ 16.928561] platform kbl_r5514_5663_max: deferred probe pending
Please avoid hanging on the existing thread if you resubmit a new patch set.
Also, more importantly, your Signed-off-by tag is missing. It's a legal requirement.
At the next time, run scripts/checkpatch.pl before the submission. It'll catch such errors.
thanks,
Takashi
sound/soc/codecs/rt5514-spi.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/sound/soc/codecs/rt5514-spi.c b/sound/soc/codecs/rt5514-spi.c index 1a25a3787935..362663abcb89 100644 --- a/sound/soc/codecs/rt5514-spi.c +++ b/sound/soc/codecs/rt5514-spi.c @@ -298,13 +298,14 @@ static int rt5514_spi_pcm_new(struct snd_soc_component *component, }
static const struct snd_soc_component_driver rt5514_spi_component = {
- .name = DRV_NAME,
- .probe = rt5514_spi_pcm_probe,
- .open = rt5514_spi_pcm_open,
- .hw_params = rt5514_spi_hw_params,
- .hw_free = rt5514_spi_hw_free,
- .pointer = rt5514_spi_pcm_pointer,
- .pcm_construct = rt5514_spi_pcm_new,
- .name = DRV_NAME,
- .probe = rt5514_spi_pcm_probe,
- .open = rt5514_spi_pcm_open,
- .hw_params = rt5514_spi_hw_params,
- .hw_free = rt5514_spi_hw_free,
- .pointer = rt5514_spi_pcm_pointer,
- .pcm_construct = rt5514_spi_pcm_new,
- .legacy_dai_naming = 1,
};
/**
2.37.3
On Thu, 03 Nov 2022 14:21:37 +0100, Takashi Iwai wrote:
On Thu, 03 Nov 2022 14:11:43 +0100, Jason Montleon wrote:
Starting with 6.0-rc1 these messages are logged and the sound card is unavailable. Adding legacy_dai_naming to the rt5514-spi causes it to function properly again.
[ 16.928454] kbl_r5514_5663_max kbl_r5514_5663_max: ASoC: CPU DAI spi-PRP0001:00 not registered [ 16.928561] platform kbl_r5514_5663_max: deferred probe pending
Please avoid hanging on the existing thread if you resubmit a new patch set.
Also, more importantly, your Signed-off-by tag is missing. It's a legal requirement.
At the next time, run scripts/checkpatch.pl before the submission. It'll catch such errors.
Also, it's better to have a few more things in the patches: - Fixes tag indicating the buggy commit to be fixed - Link (or BugLink tag) to pointing to the kernel bugzilla URL - Cc-to-stable tag, to assure the fix going to 6.0.y stable tree
Takashi
On 03.11.22 14:24, Takashi Iwai wrote:
- Link (or BugLink tag) to pointing to the kernel bugzilla URL
No, not BugLink. To quote Linus from here: https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD...
```
BugLink: https://lore.kernel.org/r/20220610205038.GA3050413@paulmck-ThinkPad-P17-Gen-... BugLink: https://lore.kernel.org/r/CAMdYzYpF4FNTBPZsEFeWRuEwSies36QM_As8osPWZSr2q-viE...
[...]
please stop making up random tags that make no sense.
Just use "Link:" ```
Ciao, Thorsten
Thanks all for the feedback. I've adjusted the commit message as asked, and updated rt5677-spi as well. From what I think I can see in past commits it looks like separate drivers are done with separate commits/patches. I didn't see any additional comments whether endianness should or shouldn't also be set so I left it off. I can make another updated if necessary.
I apologize if I'm mucking it up, This is my first kernel patch, trying to follow along and do things right.
On Thu, Nov 3, 2022 at 7:14 AM Takashi Iwai tiwai@suse.de wrote:
On Thu, 03 Nov 2022 10:54:04 +0100, Charles Keepax wrote:
On Thu, Nov 03, 2022 at 08:59:03AM +0100, Takashi Iwai wrote:
On Wed, 02 Nov 2022 23:05:14 +0100, Pierre-Louis Bossart wrote:
On 11/2/22 16:05, Jason Montleon wrote:
Starting with 6.0-rc1 these messages are logged and the sound card is unavailable. Adding legacy_dai_naming to the rt5514-spi causes it to function properly again.
[ 16.928454] kbl_r5514_5663_max kbl_r5514_5663_max: ASoC: CPU DAI spi-PRP0001:00 not registered [ 16.928561] platform kbl_r5514_5663_max: deferred probe pending
Thanks for reporting this regression, much appreciated.
a) you need to CC: maintainers Mark Brown and Takashi Iwai b) the commit title should be something like "ASoC: rt5514: fix legacy dai naming". c) it's not clear if this is actually enough. there's no legacy_dai_naming for e.g. rt5663 and the .endianness member is not set.
IIUC, rt5663.c should be fine; it used to have non_legacy_dai_naming flag and it was dropped after the switch.
But, through a quick glance, rt5677-spi.c seems to be the same pattern as rt5514-spi.c. The rt5677.c was covered properly but the *-spi.c wan't.
Yeah I think these got missed as they are effectively CPU side devices but living in the CODEC space. Looks like it would be reasonable to add legacy_dai_naming to both of them to me.
BTW, the bug was reported on bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=216641
Please respond on there and add the link to the fix patch, too.
thanks,
Takashi
Thanks, Charles
static const struct snd_soc_component_driver rt5514_spi_component = {
.name = DRV_NAME,.probe = rt5514_spi_pcm_probe,.open = rt5514_spi_pcm_open,.hw_params = rt5514_spi_hw_params,.hw_free = rt5514_spi_hw_free,.pointer = rt5514_spi_pcm_pointer,.pcm_construct = rt5514_spi_pcm_new,
.name = DRV_NAME,.probe = rt5514_spi_pcm_probe,.open = rt5514_spi_pcm_open,.hw_params = rt5514_spi_hw_params,.hw_free = rt5514_spi_hw_free,.pointer = rt5514_spi_pcm_pointer,.pcm_construct = rt5514_spi_pcm_new,.legacy_dai_naming = 1,};
/**
[Note: this mail is primarily send for documentation purposes and/or for regzbot, my Linux kernel regression tracking bot. That's why I removed most or all folks from the list of recipients, but left any that looked like a mailing lists. These mails usually contain '#forregzbot' in the subject, to make them easy to spot and filter out.]
On 02.11.22 21:05, Jason Montleon wrote:
Starting with 6.0-rc1 these messages are logged and the sound card is unavailable. Adding legacy_dai_naming to the rt5514-spi causes it to function properly again.
[ 16.928454] kbl_r5514_5663_max kbl_r5514_5663_max: ASoC: CPU DAI spi-PRP0001:00 not registered [ 16.928561] platform kbl_r5514_5663_max: deferred probe pending
Thanks for the report. To be sure below issue doesn't fall through the cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression tracking bot:
#regzbot introduced v5.19..v6.0 ^ https://bugzilla.kernel.org/show_bug.cgi?id=216641 #regzbot title sound: asoc: kbl_r5514_5663_max: sound broken #regzbot ignore-activity
Ciao, Thorsten
On 03.11.22 12:57, Thorsten Leemhuis wrote:
[Note: this mail is primarily send for documentation purposes and/or for regzbot, my Linux kernel regression tracking bot. That's why I removed most or all folks from the list of recipients, but left any that looked like a mailing lists. These mails usually contain '#forregzbot' in the subject, to make them easy to spot and filter out.]
On 02.11.22 21:05, Jason Montleon wrote:
Starting with 6.0-rc1 these messages are logged and the sound card is unavailable. Adding legacy_dai_naming to the rt5514-spi causes it to function properly again.
[ 16.928454] kbl_r5514_5663_max kbl_r5514_5663_max: ASoC: CPU DAI spi-PRP0001:00 not registered [ 16.928561] platform kbl_r5514_5663_max: deferred probe pending
Thanks for the report. To be sure below issue doesn't fall through the cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression tracking bot:
#regzbot introduced v5.19..v6.0 ^ https://bugzilla.kernel.org/show_bug.cgi?id=216641 #regzbot title sound: asoc: kbl_r5514_5663_max: sound broken #regzbot ignore-activity
#regzbot fixed-by: a1dca8774faf3f77eb34fa0ac6f3e2b82
participants (7)
-
Charles Keepax -
Jason Montleon -
Mark Brown -
Pierre-Louis Bossart -
Takashi Iwai -
Thorsten Leemhuis -
Thorsten Leemhuis