Re: [PATCH 5/5] em28xx: fix+improve+unify i2c error handling, debug messages and code comments

["Frank Schäfer" <fschaefer.oss@googlemail.com>]

Am 14.12.2012 18:03, schrieb Antti Palosaari:
> On 12/14/2012 06:28 PM, Frank Schäfer wrote:
>> - check i2c slave address range (only 7 bit addresses supported)
>> - do not pass USB specific error codes to userspace/i2c-subsystem
>> - unify the returned error codes and make them compliant with
>>    the i2c subsystem spec
>> - check number of actually transferred bytes (via USB) everywehere
>> - fix/improve debug messages
>> - improve code comments
>>
>> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
>
>
>> @@ -244,16 +294,20 @@ static int em28xx_i2c_xfer(struct i2c_adapter
>> *i2c_adap,
>>           dprintk2(2, "%s %s addr=%x len=%d:",
>>                (msgs[i].flags & I2C_M_RD) ? "read" : "write",
>>                i == num - 1 ? "stop" : "nonstop", addr, msgs[i].len);
>> +        if (addr > 0xff) {
>> +            dprintk2(2, " ERROR: 10 bit addresses not supported\n");
>> +            return -EOPNOTSUPP;
>> +        }
>
> There is own flag for 10bit I2C address. Use it (and likely not
> compare at all addr validly like that). This kind of address
> validation check is quite unnecessary - and after all if it is wanted
> then correct place is somewhere in I2C routines.

Well, to be 100% sure and strict, we should check both, the flag and the
actual address.
We support 7 bit addresses only, no matter which i2c algo is used. So
doing the address check in each i2c routine seems to be unnecessary code
duplication to me ?

BTW: with the em28xx algorithm, the i2c address is transferred as 16 bit
value. So 10 bit addresses COULD work in theory... ;)

Regards,
Frank