Re: [U-Boot] [PATCH v2 2/4] armv8: ls2088aqds: The ls2088aqds board supports the I2C driver model.

Dear Chuanhua Han,

In message 20190725043934.30236-2-chuanhua.han@nxp.com you wrote:

This patch is updating ls2088aqds board init code to support DM_I2C.

		a = 0x18;

• 		i2c_write(i2c_addr[dpmac], 6, 1, &a, 1);
  

• 		ret = i2c_write(i2c_addr[dpmac], 6, 1, &a, 1);
  

• 		if (ret)
  

• 			goto error;
  	a = 0x38;
  

• 		i2c_write(i2c_addr[dpmac], 4, 1, &a, 1);
  

• 		ret = i2c_write(i2c_addr[dpmac], 4, 1, &a, 1);
  

• 		if (ret)
  

• 			goto error;
  	a = 0x4;
  

• 		i2c_write(i2c_addr[dpmac], 8, 1, &a, 1);
  

• 		ret = i2c_write(i2c_addr[dpmac], 8, 1, &a, 1);
  

• 		if (ret)
  

• 			goto error;
  
  

• 		i2c_write(i2c_addr[dpmac], 0xf, 1,
  

• 			  &ch_a_eq[i], 1);
  

• 		i2c_write(i2c_addr[dpmac], 0x11, 1,
  

• 			  &ch_a_ctl2[j], 1);
  

• 		ret = i2c_write(i2c_addr[dpmac], 0xf,
  

• 				1, &ch_a_eq[i], 1);
  

• 		if (ret)
  

• 			goto error;
  

• 		ret = i2c_write(i2c_addr[dpmac], 0x11,
  

• 				1, &ch_a_ctl2[j], 1);
  

• 		if (ret)
  

• 			goto error;
  
  

• 		i2c_write(i2c_addr[dpmac], 0x16, 1,
  

• 			  &ch_b_eq[i], 1);
  

• 		i2c_write(i2c_addr[dpmac], 0x18, 1,
  

• 			  &ch_b_ctl2[j], 1);
  

• 		ret = i2c_write(i2c_addr[dpmac], 0x16,
  

• 				1, &ch_b_eq[i], 1);
  

• 		if (ret)
  

• 			goto error;
  

• 		ret = i2c_write(i2c_addr[dpmac], 0x18,
  

• 				1, &ch_b_ctl2[j], 1);
  

• 		if (ret)
  

• 			goto error;
  
  	a = 0x14;
  

• 		i2c_write(i2c_addr[dpmac], 0x23, 1, &a, 1);
  

• 		ret = i2c_write(i2c_addr[dpmac],
  

• 				0x23, 1, &a, 1);
  

• 		if (ret)
  

• 			goto error;
  	a = 0xb5;
  

• 		i2c_write(i2c_addr[dpmac], 0x2d, 1, &a, 1);
  

• 		ret = i2c_write(i2c_addr[dpmac],
  

• 				0x2d, 1, &a, 1);
  

• 		if (ret)
  

• 			goto error;
  	a = 0x20;
  

• 		i2c_write(i2c_addr[dpmac], 4, 1, &a, 1);
  

• 		ret = i2c_write(i2c_addr[dpmac], 4, 1, &a, 1);
  

• 		if (ret)
  

• 			goto error;
  

+#else

• 		ret = i2c_get_chip_for_busnum(0,
  

• 					      i2c_addr[dpmac],
  

• 					      1, &udev);
  

• 		if (!ret) {
  

• 			a = 0x18;
  

• 			ret = dm_i2c_write(udev, 6, &a, 1);
  

• 			if (ret)
  

• 				goto error;
  

• 			a = 0x38;
  

• 			ret = dm_i2c_write(udev, 4, &a, 1);
  

• 			if (ret)
  

• 				goto error;
  

• 			a = 0x4;
  

• 			ret = dm_i2c_write(udev, 8, &a, 1);
  

• 			if (ret)
  

• 				goto error;
  

• 			ret = dm_i2c_write(udev, 0xf,
  

• 					   &ch_a_eq[i], 1);
  

• 			if (ret)
  

• 				goto error;
  

• 			ret = dm_i2c_write(udev, 0x11,
  

• 					   &ch_a_ctl2[j], 1);
  

• 			if (ret)
  

• 				goto error;
  

• 			ret = dm_i2c_write(udev, 0x16,
  

• 					   &ch_b_eq[i], 1);
  

• 			if (ret)
  

• 				goto error;
  

• 			ret = dm_i2c_write(udev, 0x18,
  

• 					   &ch_b_ctl2[j], 1);
  

• 			if (ret)
  

• 				goto error;
  

• 			a = 0x14;
  

• 			ret = dm_i2c_write(udev, 0x23, &a, 1);
  

• 			if (ret)
  

• 				goto error;
  

• 			a = 0xb5;
  

• 			ret = dm_i2c_write(udev, 0x2d, &a, 1);
  

• 			if (ret)
  

• 				goto error;
  

• 			a = 0x20;
  

• 			ret = dm_i2c_write(udev, 4, &a, 1);
  

• 			if (ret)
  

• 				goto error;
  

This is a really long list where you repeat the very same code again and again and again. Would it not make sense to declare a data array (holding pairs of <offset>, <data> entries), and then iterrate over the loop? The could would be much easier to read and also much smaller...

Best regards,

Wolfgang Denk