netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch] fix suspend/resume on b44
@ 2005-09-20 13:28 Pavel Machek
  2005-09-20 23:26 ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Pavel Machek @ 2005-09-20 13:28 UTC (permalink / raw)
  To: Andrew Morton, Jeff Garzik, Netdev list

Fix suspend/resume on b44 by freeing/reacquiring irq. Otherwise it
hangs on resume.

Signed-off-by: Pavel Machek <pavel@suse.cz>

---
commit 7bdc8fc378f053bd4eb4210beb1d494485318512
tree 6e5679697b11eb70b73ff5275aafe7c34a90ffef
parent 17cd36a6d0fc36b61fa558cade6a98a3e99a6992
author <pavel@amd.(none)> Tue, 20 Sep 2005 15:26:37 +0200
committer <pavel@amd.(none)> Tue, 20 Sep 2005 15:26:37 +0200

 drivers/net/b44.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/net/b44.c b/drivers/net/b44.c
--- a/drivers/net/b44.c
+++ b/drivers/net/b44.c
@@ -1930,6 +1930,8 @@ static int b44_suspend(struct pci_dev *p
 	b44_free_rings(bp);
 
 	spin_unlock_irq(&bp->lock);
+
+	free_irq(dev->irq, dev);
 	pci_disable_device(pdev);
 	return 0;
 }
@@ -1946,6 +1948,9 @@ static int b44_resume(struct pci_dev *pd
 	if (!netif_running(dev))
 		return 0;
 
+	if (request_irq(dev->irq, b44_interrupt, SA_SHIRQ, dev->name, dev))
+		printk(KERN_ERR PFX "%s: request_irq failed\n", dev->name);
+
 	spin_lock_irq(&bp->lock);
 
 	b44_init_rings(bp);

-- 
if you have sharp zaurus hardware you don't need... you know my address

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] fix suspend/resume on b44
  2005-09-20 13:28 [patch] fix suspend/resume on b44 Pavel Machek
@ 2005-09-20 23:26 ` Andrew Morton
  2005-09-21 10:20   ` Pavel Machek
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2005-09-20 23:26 UTC (permalink / raw)
  To: Pavel Machek; +Cc: jgarzik, netdev

Pavel Machek <pavel@ucw.cz> wrote:
>
> Fix suspend/resume on b44 by freeing/reacquiring irq. Otherwise it
> hangs on resume.
> 
> ...
> diff --git a/drivers/net/b44.c b/drivers/net/b44.c
> --- a/drivers/net/b44.c
> +++ b/drivers/net/b44.c
> @@ -1930,6 +1930,8 @@ static int b44_suspend(struct pci_dev *p
>  	b44_free_rings(bp);
>  
>  	spin_unlock_irq(&bp->lock);
> +
> +	free_irq(dev->irq, dev);
>  	pci_disable_device(pdev);
>  	return 0;
>  }
> @@ -1946,6 +1948,9 @@ static int b44_resume(struct pci_dev *pd
>  	if (!netif_running(dev))
>  		return 0;
>  
> +	if (request_irq(dev->irq, b44_interrupt, SA_SHIRQ, dev->name, dev))
> +		printk(KERN_ERR PFX "%s: request_irq failed\n", dev->name);
> +
>  	spin_lock_irq(&bp->lock);
>  
>  	b44_init_rings(bp);
> 

Why does it hang on suspend/resume?

This came up a while back and iirc we decided that adding free_irq() to
every ->suspend() handler in the world was the wrong thing to do.  Do I
misremember?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] fix suspend/resume on b44
  2005-09-20 23:26 ` Andrew Morton
@ 2005-09-21 10:20   ` Pavel Machek
  2005-09-21 10:36     ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Pavel Machek @ 2005-09-21 10:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: jgarzik, netdev

Hi!

> > diff --git a/drivers/net/b44.c b/drivers/net/b44.c
> > --- a/drivers/net/b44.c
> > +++ b/drivers/net/b44.c
> > @@ -1930,6 +1930,8 @@ static int b44_suspend(struct pci_dev *p
> >  	b44_free_rings(bp);
> >  
> >  	spin_unlock_irq(&bp->lock);
> > +
> > +	free_irq(dev->irq, dev);
> >  	pci_disable_device(pdev);
> >  	return 0;
> >  }
> > @@ -1946,6 +1948,9 @@ static int b44_resume(struct pci_dev *pd
> >  	if (!netif_running(dev))
> >  		return 0;
> >  
> > +	if (request_irq(dev->irq, b44_interrupt, SA_SHIRQ, dev->name, dev))
> > +		printk(KERN_ERR PFX "%s: request_irq failed\n", dev->name);
> > +
> >  	spin_lock_irq(&bp->lock);
> >  
> >  	b44_init_rings(bp);
> > 
> 
> Why does it hang on suspend/resume?
> 
> This came up a while back and iirc we decided that adding free_irq() to
> every ->suspend() handler in the world was the wrong thing to do.  Do I
> misremember?

No, you remember right, but b44 needed that free_irq/request_irq even
because those ACPI changes. I'm not exactly sure why, something went
very wrong otherwise.

								Pavel
-- 
Boycott Kodak -- for their patent abuse against Java.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] fix suspend/resume on b44
  2005-09-21 10:20   ` Pavel Machek
@ 2005-09-21 10:36     ` Andrew Morton
  2005-09-21 21:13       ` Pavel Machek
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2005-09-21 10:36 UTC (permalink / raw)
  To: Pavel Machek; +Cc: jgarzik, netdev

Pavel Machek <pavel@suse.cz> wrote:
>
> Hi!
> 
> > > diff --git a/drivers/net/b44.c b/drivers/net/b44.c
> > > --- a/drivers/net/b44.c
> > > +++ b/drivers/net/b44.c
> > > @@ -1930,6 +1930,8 @@ static int b44_suspend(struct pci_dev *p
> > >  	b44_free_rings(bp);
> > >  
> > >  	spin_unlock_irq(&bp->lock);
> > > +
> > > +	free_irq(dev->irq, dev);
> > >  	pci_disable_device(pdev);
> > >  	return 0;
> > >  }
> > > @@ -1946,6 +1948,9 @@ static int b44_resume(struct pci_dev *pd
> > >  	if (!netif_running(dev))
> > >  		return 0;
> > >  
> > > +	if (request_irq(dev->irq, b44_interrupt, SA_SHIRQ, dev->name, dev))
> > > +		printk(KERN_ERR PFX "%s: request_irq failed\n", dev->name);
> > > +
> > >  	spin_lock_irq(&bp->lock);
> > >  
> > >  	b44_init_rings(bp);
> > > 
> > 
> > Why does it hang on suspend/resume?
> > 
> > This came up a while back and iirc we decided that adding free_irq() to
> > every ->suspend() handler in the world was the wrong thing to do.  Do I
> > misremember?
> 
> No, you remember right, but b44 needed that free_irq/request_irq even
> because those ACPI changes. I'm not exactly sure why, something went
> very wrong otherwise.

Well I guess we should work out what went wrong ;)

What are the symptoms?  Screaming interrupt?  Can't immediately see why. 
Does the screaming interrupt detetor trigger and disable the IRQ Line?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] fix suspend/resume on b44
  2005-09-21 10:36     ` Andrew Morton
@ 2005-09-21 21:13       ` Pavel Machek
  2005-09-21 21:22         ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Pavel Machek @ 2005-09-21 21:13 UTC (permalink / raw)
  To: Andrew Morton; +Cc: jgarzik, netdev, hmacht

Hi!

> > > > diff --git a/drivers/net/b44.c b/drivers/net/b44.c
> > > > --- a/drivers/net/b44.c
> > > > +++ b/drivers/net/b44.c
> > > > @@ -1930,6 +1930,8 @@ static int b44_suspend(struct pci_dev *p
> > > >  	b44_free_rings(bp);
> > > >  
> > > >  	spin_unlock_irq(&bp->lock);
> > > > +
> > > > +	free_irq(dev->irq, dev);
> > > >  	pci_disable_device(pdev);
> > > >  	return 0;
> > > >  }
> > > > @@ -1946,6 +1948,9 @@ static int b44_resume(struct pci_dev *pd
> > > >  	if (!netif_running(dev))
> > > >  		return 0;
> > > >  
> > > > +	if (request_irq(dev->irq, b44_interrupt, SA_SHIRQ, dev->name, dev))
> > > > +		printk(KERN_ERR PFX "%s: request_irq failed\n", dev->name);
> > > > +
> > > >  	spin_lock_irq(&bp->lock);
> > > >  
> > > >  	b44_init_rings(bp);
> > > > 
> > > 
> > > Why does it hang on suspend/resume?
> > > 
> > > This came up a while back and iirc we decided that adding free_irq() to
> > > every ->suspend() handler in the world was the wrong thing to do.  Do I
> > > misremember?
> > 
> > No, you remember right, but b44 needed that free_irq/request_irq even
> > because those ACPI changes. I'm not exactly sure why, something went
> > very wrong otherwise.
> 
> Well I guess we should work out what went wrong ;)
> 
> What are the symptoms?  Screaming interrupt?  Can't immediately see why. 
> Does the screaming interrupt detetor trigger and disable the IRQ Line?

No, it seems like BUG() triggers in
b44. https://bugzilla.novell.com/show_bug.cgi?id=116088 is for
basically 2.6.13 kernel (but it was in something as old as 2.6.5,
too).



Setting machine into suspend to disk with loaded module b44. While
resuming,
kernel oopses and machine freezes after reloading data from
swap. Image will be
appended.

If this can not be fixed in time, we could add this module to
UNLOAD_MODULES_BEFORE_SUSPEND in powersave configuratiion.

								Pavel

-- 
if you have sharp zaurus hardware you don't need... you know my address

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch] fix suspend/resume on b44
  2005-09-21 21:13       ` Pavel Machek
@ 2005-09-21 21:22         ` Andrew Morton
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2005-09-21 21:22 UTC (permalink / raw)
  To: Pavel Machek; +Cc: jgarzik, netdev, hmacht

Pavel Machek <pavel@ucw.cz> wrote:
>
> Hi!
> 
> > > > > diff --git a/drivers/net/b44.c b/drivers/net/b44.c
> > > > > --- a/drivers/net/b44.c
> > > > > +++ b/drivers/net/b44.c
> > > > > @@ -1930,6 +1930,8 @@ static int b44_suspend(struct pci_dev *p
> > > > >  	b44_free_rings(bp);
> > > > >  
> > > > >  	spin_unlock_irq(&bp->lock);
> > > > > +
> > > > > +	free_irq(dev->irq, dev);
> > > > >  	pci_disable_device(pdev);
> > > > >  	return 0;
> > > > >  }
> > > > > @@ -1946,6 +1948,9 @@ static int b44_resume(struct pci_dev *pd
> > > > >  	if (!netif_running(dev))
> > > > >  		return 0;
> > > > >  
> > > > > +	if (request_irq(dev->irq, b44_interrupt, SA_SHIRQ, dev->name, dev))
> > > > > +		printk(KERN_ERR PFX "%s: request_irq failed\n", dev->name);
> > > > > +
> > > > >  	spin_lock_irq(&bp->lock);
> > > > >  
> > > > >  	b44_init_rings(bp);
> > > > > 
> > > > 
> > > > Why does it hang on suspend/resume?
> > > > 
> > > > This came up a while back and iirc we decided that adding free_irq() to
> > > > every ->suspend() handler in the world was the wrong thing to do.  Do I
> > > > misremember?
> > > 
> > > No, you remember right, but b44 needed that free_irq/request_irq even
> > > because those ACPI changes. I'm not exactly sure why, something went
> > > very wrong otherwise.
> > 
> > Well I guess we should work out what went wrong ;)
> > 
> > What are the symptoms?  Screaming interrupt?  Can't immediately see why. 
> > Does the screaming interrupt detetor trigger and disable the IRQ Line?
> 
> No, it seems like BUG() triggers in
> b44. https://bugzilla.novell.com/show_bug.cgi?id=116088 is for
> basically 2.6.13 kernel (but it was in something as old as 2.6.5,
> too).
>

That's here:

static void b44_tx(struct b44 *bp)
{
	u32 cur, cons;

	cur  = br32(bp, B44_DMATX_STAT) & DMATX_STAT_CDMASK;
	cur /= sizeof(struct dma_desc);

	/* XXX needs updating when NETIF_F_SG is supported */
	for (cons = bp->tx_cons; cons != cur; cons = NEXT_TX(cons)) {
		struct ring_info *rp = &bp->tx_buffers[cons];
		struct sk_buff *skb = rp->skb;

		if (unlikely(skb == NULL))
			BUG();

So I'd assume that the newly-woken driver took an interrupt, decided that a
Tx interrupt was pending then went BUG when it discovered that it hadn't
sent anything.

Would be good to find out the value of istat in b44_interrupt() and poke
maintainers, rather than proferring strange workarounds ;)

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2005-09-21 21:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-09-20 13:28 [patch] fix suspend/resume on b44 Pavel Machek
2005-09-20 23:26 ` Andrew Morton
2005-09-21 10:20   ` Pavel Machek
2005-09-21 10:36     ` Andrew Morton
2005-09-21 21:13       ` Pavel Machek
2005-09-21 21:22         ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).