Hi Ben, I see in commit f553b79c03f0dbd52f6f03abe8233a2bef8cbd0d that you just changed the nouveau driver to use an internal i2c bit-banging implementation instead of i2c-algo-bit. Let me say here publicly that I disapprove this change. I believe this is a move in the wrong direction. Just because doing so fixed one bug you were seeing doesn't make it right. If i2c-algo-bit has problems, please report them to the maintainer (i.e. me) and have them fixed. If you have problems with it, others may do as well. If everyone stops using common pieces of code as soon as they have a problem, we will end up with a lot of duplicated code in the kernel, which is bad in many respects. In the commit message, you complain about the use of cond_resched() in i2c-algo-bit. You might as well be right, maybe it should be removed. It's been there pretty much forever (February 2002 at least) and while I can understand why it was put there, I would agree it isn't necessarily a good idea. Did you try removing it to see if it solved your problem? There is also a call to yield() somewhere else in the i2c-algo-bit driver. I remember having a discussion about it long ago, someone proposed to change it to cond_resched(), but it never happened. I admit I don't know the difference between cond_resched() and yield() so I can't really make a decision until someone explains it to me. I see that your reimplementation uses: #define T_HOLD 5000 which basically means you're running the I2C bus at ~100 kHz, while the original code had: port->bit.udelay = 40; which basically ran the bus at ~12.5 kHz, i.e. only a tad faster than the low limit allowed by SMBus. I warned some times ago that such low speeds could be problematic: http://lists.freedesktop.org/archives/dri-devel/2011-October/015512.html I even posted a patch: http://lists.freedesktop.org/archives/dri-devel/2011-October/015504.html Despite positive comments from the i915 and radeon driver maintainers, it was never applied. Did you try lowering the udelay value with i2c- algo-bit to see if it would solve your problem? David, any reason why you did not pick this patch? Should I resend it? In a split form maybe? Ben, again, I would really have appreciated if you had contacted me while investigating your NVS 300 issue, instead of silently running away from i2c-algo-bit. If the code is broken, let's fix it for the benefit of all users. Duplicating code around isn't going to help any. Thanks, -- Jean Delvare Suse L3 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
