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...)