[PATCH net-next v3 0/2] net: lan743x: add RMII support for PCI11x1x

[PATCH net-next v3 0/2] net: lan743x: add RMII support for PCI11x1x

[Thangaraj Samynathan]

This series adds RMII interface support for the Microchip PCI11x1x
Ethernet controller.

The PCI11x1x device supports RMII as an alternative MAC-PHY interface,
selected via the STRAP_READ software strap register. Patch 1 reads the
RMII strap bits from this register and sets the is_rmii_en flag. Patch 2
uses this flag to configure the PHY interface mode, phylink supported
interfaces, and enables RMII in hardware via the RMII_CTL register.

Change Log:
===========
v2 -> v3:
  - Update debug log to report selected interface (SGMII/RMII/RGMII)
    instead of only SGMII enable/disable state [patch 1/2]
  - Update commit message to document that EEE is disabled by setting
    lpi_capabilities = 0 [patch 2/2]

v1 -> v2:
  - Remove redundant mac_capabilities &= ~MAC_1000FD; phylink already
    handles capability reduction for RMII via phy_caps_from_interface()
    [patch 2/2]

Thangaraj Samynathan (2):
  net: lan743x: add RMII strap status detection for PCI11x1x
  net: lan743x: add support for RMII interface

 drivers/net/ethernet/microchip/lan743x_main.c | 29 +++++++++++++++++--
 drivers/net/ethernet/microchip/lan743x_main.h |  6 ++++
 2 files changed, 33 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH net-next v3 1/2] net: lan743x: add RMII strap status detection for PCI11x1x

[Thangaraj Samynathan]

Extend pci11x1x_strap_get_status() to read the RMII strap bits from
the STRAP_READ register. The is_rmii_en flag is initialized to
false and updated based on the hardware strap only if SGMII is not
already enabled. This ensures correct interface identification during
adapter initialization.

Signed-off-by: Thangaraj Samynathan <thangaraj.s@microchip.com>
---
Changes in v3:
  - Update debug log to report selected interface (SGMII/RMII/RGMII)
    instead of only SGMII enable/disable state

 drivers/net/ethernet/microchip/lan743x_main.c | 12 ++++++++++--
 drivers/net/ethernet/microchip/lan743x_main.h |  3 +++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 1cdce35e1423..0798f3f1f435 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -42,6 +42,7 @@ static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
 	u32 strap;
 	int ret;
 
+	adapter->is_rmii_en = false;
 	/* Timeout = 100 (i.e. 1 sec (10 msce * 100)) */
 	ret = lan743x_hs_syslock_acquire(adapter, 100);
 	if (ret < 0) {
@@ -73,8 +74,15 @@ static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
 			adapter->is_sgmii_en = false;
 		}
 	}
-	netif_dbg(adapter, drv, adapter->netdev,
-		  "SGMII I/F %sable\n", adapter->is_sgmii_en ? "En" : "Dis");
+
+	if (!adapter->is_sgmii_en && strap & STRAP_READ_USE_RMII_EN_) {
+		if (strap & STRAP_READ_RMII_EN_)
+			adapter->is_rmii_en = true;
+	}
+
+	netif_dbg(adapter, drv, adapter->netdev, "Selected I/F: %s\n",
+		  adapter->is_sgmii_en ? "SGMII" :
+		  adapter->is_rmii_en  ? "RMII"  : "RGMII");
 }
 
 static bool is_pci11x1x_chip(struct lan743x_adapter *adapter)
diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
index 1573c8f9c993..1f8d9294a6ef 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.h
+++ b/drivers/net/ethernet/microchip/lan743x_main.h
@@ -36,7 +36,9 @@
 #define FPGA_SGMII_OP			BIT(24)
 
 #define STRAP_READ			(0x0C)
+#define STRAP_READ_USE_RMII_EN_		BIT(23)
 #define STRAP_READ_USE_SGMII_EN_	BIT(22)
+#define STRAP_READ_RMII_EN_		BIT(7)
 #define STRAP_READ_SGMII_EN_		BIT(6)
 #define STRAP_READ_SGMII_REFCLK_	BIT(5)
 #define STRAP_READ_SGMII_2_5G_		BIT(4)
@@ -1072,6 +1074,7 @@ struct lan743x_adapter {
 	struct lan743x_rx       rx[LAN743X_USED_RX_CHANNELS];
 	bool			is_pci11x1x;
 	bool			is_sgmii_en;
+	bool			is_rmii_en;
 	/* protect ethernet syslock */
 	spinlock_t		eth_syslock_spinlock;
 	bool			eth_syslock_en;
-- 
2.34.1

[PATCH net-next v3 2/2] net: lan743x: add support for RMII interface

[Thangaraj Samynathan]

Enable RMII interface in the lan743x driver for PHY and MAC
configuration.

- Select RMII interface in lan743x_phy_interface_select().
- Update phylink supported_interfaces and MAC capabilities.
- Enable RMII via RMII_CTL in lan743x_hardware_init().
- Define RMII_CTL register and enable bit in lan743x_main.h.

EEE is not supported with RMII on PCI11x1x: the hardware does not
implement LPI signaling over RMII. Clear RMII from lpi_interfaces to
prevent phylink from enabling EEE on this interface.

Signed-off-by: Thangaraj Samynathan <thangaraj.s@microchip.com>
---
Changes in v3:
  - Update commit message to document that EEE is disabled by setting
    lpi_capabilities = 0

Changes in v2:
  - Remove redundant mac_capabilities &= ~MAC_1000FD; phylink already
    handles capability reduction for RMII via phy_caps_from_interface()

 drivers/net/ethernet/microchip/lan743x_main.c | 17 +++++++++++++++++
 drivers/net/ethernet/microchip/lan743x_main.h |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 0798f3f1f435..ad3f833c8d86 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -1402,6 +1402,8 @@ static void lan743x_phy_interface_select(struct lan743x_adapter *adapter)
 
 	if (adapter->is_pci11x1x && adapter->is_sgmii_en)
 		adapter->phy_interface = PHY_INTERFACE_MODE_SGMII;
+	else if (adapter->is_pci11x1x && adapter->is_rmii_en)
+		adapter->phy_interface = PHY_INTERFACE_MODE_RMII;
 	else if (id_rev == ID_REV_ID_LAN7430_)
 		adapter->phy_interface = PHY_INTERFACE_MODE_GMII;
 	else if ((id_rev == ID_REV_ID_LAN7431_) && (data & MAC_CR_MII_EN_))
@@ -3190,6 +3192,12 @@ static int lan743x_phylink_create(struct lan743x_adapter *adapter)
 		__set_bit(PHY_INTERFACE_MODE_MII,
 			  adapter->phylink_config.supported_interfaces);
 		break;
+	case PHY_INTERFACE_MODE_RMII:
+		__set_bit(PHY_INTERFACE_MODE_RMII,
+			  adapter->phylink_config.supported_interfaces);
+		adapter->phylink_config.lpi_capabilities = 0;
+		break;
+
 	default:
 		phy_interface_set_rgmii(adapter->phylink_config.supported_interfaces);
 	}
@@ -3197,6 +3205,9 @@ static int lan743x_phylink_create(struct lan743x_adapter *adapter)
 	memcpy(adapter->phylink_config.lpi_interfaces,
 	       adapter->phylink_config.supported_interfaces,
 	       sizeof(adapter->phylink_config.lpi_interfaces));
+	if (adapter->phy_interface == PHY_INTERFACE_MODE_RMII)
+		__clear_bit(PHY_INTERFACE_MODE_RMII,
+			    adapter->phylink_config.lpi_interfaces);
 
 	pl = phylink_create(&adapter->phylink_config, NULL,
 			    adapter->phy_interface, &lan743x_phylink_mac_ops);
@@ -3541,6 +3552,7 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
 {
 	struct lan743x_tx *tx;
 	u32 sgmii_ctl;
+	u32 rmii_ctl;
 	int index;
 	int ret;
 
@@ -3562,6 +3574,11 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
 			sgmii_ctl |= SGMII_CTL_SGMII_POWER_DN_;
 		}
 		lan743x_csr_write(adapter, SGMII_CTL, sgmii_ctl);
