Re: [PATCH v2 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver

This code is *so* much nicer than before. I hope you feel good about the changes. It makes me happy to look at this code now.

On Thu, Oct 10, 2019 at 09:34:19AM +0000, Xin Ji wrote:

+static int edid_read(struct anx7625_data *ctx,

•      u8 offset, u8 *pblock_buf)
  

+{

• int ret, cnt;
• struct device *dev = &ctx->client->dev;
• for (cnt = 0; cnt < EDID_TRY_CNT; cnt++) {

• sp_tx_aux_wr(ctx, offset);
  

• /* set I2C read com 0x01 mot = 0 and read 16 bytes */
  

• ret = sp_tx_aux_rd(ctx, 0xf1);
  

• if (ret) {
  

• 	sp_tx_rst_aux(ctx);
  

• 	DRM_DEV_DEBUG_DRIVER(dev, "edid read failed, reset!\n");
  

• 	cnt++;
  

I don't think you should increment cnt. It's just a counter.

• } else {
  

• 	ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client,
  

• 				     AP_AUX_BUFF_START,
  

• 				     MAX_DPCD_BUFFER_SIZE,
  

• 				     pblock_buf);
  

• 	if (!ret)
  

• 		break;
  

• }
  

• }
• if (cnt == EDID_TRY_CNT)

And it could mean that "cnt > EDID_TRY_CNT".

• return -EIO;
  

• return 0;

+}

+static int segments_edid_read(struct anx7625_data *ctx,

• 	      u8 segment, u8 *buf, u8 offset)
  

+{

• u8 cnt;
• int ret;
• struct device *dev = &ctx->client->dev;
• /* write address only */
• ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,

• 		AP_AUX_ADDR_7_0, 0x30);
  

• ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,

• 		 AP_AUX_COMMAND, 0x04);
  

• ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,

• 		 AP_AUX_CTRL_STATUS,
  

• 		 AP_AUX_CTRL_ADDRONLY | AP_AUX_CTRL_OP_EN);
  

• ret |= wait_aux_op_finish(ctx);
• /* write segment address */
• ret |= sp_tx_aux_wr(ctx, segment);
• /* data read */
• ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,

• 		 AP_AUX_ADDR_7_0, 0x50);
  

• if (ret) {

• DRM_ERROR("IO error : aux initial failed.\n");
  

• return ret;
  

• }
• for (cnt = 0; cnt < EDID_TRY_CNT; cnt++) {

• sp_tx_aux_wr(ctx, offset);
  

• /* set I2C read com 0x01 mot = 0 and read 16 bytes */
  

• ret = sp_tx_aux_rd(ctx, 0xf1);
  

• if (ret) {
  

• 	ret = sp_tx_rst_aux(ctx);
  

• 	DRM_DEV_ERROR(dev, "segment read failed, reset!\n");
  

• 	cnt++;
  

Same.

• } else {
  

• 	ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client,
  

• 				     AP_AUX_BUFF_START,
  

• 				     MAX_DPCD_BUFFER_SIZE, buf);
  

• 	if (!ret)
  

• 		break;
  

• }
  

• }
• if (cnt == EDID_TRY_CNT)

• return -EIO;
  

• return 0;

+}

regards, dan carpenter