[dwi2c PATCH v1] dwi2c add offsets to reads
modify the designware_i2c_xfer function to use 1 byte per address, it was set to 0 before which makes it think its reading a register type. Added offset of where it is supposed to read from. Before it was always reading from offset 0 despite specifying the offset in the higher level function.
Signed-off-by: Cooper Duffin cooperx.duffin@intel.com Signed-off-by: cduffinx cooperx.duffin@intel.com ---
drivers/i2c/designware_i2c.c | 5 +++-- drivers/i2c/i2c-uclass.c | 1 + include/i2c.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index 0b5e70af59..46f5d1e56c 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -658,8 +658,9 @@ static int designware_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, for (; nmsgs > 0; nmsgs--, msg++) { debug("i2c_xfer: chip=0x%x, len=0x%x\n", msg->addr, msg->len); if (msg->flags & I2C_M_RD) { - ret = __dw_i2c_read(i2c->regs, msg->addr, 0, 0, - msg->buf, msg->len); + ret = __dw_i2c_read(i2c->regs, msg->addr, + msg->offset, 1, msg->buf, + msg->len); } else { ret = __dw_i2c_write(i2c->regs, msg->addr, 0, 0, msg->buf, msg->len); diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c index 2aa3efe8aa..455d56c3b8 100644 --- a/drivers/i2c/i2c-uclass.c +++ b/drivers/i2c/i2c-uclass.c @@ -147,6 +147,7 @@ int dm_i2c_read(struct udevice *dev, uint offset, uint8_t *buffer, int len) ptr->flags |= I2C_M_RD; ptr->len = len; ptr->buf = buffer; + ptr->offset = offset; ptr++; } msg_count = ptr - msg; diff --git a/include/i2c.h b/include/i2c.h index 0faf8542e2..f997c2537e 100644 --- a/include/i2c.h +++ b/include/i2c.h @@ -141,6 +141,7 @@ struct i2c_msg { uint addr; uint flags; uint len; + uint offset; u8 *buf; };
Hi Cduffinx,
On Thu, 5 Nov 2020 at 14:26, cduffinx cooperx.duffin@intel.com wrote:
modify the designware_i2c_xfer function to use 1 byte per address, it was set to 0 before which makes it think its reading a register type. Added offset of where it is supposed to read from. Before it was always reading from offset 0 despite specifying the offset in the higher level function.
Signed-off-by: Cooper Duffin cooperx.duffin@intel.com Signed-off-by: cduffinx cooperx.duffin@intel.com
drivers/i2c/designware_i2c.c | 5 +++-- drivers/i2c/i2c-uclass.c | 1 + include/i2c.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-)
Thanks for the patch!
Is there a test that was failing before (test/dm/i2c.c) or should we add a new one to catch this?
Regards, Simon
Hello Simon,
I wasn’t using the test/dm/i2c I this was tested using hardware where I was using the dm_i2c_read() function. I just tried to use the test/dm/i2c where I am following the README but I keep getting "sdl2-config: Command not found". I suspect it would also fail where it is trying to read but I will have to get test/dm/i2c working before I know for sure.
Essentially my test was
int uboot_app (int argc, char * const argv[]) { uint8_t buf[10]; uint8_t read_buf[5]; uint8_t dev_addr; uint32_t bus_speed; //i2c_bus device pointer struct udevice *i2c_led; struct udevice *bus; app_startup(argv); /* Print the ABI version */ printf ("Example expects ABI version %d\n", XF_VERSION); printf ("Actual U-Boot ABI version %d\n", (int)get_version());
dev_addr = 0x60; bus_speed = 100000; //100KHz printf("starting test i2c\n"); buf[0] = 'b'; buf[1] = 'o'; buf[2] = 'o'; buf[3] = 't'; //Get i2c chip, init the code printf("init starting\n"); if(i2c_get_chip_for_busnum(0 , 0x60, 1, &i2c_led)!=0){ printf("ERROR: no device found at %x \n",dev_addr); return (0); } uclass_get_device_by_seq(UCLASS_I2C, 0, &bus); printf("Setting bus speed to %d\n", bus_speed); if(dm_i2c_set_bus_speed(bus,bus_speed)!=0){ printf("ERROR: Cannot set buspeed\n"); return (0); } printf("i2c_led name is %s\n",i2c_led->name); if(i2c_led == NULL){ printf("ERROR i2c_led 0 is null\n"); return (0); } printf("Writing\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_write(i2c_led,a,&buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("Reading\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_read(i2c_led,a,&read_buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("read buffer is %c, %c, %c, %c \n",read_buf[0],read_buf[1], read_buf[2],read_buf[3]); printf ("\n\n"); return (0); }
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is o, o, o, o
with the fix I get:
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is b, o, o, t
which is correct. Similarly if I stop auto boot I get a similar result:
Hit any key to stop autoboot: 0 device # i2c i2c - I2C sub-system
Assuming I write "boot" to the device device # i2c dev 0 Setting bus to 0 device # i2c md 0x60 0 4 0000: 6f 6f 6f 6f oooo
With the fix/patch I get:
device# i2c md 0x60 0 4 0000: 62 6f 6f 74 boot
Which is correct. Also the reads were working in linux leading to my original suspicion that there might be something going on in the driver. Hopefully that answers your question let me know if you have anymore.
Best regards,
-Cooper Duffin -----Original Message----- From: Simon Glass sjg@chromium.org Sent: Friday, November 6, 2020 10:51 AM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi Cduffinx,
On Thu, 5 Nov 2020 at 14:26, cduffinx cooperx.duffin@intel.com wrote:
modify the designware_i2c_xfer function to use 1 byte per address, it was set to 0 before which makes it think its reading a register type. Added offset of where it is supposed to read from. Before it was always reading from offset 0 despite specifying the offset in the higher level function.
Signed-off-by: Cooper Duffin cooperx.duffin@intel.com Signed-off-by: cduffinx cooperx.duffin@intel.com
drivers/i2c/designware_i2c.c | 5 +++-- drivers/i2c/i2c-uclass.c | 1 + include/i2c.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-)
Thanks for the patch!
Is there a test that was failing before (test/dm/i2c.c) or should we add a new one to catch this?
Regards, Simon
Hi CooperX,
On Fri, 6 Nov 2020 at 16:08, Duffin, CooperX cooperx.duffin@intel.com wrote:
Hello Simon,
I wasn’t using the test/dm/i2c I this was tested using hardware where I was using the dm_i2c_read() function. I just tried to use the test/dm/i2c where I am following the README but I keep getting "sdl2-config: Command not found". I suspect it would also fail where it is trying to read but I will have to get test/dm/i2c working before I know for sure.
Essentially my test was
int uboot_app (int argc, char * const argv[]) { uint8_t buf[10]; uint8_t read_buf[5]; uint8_t dev_addr; uint32_t bus_speed; //i2c_bus device pointer struct udevice *i2c_led; struct udevice *bus; app_startup(argv); /* Print the ABI version */ printf ("Example expects ABI version %d\n", XF_VERSION); printf ("Actual U-Boot ABI version %d\n", (int)get_version());
dev_addr = 0x60; bus_speed = 100000; //100KHz printf("starting test i2c\n"); buf[0] = 'b'; buf[1] = 'o'; buf[2] = 'o'; buf[3] = 't'; //Get i2c chip, init the code printf("init starting\n"); if(i2c_get_chip_for_busnum(0 , 0x60, 1, &i2c_led)!=0){ printf("ERROR: no device found at %x \n",dev_addr); return (0); } uclass_get_device_by_seq(UCLASS_I2C, 0, &bus); printf("Setting bus speed to %d\n", bus_speed); if(dm_i2c_set_bus_speed(bus,bus_speed)!=0){ printf("ERROR: Cannot set buspeed\n"); return (0); } printf("i2c_led name is %s\n",i2c_led->name); if(i2c_led == NULL){ printf("ERROR i2c_led 0 is null\n"); return (0); } printf("Writing\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_write(i2c_led,a,&buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("Reading\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_read(i2c_led,a,&read_buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("read buffer is %c, %c, %c, %c \n",read_buf[0],read_buf[1], read_buf[2],read_buf[3]); printf ("\n\n"); return (0);}
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is o, o, o, o
with the fix I get:
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is b, o, o, t
which is correct. Similarly if I stop auto boot I get a similar result:
Hit any key to stop autoboot: 0 device # i2c i2c - I2C sub-system
Assuming I write "boot" to the device device # i2c dev 0 Setting bus to 0 device # i2c md 0x60 0 4 0000: 6f 6f 6f 6f oooo
With the fix/patch I get:
device# i2c md 0x60 0 4 0000: 62 6f 6f 74 boot
Which is correct. Also the reads were working in linux leading to my original suspicion that there might be something going on in the driver. Hopefully that answers your question let me know if you have anymore.
I had a bit of a look at this. If you look at dm_i2c_read() it actually builds the address into the buffer it sends. So when it gets to __dw_i2c_write() the alen parameter is always 0. That function is actually a holdover from before driver model, so one day the alen and addr parameters will go away.
I wonder if your device does not support multiple-byte reads or writes? Can you try the i2c md with a single byte? There is a DM_I2C_CHIP_WR_ADDRESS option to handle this - see the 'i2c flags' command.
In the designware_i2c_xfer(), try printing out the bytes that it gets in each message using i2c_dump_msgs().
Also check the i2c_get_chip() function which you are using. See if chip->offset_len is set to 1 as it should be, for your device. If not, perhaps something is missing.
In short I am not really sure what is going on, but I think it needs more investigation.
BTW with this mailing list we like people to reply inline or at the bottom, not at the top.
Regards, Simon
Best regards,
-Cooper Duffin -----Original Message----- From: Simon Glass sjg@chromium.org Sent: Friday, November 6, 2020 10:51 AM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi Cduffinx,
On Thu, 5 Nov 2020 at 14:26, cduffinx cooperx.duffin@intel.com wrote:
modify the designware_i2c_xfer function to use 1 byte per address, it was set to 0 before which makes it think its reading a register type. Added offset of where it is supposed to read from. Before it was always reading from offset 0 despite specifying the offset in the higher level function.
Signed-off-by: Cooper Duffin cooperx.duffin@intel.com Signed-off-by: cduffinx cooperx.duffin@intel.com
drivers/i2c/designware_i2c.c | 5 +++-- drivers/i2c/i2c-uclass.c | 1 + include/i2c.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-)
Thanks for the patch!
Is there a test that was failing before (test/dm/i2c.c) or should we add a new one to catch this?
Regards, Simon
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Saturday, November 7, 2020 1:33 PM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi CooperX,
On Fri, 6 Nov 2020 at 16:08, Duffin, CooperX cooperx.duffin@intel.com wrote:
Hello Simon,
I wasn’t using the test/dm/i2c I this was tested using hardware where I was using the dm_i2c_read() function. I just tried to use the test/dm/i2c where I am following the README but I keep getting "sdl2-config: Command not found". I suspect it would also fail where it is trying to read but I will have to get test/dm/i2c working before I know for sure.
Essentially my test was
int uboot_app (int argc, char * const argv[]) { uint8_t buf[10]; uint8_t read_buf[5]; uint8_t dev_addr; uint32_t bus_speed; //i2c_bus device pointer struct udevice *i2c_led; struct udevice *bus; app_startup(argv); /* Print the ABI version */ printf ("Example expects ABI version %d\n", XF_VERSION); printf ("Actual U-Boot ABI version %d\n", (int)get_version());
dev_addr = 0x60; bus_speed = 100000; //100KHz printf("starting test i2c\n"); buf[0] = 'b'; buf[1] = 'o'; buf[2] = 'o'; buf[3] = 't'; //Get i2c chip, init the code printf("init starting\n"); if(i2c_get_chip_for_busnum(0 , 0x60, 1, &i2c_led)!=0){ printf("ERROR: no device found at %x \n",dev_addr); return (0); } uclass_get_device_by_seq(UCLASS_I2C, 0, &bus); printf("Setting bus speed to %d\n", bus_speed); if(dm_i2c_set_bus_speed(bus,bus_speed)!=0){ printf("ERROR: Cannot set buspeed\n"); return (0); } printf("i2c_led name is %s\n",i2c_led->name); if(i2c_led == NULL){ printf("ERROR i2c_led 0 is null\n"); return (0); } printf("Writing\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_write(i2c_led,a,&buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("Reading\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_read(i2c_led,a,&read_buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("read buffer is %c, %c, %c, %c \n",read_buf[0],read_buf[1], read_buf[2],read_buf[3]); printf ("\n\n"); return (0);}
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is o, o, o, o
with the fix I get:
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is b, o, o, t
which is correct. Similarly if I stop auto boot I get a similar result:
Hit any key to stop autoboot: 0 device # i2c i2c - I2C sub-system
Assuming I write "boot" to the device device # i2c dev 0 Setting bus to 0 device # i2c md 0x60 0 4 0000: 6f 6f 6f 6f oooo
With the fix/patch I get:
device# i2c md 0x60 0 4 0000: 62 6f 6f 74 boot
Which is correct. Also the reads were working in linux leading to my original suspicion that there might be something going on in the driver. Hopefully that answers your question let me know if you have anymore.
I had a bit of a look at this. If you look at dm_i2c_read() it actually builds the address into the buffer it sends. So when it gets to __dw_i2c_write() the alen parameter is always 0. That function is actually a holdover from before driver model, so one day the alen and addr parameters will go away.
I wonder if your device does not support multiple-byte reads or writes? Can you try the i2c md with a single byte? There is a DM_I2C_CHIP_WR_ADDRESS option to handle this - see the 'i2c flags' command.
In the designware_i2c_xfer(), try printing out the bytes that it gets in each message using i2c_dump_msgs().
Also check the i2c_get_chip() function which you are using. See if chip->offset_len is set to 1 as it should be, for your device. If not, perhaps something is missing.
In short I am not really sure what is going on, but I think it needs more investigation.
BTW with this mailing list we like people to reply inline or at the bottom, not at the top.
Regards, Simon
Best regards,
-Cooper Duffin -----Original Message----- From: Simon Glass sjg@chromium.org Sent: Friday, November 6, 2020 10:51 AM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi Cduffinx,
On Thu, 5 Nov 2020 at 14:26, cduffinx cooperx.duffin@intel.com wrote:
modify the designware_i2c_xfer function to use 1 byte per address, it was set to 0 before which makes it think its reading a register type. Added offset of where it is supposed to read from. Before it was always reading from offset 0 despite specifying the offset in the higher level function.
Signed-off-by: Cooper Duffin cooperx.duffin@intel.com Signed-off-by: cduffinx cooperx.duffin@intel.com
drivers/i2c/designware_i2c.c | 5 +++-- drivers/i2c/i2c-uclass.c | 1 + include/i2c.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-)
Thanks for the patch!
Is there a test that was failing before (test/dm/i2c.c) or should we add a new one to catch this?
Regards, Simon
[Duffin, CooperX] Thanks for the reply Simon. I tried a couple more things.
Simon Glass sjg@chromium.org wrote:
I wonder if your device does not support multiple-byte reads or writes? Can you try the i2c md with a single byte? There is a DM_I2C_CHIP_WR_ADDRESS option to handle this - see the 'i2c flags' command.
I tried with the flag DM_I2C_CHIP_WR_ADDRESS but it did not make a difference. A single byte read will yield the following:
Device # i2c md 0x60 0 1 0000: 6f o
Which is not correct in fact it doesn’t seem to matter how much I read it only picks up the second character I wrote to it. For example if I read 16 characters I get: device# i2c flags 0x60 4 device # i2c md 0x60 0 10 0000: 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f oooooooooooooooo
I can tell the writes are working because I am writing to an LED display and I can see the proper characters get displayed. Also I think my i2c device needs restarts in-between each byte for it to work properly, so I need to be able to read individual bytes with there corresponding offsets, also i2c-tools supports this.
Simon Glass sjg@chromium.org wrote:
Also check the i2c_get_chip() function which you are using. See if chip->offset_len is set to 1 as it should be, for your device. If not, perhaps something is missing.
If I use this command I get:
device # i2c olen 0x60 1
This is without the patch BTW.
+Heiko Schocher who might know more about this I2C question
On Mon, 9 Nov 2020 at 15:09, Duffin, CooperX cooperx.duffin@intel.com wrote:
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Saturday, November 7, 2020 1:33 PM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi CooperX,
On Fri, 6 Nov 2020 at 16:08, Duffin, CooperX cooperx.duffin@intel.com wrote:
Hello Simon,
I wasn’t using the test/dm/i2c I this was tested using hardware where I was using the dm_i2c_read() function. I just tried to use the test/dm/i2c where I am following the README but I keep getting "sdl2-config: Command not found". I suspect it would also fail where it is trying to read but I will have to get test/dm/i2c working before I know for sure.
Essentially my test was
int uboot_app (int argc, char * const argv[]) { uint8_t buf[10]; uint8_t read_buf[5]; uint8_t dev_addr; uint32_t bus_speed; //i2c_bus device pointer struct udevice *i2c_led; struct udevice *bus; app_startup(argv); /* Print the ABI version */ printf ("Example expects ABI version %d\n", XF_VERSION); printf ("Actual U-Boot ABI version %d\n", (int)get_version());
dev_addr = 0x60; bus_speed = 100000; //100KHz printf("starting test i2c\n"); buf[0] = 'b'; buf[1] = 'o'; buf[2] = 'o'; buf[3] = 't'; //Get i2c chip, init the code printf("init starting\n"); if(i2c_get_chip_for_busnum(0 , 0x60, 1, &i2c_led)!=0){ printf("ERROR: no device found at %x \n",dev_addr); return (0); } uclass_get_device_by_seq(UCLASS_I2C, 0, &bus); printf("Setting bus speed to %d\n", bus_speed); if(dm_i2c_set_bus_speed(bus,bus_speed)!=0){ printf("ERROR: Cannot set buspeed\n"); return (0); } printf("i2c_led name is %s\n",i2c_led->name); if(i2c_led == NULL){ printf("ERROR i2c_led 0 is null\n"); return (0); } printf("Writing\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_write(i2c_led,a,&buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("Reading\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_read(i2c_led,a,&read_buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("read buffer is %c, %c, %c, %c \n",read_buf[0],read_buf[1], read_buf[2],read_buf[3]); printf ("\n\n"); return (0);}
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is o, o, o, o
with the fix I get:
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is b, o, o, t
which is correct. Similarly if I stop auto boot I get a similar result:
Hit any key to stop autoboot: 0 device # i2c i2c - I2C sub-system
Assuming I write "boot" to the device device # i2c dev 0 Setting bus to 0 device # i2c md 0x60 0 4 0000: 6f 6f 6f 6f oooo
With the fix/patch I get:
device# i2c md 0x60 0 4 0000: 62 6f 6f 74 boot
Which is correct. Also the reads were working in linux leading to my original suspicion that there might be something going on in the driver. Hopefully that answers your question let me know if you have anymore.
I had a bit of a look at this. If you look at dm_i2c_read() it actually builds the address into the buffer it sends. So when it gets to __dw_i2c_write() the alen parameter is always 0. That function is actually a holdover from before driver model, so one day the alen and addr parameters will go away.
I wonder if your device does not support multiple-byte reads or writes? Can you try the i2c md with a single byte? There is a DM_I2C_CHIP_WR_ADDRESS option to handle this - see the 'i2c flags' command.
In the designware_i2c_xfer(), try printing out the bytes that it gets in each message using i2c_dump_msgs().
Also check the i2c_get_chip() function which you are using. See if chip->offset_len is set to 1 as it should be, for your device. If not, perhaps something is missing.
In short I am not really sure what is going on, but I think it needs more investigation.
BTW with this mailing list we like people to reply inline or at the bottom, not at the top.
Regards, Simon
Best regards,
-Cooper Duffin -----Original Message----- From: Simon Glass sjg@chromium.org Sent: Friday, November 6, 2020 10:51 AM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi Cduffinx,
On Thu, 5 Nov 2020 at 14:26, cduffinx cooperx.duffin@intel.com wrote:
modify the designware_i2c_xfer function to use 1 byte per address, it was set to 0 before which makes it think its reading a register type. Added offset of where it is supposed to read from. Before it was always reading from offset 0 despite specifying the offset in the higher level function.
Signed-off-by: Cooper Duffin cooperx.duffin@intel.com Signed-off-by: cduffinx cooperx.duffin@intel.com
drivers/i2c/designware_i2c.c | 5 +++-- drivers/i2c/i2c-uclass.c | 1 + include/i2c.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-)
Thanks for the patch!
Is there a test that was failing before (test/dm/i2c.c) or should we add a new one to catch this?
Regards, Simon
[Duffin, CooperX] Thanks for the reply Simon. I tried a couple more things.
Simon Glass sjg@chromium.org wrote:
I wonder if your device does not support multiple-byte reads or writes? Can you try the i2c md with a single byte? There is a DM_I2C_CHIP_WR_ADDRESS option to handle this - see the 'i2c flags' command.
I tried with the flag DM_I2C_CHIP_WR_ADDRESS but it did not make a difference. A single byte read will yield the following:
Device # i2c md 0x60 0 1 0000: 6f o
Which is not correct in fact it doesn’t seem to matter how much I read it only picks up the second character I wrote to it. For example if I read 16 characters I get: device# i2c flags 0x60 4 device # i2c md 0x60 0 10 0000: 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f oooooooooooooooo
I can tell the writes are working because I am writing to an LED display and I can see the proper characters get displayed. Also I think my i2c device needs restarts in-between each byte for it to work properly, so I need to be able to read individual bytes with there corresponding offsets, also i2c-tools supports this.
Simon Glass sjg@chromium.org wrote:
Also check the i2c_get_chip() function which you are using. See if chip->offset_len is set to 1 as it should be, for your device. If not, perhaps something is missing.
If I use this command I get:
device # i2c olen 0x60 1
This is without the patch BTW.
Hello Simon,
Am 17.11.2020 um 00:20 schrieb Simon Glass:
+Heiko Schocher who might know more about this I2C question
Sorry for late response ...
On Mon, 9 Nov 2020 at 15:09, Duffin, CooperX cooperx.duffin@intel.com wrote:
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Saturday, November 7, 2020 1:33 PM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi CooperX,
On Fri, 6 Nov 2020 at 16:08, Duffin, CooperX cooperx.duffin@intel.com wrote:
Hello Simon,
I wasn’t using the test/dm/i2c I this was tested using hardware where I was using the dm_i2c_read() function. I just tried to use the test/dm/i2c where I am following the README but I keep getting "sdl2-config: Command not found". I suspect it would also fail where it is trying to read but I will have to get test/dm/i2c working before I know for sure.
Essentially my test was
int uboot_app (int argc, char * const argv[]) { uint8_t buf[10]; uint8_t read_buf[5]; uint8_t dev_addr; uint32_t bus_speed; //i2c_bus device pointer struct udevice *i2c_led; struct udevice *bus; app_startup(argv); /* Print the ABI version */ printf ("Example expects ABI version %d\n", XF_VERSION); printf ("Actual U-Boot ABI version %d\n", (int)get_version());
dev_addr = 0x60; bus_speed = 100000; //100KHz printf("starting test i2c\n"); buf[0] = 'b'; buf[1] = 'o'; buf[2] = 'o'; buf[3] = 't'; //Get i2c chip, init the code printf("init starting\n"); if(i2c_get_chip_for_busnum(0 , 0x60, 1, &i2c_led)!=0){ printf("ERROR: no device found at %x \n",dev_addr); return (0); } uclass_get_device_by_seq(UCLASS_I2C, 0, &bus); printf("Setting bus speed to %d\n", bus_speed); if(dm_i2c_set_bus_speed(bus,bus_speed)!=0){ printf("ERROR: Cannot set buspeed\n"); return (0); } printf("i2c_led name is %s\n",i2c_led->name); if(i2c_led == NULL){ printf("ERROR i2c_led 0 is null\n"); return (0); } printf("Writing\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_write(i2c_led,a,&buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("Reading\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_read(i2c_led,a,&read_buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("read buffer is %c, %c, %c, %c \n",read_buf[0],read_buf[1], read_buf[2],read_buf[3]); printf ("\n\n"); return (0);}
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is o, o, o, o
with the fix I get:
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is b, o, o, t
which is correct. Similarly if I stop auto boot I get a similar result:
Hit any key to stop autoboot: 0 device # i2c i2c - I2C sub-system
Assuming I write "boot" to the device device # i2c dev 0 Setting bus to 0 device # i2c md 0x60 0 4 0000: 6f 6f 6f 6f oooo
With the fix/patch I get:
device# i2c md 0x60 0 4 0000: 62 6f 6f 74 boot
Which is correct. Also the reads were working in linux leading to my original suspicion that there might be something going on in the driver. Hopefully that answers your question let me know if you have anymore.
I had a bit of a look at this. If you look at dm_i2c_read() it actually builds the address into the buffer it sends. So when it gets to __dw_i2c_write() the alen parameter is always 0. That function is actually a holdover from before driver model, so one day the alen and addr parameters will go away.
I wonder if your device does not support multiple-byte reads or writes? Can you try the i2c md with a single byte? There is a DM_I2C_CHIP_WR_ADDRESS option to handle this - see the 'i2c flags' command.
In the designware_i2c_xfer(), try printing out the bytes that it gets in each message using i2c_dump_msgs().
Also check the i2c_get_chip() function which you are using. See if chip->offset_len is set to 1 as it should be, for your device. If not, perhaps something is missing.
In short I am not really sure what is going on, but I think it needs more investigation.
BTW with this mailing list we like people to reply inline or at the bottom, not at the top.
Regards, Simon
Best regards,
-Cooper Duffin -----Original Message----- From: Simon Glass sjg@chromium.org Sent: Friday, November 6, 2020 10:51 AM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi Cduffinx,
On Thu, 5 Nov 2020 at 14:26, cduffinx cooperx.duffin@intel.com wrote:
modify the designware_i2c_xfer function to use 1 byte per address, it was set to 0 before which makes it think its reading a register type. Added offset of where it is supposed to read from. Before it was always reading from offset 0 despite specifying the offset in the higher level function.
Signed-off-by: Cooper Duffin cooperx.duffin@intel.com Signed-off-by: cduffinx cooperx.duffin@intel.com
drivers/i2c/designware_i2c.c | 5 +++-- drivers/i2c/i2c-uclass.c | 1 + include/i2c.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-)
Thanks for the patch!
Is there a test that was failing before (test/dm/i2c.c) or should we add a new one to catch this?
Regards, Simon
[Duffin, CooperX] Thanks for the reply Simon. I tried a couple more things.
Simon Glass sjg@chromium.org wrote:
I wonder if your device does not support multiple-byte reads or writes? Can you try the i2c md with a single byte? There is a DM_I2C_CHIP_WR_ADDRESS option to handle this - see the 'i2c flags' command.
I tried with the flag DM_I2C_CHIP_WR_ADDRESS but it did not make a difference. A single byte read will yield the following:
Device # i2c md 0x60 0 1 0000: 6f o
Which is not correct in fact it doesn’t seem to matter how much I read it only picks up the second character I wrote to it. For example if I read 16 characters I get: device# i2c flags 0x60 4 device # i2c md 0x60 0 10 0000: 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f oooooooooooooooo
I can tell the writes are working because I am writing to an LED display and I can see the proper characters get displayed. Also I think my i2c device needs restarts in-between each byte for it to work properly, so I need to be able to read individual bytes with there corresponding offsets, also i2c-tools supports this.
Simon Glass sjg@chromium.org wrote:
Also check the i2c_get_chip() function which you are using. See if chip->offset_len is set to 1 as it should be, for your device. If not, perhaps something is missing.
If I use this command I get:
device # i2c olen 0x60 1
This is without the patch BTW.
Hard to say here anything usefull, without the chance to try out...
trying with an i2c epprom (imx6 based board)
=> i2c md 0x58 0 10 0000: 01 00 04 23 00 02 b1 61 00 23 00 10 00 00 03 09 ...#...a.#...... => i2c md 0x58 0 1 0000: 01 . => i2c md 0x58 1 1 0001: 00 . => i2c md 0x58 2 1 0002: 04 . => i2c md 0x58 3 1 0003: 23 # =>
=> i2c flags 0x58 0 => i2c olen 0x58 1 =>
May you can enable debug in drivers/i2c/designware_i2c.c driver?
So we should see, what sort of messages designware_i2c_xfer() gets ... and may have an idea... and may add
717 debug("i2c_xfer: chip=0x%x, len=0x%x\n", msg->addr, msg->len);
also a dump of msg->buf and msg->len.
bye, Heiko
-----Original Message----- From: Heiko Schocher hs@denx.de Sent: Tuesday, November 17, 2020 11:42 PM To: Duffin, CooperX cooperx.duffin@intel.com Cc: Simon Glass sjg@chromium.org; U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hello Simon,
Am 17.11.2020 um 00:20 schrieb Simon Glass:
+Heiko Schocher who might know more about this I2C question
Sorry for late response ...
On Mon, 9 Nov 2020 at 15:09, Duffin, CooperX cooperx.duffin@intel.com wrote:
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Saturday, November 7, 2020 1:33 PM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi CooperX,
On Fri, 6 Nov 2020 at 16:08, Duffin, CooperX cooperx.duffin@intel.com wrote:
Hello Simon,
I wasn’t using the test/dm/i2c I this was tested using hardware where I was using the dm_i2c_read() function. I just tried to use the test/dm/i2c where I am following the README but I keep getting "sdl2-config: Command not found". I suspect it would also fail where it is trying to read but I will have to get test/dm/i2c working before I know for sure.
Essentially my test was
int uboot_app (int argc, char * const argv[]) { uint8_t buf[10]; uint8_t read_buf[5]; uint8_t dev_addr; uint32_t bus_speed; //i2c_bus device pointer struct udevice *i2c_led; struct udevice *bus; app_startup(argv); /* Print the ABI version */ printf ("Example expects ABI version %d\n", XF_VERSION); printf ("Actual U-Boot ABI version %d\n", (int)get_version());
dev_addr = 0x60; bus_speed = 100000; //100KHz printf("starting test i2c\n"); buf[0] = 'b'; buf[1] = 'o'; buf[2] = 'o'; buf[3] = 't'; //Get i2c chip, init the code printf("init starting\n"); if(i2c_get_chip_for_busnum(0 , 0x60, 1, &i2c_led)!=0){ printf("ERROR: no device found at %x \n",dev_addr); return (0); } uclass_get_device_by_seq(UCLASS_I2C, 0, &bus); printf("Setting bus speed to %d\n", bus_speed); if(dm_i2c_set_bus_speed(bus,bus_speed)!=0){ printf("ERROR: Cannot set buspeed\n"); return (0); } printf("i2c_led name is %s\n",i2c_led->name); if(i2c_led == NULL){ printf("ERROR i2c_led 0 is null\n"); return (0); } printf("Writing\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_write(i2c_led,a,&buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("Reading\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_read(i2c_led,a,&read_buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("read buffer is %c, %c, %c, %c \n",read_buf[0],read_buf[1], read_buf[2],read_buf[3]); printf ("\n\n"); return (0);}
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is o, o, o, o
with the fix I get:
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is b, o, o, t
which is correct. Similarly if I stop auto boot I get a similar result:
Hit any key to stop autoboot: 0 device # i2c i2c - I2C sub-system
Assuming I write "boot" to the device device # i2c dev 0 Setting bus to 0 device # i2c md 0x60 0 4 0000: 6f 6f 6f 6f oooo
With the fix/patch I get:
device# i2c md 0x60 0 4 0000: 62 6f 6f 74 boot
Which is correct. Also the reads were working in linux leading to my original suspicion that there might be something going on in the driver. Hopefully that answers your question let me know if you have anymore.
I had a bit of a look at this. If you look at dm_i2c_read() it actually builds the address into the buffer it sends. So when it gets to __dw_i2c_write() the alen parameter is always 0. That function is actually a holdover from before driver model, so one day the alen and addr parameters will go away.
I wonder if your device does not support multiple-byte reads or writes? Can you try the i2c md with a single byte? There is a DM_I2C_CHIP_WR_ADDRESS option to handle this - see the 'i2c flags' command.
In the designware_i2c_xfer(), try printing out the bytes that it gets in each message using i2c_dump_msgs().
Also check the i2c_get_chip() function which you are using. See if chip->offset_len is set to 1 as it should be, for your device. If chip->not, perhaps something is missing.
In short I am not really sure what is going on, but I think it needs more investigation.
BTW with this mailing list we like people to reply inline or at the bottom, not at the top.
Regards, Simon
Best regards,
-Cooper Duffin -----Original Message----- From: Simon Glass sjg@chromium.org Sent: Friday, November 6, 2020 10:51 AM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi Cduffinx,
On Thu, 5 Nov 2020 at 14:26, cduffinx cooperx.duffin@intel.com wrote:
modify the designware_i2c_xfer function to use 1 byte per address, it was set to 0 before which makes it think its reading a register type. Added offset of where it is supposed to read from. Before it was always reading from offset 0 despite specifying the offset in the higher level function.
Signed-off-by: Cooper Duffin cooperx.duffin@intel.com Signed-off-by: cduffinx cooperx.duffin@intel.com
drivers/i2c/designware_i2c.c | 5 +++-- drivers/i2c/i2c-uclass.c | 1 + include/i2c.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-)
Thanks for the patch!
Is there a test that was failing before (test/dm/i2c.c) or should we add a new one to catch this?
Regards, Simon
[Duffin, CooperX] Thanks for the reply Simon. I tried a couple more things.
Simon Glass sjg@chromium.org wrote:
I wonder if your device does not support multiple-byte reads or writes? Can you try the i2c md with a single byte? There is a DM_I2C_CHIP_WR_ADDRESS option to handle this - see the 'i2c flags' command.
I tried with the flag DM_I2C_CHIP_WR_ADDRESS but it did not make a difference. A single byte read will yield the following:
Device # i2c md 0x60 0 1 0000: 6f o
Which is not correct in fact it doesn’t seem to matter how much I read it only picks up the second character I wrote to it. For example if I read 16 characters I get: device# i2c flags 0x60 4 device # i2c md 0x60 0 10 0000: 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f oooooooooooooooo
I can tell the writes are working because I am writing to an LED display and I can see the proper characters get displayed. Also I think my i2c device needs restarts in-between each byte for it to work properly, so I need to be able to read individual bytes with there corresponding offsets, also i2c-tools supports this.
Simon Glass sjg@chromium.org wrote:
Also check the i2c_get_chip() function which you are using. See if chip->offset_len is set to 1 as it should be, for your device. If chip->not, perhaps something is missing.
If I use this command I get:
device # i2c olen 0x60 1
This is without the patch BTW.
Hard to say here anything usefull, without the chance to try out...
trying with an i2c epprom (imx6 based board)
=> i2c md 0x58 0 10 0000: 01 00 04 23 00 02 b1 61 00 23 00 10 00 00 03 09 ...#...a.#...... => i2c md 0x58 0 1 0000: 01 . => i2c md 0x58 1 1 0001: 00 . => i2c md 0x58 2 1 0002: 04 . => i2c md 0x58 3 1 0003: 23 # =>
=> i2c flags 0x58 0 => i2c olen 0x58 1 =>
May you can enable debug in drivers/i2c/designware_i2c.c driver?
So we should see, what sort of messages designware_i2c_xfer() gets ... and may have an idea... and may add
717 debug("i2c_xfer: chip=0x%x, len=0x%x\n", msg->addr, msg->len);
also a dump of msg->buf and msg->len.
bye, Heiko
Hello Heiko,
I hope you are doing well, just curious if you have had a chance to look into my latest response?
Regards,
-Cooper Duffin
Hello Heiko,
I hope you are doing well, just curious if you have had a chance to look into my latest response?
Regards,
-Cooper Duffin
-----Original Message----- From: Duffin, CooperX Sent: Wednesday, December 2, 2020 1:53 PM To: hs@denx.de Cc: Simon Glass sjg@chromium.org; U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: RE: [dwi2c PATCH v1] dwi2c add offsets to reads
-----Original Message----- From: Heiko Schocher hs@denx.de Sent: Tuesday, November 17, 2020 11:42 PM To: Duffin, CooperX cooperx.duffin@intel.com Cc: Simon Glass sjg@chromium.org; U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hello Simon,
Am 17.11.2020 um 00:20 schrieb Simon Glass:
+Heiko Schocher who might know more about this I2C question
Sorry for late response ...
On Mon, 9 Nov 2020 at 15:09, Duffin, CooperX cooperx.duffin@intel.com wrote:
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Saturday, November 7, 2020 1:33 PM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi CooperX,
On Fri, 6 Nov 2020 at 16:08, Duffin, CooperX cooperx.duffin@intel.com wrote:
Hello Simon,
I wasn’t using the test/dm/i2c I this was tested using hardware where I was using the dm_i2c_read() function. I just tried to use the test/dm/i2c where I am following the README but I keep getting "sdl2-config: Command not found". I suspect it would also fail where it is trying to read but I will have to get test/dm/i2c working before I know for sure.
Essentially my test was
int uboot_app (int argc, char * const argv[]) { uint8_t buf[10]; uint8_t read_buf[5]; uint8_t dev_addr; uint32_t bus_speed; //i2c_bus device pointer struct udevice *i2c_led; struct udevice *bus; app_startup(argv); /* Print the ABI version */ printf ("Example expects ABI version %d\n", XF_VERSION); printf ("Actual U-Boot ABI version %d\n", (int)get_version());
dev_addr = 0x60; bus_speed = 100000; //100KHz printf("starting test i2c\n"); buf[0] = 'b'; buf[1] = 'o'; buf[2] = 'o'; buf[3] = 't'; //Get i2c chip, init the code printf("init starting\n"); if(i2c_get_chip_for_busnum(0 , 0x60, 1, &i2c_led)!=0){ printf("ERROR: no device found at %x \n",dev_addr); return (0); } uclass_get_device_by_seq(UCLASS_I2C, 0, &bus); printf("Setting bus speed to %d\n", bus_speed); if(dm_i2c_set_bus_speed(bus,bus_speed)!=0){ printf("ERROR: Cannot set buspeed\n"); return (0); } printf("i2c_led name is %s\n",i2c_led->name); if(i2c_led == NULL){ printf("ERROR i2c_led 0 is null\n"); return (0); } printf("Writing\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_write(i2c_led,a,&buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("Reading\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_read(i2c_led,a,&read_buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("read buffer is %c, %c, %c, %c \n",read_buf[0],read_buf[1], read_buf[2],read_buf[3]); printf ("\n\n"); return (0);}
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is o, o, o, o
with the fix I get:
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is b, o, o, t
which is correct. Similarly if I stop auto boot I get a similar result:
Hit any key to stop autoboot: 0 device # i2c i2c - I2C sub-system
Assuming I write "boot" to the device device # i2c dev 0 Setting bus to 0 device # i2c md 0x60 0 4 0000: 6f 6f 6f 6f oooo
With the fix/patch I get:
device# i2c md 0x60 0 4 0000: 62 6f 6f 74 boot
Which is correct. Also the reads were working in linux leading to my original suspicion that there might be something going on in the driver. Hopefully that answers your question let me know if you have anymore.
I had a bit of a look at this. If you look at dm_i2c_read() it actually builds the address into the buffer it sends. So when it gets to __dw_i2c_write() the alen parameter is always 0. That function is actually a holdover from before driver model, so one day the alen and addr parameters will go away.
I wonder if your device does not support multiple-byte reads or writes? Can you try the i2c md with a single byte? There is a DM_I2C_CHIP_WR_ADDRESS option to handle this - see the 'i2c flags' command.
In the designware_i2c_xfer(), try printing out the bytes that it gets in each message using i2c_dump_msgs().
Also check the i2c_get_chip() function which you are using. See if chip->offset_len is set to 1 as it should be, for your device. If chip->not, perhaps something is missing.
In short I am not really sure what is going on, but I think it needs more investigation.
BTW with this mailing list we like people to reply inline or at the bottom, not at the top.
Regards, Simon
Best regards,
-Cooper Duffin -----Original Message----- From: Simon Glass sjg@chromium.org Sent: Friday, November 6, 2020 10:51 AM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi Cduffinx,
On Thu, 5 Nov 2020 at 14:26, cduffinx cooperx.duffin@intel.com wrote:
modify the designware_i2c_xfer function to use 1 byte per address, it was set to 0 before which makes it think its reading a register type. Added offset of where it is supposed to read from. Before it was always reading from offset 0 despite specifying the offset in the higher level function.
Signed-off-by: Cooper Duffin cooperx.duffin@intel.com Signed-off-by: cduffinx cooperx.duffin@intel.com
drivers/i2c/designware_i2c.c | 5 +++-- drivers/i2c/i2c-uclass.c | 1 + include/i2c.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-)
Thanks for the patch!
Is there a test that was failing before (test/dm/i2c.c) or should we add a new one to catch this?
Regards, Simon
[Duffin, CooperX] Thanks for the reply Simon. I tried a couple more things.
Simon Glass sjg@chromium.org wrote:
I wonder if your device does not support multiple-byte reads or writes? Can you try the i2c md with a single byte? There is a DM_I2C_CHIP_WR_ADDRESS option to handle this - see the 'i2c flags' command.
I tried with the flag DM_I2C_CHIP_WR_ADDRESS but it did not make a difference. A single byte read will yield the following:
Device # i2c md 0x60 0 1 0000: 6f o
Which is not correct in fact it doesn’t seem to matter how much I read it only picks up the second character I wrote to it. For example if I read 16 characters I get: device# i2c flags 0x60 4 device # i2c md 0x60 0 10 0000: 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f oooooooooooooooo
I can tell the writes are working because I am writing to an LED display and I can see the proper characters get displayed. Also I think my i2c device needs restarts in-between each byte for it to work properly, so I need to be able to read individual bytes with there corresponding offsets, also i2c-tools supports this.
Simon Glass sjg@chromium.org wrote:
Also check the i2c_get_chip() function which you are using. See if chip->offset_len is set to 1 as it should be, for your device. If chip->not, perhaps something is missing.
If I use this command I get:
device # i2c olen 0x60 1
This is without the patch BTW.
Hard to say here anything usefull, without the chance to try out...
trying with an i2c epprom (imx6 based board)
=> i2c md 0x58 0 10 0000: 01 00 04 23 00 02 b1 61 00 23 00 10 00 00 03 09 ...#...a.#...... => i2c md 0x58 0 1 0000: 01 . => i2c md 0x58 1 1 0001: 00 . => i2c md 0x58 2 1 0002: 04 . => i2c md 0x58 3 1 0003: 23 # =>
=> i2c flags 0x58 0 => i2c olen 0x58 1 =>
May you can enable debug in drivers/i2c/designware_i2c.c driver?
So we should see, what sort of messages designware_i2c_xfer() gets ... and may have an idea... and may add
717 debug("i2c_xfer: chip=0x%x, len=0x%x\n", msg->addr, msg->len);
also a dump of msg->buf and msg->len.
bye, Heiko -- 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@denx.de
[Duffin, CooperX] Sorry About the late reply
After changing the debug to printf and adding the msg->buf I get the following:
Hit any key to stop autoboot: 0 device # i2c dev 0 Setting bus to 0 I2C bus i2c@ffc02900 version 0x3230302a device # i2c md 0x60 0 4 i2c_xfer: 2 messages i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2d398 i2c_xfer: chip=0x60, len=0x4, buf=0x7fa2d460 0000: 6f 6f 6f 6f oooo
Additionally if I run my standalone application I get this
I2C bus i2c@ffc02900 version 0x3230302a Setting bus speed to 100000 i2c_led name is generic_60 Writing i2c_xfer: 1 messages i2c_xfer: chip=0x60, len=0x2, buf=0x7fa2ced0 i2c_xfer: 1 messages i2c_xfer: chip=0x60, len=0x2, buf=0x7fa2ced0 i2c_xfer: 1 messages i2c_xfer: chip=0x60, len=0x2, buf=0x7fa2ced0 i2c_xfer: 1 messages i2c_xfer: chip=0x60, len=0x2, buf=0x7fa2ced0 Reading i2c_xfer: 2 messages i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf28 i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf88 i2c_xfer: 2 messages i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf28 i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf89 i2c_xfer: 2 messages i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf28 i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf8a i2c_xfer: 2 messages i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf28 i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf8b read buffer is o, o, o, o
Hello Duffin,
Am 14.12.20 um 18:12 schrieb Duffin, CooperX:
Hello Heiko,
I hope you are doing well, just curious if you have had a chance to look into my latest response?
Yep, thanks all fine, just to much workload ... and seems I forgot to response....
Regards,
-Cooper Duffin
Hello Heiko,
I hope you are doing well, just curious if you have had a chance to look into my latest response?
Regards,
-Cooper Duffin
-----Original Message----- From: Duffin, CooperX Sent: Wednesday, December 2, 2020 1:53 PM To: hs@denx.de Cc: Simon Glass sjg@chromium.org; U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: RE: [dwi2c PATCH v1] dwi2c add offsets to reads
-----Original Message----- From: Heiko Schocher hs@denx.de Sent: Tuesday, November 17, 2020 11:42 PM To: Duffin, CooperX cooperx.duffin@intel.com Cc: Simon Glass sjg@chromium.org; U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hello Simon,
Am 17.11.2020 um 00:20 schrieb Simon Glass:
+Heiko Schocher who might know more about this I2C question
Sorry for late response ...
On Mon, 9 Nov 2020 at 15:09, Duffin, CooperX cooperx.duffin@intel.com wrote:
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Saturday, November 7, 2020 1:33 PM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi CooperX,
On Fri, 6 Nov 2020 at 16:08, Duffin, CooperX cooperx.duffin@intel.com wrote:
Hello Simon,
I wasn’t using the test/dm/i2c I this was tested using hardware where I was using the dm_i2c_read() function. I just tried to use the test/dm/i2c where I am following the README but I keep getting "sdl2-config: Command not found". I suspect it would also fail where it is trying to read but I will have to get test/dm/i2c working before I know for sure.
Essentially my test was
int uboot_app (int argc, char * const argv[]) { uint8_t buf[10]; uint8_t read_buf[5]; uint8_t dev_addr; uint32_t bus_speed; //i2c_bus device pointer struct udevice *i2c_led; struct udevice *bus; app_startup(argv); /* Print the ABI version */ printf ("Example expects ABI version %d\n", XF_VERSION); printf ("Actual U-Boot ABI version %d\n", (int)get_version());
dev_addr = 0x60; bus_speed = 100000; //100KHz printf("starting test i2c\n"); buf[0] = 'b'; buf[1] = 'o'; buf[2] = 'o'; buf[3] = 't'; //Get i2c chip, init the code printf("init starting\n"); if(i2c_get_chip_for_busnum(0 , 0x60, 1, &i2c_led)!=0){ printf("ERROR: no device found at %x \n",dev_addr); return (0); } uclass_get_device_by_seq(UCLASS_I2C, 0, &bus); printf("Setting bus speed to %d\n", bus_speed); if(dm_i2c_set_bus_speed(bus,bus_speed)!=0){ printf("ERROR: Cannot set buspeed\n"); return (0); } printf("i2c_led name is %s\n",i2c_led->name); if(i2c_led == NULL){ printf("ERROR i2c_led 0 is null\n"); return (0); } printf("Writing\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_write(i2c_led,a,&buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("Reading\n"); for(unsigned int a =0; a< 4; a++){ if(dm_i2c_read(i2c_led,a,&read_buf[a],1)!=0){ printf("ERROR writing\n"); return (0); } } printf("read buffer is %c, %c, %c, %c \n",read_buf[0],read_buf[1], read_buf[2],read_buf[3]); printf ("\n\n"); return (0);}
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is o, o, o, o
with the fix I get:
## Starting application at 0x0C100000 ... Example expects ABI version 10 Actual U-Boot ABI version 10 starting test i2c init starting Setting bus speed to 100000 i2c_led name is generic_60 Writing Reading read buffer is b, o, o, t
which is correct. Similarly if I stop auto boot I get a similar result:
Hit any key to stop autoboot: 0 device # i2c i2c - I2C sub-system
Assuming I write "boot" to the device device # i2c dev 0 Setting bus to 0 device # i2c md 0x60 0 4 0000: 6f 6f 6f 6f oooo
With the fix/patch I get:
device# i2c md 0x60 0 4 0000: 62 6f 6f 74 boot
Which is correct. Also the reads were working in linux leading to my original suspicion that there might be something going on in the driver. Hopefully that answers your question let me know if you have anymore.
I had a bit of a look at this. If you look at dm_i2c_read() it actually builds the address into the buffer it sends. So when it gets to __dw_i2c_write() the alen parameter is always 0. That function is actually a holdover from before driver model, so one day the alen and addr parameters will go away.
I wonder if your device does not support multiple-byte reads or writes? Can you try the i2c md with a single byte? There is a DM_I2C_CHIP_WR_ADDRESS option to handle this - see the 'i2c flags' command.
In the designware_i2c_xfer(), try printing out the bytes that it gets in each message using i2c_dump_msgs().
Also check the i2c_get_chip() function which you are using. See if chip->offset_len is set to 1 as it should be, for your device. If chip->not, perhaps something is missing.
In short I am not really sure what is going on, but I think it needs more investigation.
BTW with this mailing list we like people to reply inline or at the bottom, not at the top.
Regards, Simon
Best regards,
-Cooper Duffin -----Original Message----- From: Simon Glass sjg@chromium.org Sent: Friday, November 6, 2020 10:51 AM To: Duffin, CooperX cooperx.duffin@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; uboot-snps-arc@synopsys.com; Tom Rini trini@konsulko.com; Robert Beckett bob.beckett@collabora.com; Heiko Schocher hs@denx.de; Wolgang Denk wd@denx.de; Ian Ray ian.ray@ge.com Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
Hi Cduffinx,
On Thu, 5 Nov 2020 at 14:26, cduffinx cooperx.duffin@intel.com wrote:
modify the designware_i2c_xfer function to use 1 byte per address, it was set to 0 before which makes it think its reading a register type. Added offset of where it is supposed to read from. Before it was always reading from offset 0 despite specifying the offset in the higher level function.
Signed-off-by: Cooper Duffin cooperx.duffin@intel.com Signed-off-by: cduffinx cooperx.duffin@intel.com
drivers/i2c/designware_i2c.c | 5 +++-- drivers/i2c/i2c-uclass.c | 1 + include/i2c.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-)
Thanks for the patch!
Is there a test that was failing before (test/dm/i2c.c) or should we add a new one to catch this?
Regards, Simon
[Duffin, CooperX] Thanks for the reply Simon. I tried a couple more things.
Simon Glass sjg@chromium.org wrote:
I wonder if your device does not support multiple-byte reads or writes? Can you try the i2c md with a single byte? There is a DM_I2C_CHIP_WR_ADDRESS option to handle this - see the 'i2c flags' command.
I tried with the flag DM_I2C_CHIP_WR_ADDRESS but it did not make a difference. A single byte read will yield the following:
Device # i2c md 0x60 0 1 0000: 6f o
Which is not correct in fact it doesn’t seem to matter how much I read it only picks up the second character I wrote to it. For example if I read 16 characters I get: device# i2c flags 0x60 4 device # i2c md 0x60 0 10 0000: 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f 6f oooooooooooooooo
I can tell the writes are working because I am writing to an LED display and I can see the proper characters get displayed. Also I think my i2c device needs restarts in-between each byte for it to work properly, so I need to be able to read individual bytes with there corresponding offsets, also i2c-tools supports this.
Simon Glass sjg@chromium.org wrote:
Also check the i2c_get_chip() function which you are using. See if chip->offset_len is set to 1 as it should be, for your device. If chip->not, perhaps something is missing.
If I use this command I get:
device # i2c olen 0x60 1
This is without the patch BTW.
Hard to say here anything usefull, without the chance to try out...
trying with an i2c epprom (imx6 based board)
=> i2c md 0x58 0 10 0000: 01 00 04 23 00 02 b1 61 00 23 00 10 00 00 03 09 ...#...a.#...... => i2c md 0x58 0 1 0000: 01 . => i2c md 0x58 1 1 0001: 00 . => i2c md 0x58 2 1 0002: 04 . => i2c md 0x58 3 1 0003: 23 # =>
=> i2c flags 0x58 0 => i2c olen 0x58 1 =>
May you can enable debug in drivers/i2c/designware_i2c.c driver?
So we should see, what sort of messages designware_i2c_xfer() gets ... and may have an idea... and may add
717 debug("i2c_xfer: chip=0x%x, len=0x%x\n", msg->addr, msg->len);
also a dump of msg->buf and msg->len.
bye, Heiko -- 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@denx.de
[Duffin, CooperX] Sorry About the late reply
After changing the debug to printf and adding the msg->buf I get the following:
Hit any key to stop autoboot: 0 device # i2c dev 0 Setting bus to 0 I2C bus i2c@ffc02900 version 0x3230302a device # i2c md 0x60 0 4 i2c_xfer: 2 messages i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2d398 i2c_xfer: chip=0x60, len=0x4, buf=0x7fa2d460 0000: 6f 6f 6f 6f oooo
Additionally if I run my standalone application I get this
I2C bus i2c@ffc02900 version 0x3230302a Setting bus speed to 100000 i2c_led name is generic_60 Writing i2c_xfer: 1 messages i2c_xfer: chip=0x60, len=0x2, buf=0x7fa2ced0 i2c_xfer: 1 messages i2c_xfer: chip=0x60, len=0x2, buf=0x7fa2ced0 i2c_xfer: 1 messages i2c_xfer: chip=0x60, len=0x2, buf=0x7fa2ced0 i2c_xfer: 1 messages i2c_xfer: chip=0x60, len=0x2, buf=0x7fa2ced0 Reading i2c_xfer: 2 messages i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf28 i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf88 i2c_xfer: 2 messages i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf28 i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf89 i2c_xfer: 2 messages i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf28 i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf8a i2c_xfer: 2 messages i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf28 i2c_xfer: chip=0x60, len=0x1, buf=0x7fa2cf8b read buffer is o, o, o, o
Hmm... sorry for being unclear, i was interested in the content of each buffer not only the address of it ...
bye, Heiko
participants (4)
-
cduffinx -
Duffin, CooperX
-
Heiko Schocher -
Simon Glass