Hi Kamil,
Here is my review:
On 04/29/2015 12:02 PM, Kamil Debski wrote:
> From: Hans Verkuil <hansverk@xxxxxxxxx>
>
> The added HDMI CEC framework provides a generic kernel interface for
> HDMI CEC devices.
>
> Signed-off-by: Hans Verkuil <hansverk@xxxxxxxxx>
> [k.debski@xxxxxxxxxxx: Merged CEC Updates commit by Hans Verkuil]
> [k.debski@xxxxxxxxxxx: Merged Update author commit by Hans Verkuil]
> [k.debski@xxxxxxxxxxx: change kthread handling when setting logical
> address]
> [k.debski@xxxxxxxxxxx: code cleanup and fixes]
> [k.debski@xxxxxxxxxxx: add missing CEC commands to match spec]
> [k.debski@xxxxxxxxxxx: add RC framework support]
> [k.debski@xxxxxxxxxxx: move and edit documentation]
> [k.debski@xxxxxxxxxxx: add vendor id reporting]
> [k.debski@xxxxxxxxxxx: add possibility to clear assigned logical
> addresses]
> [k.debski@xxxxxxxxxxx: documentation fixes, clenaup and expansion]
> [k.debski@xxxxxxxxxxx: reorder of API structs and add reserved fields]
> [k.debski@xxxxxxxxxxx: fix handling of events and fix 32/64bit timespec
> problem]
> [k.debski@xxxxxxxxxxx: add cec.h to include/uapi/linux/Kbuild]
> [k.debski@xxxxxxxxxxx: add sequence number handling]
> [k.debski@xxxxxxxxxxx: add passthrough mode]
> [k.debski@xxxxxxxxxxx: fix CEC defines, add missing CEC 2.0 commands]
> [k.debski@xxxxxxxxxxx: add DocBook documentation by Hans Verkuil, with
> minor additions]
> Signed-off-by: Kamil Debski <k.debski@xxxxxxxxxxx>
> ---
> 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
--
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
