[PATCH 0/3] at803x: Add quirk to disable SmartEEE

[PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Carlo Caione]

SmartEEE, compatible with IEEE802.3az standard, is designed to include
legacy MAC without EEE capability into the power saving system. SmartEEE 
is enabled by default configuration on AR8031 after power-on or hardware 
reset.

Unfortunately this is proved causing issues for certain hw 
configurations.  We add a quirk to optionally disable SmartEEE.

PATCH 3/3 depends on https://patchwork.kernel.org/patch/10781297/


Carlo Caione (3):
  net: phy: at803x: Introduce quirk to disable SmartEEE
  dt-bindings: net: at803x: Document at803x,smarteee-disabled property
  arm64: dts: imx8mq-evk: Disable SmartEEE

 .../devicetree/bindings/net/at803x.txt        | 18 +++++++++++++++
 arch/arm64/boot/dts/freescale/imx8mq-evk.dts  |  1 +
 drivers/net/phy/at803x.c                      | 22 +++++++++++++++++++
 3 files changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/at803x.txt

-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] net: phy: at803x: Introduce quirk to disable SmartEEE

[Carlo Caione]

The proprietary SmartEEE mode makes the PHY entering a sleep mode after
a configurable idle time. It does this without LPI signals coming from
MAC.

Since this is a configurable behavior and it is known to cause issues
add a way to disable it using a custom quirk.

Signed-off-by: Carlo Caione <ccaione@baylibre.com>
---
 drivers/net/phy/at803x.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index f9432d053a22..509cd7a4f7eb 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -38,12 +38,14 @@
 #define AT803X_DEVICE_ADDR			0x03
 #define AT803X_LOC_MAC_ADDR_0_15_OFFSET		0x804C
 #define AT803X_LOC_MAC_ADDR_16_31_OFFSET	0x804B
+#define AT803X_SMARTEEE_CTL3_OFFSET		0x805D
 #define AT803X_LOC_MAC_ADDR_32_47_OFFSET	0x804A
 #define AT803X_MMD_ACCESS_CONTROL		0x0D
 #define AT803X_MMD_ACCESS_CONTROL_DATA		0x0E
 #define AT803X_FUNC_DATA			0x4003
 #define AT803X_REG_CHIP_CONFIG			0x1f
 #define AT803X_BT_BX_REG_SEL			0x8000
+#define AT803X_SMARTEEE_DISABLED_VAL		0x1000
 
 #define AT803X_DEBUG_ADDR			0x1D
 #define AT803X_DEBUG_DATA			0x1E
@@ -65,12 +67,15 @@
 #define ATH8035_PHY_ID 0x004dd072
 #define AT803X_PHY_ID_MASK			0xffffffef
 
+#define AT803X_SMARTEEE_FEATURE_DISABLE		BIT(1)
+
 MODULE_DESCRIPTION("Atheros 803x PHY driver");
 MODULE_AUTHOR("Matus Ujhelyi");
 MODULE_LICENSE("GPL");
 
 struct at803x_priv {
 	bool phy_reset:1;
+	u32 quirks;
 };
 
 struct at803x_context {
@@ -210,6 +215,13 @@ static void at803x_get_wol(struct phy_device *phydev,
 		wol->wolopts |= WAKE_MAGIC;
 }
 
+static int at803x_disable_smarteee(struct phy_device *phydev)
+{
+	return phy_write_mmd(phydev, AT803X_DEVICE_ADDR,
+			     AT803X_SMARTEEE_CTL3_OFFSET,
+			     AT803X_SMARTEEE_DISABLED_VAL);
+}
+
 static int at803x_suspend(struct phy_device *phydev)
 {
 	int value;
@@ -242,6 +254,9 @@ static int at803x_probe(struct phy_device *phydev)
 	if (!priv)
 		return -ENOMEM;
 
+	if (of_property_read_bool(dev->of_node, "at803x,smarteee-disabled"))
+		priv->quirks |= AT803X_SMARTEEE_FEATURE_DISABLE;
+
 	phydev->priv = priv;
 
 	return 0;
@@ -249,6 +264,7 @@ static int at803x_probe(struct phy_device *phydev)
 
 static int at803x_config_init(struct phy_device *phydev)
 {
+	struct at803x_priv *priv = phydev->priv;
 	int ret;
 
 	ret = genphy_config_init(phydev);
@@ -269,6 +285,12 @@ static int at803x_config_init(struct phy_device *phydev)
 			return ret;
 	}
 
+	if (priv->quirks & AT803X_SMARTEEE_FEATURE_DISABLE) {
+		ret = at803x_disable_smarteee(phydev);
+		if (ret < 0)
+			return ret;
+	}
+
 	return 0;
 }
 
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] dt-bindings: net: at803x: Document at803x, smarteee-disabled property

