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

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

[Dan Murphy]

Hello

The DP83822 Ethernet PHY has the ability to connect via a Fiber port.  The
DP83825 or DP83826 do not have this ability.  In order to keep the same
driver the DP83822 and the 825/826 phy_driver call backs need to be changed
so that the DP83822 has it's own call back for config_init and adds a probe
call back.

A devicetree binding was added to set the signal polarity for the fiber
connection.  This property is only applicable in fiber mode and is optional
in fiber mode.

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   |  49 ++++++
 drivers/net/phy/dp83822.c                     | 140 +++++++++++++++++-
 2 files changed, 181 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/ti,dp83822.yaml

-- 
2.26.2

[PATCH net-next 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   | 49 +++++++++++++++++++
 1 file changed, 49 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..60afd43ad3b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
@@ -0,0 +1,49 @@
+# 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
+
+allOf:
+  - $ref: "ethernet-controller.yaml#"
+
+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 charger can be found at:
+    http://www.ti.com/lit/ds/symlink/dp83822i.pdf
+
+properties:
+  reg:
+    maxItems: 1
+
+  ti,signal-polarity-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.
+
+required:
+  - reg
+
+examples:
+  - |
+    mdio0 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      ethphy0: ethernet-phy@0 {
+        reg = <0>;
+        ti,signal-polarity-low;
+      };
+    };
-- 
2.26.2

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

[Andrew Lunn]

On Thu, May 14, 2020 at 12:30:54PM -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   | 49 +++++++++++++++++++
>  1 file changed, 49 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..60afd43ad3b6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> @@ -0,0 +1,49 @@
> +# 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
> +
> +allOf:
> +  - $ref: "ethernet-controller.yaml#"
> +
> +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

Hi Dan

You say 10/100 Mbps Ethernet PHY, but then list RGMII?

> +
> +  Specifications about the charger can be found at:
> +    http://www.ti.com/lit/ds/symlink/dp83822i.pdf
> +
> +properties:
> +  reg:
> +    maxItems: 1
> +
> +  ti,signal-polarity-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.

Are we talking about the LOS line from the SFP cage? In the SFF/SFP
binding we have:

- los-gpios : GPIO phandle and a specifier of the Receiver Loss of Signal
  Indication input gpio signal, active (signal lost) high

It would be nice to have a consistent naming.

Is it required the LOS signal is connected to the PHY? Russell King
has some patches which allows the Marvell PHY to be used as a media
converter. In that setting, i think the SFP signals are connected to
GPIOs not the PHY. The SFP core can then control the transmit disable,
module insertion detection etc. So i'm wondering if you need a
property to indicate the LOS is not connected to the PHY?

	 Andrew

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

[Dan Murphy]

Andrew

On 5/14/20 1:39 PM, Andrew Lunn wrote:
> On Thu, May 14, 2020 at 12:30:54PM -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   | 49 +++++++++++++++++++
>>   1 file changed, 49 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..60afd43ad3b6
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
>> @@ -0,0 +1,49 @@
>> +# 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
>> +
>> +allOf:
>> +  - $ref: "ethernet-controller.yaml#"
>> +
>> +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
> Hi Dan
>
> You say 10/100 Mbps Ethernet PHY, but then list RGMII?
Copied from the data sheet.
>
>> +
>> +  Specifications about the charger can be found at:
>> +    http://www.ti.com/lit/ds/symlink/dp83822i.pdf
>> +
>> +properties:
>> +  reg:
>> +    maxItems: 1
>> +
>> +  ti,signal-polarity-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.
> Are we talking about the LOS line from the SFP cage? In the SFF/SFP
> binding we have:
>
> - los-gpios : GPIO phandle and a specifier of the Receiver Loss of Signal
>    Indication input gpio signal, active (signal lost) high
>
> It would be nice to have a consistent naming.

> Is it required the LOS signal is connected to the PHY? Russell King
> has some patches which allows the Marvell PHY to be used as a media
> converter. In that setting, i think the SFP signals are connected to
> GPIOs not the PHY. The SFP core can then control the transmit disable,
> module insertion detection etc. So i'm wondering if you need a
> property to indicate the LOS is not connected to the PHY?

The LED_1 pin can be strapped to be an input to the chip for signal loss 
detection.  This is an optional feature of the PHY.

This property defines the polarity for the 822 LED_1/GPIO input pin.

The LOS is not required to be connected to the PHY.  If the preferred 
method is to use the SFP framework and Processor GPIOs then I can remove 
this from the patch set.

And if a user would like to use the feature then they can add it.

Dan


>
> 	 Andrew

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

[Andrew Lunn]

> > Hi Dan
> > 
> > You say 10/100 Mbps Ethernet PHY, but then list RGMII?
> Copied from the data sheet.

O.K. So maybe it can connect over RGMII, but then only run 100Mbps
over it, rather than 1G.

> The LED_1 pin can be strapped to be an input to the chip for signal loss
> detection.  This is an optional feature of the PHY.
> 
> This property defines the polarity for the 822 LED_1/GPIO input pin.
> 
> The LOS is not required to be connected to the PHY.  If the preferred method
> is to use the SFP framework and Processor GPIOs then I can remove this from
> the patch set.
> 
> And if a user would like to use the feature then they can add it.

Well, both options are supported by the hardware. So i'm wondering if
we need to support both. So one property indicating the LOS is
actually connected to the PHY and a second indicating the polarity?

	 Andrew

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

[Dan Murphy]

Andrew

On 5/14/20 3:50 PM, Andrew Lunn wrote:
>>> Hi Dan
>>>
>>> You say 10/100 Mbps Ethernet PHY, but then list RGMII?
>> Copied from the data sheet.
> O.K. So maybe it can connect over RGMII, but then only run 100Mbps
> over it, rather than 1G.
Yes.  This is not a 1Gbps PHY.  Max is 100Mbps.
>
>> The LED_1 pin can be strapped to be an input to the chip for signal loss
>> detection.  This is an optional feature of the PHY.
>>
>> This property defines the polarity for the 822 LED_1/GPIO input pin.
>>
>> The LOS is not required to be connected to the PHY.  If the preferred method
>> is to use the SFP framework and Processor GPIOs then I can remove this from
>> the patch set.
>>
>> And if a user would like to use the feature then they can add it.
> Well, both options are supported by the hardware. So i'm wondering if
> we need to support both. So one property indicating the LOS is
> actually connected to the PHY and a second indicating the polarity?

Why would we need 2?  The SFP core would need to know that the LOS is 
connected to the PHY.

The PHY is strapped to configure the LED_1 as a GPIO input.  I am not 
seeing a register that we can force this configuration.

Data sheet says

