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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/15/2012 03:01 PM, Frank Schäfer wrote:
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@xxxxxxxxxxxxxx>


@@ -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 ?

I will repeat me, I see it overkill to check address correctness. And as I said, that one is general validly could be done easily in I2C core - so why the hell you wish make it just only for em28xx ?

I am quite sure if that kind of address validness are saw important they are already implemented by I2C core.

Make patch for I2C which does that address validation against client 10BIT flag and sent it to the mailing list for discussion.

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

Could be, but I think 10bit is never used in real life.

regards
Antti

--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux