[PATCH net-next v2 0/2] DP83822 Fiber enablement

[PATCH net-next v2 0/2] DP83822 Fiber enablement

[Dan Murphy]

Hello

The DP83822 Ethernet PHY has the ability to connect via a Fiber port.  The
derivative PHYs DP83825 and DP83826 do not have this ability. In fiber mode
the DP83822 disables auto negotiation and has a fixed 100Mbps speed with
support for full or half duplex modes.

A devicetree binding was added to set the signal polarity for the fiber
connection.  This property is only applicable if the FX_EN strap is set in
hardware other wise the signal loss detection is disabled on the PHY.

If the FX_EN is not strapped the device can be configured to run in fiber mode
via the device tree. All be it the PHY will not perfomr signal loss detection.

Dan

Dan Murphy (2):
  dt-bindings: net: dp83822: Add TI dp83822 phy
  net: phy: DP83822: Add ability to advertise Fiber connection

 .../devicetree/bindings/net/ti,dp83822.yaml   |  80 +++++++++
 drivers/net/phy/dp83822.c                     | 161 ++++++++++++++++++
 2 files changed, 241 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ti,dp83822.yaml

-- 
2.27.0

[PATCH net-next v2 1/2] dt-bindings: net: dp83822: Add TI dp83822 phy

[Dan Murphy]

Add a dt binding for the TI dp83822 ethernet phy device.

CC: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 .../devicetree/bindings/net/ti,dp83822.yaml   | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ti,dp83822.yaml

diff --git a/Documentation/devicetree/bindings/net/ti,dp83822.yaml b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
new file mode 100644
index 000000000000..55913534cbc2
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause)
+# Copyright (C) 2020 Texas Instruments Incorporated
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/net/ti,dp83822.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: TI DP83822 ethernet PHY
+
+maintainers:
+  - Dan Murphy <dmurphy@ti.com>
+
+description: |
+  The DP83822 is a low-power, single-port, 10/100 Mbps Ethernet PHY. It
+  provides all of the physical layer functions needed to transmit and receive
+  data over standard, twisted-pair cables or to connect to an external,
+  fiber-optic transceiver. Additionally, the DP83822 provides flexibility to
+  connect to a MAC through a standard MII, RMII, or RGMII interface
+
+  Specifications about the Ethernet PHY can be found at:
+    http://www.ti.com/lit/ds/symlink/dp83822i.pdf
+
+allOf:
+  - $ref: "ethernet-phy.yaml#"
+
+properties:
+  reg:
+    maxItems: 1
+
+  ti,link-loss-low:
+    type: boolean
+    description: |
+       DP83822 PHY in Fiber mode only.
+       Sets the DP83822 to detect a link drop condition when the signal goes
+       high.  If not set then link drop will occur when the signal goes low.
+       This property is only applicable if the fiber mode support is strapped
+       to on.
+
+  ti,fiber-mode:
+    type: boolean
+    description: |
+       DP83822 PHY only.
+       If present the DP83822 PHY is configured to operate in fiber mode
+       Fiber mode support can also be strapped. If the strap pin is not set
+       correctly or not set at all then this boolean can be used to enable it.
+       If the fiber mode is not strapped then signal detection for the PHY
+       is disabled.
+       In fiber mode, auto-negotiation is disabled and the PHY can only work in
+       100base-fx (full and half duplex) modes.
+
+  rx-internal-delay-ps:
+    description: |
+       DP83822 PHY only.
+       Setting this property to a non-zero number sets the RX internal delay
+       for the PHY.  The internal delay for the PHY is fixed to 3.5ns relative
+       to receive data.
+
+  tx-internal-delay-ps:
+    description: |
+       DP83822 PHY only.
+       Setting this property to a non-zero number sets the TX internal delay
+       for the PHY.  The internal delay for the PHY is fixed to 3.5ns relative
+       to transmit data.
+
+required:
+  - reg
+
+examples:
+  - |
+    mdio0 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      ethphy0: ethernet-phy@0 {
+        reg = <0>;
+        rx-internal-delay-ps = <1>;
+        tx-internal-delay-ps = <1>;
+      };
+    };
+
+...
-- 
2.27.0

[PATCH net-next v2 2/2] net: phy: DP83822: Add ability to advertise Fiber connection

[Dan Murphy]

The DP83822 can be configured to use a Fiber connection.  The strap
register is read to determine if the device has been configured to use
a fiber connection.  With the fiber connection the PHY can be configured
to detect whether the fiber connection is active by either a high signal
or a low signal.

