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

[PATCH net-next 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 properties 'marvell,force-mdi-order-normal' and
'marvell,force-mdi-order-reverse' for that purpose.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 .../devicetree/bindings/net/marvell,aquantia.yaml      | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
index 9854fab4c4db0..c82d0be48741d 100644
--- a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
+++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
@@ -48,6 +48,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 2/2] net: phy: aquantia: allow forcing order of MDI pairs

[Daniel Golle]

Normally, the MDI reversal configuration is taken from 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 which
allows ignoring the state of the MDI_CFG pin and configuring whether
the order of MDI pairs should be normal (ABCD) or reverse (DCBA).

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

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 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..33a6eb55d99d4 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);
 }
 
+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 1/2] dt-bindings: net: marvell,aquantia: add properties to override MDI_CFG

[Conor Dooley]

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

On Wed, Aug 21, 2024 at 01:46:30PM +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.

Is that a PC way of saying that someone messed up and wired the pins
incorrectly? A concrete example of why this is required would be good ;)

> 
> Add properties 'marvell,force-mdi-order-normal' and
> 'marvell,force-mdi-order-reverse' for that purpose.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  .../devicetree/bindings/net/marvell,aquantia.yaml      | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> index 9854fab4c4db0..c82d0be48741d 100644
> --- a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> +++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> @@ -48,6 +48,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.

These properties are mutually exclusive, right? If so, the binding
should enforce that.

Thanks,
Conor.

> +
>    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 1/2] dt-bindings: net: marvell,aquantia: add properties to override MDI_CFG

[Andrew Lunn]

On Wed, Aug 21, 2024 at 01:46:30PM +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.

Could you explain that in a bit more detail. Are you talking about
changing the order of the 4 pairs? Or reversing the polarity within
pairs?

	Andrew

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

[Andrew Lunn]

On Wed, Aug 21, 2024 at 01:46:50PM +0100, Daniel Golle wrote:
> Normally, the MDI reversal configuration is taken from 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 which
> allows ignoring the state of the MDI_CFG pin and configuring whether
> the order of MDI pairs should be normal (ABCD) or reverse (DCBA).
> 
> Introduce two boolean properties which allow forcing either normal or
> reverse order of the MDI pairs from DT.

How does this interact with ethtool -s eth42 [mdix auto|on|off]

In general, you want mdix auto, so the two ends figure out how the
cable is wired and so it just works.

      Andrew

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

[Daniel Golle]

On Wed, Aug 21, 2024 at 06:00:36PM +0200, Andrew Lunn wrote:
> On Wed, Aug 21, 2024 at 01:46:30PM +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.
> 
> Could you explain that in a bit more detail. Are you talking about
> changing the order of the 4 pairs? Or reversing the polarity within
> pairs?

It's about changing the order of the 4 pairs, either ABCD or DCBA.
Polarity of each pair is not affected by this.

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

[Andrew Lunn]

On Wed, Aug 21, 2024 at 05:08:39PM +0100, Daniel Golle wrote:
> On Wed, Aug 21, 2024 at 06:00:36PM +0200, Andrew Lunn wrote:
> > On Wed, Aug 21, 2024 at 01:46:30PM +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.
> > 
> > Could you explain that in a bit more detail. Are you talking about
> > changing the order of the 4 pairs? Or reversing the polarity within
> > pairs?
> 
> It's about changing the order of the 4 pairs, either ABCD or DCBA.
> Polarity of each pair is not affected by this.

So when i do

ethtool -s 42 mdix off

to enable straight through, are you saying the PHY actually does
crossover, because the pull-up tells it the wrong thing about how the
cable is wired? And `mdix on` for crossover gives straight?

'mdix auto' just works as expected, since it tried all the
combinations until it works?

      Andrew

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

[Daniel Golle]

On Wed, Aug 21, 2024 at 06:07:06PM +0200, Andrew Lunn wrote:
> On Wed, Aug 21, 2024 at 01:46:50PM +0100, Daniel Golle wrote:
> > Normally, the MDI reversal configuration is taken from 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 which
> > allows ignoring the state of the MDI_CFG pin and configuring whether
> > the order of MDI pairs should be normal (ABCD) or reverse (DCBA).
> > 
> > Introduce two boolean properties which allow forcing either normal or
> > reverse order of the MDI pairs from DT.
> 
> How does this interact with ethtool -s eth42 [mdix auto|on|off]
> 
> In general, you want mdix auto, so the two ends figure out how the
> cable is wired and so it just works.

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.

And yes, I did verify that Auto MDI-X is enabled in the
"Autonegotiation Reserved Vendor Provisioning 1" register.

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

[kernel test robot]

