[PATCH v2 1/1] drm/bridge: Silence error messages upon probe deferral

[PATCH v2 1/1] drm/bridge: Silence error messages upon probe deferral

[Alexander Stein]

When -EPROBE_DEFER is returned do not raise an error, but silently return
this error instead. Fixes error like this:
[drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@30800000/mipi-dsi@30a00000 to encoder None-34: -517
[drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@30800000/mipi-dsi@30a00000 to encoder None-34: -517

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* Adjust the indentation

Considering Laurent's input IMHO -517 should not occur when using component
framework, e.g. drivers/gpu/drm/mcde/mcde_drv.c. This should warrant to only
print an error if it is not deferred probe.
dev_err_probe() sounds reasonable, but this is something which should be done
in drivers. It is also arguable if this message is "hidden" within a debug
statement.

 drivers/gpu/drm/drm_bridge.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 39e68e45bb124..132180a03c0eb 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -352,13 +352,15 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
 	bridge->encoder = NULL;
 	list_del(&bridge->chain_node);
 
+	if (ret != -EPROBE_DEFER) {
 #ifdef CONFIG_OF
-	DRM_ERROR("failed to attach bridge %pOF to encoder %s: %d\n",
-		  bridge->of_node, encoder->name, ret);
+		DRM_ERROR("failed to attach bridge %pOF to encoder %s: %d\n",
+			bridge->of_node, encoder->name, ret);
 #else
-	DRM_ERROR("failed to attach bridge to encoder %s: %d\n",
-		  encoder->name, ret);
+		DRM_ERROR("failed to attach bridge to encoder %s: %d\n",
+			encoder->name, ret);
 #endif
+	}
 
 	return ret;
 }
-- 
2.34.1

Re: [PATCH v2 1/1] drm/bridge: Silence error messages upon probe deferral

[Alexander Stein]

Hi everyone,

Am Dienstag, 22. August 2023, 13:52:00 CET schrieb Alexander Stein:
> When -EPROBE_DEFER is returned do not raise an error, but silently return
> this error instead. Fixes error like this:
> [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@30800000/mipi-dsi@30a00000 to encoder None-34: -517
> [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@30800000/mipi-dsi@30a00000 to encoder None-34: -517
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Changes in v2:
> * Adjust the indentation
> 
> Considering Laurent's input IMHO -517 should not occur when using component
> framework, e.g. drivers/gpu/drm/mcde/mcde_drv.c. This should warrant to only
> print an error if it is not deferred probe.
> dev_err_probe() sounds reasonable, but this is something which should be done
> in drivers. It is also arguable if this message is "hidden" within a debug
> statement.

Any additional feedback on this? I'd like to get rid of an error message which just prints
a regular probe deferral.

Best regards,
Alexander

>  drivers/gpu/drm/drm_bridge.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 39e68e45bb124..132180a03c0eb 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -352,13 +352,15 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
>  	bridge->encoder = NULL;
>  	list_del(&bridge->chain_node);
>  
> +	if (ret != -EPROBE_DEFER) {
>  #ifdef CONFIG_OF
> -	DRM_ERROR("failed to attach bridge %pOF to encoder %s: %d\n",
> -		  bridge->of_node, encoder->name, ret);
> +		DRM_ERROR("failed to attach bridge %pOF to encoder %s: %d\n",
> +			bridge->of_node, encoder->name, ret);
>  #else
> -	DRM_ERROR("failed to attach bridge to encoder %s: %d\n",
> -		  encoder->name, ret);
> +		DRM_ERROR("failed to attach bridge to encoder %s: %d\n",
> +			encoder->name, ret);
>  #endif
> +	}
>  
>  	return ret;
>  }
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH v2 1/1] drm/bridge: Silence error messages upon probe deferral

[Robert Foss]

On Tue, Feb 20, 2024 at 3:43 PM Alexander Stein
<alexander.stein@ew.tq-group.com> wrote:
>
> Hi everyone,
>
> Am Dienstag, 22. August 2023, 13:52:00 CET schrieb Alexander Stein:
> > When -EPROBE_DEFER is returned do not raise an error, but silently return
> > this error instead. Fixes error like this:
> > [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@30800000/mipi-dsi@30a00000 to encoder None-34: -517
> > [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@30800000/mipi-dsi@30a00000 to encoder None-34: -517
> >
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > Changes in v2:
> > * Adjust the indentation
> >
> > Considering Laurent's input IMHO -517 should not occur when using component
> > framework, e.g. drivers/gpu/drm/mcde/mcde_drv.c. This should warrant to only
> > print an error if it is not deferred probe.
> > dev_err_probe() sounds reasonable, but this is something which should be done
> > in drivers. It is also arguable if this message is "hidden" within a debug
> > statement.
>
> Any additional feedback on this? I'd like to get rid of an error message which just prints
> a regular probe deferral.
>
> Best regards,
> Alexander
>
> >  drivers/gpu/drm/drm_bridge.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > index 39e68e45bb124..132180a03c0eb 100644
> > --- a/drivers/gpu/drm/drm_bridge.c
> > +++ b/drivers/gpu/drm/drm_bridge.c
> > @@ -352,13 +352,15 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> >       bridge->encoder = NULL;
> >       list_del(&bridge->chain_node);
> >
> > +     if (ret != -EPROBE_DEFER) {
> >  #ifdef CONFIG_OF
> > -     DRM_ERROR("failed to attach bridge %pOF to encoder %s: %d\n",
> > -               bridge->of_node, encoder->name, ret);
> > +             DRM_ERROR("failed to attach bridge %pOF to encoder %s: %d\n",
> > +                     bridge->of_node, encoder->name, ret);
> >  #else
> > -     DRM_ERROR("failed to attach bridge to encoder %s: %d\n",
> > -               encoder->name, ret);
> > +             DRM_ERROR("failed to attach bridge to encoder %s: %d\n",
> > +                     encoder->name, ret);

checkpatch --strict warns for both of the above chunks.

CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis

With this fixed please add my r-b.