netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/3] net: phy: bcm54811: Fix the PHY initialization
@ 2025-06-23 15:10 Kamil Horák - 2N
  2025-06-23 15:10 ` [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode Kamil Horák - 2N
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Kamil Horák - 2N @ 2025-06-23 15:10 UTC (permalink / raw)
  To: florian.fainelli, bcm-kernel-feedback-list, andrew, hkallweit1,
	linux, davem, edumazet, kuba, pabeni, krzk+dt, conor+dt
  Cc: kamilh, netdev, devicetree, linux-kernel, robh

From: "Kamil Horák (2N)" <kamilh@axis.com>

PATCH 1 - Fix the BCM54811 PHY initialization so that it conforms
   to the datasheet regarding a reserved bit in the LRE Control
   register, which must be written to zero after every device reset.

PATCH 2 - Fix the BCM54811 PHY initialization by implementing MII-Lite
   mode switch

PATCH 3 - Add optional mii-lite-mode flag to switch the PHY to MII-Lite

Kamil Horák (2N) (3):
  net: phy: bcm54811: Fix the PHY initialization
  net: phy: bcm5481x: Implement MII-Lite mode
  dt-bindings: ethernet-phy: add optional mii-lite-mode flag

 .../devicetree/bindings/net/ethernet-phy.yaml |  8 +++
 drivers/net/phy/broadcom.c                    | 54 +++++++++++++++++--
 include/linux/brcmphy.h                       |  7 +++
 3 files changed, 66 insertions(+), 3 deletions(-)

-- 
2.39.5


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode
  2025-06-23 15:10 [PATCH net-next v2 0/3] net: phy: bcm54811: Fix the PHY initialization Kamil Horák - 2N
@ 2025-06-23 15:10 ` Kamil Horák - 2N
  2025-06-23 15:51   ` Maxime Chevallier
                     ` (2 more replies)
  2025-06-23 15:10 ` [PATCH net-next v2 1/3] net: phy: bcm54811: Fix the PHY initialization Kamil Horák - 2N
  2025-06-23 15:10 ` [PATCH net-next v2 3/3] dt-bindings: ethernet-phy: add optional mii-lite-mode flag Kamil Horák - 2N
  2 siblings, 3 replies; 13+ messages in thread
From: Kamil Horák - 2N @ 2025-06-23 15:10 UTC (permalink / raw)
  To: florian.fainelli, bcm-kernel-feedback-list, andrew, hkallweit1,
	linux, davem, edumazet, kuba, pabeni, krzk+dt, conor+dt
  Cc: kamilh, netdev, devicetree, linux-kernel, robh

From: Kamil Horák (2N) <kamilh@axis.com>

The Broadcom bcm54810 and bcm54811 PHYs are capable to operate in
simplified MII mode, without TXER, RXER, CRS and COL signals as defined
for the MII. While the PHY can be strapped for MII mode, the selection
between MII and MII-Lite must be done by software.
The MII-Lite mode can be used with some Ethernet controllers, usually
those used in automotive applications. The absence of COL signal
makes half-duplex link modes impossible but does not interfere with
BroadR-Reach link modes on Broadcom PHYs, because they are full-duplex
only. The MII-Lite mode can be also used on an Ethernet controller with
full MII interface by just leaving the input signals (RXER, CRS, COL)
inactive.

Signed-off-by: Kamil Horák (2N) <kamilh@axis.com>
---
 drivers/net/phy/broadcom.c | 32 +++++++++++++++++++++++++++++++-
 include/linux/brcmphy.h    |  6 ++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 75dbb88bec5a..d0ecb12d2d2e 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -16,7 +16,6 @@
 #include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/phy.h>
-#include <linux/device.h>
 #include <linux/brcmphy.h>
 #include <linux/of.h>
 #include <linux/interrupt.h>
@@ -39,6 +38,7 @@ struct bcm54xx_phy_priv {
 	int	wake_irq;
 	bool	wake_irq_enabled;
 	bool	brr_mode;
+	bool	mii_lite_mode;
 };
 
 /* Link modes for BCM58411 PHY */
@@ -680,6 +680,12 @@ static int bcm5481x_read_abilities(struct phy_device *phydev)
 
 	priv->brr_mode = of_property_read_bool(np, "brr-mode");
 
