Hi Sam, On 14/10/2021 19:49, Sam Ravnborg wrote: > Hi Neil, > > one comment below. Other than that > Acked-by: Sam Ravnborg <sam@xxxxxxxxxxxx> > > Sam > > On Thu, Oct 14, 2021 at 05:26:01PM +0200, Neil Armstrong wrote: >> The initial design was recursive to cover all port/endpoints, but only the first layer >> of endpoints should be covered by the components list. >> This also breaks the MIPI-DSI init/bridge attach sequence, thus only parse the >> first endpoints instead of recursing. >> >> Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx> >> --- >> drivers/gpu/drm/meson/meson_drv.c | 62 +++++++++++-------------------- >> 1 file changed, 21 insertions(+), 41 deletions(-) >> >> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c >> index bc0d60df04ae..b53606d8825f 100644 >> --- a/drivers/gpu/drm/meson/meson_drv.c >> +++ b/drivers/gpu/drm/meson/meson_drv.c >> @@ -427,46 +427,6 @@ static int compare_of(struct device *dev, void *data) >> return dev->of_node == data; >> } >> >> -/* Possible connectors nodes to ignore */ >> -static const struct of_device_id connectors_match[] = { >> - { .compatible = "composite-video-connector" }, >> - { .compatible = "svideo-connector" }, >> - { .compatible = "hdmi-connector" }, >> - { .compatible = "dvi-connector" }, >> - {} >> -}; >> - >> -static int meson_probe_remote(struct platform_device *pdev, >> - struct component_match **match, >> - struct device_node *parent, >> - struct device_node *remote) >> -{ >> - struct device_node *ep, *remote_node; >> - int count = 1; >> - >> - /* If node is a connector, return and do not add to match table */ >> - if (of_match_node(connectors_match, remote)) >> - return 1; >> - >> - component_match_add(&pdev->dev, match, compare_of, remote); >> - >> - for_each_endpoint_of_node(remote, ep) { >> - remote_node = of_graph_get_remote_port_parent(ep); >> - if (!remote_node || >> - remote_node == parent || /* Ignore parent endpoint */ >> - !of_device_is_available(remote_node)) { >> - of_node_put(remote_node); >> - continue; >> - } >> - >> - count += meson_probe_remote(pdev, match, remote, remote_node); >> - >> - of_node_put(remote_node); >> - } >> - >> - return count; >> -} >> - >> static void meson_drv_shutdown(struct platform_device *pdev) >> { >> struct meson_drm *priv = dev_get_drvdata(&pdev->dev); >> @@ -478,6 +438,13 @@ static void meson_drv_shutdown(struct platform_device *pdev) >> drm_atomic_helper_shutdown(priv->drm); >> } >> >> +/* Possible connectors nodes to ignore */ >> +static const struct of_device_id connectors_match[] = { >> + { .compatible = "composite-video-connector" }, >> + { .compatible = "svideo-connector" }, >> + {} >> +}; >> + >> static int meson_drv_probe(struct platform_device *pdev) >> { >> struct component_match *match = NULL; >> @@ -492,8 +459,21 @@ static int meson_drv_probe(struct platform_device *pdev) >> continue; >> } >> >> - count += meson_probe_remote(pdev, &match, np, remote); >> + /* If an analog connector is detected, count it as an output */ >> + if (of_match_node(connectors_match, remote)) { >> + ++count; >> + of_node_put(remote); >> + continue; >> + } >> + >> + DRM_DEBUG_DRIVER("parent %pOF remote match add %pOF parent %s\n", >> + np, remote, dev_name(&pdev->dev)); > Avoid the deprecated logging functions. > Use drm_dbg() or if there is no drm_device just dev_dbg(). > > I assume the driver uses DRM_xxx all over, so I understand if you keep > it as-is. Since it's in the probe function, I will move to dev_dbg(). I'll probably do a print cleanup all over the driver. > >> + >> + component_match_add(&pdev->dev, &match, compare_of, remote); >> + >> of_node_put(remote); >> + >> + ++count; >> } >> >> if (count && !match) >> -- >> 2.25.1 Thanks, Neil