Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i2cdetect "phantom devices" #7

Open
MarkMLl opened this issue Feb 22, 2022 · 12 comments
Open

i2cdetect "phantom devices" #7

MarkMLl opened this issue Feb 22, 2022 · 12 comments

Comments

@MarkMLl
Copy link

MarkMLl commented Feb 22, 2022

i2cdetect -y <bus_number> shows phantom devices at all addresses.

The upstream version of the driver at https://github.com/allanbian1017/i2c-ch341-usb has a patch which attempts to do a status check before every operation, but I find that this is incompatible with e.g. an MFRC522 (RFID reader) slave.

I can't see an easy fix for this. I've been investigating whether it would be possible to decide whether to do a status check based on whether an access was a simple read or write or a combined write/read, but this won't work since there is a limit on the write/read transfer size hence for larger transfers it is necessary to fall back to single-byte operations.

The only remaining possibility that I can see is to recognise that i2cdetect uses SMB operations and to apply a status check only to those. That would at least leave i2cdetect plus basic I2C operations working. However I've not so far been able to work out how to distinguish these cases.

@xboxplayer9889
Copy link

xboxplayer9889 commented Feb 27, 2022

Yeah, I'm using that one (that version doesn't handle GPIO), but this detects phantom devices...
( and ioctl(fileptr,I2C_SLAVE, devaddr) and I2C_SLAVE_FORCE and/or i2c_smbus_read_byte( fileptr ) always success with this
my code:
__s32 opResult = ioctl(myfile, I2C_SLAVE_FORCE, i );
if (opResult==0) {
result[i] = i2c_smbus_read_byte( myfile );
}
else {
result[i] = -1;
}
and result[i] = 255 when no device at address, should be negative result )

@MarkMLl
Copy link
Author

MarkMLl commented Feb 27, 2022

You'll have to excuse me for being unfamiliar with "correct operation" of Git etc.

I've got a tentative patch in my fork at https://github.com/MarkMLl/i2c-ch341-usb but the situation appears to be that (a) if I leave that patch disabled I can drive e.g. an mfrc522, (b) if I either enable that patch or use the pre-GPIO variant from https://github.com/allanbian1017/i2c-ch341-usb then phantom device detection swallows bytes being transferred.

So the basic problem is either that my use of the I2C API is broken, or that allanbian1017's hack doesn't work.

@xboxplayer9889
Copy link

Sorry, I think I don't understand your problem...

if I use https://github.com/allanbian1017/i2c-ch341-usb, the i2cdetec -y 4 gives correct result:
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: -- -- -- -- -- -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
50: 50 51 52 53 54 55 56 57 -- -- -- -- -- -- -- --
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
70: -- -- -- -- -- -- -- --

But with this driver:
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
10: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
20: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
30: 30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f
40: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
50: 50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
60: 60 61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e 6f
70: 70 71 72 73 74 75 76 77

(And I inspected a little more, the advised read() function also gives 255 if no device)

@MarkMLl
Copy link
Author

MarkMLl commented Feb 28, 2022

Sure, i2cdetect works properly... but have you actually tried using the I2C API to talk to a real device?

My experience- which I thought I was making clear- is that the patch which fixes i2cdetect breaks the API on e.g. an mfrc522.

@xboxplayer9889
Copy link

Sorry, I don't understand your problem....

I can think you are using Pi - and Pi I2C API, sorry I'm using PC + Linux, I tried eeproms, sensores and more. I used i2c_smbus_functions and ioctl in C/C++ and I can talk to devices well with that driver, also this driver reads eeprom properly only some error handling is not the same I think.
And I tried the GPIO basics, and that worked too.

@xboxplayer9889
Copy link

xboxplayer9889 commented Mar 1, 2022

Ahh, As I inspected this code, I think I can see what is your problem with allanbian's version. That version does a fake transfer before each real transfer to detect the if there is a device, so e.g. continuous reading you can lost packets/data and so on... maybe reading blocks can solve that.
So your modification works better?

@MarkMLl
Copy link
Author

MarkMLl commented Mar 1, 2022

Ahh, interesting: I'll investigate :-)

Dealing with other minor points that you've raised, I'm using Linux on a PC and my only reference to an RPi was because I'd tweaked my fork to get the naming to act the same.

I've been using the SPI variant of the module heavily, and have tested the GPIO stuff fairly thoroughly. PITA that one has to juggle installed/blacklisted modules when switching to an Arduino (clone) that has a CH341, there's probably a way to sort that that involves a patch to the mainstream kernel (which DKMS might or not be able to apply).

@MarkMLl
Copy link
Author

MarkMLl commented Mar 2, 2022

