Re: [PATCH] net: phy: add Marvell 88X2222 transceiver support
On Tue, Feb 02, 2021 at 04:48:01PM +, Russell King - ARM Linux admin wrote:
> On Mon, Feb 01, 2021 at 10:22:51PM +0300, Ivan Bornyakov wrote:
> > +/* PMD Transmit Disable */
> > +#defineMV_TX_DISABLE 0x0009
> > +#defineMV_TX_DISABLE_GLOBALBIT(0)
>
> Please use MDIO_PMA_TXDIS and MDIO_PMD_TXDIS_GLOBAL; this is an
> IEEE802.3 defined register.
>
> > +/* 10GBASE-R PCS Status 1 */
> > +#defineMV_10GBR_STAT MDIO_STAT1
>
> Nothing Marvell specific here, please use MDIO_STAT1 directly.
>
> > +/* 1000Base-X/SGMII Control Register */
> > +#defineMV_1GBX_CTRL0x2000
> > +
> > +/* 1000BASE-X/SGMII Status Register */
> > +#defineMV_1GBX_STAT0x2001
> > +
> > +/* 1000Base-X Auto-Negotiation Advertisement Register */
> > +#defineMV_1GBX_ADVERTISE 0x2004
>
> Marvell have had a habbit of placing other PHY instances within the
> register space. This also looks like Clause 22 layout rather than
> Clause 45 layout - please use the Clause 22 definitions for the bits
> rather than Clause 45. (so BMCR_ANENABLE, BMCR_ANRESTART for
> MV_1GBX_CTRL, etc).
>
> Please define these as:
>
> +#define MV_1GBX_CTRL(0x2000 + MII_BMCR)
> +#define MV_1GBX_STAT(0x2000 + MII_BMSR)
> +#define MV_1GBX_ADVERTISE (0x2000 + MII_ADVERTISE)
>
> to make it clear what is going on here.
>
> > +static int sfp_module_insert(void *_priv, const struct sfp_eeprom_id *id)
> > +{
> > + struct phy_device *phydev = _priv;
> > + struct device *dev = >mdio.dev;
> > + struct mv_data *priv = phydev->priv;
> > + phy_interface_t interface;
> > +
> > + __ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
> > +
> > + sfp_parse_support(phydev->sfp_bus, id, supported);
> > + interface = sfp_select_interface(phydev->sfp_bus, supported);
> > +
> > + dev_info(dev, "%s SFP module inserted", phy_modes(interface));
> > +
> > + switch (interface) {
> > + case PHY_INTERFACE_MODE_10GBASER:
> > + phydev->speed = SPEED_1;
> > + phydev->interface = PHY_INTERFACE_MODE_10GBASER;
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_1baseKR_Full_BIT,
> > +phydev->supported);
> > +
> > + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > + MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> > + mv_soft_reset(phydev);
> > + break;
> > + case PHY_INTERFACE_MODE_1000BASEX:
> > + default:
> > + phydev->speed = SPEED_1000;
> > + phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
> > + linkmode_clear_bit(ETHTOOL_LINK_MODE_1baseKR_Full_BIT,
> > + phydev->supported);
> > +
> > + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > + MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> > + mv_soft_reset(phydev);
> > + }
> > +
> > + priv->sfp_inserted = true;
> > +
> > + if (priv->net_up)
> > + mv_tx_enable(phydev);
>
> This is racy. priv->net_up is modified via the suspend/resume
> callbacks, which are called with phydev->lock held. No other locks
> are guaranteed to be held.
>
> However, this function is called with the SFP sm_mutex, and rtnl
> held. Consequently, the use of sfp_inserted and net_up in this
> function and the suspend/resume callbacks is racy.
>
> Why are you disabling the transmitter anyway? Is this for power
> saving?
>
Actually, the original thought was to down the link on the other side,
when network interface is down on our side. Power saving is a nice
side-effect.
> > +static void mv_update_interface(struct phy_device *phydev)
> > +{
> > + if ((phydev->speed == SPEED_1000 ||
> > +phydev->speed == SPEED_100 ||
> > +phydev->speed == SPEED_10) &&
> > + phydev->interface != PHY_INTERFACE_MODE_1000BASEX) {
> > + phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
> > +
> > + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > + MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> > + mv_soft_reset(phydev);
> > + } else if (phydev->speed == SPEED_1 &&
> > + phydev->interface != PHY_INTERFACE_MODE_10GBASER) {
> > + phydev->interface = PHY_INTERFACE_MODE_10GBASER;
> > +
> > + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > + MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> > + mv_soft_reset(phydev);
> > + }
>
> This looks wrong. phydev->interface is the _host_ interface, which
> you are clearly setting to XAUI here. Some network drivers depend
> on this being correct (for instance, when used with the Marvell
> 88x3310 PHY which changes its host-side interface dynamically.)
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Overall, thank
Re: [PATCH] net: phy: add Marvell 88X2222 transceiver support
On Mon, Feb 01, 2021 at 10:22:51PM +0300, Ivan Bornyakov wrote:
> +/* PMD Transmit Disable */
> +#define MV_TX_DISABLE 0x0009
> +#define MV_TX_DISABLE_GLOBALBIT(0)
Please use MDIO_PMA_TXDIS and MDIO_PMD_TXDIS_GLOBAL; this is an
IEEE802.3 defined register.
> +/* 10GBASE-R PCS Status 1 */
> +#define MV_10GBR_STAT MDIO_STAT1
Nothing Marvell specific here, please use MDIO_STAT1 directly.
> +/* 1000Base-X/SGMII Control Register */
> +#define MV_1GBX_CTRL0x2000
> +
> +/* 1000BASE-X/SGMII Status Register */
> +#define MV_1GBX_STAT0x2001
> +
> +/* 1000Base-X Auto-Negotiation Advertisement Register */
> +#define MV_1GBX_ADVERTISE 0x2004
Marvell have had a habbit of placing other PHY instances within the
register space. This also looks like Clause 22 layout rather than
Clause 45 layout - please use the Clause 22 definitions for the bits
rather than Clause 45. (so BMCR_ANENABLE, BMCR_ANRESTART for
MV_1GBX_CTRL, etc).
Please define these as:
+#defineMV_1GBX_CTRL(0x2000 + MII_BMCR)
+#defineMV_1GBX_STAT(0x2000 + MII_BMSR)
+#defineMV_1GBX_ADVERTISE (0x2000 + MII_ADVERTISE)
to make it clear what is going on here.
> +static int sfp_module_insert(void *_priv, const struct sfp_eeprom_id *id)
> +{
> + struct phy_device *phydev = _priv;
> + struct device *dev = >mdio.dev;
> + struct mv_data *priv = phydev->priv;
> + phy_interface_t interface;
> +
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
> +
> + sfp_parse_support(phydev->sfp_bus, id, supported);
> + interface = sfp_select_interface(phydev->sfp_bus, supported);
> +
> + dev_info(dev, "%s SFP module inserted", phy_modes(interface));
> +
> + switch (interface) {
> + case PHY_INTERFACE_MODE_10GBASER:
> + phydev->speed = SPEED_1;
> + phydev->interface = PHY_INTERFACE_MODE_10GBASER;
> + linkmode_set_bit(ETHTOOL_LINK_MODE_1baseKR_Full_BIT,
> + phydev->supported);
> +
> + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> + MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> + mv_soft_reset(phydev);
> + break;
> + case PHY_INTERFACE_MODE_1000BASEX:
> + default:
> + phydev->speed = SPEED_1000;
> + phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
> + linkmode_clear_bit(ETHTOOL_LINK_MODE_1baseKR_Full_BIT,
> +phydev->supported);
> +
> + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> + MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> + mv_soft_reset(phydev);
> + }
> +
> + priv->sfp_inserted = true;
> +
> + if (priv->net_up)
> + mv_tx_enable(phydev);
This is racy. priv->net_up is modified via the suspend/resume
callbacks, which are called with phydev->lock held. No other locks
are guaranteed to be held.
However, this function is called with the SFP sm_mutex, and rtnl
held. Consequently, the use of sfp_inserted and net_up in this
function and the suspend/resume callbacks is racy.
Why are you disabling the transmitter anyway? Is this for power
saving?
> +static void mv_update_interface(struct phy_device *phydev)
> +{
> + if ((phydev->speed == SPEED_1000 ||
> + phydev->speed == SPEED_100 ||
> + phydev->speed == SPEED_10) &&
> + phydev->interface != PHY_INTERFACE_MODE_1000BASEX) {
> + phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
> +
> + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> + MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> + mv_soft_reset(phydev);
> + } else if (phydev->speed == SPEED_1 &&
> +phydev->interface != PHY_INTERFACE_MODE_10GBASER) {
> + phydev->interface = PHY_INTERFACE_MODE_10GBASER;
> +
> + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> + MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> + mv_soft_reset(phydev);
> + }
This looks wrong. phydev->interface is the _host_ interface, which
you are clearly setting to XAUI here. Some network drivers depend
on this being correct (for instance, when used with the Marvell
88x3310 PHY which changes its host-side interface dynamically.)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] net: phy: add Marvell 88X2222 transceiver support
On Mon, Feb 01, 2021 at 11:56:01PM +0100, Andrew Lunn wrote:
> > +static int mv_config_init(struct phy_device *phydev)
> > +{
> > + linkmode_zero(phydev->supported);
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported);
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported);
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
> > phydev->supported);
> > + linkmode_set_bit(ETHTOOL_LINK_MODE_1baseKR_Full_BIT,
> > phydev->supported);
> > +
> > + phydev->pause = 0;
> > + phydev->asym_pause = 0;
> > + phydev->duplex = DUPLEX_FULL;
> > + phydev->autoneg = AUTONEG_DISABLE;
> > +
> > + return 0;
> > +}
> > +
> > +static void mv_update_interface(struct phy_device *phydev)
> > +{
> > + if ((phydev->speed == SPEED_1000 ||
> > +phydev->speed == SPEED_100 ||
> > +phydev->speed == SPEED_10) &&
> > + phydev->interface != PHY_INTERFACE_MODE_1000BASEX) {
> > + phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
>
> The speeds 10 and 100 seem odd here. 1000BaseX only supports 1G. It
> would have to be SGMII in order to also support 10Mbps and 100Mbps.
> Plus you are not listing 10 and 100 as a supported value.
>
> > +/* Returns negative on error, 0 if link is down, 1 if link is up */
> > +static int mv_read_status_1g(struct phy_device *phydev)
> > +{
> > + int val, link = 0;
> > +
> > + val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
> > + if (val < 0)
> > + return val;
> > +
> > + if (!(val & MDIO_STAT1_LSTATUS) ||
> > + (phydev->autoneg == AUTONEG_ENABLE && !(val &
> > MDIO_AN_STAT1_COMPLETE)))
> > + return 0;
> > +
> > + link = 1;
> > +
> > + if (phydev->autoneg == AUTONEG_DISABLE) {
> > + phydev->speed = SPEED_1000;
> > + phydev->duplex = DUPLEX_FULL;
> > +
> > + return link;
> > + }
> > +
> > + val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_PHY_STAT);
> > + if (val < 0)
> > + return val;
> > +
> > + if (val & MV_1GBX_PHY_STAT_AN_RESOLVED) {
> > + if (val & MV_1GBX_PHY_STAT_DUPLEX)
> > + phydev->duplex = DUPLEX_FULL;
> > + else
> > + phydev->duplex = DUPLEX_HALF;
> > +
> > + if (val & MV_1GBX_PHY_STAT_SPEED1000)
> > + phydev->speed = SPEED_1000;
> > + else if (val & MV_1GBX_PHY_STAT_SPEED100)
> > + phydev->speed = SPEED_100;
> > + else
> > + phydev->speed = SPEED_10;
>
> Are you sure it is not doing SGMII? Maybe it can do both, 1000BaseX
> and SGMII? You would generally use 1000BaseX to connect to a fibre
> SFP, and SGMII to connect to a copper SFP. So ideally you want to be
> able to swap between these modes as needed.
>
> > +static int mv_read_status(struct phy_device *phydev)
> > +{
> > + int link;
> > +
> > + linkmode_zero(phydev->lp_advertising);
> > + phydev->pause = 0;
> > + phydev->asym_pause = 0;
> > +
> > + switch (phydev->interface) {
> > + case PHY_INTERFACE_MODE_10GBASER:
> > + link = mv_read_status_10g(phydev);
> > + break;
> > + case PHY_INTERFACE_MODE_1000BASEX:
> > + default:
> > + link = mv_read_status_1g(phydev);
> > + break;
> > + }
> > +
> > + if (link < 0)
> > + return link;
> > +
> > + phydev->link = link;
> > +
> > + if (phydev->link)
> > + mv_link_led_on(phydev);
> > + else
> > + mv_link_led_off(phydev);
>
> You have to manually control the LED? That is odd for a PHY. Normally
> you just select a mode and it will do it all in hardware.
>
> Andrew
Thanks for the review, Andrew, your remarks are all valid. I'll
implement 1000Base-X/SGMII swapping. As for the LED, I'll just drop it
for now, as it is not essential. I'll commit LEDs config when core
funtionality will be accepted.
Re: [PATCH] net: phy: add Marvell 88X2222 transceiver support
> +static int mv_config_init(struct phy_device *phydev)
> +{
> + linkmode_zero(phydev->supported);
> + linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported);
> + linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported);
> + linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
> phydev->supported);
> + linkmode_set_bit(ETHTOOL_LINK_MODE_1baseKR_Full_BIT,
> phydev->supported);
> +
> + phydev->pause = 0;
> + phydev->asym_pause = 0;
> + phydev->duplex = DUPLEX_FULL;
> + phydev->autoneg = AUTONEG_DISABLE;
> +
> + return 0;
> +}
> +
> +static void mv_update_interface(struct phy_device *phydev)
> +{
> + if ((phydev->speed == SPEED_1000 ||
> + phydev->speed == SPEED_100 ||
> + phydev->speed == SPEED_10) &&
> + phydev->interface != PHY_INTERFACE_MODE_1000BASEX) {
> + phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
The speeds 10 and 100 seem odd here. 1000BaseX only supports 1G. It
would have to be SGMII in order to also support 10Mbps and 100Mbps.
Plus you are not listing 10 and 100 as a supported value.
> +/* Returns negative on error, 0 if link is down, 1 if link is up */
> +static int mv_read_status_1g(struct phy_device *phydev)
> +{
> + int val, link = 0;
> +
> + val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
> + if (val < 0)
> + return val;
> +
> + if (!(val & MDIO_STAT1_LSTATUS) ||
> + (phydev->autoneg == AUTONEG_ENABLE && !(val &
> MDIO_AN_STAT1_COMPLETE)))
> + return 0;
> +
> + link = 1;
> +
> + if (phydev->autoneg == AUTONEG_DISABLE) {
> + phydev->speed = SPEED_1000;
> + phydev->duplex = DUPLEX_FULL;
> +
> + return link;
> + }
> +
> + val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_PHY_STAT);
> + if (val < 0)
> + return val;
> +
> + if (val & MV_1GBX_PHY_STAT_AN_RESOLVED) {
> + if (val & MV_1GBX_PHY_STAT_DUPLEX)
> + phydev->duplex = DUPLEX_FULL;
> + else
> + phydev->duplex = DUPLEX_HALF;
> +
> + if (val & MV_1GBX_PHY_STAT_SPEED1000)
> + phydev->speed = SPEED_1000;
> + else if (val & MV_1GBX_PHY_STAT_SPEED100)
> + phydev->speed = SPEED_100;
> + else
> + phydev->speed = SPEED_10;
Are you sure it is not doing SGMII? Maybe it can do both, 1000BaseX
and SGMII? You would generally use 1000BaseX to connect to a fibre
SFP, and SGMII to connect to a copper SFP. So ideally you want to be
able to swap between these modes as needed.
> +static int mv_read_status(struct phy_device *phydev)
> +{
> + int link;
> +
> + linkmode_zero(phydev->lp_advertising);
> + phydev->pause = 0;
> + phydev->asym_pause = 0;
> +
> + switch (phydev->interface) {
> + case PHY_INTERFACE_MODE_10GBASER:
> + link = mv_read_status_10g(phydev);
> + break;
> + case PHY_INTERFACE_MODE_1000BASEX:
> + default:
> + link = mv_read_status_1g(phydev);
> + break;
> + }
> +
> + if (link < 0)
> + return link;
> +
> + phydev->link = link;
> +
> + if (phydev->link)
> + mv_link_led_on(phydev);
> + else
> + mv_link_led_off(phydev);
You have to manually control the LED? That is odd for a PHY. Normally
you just select a mode and it will do it all in hardware.
Andrew
[PATCH] net: phy: add Marvell 88X2222 transceiver support
Add basic support for the Marvell 88X multi-speed ethernet
transceiver.
This PHY provides data transmission over fiber-optic as well as Twinax
copper links. The 88X supports 2 ports of 10GBase-R and 1000Base-X
on the line-side interface. The host-side interface supports 4 ports of
10GBase-R, RXAUI, 1000Base-X and 2 ports of XAUI.
This driver, however, supports only XAUI on the host-side and
1000Base-X/10GBase-R on the line-side, for now. Interrupts are not
supported also.
Internal registers access compliant with the Clause 45 specification.
Signed-off-by: Ivan Bornyakov
---
drivers/net/phy/Kconfig | 6 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/marvell-88x.c | 480 ++
include/linux/marvell_phy.h | 1 +
4 files changed, 488 insertions(+)
create mode 100644 drivers/net/phy/marvell-88x.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 698bea312adc..a615b3660b05 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -201,6 +201,12 @@ config MARVELL_10G_PHY
help
Support for the Marvell Alaska MV88X3310 and compatible PHYs.
+config MARVELL_88X_PHY
+ tristate "Marvell 88X PHY"
+ help
+ Support for the Marvell 88X Dual-port Multi-speed Ethernet
+ Transceiver.
+
config MICREL_PHY
tristate "Micrel PHYs"
help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index a13e402074cf..de683e3abe63 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_LSI_ET1011C_PHY) += et1011c.o
obj-$(CONFIG_LXT_PHY) += lxt.o
obj-$(CONFIG_MARVELL_10G_PHY) += marvell10g.o
obj-$(CONFIG_MARVELL_PHY) += marvell.o
+obj-$(CONFIG_MARVELL_88X_PHY) += marvell-88x.o
obj-$(CONFIG_MESON_GXL_PHY)+= meson-gxl.o
obj-$(CONFIG_MICREL_KS8995MA) += spi_ks8995.o
obj-$(CONFIG_MICREL_PHY) += micrel.o
diff --git a/drivers/net/phy/marvell-88x.c
b/drivers/net/phy/marvell-88x.c
new file mode 100644
index ..e2c55db4769f
--- /dev/null
+++ b/drivers/net/phy/marvell-88x.c
@@ -0,0 +1,480 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Marvell 88x dual-port multi-speed ethernet transceiver.
+ *
+ * Supports:
+ * XAUI on the host side.
+ * 1000Base-X or 10GBase-R on the line side.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* Port PCS Configuration */
+#defineMV_PCS_CONFIG 0xF002
+#defineMV_PCS_HOST_XAUI0x73
+#defineMV_PCS_LINE_10GBR (0x71 << 8)
+#defineMV_PCS_LINE_1GBX_AN (0x7B << 8)
+
+/* Port Reset and Power Down */
+#defineMV_PORT_RST 0xF003
+#defineMV_LINE_RST_SW BIT(15)
+#defineMV_HOST_RST_SW BIT(7)
+#defineMV_PORT_RST_SW (MV_LINE_RST_SW | MV_HOST_RST_SW)
+
+/* LED0 Control */
+#defineMV_LED0_CTRL0xF020
+#defineMV_LED0_SOLID_MASK (0xf << 4)
+#defineMV_LED0_SOLID_OFF (0x0 << 4)
+#defineMV_LED0_SOLID_ON(0x7 << 4)
+
+/* PMD Transmit Disable */
+#defineMV_TX_DISABLE 0x0009
+#defineMV_TX_DISABLE_GLOBALBIT(0)
+
+/* 10GBASE-R PCS Status 1 */
+#defineMV_10GBR_STAT MDIO_STAT1
+
+/* 10GBASE-R PCS Real Time Status Register */
+#defineMV_10GBR_STAT_RT0x8002
+
+/* 1000Base-X/SGMII Control Register */
+#defineMV_1GBX_CTRL0x2000
+
+/* 1000BASE-X/SGMII Status Register */
+#defineMV_1GBX_STAT0x2001
+
+/* 1000Base-X Auto-Negotiation Advertisement Register */
+#defineMV_1GBX_ADVERTISE 0x2004
+
+/* 1000Base-X PHY Specific Status Register */
+#defineMV_1GBX_PHY_STAT0xA003
+#defineMV_1GBX_PHY_STAT_LSTATUS_RT BIT(10)
+#defineMV_1GBX_PHY_STAT_AN_RESOLVEDBIT(11)
+#defineMV_1GBX_PHY_STAT_DUPLEX BIT(13)
+#defineMV_1GBX_PHY_STAT_SPEED100 BIT(14)
+#defineMV_1GBX_PHY_STAT_SPEED1000 BIT(15)
+
+struct mv_data {
+ bool sfp_inserted;
+ bool net_up;
+};
+
+/* SFI PMA transmit enable */
+static void mv_tx_enable(struct phy_device *phydev)
+{
+ phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MV_TX_DISABLE,
+ MV_TX_DISABLE_GLOBAL);
+}
+
+/* SFI PMA transmit disable */
+static void mv_tx_disable(struct phy_device *phydev)
+{
+ phy_set_bits_mmd(phydev, MDIO_MMD_PMAPMD, MV_TX_DISABLE,
+MV_TX_DISABLE_GLOBAL);
+}
+
+static void mv_link_led_on(struct phy_device *phydev)
+{
+ phy_modify_mmd(phydev, MDIO_MMD_VEND2, MV_LED0_CTRL, MV_LED0_SOLID_MASK,
+ MV_LED0_SOLID_ON);
+}
+
+static void mv_link_led_off(struct phy_device *phydev)
+{
+ phy_modify_mmd(phydev, MDIO_MMD_VEND2, MV_LED0_CTRL,
