This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
The platform seems to enter LPI on the Rx path too often while performing
relatively high TX transfer. This eventually break the link (both Tx and
Rx), and require to bring the interface down and up again to get the Rx
path working again.
The root cause of this issue is not fully understood yet but disabling EEE
advertisement on the PHY prevent this feature to be negotiated.
With this change, the link is stable and reliable, with the expected
throughput performance.
The patchset adds options in the generic phy driver to disable EEE
advertisement, through device tree. The way it is done is very similar
to the handling of the max-speed property.
Changes since V2: [2]
- Rename "eee-advert-disable" to "eee-broken-modes" to make the intended
purpose of this option clear (flag broken configuration, not a
configuration option)
- Add DT bindings constants so the DT configuration is more user friendly
- Submit to net-next instead of net.
Changes since V1: [1]
- Disable the advertisement of EEE in the generic code instead of the
realtek driver.
[1] : http://lkml.kernel.org/r/[email protected]
[2] : http://lkml.kernel.org/r/[email protected]
Jerome Brunet (4):
net: phy: add an option to disable EEE advertisement
dt-bindings: net: add EEE capability constants
dt: bindings: add ethernet phy eee-broken-modes option documentation
ARM64: dts: meson: odroidc2: disable advertisement EEE for GbE.
Documentation/devicetree/bindings/net/phy.txt | 2 +
.../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 16 +++++
drivers/net/phy/phy.c | 3 +
drivers/net/phy/phy_device.c | 80 +++++++++++++++++++---
include/dt-bindings/net/mdio.h | 19 +++++
include/linux/phy.h | 3 +
6 files changed, 114 insertions(+), 9 deletions(-)
create mode 100644 include/dt-bindings/net/mdio.h
--
2.7.4
Signed-off-by: Jerome Brunet <[email protected]>
---
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index e6e3491d48a5..5624714d2b16 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -46,6 +46,7 @@
#include "meson-gxbb.dtsi"
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/net/mdio.h>
/ {
compatible = "hardkernel,odroid-c2", "amlogic,meson-gxbb";
@@ -98,3 +99,18 @@
pinctrl-0 = <&i2c_a_pins>;
pinctrl-names = "default";
};
+
+ðmac {
+ phy-handle = <ð_phy0>;
+
+ mdio {
+ compatible = "snps,dwmac-mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ eth_phy0: ethernet-phy@0 {
+ reg = <0>;
+ eee-broken-modes = <MDIO_EEE_1000T>;
+ };
+ };
+};
--
2.7.4
This patch adds an option to disable EEE advertisement in the generic PHY
by providing a mask of prohibited modes corresponding to the value found in
the MDIO_AN_EEE_ADV register.
On some platforms, PHY Low power idle seems to be causing issues, even
breaking the link some cases. The patch provides a convenient way for these
platforms to disable EEE advertisement and work around the issue.
Signed-off-by: Jerome Brunet <[email protected]>
---
drivers/net/phy/phy.c | 3 ++
drivers/net/phy/phy_device.c | 80 +++++++++++++++++++++++++++++++++++++++-----
include/linux/phy.h | 3 ++
3 files changed, 77 insertions(+), 9 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 73adbaa9ac86..a3981cc6448a 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1396,6 +1396,9 @@ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
{
int val = ethtool_adv_to_mmd_eee_adv_t(data->advertised);
+ /* Mask prohibited EEE modes */
+ val &= ~phydev->eee_broken_modes;
+
phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN, val);
return 0;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index ba86c191a13e..83e52f1b80f2 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1121,6 +1121,43 @@ static int genphy_config_advert(struct phy_device *phydev)
}
/**
+ * genphy_config_eee_advert - disable unwanted eee mode advertisement
+ * @phydev: target phy_device struct
+ *
+ * Description: Writes MDIO_AN_EEE_ADV after disabling unsupported energy
+ * efficent ethernet modes. Returns 0 if the PHY's advertisement hasn't
+ * changed, and 1 if it has changed.
+ */
+static int genphy_config_eee_advert(struct phy_device *phydev)
+{
+ u32 broken = phydev->eee_broken_modes;
+ u32 old_adv, adv;
+
+ /* Nothing to disable */
+ if (!broken)
+ return 0;
+
+ /* If the following call fails, we assume that EEE is not
+ * supported by the phy. If we read 0, EEE is not advertised
+ * In both case, we don't need to continue
+ */
+ adv = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN);
+ if (adv <= 0)
+ return 0;
+
+ old_adv = adv;
+ adv &= ~broken;
+
+ /* Advertising remains unchanged with the broken mask */
+ if (old_adv == adv)
+ return 0;
+
+ phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN, adv);
+
+ return 1;
+}
+
+/**
* genphy_setup_forced - configures/forces speed/duplex from @phydev
* @phydev: target phy_device struct
*
@@ -1178,15 +1215,20 @@ EXPORT_SYMBOL(genphy_restart_aneg);
*/
int genphy_config_aneg(struct phy_device *phydev)
{
- int result;
+ int err, changed;
+
+ changed = genphy_config_eee_advert(phydev);
if (AUTONEG_ENABLE != phydev->autoneg)
return genphy_setup_forced(phydev);
- result = genphy_config_advert(phydev);
- if (result < 0) /* error */
- return result;
- if (result == 0) {
+ err = genphy_config_advert(phydev);
+ if (err < 0) /* error */
+ return err;
+
+ changed |= err;
+
+ if (changed == 0) {
/* Advertisement hasn't changed, but maybe aneg was never on to
* begin with? Or maybe phy was isolated?
*/
@@ -1196,16 +1238,16 @@ int genphy_config_aneg(struct phy_device *phydev)
return ctl;
if (!(ctl & BMCR_ANENABLE) || (ctl & BMCR_ISOLATE))
- result = 1; /* do restart aneg */
+ changed = 1; /* do restart aneg */
}
/* Only restart aneg if we are advertising something different
* than we were before.
*/
- if (result > 0)
- result = genphy_restart_aneg(phydev);
+ if (changed > 0)
+ return genphy_restart_aneg(phydev);
- return result;
+ return 0;
}
EXPORT_SYMBOL(genphy_config_aneg);
@@ -1563,6 +1605,21 @@ static void of_set_phy_supported(struct phy_device *phydev)
__set_phy_supported(phydev, max_speed);
}
+static void of_set_phy_eee_broken(struct phy_device *phydev)
+{
+ struct device_node *node = phydev->mdio.dev.of_node;
+ u32 broken;
+
+ if (!IS_ENABLED(CONFIG_OF_MDIO))
+ return;
+
+ if (!node)
+ return;
+
+ if (!of_property_read_u32(node, "eee-broken-modes", &broken))
+ phydev->eee_broken_modes = broken;
+}
+
/**
* phy_probe - probe and init a PHY device
* @dev: device to probe and init
@@ -1600,6 +1657,11 @@ static int phy_probe(struct device *dev)
of_set_phy_supported(phydev);
phydev->advertising = phydev->supported;
+ /* Get the EEE modes we want to prohibit. We will ask
+ * the PHY stop advertising these mode later on
+ */
+ of_set_phy_eee_broken(phydev);
+
/* Set the state to READY by default */
phydev->state = PHY_READY;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index edde28ce163a..b53177fd38af 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -417,6 +417,9 @@ struct phy_device {
u32 advertising;
u32 lp_advertising;
+ /* Energy efficient ethernet modes which should be prohibited */
+ u32 eee_broken_modes;
+
int autoneg;
int link_timeout;
--
2.7.4
Signed-off-by: Jerome Brunet <[email protected]>
---
Documentation/devicetree/bindings/net/phy.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index 4627da3d52c4..54749b60a466 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -38,6 +38,8 @@ Optional Properties:
- enet-phy-lane-swap: If set, indicates the PHY will swap the TX/RX lanes to
compensate for the board being designed with the lanes swapped.
+- eee-broken-modes: Bits to clear in the MDIO_AN_EEE_ADV register to
+ disable EEE broken modes.
Example:
--
2.7.4
Signed-off-by: Jerome Brunet <[email protected]>
---
include/dt-bindings/net/mdio.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 include/dt-bindings/net/mdio.h
diff --git a/include/dt-bindings/net/mdio.h b/include/dt-bindings/net/mdio.h
new file mode 100644
index 000000000000..99c6d903d439
--- /dev/null
+++ b/include/dt-bindings/net/mdio.h
@@ -0,0 +1,19 @@
+/*
+ * This header provides generic constants for ethernet MDIO bindings
+ */
+
+#ifndef _DT_BINDINGS_NET_MDIO_H
+#define _DT_BINDINGS_NET_MDIO_H
+
+/*
+ * EEE capability Advertisement
+ */
+
+#define MDIO_EEE_100TX 0x0002 /* 100TX EEE cap */
+#define MDIO_EEE_1000T 0x0004 /* 1000T EEE cap */
+#define MDIO_EEE_10GT 0x0008 /* 10GT EEE cap */
+#define MDIO_EEE_1000KX 0x0010 /* 1000KX EEE cap */
+#define MDIO_EEE_10GKX4 0x0020 /* 10G KX4 EEE cap */
+#define MDIO_EEE_10GKR 0x0040 /* 10G KR EEE cap */
+
+#endif
--
2.7.4
On Mon, Nov 28, 2016 at 10:46 AM, Jerome Brunet <[email protected]> wrote:
> Signed-off-by: Jerome Brunet <[email protected]>
Tested using Baltos ir2110 device (cpsw + at8035 PHY).
Tested-by: Yegor Yefremov <[email protected]>
> ---
> include/dt-bindings/net/mdio.h | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 include/dt-bindings/net/mdio.h
>
> diff --git a/include/dt-bindings/net/mdio.h b/include/dt-bindings/net/mdio.h
> new file mode 100644
> index 000000000000..99c6d903d439
> --- /dev/null
> +++ b/include/dt-bindings/net/mdio.h
> @@ -0,0 +1,19 @@
> +/*
> + * This header provides generic constants for ethernet MDIO bindings
> + */
> +
> +#ifndef _DT_BINDINGS_NET_MDIO_H
> +#define _DT_BINDINGS_NET_MDIO_H
> +
> +/*
> + * EEE capability Advertisement
> + */
> +
> +#define MDIO_EEE_100TX 0x0002 /* 100TX EEE cap */
> +#define MDIO_EEE_1000T 0x0004 /* 1000T EEE cap */
> +#define MDIO_EEE_10GT 0x0008 /* 10GT EEE cap */
> +#define MDIO_EEE_1000KX 0x0010 /* 1000KX EEE cap */
> +#define MDIO_EEE_10GKX4 0x0020 /* 10G KX4 EEE cap */
> +#define MDIO_EEE_10GKR 0x0040 /* 10G KR EEE cap */
> +
> +#endif
> --
> 2.7.4
>
On Mon, Nov 28, 2016 at 10:46 AM, Jerome Brunet <[email protected]> wrote:
> This patch adds an option to disable EEE advertisement in the generic PHY
> by providing a mask of prohibited modes corresponding to the value found in
> the MDIO_AN_EEE_ADV register.
>
> On some platforms, PHY Low power idle seems to be causing issues, even
> breaking the link some cases. The patch provides a convenient way for these
> platforms to disable EEE advertisement and work around the issue.
>
> Signed-off-by: Jerome Brunet <[email protected]>
Tested using Baltos ir2110 device (cpsw + at8035 PHY).
Tested-by: Yegor Yefremov <[email protected]>
> ---
> drivers/net/phy/phy.c | 3 ++
> drivers/net/phy/phy_device.c | 80 +++++++++++++++++++++++++++++++++++++++-----
> include/linux/phy.h | 3 ++
> 3 files changed, 77 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 73adbaa9ac86..a3981cc6448a 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1396,6 +1396,9 @@ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
> {
> int val = ethtool_adv_to_mmd_eee_adv_t(data->advertised);
>
> + /* Mask prohibited EEE modes */
> + val &= ~phydev->eee_broken_modes;
> +
> phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN, val);
>
> return 0;
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index ba86c191a13e..83e52f1b80f2 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1121,6 +1121,43 @@ static int genphy_config_advert(struct phy_device *phydev)
> }
>
> /**
> + * genphy_config_eee_advert - disable unwanted eee mode advertisement
> + * @phydev: target phy_device struct
> + *
> + * Description: Writes MDIO_AN_EEE_ADV after disabling unsupported energy
> + * efficent ethernet modes. Returns 0 if the PHY's advertisement hasn't
> + * changed, and 1 if it has changed.
> + */
> +static int genphy_config_eee_advert(struct phy_device *phydev)
> +{
> + u32 broken = phydev->eee_broken_modes;
> + u32 old_adv, adv;
> +
> + /* Nothing to disable */
> + if (!broken)
> + return 0;
> +
> + /* If the following call fails, we assume that EEE is not
> + * supported by the phy. If we read 0, EEE is not advertised
> + * In both case, we don't need to continue
> + */
> + adv = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN);
> + if (adv <= 0)
> + return 0;
> +
> + old_adv = adv;
> + adv &= ~broken;
> +
> + /* Advertising remains unchanged with the broken mask */
> + if (old_adv == adv)
> + return 0;
> +
> + phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN, adv);
> +
> + return 1;
> +}
> +
> +/**
> * genphy_setup_forced - configures/forces speed/duplex from @phydev
> * @phydev: target phy_device struct
> *
> @@ -1178,15 +1215,20 @@ EXPORT_SYMBOL(genphy_restart_aneg);
> */
> int genphy_config_aneg(struct phy_device *phydev)
> {
> - int result;
> + int err, changed;
> +
> + changed = genphy_config_eee_advert(phydev);
>
> if (AUTONEG_ENABLE != phydev->autoneg)
> return genphy_setup_forced(phydev);
>
> - result = genphy_config_advert(phydev);
> - if (result < 0) /* error */
> - return result;
> - if (result == 0) {
> + err = genphy_config_advert(phydev);
> + if (err < 0) /* error */
> + return err;
> +
> + changed |= err;
> +
> + if (changed == 0) {
> /* Advertisement hasn't changed, but maybe aneg was never on to
> * begin with? Or maybe phy was isolated?
> */
> @@ -1196,16 +1238,16 @@ int genphy_config_aneg(struct phy_device *phydev)
> return ctl;
>
> if (!(ctl & BMCR_ANENABLE) || (ctl & BMCR_ISOLATE))
> - result = 1; /* do restart aneg */
> + changed = 1; /* do restart aneg */
> }
>
> /* Only restart aneg if we are advertising something different
> * than we were before.
> */
> - if (result > 0)
> - result = genphy_restart_aneg(phydev);
> + if (changed > 0)
> + return genphy_restart_aneg(phydev);
>
> - return result;
> + return 0;
> }
> EXPORT_SYMBOL(genphy_config_aneg);
>
> @@ -1563,6 +1605,21 @@ static void of_set_phy_supported(struct phy_device *phydev)
> __set_phy_supported(phydev, max_speed);
> }
>
> +static void of_set_phy_eee_broken(struct phy_device *phydev)
> +{
> + struct device_node *node = phydev->mdio.dev.of_node;
> + u32 broken;
> +
> + if (!IS_ENABLED(CONFIG_OF_MDIO))
> + return;
> +
> + if (!node)
> + return;
> +
> + if (!of_property_read_u32(node, "eee-broken-modes", &broken))
> + phydev->eee_broken_modes = broken;
> +}
> +
> /**
> * phy_probe - probe and init a PHY device
> * @dev: device to probe and init
> @@ -1600,6 +1657,11 @@ static int phy_probe(struct device *dev)
> of_set_phy_supported(phydev);
> phydev->advertising = phydev->supported;
>
> + /* Get the EEE modes we want to prohibit. We will ask
> + * the PHY stop advertising these mode later on
> + */
> + of_set_phy_eee_broken(phydev);
> +
> /* Set the state to READY by default */
> phydev->state = PHY_READY;
>
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index edde28ce163a..b53177fd38af 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -417,6 +417,9 @@ struct phy_device {
> u32 advertising;
> u32 lp_advertising;
>
> + /* Energy efficient ethernet modes which should be prohibited */
> + u32 eee_broken_modes;
> +
> int autoneg;
>
> int link_timeout;
> --
> 2.7.4
>
Am 28.11.2016 um 10:46 schrieb Jerome Brunet:
> This patch adds an option to disable EEE advertisement in the generic PHY
> by providing a mask of prohibited modes corresponding to the value found in
> the MDIO_AN_EEE_ADV register.
>
> On some platforms, PHY Low power idle seems to be causing issues, even
> breaking the link some cases. The patch provides a convenient way for these
> platforms to disable EEE advertisement and work around the issue.
>
> Signed-off-by: Jerome Brunet <[email protected]>
> ---
> drivers/net/phy/phy.c | 3 ++
> drivers/net/phy/phy_device.c | 80 +++++++++++++++++++++++++++++++++++++++-----
> include/linux/phy.h | 3 ++
> 3 files changed, 77 insertions(+), 9 deletions(-)
Tested-by: Andreas F?rber <[email protected]>
With this and the corresponding .dts change, Odroid-C2 survived a
317-package zypper update for me.
Thanks,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Am 28.11.2016 um 10:46 schrieb Jerome Brunet:
> Signed-off-by: Jerome Brunet <[email protected]>
> ---
> include/dt-bindings/net/mdio.h | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 include/dt-bindings/net/mdio.h
Tested-by: Andreas F?rber <[email protected]>
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Am 28.11.2016 um 10:46 schrieb Jerome Brunet:
> Signed-off-by: Jerome Brunet <[email protected]>
> ---
> Documentation/devicetree/bindings/net/phy.txt | 2 ++
> 1 file changed, 2 insertions(+)
Reviewed-by: Andreas F?rber <[email protected]>
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Am 28.11.2016 um 10:46 schrieb Jerome Brunet:
> Signed-off-by: Jerome Brunet <[email protected]>
> ---
> arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> index e6e3491d48a5..5624714d2b16 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> @@ -46,6 +46,7 @@
>
> #include "meson-gxbb.dtsi"
> #include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/net/mdio.h>
>
> / {
> compatible = "hardkernel,odroid-c2", "amlogic,meson-gxbb";
> @@ -98,3 +99,18 @@
> pinctrl-0 = <&i2c_a_pins>;
> pinctrl-names = "default";
> };
> +
> +ðmac {
> + phy-handle = <ð_phy0>;
> +
> + mdio {
> + compatible = "snps,dwmac-mdio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + eth_phy0: ethernet-phy@0 {
> + reg = <0>;
> + eee-broken-modes = <MDIO_EEE_1000T>;
> + };
> + };
> +};
I've tested this hand-applied because it applies to neither amlogic
v4.10/integ nor linux-next.git and will conflict if applied through the
net-next tree.
Note that there already is an ðmac node that you should be extending
rather than duplicating:
ðmac {
status = "okay";
pinctrl-0 = <ð_rgmii_pins>;
pinctrl-names = "default";
};
If you or your colleagues could please fix the sort order of the nodes
to be alphabetical again (ethmac after i2c_A here; between uart_A and ir
in-tree) this wouldn't happen so easily again.
I therefore suggest to not apply this patch 4/4 through net-next but
through the amlogic tree instead.
Thanks,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
On Mon, 2016-11-28 at 13:31 +0100, Andreas Färber wrote:
> Am 28.11.2016 um 10:46 schrieb Jerome Brunet:
> >
> > Signed-off-by: Jerome Brunet <[email protected]>
> > ---
> > arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 16
> > ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> > b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> > index e6e3491d48a5..5624714d2b16 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> > @@ -46,6 +46,7 @@
> >
> > #include "meson-gxbb.dtsi"
> > #include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/net/mdio.h>
> >
> > / {
> > compatible = "hardkernel,odroid-c2", "amlogic,meson-gxbb";
> > @@ -98,3 +99,18 @@
> > pinctrl-0 = <&i2c_a_pins>;
> > pinctrl-names = "default";
> > };
> > +
> > +ðmac {
> > + phy-handle = <ð_phy0>;
> > +
> > + mdio {
> > + compatible = "snps,dwmac-mdio";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + eth_phy0: ethernet-phy@0 {
> > + reg = <0>;
> > + eee-broken-modes = <MDIO_EEE_1000T>;
> > + };
> > + };
> > +};
>
6I've tested this hand-applied because it applies to neither amlogic
> v4.10/integ nor linux-next.git and will conflict if applied through
> the
> net-next tree.
I've rebased on net-next this morning. I just checked again and now
there is a conflict indeed. Something got applied between in the last 6
ours which conflict with patch 4.
>
> Note that there already is an ðmac node that you should be
> extending
> rather than duplicating:
>
> ðmac {
> status = "okay";
> pinctrl-0 = <ð_rgmii_pins>;
> pinctrl-names = "default";
> };
>
> If you or your colleagues could please fix the sort order of the
> nodes
> to be alphabetical again (ethmac after i2c_A here; between uart_A and
> ir
> in-tree) this wouldn't happen so easily again.
OK
>
> I therefore suggest to not apply this patch 4/4 through net-next but
> through the amlogic tree instead.
Agreed. The change is provided here so people can test.
If the other patches get accepted, I'll submit the dts change through
the amlogic tree.
>
> Thanks,
> Andreas
>
On 11/28/2016 10:46 AM, Jerome Brunet wrote:
> This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
> The platform seems to enter LPI on the Rx path too often while performing
> relatively high TX transfer. This eventually break the link (both Tx and
> Rx), and require to bring the interface down and up again to get the Rx
> path working again.
>
> The root cause of this issue is not fully understood yet but disabling EEE
> advertisement on the PHY prevent this feature to be negotiated.
> With this change, the link is stable and reliable, with the expected
> throughput performance.
>
> The patchset adds options in the generic phy driver to disable EEE
> advertisement, through device tree. The way it is done is very similar
> to the handling of the max-speed property.
>
> Changes since V2: [2]
> - Rename "eee-advert-disable" to "eee-broken-modes" to make the intended
> purpose of this option clear (flag broken configuration, not a
> configuration option)
> - Add DT bindings constants so the DT configuration is more user friendly
> - Submit to net-next instead of net.
>
> Changes since V1: [1]
> - Disable the advertisement of EEE in the generic code instead of the
> realtek driver.
>
> [1] : http://lkml.kernel.org/r/[email protected]
> [2] : http://lkml.kernel.org/r/[email protected]
>
>
> Jerome Brunet (4):
> net: phy: add an option to disable EEE advertisement
> dt-bindings: net: add EEE capability constants
> dt: bindings: add ethernet phy eee-broken-modes option documentation
> ARM64: dts: meson: odroidc2: disable advertisement EEE for GbE.
>
> Documentation/devicetree/bindings/net/phy.txt | 2 +
> .../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 16 +++++
> drivers/net/phy/phy.c | 3 +
> drivers/net/phy/phy_device.c | 80 +++++++++++++++++++---
> include/dt-bindings/net/mdio.h | 19 +++++
> include/linux/phy.h | 3 +
> 6 files changed, 114 insertions(+), 9 deletions(-)
> create mode 100644 include/dt-bindings/net/mdio.h
>
Tested using Nexbox A1 (S912) and Amlogic P230 (S905D) devices (DWMAC + RTL8211F).
Tested-by: Neil Armstrong <[email protected]>
From: Jerome Brunet <[email protected]>
Date: Mon, 28 Nov 2016 10:46:45 +0100
> This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
> The platform seems to enter LPI on the Rx path too often while performing
> relatively high TX transfer. This eventually break the link (both Tx and
> Rx), and require to bring the interface down and up again to get the Rx
> path working again.
>
> The root cause of this issue is not fully understood yet but disabling EEE
> advertisement on the PHY prevent this feature to be negotiated.
> With this change, the link is stable and reliable, with the expected
> throughput performance.
>
> The patchset adds options in the generic phy driver to disable EEE
> advertisement, through device tree. The way it is done is very similar
> to the handling of the max-speed property.
Patches 1-3 applied to net-next, thanks.
On 11/29/2016 04:38 PM, David Miller wrote:
> From: Jerome Brunet <[email protected]>
> Date: Mon, 28 Nov 2016 10:46:45 +0100
>
>> This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
>> The platform seems to enter LPI on the Rx path too often while performing
>> relatively high TX transfer. This eventually break the link (both Tx and
>> Rx), and require to bring the interface down and up again to get the Rx
>> path working again.
>>
>> The root cause of this issue is not fully understood yet but disabling EEE
>> advertisement on the PHY prevent this feature to be negotiated.
>> With this change, the link is stable and reliable, with the expected
>> throughput performance.
>>
>> The patchset adds options in the generic phy driver to disable EEE
>> advertisement, through device tree. The way it is done is very similar
>> to the handling of the max-speed property.
>
> Patches 1-3 applied to net-next, thanks.
Meh, there was a v4 submitted shortly after, and I objected to the whole
idea of using that kind of Device Tree properties to disable EEE, we can
send reverts though..
--
Florian
From: Florian Fainelli <[email protected]>
Date: Tue, 29 Nov 2016 16:43:20 -0800
> On 11/29/2016 04:38 PM, David Miller wrote:
>> From: Jerome Brunet <[email protected]>
>> Date: Mon, 28 Nov 2016 10:46:45 +0100
>>
>>> This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
>>> The platform seems to enter LPI on the Rx path too often while performing
>>> relatively high TX transfer. This eventually break the link (both Tx and
>>> Rx), and require to bring the interface down and up again to get the Rx
>>> path working again.
>>>
>>> The root cause of this issue is not fully understood yet but disabling EEE
>>> advertisement on the PHY prevent this feature to be negotiated.
>>> With this change, the link is stable and reliable, with the expected
>>> throughput performance.
>>>
>>> The patchset adds options in the generic phy driver to disable EEE
>>> advertisement, through device tree. The way it is done is very similar
>>> to the handling of the max-speed property.
>>
>> Patches 1-3 applied to net-next, thanks.
>
> Meh, there was a v4 submitted shortly after, and I objected to the whole
> idea of using that kind of Device Tree properties to disable EEE, we can
> send reverts though..
Sorry, I lost this in all the discussion, I can revert.
Just send me a revert of the entire merge commit
a152c91889556df17ca6d8ea134fb2cb4ac9f893 with a short
description of why and I'll apply it.
Thanks.
On 11/29/2016 05:13 PM, David Miller wrote:
> From: Florian Fainelli <[email protected]>
> Date: Tue, 29 Nov 2016 16:43:20 -0800
>
>> On 11/29/2016 04:38 PM, David Miller wrote:
>>> From: Jerome Brunet <[email protected]>
>>> Date: Mon, 28 Nov 2016 10:46:45 +0100
>>>
>>>> This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
>>>> The platform seems to enter LPI on the Rx path too often while performing
>>>> relatively high TX transfer. This eventually break the link (both Tx and
>>>> Rx), and require to bring the interface down and up again to get the Rx
>>>> path working again.
>>>>
>>>> The root cause of this issue is not fully understood yet but disabling EEE
>>>> advertisement on the PHY prevent this feature to be negotiated.
>>>> With this change, the link is stable and reliable, with the expected
>>>> throughput performance.
>>>>
>>>> The patchset adds options in the generic phy driver to disable EEE
>>>> advertisement, through device tree. The way it is done is very similar
>>>> to the handling of the max-speed property.
>>>
>>> Patches 1-3 applied to net-next, thanks.
>>
>> Meh, there was a v4 submitted shortly after, and I objected to the whole
>> idea of using that kind of Device Tree properties to disable EEE, we can
>> send reverts though..
>
> Sorry, I lost this in all the discussion, I can revert.
Yeah, I can understand why, these freaking PHYs tend to generate a lot
of noise and discussion...
>
> Just send me a revert of the entire merge commit
> a152c91889556df17ca6d8ea134fb2cb4ac9f893 with a short
> description of why and I'll apply it.
OK, I will talk with Jerome first to make sure that we are in agreement
with the solution to deploy to fix the OdroidC2 problem first.
Thanks!
--
Florian
Hi Florian, Hi Jerome,
On Wed, Nov 30, 2016 at 2:15 AM, Florian Fainelli <[email protected]> wrote:
> On 11/29/2016 05:13 PM, David Miller wrote:
>> From: Florian Fainelli <[email protected]>
>> Date: Tue, 29 Nov 2016 16:43:20 -0800
>>
>>> On 11/29/2016 04:38 PM, David Miller wrote:
>>>> From: Jerome Brunet <[email protected]>
>>>> Date: Mon, 28 Nov 2016 10:46:45 +0100
>>>>
>>>>> This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
>>>>> The platform seems to enter LPI on the Rx path too often while performing
>>>>> relatively high TX transfer. This eventually break the link (both Tx and
>>>>> Rx), and require to bring the interface down and up again to get the Rx
>>>>> path working again.
>>>>>
>>>>> The root cause of this issue is not fully understood yet but disabling EEE
>>>>> advertisement on the PHY prevent this feature to be negotiated.
>>>>> With this change, the link is stable and reliable, with the expected
>>>>> throughput performance.
>>>>>
>>>>> The patchset adds options in the generic phy driver to disable EEE
>>>>> advertisement, through device tree. The way it is done is very similar
>>>>> to the handling of the max-speed property.
>>>>
>>>> Patches 1-3 applied to net-next, thanks.
>>>
>>> Meh, there was a v4 submitted shortly after, and I objected to the whole
>>> idea of using that kind of Device Tree properties to disable EEE, we can
>>> send reverts though..
>>
>> Sorry, I lost this in all the discussion, I can revert.
>
> Yeah, I can understand why, these freaking PHYs tend to generate a lot
> of noise and discussion...
>
>>
>> Just send me a revert of the entire merge commit
>> a152c91889556df17ca6d8ea134fb2cb4ac9f893 with a short
>> description of why and I'll apply it.
>
> OK, I will talk with Jerome first to make sure that we are in agreement
> with the solution to deploy to fix the OdroidC2 problem first.
simply because I'm curious: what was the outcome of your discussion?
can we stay with the current solution or are any changes required?
Regards,
Martin