[U-Boot] [PATCH v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module
This patch add an implementation of the rtc_enable_32khz_output() that uses the driver model i2c APIs.
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
Changes in v2: - No change.
drivers/rtc/Kconfig | 10 ++++++++++ drivers/rtc/ds3231.c | 21 +++++++++++++++++++++ include/rtc.h | 6 ++++++ 3 files changed, 37 insertions(+)
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index fd0009b..040d241 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -31,6 +31,12 @@ config TPL_DM_RTC drivers to perform the actual functions. See rtc.h for a description of the API.
+config RTC_ENABLE_32KHZ_OUTPUT + bool "Enable RTC 32Khz output" + help + Some real-time clocks support the output of 32kHz square waves (such as ds3231), + the config symbol choose Real Time Clock device 32Khz output feature. + config RTC_PCF2127 bool "Enable PCF2127 driver" depends on DM_RTC @@ -41,6 +47,10 @@ config RTC_PCF2127 has a selectable I2C-bus or SPI-bus, a backup battery switch-over circuit, a programmable watchdog function, a timestamp function, and many other features.
+config DS3231_BUS_NUM + hex "I2C bus of the DS3231 device" + default 0 + config RTC_DS1307 bool "Enable DS1307 driver" depends on DM_RTC diff --git a/drivers/rtc/ds3231.c b/drivers/rtc/ds3231.c index 79b026a..dbd77a6 100644 --- a/drivers/rtc/ds3231.c +++ b/drivers/rtc/ds3231.c @@ -148,11 +148,13 @@ void rtc_reset (void) /* * Enable 32KHz output */ +#ifdef CONFIG_RTC_ENABLE_32KHZ_OUTPUT void rtc_enable_32khz_output(void) { rtc_write(RTC_STAT_REG_ADDR, RTC_STAT_BIT_BB32KHZ | RTC_STAT_BIT_EN32KHZ); } +#endif
/* * Helper functions @@ -251,6 +253,25 @@ static int ds3231_probe(struct udevice *dev) return 0; }
+#ifdef CONFIG_RTC_ENABLE_32KHZ_OUTPUT +void rtc_enable_32khz_output(void) +{ + int ret; + struct udevice *dev; + +#ifdef CONFIG_DS3231_BUS_NUM + ret = i2c_get_chip_for_busnum(CONFIG_DS3231_BUS_NUM, + CONFIG_SYS_I2C_RTC_ADDR, 1, &dev); +#else + ret = i2c_get_chip_for_busnum(0, CONFIG_SYS_I2C_RTC_ADDR, 1, &dev); +#endif + if (!ret) + dm_i2c_reg_write(dev, RTC_STAT_REG_ADDR, + RTC_STAT_BIT_BB32KHZ | + RTC_STAT_BIT_EN32KHZ); +} +#endif + static const struct rtc_ops ds3231_rtc_ops = { .get = ds3231_rtc_get, .set = ds3231_rtc_set, diff --git a/include/rtc.h b/include/rtc.h index b255bdc..df7de09 100644 --- a/include/rtc.h +++ b/include/rtc.h @@ -166,11 +166,17 @@ int rtc_read32(struct udevice *dev, unsigned int reg, u32 *valuep); */ int rtc_write32(struct udevice *dev, unsigned int reg, u32 value);
+#ifdef CONFIG_RTC_ENABLE_32KHZ_OUTPUT +void rtc_enable_32khz_output(void); +#endif + #else int rtc_get (struct rtc_time *); int rtc_set (struct rtc_time *); void rtc_reset (void); +#ifdef CONFIG_RTC_ENABLE_32KHZ_OUTPUT void rtc_enable_32khz_output(void); +#endif
/** * rtc_read8() - Read an 8-bit register
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
Changes in v2: - Move the variable declaration in the middle of the code to the beginning of the function. - Check the return value for the API of the i2c function call.
board/freescale/ls2080aqds/eth.c | 236 ++++++++++++++++++++++++++++---- board/freescale/ls2080aqds/ls2080aqds.c | 8 ++ include/configs/ls2080aqds.h | 3 + 3 files changed, 220 insertions(+), 27 deletions(-)
diff --git a/board/freescale/ls2080aqds/eth.c b/board/freescale/ls2080aqds/eth.c index f706fd4..710f075 100644 --- a/board/freescale/ls2080aqds/eth.c +++ b/board/freescale/ls2080aqds/eth.c @@ -105,9 +105,20 @@ static void sgmii_configure_repeater(int serdes_port) uint8_t ch_b_ctl2[] = {0x81, 0x82, 0x83, 0x84};
int *riser_phy_addr = &xqsgii_riser_phy_addr[0]; +#ifdef CONFIG_DM_I2C + struct udevice *udev; +#endif
/* 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 + if (ret) + goto error;
for (dpmac = 0; dpmac < 8; dpmac++) { /* Check the PHY status */ @@ -120,7 +131,15 @@ static void sgmii_configure_repeater(int serdes_port) mii_bus = 1; dpmac_id = dpmac + 9; a = 0xb; - i2c_write(0x76, 0, 0, &a, 1); +#ifndef CONFIG_DM_I2C + ret = i2c_write(0x76, 0, 0, &a, 1); +#else + ret = i2c_get_chip_for_busnum(0, 0x76, 1, &udev); + if (!ret) + ret = dm_i2c_write(udev, 0, &a, 1); +#endif + if (ret) + goto error; break; }
@@ -153,29 +172,102 @@ 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); + 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; + } + +#endif mdelay(100); ret = miiphy_read(dev[mii_bus], riser_phy_addr[dpmac], @@ -229,9 +321,20 @@ static void qsgmii_configure_repeater(int dpmac) const char *dev = "LS2080A_QDS_MDIO0"; int ret = 0; unsigned short value; +#ifdef CONFIG_DM_I2C + struct udevice *udev; +#endif
/* 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 + if (ret) + goto error;
switch (dpmac) { case 1: @@ -282,25 +385,104 @@ 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); + ret = i2c_write(i2c_phy_addr, 6, 1, &a, 1); + if (ret) + goto error; a = 0x38; - i2c_write(i2c_phy_addr, 4, 1, &a, 1); + ret = i2c_write(i2c_phy_addr, 4, 1, &a, 1); + if (ret) + goto error; a = 0x4; - i2c_write(i2c_phy_addr, 8, 1, &a, 1); + ret = i2c_write(i2c_phy_addr, 8, 1, &a, 1); + if (ret) + goto error;
- i2c_write(i2c_phy_addr, 0xf, 1, &ch_a_eq[i], 1); - i2c_write(i2c_phy_addr, 0x11, 1, &ch_a_ctl2[j], 1); + ret = i2c_write(i2c_phy_addr, 0xf, 1, &ch_a_eq[i], 1); + if (ret) + goto error; + ret = i2c_write(i2c_phy_addr, 0x11, 1, + &ch_a_ctl2[j], 1); + if (ret) + goto error;
- i2c_write(i2c_phy_addr, 0x16, 1, &ch_b_eq[i], 1); - i2c_write(i2c_phy_addr, 0x18, 1, &ch_b_ctl2[j], 1); + ret = i2c_write(i2c_phy_addr, 0x16, 1, &ch_b_eq[i], 1); + if (ret) + goto error; + ret = i2c_write(i2c_phy_addr, 0x18, 1, + &ch_b_ctl2[j], 1); + if (ret) + goto error;
a = 0x14; - i2c_write(i2c_phy_addr, 0x23, 1, &a, 1); + ret = i2c_write(i2c_phy_addr, 0x23, 1, &a, 1); + if (ret) + goto error; a = 0xb5; - i2c_write(i2c_phy_addr, 0x2d, 1, &a, 1); + ret = i2c_write(i2c_phy_addr, 0x2d, 1, &a, 1); + if (ret) + goto error; a = 0x20; - i2c_write(i2c_phy_addr, 4, 1, &a, 1); + ret = i2c_write(i2c_phy_addr, 4, 1, &a, 1); + if (ret) + goto error; +#else + ret = i2c_get_chip_for_busnum(0, + i2c_phy_addr, + 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; + } + +#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 */
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
-----Original Message----- From: Wolfgang Denk wd@denx.de Sent: 2019年7月25日 15:40 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 v2 2/4] armv8: ls2088aqds: The ls2088aqds board supports the I2C driver model.
Caution: EXT Email
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; #elseret = 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...
It seems feasible to use the array you mentioned, but some of the values to be set by i2c are obtained by index I /j in other arrays. So we keep setting the i2c register in the same way as before.
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 Any sufficiently advanced technology is indistinguishable from a rigged demo. - Andy Finkel, computer guy
Dear Chuanhua Han,
In message AM6PR04MB43572355560DF1349D6BC16097C10@AM6PR04MB4357.eurprd04.prod.outlook.com you wrote:
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...
It seems feasible to use the array you mentioned, but some of the values to be set by i2c are obtained by index I /j in other arrays.
Yes, so what? You can write these values into your data array, too, right?
So we keep setting the i2c register in the same way as before.
Please don't. This code is ugly.
Best regards,
Wolfgang Denk
Enable related configs on all ls2088aqds boards to support ds3231 rtc DM function.
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
Changes in v2: - No change.
configs/ls2088aqds_tfa_defconfig | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/configs/ls2088aqds_tfa_defconfig b/configs/ls2088aqds_tfa_defconfig index e798c59..95921b0 100644 --- a/configs/ls2088aqds_tfa_defconfig +++ b/configs/ls2088aqds_tfa_defconfig @@ -1,12 +1,12 @@ CONFIG_ARM=y CONFIG_TARGET_LS2080AQDS=y +CONFIG_SYS_MALLOC_F_LEN=0x6000 CONFIG_SYS_TEXT_BASE=0x82000000 CONFIG_TFABOOT=y CONFIG_NR_DRAM_BANKS=3 CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT=y CONFIG_SEC_FIRMWARE_ARMV8_PSCI=y CONFIG_AHCI=y -# CONFIG_SYS_MALLOC_F is not set CONFIG_FIT_VERBOSE=y CONFIG_OF_BOARD_SETUP=y CONFIG_OF_STDOUT_VIA_ALIAS=y @@ -63,3 +63,11 @@ CONFIG_DM_USB=y CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_DWC3=y CONFIG_EFI_LOADER_BOUNCE_BUFFER=y +CONFIG_DM_I2C=y +CONFIG_I2C_SET_DEFAULT_BUS_NUM=y +CONFIG_I2C_DEFAULT_BUS_NUMBER=0 +CONFIG_DM_GPIO=y +CONFIG_I2C_MUX=y +CONFIG_I2C_MUX_PCA954x=y +CONFIG_DM_RTC=y +CONFIG_DS3231_BUS_NUM=0
This patch adds the ds3232-rtc node under the i2c0->i2c-mux@77->i2c@0 for ls2088aqds boards.
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
Changes in v2: - No change.
arch/arm/dts/fsl-ls2080a-qds.dts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/arch/arm/dts/fsl-ls2080a-qds.dts b/arch/arm/dts/fsl-ls2080a-qds.dts index 2a0a528..13461b5 100644 --- a/arch/arm/dts/fsl-ls2080a-qds.dts +++ b/arch/arm/dts/fsl-ls2080a-qds.dts @@ -19,6 +19,25 @@ }; };
+&i2c0 { + status = "okay"; + pca9547@77 { + compatible = "nxp,pca9547"; + reg = <0x77>; + #address-cells = <1>; + #size-cells = <0>; + i2c@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x00>; + rtc@68 { + compatible = "dallas,ds3232"; + reg = <0x68>; + }; + }; + }; +}; + &dspi { bus-num = <0>; status = "okay";
Dear Chuanhua Han,
In message 20190725043934.30236-4-chuanhua.han@nxp.com you wrote:
This patch adds the ds3232-rtc node under the i2c0->i2c-mux@77->i2c@0 for ls2088aqds boards.
Is this bisectable? You first enable the feature in the code, and only later add the needed property to the DT? Should it not be reversed?
Best regards,
Wolfgang Denk
-----Original Message----- From: Wolfgang Denk wd@denx.de Sent: 2019年7月25日 15:41 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 v2 4/4] armv8: ls2088aqds : Add ds3232 node
Caution: EXT Email
Dear Chuanhua Han,
In message 20190725043934.30236-4-chuanhua.han@nxp.com you wrote:
This patch adds the ds3232-rtc node under the i2c0->i2c-mux@77->i2c@0 for ls2088aqds boards.
Is this bisectable? You first enable the feature in the code, and only later add the needed property to the DT? Should it not be reversed?
This should not matter because they are in the same patch set
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 Why don't you have a Linux partition installed so you can be working in a programmer-friendly environment instead of a keep-gates'-bank- account-happy one? :-) -- Tom Christiansen
Dear Chuanhua Han,
In message AM6PR04MB43577AAE78F9BC0B12E5D07897C10@AM6PR04MB4357.eurprd04.prod.outlook.com you wrote:
Is this bisectable? You first enable the feature in the code, and only later add the needed property to the DT? Should it not be reversed?
This should not matter because they are in the same patch set
You completely miss the point what bisecting means!!
Yes, of course it _does_ matter, as bisecting may apply only parts of your patch series, and omit the rest. If it selects the commit before the last, the code will break.
Please fix this!
Best regards,
Wolfgang Denk
-----Original Message----- From: Wolfgang Denk wd@denx.de Sent: 2019年7月26日 14:26 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: Re: [EXT] Re: [PATCH v2 4/4] armv8: ls2088aqds : Add ds3232 node
Caution: EXT Email
Dear Chuanhua Han,
In message <AM6PR04MB43577AAE78F9BC0B12E5D07897C10@AM6PR04MB4357.eurpr d04.prod.outlook.com> you wrote:
Is this bisectable? You first enable the feature in the code, and only later add the needed property to the DT? Should it not be reversed?
This should not matter because they are in the same patch set
You completely miss the point what bisecting means!!
Yes, of course it _does_ matter, as bisecting may apply only parts of your patch series, and omit the rest. If it selects the commit before the last, the code will break.
Please fix this!
The latest version has been revised
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 If there was anything that depressed him more than his own cynicism, it was that quite often it still wasn't as cynical as real life. - Terry Pratchett, _Guards! Guards!_
Dear Chuanhua Han,
In message 20190725043934.30236-1-chuanhua.han@nxp.com you wrote:
This patch add an implementation of the rtc_enable_32khz_output() that uses the driver model i2c APIs.
...
+config DS3231_BUS_NUM
- hex "I2C bus of the DS3231 device"
- default 0
I know this is likely just an academic question - buit what happens if we have two such devices on different busses?
Best regards,
Wolfgang Denk
-----Original Message----- From: Wolfgang Denk wd@denx.de Sent: 2019年7月25日 15:34 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 v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module
Caution: EXT Email
Dear Chuanhua Han,
In message 20190725043934.30236-1-chuanhua.han@nxp.com you wrote:
This patch add an implementation of the rtc_enable_32khz_output() that uses the driver model i2c APIs.
...
+config DS3231_BUS_NUM
hex "I2C bus of the DS3231 device"default 0I know this is likely just an academic question - buit what happens if we have two such devices on different busses?
Ah, that really needs to be considered! Then the method I came up with is: take the rtc_enable_32khz_output function with the udeice parameter to uniquely determine the specific RTC device, and then set the register. I wonder if this method is feasible?
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 "Beware of bugs in the above code; I have only proved it correct, not tried it." - Donald Knuth
participants (2)
-
Chuanhua Han -
Wolfgang Denk