Hi Peter, > >> In i2c_smbus_xfer_emulated(), the function i2c_transfer() is invoked to > >> transfer i2c messages. The number of actual transferred messages is > >> returned and saved to 'status'. If 'status' is negative, that means an > >> error occurred during the transfer process. In that case, the value of > >> 'status' is an error code to indicate the reason of the transfer failure. > >> In most cases, i2c_transfer() can transfer 'num' messages with no error. > >> And so 'status' == 'num'. However, due to unexpected errors, it is probable > >> that only partial messages are transferred by i2c_transfer(). As a result, > >> 'status' != 'num'. This special case is not checked after the invocation of > >> i2c_transfer() and can potentially lead to unexpected issues in the > >> following execution since it is expected that 'status' == 'num'. > >> > >> This patch checks the return value of i2c_transfer() and returns an error > >> code -EIO if the number of actual transferred messages 'status' is not > >> equal to 'num'. > >> > >> Signed-off-by: Wenwen Wang <wang6495@xxxxxxx> > > > > Applied to for-current, thanks! > > > > I meant to comment here but got side-tracked and never got around to it. > But see e.g. [1] and [2] for drivers that will not be happy with this > change. Maybe there are more of those? I did a scan of the drivers in > algos/ and busses/, but there are drivers all over the tree that > implements .master_xfer that I have not audited. Who knows what further > problems this patch will reveal? So, maybe we should be a bit > conservative and only WARN as a first step? I came to the conclusion: yes and no. I think this patch is correct, so it is good to have. But true, it will expose if other drivers have implemented the return value wrongly. So, I removed this patch from for-current and plan to include it in for-next instead if we can agree on that being a good way forward. That will allow for one full cycle of testing and fixing the issues found. And hopefully I have time to write a small coccinelle rule to find if constant values are returned in a function declared as master_xfer. > PS. Also busses/i2c-rcar.c seems like it might return a short "success" > for some sequence of events. But I'm not sure about that one... What do you mean with "short success for some sequence" here? Thanks, Wolfram
Attachment:
signature.asc
Description: PGP signature