Fiber mode is only applicable to the DP83822 so rework the PHY match
table so that non-fiber PHYs can still use the same driver but not call
or use any of the fiber features.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/net/phy/dp83822.c | 161 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 161 insertions(+)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 37643c468e19..b797e3d2a4e5 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -23,16 +23,29 @@
 
 #define DP83822_DEVADDR		0x1f
 
+#define MII_DP83822_CTRL_2	0x0a
+#define MII_DP83822_PHYSTS	0x10
 #define MII_DP83822_PHYSCR	0x11
 #define MII_DP83822_MISR1	0x12
 #define MII_DP83822_MISR2	0x13
 #define MII_DP83822_RCSR	0x17
 #define MII_DP83822_RESET_CTRL	0x1f
 #define MII_DP83822_GENCFG	0x465
+#define MII_DP83822_SOR1	0x467
+
+/* GENCFG */
+#define DP83822_SIG_DET_LOW	BIT(0)
+
+/* Control Register 2 bits */
+#define DP83822_FX_ENABLE	BIT(14)
 
 #define DP83822_HW_RESET	BIT(15)
 #define DP83822_SW_RESET	BIT(14)
 
+/* PHY STS bits */
+#define DP83822_PHYSTS_DUPLEX			BIT(2)
+#define DP83822_PHYSTS_10			BIT(1)
+
 /* PHYSCR Register Fields */
 #define DP83822_PHYSCR_INT_OE		BIT(0) /* Interrupt Output Enable */
 #define DP83822_PHYSCR_INTEN		BIT(1) /* Interrupt Enable */
@@ -83,6 +96,21 @@
 #define DP83822_RX_CLK_SHIFT	BIT(12)
 #define DP83822_TX_CLK_SHIFT	BIT(11)
 
+/* SOR1 bits */
+#define DP83822_FX_EN_STRAP	BIT(11)
+#define DP83822_FX_SD_EN_STRAP	BIT(8)
+
+#define MII_DP83822_FIBER_ADVERTISE	(SUPPORTED_AUI | SUPPORTED_FIBRE | \
+					 SUPPORTED_BNC | SUPPORTED_Pause | \
+					 SUPPORTED_Asym_Pause | \
+					 SUPPORTED_100baseT_Full)
+
+struct dp83822_private {
+	bool fx_signal_det_low;
+	int fx_enabled;
+	u16 fx_sd_enable;
+};
+
 static int dp83822_ack_interrupt(struct phy_device *phydev)
 {
 	int err;
@@ -270,13 +298,40 @@ static int dp8382x_disable_wol(struct phy_device *phydev)
 				  MII_DP83822_WOL_CFG, value);
 }
 
+static int dp83822_read_status(struct phy_device *phydev)
+{
+	int status = phy_read(phydev, MII_DP83822_PHYSTS);
+	int ret;
+
+	ret = genphy_read_status(phydev);
+	if (ret)
+		return ret;
+
+	if (status < 0)
+		return status;
+
+	if (status & DP83822_PHYSTS_DUPLEX)
+		phydev->duplex = DUPLEX_FULL;
+	else
+		phydev->duplex = DUPLEX_HALF;
+
+	if (status & DP83822_PHYSTS_10)
+		phydev->speed = SPEED_10;
+	else
+		phydev->speed = SPEED_100;
+
+	return 0;
+}
+
 static int dp83822_config_init(struct phy_device *phydev)
 {
+	struct dp83822_private *dp83822 = phydev->priv;
 	struct device *dev = &phydev->mdio.dev;
 	int rgmii_delay;
 	s32 rx_int_delay;
 	s32 tx_int_delay;
 	int err = 0;
+	int bmcr;
 
 	if (phy_interface_is_rgmii(phydev)) {
 		rx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
@@ -302,6 +357,48 @@ static int dp83822_config_init(struct phy_device *phydev)
 		}
 	}
 
+	if (dp83822->fx_enabled) {
+		err = phy_modify(phydev, MII_DP83822_CTRL_2,
+				 DP83822_FX_ENABLE, 1);
+		if (err < 0)
+			return err;
+
+		linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
+				 phydev->supported);
+		linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
+				 phydev->advertising);
+
+		/* Auto neg is not supported in fiber mode */
+		bmcr = phy_read(phydev, MII_BMCR);
+		if (bmcr < 0)
+			return bmcr;
+
+		if (bmcr & BMCR_ANENABLE) {
+			err =  phy_modify(phydev, MII_BMCR, BMCR_ANENABLE, 0);
+			if (err < 0)
+				return err;
+		}
+		phydev->autoneg = AUTONEG_DISABLE;
+
+		/* Setup fiber advertisement */
+		err = phy_modify_changed(phydev, MII_ADVERTISE,
+					 ADVERTISE_1000XFULL |
+					 ADVERTISE_1000XPAUSE |
+					 ADVERTISE_1000XPSE_ASYM,
+					 MII_DP83822_FIBER_ADVERTISE);
+
+		if (err < 0)
+			return err;
+
+		if (dp83822->fx_signal_det_low) {
+			err = phy_set_bits_mmd(phydev, DP83822_DEVADDR,
+					       MII_DP83822_GENCFG,
+					       DP83822_SIG_DET_LOW);
+			if (err)
+				return err;
+		}
+	}
+
 	return dp8382x_disable_wol(phydev);
 }
 
