[net-next,3/3] epic100: allow nesting of ethtool_ops begin() and complete()

Unlike most networking drivers using begin() and complete() ethtool_ops
callbacks to resume a device which is down and suspend it again when done,
epic100 does not use standard refcounted infrastructure but sets device
sleep state directly.

With the introduction of netlink ethtool interface, we may have nested
begin-complete blocks so that inner complete() would put the device back to
sleep for the rest of the outer block.

To avoid rewriting an old and not very actively developed driver, just add
a nesting counter and only perform resume and suspend on the outermost
level.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 drivers/net/ethernet/smsc/epic100.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

> @@ -1435,8 +1436,10 @@ static int ethtool_begin(struct net_device *dev)
>  	struct epic_private *ep = netdev_priv(dev);
>  	void __iomem *ioaddr = ep->ioaddr;
>  
> +	if (ep->ethtool_ops_nesting == U32_MAX)
> +		return -EBUSY;
>  	/* power-up, if interface is down */
> -	if (!netif_running(dev)) {
> +	if (ep->ethtool_ops_nesting++ && !netif_running(dev)) {
>  		ew32(GENCTL, 0x0200);
>  		ew32(NVCTL, (er32(NVCTL) & ~0x003c) | 0x4800);
>  	}

Hi Michal

In the via-velocity you added:

+       if (vptr->ethtool_ops_nesting == U32_MAX)
+               return -EBUSY;
+       if (!vptr->ethtool_ops_nesting++ && !netif_running(dev))
                velocity_set_power_state(vptr, PCI_D0);
        return 0;

These two fragments differ by a ! . Is that correct?

      Andrew

On Sun, Jan 05, 2020 at 11:08:32PM +0100, Andrew Lunn wrote:
> > @@ -1435,8 +1436,10 @@ static int ethtool_begin(struct net_device *dev)
> >  	struct epic_private *ep = netdev_priv(dev);
> >  	void __iomem *ioaddr = ep->ioaddr;
> >  
> > +	if (ep->ethtool_ops_nesting == U32_MAX)
> > +		return -EBUSY;
> >  	/* power-up, if interface is down */
> > -	if (!netif_running(dev)) {
> > +	if (ep->ethtool_ops_nesting++ && !netif_running(dev)) {
> >  		ew32(GENCTL, 0x0200);
> >  		ew32(NVCTL, (er32(NVCTL) & ~0x003c) | 0x4800);
> >  	}
> 
> Hi Michal
> 
> In the via-velocity you added:
> 
> +       if (vptr->ethtool_ops_nesting == U32_MAX)
> +               return -EBUSY;
> +       if (!vptr->ethtool_ops_nesting++ && !netif_running(dev))
>                 velocity_set_power_state(vptr, PCI_D0);
>         return 0;
> 
> These two fragments differ by a ! . Is that correct?

You are right, thank you for catching it. This should be 

	if (!ep->ethtool_ops_nesting++ && !netif_running(dev)) {

as well, we only want to wake the device up in the first (outermost)
->begin(). (It would probably do no harm to do it each time but not
doing it in the first would be wrong.)

I'll send v2 in a moment.

Michal