| Message ID | 20240620092526.2353537-1-wenst@chromium.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 282a4482e198e03781c152c88aac8aa382ef9a55 |
| Headers | show |
| Series | ASoC: mediatek: mt8195: Re-add codec entry for ETDM1_OUT_BE dai link | expand |
Il 20/06/24 11:25, Chen-Yu Tsai ha scritto: > This partially reverts commit e70b8dd26711704b1ff1f1b4eb3d048ba69e29da. > > Said commit removes the codec entry for the ETDM1_OUT_BE dai link for > some reason. This does not have the intended effect, as the remaining > DAILINK_COMP_ARRAY(COMP_EMPTY()) platform entry becomes the codec > entry, and the platform entry is completely gone. > > This causes in a KASAN out-of-bounds warning in mtk_soundcard_common_probe() > in sound/soc/mediatek/common/mtk-soundcard-driver.c: > > for_each_card_prelinks(card, i, dai_link) { > if (adsp_node && !strncmp(dai_link->name, "AFE_SOF", strlen("AFE_SOF"))) > dai_link->platforms->of_node = adsp_node; > else if (!dai_link->platforms->name && !dai_link->platforms->of_node) > dai_link->platforms->of_node = platform_node; > } > > where the code expects the platforms array to have space for at least one entry. > > Re-add the entry so that dai_link->platforms has space. > Ok, but wait a minute... the commit that you're pointing at in the Fixes tag is a commit that fixes a problem identified in commit 13f58267cda3 ("ASoC: soc.h: don't create dummy Component via COMP_DUMMY()") to keep it short, after that one, without removing the COMP_DUMMY(), the audio was broken in .. some way, I don't currently remember specifically what was happening, but I had no sound at all. If the problem is not showing up anymore, backporting this commit to the kernels kernels affected by the issue that I solved... will break sound! So... well.. that's the "some reason".... :-) Cheers, Angelo > Fixes: e70b8dd26711 ("ASoC: mediatek: mt8195: Remove afe-dai component and rework codec link") > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > --- > sound/soc/mediatek/mt8195/mt8195-mt6359.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/sound/soc/mediatek/mt8195/mt8195-mt6359.c b/sound/soc/mediatek/mt8195/mt8195-mt6359.c > index ca8751190520..2832ef78eaed 100644 > --- a/sound/soc/mediatek/mt8195/mt8195-mt6359.c > +++ b/sound/soc/mediatek/mt8195/mt8195-mt6359.c > @@ -827,6 +827,7 @@ SND_SOC_DAILINK_DEFS(ETDM2_IN_BE, > > SND_SOC_DAILINK_DEFS(ETDM1_OUT_BE, > DAILINK_COMP_ARRAY(COMP_CPU("ETDM1_OUT")), > + DAILINK_COMP_ARRAY(COMP_EMPTY()), > DAILINK_COMP_ARRAY(COMP_EMPTY())); > > SND_SOC_DAILINK_DEFS(ETDM2_OUT_BE,
On Thu, Jun 20, 2024 at 6:27 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Il 20/06/24 11:25, Chen-Yu Tsai ha scritto: > > This partially reverts commit e70b8dd26711704b1ff1f1b4eb3d048ba69e29da. > > > > Said commit removes the codec entry for the ETDM1_OUT_BE dai link for > > some reason. This does not have the intended effect, as the remaining > > DAILINK_COMP_ARRAY(COMP_EMPTY()) platform entry becomes the codec > > entry, and the platform entry is completely gone. > > > > This causes in a KASAN out-of-bounds warning in mtk_soundcard_common_probe() > > in sound/soc/mediatek/common/mtk-soundcard-driver.c: > > > > for_each_card_prelinks(card, i, dai_link) { > > if (adsp_node && !strncmp(dai_link->name, "AFE_SOF", strlen("AFE_SOF"))) > > dai_link->platforms->of_node = adsp_node; > > else if (!dai_link->platforms->name && !dai_link->platforms->of_node) > > dai_link->platforms->of_node = platform_node; > > } > > > > where the code expects the platforms array to have space for at least one entry. > > > > Re-add the entry so that dai_link->platforms has space. > > > > Ok, but wait a minute... the commit that you're pointing at in the Fixes tag is > a commit that fixes a problem identified in commit > > 13f58267cda3 ("ASoC: soc.h: don't create dummy Component via COMP_DUMMY()") > > to keep it short, after that one, without removing the COMP_DUMMY(), the audio > was broken in .. some way, I don't currently remember specifically what was > happening, but I had no sound at all. Right. But this adds back "COMP_EMPTY()", not COMP_DUMMY(). I guess I shouldn't say this is a partial revert. After painstakingly messing with the mixer controls, I tested all three audio paths (speaker, headphone, DP) and they all work correctly. I'll send a v2 updating the commit message. ChenYu > If the problem is not showing up anymore, backporting this commit to the kernels > kernels affected by the issue that I solved... will break sound! > > So... well.. that's the "some reason".... :-) > > Cheers, > Angelo > > > Fixes: e70b8dd26711 ("ASoC: mediatek: mt8195: Remove afe-dai component and rework codec link") > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > > --- > > sound/soc/mediatek/mt8195/mt8195-mt6359.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/sound/soc/mediatek/mt8195/mt8195-mt6359.c b/sound/soc/mediatek/mt8195/mt8195-mt6359.c > > index ca8751190520..2832ef78eaed 100644 > > --- a/sound/soc/mediatek/mt8195/mt8195-mt6359.c > > +++ b/sound/soc/mediatek/mt8195/mt8195-mt6359.c > > @@ -827,6 +827,7 @@ SND_SOC_DAILINK_DEFS(ETDM2_IN_BE, > > > > SND_SOC_DAILINK_DEFS(ETDM1_OUT_BE, > > DAILINK_COMP_ARRAY(COMP_CPU("ETDM1_OUT")), > > + DAILINK_COMP_ARRAY(COMP_EMPTY()), > > DAILINK_COMP_ARRAY(COMP_EMPTY())); > > > > SND_SOC_DAILINK_DEFS(ETDM2_OUT_BE, >
On Fri, Jun 21, 2024 at 12:28 PM Chen-Yu Tsai <wenst@chromium.org> wrote: > > On Thu, Jun 20, 2024 at 6:27 PM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: > > > > Il 20/06/24 11:25, Chen-Yu Tsai ha scritto: > > > This partially reverts commit e70b8dd26711704b1ff1f1b4eb3d048ba69e29da. > > > > > > Said commit removes the codec entry for the ETDM1_OUT_BE dai link for > > > some reason. This does not have the intended effect, as the remaining > > > DAILINK_COMP_ARRAY(COMP_EMPTY()) platform entry becomes the codec > > > entry, and the platform entry is completely gone. > > > > > > This causes in a KASAN out-of-bounds warning in mtk_soundcard_common_probe() > > > in sound/soc/mediatek/common/mtk-soundcard-driver.c: > > > > > > for_each_card_prelinks(card, i, dai_link) { > > > if (adsp_node && !strncmp(dai_link->name, "AFE_SOF", strlen("AFE_SOF"))) > > > dai_link->platforms->of_node = adsp_node; > > > else if (!dai_link->platforms->name && !dai_link->platforms->of_node) > > > dai_link->platforms->of_node = platform_node; > > > } > > > > > > where the code expects the platforms array to have space for at least one entry. > > > > > > Re-add the entry so that dai_link->platforms has space. > > > > > > > Ok, but wait a minute... the commit that you're pointing at in the Fixes tag is > > a commit that fixes a problem identified in commit > > > > 13f58267cda3 ("ASoC: soc.h: don't create dummy Component via COMP_DUMMY()") > > > > to keep it short, after that one, without removing the COMP_DUMMY(), the audio > > was broken in .. some way, I don't currently remember specifically what was > > happening, but I had no sound at all. In a way, by removing COMP_DUMMY() completely, you effectively replaced it with COMP_EMPTY() (from the platform entry), while the platform entry effectively became COMP_DUMMY() (which is equal to a completely empty entry). > Right. But this adds back "COMP_EMPTY()", not COMP_DUMMY(). > > I guess I shouldn't say this is a partial revert. > > After painstakingly messing with the mixer controls, I tested all three > audio paths (speaker, headphone, DP) and they all work correctly. > > I'll send a v2 updating the commit message. > > ChenYu > > > If the problem is not showing up anymore, backporting this commit to the kernels > > kernels affected by the issue that I solved... will break sound! > > > > So... well.. that's the "some reason".... :-) > > > > Cheers, > > Angelo > > > > > Fixes: e70b8dd26711 ("ASoC: mediatek: mt8195: Remove afe-dai component and rework codec link") > > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > > > --- > > > sound/soc/mediatek/mt8195/mt8195-mt6359.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/sound/soc/mediatek/mt8195/mt8195-mt6359.c b/sound/soc/mediatek/mt8195/mt8195-mt6359.c > > > index ca8751190520..2832ef78eaed 100644 > > > --- a/sound/soc/mediatek/mt8195/mt8195-mt6359.c > > > +++ b/sound/soc/mediatek/mt8195/mt8195-mt6359.c > > > @@ -827,6 +827,7 @@ SND_SOC_DAILINK_DEFS(ETDM2_IN_BE, > > > > > > SND_SOC_DAILINK_DEFS(ETDM1_OUT_BE, > > > DAILINK_COMP_ARRAY(COMP_CPU("ETDM1_OUT")), > > > + DAILINK_COMP_ARRAY(COMP_EMPTY()), > > > DAILINK_COMP_ARRAY(COMP_EMPTY())); > > > > > > SND_SOC_DAILINK_DEFS(ETDM2_OUT_BE, > >
diff --git a/sound/soc/mediatek/mt8195/mt8195-mt6359.c b/sound/soc/mediatek/mt8195/mt8195-mt6359.c index ca8751190520..2832ef78eaed 100644 --- a/sound/soc/mediatek/mt8195/mt8195-mt6359.c +++ b/sound/soc/mediatek/mt8195/mt8195-mt6359.c @@ -827,6 +827,7 @@ SND_SOC_DAILINK_DEFS(ETDM2_IN_BE, SND_SOC_DAILINK_DEFS(ETDM1_OUT_BE, DAILINK_COMP_ARRAY(COMP_CPU("ETDM1_OUT")), + DAILINK_COMP_ARRAY(COMP_EMPTY()), DAILINK_COMP_ARRAY(COMP_EMPTY())); SND_SOC_DAILINK_DEFS(ETDM2_OUT_BE,
This partially reverts commit e70b8dd26711704b1ff1f1b4eb3d048ba69e29da. Said commit removes the codec entry for the ETDM1_OUT_BE dai link for some reason. This does not have the intended effect, as the remaining DAILINK_COMP_ARRAY(COMP_EMPTY()) platform entry becomes the codec entry, and the platform entry is completely gone. This causes in a KASAN out-of-bounds warning in mtk_soundcard_common_probe() in sound/soc/mediatek/common/mtk-soundcard-driver.c: for_each_card_prelinks(card, i, dai_link) { if (adsp_node && !strncmp(dai_link->name, "AFE_SOF", strlen("AFE_SOF"))) dai_link->platforms->of_node = adsp_node; else if (!dai_link->platforms->name && !dai_link->platforms->of_node) dai_link->platforms->of_node = platform_node; } where the code expects the platforms array to have space for at least one entry. Re-add the entry so that dai_link->platforms has space. Fixes: e70b8dd26711 ("ASoC: mediatek: mt8195: Remove afe-dai component and rework codec link") Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> --- sound/soc/mediatek/mt8195/mt8195-mt6359.c | 1 + 1 file changed, 1 insertion(+)