Hi Hans, Thank you so much for all today's comments. I will consider them when preparing the next version.
From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Hans Verkuil Sent: Monday, April 27, 2015 1:27 PM > > On 04/27/2015 12:25 PM, Hans Verkuil wrote: > > On 04/23/2015 03:03 PM, Kamil Debski wrote: > >> From: Hans Verkuil <hansv...@cisco.com> > >> > >> The added HDMI CEC framework provides a generic kernel interface for > >> HDMI CEC devices. > >> > >> Signed-off-by: Hans Verkuil <hansv...@cisco.com> > >> [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil] > >> [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil] > >> [k.deb...@samsung.com: change kthread handling when setting logical > >> address] > >> [k.deb...@samsung.com: code cleanup and fixes] > >> [k.deb...@samsung.com: add missing CEC commands to match spec] > >> [k.deb...@samsung.com: add RC framework support] > >> [k.deb...@samsung.com: move and edit documentation] > >> [k.deb...@samsung.com: add vendor id reporting] > >> [k.deb...@samsung.com: add possibility to clear assigned logical > >> addresses] > >> [k.deb...@samsung.com: documentation fixes, clenaup and expansion] > >> [k.deb...@samsung.com: reorder of API structs and add reserved > >> fields] > >> [k.deb...@samsung.com: fix handling of events and fix 32/64bit > >> timespec problem] > >> [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild] > >> Signed-off-by: Kamil Debski <k.deb...@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? > > I am looking into this now. If I see this correctly this involves: - adding cec_post_event(cla_int->adap, CEC_EVENT_READY); to cec_config_thread_func - adding O_NONBLOCK check in CEC_S_ADAP_LOG_ADDRS Right? > On a related topic: non-blocking behavior for CEC_RECEIVE is well > defined, but for CEC_TRA NSMIT 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. A sequence number is a good solution, I believe. To recap: - a sequence number should be set by the framework and returned in the CEC_TRANSMIT ioctl - a new event should be added CEC_EVENT_TX_DONE and it should be posted on each transmission finish - event struct has to include a sequence field as well Is this ok? > > Regards, > > Hans Best wishes, -- Kamil Debski Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
