Re: [PATCH v2 2/2] drm: rcar-du: mipi-dsi: Use devm_drm_of_get_bridge helper

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Kieran,

On Tue, Nov 30, 2021 at 04:52:19PM +0000, Kieran Bingham wrote:
> Quoting Kieran Bingham (2021-11-30 16:25:13)
> > Instead of open coding the calls for
> >   drm_of_find_panel_or_bridge()
> >   devm_drm_panel_bridge_add()
> > 
> > use the devm_drm_of_get_bridge() helper directly.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > ---
> > v2:
> >  - New patch
> > 
> >  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 19 ++++---------------
> >  1 file changed, 4 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > index 0a9f197ef62c..1dfe20d3d0f2 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > @@ -637,7 +637,7 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> >                                         struct mipi_dsi_device *device)
> >  {
> >         struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host);
> > -       struct drm_panel *panel;
> > +       struct device *dev = dsi->dev;
> >         int ret;
> >  
> >         if (device->lanes > dsi->num_data_lanes)
> > @@ -646,20 +646,9 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> >         dsi->lanes = device->lanes;
> >         dsi->format = device->format;
> >  
> > -       ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
> > -                                         &dsi->next_bridge);
> > -       if (ret) {
> > -               dev_err_probe(dsi->dev, ret, "could not find next bridge\n");
> > -               return ret;
> > -       }
> > -
> > -       if (!dsi->next_bridge) {
> > -               dsi->next_bridge = devm_drm_panel_bridge_add(dsi->dev, panel);
> > -               if (IS_ERR(dsi->next_bridge)) {
> > -                       dev_err(dsi->dev, "failed to create panel bridge\n");
> > -                       return PTR_ERR(dsi->next_bridge);
> > -               }
> > -       }
> > +       dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> > +       if (IS_ERR(dsi->next_bridge))
> > +               return PTR_ERR(dsi->next_bridge);
> 
> I did make a change here to make this:
> 
> 	dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> 	if (IS_ERR(dsi->next_bridge)) {
> 		dev_err(dev, "failed to get next bridge\n");
> 		return PTR_ERR(dsi->next_bridge);
> 	}
> 
> But it seems I got out of sequence and saved out the wrong patch ;-(
> 
> If you think it's better with the error print, please add it while
> squashing, or if you really need, I can send an updated patch and
> retest.

I think an error message is useful, yes. I'll add it manually.

> >  
> >         /* Initialize the DRM bridge. */
> >         dsi->bridge.funcs = &rcar_mipi_dsi_bridge_ops;

-- 
Regards,

Laurent Pinchart