Dear Chuanhua Han,
In message <20190725084400.4035-1-chuanhua....@nxp.com> you wrote:
> This patch is updating ls1088aqds board init code to support DM_I2C.
>
> Signed-off-by: Chuanhua Han <chuanhua....@nxp.com>
> ---
> depends on:
> - http://patchwork.ozlabs.org/project/uboot/list/?series=110856
> - http://patchwork.ozlabs.org/project/uboot/list/?series=109459
> - http://patchwork.ozlabs.org/project/uboot/list/?series=120936
Is this patch version 3? Or soemthign new? You should explan what
you changed...
> board/freescale/ls1088a/eth_ls1088aqds.c | 74
> ++++++++++++++++++++++++++++++++
> include/configs/ls1088aqds.h | 4 +-
> 2 files changed, 76 insertions(+), 2 deletions(-)
>
> diff --git a/board/freescale/ls1088a/eth_ls1088aqds.c
> b/board/freescale/ls1088a/eth_ls1088aqds.c
> index f16b78c..3b83d5b 100644
> --- a/board/freescale/ls1088a/eth_ls1088aqds.c
> +++ b/board/freescale/ls1088a/eth_ls1088aqds.c
> @@ -97,7 +97,16 @@ static void sgmii_configure_repeater(int dpmac)
> uint8_t ch_b_ctl2[] = {0x81, 0x82, 0x83, 0x84};
>
> /* Set I2c to Slot 1 */
> +#ifndef CONFIG_DM_I2C
> i2c_write(0x77, 0, 0, &a, 1);
> +#else
> + struct udevice *udev;
> +
> + ret = i2c_get_chip_for_busnum(0, 0x77, 1, &udev);
> + if (!ret)
> + dm_i2c_write(udev, 0, &a, 1);
> +#endif
I asked you before to get rid of the declaration (struct udevice) in
the middle of code (after the i2c_write()).
I also explained that you shoudl check error codes. When you change
this here, you could as well do it for i2c_write(), like this:
#ifndef CONFIG_DM_I2C
ret = i2c_write(0x77, 0, 0, &a, 1);
#else
ret = i2c_get_chip_for_busnum(0, 0x77, 1, &udev);
#endif
if (!ret)
...
> + a = 0x18;
> + dm_i2c_write(udev, 6, &a, 1);
> + a = 0x38;
> + dm_i2c_write(udev, 4, &a, 1);
> + a = 0x4;
> + dm_i2c_write(udev, 8, &a, 1);
> +
> + dm_i2c_write(udev, 0xf, &ch_a_eq[i], 1);
> + dm_i2c_write(udev, 0x11, &ch_a_ctl2[j], 1);
> +
> + dm_i2c_write(udev, 0x16, &ch_b_eq[i], 1);
> + dm_i2c_write(udev, 0x18, &ch_b_ctl2[j], 1);
> +
> + a = 0x14;
> + dm_i2c_write(udev, 0x23, &a, 1);
> + a = 0xb5;
> + dm_i2c_write(udev, 0x2d, &a, 1);
> + a = 0x20;
> + dm_i2c_write(udev, 4, &a, 1);
> +
Error checking missing. And you really want to change this into a
loop iterating over a set of data instead of repeting the same code
again and again.
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
Contrary to popular belief, thinking does not cause brain damage.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
