Am 25.08.22 um 07:45 schrieb chr...@rtems.org:
The patch adds the i2c command. It is a single command to test and
explore I2C buses on hardware. The command lets you string together
reads and writes on a single command line to access a number of devices
so you can script basic functionality simply.

This ocommand adds to the existing i2cdetect, i2cset and i2cget however
those commands did not work cleanly on the vck-190 or did not read and
write directly at the bus level.

Chris


Hello Chris,

like said on Discord: Thanks for adding that tool. It has nearly all and a lot more functionallity than the i2cset, i2cget and i2cdetect that I added a while back. Only thing missing is the functionallity to scan the bus using zero length writes like the current i2cdetect does.

For reference I copied the discord thread to the mail below like we agreed so that it is archived. I hope the line breaks added by the mailing list system doesn't make it too unreadable.

Best regards

Christian



7:02 AM] kiwichris: I am adding a new command to RTEMS: i2c
vck190 [/] # i2c help
Usage:: i2c <command> ...
 where <command> is:
  help                          : this help
  det <-b bus>                  : detect devices on bus(es)
  rd <-8> <-16> bus:addr len    : read of data from slave
  wr <-8> <-16> bus:addr data.. : write of data to slave
  rda bus:addr addr len         : write address then read data from slave
  wra bus:addr addr data..      : write address then write data to slave
 bus:addr: decimal bus number and hex address, eg 0:74
 data: even length hex data string, eg 001122334455
vck190 [/] # i2c det
 0  20 74
 1  74 75
vck190 [/] # i2c wr 0:74 01
vck190 [/] # i2c det
 0  0c 16 17 1c 1d 1e 1f 20 46 47 4c 4d 4e 4f 50 74
 1  74 75
vck190 [/] # i2c wr 1:74 01
vck190 [/] # i2c det
 0  0c 16 17 1c 1d 1e 1f 20 46 47 4c 4d 4e 4f 50 74
 1  52 54 5d 74 75

The test detects the available devices and them selects channel 1 on the 8 channel I2C multiplexer on bus-0 followed by a detect. This is repeated on bus-1. [8:12 AM] c-mauderer: You are aware of the i2cdetect i2cget and i2cset that I added a bit back? https://docs.rtems.org/branches/master/shell/general_commands.html#i2cdetect [8:13 AM] c-mauderer: The interface is similar to the commands from Linux i2c tools. [8:13 AM] c-mauderer: I think it would cover the det, rda and wra of your command. [8:14 AM] c-mauderer: I don't have a problem if you want to replace my commands. But I don't think that we should have both.
[8:15 AM] c-mauderer: Except if there is a relevant difference
[8:16 AM] kiwichris: It did not work on the versal.
[8:16 AM] kiwichris: Also I did not know they existed before I did these.
[8:17 AM] kiwichris:
vck190 [/] # i2cdetect /dev/i2c-0
    x0 x1 x2 x3 x4 x5 x6 x7 x8 x9 xA xB xC xD xE xF
0x     -- -- -- -- -- -- -- -- -- -- --ioctl failed: Connection timed out
 -- -- -- --
1x  -- -- -- -- -- --ioctl failed: Connection timed out
 --ioctl failed: Connection timed out
 -- -- -- -- --ioctl failed: Connection timed out
 --ioctl failed: Connection timed out
 --ioctl failed: Connection timed out
 --ioctl failed: Connection timed out
 --
2x ioctl failed: Connection timed out
 -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
3x  -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
4x  -- -- -- -- -- --ioctl failed: Connection timed out
 --ioctl failed: Connection timed out
 -- -- -- -- --ioctl failed: Connection timed out
 --ioctl failed: Connection timed out
 --ioctl failed: Connection timed out
 --ioctl failed: Connection timed out
 --
5x ioctl failed: Connection timed out
 -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
6x  -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
7x  -- -- -- --ioctl failed: Connection timed out
 -- -- -- -- -- -- -- -- -- -- -- --
