Re: [PATCH v4 06/10] cec: add HDMI CEC framework

[Hans Verkuil <hverkuil@xs4all.nl>]

On 04/27/2015 12:25 PM, Hans Verkuil wrote:
> On 04/23/2015 03:03 PM, Kamil Debski wrote:
>> From: Hans Verkuil <hansverk@cisco.com>
>>
>> The added HDMI CEC framework provides a generic kernel interface for
>> HDMI CEC devices.
>>
>> Signed-off-by: Hans Verkuil <hansverk@cisco.com>
>> [k.debski@samsung.com: Merged CEC Updates commit by Hans Verkuil]
>> [k.debski@samsung.com: Merged Update author commit by Hans Verkuil]
>> [k.debski@samsung.com: change kthread handling when setting logical
>> address]
>> [k.debski@samsung.com: code cleanup and fixes]
>> [k.debski@samsung.com: add missing CEC commands to match spec]
>> [k.debski@samsung.com: add RC framework support]
>> [k.debski@samsung.com: move and edit documentation]
>> [k.debski@samsung.com: add vendor id reporting]
>> [k.debski@samsung.com: add possibility to clear assigned logical
>> addresses]
>> [k.debski@samsung.com: documentation fixes, clenaup and expansion]
>> [k.debski@samsung.com: reorder of API structs and add reserved fields]
>> [k.debski@samsung.com: fix handling of events and fix 32/64bit timespec
>> problem]
>> [k.debski@samsung.com: add cec.h to include/uapi/linux/Kbuild]
>> Signed-off-by: Kamil Debski <k.debski@samsung.com>
>> ---
>>  Documentation/cec.txt     |  396 ++++++++++++++++
>>  drivers/media/Kconfig     |    6 +
>>  drivers/media/Makefile    |    2 +
>>  drivers/media/cec.c       | 1161 +++++++++++++++++++++++++++++++++++++++++++++
>>  include/media/cec.h       |  140 ++++++
>>  include/uapi/linux/Kbuild |    1 +
>>  include/uapi/linux/cec.h  |  303 ++++++++++++
>>  7 files changed, 2009 insertions(+)
>>  create mode 100644 Documentation/cec.txt
>>  create mode 100644 drivers/media/cec.c
>>  create mode 100644 include/media/cec.h
>>  create mode 100644 include/uapi/linux/cec.h
>>
> 
>> +	case CEC_S_ADAP_LOG_ADDRS: {
>> +		struct cec_log_addrs log_addrs;
>> +
>> +		if (!(adap->capabilities & CEC_CAP_LOG_ADDRS))
>> +			return -ENOTTY;
>> +		if (copy_from_user(&log_addrs, parg, sizeof(log_addrs)))
>> +			return -EFAULT;
>> +		err = cec_claim_log_addrs(adap, &log_addrs, true);
> 
> Currently CEC_S_ADAP_LOG_ADDRS is always blocking, but since we have CEC_EVENT_READY
> I think it makes sense to just return in non-blocking mode and have cec_claim_log_addrs
> generate CEC_EVENT_READY when done. Userspace can then call G_ADAP_LOG_ADDRS to discover
> the result.
> 
> What do you think?
> 

On a related topic: non-blocking behavior for CEC_RECEIVE is well defined, but for
CEC_TRANSMIT it isn't. If reply == 0, then we need a way to inform userspace that
the transmit finished (with a possible non-zero status code). An event would be
suitable for that, but we would need a way to associate a transmit message with
the event.

One possibility might be to have the CEC framework assign a sequence number to
a transmit message which is returned by CEC_TRANSMIT and used in the event.

If reply != 0, then I think the received message should be queued up in the
receive queue, but with a non-zero reply field and with the sequence number of the
transmit message it is a reply of.

Regards,

	Hans