Index | Thread | Search

From:
Visa Hankala <visa@hankala.org>
Subject:
Re: sys/cnmac: support CN71xx 1000BASE-X ports
To:
"Kirill A. Korinsky" <kirill@korins.ky>
Cc:
tech@openbsd.org
Date:
Tue, 21 Apr 2026 16:09:59 +0000

Download raw body.

Thread
On Mon, Apr 20, 2026 at 10:07:40PM +0200, Kirill A. Korinsky wrote:
> tech@, visa@,
> 
> Some CN71xx boards describe active GMX ports only in the PIP device
> tree, and mark the CPU facing link as cavium,sgmii-mac-1000x-mode with
> cavium,disable-autonegotiation, but without a PHY handle. OpenBSD
> otherwise trusts GMX0_INF_MODE for port discovery and insists on a PHY
> attach in cn30xxgmx_attach(), so such ports never reach cnmac with a
> usable media setup.
> 
> Enumerate CN71xx SGMII ports from pip/interface@N when that description
> is present, carry the 1000x and disable-autonegotiation flags into the
> per port state, and let cnmac seed fixed 1000baseSX full duplex media
> for that case. Ports that still use a normal SGMII PHY path continue to
> go through cn30xxsmi_get_phy() and mii_attach() unchanged.
> 
> Tested on two CN71xx Octeon systems: Juniper SRX300, which uses
> 1000BASE-X DT ports, and Ubiquiti EdgeRouter 4, which does not.
> 
> This diff brings only 0/0 port on SRX300 and requires to use that command in
> uboot:
> 
>     usb reset; dhcp; fatload usb 0 ${loadaddr} boot; bootoctlinux rootdev=sd0 numcores=2
> 
> but it boots and I have network.
> 
> Ok?
> 
> Index: sys/arch/octeon/dev/cn30xxgmx.c
> ===================================================================
> RCS file: /home/cvs/src/sys/arch/octeon/dev/cn30xxgmx.c,v
> diff -u -p -r1.55 cn30xxgmx.c
> --- sys/arch/octeon/dev/cn30xxgmx.c	8 Jul 2024 08:07:45 -0000	1.55
> +++ sys/arch/octeon/dev/cn30xxgmx.c	20 Apr 2026 17:56:27 -0000
> @@ -169,21 +169,63 @@ cn30xxgmx_match(struct device *parent, v
>  }
>  
>  int
> -cn30xxgmx_get_phy_phandle(int interface, int port)
> +cn30xxgmx_get_phy_phandle(int interface, int port, int *port_1000x,
> +    int *disable_an)
>  {
>  	char name[64];
>  	int node;
>  	int phandle = 0;
>  
> +	if (port_1000x != NULL)
> +		*port_1000x = 0;
> +	if (disable_an != NULL)
> +		*disable_an = 0;
> +
>  	snprintf(name, sizeof(name),
>  	    "/soc/pip@11800a0000000/interface@%x/ethernet@%x",
>  	    interface, port);
>  	node = OF_finddevice(name);
> -	if (node != - 1)
> +	if (node != -1) {
>  		phandle = OF_getpropint(node, "phy-handle", 0);
> +		if (port_1000x != NULL) {
> +			*port_1000x = OF_getproplen(node,
> +			    "cavium,sgmii-mac-1000x-mode") >= 0;
> +		}
> +		if (disable_an != NULL) {
> +			*disable_an = OF_getproplen(node,
> +			    "cavium,disable-autonegotiation") >= 0;
> +		}
> +	}
>  	return phandle;
>  }
>  
> +int
> +cn30xxgmx_init_cn71xx(struct cn30xxgmx_softc *sc)
> +{
> +	char name[64];
> +	int child, node, reg;
> +
> +	snprintf(name, sizeof(name),
> +	    "/soc/pip@11800a0000000/interface@%x", sc->sc_unitno);
> +	node = OF_finddevice(name);
> +	if (node == -1)
> +		return 1;
> +
> +	for (child = OF_child(node); child != 0; child = OF_peer(child)) {
> +		if (!OF_is_compatible(child, "cavium,octeon-3860-pip-port"))
> +			continue;
> +		reg = OF_getpropint(child, "reg", -1);
> +		if (reg < 0 || reg >= nitems(sc->sc_port_types))
> +			continue;
> +
> +		sc->sc_port_types[reg] = GMX_SGMII_PORT;

If I read this correctly, this uses SGMII mode regardless of the
actual port mode. I think this port type setting should be conditional
to the cavium,sgmii-mac-1000x-mode property.

However, does the INF_MODE logic give an incorrect result with your
hardware?

> +		if (sc->sc_nports < reg + 1)
> +			sc->sc_nports = reg + 1;
> +	}
> +
> +	return sc->sc_nports == 0 ? 1 : 0;
> +}
> +
>  void
>  cn30xxgmx_attach(struct device *parent, struct device *self, void *aux)
>  {
> @@ -193,6 +235,7 @@ cn30xxgmx_attach(struct device *parent, 
>  	struct cn30xxgmx_softc *sc = (void *)self;
>  	struct cn30xxsmi_softc *smi;
>  	int i;
> +	int phandle;
>  	int phy_addr;
>  	int port;
>  	int status;
> @@ -224,15 +267,20 @@ cn30xxgmx_attach(struct device *parent, 
>  	printf("\n");
>  
>  	for (i = 0; i < sc->sc_nports; i++) {
> +		port_sc = &sc->sc_ports[i];
>  		if (sc->sc_port_types[i] == GMX_AGL_PORT)
>  			port = 24;
>  		else
>  			port = GMX_PORT_NUM(sc->sc_unitno, i);
> -		if (cn30xxsmi_get_phy(cn30xxgmx_get_phy_phandle(sc->sc_unitno,
> -		    i), port, &smi, &phy_addr))
> +		phandle = cn30xxgmx_get_phy_phandle(sc->sc_unitno, i,
> +		    &port_sc->sc_port_1000x,
> +		    &port_sc->sc_port_disable_an);
> +		smi = NULL;
> +		phy_addr = -1;
> +		if (!(port_sc->sc_port_1000x && phandle == 0) &&
> +		    cn30xxsmi_get_phy(phandle, port, &smi, &phy_addr))
>  			continue;
>  
> -		port_sc = &sc->sc_ports[i];
>  		port_sc->sc_port_gmx = sc;
>  		port_sc->sc_port_no = port;
>  		port_sc->sc_port_type = sc->sc_port_types[i];
> @@ -421,6 +469,9 @@ cn30xxgmx_init(struct cn30xxgmx_softc *s
>  			break;
>  		}
>  
> +		if (cn30xxgmx_init_cn71xx(sc) == 0)
> +			break;

I think this would be clearer like this:

		cn30xxgmx_init_cn71xx(sc);
		if (sc->sc_nports != 0)
			break;

However, see the above question about INF_MODE.

> +
>  		inf_mode = bus_space_read_8(sc->sc_regt, sc->sc_regh,
>  		    GMX0_INF_MODE);
>  		if ((inf_mode & INF_MODE_EN) == 0)
> @@ -1341,6 +1392,17 @@ cn30xxgmx_sgmii_enable(struct cn30xxgmx_
>  		return 1;
>  	}
>  
> +	if (sc->sc_port_disable_an) {
> +		CLR(ctl_reg, PCS_MR_CONTROL_AN_EN);
> +		CLR(ctl_reg, PCS_MR_CONTROL_RST_AN);
> +		CLR(ctl_reg, PCS_MR_CONTROL_PWR_DN);
> +		CLR(ctl_reg, PCS_MR_CONTROL_SPDLSB);
> +		SET(ctl_reg, PCS_MR_CONTROL_SPDMSB);
> +		SET(ctl_reg, PCS_MR_CONTROL_DUPLEX);
> +		PCS_WRITE_8(sc, PCS_MR_CONTROL, ctl_reg);
> +		return 0;
> +	}
> +
>  	/* Start a new SGMII autonegotiation. */
>  	SET(ctl_reg, PCS_MR_CONTROL_AN_EN);
>  	SET(ctl_reg, PCS_MR_CONTROL_RST_AN);
> @@ -1382,6 +1444,8 @@ cn30xxgmx_sgmii_speed(struct cn30xxgmx_p
>  
>  	misc_ctl = PCS_READ_8(sc, PCS_MISC_CTL);
>  	CLR(misc_ctl, PCS_MISC_CTL_SAMP_PT);
> +	if (sc->sc_port_disable_an)
> +		SET(misc_ctl, PCS_MISC_CTL_AN_OVRD);

I wonder if this should set (or clear) the PCS_MISC_CTL_MODE bit if
the SGMII MAC mode is enabled (or not). Would this help with the
hardware initialization during boot?

>  
>  	/* Disable the GMX port if the link is down. */
>  	if (cn30xxgmx_link_status(sc))
> Index: sys/arch/octeon/dev/cn30xxgmxvar.h
> ===================================================================
> RCS file: /home/cvs/src/sys/arch/octeon/dev/cn30xxgmxvar.h,v
> diff -u -p -r1.14 cn30xxgmxvar.h
> --- sys/arch/octeon/dev/cn30xxgmxvar.h	20 May 2024 23:13:33 -0000	1.14
> +++ sys/arch/octeon/dev/cn30xxgmxvar.h	20 Apr 2026 17:28:22 -0000
> @@ -61,6 +61,8 @@ struct cn30xxgmx_port_softc {
>  	bus_space_handle_t	sc_port_regh;
>  	int			sc_port_no;	/* GMX0:0, GMX0:1, ... */
>  	int			sc_port_type;
> +	int			sc_port_1000x;
> +	int			sc_port_disable_an;
>  	uint64_t		sc_link;
>  	struct mii_data		*sc_port_mii;
>  	struct arpcom		*sc_port_ac;
> Index: sys/arch/octeon/dev/if_cnmac.c
> ===================================================================
> RCS file: /home/cvs/src/sys/arch/octeon/dev/if_cnmac.c,v
> diff -u -p -r1.86 if_cnmac.c
> --- sys/arch/octeon/dev/if_cnmac.c	20 May 2024 23:13:33 -0000	1.86
> +++ sys/arch/octeon/dev/if_cnmac.c	20 Apr 2026 17:28:22 -0000
> @@ -471,6 +471,19 @@ cnmac_mediainit(struct cnmac_softc *sc)
>  	ifmedia_init(&sc->sc_mii.mii_media, 0, cnmac_mediachange,
>  	    cnmac_mediastatus);
>  
> +	if (sc->sc_gmx_port->sc_port_1000x) {
> +		ifmedia_add(&sc->sc_mii.mii_media,
> +		    IFM_ETHER | IFM_1000_SX | IFM_FDX, 0, NULL);
> +		ifmedia_set(&sc->sc_mii.mii_media,
> +		    IFM_ETHER | IFM_1000_SX | IFM_FDX);
> +		sc->sc_mii.mii_media_status = IFM_AVALID | IFM_ACTIVE;
> +		sc->sc_mii.mii_media_active =
> +		    IFM_ETHER | IFM_1000_SX | IFM_FDX;

Unless the exact Ethernet type is essential, I would use IFM_1000_T
as a "generic" type here, because the link type is not known.

> +		ifp->if_baudrate = IF_Gbps(1);
> +		ifp->if_link_state = LINK_STATE_FULL_DUPLEX;
> +		return 0;
> +	}
> +
>  	mii_attach(&sc->sc_dev, &sc->sc_mii,
>  	    0xffffffff, sc->sc_phy_addr, MII_OFFSET_ANY, MIIF_DOPAUSE);
>  
> @@ -492,6 +505,14 @@ cnmac_mediastatus(struct ifnet *ifp, str
>  {
>  	struct cnmac_softc *sc = ifp->if_softc;
>  
> +	if (sc->sc_gmx_port->sc_port_1000x) {
> +		ifmr->ifm_status = sc->sc_mii.mii_media_status;
> +		ifmr->ifm_active = (sc->sc_mii.mii_media_active &
> +		    ~IFM_ETH_FMASK) |
> +		    sc->sc_gmx_port->sc_port_flowflags;
> +		return;
> +	}
> +
>  	mii_pollstat(&sc->sc_mii);
>  	ifmr->ifm_status = sc->sc_mii.mii_media_status;
>  	ifmr->ifm_active = sc->sc_mii.mii_media_active;
> @@ -503,6 +524,9 @@ int
>  cnmac_mediachange(struct ifnet *ifp)
>  {
>  	struct cnmac_softc *sc = ifp->if_softc;
> +
> +	if (sc->sc_gmx_port->sc_port_1000x)
> +		return 0;
>  
>  	if ((ifp->if_flags & IFF_UP) == 0)
>  		return 0;
> 
> 
> -- 
> wbr, Kirill
>