+		if (adapter->is_rmii_en) {
+			rmii_ctl = lan743x_csr_read(adapter, RMII_CTL);
+			rmii_ctl |= RMII_CTL_RMII_ENABLE_;
+			lan743x_csr_write(adapter, RMII_CTL, rmii_ctl);
+		}
 	} else {
 		adapter->max_tx_channels = LAN743X_MAX_TX_CHANNELS;
 		adapter->used_tx_channels = LAN743X_USED_TX_CHANNELS;
diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
index 1f8d9294a6ef..d9495cf96b41 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.h
+++ b/drivers/net/ethernet/microchip/lan743x_main.h
@@ -325,6 +325,9 @@
 #define MAC_WUCSR2_IPV6_TCPSYN_RCD_	BIT(5)
 #define MAC_WUCSR2_IPV4_TCPSYN_RCD_	BIT(4)
 
+#define RMII_CTL			(0x710)
+#define RMII_CTL_RMII_ENABLE_		BIT(0)
+
 #define SGMII_ACC			(0x720)
 #define SGMII_ACC_SGMII_BZY_		BIT(31)
 #define SGMII_ACC_SGMII_WR_		BIT(30)
-- 
2.34.1

Re: [PATCH net-next v3 1/2] net: lan743x: add RMII strap status detection for PCI11x1x

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
Subject: net: lan743x: add RMII strap status detection for PCI11x1x

This patch extends pci11x1x_strap_get_status() to read RMII strap bits
from STRAP_READ and initialise a new is_rmii_en flag on the adapter,
updating the netif_dbg() to print the selected interface as SGMII, RMII
or RGMII.

> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
> index 1cdce35e1423..0798f3f1f435 100644
> --- a/drivers/net/ethernet/microchip/lan743x_main.c
> +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> @@ -73,8 +74,15 @@ static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
>  			adapter->is_sgmii_en = false;
>  		}
>  	}
> -	netif_dbg(adapter, drv, adapter->netdev,
> -		  "SGMII I/F %sable\n", adapter->is_sgmii_en ? "En" : "Dis");
> +
> +	if (!adapter->is_sgmii_en && strap & STRAP_READ_USE_RMII_EN_) {
> +		if (strap & STRAP_READ_RMII_EN_)
> +			adapter->is_rmii_en = true;
> +	}
> +
> +	netif_dbg(adapter, drv, adapter->netdev, "Selected I/F: %s\n",
> +		  adapter->is_sgmii_en ? "SGMII" :
> +		  adapter->is_rmii_en  ? "RMII"  : "RGMII");
>  }

[Low]
This isn't a bug, but does the new "Selected I/F: %s" debug print stay
consistent with the rest of the driver when is_rmii_en is true?

In lan743x_mdiobus_init() the non-SGMII branch still prints unconditionally:

	} else {
		dev_dbg(&adapter->pdev->dev, "RGMII operation\n");
		// Only C22 support when RGMII I/F
		adapter->mdiobus->read = lan743x_mdiobus_read_c22;
		adapter->mdiobus->write = lan743x_mdiobus_write_c22;
		adapter->mdiobus->name = "lan743x-mdiobus";

So an RMII-strapped board will report "Selected I/F: RMII" from
pci11x1x_strap_get_status() and then "RGMII operation" from
lan743x_mdiobus_init() in the same boot. The C22-only setup is correct
for both RGMII and RMII, so the only effect is the contradictory dmesg
line. Should this dev_dbg() also be updated to reflect the RMII case,
either in this patch or in the follow-up "net: lan743x: add support for
RMII interface"?

Re: [PATCH net-next v3 2/2] net: lan743x: add support for RMII interface

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
net: lan743x: add support for RMII interface

This commit adds RMII interface support to the lan743x driver, wiring
up phy_interface selection, phylink supported_interfaces and MAC
capabilities, and enabling the RMII front-end via a new RMII_CTL
register write in lan743x_hardware_init().

> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
> index 0798f3f1f435..ad3f833c8d86 100644
> --- a/drivers/net/ethernet/microchip/lan743x_main.c
> +++ b/drivers/net/ethernet/microchip/lan743x_main.c

[ ... ]

> @@ -3562,6 +3574,11 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
>  			sgmii_ctl |= SGMII_CTL_SGMII_POWER_DN_;
>  		}
>  		lan743x_csr_write(adapter, SGMII_CTL, sgmii_ctl);
> +		if (adapter->is_rmii_en) {
> +			rmii_ctl = lan743x_csr_read(adapter, RMII_CTL);
> +			rmii_ctl |= RMII_CTL_RMII_ENABLE_;
> +			lan743x_csr_write(adapter, RMII_CTL, rmii_ctl);
> +		}

[Medium]
Should the RMII_CTL programming mirror the symmetric pattern used for
SGMII_CTL just above it?

The SGMII block sets or clears the enable/power-down bits in both
directions:

	sgmii_ctl = lan743x_csr_read(adapter, SGMII_CTL);
	if (adapter->is_sgmii_en) {
		sgmii_ctl |= SGMII_CTL_SGMII_ENABLE_;
		sgmii_ctl &= ~SGMII_CTL_SGMII_POWER_DN_;
	} else {
		sgmii_ctl &= ~SGMII_CTL_SGMII_ENABLE_;
		sgmii_ctl |= SGMII_CTL_SGMII_POWER_DN_;
	}
	lan743x_csr_write(adapter, SGMII_CTL, sgmii_ctl);

The new RMII block only sets RMII_CTL_RMII_ENABLE_ when is_rmii_en is
true and never clears it on the other paths.

If RMII_CTL_RMII_ENABLE_ is left set by a previous agent (for example,
a prior driver instance with a different strap, board firmware, or
state preserved across a warm reset where the driver itself sets
HW_CFG_RST_PROTECT_ during PM suspend, and pci11x1x_strap_get_status()
already accounts for cfg_load/HW_CFG_RST_PROTECT_), can the RMII
front-end remain enabled while the driver is configured for RGMII or
SGMII?

Would adding an explicit clear in the !is_rmii_en path, mirroring the
SGMII handling, give the register a known state on every probe?

Re: [PATCH net-next v3 2/2] net: lan743x: add support for RMII interface

[Paolo Abeni]

On 6/10/26 6:26 PM, Simon Horman wrote:
> From: 'Simon Horman' <horms@kernel.org>
> 
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev
> ---
> net: lan743x: add support for RMII interface
> 
> This commit adds RMII interface support to the lan743x driver, wiring
> up phy_interface selection, phylink supported_interfaces and MAC
> capabilities, and enabling the RMII front-end via a new RMII_CTL
> register write in lan743x_hardware_init().
> 
>> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
>> index 0798f3f1f435..ad3f833c8d86 100644
>> --- a/drivers/net/ethernet/microchip/lan743x_main.c
>> +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> 
> [ ... ]
> 
>> @@ -3562,6 +3574,11 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
>>  			sgmii_ctl |= SGMII_CTL_SGMII_POWER_DN_;
>>  		}
>>  		lan743x_csr_write(adapter, SGMII_CTL, sgmii_ctl);
>> +		if (adapter->is_rmii_en) {
>> +			rmii_ctl = lan743x_csr_read(adapter, RMII_CTL);
>> +			rmii_ctl |= RMII_CTL_RMII_ENABLE_;
>> +			lan743x_csr_write(adapter, RMII_CTL, rmii_ctl);
>> +		}
> 
> [Medium]
> Should the RMII_CTL programming mirror the symmetric pattern used for
> SGMII_CTL just above it?
> 
> The SGMII block sets or clears the enable/power-down bits in both
> directions:
> 
> 	sgmii_ctl = lan743x_csr_read(adapter, SGMII_CTL);
> 	if (adapter->is_sgmii_en) {
> 		sgmii_ctl |= SGMII_CTL_SGMII_ENABLE_;
> 		sgmii_ctl &= ~SGMII_CTL_SGMII_POWER_DN_;
> 	} else {
> 		sgmii_ctl &= ~SGMII_CTL_SGMII_ENABLE_;
> 		sgmii_ctl |= SGMII_CTL_SGMII_POWER_DN_;
> 	}
> 	lan743x_csr_write(adapter, SGMII_CTL, sgmii_ctl);
> 
> The new RMII block only sets RMII_CTL_RMII_ENABLE_ when is_rmii_en is
> true and never clears it on the other paths.
> 
> If RMII_CTL_RMII_ENABLE_ is left set by a previous agent (for example,
> a prior driver instance with a different strap, board firmware, or
> state preserved across a warm reset where the driver itself sets
> HW_CFG_RST_PROTECT_ during PM suspend, and pci11x1x_strap_get_status()
> already accounts for cfg_load/HW_CFG_RST_PROTECT_), can the RMII
> front-end remain enabled while the driver is configured for RGMII or
> SGMII?
> 
> Would adding an explicit clear in the !is_rmii_en path, mirroring the
> SGMII handling, give the register a known state on every probe?

I think this one is worth addressing, as it's reported by both sashikos
and could break functionality in hard-to-debug scenarios.

/P

RE: [PATCH net-next v3 1/2] net: lan743x: add RMII strap status detection for PCI11x1x

[Thangaraj.S]

Hi Simon,
Thanks for the comments.

> -----Original Message-----
> From: Simon Horman <horms@kernel.org>
> Sent: Wednesday, June 10, 2026 9:55 PM
> To: Thangaraj Samynathan - I53494 <Thangaraj.S@microchip.com>
> Cc: 'Simon Horman' <horms@kernel.org>; netdev@vger.kernel.org; Bryan
> Whitehead - C21958 <Bryan.Whitehead@microchip.com>; UNGLinuxDriver
> <UNGLinuxDriver@microchip.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH net-next v3 1/2] net: lan743x: add RMII strap status
> detection for PCI11x1x
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> From: 'Simon Horman' <horms@kernel.org>
> 
> This is an AI-generated review of your patch. The human sending this email
> has considered the AI review valid, or at least plausible.
> Full review at: https://netdev-ai.bots.linux.dev/sashiko/
> ---
> Subject: net: lan743x: add RMII strap status detection for PCI11x1x
> 
> This patch extends pci11x1x_strap_get_status() to read RMII strap bits from
> STRAP_READ and initialise a new is_rmii_en flag on the adapter, updating the
> netif_dbg() to print the selected interface as SGMII, RMII or RGMII.
> 
> > diff --git a/drivers/net/ethernet/microchip/lan743x_main.c
> > b/drivers/net/ethernet/microchip/lan743x_main.c
> > index 1cdce35e1423..0798f3f1f435 100644
> > --- a/drivers/net/ethernet/microchip/lan743x_main.c
> > +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> > @@ -73,8 +74,15 @@ static void pci11x1x_strap_get_status(struct
> lan743x_adapter *adapter)
> >                       adapter->is_sgmii_en = false;
> >               }
> >       }
> > -     netif_dbg(adapter, drv, adapter->netdev,
> > -               "SGMII I/F %sable\n", adapter->is_sgmii_en ? "En" : "Dis");
> > +
> > +     if (!adapter->is_sgmii_en && strap & STRAP_READ_USE_RMII_EN_) {
> > +             if (strap & STRAP_READ_RMII_EN_)
> > +                     adapter->is_rmii_en = true;
> > +     }
> > +
> > +     netif_dbg(adapter, drv, adapter->netdev, "Selected I/F: %s\n",
> > +               adapter->is_sgmii_en ? "SGMII" :
> > +               adapter->is_rmii_en  ? "RMII"  : "RGMII");
> >  }
> 
> [Low]
> This isn't a bug, but does the new "Selected I/F: %s" debug print stay
> consistent with the rest of the driver when is_rmii_en is true?
> 
> In lan743x_mdiobus_init() the non-SGMII branch still prints unconditionally:
> 
>         } else {
>                 dev_dbg(&adapter->pdev->dev, "RGMII operation\n");
>                 // Only C22 support when RGMII I/F
>                 adapter->mdiobus->read = lan743x_mdiobus_read_c22;
>                 adapter->mdiobus->write = lan743x_mdiobus_write_c22;
>                 adapter->mdiobus->name = "lan743x-mdiobus";
> 
> So an RMII-strapped board will report "Selected I/F: RMII" from
> pci11x1x_strap_get_status() and then "RGMII operation" from
> lan743x_mdiobus_init() in the same boot. The C22-only setup is correct for
> both RGMII and RMII, so the only effect is the contradictory dmesg line.
> Should this dev_dbg() also be updated to reflect the RMII case, either in this
> patch or in the follow-up "net: lan743x: add support for RMII interface"?
[Thangaraj Samynathan]  Agreed, I will fix this in next revision.

Thanks,
Thangaraj Samynathan

RE: [PATCH net-next v3 2/2] net: lan743x: add support for RMII interface

[Thangaraj.S]

Hi Simon/Paolo,
Thanks for the comments.

> -----Original Message-----
> From: Paolo Abeni <pabeni@redhat.com>
> Sent: Saturday, June 13, 2026 12:55 PM
> To: Simon Horman <horms@kernel.org>; Thangaraj Samynathan - I53494
> <Thangaraj.S@microchip.com>
> Cc: netdev@vger.kernel.org; Bryan Whitehead - C21958
> <Bryan.Whitehead@microchip.com>; UNGLinuxDriver
> <UNGLinuxDriver@microchip.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH net-next v3 2/2] net: lan743x: add support for RMII
> interface
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On 6/10/26 6:26 PM, Simon Horman wrote:
> > From: 'Simon Horman' <horms@kernel.org>
> >
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > Full review at: https://sashiko.dev
> > ---
> > net: lan743x: add support for RMII interface
> >
> > This commit adds RMII interface support to the lan743x driver, wiring
> > up phy_interface selection, phylink supported_interfaces and MAC
> > capabilities, and enabling the RMII front-end via a new RMII_CTL
> > register write in lan743x_hardware_init().
> >
> >> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c
> >> b/drivers/net/ethernet/microchip/lan743x_main.c
> >> index 0798f3f1f435..ad3f833c8d86 100644
> >> --- a/drivers/net/ethernet/microchip/lan743x_main.c
> >> +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> >
> > [ ... ]
> >
> >> @@ -3562,6 +3574,11 @@ static int lan743x_hardware_init(struct
> lan743x_adapter *adapter,
> >>                      sgmii_ctl |= SGMII_CTL_SGMII_POWER_DN_;
> >>              }
> >>              lan743x_csr_write(adapter, SGMII_CTL, sgmii_ctl);
> >> +            if (adapter->is_rmii_en) {
> >> +                    rmii_ctl = lan743x_csr_read(adapter, RMII_CTL);
> >> +                    rmii_ctl |= RMII_CTL_RMII_ENABLE_;
> >> +                    lan743x_csr_write(adapter, RMII_CTL, rmii_ctl);
> >> +            }
> >
> > [Medium]
> > Should the RMII_CTL programming mirror the symmetric pattern used for
> > SGMII_CTL just above it?
[Thangaraj Samynathan] No, The RMII CTL register has no such bit similar to SGMII.
Setting Only RMII Enable is enough. 
> >
> > The SGMII block sets or clears the enable/power-down bits in both
> > directions:
> >
> >       sgmii_ctl = lan743x_csr_read(adapter, SGMII_CTL);
> >       if (adapter->is_sgmii_en) {
> >               sgmii_ctl |= SGMII_CTL_SGMII_ENABLE_;
> >               sgmii_ctl &= ~SGMII_CTL_SGMII_POWER_DN_;
> >       } else {
> >               sgmii_ctl &= ~SGMII_CTL_SGMII_ENABLE_;
> >               sgmii_ctl |= SGMII_CTL_SGMII_POWER_DN_;
> >       }
> >       lan743x_csr_write(adapter, SGMII_CTL, sgmii_ctl);
> >
> > The new RMII block only sets RMII_CTL_RMII_ENABLE_ when is_rmii_en is
> > true and never clears it on the other paths.
> >
> > If RMII_CTL_RMII_ENABLE_ is left set by a previous agent (for example,
> > a prior driver instance with a different strap, board firmware, or
> > state preserved across a warm reset where the driver itself sets
> > HW_CFG_RST_PROTECT_ during PM suspend, and
> pci11x1x_strap_get_status()
> > already accounts for cfg_load/HW_CFG_RST_PROTECT_), can the RMII
> > front-end remain enabled while the driver is configured for RGMII or
> > SGMII?
> >
> > Would adding an explicit clear in the !is_rmii_en path, mirroring the
> > SGMII handling, give the register a known state on every probe?
> 
> I think this one is worth addressing, as it's reported by both sashikos and could
> break functionality in hard-to-debug scenarios.
[Thangaraj Samynathan]  No, The hardware takes care of this. The register gets updated
On reset based on strap.
> 
Thanks,
Thangaraj Samynathan