Fix up support for the AR8327.

* Even though I got the registers around "right", it seems
  I'm not tickling the MDIO access correctly for the internal PHY
  bus.  Some of the switches are fine poking at the external PHY
  registers; others aren't.  So, enable direct PHY bus access
  for the AR8327, and leave the existing code in place for the
  others.

* Go and shuffle the register access around.  Whilst here,
  restore the 2ms delay if changing page.

* Comment out some of the stub printf()s; there's some upcoming
  work to add port VLAN support.

Tested:

* AP135 development board
* Carambola2 - AR9331 SoC

6 files changed, 237 insertions, 66 deletions

diff --git a/sys/dev/etherswitch/arswitch/arswitch.c b/sys/dev/etherswitch/arswitch/arswitch.c
index 129a77d..32509da 100644
--- a/sys/dev/etherswitch/arswitch/arswitch.c
+++ b/sys/dev/etherswitch/arswitch/arswitch.c
@@ -149,8 +149,10 @@ done:
 	DPRINTF(dev, "chipname=%s, id=%08x\n", chipname, id);
 	if (chipname != NULL) {
 		snprintf(desc, sizeof(desc),
-		    "Atheros %s Ethernet Switch",
-		    chipname);
+		    "Atheros %s Ethernet Switch (ver %d rev %d)",
+		    chipname,
+		    sc->chip_ver,
+		    sc->chip_rev);
 		device_set_desc_copy(dev, desc);
 		return (BUS_PROBE_DEFAULT);
 	}
@@ -177,9 +179,11 @@ arswitch_attach_phys(struct arswitch_softc *sc)
 		err = mii_attach(sc->sc_dev, &sc->miibus[phy], sc->ifp[phy],
 		    arswitch_ifmedia_upd, arswitch_ifmedia_sts, \
 		    BMSR_DEFCAPMASK, phy, MII_OFFSET_ANY, 0);
+#if 0
 		DPRINTF(sc->sc_dev, "%s attached to pseudo interface %s\n",
 		    device_get_nameunit(sc->miibus[phy]),
 		    sc->ifp[phy]->if_xname);
+#endif
 		if (err != 0) {
 			device_printf(sc->sc_dev,
 			    "attaching PHY %d failed\n",
@@ -304,6 +308,8 @@ arswitch_attach(device_t dev)
 	sc->hal.arswitch_vlan_get_pvid = ar8xxx_get_pvid;
 	sc->hal.arswitch_vlan_set_pvid = ar8xxx_set_pvid;
 	sc->hal.arswitch_atu_flush = ar8xxx_atu_flush;
+	sc->hal.arswitch_phy_read = arswitch_readphy_internal;
+	sc->hal.arswitch_phy_write = arswitch_writephy_internal;
 
 	/*
 	 * Attach switch related functions
@@ -320,8 +326,10 @@ arswitch_attach(device_t dev)
 		ar8316_attach(sc);
 	else if (AR8X16_IS_SWITCH(sc, AR8327))
 		ar8327_attach(sc);
-	else
+	else {
+		DPRINTF(dev, "%s: unknown switch (%d)?\n", __func__, sc->sc_switchtype);
 		return (ENXIO);
+	}
 
 	/* Common defaults. */
 	sc->info.es_nports = 5; /* XXX technically 6, but 6th not used */
@@ -348,14 +356,18 @@ arswitch_attach(device_t dev)
 		sc->numphys = AR8X16_NUM_PHYS;
 
 	/* Reset the switch. */
-	if (arswitch_reset(dev))
+	if (arswitch_reset(dev)) {
+		DPRINTF(dev, "%s: arswitch_reset: failed\n", __func__);
 		return (ENXIO);
+	}
 
 	err = sc->hal.arswitch_hw_setup(sc);
+	DPRINTF(dev, "%s: hw_setup: err=%d\n", __func__, err);
 	if (err != 0)
 		return (err);
 
 	err = sc->hal.arswitch_hw_global_setup(sc);
+	DPRINTF(dev, "%s: hw_global_setup: err=%d\n", __func__, err);
 	if (err != 0)
 		return (err);
 
@@ -368,17 +380,20 @@ arswitch_attach(device_t dev)
 	 * Attach the PHYs and complete the bus enumeration.
 	 */
 	err = arswitch_attach_phys(sc);
+	DPRINTF(dev, "%s: attach_phys: err=%d\n", __func__, err);
 	if (err != 0)
 		return (err);
 
 	/* Default to ingress filters off. */
 	err = arswitch_set_vlan_mode(sc, 0);
+	DPRINTF(dev, "%s: set_vlan_mode: err=%d\n", __func__, err);
 	if (err != 0)
 		return (err);
 
 	bus_generic_probe(dev);
 	bus_enumerate_hinted_children(dev);
 	err = bus_generic_attach(dev);
+	DPRINTF(dev, "%s: bus_generic_attach: err=%d\n", __func__, err);
 	if (err != 0)
 		return (err);
 	
@@ -803,6 +818,22 @@ arswitch_setvgroup(device_t dev, etherswitch_vlangroup_t *e)
 	return (sc->hal.arswitch_vlan_setvgroup(sc, e));
 }
 
+static int
+arswitch_readphy(device_t dev, int phy, int reg)
+{
+	struct arswitch_softc *sc = device_get_softc(dev);
+
+	return (sc->hal.arswitch_phy_read(dev, phy, reg));
+}
+
+static int
+arswitch_writephy(device_t dev, int phy, int reg, int val)
+{
+	struct arswitch_softc *sc = device_get_softc(dev);
+
+	return (sc->hal.arswitch_phy_write(dev, phy, reg, val));
+}
+
 static device_method_t arswitch_methods[] = {
 	/* Device interface */
 	DEVMETHOD(device_probe,		arswitch_probe),
diff --git a/sys/dev/etherswitch/arswitch/arswitch_8327.c b/sys/dev/etherswitch/arswitch/arswitch_8327.c
index 642827d..0bf1265 100644
--- a/sys/dev/etherswitch/arswitch/arswitch_8327.c
+++ b/sys/dev/etherswitch/arswitch/arswitch_8327.c
@@ -57,6 +57,9 @@
 #include <dev/etherswitch/arswitch/arswitchreg.h>
 #include <dev/etherswitch/arswitch/arswitchvar.h>
 #include <dev/etherswitch/arswitch/arswitch_reg.h>
+#include <dev/etherswitch/arswitch/arswitch_phy.h>
+#include <dev/etherswitch/arswitch/arswitch_vlans.h>
+
 #include <dev/etherswitch/arswitch/arswitch_8327.h>
 
 #include "mdio_if.h"
@@ -290,7 +293,7 @@ ar8327_fetch_pdata_port(struct arswitch_softc *sc,
 	    sbuf, &val) == 0)
 		pcfg->rxpause = val;
 
-#if 0
+#if 1
 	device_printf(sc->sc_dev,
 	    "%s: port %d: speed=%d, duplex=%d, txpause=%d, rxpause=%d\n",
 	    __func__,
@@ -562,6 +565,7 @@ ar8327_init_pdata(struct arswitch_softc *sc)
 	/* SGMII config */
 	bzero(&scfg, sizeof(scfg));
 	if (ar8327_fetch_pdata_sgmii(sc, &scfg)) {
+		device_printf(sc->sc_dev, "%s: SGMII cfg?\n", __func__);
 		t = scfg.sgmii_ctrl;
 		if (sc->chip_rev == 1)
 			t |= AR8327_SGMII_CTRL_EN_PLL |
@@ -657,12 +661,16 @@ ar8327_hw_global_setup(struct arswitch_softc *sc)
 }
 
 /*
- * Port setup.
+ * Port setup.  Called at attach time.
  */
 static void
 ar8327_port_init(struct arswitch_softc *sc, int port)
 {
 	uint32_t t;
+	int ports;
+
+	/* For now, port can see all other ports */
+	ports = 0x7f;
 
 	if (port == AR8X16_PORT_CPU)
 		t = sc->ar8327.port0_status;
@@ -696,7 +704,7 @@ ar8327_port_init(struct arswitch_softc *sc, int port)
 	t |= AR8X16_PORT_CTRL_STATE_FORWARD << AR8327_PORT_LOOKUP_STATE_S;
 
 	/* So this allows traffic to any port except ourselves */
-	t |= (0x7f & ~(1 << port));
+	t |= (ports & ~(1 << port));
 	arswitch_writereg(sc->sc_dev, AR8327_REG_PORT_LOOKUP(port), t);
 }
 
@@ -705,16 +713,18 @@ ar8327_port_vlan_setup(struct arswitch_softc *sc, etherswitch_port_t *p)
 {
 
 	/* XXX stub for now */
-	device_printf(sc->sc_dev, "%s: called\n", __func__);
+//	device_printf(sc->sc_dev, "%s: called\n", __func__);
 	return (0);
 }
 
+/*
+ * Get the port VLAN configuration.
+ */
 static int
 ar8327_port_vlan_get(struct arswitch_softc *sc, etherswitch_port_t *p)
 {
-
 	/* XXX stub for now */
-	device_printf(sc->sc_dev, "%s: called\n", __func__);
+//	device_printf(sc->sc_dev, "%s: called\n", __func__);
 	return (0);
 }
 
@@ -723,6 +733,13 @@ ar8327_reset_vlans(struct arswitch_softc *sc)
 {
 	int i;
 	uint32_t mode, t;
+	int ports;
+
+	ARSWITCH_LOCK_ASSERT(sc, MA_NOTOWNED);
+	ARSWITCH_LOCK(sc);
+
+	/* Clear the existing VLAN configuration */
+	memset(sc->vid, 0, sizeof(sc->vid));
 
 	/*
 	 * Disable mirroring.
@@ -732,10 +749,21 @@ ar8327_reset_vlans(struct arswitch_softc *sc)
 	    (0xF << AR8327_FWD_CTRL0_MIRROR_PORT_S));
 
 	/*
+	 * XXX TODO: disable any Q-in-Q port configuration,
+	 * tagging, egress filters, etc.
+	 */
+
+	/*
 	 * For now, let's default to one portgroup, just so traffic
-	 * flows.  All ports can see other ports.
+	 * flows.  All ports can see other ports. There are two CPU GMACs
+	 * (GMAC0, GMAC6), GMAC1..GMAC5 are external PHYs.
+	 *
+	 * (ETHERSWITCH_VLAN_PORT)
 	 */
+	ports = 0x7f;
+
 	for (i = 0; i < AR8327_NUM_PORTS; i++) {
+
 		/* set pvid = 1; there's only one vlangroup */
 		t = 1 << AR8327_PORT_VLAN0_DEF_SVID_S;
 		t |= 1 << AR8327_PORT_VLAN0_DEF_CVID_S;
@@ -749,7 +777,7 @@ ar8327_reset_vlans(struct arswitch_softc *sc)
 		arswitch_writereg(sc->sc_dev, AR8327_REG_PORT_VLAN1(i), t);
 
 		/* Ports can see other ports */
-		t = (0x7f & ~(1 << i));	/* all ports besides us */
+		t = (ports & ~(1 << i));	/* all ports besides us */
 		t |= AR8327_PORT_LOOKUP_LEARN;
 
 		/* in_port_only, forward */
@@ -769,12 +797,20 @@ ar8327_reset_vlans(struct arswitch_softc *sc)
 		    AR8327_PORT_HOL_CTRL1_EG_MIRROR_EN,
 		    0);
 	}
+
+	ARSWITCH_UNLOCK(sc);
 }
 
 static int
 ar8327_vlan_getvgroup(struct arswitch_softc *sc, etherswitch_vlangroup_t *vg)
 {
-	device_printf(sc->sc_dev, "%s: called\n", __func__);
+
+#if 0
+	/* XXX for now, no dot1q vlans */
+	if (sc->vlan_mode == ETHERSWITCH_VLAN_DOT1Q)
+		return (EINVAL);
+	return (ar8xxx_getvgroup(sc, vg));
+#endif
 	return (0);
 }
 
@@ -782,7 +818,12 @@ static int
 ar8327_vlan_setvgroup(struct arswitch_softc *sc, etherswitch_vlangroup_t *vg)
 {
 
-	device_printf(sc->sc_dev, "%s: called\n", __func__);
+#if 0
+	/* XXX for now, no dot1q vlans */
+	if (sc->vlan_mode == ETHERSWITCH_VLAN_DOT1Q)
+		return (EINVAL);
+	return (ar8xxx_setvgroup(sc, vg));
+#endif
 	return (0);
 }
 
@@ -832,17 +873,29 @@ ar8327_attach(struct arswitch_softc *sc)
 	sc->hal.arswitch_hw_global_setup = ar8327_hw_global_setup;
 
 	sc->hal.arswitch_port_init = ar8327_port_init;
+
+	sc->hal.arswitch_vlan_getvgroup = ar8327_vlan_getvgroup;
+	sc->hal.arswitch_vlan_setvgroup = ar8327_vlan_setvgroup;
 	sc->hal.arswitch_port_vlan_setup = ar8327_port_vlan_setup;
 	sc->hal.arswitch_port_vlan_get = ar8327_port_vlan_get;
 
 	sc->hal.arswitch_vlan_init_hw = ar8327_reset_vlans;
-	sc->hal.arswitch_vlan_getvgroup = ar8327_vlan_getvgroup;
-	sc->hal.arswitch_vlan_setvgroup = ar8327_vlan_setvgroup;
 	sc->hal.arswitch_vlan_get_pvid = ar8327_get_pvid;
 	sc->hal.arswitch_vlan_set_pvid = ar8327_set_pvid;
 
 	sc->hal.arswitch_atu_flush = ar8327_atu_flush;
 
+	/*
+	 * Reading the PHY via the MDIO interface currently doesn't
+	 * work correctly.
+	 *
+	 * So for now, just go direct to the PHY registers themselves.
+	 * This has always worked  on external devices, but not internal
+	 * devices (AR934x, AR724x, AR933x.)
+	 */
+	sc->hal.arswitch_phy_read = arswitch_readphy_external;
+	sc->hal.arswitch_phy_write = arswitch_writephy_external;
+
 	/* Set the switch vlan capabilities. */
 	sc->info.es_vlan_caps = ETHERSWITCH_VLAN_DOT1Q |
 	    ETHERSWITCH_VLAN_PORT | ETHERSWITCH_VLAN_DOUBLE_TAG;
diff --git a/sys/dev/etherswitch/arswitch/arswitch_phy.c b/sys/dev/etherswitch/arswitch/arswitch_phy.c
index 928ca03..43abaa1 100644
--- a/sys/dev/etherswitch/arswitch/arswitch_phy.c
+++ b/sys/dev/etherswitch/arswitch/arswitch_phy.c
@@ -67,11 +67,46 @@ static SYSCTL_NODE(_debug, OID_AUTO, arswitch, CTLFLAG_RD, 0, "arswitch");
 #endif
 
 /*
- * access PHYs integrated into the switch chip through the switch's MDIO
+ * Access PHYs integrated into the switch by going direct
+ * to the PHY space itself, rather than through the switch
+ * MDIO register.
+ */
+int
+arswitch_readphy_external(device_t dev, int phy, int reg)
+{
+	int ret;
+	struct arswitch_softc *sc;
+
+	sc = device_get_softc(dev);
+
+	ARSWITCH_LOCK(sc);
+	ret = (MDIO_READREG(device_get_parent(dev), phy, reg));
+	ARSWITCH_UNLOCK(sc);
+
+	return (ret);
+}
+
+int
+arswitch_writephy_external(device_t dev, int phy, int reg, int data)
+{
+	struct arswitch_softc *sc;
+
+	sc = device_get_softc(dev);
+
+	ARSWITCH_LOCK(sc);
+	(void) MDIO_WRITEREG(device_get_parent(dev), phy,
+	    reg, data);
+	ARSWITCH_UNLOCK(sc);
+
+	return (0);
+}
+
+/*
+ * Access PHYs integrated into the switch chip through the switch's MDIO
  * control register.
  */
 int
-arswitch_readphy(device_t dev, int phy, int reg)
+arswitch_readphy_internal(device_t dev, int phy, int reg)
 {
 	struct arswitch_softc *sc;
 	uint32_t data = 0, ctrl;
@@ -105,8 +140,10 @@ arswitch_readphy(device_t dev, int phy, int reg)
 		if ((ctrl & AR8X16_MDIO_CTRL_BUSY) == 0)
 			break;
 	}
-	if (timeout < 0)
+	if (timeout < 0) {
+		DPRINTF(dev, "arswitch_readphy(): phy=%d.%02x; timeout=%d\n", phy, reg, timeout);
 		goto fail;
+	}
 	data = arswitch_readreg_lsb(dev, a) &
 	    AR8X16_MDIO_CTRL_DATA_MASK;
 	ARSWITCH_UNLOCK(sc);
@@ -118,7 +155,7 @@ fail:
 }
 
 int
-arswitch_writephy(device_t dev, int phy, int reg, int data)
+arswitch_writephy_internal(device_t dev, int phy, int reg, int data)
 {
 	struct arswitch_softc *sc;
 	uint32_t ctrl;
diff --git a/sys/dev/etherswitch/arswitch/arswitch_phy.h b/sys/dev/etherswitch/arswitch/arswitch_phy.h
index a3b7627..885be27 100644
--- a/sys/dev/etherswitch/arswitch/arswitch_phy.h
+++ b/sys/dev/etherswitch/arswitch/arswitch_phy.h
@@ -28,7 +28,10 @@
 #ifndef	__ARSWITCH_PHY_H__
 #define	__ARSWITCH_PHY_H__
 
-extern	int arswitch_readphy(device_t dev, int phy, int reg);
-extern	int arswitch_writephy(device_t dev, int phy, int reg, int data);
+extern	int arswitch_readphy_external(device_t dev, int phy, int reg);
+extern	int arswitch_writephy_external(device_t dev, int phy, int reg, int data);
+
+extern	int arswitch_readphy_internal(device_t dev, int phy, int reg);
+extern	int arswitch_writephy_internal(device_t dev, int phy, int reg, int data);
 
 #endif	/* __ARSWITCH_PHY_H__ */
diff --git a/sys/dev/etherswitch/arswitch/arswitch_reg.c b/sys/dev/etherswitch/arswitch/arswitch_reg.c
index 3251da8..d6797a8 100644
--- a/sys/dev/etherswitch/arswitch/arswitch_reg.c
+++ b/sys/dev/etherswitch/arswitch/arswitch_reg.c
@@ -68,12 +68,13 @@ arswitch_split_setpage(device_t dev, uint32_t addr, uint16_t *phy,
 	struct arswitch_softc *sc = device_get_softc(dev);
 	uint16_t page;
 
-	page = ((addr) >> 9) & 0xffff;
-	*phy = (((addr) >> 6) & 0x07) | 0x10;
-	*reg = ((addr) >> 1) & 0x1f;
+	page = (addr >> 9) & 0x1ff;
+	*phy = (addr >> 6) & 0x7;
+	*reg = (addr >> 1) & 0x1f;
 
 	if (sc->page != page) {
 		MDIO_WRITEREG(device_get_parent(dev), 0x18, 0, page);
+		DELAY(2000);
 		sc->page = page;
 	}
 }
@@ -87,9 +88,21 @@ static inline int
 arswitch_readreg16(device_t dev, int addr)
 {
 	uint16_t phy, reg;
-	
+
 	arswitch_split_setpage(dev, addr, &phy, &reg);
-	return (MDIO_READREG(device_get_parent(dev), phy, reg));
+	return (MDIO_READREG(device_get_parent(dev), 0x10 | phy, reg));
+}
+
+/*
+ * Write half a register.  See above!
+ */
+static inline int
+arswitch_writereg16(device_t dev, int addr, int data)
+{
+	uint16_t phy, reg;
+
+	arswitch_split_setpage(dev, addr, &phy, &reg);
+	return (MDIO_WRITEREG(device_get_parent(dev), 0x10 | phy, reg, data));
 }
 
 /*
@@ -121,93 +134,123 @@ arswitch_writemmd(device_t dev, int phy, uint16_t dbg_addr,
 	    MII_ATH_MMD_DATA, dbg_data);
 }
 
-/*
- * Write half a register
- */
-static inline int
-arswitch_writereg16(device_t dev, int addr, int data)
+static uint32_t
+arswitch_reg_read32(device_t dev, int phy, int reg)
 {
-	uint16_t phy, reg;
-	
-	arswitch_split_setpage(dev, addr, &phy, &reg);
-	return (MDIO_WRITEREG(device_get_parent(dev), phy, reg, data));
+	uint16_t lo, hi;
+	lo = MDIO_READREG(device_get_parent(dev), phy, reg);
+	hi = MDIO_READREG(device_get_parent(dev), phy, reg + 1);
+
+	return (hi << 16) | lo;
 }
 
-int
-arswitch_readreg_lsb(device_t dev, int addr)
+static int
+arswitch_reg_write32(device_t dev, int phy, int reg, uint32_t value)
 {
+	struct arswitch_softc *sc;
+	int r;
+	uint16_t lo, hi;
 
-	return (arswitch_readreg16(dev, addr));
+	sc = device_get_softc(dev);
+	lo = value & 0xffff;
+	hi = (uint16_t) (value >> 16);
+
+	if (sc->mii_lo_first) {
+		r = MDIO_WRITEREG(device_get_parent(dev),
+		    phy, reg, lo);
+		r |= MDIO_WRITEREG(device_get_parent(dev),
+		    phy, reg + 1, hi);
+	} else {
+		r = MDIO_WRITEREG(device_get_parent(dev),
+		    phy, reg + 1, hi);
+		r |= MDIO_WRITEREG(device_get_parent(dev),
+		    phy, reg, lo);
+	}
+
+	return r;
 }
 
 int
-arswitch_readreg_msb(device_t dev, int addr)
+arswitch_readreg(device_t dev, int addr)
 {
+	uint16_t phy, reg;
 
-	return (arswitch_readreg16(dev, addr + 2) << 16);
+	arswitch_split_setpage(dev, addr, &phy, &reg);
+	return arswitch_reg_read32(dev, 0x10 | phy, reg);
 }
 
 int
-arswitch_writereg_lsb(device_t dev, int addr, int data)
+arswitch_writereg(device_t dev, int addr, int value)
 {
+	struct arswitch_softc *sc;
+	uint16_t phy, reg;
 
-	return (arswitch_writereg16(dev, addr, data & 0xffff));
+	sc = device_get_softc(dev);
+
+	arswitch_split_setpage(dev, addr, &phy, &reg);
+	return (arswitch_reg_write32(dev, 0x10 | phy, reg, value));
 }
 
+/*
+ * Read/write 16 bit values in the switch register space.
+ *
+ * Some of the registers are control registers (eg the MDIO
+ * data versus control space) and so need to be treated
+ * differently.
+ */
 int
-arswitch_writereg_msb(device_t dev, int addr, int data)
+arswitch_readreg_lsb(device_t dev, int addr)
 {
 
-	return (arswitch_writereg16(dev, addr + 2, (data >> 16) & 0xffff));
+	return (arswitch_readreg16(dev, addr));
 }
 
 int
-arswitch_readreg(device_t dev, int addr)
+arswitch_readreg_msb(device_t dev, int addr)
 {
 
-	return (arswitch_readreg_lsb(dev, addr) |
-	    arswitch_readreg_msb(dev, addr));
+	return (arswitch_readreg16(dev, addr + 2) << 16);
 }
 
 int
-arswitch_writereg(device_t dev, int addr, int value)
+arswitch_writereg_lsb(device_t dev, int addr, int data)
 {
-	struct arswitch_softc *sc;
-	int r;
 
-	sc = device_get_softc(dev);
+	return (arswitch_writereg16(dev, addr, data & 0xffff));
+}
 
-	/* XXX Check the first write too? */
-	if (sc->mii_lo_first) {
-		r = arswitch_writereg_lsb(dev, addr, value);
-		r |= arswitch_writereg_msb(dev, addr, value);
-	} else {
-		r = arswitch_writereg_msb(dev, addr, value);
-		r |= arswitch_writereg_lsb(dev, addr, value);
-	}
+int
+arswitch_writereg_msb(device_t dev, int addr, int data)
+{
 
-	return r;
+	return (arswitch_writereg16(dev, addr + 2, (data >> 16) & 0xffff));
 }
 
 int
 arswitch_modifyreg(device_t dev, int addr, int mask, int set)
 {
 	int value;
-	
-	value = arswitch_readreg(dev, addr);
+	uint16_t phy, reg;
+
+	arswitch_split_setpage(dev, addr, &phy, &reg);
+
+	value = arswitch_reg_read32(dev, 0x10 | phy, reg);
 	value &= ~mask;
 	value |= set;
-	return (arswitch_writereg(dev, addr, value));
+	return (arswitch_reg_write32(dev, 0x10 | phy, reg, value));
 }
 
 int
 arswitch_waitreg(device_t dev, int addr, int mask, int val, int timeout)
 {
 	int err, v;
+	uint16_t phy, reg;
+
+	arswitch_split_setpage(dev, addr, &phy, &reg);
 
 	err = -1;
 	while (1) {
-		v = arswitch_readreg(dev, addr);
+		v = arswitch_reg_read32(dev, 0x10 | phy, reg);
 		v &= mask;
 		if (v == val) {
 			err = 0;
diff --git a/sys/dev/etherswitch/arswitch/arswitchvar.h b/sys/dev/etherswitch/arswitch/arswitchvar.h
index e533e87..51bda96 100644
--- a/sys/dev/etherswitch/arswitch/arswitchvar.h
+++ b/sys/dev/etherswitch/arswitch/arswitchvar.h
@@ -60,7 +60,7 @@ struct arswitch_softc {
 	int		is_internal_switch;
 	int		chip_ver;
 	int		chip_rev;
-	int		mii_lo_first;
+	int		mii_lo_first;		/* Send low data DWORD before high */
 	ar8x16_switch_type	sc_switchtype;
 	/* should be the max of both pre-AR8327 and AR8327 ports */
 	char		*ifname[ARSWITCH_NUM_PHYS];
@@ -98,6 +98,10 @@ struct arswitch_softc {
 		    int *);
 		int (* arswitch_vlan_set_pvid) (struct arswitch_softc *, int,
 		    int);
+
+		/* PHY functions */
+		int (* arswitch_phy_read) (device_t, int, int);
+		int (* arswitch_phy_write) (device_t, int, int, int);
 	} hal;
 
 	struct {