[U-Boot] [PATCH 0/4] arm: zynq: migrate to DM I2C driver

[U-Boot] [PATCH 0/4] arm: zynq: migrate to DM I2C driver

[Luis Araneda]

This series migrate most zynq boards to using the
Cadence DM I2C driver

The first patch adds kconfig options to generalize the concept
of reading the MAC address from an I2C EEPROM. Then the zynq and
zynqmp boards were migrated with patches two and three to use
the options

Finally, the last patch migrates the boards to the Cadence I2C driver.
The boards ZC702 and ZC706 were not migrated because they have
I2C multiplexers and I don't have a board with a multiplexer to
perform testing and validation

I ran a round of reproducible builds on master and another one
after the third patch for all zynq and zynqmp boards, and the same
binaries where generated for all boards

All commits were tested on a Digilent Zybo board

Luis Araneda (4):
  drivers/misc: add options to read MAC address from EEPROM
  arm: zynq: migrate EEPROM MAC address options to kconfig
  arm64: zynqmp: migrate EEPROM MAC address options to kconfig
  arm: zynq: use i2c cadence DM driver

 arch/arm/dts/zynq-syzygy-hub.dts              |  1 +
 arch/arm/dts/zynq-zybo.dts                    | 10 ++++++
 board/xilinx/zynq/board.c                     | 27 ++++++++++++----
 board/xilinx/zynqmp/zynqmp.c                  | 10 +++---
 configs/syzygy_hub_defconfig                  |  9 +++---
 configs/topic_miami_defconfig                 |  5 ++-
 configs/topic_miamilite_defconfig             |  5 ++-
 configs/topic_miamiplus_defconfig             |  5 ++-
 configs/xilinx_zynqmp_zcu102_rev1_0_defconfig |  5 ++-
 configs/xilinx_zynqmp_zcu102_revA_defconfig   |  5 ++-
 configs/xilinx_zynqmp_zcu102_revB_defconfig   |  5 ++-
 configs/xilinx_zynqmp_zcu106_revA_defconfig   |  5 ++-
 configs/xilinx_zynqmp_zcu111_revA_defconfig   |  5 ++-
 configs/zynq_zybo_defconfig                   | 10 +++---
 drivers/misc/Kconfig                          | 32 +++++++++++++++----
 include/configs/syzygy_hub.h                  |  3 --
 include/configs/xilinx_zynqmp_zcu102.h        |  2 --
 include/configs/xilinx_zynqmp_zcu106.h        |  2 --
 include/configs/xilinx_zynqmp_zcu111.h        |  2 --
 include/configs/zynq_zybo.h                   |  2 --
 20 files changed, 98 insertions(+), 52 deletions(-)

-- 
2.18.0

[U-Boot] [PATCH 1/4] drivers/misc: add options to read MAC address from EEPROM

[Luis Araneda]

Add generic kconfig options to read MAC address(es) from an EEPROM
connected to an I2C bus

The SYS_I2C_EEPROM_* options can't be used because they depend
on I2C_EEPROM, which requires a DM driver, and not all boards
have DM I2C drivers yet

Signed-off-by: Luis Araneda <luaraneda@gmail.com>
---
 drivers/misc/Kconfig | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 17b3a805a2..ff694871bf 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -272,6 +272,32 @@ config SYS_I2C_EEPROM_ADDR_OVERFLOW
 
 endif
 
+config MAC_ADDR_IN_I2C_EEPROM
+	bool "MAC address in I2C EEPROM"
+	help
+	  Read MAC address(es) from an EEPROM
+	  connected to an I2C bus
+
+if MAC_ADDR_IN_I2C_EEPROM
+
+config MAC_ADDR_I2C_EEPROM_BUS
+	int "I2C bus connected to the EEPROM device"
+	default 0
+
+config MAC_ADDR_I2C_EEPROM_CHIP_ADDR
+	hex "Chip address of the EEPROM device"
+	default 0
+
+config MAC_ADDR_I2C_EEPROM_DATA_ADDR_LEN
+	int "Length in bytes of EEPROM data addresses"
+	default 1
+
+config MAC_ADDR_I2C_EEPROM_DATA_ADDR_START
+	hex "Start address of MAC data in EEPROM"
+	default 0
+
+endif
+
 config GDSYS_RXAUI_CTRL
 	bool "Enable gdsys RXAUI control driver"
 	depends on MISC
-- 
2.18.0

[U-Boot] [PATCH 2/4] arm: zynq: migrate EEPROM MAC address options to kconfig

[Luis Araneda]

Migrate boards to use the generic kconfig options to
read the MAC address

Signed-off-by: Luis Araneda <luaraneda@gmail.com>
---
 board/xilinx/zynq/board.c    | 7 +++----
 configs/syzygy_hub_defconfig | 4 +++-
 configs/zynq_zybo_defconfig  | 4 +++-
 include/configs/syzygy_hub.h | 1 -
 include/configs/zynq_zybo.h  | 1 -
 5 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
index 1106f5c2a8..dcbf788918 100644
--- a/board/xilinx/zynq/board.c
+++ b/board/xilinx/zynq/board.c
@@ -75,10 +75,9 @@ int board_late_init(void)
 
 int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
 {
-#if defined(CONFIG_ZYNQ_GEM_EEPROM_ADDR) && \
-    defined(CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET)
-	if (eeprom_read(CONFIG_ZYNQ_GEM_EEPROM_ADDR,
-			CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET,
+#if defined(CONFIG_MAC_ADDR_IN_I2C_EEPROM)
+	if (eeprom_read(CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR,
+			CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START,
 			ethaddr, 6))
 		printf("I2C EEPROM MAC address read failed\n");
 #endif
diff --git a/configs/syzygy_hub_defconfig b/configs/syzygy_hub_defconfig
index 6fb56afbe9..221349ba6f 100644
--- a/configs/syzygy_hub_defconfig
+++ b/configs/syzygy_hub_defconfig
@@ -39,7 +39,9 @@ CONFIG_FPGA_ZYNQPL=y
 CONFIG_DM_GPIO=y
 CONFIG_SYS_I2C_ZYNQ=y
 CONFIG_ZYNQ_I2C1=y
-CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET=0xFA
+CONFIG_MAC_ADDR_IN_I2C_EEPROM=y
+CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR=0x57
+CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START=0xFA
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ZYNQ=y
 CONFIG_PHY_MARVELL=y
diff --git a/configs/zynq_zybo_defconfig b/configs/zynq_zybo_defconfig
index 3dfc62162f..03958f399d 100644
--- a/configs/zynq_zybo_defconfig
+++ b/configs/zynq_zybo_defconfig
@@ -46,7 +46,9 @@ CONFIG_DM_GPIO=y
 CONFIG_SYS_I2C_ZYNQ=y
 CONFIG_ZYNQ_I2C0=y
 CONFIG_ZYNQ_I2C1=y
-CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET=0xFA
+CONFIG_MAC_ADDR_IN_I2C_EEPROM=y
+CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR=0x50
+CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START=0xFA
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ZYNQ=y
 CONFIG_SPI_FLASH=y
diff --git a/include/configs/syzygy_hub.h b/include/configs/syzygy_hub.h
index 88ee772848..17938398c2 100644
--- a/include/configs/syzygy_hub.h
+++ b/include/configs/syzygy_hub.h
@@ -11,7 +11,6 @@
 #define __CONFIG_SYZYGY_HUB_H
 
 #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN 1
-#define CONFIG_ZYNQ_GEM_EEPROM_ADDR    0x57
 
 #define CONFIG_EXTRA_ENV_SETTINGS       \
 	"fit_image=fit.itb\0"		\
diff --git a/include/configs/zynq_zybo.h b/include/configs/zynq_zybo.h
index 547ecb68fd..c3abf41f76 100644
--- a/include/configs/zynq_zybo.h
+++ b/include/configs/zynq_zybo.h
@@ -11,7 +11,6 @@
 #define __CONFIG_ZYNQ_ZYBO_H
 
 #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	1
-#define CONFIG_ZYNQ_GEM_EEPROM_ADDR	0x50
 #define CONFIG_DISPLAY
 #define CONFIG_I2C_EDID
 
-- 
2.18.0

[U-Boot] [PATCH 3/4] arm64: zynqmp: migrate EEPROM MAC address options to kconfig

[Luis Araneda]

Migrate boards to use the generic kconfig options to
read the MAC address

Additionally, remove the ZYNQ_GEM_I2C_MAC_OFFSET kconfig
option, since no boards is using it now

Signed-off-by: Luis Araneda <luaraneda@gmail.com>
---
 board/xilinx/zynqmp/zynqmp.c                  | 10 ++++------
 configs/xilinx_zynqmp_zcu102_rev1_0_defconfig |  5 ++++-
 configs/xilinx_zynqmp_zcu102_revA_defconfig   |  5 ++++-
 configs/xilinx_zynqmp_zcu102_revB_defconfig   |  5 ++++-
 configs/xilinx_zynqmp_zcu106_revA_defconfig   |  5 ++++-
 configs/xilinx_zynqmp_zcu111_revA_defconfig   |  5 ++++-
 drivers/misc/Kconfig                          |  6 ------
 include/configs/xilinx_zynqmp_zcu102.h        |  2 --
 include/configs/xilinx_zynqmp_zcu106.h        |  2 --
 include/configs/xilinx_zynqmp_zcu111.h        |  2 --
 10 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 81c10fcf8a..1f7b3a4c48 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -372,13 +372,11 @@ int board_early_init_r(void)
 
 int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
 {
-#if defined(CONFIG_ZYNQ_GEM_EEPROM_ADDR) && \
-    defined(CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET) && \
-    defined(CONFIG_ZYNQ_EEPROM_BUS)
-	i2c_set_bus_num(CONFIG_ZYNQ_EEPROM_BUS);
+#if defined(CONFIG_MAC_ADDR_IN_I2C_EEPROM)
+	i2c_set_bus_num(CONFIG_MAC_ADDR_I2C_EEPROM_BUS);
 
-	if (eeprom_read(CONFIG_ZYNQ_GEM_EEPROM_ADDR,
-			CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET,
+	if (eeprom_read(CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR,
+			CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START,
 			ethaddr, 6))
 		printf("I2C EEPROM MAC address read failed\n");
 #endif
diff --git a/configs/xilinx_zynqmp_zcu102_rev1_0_defconfig b/configs/xilinx_zynqmp_zcu102_rev1_0_defconfig
index 49a14d87a8..a693663cbe 100644
--- a/configs/xilinx_zynqmp_zcu102_rev1_0_defconfig
+++ b/configs/xilinx_zynqmp_zcu102_rev1_0_defconfig
@@ -63,7 +63,10 @@ CONFIG_SYS_I2C_ZYNQ=y
 CONFIG_ZYNQ_I2C0=y
 CONFIG_ZYNQ_I2C1=y
 CONFIG_MISC=y
-CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET=0x20
+CONFIG_MAC_ADDR_IN_I2C_EEPROM=y
+CONFIG_MAC_ADDR_I2C_EEPROM_BUS=5
+CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR=0x54
+CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START=0x20
 CONFIG_DM_MMC=y
 CONFIG_MMC_IO_VOLTAGE=y
 CONFIG_MMC_UHS_SUPPORT=y
diff --git a/configs/xilinx_zynqmp_zcu102_revA_defconfig b/configs/xilinx_zynqmp_zcu102_revA_defconfig
index 05dad41acb..2c271fc691 100644
--- a/configs/xilinx_zynqmp_zcu102_revA_defconfig
+++ b/configs/xilinx_zynqmp_zcu102_revA_defconfig
@@ -62,7 +62,10 @@ CONFIG_SYS_I2C_ZYNQ=y
 CONFIG_ZYNQ_I2C0=y
 CONFIG_ZYNQ_I2C1=y
 CONFIG_MISC=y
-CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET=0x20
+CONFIG_MAC_ADDR_IN_I2C_EEPROM=y
+CONFIG_MAC_ADDR_I2C_EEPROM_BUS=5
+CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR=0x54
+CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START=0x20
 CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ZYNQ=y
diff --git a/configs/xilinx_zynqmp_zcu102_revB_defconfig b/configs/xilinx_zynqmp_zcu102_revB_defconfig
index b3711b43e8..c28654c5a6 100644
--- a/configs/xilinx_zynqmp_zcu102_revB_defconfig
+++ b/configs/xilinx_zynqmp_zcu102_revB_defconfig
@@ -62,7 +62,10 @@ CONFIG_SYS_I2C_ZYNQ=y
 CONFIG_ZYNQ_I2C0=y
 CONFIG_ZYNQ_I2C1=y
 CONFIG_MISC=y
-CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET=0x20
+CONFIG_MAC_ADDR_IN_I2C_EEPROM=y
+CONFIG_MAC_ADDR_I2C_EEPROM_BUS=5
+CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR=0x54
+CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START=0x20
 CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ZYNQ=y
diff --git a/configs/xilinx_zynqmp_zcu106_revA_defconfig b/configs/xilinx_zynqmp_zcu106_revA_defconfig
index e644914070..97841eefa4 100644
--- a/configs/xilinx_zynqmp_zcu106_revA_defconfig
+++ b/configs/xilinx_zynqmp_zcu106_revA_defconfig
@@ -59,7 +59,10 @@ CONFIG_SYS_I2C_ZYNQ=y
 CONFIG_ZYNQ_I2C0=y
 CONFIG_ZYNQ_I2C1=y
 CONFIG_MISC=y
-CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET=0x20
+CONFIG_MAC_ADDR_IN_I2C_EEPROM=y
+CONFIG_MAC_ADDR_I2C_EEPROM_BUS=5
+CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR=0x54
+CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START=0x20
 CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ZYNQ=y
diff --git a/configs/xilinx_zynqmp_zcu111_revA_defconfig b/configs/xilinx_zynqmp_zcu111_revA_defconfig
index c88ba522c7..18e7b2ce49 100644
--- a/configs/xilinx_zynqmp_zcu111_revA_defconfig
+++ b/configs/xilinx_zynqmp_zcu111_revA_defconfig
@@ -53,7 +53,10 @@ CONFIG_SYS_I2C_ZYNQ=y
 CONFIG_ZYNQ_I2C0=y
 CONFIG_ZYNQ_I2C1=y
 CONFIG_MISC=y
-CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET=0x20
+CONFIG_MAC_ADDR_IN_I2C_EEPROM=y
+CONFIG_MAC_ADDR_I2C_EEPROM_BUS=5
+CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR=0x54
+CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START=0x20
 CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ZYNQ=y
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index ff694871bf..0c98dce73b 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -223,12 +223,6 @@ config SPL_I2C_EEPROM
 	  This option is an SPL-variant of the I2C_EEPROM option.
 	  See the help of I2C_EEPROM for details.
 
-config ZYNQ_GEM_I2C_MAC_OFFSET
-	hex "Set the I2C MAC offset"
-	default 0x0
-	help
-	  Set the MAC offset for i2C.
-
 if I2C_EEPROM
 
 config SYS_I2C_EEPROM_ADDR
diff --git a/include/configs/xilinx_zynqmp_zcu102.h b/include/configs/xilinx_zynqmp_zcu102.h
index ad6bc3d1bf..4a40d416a2 100644
--- a/include/configs/xilinx_zynqmp_zcu102.h
+++ b/include/configs/xilinx_zynqmp_zcu102.h
@@ -36,8 +36,6 @@
 #define CONFIG_PCA953X
 
 #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	1
-#define CONFIG_ZYNQ_EEPROM_BUS		5
-#define CONFIG_ZYNQ_GEM_EEPROM_ADDR	0x54
 
 #define CONFIG_SPD_EEPROM
 #define CONFIG_DDR_SPD
diff --git a/include/configs/xilinx_zynqmp_zcu106.h b/include/configs/xilinx_zynqmp_zcu106.h
index cc2d145ddd..16ac1d0dc6 100644
--- a/include/configs/xilinx_zynqmp_zcu106.h
+++ b/include/configs/xilinx_zynqmp_zcu106.h
@@ -36,8 +36,6 @@
 #define CONFIG_PCA953X
 
 #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	1
-#define CONFIG_ZYNQ_EEPROM_BUS		5
-#define CONFIG_ZYNQ_GEM_EEPROM_ADDR	0x54
 
 #include <configs/xilinx_zynqmp.h>
 
diff --git a/include/configs/xilinx_zynqmp_zcu111.h b/include/configs/xilinx_zynqmp_zcu111.h
index 8f8cb4f087..f8f52ef639 100644
--- a/include/configs/xilinx_zynqmp_zcu111.h
+++ b/include/configs/xilinx_zynqmp_zcu111.h
@@ -39,8 +39,6 @@
 #define CONFIG_PCA953X
 
 #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	1
-#define CONFIG_ZYNQ_EEPROM_BUS		5
-#define CONFIG_ZYNQ_GEM_EEPROM_ADDR	0x54
 
 #include <configs/xilinx_zynqmp.h>
 
-- 
2.18.0

[U-Boot] [PATCH 4/4] arm: zynq: use i2c cadence DM driver

[Luis Araneda]

Migrate most boards to the Cadence I2C driver and
use DM functions to read the MAC address from EEPROM

Additionally, remove the legacy eeprom command from
defconfig since it was only used to provide the functions
to read the MAC address

Boards ZC702 and ZC706 were not migrated because they have
I2C multiplexers and require extended validation and testing

Signed-off-by: Luis Araneda <luaraneda@gmail.com>
---
 arch/arm/dts/zynq-syzygy-hub.dts  |  1 +
 arch/arm/dts/zynq-zybo.dts        | 10 ++++++++++
 board/xilinx/zynq/board.c         | 24 ++++++++++++++++++++----
 configs/syzygy_hub_defconfig      |  5 ++---
 configs/topic_miami_defconfig     |  5 ++---
 configs/topic_miamilite_defconfig |  5 ++---
 configs/topic_miamiplus_defconfig |  5 ++---
 configs/zynq_zybo_defconfig       |  6 ++----
 include/configs/syzygy_hub.h      |  2 --
 include/configs/zynq_zybo.h       |  1 -
 10 files changed, 41 insertions(+), 23 deletions(-)

diff --git a/arch/arm/dts/zynq-syzygy-hub.dts b/arch/arm/dts/zynq-syzygy-hub.dts
index a30d667146..d89e669b71 100644
--- a/arch/arm/dts/zynq-syzygy-hub.dts
+++ b/arch/arm/dts/zynq-syzygy-hub.dts
@@ -15,6 +15,7 @@
 	aliases {
 		ethernet0 = &gem0;
 		serial0 = &uart0;
+		i2c0 = &i2c1;
 		mmc0 = &sdhci0;
 	};
 
diff --git a/arch/arm/dts/zynq-zybo.dts b/arch/arm/dts/zynq-zybo.dts
index 3844822305..b5e0f3d9b3 100644
--- a/arch/arm/dts/zynq-zybo.dts
+++ b/arch/arm/dts/zynq-zybo.dts
@@ -14,6 +14,8 @@
 		ethernet0 = &gem0;
 		serial0 = &uart1;
 		spi0 = &qspi;
+		i2c0 = &i2c0;
+		i2c1 = &i2c1;
 		mmc0 = &sdhci0;
 	};
 
@@ -49,6 +51,14 @@
 	};
 };
 
+&i2c0 {
+	status = "okay";
+};
+
+&i2c1 {
+	status = "okay";
+};
+
 &qspi {
 	u-boot,dm-pre-reloc;
 	status = "okay";
diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
index dcbf788918..5b9ee10a90 100644
--- a/board/xilinx/zynq/board.c
+++ b/board/xilinx/zynq/board.c
@@ -8,6 +8,7 @@
 #include <dm/uclass.h>
 #include <fdtdec.h>
 #include <fpga.h>
+#include <i2c.h>
 #include <mmc.h>
 #include <watchdog.h>
 #include <wdt.h>
@@ -76,10 +77,25 @@ int board_late_init(void)
 int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
 {
 #if defined(CONFIG_MAC_ADDR_IN_I2C_EEPROM)
-	if (eeprom_read(CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR,
-			CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START,
-			ethaddr, 6))
-		printf("I2C EEPROM MAC address read failed\n");
+	int ret;
+	struct udevice *bus, *dev;
+
+	ret = uclass_get_device_by_seq(UCLASS_I2C,
+				       CONFIG_MAC_ADDR_I2C_EEPROM_BUS,
+				       &bus);
+	if (!ret)
+		ret = i2c_get_chip(bus,
+				   CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR,
+				   1, &dev);
+	if (!ret)
+		ret = i2c_set_chip_offset_len(dev,
+					      CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_LEN);
+	if (!ret)
+		ret = dm_i2c_read(dev,
+				  CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START,
+				  ethaddr, 6);
+	if (ret)
+		printf("I2C EEPROM MAC address read failed (%i)\n", ret);
 #endif
 
 	return 0;
diff --git a/configs/syzygy_hub_defconfig b/configs/syzygy_hub_defconfig
index 221349ba6f..bcc3ecda49 100644
--- a/configs/syzygy_hub_defconfig
+++ b/configs/syzygy_hub_defconfig
@@ -18,7 +18,6 @@ CONFIG_BOOTCOMMAND="run $modeboot || run distro_bootcmd"
 CONFIG_SPL_STACK_R=y
 CONFIG_SPL_OS_BOOT=y
 CONFIG_SYS_PROMPT="Zynq> "
-CONFIG_CMD_EEPROM=y
 # CONFIG_CMD_FLASH is not set
 CONFIG_CMD_FPGA_LOADBP=y
 CONFIG_CMD_FPGA_LOADFS=y
@@ -37,8 +36,8 @@ CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_FPGA_XILINX=y
 CONFIG_FPGA_ZYNQPL=y
 CONFIG_DM_GPIO=y
-CONFIG_SYS_I2C_ZYNQ=y
-CONFIG_ZYNQ_I2C1=y
+CONFIG_DM_I2C=y
+CONFIG_SYS_I2C_CADENCE=y
 CONFIG_MAC_ADDR_IN_I2C_EEPROM=y
 CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR=0x57
 CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START=0xFA
diff --git a/configs/topic_miami_defconfig b/configs/topic_miami_defconfig
index af5b92cd34..408a5485f3 100644
--- a/configs/topic_miami_defconfig
+++ b/configs/topic_miami_defconfig
@@ -34,9 +34,8 @@ CONFIG_DFU_RAM=y
 CONFIG_FPGA_XILINX=y
 CONFIG_FPGA_ZYNQPL=y
 CONFIG_DM_GPIO=y
-CONFIG_SYS_I2C_ZYNQ=y
-CONFIG_ZYNQ_I2C0=y
-CONFIG_ZYNQ_I2C1=y
+CONFIG_DM_I2C=y
+CONFIG_SYS_I2C_CADENCE=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ZYNQ=y
 CONFIG_SPI_FLASH=y
diff --git a/configs/topic_miamilite_defconfig b/configs/topic_miamilite_defconfig
index ad7ba04d85..ee5f512513 100644
--- a/configs/topic_miamilite_defconfig
+++ b/configs/topic_miamilite_defconfig
@@ -34,9 +34,8 @@ CONFIG_DFU_RAM=y
 CONFIG_FPGA_XILINX=y
 CONFIG_FPGA_ZYNQPL=y
 CONFIG_DM_GPIO=y
-CONFIG_SYS_I2C_ZYNQ=y
-CONFIG_ZYNQ_I2C0=y
-CONFIG_ZYNQ_I2C1=y
+CONFIG_DM_I2C=y
+CONFIG_SYS_I2C_CADENCE=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ZYNQ=y
 CONFIG_SPI_FLASH=y
diff --git a/configs/topic_miamiplus_defconfig b/configs/topic_miamiplus_defconfig
index 7455e86622..89512df26d 100644
--- a/configs/topic_miamiplus_defconfig
+++ b/configs/topic_miamiplus_defconfig
@@ -33,9 +33,8 @@ CONFIG_DFU_RAM=y
 CONFIG_FPGA_XILINX=y
 CONFIG_FPGA_ZYNQPL=y
 CONFIG_DM_GPIO=y
-CONFIG_SYS_I2C_ZYNQ=y
-CONFIG_ZYNQ_I2C0=y
-CONFIG_ZYNQ_I2C1=y
+CONFIG_DM_I2C=y
+CONFIG_SYS_I2C_CADENCE=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ZYNQ=y
 CONFIG_SPI_FLASH=y
diff --git a/configs/zynq_zybo_defconfig b/configs/zynq_zybo_defconfig
index 03958f399d..236bc4ff70 100644
--- a/configs/zynq_zybo_defconfig
+++ b/configs/zynq_zybo_defconfig
@@ -19,7 +19,6 @@ CONFIG_SPL_OS_BOOT=y
 CONFIG_SPL_SPI_LOAD=y
 CONFIG_SYS_PROMPT="Zynq> "
 CONFIG_CMD_THOR_DOWNLOAD=y
-CONFIG_CMD_EEPROM=y
 CONFIG_CMD_DFU=y
 # CONFIG_CMD_FLASH is not set
 CONFIG_CMD_FPGA_LOADBP=y
@@ -43,9 +42,8 @@ CONFIG_DFU_RAM=y
 CONFIG_FPGA_XILINX=y
 CONFIG_FPGA_ZYNQPL=y
 CONFIG_DM_GPIO=y
-CONFIG_SYS_I2C_ZYNQ=y
-CONFIG_ZYNQ_I2C0=y
-CONFIG_ZYNQ_I2C1=y
+CONFIG_DM_I2C=y
+CONFIG_SYS_I2C_CADENCE=y
 CONFIG_MAC_ADDR_IN_I2C_EEPROM=y
 CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR=0x50
 CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START=0xFA
diff --git a/include/configs/syzygy_hub.h b/include/configs/syzygy_hub.h
index 17938398c2..e31b77c0c0 100644
--- a/include/configs/syzygy_hub.h
+++ b/include/configs/syzygy_hub.h
@@ -10,8 +10,6 @@
 #ifndef __CONFIG_SYZYGY_HUB_H
 #define __CONFIG_SYZYGY_HUB_H
 
-#define CONFIG_SYS_I2C_EEPROM_ADDR_LEN 1
-
 #define CONFIG_EXTRA_ENV_SETTINGS       \
 	"fit_image=fit.itb\0"		\
 	"bitstream_image=download.bit\0"    \
diff --git a/include/configs/zynq_zybo.h b/include/configs/zynq_zybo.h
index c3abf41f76..f3b628cbac 100644
--- a/include/configs/zynq_zybo.h
+++ b/include/configs/zynq_zybo.h
@@ -10,7 +10,6 @@
 #ifndef __CONFIG_ZYNQ_ZYBO_H
 #define __CONFIG_ZYNQ_ZYBO_H
 
-#define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	1
 #define CONFIG_DISPLAY
 #define CONFIG_I2C_EDID
 
-- 
2.18.0

[U-Boot] [PATCH 0/4] arm: zynq: migrate to DM I2C driver

[Michal Simek]

On 9.7.2018 07:00, Luis Araneda wrote:
> This series migrate most zynq boards to using the
> Cadence DM I2C driver
> 
> The first patch adds kconfig options to generalize the concept
> of reading the MAC address from an I2C EEPROM. Then the zynq and
> zynqmp boards were migrated with patches two and three to use
> the options
> 
> Finally, the last patch migrates the boards to the Cadence I2C driver.
> The boards ZC702 and ZC706 were not migrated because they have
> I2C multiplexers and I don't have a board with a multiplexer to
> perform testing and validation
> 
> I ran a round of reproducible builds on master and another one
> after the third patch for all zynq and zynqmp boards, and the same
> binaries where generated for all boards
> 
> All commits were tested on a Digilent Zybo board
> 
> Luis Araneda (4):
>   drivers/misc: add options to read MAC address from EEPROM
>   arm: zynq: migrate EEPROM MAC address options to kconfig
>   arm64: zynqmp: migrate EEPROM MAC address options to kconfig
>   arm: zynq: use i2c cadence DM driver
> 
>  arch/arm/dts/zynq-syzygy-hub.dts              |  1 +
>  arch/arm/dts/zynq-zybo.dts                    | 10 ++++++
>  board/xilinx/zynq/board.c                     | 27 ++++++++++++----
>  board/xilinx/zynqmp/zynqmp.c                  | 10 +++---
>  configs/syzygy_hub_defconfig                  |  9 +++---
>  configs/topic_miami_defconfig                 |  5 ++-
>  configs/topic_miamilite_defconfig             |  5 ++-
>  configs/topic_miamiplus_defconfig             |  5 ++-
>  configs/xilinx_zynqmp_zcu102_rev1_0_defconfig |  5 ++-
>  configs/xilinx_zynqmp_zcu102_revA_defconfig   |  5 ++-
>  configs/xilinx_zynqmp_zcu102_revB_defconfig   |  5 ++-
>  configs/xilinx_zynqmp_zcu106_revA_defconfig   |  5 ++-
>  configs/xilinx_zynqmp_zcu111_revA_defconfig   |  5 ++-
>  configs/zynq_zybo_defconfig                   | 10 +++---
>  drivers/misc/Kconfig                          | 32 +++++++++++++++----
>  include/configs/syzygy_hub.h                  |  3 --
>  include/configs/xilinx_zynqmp_zcu102.h        |  2 --
>  include/configs/xilinx_zynqmp_zcu106.h        |  2 --
>  include/configs/xilinx_zynqmp_zcu111.h        |  2 --
>  include/configs/zynq_zybo.h                   |  2 --
>  20 files changed, 98 insertions(+), 52 deletions(-)
> 

Please take a look at
https://lists.denx.de/pipermail/u-boot/2016-November/274068.html which
was the series which should replace all these boards setting in a
generic way. Unfortunately I don't know why it didn't go through.

And then regarding DM i2c. Driver is in the driver for a while but for
i2c muxes support it requires all subbusses to be listed in DTS file
which is not what it is common in Linux dt binding. That's why some work
needs to be done in this area to convert all platforms.

Thanks,
Michal

[U-Boot] [PATCH 0/4] arm: zynq: migrate to DM I2C driver

[Luis Araneda]

Hi Michal,

On Thu, Jul 19, 2018 at 3:28 AM Michal Simek <michal.simek@xilinx.com> wrote:
> Please take a look at
> https://lists.denx.de/pipermail/u-boot/2016-November/274068.html which
> was the series which should replace all these boards setting in a
> generic way. Unfortunately I don't know why it didn't go through.

The last version of that series [1] (v6, 28 patches) has the state
"Changes Requested" on patchwork, and it even has some TODOs. It's a
generic way of reading a MAC address from an EEPROM with helper
functions.
I'm not proposing something as generic and elaborated as that series,
but an improvement to what is used today.

I sent this series to move some legacy options to Kconfig.
Additionally, the Zybo Z7 has its MAC address stored on an SPI FLASH,
which will likely require a new Kconfig option (MAC_ADDR_IN_SPI_FLASH)
eventually. Something that [1] doesn't cover (from what I read).

It just occurred to me that what I'm really doing in the fourth patch
is reading the MAC address from a generic memory connected to I2C,
because it's a generic memory read operation (device address, memory
address, read operation). So I think the name of the Kconfigs could be
CONFIG_MAC_ADDR_IN_I2C_MEMORY instead of
CONFIG_MAC_ADDR_IN_I2C_EEPROM. It can even be more generic like
CONFIG_MAC_ADDR_IN_I2C. What do yo think?

> And then regarding DM i2c. Driver is in the driver for a while but for
> i2c muxes support it requires all subbusses to be listed in DTS file
> which is not what it is common in Linux dt binding. That's why some work
> needs to be done in this area to convert all platforms.

Yes, I realize that I2C muxes are a pending issue, but that's why I
only migrated the boards that are not using I2C muxes. From my point
of view, it's less work to be done when porting

Anyway, I just wanted to improve the current state of the code, I
don't need these changes for what I'm doing. So if you like to wait
for [1] to be merged, it's fine with me and we can drop this series,
otherwise, please review it so I can make changes if necessary.

[1] https://lists.denx.de/pipermail/u-boot/2017-May/291401.html

Thanks,

Luis Araneda.

[U-Boot] [PATCH 4/4] arm: zynq: use i2c cadence DM driver

[Michal Simek]

On 9.7.2018 07:00, Luis Araneda wrote:
> Migrate most boards to the Cadence I2C driver and
> use DM functions to read the MAC address from EEPROM
> 
> Additionally, remove the legacy eeprom command from
> defconfig since it was only used to provide the functions
> to read the MAC address
> 
> Boards ZC702 and ZC706 were not migrated because they have
> I2C multiplexers and require extended validation and testing
> 
> Signed-off-by: Luis Araneda <luaraneda@gmail.com>
> ---
>  arch/arm/dts/zynq-syzygy-hub.dts  |  1 +
>  arch/arm/dts/zynq-zybo.dts        | 10 ++++++++++
>  board/xilinx/zynq/board.c         | 24 ++++++++++++++++++++----
>  configs/syzygy_hub_defconfig      |  5 ++---
>  configs/topic_miami_defconfig     |  5 ++---
>  configs/topic_miamilite_defconfig |  5 ++---
>  configs/topic_miamiplus_defconfig |  5 ++---
>  configs/zynq_zybo_defconfig       |  6 ++----
>  include/configs/syzygy_hub.h      |  2 --
>  include/configs/zynq_zybo.h       |  1 -
>  10 files changed, 41 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/arm/dts/zynq-syzygy-hub.dts b/arch/arm/dts/zynq-syzygy-hub.dts
> index a30d667146..d89e669b71 100644
> --- a/arch/arm/dts/zynq-syzygy-hub.dts
> +++ b/arch/arm/dts/zynq-syzygy-hub.dts
> @@ -15,6 +15,7 @@
>  	aliases {
>  		ethernet0 = &gem0;
>  		serial0 = &uart0;
> +		i2c0 = &i2c1;
>  		mmc0 = &sdhci0;
>  	};
>  

This can be separate patch.

> diff --git a/arch/arm/dts/zynq-zybo.dts b/arch/arm/dts/zynq-zybo.dts
> index 3844822305..b5e0f3d9b3 100644
> --- a/arch/arm/dts/zynq-zybo.dts
> +++ b/arch/arm/dts/zynq-zybo.dts
> @@ -14,6 +14,8 @@
>  		ethernet0 = &gem0;
>  		serial0 = &uart1;
>  		spi0 = &qspi;
> +		i2c0 = &i2c0;
> +		i2c1 = &i2c1;
>  		mmc0 = &sdhci0;
>  	};
>  
> @@ -49,6 +51,14 @@
>  	};
>  };
>  
> +&i2c0 {
> +	status = "okay";
> +};
> +
> +&i2c1 {
> +	status = "okay";
> +};
> +

IIRC zybo has no connection from PS to i2c and it is done via PL.
And deal was that only things which are in PS part should be listed in
DTS file. It means this shouldn't be here.


>  &qspi {
>  	u-boot,dm-pre-reloc;
>  	status = "okay";
> diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
> index dcbf788918..5b9ee10a90 100644
> --- a/board/xilinx/zynq/board.c
> +++ b/board/xilinx/zynq/board.c
> @@ -8,6 +8,7 @@
>  #include <dm/uclass.h>
>  #include <fdtdec.h>
>  #include <fpga.h>
> +#include <i2c.h>
>  #include <mmc.h>
>  #include <watchdog.h>
>  #include <wdt.h>
> @@ -76,10 +77,25 @@ int board_late_init(void)
>  int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
>  {
>  #if defined(CONFIG_MAC_ADDR_IN_I2C_EEPROM)
> -	if (eeprom_read(CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR,
> -			CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START,
> -			ethaddr, 6))
> -		printf("I2C EEPROM MAC address read failed\n");
> +	int ret;
> +	struct udevice *bus, *dev;
> +
> +	ret = uclass_get_device_by_seq(UCLASS_I2C,
> +				       CONFIG_MAC_ADDR_I2C_EEPROM_BUS,
> +				       &bus);
> +	if (!ret)
> +		ret = i2c_get_chip(bus,
> +				   CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR,
> +				   1, &dev);
> +	if (!ret)
> +		ret = i2c_set_chip_offset_len(dev,
> +					      CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_LEN);
> +	if (!ret)
> +		ret = dm_i2c_read(dev,
> +				  CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START,
> +				  ethaddr, 6);
> +	if (ret)
> +		printf("I2C EEPROM MAC address read failed (%i)\n", ret);
>  #endif

In ZynqMP there is also call i2c_set_bus_num which is what you do above.

I don't think that make sense to duplicate this code in board files.
This should go to core. I think eeprom hasn't been converted to DM and
that's the thing which should happen first. Then calling sequence should
be the same in board files.


>  
>  	return 0;
> diff --git a/configs/syzygy_hub_defconfig b/configs/syzygy_hub_defconfig
> index 221349ba6f..bcc3ecda49 100644
> --- a/configs/syzygy_hub_defconfig
> +++ b/configs/syzygy_hub_defconfig
> @@ -18,7 +18,6 @@ CONFIG_BOOTCOMMAND="run $modeboot || run distro_bootcmd"
>  CONFIG_SPL_STACK_R=y
>  CONFIG_SPL_OS_BOOT=y
>  CONFIG_SYS_PROMPT="Zynq> "
> -CONFIG_CMD_EEPROM=y

this will caused regression.

>  # CONFIG_CMD_FLASH is not set
>  CONFIG_CMD_FPGA_LOADBP=y
>  CONFIG_CMD_FPGA_LOADFS=y
> @@ -37,8 +36,8 @@ CONFIG_SPL_DM_SEQ_ALIAS=y
>  CONFIG_FPGA_XILINX=y
>  CONFIG_FPGA_ZYNQPL=y
>  CONFIG_DM_GPIO=y
> -CONFIG_SYS_I2C_ZYNQ=y
> -CONFIG_ZYNQ_I2C1=y
> +CONFIG_DM_I2C=y
> +CONFIG_SYS_I2C_CADENCE=y
>  CONFIG_MAC_ADDR_IN_I2C_EEPROM=y
>  CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR=0x57
>  CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START=0xFA
> diff --git a/configs/topic_miami_defconfig b/configs/topic_miami_defconfig
> index af5b92cd34..408a5485f3 100644
> --- a/configs/topic_miami_defconfig
> +++ b/configs/topic_miami_defconfig
> @@ -34,9 +34,8 @@ CONFIG_DFU_RAM=y
>  CONFIG_FPGA_XILINX=y
>  CONFIG_FPGA_ZYNQPL=y
>  CONFIG_DM_GPIO=y
> -CONFIG_SYS_I2C_ZYNQ=y
> -CONFIG_ZYNQ_I2C0=y
> -CONFIG_ZYNQ_I2C1=y
> +CONFIG_DM_I2C=y
> +CONFIG_SYS_I2C_CADENCE=y
>  CONFIG_MMC_SDHCI=y
>  CONFIG_MMC_SDHCI_ZYNQ=y
>  CONFIG_SPI_FLASH=y

convert this board is fine. Mike: Are you ok with it?

> diff --git a/configs/topic_miamilite_defconfig b/configs/topic_miamilite_defconfig
> index ad7ba04d85..ee5f512513 100644
> --- a/configs/topic_miamilite_defconfig
> +++ b/configs/topic_miamilite_defconfig
> @@ -34,9 +34,8 @@ CONFIG_DFU_RAM=y
>  CONFIG_FPGA_XILINX=y
>  CONFIG_FPGA_ZYNQPL=y
>  CONFIG_DM_GPIO=y
> -CONFIG_SYS_I2C_ZYNQ=y
> -CONFIG_ZYNQ_I2C0=y
> -CONFIG_ZYNQ_I2C1=y
> +CONFIG_DM_I2C=y
> +CONFIG_SYS_I2C_CADENCE=y
>  CONFIG_MMC_SDHCI=y
>  CONFIG_MMC_SDHCI_ZYNQ=y
>  CONFIG_SPI_FLASH=y

convert this board is fine.

> diff --git a/configs/topic_miamiplus_defconfig b/configs/topic_miamiplus_defconfig
> index 7455e86622..89512df26d 100644
> --- a/configs/topic_miamiplus_defconfig
> +++ b/configs/topic_miamiplus_defconfig
> @@ -33,9 +33,8 @@ CONFIG_DFU_RAM=y
>  CONFIG_FPGA_XILINX=y
>  CONFIG_FPGA_ZYNQPL=y
>  CONFIG_DM_GPIO=y
> -CONFIG_SYS_I2C_ZYNQ=y
> -CONFIG_ZYNQ_I2C0=y
> -CONFIG_ZYNQ_I2C1=y
> +CONFIG_DM_I2C=y
> +CONFIG_SYS_I2C_CADENCE=y
>  CONFIG_MMC_SDHCI=y
>  CONFIG_MMC_SDHCI_ZYNQ=y
>  CONFIG_SPI_FLASH=y

ditto

> diff --git a/configs/zynq_zybo_defconfig b/configs/zynq_zybo_defconfig
> index 03958f399d..236bc4ff70 100644
> --- a/configs/zynq_zybo_defconfig
> +++ b/configs/zynq_zybo_defconfig
> @@ -19,7 +19,6 @@ CONFIG_SPL_OS_BOOT=y
>  CONFIG_SPL_SPI_LOAD=y
>  CONFIG_SYS_PROMPT="Zynq> "
>  CONFIG_CMD_THOR_DOWNLOAD=y
> -CONFIG_CMD_EEPROM=y

regression.

>  CONFIG_CMD_DFU=y
>  # CONFIG_CMD_FLASH is not set
>  CONFIG_CMD_FPGA_LOADBP=y
> @@ -43,9 +42,8 @@ CONFIG_DFU_RAM=y
>  CONFIG_FPGA_XILINX=y
>  CONFIG_FPGA_ZYNQPL=y
>  CONFIG_DM_GPIO=y
> -CONFIG_SYS_I2C_ZYNQ=y
> -CONFIG_ZYNQ_I2C0=y
> -CONFIG_ZYNQ_I2C1=y
> +CONFIG_DM_I2C=y
> +CONFIG_SYS_I2C_CADENCE=y
>  CONFIG_MAC_ADDR_IN_I2C_EEPROM=y
>  CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR=0x50
>  CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START=0xFA
> diff --git a/include/configs/syzygy_hub.h b/include/configs/syzygy_hub.h
> index 17938398c2..e31b77c0c0 100644
> --- a/include/configs/syzygy_hub.h
> +++ b/include/configs/syzygy_hub.h
> @@ -10,8 +10,6 @@
>  #ifndef __CONFIG_SYZYGY_HUB_H
>  #define __CONFIG_SYZYGY_HUB_H
>  
> -#define CONFIG_SYS_I2C_EEPROM_ADDR_LEN 1
> -
>  #define CONFIG_EXTRA_ENV_SETTINGS       \
>  	"fit_image=fit.itb\0"		\
>  	"bitstream_image=download.bit\0"    \
> diff --git a/include/configs/zynq_zybo.h b/include/configs/zynq_zybo.h
> index c3abf41f76..f3b628cbac 100644
> --- a/include/configs/zynq_zybo.h
> +++ b/include/configs/zynq_zybo.h
> @@ -10,7 +10,6 @@
>  #ifndef __CONFIG_ZYNQ_ZYBO_H
>  #define __CONFIG_ZYNQ_ZYBO_H
>  
> -#define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	1
>  #define CONFIG_DISPLAY
>  #define CONFIG_I2C_EDID
>  
> 

as above.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/d7e19702/attachment.sig>

[U-Boot] [PATCH 0/4] arm: zynq: migrate to DM I2C driver

[Michal Simek]

On 20.7.2018 08:12, Luis Araneda wrote:
> Hi Michal,
> 
> On Thu, Jul 19, 2018 at 3:28 AM Michal Simek <michal.simek@xilinx.com> wrote:
>> Please take a look at
>> https://lists.denx.de/pipermail/u-boot/2016-November/274068.html which
>> was the series which should replace all these boards setting in a
>> generic way. Unfortunately I don't know why it didn't go through.
> 
> The last version of that series [1] (v6, 28 patches) has the state
> "Changes Requested" on patchwork, and it even has some TODOs. It's a
> generic way of reading a MAC address from an EEPROM with helper
> functions.
> I'm not proposing something as generic and elaborated as that series,
> but an improvement to what is used today.
> 
> I sent this series to move some legacy options to Kconfig.
> Additionally, the Zybo Z7 has its MAC address stored on an SPI FLASH,
> which will likely require a new Kconfig option (MAC_ADDR_IN_SPI_FLASH)
> eventually. Something that [1] doesn't cover (from what I read).
> 
> It just occurred to me that what I'm really doing in the fourth patch
> is reading the MAC address from a generic memory connected to I2C,
> because it's a generic memory read operation (device address, memory
> address, read operation). So I think the name of the Kconfigs could be
> CONFIG_MAC_ADDR_IN_I2C_MEMORY instead of
> CONFIG_MAC_ADDR_IN_I2C_EEPROM. It can even be more generic like
> CONFIG_MAC_ADDR_IN_I2C. What do yo think?
> 
>> And then regarding DM i2c. Driver is in the driver for a while but for
>> i2c muxes support it requires all subbusses to be listed in DTS file
>> which is not what it is common in Linux dt binding. That's why some work
>> needs to be done in this area to convert all platforms.
> 
> Yes, I realize that I2C muxes are a pending issue, but that's why I
> only migrated the boards that are not using I2C muxes. From my point
> of view, it's less work to be done when porting
> 
> Anyway, I just wanted to improve the current state of the code, I
> don't need these changes for what I'm doing. So if you like to wait
> for [1] to be merged, it's fine with me and we can drop this series,
> otherwise, please review it so I can make changes if necessary.
> 
> [1] https://lists.denx.de/pipermail/u-boot/2017-May/291401.html

I have no problem to move ZYNQ_GEM_EEPROM_ADDR to Kconfig without
changing the name.

Oliver: Any comment about your patch series?

And I would let Joe to handle that mac address reading.
That v6 series contains a lot of patches which have been acked. The
biggest problem was about sunxi which is something what can be ignored.
If Oliver doesn't want to send v7 then I would recommend you simply take
his v6 version and that patches which are required and just send them
again. I think it won't take so much time because a lot of things were
already done.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/483b2e05/attachment.sig>

[U-Boot] [PATCH 0/4] arm: zynq: migrate to DM I2C driver

[Luis Araneda]

Hi,

On Fri, Jul 20, 2018 at 7:09 AM Michal Simek <monstr@monstr.eu> wrote:
>
> On 20.7.2018 08:12, Luis Araneda wrote:
> > [...]
> > Anyway, I just wanted to improve the current state of the code, I
> > don't need these changes for what I'm doing. So if you like to wait
> > for [1] to be merged, it's fine with me and we can drop this series,
> > otherwise, please review it so I can make changes if necessary.
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2017-May/291401.html
>
> I have no problem to move ZYNQ_GEM_EEPROM_ADDR to Kconfig without
> changing the name.

Ok, I'll send one patch moving ZYNQ_GEM_EEPROM_ADDR to Kconfig,
maintaining its name and I'm dropping my proposal for a generic
option.

Thanks,

Luis Araneda.

[U-Boot] [PATCH 4/4] arm: zynq: use i2c cadence DM driver

[Luis Araneda]

Hi Michal,

On Fri, Jul 20, 2018 at 6:57 AM Michal Simek <monstr@monstr.eu> wrote:
> On 9.7.2018 07:00, Luis Araneda wrote:
> > [...]
> > --- a/arch/arm/dts/zynq-syzygy-hub.dts
> > +++ b/arch/arm/dts/zynq-syzygy-hub.dts
> > @@ -15,6 +15,7 @@
> >       aliases {
> >               ethernet0 = &gem0;
> >               serial0 = &uart0;
> > +             i2c0 = &i2c1;
> >               mmc0 = &sdhci0;
> >       };
> >
>
> This can be separate patch.

Ok

> > diff --git a/arch/arm/dts/zynq-zybo.dts b/arch/arm/dts/zynq-zybo.dts
> > index 3844822305..b5e0f3d9b3 100644
> > --- a/arch/arm/dts/zynq-zybo.dts
> > +++ b/arch/arm/dts/zynq-zybo.dts
> > @@ -14,6 +14,8 @@
> >               ethernet0 = &gem0;
> >               serial0 = &uart1;
> >               spi0 = &qspi;
> > +             i2c0 = &i2c0;
> > +             i2c1 = &i2c1;
> >               mmc0 = &sdhci0;
> >       };
> >
> > @@ -49,6 +51,14 @@
> >       };
> >  };
> >
> > +&i2c0 {
> > +     status = "okay";
> > +};
> > +
> > +&i2c1 {
> > +     status = "okay";
> > +};
> > +
>
> IIRC zybo has no connection from PS to i2c and it is done via PL.
> And deal was that only things which are in PS part should be listed in
> DTS file. It means this shouldn't be here.

The PS has I2C peripherals. For the Zybo in particular, there are two
I2Cs, which must be routed through the PL to be connected to the
EEPROM and HDMI port respectively. That means that if the PL is not
programmed, the I2C driver binds and works properly but the SCL and
SDA signals of the I2C peripherals are not connected to anything.
So, I'm not sure if the rule is applicable here, because the I2Cs are
in PS, but to properly use them you have to program the PL.

Besides, adding the aliases and nodes was the only way I could find to
make the driver work (bind) automatically. If I don't add the aliases,
the I2C devices are not assigned a bus (-1), and if I don't add the
nodes, the driver does not bind.

If the I2C nodes don't belong in the .dts file, and I'm using DM
(which will eventually be mandatory), does that means I would have to
bind the driver manually to obtain the current functionality without
DM? I suppose that's possible (don't know how). It sounds like the
functionality provided by device-tree overlays.

> >  &qspi {
> >       u-boot,dm-pre-reloc;
> >       status = "okay";
> > diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
> > index dcbf788918..5b9ee10a90 100644
> > --- a/board/xilinx/zynq/board.c
> > +++ b/board/xilinx/zynq/board.c
> > @@ -8,6 +8,7 @@
> >  #include <dm/uclass.h>
> >  #include <fdtdec.h>
> >  #include <fpga.h>
> > +#include <i2c.h>
> >  #include <mmc.h>
> >  #include <watchdog.h>
> >  #include <wdt.h>
> > @@ -76,10 +77,25 @@ int board_late_init(void)
> >  int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
> >  {
> >  #if defined(CONFIG_MAC_ADDR_IN_I2C_EEPROM)
> > -     if (eeprom_read(CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR,
> > -                     CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START,
> > -                     ethaddr, 6))
> > -             printf("I2C EEPROM MAC address read failed\n");
> > +     int ret;
> > +     struct udevice *bus, *dev;
> > +
> > +     ret = uclass_get_device_by_seq(UCLASS_I2C,
> > +                                    CONFIG_MAC_ADDR_I2C_EEPROM_BUS,
> > +                                    &bus);
> > +     if (!ret)
> > +             ret = i2c_get_chip(bus,
> > +                                CONFIG_MAC_ADDR_I2C_EEPROM_CHIP_ADDR,
> > +                                1, &dev);
> > +     if (!ret)
> > +             ret = i2c_set_chip_offset_len(dev,
> > +                                           CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_LEN);
> > +     if (!ret)
> > +             ret = dm_i2c_read(dev,
> > +                               CONFIG_MAC_ADDR_I2C_EEPROM_DATA_ADDR_START,
> > +                               ethaddr, 6);
> > +     if (ret)
> > +             printf("I2C EEPROM MAC address read failed (%i)\n", ret);
> >  #endif
>
> In ZynqMP there is also call i2c_set_bus_num which is what you do above.
>
> I don't think that make sense to duplicate this code in board files.
> This should go to core. I think eeprom hasn't been converted to DM and
> that's the thing which should happen first. Then calling sequence should
> be the same in board files.

Ok. I'm assuming that you are referring to the eeprom command? because
I found the i2c_eeprom driver (CONFIG_I2C_EEPROM), which is using DM.
But, I'm not sure if we can use it because it has the same problems of
the I2C nodes above, and this is even worse, as the driver won't bind
unless the PL is programmed first.

> > --- a/configs/syzygy_hub_defconfig
> > +++ b/configs/syzygy_hub_defconfig
> > @@ -18,7 +18,6 @@ CONFIG_BOOTCOMMAND="run $modeboot || run distro_bootcmd"
> >  CONFIG_SPL_STACK_R=y
> >  CONFIG_SPL_OS_BOOT=y
> >  CONFIG_SYS_PROMPT="Zynq> "
> > -CONFIG_CMD_EEPROM=y
>
> this will caused regression.

Is it because someone could have been using the command?


Thanks,

Luis Araneda.

[U-Boot] [PATCH 0/4] arm: zynq: migrate to DM I2C driver

[Olliver Schinagl]

Hi all,

On 20-07-18 13:09, Michal Simek wrote:
> On 20.7.2018 08:12, Luis Araneda wrote:
>> Hi Michal,
>>
>> On Thu, Jul 19, 2018 at 3:28 AM Michal Simek <michal.simek@xilinx.com> wrote:
>>> Please take a look at
>>> https://lists.denx.de/pipermail/u-boot/2016-November/274068.html which
>>> was the series which should replace all these boards setting in a
>>> generic way. Unfortunately I don't know why it didn't go through.
Neither do I :) some of them where 'done' as far as I remember, but it 
has been a while and I have somewhat forgotten about them a little. They 
have been used in production since then however for us.

There are plans to work on our u-boot again in the next few months and 
my intention was to pick up the ones that did not get merged.

>>
>> The last version of that series [1] (v6, 28 patches) has the state
>> "Changes Requested" on patchwork, and it even has some TODOs. It's a
>> generic way of reading a MAC address from an EEPROM with helper
>> functions.
>> I'm not proposing something as generic and elaborated as that series,
>> but an improvement to what is used today.
>>
>> I sent this series to move some legacy options to Kconfig.
>> Additionally, the Zybo Z7 has its MAC address stored on an SPI FLASH,
>> which will likely require a new Kconfig option (MAC_ADDR_IN_SPI_FLASH)
>> eventually. Something that [1] doesn't cover (from what I read).
No, I did not have SPI memory available at the time, and was sort of 
waiting for the generic EEPROM framework to progress (that died).

Having said that, adding SPI eeprom would have been trivial.

>>
>> It just occurred to me that what I'm really doing in the fourth patch
>> is reading the MAC address from a generic memory connected to I2C,
>> because it's a generic memory read operation (device address, memory
>> address, read operation). So I think the name of the Kconfigs could be
>> CONFIG_MAC_ADDR_IN_I2C_MEMORY instead of
>> CONFIG_MAC_ADDR_IN_I2C_EEPROM. It can even be more generic like
>> CONFIG_MAC_ADDR_IN_I2C. What do yo think?
Well I think the generic memory framework did make sense at the time, 
and didn't compulab merge something like this at some point?

