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

[Shaohui Xie]

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 at freescale.com]
> Sent: Monday, December 14, 2015 11:55 AM
> To: 李远正; Xie Shaohui-B21989
> Cc: Ciubotariu Codrin Constantin-B43658; u-boot at 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 at freescale.com> wrote:
> >>> -----Original Message-----
> >>> From: York Sun [mailto:yorksun at freescale.com]
> >>> Sent: Thursday, November 26, 2015 1:03 AM
> >>> To: Xie Shaohui-B21989; Ciubotariu Codrin Constantin-B43658
> >>> Cc: Yuanzheng Li; u-boot at 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 at 163.com>
> >>> > Cc: York Sun <yorksun at 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
> >
> >
> >
> >
> >