Re: REGRESSION: 2.6.24: false double-clicks from USB mouse

2007-12-06 Thread Diego Zuccato
Jiri Kosina ha scritto:

>> My USB mouse recently started giving me phantom scrolls (movements of 
>> the mouse wheel, even I made sure the wheel was in a resting position) 
>> in recently kernels.  It happened too infrequently for me to care.
> Did this also start in 2.6.24, as in Mark's case?
It happens to me, too.
Wireless mouse, gives some "wheel up" signals when going into powersave
state. Kernel is 2.6.22-tmb-laptop-2mdv (Mandriva Cooker).

T:  Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  2 Spd=1.5 MxCh= 0
D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
P:  Vendor=062a ProdID= Rev= 0.00
C:* #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=02 Driver=usbhid
E:  Ad=81(I) Atr=03(Int.) MxPS=   4 Ivl=10ms

Maybe a more-or-less undocumented sequence to tell the host it's going
to sleep?

BYtE,
 Diego.
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: REGRESSION: 2.6.24: false double-clicks from USB mouse

2007-12-06 Thread Jiri Kosina
On Thu, 6 Dec 2007, Diego Zuccato wrote:

> It happens to me, too. Wireless mouse, gives some "wheel up" signals 
> when going into powersave state. Kernel is 2.6.22-tmb-laptop-2mdv 
> (Mandriva Cooker).

Is this a regression for you? Are you able to say which kernel first 
exposed this behavior in your case?

> Maybe a more-or-less undocumented sequence to tell the host it's going 
> to sleep?

usbmon and/or HID_DEBUG output would be interesting to see (how to produce 
HID debugging output has been explained earlier in this thread, for usbmon 
please look in Documentation/usb/usbmon.txt).

Thanks,

-- 
Jiri Kosina
SUSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [linux-usb-devel] [RFC PATCH 1/1] USB: FEATURE_REMOVAL: USB driver API moves to EXPORT_SYMBOL_GPL

2007-12-06 Thread Felipe Balbi
On Dec 6, 2007 3:41 AM, Greg KH <[EMAIL PROTECTED]> wrote:
> On Thu, Dec 06, 2007 at 01:15:09AM +0200, Felipe Balbi wrote:
> > Following what is mentioned in feature-removal-schedule.txt this
> > moves the two missing pieces in drivers/usb/core/driver.c to
> > EXPORT_SYMBOL_GPL.
> >
> > Non-gpl code should be done in userspace using usbfs | libusb | gadgetfs
> > instead of trying to hook up binary-only drivers to the kernel.
> >
> > Signed-off-by: Felipe Balbi <[EMAIL PROTECTED]>
> > ---
> >  Documentation/feature-removal-schedule.txt |   16 
> >  drivers/usb/core/driver.c  |4 ++--
> >  2 files changed, 2 insertions(+), 18 deletions(-)
> >
> > diff --git a/Documentation/feature-removal-schedule.txt 
> > b/Documentation/feature-removal-schedule.txt
> > index 20c4c8b..105dbc0 100644
> > --- a/Documentation/feature-removal-schedule.txt
> > +++ b/Documentation/feature-removal-schedule.txt
> > @@ -156,22 +156,6 @@ Who: Arjan van de Ven <[EMAIL PROTECTED]>
> >
> >  ---
> >
> > -What:USB driver API moves to EXPORT_SYMBOL_GPL
> > -When:February 2008
>
> But it is not February 2008 yet.  Why would you send this kind of patch
> ahead of the declared time frame?
>
> Also, this is not how the patch should be done, the
> EXPORT_SYMBOL_GPL_FUTURE() symbols will just be changed to
> EXPORT_SYMBOL_GPL(), that's all that is needed.
>
> > -EXPORT_SYMBOL(usb_driver_claim_interface);
> > +EXPORT_SYMBOL_GPL(usb_driver_claim_interface);
>
> Not many of the USB drivers actually use this function, so changing the
> marking on it will not even achieve what you are trying to do.
>
> Totally confused,

Ok.. I'll fix this and wait until feb/2008.

Thanks Greg

>
> greg k-h
>
>
> -
> SF.Net email is sponsored by: The Future of Linux Business White Paper
> from Novell.  From the desktop to the data center, Linux is going
> mainstream.  Let it simplify your IT future.
> http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
> ___
> [EMAIL PROTECTED]
> To unsubscribe, use the last form field at:
> https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
>



-- 
Best Regards,

Felipe Balbi
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] dummy_hcd: change the default power budget

2007-12-06 Thread Alan Stern
This patch (as1025) changes the default power budget for dummy-hcd to
500 mA and makes it a preprocessor parameter for easier testing.

Signed-off-by: Alan Stern <[EMAIL PROTECTED]>

---

Index: usb-2.6/drivers/usb/gadget/dummy_hcd.c
===
--- usb-2.6.orig/drivers/usb/gadget/dummy_hcd.c
+++ usb-2.6/drivers/usb/gadget/dummy_hcd.c
@@ -61,6 +61,8 @@
 #define DRIVER_DESC"USB Host+Gadget Emulator"
 #define DRIVER_VERSION "02 May 2005"
 
+#define POWER_BUDGET   500 /* in mA; use 8 for low-power port testing */
+
 static const char  driver_name [] = "dummy_hcd";
 static const char  driver_desc [] = "USB Host+Gadget Emulator";
 
@@ -1810,8 +1812,7 @@ static int dummy_start (struct usb_hcd *
 
INIT_LIST_HEAD (&dum->urbp_list);
 
-   /* only show a low-power port: just 8mA */
-   hcd->power_budget = 8;
+   hcd->power_budget = POWER_BUDGET;
hcd->state = HC_STATE_RUNNING;
hcd->uses_new_polling = 1;
 

-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Disabling interrupts during IRQ in ohci-hcd

2007-12-06 Thread David Brownell
On Wednesday 05 December 2007, Alan Stern wrote:
> On Wed, 5 Dec 2007, David Brownell wrote:
> 
> > On Wednesday 05 December 2007, Alan Stern wrote:
> > > But it will be easy enough to add IRQF_DISABLED in.  Only a few drivers 
> > > omit it.
> > 
> > I forgot that the flags are passed in to usb_add_hcd()... if we
> > go that route, the drivers that don't pass it should be updated
> > and so should hcd-pci.c; that seems the safest option to me.
> 
> The proposed patch is below.

OK by me ... ack.  And schedule merge for 2.6.24-final ...

Re Pete's complaint, I can sort of understand not wanting IRQF_DISABLED;
but that's why I commented that this option seems "safest".  Alternative
solutions to this little surprise would need more investigation, on more
varied hardware, than I think we can deliver just now.  This way has the
advantage of being "obviously correct".


> > This looks like a longstanding bug/oversight.  Color me surprised.
> 
> It's possible that it has caused undiagnosed lockups in the past.  The 
> recent changes for urb->status removal must have made them much more 
> likely.  For instance, the urb_unlink() routine formerly in hcd.c 
> used spin_lock_irqsave() but its replacement
> usb_hcd_unlink_urb_from_ep() uses only spin_lock().  That particular 
> spinlock is what led to the bug report.

Exactly.

- Dave


 
> Alan Stern
> 
> 
> Host controller IRQs are supposed to be serviced with interrupts
> disabled.  This patch (as1025) adds an IRQF_DISABLED flag to all the
> controller drivers that lack it.  It also replaces the
> spin_lock_irqsave() and spin_unlock_irqrestore() calls in uhci_irq()
> with simple spin_lock() and spin_unlock().
> 
> This fixes Bugzilla #9335.
> 
> Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
> 
> ---
> 
> Index: usb-2.6/drivers/usb/core/hcd-pci.c
> ===
> --- usb-2.6.orig/drivers/usb/core/hcd-pci.c
> +++ usb-2.6/drivers/usb/core/hcd-pci.c
> @@ -125,7 +125,7 @@ int usb_hcd_pci_probe (struct pci_dev *d
>  
>   pci_set_master (dev);
>  
> - retval = usb_add_hcd (hcd, dev->irq, IRQF_SHARED);
> + retval = usb_add_hcd(hcd, dev->irq, IRQF_DISABLED | IRQF_SHARED);
>   if (retval != 0)
>   goto err4;
>   return retval;
> Index: usb-2.6/drivers/usb/host/ehci-fsl.c
> ===
> --- usb-2.6.orig/drivers/usb/host/ehci-fsl.c
> +++ usb-2.6/drivers/usb/host/ehci-fsl.c
> @@ -122,7 +122,7 @@ int usb_hcd_fsl_probe(const struct hc_dr
>   temp = in_le32(hcd->regs + 0x1a8);
>   out_le32(hcd->regs + 0x1a8, temp | 0x3);
>  
> - retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
> + retval = usb_add_hcd(hcd, irq, IRQF_DISABLED | IRQF_SHARED);
>   if (retval != 0)
>   goto err4;
>   return retval;
> Index: usb-2.6/drivers/usb/host/ohci-ppc-of.c
> ===
> --- usb-2.6.orig/drivers/usb/host/ohci-ppc-of.c
> +++ usb-2.6/drivers/usb/host/ohci-ppc-of.c
> @@ -142,7 +142,7 @@ ohci_hcd_ppc_of_probe(struct of_device *
>  
>   ohci_hcd_init(ohci);
>  
> - rv = usb_add_hcd(hcd, irq, 0);
> + rv = usb_add_hcd(hcd, irq, IRQF_DISABLED);
>   if (rv == 0)
>   return 0;
>  
> Index: usb-2.6/drivers/usb/host/ohci-ssb.c
> ===
> --- usb-2.6.orig/drivers/usb/host/ohci-ssb.c
> +++ usb-2.6/drivers/usb/host/ohci-ssb.c
> @@ -160,7 +160,7 @@ static int ssb_ohci_attach(struct ssb_de
>   hcd->regs = ioremap_nocache(hcd->rsrc_start, hcd->rsrc_len);
>   if (!hcd->regs)
>   goto err_put_hcd;
> - err = usb_add_hcd(hcd, dev->irq, IRQF_SHARED);
> + err = usb_add_hcd(hcd, dev->irq, IRQF_DISABLED | IRQF_SHARED);
>   if (err)
>   goto err_iounmap;
>  
> Index: usb-2.6/drivers/usb/host/r8a66597-hcd.c
> ===
> --- usb-2.6.orig/drivers/usb/host/r8a66597-hcd.c
> +++ usb-2.6/drivers/usb/host/r8a66597-hcd.c
> @@ -2197,7 +2197,7 @@ static int __init r8a66597_probe(struct 
>   INIT_LIST_HEAD(&r8a66597->child_device);
>  
>   hcd->rsrc_start = res->start;
> - ret = usb_add_hcd(hcd, irq, 0);
> + ret = usb_add_hcd(hcd, irq, IRQF_DISABLED);
>   if (ret != 0) {
>   err("Failed to add hcd");
>   goto clean_up;
> Index: usb-2.6/drivers/usb/host/uhci-hcd.c
> ===
> --- usb-2.6.orig/drivers/usb/host/uhci-hcd.c
> +++ usb-2.6/drivers/usb/host/uhci-hcd.c
> @@ -378,7 +378,6 @@ static irqreturn_t uhci_irq(struct usb_h
>  {
>   struct uhci_hcd *uhci = hcd_to_uhci(hcd);
>   unsigned short status;
> - unsigned long flags;
>  
>   /*
>* Read the interrupt status, and write it back to clear the
> @@ -398,7 +397,7 @@ static irqreturn_t uhci_irq(struct usb_h
>   dev_err(uhc

Re: Disabling interrupts during IRQ in ohci-hcd

2007-12-06 Thread Pete Zaitcev
On Wed, 5 Dec 2007 17:31:20 -0500 (EST), Alan Stern <[EMAIL PROTECTED]> wrote:

> [...] In fact every HCD _must_ keep interrupts disabled 
> while doing most of its IRQ processing.  This is because an interrupt 
> could cause some random driver to submit an URB -- and URB submission 
> can't be done concurrently with IRQ processing since they both alter 
> the same data structures.

Fair enough, but this is why the HCD has to do spin_lock_irqsave
across the time when lists are being modified or otherwise inconsistent.
It should not need to keep interrupts closed across executing callbacks
for complete URBs. IMHO.

In other message you wrote that:

> However usb_hcd_unlink_urb_from_ep is documented as requiring
> that it be called with interrupts disabled.

I continue to think that methods with such requirements must not
exist (at least not at the level we operate in USB stack; I can see
a need for something like that in the architecture support code).
It is unduly onerous to make sure the requirement is met while the
code changes, and it leads to unnecessary expansion of time spent with
interrupts off.

I'm not going to appeal to Greg to have you overruled or anything, but
I remain unconvinced.

-- Pete
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Disabling interrupts during IRQ in ohci-hcd

2007-12-06 Thread Alan Stern
On Thu, 6 Dec 2007, Pete Zaitcev wrote:

> On Wed, 5 Dec 2007 17:31:20 -0500 (EST), Alan Stern <[EMAIL PROTECTED]> wrote:
> 
> > [...] In fact every HCD _must_ keep interrupts disabled 
> > while doing most of its IRQ processing.  This is because an interrupt 
> > could cause some random driver to submit an URB -- and URB submission 
> > can't be done concurrently with IRQ processing since they both alter 
> > the same data structures.
> 
> Fair enough, but this is why the HCD has to do spin_lock_irqsave
> across the time when lists are being modified or otherwise inconsistent.
> It should not need to keep interrupts closed across executing callbacks
> for complete URBs. IMHO.

Well, I tend to agree about that.  However in practice it is
unavoidable, since the callbacks are made from a subroutine called by
the code that modifies the lists.  Unless I'm mistaken, you can't pass
the flags argument to a subroutine -- it's only guaranteed to work in
the same stack frame where it was set.

In addition the USB stack's driver interface specifies that the 
completion callbacks will be made with interrupts disabled.

> In other message you wrote that:
> 
> > However usb_hcd_unlink_urb_from_ep is documented as requiring
> > that it be called with interrupts disabled.
> 
> I continue to think that methods with such requirements must not
> exist (at least not at the level we operate in USB stack; I can see
> a need for something like that in the architecture support code).
> It is unduly onerous to make sure the requirement is met while the
> code changes, and it leads to unnecessary expansion of time spent with
> interrupts off.

In this case it's the natural thing to do.  Interrupts have to be
disabled anyway at the point where usb_hcd_unlink_urb_from_ep is
called, so there's no harm in requiring it to be called with interrupts
disabled.  Particularly since the routine would just have to disable
them itself.

Alan Stern

-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] USB: use IRQF_DISABLED for HCD interrupt handlers

2007-12-06 Thread Alan Stern
Host controller IRQs are supposed to be serviced with interrupts
disabled.  This patch (as1026) adds an IRQF_DISABLED flag to all the
controller drivers that lack it.  It also replaces the
spin_lock_irqsave() and spin_unlock_irqrestore() calls in uhci_irq()
with simple spin_lock() and spin_unlock().

This fixes Bugzilla #9335.

Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
Acked-by: David Brownell <[EMAIL PROTECTED]>

---

This is definitely needed for 2.6.24 since it fixes a real bug.


Index: usb-2.6/drivers/usb/core/hcd-pci.c
===
--- usb-2.6.orig/drivers/usb/core/hcd-pci.c
+++ usb-2.6/drivers/usb/core/hcd-pci.c
@@ -125,7 +125,7 @@ int usb_hcd_pci_probe (struct pci_dev *d
 
pci_set_master (dev);
 
-   retval = usb_add_hcd (hcd, dev->irq, IRQF_SHARED);
+   retval = usb_add_hcd(hcd, dev->irq, IRQF_DISABLED | IRQF_SHARED);
if (retval != 0)
goto err4;
return retval;
Index: usb-2.6/drivers/usb/host/ehci-fsl.c
===
--- usb-2.6.orig/drivers/usb/host/ehci-fsl.c
+++ usb-2.6/drivers/usb/host/ehci-fsl.c
@@ -122,7 +122,7 @@ int usb_hcd_fsl_probe(const struct hc_dr
temp = in_le32(hcd->regs + 0x1a8);
out_le32(hcd->regs + 0x1a8, temp | 0x3);
 
-   retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
+   retval = usb_add_hcd(hcd, irq, IRQF_DISABLED | IRQF_SHARED);
if (retval != 0)
goto err4;
return retval;
Index: usb-2.6/drivers/usb/host/ohci-ppc-of.c
===
--- usb-2.6.orig/drivers/usb/host/ohci-ppc-of.c
+++ usb-2.6/drivers/usb/host/ohci-ppc-of.c
@@ -142,7 +142,7 @@ ohci_hcd_ppc_of_probe(struct of_device *
 
ohci_hcd_init(ohci);
 
-   rv = usb_add_hcd(hcd, irq, 0);
+   rv = usb_add_hcd(hcd, irq, IRQF_DISABLED);
if (rv == 0)
return 0;
 
Index: usb-2.6/drivers/usb/host/ohci-ssb.c
===
--- usb-2.6.orig/drivers/usb/host/ohci-ssb.c
+++ usb-2.6/drivers/usb/host/ohci-ssb.c
@@ -160,7 +160,7 @@ static int ssb_ohci_attach(struct ssb_de
hcd->regs = ioremap_nocache(hcd->rsrc_start, hcd->rsrc_len);
if (!hcd->regs)
goto err_put_hcd;
-   err = usb_add_hcd(hcd, dev->irq, IRQF_SHARED);
+   err = usb_add_hcd(hcd, dev->irq, IRQF_DISABLED | IRQF_SHARED);
if (err)
goto err_iounmap;
 
Index: usb-2.6/drivers/usb/host/r8a66597-hcd.c
===
--- usb-2.6.orig/drivers/usb/host/r8a66597-hcd.c
+++ usb-2.6/drivers/usb/host/r8a66597-hcd.c
@@ -2197,7 +2197,7 @@ static int __init r8a66597_probe(struct 
INIT_LIST_HEAD(&r8a66597->child_device);
 
hcd->rsrc_start = res->start;
-   ret = usb_add_hcd(hcd, irq, 0);
+   ret = usb_add_hcd(hcd, irq, IRQF_DISABLED);
if (ret != 0) {
err("Failed to add hcd");
goto clean_up;
Index: usb-2.6/drivers/usb/host/uhci-hcd.c
===
--- usb-2.6.orig/drivers/usb/host/uhci-hcd.c
+++ usb-2.6/drivers/usb/host/uhci-hcd.c
@@ -378,7 +378,6 @@ static irqreturn_t uhci_irq(struct usb_h
 {
struct uhci_hcd *uhci = hcd_to_uhci(hcd);
unsigned short status;
-   unsigned long flags;
 
/*
 * Read the interrupt status, and write it back to clear the
@@ -398,7 +397,7 @@ static irqreturn_t uhci_irq(struct usb_h
dev_err(uhci_dev(uhci), "host controller process "
"error, something bad happened!\n");
if (status & USBSTS_HCH) {
-   spin_lock_irqsave(&uhci->lock, flags);
+   spin_lock(&uhci->lock);
if (uhci->rh_state >= UHCI_RH_RUNNING) {
dev_err(uhci_dev(uhci),
"host controller halted, "
@@ -415,16 +414,16 @@ static irqreturn_t uhci_irq(struct usb_h
 * pending unlinks */
mod_timer(&hcd->rh_timer, jiffies);
}
-   spin_unlock_irqrestore(&uhci->lock, flags);
+   spin_unlock(&uhci->lock);
}
}
 
if (status & USBSTS_RD)
usb_hcd_poll_rh_status(hcd);
else {
-   spin_lock_irqsave(&uhci->lock, flags);
+   spin_lock(&uhci->lock);
uhci_scan_schedule(uhci);
-   spin_unlock_irqrestore(&uhci->lock, flags);
+   spin_unlock(&uhci->lock);
}
 
return IRQ_HANDLED;

-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-