Note: To enable 100Base-FX Signal Detection on LED_1 (pin #24), strap 
SD_EN = '1'

So we can read the straps to see if the PHY is connected as the LOS 
input and set the polarity.  But if we are in fiber mode and that pin is 
not strapped for LOS then this setting takes no affect on the PHY.  So 
even reading the straps just allows us to bypass the polarity write.

Dan

>
> 	 Andrew

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

[Andrew Lunn]

> Why would we need 2?  The SFP core would need to know that the LOS is
> connected to the PHY.

That is possible today. Just don't list the GPIO when declaring the
SFP.

> The PHY is strapped to configure the LED_1 as a GPIO input.  I am
> not seeing a register that we can force this configuration.

Ah, O.K. If it is selected via strapping only, they an additional
property is not needed.

	 Andrew

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

[Rob Herring]

On Thu, May 14, 2020 at 12:30:54PM -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   | 49 +++++++++++++++++++
>  1 file changed, 49 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..60afd43ad3b6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> @@ -0,0 +1,49 @@
> +# 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
> +
> +allOf:
> +  - $ref: "ethernet-controller.yaml#"

Not an ethernet controller. Drop. (The ethernet-phy.yaml schema will be 
applied based on node name).

> +
> +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 charger can be found at:
> +    http://www.ti.com/lit/ds/symlink/dp83822i.pdf
> +
> +properties:
> +  reg:
> +    maxItems: 1
> +
> +  ti,signal-polarity-low:

What signal? 

> +    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.

The naming is not clear that low is for link drop. So maybe:

ti,link-loss-low

Rob

[PATCH net-next 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 | 140 +++++++++++++++++++++++++++++++++++---
 1 file changed, 132 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 1dd19d0cb269..fe7443bc8b06 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -27,6 +27,11 @@
 #define MII_DP83822_MISR1	0x12
 #define MII_DP83822_MISR2	0x13
 #define MII_DP83822_RESET_CTRL	0x1f
+#define MII_DP83822_GENCFG	0x465
+#define MII_DP83822_SOR1	0x467
+
+/* GENCFG */
+#define DP83822_SIG_DET_POLARITY BIT(0)
 
 #define DP83822_HW_RESET	BIT(15)
 #define DP83822_SW_RESET	BIT(14)
@@ -77,6 +82,21 @@
 #define DP83822_WOL_INDICATION_SEL BIT(8)
 #define DP83822_WOL_CLR_INDICATION BIT(11)
 
+/* SOR1 bits */
+#define DP83822_FX_EN_STRAP	BIT(11)
+#define DP83822_FX_DUPLEX_STRAP	BIT(0)
+
+#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_detect_low;
+	int fx_enabled;
+	u16 fx_duplex_mode;
+};
+
 static int dp83822_ack_interrupt(struct phy_device *phydev)
 {
 	int err;
@@ -255,7 +275,7 @@ static int dp83822_config_intr(struct phy_device *phydev)
 	return phy_write(phydev, MII_DP83822_PHYSCR, physcr_status);
 }
 
-static int dp83822_config_init(struct phy_device *phydev)
+static int dp8382x_disable_wol(struct phy_device *phydev)
 {
 	int value = DP83822_WOL_EN | DP83822_WOL_MAGIC_EN |
 		    DP83822_WOL_SECURE_ON;
@@ -264,6 +284,41 @@ static int dp83822_config_init(struct phy_device *phydev)
 				  MII_DP83822_WOL_CFG, value);
 }
 