[8:18 AM] kiwichris: Yes I saw it was like the i2c-tools command but they also do some strange things on the bus. [8:18 AM] kiwichris: I am fine if they all exists if there are needs. My concern fixing or playing with the those commands was them then not working for some [8:19 AM] c-mauderer: The connection timeout is normally a problem with the driver. The detect only addresses a device without data. [8:19 AM] c-mauderer: That's an edge case that isn't handled in every driver. [8:20 AM] kiwichris: It is not a problem with the detect I have and no timeouts. Also writing breaks multiplexers which the vck-190 has
[8:20 AM] c-mauderer: How did you implement the detect?
[8:21 AM] kiwichris: The Linux i2c-tool has some selected groups of addresses it does different things, ie to handle write-only devices but I do not like that. [8:21 AM] kiwichris: The driver is the cadence one and it is the same the zynqmp hardware and it seems to be working as expected.
[8:22 AM] kiwichris: From the patch 🙂 ...
+      for (addr = MIN_ADDR; addr <= MAX_ADDR; ++addr) {
+        if (!i2c_set_slave(i2c, bus, addr, false)) {
+          state[addr] = 'E';
+        } else {
+          uint8_t buf;
+          ssize_t r = read(i2c->bus[bus], &buf, 1);
+          if (r == 1) {
+            state[addr] = 'X';
+          } else {
+            buf = I2C_SMBUS_WRITE;
+            ssize_t r = write(i2c->bus[bus], &buf, 1);
+            if (r == 1) {
+              state[addr] = 'X';
+            }
+          }
+        }
+      }
[8:23 AM] kiwichris: It is finding all the devices and also the ones on the multiplexed bus when it is switched [8:25 AM] c-mauderer: But that means that an actual read access of one byte is done to the device.
[8:26 AM] c-mauderer: Can mess up some status registers or other stuff.
[8:27 AM] kiwichris: Yes but it is a detect and not designed for live probing for production.
[8:27 AM] c-mauderer: Hm. You should add at least a big warning about that.
[8:28 AM] c-mauderer: And I still think that it's a driver problem that the i2cprobe doesn't work. [8:29 AM] kiwichris: I am not sure issuing a start and no length is fine for all devices. Linux does the read or write but yes the doco can explain it. [8:30 AM] c-mauderer: Yes, it's also not ideal for all devices. But it's at least tested for a lot of devices that connect to Linux systems 😉
[8:30 AM] kiwichris: But Linux reads a byte?
[8:31 AM] c-mauderer: No. The Linux i2c-tools only address the devices.
[8:31 AM] c-mauderer: If an ACK is seen on the address, the device is there.
[8:32 AM] c-mauderer: It's also a nice test case for the drivers. Most of the time it's more or less a bug that the driver doesn't support a read / write with length 0.
[8:33 AM] kiwichris: Hmm the code I looked at did a i2c_smbus_read_byte
[8:33 AM] kiwichris: It could be the wrong code
[8:33 AM] c-mauderer: https://git.rtems.org/rtems/tree/cpukit/libmisc/shell/main_i2cdetect.c#n46 [8:34 AM] kiwichris: https://sources.debian.org/src/i2c-tools/4.3-2/tools/i2cdetect.c/#L105 [8:34 AM] c-mauderer: Ah, you mean the Linux code. Give me a moment. It has been a while since I worked through that. [8:35 AM] kiwichris: Yeah I do and that is what I said. They do have lots of devices connected 🙂 🙂 [8:37 AM] kiwichris: The driver could have issues as you say but as it is I can access the devices I need too [8:43 AM] c-mauderer: Ah, yes: The read mode is only if you use a special "-r" option. The default case is the i2c_smbus_write_quick() which basically does exactly what I did in i2cdetect: Cause a write addressing of a device with zero length and take a look whether the device responds. [8:44 AM] kiwichris: Thanks, I did not have the time to look deeper. Interesting name for a write_quick when it is a null write. [8:47 AM] kiwichris: I will raise the driver question with @opticron tomorrow to see what he gets on the ZynqMP [8:47 AM] c-mauderer: Yes, it uses this odd I2C_SMBUS_QUICK define that is in the Linux i2c header: https://git.kernel.org/pub/scm/utils/i2c-tools/i2c-tools.git/tree/lib/smbus.c#n30
[8:47 AM] c-mauderer: The define is just 0. We also have it in our i2c.h
[8:48 AM] c-mauderer: https://git.rtems.org/rtems/tree/cpukit/include/linux/i2c.h#n266
[8:48 AM] kiwichris: Understood.
[8:49 AM] c-mauderer: Like I said: I don't have a problem if you want to replace the i2cdetect. It was a very quick hacked together tool. I know that I analyzed I2C tools back then already to see how they do the detection. But I think we should support that "write zero bytes" method too. [8:49 AM] c-mauderer: Like I said: Has been useful for me in the past to detect driver bugs. [8:50 AM] kiwichris: Yes I agree the 0 method is one that should be there and maybe with the read as well. I need to do things like wr 0:74 01 wra 0:45 2 aa wra 0:45 3 55 etc
[8:51 AM] kiwichris: Where 74 is a selector and then a few writes
[8:52 AM] kiwichris: I can add the zero wait as a v2 change if the change is something we want to add into RTEMS. Pretty relaxed about it. [9:03 AM] c-mauderer: Regarding the I2C addresses: Do I see it correctly that you allow addresses from 0 to 255 for 8 bit addresses. So that would include the I2C read/write bit? So an address of 0x04 and 0x05 would address the same device but one time in read and one time in write mode? [9:04 AM] c-mauderer: ah, sorry. That is for the address for eeprom like devices.
[9:04 AM] c-mauderer: Missed that.
[9:04 AM] kiwichris: Ah yes it was 127 as I knew that then changed it. I also need to add ten bit address support [9:05 AM] kiwichris: I have 8 or 16 addresses. I am actually not sure the 16bit is byte ordered in the buffer the right way. [9:06 AM] kiwichris: I would like to support ten bit addressing. This is for drivers like the AXI I2C IP from Xilinx that have output ports the driver can access for multiplexing [9:07 AM] c-mauderer: All in all: Sounds like a great tool. That can do quite a lot more than my i2c* commands and I think it should replace them. Currently the only thing that I found missing is the detect using a zero-length-write like discussed. Otherwise I think it can do more. Thanks for adding that new tool. [9:08 AM] kiwichris: Yes if it is going to replace those commands (which I found after I had done this) it must cover what they do. [9:09 AM] kiwichris: It is great being here to discuss this here and sort if out. Way faster 🙂 So a big thanks [9:09 AM] c-mauderer: Yes. Discord is great for discussions. I'm only missing a public archive.
[9:09 AM] kiwichris: Yeap I agree.
[9:09 AM] c-mauderer: Should we copy the discussion into a mail and send it as response to the patch?
[9:10 AM] kiwichris: Good idea 🙂
[9:10 AM] c-mauderer: OK. I'll send a mail.


--
--------------------------------------------
embedded brains GmbH
Herr Christian MAUDERER
Dornierstr. 4
82178 Puchheim
Germany
email:  christian.maude...@embedded-brains.de
phone:  +49-89-18 94 741 - 18
mobile: +49-176-152 206 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Reply via email to