2022-06-17 20:57:29

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [PATCH] media: mediatek: vcodec: Drop platform_get_resource(IORESOURCE_IRQ)

Commit a1a2b7125e10 ("of/platform: Drop static setup of IRQ resource
from DT core") removed support for calling platform_get_resource(...,
IORESOURCE_IRQ, ...) on DT-based drivers, but the probe() function of
mtk-vcodec's encoder was still making use of it. This caused the encoder
driver to fail probe.

Since the platform_get_resource() call was only being used to check for
the presence of the interrupt (its returned resource wasn't even used)
and platform_get_irq() was already being used to get the IRQ, simply
drop the use of platform_get_resource(IORESOURCE_IRQ) and handle the
failure of platform_get_irq(), to get the driver probing again.

Fixes: a1a2b7125e10 ("of/platform: Drop static setup of IRQ resource from DT core")
Signed-off-by: Nícolas F. R. A. Prado <[email protected]>

---
Tested on mt8192-asurada-spherion.

.../media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c
index 95e8c29ccc65..b91c27e79796 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c
@@ -272,14 +272,12 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
goto err_res;
}

- res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (res == NULL) {
- dev_err(&pdev->dev, "failed to get irq resource");
- ret = -ENOENT;
+ dev->enc_irq = platform_get_irq(pdev, 0);
+ if (dev->enc_irq < 0) {
+ ret = dev->enc_irq;
goto err_res;
}

- dev->enc_irq = platform_get_irq(pdev, 0);
irq_set_status_flags(dev->enc_irq, IRQ_NOAUTOEN);
ret = devm_request_irq(&pdev->dev, dev->enc_irq,
mtk_vcodec_enc_irq_handler,
--
2.36.1


Subject: Re: [PATCH] media: mediatek: vcodec: Drop platform_get_resource(IORESOURCE_IRQ)

Il 17/06/22 22:39, Nícolas F. R. A. Prado ha scritto:
> Commit a1a2b7125e10 ("of/platform: Drop static setup of IRQ resource
> from DT core") removed support for calling platform_get_resource(...,
> IORESOURCE_IRQ, ...) on DT-based drivers, but the probe() function of
> mtk-vcodec's encoder was still making use of it. This caused the encoder
> driver to fail probe.
>
> Since the platform_get_resource() call was only being used to check for
> the presence of the interrupt (its returned resource wasn't even used)
> and platform_get_irq() was already being used to get the IRQ, simply
> drop the use of platform_get_resource(IORESOURCE_IRQ) and handle the
> failure of platform_get_irq(), to get the driver probing again.
>
> Fixes: a1a2b7125e10 ("of/platform: Drop static setup of IRQ resource from DT core")
> Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
>
> ---
> Tested on mt8192-asurada-spherion.

Yep, that's totally necessary... Except the Fixes tag should be wrong here,
as you're not fixing that commit, but the mtk-vcodec driver in relation to
what's happening due to said commit.

I get that you're trying to tell everyone "this is an urgent fix", though,
and I agree that this *has to* get in v5.19 to avoid breaking this driver.

Finally, for the code:

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>

...but I think that you have to send a v2 that drops that Fixes tag.

Cheers,
Angelo

2022-06-20 15:26:03

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: Re: [PATCH] media: mediatek: vcodec: Drop platform_get_resource(IORESOURCE_IRQ)

On Mon, Jun 20, 2022 at 09:58:01AM +0200, AngeloGioacchino Del Regno wrote:
> Il 17/06/22 22:39, N?colas F. R. A. Prado ha scritto:
> > Commit a1a2b7125e10 ("of/platform: Drop static setup of IRQ resource
> > from DT core") removed support for calling platform_get_resource(...,
> > IORESOURCE_IRQ, ...) on DT-based drivers, but the probe() function of
> > mtk-vcodec's encoder was still making use of it. This caused the encoder
> > driver to fail probe.
> >
> > Since the platform_get_resource() call was only being used to check for
> > the presence of the interrupt (its returned resource wasn't even used)
> > and platform_get_irq() was already being used to get the IRQ, simply
> > drop the use of platform_get_resource(IORESOURCE_IRQ) and handle the
> > failure of platform_get_irq(), to get the driver probing again.
> >
> > Fixes: a1a2b7125e10 ("of/platform: Drop static setup of IRQ resource from DT core")
> > Signed-off-by: N?colas F. R. A. Prado <[email protected]>
> >
> > ---
> > Tested on mt8192-asurada-spherion.
>
> Yep, that's totally necessary... Except the Fixes tag should be wrong here,
> as you're not fixing that commit, but the mtk-vcodec driver in relation to
> what's happening due to said commit.
>
> I get that you're trying to tell everyone "this is an urgent fix", though,
> and I agree that this *has to* get in v5.19 to avoid breaking this driver.

The commit I'm pointing to in the Fixes tag [1] says "Now that all the DT
drivers have switched to platform_get_irq() we can now safely drop the static
setup of IRQ resource from DT core code.", and while there were patches by the
author [2] to do the required changes on some drivers beforehand, some were
missed, including the mtk-vcodec-enc driver which I fix here. I see at least one
other driver that was missed [3], and it also used the same Fixes tag.

I know that my change isn't meant to be part of the commit in the Fixes, but I
still think it makes the most sense to keep this tag, since its meaning is that
that commit broke the support for this driver and this commit fixes it. Wherever
that commit is applied, this one should too to make sure the mtk-vcodec driver
keeps working.

Thanks,
N?colas

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/

>
> Finally, for the code:
>
> Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
>
> ...but I think that you have to send a v2 that drops that Fixes tag.
>
> Cheers,
> Angelo