| Message ID | 20230822115200.712231-1-alexander.stein@ew.tq-group.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/1] drm/bridge: Silence error messages upon probe deferral | expand |
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; > } >
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.
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; }
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(-)