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.
I bet that their coccinelle rules will generate some false alarms with the current code.
> Intention of the patch is to make the tuner access exclusive. Thats the
> reason why the mutex is locked when gate is opened and unlocked when the
> gate is closed.
>
> In case the opening fails the close call is not made und thus mutex is
> not unlocked twice.
>
> There one problem pending with: when there is an error during "tuner
> access" the gate will not be closed and thus the mutex will not be
> unlocked. For this case a patch from Oliver Endriss is attached.
After the patch, the lock is fine, but we still need to better document it,
in order to save time from reviewers and janitors.
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
