Messages in this thread | | | Subject | Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver | From | Florian Fainelli <> | Date | Tue, 10 May 2016 16:26:00 -0700 |
| |
On 05/10/2016 04:18 PM, Timur Tabi wrote: > Florian Fainelli wrote: >> Are you utilizing the PHYLIB APIs properly? You need at least a >> phy_start() to start the PHY state machine, and an adjust_link callback >> to be provided to phy_connect() (or of_phy_connect()) to manage link >> state changes. And that's the very basic minimum here, there could be >> additional APIs that you may end up using. >> >> There are tons of example in tree of drivers doing this, bcmgenet, >> bcmsysport, tg3 etc. > > Thank you. I think I finally got phylib working, more or less. > > Unfortunately, it seems I have some kind of race condition. The driver > has a lot that's wrong with it, and I'm trying to fix it all. One crazy > the driver does is it create a workqueue to handle a lot of the tasks > that would normally be handled in the interrupt handler itself.
That sounds like a typicall top half/bottom half split, fair enough.
> > With phylib support, I know my driver can call phy_mac_interrupt() when > it gets a link status change interrupt. I then have an .adjust_link > callback which starts or stops the mac accordingly.
The Ethernet MAC should be started in ndo_open() and stopped in ndo_close(), in between, there are link state changes, but you are not supposed to stop or start your Ethernet MAC and its DMA for instance during link change, if that is a HW requirement, your HW is pretty funky.
> > My problem is that I'm not really sure what adjust_link is supposed to > be doing.
Well, it's pretty simple, it is about re-configuring your Ethernet MAC based on what the PHY link state mandates: duplex, pause, speed changes, EEE etc is what this callback is supposed to take care of, at the Ethernet MAC level.
> In addition, it seems that I need to keep the workqueue > running, otherwise the interface will not function. I bring the > interface up, and the driver reports success, but pings do not work. > > I'm getting really frustrated. The sample code isn't really helping a > whole lot, because I lack a fundamental understanding of what needs to > be done. None of the documentation I've read is helpful, and I don't > know how to debug it.
Seriously, no documentation is helpful? The PHY library seems pretty well documented to me, but I suppose I have a bias, oh, and patches are welcome of course.
> > Can you give me some advice on how to debug this?
Take a look at drivers/net/ethernet/broadcom/genet/bcmgenet.c and see how it deals with managing link state changes for instance. The code is pretty straight forward: link interrupt (and other causes) trigger a workqueue schedule, which then processes link state changes and calls phy_mac_interrupt(), which in turn makes the PHY library adjust the interface carrier state. -- Florian
| |