Re: [PATCHv2 08/13] Documentation: media: document EDID_PRESENT cap and flag.

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: cec: docs reference undefined EDID_PRESENT uAPI macros
--

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

Documentation: media: document EDID_PRESENT cap and flag.

This commit adds documentation for the CEC_CAP_HAS_EDID_PRESENT capability
and the CEC_LOG_ADDRS_FL_EDID_PRESENT flag to the CEC uAPI documentation.

> diff --git a/Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst b/Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst
> index 1d5248979a6da..abd62ac64541b 100644
> --- a/Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst
> +++ b/Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst
> @@ -143,6 +143,15 @@ returns the information to the application. The ioctl never fails.
>        - 0x00000200
>        - If this capability is set, then
>          :ref:`CEC_MSG_FL_REPLY_VENDOR_ID <cec-msg-flags>` can be used.
> +    * .. _`CEC-CAP-HAS-EDID-PRESENT`:
> +
> +      - ``CEC_CAP_HAS_EDID_PRESENT``
> +      - 0x00000400
> +      - If this capability is set, then
> +        :ref:`CEC_LOG_ADDRS_FL_EDID_PRESENT <cec-log-addrs-flags>` is valid.
> +	This makes it possible to tell apart the case where the Physical Address
> +	is f.f.f.f because no EDID is present, and the case where it is f.f.f.f
> +	because the EDID does not provide a valid Physical Address.
>  
>  Return Value
>  ============
> diff --git a/Documentation/userspace-api/media/cec/cec-ioc-adap-g-log-addrs.rst b/Documentation/userspace-api/media/cec/cec-ioc-adap-g-log-addrs.rst
> index 8397bf5737988..2088793b1506c 100644
> --- a/Documentation/userspace-api/media/cec/cec-ioc-adap-g-log-addrs.rst
> +++ b/Documentation/userspace-api/media/cec/cec-ioc-adap-g-log-addrs.rst
> @@ -192,6 +192,15 @@ logical address types are already defined will return with error ``EBUSY``.
>        - If this flag is set, then the CEC device failed to claim a free logical
>          address and is in the unconfigured state. This can never happen if
>  	``CEC_LOG_ADDRS_FL_ALLOW_UNREG_FALLBACK`` was set as well.
> +    * .. _`CEC-LOG-ADDRS-FL-EDID-PRESENT`:
> +
> +      - ``CEC_LOG_ADDRS_FL_EDID_PRESENT``
> +      - 16
> +      - If this flag is set, then an EDID is present. This helps tell apart the
> +        case where the Physical Address is f.f.f.f because no EDID is present,
> +	and the case where it is f.f.f.f because the EDID does not provide a
> +	valid Physical Address. This flag can only be used if
> +	``CEC_CAP_HAS_EDID_PRESENT`` is set in the adapter capabilities.

[Severity: Medium]
Are the corresponding uAPI definitions and kernel implementation for
CEC_CAP_HAS_EDID_PRESENT and CEC_LOG_ADDRS_FL_EDID_PRESENT missing from the
tree?

In include/uapi/linux/cec.h, the capability definitions currently stop at
CEC_CAP_REPLY_VENDOR_ID:

    /* CEC_ADAP_G_CONNECTOR_INFO is available */
    #define CEC_CAP_CONNECTOR_INFO	(1 << 8)
    /* CEC_MSG_FL_REPLY_VENDOR_ID is available */
    #define CEC_CAP_REPLY_VENDOR_ID	(1 << 9)

And the struct cec_log_addrs flag definitions stop at
CEC_LOG_ADDRS_FL_CONFIG_FAILED:

    /* CDC-Only device: supports only CDC messages */
    #define CEC_LOG_ADDRS_FL_CDC_ONLY		(1 << 2)
    /* Configuration failed */
    #define CEC_LOG_ADDRS_FL_CONFIG_FAILED		(1 << 3)

Neither symbol appears to be defined in include/uapi/linux/cec.h or handled
anywhere in drivers/media/cec/ (where cec-api.c still masks log_addrs.flags
to CEC_LOG_ADDRS_FL_ALLOW_UNREG_FALLBACK | CEC_LOG_ADDRS_FL_ALLOW_RC_PASSTHRU
| CEC_LOG_ADDRS_FL_CDC_ONLY). Because
Documentation/userspace-api/media/cec/cec-header.rst also parses
include/uapi/linux/cec.h to generate cross-references, the generated header
documentation is out of sync with these ioctl pages.

Also, in Documentation/userspace-api/media/cec/cec-ioc-adap-g-log-addrs.rst,
should the description clarify whether CEC_LOG_ADDRS_FL_EDID_PRESENT is a
read-only flag set by the driver or accepted by CEC_ADAP_S_LOG_ADDRS?

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