Hello,
I'm currently fiddling with a custom embedded prototype board using
the ibm_newemac driver with some currently-unsupported PHYs. Those
PHYs *are* supported by phylib, but the emac driver seems to have its
own PHY layer cribbed from the sungem driver. I'm curious if there's
some particular reason it hasn't been ported (aside from "nobody has
bothered yet").
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. As 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. 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?
As an aside, would there be any interest in adding an ethtool option
to enable the features of many PHYs which allow them to transmit data
even if their receive port has no link (specifically fiber cards, but
a few copper cards can do something similar). From a cursory glance,
a third "duplex" configuration option would seem to make the most
sense from an end-user point of view. Specifically, the existing
options are "full" (simultaneous transmit/receive), "half"
(time-multiplexed transmit/receive). Perhaps a "tx" or "tx-only"
option would be sensible? Now, the receiving card probably doesn't
need an "rx-only" mode as "full" should do that just fine; though
perhaps some cards with unidirectional-link-detection would use
"rx-only" to intentionally disable that support. Once an extra couple
constants are added to linux/ethtool.h and the appropriate config
options added to ethtool, it should just be up to the drivers to
detect and handle those config options, no?
Comments and suggestions much appreciated.
Cheers,
Kyle Moffett
On Fri, Apr 17, 2009 at 8:32 PM, Kyle Moffett <[email protected]> wrote:
> Hello,
>
> I'm currently fiddling with a custom embedded prototype board using
> the ibm_newemac driver with some currently-unsupported PHYs. ?Those
> PHYs *are* supported by phylib, but the emac driver seems to have its
> own PHY layer cribbed from the sungem driver. ?I'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. ?As 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. ?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.
josh
On Mon, Apr 20, 2009 at 8:29 AM, Josh Boyer <[email protected]> wrote:
> On Fri, Apr 17, 2009 at 8:32 PM, Kyle Moffett <[email protected]> wrote:
>> Hello,
>>
>> I'm currently fiddling with a custom embedded prototype board using
>> the ibm_newemac driver with some currently-unsupported PHYs. Those
>> PHYs *are* supported by phylib, but the emac driver seems to have its
>> own PHY layer cribbed from the sungem driver. I'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).
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
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.
On Wed, Apr 22, 2009 at 4:21 AM, Benjamin Herrenschmidt
<[email protected]> 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. 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.
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
On Thu, Apr 30, 2009 at 9:04 AM, Kyle Moffett <[email protected]> wrote:
> On Wed, Apr 22, 2009 at 4:21 AM, Benjamin Herrenschmidt
> <[email protected]> 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. ?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.
>
> 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).
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=linux/kernel/git/davem/net-next-2.6.git;a=commit;h=8bc487d150b939e69830c39322df4ee486efe381
and here is an example of a driver change:
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commit;h=1dd2d06c0459a2f1bffc56765e3cc57427818867
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
On Thu, Apr 30, 2009 at 11:11 AM, Grant Likely
<[email protected]> 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. See here:
>
> http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commit;h=8bc487d150b939e69830c39322df4ee486efe381
>
> and here is an example of a driver change:
>
> http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commit;h=1dd2d06c0459a2f1bffc56765e3cc57427818867
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 = <4>", "phy-mask = <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
> 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.
On Thu, Apr 30, 2009 at 5:18 PM, Benjamin Herrenschmidt
<[email protected]> 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). 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.
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