From: Jakub Kicinski <kuba@kernel.org>
To: Hariprasad Kelam <hkelam@marvell.com>
Cc: <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<davem@davemloft.net>, <sgoutham@marvell.com>,
<lcherian@marvell.com>, <gakula@marvell.com>,
<jerinj@marvell.com>, <sbhatta@marvell.com>,
Christina Jacob <cjacob@marvell.com>
Subject: Re: [net-next PATCH 3/7] octeontx2-pf: ethtool fec mode support
Date: Fri, 22 Jan 2021 20:29:53 -0800 [thread overview]
Message-ID: <20210122202953.59f806c0@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <1611215609-92301-4-git-send-email-hkelam@marvell.com>
On Thu, 21 Jan 2021 13:23:25 +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
>
> Signed-off-by: Christina Jacob <cjacob@marvell.com>
> Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
> Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
> ---
> .../ethernet/marvell/octeontx2/nic/otx2_common.c | 23 +++
> .../ethernet/marvell/octeontx2/nic/otx2_common.h | 6 +
> .../ethernet/marvell/octeontx2/nic/otx2_ethtool.c | 174 ++++++++++++++++++++-
> .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 3 +
> 4 files changed, 204 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..d09119b 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);
> + 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];
> @@ -1491,6 +1507,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;
double indented
> +}
> +
> void mbox_handler_nix_txsch_alloc(struct otx2_nic *pf,
> struct nix_txsch_alloc_rsp *rsp)
> {
> @@ -183,12 +210,42 @@ 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;
> +
> + if (pfvf->linfo.fec == OTX2_FEC_NONE)
> + return;
Don't hide the stats based on configuration.
Getting number of stats and requesting them are two different syscalls.
> + 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_fecparam(struct net_device *netdev,
> + struct ethtool_fecparam *fecparam)
> +{
> + struct otx2_nic *pfvf = netdev_priv(netdev);
> + struct cgx_fw_data *rsp;
> + int fec[] = {
const
> + 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) {
> + case ETHTOOL_FEC_OFF:
> + fec = OTX2_FEC_NONE;
> + break;
> + case ETHTOOL_FEC_RS:
> + fec = OTX2_FEC_RS;
> + break;
> + case ETHTOOL_FEC_BASER:
> + fec = OTX2_FEC_BASER;
> + break;
> + default:
> + fec = OTX2_FEC_NONE;
IIRC fecparam->fec is a bitmask, you can't assume other than one bit
set is NONE.
> + break;
> + }
> +
> + 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 = -EAGAIN;
Why -EAGAIN? When does msg allocation fail?
> + 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;
> + pfvf->hw.cgx_fec_corr_blks = 0;
> + pfvf->hw.cgx_fec_uncorr_blks = 0;
Are you clearing stats every time FEC changes?
> +
spurious new line
> + } else {
> + err = rsp->fec;
> + }
> +
> +end: mutex_unlock(&mbox->lock);
label on a separate line
> + return err;
> +}
next prev parent reply other threads:[~2021-01-23 4:30 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-21 7:53 [net-next PATCH 0/7] ethtool support for fec and link configuration Hariprasad Kelam
2021-01-21 7:53 ` [net-next PATCH 1/7] octeontx2-af: forward error correction configuration Hariprasad Kelam
2021-01-21 7:53 ` [net-next PATCH 2/7] octeontx2-af: Add new CGX_CMD to get PHY FEC statistics Hariprasad Kelam
2021-01-21 7:53 ` [net-next PATCH 3/7] octeontx2-pf: ethtool fec mode support Hariprasad Kelam
2021-01-23 4:29 ` Jakub Kicinski [this message]
2021-01-24 17:16 ` [EXT] " Hariprasad Kelam
2021-01-21 7:53 ` [net-next PATCH 4/7] octeontx2-af: Physical link configuration support Hariprasad Kelam
2021-01-21 7:53 ` [net-next PATCH 5/7] octeontx2-af: advertised link modes support on cgx Hariprasad Kelam
2021-01-21 7:53 ` [net-next PATCH 6/7] octeontx2-pf: ethtool physical link status Hariprasad Kelam
2021-01-21 7:53 ` [net-next PATCH 7/7] octeontx2-pf: ethtool physical link configuration Hariprasad Kelam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210122202953.59f806c0@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
--to=kuba@kernel.org \
--cc=cjacob@marvell.com \
--cc=davem@davemloft.net \
--cc=gakula@marvell.com \
--cc=hkelam@marvell.com \
--cc=jerinj@marvell.com \
--cc=lcherian@marvell.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=sbhatta@marvell.com \
--cc=sgoutham@marvell.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).