@@ -321,6 +418,68 @@ static int dp83822_phy_reset(struct phy_device *phydev)
 	return phydev->drv->config_init(phydev);
 }
 
+#ifdef CONFIG_OF_MDIO
+static int dp83822_of_init(struct phy_device *phydev)
+{
+	struct dp83822_private *dp83822 = phydev->priv;
+	struct device *dev = &phydev->mdio.dev;
+
+	/* Signal detection for the PHY is only enabled if the FX_EN and the
+	 * SD_EN pins are strapped. Signal detection can only enabled if FX_EN
+	 * is strapped otherwise signal detection is disabled for the PHY.
+	 */
+	if (dp83822->fx_enabled && dp83822->fx_sd_enable)
+		dp83822->fx_signal_det_low = device_property_present(dev,
+								     "ti,link-loss-low");
+	if (!dp83822->fx_enabled)
+		dp83822->fx_enabled = device_property_present(dev,
+							      "ti,fiber-mode");
+
+	return 0;
+}
+#else
+static int dp83822_of_init(struct phy_device *phydev)
+{
+	return 0;
+}
+#endif /* CONFIG_OF_MDIO */
+
+static int dp83822_read_straps(struct phy_device *phydev)
+{
+	struct dp83822_private *dp83822 = phydev->priv;
+	int val;
+
+	val = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_SOR1);
+	if (val < 0)
+		return val;
+
+	dp83822->fx_enabled = val & DP83822_FX_EN_STRAP;
+	dp83822->fx_sd_enable = val & DP83822_FX_SD_EN_STRAP;
+
+	return 0;
+}
+
+static int dp83822_probe(struct phy_device *phydev)
+{
+	struct dp83822_private *dp83822;
+	int ret;
+
+	dp83822 = devm_kzalloc(&phydev->mdio.dev, sizeof(*dp83822),
+			       GFP_KERNEL);
+	if (!dp83822)
+		return -ENOMEM;
+
+	phydev->priv = dp83822;
+
+	ret = dp83822_read_straps(phydev);
+	if (ret)
+		return ret;
+
+	dp83822_of_init(phydev);
+
+	return 0;
+}
+
 static int dp83822_suspend(struct phy_device *phydev)
 {
 	int value;
@@ -352,8 +511,10 @@ static int dp83822_resume(struct phy_device *phydev)
 		PHY_ID_MATCH_MODEL(_id),			\
 		.name		= (_name),			\
 		/* PHY_BASIC_FEATURES */			\
+		.probe          = dp83822_probe,		\
 		.soft_reset	= dp83822_phy_reset,		\
 		.config_init	= dp83822_config_init,		\
+		.read_status	= dp83822_read_status,		\
 		.get_wol = dp83822_get_wol,			\
 		.set_wol = dp83822_set_wol,			\
 		.ack_interrupt = dp83822_ack_interrupt,		\
-- 
2.27.0

Re: [PATCH net-next v2 2/2] net: phy: DP83822: Add ability to advertise Fiber connection

[Andrew Lunn]

> +#define MII_DP83822_FIBER_ADVERTISE	(SUPPORTED_AUI | SUPPORTED_FIBRE | \
> +					 SUPPORTED_BNC | SUPPORTED_Pause | \
> +					 SUPPORTED_Asym_Pause | \
> +					 SUPPORTED_100baseT_Full)
> +
> +		/* Setup fiber advertisement */
> +		err = phy_modify_changed(phydev, MII_ADVERTISE,
> +					 ADVERTISE_1000XFULL |
> +					 ADVERTISE_1000XPAUSE |
> +					 ADVERTISE_1000XPSE_ASYM,
> +					 MII_DP83822_FIBER_ADVERTISE);

That looks very odd. SUPPORTED_AUI #define has nothing to do with
MII_ADVERTISE register. It is not a bit you can read/write in that
register.

	Andrew

