Re: [Qemu-devel] [PATCH 2/2] Add AT24Cxx I2C EEPROM device model

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2012-11-20 15:40, Stefan Hajnoczi wrote:
> On Tue, Nov 20, 2012 at 3:38 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2012-11-20 15:06, Stefan Hajnoczi wrote:
>>> On Tue, Nov 20, 2012 at 02:37:07PM +0100, Jan Kiszka wrote:
>>>> On 2012-11-20 14:27, Stefan Hajnoczi wrote:
>>>>> On Mon, Nov 19, 2012 at 03:24:39PM +0100, Jan Kiszka wrote:
>>>>>> +static void at24_flush_transfer_buffer(AT24State *s)
>>>>>> +{
>>>>>> +    if (s->cached_sector < 0 || !s->cache_dirty) {
>>>>>> +        return;
>>>>>> +    }
>>>>>> +    bdrv_write(s->bs, s->cached_sector, s->sector_buffer, 1);
>>>>> [...]
>>>>>> +static int at24_cache_sector(AT24State *s, int sector)
>>>>>> +{
>>>>>> +    int ret;
>>>>>> +
>>>>>> +    if (s->cached_sector == sector) {
>>>>>> +        return 0;
>>>>>> +    }
>>>>>> +    ret = bdrv_read(s->bs, sector, s->sector_buffer, 1);
>>>>>
>>>>> Can you use bdrv_aio_writev()/bdrv_aio_readv()?  We should avoid adding
>>>>> new synchronous block I/O.  Because it forces us to run a nested event
>>>>> loop that blocks the guest until I/O completes.
>>>>
>>>> The call is synchronous as the I2C bus model is as well. How do I model
>>>> this with bdrv_aio_*?
>>>
>>> The bus model needs to be asynchronous.
>>
>> That's not easy, even in theory. I2C allows a slave to defer its answer
>> by holding the clock line down, but not all masters support this, and
>> some (e.g. SMBus) even demand an upper limit. So we risk timeouts of the
>> guest when deferring normally synchronous accesses like on these EEPROMs.
>>
>> IOW: There will likely be a need for synchronously waiting on block
>> layer I/O requests also in the future.
> 
> How large are the EEPROMs?  Perhaps the data should be in memory?

Up to 128K with these models. But that only solves the read delay.
Writes can be delayed by the EEPROM but have an upper limit for the
completion time (between 5 and 20 ms).

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux