[PATCH] ASoC: mediatek: mt8173: Enable IRQ when pdata is ready

[PATCH] ASoC: mediatek: mt8173: Enable IRQ when pdata is ready

[Ricardo Ribalda]

If the device does not come straight from reset, we might receive an IRQ
before we are ready to handle it.

Fixes:

[    2.334737] Unable to handle kernel read from unreadable memory at virtual address 00000000000001e4
[    2.522601] Call trace:
[    2.525040]  regmap_read+0x1c/0x80
[    2.528434]  mt8173_afe_irq_handler+0x40/0xf0
...
[    2.598921]  start_kernel+0x338/0x42c

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
To: Liam Girdwood <lgirdwood@gmail.com>
To: Mark Brown <broonie@kernel.org>
To: Jaroslav Kysela <perex@perex.cz>
To: Takashi Iwai <tiwai@suse.com>
To: Matthias Brugger <matthias.bgg@gmail.com>
Cc: alsa-devel@alsa-project.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mediatek@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 sound/soc/mediatek/mt8173/mt8173-afe-pcm.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/sound/soc/mediatek/mt8173/mt8173-afe-pcm.c b/sound/soc/mediatek/mt8173/mt8173-afe-pcm.c
index dcaeeeb8aac7..bc155dd937e0 100644
--- a/sound/soc/mediatek/mt8173/mt8173-afe-pcm.c
+++ b/sound/soc/mediatek/mt8173/mt8173-afe-pcm.c
@@ -1070,16 +1070,6 @@ static int mt8173_afe_pcm_dev_probe(struct platform_device *pdev)
 
 	afe->dev = &pdev->dev;
 
-	irq_id = platform_get_irq(pdev, 0);
-	if (irq_id <= 0)
-		return irq_id < 0 ? irq_id : -ENXIO;
-	ret = devm_request_irq(afe->dev, irq_id, mt8173_afe_irq_handler,
-			       0, "Afe_ISR_Handle", (void *)afe);
-	if (ret) {
-		dev_err(afe->dev, "could not request_irq\n");
-		return ret;
-	}
-
 	afe->base_addr = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(afe->base_addr))
 		return PTR_ERR(afe->base_addr);
@@ -1185,6 +1175,16 @@ static int mt8173_afe_pcm_dev_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_cleanup_components;
 
+	irq_id = platform_get_irq(pdev, 0);
+	if (irq_id <= 0)
+		return irq_id < 0 ? irq_id : -ENXIO;
+	ret = devm_request_irq(afe->dev, irq_id, mt8173_afe_irq_handler,
+			       0, "Afe_ISR_Handle", (void *)afe);
+	if (ret) {
+		dev_err(afe->dev, "could not request_irq\n");
+		goto err_pm_disable;
+	}
+
 	dev_info(&pdev->dev, "MT8173 AFE driver initialized.\n");
 	return 0;
 

---
base-commit: 4312098baf37ee17a8350725e6e0d0e8590252d4
change-id: 20221128-mt8173-afe-5fda4512e8b5

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>

Re: [PATCH] ASoC: mediatek: mt8173: Enable IRQ when pdata is ready

[AngeloGioacchino Del Regno]

Il 28/11/22 11:49, Ricardo Ribalda ha scritto:
> If the device does not come straight from reset, we might receive an IRQ
> before we are ready to handle it.
> 
> Fixes:

I agree. That's a coding mistake... but...

This commit needs a Fixes tag, as this is indeed a fix.... kexec isn't anything
new, so all kernel versions are affected by this bug.

Moreover, I can see this pattern repeated across *all* MediaTek AFE drivers:
while at it, can you please replicate this change on all of them, each with
their appropriate Fixes tag?

That would make this fix complete.

Thanks!
Angelo

Re: [PATCH] ASoC: mediatek: mt8173: Enable IRQ when pdata is ready

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

On Mon, Nov 28, 2022 at 02:17:53PM +0100, AngeloGioacchino Del Regno wrote:

> This commit needs a Fixes tag, as this is indeed a fix.... kexec isn't anything
> new, so all kernel versions are affected by this bug.

Fixes tags are a nice to have, they're not 100% a requirement - they're
a lot more useful when they're fixing a bug that was introduced rather
than just something that never worked.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: mediatek: mt8173: Enable IRQ when pdata is ready

[Ricardo Ribalda]

Hi

On Mon, 28 Nov 2022 at 14:34, Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Nov 28, 2022 at 02:17:53PM +0100, AngeloGioacchino Del Regno wrote:
>
> > This commit needs a Fixes tag, as this is indeed a fix.... kexec isn't anything
> > new, so all kernel versions are affected by this bug.
>
> Fixes tags are a nice to have, they're not 100% a requirement - they're
> a lot more useful when they're fixing a bug that was introduced rather
> than just something that never worked.

I do not have any strong opinion here. If you want to add the tag. It should be:

Fixes: ee0bcaff109f ("ASoC: mediatek: Add AFE platform driver")

Let me know if I shall resend.

Thanks!

-- 
Ricardo Ribalda

Re: [PATCH] ASoC: mediatek: mt8173: Enable IRQ when pdata is ready

[Mark Brown]

On Mon, 28 Nov 2022 11:49:16 +0100, Ricardo Ribalda wrote:
> If the device does not come straight from reset, we might receive an IRQ
> before we are ready to handle it.
> 
> Fixes:
> 
> [    2.334737] Unable to handle kernel read from unreadable memory at virtual address 00000000000001e4
> [    2.522601] Call trace:
> [    2.525040]  regmap_read+0x1c/0x80
> [    2.528434]  mt8173_afe_irq_handler+0x40/0xf0
> ...
> [    2.598921]  start_kernel+0x338/0x42c
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: mediatek: mt8173: Enable IRQ when pdata is ready
      commit: 4cbb264d4e9136acab2c8fd39e39ab1b1402b84b

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark