Hi Andi, :)
> -----Original Message-----
> From: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
> Sent: Friday, May 19, 2023 9:27 AM
> To: Inki Dae <inki.dae@xxxxxxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx; linux-samsung-soc@xxxxxxxxxxxxxxx; Andi
> Shyti <andi.shyti@xxxxxxxxxx>
> Subject: Re: [PATCH] drm/exynos: vidi: fix a wrong error return
>
> Hi Inki,
>
> On Fri, May 19, 2023 at 09:04:07AM +0900, Inki Dae wrote:
> > Fix a wrong error return by dropping an error return.
> >
> > When vidi driver is remvoed, if ctx->raw_edid isn't same as fake_edid_info
> > then only what we have to is to free ctx->raw_edid so that driver removing
> > can work correctly - it's not an error case.
> >
> > Signed-off-by: Inki Dae <inki.dae@xxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/exynos/exynos_drm_vidi.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> > index 4d56c8c799c5..f5e1adfcaa51 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> > @@ -469,8 +469,6 @@ static int vidi_remove(struct platform_device *pdev)
> > if (ctx->raw_edid != (struct edid *)fake_edid_info) {
> > kfree(ctx->raw_edid);
> > ctx->raw_edid = NULL;
> > -
> > - return -EINVAL;
>
> It doesn't look right to me, I think the correct patch should be:
>
> - if (ctx->raw_edid != (struct edid *)fake_edid_info) {
> - kfree(ctx->raw_edid);
> - ctx->raw_edid = NULL;
> -
> - return -EINVAL;
> - }
> -
> + ctx->raw_edid = NULL;
>
> because "ctx->raw_edid" points to a non allocated memory in the
> .data segment and you cannot free it.
>
> A follow-up cleanup should be to remove the "const" from
> fake_edid_info because you are assigning its address to pointers
> (raw_edid), so that what's the point for having it const? You are
> just fooling the compiler :)
Thanks for review comment.
"ctx->raw_edid != fake_edid_info" means that the edid sent by the user through
the ictl system call - vidi_connection_ioctl - is used instead of fake one -
face_edid_info.
In this case, ctx->raw_edid object needs to be released because ctx->raw_edid
object is allocated and the edid object sent by user is copied to the ctx-
>raw_edid by kmemdup(). :)
Thanks,
Inki Dae
>
> Andi
>
> > }
> >
> > component_del(&pdev->dev, &vidi_component_ops);
> > --
> > 2.25.1
