Re: [PATCH v4 1/4] media: imagination: e5010: Properly Release m2m_dev if probe fails

[Devarsh Thakkar]

Hi Brandon,

On 18/09/26 14:38, Devarsh Thakkar wrote:
> Hi Brandon,
>
> Thanks for the patch.

I think this patch [0] is already addressing the same problem.

[0]: 
https://lore.kernel.org/all/fc08a754-1e0b-42ab-ac53-94bff728aef3@xxxxxx/#r

Since the above patch was posted earlier and hoping the next revision 
for same is coming soon, is it possible for you to drop this patch ( 
media: imagination: e5010: Properly Release m2m_dev if probe fails)

in the next revision for the series?

Regards
Devarsh

> On 01/09/26 03:02, Brandon Brnich wrote:
> > After the call to v4l2_m2m_init() is successful, the following
> > initialization functions should go to fail_after_video_register_device.
> >
> > Fixes: a1e294045885 ("media: imagination: Add E5010 JPEG Encoder driver")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Brandon Brnich <b-brnich@xxxxxx>
> > ---
> >   drivers/media/platform/imagination/e5010-jpeg-enc.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/ 
> > drivers/media/platform/imagination/e5010-jpeg-enc.c
> > index 42ad9ee3993b4..d3c11337c815d 100644
> > --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c
> > +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c
> > @@ -1072,14 +1072,14 @@ static int e5010_probe(struct platform_device 
> > *pdev)
> >       if (IS_ERR(e5010->core_base)) {
> >           ret = PTR_ERR(e5010->core_base);
> >           dev_err_probe(dev, ret, "Missing 'core' resources area\n");
> > -        goto fail_after_v4l2_register;
> > +        goto fail_after_video_register_device;
>
> The label naming looks incorrect semantically since 
> video_register_device happens after all this at last actually. Please 
> change it to fail_after_v4l2_m2m_init instead.
>
> >       }
> >       e5010->mmu_base = devm_platform_ioremap_resource_byname(pdev, 
> > "mmu");
> >       if (IS_ERR(e5010->mmu_base)) {
> >           ret = PTR_ERR(e5010->mmu_base);
> >           dev_err_probe(dev, ret, "Missing 'mmu' resources area\n");
> > -        goto fail_after_v4l2_register;
> > +        goto fail_after_video_register_device;
> >       }
>
> goto fail_after_v4l2_m2m_init
>
> >       e5010->last_context_run = NULL;
> > @@ -1089,14 +1089,14 @@ static int e5010_probe(struct platform_device 
> > *pdev)
> >                      E5010_MODULE_NAME, e5010);
> >       if (ret) {
> >           dev_err_probe(dev, ret, "failed to register IRQ %d\n", irq);
> > -        goto fail_after_v4l2_register;
> > +        goto fail_after_video_register_device;
> >       }
>
> goto fail_after_v4l2_m2m_init
>
> >       e5010->clk = devm_clk_get(dev, NULL);
> >       if (IS_ERR(e5010->clk)) {
> >           ret = PTR_ERR(e5010->clk);
> >           dev_err_probe(dev, ret, "failed to get clock\n");
> > -        goto fail_after_v4l2_register;
> > +        goto fail_after_video_register_device;
> >       }
>
> goto fail_after_v4l2_m2m_init
> >       pm_runtime_enable(dev);
>
> Regards
> Devarsh