-----Original Message-----
From: Simon Glass <s...@chromium.org>
Sent: Saturday, November 7, 2020 1:33 PM
To: Duffin, CooperX <cooperx.duf...@intel.com>
Cc: U-Boot Mailing List <u-boot@lists.denx.de>; uboot-snps-...@synopsys.com;
Tom Rini <tr...@konsulko.com>; Robert Beckett <bob.beck...@collabora.com>;
Heiko Schocher <h...@denx.de>; Wolgang Denk <w...@denx.de>; Ian Ray
<ian....@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.duf...@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 <s...@chromium.org>
> Sent: Friday, November 6, 2020 10:51 AM
> To: Duffin, CooperX <cooperx.duf...@intel.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>;
> uboot-snps-...@synopsys.com; Tom Rini <tr...@konsulko.com>; Robert
> Beckett <bob.beck...@collabora.com>; Heiko Schocher <h...@denx.de>;
> Wolgang Denk <w...@denx.de>; Ian Ray <ian....@ge.com>
> Subject: Re: [dwi2c PATCH v1] dwi2c add offsets to reads
>
> Hi Cduffinx,
>
> On Thu, 5 Nov 2020 at 14:26, cduffinx <cooperx.duf...@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.duf...@intel.com>
> > Signed-off-by: cduffinx <cooperx.duf...@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 <s...@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 <s...@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.
