Re: [PATCH v3] drm/amdgpu: add return result for amdgpu_i2c_{get/put}_byte

[Christian König]

Am 24.04.24 um 11:36 schrieb Bob Zhou:

After amdgpu_i2c_get_byte fail, amdgpu_i2c_put_byte shouldn't be
conducted to put wrong value.
So return and check the i2c transfer result.

Signed-off-by: Bob Zhou <bob.z...@amd.com>
Suggested-by: Christian König <christian.koe...@amd.com>

Reviewed-by: Christian König <christian.koe...@amd.com>

---
  drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c | 47 +++++++++++++++----------
  1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c
index 82608df43396..e0f3bff335c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c
@@ -280,7 +280,7 @@ amdgpu_i2c_lookup(struct amdgpu_device *adev,
        return NULL;
  }

-static void amdgpu_i2c_get_byte(struct amdgpu_i2c_chan *i2c_bus,

+static int amdgpu_i2c_get_byte(struct amdgpu_i2c_chan *i2c_bus,
                                 u8 slave_addr,
                                 u8 addr,
                                 u8 *val)
@@ -305,16 +305,18 @@ static void amdgpu_i2c_get_byte(struct amdgpu_i2c_chan 
*i2c_bus,
        out_buf[0] = addr;
        out_buf[1] = 0;

- if (i2c_transfer(&i2c_bus->adapter, msgs, 2) == 2) {

-               *val = in_buf[0];
-               DRM_DEBUG("val = 0x%02x\n", *val);
-       } else {
-               DRM_DEBUG("i2c 0x%02x 0x%02x read failed\n",
-                         addr, *val);
+       if (i2c_transfer(&i2c_bus->adapter, msgs, 2) != 2) {
+               DRM_DEBUG("i2c 0x%02x read failed\n", addr);
+               return -EIO;
        }
+
+       *val = in_buf[0];
+       DRM_DEBUG("val = 0x%02x\n", *val);
+
+       return 0;
  }

-static void amdgpu_i2c_put_byte(struct amdgpu_i2c_chan *i2c_bus,

+static int amdgpu_i2c_put_byte(struct amdgpu_i2c_chan *i2c_bus,
                                 u8 slave_addr,
                                 u8 addr,
                                 u8 val)
@@ -330,9 +332,12 @@ static void amdgpu_i2c_put_byte(struct amdgpu_i2c_chan 
*i2c_bus,
        out_buf[0] = addr;
        out_buf[1] = val;

- if (i2c_transfer(&i2c_bus->adapter, &msg, 1) != 1)

-               DRM_DEBUG("i2c 0x%02x 0x%02x write failed\n",
-                         addr, val);
+       if (i2c_transfer(&i2c_bus->adapter, &msg, 1) != 1) {
+               DRM_DEBUG("i2c 0x%02x 0x%02x write failed\n", addr, val);
+               return -EIO;
+       }
+
+       return 0;
  }

/* ddc router switching */

@@ -347,16 +352,18 @@ amdgpu_i2c_router_select_ddc_port(const struct 
amdgpu_connector *amdgpu_connecto
        if (!amdgpu_connector->router_bus)
                return;

- amdgpu_i2c_get_byte(amdgpu_connector->router_bus,

+       if (amdgpu_i2c_get_byte(amdgpu_connector->router_bus,
                            amdgpu_connector->router.i2c_addr,
-                           0x3, &val);
+                           0x3, &val))
+               return;
        val &= ~amdgpu_connector->router.ddc_mux_control_pin;
        amdgpu_i2c_put_byte(amdgpu_connector->router_bus,
                            amdgpu_connector->router.i2c_addr,
                            0x3, val);
-       amdgpu_i2c_get_byte(amdgpu_connector->router_bus,
+       if (amdgpu_i2c_get_byte(amdgpu_connector->router_bus,
                            amdgpu_connector->router.i2c_addr,
-                           0x1, &val);
+                           0x1, &val))
+               return;
        val &= ~amdgpu_connector->router.ddc_mux_control_pin;
        val |= amdgpu_connector->router.ddc_mux_state;
        amdgpu_i2c_put_byte(amdgpu_connector->router_bus,
@@ -376,16 +383,18 @@ amdgpu_i2c_router_select_cd_port(const struct 
amdgpu_connector *amdgpu_connector
        if (!amdgpu_connector->router_bus)
                return;

- amdgpu_i2c_get_byte(amdgpu_connector->router_bus,

+       if (amdgpu_i2c_get_byte(amdgpu_connector->router_bus,
                            amdgpu_connector->router.i2c_addr,
-                           0x3, &val);
+                           0x3, &val))
+               return;
        val &= ~amdgpu_connector->router.cd_mux_control_pin;
        amdgpu_i2c_put_byte(amdgpu_connector->router_bus,
                            amdgpu_connector->router.i2c_addr,
                            0x3, val);
-       amdgpu_i2c_get_byte(amdgpu_connector->router_bus,
+       if (amdgpu_i2c_get_byte(amdgpu_connector->router_bus,
                            amdgpu_connector->router.i2c_addr,
-                           0x1, &val);
+                           0x1, &val))
+               return;
        val &= ~amdgpu_connector->router.cd_mux_control_pin;
        val |= amdgpu_connector->router.cd_mux_state;
        amdgpu_i2c_put_byte(amdgpu_connector->router_bus,