[Carlo Caione]

SmartEEE is a proprietary protocol that allows legacy MAC/SoC devices
without 802.3az support to function as a complete 802.3az system.

This is know to cause issues so a new property is added to optionally
disable this feature.

Signed-off-by: Carlo Caione <ccaione@baylibre.com>
---
 .../devicetree/bindings/net/at803x.txt         | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/at803x.txt

diff --git a/Documentation/devicetree/bindings/net/at803x.txt b/Documentation/devicetree/bindings/net/at803x.txt
new file mode 100644
index 000000000000..931cb34534fb
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/at803x.txt
@@ -0,0 +1,18 @@
+* Atheros AR803x Integrated 10/100/1000 Mbps Ethernet Transceiver
+
+Required properties:
+	- reg - The ID number for the phy, usually a small integer
+
+Optional property:
+	- at803x,smarteee-disabled: Disable SmartEEE feature
+
+Example:
+
+	ethernet-phy@0 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <0>;
+		at803x,smarteee-disabled
+	};
+
+Datasheet can be found:
+https://media.digikey.com/pdf/Data%20Sheets/CSR%20PDFs/AR8031_DS_(Atheros)_Rev1.0_Aug2011.pdf
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] arm64: dts: imx8mq-evk: Disable SmartEEE

[Carlo Caione]

Disable SmartEEE mode (active by default) because this is known to cause
issues on certain PHYs.

Signed-off-by: Carlo Caione <ccaione@baylibre.com>
---

This patch depends on https://patchwork.kernel.org/patch/10781297/

---
 arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
index 02f6da92957f..f324fd1e78d0 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
@@ -48,6 +48,7 @@
 		ethphy0: ethernet-phy@0 {
 			compatible = "ethernet-phy-ieee802.3-c22";
 			reg = <0>;
+			at803x,smarteee-disabled;
 		};
 	};
 };
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Russell King - ARM Linux admin]

On Fri, Jan 25, 2019 at 12:55:10PM +0000, Carlo Caione wrote:
> SmartEEE, compatible with IEEE802.3az standard, is designed to include
> legacy MAC without EEE capability into the power saving system. SmartEEE 
> is enabled by default configuration on AR8031 after power-on or hardware 
> reset.
> 
> Unfortunately this is proved causing issues for certain hw 
> configurations.  We add a quirk to optionally disable SmartEEE.

It may be a good idea to detail what "certain hw configurations" are
and/or what the "issues" are, so it's easier to find this if/when
others encounter the same issue.

> 
> PATCH 3/3 depends on https://patchwork.kernel.org/patch/10781297/
> 
> 
> Carlo Caione (3):
>   net: phy: at803x: Introduce quirk to disable SmartEEE
>   dt-bindings: net: at803x: Document at803x,smarteee-disabled property
>   arm64: dts: imx8mq-evk: Disable SmartEEE
> 
>  .../devicetree/bindings/net/at803x.txt        | 18 +++++++++++++++
>  arch/arm64/boot/dts/freescale/imx8mq-evk.dts  |  1 +
>  drivers/net/phy/at803x.c                      | 22 +++++++++++++++++++
>  3 files changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/at803x.txt
> 
> -- 
> 2.19.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Heiner Kallweit]

On 25.01.2019 14:06, Russell King - ARM Linux admin wrote:
> On Fri, Jan 25, 2019 at 12:55:10PM +0000, Carlo Caione wrote:
>> SmartEEE, compatible with IEEE802.3az standard, is designed to include
>> legacy MAC without EEE capability into the power saving system. SmartEEE 
>> is enabled by default configuration on AR8031 after power-on or hardware 
>> reset.
>>
>> Unfortunately this is proved causing issues for certain hw 
>> configurations.  We add a quirk to optionally disable SmartEEE.
> 
> It may be a good idea to detail what "certain hw configurations" are
> and/or what the "issues" are, so it's easier to find this if/when
> others encounter the same issue.
> 
Question would also be whether the issue with SmartEEE occurs also
if EEE isn't advertized and therefore not active.
Then we could use the existing DT properties to flag broken EEE
modes, see of_set_phy_eee_broken().

>>
>> PATCH 3/3 depends on https://patchwork.kernel.org/patch/10781297/
>>
>>
>> Carlo Caione (3):
>>   net: phy: at803x: Introduce quirk to disable SmartEEE
>>   dt-bindings: net: at803x: Document at803x,smarteee-disabled property
>>   arm64: dts: imx8mq-evk: Disable SmartEEE
>>
>>  .../devicetree/bindings/net/at803x.txt        | 18 +++++++++++++++
>>  arch/arm64/boot/dts/freescale/imx8mq-evk.dts  |  1 +
>>  drivers/net/phy/at803x.c                      | 22 +++++++++++++++++++
>>  3 files changed, 41 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/at803x.txt
>>
>> -- 
>> 2.19.1
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] dt-bindings: net: at803x: Document at803x,smarteee-disabled property

