Looking at this patch because I have been alerted about the ABI compat handling.
I see some details that should have been caught in earlier reviews.
30/09/2022 20:46, Nicolas Chautru:
> +/*
> + * Maximum size to be used to manage the enum rte_bbdev_enqueue_status
> including padding for future
This line is long.
It is always better to split lines logically,
for instance here, before "including".
> + * enum insertion
It could be made clear that the real enum size is smaller or equal.
> + */
> +#define RTE_BBDEV_ENQ_STATUS_SIZE_MAX 6
[...]
> +enum rte_bbdev_enqueue_status {
> + RTE_BBDEV_ENQ_STATUS_NONE, /**< Nothing to report */
> + RTE_BBDEV_ENQ_STATUS_QUEUE_FULL, /**< Not enough room in queue */
> + RTE_BBDEV_ENQ_STATUS_RING_FULL, /**< Not enough room in ring */
> + RTE_BBDEV_ENQ_STATUS_INVALID_OP, /**< Operation was rejected as
> invalid */
> +};
A comment is missing at the end of the enum to remind updating the MAX.
But the big question is why do we need this "MAX" value?
The guideline is to avoid using such MAX value for long term compatibility.
[...]
> +/**
> + * Converts queue status from enum to string
Should be imperative form: "Convert".
A dot is missing at the end of the sentence.
> + *
> + * @param status
> + * Queue status as enum
> + *
> + * @returns
> + * Queue status as string or NULL if op_type is invalid
It is not aligned with above parameter.
Choose an indentation format and keep it consistent.
[...]
> # added in 22.11
> rte_bbdev_device_status_str;
> + rte_bbdev_enqueue_status_str;
> rte_bbdev_enqueue_fft_ops;
> rte_bbdev_dequeue_fft_ops;
It is not alphabetical order.
