[U-Boot] [PATCH v1] ddr, fsl: add DM_I2C support

Heiko Schocher hs at denx.de
Mon Aug 26 05:37:32 UTC 2019


Hello Prabhakar,

Am 22.08.2019 um 13:33 schrieb Prabhakar Kushwaha:
> Dear Heiko,
> 
> 
>> -----Original Message-----
>> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Heiko Schocher
>> Sent: Tuesday, July 16, 2019 9:29 AM
>> To: u-boot at lists.denx.de
>> Cc: York Sun <york.sun at nxp.com>
>> Subject: [U-Boot] [PATCH v1] ddr, fsl: add DM_I2C support
>>
>> add DM_I2C support for this driver.
>>
>> Signed-off-by: Heiko Schocher <hs at denx.de>
>>
>> ---
>>
>> Did not fixed checkpatch warning:
>>
>> CHECK: Prefer kernel type 'u8' over 'uint8_t'
>> +       uint8_t buf = 0;
>>
>> Travis build, see:
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis
>> -ci.org%2Fhsdenx%2Fu-boot-
>> test%2Fbuilds%2F558858904&data=02%7C01%7Cprabhakar.kushwaha%
>> 40nxp.com%7C2c5f1ecc6ff9417a8ccb08d709a20297%7C686ea1d3bc2b4c6fa92
>> cd99c5c301635%7C0%7C0%7C636988463754459178&sdata=H%2FkQV%2
>> Bavu2EfajG3El5M%2FsKyuSPO6Nn0QRMVpzsvsUY%3D&reserved=0
>>
>>   drivers/ddr/fsl/main.c | 88 ++++++++++++++++++++++++++++++++++++--
>> ----
> 
> How you tested both NXP's powerpc and ARM platforms as fsl i2c is yet to move to DM model?
> Or my understanding is wrong.

I only tested on powerpc (preciser socrates_defconfig) platform ...

But I see that now in current mainline there is applied a change in drivers/ddr/fsl/main.c
from commit:

commit 0eba65d2013e5517e70cc9b3d467ba8183b54cd9
Author: Chuanhua Han <chuanhua.han at nxp.com>
Date:   Wed Jul 10 21:00:20 2019 +0800

boards: lx2160a: Add support of I2C driver model

     DM_I2C_COMPAT is a compatibility layer that allows using the non-DM I2C
     API when DM_I2C is used. When DM_I2C_COMPAT is not enabled for
     compilation, a compilation error will be generated. This patch solves
     the problem that the i2c-related api of the lx2160a platform does not
     support dm.

     Signed-off-by: Chuanhua Han <chuanhua.han at nxp.com>
     Reviewed-by: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>

Which does some similiar change ... but I think this already applied
patch introduces more ifdefs and code ...


Hmm.. just tried to compile U-Boot for the socrates board (with DM
changes) without my patch ... I get compilererror:

/home/hs/data/Entwicklung/abb/uboot-rework/u-boot/drivers/ddr/fsl/main.c: In function '__get_spd':
/home/hs/data/Entwicklung/abb/uboot-rework/u-boot/drivers/ddr/fsl/main.c:144:52: error: 'dev' 
undeclared (first use in this function)
   ret = i2c_get_chip_for_busnum(0, i2c_address, 1, &dev);
                                                     ^~~
/home/hs/data/Entwicklung/abb/uboot-rework/u-boot/drivers/ddr/fsl/main.c:144:52: note: each 
undeclared identifier is reported only once for each function it appears in
make[2]: *** [/home/hs/data/Entwicklung/abb/uboot-rework/u-boot/scripts/Makefile.build:278: 
drivers/ddr/fsl/main.o] Fehler 1
make[1]: *** [/home/hs/data/Entwicklung/abb/uboot-rework/u-boot/Makefile:1588: drivers/ddr/fsl] Fehler 2
make[1]: *** Es wird auf noch nicht beendete Prozesse gewartet....
   CC      drivers/i2c/fsl_i2c.o
   CC      cmd/elf.o

