Re: [Patch v2 net-next 3/7] octeontx2-pf: ethtool fec mode support

Hi Willem,

-----Original Message-----
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Sent: Thursday, January 28, 2021 2:01 AM
To: Hariprasad Kelam <redacted>
Cc: Network Development <redacted>; LKML <linux-
kernel@vger.kernel.org>; David Miller [off-list ref]; Jakub
Kicinski [off-list ref]; Sunil Kovvuri Goutham
[off-list ref]; Linu Cherian [off-list ref];
Geethasowjanya Akula [off-list ref]; Jerin Jacob Kollanukkaran
[off-list ref]; Subbaraya Sundeep Bhatta [off-list ref];
Christina Jacob [off-list ref]
Subject: [EXT] Re: [Patch v2 net-next 3/7] octeontx2-pf: ethtool fec mode
support

External Email

----------------------------------------------------------------------
On Wed, Jan 27, 2021 at 4:03 AM Hariprasad Kelam [off-list ref]
wrote:

quoted

From: Christina Jacob <redacted>

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 <redacted>
Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
Signed-off-by: Hariprasad Kelam <redacted>
---
 .../ethernet/marvell/octeontx2/nic/otx2_common.c   |  23 +++
 .../ethernet/marvell/octeontx2/nic/otx2_common.h   |   6 +
 .../ethernet/marvell/octeontx2/nic/otx2_ethtool.c  | 181

++++++++++++++++++++-

quoted

 .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c   |   3 +
 4 files changed, 211 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 bdfa2e2..f7e5450 100644

--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c

@@ -60,6 +60,22 @@ 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) {
+               mutex_unlock(&pfvf->mbox.lock);
+               return;
+       }
+       otx2_sync_mbox_msg(&pfvf->mbox);

Perhaps simpler to have a single exit from the critical section:

Agreed will fix this in next version.

  if (req)
    otx2_update_lmac_fec_stats

quoted

+       mutex_unlock(&pfvf->mbox.lock); }

Also, should this function return an error on failure? The caller returns errors
in other cases.

Caller of this function otx2_get_sset_count . Where driver suppose to return number of stats. 
This function is just to update local netdev counters fec_corr_blks/ fec_uncorr_blks. So failure
Of this function should not effect.

Thanks,
Hariprasad k