All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module
@ 2019-07-25  4:39 Chuanhua Han
  2019-07-25  4:39 ` [U-Boot] [PATCH v2 2/4] armv8: ls2088aqds: The ls2088aqds board supports the I2C driver model Chuanhua Han
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Chuanhua Han @ 2019-07-25  4:39 UTC (permalink / raw)
  To: u-boot

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
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 2/4] armv8: ls2088aqds: The ls2088aqds board supports the I2C driver model.
  2019-07-25  4:39 [U-Boot] [PATCH v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module Chuanhua Han
@ 2019-07-25  4:39 ` Chuanhua Han
  2019-07-25  7:39   ` Wolfgang Denk
  2019-07-25  4:39 ` [U-Boot] [PATCH v2 3/4] configs: ls2088aqds: Enable DM support for ds3231 rtc Chuanhua Han
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Chuanhua Han @ 2019-07-25  4:39 UTC (permalink / raw)
  To: u-boot

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 */
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 3/4] configs: ls2088aqds: Enable DM support for ds3231 rtc
  2019-07-25  4:39 [U-Boot] [PATCH v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module Chuanhua Han
  2019-07-25  4:39 ` [U-Boot] [PATCH v2 2/4] armv8: ls2088aqds: The ls2088aqds board supports the I2C driver model Chuanhua Han
@ 2019-07-25  4:39 ` Chuanhua Han
  2019-07-25  4:39 ` [U-Boot] [PATCH v2 4/4] armv8: ls2088aqds : Add ds3232 node Chuanhua Han
  2019-07-25  7:34 ` [U-Boot] [PATCH v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module Wolfgang Denk
  3 siblings, 0 replies; 13+ messages in thread
From: Chuanhua Han @ 2019-07-25  4:39 UTC (permalink / raw)
  To: u-boot

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
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 4/4] armv8: ls2088aqds : Add ds3232 node
  2019-07-25  4:39 [U-Boot] [PATCH v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module Chuanhua Han
  2019-07-25  4:39 ` [U-Boot] [PATCH v2 2/4] armv8: ls2088aqds: The ls2088aqds board supports the I2C driver model Chuanhua Han
  2019-07-25  4:39 ` [U-Boot] [PATCH v2 3/4] configs: ls2088aqds: Enable DM support for ds3231 rtc Chuanhua Han
@ 2019-07-25  4:39 ` Chuanhua Han
  2019-07-25  7:41   ` Wolfgang Denk
  2019-07-25  7:34 ` [U-Boot] [PATCH v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module Wolfgang Denk
  3 siblings, 1 reply; 13+ messages in thread
From: Chuanhua Han @ 2019-07-25  4:39 UTC (permalink / raw)
  To: u-boot

This patch adds the ds3232-rtc node under the i2c0->i2c-mux at 77->i2c at 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 at 77 {
+		compatible = "nxp,pca9547";
+		reg = <0x77>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		i2c at 0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x00>;
+			rtc at 68 {
+				compatible = "dallas,ds3232";
+				reg = <0x68>;
+			};
+		};
+	};
+};
+
 &dspi {
 	bus-num = <0>;
 	status = "okay";
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module
  2019-07-25  4:39 [U-Boot] [PATCH v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module Chuanhua Han
                   ` (2 preceding siblings ...)
  2019-07-25  4:39 ` [U-Boot] [PATCH v2 4/4] armv8: ls2088aqds : Add ds3232 node Chuanhua Han
@ 2019-07-25  7:34 ` Wolfgang Denk
  2019-07-25  9:33   ` [U-Boot] [EXT] " Chuanhua Han
  3 siblings, 1 reply; 13+ messages in thread
From: Wolfgang Denk @ 2019-07-25  7:34 UTC (permalink / raw)
  To: u-boot

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

-- 
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
"Beware of bugs in the above code; I have only proved it correct, not
tried it."                                             - Donald Knuth

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 2/4] armv8: ls2088aqds: The ls2088aqds board supports the I2C driver model.
  2019-07-25  4:39 ` [U-Boot] [PATCH v2 2/4] armv8: ls2088aqds: The ls2088aqds board supports the I2C driver model Chuanhua Han
@ 2019-07-25  7:39   ` Wolfgang Denk
  2019-07-25 11:36     ` [U-Boot] [EXT] " Chuanhua Han
  0 siblings, 1 reply; 13+ messages in thread
From: Wolfgang Denk @ 2019-07-25  7:39 UTC (permalink / raw)
  To: u-boot

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

-- 
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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2 4/4] armv8: ls2088aqds : Add ds3232 node
  2019-07-25  4:39 ` [U-Boot] [PATCH v2 4/4] armv8: ls2088aqds : Add ds3232 node Chuanhua Han
@ 2019-07-25  7:41   ` Wolfgang Denk
  2019-07-25  9:23     ` [U-Boot] [EXT] " Chuanhua Han
  0 siblings, 1 reply; 13+ messages in thread
From: Wolfgang Denk @ 2019-07-25  7:41 UTC (permalink / raw)
  To: u-boot

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 at 77->i2c at 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

-- 
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
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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [EXT] Re: [PATCH v2 4/4] armv8: ls2088aqds : Add ds3232 node
  2019-07-25  7:41   ` Wolfgang Denk
@ 2019-07-25  9:23     ` Chuanhua Han
  2019-07-26  6:26       ` Wolfgang Denk
  0 siblings, 1 reply; 13+ messages in thread
From: Chuanhua Han @ 2019-07-25  9:23 UTC (permalink / raw)
  To: u-boot



> -----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 at aribaud.net; Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com>; Priyanka Jain <priyanka.jain@nxp.com>;
> Rajesh Bhagat <rajesh.bhagat@nxp.com>; u-boot at lists.denx.de;
> lukma at denx.de; trini at 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 at 77->i2c at 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 at 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [EXT] Re: [PATCH v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module
  2019-07-25  7:34 ` [U-Boot] [PATCH v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module Wolfgang Denk
@ 2019-07-25  9:33   ` Chuanhua Han
  0 siblings, 0 replies; 13+ messages in thread
From: Chuanhua Han @ 2019-07-25  9:33 UTC (permalink / raw)
  To: u-boot



> -----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 at aribaud.net; Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com>; Priyanka Jain <priyanka.jain@nxp.com>;
> Rajesh Bhagat <rajesh.bhagat@nxp.com>; u-boot at lists.denx.de;
> lukma at denx.de; trini at 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 0
> > +
> 
> I 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 at denx.de
> "Beware of bugs in the above code; I have only proved it correct, not
> tried it."                                             - Donald Knuth

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [EXT] Re: [PATCH v2 2/4] armv8: ls2088aqds: The ls2088aqds board supports the I2C driver model.
  2019-07-25  7:39   ` Wolfgang Denk
@ 2019-07-25 11:36     ` Chuanhua Han
  2019-07-26  6:28       ` Wolfgang Denk
  0 siblings, 1 reply; 13+ messages in thread
From: Chuanhua Han @ 2019-07-25 11:36 UTC (permalink / raw)
  To: u-boot



> -----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 at aribaud.net; Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com>; Priyanka Jain <priyanka.jain@nxp.com>;
> Rajesh Bhagat <rajesh.bhagat@nxp.com>; u-boot at lists.denx.de;
> lukma at denx.de; trini at 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; #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...
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 at denx.de Any
> sufficiently advanced  technology  is  indistinguishable  from  a
> rigged demo.                              - Andy Finkel, computer guy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [EXT] Re: [PATCH v2 4/4] armv8: ls2088aqds : Add ds3232 node
  2019-07-25  9:23     ` [U-Boot] [EXT] " Chuanhua Han
@ 2019-07-26  6:26       ` Wolfgang Denk
  2019-07-26  6:28         ` Chuanhua Han
  0 siblings, 1 reply; 13+ messages in thread
From: Wolfgang Denk @ 2019-07-26  6:26 UTC (permalink / raw)
  To: u-boot

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

-- 
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
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!_

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [EXT] Re: [PATCH v2 4/4] armv8: ls2088aqds : Add ds3232 node
  2019-07-26  6:26       ` Wolfgang Denk
@ 2019-07-26  6:28         ` Chuanhua Han
  0 siblings, 0 replies; 13+ messages in thread
From: Chuanhua Han @ 2019-07-26  6:28 UTC (permalink / raw)
  To: u-boot



> -----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 at aribaud.net; Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com>; Priyanka Jain <priyanka.jain@nxp.com>;
> Rajesh Bhagat <rajesh.bhagat@nxp.com>; u-boot at lists.denx.de;
> lukma at denx.de; trini at 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 at 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!_

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [EXT] Re: [PATCH v2 2/4] armv8: ls2088aqds: The ls2088aqds board supports the I2C driver model.
  2019-07-25 11:36     ` [U-Boot] [EXT] " Chuanhua Han
@ 2019-07-26  6:28       ` Wolfgang Denk
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfgang Denk @ 2019-07-26  6:28 UTC (permalink / raw)
  To: u-boot

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

-- 
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
Physician: One upon whom we set our hopes when ill and our dogs  when
well.                                                - Ambrose Bierce

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-07-26  6:28 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-25  4:39 [U-Boot] [PATCH v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module Chuanhua Han
2019-07-25  4:39 ` [U-Boot] [PATCH v2 2/4] armv8: ls2088aqds: The ls2088aqds board supports the I2C driver model Chuanhua Han
2019-07-25  7:39   ` Wolfgang Denk
2019-07-25 11:36     ` [U-Boot] [EXT] " Chuanhua Han
2019-07-26  6:28       ` Wolfgang Denk
2019-07-25  4:39 ` [U-Boot] [PATCH v2 3/4] configs: ls2088aqds: Enable DM support for ds3231 rtc Chuanhua Han
2019-07-25  4:39 ` [U-Boot] [PATCH v2 4/4] armv8: ls2088aqds : Add ds3232 node Chuanhua Han
2019-07-25  7:41   ` Wolfgang Denk
2019-07-25  9:23     ` [U-Boot] [EXT] " Chuanhua Han
2019-07-26  6:26       ` Wolfgang Denk
2019-07-26  6:28         ` Chuanhua Han
2019-07-25  7:34 ` [U-Boot] [PATCH v2 1/4] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module Wolfgang Denk
2019-07-25  9:33   ` [U-Boot] [EXT] " Chuanhua Han

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.