[Florian Fainelli]

On 1/25/19 4:55 AM, Carlo Caione wrote:
> SmartEEE is a proprietary protocol that allows legacy MAC/SoC devices
> without 802.3az support to function as a complete 802.3az system.
> 
> This is know to cause issues so a new property is added to optionally
> disable this feature.
> 
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>
> ---
>  .../devicetree/bindings/net/at803x.txt         | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/at803x.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/at803x.txt b/Documentation/devicetree/bindings/net/at803x.txt
> new file mode 100644
> index 000000000000..931cb34534fb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/at803x.txt
> @@ -0,0 +1,18 @@
> +* Atheros AR803x Integrated 10/100/1000 Mbps Ethernet Transceiver
> +
> +Required properties:
> +	- reg - The ID number for the phy, usually a small integer
> +
> +Optional property:
> +	- at803x,smarteee-disabled: Disable SmartEEE feature

Should this be named at803x,smarteee-broken instead of "disabled"?
Because disabled is effectively what will happen after the driver looks
up that property and acts on it, but this does not really describe the
HW state unless there is appropriate software running.

Similar question to Heiner, how do SmartEEE and standard EEE relate to
each other? Can we use the existing broken-eee properties?

> +
> +Example:
> +
> +	ethernet-phy@0 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <0>;
> +		at803x,smarteee-disabled
> +	};
> +
> +Datasheet can be found:
> +https://media.digikey.com/pdf/Data%20Sheets/CSR%20PDFs/AR8031_DS_(Atheros)_Rev1.0_Aug2011.pdf
> 


-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Carlo Caione]

On 25/01/19 13:06, Russell King - ARM Linux admin wrote:
>On Fri, Jan 25, 2019 at 12:55:10PM +0000, Carlo Caione wrote:
>> SmartEEE, compatible with IEEE802.3az standard, is designed to include
>> legacy MAC without EEE capability into the power saving system. SmartEEE
>> is enabled by default configuration on AR8031 after power-on or hardware
>> reset.
>>
>> Unfortunately this is proved causing issues for certain hw
>> configurations.  We add a quirk to optionally disable SmartEEE.
>
>It may be a good idea to detail what "certain hw configurations" are
>and/or what the "issues" are, so it's easier to find this if/when
>others encounter the same issue.

This fix was upstreamed from the imx downstream kernel so I do not have 
much informations on that but something is reported in the U-Boot fix 
for the same problem at [0] so I'll add more info in the next revision.

Cheers,

[0] https://patchwork.ozlabs.org/patch/1015783/

-- 
Carlo Caione

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Heiner Kallweit]

On 25.01.2019 19:48, Carlo Caione wrote:
> On 25/01/19 13:06, Russell King - ARM Linux admin wrote:
>> On Fri, Jan 25, 2019 at 12:55:10PM +0000, Carlo Caione wrote:
>>> SmartEEE, compatible with IEEE802.3az standard, is designed to include
>>> legacy MAC without EEE capability into the power saving system. SmartEEE
>>> is enabled by default configuration on AR8031 after power-on or hardware
>>> reset.
>>>
>>> Unfortunately this is proved causing issues for certain hw
>>> configurations.  We add a quirk to optionally disable SmartEEE.
>>
>> It may be a good idea to detail what "certain hw configurations" are
>> and/or what the "issues" are, so it's easier to find this if/when
>> others encounter the same issue.
> 
> This fix was upstreamed from the imx downstream kernel so I do not have much informations on that but something is reported in the U-Boot fix for the same problem at [0] so I'll add more info in the next revision.
> 
> Cheers,
> 
> [0] https://patchwork.ozlabs.org/patch/1015783/
> 
Thanks for the link. According to the description SmartEEE works only if
both link partners support it (hence are Atheros PHY's). I would assume
that this usually isn't the case, shouldn't the feature be an opt-in
therefore?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Russell King - ARM Linux admin]

