Dear Chuanhua Han,
In message <20190726032729.14381-2-chuanhua....@nxp.com> you wrote:
>
> /* Set I2c to Slot 1 */
> - i2c_write(0x77, 0, 0, &a, 1);
> +#ifndef CONFIG_DM_I2C
> + ret = i2c_write(0x77, 0, 0, &a, 1);
> +#else
> + ret = i2c_get_chip_for_busnum(0, 0x77, 1, &udev);
> + if (!ret)
> + ret = dm_i2c_write(udev, 0, &a, 1);
> +#endif
See my other comment. You should add error checking for the
i2c_write(), too.
> + 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;
> + }
> +
NAK. You really want to change this ugly code, please.
> /* Set I2c to Slot 1 */
> - i2c_write(0x77, 0, 0, &a, 1);
> +#ifndef CONFIG_DM_I2C
> + ret = i2c_write(0x77, 0, 0, &a, 1);
> +#else
> + ret = i2c_get_chip_for_busnum(0, 0x77, 1, &udev);
> + if (!ret)
> + ret = dm_i2c_write(udev, 0, &a, 1);
> +#endif
Why do you do it here, but not above? You need to work more
consisntently!
> diff --git a/board/freescale/ls2080aqds/ls2080aqds.c
> b/board/freescale/ls2080aqds/ls2080aqds.c
> index a0a3301..ec22cf8 100644
> --- a/board/freescale/ls2080aqds/ls2080aqds.c
> +++ b/board/freescale/ls2080aqds/ls2080aqds.c
> @@ -161,7 +161,15 @@ int select_i2c_ch_pca9547(u8 ch)
> {
> int ret;
>
> +#ifndef CONFIG_DM_I2C
> ret = i2c_write(I2C_MUX_PCA_ADDR_PRI, 0, 1, &ch, 1);
> +#else
> + struct udevice *dev;
No declarations after code, please!
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: w...@denx.de
Single tasking: Just Say No.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
