Re: [PATCH] drm/bridge: nxp-ptn3460: fix i2c_master_send() error checking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/12/2023 17:59, Dan Carpenter wrote:
On Mon, Dec 04, 2023 at 02:53:05PM +0100, Neil Armstrong wrote:
On 04/12/2023 13:29, Dan Carpenter wrote:
The i2c_master_send/recv() functions return negative error codes or the
number of bytes that were able to be sent/received.  This code has
two problems.  1)  Instead of checking if all the bytes were sent or
received, it checks that at least one byte was sent or received.
2) If there was a partial send/receive then we should return a negative
error code but this code returns success.

Fixes: a9fe713d7d45 ("drm/bridge: Add PTN3460 bridge driver")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
---
This is from static analysis and code review.  It's always a concern
when you add stricter error handling that something will break.

   drivers/gpu/drm/bridge/nxp-ptn3460.c | 10 +++++-----
   1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c b/drivers/gpu/drm/bridge/nxp-ptn3460.c
index d81920227a8a..9b7eb8c669c1 100644
--- a/drivers/gpu/drm/bridge/nxp-ptn3460.c
+++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c
@@ -56,13 +56,13 @@ static int ptn3460_read_bytes(struct ptn3460_bridge *ptn_bridge, char addr,
   	ret = i2c_master_send(ptn_bridge->client, &addr, 1);
   	if (ret <= 0) {
   		DRM_ERROR("Failed to send i2c command, ret=%d\n", ret);
-		return ret;
+		return ret ?: -EIO;
   	}
   	ret = i2c_master_recv(ptn_bridge->client, buf, len);
-	if (ret <= 0) {
+	if (ret != len) {

This is impossible, i2c_transfer_buffer_flags() returns len as-is if no error, so
ret can only be negative or equal to len. The original code is right.

It works, but it's not "right".  The <= 0 could be changed to < 0.  The
"len" variable is EDID_LENGTH (128).

So indeed, switching to < 0 is the most reasonable, no need to change the ret value in this case.

Neil


regards,
dan carpenter






[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux