diff mbox series

[v1,5/7] ARM i.MX6q: remove Atheros AR8035 SmartEEE fixup

Message ID 20210203091857.16936-6-o.rempel@pengutronix.de
State New
Headers show
Series remove different PHY fixups | expand

Commit Message

Oleksij Rempel Feb. 3, 2021, 9:18 a.m. UTC
This fixup removes the Lpi_en bit.

If this patch breaks functionality of your board, use following device
tree properties:

	ethernet-phy@X {
		reg = <0xX>;
		eee-broken-1000t;
		eee-broken-100tx;
		....
	};

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 arch/arm/mach-imx/mach-imx6q.c | 21 ---------------------
 1 file changed, 21 deletions(-)

Comments

Russell King (Oracle) Feb. 3, 2021, 9:56 a.m. UTC | #1
On Wed, Feb 03, 2021 at 10:18:55AM +0100, Oleksij Rempel wrote:
> This fixup removes the Lpi_en bit.
> 
> If this patch breaks functionality of your board, use following device
> tree properties:
> 
> 	ethernet-phy@X {
> 		reg = <0xX>;
> 		eee-broken-1000t;
> 		eee-broken-100tx;
> 		....
> 	};

That is the historical fix for this problem, but there is a better
solution now in net-next - configuring the Tw parameter for gigabit
connections. That solves the random link drop issue when EEE is
enabled.

Support for this configuration has only recently been merged into
net-next and other trees for this merge window, so I ask that you
hold off at least this patch until the next cycle.
Oleksij Rempel Feb. 8, 2021, 9:20 a.m. UTC | #2
On Wed, Feb 03, 2021 at 09:56:28AM +0000, Russell King - ARM Linux admin wrote:
> On Wed, Feb 03, 2021 at 10:18:55AM +0100, Oleksij Rempel wrote:
> > This fixup removes the Lpi_en bit.
> > 
> > If this patch breaks functionality of your board, use following device
> > tree properties:
> > 
> > 	ethernet-phy@X {
> > 		reg = <0xX>;
> > 		eee-broken-1000t;
> > 		eee-broken-100tx;
> > 		....
> > 	};
> 
> That is the historical fix for this problem, but there is a better
> solution now in net-next - configuring the Tw parameter for gigabit
> connections. That solves the random link drop issue when EEE is
> enabled.

Do you mean this properties?
  qca,smarteee-tw-us-1g
  qca,smarteee-tw-us-100m

Do you have some recommendations, which values can be here used? Are
they same for all MACs? Or, can we calculate this values automatically?

Beside, I have seen this patch: "ARM: dts: imx6qdl-sr-som: fix some
cubox-i platforms"

I had similar issue and it was triggered by the boot loader, which
enabled 60 Ohm on-die termination. The fix was to remove this lines in
the boot loader: wm 32 0x020e07ac 0x00000200 /* 60 Ohm ODT */

> Support for this configuration has only recently been merged into
> net-next and other trees for this merge window, so I ask that you
> hold off at least this patch until the next cycle.

ok.

Regards,
Oleksij
Russell King (Oracle) Feb. 8, 2021, 2:21 p.m. UTC | #3
On Mon, Feb 08, 2021 at 10:20:38AM +0100, Oleksij Rempel wrote:
> On Wed, Feb 03, 2021 at 09:56:28AM +0000, Russell King - ARM Linux admin wrote:
> > That is the historical fix for this problem, but there is a better
> > solution now in net-next - configuring the Tw parameter for gigabit
> > connections. That solves the random link drop issue when EEE is
> > enabled.
> 
> Do you mean this properties?
>   qca,smarteee-tw-us-1g
>   qca,smarteee-tw-us-100m
> 
> Do you have some recommendations, which values can be here used? Are
> they same for all MACs? Or, can we calculate this values automatically?

I don't think there's a way to "calculate" them.

The AR8035 default is 17us for 1G and 23us for 100M. Increasing the
1G Tw to 26us or 27us fixes it on several different Solidrun platforms
(iMX6 Hummingboards and Cubox-i, and LX2160A based). The boards all
have differing layouts, so I don't think it's layout or SoC specific
(which is good news.)

These figures have been arrived at by repetitive long-term testing and
observing whether there are sporadic link drops over these platforms.

> Beside, I have seen this patch: "ARM: dts: imx6qdl-sr-som: fix some
> cubox-i platforms"

That's for a different problem: moving these settings to DT broke
some Cubox-i platforms because of the weird ways that the AR8035
configures the address bits, using the LED pin. Tying a LED to the
LED pin is not sufficient to guarantee that a board always configures
the PHY to a particular address, so it can appear on address 0 or 4
depending on noise, temperature, supply voltage, and PHY chip
thresholds.
diff mbox series

Patch

diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index d12b571a61ac..c9d7c29d95e1 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -68,32 +68,11 @@  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_PLX, 0x8609, ventana_pciesw_early_fixup);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_PLX, 0x8606, ventana_pciesw_early_fixup);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_PLX, 0x8604, ventana_pciesw_early_fixup);
 
-static int ar8035_phy_fixup(struct phy_device *dev)
-{
-	u16 val;
-
-	/* Ar803x phy SmartEEE feature cause link status generates glitch,
-	 * which cause ethernet link down/up issue, so disable SmartEEE
-	 */
-	phy_write(dev, 0xd, 0x3);
-	phy_write(dev, 0xe, 0x805d);
-	phy_write(dev, 0xd, 0x4003);
-
-	val = phy_read(dev, 0xe);
-	phy_write(dev, 0xe, val & ~(1 << 8));
-
-	return 0;
-}
-
-#define PHY_ID_AR8035 0x004dd072
-
 static void __init imx6q_enet_phy_init(void)
 {
 	if (IS_BUILTIN(CONFIG_PHYLIB)) {
 		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
 				ksz9021rn_phy_fixup);
-		phy_register_fixup_for_uid(PHY_ID_AR8035, 0xffffffef,
-				ar8035_phy_fixup);
 	}
 }