:-(

Ok, fixed this fast and dirty with:

diff --git a/drivers/ddr/fsl/main.c b/drivers/ddr/fsl/main.c
index ac0783b428..5f05a2c863 100644
--- a/drivers/ddr/fsl/main.c
+++ b/drivers/ddr/fsl/main.c
@@ -91,6 +91,7 @@ static void __get_spd(generic_spd_eeprom_t *spd, u8 i2c_address)
  #ifdef CONFIG_SYS_FSL_DDR4
         uint8_t dummy = 0;
  #endif
+       struct udevice *dev;

  #ifndef CONFIG_DM_I2C
         i2c_set_bus_num(CONFIG_SYS_SPD_BUS_NUM);

and now code compiles and after flashing U-Boot I see:

U-Boot 2019.10-rc2-00151-ge4b57b4557-dirty (Aug 26 2019 - 07:17:47 +0200)

CPU:   8544, Version: 2.1, (0x80340121)
Core:  e500, Version: 2.3, (0x80210023)
Clock Configuration:
        CPU0:666.667 MHz,
        CCB:333.333 MHz,
        DDR:166.667 MHz (333.333 MT/s data rate), LBC:333.333 MHz
L1:    D-cache 32 KiB enabled
        I-cache 32 KiB enabled
Model: abb,socrates
Board: Socrates, serial# 1001321865
PCI1:  32 bit, 33 MHz (PCI_CLK)
DRAM:  Detected UDIMM MSC2S512M667C1-H
512 MiB (DDR2, 64-bit, CL=3, ECC off)
Flash: 32 MiB
L2:    256 KiB enabled
NAND:  1024 MiB
Loading Environment from Flash... OK
In:    serial
Out:   serial
Err:   serial
Net:   TSEC0, TSEC1
Hit any key to stop autoboot:  0
=>

So this works also for me ...

Hmm... the change from commit 0eba65d2013e5517e70cc9b3d467ba8183b54cd9
doubles code for DM and none DM case ... I don;t like this.

May you can look through my rebased patch [1] and may test it ?

If this is OK, I can send it as a v2.

bye,
Heiko

[1] rebased patch
 From 0180ade1202d806db7b577230b240605e5ccea7e Mon Sep 17 00:00:00 2001
From: Heiko Schocher <hs at denx.de>
Date: Mon, 26 Aug 2019 07:09:27 +0200
Subject: [PATCH] ddr, fsl: add DM_I2C support

add DM_I2C support for this driver.

Signed-off-by: Heiko Schocher <hs at denx.de>
Patch-cc: York Sun <york.sun at nxp.com>
Patch-cc: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>
Patch-cc: Chuanhua Han <chuanhua.han at nxp.com>

Series-to: uboot
Series-version: 2

Commit-notes:

Did not fixed checkpatch warning:

CHECK: Prefer kernel type 'u8' over 'uint8_t'
+       uint8_t buf = 0;

Travis build, see:
END

Signed-off-by: Heiko Schocher <hs at denx.de>
---
  drivers/ddr/fsl/main.c | 119 +++++++++++++++++++++++++++++++------------------
  1 file changed, 76 insertions(+), 43 deletions(-)

diff --git a/drivers/ddr/fsl/main.c b/drivers/ddr/fsl/main.c
index ac0783b428..a19041d799 100644
--- a/drivers/ddr/fsl/main.c
+++ b/drivers/ddr/fsl/main.c
@@ -10,6 +10,7 @@
   */

  #include <common.h>
+#include <dm.h>
  #include <i2c.h>
  #include <fsl_ddr_sdram.h>
  #include <fsl_ddr.h>
@@ -82,20 +83,82 @@ u8 spd_i2c_addr[CONFIG_SYS_NUM_DDR_CTLRS][CONFIG_DIMM_SLOTS_PER_CTLR] = {

  #endif

+#if defined(CONFIG_DM_I2C)
+#define DEV_TYPE struct udevice
+#else
+/* Local udevice */
+struct ludevice {
+	u8 chip;
+};
+
+#define DEV_TYPE struct ludevice
+
+#endif
+
  #define SPD_SPA0_ADDRESS	0x36
  #define SPD_SPA1_ADDRESS	0x37

-static void __get_spd(generic_spd_eeprom_t *spd, u8 i2c_address)
+static int ddr_i2c_read(DEV_TYPE *dev, unsigned int addr,
+			int alen, uint8_t *buf, int len)
  {
  	int ret;
+
+#ifdef CONFIG_DM_I2C
+	ret = dm_i2c_read(dev, 0, buf, len);
+#else
+	ret = i2c_read(dev->chip, addr, alen, buf, len);
+#endif
+
+	return ret;
+}
+
  #ifdef CONFIG_SYS_FSL_DDR4
-	uint8_t dummy = 0;
+static int ddr_i2c_dummy_write(unsigned int chip_addr)
+{
+	uint8_t buf = 0;
+
+#ifdef CONFIG_DM_I2C
+	struct udevice *dev;
+	int ret;
+
+	ret = i2c_get_chip_for_busnum(CONFIG_SYS_SPD_BUS_NUM, i2c_address,
+				      1, &dev);
+	if (ret) {
+		printf("%s: Cannot find udev for a bus %d\n", __func__,
+		       CONFIG_SYS_SPD_BUS_NUM);
+		return ret;
+	}
+
+	return dm_i2c_write(dev, 0, buf, 1);
+#else
+	return i2c_write(chip_addr, 0, 1, &buf, 1);
  #endif

-#ifndef CONFIG_DM_I2C
-	i2c_set_bus_num(CONFIG_SYS_SPD_BUS_NUM);
+	return 0;
+}
  #endif

+static void __get_spd(generic_spd_eeprom_t *spd, u8 i2c_address)
+{
+	int ret;
+	DEV_TYPE *dev;
+
+#if defined(CONFIG_DM_I2C)
+	ret = i2c_get_chip_for_busnum(CONFIG_SYS_SPD_BUS_NUM, i2c_address,
+				      1, &dev);
+	if (ret) {
+		printf("%s: Cannot find udev for a bus %d\n", __func__,
+		       CONFIG_SYS_SPD_BUS_NUM);
+		return;
+	}
+#else /* Non DM I2C support - will be removed */
+	struct ludevice ldev = {
+		.chip = i2c_address,
+	};
+	dev = &ldev;
+
+	i2c_set_bus_num(CONFIG_SYS_SPD_BUS_NUM);
+#endif

  #ifdef CONFIG_SYS_FSL_DDR4
  	/*
@@ -104,49 +167,19 @@ static void __get_spd(generic_spd_eeprom_t *spd, u8 i2c_address)
  	 * To access the upper 256 bytes, we need to set EE page address to 1
  	 * See Jedec standar No. 21-C for detail
  	 */
-#ifndef CONFIG_DM_I2C
-	i2c_write(SPD_SPA0_ADDRESS, 0, 1, &dummy, 1);
-	ret = i2c_read(i2c_address, 0, 1, (uchar *)spd, 256);
+	ddr_i2c_dummy_write(SPD_SPA0_ADDRESS);
+	ret = ddr_i2c_read(dev, 0, 1, (uchar *)spd, 256);
  	if (!ret) {
-		i2c_write(SPD_SPA1_ADDRESS, 0, 1, &dummy, 1);
-		ret = i2c_read(i2c_address, 0, 1,
-			       (uchar *)((ulong)spd + 256),
-			       min(256,
-				   (int)sizeof(generic_spd_eeprom_t) - 256));
+		ddr_i2c_dummy_write(SPD_SPA1_ADDRESS);
+		ret = ddr_i2c_read(dev, 0, 1, (uchar *)((ulong)spd + 256),
+				   min(256,
+				       (int)sizeof(generic_spd_eeprom_t)
+				       - 256));
  	}
-#else
-	struct udevice *dev;
-	int read_len = min(256, (int)sizeof(generic_spd_eeprom_t) - 256);

-	ret = i2c_get_chip_for_busnum(0, SPD_SPA0_ADDRESS, 1, &dev);
-	if (!ret)
-		dm_i2c_write(dev, 0, &dummy, 1);
-	ret = i2c_get_chip_for_busnum(0, i2c_address, 1, &dev);
-	if (!ret) {
-		if (!dm_i2c_read(dev, 0, (uchar *)spd, 256)) {
-			if (!i2c_get_chip_for_busnum(0, SPD_SPA1_ADDRESS,
-						     1, &dev))
-				dm_i2c_write(dev, 0, &dummy, 1);
-			if (!i2c_get_chip_for_busnum(0, i2c_address, 1, &dev))
-				ret = dm_i2c_read(dev, 0,
-						  (uchar *)((ulong)spd + 256),
-						  read_len);
-		}
-	}
-#endif
-
-#else
-
-#ifndef CONFIG_DM_I2C
-	ret = i2c_read(i2c_address, 0, 1, (uchar *)spd,
-			sizeof(generic_spd_eeprom_t));
  #else
-	ret = i2c_get_chip_for_busnum(0, i2c_address, 1, &dev);
-	if (!ret)
-		ret = dm_i2c_read(dev, 0, (uchar *)spd,
-				  sizeof(generic_spd_eeprom_t));
-#endif
-
+	ret = ddr_i2c_read(dev, 0, 1, (uchar *)spd,
+			   sizeof(generic_spd_eeprom_t));
  #endif

  	if (ret) {
-- 
2.13.6


-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de


More information about the U-Boot mailing list