Re: [patch] Fix AF9015 Dual tuner i2c write failures

[Antti Palosaari <crope@iki.fi>]

On 03/05/2011 12:44 AM, Andrew de Quincey wrote:
>>> Adding a "bus lock" to af9015_i2c_xfer() will not work as demod/tuner
>>> accesses will take multiple i2c transactions.
>>>
>>> Therefore, the following patch overrides the dvb_frontend_ops
>>> functions to add a per-device lock around them: only one frontend can
>>> now use the i2c bus at a time. Testing with the scripts above shows
>>> this has eliminated the errors.
>>
>> This have annoyed me too, but since it does not broken functionality much I
>> haven't put much effort for fixing it. I like that fix since it is in AF9015
>> driver where it logically belongs to. But it looks still rather complex. I
>> see you have also considered "bus lock" to af9015_i2c_xfer() which could be
>> much smaller in code size (that's I have tried to implement long time back).
>>
>> I would like to ask if it possible to check I2C gate open / close inside
>> af9015_i2c_xfer() and lock according that? Something like:
>
> Hmm, I did think about that, but I felt overriding the functions was
> just cleaner: I felt it was more obvious what it was doing. Doing
> exactly this sort of tweaking was one of the main reasons we added
> that function overriding feature.
>
> I don't like the idea of returning "error locked by FE" since that'll
> mean the tuning will randomly fail sometimes in a way visible to
> userspace (unless we change the core dvb_frontend code), which was one
> of the things I was trying to avoid. Unless, of course, I've
> misunderstood your proposal.

Not returning error, but waiting in lock like that:
if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
   return -EAGAIN;

> However, looking at the code again, I realise it is possible to
> simplify it. Since its only the demod gates that cause a problem, we
> only /actually/ need to lock the get_frontend() and set_frontend()
> calls.

I don't understand why .get_frontend() causes problem, since it does not 
access tuner at all. It only reads demod registers. The main problem is 
(like schema in af9015.c shows) that there is two tuners on same I2C bus 
using same address. And demod gate is only way to open access for 
desired tuner only.

You should block traffic based of tuner not demod. And I think those 
callbacks which are needed for override are tuner driver callbacks. 
Consider situation device goes it v4l-core calls same time both tuner 
.sleep() == problem.

Antti
-- 
http://palosaari.fi/