Re: [Y2038] [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question
On Wednesday 06 May 2015 17:58:01 Hans Verkuil wrote: On 05/04/2015 12:14 PM, Arnd Bergmann wrote: On Monday 04 May 2015 09:42:36 Hans Verkuil wrote: Ping! (Added Arnd to the CC list) Hi Hans, sorry I missed this the first time On 04/27/2015 09:40 AM, Hans Verkuil wrote: Added the y2038 mailinglist since I would like to get their input for this API. Y2038 experts, can you take a look at my comment in the code below? Thanks! Arnd, I just saw your patch series adding struct __kernel_timespec to uapi/linux/time.h. I get the feeling that it might take a few kernel cycles before we have a timespec64 available in userspace. Based on that I think this CEC API should drop the timestamps for now and wait until timespec64 becomes available before adding it. The timestamps are a nice-to-have, but not critical. So adding it later shouldn't be a problem. What is your opinion? It will take a little while for the patches to make it in, I would guess 4.3 at the earliest. Using your own struct works just as well and would be less ambiguous. However, for timestamps, I would recommend not using timespec anyway. Instead, just use a single 64-bit nanosecond value from ktime_get_ns() (or ktime_get_boot_ns() if you need a time that keeps ticking across suspend). This is more efficient to get and simpler to use as long as you don't need to convert from nanosecond to timespec. Possibly stupid follow-up question: is ktime_get_ns() just a different representation as ktime_get_ts64()? Yes. Or is there some offset between the two? They seem to be identical based on a quick test, but I'd like to be certain that that's always the case. Users need to be able to relate this timestamp to a struct timespec as returned by V4L2 (and others). * ktime_get_ns() uses the same timebase as ktime_get_ts64(). * ktime_get_boot_ns() uses the same timebase as ktime_get_boottime() or getboottime64(), which differs from the first after suspend * ktime_get_real_ns() uses the same time as gettimeofday() in user space, which is always different from the other two. Arnd -- 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
Re: [Y2038] [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question
Hi Arnd, On 05/04/2015 12:14 PM, Arnd Bergmann wrote: On Monday 04 May 2015 09:42:36 Hans Verkuil wrote: Ping! (Added Arnd to the CC list) Hi Hans, sorry I missed this the first time On 04/27/2015 09:40 AM, Hans Verkuil wrote: Added the y2038 mailinglist since I would like to get their input for this API. Y2038 experts, can you take a look at my comment in the code below? Thanks! Arnd, I just saw your patch series adding struct __kernel_timespec to uapi/linux/time.h. I get the feeling that it might take a few kernel cycles before we have a timespec64 available in userspace. Based on that I think this CEC API should drop the timestamps for now and wait until timespec64 becomes available before adding it. The timestamps are a nice-to-have, but not critical. So adding it later shouldn't be a problem. What is your opinion? It will take a little while for the patches to make it in, I would guess 4.3 at the earliest. Using your own struct works just as well and would be less ambiguous. However, for timestamps, I would recommend not using timespec anyway. Instead, just use a single 64-bit nanosecond value from ktime_get_ns() (or ktime_get_boot_ns() if you need a time that keeps ticking across suspend). This is more efficient to get and simpler to use as long as you don't need to convert from nanosecond to timespec. Possibly stupid follow-up question: is ktime_get_ns() just a different representation as ktime_get_ts64()? Or is there some offset between the two? They seem to be identical based on a quick test, but I'd like to be certain that that's always the case. Users need to be able to relate this timestamp to a struct timespec as returned by V4L2 (and others). Thanks! Hans -- 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
Re: [Y2038] [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question
Ping! (Added Arnd to the CC list)
On 04/27/2015 09:40 AM, Hans Verkuil wrote:
Added the y2038 mailinglist since I would like to get their input for
this API.
Y2038 experts, can you take a look at my comment in the code below?
Thanks!
Arnd, I just saw your patch series adding struct __kernel_timespec to
uapi/linux/time.h. I get the feeling that it might take a few kernel
cycles before we have a timespec64 available in userspace. Based on that
I think this CEC API should drop the timestamps for now and wait until
timespec64 becomes available before adding it.
The timestamps are a nice-to-have, but not critical. So adding it later
shouldn't be a problem. What is your opinion?
Hans
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
snip
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 4842a98..5854cfd 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -81,6 +81,7 @@ header-y += capi.h
header-y += cciss_defs.h
header-y += cciss_ioctl.h
header-y += cdrom.h
+header-y += cec.h
header-y += cgroupstats.h
header-y += chio.h
header-y += cm4000_cs.h
diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
new file mode 100644
index 000..bb6d66c
--- /dev/null
+++ b/include/uapi/linux/cec.h
@@ -0,0 +1,303 @@
+#ifndef _CEC_H
+#define _CEC_H
+
+#include linux/types.h
+
+struct cec_time {
+__u64 sec;
+__u64 nsec;
+};
I don't like having to introduce a new struct for time here. Basically we are
only doing this because there is still no public struct timespec64.
When will that struct become available for use in a public API? If it is 4.2,
then we can start using it. If it will take longer, then what alternative can
we use to prevent having to change the API later?
One alternative might be to drop it for now and just reserve space in the
structs to add it later.
Input from the y2038 experts will be welcome!
Regards,
Hans
+
+struct cec_msg {
+struct cec_time ts;
+__u32 len;
+__u32 status;
+__u32 timeout;
+/* timeout (in ms) is used to timeout CEC_RECEIVE.
+ Set to 0 if you want to wait forever. */
+__u8 msg[16];
+__u8 reply;
+/* If non-zero, then wait for a reply with this opcode.
+ If there was an error when sending the msg or FeatureAbort
+ was returned, then reply is set to 0.
+ If reply is non-zero upon return, then len/msg are set to
+ the received message.
+ If reply is zero upon return and status has the
+ CEC_TX_STATUS_FEATURE_ABORT bit set, then len/msg are set to the
+ received feature abort message.
+ If reply is zero upon return and status has the
+ CEC_TX_STATUS_REPLY_TIMEOUT
+ bit set, then no reply was seen at all.
+ This field is ignored with CEC_RECEIVE.
+ If reply is non-zero for CEC_TRANSMIT and the message is a broadcast,
+ then -EINVAL is returned.
+ if reply is non-zero, then timeout is set to 1000 (the required
+ maximum response time).
+ */
+__u8 reserved[31];
+};
+
+static inline __u8 cec_msg_initiator(const struct cec_msg *msg)
+{
+return msg-msg[0] 4;
+}
+
+static inline __u8 cec_msg_destination(const struct cec_msg *msg)
+{
+return msg-msg[0] 0xf;
+}
+
+static inline bool
Re: [Y2038] [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question
On Monday 04 May 2015 09:42:36 Hans Verkuil wrote: Ping! (Added Arnd to the CC list) Hi Hans, sorry I missed this the first time On 04/27/2015 09:40 AM, Hans Verkuil wrote: Added the y2038 mailinglist since I would like to get their input for this API. Y2038 experts, can you take a look at my comment in the code below? Thanks! Arnd, I just saw your patch series adding struct __kernel_timespec to uapi/linux/time.h. I get the feeling that it might take a few kernel cycles before we have a timespec64 available in userspace. Based on that I think this CEC API should drop the timestamps for now and wait until timespec64 becomes available before adding it. The timestamps are a nice-to-have, but not critical. So adding it later shouldn't be a problem. What is your opinion? It will take a little while for the patches to make it in, I would guess 4.3 at the earliest. Using your own struct works just as well and would be less ambiguous. However, for timestamps, I would recommend not using timespec anyway. Instead, just use a single 64-bit nanosecond value from ktime_get_ns() (or ktime_get_boot_ns() if you need a time that keeps ticking across suspend). This is more efficient to get and simpler to use as long as you don't need to convert from nanosecond to timespec. Arnd -- 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
Re: [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question
Added the y2038 mailinglist since I would like to get their input for
this API.
Y2038 experts, can you take a look at my comments in the code below?
Thanks!
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
snip
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 4842a98..5854cfd 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -81,6 +81,7 @@ header-y += capi.h
header-y += cciss_defs.h
header-y += cciss_ioctl.h
header-y += cdrom.h
+header-y += cec.h
header-y += cgroupstats.h
header-y += chio.h
header-y += cm4000_cs.h
diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
new file mode 100644
index 000..bb6d66c
--- /dev/null
+++ b/include/uapi/linux/cec.h
@@ -0,0 +1,303 @@
+#ifndef _CEC_H
+#define _CEC_H
+
+#include linux/types.h
+
+struct cec_time {
+ __u64 sec;
+ __u64 nsec;
+};
I don't like having to introduce a new struct for time here. Basically we are
only doing this because there is still no public struct timespec64.
When will that struct become available for use in a public API? If it is 4.2,
then we can start using it. If it will take longer, then what alternative can
we use to prevent having to change the API later?
One alternative might be to drop it for now and just reserve space in the
structs to add it later.
Input from the y2038 experts will be welcome!
Regards,
Hans
+
+struct cec_msg {
+ struct cec_time ts;
+ __u32 len;
+ __u32 status;
+ __u32 timeout;
+ /* timeout (in ms) is used to timeout CEC_RECEIVE.
+Set to 0 if you want to wait forever. */
+ __u8 msg[16];
+ __u8 reply;
+ /* If non-zero, then wait for a reply with this opcode.
+If there was an error when sending the msg or FeatureAbort
+was returned, then reply is set to 0.
+If reply is non-zero upon return, then len/msg are set to
+the received message.
+If reply is zero upon return and status has the
+CEC_TX_STATUS_FEATURE_ABORT bit set, then len/msg are set to the
+received feature abort message.
+If reply is zero upon return and status has the
+CEC_TX_STATUS_REPLY_TIMEOUT
+bit set, then no reply was seen at all.
+This field is ignored with CEC_RECEIVE.
+If reply is non-zero for CEC_TRANSMIT and the message is a broadcast,
+then -EINVAL is returned.
+if reply is non-zero, then timeout is set to 1000 (the required
+maximum response time).
+ */
+ __u8 reserved[31];
+};
+
+static inline __u8 cec_msg_initiator(const struct cec_msg *msg)
+{
+ return msg-msg[0] 4;
+}
+
+static inline __u8 cec_msg_destination(const struct cec_msg *msg)
+{
+ return msg-msg[0] 0xf;
+}
+
+static inline bool cec_msg_is_broadcast(const struct cec_msg *msg)
+{
+ return (msg-msg[0] 0xf) == 0xf;
+}
+
+/* cec status field */
+#define CEC_TX_STATUS_OK(0)
+#define CEC_TX_STATUS_ARB_LOST (1 0)
+#define CEC_TX_STATUS_RETRY_TIMEOUT (1 1)
+#define CEC_TX_STATUS_FEATURE_ABORT (1 2)
+#define CEC_TX_STATUS_REPLY_TIMEOUT (1 3)
+#define CEC_RX_STATUS_READY (0)
+
+#define CEC_LOG_ADDR_INVALID 0xff
+
+/* The maximum number of logical addresses one device can be assigned to.
+ * The CEC 2.0 spec allows
