2021-02-01 05:47:08

by Hariprasad Kelam

[permalink] [raw]
Subject: [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support

From: Christina Jacob <[email protected]>

Add ethtool support to configure fec modes baser/rs and
support to fecth FEC stats from CGX as well PHY.

Configure fec mode
- ethtool --set-fec eth0 encoding rs/baser/off/auto
Query fec mode
- ethtool --show-fec eth0

Signed-off-by: Christina Jacob <[email protected]>
Signed-off-by: Sunil Goutham <[email protected]>
Signed-off-by: Hariprasad Kelam <[email protected]>
---
.../ethernet/marvell/octeontx2/nic/otx2_common.c | 20 +++
.../ethernet/marvell/octeontx2/nic/otx2_common.h | 6 +
.../ethernet/marvell/octeontx2/nic/otx2_ethtool.c | 181 ++++++++++++++++++++-
.../net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 3 +
4 files changed, 208 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
index 5ddedc3..1e67072 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
@@ -60,6 +60,19 @@ void otx2_update_lmac_stats(struct otx2_nic *pfvf)
mutex_unlock(&pfvf->mbox.lock);
}

+void otx2_update_lmac_fec_stats(struct otx2_nic *pfvf)
+{
+ struct msg_req *req;
+
+ if (!netif_running(pfvf->netdev))
+ return;
+ mutex_lock(&pfvf->mbox.lock);
+ req = otx2_mbox_alloc_msg_cgx_fec_stats(&pfvf->mbox);
+ if (req)
+ otx2_sync_mbox_msg(&pfvf->mbox);
+ mutex_unlock(&pfvf->mbox.lock);
+}
+
int otx2_update_rq_stats(struct otx2_nic *pfvf, int qidx)
{
struct otx2_rcv_queue *rq = &pfvf->qset.rq[qidx];
@@ -1492,6 +1505,13 @@ void mbox_handler_cgx_stats(struct otx2_nic *pfvf,
pfvf->hw.cgx_tx_stats[id] = rsp->tx_stats[id];
}

+void mbox_handler_cgx_fec_stats(struct otx2_nic *pfvf,
+ struct cgx_fec_stats_rsp *rsp)
+{
+ pfvf->hw.cgx_fec_corr_blks += rsp->fec_corr_blks;
+ pfvf->hw.cgx_fec_uncorr_blks += rsp->fec_uncorr_blks;
+}
+
void mbox_handler_nix_txsch_alloc(struct otx2_nic *pf,
struct nix_txsch_alloc_rsp *rsp)
{
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
index 143ae04..b3f3de9 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
@@ -204,6 +204,8 @@ struct otx2_hw {
struct otx2_drv_stats drv_stats;
u64 cgx_rx_stats[CGX_RX_STATS_COUNT];
u64 cgx_tx_stats[CGX_TX_STATS_COUNT];
+ u64 cgx_fec_corr_blks;
+ u64 cgx_fec_uncorr_blks;
u8 cgx_links; /* No. of CGX links present in HW */
u8 lbk_links; /* No. of LBK links present in HW */
};
@@ -660,6 +662,9 @@ void mbox_handler_nix_txsch_alloc(struct otx2_nic *pf,
struct nix_txsch_alloc_rsp *rsp);
void mbox_handler_cgx_stats(struct otx2_nic *pfvf,
struct cgx_stats_rsp *rsp);
+void mbox_handler_cgx_fec_stats(struct otx2_nic *pfvf,
+ struct cgx_fec_stats_rsp *rsp);
+void otx2_set_fec_stats_count(struct otx2_nic *pfvf);
void mbox_handler_nix_bp_enable(struct otx2_nic *pfvf,
struct nix_bp_cfg_rsp *rsp);

@@ -668,6 +673,7 @@ void otx2_get_dev_stats(struct otx2_nic *pfvf);
void otx2_get_stats64(struct net_device *netdev,
struct rtnl_link_stats64 *stats);
void otx2_update_lmac_stats(struct otx2_nic *pfvf);
+void otx2_update_lmac_fec_stats(struct otx2_nic *pfvf);
int otx2_update_rq_stats(struct otx2_nic *pfvf, int qidx);
int otx2_update_sq_stats(struct otx2_nic *pfvf, int qidx);
void otx2_set_ethtool_ops(struct net_device *netdev);
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
index e0199f0..e5b1a57 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
@@ -66,6 +66,8 @@ static const unsigned int otx2_n_dev_stats = ARRAY_SIZE(otx2_dev_stats);
static const unsigned int otx2_n_drv_stats = ARRAY_SIZE(otx2_drv_stats);
static const unsigned int otx2_n_queue_stats = ARRAY_SIZE(otx2_queue_stats);

+static struct cgx_fw_data *otx2_get_fwdata(struct otx2_nic *pfvf);
+
static void otx2_get_drvinfo(struct net_device *netdev,
struct ethtool_drvinfo *info)
{
@@ -128,6 +130,12 @@ static void otx2_get_strings(struct net_device *netdev, u32 sset, u8 *data)

strcpy(data, "reset_count");
data += ETH_GSTRING_LEN;
+ if (pfvf->linfo.fec) {
+ sprintf(data, "Fec Corrected Errors: ");
+ data += ETH_GSTRING_LEN;
+ sprintf(data, "Fec Uncorrected Errors: ");
+ data += ETH_GSTRING_LEN;
+ }
}

static void otx2_get_qset_stats(struct otx2_nic *pfvf,
@@ -160,11 +168,30 @@ static void otx2_get_qset_stats(struct otx2_nic *pfvf,
}
}

+static int otx2_get_phy_fec_stats(struct otx2_nic *pfvf)
+{
+ struct msg_req *req;
+ int rc = -ENOMEM;
+
+ mutex_lock(&pfvf->mbox.lock);
+ req = otx2_mbox_alloc_msg_cgx_get_phy_fec_stats(&pfvf->mbox);
+ if (!req)
+ goto end;
+
+ if (!otx2_sync_mbox_msg(&pfvf->mbox))
+ rc = 0;
+end:
+ mutex_unlock(&pfvf->mbox.lock);
+ return rc;
+}
+
/* Get device and per queue statistics */
static void otx2_get_ethtool_stats(struct net_device *netdev,
struct ethtool_stats *stats, u64 *data)
{
struct otx2_nic *pfvf = netdev_priv(netdev);
+ u64 fec_corr_blks, fec_uncorr_blks;
+ struct cgx_fw_data *rsp;
int stat;

otx2_get_dev_stats(pfvf);
@@ -183,12 +210,43 @@ static void otx2_get_ethtool_stats(struct net_device *netdev,
for (stat = 0; stat < CGX_TX_STATS_COUNT; stat++)
*(data++) = pfvf->hw.cgx_tx_stats[stat];
*(data++) = pfvf->reset_count;
+
+ /* Do not request fec stats if interface fec mode is none */
+ if (pfvf->linfo.fec == OTX2_FEC_NONE)
+ return;
+
+ fec_corr_blks = pfvf->hw.cgx_fec_corr_blks;
+ fec_uncorr_blks = pfvf->hw.cgx_fec_uncorr_blks;
+
+ rsp = otx2_get_fwdata(pfvf);
+ if (!IS_ERR(rsp) && rsp->fwdata.phy.misc.has_fec_stats &&
+ !otx2_get_phy_fec_stats(pfvf)) {
+ /* Fetch fwdata again because it's been recently populated with
+ * latest PHY FEC stats.
+ */
+ rsp = otx2_get_fwdata(pfvf);
+ if (!IS_ERR(rsp)) {
+ struct fec_stats_s *p = &rsp->fwdata.phy.fec_stats;
+
+ if (pfvf->linfo.fec == OTX2_FEC_BASER) {
+ fec_corr_blks = p->brfec_corr_blks;
+ fec_uncorr_blks = p->brfec_uncorr_blks;
+ } else {
+ fec_corr_blks = p->rsfec_corr_cws;
+ fec_uncorr_blks = p->rsfec_uncorr_cws;
+ }
+ }
+ }
+
+ *(data++) = fec_corr_blks;
+ *(data++) = fec_uncorr_blks;
}

static int otx2_get_sset_count(struct net_device *netdev, int sset)
{
struct otx2_nic *pfvf = netdev_priv(netdev);
- int qstats_count;
+ int qstats_count, fec_stats_count = 0;
+ bool if_up = netif_running(netdev);

if (sset != ETH_SS_STATS)
return -EINVAL;
@@ -196,8 +254,16 @@ static int otx2_get_sset_count(struct net_device *netdev, int sset)
qstats_count = otx2_n_queue_stats *
(pfvf->hw.rx_queues + pfvf->hw.tx_queues);

+ /* Do not show fec stats if interface fec mode is none */
+ if (!if_up || !pfvf->linfo.fec)
+ return otx2_n_dev_stats + otx2_n_drv_stats + qstats_count +
+ CGX_RX_STATS_COUNT + CGX_TX_STATS_COUNT + 1;
+
+ fec_stats_count = 2;
+ otx2_update_lmac_fec_stats(pfvf);
+
return otx2_n_dev_stats + otx2_n_drv_stats + qstats_count +
- CGX_RX_STATS_COUNT + CGX_TX_STATS_COUNT + 1;
+ CGX_RX_STATS_COUNT + CGX_TX_STATS_COUNT + 1 + fec_stats_count;
}

/* Get no of queues device supports and current queue count */
@@ -859,6 +925,115 @@ static int otx2_get_ts_info(struct net_device *netdev,
return 0;
}

+static struct cgx_fw_data *otx2_get_fwdata(struct otx2_nic *pfvf)
+{
+ struct cgx_fw_data *rsp = NULL;
+ struct msg_req *req;
+ int err = 0;
+
+ mutex_lock(&pfvf->mbox.lock);
+ req = otx2_mbox_alloc_msg_cgx_get_aux_link_info(&pfvf->mbox);
+ if (!req) {
+ mutex_unlock(&pfvf->mbox.lock);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ err = otx2_sync_mbox_msg(&pfvf->mbox);
+ if (!err) {
+ rsp = (struct cgx_fw_data *)
+ otx2_mbox_get_rsp(&pfvf->mbox.mbox, 0, &req->hdr);
+ } else {
+ rsp = ERR_PTR(err);
+ }
+
+ mutex_unlock(&pfvf->mbox.lock);
+ return rsp;
+}
+
+static int otx2_get_fecparam(struct net_device *netdev,
+ struct ethtool_fecparam *fecparam)
+{
+ struct otx2_nic *pfvf = netdev_priv(netdev);
+ struct cgx_fw_data *rsp;
+ const int fec[] = {
+ ETHTOOL_FEC_OFF,
+ ETHTOOL_FEC_BASER,
+ ETHTOOL_FEC_RS,
+ ETHTOOL_FEC_BASER | ETHTOOL_FEC_RS};
+#define FEC_MAX_INDEX 3
+ if (pfvf->linfo.fec < FEC_MAX_INDEX)
+ fecparam->active_fec = fec[pfvf->linfo.fec];
+
+ rsp = otx2_get_fwdata(pfvf);
+ if (IS_ERR(rsp))
+ return PTR_ERR(rsp);
+
+ if (rsp->fwdata.supported_fec <= FEC_MAX_INDEX) {
+ if (!rsp->fwdata.supported_fec)
+ fecparam->fec = ETHTOOL_FEC_NONE;
+ else
+ fecparam->fec = fec[rsp->fwdata.supported_fec];
+ }
+ return 0;
+}
+
+static int otx2_set_fecparam(struct net_device *netdev,
+ struct ethtool_fecparam *fecparam)
+{
+ struct otx2_nic *pfvf = netdev_priv(netdev);
+ struct mbox *mbox = &pfvf->mbox;
+ struct fec_mode *req, *rsp;
+ int err = 0, fec = 0;
+
+ switch (fecparam->fec) {
+ /* Firmware does not support AUTO mode consider it as FEC_NONE */
+ case ETHTOOL_FEC_OFF:
+ case ETHTOOL_FEC_AUTO:
+ case ETHTOOL_FEC_NONE:
+ fec = OTX2_FEC_NONE;
+ break;
+ case ETHTOOL_FEC_RS:
+ fec = OTX2_FEC_RS;
+ break;
+ case ETHTOOL_FEC_BASER:
+ fec = OTX2_FEC_BASER;
+ break;
+ default:
+ netdev_warn(pfvf->netdev, "Unsupported FEC mode: %d",
+ fecparam->fec);
+ return -EINVAL;
+ }
+
+ if (fec == pfvf->linfo.fec)
+ return 0;
+
+ mutex_lock(&mbox->lock);
+ req = otx2_mbox_alloc_msg_cgx_set_fec_param(&pfvf->mbox);
+ if (!req) {
+ err = -ENOMEM;
+ goto end;
+ }
+ req->fec = fec;
+ err = otx2_sync_mbox_msg(&pfvf->mbox);
+ if (err)
+ goto end;
+
+ rsp = (struct fec_mode *)otx2_mbox_get_rsp(&pfvf->mbox.mbox,
+ 0, &req->hdr);
+ if (rsp->fec >= 0) {
+ pfvf->linfo.fec = rsp->fec;
+ /* clear stale counters */
+ pfvf->hw.cgx_fec_corr_blks = 0;
+ pfvf->hw.cgx_fec_uncorr_blks = 0;
+ } else {
+ err = rsp->fec;
+ }
+
+end:
+ mutex_unlock(&mbox->lock);
+ return err;
+}
+
static const struct ethtool_ops otx2_ethtool_ops = {
.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
ETHTOOL_COALESCE_MAX_FRAMES,
@@ -886,6 +1061,8 @@ static const struct ethtool_ops otx2_ethtool_ops = {
.get_pauseparam = otx2_get_pauseparam,
.set_pauseparam = otx2_set_pauseparam,
.get_ts_info = otx2_get_ts_info,
+ .get_fecparam = otx2_get_fecparam,
+ .set_fecparam = otx2_set_fecparam,
};

void otx2_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index 07ec85a..d024dac 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -779,6 +779,9 @@ static void otx2_process_pfaf_mbox_msg(struct otx2_nic *pf,
case MBOX_MSG_CGX_STATS:
mbox_handler_cgx_stats(pf, (struct cgx_stats_rsp *)msg);
break;
+ case MBOX_MSG_CGX_FEC_STATS:
+ mbox_handler_cgx_fec_stats(pf, (struct cgx_fec_stats_rsp *)msg);
+ break;
default:
if (msg->rc)
dev_err(pf->dev,
--
2.7.4


2021-02-03 01:03:35

by Jesse Brandeburg

[permalink] [raw]
Subject: Re: [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support

Hariprasad Kelam wrote:

> From: Christina Jacob <[email protected]>
>
> Add ethtool support to configure fec modes baser/rs and
> support to fecth FEC stats from CGX as well PHY.
>
> Configure fec mode
> - ethtool --set-fec eth0 encoding rs/baser/off/auto
> Query fec mode
> - ethtool --show-fec eth0
>
> Signed-off-by: Christina Jacob <[email protected]>
> Signed-off-by: Sunil Goutham <[email protected]>
> Signed-off-by: Hariprasad Kelam <[email protected]>

Reviewed-by: Jesse Brandeburg <[email protected]>

2021-02-03 01:16:52

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support

On Mon, 1 Feb 2021 10:54:40 +0530 Hariprasad Kelam wrote:
> From: Christina Jacob <[email protected]>
>
> Add ethtool support to configure fec modes baser/rs and
> support to fecth FEC stats from CGX as well PHY.
>
> Configure fec mode
> - ethtool --set-fec eth0 encoding rs/baser/off/auto
> Query fec mode
> - ethtool --show-fec eth0

> + if (pfvf->linfo.fec) {
> + sprintf(data, "Fec Corrected Errors: ");
> + data += ETH_GSTRING_LEN;
> + sprintf(data, "Fec Uncorrected Errors: ");
> + data += ETH_GSTRING_LEN;

Once again, you can't dynamically hide stats. ethtool makes 3 separate
system calls - to get the number of stats, get the names, and get the
values. If someone changes the FEC config in between those user space
dumping stats will get confused.

> + }
> }

> +static int otx2_get_fecparam(struct net_device *netdev,
> + struct ethtool_fecparam *fecparam)
> +{
> + struct otx2_nic *pfvf = netdev_priv(netdev);
> + struct cgx_fw_data *rsp;
> + const int fec[] = {
> + ETHTOOL_FEC_OFF,
> + ETHTOOL_FEC_BASER,
> + ETHTOOL_FEC_RS,
> + ETHTOOL_FEC_BASER | ETHTOOL_FEC_RS};
> +#define FEC_MAX_INDEX 3
> + if (pfvf->linfo.fec < FEC_MAX_INDEX)

This should be <

> + fecparam->active_fec = fec[pfvf->linfo.fec];


> + rsp = otx2_get_fwdata(pfvf);
> + if (IS_ERR(rsp))
> + return PTR_ERR(rsp);
> +
> + if (rsp->fwdata.supported_fec <= FEC_MAX_INDEX) {
> + if (!rsp->fwdata.supported_fec)
> + fecparam->fec = ETHTOOL_FEC_NONE;
> + else
> + fecparam->fec = fec[rsp->fwdata.supported_fec];
> + }
> + return 0;
> +}
> +
> +static int otx2_set_fecparam(struct net_device *netdev,
> + struct ethtool_fecparam *fecparam)
> +{
> + struct otx2_nic *pfvf = netdev_priv(netdev);
> + struct mbox *mbox = &pfvf->mbox;
> + struct fec_mode *req, *rsp;
> + int err = 0, fec = 0;
> +
> + switch (fecparam->fec) {
> + /* Firmware does not support AUTO mode consider it as FEC_NONE */
> + case ETHTOOL_FEC_OFF:
> + case ETHTOOL_FEC_AUTO:
> + case ETHTOOL_FEC_NONE:

I _think_ NONE is for drivers to report that they don't support FEC
settings. It's an output only parameter. On input OFF should be used.

> + fec = OTX2_FEC_NONE;
> + break;
> + case ETHTOOL_FEC_RS:
> + fec = OTX2_FEC_RS;
> + break;
> + case ETHTOOL_FEC_BASER:
> + fec = OTX2_FEC_BASER;
> + break;
> + default:
> + netdev_warn(pfvf->netdev, "Unsupported FEC mode: %d",
> + fecparam->fec);
> + return -EINVAL;
> + }
> +
> + if (fec == pfvf->linfo.fec)
> + return 0;
> +
> + mutex_lock(&mbox->lock);
> + req = otx2_mbox_alloc_msg_cgx_set_fec_param(&pfvf->mbox);
> + if (!req) {
> + err = -ENOMEM;
> + goto end;
> + }
> + req->fec = fec;
> + err = otx2_sync_mbox_msg(&pfvf->mbox);
> + if (err)
> + goto end;
> +
> + rsp = (struct fec_mode *)otx2_mbox_get_rsp(&pfvf->mbox.mbox,
> + 0, &req->hdr);
> + if (rsp->fec >= 0) {
> + pfvf->linfo.fec = rsp->fec;
> + /* clear stale counters */
> + pfvf->hw.cgx_fec_corr_blks = 0;
> + pfvf->hw.cgx_fec_uncorr_blks = 0;

Stats are supposed to be cumulative. Don't reset the stats just because
someone changed the FEC mode. You can miss errors this way.

> + } else {
> + err = rsp->fec;
> + }

2021-02-05 00:48:11

by Hariprasad Kelam

[permalink] [raw]
Subject: Re: [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support

Hi Jakub,

> -----Original Message-----
> From: Jakub Kicinski <[email protected]>
> Sent: Wednesday, February 3, 2021 6:42 AM
> To: Hariprasad Kelam <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Sunil Kovvuri Goutham <[email protected]>; Linu
> Cherian <[email protected]>; Geethasowjanya Akula
> <[email protected]>; Jerin Jacob Kollanukkaran <[email protected]>;
> Subbaraya Sundeep Bhatta <[email protected]>
> Subject: [EXT] Re: [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode
> support
>
> On Mon, 1 Feb 2021 10:54:40 +0530 Hariprasad Kelam wrote:
> > From: Christina Jacob <[email protected]>
> >
> > Add ethtool support to configure fec modes baser/rs and support to
> > fecth FEC stats from CGX as well PHY.
> >
> > Configure fec mode
> > - ethtool --set-fec eth0 encoding rs/baser/off/auto Query fec mode
> > - ethtool --show-fec eth0
>
> > + if (pfvf->linfo.fec) {
> > + sprintf(data, "Fec Corrected Errors: ");
> > + data += ETH_GSTRING_LEN;
> > + sprintf(data, "Fec Uncorrected Errors: ");
> > + data += ETH_GSTRING_LEN;
>
> Once again, you can't dynamically hide stats. ethtool makes 3 separate
> system calls - to get the number of stats, get the names, and get the values. If
> someone changes the FEC config in between those user space dumping stats
> will get confused.
>
Agreed. Will fix this in next version.
> > + }
> > }
>
> > +static int otx2_get_fecparam(struct net_device *netdev,
> > + struct ethtool_fecparam *fecparam) {
> > + struct otx2_nic *pfvf = netdev_priv(netdev);
> > + struct cgx_fw_data *rsp;
> > + const int fec[] = {
> > + ETHTOOL_FEC_OFF,
> > + ETHTOOL_FEC_BASER,
> > + ETHTOOL_FEC_RS,
> > + ETHTOOL_FEC_BASER | ETHTOOL_FEC_RS}; #define
> FEC_MAX_INDEX 3
> > + if (pfvf->linfo.fec < FEC_MAX_INDEX)
>
> This should be <
Agreed . Current code miss the "ETHTOOL_FEC_BASER | ETHTOOL_FEC_RS" condition, will fix this in next version.
>
> > + fecparam->active_fec = fec[pfvf->linfo.fec];
>
>
> > + rsp = otx2_get_fwdata(pfvf);
> > + if (IS_ERR(rsp))
> > + return PTR_ERR(rsp);
> > +
> > + if (rsp->fwdata.supported_fec <= FEC_MAX_INDEX) {
> > + if (!rsp->fwdata.supported_fec)
> > + fecparam->fec = ETHTOOL_FEC_NONE;
> > + else
> > + fecparam->fec = fec[rsp->fwdata.supported_fec];
> > + }
> > + return 0;
> > +}
> > +
> > +static int otx2_set_fecparam(struct net_device *netdev,
> > + struct ethtool_fecparam *fecparam) {
> > + struct otx2_nic *pfvf = netdev_priv(netdev);
> > + struct mbox *mbox = &pfvf->mbox;
> > + struct fec_mode *req, *rsp;
> > + int err = 0, fec = 0;
> > +
> > + switch (fecparam->fec) {
> > + /* Firmware does not support AUTO mode consider it as FEC_NONE
> */
> > + case ETHTOOL_FEC_OFF:
> > + case ETHTOOL_FEC_AUTO:
> > + case ETHTOOL_FEC_NONE:
>
> I _think_ NONE is for drivers to report that they don't support FEC settings.
> It's an output only parameter. On input OFF should be used.
>
Thanks for pointing this. Cross checked code also _NONE is output is only parameter.
Will fix in next version.

> > + fec = OTX2_FEC_NONE;
> > + break;
> > + case ETHTOOL_FEC_RS:
> > + fec = OTX2_FEC_RS;
> > + break;
> > + case ETHTOOL_FEC_BASER:
> > + fec = OTX2_FEC_BASER;
> > + break;
> > + default:
> > + netdev_warn(pfvf->netdev, "Unsupported FEC mode: %d",
> > + fecparam->fec);
> > + return -EINVAL;
> > + }
> > +
> > + if (fec == pfvf->linfo.fec)
> > + return 0;
> > +
> > + mutex_lock(&mbox->lock);
> > + req = otx2_mbox_alloc_msg_cgx_set_fec_param(&pfvf->mbox);
> > + if (!req) {
> > + err = -ENOMEM;
> > + goto end;
> > + }
> > + req->fec = fec;
> > + err = otx2_sync_mbox_msg(&pfvf->mbox);
> > + if (err)
> > + goto end;
> > +
> > + rsp = (struct fec_mode *)otx2_mbox_get_rsp(&pfvf->mbox.mbox,
> > + 0, &req->hdr);
> > + if (rsp->fec >= 0) {
> > + pfvf->linfo.fec = rsp->fec;
> > + /* clear stale counters */
> > + pfvf->hw.cgx_fec_corr_blks = 0;
> > + pfvf->hw.cgx_fec_uncorr_blks = 0;
>
> Stats are supposed to be cumulative. Don't reset the stats just because
> someone changed the FEC mode. You can miss errors this way.
>
Thanks for pointing this. Will fix in next version.

Thanks,
Hariprasad k
> > + } else {
> > + err = rsp->fec;
> > + }