On Sat, Jan 23, 2010 at 1:14 AM, Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxx> wrote: > Devin Heitmueller wrote: >> On Fri, Jan 22, 2010 at 3:22 PM, Manu Abraham <abraham.manu@xxxxxxxxx> wrote: >>> On Fri, Jan 22, 2010 at 11:40 PM, Devin Heitmueller >>> <dheitmueller@xxxxxxxxxxxxxx> wrote: >>>> Also, the dvb_frontend.c makes calls to i2c_gate_ctrl() at various >>>> points, so you would need to ensure that none of those occur before >>>> calling into your driver as there could potentially be a deadlock >>>> there too. >>> Ok, thanks for the pointer. The gate control is never called >>> externally in reality. I will wait a little while for this patch to be >>> applied. It removes the exported function and thereby an unnecessary >>> dereference. >>> >>> http://jusst.de/hg/stv090x/rev/b3d28f5b2b53 >> >> If it never needs to be called externally, then removing it from the >> dvb_frontend_ops does eliminate the risk entirely. The case I >> frequently see it called from dvb_frontend.c is for powering down the >> tuner when the dvb frontend thread shuts down. > > The removal of the external call to the gate function removes the risk that > an external call, at dvb core, to keep it locked. Yet, the code there is completely > symetric nowadays. > > However, the proper documentation of the lock is needed to avoid the risk > of some patch to keep the mutex locked. > > As, even the initial lock changeset has this problem (later fixed by > http://jusst.de/hg/stv090x/rev/3a8f35abc0f2), it shows that a good documentation > is required. > > As you've talked about a FSM, the lock itself is a FSM with 2 states. All I'm > asking is that you document that the lock FSM has changed its state in the name > of the function that alters the state of the lock. > Sure, of course, i will add some comments into the patch that I have queued up, before pull request. Don't worry, be at peace. :-) Regards, Manu -- 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
