Re: [PATCH] I.MX6: Fix Ethernet PHY mode on Ventana boards
On Tue, Dec 8, 2015 at 9:50 PM, Krzysztof Hałasa wrote: > David Miller writes: > >> Since this only touched DT files in the ARM platform code, maybe >> the ARM tree is the best path for this patch rather than mine? > > I think so. > > Tim, would you like to handle this patch yourself, or should I send it > to the ARM contacts? Krzysztof, Go ahead and send it. You can add my: Acked-by: Tim Harvey I know Shawn would be happy to pick this up as well as he has an imx-fixes tree: http://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/log/?h=imx/soc Tim -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] I.MX6: Fix Ethernet PHY mode on Ventana boards
David Miller writes: > Since this only touched DT files in the ARM platform code, maybe > the ARM tree is the best path for this patch rather than mine? I think so. Tim, would you like to handle this patch yourself, or should I send it to the ARM contacts? -- Krzysztof Halasa Industrial Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] I.MX6: Fix Ethernet PHY mode on Ventana boards
From: khal...@piap.pl (Krzysztof Hałasa) Date: Mon, 07 Dec 2015 13:56:23 +0100 > Gateworks Ventana boards seem to need "RGMII-ID" (internal delay) > PHY mode, instead of simple "RGMII", for their Marvell 88E1510 > transceiver. Otherwise, the Ethernet MAC doesn't work with Marvell PHY > driver (TX doesn't seem to work correctly). > > Tested on GW5400 rev. C. > > This bug affects ARM Fedora 23. > > Signed-off-by: Krzysztof Hałasa Since this only touched DT files in the ARM platform code, maybe the ARM tree is the best path for this patch rather than mine? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] I.MX6: Fix Ethernet PHY mode on Ventana boards
> This may be a stupid question but what does the Marvell phy driver > offer that the generic phy driver does not? It reliably configures the RGMII delays :-) No need to depend on your bootloader doing it. It allows interrupts to be used instead of polling the PHY all the time, assuming you have the interrupt wired up. There are peculiarities for enabling autoneg which is knows about. It might put the LEDs into a well known state, but that depends on the model. It might enable auto-crossover on some PHYs. It has some errata workarounds, support for WOL. You can put a list of register writes in DT which will be performed when auto neg is enabled. This could be used for example to configure the LEDs. Not yet posted, but i have a patch to add HWMON support to read the temperature sensor in some of the PHYs. Andrew -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] I.MX6: Fix Ethernet PHY mode on Ventana boards
On Tue, Dec 8, 2015 at 6:13 AM, Krzysztof Hałasa wrote:
> Tim Harvey writes:
>
>> It sounds like your saying this controls whether the phy is in charge
>> of delay vs the MAC. I have never needed to set this and haven't found
>> where its actually used (in at least 4.3). Is this caused by something
>> new in the kernel I haven't seen yet or is it possible you have board
>> that has an Ethernet issue?
>
> I think you aren't using the Marvell PHY driver :-)
> The existing DTS files work fine with the "default" PHY driver.
> It's only the Marvell driver (drivers/net/phy/marvell.c, aka
> CONFIG_MARVELL_PHY) which have the issue.
>
Kryzsztof,
True - I have not used CONFIG_MARVELL_PHY and didn't know of its existance.
> The problem is caused by the following code in m88e1121_config_aneg():
>
> if (phy_interface_is_rgmii(phydev)) {
>
> mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG) &
> MII_88E1121_PHY_MSCR_DELAY_MASK;
>
> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
> mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
> MII_88E1121_PHY_MSCR_TX_DELAY);
> else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> mscr |= MII_88E1121_PHY_MSCR_RX_DELAY;
> else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
> mscr |= MII_88E1121_PHY_MSCR_TX_DELAY;
>
> err = phy_write(phydev, MII_88E1121_PHY_MSCR_REG, mscr);
> if (err < 0)
> return err;
> }
>
> mscr initially holds the correct value (one with RX_DELAY and TX_DELAY
> bits set). Since the _*ID are not specified, these bits are reset and
> (now incorrect) value is written back to the register.
> OTOH the generic PHY driver knows nothing about the MSCR and thus
> doesn't "corrupt" it.
>
> The whole -ID thing is about signal trace lengths (on PCB), where the
> non-ID setup requires special, longer traces for certain signals to
> compensate for propagation times etc. The -ID version does it internally
> (when enabled) and the PCB layout can be a bit simpler.
right - I understand what the delay lines are, it just struck me as
strange that I've never needed this.
>
> To be honest I only checked it on a single GW5400 rev. C, but I'd expect
> it to be exactly the same across all GW5[234]*. Can test on GW5[23] if
> there is any doubt (in few days I guess).
>
no doubt now. I re-created your config and see the problem as well.
The marvell phy driver (drivers/net/phy/marvell.c) has a very lacking
Kconfig stating its a PHY driver for the 88E1011S when in fact it has
id's for 12 ID's including the 88E1510 we use on the GW51xx, GW52xx,
GW53xx, GW54xx.
This may be a stupid question but what does the Marvell phy driver
offer that the generic phy driver does not?
Thanks for finding this and submitting a patch!
Acked-by: Tim Harvey
Tim
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] I.MX6: Fix Ethernet PHY mode on Ventana boards
Tim Harvey writes:
> It sounds like your saying this controls whether the phy is in charge
> of delay vs the MAC. I have never needed to set this and haven't found
> where its actually used (in at least 4.3). Is this caused by something
> new in the kernel I haven't seen yet or is it possible you have board
> that has an Ethernet issue?
I think you aren't using the Marvell PHY driver :-)
The existing DTS files work fine with the "default" PHY driver.
It's only the Marvell driver (drivers/net/phy/marvell.c, aka
CONFIG_MARVELL_PHY) which have the issue.
The problem is caused by the following code in m88e1121_config_aneg():
if (phy_interface_is_rgmii(phydev)) {
mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG) &
MII_88E1121_PHY_MSCR_DELAY_MASK;
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
MII_88E1121_PHY_MSCR_TX_DELAY);
else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
mscr |= MII_88E1121_PHY_MSCR_RX_DELAY;
else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
mscr |= MII_88E1121_PHY_MSCR_TX_DELAY;
err = phy_write(phydev, MII_88E1121_PHY_MSCR_REG, mscr);
if (err < 0)
return err;
}
mscr initially holds the correct value (one with RX_DELAY and TX_DELAY
bits set). Since the _*ID are not specified, these bits are reset and
(now incorrect) value is written back to the register.
OTOH the generic PHY driver knows nothing about the MSCR and thus
doesn't "corrupt" it.
The whole -ID thing is about signal trace lengths (on PCB), where the
non-ID setup requires special, longer traces for certain signals to
compensate for propagation times etc. The -ID version does it internally
(when enabled) and the PCB layout can be a bit simpler.
To be honest I only checked it on a single GW5400 rev. C, but I'd expect
it to be exactly the same across all GW5[234]*. Can test on GW5[23] if
there is any doubt (in few days I guess).
This issue doesn't show itself on Yocto and OpenWrt, because they
apparently don't enable marvell PHY driver. Fedora 23 does and it
doesn't work (this probably means it could be worth it to push it to
stable).
--
Krzysztof Halasa
Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] I.MX6: Fix Ethernet PHY mode on Ventana boards
On Mon, Dec 7, 2015 at 4:56 AM, Krzysztof Hałasa wrote:
> Gateworks Ventana boards seem to need "RGMII-ID" (internal delay)
> PHY mode, instead of simple "RGMII", for their Marvell 88E1510
> transceiver. Otherwise, the Ethernet MAC doesn't work with Marvell PHY
> driver (TX doesn't seem to work correctly).
>
> Tested on GW5400 rev. C.
>
> This bug affects ARM Fedora 23.
>
> Signed-off-by: Krzysztof Hałasa
>
> diff --git a/arch/arm/boot/dts/imx6q-gw5400-a.dts
> b/arch/arm/boot/dts/imx6q-gw5400-a.dts
> index 822ffb2..6c168dc 100644
> --- a/arch/arm/boot/dts/imx6q-gw5400-a.dts
> +++ b/arch/arm/boot/dts/imx6q-gw5400-a.dts
> @@ -154,7 +154,7 @@
> &fec {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_enet>;
> - phy-mode = "rgmii";
> + phy-mode = "rgmii-id";
> phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_HIGH>;
> status = "okay";
> };
> diff --git a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
> b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
> index f2867c4..90496aa 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
> @@ -94,7 +94,7 @@
> &fec {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_enet>;
> - phy-mode = "rgmii";
> + phy-mode = "rgmii-id";
> phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
> status = "okay";
> };
> diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
> b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
> index 4493f6e..0a6730b 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
> @@ -154,7 +154,7 @@
> &fec {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_enet>;
> - phy-mode = "rgmii";
> + phy-mode = "rgmii-id";
> phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
> status = "okay";
> };
> diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
> b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
> index cfad214..2c549ad 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
> @@ -174,7 +174,7 @@
> &fec {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_enet>;
> - phy-mode = "rgmii";
> + phy-mode = "rgmii-id";
> phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
> status = "okay";
> };
> diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
> b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
> index 535b536..b4ea087 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
> @@ -164,7 +164,7 @@
> &fec {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_enet>;
> - phy-mode = "rgmii";
> + phy-mode = "rgmii-id";
> phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
> status = "okay";
> };
>
> --
> Krzysztof Halasa
>
> Industrial Research Institute for Automation and Measurements PIAP
> Al. Jerozolimskie 202, 02-486 Warsaw, Poland
Krzysztof,
It sounds like your saying this controls whether the phy is in charge
of delay vs the MAC. I have never needed to set this and haven't found
where its actually used (in at least 4.3). Is this caused by something
new in the kernel I haven't seen yet or is it possible you have board
that has an Ethernet issue?
Regards,
Tim
Tim Harvey - Principal Software Engineer
Gateworks Corporation - http://www.gateworks.com/
3026 S. Higuera St. San Luis Obispo CA 93401
805-781-2000
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] I.MX6: Fix Ethernet PHY mode on Ventana boards
On Mon, Dec 7, 2015 at 10:56 AM, Krzysztof Hałasa wrote: > Gateworks Ventana boards seem to need "RGMII-ID" (internal delay) > PHY mode, instead of simple "RGMII", for their Marvell 88E1510 > transceiver. Otherwise, the Ethernet MAC doesn't work with Marvell PHY > driver (TX doesn't seem to work correctly). > > Tested on GW5400 rev. C. > > This bug affects ARM Fedora 23. > > Signed-off-by: Krzysztof Hałasa It seems you missed to Cc the i.MX maintainer, Shawn Guo. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] I.MX6: Fix Ethernet PHY mode on Ventana boards
Gateworks Ventana boards seem to need "RGMII-ID" (internal delay)
PHY mode, instead of simple "RGMII", for their Marvell 88E1510
transceiver. Otherwise, the Ethernet MAC doesn't work with Marvell PHY
driver (TX doesn't seem to work correctly).
Tested on GW5400 rev. C.
This bug affects ARM Fedora 23.
Signed-off-by: Krzysztof Hałasa
diff --git a/arch/arm/boot/dts/imx6q-gw5400-a.dts
b/arch/arm/boot/dts/imx6q-gw5400-a.dts
index 822ffb2..6c168dc 100644
--- a/arch/arm/boot/dts/imx6q-gw5400-a.dts
+++ b/arch/arm/boot/dts/imx6q-gw5400-a.dts
@@ -154,7 +154,7 @@
&fec {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_enet>;
- phy-mode = "rgmii";
+ phy-mode = "rgmii-id";
phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_HIGH>;
status = "okay";
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
index f2867c4..90496aa 100644
--- a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
@@ -94,7 +94,7 @@
&fec {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_enet>;
- phy-mode = "rgmii";
+ phy-mode = "rgmii-id";
phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
status = "okay";
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
index 4493f6e..0a6730b 100644
--- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
@@ -154,7 +154,7 @@
&fec {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_enet>;
- phy-mode = "rgmii";
+ phy-mode = "rgmii-id";
phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
status = "okay";
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
index cfad214..2c549ad 100644
--- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
@@ -174,7 +174,7 @@
&fec {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_enet>;
- phy-mode = "rgmii";
+ phy-mode = "rgmii-id";
phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
status = "okay";
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
index 535b536..b4ea087 100644
--- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
@@ -164,7 +164,7 @@
&fec {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_enet>;
- phy-mode = "rgmii";
+ phy-mode = "rgmii-id";
phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
status = "okay";
};
--
Krzysztof Halasa
Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
