TSI ethernet PHY question

TSI ethernet PHY question

[Benjamin Herrenschmidt]

Hi Folks !

While investigating some trouble we've had with networking on an Holly
eval board (TSI109 with IBM 750CL and Broadcom 5461A), I've had a look
at the PHY management code. There, it has a little workaround for
BCM54xx PHYs writing some hard coded values to some broadcom private
registers.

It's my experience that every revision of broadcom PHY around needs
different workarounds in different places though. Thus, I'd like to
better understand:

 - What is this workaround necessary for ?
 - What exact PHY model & version is it for ?

In addition I'd like to know if the driver is known to be used in
situations where the PHY ID cannot be probed via MDIO ?

I'm basically contemplating moving the driver to the generic phylib,
which would mean adding a phylib specific driver for that broadcom chip
that contains that workaround, but I need to know which exact chip
revision needs it. There are lots of BCM 54xx and I'm pretty sure for
example that this won't be needed on a BCM 5411 for example (or a
different one). In fact, I do intend to port sungem to phylib as well
which mean that I'll dump a whole load of BCM PHY specific drivers in
there.

Cheers,
Ben.
 

RE: TSI ethernet PHY question

[Alexandre Bounine]

Hi Ben,

The BCM PHY workaround is exactly for the Holly board with BCM5461A on
it.  The BCM54xx option which is confusing in this context originally
was used with board type option (other boards used BCM PHY). The
workaround added for Holly is caused by use of BCM5461A
Quality/TXC_RXC_DELAY pin.

This pin is a dual function pin. In normal operation, it is used to
drive an LED to indicate signal quality. The LED connected to this pin
acts as a pull up to VCC.

On power up, because this pin is pulled high by the LED, the
TXC_RXC_DELAY mode is enabled, causing a 1.9ns delay between the clock
and data on the GMII interface. Tsi109 could not operate properly with
this delay. The TXC_RXC_DELAY mode has to be disabled by software.

If the Quality/TXC_RXC_DELAY pin is left not connected PHY will work in
normal mode without delay and therefore the workaround is not required.
=20
>>In addition I'd like to know if the driver is known to be used in
>>situations where the PHY ID cannot be probed via MDIO ?=20

No.

>>I'm basically contemplating moving the driver to the generic phylib,
>>which would mean adding a phylib specific driver for that broadcom
chip
>>that contains that workaround, but I need to know which exact chip=20=20
>>revision needs it.

I think that for situations like one on the Holly board we may need
board-specific hooks which modify normal initialization. As in our case:
no LED - no trouble.

I have put into my plans switching Tsi108/9 driver to common PHY lib
(after Josh released his patch for Holly) but it looks like you will
beat me here - I still have to close some other tasks. Let me know if I
can help with anything around Tsi109.

Alex.


=20
-----Original Message-----
From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]=20
Sent: Wednesday, May 23, 2007 3:03 AM
To: tie-fei.zang@freescale.com; Alexandre Bounine
Cc: linuxppc-dev list; David Gibson
Subject: TSI ethernet PHY question

Hi Folks !

While investigating some trouble we've had with networking on an Holly
eval board (TSI109 with IBM 750CL and Broadcom 5461A), I've had a look
at the PHY management code. There, it has a little workaround for
BCM54xx PHYs writing some hard coded values to some broadcom private
registers.

It's my experience that every revision of broadcom PHY around needs
different workarounds in different places though. Thus, I'd like to
better understand:

 - What is this workaround necessary for ?
 - What exact PHY model & version is it for ?

In addition I'd like to know if the driver is known to be used in
situations where the PHY ID cannot be probed via MDIO ?

I'm basically contemplating moving the driver to the generic phylib,
which would mean adding a phylib specific driver for that broadcom chip
that contains that workaround, but I need to know which exact chip
revision needs it. There are lots of BCM 54xx and I'm pretty sure for
example that this won't be needed on a BCM 5411 for example (or a
different one). In fact, I do intend to port sungem to phylib as well
which mean that I'll dump a whole load of BCM PHY specific drivers in
there.

Cheers,
Ben.
=20


---=0D
=0D
Important Notice: This message is intended for the use of the individual to=
 whom it is addressed and may contain information which is privileged, conf=
idential and/or exempt from disclosure under applicable law. If the reader =
of this message is not the intended recipient, or is not the employee or ag=
ent responsible for delivering the message to the intended recipient, you a=
re hereby notified that any dissemination, distribution, or copying of this=
 communication is strictly prohibited. If you have received this communicat=
ion in error, please notify the sender immediately by telephone or return e=
-mail and delete the original message from your systems. Thank you. =0D

Re: TSI ethernet PHY question

[Segher Boessenkool]

> On power up, because this pin is pulled high by the LED, the
> TXC_RXC_DELAY mode is enabled, causing a 1.9ns delay between the clock
> and data on the GMII interface. Tsi109 could not operate properly with
> this delay. The TXC_RXC_DELAY mode has to be disabled by software.
>
> If the Quality/TXC_RXC_DELAY pin is left not connected PHY will work in
> normal mode without delay and therefore the workaround is not required.

So it is a board-specific workaround.

> I think that for situations like one on the Holly board we may need
> board-specific hooks which modify normal initialization. As in our 
> case:
> no LED - no trouble.

Should be initiated from the platform code yes; it could
be as simple as passing a flag to the init function.

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

Don't top-post :-)


Segher

RE: TSI ethernet PHY question

[Benjamin Herrenschmidt]

> On power up, because this pin is pulled high by the LED, the
> TXC_RXC_DELAY mode is enabled, causing a 1.9ns delay between the clock
> and data on the GMII interface. Tsi109 could not operate properly with
> this delay. The TXC_RXC_DELAY mode has to be disabled by software.
> 
> If the Quality/TXC_RXC_DELAY pin is left not connected PHY will work in
> normal mode without delay and therefore the workaround is not required.

Ok, so this is a workaround that is specific to the Holly board...
interesting. I have to figure out what is the best way of having it in a
"generic" PHY driver for the BCM5461A chip.

Right now, we're seeing a problem where the driver doesn't detect a link
up. It detects the link at boot fine. If we disconnect the cable, it
sees the link going down, but it never sees the link going back up. I
haven't investigated much yet but I wanted to understand this workaround
in case it could be related.

> I think that for situations like one on the Holly board we may need
> board-specific hooks which modify normal initialization. As in our case:
> no LED - no trouble.

Yes, I need to look into that.

> I have put into my plans switching Tsi108/9 driver to common PHY lib
> (after Josh released his patch for Holly) but it looks like you will
> beat me here - I still have to close some other tasks. Let me know if I
> can help with anything around Tsi109.

We'll see, I said I'm contemplating the idea, doesn't mean I'll actually
do it anytime soon :-)

Cheers,
Ben.

Re: TSI ethernet PHY question

[Andy Fleming]

On May 23, 2007, at 17:52, Benjamin Herrenschmidt wrote:

>
>> On power up, because this pin is pulled high by the LED, the
>> TXC_RXC_DELAY mode is enabled, causing a 1.9ns delay between the  
>> clock
>> and data on the GMII interface. Tsi109 could not operate properly  
>> with
>> this delay. The TXC_RXC_DELAY mode has to be disabled by software.
>>
>> If the Quality/TXC_RXC_DELAY pin is left not connected PHY will  
>> work in
>> normal mode without delay and therefore the workaround is not  
>> required.
>
> Ok, so this is a workaround that is specific to the Holly board...
> interesting. I have to figure out what is the best way of having it  
> in a
> "generic" PHY driver for the BCM5461A chip.

Yeah, this is a whole category of thing that the PHY Lib doesn't  
handle very well.  My problem is that I can envision several  
categories of board-specific workarounds:

1) board-specific initialization
2) board-specific fixups after a reset
3) board-specific fixups when changing/reading the PHY link state
4) board-specific interrupt fixups
5) Others I haven't thought of

I'm loathe to turn the PHY code into the PCI code by having almost  
every operation call out to board-specific hooks, so it would be good  
if we could get it down to one or two.

For instance, I think #1 might usually be workable by passing in PHY- 
specific flags, and letting the PHY driver deal with it in  
config_init.  This might even be workable for the BCM5461A chip as  
mentioned above.  You could define a BCM5461A_TXC_RXC_DELAY_DISABLE  
flag, and have the config_init code check for that flag and perform  
the necessary disable.

I had hoped most board-specific situations could be handled that way,  
but there's also the case where resetting the PHY requires board code  
to respond (by setting a board register, or somesuch).

It's really quite a mess.  Any suggestions are quite welcome.

Andy

Re: TSI ethernet PHY question

[Benjamin Herrenschmidt]

> For instance, I think #1 might usually be workable by passing in PHY- 
> specific flags, and letting the PHY driver deal with it in  
> config_init.  This might even be workable for the BCM5461A chip as  
> mentioned above.  You could define a BCM5461A_TXC_RXC_DELAY_DISABLE  
> flag, and have the config_init code check for that flag and perform  
> the necessary disable.

