Re: Porting the ibm_newemac driver to use phylib (and other PHY/MAC questions)

Re: Porting the ibm_newemac driver to use phylib (and other PHY/MAC questions)

[Josh Boyer]

On Fri, Apr 17, 2009 at 8:32 PM, Kyle Moffett <kyle@moffetthome.net> wrote:
> Hello,
>
> I'm currently fiddling with a custom embedded prototype board using
> the ibm_newemac driver with some currently-unsupported PHYs. =A0Those
> PHYs *are* supported by phylib, but the emac driver seems to have its
> own PHY layer cribbed from the sungem driver. =A0I'm curious if there's
> some particular reason it hasn't been ported (aside from "nobody has
> bothered yet").

IIRC, Ben had some issues with how phylib and the EMAC would need to
interact.  Not sure if he has those written down somewhere or not.
(CC'd).

> I've temporarily hacked a PHY driver together for the moment, but it
> would be much easier for us to maintain and update our board if the
> PHY drivers were integrated. =A0As a result I'm also interested in how
> complicated it might be to port the driver (and possibly sungem as
> well) over to phylib, if that is indeed feasible. =A0Also, if I end up
> going that route, are there others available with other hardware
> variants who would be willing to test my patches on their boards?

I have a large variety of boards that I can test with since the entire
4xx line relies on this driver for on-board network.

josh

Re: Porting the ibm_newemac driver to use phylib (and other PHY/MAC questions)

[Kyle Moffett]

On Mon, Apr 20, 2009 at 8:29 AM, Josh Boyer <jwboyer@linux.vnet.ibm.com> wr=
ote:
> On Fri, Apr 17, 2009 at 8:32 PM, Kyle Moffett <kyle@moffetthome.net> wrot=
e:
>> Hello,
>>
>> I'm currently fiddling with a custom embedded prototype board using
>> the ibm_newemac driver with some currently-unsupported PHYs. =C2=A0Those
>> PHYs *are* supported by phylib, but the emac driver seems to have its
>> own PHY layer cribbed from the sungem driver. =C2=A0I'm curious if there=
's
>> some particular reason it hasn't been ported (aside from "nobody has
>> bothered yet").
>
> IIRC, Ben had some issues with how phylib and the EMAC would need to
> interact. =C2=A0Not sure if he has those written down somewhere or not.
> (CC'd).

Hmm, yeah, I'd be interested to see those.  There's enough similar
between phylib and the EMAC and sungem drivers that I'm considering a
series of somewhat-mechanical patches to make EMAC and sungem use the
"struct phy_device" and "struct mii_bus" from phylib, possibly
abstracting out some helper functions along the way.

>> Also, if I end up
>> going that route, are there others available with other hardware
>> variants who would be willing to test my patches on their boards?
>
> I have a large variety of boards that I can test with since the entire
> 4xx line relies on this driver for on-board network.

Wonderful!  If/when I hack together a patch series I'll make sure to
put you on the CC list.  Thanks!

Cheers,
Kyle Moffett

Re: Porting the ibm_newemac driver to use phylib (and other PHY/MAC questions)

[Benjamin Herrenschmidt]

On Mon, 2009-04-20 at 20:10 -0400, Kyle Moffett wrote:
> > IIRC, Ben had some issues with how phylib and the EMAC would need to
> > interact.  Not sure if he has those written down somewhere or not.
> > (CC'd).
> 
> Hmm, yeah, I'd be interested to see those.  There's enough similar
> between phylib and the EMAC and sungem drivers that I'm considering a
> series of somewhat-mechanical patches to make EMAC and sungem use the
> "struct phy_device" and "struct mii_bus" from phylib, possibly
> abstracting out some helper functions along the way.

Yup, emac and sungem predate phylib.

I had a quick look at what it would take to port at least emac over, the
main issue was that I want to be able to sleep (ie, take a mutex) in my
mdio read/write functions, and back then, phylib wouldn't let me do that
due to spinlock and timer/softirq usage.

Ben.

Re: Porting the ibm_newemac driver to use phylib (and other PHY/MAC questions)

[Kyle Moffett]

On Wed, Apr 22, 2009 at 4:21 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Mon, 2009-04-20 at 20:10 -0400, Kyle Moffett wrote:
>> > IIRC, Ben had some issues with how phylib and the EMAC would need to
>> > interact. =C2=A0Not sure if he has those written down somewhere or not=
.
>> > (CC'd).
>>
>> Hmm, yeah, I'd be interested to see those. =C2=A0There's enough similar
>> between phylib and the EMAC and sungem drivers that I'm considering a
>> series of somewhat-mechanical patches to make EMAC and sungem use the
>> "struct phy_device" and "struct mii_bus" from phylib, possibly
>> abstracting out some helper functions along the way.
>
> Yup, emac and sungem predate phylib.
>
> I had a quick look at what it would take to port at least emac over, the
> main issue was that I want to be able to sleep (ie, take a mutex) in my
> mdio read/write functions, and back then, phylib wouldn't let me do that
> due to spinlock and timer/softirq usage.

Ok, I've made some progress in the port, but right now I'm trying to
puzzle out what the "gpcs" bits in the code are.  From the few
publicly available docs and some mailing list posts, the gpcs address
appears to be some kind of integrated virtual PHY used when 460GT-ish
chips are communicating via an SGMII bus.  My current plan of action
is to separate the "gpcs" out into a separate PHY device controlled by
the emac code.

I'm also curious about the intent of the "mdio_instance" pointer (IE:
the "mdio-device" property).  Is that used when all the PHY devices
are attached to the MDIO bus of only one of the (multiple) emac
devices?  Or is that for when two emac chipsets are connected to the
same MDIO bus wire?  (or both?)  What keeps the emac_instance pointed
to by the "mdio_instance" from going away while the other emac chipset
is using it?

In either case, I plan to have the device actually holding the MDIO
bus run the mdiobus_alloc() and mdiobus_register() functions, then the
other emac instance will simply take a reference to that MDIO bus
(which would also pin down the emac instance that owns it).

Cheers,
Kyle Moffett

Re: Porting the ibm_newemac driver to use phylib (and other PHY/MAC questions)

[Grant Likely]

On Thu, Apr 30, 2009 at 9:04 AM, Kyle Moffett <kyle@moffetthome.net> wrote:
> On Wed, Apr 22, 2009 at 4:21 AM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>> On Mon, 2009-04-20 at 20:10 -0400, Kyle Moffett wrote:
>>> > IIRC, Ben had some issues with how phylib and the EMAC would need to
>>> > interact. =A0Not sure if he has those written down somewhere or not.
>>> > (CC'd).
>>>
>>> Hmm, yeah, I'd be interested to see those. =A0There's enough similar
>>> between phylib and the EMAC and sungem drivers that I'm considering a
>>> series of somewhat-mechanical patches to make EMAC and sungem use the
>>> "struct phy_device" and "struct mii_bus" from phylib, possibly
>>> abstracting out some helper functions along the way.
>>
>> Yup, emac and sungem predate phylib.
>>
>> I had a quick look at what it would take to port at least emac over, the
>> main issue was that I want to be able to sleep (ie, take a mutex) in my
>> mdio read/write functions, and back then, phylib wouldn't let me do that
>> due to spinlock and timer/softirq usage.
>
> Ok, I've made some progress in the port, but right now I'm trying to
> puzzle out what the "gpcs" bits in the code are. =A0From the few
> publicly available docs and some mailing list posts, the gpcs address
> appears to be some kind of integrated virtual PHY used when 460GT-ish
> chips are communicating via an SGMII bus. =A0My current plan of action
> is to separate the "gpcs" out into a separate PHY device controlled by
> the emac code.
>
> I'm also curious about the intent of the "mdio_instance" pointer (IE:
> the "mdio-device" property). =A0Is that used when all the PHY devices
> are attached to the MDIO bus of only one of the (multiple) emac
> devices? =A0Or is that for when two emac chipsets are connected to the
> same MDIO bus wire? =A0(or both?) =A0What keeps the emac_instance pointed
> to by the "mdio_instance" from going away while the other emac chipset
> is using it?
>
> In either case, I plan to have the device actually holding the MDIO
> bus run the mdiobus_alloc() and mdiobus_register() functions, then the
> other emac instance will simply take a reference to that MDIO bus
> (which would also pin down the emac instance that owns it).

Just a heads up Kyle; there are changes queued in the netdev tree
which add OF helpers for MDIO bus drivers and MAC drivers.  See here:

http://git.kernel.org/?p=3Dlinux/kernel/git/davem/net-next-2.6.git;a=3Dcomm=
it;h=3D8bc487d150b939e69830c39322df4ee486efe381

and here is an example of a driver change:

http://git.kernel.org/?p=3Dlinux/kernel/git/davem/net-next-2.6.git;a=3Dcomm=
it;h=3D1dd2d06c0459a2f1bffc56765e3cc57427818867

Cheers,
g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: Porting the ibm_newemac driver to use phylib (and other PHY/MAC questions)

[Kyle Moffett]

On Thu, Apr 30, 2009 at 11:11 AM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> Just a heads up Kyle; there are changes queued in the netdev tree
> which add OF helpers for MDIO bus drivers and MAC drivers. =C2=A0See here=
:
>
> http://git.kernel.org/?p=3Dlinux/kernel/git/davem/net-next-2.6.git;a=3Dco=
mmit;h=3D8bc487d150b939e69830c39322df4ee486efe381
>
> and here is an example of a driver change:
>
> http://git.kernel.org/?p=3Dlinux/kernel/git/davem/net-next-2.6.git;a=3Dco=
mmit;h=3D1dd2d06c0459a2f1bffc56765e3cc57427818867

Hmm, very interesting!  Thanks!  Although I'm not sure that those OF
helpers are entirely usable for the emac driver at the moment, as the
device trees for the existing boards simply don't have PHYs present in
them.  Most of the ibm_newemac board device-trees just have one of:
"phy-address =3D <4>", "phy-mask =3D <0xffff0000>", or nothing at all (for
autodetection).  I will probably need to leave in support for the old
PHY mask parsing to preserve backwards compatibility.

My main concern at the moment is cleaning up the driver's general PHY
handling.  I got started on this whole mess when I was trying to write
some hackish PHY drivers for a weird custom board I've got here.  I
couldn't figure out why the hell all my PHY register changes in the
phy_ops->init function kept getting cleared, until I noticed 2 things:
 The emac_reset_phy() function gets called occasionally and does not
call the ->init() function again afterwards.  The
genmii_setup_forced() function (in the EMAC driver) unconditionally
ORs the BCMR_RESET flag into the MII_BCMR register.  Both of those
meant that any early setup I did for my PHY was getting completely
cleared on a regular basis, with no decent way for me to patch it back
up again.

Cheers,
Kyle Moffett

Re: Porting the ibm_newemac driver to use phylib (and other PHY/MAC questions)

[Benjamin Herrenschmidt]

> Ok, I've made some progress in the port, but right now I'm trying to
> puzzle out what the "gpcs" bits in the code are.  From the few
> publicly available docs and some mailing list posts, the gpcs address
> appears to be some kind of integrated virtual PHY used when 460GT-ish
> chips are communicating via an SGMII bus.  My current plan of action
> is to separate the "gpcs" out into a separate PHY device controlled by
> the emac code.

Well... GPCS is indeed some kind of internal PHY if you like but it's
not MDIO controlled and I don't really see a need to move it to the
phylib as it's so specific to the NIC itself. But if you think it cleans
the code up significantly... There are also some subtle differences in
how the NIC is programmed when using GPCS vs. xMII

> I'm also curious about the intent of the "mdio_instance" pointer (IE:
> the "mdio-device" property).  Is that used when all the PHY devices
> are attached to the MDIO bus of only one of the (multiple) emac
> devices? 

It's common especially on older SoCs using EMAC to have only one of
the EMAC instance with an MDIO bus for configuring the PHYs. This is one
of the reasons why I have the mutex in the low level MDIO access
routines since 2 EMACs can try to talk to the same MDIO, and this is the
problem I had with phylib back then which was doing everything in atomic
contexts.

> What keeps the emac_instance pointed
> to by the "mdio_instance" from going away while the other emac chipset
> is using it?

Nothing other than those are all in a SoC and so generally don't do
hotplug, but I suppose that with things like kvm which could potentially
hotplug devices between partitions, one would have to get a bit more
careful.
 
> In either case, I plan to have the device actually holding the MDIO
> bus run the mdiobus_alloc() and mdiobus_register() functions, then the
> other emac instance will simply take a reference to that MDIO bus
> (which would also pin down the emac instance that owns it).

Sounds good at first.

Cheers,
Ben.

Re: Porting the ibm_newemac driver to use phylib (and other PHY/MAC questions)

[Kyle Moffett]

On Thu, Apr 30, 2009 at 5:18 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> Well... GPCS is indeed some kind of internal PHY if you like but it's
> not MDIO controlled and I don't really see a need to move it to the
> phylib as it's so specific to the NIC itself. But if you think it cleans
> the code up significantly... There are also some subtle differences in
> how the NIC is programmed when using GPCS vs. xMII

Ok, thanks, that's helpful information.  Mainly I was trying to puzzle
out why the driver sometimes resets the GPCS and other times resets
the real PHY.  It seems to be rather inconsistent when it resets
different devices.  For instance, one of the Marvell PHY's init()
helper resets the PHY, which was getting in the way of me ensuring
that the init() helpers are called after every PHY reset (because I
need to reinitialize the state that just got erased by the reset).

The challenge is working out which of the implicit ordering is
required for correct operation and which is simply an artifact of the
current implementation.


>> I'm also curious about the intent of the "mdio_instance" pointer (IE:
>> the "mdio-device" property). =C2=A0Is that used when all the PHY devices
>> are attached to the MDIO bus of only one of the (multiple) emac
>> devices?
>
> It's common especially on older SoCs using EMAC to have only one of
> the EMAC instance with an MDIO bus for configuring the PHYs. This is one
> of the reasons why I have the mutex in the low level MDIO access
> routines since 2 EMACs can try to talk to the same MDIO, and this is the
> problem I had with phylib back then which was doing everything in atomic
> contexts.

Ok, good, the current mdiobus code seems to make handling this a good
deal easier.


>> What keeps the emac_instance pointed
>> to by the "mdio_instance" from going away while the other emac chipset
>> is using it?
>
> Nothing other than those are all in a SoC and so generally don't do
> hotplug, but I suppose that with things like kvm which could potentially
> hotplug devices between partitions, one would have to get a bit more
> careful.

Yeah, I was experimenting with fiddling with the bind/unbind files in
sysfs and determined that the device-removal code is completely
broken.  As far as I can tell, any attempts to unregister one of the
emac devices cause an OOPS, even if it isn't used by another emac for
MDIO.  I may just start by nuking the broken device-removal code
entirely, and re-implement it properly once the rework is done.

Thanks for the info!

Cheers,
Kyle Moffett

Re: Porting the ibm_newemac driver to use phylib (and other PHY/MAC questions)

[Kyle Moffett]

On Thu, Apr 30, 2009 at 6:21 PM, Kyle Moffett <kyle@moffetthome.net> wrote:
>>> I'm also curious about the intent of the "mdio_instance" pointer (IE:
>>> the "mdio-device" property). =C2=A0Is that used when all the PHY device=
s
>>> are attached to the MDIO bus of only one of the (multiple) emac
>>> devices?
>>
>> It's common especially on older SoCs using EMAC to have only one of
>> the EMAC instance with an MDIO bus for configuring the PHYs. This is one
>> of the reasons why I have the mutex in the low level MDIO access
>> routines since 2 EMACs can try to talk to the same MDIO, and this is the
>> problem I had with phylib back then which was doing everything in atomic
>> contexts.
>
> Ok, good, the current mdiobus code seems to make handling this a good
> deal easier.

Ok, I've dug through the docs on the 460EPx (the CPU I'm using), and
I'd like some confirmation of the following:

*  The EMAC hardware itself internally has its own dedicated
MDIO/MDClk lines, driven by the STACR register.

*  On many/most cpus, there is only a single set of external
MDIO/MDClk pins, driven either off the ZMII bridge or the RGMII
bridge.

*  Both bridge-types have their own internal register for switching
the external MDIO/MDClk pins between the two sets of internal
EMAC<=3D>bridge links.

*  Some SoCs have both an ZMII and an RGMII bridge, and the external
MDIO/MDClk pins are only connected to one of the two bridges (How do I
know which one?  Alternatively, do I just program both and hope for
the best?).

*  Some older SoCs simply export the MDIO/MDClk pins from one of their
internal EMAC chips and don't bother with running it through the
multiplexing bridge.

Are there any SoCs which actually export the MDIO/MDClk pins from
both/all of their EMACs?

Cheers,
Kyle Moffett

Re: Porting the ibm_newemac driver to use phylib (and other PHY/MAC questions)

[Benjamin Herrenschmidt]

On Sun, 2009-05-03 at 00:26 -0400, Kyle Moffett wrote:

> Ok, I've dug through the docs on the 460EPx (the CPU I'm using), and
> I'd like some confirmation of the following:
> 
> *  The EMAC hardware itself internally has its own dedicated
> MDIO/MDClk lines, driven by the STACR register.

Yes, though not all EMACs cells have this wired to anything. For
example, the 405EP has 2 EMACs but only EMAC0 has MDIO, which is
used to control both PHYs. Later variants multiplex the MDIOs via
a programmable switch in the ZMII or RGMII though.

> *  On many/most cpus, there is only a single set of external
> MDIO/MDClk pins, driven either off the ZMII bridge or the RGMII
> bridge.

Yes, though on the very old ones, the ZMII bridge is effectively
invisible (if it exists at all) and only EMAC0 MDIO pins are wired out.

> *  Both bridge-types have their own internal register for switching
> the external MDIO/MDClk pins between the two sets of internal
> EMAC<=>bridge links.

Yes.

> *  Some SoCs have both an ZMII and an RGMII bridge, and the external
> MDIO/MDClk pins are only connected to one of the two bridges (How do I
> know which one?  Alternatively, do I just program both and hope for
> the best?).

That's been my approach so far :-)

> *  Some older SoCs simply export the MDIO/MDClk pins from one of their
> internal EMAC chips and don't bother with running it through the
> multiplexing bridge.

Yes.

> Are there any SoCs which actually export the MDIO/MDClk pins from
> both/all of their EMACs?

I don't know of such a beast.

Cheers,
Ben.

> Cheers,
> Kyle Moffett