| Message ID | 20190724025904.469-2-chuanhua.han@nxp.com |
|---|---|
| State | Superseded |
| Delegated to: | Prabhakar Kushwaha |
| Headers | show |
| Series | [U-Boot,1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module | expand |
Dear Chuanhua Han, In message <20190724025904.469-2-chuanhua.han@nxp.com> you wrote: ... > /* Set I2c to Slot 1 */ > +#ifndef CONFIG_DM_I2C > i2c_write(0x77, 0, 0, &a, 1); > +#else > + struct udevice *udev; Please - no declarations in the in the middle of the code (even if #ifdef'ed). Thanks. Best regards, Wolfgang Denk
Dear Chuanhua, In message <20190724025904.469-2-chuanhua.han@nxp.com> you wrote: > This patch is updating ls2088aqds board init code to support DM_I2C. I have a generic question about error handling in your code: > + 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); ... > i2c_write(0x77, 0, 0, &a, 1); ... You use a large number of functions without ever checking their return codes. But all these functions can fail for a number of reasons. Should you not always check for errors? Best regards, Wolfgang Denk
> -----Original Message----- > From: Wolfgang Denk <wd@denx.de> > Sent: 2019年7月24日 14:55 > To: Chuanhua Han <chuanhua.han@nxp.com> > Cc: albert.u.boot@aribaud.net; Prabhakar Kushwaha > <prabhakar.kushwaha@nxp.com>; Priyanka Jain <priyanka.jain@nxp.com>; > Rajesh Bhagat <rajesh.bhagat@nxp.com>; u-boot@lists.denx.de; > lukma@denx.de; trini@konsulko.com > Subject: [EXT] Re: [PATCH 2/4] armv8: ls2088aqds: The ls2088aqds board > supports the I2C driver model. > > Caution: EXT Email > > Dear Chuanhua, > > In message <20190724025904.469-2-chuanhua.han@nxp.com> you wrote: > > This patch is updating ls2088aqds board init code to support DM_I2C. > > I have a generic question about error handling in your code: > > > + 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); > ... > > i2c_write(0x77, 0, 0, &a, 1); > ... > > You use a large number of functions without ever checking their return codes. > But all these functions can fail for a number of reasons. Should you not always > check for errors? yes,should to check for errors! > > 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@denx.de He > had quite a powerful intellect, but it was as powerful like a locomotive, > and ran on rails and was therefore almost impossible to > steer. - Terry Pratchett, _Lords and Ladies_
diff --git a/board/freescale/ls2080aqds/eth.c b/board/freescale/ls2080aqds/eth.c index f706fd4..b1d6827 100644 --- a/board/freescale/ls2080aqds/eth.c +++ b/board/freescale/ls2080aqds/eth.c @@ -107,7 +107,16 @@ static void sgmii_configure_repeater(int serdes_port) int *riser_phy_addr = &xqsgii_riser_phy_addr[0]; /* 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 + for (dpmac = 0; dpmac < 8; dpmac++) { /* Check the PHY status */ @@ -120,7 +129,14 @@ static void sgmii_configure_repeater(int serdes_port) mii_bus = 1; dpmac_id = dpmac + 9; a = 0xb; +#ifndef CONFIG_DM_I2C i2c_write(0x76, 0, 0, &a, 1); +#else + ret = i2c_get_chip_for_busnum(0, 0x76, 1, &udev); + if (!ret) + dm_i2c_write(udev, 0, &a, 1); +#endif + break; } @@ -153,6 +169,7 @@ static void sgmii_configure_repeater(int serdes_port) for (i = 0; i < 4; i++) { for (j = 0; j < 4; j++) { +#ifndef CONFIG_DM_I2C a = 0x18; i2c_write(i2c_addr[dpmac], 6, 1, &a, 1); a = 0x38; @@ -176,6 +193,31 @@ static void sgmii_configure_repeater(int serdes_port) i2c_write(i2c_addr[dpmac], 0x2d, 1, &a, 1); a = 0x20; i2c_write(i2c_addr[dpmac], 4, 1, &a, 1); +#else + i2c_get_chip_for_busnum(0, i2c_addr[dpmac], + 1, &udev); + + 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); + +#endif mdelay(100); ret = miiphy_read(dev[mii_bus], riser_phy_addr[dpmac], @@ -231,7 +273,15 @@ static void qsgmii_configure_repeater(int dpmac) unsigned short value; /* 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 switch (dpmac) { case 1: @@ -282,6 +332,7 @@ static void qsgmii_configure_repeater(int dpmac) for (i = 0; i < 4; i++) { for (j = 0; j < 4; j++) { +#ifndef CONFIG_DM_I2C a = 0x18; i2c_write(i2c_phy_addr, 6, 1, &a, 1); a = 0x38; @@ -301,6 +352,30 @@ static void qsgmii_configure_repeater(int dpmac) i2c_write(i2c_phy_addr, 0x2d, 1, &a, 1); a = 0x20; i2c_write(i2c_phy_addr, 4, 1, &a, 1); +#else + i2c_get_chip_for_busnum(0, i2c_phy_addr, 1, &udev); + 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); + +#endif + mdelay(100); ret = miiphy_read(dev, phy_addr, 0x11, &value); if (ret > 0) diff --git a/board/freescale/ls2080aqds/ls2080aqds.c b/board/freescale/ls2080aqds/ls2080aqds.c index a0a3301..21038a5 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; + + ret = i2c_get_chip_for_busnum(0, I2C_MUX_PCA_ADDR_PRI, 1, &dev); + if (!ret) + ret = dm_i2c_write(dev, 0, &ch, 1); +#endif if (ret) { puts("PCA: failed to select proper channel\n"); return ret; diff --git a/include/configs/ls2080aqds.h b/include/configs/ls2080aqds.h index 18f30b5..05ff770 100644 --- a/include/configs/ls2080aqds.h +++ b/include/configs/ls2080aqds.h @@ -16,7 +16,9 @@ unsigned long get_board_ddr_clk(void); #ifdef CONFIG_FSL_QSPI #define CONFIG_QIXIS_I2C_ACCESS +#ifndef CONFIG_DM_I2C #define CONFIG_SYS_I2C_EARLY_INIT +#endif #define CONFIG_SYS_I2C_IFDR_DIV 0x7e #endif @@ -324,6 +326,7 @@ unsigned long get_board_ddr_clk(void); */ #define RTC #define CONFIG_RTC_DS3231 1 +#define CONFIG_RTC_ENABLE_32KHZ_OUTPUT #define CONFIG_SYS_I2C_RTC_ADDR 0x68 /* EEPROM */
This patch is updating ls2088aqds board init code to support DM_I2C. Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com> --- depends on: - http://patchwork.ozlabs.org/project/uboot/list/?series=118772 - http://patchwork.ozlabs.org/project/uboot/list/?series=117226 board/freescale/ls2080aqds/eth.c | 75 +++++++++++++++++++++++++++++++++ board/freescale/ls2080aqds/ls2080aqds.c | 8 ++++ include/configs/ls2080aqds.h | 3 ++ 3 files changed, 86 insertions(+)