Re: [PATCH 11/12] drm/vc4: move to devm_platform_ioremap_resource() usage

Maxime Ripard
2025-02-14T09:44:07Z

[-- Attachment #1: Type: text/plain, Size: 3392 bytes --]

On Tue, Feb 11, 2025 at 12:08:57PM -0500, Anusha Srivatsa wrote:
> On Thu, Feb 6, 2025 at 11:13 AM Maxime Ripard <mripard@kernel.org> wrote:
>
> > On Wed, Feb 05, 2025 at 03:08:07PM -0500, Anusha Srivatsa wrote:
> > > Replace platform_get_resource_byname + devm_ioremap_resource
> > > with just devm_platform_ioremap_resource()
> > >
> > > Used Coccinelle to do this change. SmPl patch:
> > > //rule s/(devm_)platform_get_resource_byname +
> > > //(devm_)ioremap/devm_platform_ioremap_resource.
> > > @rule_3@
> > > identifier res;
> > > expression ioremap;
> > > identifier pdev;
> > > constant mem;
> > > expression name;
> > > @@
> > > -struct resource *res;
> > > <+...
> > > -res = platform_get_resource_byname(pdev,mem,name);
> > > <...
> > > -if (!res) {
> > > -...
> > > -}
> > > ...>
> > > -ioremap = devm_ioremap(...);
> > > +ioremap = devm_platform_ioremap_resource_byname(pdev,name);
> > > ...+>
> > >
> > > v2: Change the SmPl patch to work on multiple occurences of
> > > the pattern. This also fixes the compilation error.
> > >
> > > Cc: Maxime Ripard <mripard@kernel.org>
> > > Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > Cc: Maíra Canal <mcanal@igalia.com>
> > > Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
> > > ---
> > > drivers/gpu/drm/vc4/vc4_hdmi.c | 55
> > +++++++++++-------------------------------
> > > 1 file changed, 14 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > index
> > 47d9ada98430634cfd8c1e21c2a4d00d501bab7e..066f1246dab420ee889845b0c573d80ce7c88595
> > 100644
> > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > @@ -2951,71 +2951,44 @@ static int vc5_hdmi_init_resources(struct
> > drm_device *drm,
> > > {
> > > struct platform_device *pdev = vc4_hdmi->pdev;
> > > struct device *dev = &pdev->dev;
> > > - struct resource *res;
> > > int ret;
> > >
> > > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hdmi");
> > > - if (!res)
> > > - return -ENODEV;
> > > -
> > > - vc4_hdmi->hdmicore_regs = devm_ioremap(dev, res->start,
> > > - resource_size(res));
> > > + vc4_hdmi->hdmicore_regs =
> > devm_platform_ioremap_resource_byname(pdev,
> > > +
> > "hdmi");
> > > if (!vc4_hdmi->hdmicore_regs)
> > > return -ENOMEM;
> > >
> > > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hd");
> > > - if (!res)
> > > - return -ENODEV;
> > > -
> > > - vc4_hdmi->hd_regs = devm_ioremap(dev, res->start,
> > resource_size(res));
> > > + vc4_hdmi->hd_regs = devm_platform_ioremap_resource_byname(pdev,
> > "hd");
> > > if (!vc4_hdmi->hd_regs)
> > > return -ENOMEM;
> >
> > I *think* that one is shared between both HDMI controllers on the
> > RaspberryPi4, so we can't claim them from both instances. We should add
> > a comment there to document that it's on purpose.
>
> How about vc4_hdmi->hdmicore_regs? It also has another instance
> vc4_hdmi_init_resources(). Looks like that also doesnt need any converting
> and shold be left as is.

No, each controller will have its own set of registers there, so it can
be converted.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]