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

Message ID 20230428102728.51956-2-denis.pryazhennikov@arknetworks.am (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series improve FEC API usage |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Denis Pryazhennikov April 28, 2023, 10:27 a.m. UTC
  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(-)
  

Comments

Ferruh Yigit May 2, 2023, 2:57 p.m. UTC | #1
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.
  

Patch

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.
+ *   If only the AUTO bit is set, the decision on which FEC
+ *   mode to use will be made by HW/FW or driver.
+ *   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.
+ *   If AUTO bit is clear, specify FEC mode to be used
+ *   (only one valid mode per speed may be set).
+ *   NOFEC will be used if specified FEC modes are not
+ *   supported.
  * @return
  *   - (0) if successful.
- *   - (-EINVAL) if the FEC mode is not valid.
+ *   - (-EINVAL) if *fec_capa* is not valid.
  *   - (-ENOTSUP) if underlying hardware OR driver doesn't support.
  *   - (-EIO) if device is removed.
  *   - (-ENODEV)  if *port_id* invalid.