Hi Daniel,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Golle/net-phy-aquantia-allow-forcing-order-of-MDI-pairs/20240821-210717
base:   net-next/main
patch link:    https://lore.kernel.org/r/ed46220cc4c52d630fc481c8148fc749242c368d.1724244281.git.daniel%40makrotopia.org
patch subject: [PATCH net-next 2/2] net: phy: aquantia: allow forcing order of MDI pairs
config: x86_64-randconfig-123-20240822 (https://download.01.org/0day-ci/archive/20240822/202408221406.WtGcNGxX-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240822/202408221406.WtGcNGxX-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408221406.WtGcNGxX-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/phy/aquantia/aquantia_main.c:483:5: sparse: sparse: symbol 'aqr107_config_mdi' was not declared. Should it be static?
   drivers/net/phy/aquantia/aquantia_main.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/umh.h, include/linux/kmod.h, ...):
   include/linux/page-flags.h:235:46: sparse: sparse: self-comparison always evaluates to false
   include/linux/page-flags.h:235:46: sparse: sparse: self-comparison always evaluates to false

vim +/aqr107_config_mdi +483 drivers/net/phy/aquantia/aquantia_main.c

   482	
 > 483	int aqr107_config_mdi(struct phy_device *phydev)
   484	{
   485		struct device_node *np = phydev->mdio.dev.of_node;
   486		bool force_normal, force_reverse;
   487	
   488		force_normal = of_property_read_bool(np, "marvell,force-mdi-order-normal");
   489		force_reverse = of_property_read_bool(np, "marvell,force-mdi-order-reverse");
   490	
   491		if (force_normal && force_reverse)
   492			return -EINVAL;
   493	
   494		if (force_normal)
   495			return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, PMAPMD_RSVD_VEND_PROV,
   496					      PMAPMD_RSVD_VEND_PROV_MDI_CONF,
   497					      PMAPMD_RSVD_VEND_PROV_MDI_FORCE);
   498	
   499		if (force_reverse)
   500			return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, PMAPMD_RSVD_VEND_PROV,
   501					      PMAPMD_RSVD_VEND_PROV_MDI_CONF,
   502					      PMAPMD_RSVD_VEND_PROV_MDI_REVERSE |
   503					      PMAPMD_RSVD_VEND_PROV_MDI_FORCE);
   504	
   505		return 0;
   506	}
   507	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

Hi Daniel,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Golle/net-phy-aquantia-allow-forcing-order-of-MDI-pairs/20240821-210717
base:   net-next/main
patch link:    https://lore.kernel.org/r/ed46220cc4c52d630fc481c8148fc749242c368d.1724244281.git.daniel%40makrotopia.org
patch subject: [PATCH net-next 2/2] net: phy: aquantia: allow forcing order of MDI pairs
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240822/202408221537.gmrL3l3n-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240822/202408221537.gmrL3l3n-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408221537.gmrL3l3n-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/phy/aquantia/aquantia_main.c:483:5: warning: no previous prototype for 'aqr107_config_mdi' [-Wmissing-prototypes]
     483 | int aqr107_config_mdi(struct phy_device *phydev)
         |     ^~~~~~~~~~~~~~~~~


vim +/aqr107_config_mdi +483 drivers/net/phy/aquantia/aquantia_main.c

   482	
 > 483	int aqr107_config_mdi(struct phy_device *phydev)
   484	{
   485		struct device_node *np = phydev->mdio.dev.of_node;
   486		bool force_normal, force_reverse;
   487	
   488		force_normal = of_property_read_bool(np, "marvell,force-mdi-order-normal");
   489		force_reverse = of_property_read_bool(np, "marvell,force-mdi-order-reverse");
   490	
   491		if (force_normal && force_reverse)
   492			return -EINVAL;
   493	
   494		if (force_normal)
   495			return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, PMAPMD_RSVD_VEND_PROV,
   496					      PMAPMD_RSVD_VEND_PROV_MDI_CONF,
   497					      PMAPMD_RSVD_VEND_PROV_MDI_FORCE);
   498	
   499		if (force_reverse)
   500			return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, PMAPMD_RSVD_VEND_PROV,
   501					      PMAPMD_RSVD_VEND_PROV_MDI_CONF,
   502					      PMAPMD_RSVD_VEND_PROV_MDI_REVERSE |
   503					      PMAPMD_RSVD_VEND_PROV_MDI_FORCE);
   504	
   505		return 0;
   506	}
   507	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

Hi Daniel,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Golle/net-phy-aquantia-allow-forcing-order-of-MDI-pairs/20240821-210717
base:   net-next/main
patch link:    https://lore.kernel.org/r/ed46220cc4c52d630fc481c8148fc749242c368d.1724244281.git.daniel%40makrotopia.org
patch subject: [PATCH net-next 2/2] net: phy: aquantia: allow forcing order of MDI pairs
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240822/202408221523.6Cc0ADf9-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 26670e7fa4f032a019d23d56c6a02926e854e8af)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240822/202408221523.6Cc0ADf9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408221523.6Cc0ADf9-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/net/phy/aquantia/aquantia_main.c:15:
   In file included from include/linux/phy.h:16:
   In file included from include/linux/ethtool.h:18:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:10:
   In file included from include/linux/mm.h:2228:
   include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     514 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   In file included from drivers/net/phy/aquantia/aquantia_main.c:15:
   In file included from include/linux/phy.h:16:
   In file included from include/linux/ethtool.h:18:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/net/phy/aquantia/aquantia_main.c:15:
   In file included from include/linux/phy.h:16:
   In file included from include/linux/ethtool.h:18:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/net/phy/aquantia/aquantia_main.c:15:
   In file included from include/linux/phy.h:16:
   In file included from include/linux/ethtool.h:18:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
>> drivers/net/phy/aquantia/aquantia_main.c:483:5: warning: no previous prototype for function 'aqr107_config_mdi' [-Wmissing-prototypes]
     483 | int aqr107_config_mdi(struct phy_device *phydev)
         |     ^
   drivers/net/phy/aquantia/aquantia_main.c:483:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
     483 | int aqr107_config_mdi(struct phy_device *phydev)
         | ^
         | static 
   8 warnings generated.


vim +/aqr107_config_mdi +483 drivers/net/phy/aquantia/aquantia_main.c

   482	
 > 483	int aqr107_config_mdi(struct phy_device *phydev)
   484	{
   485		struct device_node *np = phydev->mdio.dev.of_node;
   486		bool force_normal, force_reverse;
   487	
   488		force_normal = of_property_read_bool(np, "marvell,force-mdi-order-normal");
   489		force_reverse = of_property_read_bool(np, "marvell,force-mdi-order-reverse");
   490	
   491		if (force_normal && force_reverse)
   492			return -EINVAL;
   493	
   494		if (force_normal)
   495			return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, PMAPMD_RSVD_VEND_PROV,
   496					      PMAPMD_RSVD_VEND_PROV_MDI_CONF,
   497					      PMAPMD_RSVD_VEND_PROV_MDI_FORCE);
   498	
   499		if (force_reverse)
   500			return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, PMAPMD_RSVD_VEND_PROV,
   501					      PMAPMD_RSVD_VEND_PROV_MDI_CONF,
   502					      PMAPMD_RSVD_VEND_PROV_MDI_REVERSE |
   503					      PMAPMD_RSVD_VEND_PROV_MDI_FORCE);
   504	
   505		return 0;
   506	}
   507	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Andrew Lunn]

On Wed, Aug 21, 2024 at 05:18:44PM +0100, Daniel Golle wrote:
> On Wed, Aug 21, 2024 at 06:07:06PM +0200, Andrew Lunn wrote:
> > On Wed, Aug 21, 2024 at 01:46:50PM +0100, Daniel Golle wrote:
> > > Normally, the MDI reversal configuration is taken from 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 which
> > > allows ignoring the state of the MDI_CFG pin and configuring whether
> > > the order of MDI pairs should be normal (ABCD) or reverse (DCBA).
> > > 
> > > Introduce two boolean properties which allow forcing either normal or
> > > reverse order of the MDI pairs from DT.
> > 
> > How does this interact with ethtool -s eth42 [mdix auto|on|off]
> > 
> > In general, you want mdix auto, so the two ends figure out how the
> > cable is wired and so it just works.
> 
> 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.
> 
> And yes, I did verify that Auto MDI-X is enabled in the
> "Autonegotiation Reserved Vendor Provisioning 1" register.

Is it possible to read the strap configuration?  All DT needs to
indicate is that the strap is inverted.

	Andrew

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

[Daniel Golle]

On Fri, Aug 23, 2024 at 03:28:39AM +0200, Andrew Lunn wrote:
> On Wed, Aug 21, 2024 at 05:18:44PM +0100, Daniel Golle wrote:
> > On Wed, Aug 21, 2024 at 06:07:06PM +0200, Andrew Lunn wrote:
> > > On Wed, Aug 21, 2024 at 01:46:50PM +0100, Daniel Golle wrote:
> > > > Normally, the MDI reversal configuration is taken from 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 which
> > > > allows ignoring the state of the MDI_CFG pin and configuring whether
> > > > the order of MDI pairs should be normal (ABCD) or reverse (DCBA).
> > > > 
> > > > Introduce two boolean properties which allow forcing either normal or
> > > > reverse order of the MDI pairs from DT.
> > > 
> > > How does this interact with ethtool -s eth42 [mdix auto|on|off]
> > > 
> > > In general, you want mdix auto, so the two ends figure out how the
> > > cable is wired and so it just works.
> > 
> > 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.
> > 
> > And yes, I did verify that Auto MDI-X is enabled in the
> > "Autonegotiation Reserved Vendor Provisioning 1" register.
> 
> Is it possible to read the strap configuration?  All DT needs to
> indicate is that the strap is inverted.

Sadly no, because there is only a single r/w bit which could already
have been changed by the bootloader as well. We risk that vendor
bootloader updates then require modifying DT again once they "fix it"
there. I'd rather have two properties to force either ABCD or DCBA pair
order.

If it got to be a single property, then we can either use a string with
pre-defined values "abcd" and "dcba", or use macro defined integer
values in include/dt-bindings such as

#define AQUANTIA_MII_PAIR_ORDER_ABCD 0
#define AQUANTIA_MII_PAIR_ORDER_DCBA 1

In both when using a single property for overriding the bootstrap value,
absence of that property would mean to not touch what ever has been
setup by bootstrap pin (or force-mode configured by the bootloader,
which I've also seen already, and that also seems to survive PHY
resets).