Oliver Endriss wrote: > 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. Yes, I'm aware of that. Maybe the better is to patch the KABI to have two separate functions, or to have a separate ops to handle the tuner mutex. > >> ... >> 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. Ok, thanks. Cheers, Mauro. -- 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