+static int dp83822_config_init(struct phy_device *phydev)
+{
+	struct dp83822_private *dp83822 = phydev->priv;
+	int err = 0;
+
+	if (dp83822->fx_enabled) {
+		linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
+				 phydev->supported);
+		linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
+				 phydev->advertising);
+
+		/*  Auto negotiation is not available in fiber mode */
+		phydev->autoneg = AUTONEG_DISABLE;
+		phydev->speed = SPEED_100;
+		phydev->duplex = DUPLEX_FULL;
+
+		/* 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;
+	}
+
+	return dp8382x_disable_wol(phydev);
+}
+
+static int dp8382x_config_init(struct phy_device *phydev)
+{
+	return dp8382x_disable_wol(phydev);
+}
+
 static int dp83822_phy_reset(struct phy_device *phydev)
 {
 	int err;
@@ -272,7 +327,60 @@ static int dp83822_phy_reset(struct phy_device *phydev)
 	if (err < 0)
 		return err;
 
-	dp83822_config_init(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;
+
+	if (dp83822->fx_enabled)
+		dp83822->fx_signal_detect_low = device_property_present(dev,
+									"ti,signal-polarity-low");
+
+	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;
+	u16 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_duplex_mode = val & DP83822_FX_DUPLEX_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;
 }
@@ -308,6 +416,7 @@ 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,		\
 		.get_wol = dp83822_get_wol,			\
@@ -318,14 +427,29 @@ static int dp83822_resume(struct phy_device *phydev)
 		.resume = dp83822_resume,			\
 	}
 
+#define DP8382X_PHY_DRIVER(_id, _name)				\
+	{							\
+		PHY_ID_MATCH_MODEL(_id),			\
+		.name		= (_name),			\
+		/* PHY_BASIC_FEATURES */			\
+		.soft_reset	= dp83822_phy_reset,		\
+		.config_init	= dp8382x_config_init,		\
+		.get_wol = dp83822_get_wol,			\
+		.set_wol = dp83822_set_wol,			\
+		.ack_interrupt = dp83822_ack_interrupt,		\
+		.config_intr = dp83822_config_intr,		\
+		.suspend = dp83822_suspend,			\
+		.resume = dp83822_resume,			\
+	}
+
 static struct phy_driver dp83822_driver[] = {
 	DP83822_PHY_DRIVER(DP83822_PHY_ID, "TI DP83822"),
-	DP83822_PHY_DRIVER(DP83825I_PHY_ID, "TI DP83825I"),
-	DP83822_PHY_DRIVER(DP83826C_PHY_ID, "TI DP83826C"),
-	DP83822_PHY_DRIVER(DP83826NC_PHY_ID, "TI DP83826NC"),
-	DP83822_PHY_DRIVER(DP83825S_PHY_ID, "TI DP83825S"),
-	DP83822_PHY_DRIVER(DP83825CM_PHY_ID, "TI DP83825M"),
-	DP83822_PHY_DRIVER(DP83825CS_PHY_ID, "TI DP83825CS"),
+	DP8382X_PHY_DRIVER(DP83825I_PHY_ID, "TI DP83825I"),
+	DP8382X_PHY_DRIVER(DP83826C_PHY_ID, "TI DP83826C"),
+	DP8382X_PHY_DRIVER(DP83826NC_PHY_ID, "TI DP83826NC"),
+	DP8382X_PHY_DRIVER(DP83825S_PHY_ID, "TI DP83825S"),
+	DP8382X_PHY_DRIVER(DP83825CM_PHY_ID, "TI DP83825M"),
+	DP8382X_PHY_DRIVER(DP83825CS_PHY_ID, "TI DP83825CS"),
 };
 module_phy_driver(dp83822_driver);
 
-- 
2.26.2

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

[Andrew Lunn]

> +static int dp83822_config_init(struct phy_device *phydev)
> +{
> +	struct dp83822_private *dp83822 = phydev->priv;
> +	int err = 0;
> +
> +	if (dp83822->fx_enabled) {
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
> +				 phydev->supported);
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
> +				 phydev->advertising);
> +
> +		/*  Auto negotiation is not available in fiber mode */
> +		phydev->autoneg = AUTONEG_DISABLE;
> +		phydev->speed = SPEED_100;
> +		phydev->duplex = DUPLEX_FULL;

Hi Dan

This is normally determined by reading the ability registers,
genphy_read_abilities(). When strapped to fibre mode, does it still
indicate all the usual copper capabilities, which it can not actually
do?

	Andrew

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

[Dan Murphy]

Andrew

On 5/14/20 1:52 PM, Andrew Lunn wrote:
>> +static int dp83822_config_init(struct phy_device *phydev)
>> +{
>> +	struct dp83822_private *dp83822 = phydev->priv;
>> +	int err = 0;
>> +
>> +	if (dp83822->fx_enabled) {
>> +		linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
>> +				 phydev->supported);
>> +		linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
>> +				 phydev->advertising);
>> +
>> +		/*  Auto negotiation is not available in fiber mode */
>> +		phydev->autoneg = AUTONEG_DISABLE;
>> +		phydev->speed = SPEED_100;
>> +		phydev->duplex = DUPLEX_FULL;
> Hi Dan
>
> This is normally determined by reading the ability registers,
> genphy_read_abilities(). When strapped to fibre mode, does it still
> indicate all the usual copper capabilities, which it can not actually
> do?

Auto negotiation is not available when in Fiber mode for this PHY.  The 
Speed is locked at 100Mbps for fiber.

Duplex can be either FULL or HALF so that should be removed.

I am verifying with the PHY team on the BMSR register but I do not see 
any bits for FX there.

If we remove these settings then I will need to read the PHY_STS 
register to manage the speed and mode of the fiber.  This register 
reports the PHY link status regardless of the mode.

Dan


>
> 	Andrew