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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
