[U-Boot] [PATCH v3] Support for multiple SGMII/TBI interfaces for TSEC ethernet

[Andy Fleming]

On Mon, Sep 15, 2008 at 5:17 PM, Peter Tyser <ptyser at xes-inc.com> wrote:
> On Mon, 2008-09-15 at 16:13 -0500, Andy Fleming wrote:
>> > @@ -299,12 +301,10 @@ static int init_phy(struct eth_device *dev)
>> >  {
>> >        struct tsec_private *priv = (struct tsec_private *)dev->priv;
>> >        struct phy_info *curphy;
>> > -       volatile tsec_t *phyregs = priv->phyregs;
>> >        volatile tsec_t *regs = priv->regs;
>> >
>> >        /* Assign a Physical address to the TBI */
>> >        regs->tbipa = CFG_TBIPA_VALUE;
>> > -       phyregs->tbipa = CFG_TBIPA_VALUE;
>> >        asm("sync");
>>
>>
>> What was the purpose of doing this?  The problem I have with it is in
>> the odd situation where the TSEC whose MII regs are connected to the
>> bus is not enabled.  It would mean that the TBIPA would never be set
>> to CFG_TBIPA_VALUE.
>
> I don't quite understand what you mean.  My understanding was that if a
> TSEC is not enabled, the TBIPA for that TSEC should not be enabled
> either.
>
> The original code was writing the TBIPA value 2 times for every TSEC -
> once at the TSEC register address in cpu space, and a second time at the
> TSEC's MII register address in cpu space.
>

Yes, unfortunately, it might be necessary.  Imagine this scenario:

1) TSEC0 is not enabled
2) The current value of TSEC0's TBIPA register contains the address of
a PHY we want to use
3) One of the other TSECs comes up, and tries to access its PHY at that address.

You see, the TBIPA register setting has *two* purposes.  Most
controllers aren't connected to any external PHYs (via their MII
regs), so the purpose of TBIPA is to tell the controller what address
you want to use to access the TBI PHY.  However, for controllers
attached to an external PHY, setting the TBIPA register serves not
only to define where the TBI PHY is, but where *other* PHYs are not.
If TSEC0's TBIPA register is not set, it might conflict with a PHY
that doesn't belong to TSEC0.

It's admittedly an unlikely corner case, but I trust in the ability of
board designers to break my code in such ways.  My trust has been
validated several times.  :)

So while I agree with you that it's almost always wasteful, it's
necessary without more significant changes.  One other option would be
to only write priv->phyregs->tbipa (n times, even though it would
almost always be written to the same place all n times).  Then,
priv->regs->tbipa would contain whatever was in the register to start
with, which is fine, since the address only matters for the regs which
access external PHYs.

One other note:  currently we don't support accessing the TBI PHYs
through the mii utilities.  This is mostly due to difficulties in
finding addresses for the TBI PHYs that don't conflict with existing
PHY addresses.  If you have some clever ideas that don't involve
hard-coding the addresses (I have to anticipate the advent of chips
with 10+ ethernet controllers), I'm open to hearing them.  :)  It's an
issue I intend to address, but it's not getting any timeslices this
quarter.

Andy