The return value of sp_tx_rst_aux() is not propagated, which means both compiler warnings and potential errors not being handled.
Fixes: 8bdfc5dae4e3 ("drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP")
Reported-by: kernel test robot lkp@intel.com Signed-off-by: Robert Foss robert.foss@linaro.org ---
Changes since v1: - Instead of discarding the error, propagate it until it is handled properly
drivers/gpu/drm/bridge/analogix/anx7625.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 14d73fb1dd15b..ea414cd349b5c 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -720,7 +720,7 @@ static int edid_read(struct anx7625_data *ctx, ret = sp_tx_aux_rd(ctx, 0xf1);
if (ret) { - sp_tx_rst_aux(ctx); + ret = sp_tx_rst_aux(ctx); DRM_DEV_DEBUG_DRIVER(dev, "edid read fail, reset!\n"); } else { ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client, @@ -735,7 +735,7 @@ static int edid_read(struct anx7625_data *ctx, if (cnt > EDID_TRY_CNT) return -EIO;
- return 0; + return ret; }
static int segments_edid_read(struct anx7625_data *ctx, @@ -785,7 +785,7 @@ static int segments_edid_read(struct anx7625_data *ctx, if (cnt > EDID_TRY_CNT) return -EIO;
- return 0; + return ret; }
static int sp_tx_edid_read(struct anx7625_data *ctx, @@ -887,7 +887,11 @@ static int sp_tx_edid_read(struct anx7625_data *ctx, }
/* Reset aux channel */ - sp_tx_rst_aux(ctx); + ret = sp_tx_rst_aux(ctx); + if (ret < 0) { + DRM_DEV_ERROR(dev, "Failed to reset aux channel!\n"); + return ret; + }
return (blocks_num + 1); }
During the sp_tx_edid_read() call the return value of sp_tx_edid_read() is ignored, which could cause potential errors to go unhandled.
All errors which are returned by sp_tx_edid_read() are handled in anx7625_get_edid().
Signed-off-by: Robert Foss robert.foss@linaro.org --- drivers/gpu/drm/bridge/analogix/anx7625.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index ea414cd349b5c..abc8db77bfd36 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -845,8 +845,11 @@ static int sp_tx_edid_read(struct anx7625_data *ctx, if (g_edid_break == 1) break;
- segments_edid_read(ctx, count / 2, - pblock_buf, offset); + ret = segments_edid_read(ctx, count / 2, + pblock_buf, offset); + if (ret < 0) + return ret; + memcpy(&pedid_blocks_buf[edid_pos], pblock_buf, MAX_DPCD_BUFFER_SIZE); @@ -863,8 +866,11 @@ static int sp_tx_edid_read(struct anx7625_data *ctx, if (g_edid_break == 1) break;
- segments_edid_read(ctx, count / 2, - pblock_buf, offset); + ret = segments_edid_read(ctx, count / 2, + pblock_buf, offset); + if (ret < 0) + return ret; + memcpy(&pedid_blocks_buf[edid_pos], pblock_buf, MAX_DPCD_BUFFER_SIZE);
Hi Robert,
On Wed, Aug 18, 2021 at 07:13:18PM +0200, Robert Foss wrote:
During the sp_tx_edid_read() call the return value of sp_tx_edid_read() is ignored, which could cause potential errors to go unhandled.
All errors which are returned by sp_tx_edid_read() are handled in anx7625_get_edid().
Signed-off-by: Robert Foss robert.foss@linaro.org
drivers/gpu/drm/bridge/analogix/anx7625.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index ea414cd349b5c..abc8db77bfd36 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -845,8 +845,11 @@ static int sp_tx_edid_read(struct anx7625_data *ctx, if (g_edid_break == 1) break;
segments_edid_read(ctx, count / 2,pblock_buf, offset);
ret = segments_edid_read(ctx, count / 2,pblock_buf, offset);if (ret < 0)return ret;
This could be just "if (ret)". Same goes for the next case.
With or without this simplification: Reviewed-by: Sam Ravnborg sam@ravnborg.org
I assume you will apply the patches.
Sam
memcpy(&pedid_blocks_buf[edid_pos], pblock_buf, MAX_DPCD_BUFFER_SIZE);@@ -863,8 +866,11 @@ static int sp_tx_edid_read(struct anx7625_data *ctx, if (g_edid_break == 1) break;
segments_edid_read(ctx, count / 2,pblock_buf, offset);
ret = segments_edid_read(ctx, count / 2,pblock_buf, offset);if (ret < 0)return ret;memcpy(&pedid_blocks_buf[edid_pos], pblock_buf, MAX_DPCD_BUFFER_SIZE);-- 2.30.2
Thanks Sam!
On Wed, 18 Aug 2021 at 19:40, Sam Ravnborg sam@ravnborg.org wrote:
Hi Robert,
On Wed, Aug 18, 2021 at 07:13:18PM +0200, Robert Foss wrote:
During the sp_tx_edid_read() call the return value of sp_tx_edid_read() is ignored, which could cause potential errors to go unhandled.
All errors which are returned by sp_tx_edid_read() are handled in anx7625_get_edid().
Signed-off-by: Robert Foss robert.foss@linaro.org
drivers/gpu/drm/bridge/analogix/anx7625.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index ea414cd349b5c..abc8db77bfd36 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -845,8 +845,11 @@ static int sp_tx_edid_read(struct anx7625_data *ctx, if (g_edid_break == 1) break;
segments_edid_read(ctx, count / 2,pblock_buf, offset);
ret = segments_edid_read(ctx, count / 2,pblock_buf, offset);if (ret < 0)return ret;This could be just "if (ret)". Same goes for the next case.
With or without this simplification: Reviewed-by: Sam Ravnborg sam@ravnborg.org
I assume you will apply the patches.
Applied to drm-misc-next
Hi Robert,
On Wed, Aug 18, 2021 at 07:13:17PM +0200, Robert Foss wrote:
The return value of sp_tx_rst_aux() is not propagated, which means both compiler warnings and potential errors not being handled.
Fixes: 8bdfc5dae4e3 ("drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP")
Reported-by: kernel test robot lkp@intel.com Signed-off-by: Robert Foss robert.foss@linaro.org
Reviewed-by: Sam Ravnborg sam@ravnborg.org
dri-devel@lists.freedesktop.org
-
Robert Foss -
Sam Ravnborg