+	/* Enable MII Lite (No TXER, RXER, CRS, COL) if configured */
+	err = bcm_phy_modify_exp(phydev, BCM_EXP_SYNC_ETHERNET,
+				 BCM_EXP_SYNC_ETHERNET_MII_LITE,
+				 priv->mii_lite_mode ?
+				 BCM_EXP_SYNC_ETHERNET_MII_LITE : 0);
+
 	/* Set BroadR-Reach mode as configured in the DT. */
 	err = bcm5481x_set_brrmode(phydev, priv->brr_mode);
 	if (err)
@@ -1140,6 +1146,7 @@ static int bcm54xx_phy_probe(struct phy_device *phydev)
 	struct bcm54xx_phy_priv *priv;
 	struct gpio_desc *wakeup_gpio;
 	int ret = 0;
+	struct device_node *np = phydev->mdio.dev.of_node;
 
 	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -1159,6 +1166,29 @@ static int bcm54xx_phy_probe(struct phy_device *phydev)
 	if (IS_ERR(priv->ptp))
 		return PTR_ERR(priv->ptp);
 
+	priv->mii_lite_mode = of_property_read_bool(np, "mii-lite-mode");
+	if (!phy_interface_is_rgmii(phydev) ||
+	    phydev->interface == PHY_INTERFACE_MODE_MII) {
+		/* Enable MII Lite (No TXER, RXER, CRS, COL) if configured */
+		ret = bcm_phy_modify_exp(phydev, BCM_EXP_SYNC_ETHERNET,
+					 BCM_EXP_SYNC_ETHERNET_MII_LITE,
+					 priv->mii_lite_mode ?
+					 BCM_EXP_SYNC_ETHERNET_MII_LITE : 0);
+		if (ret < 0)
+			return ret;
+		/* Misc Control: GMII/MII Mode (not RGMII) */
+		ret = phy_write(phydev, MII_BCM54XX_AUX_CTL,
+				MII_BCM54XX_AUXCTL_MISC_WREN |
+				MII_BCM54XX_AUXCTL_SHDWSEL_MASK |
+				MII_BCM54XX_AUXCTL_SHDWSEL_MISC |
+				(MII_BCM54XX_AUXCTL_SHDWSEL_MISC
+				  << MII_BCM54XX_AUXCTL_SHDWSEL_READ_SHIFT) |
+				MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_SKEW_EN |
+				MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RSVD);
+		if (ret < 0)
+			return ret;
+	}
+
 	/* We cannot utilize the _optional variant here since we want to know
 	 * whether the GPIO descriptor exists or not to advertise Wake-on-LAN
 	 * support or not.
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index 350846b010e9..115a964f3006 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -183,6 +183,12 @@
 #define BCM_LED_MULTICOLOR_ACT		0x9
 #define BCM_LED_MULTICOLOR_PROGRAM	0xa
 
+/*
+ * Broadcom Synchronous Ethernet Controls (expansion register 0x0E)
+ */
+#define BCM_EXP_SYNC_ETHERNET		(MII_BCM54XX_EXP_SEL_ER + 0x0E)
+#define BCM_EXP_SYNC_ETHERNET_MII_LITE	BIT(11)
+
 /*
  * BCM5482: Shadow registers
  * Shadow values go into bits [14:10] of register 0x1c to select a shadow
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 1/3] net: phy: bcm54811: Fix the PHY initialization
  2025-06-23 15:10 [PATCH net-next v2 0/3] net: phy: bcm54811: Fix the PHY initialization Kamil Horák - 2N
  2025-06-23 15:10 ` [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode Kamil Horák - 2N
@ 2025-06-23 15:10 ` Kamil Horák - 2N
  2025-06-23 15:56   ` Russell King (Oracle)
  2025-06-23 15:10 ` [PATCH net-next v2 3/3] dt-bindings: ethernet-phy: add optional mii-lite-mode flag Kamil Horák - 2N
  2 siblings, 1 reply; 13+ messages in thread
From: Kamil Horák - 2N @ 2025-06-23 15:10 UTC (permalink / raw)
  To: florian.fainelli, bcm-kernel-feedback-list, andrew, hkallweit1,
	linux, davem, edumazet, kuba, pabeni, krzk+dt, conor+dt
  Cc: kamilh, netdev, devicetree, linux-kernel, robh

From: Kamil Horák (2N) <kamilh@axis.com>

Reset the bit 12 in PHY's LRE Control register upon initialization.
According to the datasheet, this bit must be written to zero after
every device reset.

Signed-off-by: Kamil Horák (2N) <kamilh@axis.com>
---
 drivers/net/phy/broadcom.c | 22 ++++++++++++++++++++--
 include/linux/brcmphy.h    |  1 +
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 9b1de54fd483..75dbb88bec5a 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -420,11 +420,29 @@ static int bcm54811_config_init(struct phy_device *phydev)
 			return err;
 	}
 
+	err = bcm5481x_set_brrmode(phydev, priv->brr_mode);
+	if (err < 0)
+		return err;
+
 	/* With BCM54811, BroadR-Reach implies no autoneg */
-	if (priv->brr_mode)
+	if (priv->brr_mode) {
 		phydev->autoneg = 0;
+		/* Disable Long Distance Signaling, the BRR mode autoneg */
+		err = phy_modify(phydev, MII_BCM54XX_LRECR, LRECR_LDSEN, 0);
+		if (err < 0)
+			return err;
+	}
 
-	return bcm5481x_set_brrmode(phydev, priv->brr_mode);
+	if (!phy_interface_is_rgmii(phydev) ||
+	    phydev->interface == PHY_INTERFACE_MODE_MII) {
+		/* Misc Control: GMII/MII Mode (not RGMII) */
+		err = bcm54xx_auxctl_write(phydev,
+					   MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
+					   MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_SKEW_EN |
+					   MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RSVD
+		);
+	}
+	return err;
 }
 
 static int bcm54xx_config_init(struct phy_device *phydev)
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index 028b3e00378e..350846b010e9 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -137,6 +137,7 @@
 
 #define MII_BCM54XX_AUXCTL_SHDWSEL_MISC			0x07
 #define MII_BCM54XX_AUXCTL_SHDWSEL_MISC_WIRESPEED_EN	0x0010
+#define MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RSVD		0x0060
 #define MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_EN	0x0080
 #define MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_SKEW_EN	0x0100
 #define MII_BCM54XX_AUXCTL_MISC_FORCE_AMDIX		0x0200
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 3/3] dt-bindings: ethernet-phy: add optional mii-lite-mode flag
  2025-06-23 15:10 [PATCH net-next v2 0/3] net: phy: bcm54811: Fix the PHY initialization Kamil Horák - 2N
  2025-06-23 15:10 ` [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode Kamil Horák - 2N
  2025-06-23 15:10 ` [PATCH net-next v2 1/3] net: phy: bcm54811: Fix the PHY initialization Kamil Horák - 2N
@ 2025-06-23 15:10 ` Kamil Horák - 2N
  2025-06-23 15:59   ` Russell King (Oracle)
  2 siblings, 1 reply; 13+ messages in thread
From: Kamil Horák - 2N @ 2025-06-23 15:10 UTC (permalink / raw)
  To: florian.fainelli, bcm-kernel-feedback-list, andrew, hkallweit1,
	linux, davem, edumazet, kuba, pabeni, krzk+dt, conor+dt
  Cc: kamilh, netdev, devicetree, linux-kernel, robh

From: Kamil Horák (2N) <kamilh@axis.com>

The Broadcom bcm54810 and bcm54811 PHYs support MII and MII-Lite
interface modes. The MII-Lite mode does not use TXR, RXER, CRS and COL
signals. However, the hardware strapping only selects MII mode,
distinction between MII and MII-Lite must be done by software.

Add optional mii-lite-mode flag to switch the PHY to MII-Lite mode.

Signed-off-by: Kamil Horák (2N) <kamilh@axis.com>
---
 Documentation/devicetree/bindings/net/ethernet-phy.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 71e2cd32580f..edfd16044770 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -101,6 +101,14 @@ properties:
       1BR-10 names. The PHY must be configured to operate in BroadR-Reach mode
       by software.
 
+  mii-lite-mode:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      If set, indicates the use of MII-Lite variant of MII, without the
+      functions of TXER, RXER, CRS and COL signals for Broadcom PHYs. These
+      PHYs can be strapped to use MII mode but the MII or MII-Lite selection
+      must be done by software.
+
   clocks:
     maxItems: 1
     description:
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode
  2025-06-23 15:10 ` [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode Kamil Horák - 2N
@ 2025-06-23 15:51   ` Maxime Chevallier
  2025-06-23 17:55     ` Andrew Lunn
  2025-06-24  7:59     ` Kamil Horák (2N)
  2025-06-23 15:58   ` Russell King (Oracle)
  2025-06-23 18:08   ` Andrew Lunn
  2 siblings, 2 replies; 13+ messages in thread
From: Maxime Chevallier @ 2025-06-23 15:51 UTC (permalink / raw)
  To: Kamil Horák - 2N
  Cc: florian.fainelli, bcm-kernel-feedback-list, andrew, hkallweit1,
	linux, davem, edumazet, kuba, pabeni, krzk+dt, conor+dt, netdev,
	devicetree, linux-kernel, robh

Hi Kamil,

On Mon, 23 Jun 2025 17:10:46 +0200
Kamil Horák - 2N <kamilh@axis.com> wrote:

> From: Kamil Horák (2N) <kamilh@axis.com>
> 
> The Broadcom bcm54810 and bcm54811 PHYs are capable to operate in
> simplified MII mode, without TXER, RXER, CRS and COL signals as defined
> for the MII. While the PHY can be strapped for MII mode, the selection
> between MII and MII-Lite must be done by software.
> The MII-Lite mode can be used with some Ethernet controllers, usually
> those used in automotive applications. The absence of COL signal
> makes half-duplex link modes impossible but does not interfere with
> BroadR-Reach link modes on Broadcom PHYs, because they are full-duplex
> only. The MII-Lite mode can be also used on an Ethernet controller with
> full MII interface by just leaving the input signals (RXER, CRS, COL)
> inactive.

I'm following-up to Andrew's suggestion of making it a dedicated
phy-mode. You say that this requires only phy-side configuration,
however you also say that with MII-lite, you can't do half-duplex.

Looking at the way we configure the MAC to PHY link, how can the MAC
driver know that HD isn't available if this is a phy-only property ?

Relying on the fact that the PHYs that use MII-Lite will only ever
setup a full-duplex link with the partner seems a bit fragile, when we
could indicate that this new MII-Lite mode only supports 10FD/100FD,
through this mapping code here :

https://elixir.bootlin.com/linux/v6.16-rc2/source/drivers/net/phy/phy_caps.c#L282

Besides that, given that this is a physically different MAC to PHY
interface (missing signals compared to MII), one could also argue that
this warrants a dedicated phy-mode.

Maxime

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v2 1/3] net: phy: bcm54811: Fix the PHY initialization
  2025-06-23 15:10 ` [PATCH net-next v2 1/3] net: phy: bcm54811: Fix the PHY initialization Kamil Horák - 2N
@ 2025-06-23 15:56   ` Russell King (Oracle)
  2025-06-24 11:43     ` Kamil Horák (2N)
  0 siblings, 1 reply; 13+ messages in thread
From: Russell King (Oracle) @ 2025-06-23 15:56 UTC (permalink / raw)
  To: Kamil Horák - 2N
  Cc: florian.fainelli, bcm-kernel-feedback-list, andrew, hkallweit1,
	davem, edumazet, kuba, pabeni, krzk+dt, conor+dt, netdev,
	devicetree, linux-kernel, robh

On Mon, Jun 23, 2025 at 05:10:47PM +0200, Kamil Horák - 2N wrote:
>  	/* With BCM54811, BroadR-Reach implies no autoneg */
> -	if (priv->brr_mode)
> +	if (priv->brr_mode) {
>  		phydev->autoneg = 0;

This, to me, looks extremely buggy. Setting phydev->autoneg to zero does
not prevent userspace enabling autoneg later. It also doesn't report to
userspace that autoneg is disabled. Not your problem, but a latent bug
in this driver.

> +		/* Disable Long Distance Signaling, the BRR mode autoneg */
> +		err = phy_modify(phydev, MII_BCM54XX_LRECR, LRECR_LDSEN, 0);
> +		if (err < 0)
> +			return err;
> +	}
>  
> -	return bcm5481x_set_brrmode(phydev, priv->brr_mode);
> +	if (!phy_interface_is_rgmii(phydev) ||
> +	    phydev->interface == PHY_INTERFACE_MODE_MII) {

Not sure this condition actually reflects what you're trying to
achieve, because if we're using PHY_INTERFACE_MODE_MII, then
!phy_interface_is_rgmii(phydev) will be true because phydev->interface
isn't one of the RGMII modes. So, I think this can be reduced to simply

	if (!phy_interface_is_rgmii(phydev)) {

> +		/* Misc Control: GMII/MII Mode (not RGMII) */
> +		err = bcm54xx_auxctl_write(phydev,
> +					   MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
> +					   MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_SKEW_EN |
> +					   MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RSVD
> +		);

I don't think this addition is described in the commit message.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode
  2025-06-23 15:10 ` [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode Kamil Horák - 2N
  2025-06-23 15:51   ` Maxime Chevallier
@ 2025-06-23 15:58   ` Russell King (Oracle)
  2025-06-23 18:08   ` Andrew Lunn
  2 siblings, 0 replies; 13+ messages in thread
From: Russell King (Oracle) @ 2025-06-23 15:58 UTC (permalink / raw)
  To: Kamil Horák - 2N
  Cc: florian.fainelli, bcm-kernel-feedback-list, andrew, hkallweit1,
	davem, edumazet, kuba, pabeni, krzk+dt, conor+dt, netdev,
	devicetree, linux-kernel, robh

On Mon, Jun 23, 2025 at 05:10:46PM +0200, Kamil Horák - 2N wrote:
> +	priv->mii_lite_mode = of_property_read_bool(np, "mii-lite-mode");

Do we really want a property for this, or is it really a separate
interface mode? Should we introduce PHY_INTERFACE_MODE_MII_LITE ?

> +	if (!phy_interface_is_rgmii(phydev) ||
> +	    phydev->interface == PHY_INTERFACE_MODE_MII) {

Same comment on this as in patch 1.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v2 3/3] dt-bindings: ethernet-phy: add optional mii-lite-mode flag
  2025-06-23 15:10 ` [PATCH net-next v2 3/3] dt-bindings: ethernet-phy: add optional mii-lite-mode flag Kamil Horák - 2N
@ 2025-06-23 15:59   ` Russell King (Oracle)
  0 siblings, 0 replies; 13+ messages in thread
From: Russell King (Oracle) @ 2025-06-23 15:59 UTC (permalink / raw)
  To: Kamil Horák - 2N
  Cc: florian.fainelli, bcm-kernel-feedback-list, andrew, hkallweit1,
	davem, edumazet, kuba, pabeni, krzk+dt, conor+dt, netdev,
	devicetree, linux-kernel, robh

On Mon, Jun 23, 2025 at 05:10:48PM +0200, Kamil Horák - 2N wrote:
> From: Kamil Horák (2N) <kamilh@axis.com>
> 
> The Broadcom bcm54810 and bcm54811 PHYs support MII and MII-Lite
> interface modes. The MII-Lite mode does not use TXR, RXER, CRS and COL
> signals. However, the hardware strapping only selects MII mode,
> distinction between MII and MII-Lite must be done by software.
> 
> Add optional mii-lite-mode flag to switch the PHY to MII-Lite mode.
> 
> Signed-off-by: Kamil Horák (2N) <kamilh@axis.com>
> ---
>  Documentation/devicetree/bindings/net/ethernet-phy.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index 71e2cd32580f..edfd16044770 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -101,6 +101,14 @@ properties:
>        1BR-10 names. The PHY must be configured to operate in BroadR-Reach mode
>        by software.
>  
> +  mii-lite-mode:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      If set, indicates the use of MII-Lite variant of MII, without the
> +      functions of TXER, RXER, CRS and COL signals for Broadcom PHYs. These
> +      PHYs can be strapped to use MII mode but the MII or MII-Lite selection
> +      must be done by software.

I'll put the same question here as I did on patch 2. Do we want this to
be a separate property, or would it make more sense as a different
phy-mode value?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode
  2025-06-23 15:51   ` Maxime Chevallier
@ 2025-06-23 17:55     ` Andrew Lunn
  2025-06-23 18:18       ` Russell King (Oracle)
  2025-06-24  7:59     ` Kamil Horák (2N)
  1 sibling, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2025-06-23 17:55 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: Kamil Horák - 2N, florian.fainelli, bcm-kernel-feedback-list,
	hkallweit1, linux, davem, edumazet, kuba, pabeni, krzk+dt,
	conor+dt, netdev, devicetree, linux-kernel, robh

On Mon, Jun 23, 2025 at 05:51:35PM +0200, Maxime Chevallier wrote:
> Hi Kamil,
> 
> On Mon, 23 Jun 2025 17:10:46 +0200
> Kamil Horák - 2N <kamilh@axis.com> wrote:
> 
> > From: Kamil Horák (2N) <kamilh@axis.com>
> > 
> > The Broadcom bcm54810 and bcm54811 PHYs are capable to operate in
> > simplified MII mode, without TXER, RXER, CRS and COL signals as defined
> > for the MII. While the PHY can be strapped for MII mode, the selection
> > between MII and MII-Lite must be done by software.
> > The MII-Lite mode can be used with some Ethernet controllers, usually
> > those used in automotive applications. The absence of COL signal
> > makes half-duplex link modes impossible but does not interfere with
> > BroadR-Reach link modes on Broadcom PHYs, because they are full-duplex
> > only. The MII-Lite mode can be also used on an Ethernet controller with
> > full MII interface by just leaving the input signals (RXER, CRS, COL)
> > inactive.
> 
> I'm following-up to Andrew's suggestion of making it a dedicated
> phy-mode. You say that this requires only phy-side configuration,
> however you also say that with MII-lite, you can't do half-duplex.
> 
> Looking at the way we configure the MAC to PHY link, how can the MAC
> driver know that HD isn't available if this is a phy-only property ?

One would hope that when the PHY is configured to -lite, it changes
its abilities register to indicate it does not support half duplex
modes? But without looking at the datasheet, i've no idea if it
actually does.

There is also an ordering issuer, it needs to be put into -lite mode
before phy_probe reads the abilities, which is after the probe()
method is called. However, at this point, we don't know the interface
mode, that only comes later.

So this gets interesting, and there is no indication in the commit
message this has been thought about.

    Andrew

---
pw-bot: cr

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode
  2025-06-23 15:10 ` [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode Kamil Horák - 2N
  2025-06-23 15:51   ` Maxime Chevallier
  2025-06-23 15:58   ` Russell King (Oracle)
@ 2025-06-23 18:08   ` Andrew Lunn
  2 siblings, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2025-06-23 18:08 UTC (permalink / raw)
  To: Kamil Horák - 2N
  Cc: florian.fainelli, bcm-kernel-feedback-list, hkallweit1, linux,
	davem, edumazet, kuba, pabeni, krzk+dt, conor+dt, netdev,
	devicetree, linux-kernel, robh

On Mon, Jun 23, 2025 at 05:10:46PM +0200, Kamil Horák - 2N wrote:
> From: Kamil Horák (2N) <kamilh@axis.com>
> 
> The Broadcom bcm54810 and bcm54811 PHYs are capable to operate in
> simplified MII mode, without TXER, RXER, CRS and COL signals as defined
> for the MII. While the PHY can be strapped for MII mode, the selection
> between MII and MII-Lite must be done by software.
> The MII-Lite mode can be used with some Ethernet controllers, usually
> those used in automotive applications. The absence of COL signal
> makes half-duplex link modes impossible but does not interfere with
> BroadR-Reach link modes on Broadcom PHYs, because they are full-duplex
> only. The MII-Lite mode can be also used on an Ethernet controller with
> full MII interface by just leaving the input signals (RXER, CRS, COL)
> inactive.

So everybody seems to be a general agreement this needs more thought.

How does a MAC know its RXER, CRS, COL inputs are inactive? Are you
expecting boards use pull up resistors? Or is the MAC driver expected
to poke around in the PHY node and find this lite property? That would
not be accepted. A phy-mode make would this clear, but it does require
every MAC which could be connected to this PHY needs to also accept
this PHY mode. Which comes back to, are pull-ups enough, so the MAC
has no idea it is connected to a -lite PHY?

	Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode
  2025-06-23 17:55     ` Andrew Lunn
@ 2025-06-23 18:18       ` Russell King (Oracle)
  0 siblings, 0 replies; 13+ messages in thread
From: Russell King (Oracle) @ 2025-06-23 18:18 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Maxime Chevallier, Kamil Horák - 2N, florian.fainelli,
	bcm-kernel-feedback-list, hkallweit1, davem, edumazet, kuba,
	pabeni, krzk+dt, conor+dt, netdev, devicetree, linux-kernel, robh

On Mon, Jun 23, 2025 at 07:55:03PM +0200, Andrew Lunn wrote:
> On Mon, Jun 23, 2025 at 05:51:35PM +0200, Maxime Chevallier wrote:
> > Hi Kamil,
> > 
> > On Mon, 23 Jun 2025 17:10:46 +0200
> > Kamil Horák - 2N <kamilh@axis.com> wrote:
> > 
> > > From: Kamil Horák (2N) <kamilh@axis.com>
> > > 
> > > The Broadcom bcm54810 and bcm54811 PHYs are capable to operate in
> > > simplified MII mode, without TXER, RXER, CRS and COL signals as defined
> > > for the MII. While the PHY can be strapped for MII mode, the selection
> > > between MII and MII-Lite must be done by software.
> > > The MII-Lite mode can be used with some Ethernet controllers, usually
> > > those used in automotive applications. The absence of COL signal
> > > makes half-duplex link modes impossible but does not interfere with
> > > BroadR-Reach link modes on Broadcom PHYs, because they are full-duplex
> > > only. The MII-Lite mode can be also used on an Ethernet controller with
> > > full MII interface by just leaving the input signals (RXER, CRS, COL)
> > > inactive.
> > 
> > I'm following-up to Andrew's suggestion of making it a dedicated
> > phy-mode. You say that this requires only phy-side configuration,
> > however you also say that with MII-lite, you can't do half-duplex.
> > 
> > Looking at the way we configure the MAC to PHY link, how can the MAC
> > driver know that HD isn't available if this is a phy-only property ?
> 
> One would hope that when the PHY is configured to -lite, it changes
> its abilities register to indicate it does not support half duplex
> modes? But without looking at the datasheet, i've no idea if it
> actually does.
> 
> There is also an ordering issuer, it needs to be put into -lite mode
> before phy_probe reads the abilities, which is after the probe()
> method is called. However, at this point, we don't know the interface
> mode, that only comes later.
> 
> So this gets interesting, and there is no indication in the commit
> message this has been thought about.

... which is another reaosn for using phylink, because phylink
restricts the abilities of the PHY (and its advertisement) according
to the PHY interface mode.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode
  2025-06-23 15:51   ` Maxime Chevallier
  2025-06-23 17:55     ` Andrew Lunn
@ 2025-06-24  7:59     ` Kamil Horák (2N)
  1 sibling, 0 replies; 13+ messages in thread
From: Kamil Horák (2N) @ 2025-06-24  7:59 UTC (permalink / raw)
  To: Maxime Chevallier; +Cc: netdev



On 6/23/25 17:51, Maxime Chevallier wrote:
> Hi Kamil,
> 
> On Mon, 23 Jun 2025 17:10:46 +0200
> Kamil Horák - 2N <kamilh@axis.com> wrote:
> 
>> From: Kamil Horák (2N) <kamilh@axis.com>
>>
>> The Broadcom bcm54810 and bcm54811 PHYs are capable to operate in
>> simplified MII mode, without TXER, RXER, CRS and COL signals as defined
>> for the MII. While the PHY can be strapped for MII mode, the selection
>> between MII and MII-Lite must be done by software.
>> The MII-Lite mode can be used with some Ethernet controllers, usually
>> those used in automotive applications. The absence of COL signal
>> makes half-duplex link modes impossible but does not interfere with
>> BroadR-Reach link modes on Broadcom PHYs, because they are full-duplex
>> only. The MII-Lite mode can be also used on an Ethernet controller with
>> full MII interface by just leaving the input signals (RXER, CRS, COL)
>> inactive.
> 
> I'm following-up to Andrew's suggestion of making it a dedicated
> phy-mode. You say that this requires only phy-side configuration,
> however you also say that with MII-lite, you can't do half-duplex.
> 
OK let's go this way. I was not fully aware about the fact that MII-Lite 
were actually Broadcom's specialty, only found in its two-wire Ethernet 
PHYs.

> Looking at the way we configure the MAC to PHY link, how can the MAC
> driver know that HD isn't available if this is a phy-only property ?
It is a very special case, because the Broadcom two-wire PHY would never 
go half-duplex in BroadR-Reach mode. However, it is capable of "normal" 
(IEEE) modes as well and in such case, half-duplex modes must be avoided 
with MII-Lite. I doubt anyone would create a hardware capable of both 
IEEE and BRR modes - it would be difficult to do, perhaps only using 
kind of hardware plugin interface modules.

The MAC driver should handle the situation properly (and the imx6ul SoC 
in our case does so), considering the unconnected MII signals are 
inactive. These signals even do not need to be put on external pads and 
pulled up or down.

> 
> Relying on the fact that the PHYs that use MII-Lite will only ever
> setup a full-duplex link with the partner seems a bit fragile, when we
> could indicate that this new MII-Lite mode only supports 10FD/100FD,
> through this mapping code here :
> 
> https://elixir.bootlin.com/linux/v6.16-rc2/source/drivers/net/phy/phy_caps.c#L282
> 
> Besides that, given that this is a physically different MAC to PHY
> interface (missing signals compared to MII), one could also argue that
> this warrants a dedicated phy-mode.
So I'll change it so that there will be separate MII-Lite PHY mode, only 
usable with Broadcom PHYs. It would be possible to wire other PHYs the 
same way, at least there seems to be some support in Marvell, Microchip 
and Texas Instruments ones.

> 
> Maxime


Kamil

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v2 1/3] net: phy: bcm54811: Fix the PHY initialization
  2025-06-23 15:56   ` Russell King (Oracle)
@ 2025-06-24 11:43     ` Kamil Horák (2N)
  0 siblings, 0 replies; 13+ messages in thread
From: Kamil Horák (2N) @ 2025-06-24 11:43 UTC (permalink / raw)
  To: netdev



On 6/23/25 17:56, Russell King (Oracle) wrote:
 > On Mon, Jun 23, 2025 at 05:10:47PM +0200, Kamil Horák - 2N wrote:
 >>       /* With BCM54811, BroadR-Reach implies no autoneg */
 >> -    if (priv->brr_mode)
 >> +    if (priv->brr_mode) {
 >>           phydev->autoneg = 0;
 >
 > This, to me, looks extremely buggy. Setting phydev->autoneg to zero does
 > not prevent userspace enabling autoneg later. It also doesn't report to
 > userspace that autoneg is disabled. Not your problem, but a latent bug
 > in this driver.
So I'll try to do this cleanly. Hope it is possible somehow, because of 
the bcm54811 and bcm54810 properties: the bcm54810 is fully featured PHY 
version which includes auto-negotiation of its own (called LDS or Long 
Distance Signalling) by Broadcom. The bcm54811 is limited, mainly in 
absence of LDS - but the controlling bit is set upon reset, exactly as 
in the bcm54810. The bcm54811 datasheet states "Reserved, reset value 1, 
must be written to zero after every device reset" without describing the 
purpose of the bit. The bcm54810 datasheet describes the bit as "LDS 
Enable" = enable auto-negotiation in BroadR-Reach mode. In other words, 
the LDS control bit exists on the bcm54811 and must be zeroed in order 
to be able to use the PHY in forced speed / master / slave mode. Strange 
enough. Still we need to disable any auto-negotioation if there is 
bcm54811 in BroadR-Reach link mode. For bcm54811 in IEEE mode (using 
"normal" wiring for 1000/100/10MBit, standard IEEE PHY handling would be 
used, including the possibility of half/full duplex.


 >
 >> +        /* Disable Long Distance Signaling, the BRR mode autoneg */
 >> +        err = phy_modify(phydev, MII_BCM54XX_LRECR, LRECR_LDSEN, 0);
 >> +        if (err < 0)
 >> +            return err;
 >> +    }
 >>   -    return bcm5481x_set_brrmode(phydev, priv->brr_mode);
 >> +    if (!phy_interface_is_rgmii(phydev) ||
 >> +        phydev->interface == PHY_INTERFACE_MODE_MII) {
 >
 > Not sure this condition actually reflects what you're trying to
 > achieve, because if we're using PHY_INTERFACE_MODE_MII, then
 > !phy_interface_is_rgmii(phydev) will be true because phydev->interface
 > isn't one of the RGMII modes. So, I think this can be reduced to simply
 >
 >     if (!phy_interface_is_rgmii(phydev)) {
 >
 >> +        /* Misc Control: GMII/MII Mode (not RGMII) */
 >> +        err = bcm54xx_auxctl_write(phydev,
 >> +                       MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
 >> +                       MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_SKEW_EN |
 >> +                       MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RSVD
 >> +        );
 >
 > I don't think this addition is described in the commit message.
 >
This will change with introducing of new interface mode for MII-Lite as 
recommended in other answers to this group of patches.


Kamil

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-06-24 11:43 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-23 15:10 [PATCH net-next v2 0/3] net: phy: bcm54811: Fix the PHY initialization Kamil Horák - 2N
2025-06-23 15:10 ` [PATCH net-next v2 2/3] net: phy: bcm5481x: Implement MII-Lite mode Kamil Horák - 2N
2025-06-23 15:51   ` Maxime Chevallier
2025-06-23 17:55     ` Andrew Lunn
2025-06-23 18:18       ` Russell King (Oracle)
2025-06-24  7:59     ` Kamil Horák (2N)
2025-06-23 15:58   ` Russell King (Oracle)
2025-06-23 18:08   ` Andrew Lunn
2025-06-23 15:10 ` [PATCH net-next v2 1/3] net: phy: bcm54811: Fix the PHY initialization Kamil Horák - 2N
2025-06-23 15:56   ` Russell King (Oracle)
2025-06-24 11:43     ` Kamil Horák (2N)
2025-06-23 15:10 ` [PATCH net-next v2 3/3] dt-bindings: ethernet-phy: add optional mii-lite-mode flag Kamil Horák - 2N
2025-06-23 15:59   ` Russell King (Oracle)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).