On Friday 16 May 2008, Jean Delvare wrote: > > > default: > > > dev_err(&adapter->dev, "smbus_access called with invalid size > > > (%d)\n", > > > size); > > > - return -1; > > > + return -EOPNOTSUPP; > > > } > > > > I'd rather use -EINVAL here. (...) > > Hmh, scratch this. Thinking about it some more (night helps) > -EOPNOTSUPP is consistent with what we did for the bus drivers after > all, and it also anticipates addition of new transaction types to > <linux/i2c.h> or removal of support for some transaction types from > i2c_smbus_xfer_emulated() ..
Right. > > (..) > > BTW, feel free to adjust the debugging message above, as you did in > > some bus drivers already, to clearly say that we're speaking about a > > transaction type and not a "size". I must have missed a few. Those updates weren't initially on a list of things-to-change, but I guess some of the issues were egregious enough to make me add them -- after that particular confusion percolated long enough. The third or fourth time through a piece of code, it's a bit easier to notice that sort of confusion. (Probably worth someone's time to switch *ALL* drivers from calling that a "size". I think that bit of history needs to be edited out of existence...) > > All the rest looks OK to me from a functional point of view. As far as > > style is concerned, please keep the alignment on opening parenthesis > > when the original code did that. i2c-core uses this style consistently > > and I like it. > > I'll do all this myself now, no need to resend. Good. In general, when it's down to the last set of small comments like this, it's probably easier all around if you do stuff like that (rather than writing it up in email, having someone like me both read it and make time to act on it, then have you re-review). That's fairly common maintainer practice. - Dave _______________________________________________ i2c mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/i2c
