Re: [PATCHv3 10/13] media: uapi: cec.h: add CEC_EVENT_FL_STATE_HPD_PRESENT flag

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: cec: unimplemented and undocumented CEC_EVENT_FL_STATE_HPD_PRESENT flag
--

commit 6f18f1544823dafcf8a17b474c711348256f2340
Author: Hans Verkuil <hverkuil+cisco@kernel.org>

media: uapi: cec.h: add CEC_EVENT_FL_STATE_HPD_PRESENT flag

This commit adds the CEC_EVENT_FL_STATE_HPD_PRESENT flag to
include/uapi/linux/cec.h to report whether HPD is present.

> diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
> index fdfc97a6e4ec0..d9300a6010806 100644
> --- a/include/uapi/linux/cec.h
> +++ b/include/uapi/linux/cec.h
> @@ -453,6 +453,7 @@ struct cec_connector_info {
>  
>  #define CEC_EVENT_FL_INITIAL_STATE	(1 << 0)
>  #define CEC_EVENT_FL_DROPPED_EVENTS	(1 << 1)
> +#define CEC_EVENT_FL_STATE_HPD_PRESENT	(1 << 2)

[Severity: Medium]
The commit message states that CEC_EVENT_FL_STATE_HPD_PRESENT will report
whether HPD is present or not, but nothing in the kernel ever sets or
references this flag.

When userspace dequeues CEC_EVENT_STATE_CHANGE events via CEC_DQEVENT, the
events are populated in cec_post_state_event() in
drivers/media/cec/core/cec-adap.c:

	struct cec_event ev = {
		.event = CEC_EVENT_STATE_CHANGE,
	};

and on device open in cec_open() in drivers/media/cec/core/cec-api.c:

	struct cec_event ev = {
		.event = CEC_EVENT_STATE_CHANGE,
		.flags = CEC_EVENT_FL_INITIAL_STATE,
	};

Neither path sets CEC_EVENT_FL_STATE_HPD_PRESENT in ev.flags, so any
application checking ev.flags & CEC_EVENT_FL_STATE_HPD_PRESENT will always
observe 0.

In addition, Documentation/userspace-api/media/cec/cec-ioc-dqevent.rst
(under the CEC Event Flags table) and include/uapi/linux/cec.h do not
document CEC_EVENT_FL_STATE_HPD_PRESENT or specify which CEC_EVENT_* types
carry it.

Finally, without a corresponding capability flag or tri-state indicator
(unlike struct cec_event_state_change.have_conn_info, which is gated by
CEC_CAP_CONNECTOR_INFO), how can userspace distinguish between HPD being
low, a CEC driver that does not report HPD state, and an older kernel that
predates CEC_EVENT_FL_STATE_HPD_PRESENT? Should this UAPI definition be
held back until an in-kernel user, documentation, and capability signaling
are included?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/81a832c5568748e37f081d217998f80880ab8dc4.1793863347.git.hverkuil+cisco@kernel.org?part=1