diff mbox series

[1/2] drm/aspeed: Look up syscon by phandle

Message ID 20210111044638.290909-2-joel@jms.id.au
State Superseded
Headers show
Series drm: aspeed: Support more chip families | expand

Commit Message

Joel Stanley Jan. 11, 2021, 4:46 a.m. UTC
This scales better to multiple families of SoC. The lookup by compatible
can be removed in a future change.

Signed-off-by: Joel Stanley <joel@jms.id.au>

---
 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

-- 
2.29.2

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

Comments

Jeremy Kerr Feb. 2, 2021, 4:38 a.m. UTC | #1
Hi Joel,

Sounds like a good idea! One comment though:

> @@ -111,10 +112,13 @@ static int aspeed_gfx_load(struct drm_device *drm)
>         if (IS_ERR(priv->base))
>                 return PTR_ERR(priv->base);
>  
> -       priv->scu = syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu");
> +       priv->scu = syscon_regmap_lookup_by_phandle(np, "syscon");
>         if (IS_ERR(priv->scu)) {
> -               dev_err(&pdev->dev, "failed to find SCU regmap\n");
> -               return PTR_ERR(priv->scu);
> +               priv->scu = syscon_regmap_lookup_by_compatible("aspeed,aspeed-scu");

Is this (more generic) compatible value guaranteed to exist alongside
aspeed,ast2500-scu? The scu binding only specifies the model-specific
ones:

    Documentation/devicetree/bindings/mfd/aspeed-scu.txt:

    Required properties:
    - compatible:	One of:
                    "aspeed,ast2400-scu", "syscon", "simple-mfd"
                    "aspeed,ast2500-scu", "syscon", "simple-mfd"

- the only mention of the new compatible value that I can find is this
thread. Maybe we should retain the existing one to keep the fallback
case working?

Cheers,


Jeremy
Joel Stanley Feb. 2, 2021, 5 a.m. UTC | #2
On Tue, 2 Feb 2021 at 04:39, Jeremy Kerr <jk@ozlabs.org> wrote:
>

> Hi Joel,

>

> Sounds like a good idea! One comment though:

>

> > @@ -111,10 +112,13 @@ static int aspeed_gfx_load(struct drm_device *drm)

> >         if (IS_ERR(priv->base))

> >                 return PTR_ERR(priv->base);

> >

> > -       priv->scu = syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu");

> > +       priv->scu = syscon_regmap_lookup_by_phandle(np, "syscon");

> >         if (IS_ERR(priv->scu)) {

> > -               dev_err(&pdev->dev, "failed to find SCU regmap\n");

> > -               return PTR_ERR(priv->scu);

> > +               priv->scu = syscon_regmap_lookup_by_compatible("aspeed,aspeed-scu");

>

> Is this (more generic) compatible value guaranteed to exist alongside

> aspeed,ast2500-scu? The scu binding only specifies the model-specific

> ones:

>

>     Documentation/devicetree/bindings/mfd/aspeed-scu.txt:

>

>     Required properties:

>     - compatible:       One of:

>                     "aspeed,ast2400-scu", "syscon", "simple-mfd"

>                     "aspeed,ast2500-scu", "syscon", "simple-mfd"

>

> - the only mention of the new compatible value that I can find is this

> thread. Maybe we should retain the existing one to keep the fallback

> case working?


Yes, it was a mistake to change ast2500-scu to aspeed-scu. The only
reason to keep the lookup_by_compatible was to decouple this patch
from the device tree changes.

I will send a v2 with syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu").
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
index 457ec04950f7..8ada7e944147 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
@@ -103,6 +103,7 @@  static int aspeed_gfx_load(struct drm_device *drm)
 {
 	struct platform_device *pdev = to_platform_device(drm->dev);
 	struct aspeed_gfx *priv = to_aspeed_gfx(drm);
+	struct device_node *np = pdev->dev.of_node;
 	struct resource *res;
 	int ret;
 
@@ -111,10 +112,13 @@  static int aspeed_gfx_load(struct drm_device *drm)
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-	priv->scu = syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu");
+	priv->scu = syscon_regmap_lookup_by_phandle(np, "syscon");
 	if (IS_ERR(priv->scu)) {
-		dev_err(&pdev->dev, "failed to find SCU regmap\n");
-		return PTR_ERR(priv->scu);
+		priv->scu = syscon_regmap_lookup_by_compatible("aspeed,aspeed-scu");
+		if (IS_ERR(priv->scu)) {
+			dev_err(&pdev->dev, "failed to find SCU regmap\n");
+			return PTR_ERR(priv->scu);
+		}
 	}
 
 	ret = of_reserved_mem_device_init(drm->dev);