[PATCH net-next v2 1/2] dt-bindings: net: marvell,aquantia: add properties to override MDI_CFG

[PATCH net-next v2 1/2] dt-bindings: net: marvell,aquantia: add properties to override MDI_CFG

[Daniel Golle]

Usually the MDI pair order reversal configuration is defined by
bootstrap pin MDI_CFG. Some designs, however, require overriding the MDI
pair order and force either normal or reverse order.

Add mutually exclusive properties 'marvell,force-mdi-order-normal' and
'marvell,force-mdi-order-reverse' for that purpose.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: enforce mutually exclusive relationship of the two new properties in
    dt-schema.

 .../bindings/net/marvell,aquantia.yaml           | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
index 9854fab4c4db0..03b0cff239f70 100644
--- a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
+++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
@@ -22,6 +22,12 @@ description: |
 
 allOf:
   - $ref: ethernet-phy.yaml#
+  - if:
+      required:
+        - marvell,force-mdi-order-normal
+    then:
+      properties:
+        marvell,force-mdi-order-reverse: false
 
 select:
   properties:
@@ -48,6 +54,16 @@ properties:
   firmware-name:
     description: specify the name of PHY firmware to load
 
+  marvell,force-mdi-order-normal:
+    type: boolean
+    description:
+      force normal order of MDI pairs, overriding MDI_CFG bootstrap pin.
+
+  marvell,force-mdi-order-reverse:
+    type: boolean
+    description:
+      force reverse order of MDI pairs, overriding MDI_CFG bootstrap pin.
+
   nvmem-cells:
     description: phandle to the firmware nvmem cell
     maxItems: 1
-- 
2.46.0

[PATCH net-next v2 2/2] net: phy: aquantia: allow forcing order of MDI pairs

[Daniel Golle]

Despite supporting Auto MDI-X, it looks like Aquantia only supports
swapping pair (1,2) with pair (3,6) like it used to be for MDI-X on
100MBit/s networks.

When all 4 pairs are in use (for 1000MBit/s or faster) the link does not
come up with pair order is not configured correctly, either using
MDI_CFG pin or using the "PMA Receive Reserved Vendor Provisioning 1"
register.

Normally, the order of MDI pairs being either ABCD or DCBA is configured
by pulling the MDI_CFG pin.

However, some hardware designs require overriding the value configured
by that bootstrap pin. The PHY allows doing that by setting a bit in
"PMA Receive Reserved Vendor Provisioning 1" register which allows
ignoring the state of the MDI_CFG pin and another bit configuring
whether the order of MDI pairs should be normal (ABCD) or reverse
(DCBA). Pair polarity is not affected and remains identical in both
settings.

Introduce two mutually exclusive boolean properties which allow forcing
either normal or reverse order of the MDI pairs from DT.

If none of the two new properties is present, the behavior is unchanged
and MDI pair order configuration is untouched (ie. either the result of
MDI_CFG pin pull-up/pull-down, or pair order override already configured
by the bootloader before Linux is started).

Forcing normal pair order is required on the Adtran SDG-8733A Wi-Fi 7
residential gateway.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: add missing 'static' keyword, improve commit description

 drivers/net/phy/aquantia/aquantia_main.c | 35 ++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index e982e9ce44a59..32fdd203fcf05 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/delay.h>
 #include <linux/bitfield.h>
+#include <linux/of.h>
 #include <linux/phy.h>
 
 #include "aquantia.h"
@@ -71,6 +72,11 @@
 #define MDIO_AN_TX_VEND_INT_MASK2		0xd401
 #define MDIO_AN_TX_VEND_INT_MASK2_LINK		BIT(0)
 
+#define PMAPMD_RSVD_VEND_PROV			0xe400
+#define PMAPMD_RSVD_VEND_PROV_MDI_CONF		GENMASK(1, 0)
+#define PMAPMD_RSVD_VEND_PROV_MDI_REVERSE	BIT(0)
+#define PMAPMD_RSVD_VEND_PROV_MDI_FORCE		BIT(1)
+
 #define MDIO_AN_RX_LP_STAT1			0xe820
 #define MDIO_AN_RX_LP_STAT1_1000BASET_FULL	BIT(15)
 #define MDIO_AN_RX_LP_STAT1_1000BASET_HALF	BIT(14)
@@ -474,6 +480,31 @@ static void aqr107_chip_info(struct phy_device *phydev)
 		   fw_major, fw_minor, build_id, prov_id);
 }
 
