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

[Wolfgang Denk]

Dear Chuanhua Han,

In message <20190725043934.30236-2-chuanhua.han at 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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Any sufficiently advanced  technology  is  indistinguishable  from  a
rigged demo.                              - Andy Finkel, computer guy