On Mon, 1 Feb 2021 10:54:40 +0530 Hariprasad Kelam wrote:
> From: Christina Jacob <cja...@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.
> + }
> }
> +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;
> + }