+static int aqr107_config_mdi(struct phy_device *phydev)
+{
+	struct device_node *np = phydev->mdio.dev.of_node;
+	bool force_normal, force_reverse;
+
+	force_normal = of_property_read_bool(np, "marvell,force-mdi-order-normal");
+	force_reverse = of_property_read_bool(np, "marvell,force-mdi-order-reverse");
+
+	if (force_normal && force_reverse)
+		return -EINVAL;
+
+	if (force_normal)
+		return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, PMAPMD_RSVD_VEND_PROV,
+				      PMAPMD_RSVD_VEND_PROV_MDI_CONF,
+				      PMAPMD_RSVD_VEND_PROV_MDI_FORCE);
+
+	if (force_reverse)
+		return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, PMAPMD_RSVD_VEND_PROV,
+				      PMAPMD_RSVD_VEND_PROV_MDI_CONF,
+				      PMAPMD_RSVD_VEND_PROV_MDI_REVERSE |
+				      PMAPMD_RSVD_VEND_PROV_MDI_FORCE);
+
+	return 0;
+}
+
 static int aqr107_config_init(struct phy_device *phydev)
 {
 	struct aqr107_priv *priv = phydev->priv;
@@ -503,6 +534,10 @@ static int aqr107_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
+	ret = aqr107_config_mdi(phydev);
+	if (ret)
+		return ret;
+
 	/* Restore LED polarity state after reset */
 	for_each_set_bit(led_active_low, &priv->leds_active_low, AQR_MAX_LEDS) {
 		ret = aqr_phy_led_active_low_set(phydev, index, led_active_low);
-- 
2.46.0

Re: [PATCH net-next v2 2/2] net: phy: aquantia: allow forcing order of MDI pairs

[Andrew Lunn]

On Wed, Aug 28, 2024 at 11:52:09PM +0100, Daniel Golle wrote:
> Despite supporting Auto MDI-X, it looks like Aquantia only supports
> swapping pair (1,2) with pair (3,6) like it used to be for MDI-X on
> 100MBit/s networks.
> 
> When all 4 pairs are in use (for 1000MBit/s or faster) the link does not
> come up with pair order is not configured correctly, either using
> MDI_CFG pin or using the "PMA Receive Reserved Vendor Provisioning 1"
> register.
> 
> Normally, the order of MDI pairs being either ABCD or DCBA is configured
> by pulling the MDI_CFG pin.
> 
> However, some hardware designs require overriding the value configured
> by that bootstrap pin. The PHY allows doing that by setting a bit in
> "PMA Receive Reserved Vendor Provisioning 1" register which allows
> ignoring the state of the MDI_CFG pin and another bit configuring
> whether the order of MDI pairs should be normal (ABCD) or reverse
> (DCBA). Pair polarity is not affected and remains identical in both
> settings.
> 
> Introduce two mutually exclusive boolean properties which allow forcing
> either normal or reverse order of the MDI pairs from DT.
> 
> If none of the two new properties is present, the behavior is unchanged
> and MDI pair order configuration is untouched (ie. either the result of
> MDI_CFG pin pull-up/pull-down, or pair order override already configured
> by the bootloader before Linux is started).
> 
> Forcing normal pair order is required on the Adtran SDG-8733A Wi-Fi 7
> residential gateway.

Is there an in-tree dts file for this? We like to see that options
which are added are actually used.

      Andrew

Re: [PATCH net-next v2 2/2] net: phy: aquantia: allow forcing order of MDI pairs

[Daniel Golle]

On Thu, Aug 29, 2024 at 01:05:00AM +0200, Andrew Lunn wrote:
> On Wed, Aug 28, 2024 at 11:52:09PM +0100, Daniel Golle wrote:
> > Despite supporting Auto MDI-X, it looks like Aquantia only supports
> > swapping pair (1,2) with pair (3,6) like it used to be for MDI-X on
> > 100MBit/s networks.
> > 
> > When all 4 pairs are in use (for 1000MBit/s or faster) the link does not
> > come up with pair order is not configured correctly, either using
> > MDI_CFG pin or using the "PMA Receive Reserved Vendor Provisioning 1"
> > register.
> > 
> > Normally, the order of MDI pairs being either ABCD or DCBA is configured
> > by pulling the MDI_CFG pin.
> > 
> > However, some hardware designs require overriding the value configured
> > by that bootstrap pin. The PHY allows doing that by setting a bit in
> > "PMA Receive Reserved Vendor Provisioning 1" register which allows
> > ignoring the state of the MDI_CFG pin and another bit configuring
> > whether the order of MDI pairs should be normal (ABCD) or reverse
> > (DCBA). Pair polarity is not affected and remains identical in both
> > settings.
> > 
> > Introduce two mutually exclusive boolean properties which allow forcing
> > either normal or reverse order of the MDI pairs from DT.
> > 
> > If none of the two new properties is present, the behavior is unchanged
> > and MDI pair order configuration is untouched (ie. either the result of
> > MDI_CFG pin pull-up/pull-down, or pair order override already configured
> > by the bootloader before Linux is started).
> > 
> > Forcing normal pair order is required on the Adtran SDG-8733A Wi-Fi 7
> > residential gateway.
> 
> Is there an in-tree dts file for this? We like to see that options
> which are added are actually used.

I planning to submit DTS for all the Adtran 8700 series once the
MediaTek MT7988 SoC Ethernet is fully supported. At this point I'm still
waiting for feedback on how to organize the PCS drivers for that SoC,
see https://patchwork.kernel.org/comment/25954425/

Re: [PATCH net-next v2 1/2] dt-bindings: net: marvell,aquantia: add properties to override MDI_CFG

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2164 bytes --]

On Wed, Aug 28, 2024 at 11:51:49PM +0100, Daniel Golle wrote:
> Usually the MDI pair order reversal configuration is defined by
> bootstrap pin MDI_CFG. Some designs, however, require overriding the MDI
> pair order and force either normal or reverse order.
> 
> Add mutually exclusive properties 'marvell,force-mdi-order-normal' and
> 'marvell,force-mdi-order-reverse' for that purpose.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> v2: enforce mutually exclusive relationship of the two new properties in
>     dt-schema.
> 
>  .../bindings/net/marvell,aquantia.yaml           | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> index 9854fab4c4db0..03b0cff239f70 100644
> --- a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> +++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> @@ -22,6 +22,12 @@ description: |
>  
>  allOf:
>    - $ref: ethernet-phy.yaml#
> +  - if:
> +      required:
> +        - marvell,force-mdi-order-normal
> +    then:
> +      properties:
> +        marvell,force-mdi-order-reverse: false

Ordinarily, when there are property related constraints in here, the
allOf is moved after the property definitions, since it is odd to talk
about rules for properties prior to defining them. If you resend, please
move these down. Otherwise,
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

>  
>  select:
>    properties:
> @@ -48,6 +54,16 @@ properties:
>    firmware-name:
>      description: specify the name of PHY firmware to load
>  
> +  marvell,force-mdi-order-normal:
> +    type: boolean
> +    description:
> +      force normal order of MDI pairs, overriding MDI_CFG bootstrap pin.
> +
> +  marvell,force-mdi-order-reverse:
> +    type: boolean
> +    description:
> +      force reverse order of MDI pairs, overriding MDI_CFG bootstrap pin.
> +
>    nvmem-cells:
>      description: phandle to the firmware nvmem cell
>      maxItems: 1
> -- 
> 2.46.0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next v2 1/2] dt-bindings: net: marvell,aquantia: add properties to override MDI_CFG

[Rob Herring]

On Wed, Aug 28, 2024 at 11:51:49PM +0100, Daniel Golle wrote:
> Usually the MDI pair order reversal configuration is defined by
> bootstrap pin MDI_CFG. Some designs, however, require overriding the MDI
> pair order and force either normal or reverse order.
> 
> Add mutually exclusive properties 'marvell,force-mdi-order-normal' and
> 'marvell,force-mdi-order-reverse' for that purpose.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> v2: enforce mutually exclusive relationship of the two new properties in
>     dt-schema.
> 
>  .../bindings/net/marvell,aquantia.yaml           | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> index 9854fab4c4db0..03b0cff239f70 100644
> --- a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> +++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> @@ -22,6 +22,12 @@ description: |
>  
>  allOf:
>    - $ref: ethernet-phy.yaml#
> +  - if:
> +      required:
> +        - marvell,force-mdi-order-normal
> +    then:
> +      properties:
> +        marvell,force-mdi-order-reverse: false
>  
>  select:
>    properties:
> @@ -48,6 +54,16 @@ properties:
>    firmware-name:
>      description: specify the name of PHY firmware to load
>  
> +  marvell,force-mdi-order-normal:
> +    type: boolean
> +    description:
> +      force normal order of MDI pairs, overriding MDI_CFG bootstrap pin.
> +
> +  marvell,force-mdi-order-reverse:
> +    type: boolean
> +    description:
> +      force reverse order of MDI pairs, overriding MDI_CFG bootstrap pin.

Why 2 properties for 1 setting? Just do something like this:

marvell,mdi-cfg-order = <0|1>;

1 means reverse, 0 means normal (or vise-versa). Not present then means 
use MDI_CFG setting. Then the binding naturally avoids nonsensical 
combinations of properties without the schema having to enforce it.

Feel free to tweak the naming/values. I would make 0 and 1 align with 
MDI_CFG states, but I don't know what those are.

Rob