All of lore.kernel.org
 help / color / mirror / Atom feed
* [Patch v3 net-next 0/7] ethtool support for fec and link configuration
@ 2021-02-01  5:24 Hariprasad Kelam
  2021-02-01  5:24 ` [Patch v3 net-next 1/7] octeontx2-af: forward error correction configuration Hariprasad Kelam
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Hariprasad Kelam @ 2021-02-01  5:24 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: kuba, davem, willemdebruijn.kernel, andrew, sgoutham, lcherian,
	gakula, jerinj, sbhatta, hkelam

This series of patches add support for forward error correction(fec) and
physical link configuration. Patches 1&2 adds necessary mbox handlers for fec
mode configuration request and to fetch stats. Patch 3 registers driver
callbacks for fec mode configuration and display. Patch 4&5 adds support of mbox
handlers for configuring link parameters like speed/duplex and autoneg etc.
Patche 6&7 registers driver callbacks for physical link configuration.

Change-log:
v2:
	- Fixed review comments
	- Corrected indentation issues
        - Return -ENOMEM incase of mbox allocation failure
	- added validation for input fecparams bitmask values
        - added more comments

V3:
	- Removed inline functions
        - Make use of ethtool helpers APIs to display supported
          advertised modes
        - corrected indentation issues
        - code changes such that return early in case of failure
          to aid branch prediction


Christina Jacob (6):
  octeontx2-af: forward error correction configuration
  octeontx2-pf: ethtool fec mode support
  octeontx2-af: Physical link configuration support
  octeontx2-af: advertised link modes support on cgx
  octeontx2-pf: ethtool physical link status
  octeontx2-pf: ethtool physical link configuration

Felix Manlunas (1):
  octeontx2-af: Add new CGX_CMD to get PHY FEC statistics

 drivers/net/ethernet/marvell/octeontx2/af/cgx.c    | 258 ++++++++++++-
 drivers/net/ethernet/marvell/octeontx2/af/cgx.h    |  10 +
 .../net/ethernet/marvell/octeontx2/af/cgx_fw_if.h  |  70 +++-
 drivers/net/ethernet/marvell/octeontx2/af/mbox.h   |  87 ++++-
 drivers/net/ethernet/marvell/octeontx2/af/rvu.h    |   4 +
 .../net/ethernet/marvell/octeontx2/af/rvu_cgx.c    |  80 +++++
 .../ethernet/marvell/octeontx2/nic/otx2_common.c   |  20 ++
 .../ethernet/marvell/octeontx2/nic/otx2_common.h   |   6 +
 .../ethernet/marvell/octeontx2/nic/otx2_ethtool.c  | 399 ++++++++++++++++++++-
 .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c   |   3 +
 10 files changed, 930 insertions(+), 7 deletions(-)

--
2.7.4

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support
@ 2021-02-04 15:33 Hariprasad Kelam
  0 siblings, 0 replies; 20+ messages in thread
From: Hariprasad Kelam @ 2021-02-04 15:33 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, linux-kernel, davem, willemdebruijn.kernel, andrew,
	Sunil Kovvuri Goutham, Linu Cherian, Geethasowjanya Akula,
	Jerin Jacob Kollanukkaran, Subbaraya Sundeep Bhatta

Hi Jakub,

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, February 3, 2021 6:42 AM
> To: Hariprasad Kelam <hkelam@marvell.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> davem@davemloft.net; willemdebruijn.kernel@gmail.com;
> andrew@lunn.ch; Sunil Kovvuri Goutham <sgoutham@marvell.com>; Linu
> Cherian <lcherian@marvell.com>; Geethasowjanya Akula
> <gakula@marvell.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Subbaraya Sundeep Bhatta <sbhatta@marvell.com>
> 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 <cjacob@marvell.com>
> >
> > 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;
> > +	}

^ permalink raw reply	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2022-12-02  7:57 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-01  5:24 [Patch v3 net-next 0/7] ethtool support for fec and link configuration Hariprasad Kelam
2021-02-01  5:24 ` [Patch v3 net-next 1/7] octeontx2-af: forward error correction configuration Hariprasad Kelam
2021-02-03  0:43   ` Jesse Brandeburg
2021-02-01  5:24 ` [Patch v3 net-next 2/7] octeontx2-af: Add new CGX_CMD to get PHY FEC statistics Hariprasad Kelam
2021-02-03  0:43   ` Jesse Brandeburg
2021-02-01  5:24 ` [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support Hariprasad Kelam
2021-02-03  0:44   ` Jesse Brandeburg
2021-02-03  1:12   ` Jakub Kicinski
2021-02-01  5:24 ` [Patch v3 net-next 4/7] octeontx2-af: Physical link configuration support Hariprasad Kelam
2021-02-03  0:44   ` Jesse Brandeburg
2021-02-01  5:24 ` [Patch v3 net-next 5/7] octeontx2-af: advertised link modes support on cgx Hariprasad Kelam
2021-02-03  0:45   ` Jesse Brandeburg
2022-12-01  2:02   ` Chris Packham
2022-12-02  7:56     ` Hariprasad Kelam
2021-02-01  5:24 ` [Patch v3 net-next 6/7] octeontx2-pf: ethtool physical link status Hariprasad Kelam
2021-02-03  0:46   ` Jesse Brandeburg
2021-02-01  5:24 ` [Patch v3 net-next 7/7] octeontx2-pf: ethtool physical link configuration Hariprasad Kelam
2021-02-03  0:48   ` Jesse Brandeburg
2021-02-03  0:41 ` [Patch v3 net-next 0/7] ethtool support for fec and " Jesse Brandeburg
2021-02-04 15:33 [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support Hariprasad Kelam

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.