[RESEND,v5,4/5] drm/bridge/sii8620: fix resource acquisition error handling
diff mbox series

Message ID 20200624114127.3016-5-a.hajda@samsung.com
State New
Headers show
Series
  • [RESEND,v5,1/5] driver core: add probe_err log helper
Related show

Commit Message

Andrzej Hajda June 24, 2020, 11:41 a.m. UTC
In case of error during resource acquisition driver should print error
message only in case it is not deferred probe, using probe_err helper
solves the issue. Moreover it records defer probe reason for debugging.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/bridge/sil-sii8620.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

Comments

Mark Brown June 24, 2020, 1:25 p.m. UTC | #1
On Wed, Jun 24, 2020 at 01:41:26PM +0200, Andrzej Hajda wrote:
> In case of error during resource acquisition driver should print error
> message only in case it is not deferred probe, using probe_err helper
> solves the issue. Moreover it records defer probe reason for debugging.

If we silently ignore all deferred probe errors we make it hard for
anyone who is experiencing issues with deferred probe to figure out what
they're missing.  We should at least be logging problems at debug level
so there's something for people to go on without having to hack the
kernel source.
Mark Brown June 24, 2020, 2:11 p.m. UTC | #2
On Wed, Jun 24, 2020 at 03:43:10PM +0200, Andrzej Hajda wrote:
> 
> On 24.06.2020 15:25, Mark Brown wrote:

> > If we silently ignore all deferred probe errors we make it hard for
> > anyone who is experiencing issues with deferred probe to figure out what
> > they're missing.  We should at least be logging problems at debug level
> > so there's something for people to go on without having to hack the
> > kernel source.

> But you can always do:

> cat /sys/kernel/debug/devices_deferred

> And you will find there deferred probe reason (thanks to patch 2/5).

Right, my point is more that we shouldn't be promoting discarding the
diagnostics entirely but rather saying that we want to redirect those
somewhere else.

> Eventually if you want it in dmesg anyway, one can adjust probe_err function
> to log probe error on debug level as well.

That would most likely be very useful as a boot option for problems that
occur before we get a console up.

Patch
diff mbox series

diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index 92acd336aa89..2f825b2d0098 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -2299,10 +2299,8 @@  static int sii8620_probe(struct i2c_client *client,
 	INIT_LIST_HEAD(&ctx->mt_queue);
 
 	ctx->clk_xtal = devm_clk_get(dev, "xtal");
-	if (IS_ERR(ctx->clk_xtal)) {
-		dev_err(dev, "failed to get xtal clock from DT\n");
-		return PTR_ERR(ctx->clk_xtal);
-	}
+	if (IS_ERR(ctx->clk_xtal))
+		return probe_err(dev, ctx->clk_xtal, "failed to get xtal clock from DT\n");
 
 	if (!client->irq) {
 		dev_err(dev, "no irq provided\n");
@@ -2313,16 +2311,12 @@  static int sii8620_probe(struct i2c_client *client,
 					sii8620_irq_thread,
 					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
 					"sii8620", ctx);
-	if (ret < 0) {
-		dev_err(dev, "failed to install IRQ handler\n");
-		return ret;
-	}
+	if (ret < 0)
+		return probe_err(dev, ret, "failed to install IRQ handler\n");
 
 	ctx->gpio_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
-	if (IS_ERR(ctx->gpio_reset)) {
-		dev_err(dev, "failed to get reset gpio from DT\n");
-		return PTR_ERR(ctx->gpio_reset);
-	}
+	if (IS_ERR(ctx->gpio_reset))
+		return probe_err(dev, ctx->gpio_reset, "failed to get reset gpio from DT\n");
 
 	ctx->supplies[0].supply = "cvcc10";
 	ctx->supplies[1].supply = "iovcc18";