Change in coreboot[master]: drivers/analogix/anx7625: Fix edid_read()

[Yu-Ping Wu (Code Review)]

Yu-Ping Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59540 )

Change subject: drivers/analogix/anx7625: Fix edid_read()
......................................................................

drivers/analogix/anx7625: Fix edid_read()

The current implementations of edid_read() and segments_edid_read() have
a few problems:

1. The type of variable `c` ic incorrect, not matching the return type
   of sp_tx_aux_rd(). In addition, the meaning of `c` is unknown.
2. It is pointless to do `cnt++` when sp_tx_aux_rd() fails.
3. These two functions ignore the return value of
   anx7625_reg_block_read().
4. In segments_edid_read(), anx7625_reg_write() might return a positive
   value on failure.

Fix all of the 4 issues, and modify the code to be closer to kernel's
implementation (drivers/gpu/drm/bridge/analogix/anx7625.c). Note that,
however, unlike in kernel, anx7625_reg_block_read() here doesn't return
the number of bytes. On success, 0 is returned instead.

In addition, change the return value to -1 for edid_read() and
segments_edid_read() on failure.

BUG=b:207055969
TEST=emerge-asurada coreboot
BRANCH=none

Change-Id: Ife9d7d97df2926b4581ba519a152c9efed8cd969
Signed-off-by: Yu-Ping Wu 
---
M src/drivers/analogix/anx7625/anx7625.c
1 file changed, 26 insertions(+), 30 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/59540/1

diff --git a/src/drivers/analogix/anx7625/anx7625.c b/src/drivers/analogix/anx7625/anx7625.c
index 5fbcf20..638e793 100644
--- a/src/drivers/analogix/anx7625/anx7625.c
+++ b/src/drivers/analogix/anx7625/anx7625.c
@@ -558,34 +558,32 @@

static int edid_read(uint8_t bus, uint8_t offset, uint8_t *pblock_buf)
 {
-	uint8_t c, cnt = 0;
+	int ret, cnt;

-	c = 0;
    for (cnt = 0; cnt < 3; cnt++) {
    	sp_tx_aux_wr(bus, offset);
    	/* set I2C read com 0x01 mot = 0 and read 16 bytes */
-		c = sp_tx_aux_rd(bus, 0xf1);
+		ret = sp_tx_aux_rd(bus, 0xf1);

-		if (c == 1) {
+		if (ret) {
    		sp_tx_rst_aux(bus);
    		ANXERROR("edid read failed, reset!\n");
-			cnt++;
    	} else {
-			anx7625_reg_block_read(bus, RX_P0_ADDR,
-					AP_AUX_BUFF_START,
-					MAX_DPCD_BUFFER_SIZE, pblock_buf);
-			return 0;
+			if (anx7625_reg_block_read(bus, RX_P0_ADDR,
+						   AP_AUX_BUFF_START,
+						   MAX_DPCD_BUFFER_SIZE,
+						   pblock_buf) == 0)
+				return 0;
    	}
    }

-	return 1;
+	return -1;
 }

static int segments_edid_read(uint8_t bus, uint8_t segment, uint8_t *buf,
    		      uint8_t offset)
 {
-	uint8_t c, cnt = 0;
-	int ret;
+	int ret, cnt;

/* write address only */
    ret = anx7625_reg_write(bus, RX_P0_ADDR, AP_AUX_ADDR_7_0, 0x30);
@@ -599,7 +597,7 @@
    /* data read */
    ret |= anx7625_reg_write(bus, RX_P0_ADDR, AP_AUX_ADDR_7_0, 0x50);

-	if (ret < 0) {
+	if (ret) {
    	ANXERROR("IO error: aux initial failed.\n");
    	return ret;
    }
@@ -607,21 +605,21 @@
    for (cnt = 0; cnt < 3; cnt++) {
    	sp_tx_aux_wr(bus, offset);
    	/* set I2C read com 0x01 mot = 0 and read 16 bytes */
-		c = sp_tx_aux_rd(bus, 0xf1);
+		ret = sp_tx_aux_rd(bus, 0xf1);

-		if (c == 1) {
-			ret = sp_tx_rst_aux(bus);
+		if (ret) {
+			sp_tx_rst_aux(bus);
    		ANXERROR("segment read failed, reset!\n");
-			cnt++;
    	} else {
-			ret = anx7625_reg_block_read(bus, RX_P0_ADDR,
-					AP_AUX_BUFF_START,
-					MAX_DPCD_BUFFER_SIZE, buf);
-			return ret;
+			if (anx7625_reg_block_read(bus, RX_P0_ADDR,
+						   AP_AUX_BUFF_START,
+						   MAX_DPCD_BUFFER_SIZE,
+						   buf) == 0)
+				return 0;
    	}
    }

-	return ret;
+	return -1;
 }

static int sp_tx_edid_read(uint8_t bus, uint8_t *pedid_blocks_buf,
@@ -630,9 +628,7 @@
    uint8_t offset, edid_pos;
    int count, blocks_num;
    uint8_t pblock_buf[MAX_DPCD_BUFFER_SIZE];
-	uint8_t i;
-	uint8_t g_edid_break = 0;
-	int ret;
+	int i, ret, g_edid_break = 0;

/* address initial */
    ret = anx7625_reg_write(bus, RX_P0_ADDR, AP_AUX_ADDR_7_0, 0x50);
@@ -656,10 +652,10 @@
    	case 1:
    		for (i = 0; i < 8; i++) {
    			offset = (i + count * 8) * MAX_DPCD_BUFFER_SIZE;
-				g_edid_break = edid_read(bus, offset,
-						pblock_buf);
+				g_edid_break = !!edid_read(bus, offset,
+							   pblock_buf);

-				if (g_edid_break == 1)
+				if (g_edid_break)
    				break;

if (offset <= size - MAX_DPCD_BUFFER_SIZE)
@@ -677,11 +673,11 @@
    			edid_pos = (i + count * 8) *
    				MAX_DPCD_BUFFER_SIZE;

-				if (g_edid_break == 1)
+				if (g_edid_break)
    				break;

segments_edid_read(bus, count / 2,
-						pblock_buf, offset);
+						   pblock_buf, offset);
    			if (edid_pos <= size - MAX_DPCD_BUFFER_SIZE)
    				memcpy(&pedid_blocks_buf[edid_pos],
    				       pblock_buf,