Re: [linux-usb-devel] usb-serial: Custom ioctl
On Fri, Feb 21, 2003 at 10:04:14PM +0100, Thomas Jarosch wrote:
This is a patch against the ftdi_sio 1.3.0 driver + interim patch.
The new driver should be out soon...
Can you post it not compressed so we can read it?
Sure. I compressed it to save bandwith...
Thomas
diff -u -r -b driver.clean/ftdi_sio.c driver.new/ftdi_sio.c
--- driver.clean/ftdi_sio.c 2003-02-19 18:42:20.0 +0100
+++ driver.new/ftdi_sio.c 2003-02-19 20:08:14.0 +0100
-932,7 +932,7
int user_bytes_sent = 0 ; /* amount of user data sent */
/* Variables for urb pool management */
- unsigned char *current_position = buf;
+ unsigned char *current_position = (unsigned char *)buf;
int i;
struct urb *urb; /* pointer to urb from urb pool */
unsigned long flags;
-1051,7 +1051,6
{
struct usb_serial_port *port = (struct usb_serial_port *)urb-context;
struct usb_serial *serial = get_usb_serial (port, __FUNCTION__);
- unsigned long flags;
dbg(%s, __FUNCTION__);
-1417,9 +1416,12
struct usb_serial *serial = port-serial;
struct ftdi_private *priv = (struct ftdi_private *)port-private;
+ ftdi_bitbang_t bitbang;
+ ftdi_eeprom_t eeprom;
+
__u16 urb_value=0; /* Will hold the new flags */
char buf[2];
- int ret, mask;
+ int ret, mask, i;
dbg(%s cmd 0x%04x, __FUNCTION__, cmd);
-1572,17 +1574,157
*/
}
}
- /* NOTREACHED */
+ }
+
+ if (_IOC_TYPE(cmd) == FTDI_IOCTL_CHAR) {
+ switch (_IOC_NR (cmd)) {
+
+ case _IOC_NR(FTDI_IOCTL_SET_BITBANG_MODE):
+ copy_from_user (bitbang, (void *)arg, sizeof (ftdi_bitbang_t));
+
+ urb_value = bitbang.bitmask;
+ urb_value += bitbang.enable 8;
+
+ if ((ret = usb_control_msg(serial-dev,
+ usb_sndctrlpipe(serial-dev, 0),
+ FTDI_SET_BITBANG_MODE_REQUEST,
+ FTDI_SET_BITBANG_MODE_REQUEST_TYPE,
+ urb_value, 0,
+ buf, 0, WDR_TIMEOUT)) 0 ) {
+err(%s: Could not set Bit Bang mode - err: %d,
+ __FUNCTION__, ret);
+return(ret);
+ }
+
+ if (bitbang.enable)
+dbg (%s: Enabled Bit Bang mode\n, __FUNCTION__);
+ else
+dbg (%s: Disabled Bit Bang mode\n, __FUNCTION__);
+
+ return 0;
+ break;
+ case _IOC_NR(FTDI_IOCTL_GET_DATAPINS):
+ if ((ret = usb_control_msg(serial-dev,
+ usb_rcvctrlpipe(serial-dev, 0),
+ FTDI_GET_DATAPINS_REQUEST,
+ FTDI_GET_DATAPINS_REQUEST_TYPE,
+ 0, 0,
+ buf, 1, WDR_TIMEOUT)) 0 ) {
+err(%s: Could not read pins directly - err: %d,
+ __FUNCTION__, ret);
+return(ret);
+ }
+
+ dbg (%s: Read pins: %d\n, __FUNCTION__, (unsigned char)buf[0]);
+
+ if (copy_to_user((void *)arg, buf, 1))
+return -EFAULT;
+ return 0;
+ break;
+ case _IOC_NR(FTDI_IOCTL_GET_EEPROM):
+ for (i = 0; i 64; i++) {
+if ((ret = usb_control_msg(serial-dev,
+ usb_rcvctrlpipe(serial-dev, 0),
+ FTDI_E2_READ_REQUEST,
+ FTDI_E2_READ_REQUEST_TYPE,
+ 0, i,
+ buf, 2, WDR_TIMEOUT)) 0 ) {
+ err(%s: Could not read pins directly - err: %d,
+ __FUNCTION__, ret);
+ return(ret);
+}
+
+eeprom.data[i*2] = buf[0];
+eeprom.data[(i*2)+1] = buf[1];
+ }
+
+ if (copy_to_user((void *)arg, eeprom, sizeof(ftdi_eeprom_t)))
+return -EFAULT;
+
+ return 0;
+ break;
+
+ case _IOC_NR(FTDI_IOCTL_SET_EEPROM):
+ copy_from_user (eeprom, (void *)arg, sizeof (ftdi_eeprom_t));
+
+ for (i = 0; i 64; i++) {
+urb_value = eeprom.data[i*2];
+urb_value += eeprom.data[(i*2)+1] 8;
+
+if ((ret = usb_control_msg(serial-dev,
+ usb_sndctrlpipe(serial-dev, 0),
+ FTDI_E2_WRITE_REQUEST,
+ FTDI_E2_WRITE_REQUEST_TYPE,
+ urb_value, i,
+ buf, 0, WDR_TIMEOUT)) 0 ) {
+ err(%s: Could write eeprom - err: %d,
+ __FUNCTION__, ret);
+ return(ret);
+}
+
+ }
+
+ return 0;
+ break;
+ case _IOC_NR(FTDI_IOCTL_ERASE_EEPROM):
+ info (Erasing eeprom!);
+
+ if ((ret = usb_control_msg(serial-dev,
+ usb_sndctrlpipe(serial-dev, 0),
+ FTDI_E2_ERASE_REQUEST,
+ FTDI_E2_ERASE_REQUEST_TYPE,
+ 0, 0,
+ buf, 0, WDR_TIMEOUT)) 0 ) {
+err(%s: Could not erase eeprom - err: %d,
+ __FUNCTION__, ret);
+return(ret);
+ }
+
+ return 0;
+ break;
+ case _IOC_NR(FTDI_IOCTL_SET_LATENCY):
+ urb_value = 0;
+ copy_from_user (urb_value, (void *)arg, 1);
+
+ if ((ret = usb_control_msg(serial-dev,
+ usb_sndctrlpipe(serial-dev, 0),
+ FTDI_SET_LATENCY_REQUEST,
+ FTDI_SET_LATENCY_REQUEST_TYPE,
+ urb_value, 0,
+ buf, 0, WDR_TIMEOUT)) 0 ) {
+err(%s: Could not set latency - err: %d,
+ __FUNCTION__, ret);
+return(ret);
+ }
+ return 0;
+ break;
+ case _IOC_NR(FTDI_IOCTL_GET_LATENCY):
+ if ((ret = usb_control_msg(serial-dev,
+ usb_rcvctrlpipe(serial-dev, 0),
+ FTDI_GET_LATENCY_REQUEST,
+ FTDI_GET_LATENCY_REQUEST_TYPE,
+ 0, 0,
+ buf, 1, WDR_TIMEOUT)) 0 ) {
+err(%s: Could not get latency - err: %d,
+
Re: [linux-usb-devel] Re: [PATCH] USB changes for 2.5.62
Am Samstag, 22. Februar 2003 00:15 schrieben Sie: A per-device list is sufficient, and a lot cheaper to maintain. Doing it per-interface would add work to the submit path that's hardly ever needed ... better to have that work delayed until it's actually needed. Then how to implement usb_reset_device() ? In some straightforward manner. Remembering that the two representations are isomorphic, why should there be any issue? Under some circumstances, most obviously the disconnect ioctl, we need to disconnect only one of the interfaces of a device. How are we to do that if we queue by device only? Regards Oliver --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Death by usb_dec_dev_use
On Sun, Feb 23, 2003 at 12:36:02AM +0100, Duncan Sands wrote: The philosophy is often called programming by contract. And the contract is: usbcore agrees to work with your driver and do everything right, if your driver agrees to do a few specific things. One of those specific things is never using the device after you return from disconnect(), either directly (as you're doing here) or indirectly (urb still pending, say). If you break such programming contracts, bad things happen; that's part of why the contract exists. There are lots of components interacting, relying on contracts to be followed, and if they aren't ... it's impractical to guarantee that all the failure modes will always be benign. Pity this contract is undocumented. And flat wrong. For 2.5, your driver can use the struct usb_device for as long as it wants to, provided it got a reference to it in the first place with usb_get_dev(). It makes driver writing a hell of a lot easer, just take a look at all of the nasty lock mess I took out of the usb-serial core because of this. For 2.4, I want to backport these changes to the core to also ensure that this contract is not needed. But I haven't had the time to do so yet. Again, patches gleefully accepted :) thanks, greg k-h --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Discussion of problems in usb-skeleton.c
On Fri, Feb 21, 2003 at 01:12:47PM -0500, Alan Stern wrote: I noticed that usb-skeleton.c still has a TODO entry referring to a race involving urb-status, so I took a closer look. Fixing that race will be easy, and I will be happy to submit a patch for it. But there are two other problems, partially related. Is it worthwhile to try to address them all? Is there no point, seeing as how usb-skeleton.c is just intended as a programming guide? But on the other hand, shouldn't a guide be _especially_ careful to do everything right? Yes, it should be correct. Here are the problems. (1) Device writes use a normal, asynchronous usb_submit_urb() call to do their work. They return without waiting to see if the bulk data transfer succeeded. Thus any problems during data transmission will not be reflected back to the user. That's pretty typical for a number of drivers, unfortunatly. (2) The release() routine unlinks an ongoing write in progress. This means that the last buffer of data written to the device before it is closed will quite likely never be delivered. Owing to problem (1), the user will not be aware of this. Ooh, good catch. Changing the write() routine to use usb_bulk_msg() would fix all these problems, but this would defeat the purpose of showing people how to use usb_submit_urb(), usb_unlink_urb(), and completion handlers. What do you think is the right thing to do here? Don't know, I'd like to show how to do the async urb usage properly, the sync calls are pretty easy for people to work out how to use. How about two different example drivers? (Incidentally, usb-skeleton.c is a good example of a driver where the disconnect() routine can exit with urbs still in flight.) Ah, another good reason for the urb reference count to be added :) thanks, greg k-h --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Death by usb_dec_dev_use
Present for you, Greg! I moved usb_put_dev to the disconnect routine (of course, there is a usb_get_dev in probe). Now I get the following oops on 2.5 during system shutdown: uhci-hcd 00:0b.0: remove, state 3 usb usb1: USB disconnect, address 1 usb 1-2: USB disconnect, address 2 Unable to handle kernel paging request at virtual address 6b6b6c7f printing eip: c01b8e6c *pde = Oops: CPU:0 EIP:0060:[atomic_dec_and_lock+12/160]Not tainted EFLAGS: 00010206 EIP is at atomic_dec_and_lock+0xc/0xa0 eax: 6b6b6c7f ebx: 6b6b6c7f ecx: c02c5234 edx: 00c0 esi: ca826000 edi: c0292140 ebp: c8037de8 esp: c8037ddc ds: 007b es: 007b ss: 0068 Process rmmod (pid: 1287, threadinfo=c8036000 task=ca638d20) Stack: 6b6b6c6f ca826000 c8037dfc c01b7b75 6b6b6c7f c0292140 ca8260f4 c8037e34 cc8b3e23 6b6b6c6f cbfe42dc ca82608c ca82609c 0001 0246 c016f69c ca621344 c02e5080 cc8b5bec cc8b5b78 cc8b5b60 c8037e50 cc8cf1d9 Call Trace: [kobject_put+21/128] kobject_put+0x15/0x80 [cc8b3e23] udsl_usb_disconnect+0x323/0x380 [speedtch] [dput+28/608] dput+0x1c/0x260 [cc8b5bec] udsl_usb_driver+0x8c/0xc0 [speedtch] [cc8b5b78] udsl_usb_driver+0x18/0xc0 [speedtch] [cc8b5b60] udsl_usb_driver+0x0/0xc0 [speedtch] [cc8cf1d9] usb_device_remove+0xb9/0x100 [usbcore] [cc8b5b78] udsl_usb_driver+0x18/0xc0 [speedtch] [device_release_driver+87/96] device_release_driver+0x57/0x60 [cc8e5c60] usb_bus_type+0x0/0xe0 [usbcore] [cc8e5ca0] usb_bus_type+0x40/0xe0 [usbcore] [bus_remove_device+101/192] bus_remove_device+0x65/0xc0 [device_del+106/160] device_del+0x6a/0xa0 [device_unregister+13/26] device_unregister+0xd/0x1a [cc8cfc09] usb_disconnect+0x89/0x100 [usbcore] [cc8cfc5c] usb_disconnect+0xdc/0x100 [usbcore] [cc8d7979] usb_hcd_pci_remove+0x79/0x1a0 [usbcore] [cc8c4b08] uhci_pci_driver+0x28/0xa0 [uhci_hcd] [cc8c4b08] uhci_pci_driver+0x28/0xa0 [uhci_hcd] [pci_device_remove+44/64] pci_device_remove+0x2c/0x40 [device_release_driver+87/96] device_release_driver+0x57/0x60 [cc8c4b60] uhci_pci_driver+0x80/0xa0 [uhci_hcd] [cc8c4b60] uhci_pci_driver+0x80/0xa0 [uhci_hcd] [driver_detach+29/64] driver_detach+0x1d/0x40 [bus_remove_driver+66/128] bus_remove_driver+0x42/0x80 [cc8c4b08] uhci_pci_driver+0x28/0xa0 [uhci_hcd] [cc8c4b08] uhci_pci_driver+0x28/0xa0 [uhci_hcd] [driver_unregister+14/57] driver_unregister+0xe/0x39 [cc8c4b08] uhci_pci_driver+0x28/0xa0 [uhci_hcd] [cc8c4b80] +0x0/0x120 [uhci_hcd] [cc8c26ad] +0xd/0x60 [uhci_hcd] [cc8c4b08] uhci_pci_driver+0x28/0xa0 [uhci_hcd] [sys_delete_module+441/512] sys_delete_module+0x1b9/0x200 [do_munmap+256/448] do_munmap+0x100/0x1c0 [sys_munmap+65/96] sys_munmap+0x41/0x60 [syscall_call+7/11] syscall_call+0x7/0xb Code: 8b 33 89 f2 4a 74 19 89 f0 f0 0f b1 13 39 f0 b9 00 00 00 00 --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Death by usb_dec_dev_use
On Sat, Feb 22, 2003 at 07:13:26PM -0500, Johannes Erdfelt wrote: The intent was for 2.4 to have this same contract. Having a reference count on the device structure is pointless if that weren't the case. Yes, and because of this, the driver model changes in 2.5 were very easy, thanks for doing all of the tough work :) thanks, greg k-h --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Death by usb_dec_dev_use
On Sat, 2003-02-22 at 23:36, Duncan Sands wrote: The philosophy is often called programming by contract. And the contract is: usbcore agrees to work with your driver and do everything right, if your driver agrees to do a few specific things. One of those specific things is never using the device after you return from disconnect(), either directly (as you're doing here) or indirectly (urb still pending, say). If you break such programming contracts, bad things happen; that's part of why the contract exists. There are lots of components interacting, relying on contracts to be followed, and if they aren't ... it's impractical to guarantee that all the failure modes will always be benign. Pity this contract is undocumented. So add a patch to document it now. Otherwise in 6 months time the same problem will occur --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Death by usb_dec_dev_use
On Sunday 23 February 2003 01:05, Greg KH wrote: On Sun, Feb 23, 2003 at 01:03:31AM +0100, Duncan Sands wrote: Present for you, Greg! I moved usb_put_dev to the disconnect routine (of course, there is a usb_get_dev in probe). Now I get the following oops on 2.5 during system shutdown: Do you get the same oops if you just try to rmmod the uhci-hcd driver with the device plugged into it? I do. Duncan. --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Death by usb_dec_dev_use
On Sunday 23 February 2003 01:03, Duncan Sands wrote: Present for you, Greg! I moved usb_put_dev to the disconnect routine (of course, there is a usb_get_dev in probe). Now I get the following oops on 2.5 during system shutdown: Mea culpa - I was borking the system just before the call to usb_put_dev. There is no problem with 2.5 reference counting after all. Sorry about that, Duncan. --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Death by usb_dec_dev_use
Greg KH wrote: On Sun, Feb 23, 2003 at 12:36:02AM +0100, Duncan Sands wrote: The philosophy is often called programming by contract. And the contract is: usbcore agrees to work with your driver and do everything right, if your driver agrees to do a few specific things. One of those specific things is never using the device after you return from disconnect(), either directly (as you're doing here) or indirectly (urb still pending, say). Pity this contract is undocumented. And flat wrong. That's pretty extreme -- are you like totally pooh-poohing on all the programming by contract disciplines? If so: why? and how else would you specify interfaces? Or if not: then what particular aspect(s) of that contract seem wrong to you? A better way to say it may be: if that's not what disconnect() means, than what the heck _does_ it mean? There are a whole LOT of answers in current use. Clearly the idea is to prohibit some kinds of behavior. Which ones? And why _not_ all of them? As Alan said, that contract needs to finally get written down. There's too much mis-understanding, and even in 2.5 we've not yet gotten rid of all the HCD-specific behaviors (or bugs). I'm comfortable saying that for 2.4, if your driver still hasn't implemented that policy, it's got needless instability. That has improved in 2.5 kernels ... a lot more drivers live within that policy, AND I think there are some more forgiving code paths. - Dave --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Re: [PATCH] USB changes for 2.5.62
This is fundamentally the problem. Your vision of how stuff is supposed to work in the USB code is solely your vision and doesn't match what was intended when I wrote the code or what the majority of the code does. You can keep thinking whatever you want, but it ain't reality. ERm, person 1's code doesn't match person 2's code, yes? Just because you think you want to, or can, change how the API works, doesn't make it so. 2.4 USB doesn't work like you think it does. Not the code you wrote, true. But then, the code Roman wrote doesn't work like _you_ think it does either; you recently admitted that when you made the 2.3 refcounting changes, you skipped that part. And so there's still an issue lingering in 2.5.62 ... and finger pointing isn't going to improve anything. What's the big deal? There are issues we've both raised, and yelling over email doesn't help. You want to see the pure memory-management aspects done with refcounting. Great. I don't want to see the non-memory-management ones done that way. What's the problem? --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Re: [PATCH] USB changes for 2.5.62
Back to the patch I've attached. It's against 2.4.19-pre3 (I'll update to Greg's CVS tree) and hasn't had the level of testing necessary to call it good. I also want to recheck that I caught all of the necessary paths. Seems like the main change there is that after disconnect, EDs get cleaned up in the IRQ handler ... which means that when the IRQ handler doesn't fire, nothing gets cleaned up. That means some power management situations will (unnecessarily) leak a lot, as will clean-up after the host controller dies (maybe because of UE or cardbus eject). It won't leak EDs on power management. The cleanup will just be deferred until power comes back on. Big deal. Not on the D3cold resume path -- that can't be deferred. FWIW, we were hitting the BUG() call in sohci_free_dev(). With what device driver? As I've said, EVERY time I've looked at such problems, the device driver has been at fault. Sometimes quite blatantly, sometimes less so. EVERY TIME. usbnet. Curious -- the first such problem report I ever recall. I'd have expected to have heard more reports ... but that driver's usage growth has been pretty gradual, I know there have to be problems that would show up under more serious loads. It does already go to great lengths to ensure it's done with all of the references it takes to the device, but it's not the only source of references to the device. You mean, from the network layer? I thought I adopted the policy where the usbnet disconnect() routine also blocked until the network layer stopped using the device. Not necessarily an ideal policy on busier links, but there've been on complaints. Umm, I don't know why you still have this warped view of how the reference counting in 2.4 is supposed to work, but it's perfectly legal to have references past when disconnect() is called. Not from the hardware it isn't, and that's what deallocate() has always expected to be deallocating. - Dave --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Death by usb_dec_dev_use
On Sat, Feb 22, 2003, David Brownell [EMAIL PROTECTED] wrote: The intent was for 2.4 to have this same contract. Having a reference count on the device structure is pointless if that weren't the case. However, there may be bugs there in HCDs, the core, or drivers with respect to this because it was never clearly defined or enforced. I just want to point out that from a pragmatic perspective, those changes (bug fixes if you insist) won't break drivers that have already been adhering to the stricter contract I described. Yes and no. The OHCI fix I sent was necessary even with your stricter contract. I remember trying to figure out how I could write code that'd shut down cleanly on *all* HCDs back in 2.4.early, and the result was just what I described: after disconnect(), if driver never touches the device again, it wouldn't oops with usb-uhci, uhci, or usb-ohci. But if they touched it later, there were problems. As long as they have a reference, it's not a problem with uhci. usb-uhci looks good to me too. usb-ohci looks good after the patch I've made. The core needs another fix (the device id fix I've referred to in other emails). After that, if the drivers are stricter, it's not a problem. Otherwise, it's not in the drivers control and thusly not something they can do anything about. JE --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Re: [PATCH] USB changes for 2.5.62
On Sat, Feb 22, 2003, David Brownell [EMAIL PROTECTED] wrote: Back to the patch I've attached. It's against 2.4.19-pre3 (I'll update to Greg's CVS tree) and hasn't had the level of testing necessary to call it good. I also want to recheck that I caught all of the necessary paths. Seems like the main change there is that after disconnect, EDs get cleaned up in the IRQ handler ... which means that when the IRQ handler doesn't fire, nothing gets cleaned up. That means some power management situations will (unnecessarily) leak a lot, as will clean-up after the host controller dies (maybe because of UE or cardbus eject). It won't leak EDs on power management. The cleanup will just be deferred until power comes back on. Big deal. Not on the D3cold resume path -- that can't be deferred. I haven't looked at the PM paths yet, but if the D3cold path is via normal PCI exit paths, then it'll be fine with the module removal fix I'm testing. Either way, if the device is powered off, it's safe to cleanup the list earlier since the HC isn't gonna use it anymore. FWIW, we were hitting the BUG() call in sohci_free_dev(). With what device driver? As I've said, EVERY time I've looked at such problems, the device driver has been at fault. Sometimes quite blatantly, sometimes less so. EVERY TIME. usbnet. Curious -- the first such problem report I ever recall. I'd have expected to have heard more reports ... but that driver's usage growth has been pretty gradual, I know there have to be problems that would show up under more serious loads. It's a race condition. It was very difficult to reproduce. One out of every 20 times and I could only reproduce it on some slower machines. My development P4 Xeon systems apparentely narrowed the race to the point where it was next to impossible to duplicate. It does already go to great lengths to ensure it's done with all of the references it takes to the device, but it's not the only source of references to the device. You mean, from the network layer? I thought I adopted the policy where the usbnet disconnect() routine also blocked until the network layer stopped using the device. Not necessarily an ideal policy on busier links, but there've been on complaints. The code in usbnet looks fine (atleast I haven't seen anything wrong yet). The problem is with other drivers which take a reference count. Like say usbdevfs and sending a control message. Umm, I don't know why you still have this warped view of how the reference counting in 2.4 is supposed to work, but it's perfectly legal to have references past when disconnect() is called. Not from the hardware it isn't, and that's what deallocate() has always expected to be deallocating. Once again, it's not an issue. Look at the code. Think about it. Reference counting will defer the deallocate() callback until when everyone is done with the device. The only problem with 2.4 I've found so far is the fact that the device id gets freed in disconnect() when it should be usb_free_dev() when the last reference is gone. JE --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Death by usb_dec_dev_use
On Sat, Feb 22, 2003 at 06:59:54PM -0800, David Brownell wrote: Greg KH wrote: On Sun, Feb 23, 2003 at 12:36:02AM +0100, Duncan Sands wrote: The philosophy is often called programming by contract. And the contract is: usbcore agrees to work with your driver and do everything right, if your driver agrees to do a few specific things. One of those specific things is never using the device after you return from disconnect(), either directly (as you're doing here) or indirectly (urb still pending, say). Pity this contract is undocumented. And flat wrong. That's pretty extreme -- are you like totally pooh-poohing on all the programming by contract disciplines? If so: why? and how else would you specify interfaces? Or if not: then what particular aspect(s) of that contract seem wrong to you? No, I'm saying the contract you wrote above is wrong. The idea of programming by contract is fine, just write down the correct contract :) A better way to say it may be: if that's not what disconnect() means, than what the heck _does_ it mean? There are a whole LOT of answers in current use. Clearly the idea is to prohibit some kinds of behavior. Which ones? And why _not_ all of them? It means that the device is physically gone, that's it. What the driver wants to do with this information is up to it. But the core has to handle attempts to use this device after it is gone, like it now does. As Alan said, that contract needs to finally get written down. There's too much mis-understanding, and even in 2.5 we've not yet gotten rid of all the HCD-specific behaviors (or bugs). True, and lots of drivers need to be cleaned up based on this. I'm going to be looking through them for the next few weeks I bet... And yes, writing it down is also a very good idea. There is a How to write a usb driver document in the kernel, along with a lot of great comments that get built into the kernel api. Also, there is an old USB Programming guide that needs to be updated a lot. If someone wants to take all of that and combine it into one document within the kernel, it would be greatly appreciated. thanks, greg k-h --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Re: [linux-usb-devel] Re: [PATCH] USB changes for 2.5.58
Okay, I see Linus has now accepted this into his tree. It should propagate to the USB development trees soon. One question: What else is needed? We set the device offline, error/complete all pending commands, and the need to (somehow) make certain we're in a good state for calling scsi_remove_host(). How do we make that final guarantee? There was talk that scsi_set_device_offline() would take care of that for us by waking up the error handler... there seems to be code to do that There was talk of using the release() function from the SCSI template to actually release resources So, what's the plan? Matt On Mon, Feb 17, 2003 at 11:37:37AM -0800, Mike Anderson wrote: Matthew Dharm [EMAIL PROTECTED] wrote: Any updates on this? I saw some patches, but they don't seem to be in my tree (the usb tree, which is synced from Linus' tree). People are starting to reports OOPSes to me because of this being missing Matt The scsi_set_device_offline interface is part of the last patch (scsi error) I sent to linux-scsi. I updated my patch post some comments from the list, but I am working on issue with the patch before I resend. -andmike -- Michael Anderson [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html -- Matthew Dharm Home: [EMAIL PROTECTED] Maintainer, Linux USB Mass Storage Driver But where are the THEMES?! How do you expect me to use an OS without themes?! -- Stef User Friendly, 10/9/1998 pgp0.pgp Description: PGP signature
