[PATCH 3/3] drm: bridge: Switch to devm_drm_of_get_bridge

[Linus Walleij]

On Fri, Dec 10, 2021 at 6:49 PM Jagan Teki <jagan at amarulasolutions.com> wrote:
>
> devm_drm_of_get_bridge is capable of looking up the downstream
> bridge and panel and trying to add a panel bridge if the panel
> is found.
>
> Replace explicit finding calls with devm_drm_of_get_bridge.
>
> Cc: Philipp Zabel <p.zabel at pengutronix.de>
> Cc: Chun-Kuang Hu <chunkuang.hu at kernel.org>
> Cc: Linus Walleij <linus.walleij at linaro.org>
> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>

Nice overall!

> -       /* Look for a panel as a child to this node */
> -       for_each_available_child_of_node(dev->of_node, child) {
> -               panel = of_drm_find_panel(child);
> -               if (IS_ERR(panel)) {
> -                       dev_err(dev, "failed to find panel try bridge (%ld)\n",
> -                               PTR_ERR(panel));
> -                       panel = NULL;
> -
> -                       bridge = of_drm_find_bridge(child);
> -                       if (!bridge) {
> -                               dev_err(dev, "failed to find bridge\n");
> -                               return -EINVAL;
> -                       }
> -               }
> -       }
> -       if (panel) {
> -               bridge = drm_panel_bridge_add_typed(panel,
> -                                                   DRM_MODE_CONNECTOR_DSI);

And we are guaranteed that the right type of connector will be
used here? (Just checking.)

> -               if (IS_ERR(bridge)) {
> -                       dev_err(dev, "error adding panel bridge\n");
> -                       return PTR_ERR(bridge);
> -               }
> -               dev_info(dev, "connected to panel\n");
> -               d->panel = panel;

How does this assignment happen after your patch?
I'm using that...

devm_drm_of_get_bridge() needs some more argument right?

> -       } else if (bridge) {
> -               /* TODO: AV8100 HDMI encoder goes here for example */
> -               dev_info(dev, "connected to non-panel bridge (unsupported)\n");
> -               return -ENODEV;
> -       } else {
> -               dev_err(dev, "no panel or bridge\n");
> -               return -ENODEV;
> +       bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0);
> +       if (IS_ERR(bridge)) {
> +               dev_err(dev, "error to get bridge\n");
> +               return PTR_ERR(bridge);

I'm gonna want to test this somehow on the hardware. But the TODO comment
there wasn't supposed to be deleted if I will still need to take some special
action whether this is a panel bridge or some other bridge.

Yours,
Linus Walleij