Hi Kamil,
Here is my review:
On 04/29/2015 12:02 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] [k.debski@samsung.com: add sequence number handling] [k.debski@samsung.com: add passthrough mode] [k.debski@samsung.com: fix CEC defines, add missing CEC 2.0 commands] [k.debski@samsung.com: add DocBook documentation by Hans Verkuil, with minor additions] 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 | 1200 +++++++++++++++++++++++++++++++++++++++++++++ include/media/cec.h | 142 ++++++ include/uapi/linux/Kbuild | 1 + include/uapi/linux/cec.h | 337 +++++++++++++ 7 files changed, 2084 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
diff --git a/Documentation/cec.txt b/Documentation/cec.txt new file mode 100644 index 0000000..2b6c08a --- /dev/null +++ b/Documentation/cec.txt
<snip>
+The following capabilities are defined:
+/* Userspace has to configure the adapter state (enable/disable) */ +#define CEC_CAP_STATE (1 << 0) +/* Userspace has to configure the physical address */ +#define CEC_CAP_PHYS_ADDR (1 << 1) +/* Userspace has to configure the logical addresses */ +#define CEC_CAP_LOG_ADDRS (1 << 2) +/* Userspace can transmit messages */ +#define CEC_CAP_TRANSMIT (1 << 3) +/* Userspace can receive messages */ +#define CEC_CAP_RECEIVE (1 << 4) +/* Userspace has to configure the vendor id */ +#define CEC_CAP_VENDOR_ID (1 << 5) +/* The hardware has the possibility to work in the promiscuous mode */ +#define CEC_CAP_PROMISCUOUS (1 << 6)
CAP_PROMISCUOUS doesn't exist anymore, this can be removed.
Instead add the PASSTHROUGH capability.
Frankly, I wonder if this cec.txt document should document the userspace API at all now that that is part of DocBook. I think it is better if it only describes the kernel API and just refers to the DocBook for the userspace parts.
CEC_G/S_PASSTHROUGH aren't described here as well, which only shows that the userspace API should just be dropped.
<snip>
diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
<snip>
+/* The hardware has the possibility to work in the passthrough */
s/the passthrough/passthrough mode/
+#define CEC_CAP_PASSTHROUGH (1 << 6)
<snip>
+#define CEC_G_EVENT _IOWR('a', 9, struct cec_event) +/*
- Read and set the vendor ID of the CEC adapter.
- */
+#define CEC_G_VENDOR_ID _IOR('a', 9, __u32)
9 -> 10, same for the following ioctls, all need to be increased by 1 since current G_EVENT and G_VENDOR_ID have the same ioctl number.
+#define CEC_S_VENDOR_ID _IOW('a', 10, __u32) +/*
- Enable/disable the passthrough mode
- */
+#define CEC_G_PASSTHROUGH _IOR('a', 11, __u32) +#define CEC_S_PASSTHROUGH _IOW('a', 12, __u32)
+#endif
Regards,
Hans