[PATCH] ucc_geth: add support for netpoll

[PATCH] ucc_geth: add support for netpoll

[Anton Vorontsov]

This patch adds netpoll support for the QE UCC Gigabit Ethernet
driver. The approach is very similar to the gianfar driver.

Tested using netconsole.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/net/ucc_geth.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 18a6f48..06807ce 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3691,6 +3691,22 @@ static irqreturn_t ucc_geth_irq_handler(int irq, void *info)
 	return IRQ_HANDLED;
 }
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+/*
+ * Polling 'interrupt' - used by things like netconsole to send skbs
+ * without having to re-enable interrupts. It's not called while
+ * the interrupt routine is executing.
+ */
+static void ucc_netpoll(struct net_device *dev)
+{
+	struct ucc_geth_private *ugeth = netdev_priv(dev);
+
+	disable_irq(ugeth->ug_info->uf_info.irq);
+	ucc_geth_irq_handler(ugeth->ug_info->uf_info.irq, dev);
+	enable_irq(ugeth->ug_info->uf_info.irq);
+}
+#endif /* CONFIG_NET_POLL_CONTROLLER */
+
 /* Called when something needs to use the ethernet device */
 /* Returns 0 for success. */
 static int ucc_geth_open(struct net_device *dev)
@@ -3969,6 +3985,9 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
 	dev->poll = ucc_geth_poll;
 	dev->weight = UCC_GETH_DEV_WEIGHT;
 #endif				/* CONFIG_UGETH_NAPI */
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	dev->poll_controller = ucc_netpoll;
+#endif
 	dev->stop = ucc_geth_close;
 	dev->get_stats = ucc_geth_get_stats;
 //    dev->change_mtu = ucc_geth_change_mtu;
-- 
1.5.0.6

Re: [PATCH] ucc_geth: add support for netpoll

[Sergei Shtylyov]

Hello.

Anton Vorontsov wrote:

> This patch adds netpoll support for the QE UCC Gigabit Ethernet
> driver. The approach is very similar to the gianfar driver.

    It's rather contrarywise -- this is standard approach and gianfar with its 
3 TSEC IRQs has a quite non-standard poll_controller() implementation.

> Tested using netconsole.

    KGDBoE is considered a better test (I hope you've also tested with it).

> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
> index 18a6f48..06807ce 100644
> --- a/drivers/net/ucc_geth.c
> +++ b/drivers/net/ucc_geth.c
> @@ -3691,6 +3691,22 @@ static irqreturn_t ucc_geth_irq_handler(int irq, void *info)
>  	return IRQ_HANDLED;
>  }
>  
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> +/*
> + * Polling 'interrupt' - used by things like netconsole to send skbs
> + * without having to re-enable interrupts. It's not called while
> + * the interrupt routine is executing.
> + */
> +static void ucc_netpoll(struct net_device *dev)
> +{
> +	struct ucc_geth_private *ugeth = netdev_priv(dev);
> +
> +	disable_irq(ugeth->ug_info->uf_info.irq);
> +	ucc_geth_irq_handler(ugeth->ug_info->uf_info.irq, dev);
> +	enable_irq(ugeth->ug_info->uf_info.irq);

    Why not make it less complex (for a reader and gcc too :-) ?

	struct ucc_geth_private *ugeth = netdev_priv(dev);
	int irq = ugeth->ug_info->uf_info.irq;

	disable_irq(irq);
	ucc_geth_irq_handler(irq, dev);
	enable_irq(irq);

> +}
> +#endif /* CONFIG_NET_POLL_CONTROLLER */
> +
>  /* Called when something needs to use the ethernet device */
>  /* Returns 0 for success. */
>  static int ucc_geth_open(struct net_device *dev)

WBR, Sergei

Re: [PATCH] ucc_geth: add support for netpoll

[Anton Vorontsov]

On Sat, Oct 27, 2007 at 05:09:51PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> Anton Vorontsov wrote:
> 
> > This patch adds netpoll support for the QE UCC Gigabit Ethernet
> > driver. The approach is very similar to the gianfar driver.
> 
>     It's rather contrarywise -- this is standard approach and gianfar with its 
> 3 TSEC IRQs has a quite non-standard poll_controller() implementation.

Oh.. well, right -- gianfar a bit more comlex in that regard.

