On Jun 2 09:52, Cédric Le Goater wrote: > On 6/1/22 23:08, Klaus Jensen wrote: > > From: Klaus Jensen <k.jen...@samsung.com> > > > > Hi all, > > > > This RFC series adds I2C "slave mode" support for the Aspeed I2C > > I think you can remove the RFC prefix. > > > controller as well as the necessary infrastructure in the i2c core to > > support this. > > > > v2 changes > > ~~~~~~~~~~ > > I finally got around to working on this again. I'm sorry for not > > bringing a v2 to the table earlier. > > > > Mad props to Peter and Jonathan for putting this series to work and > > pushing it forward! Thanks! > > > > This series is based off Cédric's aspeed-7.1 tree, so it includes the > > register fields. This is all "old register mode", but Peter seems to > > have added support in new mode. > > > > There are some loose ends of course, i.e send_async doesn't handle > > broadcast and asynchronous slaves being sent stuff can't nack. But I > > wanted to get some feedback on the interface before I tackle that. > > > > This series > > ~~~~~~~~~~~ > > Patch 1 and 2 are small Aspeed I2C changes/additions. > > > > Patch 3 adds support for multiple masters in the i2c core, allowing > > slaves to master the bus and (safely) issue i2c_send/recv(). > > > > Patch 4 adds an asynchronous send i2c_send_async(I2CBus *, uint8) on the > > bus that must be paired with an explicit ack using i2c_ack(I2CBus *). We > > have previously discussed how we wanted to handle the issue that some > > slaves implement this and some do not. Using a QOM interface was up, but > > couldn't figure out a good way to do it. I ended up decided against it > > since I believe this have to be a run-time check anyway. The problem is > > that a slave can master the bus and try to communicate with *anyone* on > > the bus - and there is no reason why we should only allow asynchronous > > slaves on the bus in that case, or whatever we would want to do when > > devices are plugged. So, instead, the current master can issue an > > i2c_start_send() and if that fails (because it isnt implemented by the > > target slave) it can either bail out or use i2c_start_send_async() if it > > itself supports it. This works the other way around as well of course, > > but it is probably simpler to handle slaves that respond to > > i2c_start_send(). This approach relies on adding a new i2c_event, which > > is why a bunch of other devices needs changes in their event handling. > > > > Patch 5 adds *partial* slave mode functionality to the emulated Aspeed > > I2C controller, that is, it only supports asynchronous sends started by > > another slave that is currently mastering the bus. No asynchronous > > receive. > > If there are no objections, I think this is a good way to move forward > and improve this initial implementation when the need arises. >
There is an outstanding issue with the SLAVE_ADDR_RX_MATCH interrupt bit (bit 7). Remember from my first series I had a workaround to make sure it wasnt masked. I posted this upstream to linux https://lore.kernel.org/lkml/20220602054842.122271-1-...@irrelevant.dk/ Not sure if that is the right way to fix it. You mentioned something about "fixing" a mask on the ast2600? But with the above patch, all works an intended and no "workaround" required. > > Finally, patch 6 adds an example device using this new API. The device > > is a simple "echo" device that upon being sent a set of bytes uses the > > first byte as the address of the slave to echo to. > > > > With this combined I am able to boot up Linux on an emulated Aspeed 2600 > > evaluation board and have the i2c echo device write into a Linux slave > > EEPROM. Assuming the echo device is on address 0x42: > > > > # echo slave-24c02 0x1064 > /sys/bus/i2c/devices/i2c-15/new_device > > i2c i2c-15: new_device: Instantiated device slave-24c02 at 0x64 > > # i2cset -y 15 0x42 0x64 0x00 0xaa i > > # hexdump /sys/bus/i2c/devices/15-1064/slave-eeprom > > 0000000 ffaa ffff ffff ffff ffff ffff ffff ffff > > 0000010 ffff ffff ffff ffff ffff ffff ffff ffff > > * > > 0000100 > > I have started working on buildroot images : > > https://github.com/legoater/buildroot/commits/aspeed > > The resulting files are quite small : > > $ ll output/images/ > total 86040 > drwxr-xr-x 2 legoater legoater 4096 Jun 1 20:01 ./ > drwxrwxr-x 6 legoater legoater 4096 Jun 1 19:40 ../ > -rwxr-xr-x 1 legoater legoater 36837 Jun 1 20:01 > aspeed-ast2600-evb.dtb* > -rw-r--r-- 1 legoater legoater 67108864 Jun 1 20:01 flash.img > -rw-r--r-- 1 legoater legoater 6682796 Jun 1 20:01 image.itb > -rw-r--r-- 1 legoater legoater 1846 Jun 1 20:01 image.its > -rw-r--r-- 1 legoater legoater 3168768 Jun 1 20:01 rootfs.cpio > -rw-r--r-- 1 legoater legoater 1026660 Jun 1 20:01 rootfs.cpio.xz > -rw-r--r-- 1 legoater legoater 3788800 Jun 1 20:01 rootfs.tar > -rw-r--r-- 1 legoater legoater 653777 Jun 1 20:00 u-boot.bin > -rw-r--r-- 1 legoater legoater 5617280 Jun 1 20:01 zImage > > I will probably host them on GH and we could use them under avocado > to extend the tests. > > > They should boot real HW. I will submit the defconfigs to buildroot > after more tests and cleanups. > Nice!
signature.asc
Description: PGP signature
