Oliver Endriss wrote:
> Mauro Carvalho Chehab wrote:
>> Oliver Endriss wrote:
>>> Manu Abraham wrote:
>>>> On Mon, Jan 11, 2010 at 12:17 AM, Manu Abraham <abraham.m...@gmail.com>
>>>> wrote:
>>>>> Mauro,
>>>>>
>>>>>
>>>>> Following the changesets 13830 - 13894 from my earlier pull request,
>>>> Correction, 13880 - 13894 inclusive of both.
>>>>
>>>>> Please pull the following changeset as well
>>>>>
>>>>>
>>>>> http://jusst.de/hg/stv090x/rev/80c74966d955
>>>>>
>>>>>
>>>>> It fixes a nasty bug as described at
>>>>> http://osdir.com/ml/linux-media/2009-11/msg00643.html
>>>>>
>>>>>
>>>>> Regards,
>>>>> Manu
>>> Mauro,
>>>
>>> when will you pull-in these changesets?
>>> You are blocking the submission of the ngene driver.
>> Hi Oliver,
>>
>> Since I returned from vacations this week, I had a pile of tasks to
>> handle, both at the subsystem and at the work.
>>
>> Unfortunately, having to deal with both -hg and -git eats lot of my
>> time, to be sure that nothing is missed. This time, I had to re-generate
>> all my local -git trees that somehow had loose some patches in the process.
>> Due to that, the patch merge is taking longer than I've expected.
>>
>> I still have a few issues pending at the subsystem, including this stv090x
>> pull request, the omap pull (that it is very complex, as it requires both
>> -hg and -git patch insertions), lnbh24 fix, my own proposals to dvb-apps,
>> and two upstream submissions.
>>
>> My intention is to finish those tasks during this weekend if time
>> allows me to do everything.
>>
>> In the specific case of stv090x, I intend to review again the locks,
>> in the light of your comments.
>
> I just wondered why you did not pull-in these changesets.
I finished reviewing the locking schema and it seemed OK with your fixes.
Still, I think that the i2c_gate_control function is poorly documented. Somehow,
it should be commented that the lock is taken when enable=1 and is dropped when
enable=0, warning anyone that may be analyzing the code about that, especially
since the lock function is exported to dvb core. It is not common to use a
locking schema like that, where the information that the code is locked is
hidden.
IMO, the better way would be to split the function in two, like:
i2c_gate_enable_lock(struct dvb_frontend *fe);
i2c_gate_disable_unlock(struct dvb_frontend *fe);
And use those two functions internally. You'll still need to have an
i2c_gate_control() function that has the locks inside, due to the calls
done at dvb_frontend.c, but the code will be better documented.
In order to not add more delay to ngene, I'll apply the stv090x patches, but
please better document the i2c_gate_control on your next patch series.
Btw, with respect to dvb-core, we'll need to do soon a lock review, since
dvbdev.c uses the BKL, and it will be removed on 2.6.34 or 2.6.35.
>
> The first lnbh24 patch was pulled so fast, that I had no chance to
> respond. This patch was submitted much later than the stv090x stuff.
> Btw, it breaks the ngene driver, so it is important to apply the
> lnbh24 fix...
Yes, it were submitted latter, but I've reviewed stv090x before lnbh24.
Anyway, it is the next on my review list.
>
> CU
> Oliver
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
