Hi, Mauro Carvalho Chehab wrote: > Hi Andreas, > > Andreas Regel wrote: > > > you don't have to look at stv090x_i2c_gate_ctrl alone. stv090x driver > > contains code like this: > > > > if (stv090x_i2c_gate_ctrl(fe, 1) < 0) > > goto err; > > > > tuner access > > > > if (stv090x_i2c_gate_ctrl(fe, 0) < 0) > > goto err; > > Ok. It is very unusual to have a lock internally like the above, since > the code becomes poorly documented. > > In the cases that we want to do things like that, it is documented like: > > if (stv090x_gate_enable_lock(fe) < 0) > goto err; > > tuner access > > if (stv090x_gate_disable_unlock(fe, 0) < 0) > goto err; > > This way, it is clear to anyone that is reviewing the code that the functions > are returning with the lock on a different state and prevent the kernel janitors > to send patches that will try to "fix" the lock. You cannot do that, because there is _one_ entry in "struct dvb_frontend_ops": .i2c_gate_ctrl = stv090x_i2c_gate_ctrl; This function must open/close the I2C gate. > ... > After the patch, the lock is fine, but we still need to better document it, > in order to save time from reviewers and janitors. I agree that some comments should be added to stv090x_i2c_gate_ctrl, explaining why the mutex is required. CU Oliver -- ---------------------------------------------------------------- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ ---------------------------------------------------------------- -- 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
