Re: [PATCH 10/13] ftgmac100: Add a reset task and use it for link changes

2017-04-02 Thread Benjamin Herrenschmidt
On Mon, 2017-04-03 at 07:24 +1000, Benjamin Herrenschmidt wrote: > On Sun, 2017-04-02 at 20:42 +0200, Andrew Lunn wrote: > > > Have you run lockdep tests on this? > > Good idea. For some reason I run it regularly on powerpc but it never > occurred to me to try it on that 800Mhz ARM11 :-) That wa

Re: [PATCH 10/13] ftgmac100: Add a reset task and use it for link changes

2017-04-02 Thread Benjamin Herrenschmidt
On Sun, 2017-04-02 at 20:42 +0200, Andrew Lunn wrote: > Have you run lockdep tests on this? Good idea. For some reason I run it regularly on powerpc but it never occurred to me to try it on that 800Mhz ARM11 :-) Cheers, Ben.

Re: [PATCH 10/13] ftgmac100: Add a reset task and use it for link changes

2017-04-02 Thread Benjamin Herrenschmidt
On Sun, 2017-04-02 at 20:42 +0200, Andrew Lunn wrote: > Have you run lockdep tests on this? I think it is normal to take the > rtnl lock first, then the phydev lock. Try some ethtool operations > and see if you get a splat. You are right. I actually realized that was wrong last night ;-) It will s

Re: [PATCH 10/13] ftgmac100: Add a reset task and use it for link changes

2017-04-02 Thread Benjamin Herrenschmidt
On Sun, 2017-04-02 at 13:35 +1000, Benjamin Herrenschmidt wrote: > +   /* Block PHY polling */ > +   if (netdev->phydev) > +   mutex_lock(&netdev->phydev->lock); > + > +   rtnl_lock(); Self-given brown paper bag, those two need to be taken the other way around. I'll send an

Re: [PATCH 10/13] ftgmac100: Add a reset task and use it for link changes

2017-04-02 Thread Andrew Lunn
> +static void ftgmac100_reset_task(struct work_struct *work) > +{ > + struct ftgmac100 *priv = container_of(work, struct ftgmac100, > + reset_task); > + struct net_device *netdev = priv->netdev; > + int err; > + > + netdev_dbg(netdev, "Rese

[PATCH 10/13] ftgmac100: Add a reset task and use it for link changes

2017-04-01 Thread Benjamin Herrenschmidt
Link speed changes require a full HW reset. This isn't done properly at the moment. It will involve delays and thus isn't suitable to do from the link poll callback. So let's create a reset_task that we can queue up when the link changes. It will be useful for various cases of error handling as we