Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] media: uapi: cec.h: unimplemented EDID capability and flag stripped in core - [Low] media: uapi: cec.h: missing docs cause broken Sphinx xrefs for EDID macros --
commit c0670843ed6bd1cba28d00e51e6a79ac904407a7 Author: Hans Verkuil <[email protected]> media: uapi: cec.h: add CEC_CAP_HAS_EDID_PRESENT/CEC_LOG_ADDRS_FL_EDID_PRESENT This commit adds the CEC_CAP_HAS_EDID_PRESENT capability and the CEC_LOG_ADDRS_FL_EDID_PRESENT logical address flag to the CEC UAPI header to indicate if an EDID is present. > diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h > index fdfc97a6e4ec0..8f7c7a4cb505e 100644 > --- a/include/uapi/linux/cec.h > +++ b/include/uapi/linux/cec.h > @@ -325,27 +325,27 @@ static inline int cec_is_unconfigured(__u16 > log_addr_mask) > #define CEC_MODE_FOLLOWER_MSK 0xf0 > > /* Userspace has to configure the physical address */ > -#define CEC_CAP_PHYS_ADDR (1 << 0) > +#define CEC_CAP_PHYS_ADDR (1 << 0) > /* Userspace has to configure the logical addresses */ > -#define CEC_CAP_LOG_ADDRS (1 << 1) > +#define CEC_CAP_LOG_ADDRS (1 << 1) > /* Userspace can transmit messages (and thus become follower as well) */ > -#define CEC_CAP_TRANSMIT (1 << 2) > -/* > - * Passthrough all messages instead of processing them. > - */ > -#define CEC_CAP_PASSTHROUGH (1 << 3) > +#define CEC_CAP_TRANSMIT (1 << 2) > +/* Passthrough all messages instead of processing them. */ > +#define CEC_CAP_PASSTHROUGH (1 << 3) > /* Supports remote control */ > -#define CEC_CAP_RC (1 << 4) > +#define CEC_CAP_RC (1 << 4) > /* Hardware can monitor all messages, not just directed and broadcast. */ > -#define CEC_CAP_MONITOR_ALL (1 << 5) > +#define CEC_CAP_MONITOR_ALL (1 << 5) > /* Hardware can use CEC only if the HDMI HPD pin is high. */ > -#define CEC_CAP_NEEDS_HPD (1 << 6) > +#define CEC_CAP_NEEDS_HPD (1 << 6) > /* Hardware can monitor CEC pin transitions */ > -#define CEC_CAP_MONITOR_PIN (1 << 7) > +#define CEC_CAP_MONITOR_PIN (1 << 7) > /* CEC_ADAP_G_CONNECTOR_INFO is available */ > -#define CEC_CAP_CONNECTOR_INFO (1 << 8) > +#define CEC_CAP_CONNECTOR_INFO (1 << 8) > /* CEC_MSG_FL_REPLY_VENDOR_ID is available */ > -#define CEC_CAP_REPLY_VENDOR_ID (1 << 9) > +#define CEC_CAP_REPLY_VENDOR_ID (1 << 9) > +/* CEC_LOG_ADDRS_FL_EDID_PRESENT is available */ > +#define CEC_CAP_HAS_EDID_PRESENT (1 << 10) [Severity: Low] Does adding CEC_CAP_HAS_EDID_PRESENT and CEC_LOG_ADDRS_FL_EDID_PRESENT without corresponding documentation entries introduce broken Sphinx cross-reference warnings during make htmldocs? Documentation/userspace-api/media/cec/cec-header.rst includes include/uapi/linux/cec.h using the kernel-include directive: .. kernel-include:: include/uapi/linux/cec.h :generate-cross-refs: :exception-file: cec.h.rst.exceptions :toc: :warn-broken: ParseDataStructs.parse_file() in tools/lib/python/kdoc/parse_data_structs.py generates :ref: cross-references for every #define macro in include/uapi/linux/cec.h unless the symbol is listed in Documentation/userspace-api/media/cec/cec.h.rst.exceptions. Because CEC_CAP_HAS_EDID_PRESENT and CEC_LOG_ADDRS_FL_EDID_PRESENT are not documented in Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst and Documentation/userspace-api/media/cec/cec-ioc-adap-g-log-addrs.rst, check_missing_refs() in Documentation/sphinx/kernel_include.py reports unresolved references for std:ref:cec-cap-has-edid-present and std:ref:cec-log-addrs-fl-edid-present. > /** > * struct cec_caps - CEC capabilities structure. > @@ -405,6 +405,8 @@ struct cec_log_addrs { > #define CEC_LOG_ADDRS_FL_CDC_ONLY (1 << 2) > /* Configuration failed */ > #define CEC_LOG_ADDRS_FL_CONFIG_FAILED (1 << 3) > +/* EDID is present */ > +#define CEC_LOG_ADDRS_FL_EDID_PRESENT (1 << 4) [Severity: Medium] How are CEC_CAP_HAS_EDID_PRESENT and CEC_LOG_ADDRS_FL_EDID_PRESENT intended to be used when neither is referenced in the kernel and cec_adap_s_log_addrs() strips the flag? When userspace calls ioctl(fd, CEC_ADAP_S_LOG_ADDRS, &log_addrs), drivers/media/cec/core/cec-api.c:cec_adap_s_log_addrs() masks out CEC_LOG_ADDRS_FL_EDID_PRESENT before calling __cec_s_log_addrs(): log_addrs.flags &= CEC_LOG_ADDRS_FL_ALLOW_UNREG_FALLBACK | CEC_LOG_ADDRS_FL_ALLOW_RC_PASSTHRU | CEC_LOG_ADDRS_FL_CDC_ONLY; In addition, __cec_s_log_addrs() in drivers/media/cec/core/cec-adap.c overwrites adap->log_addrs = *log_addrs without setting or preserving CEC_LOG_ADDRS_FL_EDID_PRESENT, and no core or driver code sets CEC_CAP_HAS_EDID_PRESENT in adap->capabilities. Should the in-kernel implementation in drivers/media/cec/ accompany these UAPI definitions so that the flag is not silently cleared? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/b168e5e844ef01f26cbf5aaba92761823df83ff1.1793691124.git.hverkuil+ci...@kernel.org?part=1
