-----Original Message-----
From: Heiko Schocher <h...@denx.de>
Sent: Tuesday, November 17, 2020 11:42 PM
To: Duffin, CooperX <cooperx.duf...@intel.com>
Cc: Simon Glass <s...@chromium.org>; U-Boot Mailing List
<u-boot@lists.denx.de>; uboot-snps-...@synopsys.com; Tom Rini
<tr...@konsulko.com>; Robert Beckett <bob.beck...@collabora.com>; Wolgang Denk
<w...@denx.de>; Ian Ray <ian....@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.duf...@intel.com> wrote:
>>
>> -----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
>> 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 <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
>>> 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: h...@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
