Re: [PATCH 2/3] drm: pl111: Simplify vexpress init

[Rob Herring <robh@kernel.org>]

On Thu, Apr 9, 2020 at 9:16 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Rob.
>
> On Wed, Apr 08, 2020 at 07:39:46PM -0600, Rob Herring wrote:
> > The init VExpress variants currently instantiates a 'muxfpga' driver for
> > the sole purpose of getting a regmap for it. There's no reason to
> > instantiate a driver and doing so just complicates things. The muxfpga
> > driver also isn't unregistered properly on module unload. Let's
> > just simplify all this this by just calling
> > devm_regmap_init_vexpress_config() directly.
> >
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
>
> Procastinating, so I took a look at this.
> Nice simplification - on nit below.
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > ---
> >  drivers/gpu/drm/pl111/pl111_versatile.c | 21 +++----------
> >  drivers/gpu/drm/pl111/pl111_vexpress.c  | 42 -------------------------
> >  drivers/gpu/drm/pl111/pl111_vexpress.h  |  7 -----
> >  3 files changed, 4 insertions(+), 66 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
> > index 09aeaffb7660..8c2551088f26 100644
> > --- a/drivers/gpu/drm/pl111/pl111_versatile.c
> > +++ b/drivers/gpu/drm/pl111/pl111_versatile.c
> > @@ -8,6 +8,7 @@
> >  #include <linux/of.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/regmap.h>
> > +#include <linux/vexpress.h>
> >
> >  #include "pl111_versatile.h"
> >  #include "pl111_vexpress.h"
> > @@ -325,17 +326,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> >       versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
> >
> >       /* Versatile Express special handling */
> > -     if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> > +     if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> >               struct platform_device *pdev;
> > -
> > -             /* Registers a driver for the muxfpga */
> > -             ret = vexpress_muxfpga_init();
> > -             if (ret) {
> > -                     dev_err(dev, "unable to initialize muxfpga driver\n");
> > -                     of_node_put(np);
> > -                     return ret;
> > -             }
> > -
> >               /* Call into deep Vexpress configuration API */
> >               pdev = of_find_device_by_node(np);
> >               if (!pdev) {
> > @@ -343,13 +335,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> >                       of_node_put(np);
> >                       return -EPROBE_DEFER;
> >               }
> > -             map = dev_get_drvdata(&pdev->dev);
> > -             if (!map) {
> > -                     dev_err(dev, "sysreg has not yet probed\n");
> > -                     platform_device_put(pdev);
> > -                     of_node_put(np);
> > -                     return -EPROBE_DEFER;
> > -             }
> > +             map = devm_regmap_init_vexpress_config(&pdev->dev);
> > +             platform_device_put(pdev);
> >       } else {
> >               map = syscon_node_to_regmap(np);
> >       }
>
> On the following line there is:
>         if (IS_ERR(map)) {
>                 dev_err(dev, "no Versatile syscon regmap\n");
>                 return PTR_ERR(map);
>         }
>
> The error message no longer tell if this was
> devm_regmap_init_vexpress_config() or syscon_node_to_regmap() that
> caused the error.

Hopefully you'd know what platform you are on.

In any case, it's changed after patch 3.

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