> 
> > Tested using netconsole.
> 
>     KGDBoE is considered a better test (I hope you've also tested with it).

At the time of posting it was tested using netconsole only, a few
days later it's was tested using KGDBoE also. So, it works indeed.

> > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
> > index 18a6f48..06807ce 100644
> > --- a/drivers/net/ucc_geth.c
> > +++ b/drivers/net/ucc_geth.c
> > @@ -3691,6 +3691,22 @@ static irqreturn_t ucc_geth_irq_handler(int irq, void *info)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > +#ifdef CONFIG_NET_POLL_CONTROLLER
> > +/*
> > + * Polling 'interrupt' - used by things like netconsole to send skbs
> > + * without having to re-enable interrupts. It's not called while
> > + * the interrupt routine is executing.
> > + */
> > +static void ucc_netpoll(struct net_device *dev)
> > +{
> > +	struct ucc_geth_private *ugeth = netdev_priv(dev);
> > +
> > +	disable_irq(ugeth->ug_info->uf_info.irq);
> > +	ucc_geth_irq_handler(ugeth->ug_info->uf_info.irq, dev);
> > +	enable_irq(ugeth->ug_info->uf_info.irq);
> 
>     Why not make it less complex (for a reader and gcc too :-) ?

Yup, I'm agree here but it's too late. Again. ;-)

This patch already accepted into the -mm (a week or so after the
silence), so.. now I'd rather not bother Andrew with such really
cosmetic changes. But if Jeff would directly apply modfied patch,
I'll send it. ;-)


Anyhow, I'm sincerely appreciate your comments.

Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

RE: [PATCH] ucc_geth: add support for netpoll

[Li Yang-r58472]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbou@mail.ru]=20
> Sent: Saturday, October 27, 2007 10:38 PM
> To: Sergei Shtylyov
> Cc: Anton Vorontsov; netdev@vger.kernel.org; Li Yang-r58472;=20
> linux-kernel@vger.kernel.org; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] ucc_geth: add support for netpoll
>=20
> On Sat, Oct 27, 2007 at 05:09:51PM +0400, Sergei Shtylyov wrote:
> > Hello.
> >=20
> > Anton Vorontsov wrote:
> >=20
> > > This patch adds netpoll support for the QE UCC Gigabit Ethernet=20
> > > driver. The approach is very similar to the gianfar driver.
> >=20
> >     It's rather contrarywise -- this is standard approach=20
> and gianfar=20
> > with its
> > 3 TSEC IRQs has a quite non-standard poll_controller()=20
> implementation.
>=20
> Oh.. well, right -- gianfar a bit more comlex in that regard.
>=20
> >=20
> > > Tested using netconsole.
> >=20
> >     KGDBoE is considered a better test (I hope you've also=20
> tested with it).
>=20
> At the time of posting it was tested using netconsole only, a=20
> few days later it's was tested using KGDBoE also. So, it works indeed.
>=20
> > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>=20
> diff --git=20
> > > a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c index=20
> > > 18a6f48..06807ce 100644
> > > --- a/drivers/net/ucc_geth.c
> > > +++ b/drivers/net/ucc_geth.c
> > > @@ -3691,6 +3691,22 @@ static irqreturn_t=20
> ucc_geth_irq_handler(int irq, void *info)
> > >  	return IRQ_HANDLED;
> > >  }
> > > =20
> > > +#ifdef CONFIG_NET_POLL_CONTROLLER
> > > +/*
> > > + * Polling 'interrupt' - used by things like netconsole to send=20
> > > +skbs
> > > + * without having to re-enable interrupts. It's not called while
> > > + * the interrupt routine is executing.
> > > + */
> > > +static void ucc_netpoll(struct net_device *dev) {
> > > +	struct ucc_geth_private *ugeth =3D netdev_priv(dev);
> > > +
> > > +	disable_irq(ugeth->ug_info->uf_info.irq);
> > > +	ucc_geth_irq_handler(ugeth->ug_info->uf_info.irq, dev);
> > > +	enable_irq(ugeth->ug_info->uf_info.irq);
> >=20
> >     Why not make it less complex (for a reader and gcc too :-) ?
>=20
> Yup, I'm agree here but it's too late. Again. ;-)
>=20
> This patch already accepted into the -mm (a week or so after=20
> the silence), so.. now I'd rather not bother Andrew with such=20
> really cosmetic changes. But if Jeff would directly apply=20
> modfied patch, I'll send it. ;-)

