Re: [PATCH] drm/mgag200: Fail on I2C initialization errors

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Hi

Am 05.05.22 um 17:49 schrieb Jocelyn Falempe:
> On 05/05/2022 17:22, Thomas Zimmermann wrote:
> > Initialization of the I2C adapter was allowed to fail. The mgag200
> > driver would have continued without DDC support. Had this happened in
> > practice, it would have led to segmentation faults in the connector
> > code. Resolve this problem by failing driver initialization on I2C-
> > related errors.
>
> Reviewed-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>

Thanks a lot. I'll fix the reports from the automated tests and resubmit 
this patch as part of a larger patchset.

Best regards
Thomas

> > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > ---
> >   drivers/gpu/drm/mgag200/mgag200_i2c.c  | 13 ++++++++-----
> >   drivers/gpu/drm/mgag200/mgag200_mode.c |  6 ++++--
> >   2 files changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mgag200/mgag200_i2c.c 
> > b/drivers/gpu/drm/mgag200/mgag200_i2c.c
> > index ac8e34eef5138..31e2c641a7814 100644
> > --- a/drivers/gpu/drm/mgag200/mgag200_i2c.c
> > +++ b/drivers/gpu/drm/mgag200/mgag200_i2c.c
> > @@ -120,7 +120,7 @@ struct mga_i2c_chan *mgag200_i2c_create(struct 
> > drm_device *dev)
> >       i2c = kzalloc(sizeof(struct mga_i2c_chan), GFP_KERNEL);
> >       if (!i2c)
> > -        return NULL;
> > +        return ERR_PTR(-ENOMEM);
> >       i2c->data = data;
> >       i2c->clock = clock;
> > @@ -142,11 +142,14 @@ struct mga_i2c_chan *mgag200_i2c_create(struct 
> > drm_device *dev)
> >       i2c->bit.getscl        = mga_gpio_getscl;
> >       ret = i2c_bit_add_bus(&i2c->adapter);
> > -    if (ret) {
> > -        kfree(i2c);
> > -        i2c = NULL;
> > -    }
> > +    if (ret)
> > +        goto err_kfree;
> > +
> >       return i2c;
> > +
> > +err_kfree:
> > +    kfree(i2c);
> > +    return ERR_PTR(ret);
> >   }
> >   void mgag200_i2c_destroy(struct mga_i2c_chan *i2c)
> > diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
> > b/drivers/gpu/drm/mgag200/mgag200_mode.c
> > index abde7655477db..79557ca056f08 100644
> > --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> > +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> > @@ -815,8 +815,10 @@ static int mgag200_vga_connector_init(struct 
> > mga_device *mdev)
> >       int ret;
> >       i2c = mgag200_i2c_create(dev);
> > -    if (!i2c)
> > -        drm_warn(dev, "failed to add DDC bus\n");
> > +    if (IS_ERR(i2c)) {
> > +        drm_err(dev, "failed to add DDC bus: %d\n", ret);
> > +        return PTR_ERR(i2c);
> > +    }
> >       ret = drm_connector_init_with_ddc(dev, connector,
> >                         &mga_vga_connector_funcs,

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachment: OpenPGP_signature
Description: OpenPGP digital signature