[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