[PATCH v2 03/11] eventdev: update documentation on device capability flags

[Mattias Rönnblom]

On 2024-01-31 15:09, Bruce Richardson wrote:
> On Tue, Jan 23, 2024 at 10:18:53AM +0100, Mattias Rönnblom wrote:
>> On 2024-01-19 18:43, Bruce Richardson wrote:
>>> Update the device capability docs, to:
>>>
>>> * include more cross-references
>>> * split longer text into paragraphs, in most cases with each flag having
>>>     a single-line summary at the start of the doc block
>>> * general comment rewording and clarification as appropriate
>>>
>>> Signed-off-by: Bruce Richardson <bruce.richardson at intel.com>
>>> ---
>>>    lib/eventdev/rte_eventdev.h | 130 ++++++++++++++++++++++++++----------
>>>    1 file changed, 93 insertions(+), 37 deletions(-)
>>>
> <snip>
>>>     * If this capability is not set, the queue only supports events of the
>>> - *  *RTE_SCHED_TYPE_* type that it was created with.
>>> + * *RTE_SCHED_TYPE_* type that it was created with.
>>> + * Any events of other types scheduled to the queue will handled in an
>>> + * implementation-dependent manner. They may be dropped by the
>>> + * event device, or enqueued with the scheduling type adjusted to the
>>> + * correct/supported value.
>>
>> Having the application setting sched_type when it was already set on a the
>> level of the queue never made sense to me.
>>
>> I can't see any reasons why this field shouldn't be ignored by the event
>> device on non-RTE_EVENT_QUEUE_CFG_ALL_TYPES queues.
>>
>> If the behavior is indeed undefined, I think it's better to just say
>> "undefined" rather than the above speculation.
>>
> 
> Updating in v3 to just say it's undefined.
> 
>>>     *
>>> - * @see RTE_SCHED_TYPE_* values
> <snip>
>>>    #define RTE_EVENT_DEV_CAP_RUNTIME_QUEUE_ATTR (1ULL << 11)
>>>    /**< Event device is capable of changing the queue attributes at runtime i.e
>>> - * after rte_event_queue_setup() or rte_event_start() call sequence. If this
>>> - * flag is not set, eventdev queue attributes can only be configured during
>>> + * after rte_event_queue_setup() or rte_event_dev_start() call sequence.
>>> + *
>>> + * If this flag is not set, eventdev queue attributes can only be configured during
>>>     * rte_event_queue_setup().
>>
>> "event queue" or just "queue".
>>
> Ack.
> 
>>> + *
>>> + * @see rte_event_queue_setup
>>>     */
>>>    #define RTE_EVENT_DEV_CAP_PROFILE_LINK (1ULL << 12)
>>> -/**< Event device is capable of supporting multiple link profiles per event port
>>> - * i.e., the value of `rte_event_dev_info::max_profiles_per_port` is greater
>>> - * than one.
>>> +/**< Event device is capable of supporting multiple link profiles per event port.
>>> + *
>>> + *
>>> + * When set, the value of `rte_event_dev_info::max_profiles_per_port` is greater
>>> + * than one, and multiple profiles may be configured and then switched at runtime.
>>> + * If not set, only a single profile may be configured, which may itself be
>>> + * runtime adjustable (if @ref RTE_EVENT_DEV_CAP_RUNTIME_PORT_LINK is set).
>>> + *
>>> + * @see rte_event_port_profile_links_set rte_event_port_profile_links_get
>>> + * @see rte_event_port_profile_switch
>>> + * @see RTE_EVENT_DEV_CAP_RUNTIME_PORT_LINK
>>>     */
>>>    /* Event device priority levels */
>>>    #define RTE_EVENT_DEV_PRIORITY_HIGHEST   0
>>> -/**< Highest priority expressed across eventdev subsystem
>>> +/**< Highest priority expressed across eventdev subsystem.
>>
>> "The highest priority an event device may support."
>> or
>> "The highest priority any event device may support."
>>
>> Maybe this is a further improvement, beyond punctuation? "across eventdev
>> subsystem" sounds awkward.
>>
> 
> Still not very clear. Talking about device support implies that its
> possible some devices may not support it. How about:
>  > "highest priority level for events and queues".
> 

Sounds good. I guess it's totally, 100% obvious highest means most urgent?

Otherwise, "highest (i.e., most urgent) priority level for events queues"