2021-02-01 06:47:17

by Hariprasad Kelam

[permalink] [raw]
Subject: [Patch v3 net-next 0/7] ethtool support for fec and link configuration

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


2021-02-01 06:48:56

by Hariprasad Kelam

[permalink] [raw]
Subject: [Patch v3 net-next 2/7] octeontx2-af: Add new CGX_CMD to get PHY FEC statistics

From: Felix Manlunas <[email protected]>

This patch adds support to fetch fec stats from PHY. The stats are
put in the shared data struct fwdata. A PHY driver indicates
that it has FEC stats by setting the flag fwdata.phy.misc.has_fec_stats

Besides CGX_CMD_GET_PHY_FEC_STATS, also add CGX_CMD_PRBS and
CGX_CMD_DISPLAY_EYE to enum cgx_cmd_id so that Linux's enum list is in sync
with firmware's enum list.

Signed-off-by: Felix Manlunas <[email protected]>
Signed-off-by: Christina Jacob <[email protected]>
Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
Signed-off-by: Hariprasad Kelam <[email protected]>
---
drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 12 ++++++
drivers/net/ethernet/marvell/octeontx2/af/cgx.h | 1 +
.../net/ethernet/marvell/octeontx2/af/cgx_fw_if.h | 5 +++
drivers/net/ethernet/marvell/octeontx2/af/mbox.h | 43 ++++++++++++++++++++++
drivers/net/ethernet/marvell/octeontx2/af/rvu.h | 4 ++
.../net/ethernet/marvell/octeontx2/af/rvu_cgx.c | 32 ++++++++++++++++
6 files changed, 97 insertions(+)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
index fe5512d..b636341 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
@@ -861,6 +861,18 @@ int cgx_set_fec(u64 fec, int cgx_id, int lmac_id)
return cgx->lmac_idmap[lmac_id]->link_info.fec;
}

+int cgx_get_phy_fec_stats(void *cgxd, int lmac_id)
+{
+ struct cgx *cgx = cgxd;
+ u64 req = 0, resp;
+
+ if (!cgx)
+ return -ENODEV;
+
+ req = FIELD_SET(CMDREG_ID, CGX_CMD_GET_PHY_FEC_STATS, req);
+ return cgx_fwi_cmd_generic(req, &resp, cgx, lmac_id);
+}
+
static int cgx_fwi_link_change(struct cgx *cgx, int lmac_id, bool enable)
{
u64 req = 0;
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
index 1824e95..c5294b7 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
@@ -154,5 +154,6 @@ void cgx_lmac_ptp_config(void *cgxd, int lmac_id, bool enable);
u8 cgx_lmac_get_p2x(int cgx_id, int lmac_id);
int cgx_set_fec(u64 fec, int cgx_id, int lmac_id);
int cgx_get_fec_stats(void *cgxd, int lmac_id, struct cgx_fec_stats_rsp *rsp);
+int cgx_get_phy_fec_stats(void *cgxd, int lmac_id);

#endif /* CGX_H */
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h b/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
index 3485596..65f832a 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
@@ -89,6 +89,11 @@ enum cgx_cmd_id {
CGX_CMD_SET_AN,
CGX_CMD_GET_ADV_LINK_MODES,
CGX_CMD_GET_ADV_FEC,
+ CGX_CMD_GET_PHY_MOD_TYPE, /* line-side modulation type: NRZ or PAM4 */
+ CGX_CMD_SET_PHY_MOD_TYPE,
+ CGX_CMD_PRBS,
+ CGX_CMD_DISPLAY_EYE,
+ CGX_CMD_GET_PHY_FEC_STATS,
};

/* async event ids */
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
index a59a355..204040e 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
@@ -151,6 +151,8 @@ M(CGX_CFG_PAUSE_FRM, 0x20E, cgx_cfg_pause_frm, cgx_pause_frm_cfg, \
cgx_pause_frm_cfg) \
M(CGX_FEC_SET, 0x210, cgx_set_fec_param, fec_mode, fec_mode) \
M(CGX_FEC_STATS, 0x211, cgx_fec_stats, msg_req, cgx_fec_stats_rsp) \
+M(CGX_GET_PHY_FEC_STATS, 0x212, cgx_get_phy_fec_stats, msg_req, msg_rsp) \
+M(CGX_FW_DATA_GET, 0x213, cgx_get_aux_link_info, msg_req, cgx_fw_data) \
/* NPA mbox IDs (range 0x400 - 0x5FF) */ \
/* NPA mbox IDs (range 0x400 - 0x5FF) */ \
M(NPA_LF_ALLOC, 0x400, npa_lf_alloc, \
@@ -411,6 +413,47 @@ struct fec_mode {
int fec;
};

+struct sfp_eeprom_s {
+#define SFP_EEPROM_SIZE 256
+ u16 sff_id;
+ u8 buf[SFP_EEPROM_SIZE];
+ u64 reserved;
+};
+
+struct phy_s {
+ struct {
+ u64 can_change_mod_type:1;
+ u64 mod_type:1;
+ u64 has_fec_stats:1;
+ } misc;
+ struct fec_stats_s {
+ u32 rsfec_corr_cws;
+ u32 rsfec_uncorr_cws;
+ u32 brfec_corr_blks;
+ u32 brfec_uncorr_blks;
+ } fec_stats;
+};
+
+struct cgx_lmac_fwdata_s {
+ u16 rw_valid;
+ u64 supported_fec;
+ u64 supported_an;
+ u64 supported_link_modes;
+ /* only applicable if AN is supported */
+ u64 advertised_fec;
+ u64 advertised_link_modes;
+ /* Only applicable if SFP/QSFP slot is present */
+ struct sfp_eeprom_s sfp_eeprom;
+ struct phy_s phy;
+#define LMAC_FWDATA_RESERVED_MEM 1021
+ u64 reserved[LMAC_FWDATA_RESERVED_MEM];
+};
+
+struct cgx_fw_data {
+ struct mbox_msghdr hdr;
+ struct cgx_lmac_fwdata_s fwdata;
+};
+
/* NPA mbox message formats */

/* NPA mailbox error codes
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
index b1a6ecf..49b493b 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
@@ -350,6 +350,10 @@ struct rvu_fwdata {
u64 msixtr_base;
#define FWDATA_RESERVED_MEM 1023
u64 reserved[FWDATA_RESERVED_MEM];
+#define CGX_MAX 5
+#define CGX_LMACS_MAX 4
+ struct cgx_lmac_fwdata_s cgx_fw_data[CGX_MAX][CGX_LMACS_MAX];
+ /* Do not add new fields below this line */
};

struct ptp;
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
index 0d806c5..c6fd2d5 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
@@ -692,6 +692,19 @@ int rvu_mbox_handler_cgx_cfg_pause_frm(struct rvu *rvu,
return 0;
}

+int rvu_mbox_handler_cgx_get_phy_fec_stats(struct rvu *rvu, struct msg_req *req,
+ struct msg_rsp *rsp)
+{
+ int pf = rvu_get_pf(req->hdr.pcifunc);
+ u8 cgx_id, lmac_id;
+
+ if (!is_pf_cgxmapped(rvu, pf))
+ return -EPERM;
+
+ rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
+ return cgx_get_phy_fec_stats(rvu_cgx_pdata(cgx_id, rvu), lmac_id);
+}
+
/* Finds cumulative status of NIX rx/tx counters from LF of a PF and those
* from its VFs as well. ie. NIX rx/tx counters at the CGX port level
*/
@@ -798,3 +811,22 @@ int rvu_mbox_handler_cgx_set_fec_param(struct rvu *rvu,
rsp->fec = cgx_set_fec(req->fec, cgx_id, lmac_id);
return 0;
}
+
+int rvu_mbox_handler_cgx_get_aux_link_info(struct rvu *rvu, struct msg_req *req,
+ struct cgx_fw_data *rsp)
+{
+ int pf = rvu_get_pf(req->hdr.pcifunc);
+ u8 cgx_id, lmac_id;
+
+ if (!rvu->fwdata)
+ return -ENXIO;
+
+ if (!is_pf_cgxmapped(rvu, pf))
+ return -EPERM;
+
+ rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
+
+ memcpy(&rsp->fwdata, &rvu->fwdata->cgx_fw_data[cgx_id][lmac_id],
+ sizeof(struct cgx_lmac_fwdata_s));
+ return 0;
+}
--
2.7.4

2021-02-01 06:49:05

by Hariprasad Kelam

[permalink] [raw]
Subject: [Patch v3 net-next 4/7] octeontx2-af: Physical link configuration support

From: Christina Jacob <[email protected]>

CGX LMAC, the physical interface support link configuration parameters
like speed, auto negotiation, duplex etc. Firmware saves these into
memory region shared between firmware and this driver.

This patch adds mailbox handler set_link_mode, fw_data_get to
configure and read these parameters.

Signed-off-by: Christina Jacob <[email protected]>
Signed-off-by: Sunil Goutham <[email protected]>
Signed-off-by: Hariprasad Kelam <[email protected]>
---
drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 58 +++++++++++++++++++++-
drivers/net/ethernet/marvell/octeontx2/af/cgx.h | 2 +
.../net/ethernet/marvell/octeontx2/af/cgx_fw_if.h | 18 ++++++-
drivers/net/ethernet/marvell/octeontx2/af/mbox.h | 21 ++++++++
.../net/ethernet/marvell/octeontx2/af/rvu_cgx.c | 17 +++++++
5 files changed, 113 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
index b636341..5b7d858 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
@@ -660,6 +660,39 @@ static inline void cgx_link_usertable_init(void)
cgx_lmactype_string[LMAC_MODE_USXGMII] = "USXGMII";
}

+static int cgx_link_usertable_index_map(int speed)
+{
+ switch (speed) {
+ case SPEED_10:
+ return CGX_LINK_10M;
+ case SPEED_100:
+ return CGX_LINK_100M;
+ case SPEED_1000:
+ return CGX_LINK_1G;
+ case SPEED_2500:
+ return CGX_LINK_2HG;
+ case SPEED_5000:
+ return CGX_LINK_5G;
+ case SPEED_10000:
+ return CGX_LINK_10G;
+ case SPEED_20000:
+ return CGX_LINK_20G;
+ case SPEED_25000:
+ return CGX_LINK_25G;
+ case SPEED_40000:
+ return CGX_LINK_40G;
+ case SPEED_50000:
+ return CGX_LINK_50G;
+ case 80000:
+ return CGX_LINK_80G;
+ case SPEED_100000:
+ return CGX_LINK_100G;
+ case SPEED_UNKNOWN:
+ return CGX_LINK_NONE;
+ }
+ return CGX_LINK_NONE;
+}
+
static inline void link_status_user_format(u64 lstat,
struct cgx_link_user_info *linfo,
struct cgx *cgx, u8 lmac_id)
@@ -669,6 +702,7 @@ static inline void link_status_user_format(u64 lstat,
linfo->link_up = FIELD_GET(RESP_LINKSTAT_UP, lstat);
linfo->full_duplex = FIELD_GET(RESP_LINKSTAT_FDUPLEX, lstat);
linfo->speed = cgx_speed_mbps[FIELD_GET(RESP_LINKSTAT_SPEED, lstat)];
+ linfo->an = FIELD_GET(RESP_LINKSTAT_AN, lstat);
linfo->fec = FIELD_GET(RESP_LINKSTAT_FEC, lstat);
linfo->lmac_type_id = cgx_get_lmac_type(cgx, lmac_id);
lmac_string = cgx_lmactype_string[linfo->lmac_type_id];
@@ -697,6 +731,9 @@ static inline void cgx_link_change_handler(u64 lstat,
lmac->link_info = event.link_uinfo;
linfo = &lmac->link_info;

+ if (err_type == CGX_ERR_SPEED_CHANGE_INVALID)
+ return;
+
/* Ensure callback doesn't get unregistered until we finish it */
spin_lock(&lmac->event_cb_lock);

@@ -725,7 +762,8 @@ static inline bool cgx_cmdresp_is_linkevent(u64 event)

id = FIELD_GET(EVTREG_ID, event);
if (id == CGX_CMD_LINK_BRING_UP ||
- id == CGX_CMD_LINK_BRING_DOWN)
+ id == CGX_CMD_LINK_BRING_DOWN ||
+ id == CGX_CMD_MODE_CHANGE)
return true;
else
return false;
@@ -840,6 +878,24 @@ int cgx_get_fwdata_base(u64 *base)
return err;
}

+int cgx_set_link_mode(void *cgxd, struct cgx_set_link_mode_args args,
+ int cgx_id, int lmac_id)
+{
+ struct cgx *cgx = cgxd;
+ u64 req = 0, resp;
+
+ if (!cgx)
+ return -ENODEV;
+
+ req = FIELD_SET(CMDREG_ID, CGX_CMD_MODE_CHANGE, req);
+ req = FIELD_SET(CMDMODECHANGE_SPEED,
+ cgx_link_usertable_index_map(args.speed), req);
+ req = FIELD_SET(CMDMODECHANGE_DUPLEX, args.duplex, req);
+ req = FIELD_SET(CMDMODECHANGE_AN, args.an, req);
+ req = FIELD_SET(CMDMODECHANGE_PORT, args.ports, req);
+ req = FIELD_SET(CMDMODECHANGE_FLAGS, args.flags, req);
+ return cgx_fwi_cmd_generic(req, &resp, cgx, lmac_id);
+}
int cgx_set_fec(u64 fec, int cgx_id, int lmac_id)
{
u64 req = 0, resp;
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
index c5294b7..b458ad0 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
@@ -155,5 +155,7 @@ u8 cgx_lmac_get_p2x(int cgx_id, int lmac_id);
int cgx_set_fec(u64 fec, int cgx_id, int lmac_id);
int cgx_get_fec_stats(void *cgxd, int lmac_id, struct cgx_fec_stats_rsp *rsp);
int cgx_get_phy_fec_stats(void *cgxd, int lmac_id);
+int cgx_set_link_mode(void *cgxd, struct cgx_set_link_mode_args args,
+ int cgx_id, int lmac_id);

#endif /* CGX_H */
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h b/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
index 65f832a..70610e7 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
@@ -43,7 +43,13 @@ enum cgx_error_type {
CGX_ERR_TRAINING_FAIL,
CGX_ERR_RX_EQU_FAIL,
CGX_ERR_SPUX_BER_FAIL,
- CGX_ERR_SPUX_RSFEC_ALGN_FAIL, /* = 22 */
+ CGX_ERR_SPUX_RSFEC_ALGN_FAIL,
+ CGX_ERR_SPUX_MARKER_LOCK_FAIL,
+ CGX_ERR_SET_FEC_INVALID,
+ CGX_ERR_SET_FEC_FAIL,
+ CGX_ERR_MODULE_INVALID,
+ CGX_ERR_MODULE_NOT_PRESENT,
+ CGX_ERR_SPEED_CHANGE_INVALID,
};

/* LINK speed types */
@@ -59,6 +65,7 @@ enum cgx_link_speed {
CGX_LINK_25G,
CGX_LINK_40G,
CGX_LINK_50G,
+ CGX_LINK_80G,
CGX_LINK_100G,
CGX_LINK_SPEED_MAX,
};
@@ -75,7 +82,7 @@ enum cgx_cmd_id {
CGX_CMD_INTERNAL_LBK,
CGX_CMD_EXTERNAL_LBK,
CGX_CMD_HIGIG,
- CGX_CMD_LINK_STATE_CHANGE,
+ CGX_CMD_LINK_STAT_CHANGE,
CGX_CMD_MODE_CHANGE, /* hot plug support */
CGX_CMD_INTF_SHUTDOWN,
CGX_CMD_GET_MKEX_PRFL_SIZE,
@@ -219,4 +226,11 @@ struct cgx_lnk_sts {
#define CMDLINKCHANGE_SPEED GENMASK_ULL(13, 10)

#define CMDSETFEC GENMASK_ULL(9, 8)
+/* command argument to be passed for cmd ID - CGX_CMD_MODE_CHANGE */
+#define CMDMODECHANGE_SPEED GENMASK_ULL(11, 8)
+#define CMDMODECHANGE_DUPLEX GENMASK_ULL(12, 12)
+#define CMDMODECHANGE_AN GENMASK_ULL(13, 13)
+#define CMDMODECHANGE_PORT GENMASK_ULL(21, 14)
+#define CMDMODECHANGE_FLAGS GENMASK_ULL(29, 22)
+
#endif /* __CGX_FW_INTF_H__ */
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
index 204040e..a050902 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
@@ -153,6 +153,8 @@ M(CGX_FEC_SET, 0x210, cgx_set_fec_param, fec_mode, fec_mode) \
M(CGX_FEC_STATS, 0x211, cgx_fec_stats, msg_req, cgx_fec_stats_rsp) \
M(CGX_GET_PHY_FEC_STATS, 0x212, cgx_get_phy_fec_stats, msg_req, msg_rsp) \
M(CGX_FW_DATA_GET, 0x213, cgx_get_aux_link_info, msg_req, cgx_fw_data) \
+M(CGX_SET_LINK_MODE, 0x214, cgx_set_link_mode, cgx_set_link_mode_req,\
+ cgx_set_link_mode_rsp) \
/* NPA mbox IDs (range 0x400 - 0x5FF) */ \
/* NPA mbox IDs (range 0x400 - 0x5FF) */ \
M(NPA_LF_ALLOC, 0x400, npa_lf_alloc, \
@@ -383,6 +385,7 @@ struct cgx_link_user_info {
uint64_t full_duplex:1;
uint64_t lmac_type_id:4;
uint64_t speed:20; /* speed in Mbps */
+ uint64_t an:1; /* AN supported or not */
uint64_t fec:2; /* FEC type if enabled else 0 */
#define LMACTYPE_STR_LEN 16
char lmac_type[LMACTYPE_STR_LEN];
@@ -454,6 +457,24 @@ struct cgx_fw_data {
struct cgx_lmac_fwdata_s fwdata;
};

+struct cgx_set_link_mode_args {
+ u32 speed;
+ u8 duplex;
+ u8 an;
+ u8 ports;
+ u8 flags;
+};
+
+struct cgx_set_link_mode_req {
+ struct mbox_msghdr hdr;
+ struct cgx_set_link_mode_args args;
+};
+
+struct cgx_set_link_mode_rsp {
+ struct mbox_msghdr hdr;
+ int status;
+};
+
/* NPA mbox message formats */

/* NPA mailbox error codes
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
index c6fd2d5..1247bb7 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
@@ -830,3 +830,20 @@ int rvu_mbox_handler_cgx_get_aux_link_info(struct rvu *rvu, struct msg_req *req,
sizeof(struct cgx_lmac_fwdata_s));
return 0;
}
+
+int rvu_mbox_handler_cgx_set_link_mode(struct rvu *rvu,
+ struct cgx_set_link_mode_req *req,
+ struct cgx_set_link_mode_rsp *rsp)
+{
+ int pf = rvu_get_pf(req->hdr.pcifunc);
+ u8 cgx_idx, lmac;
+ void *cgxd;
+
+ if (!is_cgx_config_permitted(rvu, req->hdr.pcifunc))
+ return -EPERM;
+
+ rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_idx, &lmac);
+ cgxd = rvu_cgx_pdata(cgx_idx, rvu);
+ rsp->status = cgx_set_link_mode(cgxd, req->args, cgx_idx, lmac);
+ return 0;
+}
--
2.7.4

2021-02-01 06:50:14

by Hariprasad Kelam

[permalink] [raw]
Subject: [Patch v3 net-next 5/7] octeontx2-af: advertised link modes support on cgx

From: Christina Jacob <[email protected]>

CGX supports setting advertised link modes on physical link.
This patch adds support to derive cgx mode from ethtool
link mode and pass it to firmware to configure the same.

Signed-off-by: Christina Jacob <[email protected]>
Signed-off-by: Sunil Goutham <[email protected]>
Signed-off-by: Hariprasad Kelam <[email protected]>
---
drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 114 ++++++++++++++++++++-
.../net/ethernet/marvell/octeontx2/af/cgx_fw_if.h | 32 +++++-
drivers/net/ethernet/marvell/octeontx2/af/mbox.h | 3 +-
3 files changed, 146 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
index 5b7d858..9c62129 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
@@ -14,6 +14,7 @@
#include <linux/pci.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
+#include <linux/ethtool.h>
#include <linux/phy.h>
#include <linux/of.h>
#include <linux/of_mdio.h>
@@ -646,6 +647,7 @@ static inline void cgx_link_usertable_init(void)
cgx_speed_mbps[CGX_LINK_25G] = 25000;
cgx_speed_mbps[CGX_LINK_40G] = 40000;
cgx_speed_mbps[CGX_LINK_50G] = 50000;
+ cgx_speed_mbps[CGX_LINK_80G] = 80000;
cgx_speed_mbps[CGX_LINK_100G] = 100000;

cgx_lmactype_string[LMAC_MODE_SGMII] = "SGMII";
@@ -693,6 +695,110 @@ static int cgx_link_usertable_index_map(int speed)
return CGX_LINK_NONE;
}

+static void set_mod_args(struct cgx_set_link_mode_args *args,
+ u32 speed, u8 duplex, u8 autoneg, u64 mode)
+{
+ /* Fill default values incase of user did not pass
+ * valid parameters
+ */
+ if (args->duplex == DUPLEX_UNKNOWN)
+ args->duplex = duplex;
+ if (args->speed == SPEED_UNKNOWN)
+ args->speed = speed;
+ if (args->an == AUTONEG_UNKNOWN)
+ args->an = autoneg;
+ args->mode = mode;
+ args->ports = 0;
+}
+
+static void otx2_map_ethtool_link_modes(u64 bitmask,
+ struct cgx_set_link_mode_args *args)
+{
+ switch (bitmask) {
+ case ETHTOOL_LINK_MODE_10baseT_Half_BIT:
+ set_mod_args(args, 10, 1, 1, BIT_ULL(CGX_MODE_SGMII));
+ break;
+ case ETHTOOL_LINK_MODE_10baseT_Full_BIT:
+ set_mod_args(args, 10, 0, 1, BIT_ULL(CGX_MODE_SGMII));
+ break;
+ case ETHTOOL_LINK_MODE_100baseT_Half_BIT:
+ set_mod_args(args, 100, 1, 1, BIT_ULL(CGX_MODE_SGMII));
+ break;
+ case ETHTOOL_LINK_MODE_100baseT_Full_BIT:
+ set_mod_args(args, 100, 0, 1, BIT_ULL(CGX_MODE_SGMII));
+ break;
+ case ETHTOOL_LINK_MODE_1000baseT_Half_BIT:
+ set_mod_args(args, 1000, 1, 1, BIT_ULL(CGX_MODE_SGMII));
+ break;
+ case ETHTOOL_LINK_MODE_1000baseT_Full_BIT:
+ set_mod_args(args, 1000, 0, 1, BIT_ULL(CGX_MODE_SGMII));
+ break;
+ case ETHTOOL_LINK_MODE_1000baseX_Full_BIT:
+ set_mod_args(args, 1000, 0, 0, BIT_ULL(CGX_MODE_1000_BASEX));
+ break;
+ case ETHTOOL_LINK_MODE_10000baseT_Full_BIT:
+ set_mod_args(args, 1000, 0, 1, BIT_ULL(CGX_MODE_QSGMII));
+ break;
+ case ETHTOOL_LINK_MODE_10000baseSR_Full_BIT:
+ set_mod_args(args, 10000, 0, 0, BIT_ULL(CGX_MODE_10G_C2C));
+ break;
+ case ETHTOOL_LINK_MODE_10000baseLR_Full_BIT:
+ set_mod_args(args, 10000, 0, 0, BIT_ULL(CGX_MODE_10G_C2M));
+ break;
+ case ETHTOOL_LINK_MODE_10000baseKR_Full_BIT:
+ set_mod_args(args, 10000, 0, 1, BIT_ULL(CGX_MODE_10G_KR));
+ break;
+ case ETHTOOL_LINK_MODE_25000baseSR_Full_BIT:
+ set_mod_args(args, 25000, 0, 0, BIT_ULL(CGX_MODE_25G_C2C));
+ break;
+ case ETHTOOL_LINK_MODE_25000baseCR_Full_BIT:
+ set_mod_args(args, 25000, 0, 1, BIT_ULL(CGX_MODE_25G_CR));
+ break;
+ case ETHTOOL_LINK_MODE_25000baseKR_Full_BIT:
+ set_mod_args(args, 25000, 0, 1, BIT_ULL(CGX_MODE_25G_KR));
+ break;
+ case ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT:
+ set_mod_args(args, 40000, 0, 0, BIT_ULL(CGX_MODE_40G_C2C));
+ break;
+ case ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT:
+ set_mod_args(args, 40000, 0, 0, BIT_ULL(CGX_MODE_40G_C2M));
+ break;
+ case ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT:
+ set_mod_args(args, 40000, 0, 1, BIT_ULL(CGX_MODE_40G_CR4));
+ break;
+ case ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT:
+ set_mod_args(args, 40000, 0, 1, BIT_ULL(CGX_MODE_40G_KR4));
+ break;
+ case ETHTOOL_LINK_MODE_50000baseSR_Full_BIT:
+ set_mod_args(args, 50000, 0, 0, BIT_ULL(CGX_MODE_50G_C2C));
+ break;
+ case ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT:
+ set_mod_args(args, 50000, 0, 0, BIT_ULL(CGX_MODE_50G_C2M));
+ break;
+ case ETHTOOL_LINK_MODE_50000baseCR_Full_BIT:
+ set_mod_args(args, 50000, 0, 1, BIT_ULL(CGX_MODE_50G_CR));
+ break;
+ case ETHTOOL_LINK_MODE_50000baseKR_Full_BIT:
+ set_mod_args(args, 50000, 0, 1, BIT_ULL(CGX_MODE_50G_KR));
+ break;
+ case ETHTOOL_LINK_MODE_100000baseSR4_Full_BIT:
+ set_mod_args(args, 100000, 0, 0, BIT_ULL(CGX_MODE_100G_C2C));
+ break;
+ case ETHTOOL_LINK_MODE_100000baseLR4_ER4_Full_BIT:
+ set_mod_args(args, 100000, 0, 0, BIT_ULL(CGX_MODE_100G_C2M));
+ break;
+ case ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT:
+ set_mod_args(args, 100000, 0, 1, BIT_ULL(CGX_MODE_100G_CR4));
+ break;
+ case ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT:
+ set_mod_args(args, 100000, 0, 1, BIT_ULL(CGX_MODE_100G_KR4));
+ break;
+ default:
+ set_mod_args(args, 0, 1, 0, BIT_ULL(CGX_MODE_MAX));
+ break;
+ }
+}
+
static inline void link_status_user_format(u64 lstat,
struct cgx_link_user_info *linfo,
struct cgx *cgx, u8 lmac_id)
@@ -887,13 +993,19 @@ int cgx_set_link_mode(void *cgxd, struct cgx_set_link_mode_args args,
if (!cgx)
return -ENODEV;

+ if (args.mode)
+ otx2_map_ethtool_link_modes(args.mode, &args);
+ if (!args.speed && args.duplex && !args.an)
+ return -EINVAL;
+
req = FIELD_SET(CMDREG_ID, CGX_CMD_MODE_CHANGE, req);
req = FIELD_SET(CMDMODECHANGE_SPEED,
cgx_link_usertable_index_map(args.speed), req);
req = FIELD_SET(CMDMODECHANGE_DUPLEX, args.duplex, req);
req = FIELD_SET(CMDMODECHANGE_AN, args.an, req);
req = FIELD_SET(CMDMODECHANGE_PORT, args.ports, req);
- req = FIELD_SET(CMDMODECHANGE_FLAGS, args.flags, req);
+ req = FIELD_SET(CMDMODECHANGE_FLAGS, args.mode, req);
+
return cgx_fwi_cmd_generic(req, &resp, cgx, lmac_id);
}
int cgx_set_fec(u64 fec, int cgx_id, int lmac_id)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h b/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
index 70610e7..dde2bd0 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
@@ -70,6 +70,36 @@ enum cgx_link_speed {
CGX_LINK_SPEED_MAX,
};

+enum CGX_MODE_ {
+ CGX_MODE_SGMII,
+ CGX_MODE_1000_BASEX,
+ CGX_MODE_QSGMII,
+ CGX_MODE_10G_C2C,
+ CGX_MODE_10G_C2M,
+ CGX_MODE_10G_KR,
+ CGX_MODE_20G_C2C,
+ CGX_MODE_25G_C2C,
+ CGX_MODE_25G_C2M,
+ CGX_MODE_25G_2_C2C,
+ CGX_MODE_25G_CR,
+ CGX_MODE_25G_KR,
+ CGX_MODE_40G_C2C,
+ CGX_MODE_40G_C2M,
+ CGX_MODE_40G_CR4,
+ CGX_MODE_40G_KR4,
+ CGX_MODE_40GAUI_C2C,
+ CGX_MODE_50G_C2C,
+ CGX_MODE_50G_C2M,
+ CGX_MODE_50G_4_C2C,
+ CGX_MODE_50G_CR,
+ CGX_MODE_50G_KR,
+ CGX_MODE_80GAUI_C2C,
+ CGX_MODE_100G_C2C,
+ CGX_MODE_100G_C2M,
+ CGX_MODE_100G_CR4,
+ CGX_MODE_100G_KR4,
+ CGX_MODE_MAX /* = 29 */
+};
/* REQUEST ID types. Input to firmware */
enum cgx_cmd_id {
CGX_CMD_NONE,
@@ -231,6 +261,6 @@ struct cgx_lnk_sts {
#define CMDMODECHANGE_DUPLEX GENMASK_ULL(12, 12)
#define CMDMODECHANGE_AN GENMASK_ULL(13, 13)
#define CMDMODECHANGE_PORT GENMASK_ULL(21, 14)
-#define CMDMODECHANGE_FLAGS GENMASK_ULL(29, 22)
+#define CMDMODECHANGE_FLAGS GENMASK_ULL(63, 22)

#endif /* __CGX_FW_INTF_H__ */
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
index a050902..05a6da2 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
@@ -462,10 +462,11 @@ struct cgx_set_link_mode_args {
u8 duplex;
u8 an;
u8 ports;
- u8 flags;
+ u64 mode;
};

struct cgx_set_link_mode_req {
+#define AUTONEG_UNKNOWN 0xff
struct mbox_msghdr hdr;
struct cgx_set_link_mode_args args;
};
--
2.7.4

2021-02-03 01:01:59

by Jesse Brandeburg

[permalink] [raw]
Subject: Re: [Patch v3 net-next 2/7] octeontx2-af: Add new CGX_CMD to get PHY FEC statistics

Hariprasad Kelam wrote:

> From: Felix Manlunas <[email protected]>
>
> This patch adds support to fetch fec stats from PHY. The stats are
> put in the shared data struct fwdata. A PHY driver indicates
> that it has FEC stats by setting the flag fwdata.phy.misc.has_fec_stats
>
> Besides CGX_CMD_GET_PHY_FEC_STATS, also add CGX_CMD_PRBS and
> CGX_CMD_DISPLAY_EYE to enum cgx_cmd_id so that Linux's enum list is in sync
> with firmware's enum list.
>
> Signed-off-by: Felix Manlunas <[email protected]>
> Signed-off-by: Christina Jacob <[email protected]>
> Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
> Signed-off-by: Hariprasad Kelam <[email protected]>

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

2021-02-03 01:02:09

by Jesse Brandeburg

[permalink] [raw]
Subject: Re: [Patch v3 net-next 4/7] octeontx2-af: Physical link configuration support

Hariprasad Kelam wrote:

> From: Christina Jacob <[email protected]>
>
> CGX LMAC, the physical interface support link configuration parameters
> like speed, auto negotiation, duplex etc. Firmware saves these into
> memory region shared between firmware and this driver.
>
> This patch adds mailbox handler set_link_mode, fw_data_get to
> configure and read these parameters.
>
> 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:02:13

by Jesse Brandeburg

[permalink] [raw]
Subject: Re: [Patch v3 net-next 5/7] octeontx2-af: advertised link modes support on cgx

Hariprasad Kelam wrote:

> From: Christina Jacob <[email protected]>
>
> CGX supports setting advertised link modes on physical link.
> This patch adds support to derive cgx mode from ethtool
> link mode and pass it to firmware to configure the same.
>
> 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:03:48

by Jesse Brandeburg

[permalink] [raw]
Subject: Re: [Patch v3 net-next 0/7] ethtool support for fec and link configuration

Hariprasad Kelam wrote:

> 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.

For the series, in addition to the fact that Willem already replied to
the previous posting of v3 that it looked good.

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

2022-12-01 03:11:15

by Chris Packham

[permalink] [raw]
Subject: Re: [Patch v3 net-next 5/7] octeontx2-af: advertised link modes support on cgx

Hi Hariprasad, Christina,

Sorry for poking an old thread but Richard and I were looking at some
code and noticed something odd about it.

On 1/02/21 18:24, Hariprasad Kelam wrote:
> From: Christina Jacob <[email protected]>
>
> CGX supports setting advertised link modes on physical link.
> This patch adds support to derive cgx mode from ethtool
> link mode and pass it to firmware to configure the same.
>
> Signed-off-by: Christina Jacob <[email protected]>
> Signed-off-by: Sunil Goutham <[email protected]>
> Signed-off-by: Hariprasad Kelam <[email protected]>
> ---
> drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 114 ++++++++++++++++++++-
> .../net/ethernet/marvell/octeontx2/af/cgx_fw_if.h | 32 +++++-
> drivers/net/ethernet/marvell/octeontx2/af/mbox.h | 3 +-
> 3 files changed, 146 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> index 5b7d858..9c62129 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> @@ -14,6 +14,7 @@
> #include <linux/pci.h>
> #include <linux/netdevice.h>
> #include <linux/etherdevice.h>
> +#include <linux/ethtool.h>
> #include <linux/phy.h>
> #include <linux/of.h>
> #include <linux/of_mdio.h>
> @@ -646,6 +647,7 @@ static inline void cgx_link_usertable_init(void)
> cgx_speed_mbps[CGX_LINK_25G] = 25000;
> cgx_speed_mbps[CGX_LINK_40G] = 40000;
> cgx_speed_mbps[CGX_LINK_50G] = 50000;
> + cgx_speed_mbps[CGX_LINK_80G] = 80000;
> cgx_speed_mbps[CGX_LINK_100G] = 100000;
>
> cgx_lmactype_string[LMAC_MODE_SGMII] = "SGMII";
> @@ -693,6 +695,110 @@ static int cgx_link_usertable_index_map(int speed)
> return CGX_LINK_NONE;
> }
>
> +static void set_mod_args(struct cgx_set_link_mode_args *args,
> + u32 speed, u8 duplex, u8 autoneg, u64 mode)
> +{
> + /* Fill default values incase of user did not pass
> + * valid parameters
> + */
> + if (args->duplex == DUPLEX_UNKNOWN)
> + args->duplex = duplex;
> + if (args->speed == SPEED_UNKNOWN)
> + args->speed = speed;
> + if (args->an == AUTONEG_UNKNOWN)
> + args->an = autoneg;
> + args->mode = mode;
> + args->ports = 0;
> +}
> +
> +static void otx2_map_ethtool_link_modes(u64 bitmask,
> + struct cgx_set_link_mode_args *args)
> +{
> + switch (bitmask) {
> + case ETHTOOL_LINK_MODE_10baseT_Half_BIT:
> + set_mod_args(args, 10, 1, 1, BIT_ULL(CGX_MODE_SGMII));
> + break;
> + case ETHTOOL_LINK_MODE_10baseT_Full_BIT:
> + set_mod_args(args, 10, 0, 1, BIT_ULL(CGX_MODE_SGMII));
> + break;
> + case ETHTOOL_LINK_MODE_100baseT_Half_BIT:
> + set_mod_args(args, 100, 1, 1, BIT_ULL(CGX_MODE_SGMII));
> + break;
> + case ETHTOOL_LINK_MODE_100baseT_Full_BIT:
> + set_mod_args(args, 100, 0, 1, BIT_ULL(CGX_MODE_SGMII));
> + break;
> + case ETHTOOL_LINK_MODE_1000baseT_Half_BIT:
> + set_mod_args(args, 1000, 1, 1, BIT_ULL(CGX_MODE_SGMII));
> + break;
> + case ETHTOOL_LINK_MODE_1000baseT_Full_BIT:
> + set_mod_args(args, 1000, 0, 1, BIT_ULL(CGX_MODE_SGMII));
> + break;
> + case ETHTOOL_LINK_MODE_1000baseX_Full_BIT:
> + set_mod_args(args, 1000, 0, 0, BIT_ULL(CGX_MODE_1000_BASEX));
> + break;
> + case ETHTOOL_LINK_MODE_10000baseT_Full_BIT:
> + set_mod_args(args, 1000, 0, 1, BIT_ULL(CGX_MODE_QSGMII));

Here the speed is set to 1000 but it looks like this should be 10000. I
would have sent a patch to fix that but the mode is also a bit
confusing. Normally I'd expect QSGMII to be used for Quad SGMII (i.e.
4x1G Ethernet ports multiplexed over a single SERDES). I don't see any
other mode in enum CGX_MODE_ that is obviously for 10GBase-T so I
thought I'd bring this to your attention. I'd still be happy to whip up
a patch if you could confirm what the correct mode should be.

> + break;
> + case ETHTOOL_LINK_MODE_10000baseSR_Full_BIT:
> + set_mod_args(args, 10000, 0, 0, BIT_ULL(CGX_MODE_10G_C2C));
> + break;
> + case ETHTOOL_LINK_MODE_10000baseLR_Full_BIT:
> + set_mod_args(args, 10000, 0, 0, BIT_ULL(CGX_MODE_10G_C2M));
> + break;
> + case ETHTOOL_LINK_MODE_10000baseKR_Full_BIT:
> + set_mod_args(args, 10000, 0, 1, BIT_ULL(CGX_MODE_10G_KR));
> + break;
> + case ETHTOOL_LINK_MODE_25000baseSR_Full_BIT:
> + set_mod_args(args, 25000, 0, 0, BIT_ULL(CGX_MODE_25G_C2C));
> + break;
> + case ETHTOOL_LINK_MODE_25000baseCR_Full_BIT:
> + set_mod_args(args, 25000, 0, 1, BIT_ULL(CGX_MODE_25G_CR));
> + break;
> + case ETHTOOL_LINK_MODE_25000baseKR_Full_BIT:
> + set_mod_args(args, 25000, 0, 1, BIT_ULL(CGX_MODE_25G_KR));
> + break;
> + case ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT:
> + set_mod_args(args, 40000, 0, 0, BIT_ULL(CGX_MODE_40G_C2C));
> + break;
> + case ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT:
> + set_mod_args(args, 40000, 0, 0, BIT_ULL(CGX_MODE_40G_C2M));
> + break;
> + case ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT:
> + set_mod_args(args, 40000, 0, 1, BIT_ULL(CGX_MODE_40G_CR4));
> + break;
> + case ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT:
> + set_mod_args(args, 40000, 0, 1, BIT_ULL(CGX_MODE_40G_KR4));
> + break;
> + case ETHTOOL_LINK_MODE_50000baseSR_Full_BIT:
> + set_mod_args(args, 50000, 0, 0, BIT_ULL(CGX_MODE_50G_C2C));
> + break;
> + case ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT:
> + set_mod_args(args, 50000, 0, 0, BIT_ULL(CGX_MODE_50G_C2M));
> + break;
> + case ETHTOOL_LINK_MODE_50000baseCR_Full_BIT:
> + set_mod_args(args, 50000, 0, 1, BIT_ULL(CGX_MODE_50G_CR));
> + break;
> + case ETHTOOL_LINK_MODE_50000baseKR_Full_BIT:
> + set_mod_args(args, 50000, 0, 1, BIT_ULL(CGX_MODE_50G_KR));
> + break;
> + case ETHTOOL_LINK_MODE_100000baseSR4_Full_BIT:
> + set_mod_args(args, 100000, 0, 0, BIT_ULL(CGX_MODE_100G_C2C));
> + break;
> + case ETHTOOL_LINK_MODE_100000baseLR4_ER4_Full_BIT:
> + set_mod_args(args, 100000, 0, 0, BIT_ULL(CGX_MODE_100G_C2M));
> + break;
> + case ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT:
> + set_mod_args(args, 100000, 0, 1, BIT_ULL(CGX_MODE_100G_CR4));
> + break;
> + case ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT:
> + set_mod_args(args, 100000, 0, 1, BIT_ULL(CGX_MODE_100G_KR4));
> + break;
> + default:
> + set_mod_args(args, 0, 1, 0, BIT_ULL(CGX_MODE_MAX));
> + break;
> + }
> +}
> +
> static inline void link_status_user_format(u64 lstat,
> struct cgx_link_user_info *linfo,
> struct cgx *cgx, u8 lmac_id)
> @@ -887,13 +993,19 @@ int cgx_set_link_mode(void *cgxd, struct cgx_set_link_mode_args args,
> if (!cgx)
> return -ENODEV;
>
> + if (args.mode)
> + otx2_map_ethtool_link_modes(args.mode, &args);
> + if (!args.speed && args.duplex && !args.an)
> + return -EINVAL;
> +
> req = FIELD_SET(CMDREG_ID, CGX_CMD_MODE_CHANGE, req);
> req = FIELD_SET(CMDMODECHANGE_SPEED,
> cgx_link_usertable_index_map(args.speed), req);
> req = FIELD_SET(CMDMODECHANGE_DUPLEX, args.duplex, req);
> req = FIELD_SET(CMDMODECHANGE_AN, args.an, req);
> req = FIELD_SET(CMDMODECHANGE_PORT, args.ports, req);
> - req = FIELD_SET(CMDMODECHANGE_FLAGS, args.flags, req);
> + req = FIELD_SET(CMDMODECHANGE_FLAGS, args.mode, req);
> +
> return cgx_fwi_cmd_generic(req, &resp, cgx, lmac_id);
> }
> int cgx_set_fec(u64 fec, int cgx_id, int lmac_id)
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h b/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
> index 70610e7..dde2bd0 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
> @@ -70,6 +70,36 @@ enum cgx_link_speed {
> CGX_LINK_SPEED_MAX,
> };
>
> +enum CGX_MODE_ {
> + CGX_MODE_SGMII,
> + CGX_MODE_1000_BASEX,
> + CGX_MODE_QSGMII,
> + CGX_MODE_10G_C2C,
> + CGX_MODE_10G_C2M,
> + CGX_MODE_10G_KR,
> + CGX_MODE_20G_C2C,
> + CGX_MODE_25G_C2C,
> + CGX_MODE_25G_C2M,
> + CGX_MODE_25G_2_C2C,
> + CGX_MODE_25G_CR,
> + CGX_MODE_25G_KR,
> + CGX_MODE_40G_C2C,
> + CGX_MODE_40G_C2M,
> + CGX_MODE_40G_CR4,
> + CGX_MODE_40G_KR4,
> + CGX_MODE_40GAUI_C2C,
> + CGX_MODE_50G_C2C,
> + CGX_MODE_50G_C2M,
> + CGX_MODE_50G_4_C2C,
> + CGX_MODE_50G_CR,
> + CGX_MODE_50G_KR,
> + CGX_MODE_80GAUI_C2C,
> + CGX_MODE_100G_C2C,
> + CGX_MODE_100G_C2M,
> + CGX_MODE_100G_CR4,
> + CGX_MODE_100G_KR4,
> + CGX_MODE_MAX /* = 29 */
> +};
> /* REQUEST ID types. Input to firmware */
> enum cgx_cmd_id {
> CGX_CMD_NONE,
> @@ -231,6 +261,6 @@ struct cgx_lnk_sts {
> #define CMDMODECHANGE_DUPLEX GENMASK_ULL(12, 12)
> #define CMDMODECHANGE_AN GENMASK_ULL(13, 13)
> #define CMDMODECHANGE_PORT GENMASK_ULL(21, 14)
> -#define CMDMODECHANGE_FLAGS GENMASK_ULL(29, 22)
> +#define CMDMODECHANGE_FLAGS GENMASK_ULL(63, 22)
>
> #endif /* __CGX_FW_INTF_H__ */
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> index a050902..05a6da2 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -462,10 +462,11 @@ struct cgx_set_link_mode_args {
> u8 duplex;
> u8 an;
> u8 ports;
> - u8 flags;
> + u64 mode;
> };
>
> struct cgx_set_link_mode_req {
> +#define AUTONEG_UNKNOWN 0xff
> struct mbox_msghdr hdr;
> struct cgx_set_link_mode_args args;
> };

Thanks,
Chris

2022-12-02 08:22:50

by Hariprasad Kelam

[permalink] [raw]
Subject: RE: [Patch v3 net-next 5/7] octeontx2-af: advertised link modes support on cgx

Hi Chris ,

See inline,

Hi Hariprasad, Christina,

Sorry for poking an old thread but Richard and I were looking at some code and noticed something odd about it.

On 1/02/21 18:24, Hariprasad Kelam wrote:
> From: Christina Jacob <[email protected]>
>
> CGX supports setting advertised link modes on physical link.
> This patch adds support to derive cgx mode from ethtool link mode and
> pass it to firmware to configure the same.
>
> Signed-off-by: Christina Jacob <[email protected]>
> Signed-off-by: Sunil Goutham <[email protected]>
> Signed-off-by: Hariprasad Kelam <[email protected]>
> ---
> drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 114 ++++++++++++++++++++-
> .../net/ethernet/marvell/octeontx2/af/cgx_fw_if.h | 32 +++++-
> drivers/net/ethernet/marvell/octeontx2/af/mbox.h | 3 +-
> 3 files changed, 146 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> index 5b7d858..9c62129 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> @@ -14,6 +14,7 @@
> #include <linux/pci.h>
> #include <linux/netdevice.h>
> #include <linux/etherdevice.h>
> +#include <linux/ethtool.h>
> #include <linux/phy.h>
> #include <linux/of.h>
> #include <linux/of_mdio.h>
> @@ -646,6 +647,7 @@ static inline void cgx_link_usertable_init(void)
> cgx_speed_mbps[CGX_LINK_25G] = 25000;
> cgx_speed_mbps[CGX_LINK_40G] = 40000;
> cgx_speed_mbps[CGX_LINK_50G] = 50000;
> + cgx_speed_mbps[CGX_LINK_80G] = 80000;
> cgx_speed_mbps[CGX_LINK_100G] = 100000;
>
> cgx_lmactype_string[LMAC_MODE_SGMII] = "SGMII"; @@ -693,6 +695,110
> @@ static int cgx_link_usertable_index_map(int speed)
> return CGX_LINK_NONE;
> }
>
> +static void set_mod_args(struct cgx_set_link_mode_args *args,
> + u32 speed, u8 duplex, u8 autoneg, u64 mode) {
> + /* Fill default values incase of user did not pass
> + * valid parameters
> + */
> + if (args->duplex == DUPLEX_UNKNOWN)
> + args->duplex = duplex;
> + if (args->speed == SPEED_UNKNOWN)
> + args->speed = speed;
> + if (args->an == AUTONEG_UNKNOWN)
> + args->an = autoneg;
> + args->mode = mode;
> + args->ports = 0;
> +}
> +
> +static void otx2_map_ethtool_link_modes(u64 bitmask,
> + struct cgx_set_link_mode_args *args) {
> + switch (bitmask) {
> + case ETHTOOL_LINK_MODE_10baseT_Half_BIT:
> + set_mod_args(args, 10, 1, 1, BIT_ULL(CGX_MODE_SGMII));
> + break;
> + case ETHTOOL_LINK_MODE_10baseT_Full_BIT:
> + set_mod_args(args, 10, 0, 1, BIT_ULL(CGX_MODE_SGMII));
> + break;
> + case ETHTOOL_LINK_MODE_100baseT_Half_BIT:
> + set_mod_args(args, 100, 1, 1, BIT_ULL(CGX_MODE_SGMII));
> + break;
> + case ETHTOOL_LINK_MODE_100baseT_Full_BIT:
> + set_mod_args(args, 100, 0, 1, BIT_ULL(CGX_MODE_SGMII));
> + break;
> + case ETHTOOL_LINK_MODE_1000baseT_Half_BIT:
> + set_mod_args(args, 1000, 1, 1, BIT_ULL(CGX_MODE_SGMII));
> + break;
> + case ETHTOOL_LINK_MODE_1000baseT_Full_BIT:
> + set_mod_args(args, 1000, 0, 1, BIT_ULL(CGX_MODE_SGMII));
> + break;
> + case ETHTOOL_LINK_MODE_1000baseX_Full_BIT:
> + set_mod_args(args, 1000, 0, 0, BIT_ULL(CGX_MODE_1000_BASEX));
> + break;
> + case ETHTOOL_LINK_MODE_10000baseT_Full_BIT:
> + set_mod_args(args, 1000, 0, 1, BIT_ULL(CGX_MODE_QSGMII));

Here the speed is set to 1000 but it looks like this should be 10000. I would have sent a patch to fix that but the mode is also a bit confusing. Normally I'd expect QSGMII to be used for Quad SGMII (i.e.
4x1G Ethernet ports multiplexed over a single SERDES). I don't see any other mode in enum CGX_MODE_ that is obviously for 10GBase-T so I thought I'd bring this to your attention. I'd still be happy to whip up a patch if you could confirm what the correct mode should be.

>> 10GBase-T to QSGMII mapping is wrong. To correct it, we need to map proper
ETHTOOL_LINK_MODEX to QSGMII keeping all other parameters the same.
As transmit and receive data paths of QSGMII leverage the 1000BASE-X , we may use
ETHTOOL_LINK_MODE_1000baseSX_Full_BIT.
If there are no objections to the above mode, go ahead and submit the patch.

Thanks,
Hariprasad k


> + break;
> + case ETHTOOL_LINK_MODE_10000baseSR_Full_BIT:
> + set_mod_args(args, 10000, 0, 0, BIT_ULL(CGX_MODE_10G_C2C));
> + break;
> + case ETHTOOL_LINK_MODE_10000baseLR_Full_BIT:
> + set_mod_args(args, 10000, 0, 0, BIT_ULL(CGX_MODE_10G_C2M));
> + break;
> + case ETHTOOL_LINK_MODE_10000baseKR_Full_BIT:
> + set_mod_args(args, 10000, 0, 1, BIT_ULL(CGX_MODE_10G_KR));
> + break;
> + case ETHTOOL_LINK_MODE_25000baseSR_Full_BIT:
> + set_mod_args(args, 25000, 0, 0, BIT_ULL(CGX_MODE_25G_C2C));
> + break;
> + case ETHTOOL_LINK_MODE_25000baseCR_Full_BIT:
> + set_mod_args(args, 25000, 0, 1, BIT_ULL(CGX_MODE_25G_CR));
> + break;
> + case ETHTOOL_LINK_MODE_25000baseKR_Full_BIT:
> + set_mod_args(args, 25000, 0, 1, BIT_ULL(CGX_MODE_25G_KR));
> + break;
> + case ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT:
> + set_mod_args(args, 40000, 0, 0, BIT_ULL(CGX_MODE_40G_C2C));
> + break;
> + case ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT:
> + set_mod_args(args, 40000, 0, 0, BIT_ULL(CGX_MODE_40G_C2M));
> + break;
> + case ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT:
> + set_mod_args(args, 40000, 0, 1, BIT_ULL(CGX_MODE_40G_CR4));
> + break;
> + case ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT:
> + set_mod_args(args, 40000, 0, 1, BIT_ULL(CGX_MODE_40G_KR4));
> + break;
> + case ETHTOOL_LINK_MODE_50000baseSR_Full_BIT:
> + set_mod_args(args, 50000, 0, 0, BIT_ULL(CGX_MODE_50G_C2C));
> + break;
> + case ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT:
> + set_mod_args(args, 50000, 0, 0, BIT_ULL(CGX_MODE_50G_C2M));
> + break;
> + case ETHTOOL_LINK_MODE_50000baseCR_Full_BIT:
> + set_mod_args(args, 50000, 0, 1, BIT_ULL(CGX_MODE_50G_CR));
> + break;
> + case ETHTOOL_LINK_MODE_50000baseKR_Full_BIT:
> + set_mod_args(args, 50000, 0, 1, BIT_ULL(CGX_MODE_50G_KR));
> + break;
> + case ETHTOOL_LINK_MODE_100000baseSR4_Full_BIT:
> + set_mod_args(args, 100000, 0, 0, BIT_ULL(CGX_MODE_100G_C2C));
> + break;
> + case ETHTOOL_LINK_MODE_100000baseLR4_ER4_Full_BIT:
> + set_mod_args(args, 100000, 0, 0, BIT_ULL(CGX_MODE_100G_C2M));
> + break;
> + case ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT:
> + set_mod_args(args, 100000, 0, 1, BIT_ULL(CGX_MODE_100G_CR4));
> + break;
> + case ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT:
> + set_mod_args(args, 100000, 0, 1, BIT_ULL(CGX_MODE_100G_KR4));
> + break;
> + default:
> + set_mod_args(args, 0, 1, 0, BIT_ULL(CGX_MODE_MAX));
> + break;
> + }
> +}
> +
> static inline void link_status_user_format(u64 lstat,
> struct cgx_link_user_info *linfo,
> struct cgx *cgx, u8 lmac_id) @@ -887,13 +993,19 @@ int
> cgx_set_link_mode(void *cgxd, struct cgx_set_link_mode_args args,
> if (!cgx)
> return -ENODEV;
>
> + if (args.mode)
> + otx2_map_ethtool_link_modes(args.mode, &args);
> + if (!args.speed && args.duplex && !args.an)
> + return -EINVAL;
> +
> req = FIELD_SET(CMDREG_ID, CGX_CMD_MODE_CHANGE, req);
> req = FIELD_SET(CMDMODECHANGE_SPEED,
> cgx_link_usertable_index_map(args.speed), req);
> req = FIELD_SET(CMDMODECHANGE_DUPLEX, args.duplex, req);
> req = FIELD_SET(CMDMODECHANGE_AN, args.an, req);
> req = FIELD_SET(CMDMODECHANGE_PORT, args.ports, req);
> - req = FIELD_SET(CMDMODECHANGE_FLAGS, args.flags, req);
> + req = FIELD_SET(CMDMODECHANGE_FLAGS, args.mode, req);
> +
> return cgx_fwi_cmd_generic(req, &resp, cgx, lmac_id);
> }
> int cgx_set_fec(u64 fec, int cgx_id, int lmac_id) diff --git
> a/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
> b/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
> index 70610e7..dde2bd0 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
> @@ -70,6 +70,36 @@ enum cgx_link_speed {
> CGX_LINK_SPEED_MAX,
> };
>
> +enum CGX_MODE_ {
> + CGX_MODE_SGMII,
> + CGX_MODE_1000_BASEX,
> + CGX_MODE_QSGMII,
> + CGX_MODE_10G_C2C,
> + CGX_MODE_10G_C2M,
> + CGX_MODE_10G_KR,
> + CGX_MODE_20G_C2C,
> + CGX_MODE_25G_C2C,
> + CGX_MODE_25G_C2M,
> + CGX_MODE_25G_2_C2C,
> + CGX_MODE_25G_CR,
> + CGX_MODE_25G_KR,
> + CGX_MODE_40G_C2C,
> + CGX_MODE_40G_C2M,
> + CGX_MODE_40G_CR4,
> + CGX_MODE_40G_KR4,
> + CGX_MODE_40GAUI_C2C,
> + CGX_MODE_50G_C2C,
> + CGX_MODE_50G_C2M,
> + CGX_MODE_50G_4_C2C,
> + CGX_MODE_50G_CR,
> + CGX_MODE_50G_KR,
> + CGX_MODE_80GAUI_C2C,
> + CGX_MODE_100G_C2C,
> + CGX_MODE_100G_C2M,
> + CGX_MODE_100G_CR4,
> + CGX_MODE_100G_KR4,
> + CGX_MODE_MAX /* = 29 */
> +};
> /* REQUEST ID types. Input to firmware */
> enum cgx_cmd_id {
> CGX_CMD_NONE,
> @@ -231,6 +261,6 @@ struct cgx_lnk_sts {
> #define CMDMODECHANGE_DUPLEX GENMASK_ULL(12, 12)
> #define CMDMODECHANGE_AN GENMASK_ULL(13, 13)
> #define CMDMODECHANGE_PORT GENMASK_ULL(21, 14)
> -#define CMDMODECHANGE_FLAGS GENMASK_ULL(29, 22)
> +#define CMDMODECHANGE_FLAGS GENMASK_ULL(63, 22)
>
> #endif /* __CGX_FW_INTF_H__ */
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> index a050902..05a6da2 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -462,10 +462,11 @@ struct cgx_set_link_mode_args {
> u8 duplex;
> u8 an;
> u8 ports;
> - u8 flags;
> + u64 mode;
> };
>
> struct cgx_set_link_mode_req {
> +#define AUTONEG_UNKNOWN 0xff
> struct mbox_msghdr hdr;
> struct cgx_set_link_mode_args args;
> };

Thanks,
Chris