Re: [RFC PATCH 0/4] hw/i2c: i2c slave mode support

[Peter Delevoryas]

> On Apr 6, 2022, at 11:41 AM, Klaus Jensen <its@irrelevant.dk> wrote:
> 
> On Apr  6 17:03, Peter Delevoryas wrote:
>> 
>> 
>>> On Apr 5, 2022, at 11:07 PM, Klaus Jensen <its@irrelevant.dk> wrote:
>>> 
>>> On Apr 5 20:52, Peter Delevoryas wrote:
>>>> 
>>>> 
>>>>> On Mar 31, 2022, at 9:57 AM, Klaus Jensen <its@irrelevant.dk> wrote:
>>>>> 
>>>>> From: Klaus Jensen <k.jensen@samsung.com>
>>>>> 
>>>>> Hi all,
>>>>> 
>>>>> This RFC series adds I2C "slave mode" support for the Aspeed I2C
>>>>> controller as well as the necessary infrastructure in the i2c core to
>>>>> support this.
>>>>> 
>>>>> Background
>>>>> ~~~~~~~~~~
>>>>> We are working on an emulated NVM Express Management Interface[1] for
>>>>> testing and validation purposes. NVMe-MI is based on the MCTP
>>>>> protocol[2] which may use a variety of underlying transports. The one we
>>>>> are interested in is I2C[3].
>>>>> 
>>>>> The first general trickery here is that all MCTP transactions are based
>>>>> on the SMBus Block Write bus protocol[4]. This means that the slave must
>>>>> be able to master the bus to communicate. As you know, hw/i2c/core.c
>>>>> currently does not support this use case.
>>>> 
>>>> This is great, I’m attempting to use your changes right now for the same 
>>>> thing (MCTP).
>>>> 
>>> 
>>> Awesome!
>>> 
>>>>> 
>>>>> The second issue is how to interact with these mastering devices. Jeremy
>>>>> and Matt (CC'ed) have been working on an MCTP stack for the Linux Kernel
>>>>> (already upstream) and an I2C binding driver[5] is currently under
>>>>> review. This binding driver relies on I2C slave mode support in the I2C
>>>>> controller.
>>>>> 
>>>>> This series
>>>>> ~~~~~~~~~~~
>>>>> Patch 1 adds support for multiple masters in the i2c core, allowing
>>>>> slaves to master the bus and safely issue i2c_send/recv(). Patch 2 adds
>>>>> an asynchronous send i2c_send_async(I2CBus *, uint8) on the bus that
>>>>> must be paired with an explicit ack using i2c_ack(I2CBus *).
>>>>> 
>>>>> Patch 3 adds the slave mode functionality to the emulated Aspeed I2C
>>>>> controller. The implementation is probably buggy since I had to rely on
>>>>> the implementation of the kernel driver to reverse engineer the behavior
>>>>> of the controller slave mode (I do not have access to a spec sheet for
>>>>> the Aspeed, but maybe someone can help me out with that?).
>>>>> 
>>>>> Finally, patch 4 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
>>>> 
>>>> When I try this with my system, it seems like the i2c-echo device takes 
>>>> over
>>>> the bus but never echoes the data to the EEPROM. Am I missing something to
>>>> make this work? It seems like the “i2c_send_async” calls aren’t happening,
>>>> which must be because the bottom half isn’t being scheduled, right? After
>>>> the i2c_do_start_transfer, how is the bottom half supposed to be scheduled
>>>> again? Is the slave receiving (the EEPROM) supposed to call i2c_ack or 
>>>> something?
>>>> 
>>>> root@bmc-oob:~# echo 24c02 0x1064 > /sys/bus/i2c/devices/i2c-8/new_device
>>>> [ 135.559719] at24 8-1064: 256 byte 24c02 EEPROM, writable, 1 bytes/write
>>>> [ 135.562661] i2c i2c-8: new_device: Instantiated device 24c02 at 0x64
>>>> root@bmc-oob:~# i2cset -y 8 0x42 0x64 0x00 0xaa i
>>>> i2c_echo_event: start send
>>>> i2c_echo_send: data[0] = 0x64
>>>> i2c_echo_send: data[1] = 0x00
>>>> i2c_echo_send: data[2] = 0xaa
>>>> i2c_echo_event: scheduling bottom-half
>>>> i2c_echo_bh: attempting to gain mastery of bus
>>>> i2c_echo_bh: starting a send to address 0x64
>>>> root@bmc-oob:~# hexdump -C /sys/bus/i2c/devices/8-1064/eeprom
>>>> 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
>>>> *
>>>> 00000100
>>>> 
>>>> Thanks again for this, it’s exactly what I needed.
>>>> 
>>> 
>>> Hmmm. The only obvious difference I see here is that I write
>>> "slave-24c02" and not just "24c02" to new_device. Not sure if that has
>>> any implications? Also, looks like your EEPROM is initialized with
>>> zeroes, mine is all ones. This hints at the device being instantiated is
>>> different. I'm also not seeing the 'at24', which upon looking in the
>>> kernel code is a different device?
>> 
>> Are you letting the kernel control the EEPROM?
>> 
>> If I actually let the kernel control it, then I can’t use i2cset, because
>> the kernel seems to be keeping the bus busy/etc. I tried i2c bus 9 this time.
>> 
>> root@bmc-oob:~# i2cset -y 9 0x64 0x00 0xaa i
>> Error: Could not set address to 0x64: Device or resource busy
>> 
>> However, if I don’t instantiate a kernel device, and I just use 
>> i2cset/i2cget,
>> I can control the EEPROM:
>> 
>> root@bmc-oob:~# i2cset -y 9 0x64 0x00 0xcc i
>> root@bmc-oob:~# i2cget -y 9 0x64 0x00 i
>> 0xcc 0xb9 0x4d 0xe1 0x42 0x56 0x00 0x00 0xc5 0x5b 0x28 0xe1 0x42 0x56 0x00 
>> 0x00 0x00 0x61 0x13 0xe2 0x42 0x56 0x00 0x00 0xb7 0x64 0x28 0xe1 0x42
>> 0x56 0x00 0x00
>> 
>> Unfortunately, i2c-echo still doesn’t seem to send its data:
>> 
>> root@bmc-oob:~# i2cset -y 9 0x42 0x64 0x00 0xaa i
>> i2c_echo_event: start send
>> i2c_echo_send: data[0] = 0x64
>> i2c_echo_send: data[1] = 0x00
>> i2c_echo_send: data[2] = 0xaa
>> i2c_echo_event: scheduling bottom-half
>> i2c_echo_bh: attempting to gain mastery of bus
>> i2c_echo_bh: starting a send to address 0x64
>> 
>> What is the exact sequence of events once i2c-echo
>> starts a new transfer? Does the slave device ACK
>> the start, or does it just wait for data to be sent?
>> 
>> And then if I try to read the EEPROM:
>> 
>> root@bmc-oob:~# i2cget -y 9 0x64 0x00 i
>> smbus: error: Unexpected send start condition in state 1
>> smbus: error: Unexpected write in state -1
>> smbus: error: Unexpected recv start condition in state -1
>> smbus: error: Unexpected read in state -1
>> smbus: error: Unexpected read in state -1
>> smbus: error: Unexpected read in state -1
>> 
>> I’ll try debugging/refactoring further to see why it’s not working.
>> 
>> By the way, this is my i2c_init code in QEMU to ensure
>> a QEMU EEPROM model is present:
>> 
>> static void fby35_i2c_init(AspeedMachineState *bmc)
>> {
>>    I2CBus *i2c[16];
>> 
>>    for (int i = 0; i < 16; i++) {
>>        i2c[i] = aspeed_i2c_get_bus(&bmc->soc.i2c, i);
>>        assert(i2c[i] != NULL);
>>    }
>> 
>>    i2c_slave_create_simple(i2c[9], "i2c-echo", 0x42);
>>    uint8_t buf[256] = {0xff};
>>    smbus_eeprom_init_one(i2c[9], 0x64, buf);
>> }
>> 
>> This is an AST2600-based board too.
>> 
> 
> Oh. You are trying to echo to an actual EEPROM device on the board? In

Ohhh erg yes, I was.

> that case yes. The new async API currently only works with the slave
> device on the i2c controller. The i2c echo device cannot talk to any
> other slave devices since they do not implement the asynchronous send.

Oh that makes total sense.

I was not aware of the whole “slave-eeprom” backend, I thought you
were instantiating a regular eeprom. After enabling that driver in
my Kconfig and using “slave-eeprom”, it all worked! Thanks, sorry
for the confusion. I’m excited to test this out with some more things
now!

root@bmc-oob:~# hexdump -C /sys/bus/i2c/devices/i2c-9/9-0064/slave-eeprom
00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
00000100
root@bmc-oob:~# i2cset -y 9 0x42 0x64 0x00 0xaa i
i2c_echo_event: start send
i2c_echo_send: data[0] = 0x64
i2c_echo_send: data[1] = 0x00
i2c_echo_send: data[2] = 0xaa
i2c_echo_event: scheduling bottom-half
i2c_echo_bh: attempting to gain mastery of bus
i2c_echo_bh: starting a send to address 0x64
i2c_ack: ack'd slave async send
i2c_echo_bh: async sending data[1] (0x00)
i2c_send_async: slave 0x64 data=0x00
i2c_ack: ack'd slave async send
i2c_echo_bh: async sending data[2] (0xaa)
i2c_send_async: slave 0x64 data=0xaa
i2c_ack: ack'd slave async send
root@bmc-oob:~# hexdump -C /sys/bus/i2c/devices/i2c-9/9-0064/slave-eeprom
00000000  aa ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000010  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
00000100