[U-Boot] [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal delay on Tx

[U-Boot] [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal delay on Tx

[York Sun]

On 04/04/2017 04:44 AM, Madalin Bucur wrote:
> Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
> ---
>   drivers/net/fm/ls1046.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/fm/ls1046.c b/drivers/net/fm/ls1046.c
> index bf55554..6c91fb9 100644
> --- a/drivers/net/fm/ls1046.c
> +++ b/drivers/net/fm/ls1046.c
> @@ -72,12 +72,12 @@ phy_interface_t fman_port_enet_if(enum fm_port port)
>   	if (port == FM1_DTSEC3)
>   		if ((rcwsr13 & FSL_CHASSIS2_RCWSR13_EC1) ==
>   				FSL_CHASSIS2_RCWSR13_EC1_DTSEC3_RGMII)
> -			return PHY_INTERFACE_MODE_RGMII;
> +			return PHY_INTERFACE_MODE_RGMII_TXID;
>   
>   	if (port == FM1_DTSEC4)
>   		if ((rcwsr13 & FSL_CHASSIS2_RCWSR13_EC2) ==
>   				FSL_CHASSIS2_RCWSR13_EC2_DTSEC4_RGMII)
> -			return PHY_INTERFACE_MODE_RGMII;
> +			return PHY_INTERFACE_MODE_RGMII_TXID;
>   
>   	/* handle SGMII, only MAC 2/5/6/9/10 available */
>   	switch (port) {
> 

I don't know what test you have run. This change breaks network on 
LS1043ARDB FM1 at DTSEC3.

York

[U-Boot] [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal delay on Tx

[Madalin-cristian Bucur]

> -----Original Message-----
> From: York Sun
> Sent: Wednesday, August 02, 2017 11:36 PM
> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>; u-boot at lists.denx.de;
> joe.hershberger at ni.com
> Cc: Mingkai Hu <mingkai.hu@nxp.com>
> Subject: Re: [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal delay
> on Tx
> 
> On 04/04/2017 04:44 AM, Madalin Bucur wrote:
> > Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
> > ---
> >   drivers/net/fm/ls1046.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/fm/ls1046.c b/drivers/net/fm/ls1046.c
> > index bf55554..6c91fb9 100644
> > --- a/drivers/net/fm/ls1046.c
> > +++ b/drivers/net/fm/ls1046.c
> > @@ -72,12 +72,12 @@ phy_interface_t fman_port_enet_if(enum fm_port port)
> >   	if (port == FM1_DTSEC3)
> >   		if ((rcwsr13 & FSL_CHASSIS2_RCWSR13_EC1) ==
> >   				FSL_CHASSIS2_RCWSR13_EC1_DTSEC3_RGMII)
> > -			return PHY_INTERFACE_MODE_RGMII;
> > +			return PHY_INTERFACE_MODE_RGMII_TXID;
> >
> >   	if (port == FM1_DTSEC4)
> >   		if ((rcwsr13 & FSL_CHASSIS2_RCWSR13_EC2) ==
> >   				FSL_CHASSIS2_RCWSR13_EC2_DTSEC4_RGMII)
> > -			return PHY_INTERFACE_MODE_RGMII;
> > +			return PHY_INTERFACE_MODE_RGMII_TXID;
> >
> >   	/* handle SGMII, only MAC 2/5/6/9/10 available */
> >   	switch (port) {
> >
> 
> I don't know what test you have run. This change breaks network on
> LS1043ARDB FM1 at DTSEC3.
> 
> York

Hi York,

You're likely using an old kernel, if you move to one that has removed the
hardcoded internal delay for RGMII [1] you'll notice that RGMII no longer
works with the old u-boot and you need this patch. We're in this situation
because someone decided to hardcode internal delay for RGMII mode [2] in the
Linux RTL8211F PHY driver. Someone else hardcoded the PHY fixup in u-boot to
RGMII instead of RGMII_TXID [3] to align to that initial mistake. The issue
with this u-boot fixup is that it overwrites device tree information, so one
cannot correct this from the dts.

Regards,
Madalin

[1] Linux kernel commit that removes hardcoded Tx internal delay for RTL8211F

commit e3230494b57ece68750e3e32d3e53d6b00917058
Author: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date:   Fri Nov 25 14:12:01 2016 +0100

    net: phy: realtek: fix enabling of the TX-delay for RTL8211F

    The old logic always enabled the TX-delay when the phy-mode was set to
    PHY_INTERFACE_MODE_RGMII. There are dedicated phy-modes which tell the
    PHY driver to enable the RX and/or TX delays:
    - PHY_INTERFACE_MODE_RGMII should disable the RX and TX delay in the
      PHY (if required, the MAC should add the delays in this case)
    - PHY_INTERFACE_MODE_RGMII_ID should enable RX and TX delay in the PHY
    - PHY_INTERFACE_MODE_RGMII_TXID should enable the TX delay in the PHY
    - PHY_INTERFACE_MODE_RGMII_RXID should enable the RX delay in the PHY
      (currently not supported by RTL8211F)

    With this patch we enable the TX delay for PHY_INTERFACE_MODE_RGMII_ID
    and PHY_INTERFACE_MODE_RGMII_TXID.
    Additionally we now explicity disable the TX-delay, which seems to be
    enabled automatically after a hard-reset of the PHY (by triggering it's
    reset pin) to get a consistent state (as defined by the phy-mode).

    This fixes a compatibility problem with some SoCs where the TX-delay was
    also added by the MAC. With the TX-delay being applied twice the TX
    clock was off and TX traffic was broken or very slow (<10Mbit/s) on
    1000Mbit/s links.

    Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
    Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

[2] Linux patch that hardcoded Tx ID for RGMII on RTL8211F

commit 3447cf2e9a11fd672de8b3cc613eabd97fe82053
Author: Shengzhou Liu <Shengzhou.Liu@freescale.com>
Date:   Thu Jun 18 16:42:47 2015 +0800

    net/phy: Add support for Realtek RTL8211F

    RTL8211F has different register definitions from RTL8211E.
    Specially it needs to enable TXDLY in case of RGMII.

    Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

[3] u-boot commit that hardcoded RGMII instead of the required RGMII_TXID

commit 9d3b8bd166e6c23ab83d0dcd49d81c6f0bf39e17
Author: Mingkai Hu <mingkai.hu@nxp.com>
Date:   Tue Jul 5 16:01:56 2016 +0800

    drivers: net/fm: Add Fman support for LS1046A

    The Fman module on LS1046A is similiar with that on LS1043A but
    LS1046A has one more XFI (10GbE) interface.

    Signed-off-by: Shaohui Xie <Shaohui.Xie@nxp.com>
    Signed-off-by: Mingkai Hu <mingkai.hu@nxp.com>
    Signed-off-by: Gong Qianyu <Qianyu.Gong@nxp.com>
    Reviewed-by: York Sun <york.sun@nxp.com>

[U-Boot] [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal delay on Tx

[York Sun]

On 08/02/2017 11:01 PM, Madalin-cristian Bucur wrote:
>> -----Original Message-----
>> From: York Sun
>> Sent: Wednesday, August 02, 2017 11:36 PM
>> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>; u-boot at lists.denx.de;
>> joe.hershberger at ni.com
>> Cc: Mingkai Hu <mingkai.hu@nxp.com>
>> Subject: Re: [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal delay
>> on Tx
>>
>> On 04/04/2017 04:44 AM, Madalin Bucur wrote:
>>> Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
>>> ---
>>>    drivers/net/fm/ls1046.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/fm/ls1046.c b/drivers/net/fm/ls1046.c
>>> index bf55554..6c91fb9 100644
>>> --- a/drivers/net/fm/ls1046.c
>>> +++ b/drivers/net/fm/ls1046.c
>>> @@ -72,12 +72,12 @@ phy_interface_t fman_port_enet_if(enum fm_port port)
>>>    	if (port == FM1_DTSEC3)
>>>    		if ((rcwsr13 & FSL_CHASSIS2_RCWSR13_EC1) ==
>>>    				FSL_CHASSIS2_RCWSR13_EC1_DTSEC3_RGMII)
>>> -			return PHY_INTERFACE_MODE_RGMII;
>>> +			return PHY_INTERFACE_MODE_RGMII_TXID;
>>>
>>>    	if (port == FM1_DTSEC4)
>>>    		if ((rcwsr13 & FSL_CHASSIS2_RCWSR13_EC2) ==
>>>    				FSL_CHASSIS2_RCWSR13_EC2_DTSEC4_RGMII)
>>> -			return PHY_INTERFACE_MODE_RGMII;
>>> +			return PHY_INTERFACE_MODE_RGMII_TXID;
>>>
>>>    	/* handle SGMII, only MAC 2/5/6/9/10 available */
>>>    	switch (port) {
>>>
>>
>> I don't know what test you have run. This change breaks network on
>> LS1043ARDB FM1 at DTSEC3.
>>
>> York
> 
> Hi York,
> 
> You're likely using an old kernel, if you move to one that has removed the
> hardcoded internal delay for RGMII [1] you'll notice that RGMII no longer
> works with the old u-boot and you need this patch. We're in this situation
> because someone decided to hardcode internal delay for RGMII mode [2] in the
> Linux RTL8211F PHY driver. Someone else hardcoded the PHY fixup in u-boot to
> RGMII instead of RGMII_TXID [3] to align to that initial mistake. The issue
> with this u-boot fixup is that it overwrites device tree information, so one
> cannot correct this from the dts.
> 

Madalin,

There may be some gap. With this change, network doesn't work under U-Boot.

York

[U-Boot] [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal delay on Tx

[Madalin-cristian Bucur]

> -----Original Message-----
> From: York Sun
> Sent: Thursday, August 03, 2017 7:14 PM
> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>; u-boot at lists.denx.de;
> joe.hershberger at ni.com
> Cc: Mingkai Hu <mingkai.hu@nxp.com>; Shengzhou Liu
> <shengzhou.liu@nxp.com>; martin.blumenstingl at googlemail.com
> Subject: Re: [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal delay
> on Tx
> 
> On 08/02/2017 11:01 PM, Madalin-cristian Bucur wrote:
> >> -----Original Message-----
> >> From: York Sun
> >> Sent: Wednesday, August 02, 2017 11:36 PM
> >> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>; u-
> boot at lists.denx.de;
> >> joe.hershberger at ni.com
> >> Cc: Mingkai Hu <mingkai.hu@nxp.com>
> >> Subject: Re: [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal
> delay
> >> on Tx
> >>
> >> On 04/04/2017 04:44 AM, Madalin Bucur wrote:
> >>> Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
> >>> ---
> >>>    drivers/net/fm/ls1046.c | 4 ++--
> >>>    1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/net/fm/ls1046.c b/drivers/net/fm/ls1046.c
> >>> index bf55554..6c91fb9 100644
> >>> --- a/drivers/net/fm/ls1046.c
> >>> +++ b/drivers/net/fm/ls1046.c
> >>> @@ -72,12 +72,12 @@ phy_interface_t fman_port_enet_if(enum fm_port
> port)
> >>>    	if (port == FM1_DTSEC3)
> >>>    		if ((rcwsr13 & FSL_CHASSIS2_RCWSR13_EC1) ==
> >>>    				FSL_CHASSIS2_RCWSR13_EC1_DTSEC3_RGMII)
> >>> -			return PHY_INTERFACE_MODE_RGMII;
> >>> +			return PHY_INTERFACE_MODE_RGMII_TXID;
> >>>
> >>>    	if (port == FM1_DTSEC4)
> >>>    		if ((rcwsr13 & FSL_CHASSIS2_RCWSR13_EC2) ==
> >>>    				FSL_CHASSIS2_RCWSR13_EC2_DTSEC4_RGMII)
> >>> -			return PHY_INTERFACE_MODE_RGMII;
> >>> +			return PHY_INTERFACE_MODE_RGMII_TXID;
> >>>
> >>>    	/* handle SGMII, only MAC 2/5/6/9/10 available */
> >>>    	switch (port) {
> >>>
> >>
> >> I don't know what test you have run. This change breaks network on
> >> LS1043ARDB FM1 at DTSEC3.
> >>
> >> York
> >
> > Hi York,
> >
> > You're likely using an old kernel, if you move to one that has removed
> the
> > hardcoded internal delay for RGMII [1] you'll notice that RGMII no
> longer
> > works with the old u-boot and you need this patch. We're in this
> situation
> > because someone decided to hardcode internal delay for RGMII mode [2] in
> the
> > Linux RTL8211F PHY driver. Someone else hardcoded the PHY fixup in u-
> boot to
> > RGMII instead of RGMII_TXID [3] to align to that initial mistake. The
> issue
> > with this u-boot fixup is that it overwrites device tree information, so
> one
> > cannot correct this from the dts.
> >
> 
> Madalin,
> 
> There may be some gap. With this change, network doesn't work under U-
> Boot.
> 
> York

It solved the Linux issue but it is incomplete for u-boot as the RGMII_TXID is
not supported by memac. I've sent a patch for that too.

Madalin

[U-Boot] [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal delay on Tx

[York Sun]

On 08/03/2017 11:34 PM, Madalin-cristian Bucur wrote:
>> -----Original Message-----
>> From: York Sun
>> Sent: Thursday, August 03, 2017 7:14 PM
>> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>; u-boot at lists.denx.de;
>> joe.hershberger at ni.com
>> Cc: Mingkai Hu <mingkai.hu@nxp.com>; Shengzhou Liu
>> <shengzhou.liu@nxp.com>; martin.blumenstingl at googlemail.com
>> Subject: Re: [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal delay
>> on Tx
>>
>> On 08/02/2017 11:01 PM, Madalin-cristian Bucur wrote:
>>>> -----Original Message-----
>>>> From: York Sun
>>>> Sent: Wednesday, August 02, 2017 11:36 PM
>>>> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>; u-
>> boot at lists.denx.de;
>>>> joe.hershberger at ni.com
>>>> Cc: Mingkai Hu <mingkai.hu@nxp.com>
>>>> Subject: Re: [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal
>> delay
>>>> on Tx
>>>>
>>>> On 04/04/2017 04:44 AM, Madalin Bucur wrote:
>>>>> Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
>>>>> ---
>>>>>     drivers/net/fm/ls1046.c | 4 ++--
>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/fm/ls1046.c b/drivers/net/fm/ls1046.c
>>>>> index bf55554..6c91fb9 100644
>>>>> --- a/drivers/net/fm/ls1046.c
>>>>> +++ b/drivers/net/fm/ls1046.c
>>>>> @@ -72,12 +72,12 @@ phy_interface_t fman_port_enet_if(enum fm_port
>> port)
>>>>>     	if (port == FM1_DTSEC3)
>>>>>     		if ((rcwsr13 & FSL_CHASSIS2_RCWSR13_EC1) ==
>>>>>     				FSL_CHASSIS2_RCWSR13_EC1_DTSEC3_RGMII)
>>>>> -			return PHY_INTERFACE_MODE_RGMII;
>>>>> +			return PHY_INTERFACE_MODE_RGMII_TXID;
>>>>>
>>>>>     	if (port == FM1_DTSEC4)
>>>>>     		if ((rcwsr13 & FSL_CHASSIS2_RCWSR13_EC2) ==
>>>>>     				FSL_CHASSIS2_RCWSR13_EC2_DTSEC4_RGMII)
>>>>> -			return PHY_INTERFACE_MODE_RGMII;
>>>>> +			return PHY_INTERFACE_MODE_RGMII_TXID;
>>>>>
>>>>>     	/* handle SGMII, only MAC 2/5/6/9/10 available */
>>>>>     	switch (port) {
>>>>>
>>>>
>>>> I don't know what test you have run. This change breaks network on
>>>> LS1043ARDB FM1 at DTSEC3.
>>>>
>>>> York
>>>
>>> Hi York,
>>>
>>> You're likely using an old kernel, if you move to one that has removed
>> the
>>> hardcoded internal delay for RGMII [1] you'll notice that RGMII no
>> longer
>>> works with the old u-boot and you need this patch. We're in this
>> situation
>>> because someone decided to hardcode internal delay for RGMII mode [2] in
>> the
>>> Linux RTL8211F PHY driver. Someone else hardcoded the PHY fixup in u-
>> boot to
>>> RGMII instead of RGMII_TXID [3] to align to that initial mistake. The
>> issue
>>> with this u-boot fixup is that it overwrites device tree information, so
>> one
>>> cannot correct this from the dts.
>>>
>>
>> Madalin,
>>
>> There may be some gap. With this change, network doesn't work under U-
>> Boot.
>>
>> York
> 
> It solved the Linux issue but it is incomplete for u-boot as the RGMII_TXID is
> not supported by memac. I've sent a patch for that too.
> 

Thank you. I will try it.

York

[U-Boot] [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal delay on Tx

[Joe Hershberger]

On Mon, Apr 3, 2017 at 9:43 AM, Madalin Bucur <madalin.bucur@nxp.com> wrote:
> Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 2/2] armv8/ls1046a: RGMII PHY requires internal delay on Tx

[Madalin Bucur]

Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
---
 drivers/net/fm/ls1046.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/fm/ls1046.c b/drivers/net/fm/ls1046.c
index bf55554..6c91fb9 100644
--- a/drivers/net/fm/ls1046.c
+++ b/drivers/net/fm/ls1046.c
@@ -72,12 +72,12 @@ phy_interface_t fman_port_enet_if(enum fm_port port)
 	if (port == FM1_DTSEC3)
 		if ((rcwsr13 & FSL_CHASSIS2_RCWSR13_EC1) ==
 				FSL_CHASSIS2_RCWSR13_EC1_DTSEC3_RGMII)
-			return PHY_INTERFACE_MODE_RGMII;
+			return PHY_INTERFACE_MODE_RGMII_TXID;
 
 	if (port == FM1_DTSEC4)
 		if ((rcwsr13 & FSL_CHASSIS2_RCWSR13_EC2) ==
 				FSL_CHASSIS2_RCWSR13_EC2_DTSEC4_RGMII)
-			return PHY_INTERFACE_MODE_RGMII;
+			return PHY_INTERFACE_MODE_RGMII_TXID;
 
 	/* handle SGMII, only MAC 2/5/6/9/10 available */
 	switch (port) {
-- 
2.1.0