On Fri, Jan 25, 2019 at 08:07:56PM +0100, Heiner Kallweit wrote:
> On 25.01.2019 19:48, Carlo Caione wrote:
> > On 25/01/19 13:06, Russell King - ARM Linux admin wrote:
> >> On Fri, Jan 25, 2019 at 12:55:10PM +0000, Carlo Caione wrote:
> >>> SmartEEE, compatible with IEEE802.3az standard, is designed to include
> >>> legacy MAC without EEE capability into the power saving system. SmartEEE
> >>> is enabled by default configuration on AR8031 after power-on or hardware
> >>> reset.
> >>>
> >>> Unfortunately this is proved causing issues for certain hw
> >>> configurations.  We add a quirk to optionally disable SmartEEE.
> >>
> >> It may be a good idea to detail what "certain hw configurations" are
> >> and/or what the "issues" are, so it's easier to find this if/when
> >> others encounter the same issue.
> > 
> > This fix was upstreamed from the imx downstream kernel so I do not have much informations on that but something is reported in the U-Boot fix for the same problem at [0] so I'll add more info in the next revision.
> > 
> > Cheers,
> > 
> > [0] https://patchwork.ozlabs.org/patch/1015783/
> > 
> Thanks for the link. According to the description SmartEEE works only if
> both link partners support it (hence are Atheros PHY's). I would assume
> that this usually isn't the case, shouldn't the feature be an opt-in
> therefore?

I'm not convinced about that - I've been using AtherOS SmartEEE with
i.MX6 for quite some time (I have patches which allow me to read out
the stats via ethtool, and configure it - they're specific to the
FEC driver though.)  SmartEEE works just like EEE for me, and I
notice no problems.

I have i.MX6 boards connected to Netgear GS116 and similar switches,
and also Marvell DSA switches on other platforms.

This is why I said above about "more detail".  Then maybe folk would
know what to look for to see whether there is a problem.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Heiner Kallweit]

On 25.01.2019 20:19, Russell King - ARM Linux admin wrote:
> On Fri, Jan 25, 2019 at 08:07:56PM +0100, Heiner Kallweit wrote:
>> On 25.01.2019 19:48, Carlo Caione wrote:
>>> On 25/01/19 13:06, Russell King - ARM Linux admin wrote:
>>>> On Fri, Jan 25, 2019 at 12:55:10PM +0000, Carlo Caione wrote:
>>>>> SmartEEE, compatible with IEEE802.3az standard, is designed to include
>>>>> legacy MAC without EEE capability into the power saving system. SmartEEE
>>>>> is enabled by default configuration on AR8031 after power-on or hardware
>>>>> reset.
>>>>>
>>>>> Unfortunately this is proved causing issues for certain hw
>>>>> configurations.  We add a quirk to optionally disable SmartEEE.
>>>>
>>>> It may be a good idea to detail what "certain hw configurations" are
>>>> and/or what the "issues" are, so it's easier to find this if/when
>>>> others encounter the same issue.
>>>
>>> This fix was upstreamed from the imx downstream kernel so I do not have much informations on that but something is reported in the U-Boot fix for the same problem at [0] so I'll add more info in the next revision.
>>>
>>> Cheers,
>>>
>>> [0] https://patchwork.ozlabs.org/patch/1015783/
>>>
>> Thanks for the link. According to the description SmartEEE works only if
>> both link partners support it (hence are Atheros PHY's). I would assume
>> that this usually isn't the case, shouldn't the feature be an opt-in
>> therefore?
> 
> I'm not convinced about that - I've been using AtherOS SmartEEE with
> i.MX6 for quite some time (I have patches which allow me to read out
> the stats via ethtool, and configure it - they're specific to the
> FEC driver though.)  SmartEEE works just like EEE for me, and I
> notice no problems.
> 
> I have i.MX6 boards connected to Netgear GS116 and similar switches,
> and also Marvell DSA switches on other platforms.
> 
> This is why I said above about "more detail".  Then maybe folk would
> know what to look for to see whether there is a problem.
> 
OK, then the description of the referenced patch isn't fully correct
and SmartEEE and EEE are partially compatible, and the problem is
just that we don't know exactly to which extent.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Carlo Caione]

On 25/01/19 20:27, Heiner Kallweit wrote:

/cut
>>> Thanks for the link. According to the description SmartEEE works 
>>> only if
>>> both link partners support it (hence are Atheros PHY's). I would assume
>>> that this usually isn't the case, shouldn't the feature be an opt-in
>>> therefore?
>>
>> I'm not convinced about that - I've been using AtherOS SmartEEE with
>> i.MX6 for quite some time (I have patches which allow me to read out
>> the stats via ethtool, and configure it - they're specific to the
>> FEC driver though.)  SmartEEE works just like EEE for me, and I
>> notice no problems.
>>
>> I have i.MX6 boards connected to Netgear GS116 and similar switches,
>> and also Marvell DSA switches on other platforms.
>>
>> This is why I said above about "more detail".  Then maybe folk would
>> know what to look for to see whether there is a problem.
>>
>OK, then the description of the referenced patch isn't fully correct
>and SmartEEE and EEE are partially compatible, and the problem is
>just that we don't know exactly to which extent.

Reading from the datasheet at [0] it seems that SmartEEE is compatible 
with EEE but it's something different.

With SmartEEE the PHY can actually enter LPI state even if this is not 
supported by the link partner since the LPI pattern is generated inside 
the PHY itself, so auto-implementing a sort of EEE.

So AFAICT EEE and SmartEEE are two different things requiring two 
different properties in the DTS if we want to have the possibility to 
disable it.

Cheers,

[0] 
https://media.digikey.com/pdf/Data%20Sheets/CSR%20PDFs/AR8031_DS_(Atheros)_Rev1.0_Aug2011.pdf

-- 
Carlo Caione

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Russell King - ARM Linux admin]

On Mon, Jan 28, 2019 at 10:46:20AM +0000, Carlo Caione wrote:
> On 25/01/19 20:27, Heiner Kallweit wrote:
> > OK, then the description of the referenced patch isn't fully correct
> > and SmartEEE and EEE are partially compatible, and the problem is
> > just that we don't know exactly to which extent.
> 
> Reading from the datasheet at [0] it seems that SmartEEE is compatible with
> EEE but it's something different.

As I understand it, SmartEEE is just like normal EEE as far as the link
partner is concerned.  The difference is between the PHY and MAC.  What
follows is a simplified understanding of the differences.

In conventional EEE, the MAC takes part in EEE - the local MAC requests
that its attached PHY enters LPI mode, which signals to the link partner
PHY that the data stream on the link is going to pause.  When the local
MAC wants to transmit, it first has to signal to the attached PHY that
the link should be woken up, and the MAC has to wait for the link to
exit LPI before transmitting.

With SmartEEE, the decision to enter LPI mode is taken not by the local
MAC but by the PHY instead, since SmartEEE is designed to produce power
savings for MACs that do not support LPI.  Of course, they won't achieve
the same power saving as real EEE, but they do help to reduce the power
dissipation in the PHY.

PHYs that support this buffer some data from the MAC, and that buffering
has to be sufficient for the delay in the link coming out of LPI mode
without losing data.

As far as the link partner is concerned, EEE and SmartEEE appear the
same.

> With SmartEEE the PHY can actually enter LPI state even if this is not
> supported by the link partner since the LPI pattern is generated inside the
> PHY itself, so auto-implementing a sort of EEE.

It sounds to me like you have this the wrong way round.  SmartEEE isn't
about the link partner not supporting LPI, it's about the local MAC not
supporting EEE, but still getting some power savings.

EEE (of either kind) can only be entered on the link when both the
local PHY and remote PHY indicate support for EEE, and have negotiated
that they support EEE.

Most of the power dissipation is from driving the signals into the
network cable (which is a lossy environment) to ensure that the far
end has sufficient signal to keep the link established.  SmartEEE is
about giving a way to enter 802.3az EEE mode on the _cable_ with a
MAC that does not support EEE.

I've monitored the signals on a link with an Atheros AR8035 with
SmartEEE mode enabled, and a Marvell 88e1514 PHY in a Marvell switch
on the other end, and seen the signals on the cable quiesce as a
result of SmartEEE, but only when _both_ partners are set to allow
EEE in the negotiated link mode.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Heiner Kallweit]

