| Message ID | 20190716035911.1557636-1-hs@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Prabhakar Kushwaha |
| Headers | show |
| Series | [U-Boot,v1] ddr, fsl: add DM_I2C support | expand |
Dear Heiko, > -----Original Message----- > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Heiko Schocher > Sent: Tuesday, July 16, 2019 9:29 AM > To: u-boot@lists.denx.de > Cc: York Sun <york.sun@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@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. --pk
Hello Prabhakar, Am 22.08.2019 um 13:33 schrieb Prabhakar Kushwaha: > Dear Heiko, > > >> -----Original Message----- >> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Heiko Schocher >> Sent: Tuesday, July 16, 2019 9:29 AM >> To: u-boot@lists.denx.de >> Cc: York Sun <york.sun@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@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@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@nxp.com> Reviewed-by: Prabhakar Kushwaha <prabhakar.kushwaha@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@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@denx.de> Patch-cc: York Sun <york.sun@nxp.com> Patch-cc: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com> Patch-cc: Chuanhua Han <chuanhua.han@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@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) {
Dear Heiko, > -----Original Message----- > From: Heiko Schocher <hs@denx.de> > Sent: Monday, August 26, 2019 11:08 AM > To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com> > Cc: u-boot@lists.denx.de; York Sun <york.sun@nxp.com>; Chuanhua Han > <chuanhua.han@nxp.com> > Subject: Re: [U-Boot] [PATCH v1] ddr, fsl: add DM_I2C support > > Hello Prabhakar, > > Am 22.08.2019 um 13:33 schrieb Prabhakar Kushwaha: > > Dear Heiko, > > > > > >> -----Original Message----- > >> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Heiko > >> Schocher > >> Sent: Tuesday, July 16, 2019 9:29 AM > >> To: u-boot@lists.denx.de > >> Cc: York Sun <york.sun@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@denx.de> > >> > >> --- > >> > >> Did not fixed checkpatch warning: > >> > >> CHECK: Prefer kernel type 'u8' over 'uint8_t' > >> + uint8_t buf = 0; > >> > >> Travis build, see: > >> https://travis > >> -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@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@nxp.com> > Reviewed-by: Prabhakar Kushwaha <prabhakar.kushwaha@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 > > :-( > My bad. I only tested build/run on NXP's ARM platforms. As I am working on PowerPC pull request, I missed it. > 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@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@denx.de> > Patch-cc: York Sun <york.sun@nxp.com> > Patch-cc: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com> > Patch-cc: Chuanhua Han <chuanhua.han@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 > Please send v2 version. I will test on both ARM and PowerPC platforms And also get reviewed from internal IP owners . . --pk
Hello Prabhakar, Am 26.08.2019 um 10:11 schrieb Prabhakar Kushwaha: > Dear Heiko, > >> -----Original Message----- >> From: Heiko Schocher <hs@denx.de> >> Sent: Monday, August 26, 2019 11:08 AM >> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com> >> Cc: u-boot@lists.denx.de; York Sun <york.sun@nxp.com>; Chuanhua Han >> <chuanhua.han@nxp.com> >> Subject: Re: [U-Boot] [PATCH v1] ddr, fsl: add DM_I2C support >> >> Hello Prabhakar, >> >> Am 22.08.2019 um 13:33 schrieb Prabhakar Kushwaha: >>> Dear Heiko, >>> >>> >>>> -----Original Message----- >>>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Heiko >>>> Schocher >>>> Sent: Tuesday, July 16, 2019 9:29 AM >>>> To: u-boot@lists.denx.de >>>> Cc: York Sun <york.sun@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@denx.de> >>>> >>>> --- >>>> >>>> Did not fixed checkpatch warning: >>>> >>>> CHECK: Prefer kernel type 'u8' over 'uint8_t' >>>> + uint8_t buf = 0; >>>> >>>> Travis build, see: >>>> https://travis >>>> -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@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@nxp.com> >> Reviewed-by: Prabhakar Kushwaha <prabhakar.kushwaha@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 >> >> :-( >> > > > My bad. I only tested build/run on NXP's ARM platforms. > As I am working on PowerPC pull request, I missed it. > > > > >> 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@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@denx.de> >> Patch-cc: York Sun <york.sun@nxp.com> >> Patch-cc: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com> >> Patch-cc: Chuanhua Han <chuanhua.han@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 >> > > Please send v2 version. I will test on both ARM and PowerPC platforms > And also get reviewed from internal IP owners . . Ok, started travis build, if it is fine, I send the v2. bye, Heiko
diff --git a/drivers/ddr/fsl/main.c b/drivers/ddr/fsl/main.c index e1f69a1d25..cb19d0f0ff 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,17 +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 + 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 /* @@ -101,18 +167,18 @@ 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 */ - 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 - ret = i2c_read(i2c_address, 0, 1, (uchar *)spd, - sizeof(generic_spd_eeprom_t)); + ret = ddr_i2c_read(dev, 0, 1, (uchar *)spd, + sizeof(generic_spd_eeprom_t)); #endif if (ret) {
add DM_I2C support for this driver. Signed-off-by: Heiko Schocher <hs@denx.de> --- Did not fixed checkpatch warning: CHECK: Prefer kernel type 'u8' over 'uint8_t' + uint8_t buf = 0; Travis build, see: https://travis-ci.org/hsdenx/u-boot-test/builds/558858904 drivers/ddr/fsl/main.c | 88 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 77 insertions(+), 11 deletions(-)