>>
>>> And then regarding DM i2c. Driver is in the driver for a while but for
>>> i2c muxes support it requires all subbusses to be listed in DTS file
>>> which is not what it is common in Linux dt binding. That's why some work
>>> needs to be done in this area to convert all platforms.
>>
>> Yes, I realize that I2C muxes are a pending issue, but that's why I
>> only migrated the boards that are not using I2C muxes. From my point
>> of view, it's less work to be done when porting
>>
>> Anyway, I just wanted to improve the current state of the code, I
>> don't need these changes for what I'm doing. So if you like to wait
>> for [1] to be merged, it's fine with me and we can drop this series,
>> otherwise, please review it so I can make changes if necessary.
>>
>> [1] https://lists.denx.de/pipermail/u-boot/2017-May/291401.html
> 
> I have no problem to move ZYNQ_GEM_EEPROM_ADDR to Kconfig without
> changing the name.
> 
> Oliver: Any comment about your patch series?
What I just wrote above :)
> 
> And I would let Joe to handle that mac address reading.
> That v6 series contains a lot of patches which have been acked. The
> biggest problem was about sunxi which is something what can be ignored.
> If Oliver doesn't want to send v7 then I would recommend you simply take
> his v6 version and that patches which are required and just send them
> again. I think it won't take so much time because a lot of things were
> already done.
I don't mind starting to work on a v7; i think one of the issues I ran 
into was turn around time which took quite a while. I may even have v7 
locally actually! I'll see what can be done within the comming period; 
but again, we are going to update/add more support to our u-boot so I 
should be working on this again soon-ish.

> 
> Thanks,
> Michal
> 
>