[PATCH net-next] octeontx2-pf: report port connector type in ethtool link ksettings

[PATCH net-next] octeontx2-pf: report port connector type in ethtool link ksettings

[Ratheesh Kannoth]

From: Hariprasad Kelam <hkelam@marvell.com>

otx2_get_link_ksettings() already fills speed, duplex, autoneg, link
modes and FEC from CGX firmware data, but leaves cmd->base.port unset.
As a result, ethtool does not show the physical connector type even
though the firmware provides it in fwdata.port.

Populate cmd->base.port from rsp->fwdata.port so users can see values
such as PORT_TP or PORT_AUI via ethtool.

Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
index 9bee1b91eeaa..65630de43539 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
@@ -1231,6 +1231,8 @@ static int otx2_get_link_ksettings(struct net_device *netdev,
 				OTX2_MODE_SUPPORTED, cmd);
 	otx2_get_fec_info(rsp->fwdata.supported_fec,
 			  OTX2_MODE_SUPPORTED, cmd);
+
+	cmd->base.port = rsp->fwdata.port;
 	return 0;
 }
 
-- 
2.43.0

Re: [PATCH net-next] octeontx2-pf: report port connector type in ethtool link ksettings

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] otx2_get_link_ksettings()
  (drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c:1235)…

--

octeontx2-pf: report port connector type in ethtool link ksettings

This adds a single assignment to otx2_get_link_ksettings() so that
cmd->base.port is filled from the CGX firmware data word
rsp->fwdata.port, with the intent of letting ethtool display the
physical connector type.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> index 9bee1b91eeaa6..65630de435397 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> @@ -1231,6 +1231,8 @@ static int otx2_get_link_ksettings(struct net_device *netdev,
>  				OTX2_MODE_SUPPORTED, cmd);
>  	otx2_get_fec_info(rsp->fwdata.supported_fec,
>  			  OTX2_MODE_SUPPORTED, cmd);
> +
> +	cmd->base.port = rsp->fwdata.port;

[Severity: Medium]

Should this raw firmware word be translated before it is exported as the
ethtool connector type?

The source field is a 64-bit value in af/mbox.h:

struct cgx_lmac_fwdata_s {
	...
	u32 lmac_type;
	u32 portm_idx;
	u64 mgmt_port:1;
	u64 advertised_an:1;
	u64 port;
	...
};

The destination is the __u8 uAPI field in struct ethtool_link_settings,
documented as "@port: Physical connector type; one of %PORT_*", whose value
space is sparse and exhaustive (PORT_TP 0x00 .. PORT_DA 0x05, PORT_NONE
0xef, PORT_OTHER 0xff).

So the assignment silently narrows to 8 bits, and a firmware value of, say,
0x100 becomes 0x00, which userspace prints as Twisted Pair.  A value in
0x06..0xee is not a legal PORT_* code at all and ethtool prints "Unknown!".
The same byte then becomes the base value for ethnl_update_u8(&lsettings->port, ...)
on the netlink set path.

Would a switch or table mapping the firmware encoding to PORT_*, with
PORT_OTHER for anything unrecognised, be preferable here?

Every other consumer of the same firmware structure in this function does
translate: otx2_get_link_mode_info() maps CGX mode bits to
ETHTOOL_LINK_MODE_* bits, otx2_get_fec_info() maps OTX2_FEC_* to
ETHTOOL_LINK_MODE_FEC_*, and otx2_get_fecparam() bounds-checks against
OTX2_FEC_MAX_INDEX.

Is there anything that tells the driver whether the firmware populated this
word at all?

The field was carved out of the reserved tail of struct cgx_lmac_fwdata_s by
commit 49f02e6877d1be ("Octeontx2-af: Debugfs support for firmware data",
LMAC_FWDATA_RESERVED_MEM 1021 -> 1018) without bumping RVU_FWDATA_VERSION,
which is still 0x0001 in af/rvu.h.  The only check applied to the ATF region
is the outer header check:

static inline bool is_rvu_fwdata_valid(struct rvu *rvu)
{
	return (rvu->fwdata->header_magic == RVU_FWDATA_HEADER_MAGIC) &&
		(rvu->fwdata->version == RVU_FWDATA_VERSION);
}

and rvu_mbox_handler_cgx_get_aux_link_info() copies the region verbatim:

		memcpy(&rsp->fwdata,
		       &rvu->fwdata->cgx_fw_data[cgx_id][lmac_id],
		       sizeof(struct cgx_lmac_fwdata_s));

The struct's own rw_valid indicator is not consulted by the new code.  On
firmware that does not fill this word it reads 0, which is exactly PORT_TP,
so a fibre or DAC CGX/RPM SERDES port would be reported as Twisted Pair
rather than left unspecified.  Could the assignment be gated on
fwdata.rw_valid, or on a bumped firmware-data version, and fall back to
PORT_OTHER or PORT_NONE?

One more question about the commit message:

	Populate cmd->base.port from rsp->fwdata.port so users can see values
	such as PORT_TP or PORT_AUI via ethtool.

This states the firmware supplies PORT_* values, but a grep of
drivers/net/ethernet/marvell/octeontx2/ shows this new line is the only
reference to fwdata.port in the tree, and there is no firmware-encoding to
PORT_* table anywhere.  Can the firmware encoding, and the firmware version
that started populating the field, be documented in the changelog?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260901045857.2779326-1-rkannoth%40marvell.com

Re: [PATCH net-next] octeontx2-pf: report port connector type in ethtool link ksettings

[Ratheesh Kannoth]

On 2026-09-04 at 13:29:24, netdev-bot+sashiko@kernel.org (netdev-bot+sashiko@kernel.org) wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
>
> - [Medium] otx2_get_link_ksettings()
>   (drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c:1235)…
>
> --
>
> octeontx2-pf: report port connector type in ethtool link ksettings

> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260901045857.2779326-1-rkannoth%40marvell.com

i think, below sole issue can be ignored.

+---+----------+--------+----------------------+---------------------------------------------------+
| # | Severity | Status | Location             | Issue                                             |
+===+==========+========+======================+===================================================+
| 1 | Medium   | New    | otx2_get_link_ksettin| Raw rsp->fwdata.port (u64 firmware field) is      |
|   |          |        | s()                  | assigned directly to cmd->base.port (__u8 ethtool |
|   |          |        | drivers/net/ethernet/| PORT_* field) with no translation, bounds check, o|
|   |          |        | arvell/octeontx2/nic/| validity gate.                                    |
|   |          |        | tx2_ethtool.c:1235   |                                                   |
+---+----------+--------+----------------------+---------------------------------------------------+

Re: [PATCH net-next] octeontx2-pf: report port connector type in ethtool link ksettings

[Simon Horman]

On Fri, Sep 04, 2026 at 02:31:23PM +0530, Ratheesh Kannoth wrote:
> On 2026-09-04 at 13:29:24, netdev-bot+sashiko@kernel.org (netdev-bot+sashiko@kernel.org) wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential
> > issue(s) to consider:
> >
> > Critical: 0 · High: 0 · Medium: 1 · Low: 0
> >
> > - [Medium] otx2_get_link_ksettings()
> >   (drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c:1235)…
> >
> > --
> >
> > octeontx2-pf: report port connector type in ethtool link ksettings
> 
> > Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260901045857.2779326-1-rkannoth%40marvell.com
> 
> i think, below sole issue can be ignored.

I'm assuming this is because the firmware always returns valid ethtool values.
If so, it might be worth spelling that out in the commit message.
But I'm not going to get hung up on that.

Reviewed-by: Simon Horman <horms@kernel.org>

> 
> +---+----------+--------+----------------------+---------------------------------------------------+
> | # | Severity | Status | Location             | Issue                                             |
> +===+==========+========+======================+===================================================+
> | 1 | Medium   | New    | otx2_get_link_ksettin| Raw rsp->fwdata.port (u64 firmware field) is      |
> |   |          |        | s()                  | assigned directly to cmd->base.port (__u8 ethtool |
> |   |          |        | drivers/net/ethernet/| PORT_* field) with no translation, bounds check, o|
> |   |          |        | arvell/octeontx2/nic/| validity gate.                                    |
> |   |          |        | tx2_ethtool.c:1235   |                                                   |
> +---+----------+--------+----------------------+---------------------------------------------------+