arm: zynq: migrate to DM I2C driver

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 <[email protected]>

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

arch/arm/dts/zynq-syzygy-hub.dts +0 -0

index a30d667146..d89e669b71 100644

arch/arm/dts/zynq-zybo.dts +2 -0

index 3844822305..b5e0f3d9b3 100644
@@ -14,6 +14,8 @@
 		ethernet0 = &gem0;
 		serial0 = &uart1;
 		spi0 = &qspi;
+		i2c0 = &i2c0;
+		i2c1 = &i2c1;
 		mmc0 = &sdhci0;
 	};
 

@@ -49,6 +51,14 @@
 	};

arch/arm/dts/zynq-zybo.dts +0 -0

 &qspi {
 	u-boot,dm-pre-reloc;
 	status = "okay";

board/xilinx/zynq/board.c +11 -4

index dcbf788918..5b9ee10a90 100644
@@ -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);

configs/syzygy_hub_defconfig +2 -2

 # 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

configs/topic_miami_defconfig +0 -0

index af5b92cd34..408a5485f3 100644
@@ -34,9 +34,8 @@ CONFIG_DFU_RAM=y
 CONFIG_FPGA_XILINX=y

configs/topic_miamilite_defconfig +0 -0

index ad7ba04d85..ee5f512513 100644
@@ -34,9 +34,8 @@ CONFIG_DFU_RAM=y
 CONFIG_FPGA_XILINX=y

configs/topic_miamiplus_defconfig +0 -0

index 7455e86622..89512df26d 100644
@@ -33,9 +33,8 @@ CONFIG_DFU_RAM=y
 CONFIG_FPGA_XILINX=y

configs/zynq_zybo_defconfig +2 -3

 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

include/configs/syzygy_hub.h +0 -2

index 17938398c2..e31b77c0c0 100644
@@ -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"    \

include/configs/zynq_zybo.h +0 -0

index c3abf41f76..f3b628cbac 100644

arch/arm/dts/zynq-syzygy-hub.dts line 21

Quoted 11 lines · Luis Araneda

[...]

arch/arm/dts/zynq-syzygy-hub.dts +1 -0

@@ -15,6 +15,7 @@
      aliases {
              ethernet0 = &gem0;
              serial0 = &uart0;
+             i2c0 = &i2c1;
              mmc0 = &sdhci0;
      };
 

This can be separate patch.

Quoted 17 lines

arch/arm/dts/zynq-zybo.dts +2 -0

index 3844822305..b5e0f3d9b3 100644
@@ -14,6 +14,8 @@
              ethernet0 = &gem0;
              serial0 = &uart1;
              spi0 = &qspi;
+             i2c0 = &i2c0;
+             i2c1 = &i2c1;
              mmc0 = &sdhci0;
      };
 

@@ -49,6 +51,14 @@
      };
 };
 

arch/arm/dts/zynq-zybo.dts line 61

Quoted 8 lines

arch/arm/dts/zynq-zybo.dts +8 -0

+&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.

Quoted 33 lines

arch/arm/dts/zynq-zybo.dts +0 -0

 &qspi {
      u-boot,dm-pre-reloc;
      status = "okay";

board/xilinx/zynq/board.c +11 -4

index dcbf788918..5b9ee10a90 100644
@@ -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);

board/xilinx/zynq/board.c int board_late_init(void) · line 99

Quoted 10 lines

board/xilinx/zynq/board.c +9 -0

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

configs/syzygy_hub_defconfig CONFIG_BOOTCOMMAND="run $modeboot || run distro_bootcmd" · line 21

Quoted 7 lines

configs/syzygy_hub_defconfig +0 -1

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

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.

Hi Michal,

Quoted 4 lines · Michal Simek

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?

Quoted 4 lines

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

Quoted 6 lines · Luis Araneda

[...]
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.

Quoted 1 line

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

Quoted 7 lines · Luis Araneda

Hi Michal,

Quoted 4 lines · Michal Simek

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.

Quoted 8 lines

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.

Quoted 4 lines

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

Quoted 8 lines

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?

Quoted 15 lines

Quoted 5 lines

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.

Quoted 1 line

[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