Oops.  The original patch happened to hit the Junk mail box. :(  I think
the patch is good to merge after the cosmetic change.  I can do it in
next pull request to Jeff.

Thanks
- Leo

Re: [PATCH] ucc_geth: add support for netpoll

[Anton Vorontsov]

On Mon, Oct 29, 2007 at 02:12:07PM +0800, Li Yang-r58472 wrote:
[...]
> > > > +#ifdef CONFIG_NET_POLL_CONTROLLER
> > > > +/*
> > > > + * Polling 'interrupt' - used by things like netconsole to send 
> > > > +skbs
> > > > + * without having to re-enable interrupts. It's not called while
> > > > + * the interrupt routine is executing.
> > > > + */
> > > > +static void ucc_netpoll(struct net_device *dev) {
> > > > +	struct ucc_geth_private *ugeth = netdev_priv(dev);
> > > > +
> > > > +	disable_irq(ugeth->ug_info->uf_info.irq);
> > > > +	ucc_geth_irq_handler(ugeth->ug_info->uf_info.irq, dev);
> > > > +	enable_irq(ugeth->ug_info->uf_info.irq);
> > > 
> > >     Why not make it less complex (for a reader and gcc too :-) ?
> > 
> > Yup, I'm agree here but it's too late. Again. ;-)
> > 
> > This patch already accepted into the -mm (a week or so after 
> > the silence), so.. now I'd rather not bother Andrew with such 
> > really cosmetic changes. But if Jeff would directly apply 
> > modfied patch, I'll send it. ;-)
> 
> Oops.  The original patch happened to hit the Junk mail box. :(

That one as well? http://lkml.org/lkml/2007/10/11/128

> I think
> the patch is good to merge after the cosmetic change.  I can do it in
> next pull request to Jeff.

Ok, great. Thanks.

Here it is:

- - - -
From: Anton Vorontsov <avorontsov@ru.mvista.com>
Subject: [PATCH] ucc_geth: add support for netpoll

This patch adds netpoll support for the QE UCC Gigabit Ethernet
driver. Tested using netconsole and KGDBoE.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/net/ucc_geth.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index bec413b..94e78d8 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3678,6 +3678,23 @@ static irqreturn_t ucc_geth_irq_handler(int irq, void *info)
 	return IRQ_HANDLED;
 }
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+/*
+ * Polling 'interrupt' - used by things like netconsole to send skbs
+ * without having to re-enable interrupts. It's not called while
+ * the interrupt routine is executing.
+ */
+static void ucc_netpoll(struct net_device *dev)
+{
+	struct ucc_geth_private *ugeth = netdev_priv(dev);
+	int irq = ugeth->ug_info->uf_info.irq;
+
+	disable_irq(irq);
+	ucc_geth_irq_handler(irq, dev);
+	enable_irq(irq);
+}
+#endif /* CONFIG_NET_POLL_CONTROLLER */
+
 /* Called when something needs to use the ethernet device */
 /* Returns 0 for success. */
 static int ucc_geth_open(struct net_device *dev)
@@ -3963,6 +3980,9 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
 #ifdef CONFIG_UGETH_NAPI
 	netif_napi_add(dev, &ugeth->napi, ucc_geth_poll, UCC_GETH_DEV_WEIGHT);
 #endif				/* CONFIG_UGETH_NAPI */
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	dev->poll_controller = ucc_netpoll;
+#endif
 	dev->stop = ucc_geth_close;
 //    dev->change_mtu = ucc_geth_change_mtu;
 	dev->mtu = 1500;
-- 
1.5.2.2

Re: [PATCH] ucc_geth: add support for netpoll

[Anton Vorontsov]

On Mon, Oct 29, 2007 at 03:17:44PM +0300, Anton Vorontsov wrote:
[...]
> > Oops.  The original patch happened to hit the Junk mail box. :(
> 
> That one as well? http://lkml.org/lkml/2007/10/11/128
> 
> > I think
> > the patch is good to merge after the cosmetic change.  I can do it in
> > next pull request to Jeff.
> 
> Ok, great. Thanks.

I'm wondering if you missed that email again. Maybe your mail
client/server doing weird things with emails from @ru.mvista.com?

Thanks.

> Here it is:
> 
> - - - -
> From: Anton Vorontsov <avorontsov@ru.mvista.com>
> Subject: [PATCH] ucc_geth: add support for netpoll
> 
> This patch adds netpoll support for the QE UCC Gigabit Ethernet
> driver. Tested using netconsole and KGDBoE.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
>  drivers/net/ucc_geth.c |   20 ++++++++++++++++++++
>  1 files changed, 20 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
> index bec413b..94e78d8 100644
> --- a/drivers/net/ucc_geth.c
> +++ b/drivers/net/ucc_geth.c
> @@ -3678,6 +3678,23 @@ static irqreturn_t ucc_geth_irq_handler(int irq, void *info)
>  	return IRQ_HANDLED;
>  }
>  
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> +/*
> + * Polling 'interrupt' - used by things like netconsole to send skbs
> + * without having to re-enable interrupts. It's not called while
> + * the interrupt routine is executing.
> + */
> +static void ucc_netpoll(struct net_device *dev)
> +{
> +	struct ucc_geth_private *ugeth = netdev_priv(dev);
> +	int irq = ugeth->ug_info->uf_info.irq;
> +
> +	disable_irq(irq);
> +	ucc_geth_irq_handler(irq, dev);
> +	enable_irq(irq);
> +}
> +#endif /* CONFIG_NET_POLL_CONTROLLER */
> +
>  /* Called when something needs to use the ethernet device */
>  /* Returns 0 for success. */
>  static int ucc_geth_open(struct net_device *dev)
> @@ -3963,6 +3980,9 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
>  #ifdef CONFIG_UGETH_NAPI
>  	netif_napi_add(dev, &ugeth->napi, ucc_geth_poll, UCC_GETH_DEV_WEIGHT);
>  #endif				/* CONFIG_UGETH_NAPI */
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> +	dev->poll_controller = ucc_netpoll;
> +#endif
>  	dev->stop = ucc_geth_close;
>  //    dev->change_mtu = ucc_geth_change_mtu;
>  	dev->mtu = 1500;
> -- 
> 1.5.2.2

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

RE: [PATCH] ucc_geth: add support for netpoll

[Li Yang-r58472]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbou@mail.ru]=20
> Sent: Thursday, November 01, 2007 5:59 AM
> To: Li Yang-r58472
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;=20
> linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] ucc_geth: add support for netpoll
>=20
> On Mon, Oct 29, 2007 at 03:17:44PM +0300, Anton Vorontsov wrote:
> [...]
> > > Oops.  The original patch happened to hit the Junk mail box. :(
> >=20
> > That one as well? http://lkml.org/lkml/2007/10/11/128
> >=20
> > > I think
> > > the patch is good to merge after the cosmetic change.  I=20
> can do it=20
> > > in next pull request to Jeff.
> >=20
> > Ok, great. Thanks.
>=20
> I'm wondering if you missed that email again. Maybe your mail=20
> client/server doing weird things with emails from @ru.mvista.com?

No.  I have explicitly add you to the whitelist. :) Please be patient,
isn't this patch a new feature which can only be integrated in the merge
window?  Thanks.

- Leo

Re: [PATCH] ucc_geth: add support for netpoll

[Anton Vorontsov]

On Thu, Nov 01, 2007 at 10:33:24AM +0800, Li Yang-r58472 wrote:
> > -----Original Message-----
> > From: Anton Vorontsov [mailto:cbou@mail.ru] 
> > Sent: Thursday, November 01, 2007 5:59 AM
> > To: Li Yang-r58472
> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; 
> > linuxppc-dev@ozlabs.org
> > Subject: Re: [PATCH] ucc_geth: add support for netpoll
> > 
> > On Mon, Oct 29, 2007 at 03:17:44PM +0300, Anton Vorontsov wrote:
> > [...]
> > > > Oops.  The original patch happened to hit the Junk mail box. :(
> > > 
> > > That one as well? http://lkml.org/lkml/2007/10/11/128
> > > 
> > > > I think
> > > > the patch is good to merge after the cosmetic change.  I 
> > can do it 
> > > > in next pull request to Jeff.
> > > 
> > > Ok, great. Thanks.
> > 
> > I'm wondering if you missed that email again. Maybe your mail 
> > client/server doing weird things with emails from @ru.mvista.com?
> 
> No.  I have explicitly add you to the whitelist. :)

Hehe, thanks. ;-)

> Please be patient,
> isn't this patch a new feature which can only be integrated in the merge
> window?

Sure it is. I didn't mean to "hurry up" you, of course not.

Just wondered if you've solved issues with getting my emails. Such
wonders are quite normal if there was a precedent lately. ;-)


Sorry for troubling you,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2