Re: [PATCH] usb/core/hcd-pci.c: don't free_irq() on suspend

Re: [PATCH] usb/core/hcd-pci.c: don't free_irq() on suspend

[Greg KH]

<top-post on purpose...>

Daniel, are you sure about this patch (the second part specifically)?
It directly conflicts with a set of patches in my current tree in this
area that fix all of the reported suspend/resume issues with usb host
controllers (that patch series written by David Brownell.)

Yeah, I see that we shouldn't have been dropping the irq on suspend and
getting a new one on resume, that's not good and could have caused
problems for people.

But could you at least drop the linux-usb-devel mailing list a note that
you are having issues, and post the proposed patch?  Directly sending it
to Linus is a bit rude, it's not like the USB developers aren't
responsive to emails (yeah, I've been a bit slow at times these past few
weeks, but my traveling all over the place for the past month is now
over, and I'm not going anywhere for a long time...)

David, this conflicts with your usb/usb-pm-06.patch in my quilt tree.
I'll try to resolve the merge with my best guess, but you should check
that I got it right...

thanks,

greg k-h


On Fri, Sep 30, 2005 at 02:01:17PM -0700, Linux Kernel Mailing List wrote:
> tree 04e9dcf7801245d9a31fcab36dde7dd3a846c86b
> parent 1294b118cb53fb14515666e2b218ad5ab40318c1
> author Daniel Ritz <daniel.ritz@gmx.ch> Thu, 29 Sep 2005 21:39:32 +0200
> committer Linus Torvalds <torvalds@g5.osdl.org> Fri, 30 Sep 2005 23:23:30 -0700
> 
> [PATCH] usb/core/hcd-pci.c: don't free_irq() on suspend
> 
> the free_irq() in USB suspend breaks resume on some setups where USB
> (ohci/ehci) shares the interrupt with an other device.
> 
> Signed-off-by: Daniel Ritz <daniel.ritz@gmx.ch>
> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
> 
>  drivers/usb/core/hcd-pci.c |    9 ---------
>  1 files changed, 9 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
> @@ -242,7 +242,6 @@ int usb_hcd_pci_suspend (struct pci_dev 
>  	case HC_STATE_SUSPENDED:
>  		/* no DMA or IRQs except when HC is active */
>  		if (dev->current_state == PCI_D0) {
> -			free_irq (hcd->irq, hcd);
>  			pci_save_state (dev);
>  			pci_disable_device (dev);
>  		}
> @@ -374,14 +373,6 @@ int usb_hcd_pci_resume (struct pci_dev *
>  
>  	hcd->state = HC_STATE_RESUMING;
>  	hcd->saw_irq = 0;
> -	retval = request_irq (dev->irq, usb_hcd_irq, SA_SHIRQ,
> -				hcd->irq_descr, hcd);
> -	if (retval < 0) {
> -		dev_err (hcd->self.controller,
> -			"can't restore IRQ after resume!\n");
> -		usb_hc_died (hcd);
> -		return retval;
> -	}
>  
>  	retval = hcd->driver->resume (hcd);
>  	if (!HC_IS_RUNNING (hcd->state)) {
> -
> To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Re: [PATCH] usb/core/hcd-pci.c: don't free_irq() on suspend

[David Brownell]

> David, this conflicts with your usb/usb-pm-06.patch in my quilt tree.
> I'll try to resolve the merge with my best guess, but you should check
> that I got it right...

The patch is OK with me, I was going to ack it just in case it
grew little penguin fins ... I see it already has, and swam all
the way into 2.6.14-rc3, so little point ... ;)

Yes, I'll double check your merge.

- Dave

Re: [PATCH] usb/core/hcd-pci.c: don't free_irq() on suspend

[Daniel Ritz]

[ restored some of the cc: list of the original thread ]

On Saturday 01 October 2005 01.38, Greg KH wrote:
> <top-post on purpose...>
> 
> Daniel, are you sure about this patch (the second part specifically)?

well, the first hunk doesn't make sense without the second and vice versa,
isn't it? to answer your question: yes.

> It directly conflicts with a set of patches in my current tree in this
> area that fix all of the reported suspend/resume issues with usb host
> controllers (that patch series written by David Brownell.)
> 
> Yeah, I see that we shouldn't have been dropping the irq on suspend and
> getting a new one on resume, that's not good and could have caused
> problems for people.
> 
> But could you at least drop the linux-usb-devel mailing list a note that
> you are having issues, and post the proposed patch?  Directly sending it

i don't have problems, it was rafael...i just happend to look at it because
yenta_socket was involved...see the following link for more background:
	http://marc.theaimsgroup.com/?t=112275164900002&r=1&w=4

> to Linus is a bit rude, it's not like the USB developers aren't

sorry for that, but i actually asked for a round in -mm. it just happend
that linus was on to: and the rest on cc: by pressing reply-to-all in kmail

the original thread
	http://marc.theaimsgroup.com/?t=112618280600003&r=1&w=4

> responsive to emails (yeah, I've been a bit slow at times these past few
> weeks, but my traveling all over the place for the past month is now
> over, and I'm not going anywhere for a long time...)
> 
> David, this conflicts with your usb/usb-pm-06.patch in my quilt tree.
> I'll try to resolve the merge with my best guess, but you should check
> that I got it right...
> 
> thanks,
> 
> greg k-h
> 

rgds
-daniel