On Wednesday 25 September 2013 18:31:51 Guennadi Liakhovetski wrote: > On Wed, 25 Sep 2013, Valentine wrote: > > On 09/25/2013 06:08 PM, Guennadi Liakhovetski wrote: [snip] > > > > > > +/* I2C I/O operations */ > > > > > > +static s32 adv_smbus_read_byte_data(struct i2c_client *client, u8 > > > > > > command) > > > > > > +{ > > > > > > + s32 ret, i; > > > > > > + > > > > > > + for (i = 0; i < 3; i++) { > > > > > > + ret = i2c_smbus_read_byte_data(client, command); > > > > > > > > > > Uhm, why do you need to do this 3 times?... I see adv7842 does that > > > > > too... > > > > > but e.g. adv7604 dies this only for writing and not for reading... > > > > > > > > Just thought it would be safe to retry in case of a failure. > > > > Other drivers seem to retry I2C I/O as well. This is probably related > > > > to the possible cable issues. Not sure. Anyways, retrying doesn't seem > > > > to hurt, does it? > > > > > > It does, because it means there's something we don't understand. IMHO it > > > should either work from the first time, or there should be an error, > > > that we understand with a following error processing, that _might_ > > > include re-trying. Re-trying just in case isn't good. Especially if no > > > error has been observed. > > > > I have observed very rare errors when reading HDMI status. Just a couple > > of times during this week. I'm not sure of the nature of those errors. > > Just thought it would be OK to retry since other decoder drivers do so as > > well. > > Ok, understand. As I said, I personally don't like that, but, I think, > Laurent will have a final word on this. I don't like the idea of blindly retrying, especially for write operations. If a read fails due to a flaky cable, there's equal chances that a write would fail as well, which could result in writing random values to random registers. Given the side effects that this could have, I'd much rather not retry I/O operations and have the users fix electrical issues. Hiding something this serious could be dangerous. -- Regards, Laurent Pinchart -- 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