Re: PULL http://jusst.de/hg/stv090x

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux