Re: [PATCH] drm/vc4: use platform_register_drivers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 2017-03-19 at 11:28 +0000, Emil Velikov wrote:
> Hi Philipp,
> 
> I think you patch is OK, just a small question about the existing code.
> It might be better suited for Eric... not sure.
> 
> On 17 March 2017 at 17:00, Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote:
> > Use platform_register_drivers instead of open coding the iteration over
> > component platform drivers in the vc4_drv module.
> >
> > Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> > ---
> >  drivers/gpu/drm/vc4/vc4_drv.c | 22 ++++++++--------------
> >  1 file changed, 8 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> > index 205c1961ffb4c..61e674baf3a6f 100644
> > --- a/drivers/gpu/drm/vc4/vc4_drv.c
> > +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> > @@ -349,26 +349,20 @@ static struct platform_driver vc4_platform_driver = {
> >
> >  static int __init vc4_drm_register(void)
> >  {
> > -       int i, ret;
> > +       int ret;
> > +
> > +       ret = platform_register_drivers(component_drivers,
> > +                                       ARRAY_SIZE(component_drivers));
> > +       if (ret)
> > +               return ret;
> >
> > -       for (i = 0; i < ARRAY_SIZE(component_drivers); i++) {
> > -               ret = platform_driver_register(component_drivers[i]);
> > -               if (ret) {
> > -                       while (--i >= 0)
> > -                               platform_driver_unregister(component_drivers[i]);
> > -                       return ret;
> > -               }
> > -       }
> >         return platform_driver_register(&vc4_platform_driver);
> Is there any reason why vc4_platform_driver isn't part of the
> component_drivers array ?

It is separate from the array because the same array is also used to set
up the matches for the component_master_add_with_match call from the
vc4_platform_driver's probe function.

> >  }
> >
> >  static void __exit vc4_drm_unregister(void)
> >  {
> > -       int i;
> > -
> > -       for (i = ARRAY_SIZE(component_drivers) - 1; i >= 0; i--)
> > -               platform_driver_unregister(component_drivers[i]);
> > -
> > +       platform_unregister_drivers(component_drivers,
> > +                                   ARRAY_SIZE(component_drivers));
> >         platform_driver_unregister(&vc4_platform_driver);
> Order seems wrong here - shouldn't one unregister vc4_platform_driver
> first here ?
> Perhaps that 's the reason why it is handled separately in vc4_drm_register.
> 
> There's [seemingly] no comment that covers this so it seems like a
> copy/paste mistake.

I think it would make sense to unregister the vc4_platform_driver first,
but since this was there from the start I don't know if this was done
differently on purpose.

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux