Re: [Qemu-devel] [PATCH v2] i2c: Fix SMBus read transactions to avoid double events

[Peter Maydell]

On 23 October 2016 at 22:38, Corey Minyard <address@hidden> wrote:
> On 10/22/2016 04:20 AM, Peter Maydell wrote:
>>>>
>>>> I got here because Coverity complains that the
>>>> i2c_start_transfer() calls in smbus.c don't check their
>>>> return value. That suggests to me that we'd be better off
>>>> having a different function (i2c_restart_transfer() ??)
>>>> for the "I know we already did this once, so don't try to
>>>> re-determine who to send this to" case, rather than trying to
>>>> handle both cases in the same function.
>>>
>>>
>>> Perhaps so.  Or maybe i2c_continue_transfer().  That would
>>> be more clear.  The second operation can't fail, but relying
>>> on that is frail.
>>
>> i2c_continue_transfer() sounds like a better name, yes.
>> Would you like to write a patch that takes that approach?
>> If you cc me I'll review it and put it through the
>> target-arm tree.
>
>
> I was working on this and looking at old information, and realized that
> this won't work.  The same problem exists in I2C devices when
> doing SMBus emulation, the device driver in the OS will cause a
> i2c_start_transfer() to be done twice without an intervening
> i2c_end_transfer().  Though a continue function would fix the smbus
> case, it won't fix the i2c case.
>
> I can't think of a better way to do it than I have done.

OK, thanks for looking into this. I'll apply this patch to
target-arm.next, and we can think about something else for
the coverity issue. (Perhaps just assert() that i2c_start_transfer()
returns success? The bus contents shouldn't be able to change
I think...)

thanks
-- PMM