On 28.01.2019 16:57, Russell King - ARM Linux admin wrote:
> On Mon, Jan 28, 2019 at 10:46:20AM +0000, Carlo Caione wrote:
>> On 25/01/19 20:27, Heiner Kallweit wrote:
>>> OK, then the description of the referenced patch isn't fully correct
>>> and SmartEEE and EEE are partially compatible, and the problem is
>>> just that we don't know exactly to which extent.
>>
>> Reading from the datasheet at [0] it seems that SmartEEE is compatible with
>> EEE but it's something different.
> 
> As I understand it, SmartEEE is just like normal EEE as far as the link
> partner is concerned.  The difference is between the PHY and MAC.  What
> follows is a simplified understanding of the differences.
> 
> In conventional EEE, the MAC takes part in EEE - the local MAC requests
> that its attached PHY enters LPI mode, which signals to the link partner
> PHY that the data stream on the link is going to pause.  When the local
> MAC wants to transmit, it first has to signal to the attached PHY that
> the link should be woken up, and the MAC has to wait for the link to
> exit LPI before transmitting.
> 
> With SmartEEE, the decision to enter LPI mode is taken not by the local
> MAC but by the PHY instead, since SmartEEE is designed to produce power
> savings for MACs that do not support LPI.  Of course, they won't achieve
> the same power saving as real EEE, but they do help to reduce the power
> dissipation in the PHY.
> 
> PHYs that support this buffer some data from the MAC, and that buffering
> has to be sufficient for the delay in the link coming out of LPI mode
> without losing data.
> 
> As far as the link partner is concerned, EEE and SmartEEE appear the
> same.
> 
>> With SmartEEE the PHY can actually enter LPI state even if this is not
>> supported by the link partner since the LPI pattern is generated inside the
>> PHY itself, so auto-implementing a sort of EEE.
> 
> It sounds to me like you have this the wrong way round.  SmartEEE isn't
> about the link partner not supporting LPI, it's about the local MAC not
> supporting EEE, but still getting some power savings.
> 
> EEE (of either kind) can only be entered on the link when both the
> local PHY and remote PHY indicate support for EEE, and have negotiated
> that they support EEE.
> 
> Most of the power dissipation is from driving the signals into the
> network cable (which is a lossy environment) to ensure that the far
> end has sufficient signal to keep the link established.  SmartEEE is
> about giving a way to enter 802.3az EEE mode on the _cable_ with a
> MAC that does not support EEE.
> 
> I've monitored the signals on a link with an Atheros AR8035 with
> SmartEEE mode enabled, and a Marvell 88e1514 PHY in a Marvell switch
> on the other end, and seen the signals on the cable quiesce as a
> result of SmartEEE, but only when _both_ partners are set to allow
> EEE in the negotiated link mode.
> 

If SmartEEE is compatible with EEE on a PHY level, then I'd expect that
advertisement of SmartEEE at different speeds can be controlled via the
standard EEE MMD regs. Is this the case?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Russell King - ARM Linux admin]

On Mon, Jan 28, 2019 at 07:23:49PM +0100, Heiner Kallweit wrote:
> On 28.01.2019 16:57, Russell King - ARM Linux admin wrote:
> > On Mon, Jan 28, 2019 at 10:46:20AM +0000, Carlo Caione wrote:
> >> On 25/01/19 20:27, Heiner Kallweit wrote:
> >>> OK, then the description of the referenced patch isn't fully correct
> >>> and SmartEEE and EEE are partially compatible, and the problem is
> >>> just that we don't know exactly to which extent.
> >>
> >> Reading from the datasheet at [0] it seems that SmartEEE is compatible with
> >> EEE but it's something different.
> > 
> > As I understand it, SmartEEE is just like normal EEE as far as the link
> > partner is concerned.  The difference is between the PHY and MAC.  What
> > follows is a simplified understanding of the differences.
> > 
> > In conventional EEE, the MAC takes part in EEE - the local MAC requests
> > that its attached PHY enters LPI mode, which signals to the link partner
> > PHY that the data stream on the link is going to pause.  When the local
> > MAC wants to transmit, it first has to signal to the attached PHY that
> > the link should be woken up, and the MAC has to wait for the link to
> > exit LPI before transmitting.
> > 
> > With SmartEEE, the decision to enter LPI mode is taken not by the local
> > MAC but by the PHY instead, since SmartEEE is designed to produce power
> > savings for MACs that do not support LPI.  Of course, they won't achieve
> > the same power saving as real EEE, but they do help to reduce the power
> > dissipation in the PHY.
> > 
> > PHYs that support this buffer some data from the MAC, and that buffering
> > has to be sufficient for the delay in the link coming out of LPI mode
> > without losing data.
> > 
> > As far as the link partner is concerned, EEE and SmartEEE appear the
> > same.
> > 
> >> With SmartEEE the PHY can actually enter LPI state even if this is not
> >> supported by the link partner since the LPI pattern is generated inside the
> >> PHY itself, so auto-implementing a sort of EEE.
> > 
> > It sounds to me like you have this the wrong way round.  SmartEEE isn't
> > about the link partner not supporting LPI, it's about the local MAC not
> > supporting EEE, but still getting some power savings.
> > 
> > EEE (of either kind) can only be entered on the link when both the
> > local PHY and remote PHY indicate support for EEE, and have negotiated
> > that they support EEE.
> > 
> > Most of the power dissipation is from driving the signals into the
> > network cable (which is a lossy environment) to ensure that the far
> > end has sufficient signal to keep the link established.  SmartEEE is
> > about giving a way to enter 802.3az EEE mode on the _cable_ with a
> > MAC that does not support EEE.
> > 
> > I've monitored the signals on a link with an Atheros AR8035 with
> > SmartEEE mode enabled, and a Marvell 88e1514 PHY in a Marvell switch
> > on the other end, and seen the signals on the cable quiesce as a
> > result of SmartEEE, but only when _both_ partners are set to allow
> > EEE in the negotiated link mode.
> > 
> 
> If SmartEEE is compatible with EEE on a PHY level, then I'd expect that
> advertisement of SmartEEE at different speeds can be controlled via the
> standard EEE MMD regs. Is this the case?

There is no "advertisement of SmartEEE" - it's just EEE.  That is
because as far as the link partner is concerned, SmartEEE is just
EEE.

Carlio posted a link to one of the datasheets for the family.  In
there, it describes the EEE capability register, which describes
what is supported, and the EEE wake error counter register.

It also describes the EEE advertisement and link parter advertisement
registers.

All these registers correspond to the 802.3 section 45 defined MMD
and address offsets found in Clause 45 compliant PHYs, and these
registers control not only EEE but also SmartEEE.

Please stop thinking that SmartEEE is different on the link partner
side from EEE - as far as the link partner is concerned, there is
no difference.  The difference is all to do with the MAC side of
the local PHY.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Carlo Caione]

On 28/01/2019 19:04, Russell King - ARM Linux admin wrote:

Hi Russell,

> There is no "advertisement of SmartEEE" - it's just EEE.  That is
> because as far as the link partner is concerned, SmartEEE is just
> EEE.
> 
> Carlio posted a link to one of the datasheets for the family.  In
> there, it describes the EEE capability register, which describes
> what is supported, and the EEE wake error counter register.
> 
> It also describes the EEE advertisement and link parter advertisement
> registers.
> 
> All these registers correspond to the 802.3 section 45 defined MMD
> and address offsets found in Clause 45 compliant PHYs, and these
> registers control not only EEE but also SmartEEE.
> 
> Please stop thinking that SmartEEE is different on the link partner
> side from EEE - as far as the link partner is concerned, there is
> no difference.  The difference is all to do with the MAC side of
> the local PHY.

Thank you for clarifying how the SmartEEE is really working.

Now, the problem is that the MMD registers controlling the EEE (7.3c and 
7.3d, touched by the "eee-broken-*" property) are not the same as the 
ones for the SmartEEE (3.805b, 3.805c, 3.805d). So, is it worth to add a 
new DT property to deal also with the cases where we want to selectively 
disable the SmartEEE?

Thanks,

--
Carlo Caione

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Russell King - ARM Linux admin]

On Wed, Jan 30, 2019 at 10:16:39AM +0000, Carlo Caione wrote:
> On 28/01/2019 19:04, Russell King - ARM Linux admin wrote:
> 
> Hi Russell,
> 
> > There is no "advertisement of SmartEEE" - it's just EEE.  That is
> > because as far as the link partner is concerned, SmartEEE is just
> > EEE.
> > 
> > Carlio posted a link to one of the datasheets for the family.  In
> > there, it describes the EEE capability register, which describes
> > what is supported, and the EEE wake error counter register.
> > 
> > It also describes the EEE advertisement and link parter advertisement
> > registers.
> > 
> > All these registers correspond to the 802.3 section 45 defined MMD
> > and address offsets found in Clause 45 compliant PHYs, and these
> > registers control not only EEE but also SmartEEE.
> > 
> > Please stop thinking that SmartEEE is different on the link partner
> > side from EEE - as far as the link partner is concerned, there is
> > no difference.  The difference is all to do with the MAC side of
> > the local PHY.
> 
> Thank you for clarifying how the SmartEEE is really working.
> 
> Now, the problem is that the MMD registers controlling the EEE (7.3c and
> 7.3d, touched by the "eee-broken-*" property) are not the same as the ones
> for the SmartEEE (3.805b, 3.805c, 3.805d). So, is it worth to add a new DT
> property to deal also with the cases where we want to selectively disable
> the SmartEEE?

That's because you're confusing what the registers are.  Please take
the time to read the data sheet for the AR803x devices, and the 802.3
specifications, and you will save yourself from asking a lot of
questions by email.

7.3c (or 7.60 in decimal) is the EEE advertisement register.  This is
  what gets sent _to_ the link partner.

7.3d (or 7.61 in decimal) is the EEE link partner ability register.
  This is what the link partner sent to us _from_ its own EEE
  advertisement register.

The eee-broken-* properties allow the EEE advertisement to be altered,
thereby preventing EEE being negotiated for the various link modes.
Disabling EEE for a particular link mode means that EEE (in any form)
will not operate while the link is operating at that speed.

3.805b, 3.805c, 3.805d in the AR803x family are specifically to do
with the SmartEEE implementation.

3.805b controls how much time between the link waking up and buffered
  data already received from the MAC being sent.
3.805c controls the time from the last activity to entering low power
  mode, additional bits in 3.805d.
3.805d contains the SmartEEE enable bit, as well as the additional bits
  from 3.805c.

Most of these registers are only useful when you have a MAC that has no
EEE functionality - that is where SmartEEE can be enabled to provide the
power savings, and in order to implement EEE, there are various timeouts
required by the protocol.  SmartEEE allows these timeouts to be
programmed via the above register.

When using a MAC that has EEE functionality, SmartEEE should be disabled
via 3.805d to allow _full_ 802.3az EEE (from local MAC to link partner)
to operate, rather than SmartEEE (from local PHY to link partner.)

Otherwise, using the existing "eee-broken-*" properties to disable the
link modes where EEE fails would be the correct way forward, and should
be used in preference to disabling SmartEEE.

However, no one has mentioned what the problem that is trying to be
addressed.  Is it data corruption?  Is it that the link fails?  Is it
lost packets?  Is it that the MAC supports EEE?  I think there needs to
be some better understanding of the problem at hand before trying to
address it.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] at803x: Add quirk to disable SmartEEE

[Vladimir Oltean]

On Jan. 30, 2019, 10:47 a.m., Russell King - ARM Linux admin wrote:
> On Wed, Jan 30, 2019 at 10:16:39AM +0000, Carlo Caione wrote:
> > On 28/01/2019 19:04, Russell King - ARM Linux admin wrote:
> > 
> > Hi Russell,
> > 
> > > There is no "advertisement of SmartEEE" - it's just EEE.  That is
> > > because as far as the link partner is concerned, SmartEEE is just
> > > EEE.
> > > 
> > > Carlio posted a link to one of the datasheets for the family.  In
> > > there, it describes the EEE capability register, which describes
> > > what is supported, and the EEE wake error counter register.
> > > 
> > > It also describes the EEE advertisement and link parter advertisement
> > > registers.
> > > 
> > > All these registers correspond to the 802.3 section 45 defined MMD
> > > and address offsets found in Clause 45 compliant PHYs, and these
> > > registers control not only EEE but also SmartEEE.
> > > 
> > > Please stop thinking that SmartEEE is different on the link partner
> > > side from EEE - as far as the link partner is concerned, there is
> > > no difference.  The difference is all to do with the MAC side of
> > > the local PHY.
> > 
> > Thank you for clarifying how the SmartEEE is really working.
> > 
> > Now, the problem is that the MMD registers controlling the EEE (7.3c and
> > 7.3d, touched by the "eee-broken-*" property) are not the same as the ones
> > for the SmartEEE (3.805b, 3.805c, 3.805d). So, is it worth to add a new DT
> > property to deal also with the cases where we want to selectively disable
> > the SmartEEE?
> 
> That's because you're confusing what the registers are.  Please take
> the time to read the data sheet for the AR803x devices, and the 802.3
> specifications, and you will save yourself from asking a lot of
> questions by email.
> 
> 7.3c (or 7.60 in decimal) is the EEE advertisement register.  This is
>   what gets sent _to_ the link partner.
> 
> 7.3d (or 7.61 in decimal) is the EEE link partner ability register.
>   This is what the link partner sent to us _from_ its own EEE
>   advertisement register.
> 
> The eee-broken-* properties allow the EEE advertisement to be altered,
> thereby preventing EEE being negotiated for the various link modes.
> Disabling EEE for a particular link mode means that EEE (in any form)
> will not operate while the link is operating at that speed.
> 
> 3.805b, 3.805c, 3.805d in the AR803x family are specifically to do
> with the SmartEEE implementation.
> 
> 3.805b controls how much time between the link waking up and buffered
>   data already received from the MAC being sent.
> 3.805c controls the time from the last activity to entering low power
>   mode, additional bits in 3.805d.
> 3.805d contains the SmartEEE enable bit, as well as the additional bits
>   from 3.805c.
> 
> Most of these registers are only useful when you have a MAC that has no
> EEE functionality - that is where SmartEEE can be enabled to provide the
> power savings, and in order to implement EEE, there are various timeouts
> required by the protocol.  SmartEEE allows these timeouts to be
> programmed via the above register.
> 
> When using a MAC that has EEE functionality, SmartEEE should be disabled
> via 3.805d to allow _full_ 802.3az EEE (from local MAC to link partner)
> to operate, rather than SmartEEE (from local PHY to link partner.)
> 
> Otherwise, using the existing "eee-broken-*" properties to disable the
> link modes where EEE fails would be the correct way forward, and should
> be used in preference to disabling SmartEEE.
> 
> However, no one has mentioned what the problem that is trying to be
> addressed.  Is it data corruption?  Is it that the link fails?  Is it
> lost packets?  Is it that the MAC supports EEE?  I think there needs to
> be some better understanding of the problem at hand before trying to
> address it.


Hi Russell, Heiner, Carlo, Florian,

You could have copied me when referencing the U-boot patch.

It is indeed correct that disabling regular EEE advertisement does also disable
SmartEEE. Somehow I hadn't taken this thought one step further to realize that
the regular eee-broken-1000t DT binding is enough to take care of this.

In my case it was indeed a situation of packet loss when the PHY should have
buffered it, and nobody debugged it to find the root cause. While true that the
Layerscape MACs in general need to disable EEE advertisement, in this
particular case I can't rule out an electrical issue on the PHY's voltage
rails. This is especially plausible since the MDIO interface of this PHY needed
some rework anyway, whereas the RGMII side presented no more packet loss after
disabling the PHY's low power mode.

Thank you,
-Vladimir


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel