diff mbox

[U-Boot] eth: dtsec: fix TBI ANA setting bug in dtsec_configure_serdes()

Message ID 1448435067-16438-1-git-send-email-liyuanzheng01@163.com
State Not Applicable
Delegated to: York Sun
Headers show

Commit Message

Yuanzheng Li Nov. 25, 2015, 7:04 a.m. UTC 
The TBI_ANA register is configurated with the wrong value 0x4001, refer
to QorIQ Data Path Acceleration Architecture (DPAA) Reference Manual.
It set the reserved areas, bit 1 and bit 11 to bit 15 in big endian,
which should be cleared. But the normal functions of the auto-negotiation,
e.g. Pause and Full Duplex, do not be set.

There is no problem in the p2041rdb board, because the ppc is connected
directly with the phy chip which support auto-negotiation by default in
SGMII interface. But the link problem will occur when it is connected with
a switch chip like BCM5389, the switch chip disable auto-negotiation by
default, and the ppc also disable auto-negotiation, then there is no link
between them.

So use the vlue TBIANA_SETTINGS to enable the ppc's auto-negotiation.

Signed-off-by: Yuanzheng Li <liyuanzheng01@163.com>
Cc: York Sun <yorksun@freescale.com>
---
 drivers/net/fm/eth.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

York Sun Nov. 25, 2015, 5:02 p.m. UTC | #1 
+ Experts

On 11/24/2015 11:04 PM, Yuanzheng Li wrote:
> The TBI_ANA register is configurated with the wrong value 0x4001, refer
> to QorIQ Data Path Acceleration Architecture (DPAA) Reference Manual.
> It set the reserved areas, bit 1 and bit 11 to bit 15 in big endian,
> which should be cleared. But the normal functions of the auto-negotiation,
> e.g. Pause and Full Duplex, do not be set.
> 
> There is no problem in the p2041rdb board, because the ppc is connected
> directly with the phy chip which support auto-negotiation by default in
> SGMII interface. But the link problem will occur when it is connected with
> a switch chip like BCM5389, the switch chip disable auto-negotiation by
> default, and the ppc also disable auto-negotiation, then there is no link
> between them.
> 
> So use the vlue TBIANA_SETTINGS to enable the ppc's auto-negotiation.
> 
> Signed-off-by: Yuanzheng Li <liyuanzheng01@163.com>
> Cc: York Sun <yorksun@freescale.com>
> ---
>  drivers/net/fm/eth.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c
> index eb8e936..78c0988 100644
> --- a/drivers/net/fm/eth.c
> +++ b/drivers/net/fm/eth.c
> @@ -81,7 +81,7 @@ qsgmii_loop:
>  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0, TBI_TBICON,
>  			TBICON_CLK_SELECT);
>  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0, TBI_ANA,
> -			TBIANA_SGMII_ACK);
> +			TBIANA_SETTINGS);
>  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0,
>  			TBI_CR, TBICR_SETTINGS);
>  #endif
> 

Yuanzheng,

After your change, TBIANA_SGMII_ACK is not used and should be removed.

Shaohui and Codrin,

Please comment.

York
shaohui xie Nov. 26, 2015, 4:17 a.m. UTC | #2 
> -----Original Message-----
> From: York Sun [mailto:yorksun@freescale.com]
> Sent: Thursday, November 26, 2015 1:03 AM
> To: Xie Shaohui-B21989; Ciubotariu Codrin Constantin-B43658
> Cc: Yuanzheng Li; u-boot@lists.denx.de; Liu Dave-R63238
> Subject: Re: [PATCH] eth: dtsec: fix TBI ANA setting bug in
> dtsec_configure_serdes()
> 
> + Experts
> 
> On 11/24/2015 11:04 PM, Yuanzheng Li wrote:
> > The TBI_ANA register is configurated with the wrong value 0x4001,
> > refer to QorIQ Data Path Acceleration Architecture (DPAA) Reference
> Manual.
> > It set the reserved areas, bit 1 and bit 11 to bit 15 in big endian,
> > which should be cleared. But the normal functions of the
> > auto-negotiation, e.g. Pause and Full Duplex, do not be set.
[S.H] The value 0x4001 is so special, as mentioned, it does not match the 
DPAA RM, So I dig some docs and found AN3869, in which the 0x4001 is the
desired one to write to TBI_ANA for SGMII.
 
> >
> > There is no problem in the p2041rdb board, because the ppc is
> > connected directly with the phy chip which support auto-negotiation by
> > default in SGMII interface. But the link problem will occur when it is
> > connected with a switch chip like BCM5389, the switch chip disable
> > auto-negotiation by default, and the ppc also disable
> > auto-negotiation, then there is no link between them.
[S.H] This seems a phyless connection. If BCM5389 disables AN, then PPC should
Also disable AN by setting TBI_CR.

> >
> > So use the vlue TBIANA_SETTINGS to enable the ppc's auto-negotiation.
[S.H] The TBI_ANA is " AN Advertisement Register ", it does not enable/disable AN.
A proper way should be to distinguish the phy and phyless connections and configure
The TBI registers accordingly.

Thanks,
Shaohui

> >
> > Signed-off-by: Yuanzheng Li <liyuanzheng01@163.com>
> > Cc: York Sun <yorksun@freescale.com>
> > ---
> >  drivers/net/fm/eth.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c index
> > eb8e936..78c0988 100644
> > --- a/drivers/net/fm/eth.c
> > +++ b/drivers/net/fm/eth.c
> > @@ -81,7 +81,7 @@ qsgmii_loop:
> >  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0, TBI_TBICON,
> >  			TBICON_CLK_SELECT);
> >  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0, TBI_ANA,
> > -			TBIANA_SGMII_ACK);
> > +			TBIANA_SETTINGS);
> >  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0,
> >  			TBI_CR, TBICR_SETTINGS);
> >  #endif
> >
> 
> Yuanzheng,
> 
> After your change, TBIANA_SGMII_ACK is not used and should be removed.
> 
> Shaohui and Codrin,
> 
> Please comment.
> 
> York
Yuanzheng Li Dec. 9, 2015, 2:25 p.m. UTC | #3 
The value TBIANA_SETTINGS is also work on the p2041rdb board, so it can work 
on the both phy and phyless connections.



At 2015-11-26 12:17:42, "Shaohui Xie" <Shaohui.Xie@freescale.com> wrote:
>> -----Original Message-----
>> From: York Sun [mailto:yorksun@freescale.com]
>> Sent: Thursday, November 26, 2015 1:03 AM
>> To: Xie Shaohui-B21989; Ciubotariu Codrin Constantin-B43658
>> Cc: Yuanzheng Li; u-boot@lists.denx.de; Liu Dave-R63238
>> Subject: Re: [PATCH] eth: dtsec: fix TBI ANA setting bug in
>> dtsec_configure_serdes()
>> 
>> + Experts
>> 
>> On 11/24/2015 11:04 PM, Yuanzheng Li wrote:
>> > The TBI_ANA register is configurated with the wrong value 0x4001,
>> > refer to QorIQ Data Path Acceleration Architecture (DPAA) Reference
>> Manual.
>> > It set the reserved areas, bit 1 and bit 11 to bit 15 in big endian,
>> > which should be cleared. But the normal functions of the
>> > auto-negotiation, e.g. Pause and Full Duplex, do not be set.
>[S.H] The value 0x4001 is so special, as mentioned, it does not match the 
>DPAA RM, So I dig some docs and found AN3869, in which the 0x4001 is the
>desired one to write to TBI_ANA for SGMII.
> 
>> >
>> > There is no problem in the p2041rdb board, because the ppc is
>> > connected directly with the phy chip which support auto-negotiation by
>> > default in SGMII interface. But the link problem will occur when it is
>> > connected with a switch chip like BCM5389, the switch chip disable
>> > auto-negotiation by default, and the ppc also disable
>> > auto-negotiation, then there is no link between them.
>[S.H] This seems a phyless connection. If BCM5389 disables AN, then PPC should
>Also disable AN by setting TBI_CR.
>
>> >
>> > So use the vlue TBIANA_SETTINGS to enable the ppc's auto-negotiation.
>[S.H] The TBI_ANA is " AN Advertisement Register ", it does not enable/disable AN.
>A proper way should be to distinguish the phy and phyless connections and configure
>The TBI registers accordingly.
>
>Thanks,
>Shaohui

>
>> >
>> > Signed-off-by: Yuanzheng Li <liyuanzheng01@163.com>
>> > Cc: York Sun <yorksun@freescale.com>
>> > ---
>> >  drivers/net/fm/eth.c |    2 +-
>> >  1 files changed, 1 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c index
>> > eb8e936..78c0988 100644
>> > --- a/drivers/net/fm/eth.c
>> > +++ b/drivers/net/fm/eth.c
>> > @@ -81,7 +81,7 @@ qsgmii_loop:
>> >  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0, TBI_TBICON,
>> >  			TBICON_CLK_SELECT);
>> >  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0, TBI_ANA,
>> > -			TBIANA_SGMII_ACK);
>> > +			TBIANA_SETTINGS);
>> >  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0,
>> >  			TBI_CR, TBICR_SETTINGS);
>> >  #endif
>> >
>> 
>> Yuanzheng,
>> 
>> After your change, TBIANA_SGMII_ACK is not used and should be removed.
>> 
>> Shaohui and Codrin,
>> 
>> Please comment.
>> 
>> York
York Sun Dec. 14, 2015, 3:54 a.m. UTC | #4 
Shaohui,

Please comment/confirm.

York

On 12/09/2015 10:25 PM, 李远正 wrote:
> The value TBIANA_SETTINGS is also work on the p2041rdb board, so it can work 
> on the both phy and phyless connections.
> 
> 
> 
> At 2015-11-26 12:17:42, "Shaohui Xie" <Shaohui.Xie@freescale.com> wrote:
>>> -----Original Message-----
>>> From: York Sun [mailto:yorksun@freescale.com]
>>> Sent: Thursday, November 26, 2015 1:03 AM
>>> To: Xie Shaohui-B21989; Ciubotariu Codrin Constantin-B43658
>>> Cc: Yuanzheng Li; u-boot@lists.denx.de; Liu Dave-R63238
>>> Subject: Re: [PATCH] eth: dtsec: fix TBI ANA setting bug in
>>> dtsec_configure_serdes()
>>> 
>>> + Experts
>>> 
>>> On 11/24/2015 11:04 PM, Yuanzheng Li wrote:
>>> > The TBI_ANA register is configurated with the wrong value 0x4001,
>>> > refer to QorIQ Data Path Acceleration Architecture (DPAA) Reference
>>> Manual.
>>> > It set the reserved areas, bit 1 and bit 11 to bit 15 in big endian,
>>> > which should be cleared. But the normal functions of the
>>> > auto-negotiation, e.g. Pause and Full Duplex, do not be set.
>>[S.H] The value 0x4001 is so special, as mentioned, it does not match the 
>>DPAA RM, So I dig some docs and found AN3869, in which the 0x4001 is the
>>desired one to write to TBI_ANA for SGMII.
>> 
>>> >
>>> > There is no problem in the p2041rdb board, because the ppc is
>>> > connected directly with the phy chip which support auto-negotiation by
>>> > default in SGMII interface. But the link problem will occur when it is
>>> > connected with a switch chip like BCM5389, the switch chip disable
>>> > auto-negotiation by default, and the ppc also disable
>>> > auto-negotiation, then there is no link between them.
>>[S.H] This seems a phyless connection. If BCM5389 disables AN, then PPC should
>>Also disable AN by setting TBI_CR.
>>
>>> >
>>> > So use the vlue TBIANA_SETTINGS to enable the ppc's auto-negotiation.
>>[S.H] The TBI_ANA is " AN Advertisement Register ", it does not enable/disable AN.
>>A proper way should be to distinguish the phy and phyless connections and configure
>>The TBI registers accordingly.
>>
>>Thanks,
>>Shaohui
>>
>>> >
>>> > Signed-off-by: Yuanzheng Li <liyuanzheng01@163.com>
>>> > Cc: York Sun <yorksun@freescale.com>
>>> > ---
>>> >  drivers/net/fm/eth.c |    2 +-
>>> >  1 files changed, 1 insertions(+), 1 deletions(-)
>>> >
>>> > diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c index
>>> > eb8e936..78c0988 100644
>>> > --- a/drivers/net/fm/eth.c
>>> > +++ b/drivers/net/fm/eth.c
>>> > @@ -81,7 +81,7 @@ qsgmii_loop:
>>> >  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0, TBI_TBICON,
>>> >  			TBICON_CLK_SELECT);
>>> >  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0, TBI_ANA,
>>> > -			TBIANA_SGMII_ACK);
>>> > +			TBIANA_SETTINGS);
>>> >  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0,
>>> >  			TBI_CR, TBICR_SETTINGS);
>>> >  #endif
>>> >
>>> 
>>> Yuanzheng,
>>> 
>>> After your change, TBIANA_SGMII_ACK is not used and should be removed.
>>> 
>>> Shaohui and Codrin,
>>> 
>>> Please comment.
>>> 
>>> York
> 
> 
> 
>  
>
shaohui xie Dec. 14, 2015, 7:07 a.m. UTC | #5 
Hi,

As mentioned, the value 0x4001 is the desired one in AN3869 for SGMII setting,
quoted as below:
" Program TBI ANA = 0x4001 (SGMII) or 0x01A0 (1000BASE-X). "

I think the AN3869 should be followed, change SGMII setting just because it 
worked on P2041rdb is not a good reason.

To distinguish SGMII from other connections should be the way to program TBI ANA,
This is also follow the AN3869.

Best Regards, 
Shaohui Xie


> -----Original Message-----

> From: York Sun [mailto:yorksun@freescale.com]

> Sent: Monday, December 14, 2015 11:55 AM

> To: 李远正; Xie Shaohui-B21989

> Cc: Ciubotariu Codrin Constantin-B43658; u-boot@lists.denx.de; Liu Dave-

> R63238

> Subject: Re: [PATCH] eth: dtsec: fix TBI ANA setting bug in

> dtsec_configure_serdes()

> 

> Shaohui,

> 

> Please comment/confirm.

> 

> York

> 

> On 12/09/2015 10:25 PM, 李远正 wrote:

> > The value TBIANA_SETTINGS is also work on the p2041rdb board, so it

> > can work on the both phy and phyless connections.

> >

> >

> >

> > At 2015-11-26 12:17:42, "Shaohui Xie" <Shaohui.Xie@freescale.com> wrote:

> >>> -----Original Message-----

> >>> From: York Sun [mailto:yorksun@freescale.com]

> >>> Sent: Thursday, November 26, 2015 1:03 AM

> >>> To: Xie Shaohui-B21989; Ciubotariu Codrin Constantin-B43658

> >>> Cc: Yuanzheng Li; u-boot@lists.denx.de; Liu Dave-R63238

> >>> Subject: Re: [PATCH] eth: dtsec: fix TBI ANA setting bug in

> >>> dtsec_configure_serdes()

> >>>

> >>> + Experts

> >>>

> >>> On 11/24/2015 11:04 PM, Yuanzheng Li wrote:

> >>> > The TBI_ANA register is configurated with the wrong value 0x4001,

> >>> > refer to QorIQ Data Path Acceleration Architecture (DPAA)

> >>> > Reference

> >>> Manual.

> >>> > It set the reserved areas, bit 1 and bit 11 to bit 15 in big

> >>> > endian, which should be cleared. But the normal functions of the

> >>> > auto-negotiation, e.g. Pause and Full Duplex, do not be set.

> >>[S.H] The value 0x4001 is so special, as mentioned, it does not match

> >>the DPAA RM, So I dig some docs and found AN3869, in which the 0x4001

> >>is the desired one to write to TBI_ANA for SGMII.

> >>

> >>> >

> >>> > There is no problem in the p2041rdb board, because the ppc is

> >>> > connected directly with the phy chip which support

> >>> > auto-negotiation by default in SGMII interface. But the link

> >>> > problem will occur when it is connected with a switch chip like

> >>> > BCM5389, the switch chip disable auto-negotiation by default, and

> >>> > the ppc also disable auto-negotiation, then there is no link

> between them.

> >>[S.H] This seems a phyless connection. If BCM5389 disables AN, then

> >>PPC should Also disable AN by setting TBI_CR.

> >>

> >>> >

> >>> > So use the vlue TBIANA_SETTINGS to enable the ppc's auto-

> negotiation.

> >>[S.H] The TBI_ANA is " AN Advertisement Register ", it does not

> enable/disable AN.

> >>A proper way should be to distinguish the phy and phyless connections

> >>and configure The TBI registers accordingly.

> >>

> >>Thanks,

> >>Shaohui

> >>

> >>> >

> >>> > Signed-off-by: Yuanzheng Li <liyuanzheng01@163.com>

> >>> > Cc: York Sun <yorksun@freescale.com>

> >>> > ---

> >>> >  drivers/net/fm/eth.c |    2 +-

> >>> >  1 files changed, 1 insertions(+), 1 deletions(-)

> >>> >

> >>> > diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c index

> >>> > eb8e936..78c0988 100644

> >>> > --- a/drivers/net/fm/eth.c

> >>> > +++ b/drivers/net/fm/eth.c

> >>> > @@ -81,7 +81,7 @@ qsgmii_loop:

> >>> >  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0,

> TBI_TBICON,

> >>> >  			TBICON_CLK_SELECT);

> >>> >  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0,

> TBI_ANA,

> >>> > -			TBIANA_SGMII_ACK);

> >>> > +			TBIANA_SETTINGS);

> >>> >  	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0,

> >>> >  			TBI_CR, TBICR_SETTINGS);

> >>> >  #endif

> >>> >

> >>>

> >>> Yuanzheng,

> >>>

> >>> After your change, TBIANA_SGMII_ACK is not used and should be removed.

> >>>

> >>> Shaohui and Codrin,

> >>>

> >>> Please comment.

> >>>

> >>> York

> >

> >

> >

> >

> >
York Sun March 22, 2016, 6:57 p.m. UTC | #6 
On 12/13/2015 11:07 PM, Xie Shaohui-B21989 wrote:
> Hi,
> 
> As mentioned, the value 0x4001 is the desired one in AN3869 for SGMII setting,
> quoted as below:
> " Program TBI ANA = 0x4001 (SGMII) or 0x01A0 (1000BASE-X). "
> 
> I think the AN3869 should be followed, change SGMII setting just because it 
> worked on P2041rdb is not a good reason.
> 
> To distinguish SGMII from other connections should be the way to program TBI ANA,
> This is also follow the AN3869.
> 
> Best Regards, 
> Shaohui Xie
> 
> 

Looks like this patch is dead. I will mark is NA.

York
diff mbox

Patch

diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c
index eb8e936..78c0988 100644
--- a/drivers/net/fm/eth.c
+++ b/drivers/net/fm/eth.c
@@ -81,7 +81,7 @@  qsgmii_loop:
 	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0, TBI_TBICON,
 			TBICON_CLK_SELECT);
 	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0, TBI_ANA,
-			TBIANA_SGMII_ACK);
+			TBIANA_SETTINGS);
 	tsec_local_mdio_write(phyregs, in_be32(&regs->tbipa), 0,
 			TBI_CR, TBICR_SETTINGS);
 #endif