Re: [PATCH v4 02/11] drm/bridge: it6505: fix aux operation for edid read

[AngeloGioacchino Del Regno]

Il 26/09/24 09:47, Hermes Wu ha scritto:

From: Hermes Wu <hermes...@ite.com.tw>

The EDID read operation can reach the maximum size of the AUX FIFO buffer.

Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
Signed-off-by: Hermes Wu <hermes...@ite.com.tw>
---
  drivers/gpu/drm/bridge/ite-it6505.c | 8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c 
b/drivers/gpu/drm/bridge/ite-it6505.c
index 28a8043229d3..b451d3c2ac1d 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -1078,8 +1078,11 @@ static ssize_t it6505_aux_do_transfer(struct it6505 
*it6505,
        int i, ret_size, ret = 0, request_size;

int fifo_max_sz = (cmd == CMD_AUX_I2C_EDID_READ) ?
                  AUX_FIFO_MAX_SIZE : 4;

which later becomes

int fifo_max_sz = (cmd == CMD_AUX_I2C_EDID_READ || cmd == CMD_AUX_GET_KSV_LIST) 
?
                  AUX_FIFO_MAX_SIZE : 4;

otherwise you can do it "the long way"...

if (cmd == .....)
        fifo_max_sz = AUX_FIFO_MAX_SIZE;
else
        fifo_max_sz = 4;

The point is, cmd won't change during iterations, so it's useless to put that if
branch inside of that loop below.

mutex_lock(&it6505->aux_lock);

-       for (i = 0; i < size; i += 4) {
-               request_size = min((int)size - i, 4);

                request_size = min_t(int, (int)size - i, fifo_max_sz);

P.S.: Also, I'd consider changing this to a do-while instead...

Cheers,
Angelo

+       for (i = 0; i < size; ) {
+               if (cmd == CMD_AUX_I2C_EDID_READ)
+                       request_size = min_t(int, (int)size - i, 
AUX_FIFO_MAX_SIZE);
+               else
+                       request_size = min_t(int, (int)size - i, 4);
                ret_size = it6505_aux_operation(it6505, cmd, address + i,
                                                buffer + i, request_size,
                                                reply);
@@ -1088,6 +1091,7 @@ static ssize_t it6505_aux_do_transfer(struct it6505 
*it6505,
                        goto aux_op_err;
                }

+ i += request_size;

                ret += ret_size;
        }