Re: [PATCH net-next v2 2/2] net: phy: DP83822: Add ability to advertise Fiber connection

[Andrew Lunn]

> @@ -302,6 +357,48 @@ static int dp83822_config_init(struct phy_device *phydev)
>  		}
>  	}
>  
> +	if (dp83822->fx_enabled) {
> +		err = phy_modify(phydev, MII_DP83822_CTRL_2,
> +				 DP83822_FX_ENABLE, 1);
> +		if (err < 0)
> +			return err;
> +
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
> +				 phydev->supported);
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
> +				 phydev->advertising);
> +
> +		/* Auto neg is not supported in fiber mode */
> +		bmcr = phy_read(phydev, MII_BMCR);
> +		if (bmcr < 0)
> +			return bmcr;
> +
> +		if (bmcr & BMCR_ANENABLE) {
> +			err =  phy_modify(phydev, MII_BMCR, BMCR_ANENABLE, 0);
> +			if (err < 0)
> +				return err;
> +		}
> +		phydev->autoneg = AUTONEG_DISABLE;

You should also be removing ETHTOOL_LINK_MODE_Autoneg_BIT from
phydev->supported, to make it clear autoneg is not supported. Assuming
genphy_read_abilities() cannot figure this out for itself.

			Andrew

Re: [PATCH net-next v2 1/2] dt-bindings: net: dp83822: Add TI dp83822 phy

[Rob Herring]

On Fri, 10 Jul 2020 09:37:32 -0500, Dan Murphy wrote:
> Add a dt binding for the TI dp83822 ethernet phy device.
> 
> CC: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  .../devicetree/bindings/net/ti,dp83822.yaml   | 80 +++++++++++++++++++
>  1 file changed, 80 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/ti,dp83822.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH net-next v2 2/2] net: phy: DP83822: Add ability to advertise Fiber connection

[Dan Murphy]

Andrew

On 7/11/20 1:54 PM, Andrew Lunn wrote:
>> @@ -302,6 +357,48 @@ static int dp83822_config_init(struct phy_device *phydev)
>>   		}
>>   	}
>>   
>> +	if (dp83822->fx_enabled) {
>> +		err = phy_modify(phydev, MII_DP83822_CTRL_2,
>> +				 DP83822_FX_ENABLE, 1);
>> +		if (err < 0)
>> +			return err;
>> +
>> +		linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
>> +				 phydev->supported);
>> +		linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
>> +				 phydev->advertising);
>> +
>> +		/* Auto neg is not supported in fiber mode */
>> +		bmcr = phy_read(phydev, MII_BMCR);
>> +		if (bmcr < 0)
>> +			return bmcr;
>> +
>> +		if (bmcr & BMCR_ANENABLE) {
>> +			err =  phy_modify(phydev, MII_BMCR, BMCR_ANENABLE, 0);
>> +			if (err < 0)
>> +				return err;
>> +		}
>> +		phydev->autoneg = AUTONEG_DISABLE;
> You should also be removing ETHTOOL_LINK_MODE_Autoneg_BIT from
> phydev->supported, to make it clear autoneg is not supported. Assuming
> genphy_read_abilities() cannot figure this out for itself.

In our testing we are finding that it cannot determine that for itself 
so I will have to clear the bit.

Dan

Re: [PATCH net-next v2 2/2] net: phy: DP83822: Add ability to advertise Fiber connection

[Dan Murphy]

Andrew

On 7/11/20 1:45 PM, Andrew Lunn wrote:
>> +#define MII_DP83822_FIBER_ADVERTISE	(SUPPORTED_AUI | SUPPORTED_FIBRE | \
>> +					 SUPPORTED_BNC | SUPPORTED_Pause | \
>> +					 SUPPORTED_Asym_Pause | \
>> +					 SUPPORTED_100baseT_Full)
>> +
>> +		/* Setup fiber advertisement */
>> +		err = phy_modify_changed(phydev, MII_ADVERTISE,
>> +					 ADVERTISE_1000XFULL |
>> +					 ADVERTISE_1000XPAUSE |
>> +					 ADVERTISE_1000XPSE_ASYM,
>> +					 MII_DP83822_FIBER_ADVERTISE);
> That looks very odd. SUPPORTED_AUI #define has nothing to do with
> MII_ADVERTISE register. It is not a bit you can read/write in that
> register.

ACK removed the SUPPORTED_AUI.

I also going to update the MII_DP83822_FIBER_ADVERTISE defines from 
SUPPORTED_* to ADVERTISED_*

Dan


> 	Andrew