Re: [linux-usb-devel] [Bugme-new] [Bug 8310] New: USB device names are not sanitized for UTF-8
Alan Stern schrieb: However I did make up a list of source files which seem to use UTF-8 in a nontrivial or interesting way: ./drivers/s390/char/keyboard.c ./drivers/firmware/efivars.c ./drivers/char/n_tty.c ./drivers/char/vt.c ./drivers/char/keyboard.c ./fs/nls/nls_base.c ./fs/nls/nls_utf8.c ./fs/cifs/sess.c ./fs/udf/unicode.c ./fs/hfsplus/options.c ./fs/befs/linuxvfs.c ./fs/ncpfs/ncplib_kernel.c ./fs/isofs/joliet.c ./fs/fat/dir.c ./fs/vfat/namei.c ./arch/ia64/kernel/efi.c Mostly the usual suspects. Only fat is unusual in this list. - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
[linux-usb-devel] WARNING: at drivers/usb/core/urb.c:293 usb_submit_urb() [Was: 2.6.22-rc4-mm2]
Andrew Morton napsal(a): ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc4/2.6.22-rc4-mm2/ Hi, after plugging my camera in, I get this: usb 1-1: new full speed USB device using uhci_hcd and address 2 usb 1-1: new device found, idVendor=07b4, idProduct=0105 usb 1-1: new device strings: Mfr=1, Product=2, SerialNumber=3 usb 1-1: Product: C765UZ usb 1-1: Manufacturer: OLYMPUS usb 1-1: SerialNumber: 000375531837 usb 1-1: configuration #1 chosen from 1 choice Initializing USB Mass Storage driver... scsi7 : SCSI emulation for USB Mass Storage devices usbcore: registered new interface driver usb-storage USB Mass Storage support registered. WARNING: at /home/l/latest/xxx/drivers/usb/core/urb.c:293 usb_submit_urb() [c010516a] dump_trace+0x1d8/0x207 [c01051b3] show_trace_log_lvl+0x1a/0x30 [c0105db9] show_trace+0x12/0x14 [c0105dd1] dump_stack+0x16/0x18 [c0284034] usb_submit_urb+0x1ea/0x200 [c02857fe] usb_sg_wait+0xba/0x14e [f8985098] usb_stor_bulk_transfer_sg+0x99/0xe3 [usb_storage] [f89854cc] usb_stor_Bulk_transport+0x127/0x277 [usb_storage] [f8985637] usb_stor_invoke_transport+0x1b/0x2f4 [usb_storage] [f89848b9] usb_stor_transparent_scsi_command+0x8/0xa [usb_storage] [f8986354] usb_stor_control_thread+0x130/0x195 [usb_storage] [c0136d07] kthread+0x37/0x59 [c0104bfb] kernel_thread_helper+0x7/0x1c === It's not usable, some threads end up in D state. Relevant part of sysrq-t: scsi_eh_7 D 0080027D 0 23842 2 (L-TLB) c2e52f34 0046 45ad4b85 0080027d c2e52f1c c3e88230 45ad46cf 0080027d c012262a c3e88230 c3e883b8 c180b980 486cfe61 027d c037487f c2e52f74 0046 d560826b 0080027a c011c00f c2cecac0 c0545080 c0545080 Call Trace: [c0374f8d] wait_for_completion+0x87/0xbc [f898462e] command_abort+0x58/0x74 [usb_storage] [c0263be3] __scsi_try_to_abort_cmd+0x1c/0x1e [c0264f10] scsi_error_handler+0x241/0x2bf [c0136d07] kthread+0x37/0x59 [c0104bfb] kernel_thread_helper+0x7/0x1c === usb-storage D 0080027B 0 23843 2 (L-TLB) c2f76e94 0046 fecf46f3 0080027b c2f76e7c fecf4091 0080027b c012262a c3e89870 c3e899f8 c180b980 018ef9cf 027c c1ce9c00 c2f76e60 c0125fae c0408a64 c2f76e6c c2f76e6c c1f98e40 c0545080 c0545080 Call Trace: [c0374f8d] wait_for_completion+0x87/0xbc [c028585a] usb_sg_wait+0x116/0x14e [f8985098] usb_stor_bulk_transfer_sg+0x99/0xe3 [usb_storage] [f89854cc] usb_stor_Bulk_transport+0x127/0x277 [usb_storage] [f8985637] usb_stor_invoke_transport+0x1b/0x2f4 [usb_storage] [f89848b9] usb_stor_transparent_scsi_command+0x8/0xa [usb_storage] [f8986354] usb_stor_control_thread+0x130/0x195 [usb_storage] [c0136d07] kthread+0x37/0x59 [c0104bfb] kernel_thread_helper+0x7/0x1c === usb-stor-scan D 0080027B 0 23844 2 (L-TLB) c46fecbc 0046 febe1f98 0080027b c46feca4 c46fec68 febe17da 0080027b c012262a c4345260 c43453e8 c180b980 017dd274 027c c43ec000 c46fec7c c01e2964 c46fec84 c02541f8 c46fecb0 c1f98e40 c0545080 c0545080 Call Trace: [c0374f8d] wait_for_completion+0x87/0xbc [c01da63d] blk_execute_rq+0x5a/0x94 [c0266479] scsi_execute+0xc3/0xd7 [c02664f3] scsi_execute_req+0x66/0xc4 [c026762f] scsi_probe_and_add_lun+0x19b/0x891 [c0268265] __scsi_scan_target+0xd3/0x59f [c02687a0] scsi_scan_channel+0x6f/0x84 [c0268818] scsi_scan_host_selected+0x63/0xd5 [c02688f5] do_scsi_scan_host+0x6b/0x6d [c026897e] scsi_scan_host+0x87/0x153 [f8986416] usb_stor_scan_thread+0x5d/0x17b [usb_storage] [c0136d07] kthread+0x37/0x59 [c0104bfb] kernel_thread_helper+0x7/0x1c === Are you aware of such problems? Any other info needed? thanks, -- http://www.fi.muni.cz/~xslaby/Jiri Slaby faculty of informatics, masaryk university, brno, cz e-mail: jirislaby gmail com, gpg pubkey fingerprint: B674 9967 0407 CE62 ACC8 22A0 32CC 55C3 39D4 7A7E - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] WARNING: at drivers/usb/core/urb.c:293 usb_submit_urb() [Was: 2.6.22-rc4-mm2]
Jiri Slaby napsal(a): Andrew Morton napsal(a): ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc4/2.6.22-rc4-mm2/ after plugging my camera in, I get this: [...] USB Mass Storage support registered. WARNING: at /home/l/latest/xxx/drivers/usb/core/urb.c:293 usb_submit_urb() [...] Are you aware of such problems? Any other info needed? Aha, you are, going to try http://lkml.org/lkml/2007/6/7/197 regards, -- http://www.fi.muni.cz/~xslaby/Jiri Slaby faculty of informatics, masaryk university, brno, cz e-mail: jirislaby gmail com, gpg pubkey fingerprint: B674 9967 0407 CE62 ACC8 22A0 32CC 55C3 39D4 7A7E - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c
On 6/12/07, Peter Korsgaard [EMAIL PROTECTED] wrote: Grant == Grant Likely [EMAIL PROTECTED] writes: Hi, Grant Rather than c67x00-hub.c being compiled seperately, the Grant original code had c67x00-hub.c *included* by c67x00-hcd.c. Grant This is a very bad idea. Simplest solution is to merge the Grant two files into one and be done with it. Yeah, it isn't exactly pretty, but it's what the other hcd drivers do, E.G.: % grep -rs include.*hub.c *c ehci-hcd.c:#include ehci-hub.c ohci-hcd.c:#include ohci-hub.c uhci-hcd.c:#include uhci-hub.c I don't quite know why they do it like that though .. True, but that doesn't mean that it's a good idea to follow the lead. There are lots of other examples of ugly code in the kernel that is tolerated just because nobody has cleaned it up yet, but is still unacceptable for new code. We know it's an ugly thing to do, and the fix is simple and easy. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. [EMAIL PROTECTED] (403) 399-0195 - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
[linux-usb-devel] [patch]autosuspend for usblcd
Hi,
this patch implements autosuspend for the usblcd driver. It uses
the new usb_anchor infrastructure. Many thanks to Georges for testing.
Regards
Oliver
Signed-off-by: Oliver Neukum [EMAIL PROTECTED]
--
--- a/drivers/usb/misc/usblcd.c 2007-06-11 15:46:44.0 +0200
+++ b/drivers/usb/misc/usblcd.c 2007-06-12 14:54:52.0 +0200
@@ -45,6 +45,7 @@ struct usb_lcd {
struct kref kref;
struct semaphorelimit_sem; /* to stop writes at
full throttle from
* using up all RAM */
+ struct usb_anchor submitted; /* URBs to wait for
before suspend */
};
#define to_lcd_dev(d) container_of(d, struct usb_lcd, kref)
@@ -67,7 +68,7 @@ static int lcd_open(struct inode *inode,
{
struct usb_lcd *dev;
struct usb_interface *interface;
- int subminor;
+ int subminor, r;
subminor = iminor(inode);
@@ -85,6 +86,13 @@ static int lcd_open(struct inode *inode,
/* increment our usage count for the device */
kref_get(dev-kref);
+ /* grab a power reference */
+ r = usb_autopm_get_interface(interface);
+ if (r 0) {
+ kref_put(dev-kref, lcd_delete);
+ return r;
+ }
+
/* save our object in the file's private structure */
file-private_data = dev;
@@ -100,6 +108,7 @@ static int lcd_release(struct inode *ino
return -ENODEV;
/* decrement the count on our device */
+ usb_autopm_put_interface(dev-interface);
kref_put(dev-kref, lcd_delete);
return 0;
}
@@ -225,12 +234,14 @@ static ssize_t lcd_write(struct file *fi
usb_sndbulkpipe(dev-udev,
dev-bulk_out_endpointAddr),
buf, count, lcd_write_bulk_callback, dev);
urb-transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+
+ usb_anchor_urb(urb, dev-submitted);
/* send the data out the bulk port */
retval = usb_submit_urb(urb, GFP_KERNEL);
if (retval) {
err(USBLCD: %s - failed submitting write urb, error %d,
__FUNCTION__, retval);
- goto error;
+ goto error_unanchor;
}
/* release our reference to this urb, the USB core will eventually free
it entirely */
@@ -238,7 +249,8 @@ static ssize_t lcd_write(struct file *fi
exit:
return count;
-
+error_unanchor:
+ usb_unanchor_urb(urb);
error:
usb_buffer_free(dev-udev, count, buf, urb-transfer_dma);
usb_free_urb(urb);
@@ -283,6 +295,7 @@ static int lcd_probe(struct usb_interfac
}
kref_init(dev-kref);
sema_init(dev-limit_sem, USB_LCD_CONCURRENT_WRITES);
+ init_usb_anchor(dev-submitted);
dev-udev = usb_get_dev(interface_to_usbdev(interface));
dev-interface = interface;
@@ -350,6 +363,30 @@ error:
return retval;
}
+static void lcd_draw_down(struct usb_lcd *dev)
+{
+ int time;
+
+ time = usb_wait_anchor_empty_timeout(dev-submitted, 1000);
+ if (!time)
+ usb_kill_anchored_urbs(dev-submitted);
+}
+
+static int lcd_suspend(struct usb_interface *intf, pm_message_t message)
+{
+ struct usb_lcd *dev = usb_get_intfdata(intf);
+
+ if (!dev)
+ return 0;
+ lcd_draw_down(dev);
+ return 0;
+}
+
+static int lcd_resume (struct usb_interface *intf)
+{
+ return 0;
+}
+
static void lcd_disconnect(struct usb_interface *interface)
{
struct usb_lcd *dev;
@@ -371,7 +408,10 @@ static struct usb_driver lcd_driver = {
.name = usblcd,
.probe =lcd_probe,
.disconnect = lcd_disconnect,
+ .suspend = lcd_suspend,
+ .resume = lcd_resume,
.id_table = id_table,
+ .supports_autosuspend = 1,
};
static int __init usb_lcd_init(void)
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c
On Wed, 13 Jun 2007, Grant Likely wrote: On 6/13/07, Alan Stern [EMAIL PROTECTED] wrote: On Wed, 13 Jun 2007, Grant Likely wrote: On 6/12/07, Peter Korsgaard [EMAIL PROTECTED] wrote: Grant == Grant Likely [EMAIL PROTECTED] writes: Hi, Grant Rather than c67x00-hub.c being compiled seperately, the Grant original code had c67x00-hub.c *included* by c67x00-hcd.c. Grant This is a very bad idea. What's so bad about it? It's an elegant solution to the problem of breaking a very long driver up into smaller, more digestible pieces without polluting the kernel's namespace with lots of extra global symbols. Primarily because it breaks convention. Convention is that you #include .h files, and you compile and link .c files. Convention is important because it reflects the common patterns we use when reading and writing (but mostly reading) code. The problem is that there are conflicting conventions. You mentioned one. But there's another: .h files contain declarations and things that should be shared among multiple source files, and .c files contain things that generate object code (executable routines, static definitions, and so on). The idea is that sharing something which generates object code would be a mistake, since every source file which included it would generate a copy of those same objects. So if you want to #include a file which generates object code, one convention says it should be named .h and the other says it should be named .c. A possible solution would be to use yet a different suffix, but I think that would only make matters worse. (Just to add to the confusion, some people feel that .c files shouldn't include much that _doesn't_ generate object code. Hence they put top-level declarations in a .h file, even though it is #included in only one .c file. This is mainly a matter of taste...) Yes there are exceptions, and yes it can be done, but there better be a damn good reason for doing so. In this particular case, I really don't think it is warranted. The reason for doing it is the second convention. IMO that's just as good a reason for doing it as the first convention is for not doing it. We're not talking about a great deal of code, and we're *already* polluting the kernel namespace with c67x00_* function names because the driver is already in multiple pieces. Sorry, I don't know what you mean. How does the fact that uhci-hcd is in multiple pieces create names like c67x00_*? Besides, the fact that we are already doing it doesn't justify unnecessarily doing even more. This issue has also come up on the LKML also. See this thread: http://thread.gmane.org/gmane.linux.kernel/498633 I read that thread some time ago. If you look at it carefully, you'll find that Linus is arguing in favor of the second convention -- mine, not yours. What's so ugly about breaking a driver up into pieces? Leaving it in one giant piece would be much more ugly IMO. Breaking into pieces: Good, and I fully agree. Doing it in non-standard way: Not so good as it trades one kind of ugliness for another. But what should one do when there are two conflicting standards? Alan Stern - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c
On 6/13/07, Alan Stern [EMAIL PROTECTED] wrote: On Wed, 13 Jun 2007, Grant Likely wrote: On 6/13/07, Alan Stern [EMAIL PROTECTED] wrote: On Wed, 13 Jun 2007, Grant Likely wrote: On 6/12/07, Peter Korsgaard [EMAIL PROTECTED] wrote: Grant == Grant Likely [EMAIL PROTECTED] writes: Hi, Grant Rather than c67x00-hub.c being compiled seperately, the Grant original code had c67x00-hub.c *included* by c67x00-hcd.c. Grant This is a very bad idea. What's so bad about it? It's an elegant solution to the problem of breaking a very long driver up into smaller, more digestible pieces without polluting the kernel's namespace with lots of extra global symbols. Primarily because it breaks convention. Convention is that you #include .h files, and you compile and link .c files. Convention is important because it reflects the common patterns we use when reading and writing (but mostly reading) code. The problem is that there are conflicting conventions. You mentioned one. But there's another: .h files contain declarations and things that should be shared among multiple source files, and .c files contain things that generate object code (executable routines, static definitions, and so on). The idea is that sharing something which generates object code would be a mistake, since every source file which included it would generate a copy of those same objects. I agree 100% So if you want to #include a file which generates object code, one convention says it should be named .h and the other says it should be named .c. A possible solution would be to use yet a different suffix, but I think that would only make matters worse. Right, so I disagree with both approaches. If it generates object code, it should go into a .c file which is compiled and linked on it's own. (Just to add to the confusion, some people feel that .c files shouldn't include much that _doesn't_ generate object code. Hence they put top-level declarations in a .h file, even though it is #included in only one .c file. This is mainly a matter of taste...) Heeheehee Yes there are exceptions, and yes it can be done, but there better be a damn good reason for doing so. In this particular case, I really don't think it is warranted. The reason for doing it is the second convention. IMO that's just as good a reason for doing it as the first convention is for not doing it. I think we're crossing wires here. In this particular case, I think the hub support code is sufficiently small that it doesn't need to be split off into a separate file. (It's only 180 lines) I'm not suggesting that the hub support stuff be moved to a .h file. If the code was larger, I argue that c67x00-hub.c should be compiled separately from c67x00-hcd.c and that 'c67x00-hub.c' be added to 'c67x00-$(CONFIG_USB_C67X00_HCD)' in the Makefile. We're not talking about a great deal of code, and we're *already* polluting the kernel namespace with c67x00_* function names because the driver is already in multiple pieces. Sorry, I don't know what you mean. How does the fact that uhci-hcd is in multiple pieces create names like c67x00_*? Besides, the fact that we are already doing it doesn't justify unnecessarily doing even more. Heh, 'polluted' is too loaded a term, and it suggests something I didn't mean. When the driver is built into the kernel, there are a number of c67x00_* symbols which are exported. These symbols are not used anywhere other than in the c67x00 driver code. However, this is necessary because the overall driver splits the various subsystems into separate .c files which are linked together. This approach is well supported by convention in the kernel, and all the non-static symbols use the c67x00_ prefix to avoid collisions. For example, see ib_core-y in drivers/infiniband/core/Makefile and pcieportdrv-y in drivers/pci/pcie/Makefile. Since the driver already makes use of this approach, I don't think it makes sense to use a difference approach for the root hub support code. (Again, I'm specifically talking about the c67x00 driver here; I've not looked at the *hci drivers in detail). Of course, when it is built as a module, none of those symbols show up because they are not exported. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. [EMAIL PROTECTED] (403) 399-0195 - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
[linux-usb-devel] Possible bug in isp116x-hcd
Olav:
While going through the HCDs, I found this possible bug in your driver.
Do you agree that the patch below is needed? If yes, then I will fold
it in with the new urb-status updates.
Alan Stern
Index: usb-2.6/drivers/usb/host/isp116x-hcd.c
===
--- usb-2.6.orig/drivers/usb/host/isp116x-hcd.c
+++ usb-2.6/drivers/usb/host/isp116x-hcd.c
@@ -583,12 +583,15 @@ static void finish_atl_transfers(struct
unpack_fifo(isp116x);
postproc_atl_queue(isp116x);
for (ep = isp116x-atl_active; ep; ep = ep-active) {
+ if (list_empty(ep-hep-urb_list))
+ continue;
urb =
container_of(ep-hep-urb_list.next, struct urb, urb_list);
+
/* USB_PID_ACK check here avoids finishing of
control transfers, for which TD_DATAUNDERRUN
occured, while URB_SHORT_NOT_OK was set */
- if (urb urb-status != -EINPROGRESS
+ if (urb-status != -EINPROGRESS
ep-nextpid != USB_PID_ACK)
finish_request(isp116x, ep, urb);
}
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] [RFC] Eliminate usb_unlink_urb()'s return code
Am Mittwoch, 13. Juni 2007 schrieb Alan Stern: IMO the fine distinctions above simply don't matter. Drivers don't care anyway, since they can't know what stage the URB is at when they try to unlink it. The only possible place where this might matter is in usbtest, which specifically intends to test the unlink code paths. Even there, I'm not sure that usb_unlink_urb needs to return a meaningful value; the URB's final status code ought to be sufficient. So -- is there any objection if I change usb_unlink_urb to return void? As far as I am concerned, change it to void. Regards Oliver - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c
On Wed, 13 Jun 2007, Grant Likely wrote: I think we're crossing wires here. In this particular case, I think the hub support code is sufficiently small that it doesn't need to be split off into a separate file. (It's only 180 lines) I'm not suggesting that the hub support stuff be moved to a .h file. If the code was larger, I argue that c67x00-hub.c should be compiled separately from c67x00-hcd.c and that 'c67x00-hub.c' be added to 'c67x00-$(CONFIG_USB_C67X00_HCD)' in the Makefile. Heh, 'polluted' is too loaded a term, and it suggests something I didn't mean. When the driver is built into the kernel, there are a number of c67x00_* symbols which are exported. These symbols are not used anywhere other than in the c67x00 driver code. However, this is necessary because the overall driver splits the various subsystems into separate .c files which are linked together. This approach is well supported by convention in the kernel, and all the non-static symbols use the c67x00_ prefix to avoid collisions. For example, see ib_core-y in drivers/infiniband/core/Makefile and pcieportdrv-y in drivers/pci/pcie/Makefile. Since the driver already makes use of this approach, I don't think it makes sense to use a difference approach for the root hub support code. (Again, I'm specifically talking about the c67x00 driver here; I've not looked at the *hci drivers in detail). But you did say earlier that the way other drivers were written was a bad idea... In any case I agree that the root-hub code in c67x00 should be written to match the style used by the rest of the driver. Of course, when it is built as a module, none of those symbols show up because they are not exported. True. And true as well of the other drivers which #include *.c files. Alan Stern - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
[linux-usb-devel] [patch]race leading to use after free in io_edgeport
Hi,
usb_unlink_urb() is asynchronous, therefore an URB's buffer may not
be freed without waiting for the completion handler. This patch switches
to usb_kill_urb(), which is synchronous.
Thanks to Alan for making me look at the remaining users of usb_unlink_urb()
Regards
Oliver
Signed-off-by: Oliver Neukum [EMAIL PROTECTED]
--
--- a/drivers/usb/serial/io_edgeport.c 2007-06-13 18:35:25.0 +0200
+++ b/drivers/usb/serial/io_edgeport.c 2007-06-13 18:36:04.0 +0200
@@ -3046,11 +3046,11 @@ static void edge_shutdown (struct usb_se
}
/* free up our endpoint stuff */
if (edge_serial-is_epic) {
- usb_unlink_urb(edge_serial-interrupt_read_urb);
+ usb_kill_urb(edge_serial-interrupt_read_urb);
usb_free_urb(edge_serial-interrupt_read_urb);
kfree(edge_serial-interrupt_in_buffer);
- usb_unlink_urb(edge_serial-read_urb);
+ usb_kill_urb(edge_serial-read_urb);
usb_free_urb(edge_serial-read_urb);
kfree(edge_serial-bulk_in_buffer);
}
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
[linux-usb-devel] Bug and simplifications in r8a66597-hcd
Yoshihiro:
While going through the HCDs, I found this bug and some simplifications
in your driver. While you hold a spinlock, memory allocation cannot
use anything other than GFP_ATOMIC.
Do you agree that the patch below is correct? If yes, then I will fold
it in with the new urb-status updates.
Alan Stern
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
@@ -1197,7 +1197,7 @@ static void packet_read(struct r8a66597
td-short_packet = 1;
if (urb-transfer_buffer_length != urb-actual_length
urb-transfer_flags URB_SHORT_NOT_OK)
- td-urb-status = -EREMOTEIO;
+ urb-status = -EREMOTEIO;
}
if (usb_pipeisoc(urb-pipe)) {
urb-iso_frame_desc[td-iso_cnt].actual_length = size;
@@ -1222,8 +1222,8 @@ static void packet_read(struct r8a66597
}
if (finish pipenum != 0) {
- if (td-urb-status == -EINPROGRESS)
- td-urb-status = 0;
+ if (urb-status == -EINPROGRESS)
+ urb-status = 0;
finish_request(r8a66597, td, pipenum, urb);
}
}
@@ -1247,7 +1247,7 @@ static void packet_write(struct r8a66597
pipe_stop(r8a66597, td-pipe);
pipe_irq_disable(r8a66597, pipenum);
err(out write fifo not ready. (%d), pipenum);
- finish_request(r8a66597, td, pipenum, td-urb);
+ finish_request(r8a66597, td, pipenum, urb);
return;
}
@@ -1693,13 +1693,12 @@ static void set_address_zero(struct r8a6
static struct r8a66597_td *r8a66597_make_td(struct r8a66597 *r8a66597,
struct urb *urb,
- struct usb_host_endpoint *hep,
- gfp_t mem_flags)
+ struct usb_host_endpoint *hep)
{
struct r8a66597_td *td;
u16 pipenum;
- td = kzalloc(sizeof(struct r8a66597_td), mem_flags);
+ td = kzalloc(sizeof(struct r8a66597_td), GFP_ATOMIC);
if (td == NULL)
return NULL;
@@ -1738,7 +1737,8 @@ static int r8a66597_urb_enqueue(struct u
}
if (!hep-hcpriv) {
- hep-hcpriv = kzalloc(sizeof(struct r8a66597_pipe), mem_flags);
+ hep-hcpriv = kzalloc(sizeof(struct r8a66597_pipe),
+ GFP_ATOMIC);
if (!hep-hcpriv) {
ret = -ENOMEM;
goto error;
@@ -1752,7 +1752,7 @@ static int r8a66597_urb_enqueue(struct u
init_pipe_config(r8a66597, urb);
set_address_zero(r8a66597, urb);
- td = r8a66597_make_td(r8a66597, urb, hep, mem_flags);
+ td = r8a66597_make_td(r8a66597, urb, hep);
if (td == NULL) {
ret = -ENOMEM;
goto error;
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] ehci_hcd causes box to resume immediately after suspend to RAM
On Monday, 11 June 2007 22:10, Alan Stern wrote: On Mon, 11 Jun 2007, Rafael J. Wysocki wrote: At that point, does lspci -vv show that the controller is trying to signal a wakeup event? That is, is the PME# signal asserted? (Not that knowing this will help very much -- I'm not sure what we could do with that information, and in any case there are other ways besides PME# for on-board devices to report wakeup requests. I ask mainly out of curiousity.) It shows this literally: 00:1d.7 USB Controller: Intel Corporation 82801DB/DBM (ICH4/ICH4-M) USB2 EHCI Controller (rev 02) (prog-if 20 [EHCI]) Subsystem: ASUSTeK Computer Inc. Unknown device 8089 Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium TAbort- TAbort- MAbort- SERR- PERR- Interrupt: pin D routed to IRQ 20 Region 0: Memory at febffc00 (32-bit, non-prefetchable) [size=1K] Capabilities: [50] Power Management version 2 Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA PME(D0+,D1-,D2-,D3hot+,D3cold+) Status: D3 PME-Enable+ DSel=0 DScale=0 PME- Capabilities: [58] Debug port Which means that the controller is in D3 and it supports PME#, but PME# isn't turned on. But as I said, Intel controllers may signal wakeup requests in a different way (UHCI controllers definitely do, but maybe not EHCI). The simplest workaround should be to disable remote wakeup for that controller: echo disable /sys/bus/pci/devices/.../power/wakeup I tried that but it didn't help. Namely, the box resumed right after suspending as it had done before. The only way to prevent it from resuming immediately after the suspend is to 'rmmod ehci_hcd' before the suspend. Hmmm... If you turn on CONFIG_USB_DEBUG, what shows up in /sys/class/usb_host/usb_hostN/registers where N is the bus number of the controller? bus pci, device :00:1d.7 (driver 10 Dec 2004) EHCI Host Controller EHCI 1.00, hcd state 4 ownership 0001 SMI sts/enable 0x8008 structural params 0x00103206 capability params 0x6871 status 1008 Halt FLR command 01 (park)=0 ithresh=1 period=1024 HALT intrenable 37 IAA FATAL PCD ERR INT uframe 36f1 port 1 status 701000 POWER sig=se0 port 2 status 701000 POWER sig=se0 port 3 status 701000 POWER sig=se0 port 4 status 701000 POWER sig=se0 port 5 status 701000 POWER sig=se0 port 6 status 701000 POWER sig=se0 irq normal 0 err 0 reclaim 0 (lost 0) complete 0 unlink 0 Also, can you post a dmesg log (with CONFIG_USB_DEBUG enabled) showing what happens during the suspend and immediate resume? [That's after I have disabled the wakeup on the EHCI controller.] PM: Preparing system for mem sleep Stopping tasks ... done. Suspending console(s) pnp: Device 00:07 disabled. pnp: Device 00:05 disabled. ehci_hcd :00:1d.7: -- PCI D3 uhci_hcd :00:1d.2: uhci_suspend uhci_hcd :00:1d.2: -- PCI D0/legacy uhci_hcd :00:1d.1: uhci_suspend uhci_hcd :00:1d.1: -- PCI D0/legacy uhci_hcd :00:1d.0: uhci_suspend uhci_hcd :00:1d.0: -- PCI D0/legacy PM: Entering mem sleep Intel machine check architecture supported. Intel machine check reporting enabled on CPU#0. CPU0: Intel P4/Xeon Extended MCE MSRs (12) available CPU0: Thermal monitoring enabled Back to C! PM: Finishing wakeup. uhci_hcd :00:1d.0: PCI legacy resume ACPI: PCI Interrupt :00:1d.0[A] - GSI 16 (level, low) - IRQ 18 PCI: Setting latency timer of device :00:1d.0 to 64 uhci_hcd :00:1d.0: uhci_resume uhci_hcd :00:1d.0: uhci_check_and_reset_hc: legsup = 0x2f00 uhci_hcd :00:1d.0: Performing full reset usb usb2: root hub lost power or was reset usb usb2: suspend_rh uhci_hcd :00:1d.1: PCI legacy resume ACPI: PCI Interrupt :00:1d.1[B] - GSI 19 (level, low) - IRQ 19 PCI: Setting latency timer of device :00:1d.1 to 64 uhci_hcd :00:1d.1: uhci_resume uhci_hcd :00:1d.1: uhci_check_and_reset_hc: legsup = 0x2000 uhci_hcd :00:1d.1: Performing full reset usb usb3: root hub lost power or was reset usb usb3: suspend_rh uhci_hcd :00:1d.2: PCI legacy resume ACPI: PCI Interrupt :00:1d.2[C] - GSI 18 (level, low) - IRQ 16 PCI: Setting latency timer of device :00:1d.2 to 64 uhci_hcd :00:1d.2: uhci_resume uhci_hcd :00:1d.2: uhci_check_and_reset_hc: legsup = 0x2000 uhci_hcd :00:1d.2: Performing full reset usb usb4: root hub lost power or was reset usb usb4: suspend_rh ehci_hcd :00:1d.7: PCI D0, from previous PCI D3 ACPI: PCI Interrupt :00:1d.7[D] - GSI 23 (level, low) - IRQ 17 PCI: Setting latency timer of device :00:1d.7 to 64 PM: Writing back config space on device :00:1e.0 at offset 7 (was 2280d0d0, writing 280d0d0) PCI: Setting latency timer of device :00:1e.0 to 64 PM: Writing back config space on device :00:1f.1 at offset 9 (was 0, writing 3000) PM: Writing back config
Re: [linux-usb-devel] [PATCH] USB: ehci_hub_control() (Kernel 2.6.21.4)
From: Craig W. Nadler [EMAIL PROTECTED]
EHCI_HUB_CTRL: Fixes a bug with setting the value of the port number.
The ehci_hub_control() function in driver/usb/host/ehci-hub.c takes a u16 parameter
called wIndex. The least significant byte of this parameter holds the port number
relevant to the function call. The other byte in wIndex is sometimes used to hold
another value. A pointer to the port_status register for the relevant port was
being set using wIndex as a subscript to an array without masking off the other
byte. This patch adds a local unsigned variable called port_num which is set to
((wIndex 0xFF) - 1). The pointer for the port_status register is then set
using port_num as the subscript. Port_num is also used anywhere in
ehci_hub_control() where wIndex contains the port number.
Signed-off-by: Craig W. Nadler [EMAIL PROTECTED]
---
diff -uprN a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
--- a/drivers/usb/host/ehci-hub.c 2007-06-07 17:27:31.0 -0400
+++ b/drivers/usb/host/ehci-hub.c 2007-06-11 18:28:30.0 -0400
@@ -444,11 +444,12 @@ static int ehci_hub_control (
) {
struct ehci_hcd *ehci = hcd_to_ehci (hcd);
int ports = HCS_N_PORTS (ehci-hcs_params);
- u32 __iomem *status_reg = ehci-regs-port_status[wIndex - 1];
+ unsigned port_num = (wIndex 0xFF) - 1;
+ unsigned selector = (wIndex 8) 0xFF;
+ u32 __iomem *status_reg = ehci-regs-port_status[port_num];
u32 temp, status;
unsigned long flags;
int retval = 0;
- unsigned selector;
/*
* FIXME: support SetPortFeatures USB_PORT_FEAT_INDICATOR.
@@ -470,9 +471,8 @@ static int ehci_hub_control (
}
break;
case ClearPortFeature:
- if (!wIndex || wIndex ports)
+ if (port_num = ports)
goto error;
- wIndex--;
temp = ehci_readl(ehci, status_reg);
/*
@@ -502,7 +502,7 @@ static int ehci_hub_control (
temp = ~(PORT_RWC_BITS | PORT_WAKE_BITS);
ehci_writel(ehci, temp | PORT_RESUME,
status_reg);
-ehci-reset_done [wIndex] = jiffies
+ehci-reset_done[port_num] = jiffies
+ msecs_to_jiffies (20);
}
break;
@@ -541,9 +541,8 @@ static int ehci_hub_control (
//cpu_to_le32s ((u32 *) buf);
break;
case GetPortStatus:
- if (!wIndex || wIndex ports)
+ if (port_num = ports)
goto error;
- wIndex--;
status = 0;
temp = ehci_readl(ehci, status_reg);
@@ -559,20 +558,20 @@ static int ehci_hub_control (
if (temp PORT_RESUME) {
/* Remote Wakeup received? */
- if (!ehci-reset_done[wIndex]) {
+ if (!ehci-reset_done[port_num]) {
/* resume signaling for 20 msec */
-ehci-reset_done[wIndex] = jiffies
+ehci-reset_done[port_num] = jiffies
+ msecs_to_jiffies(20);
/* check the port again */
mod_timer(ehci_to_hcd(ehci)-rh_timer,
- ehci-reset_done[wIndex]);
+ ehci-reset_done[port_num]);
}
/* resume completed? */
else if (time_after_eq(jiffies,
- ehci-reset_done[wIndex])) {
+ ehci-reset_done[port_num])) {
status |= 1 USB_PORT_FEAT_C_SUSPEND;
-ehci-reset_done[wIndex] = 0;
+ehci-reset_done[port_num] = 0;
/* stop resume signaling */
temp = ehci_readl(ehci, status_reg);
@@ -584,7 +583,7 @@ static int ehci_hub_control (
if (retval != 0) {
ehci_err(ehci,
port %d resume error %d\n,
- wIndex + 1, retval);
+ port_num + 1, retval);
goto error;
}
temp = ~(PORT_SUSPEND|PORT_RESUME|(310));
@@ -594,9 +593,9 @@ static int ehci_hub_control (
/* whoever resets must GetPortStatus to complete it!! */
if ((temp PORT_RESET)
time_after_eq(jiffies,
- ehci-reset_done[wIndex])) {
+ ehci-reset_done[port_num])) {
status |= 1 USB_PORT_FEAT_C_RESET;
- ehci-reset_done [wIndex] = 0;
+ ehci-reset_done[port_num] = 0;
/* force reset to complete */
ehci_writel(ehci, temp ~(PORT_RWC_BITS | PORT_RESET),
@@ -608,22 +607,22 @@ static int ehci_hub_control (
PORT_RESET, 0, 750);
if (retval != 0) {
ehci_err (ehci, port %d reset error %d\n,
- wIndex + 1, retval);
+ port_num + 1, retval);
goto error;
}
/* see what we found out */
- temp = check_reset_complete (ehci, wIndex, status_reg,
+ temp = check_reset_complete (ehci, port_num, status_reg,
ehci_readl(ehci, status_reg));
}
/* transfer dedicated ports to the companion hc */
if ((temp PORT_CONNECT)
-test_bit(wIndex, ehci-companion_ports)) {
+test_bit(port_num, ehci-companion_ports)) {
temp = ~PORT_RWC_BITS;
temp |= PORT_OWNER;
ehci_writel(ehci, temp, status_reg);
- ehci_dbg(ehci, port %d -- companion\n, wIndex + 1);
+ ehci_dbg(ehci, port %d -- companion\n, port_num + 1);
temp = ehci_readl(ehci, status_reg);
}
@@ -652,7 +651,7 @@ static int ehci_hub_control (
#ifndef EHCI_VERBOSE_DEBUG
if (status ~0x) /* only if wPortChange is interesting */
#endif
- dbg_port (ehci, GetStatus, wIndex + 1, temp);
+ dbg_port (ehci, GetStatus, port_num + 1, temp);
Re: [linux-usb-devel] [PATCH] USB IAD Support (kernel2.6.21.3)
Alan Stern wrote:
On Mon, 11 Jun 2007, Craig W. Nadler wrote:
+
+ first_intf = intf_assoc-bFirstInterface;
+ last_intf = first_intf + (intf_assoc-bInterfaceCount - 1);
+ if (inum = first_intf inum = last_intf) {
+ if (!retval) {
+ retval = intf_assoc;
+ } else {
+ dev_err (dev-dev, Interface #%d referenced
+by multiple IADs\n, inum);
+ }
Are you really sure this needs to be an error and not a warning?
Remember also what I said earlier about putting extraneous space
characters between a function name and the following open paren.
The IAD spec. states that an interface can only be referenced by one
IAD. The structure of the IAD also forces the interfaces referenced to
be consecutively numbered. The interfaces are referenced in the
descriptor by the interface number of the first associated interface and
the number of interfaces.
diff -uprN a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h 2007-05-24 17:22:47.0 -0400
+++ b/include/linux/usb.h 2007-06-09 03:20:51.0 -0400
@@ -245,6 +250,11 @@ struct usb_host_config {
struct usb_config_descriptordesc;
char *string; /* iConfiguration string, if present */
+
+ /* List of any Interface Association Descriptors in this
+* configuration. */
+ struct usb_interface_assoc_descriptor *intf_assoc[USB_MAXIADS];
This may be just my own confusion, but is it necessary to list
interface association descriptors in both the host_interface and the
host_config structures?
The list of IADs in the host_config is all the IADs for that
configuration. The IAD pointer in the host_interface structure is only
the one that references that interface. The first is used if you're
looking at the configuration and trying to find groups of associated
interfaces. The second is used if you are probing an interface and
trying to find other interfaces associated with it.
An updated patch is attached.
Best Regards,
Craig Nadler
diff -uprN a/drivers/usb/core/config.c b/drivers/usb/core/config.c
--- a/drivers/usb/core/config.c 2007-06-07 17:27:31.0 -0400
+++ b/drivers/usb/core/config.c 2007-06-13 23:05:33.0 -0400
@@ -232,6 +232,7 @@ static int usb_parse_configuration(struc
struct usb_descriptor_header *header;
int len, retval;
u8 inums[USB_MAXINTERFACES], nalts[USB_MAXINTERFACES];
+ unsigned iad_num = 0;
memcpy(config-desc, buffer, USB_DT_CONFIG_SIZE);
if (config-desc.bDescriptorType != USB_DT_CONFIG ||
@@ -309,6 +310,20 @@ static int usb_parse_configuration(struc
++n;
}
+ } else if (header-bDescriptorType ==
+USB_DT_INTERFACE_ASSOCIATION) {
+ if (iad_num == USB_MAXIADS) {
+dev_warn(ddev, found more Interface
+ Association Descriptors
+ than allocated for in
+ configuration %d\n, cfgno);
+ } else {
+config-intf_assoc[iad_num] =
+ (struct usb_interface_assoc_descriptor
+ *)header;
+iad_num++;
+ }
+
} else if (header-bDescriptorType == USB_DT_DEVICE ||
header-bDescriptorType == USB_DT_CONFIG)
dev_warn(ddev, config %d contains an unexpected
diff -uprN a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
--- a/drivers/usb/core/devices.c 2007-06-07 17:27:31.0 -0400
+++ b/drivers/usb/core/devices.c 2007-06-13 22:02:16.0 -0400
@@ -102,6 +102,10 @@ static const char *format_config =
/* C: #Ifs=dd Cfg#=dd Atr=xx MPwr=dddmA */
C:%c #Ifs=%2d Cfg#=%2d Atr=%02x MxPwr=%3dmA\n;
+static const char *format_iad =
+/* A: FirstIf#=dd IfCount=dd Cls=xx(s) Sub=xx Prot=xx */
+ A: FirstIf#=%2d IfCount=%2d Cls=%02x(%-5s) Sub=%02x Prot=%02x\n;
+
static const char *format_iface =
/* I: If#=dd Alt=dd #EPs=dd Cls=xx(s) Sub=xx Prot=xx Driver=*/
I:%c If#=%2d Alt=%2d #EPs=%2d Cls=%02x(%-5s) Sub=%02x Prot=%02x Driver=%s\n;
@@ -146,6 +150,7 @@ static const struct class_info clas_info
{USB_CLASS_STILL_IMAGE, still},
{USB_CLASS_CSCID, scard},
{USB_CLASS_CONTENT_SEC, c-sec},
+ {USB_CLASS_VIDEO, video},
{-1,unk.} /* leave as last */
};
@@ -288,6 +293,21 @@ static char *usb_dump_interface(
return start;
}
+static char *usb_dump_iad_descriptor(char *start, char *end,
+ const struct usb_interface_assoc_descriptor *iad)
+{
+ if (start end)
+ return start;
+ start += sprintf(start, format_iad,
+ iad-bFirstInterface,
+ iad-bInterfaceCount,
+ iad-bFunctionClass,
+ class_decode(iad-bFunctionClass),
+ iad-bFunctionSubClass,
+ iad-bFunctionProtocol);
+ return start;
+}
+
/* TBD:
* 0. TBDs
* 1. marking active interface altsettings (code lists all, but should mark
@@ -324,6 +344,12 @@ static char *usb_dump_config (
if (!config) /* getting these some in 2.3.7; none in 2.3.6 */
