net: pxa168_eth: add netconsole support

This implements ndo_poll_controller callback which is necessary to
enable netconsole.

Signed-off-by: Alexander Monakov <amonakov@ispras.ru>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
---
Hello,

I'm using this to enable netconsole on a consumer device built around the
Marvell Berlin BG2CD SoC.

Thanks.
Alexander

 drivers/net/ethernet/marvell/pxa168_eth.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

From: Alexander Monakov <amonakov@ispras.ru>
Date: Sat, 27 Jan 2018 23:29:07 +0300

> @@ -1362,6 +1362,14 @@ static int pxa168_eth_do_ioctl(struct net_device *dev, struct ifreq *ifr,
>  	return -EOPNOTSUPP;
>  }
>  
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> +static void pxa168_eth_netpoll(struct net_device *dev)
> +{
> +	struct pxa168_eth_private *pep = netdev_priv(dev);
> +	napi_schedule(&pep->napi);
> +}
> +#endif

This definitely is not sufficient.

Look at what other drivers do.

You have to invoke the interrupt handler with the device's interrupts disabled,
collect the events, and most importantly mask chip interrupts before scheduling
the NAPI instance.

Thank you.

> > +#ifdef CONFIG_NET_POLL_CONTROLLER
> > +static void pxa168_eth_netpoll(struct net_device *dev)
> > +{
> > +	struct pxa168_eth_private *pep = netdev_priv(dev);
> > +	napi_schedule(&pep->napi);
> > +}
> > +#endif
> 
> This definitely is not sufficient.
> 
> Look at what other drivers do.

Sorry, I did that, and got confused because some drivers bracket the interrupt
handler with disable_irq/enable_irq, some other drivers use local_irq_save/
local_irq_restore (which seems like a no-op because netconsole already uses
spin_lock_irqsave and netpoll_send_udp checks irqs_disabled), and a few drivers
call bare napi_schedule.

> You have to invoke the interrupt handler with the device's interrupts disabled,

Is this required for correctness? I have to admit I'm unsure why.

> collect the events, and most importantly mask chip interrupts before scheduling
> the NAPI instance.

And is this a matter of efficiency (not calling napi_schedule when there's
nothing to do and not keeping interrupts enabled for its duration), or also
a matter of correctness?

Sorry I'm not sending a revised patch yet, but without a firm understanding
of why changes are needed that would be a bit of a sin.

Thanks.
Alexander

From: Alexander Monakov <amonakov@ispras.ru>
Date: Wed, 31 Jan 2018 17:05:47 +0300 (MSK)

> And is this a matter of efficiency (not calling napi_schedule when there's
> nothing to do and not keeping interrupts enabled for its duration), or also
> a matter of correctness?

If you don't mask interrupts properly around scheduling and finishing
NAPI poll, you can lose events so it is a matter of correctness.