Re: [RFC PATCH 1/3] ethdev: update documentation for API to set FEC

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 4/28/2023 11:27 AM, Denis Pryazhennikov wrote:
> The documentation for the rte_eth_fec_set() is updated
> to provide more detailed information about how FEC modes are
> handled. It also includes a description of the case when only
> the AUTO bit is set.
> 
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Acked-by: Ivan Malov <ivan.malov@arknetworks.am>
> Acked-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
> ---
>  lib/ethdev/rte_ethdev.h | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 99fe9e238b20..0f10ac944061 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -4227,13 +4227,19 @@ int rte_eth_fec_get(uint16_t port_id, uint32_t *fec_capa);
>   * @param port_id
>   *   The port identifier of the Ethernet device.
>   * @param fec_capa
> - *   A bitmask of allowed FEC modes. If AUTO bit is set, other
> - *   bits specify FEC modes which may be negotiated. If AUTO
> - *   bit is clear, specify FEC modes to be used (only one valid
> - *   mode per speed may be set).
> + *   A bitmask of allowed FEC modes.

ack

> + *   If only the AUTO bit is set, the decision on which FEC
> + *   mode to use will be made by HW/FW or driver.

ack

> + *   If the AUTO bit is set, other bits specify FEC modes
> + *   which may be negotiated. It means that only specified
> + *   FEC modes can be set.

What about some simplification, maybe something like:

"
If the AUTO bit is set with some FEC modes, only specified FEC modes can
be set.
"

> + *   If AUTO bit is clear, specify FEC mode to be used
> + *   (only one valid mode per speed may be set).

ack

> + *   NOFEC will be used if specified FEC modes are not
> + *   supported.

If FEC modes are not supported, I think it is returning error, why
change it?

>   * @return
>   *   - (0) if successful.
> - *   - (-EINVAL) if the FEC mode is not valid.
> + *   - (-EINVAL) if *fec_capa* is not valid.

I think original was correct, if FEC mode is not valid, dev_ops returns
EINVAL, which cause API to return the same.

>   *   - (-ENOTSUP) if underlying hardware OR driver doesn't support.
>   *   - (-EIO) if device is removed.
>   *   - (-ENODEV)  if *port_id* invalid.