For powerpc, I have a solution at hand, it's the device-tree :-)

Any struct device in the system can have a device node pointer via the
dev_sysdata thingy I added recently. So we can have some code for
powerpc that properly hooks up the PHY to an (optional) device-node
which can then contains properties describing what kind of workarounds
need to be applied.

For example, we can have a txc-rxc-delay-disable property on Holly.

The problem is that of course the PHY driver will need some powerpc
specific code to go fetch that. An option would be to instead use flags
and have a piece of powerpc specific code that translates those
device-tree properties into flags so that other archs can use the flags
using their own ways of passing them in.

I'm not too hot with the flag stuff tho since those really need to be
defined per PHY model/family. But I suppose that's fair enough.

Ben.

Re: TSI ethernet PHY question

[Segher Boessenkool]

> For powerpc, I have a solution at hand, it's the device-tree :-)
>
> Any struct device in the system can have a device node pointer via the
> dev_sysdata thingy I added recently. So we can have some code for
> powerpc that properly hooks up the PHY to an (optional) device-node
> which can then contains properties describing what kind of workarounds
> need to be applied.
>
> For example, we can have a txc-rxc-delay-disable property on Holly.

This is equivalent to the ethernet driver passing this information
to phylib via the init arguments.

You still have the same problems as Andy described where the
necessary workaround is not something local to phylib, but
needs cooperation of the ethernet code or the soc code or
some other platform code.

Since the specific bug we're talking about here is not a
problem with the PHY, but a miswiring on the board, I wouldn't
put a flag for the workaround in the phy node in the device
tree.  It certainly is an option though.

> The problem is that of course the PHY driver will need some powerpc
> specific code to go fetch that.

The ethernet driver can handle it, instead.

> An option would be to instead use flags
> and have a piece of powerpc specific code that translates those
> device-tree properties into flags so that other archs can use the flags
> using their own ways of passing them in.

It really shouldn't ever be needed on any other board, but
sure.

> I'm not too hot with the flag stuff tho since those really need to be
> defined per PHY model/family. But I suppose that's fair enough.

Divide the flags arg into a generic and a chip-specific part?


Segher

Re: TSI ethernet PHY question

[Benjamin Herrenschmidt]

On Fri, 2007-05-25 at 01:54 +0200, Segher Boessenkool wrote:

> This is equivalent to the ethernet driver passing this information
> to phylib via the init arguments.
> 
> You still have the same problems as Andy described where the
> necessary workaround is not something local to phylib, but
> needs cooperation of the ethernet code or the soc code or
> some other platform code.

If it's in the PHY device node, the ethernet driver doesn't need to be
involved more than calling some generic helper that finds the right
node, parses it and generates known flags.

> Since the specific bug we're talking about here is not a
> problem with the PHY, but a miswiring on the board, I wouldn't
> put a flag for the workaround in the phy node in the device
> tree.  It certainly is an option though.

Why ? That's the perfect place to put it in !

> > The problem is that of course the PHY driver will need some powerpc
> > specific code to go fetch that.
> 
> The ethernet driver can handle it, instead.

I don't understand why you want to involve the ethernet driver in
something that doesn't have much to do with it. A pin of the PHY is
miswired causing something to be enabled that shouldn't be. Ok, there is
indeed the fact that the problem is partially related to the TSI
ethernet not supporting when that PHY feature is "enabled" but still...
it's a PHY setting, totally specific to a given PHY revision, I'm not
sure there's much point in having it in the eth driver.

Ben.

Re: TSI ethernet PHY question

[Segher Boessenkool]

>> You still have the same problems as Andy described where the
>> necessary workaround is not something local to phylib, but
>> needs cooperation of the ethernet code or the soc code or
>> some other platform code.
>
> If it's in the PHY device node, the ethernet driver doesn't need to be
> involved more than calling some generic helper that finds the right
> node, parses it and generates known flags.

I am not talking about the workaround for _this_ bug, but
about other PHY workarounds that _do_ need cooperation of
other devices.  How should those be described in the
device tree?

>> Since the specific bug we're talking about here is not a
>> problem with the PHY, but a miswiring on the board, I wouldn't
>> put a flag for the workaround in the phy node in the device
>> tree.  It certainly is an option though.
>
> Why ? That's the perfect place to put it in !

Only if you think the device tree is a configuration
mechanism for the OS.  Your workaround is in the PHY,
sure; but the _bug_ is in the board.

However in this case you could put a property in the
PHY node, similar things have been done before.  It's
ugly and doesn't solve any problem (it is just as much
work to parse the board model as to find this magic
property), and you *still* should pass in the flag
from the platform layer, and not have the phylib try
to handle it by itself.

>>> The problem is that of course the PHY driver will need some powerpc
>>> specific code to go fetch that.
>>
>> The ethernet driver can handle it, instead.
>
> I don't understand why you want to involve the ethernet driver in
> something that doesn't have much to do with it.

The ethernet driver is a powerpc-specific driver, that's
one thing.  Also, the workaround should be initiated by
the platform code, so has to go through the ethernet driver
(since it instantiates the phylib driver).

> A pin of the PHY is
> miswired causing something to be enabled that shouldn't be. Ok, there 
> is
> indeed the fact that the problem is partially related to the TSI
> ethernet not supporting when that PHY feature is "enabled" but still...
> it's a PHY setting, totally specific to a given PHY revision, I'm not
> sure there's much point in having it in the eth driver.

For many similar workarounds, the ethernet driver _does_ have
to cooperate in the workaround.  For some other such workarounds,
the soc code has to be involved.  Etc. etc.

You can do a quick "fix" now by doing this magic property
thing, and it sure is a *quick* fix; but later on you'll
have to do some other workarounds the proper way.  And
you'll be stuck with the property forever.  Not such a
big deal, sure; hey, I already _did_ say I'm okay with it,
right?  It's just the "wrong" thing to do ;-)


Segher

Re: TSI ethernet PHY question

[Benjamin Herrenschmidt]

On Fri, 2007-05-25 at 02:57 +0200, Segher Boessenkool wrote:
> 
> However in this case you could put a property in the
> PHY node, similar things have been done before.  It's
> ugly and doesn't solve any problem (it is just as much
> work to parse the board model as to find this magic
> property), and you *still* should pass in the flag
> from the platform layer, and not have the phylib try
> to handle it by itself.

I disagree, it's not ugly and nicely solves the problem.

For example, imagine you have 2 PHYs on a board and only one needs the
workaround ? Really, the PHY node is the best place for it.

> The ethernet driver is a powerpc-specific driver, that's
> one thing.  Also, the workaround should be initiated by
> the platform code, so has to go through the ethernet driver
> (since it instantiates the phylib driver).

Still... it can be done via generic calls in powerpc ethernet drivers
that set flags in phylib based on things in the device-tree.

> For many similar workarounds, the ethernet driver _does_ have
> to cooperate in the workaround.  For some other such workarounds,
> the soc code has to be involved.  Etc. etc.

> You can do a quick "fix" now by doing this magic property
> thing, and it sure is a *quick* fix; but later on you'll
> have to do some other workarounds the proper way.  And
> you'll be stuck with the property forever.  Not such a
> big deal, sure; hey, I already _did_ say I'm okay with it,
> right?  It's just the "wrong" thing to do ;-)

I have no bloody idea what you consider "the proper way"

I think it's the right thing to do.

Ben.

Re: TSI ethernet PHY question

[David Gibson]

On Fri, May 25, 2007 at 01:54:35AM +0200, Segher Boessenkool wrote:
> > For powerpc, I have a solution at hand, it's the device-tree :-)
> >
> > Any struct device in the system can have a device node pointer via the
> > dev_sysdata thingy I added recently. So we can have some code for
> > powerpc that properly hooks up the PHY to an (optional) device-node
> > which can then contains properties describing what kind of workarounds
> > need to be applied.
> >
> > For example, we can have a txc-rxc-delay-disable property on Holly.
> 
> This is equivalent to the ethernet driver passing this information
> to phylib via the init arguments.
> 
> You still have the same problems as Andy described where the
> necessary workaround is not something local to phylib, but
> needs cooperation of the ethernet code or the soc code or
> some other platform code.
> 
> Since the specific bug we're talking about here is not a
> problem with the PHY, but a miswiring on the board, I wouldn't
> put a flag for the workaround in the phy node in the device
> tree.  It certainly is an option though.

Uh.. something to bear in mind is that although it is a board
miswiring, it's of a type that it will plausibly occur in other
boards.  IIRC, if a LED is attached to this PHY the workaround is
necessary, or something similar.  So there is value in having a
particular flag for this rather than just looking at the board model.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: TSI ethernet PHY question

[Paul Mackerras]

Segher Boessenkool writes:

> Only if you think the device tree is a configuration
> mechanism for the OS.

Well, actually, it's whatever we want it to be...

There is some value in making the device tree abstract and general,
but not an overwhelming value that trumps all other considerations.

Paul.

Re: TSI ethernet PHY question

[Zang Roy-r61911]

On Fri, 2007-05-25 at 10:00, David Gibson wrote:
> On Fri, May 25, 2007 at 01:54:35AM +0200, Segher Boessenkool wrote:
> > > For powerpc, I have a solution at hand, it's the device-tree :-)
> > >
> > > Any struct device in the system can have a device node pointer via
> the
> > > dev_sysdata thingy I added recently. So we can have some code for
> > > powerpc that properly hooks up the PHY to an (optional)
> device-node
> > > which can then contains properties describing what kind of
> workarounds
> > > need to be applied.
> > >
> > > For example, we can have a txc-rxc-delay-disable property on
> Holly.
> > 
> > This is equivalent to the ethernet driver passing this information
> > to phylib via the init arguments.
> > 
> > You still have the same problems as Andy described where the
> > necessary workaround is not something local to phylib, but
> > needs cooperation of the ethernet code or the soc code or
> > some other platform code.
> > 
> > Since the specific bug we're talking about here is not a
> > problem with the PHY, but a miswiring on the board, I wouldn't
> > put a flag for the workaround in the phy node in the device
> > tree.  It certainly is an option though.
> 
> Uh.. something to bear in mind is that although it is a board
> miswiring, it's of a type that it will plausibly occur in other
> boards.  IIRC, if a LED is attached to this PHY the workaround is
> necessary, or something similar.  So there is value in having a
> particular flag for this rather than just looking at the board model.
> 
But it is indeed a board specific issue. phylib is a good place. But we
should consider how can we pass this information to phylib. device tree
is a choice, but how about the platform not using device tree?

Roy

Re: TSI ethernet PHY question

[Segher Boessenkool]

>> Since the specific bug we're talking about here is not a
>> problem with the PHY, but a miswiring on the board, I wouldn't
>> put a flag for the workaround in the phy node in the device
>> tree.  It certainly is an option though.
>
> Uh.. something to bear in mind is that although it is a board
> miswiring, it's of a type that it will plausibly occur in other
> boards.

It is possible yes.

> IIRC, if a LED is attached to this PHY the workaround is
> necessary, or something similar.

Only if it is attached the wrong way, so it acts as a
pull-up (or was it pull-down) during chip reset.

>   So there is value in having a
> particular flag for this rather than just looking at the board model.

A bit, maybe.


Segher

Re: TSI ethernet PHY question

[Segher Boessenkool]

>> However in this case you could put a property in the
>> PHY node, similar things have been done before.  It's
>> ugly and doesn't solve any problem (it is just as much
>> work to parse the board model as to find this magic
>> property), and you *still* should pass in the flag
>> from the platform layer, and not have the phylib try
>> to handle it by itself.
>
> I disagree, it's not ugly and nicely solves the problem.

It doesn't solve the conceived problem that making this
a board-dependent workaround would be "too hard".

> For example, imagine you have 2 PHYs on a board and only one needs the
> workaround ? Really, the PHY node is the best place for it.

Then the board code will know that.

>> The ethernet driver is a powerpc-specific driver, that's
>> one thing.  Also, the workaround should be initiated by
>> the platform code, so has to go through the ethernet driver
>> (since it instantiates the phylib driver).
>
> Still... it can be done via generic calls in powerpc ethernet drivers
> that set flags in phylib based on things in the device-tree.

Yes, exactly.

>> For many similar workarounds, the ethernet driver _does_ have
>> to cooperate in the workaround.  For some other such workarounds,
>> the soc code has to be involved.  Etc. etc.
>
>> You can do a quick "fix" now by doing this magic property
>> thing, and it sure is a *quick* fix; but later on you'll
>> have to do some other workarounds the proper way.  And
>> you'll be stuck with the property forever.  Not such a
>> big deal, sure; hey, I already _did_ say I'm okay with it,
>> right?  It's just the "wrong" thing to do ;-)
>
> I have no bloody idea what you consider "the proper way"

Make board-specific workarounds triggered by the board code.

> I think it's the right thing to do.

And I don't :-)  But I already said I'm okay with it, it's
only one property and it won't hurt anything else.


Segher

Re: TSI ethernet PHY question

[Segher Boessenkool]

>> Only if you think the device tree is a configuration
>> mechanism for the OS.
>
> Well, actually, it's whatever we want it to be...

Oh not this discussion again.

> There is some value in making the device tree abstract and general,
> but not an overwhelming value that trumps all other considerations.

Yes, there can be other considerations; however, not making
a mess out of it all is certainly a very important one in
my book.


Segher