I've been comparing the I2C implementation using the onboard facilities of a Raspberry Pi with the implementations by Gunar (i.e. this project) and Allan. I don't want to cloud the issue by using my own code, since I make no claim to be a competent kernel developer.

All three module/driver variants appear correct with single-byte I2C reads, where the specific sequence is that a register number is written to the slave and a single byte of data read back. This appears to be irrespective of whether a pair of standard kernel calls is used (i.e. register number followed by data as a two-byte write) or the I2C write-read ioctl is used (i.e. to write a register number and immediately read a single byte of data).

The RPi and Gunar variants appear correct with multi-byte reads, where a register number is written to the slave and a sequence of bytes read back. These are being handled by single-byte write and read operations since the data I'm using comes back in chunks of 64 bytes but somewhere there's a 63-byte transfer limitation in the write-read ioctl.

...
To 01 write 00
From 09 read 00 EB 66 BA 57 BF 23 95 D0 E3 0D 3D 27 89 5C DE 9D 3B A7 00 21 5B 89 82 51 3A EB 02 0C A5 00 49 7C 84 4D B3 CC D2 1B 81 5D 48 76 D5 71 61 21 A9 86 96 83 38 CF 9D 5B 6D DC 15 BA 3E 7D 95 3B 2F
To 36 write 00
From 37 read 92
...

The Allan variant only transfers half the data, the trailing zeroes below being the pre-initialised buffer:
...
To 01 write 00
From 09 read 66 57 23 D0 0D 27 5C 9D A7 21 89 51 EB 0C 00 7C 4D CC 1B 5D 76 71 21 86 83 CF 5B DC BA 7D 3B 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
To 36 write 00
From 37 read 92
...

My own fork behaves like either the Gunar or the Allan variants, depending on whether I incorporate the phantom-device fix.

I'm not sure that there's an application-level way round this, since it's basically down to how the I2C bus and the supporting kernel module/driver handles a certain type of chip.

The bottom line is that there's a problem in Allan's code upstream, which only becomes apparent with certain types of slave.

@xboxplayer9889
Copy link

xboxplayer9889 commented Mar 2, 2022

Ty, as I said, Allan's drops data because of device detection fake messages, but I changed my code when 16 bit addressed eeprom (e.g Atmel 24c256) read to read 16byte blocks and worked well...

`
outbuf[0] = int(memaddr/256);
outbuf[1] = int(memaddr%256);

msgs[0].addr = slave_addr;
msgs[0].flags = 0;
msgs[0].len = 2;
msgs[0].buf = outbuf;

msgs[1].addr = slave_addr;
msgs[1].flags = I2C_M_RD | I2C_M_NOSTART;
msgs[1].len = 16;
msgs[1].buf = inbuf;

msgset[0].msgs = msgs;
msgset[0].nmsgs = 2;
ioctl( fileptr, I2C_RDWR, &msgset )

`

but falls when reading bytes with e.g. i2c_smbus_read(), And if but works with i2c_smbus_read_byte/word_data( file, address, command=8bit address), when you always specify mem/red address.
I think your I2C API thing is a simple fileopen, read, write, fileclose where you can write a byte then read a byte, but there are devices that knows more, and maybe some func is specific. For your API thing need this driver...
Further more
I haven't found ch341a register and programming manual/datasheet, but I2C spec has an ACK by device after all bytes sent to them, so ch341a ic should detect that there weren't answer => device busy/no device
But I don't know how to ask ch341a last i2c state through usb_xfer... that would be the correct device detection.
Also ACK not always necessary....

@xboxplayer9889
Copy link

Some question, maybe ch341a datasheet can answer

  • does usb_bulk_msg return value deal with i2c state?
  • can usb usb_get_std_status( dev, USB_RECIP_*, interfaceid, &status ); give detailed info about i2c interface?
  • are I2C_SLAVE and I2C_SLAVE_FORCE implemented in ch341a and in this driver?
    ...

@MarkMLl
Copy link
Author

MarkMLl commented Mar 3, 2022

I2C_SLAVE is definitely implemented, I'd expect I2C_SLAVE_FORCE to be a kernel issue so that's probably implemented.

The others I'll have to research and it might take me some time.

In any event: provided that Gunar's code is used without Allan's patch then the standard I2C API works, and we've got Allan's driver (or my fork) to fall back on if i2cdetect is needed during development.

@xboxplayer9889
Copy link

If found something about i2cstatus
retval = usb_control_msg(dev, pipe, I2C_STATUS = 0x52?, VENDOR_READ_TYPE = 0XC0 ? , value, index, buf, 8, timeout );
but I have to understand

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants