login
Search
Hello Willy, Greg and list, I have ported adutux driver for ADU series device list from 2.6 to 2.4. More on devices: http://www.ontrak.net/products.htm#Table%205 Once I needed to make ADU200 work under 2.4 enterprise kernel and wasn't able to do this. My organization decided to use another device for our private purposes. I was always interested in kernel hacking and thought it's a good point to start it from. Just to add I'm not related to Ontrak in anyway. Also I'm not a Google person. Did it just for fun. A few technical notes: - I was trying to leave as much as possible adutux driver code from 2.6 as it's in the 2.6 mainline for a long time and seems like it works OK there. - Used one shot interrupt urbs that's why all intervals are 0. - Reused 67 minor number from 2.6 kernel for this device. - All 2.4 related changes are taken/reused from usb-skeleton. Patch is based on the latest 2.4.35.3. Performed a lot of tests but only under UHCI controller and ADU200 and all seems to be OK. I would like someone to perform code review as it's my first attempt to the kernel programming. Any comments, propositions are welcome. Signed-off-by: Vitaliy Ivanov <vitalivanov@gmail.com> Tested-by: Vitaliy Ivanov <vitalivanov@gmail.com> -- diff -uprN -X dontdiff KERNELS/linux-2.4.35.3/Documentation/Configure.help linux-2.4.35.3.build/Documentation/Configure.help -- KERNELS/linux-2.4.35.3/Documentation/Configure.help 2007-09-24 01:02:58.000000000 +0300 +++ linux-2.4.35.3.build/Documentation/Configure.help 2007-10-14 19:19:06.000000000 +0300 @@ -16123,6 +16123,17 @@ CONFIG_USB_RIO500 The module will be called rio500.o. If you want to compile it as a module, say M here and read <file:Documenatation/modules.txt>. +Ontrak Control Systems ADU devices support +CONFIG_USB_ADUTUX + Say Y here if you want to connect ADU 100/200 series devices to your + computer's USB port. + Details at: <http://www.ontrak.net/products.htm#Table%205> + + This code is also available as ...
Hello Vitaliy, At first glance, your backport looks clean. I have one comment however, about the author and version. Since it's a backport from an existing driver and not one you wrote yourself (eventhough you did the backporting work), the MODULE_AUTHOR should not be changed (but I think you can add yourself to it after a comma). The version should reflect the version you derived it from, at least for bug tracking purposes. Also, while I understand you would be very glad to get your work merged (we all once had our first piece of code), I'd like to mention that you seem to be the only user of this hardware under 2.4 (since it is currently not supported). I'm not sure it's very reasonable to merge a driver in 2.4 right now for just one user. Even more, I understand that you finally moved to other hardware, so my feeling is that you did this work as an exercice (which was cleanly performed, BTW), but that it will not get any real use in 2.4. Since 2.4 is moving very slowly, there should be no problem applying this patch to any version if you really need to use it. Maybe it would even work with your 2.4 enterprise kernel. Note that I'm not radically opposed to merge support for new drivers. If you provide us with really good arguments for a merge, maybe I'll change my opinion, but I doubt about it, since the only users of this device must currently be running 2.6. Thanks, Willy ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Willy, Yes, I would like it to be merged... But not just to see my name in the kernel sources. I'm the only one user of this hardware under 2.4 because it's some kind of trick to make it work under 2.4 w/o its support in the kernel. We moved to the other hardware because of our reasons and some customers can move to the other OS where this hardware will be supported. I'm sure that we're not the first who tried to make it work in 2.4. Original driver was created for 2.5 and > because interrupt out urbs I'm not going to force you to do this, also I can't do this:). But after going through the recent news where Greg proposed to create drivers for companies for free it looks really reasonable. I understand that we are talking about 2.4 but if you will simply run diff for adutux of 2.4 and 2.6 you will see that changes are really trivial. Also IMHO the more drivers are in the tree the more users will use it. Once it will be merged in the mainline then it will be backported to enterprise kernels and would gain wide usage. Best, Vitaliy ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
But something's strange. If people were using 2.4, this hardware has never worked for them, right ? Why suddenly would they decide that they have to switch OS or hardware ? Or maybe this hardware replaces That's what I've seen. I can propose you something (unless someone else raises his hand saying "no") : you update your patch with a short description of what the hardware module is supposed to be used for, and you accept to step up as the maintainer for this backport, which will imply that you put your name and mail in the MAINTAINERS file. That way, if you're the only user, nobody will be annoyed, and if there are other users and some of them have problems, I don't waste my time on something I don't know at all. If you agree with this deal (which I think is fair), then I'm willing to merge your patch into Not necessarily. 2.4 is currently used by people who already are in 2.4 and cannot/do not want to switch, and by people who are looking for close to zero maintenance. Drivers are often a reason to switch away from 2.4, I don't believe that. Enterprise kernels will not evolve much and will probably not enable it as long as they have not tested it. Regards, Willy ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Willy, It's completely fair. I spent some time on lkml and I liked it. I've a device and I can check/correct any issue we'll have with it(hope there won't be any;)). So it's OK for me to be a maintainer for this driver. Here is final patch with all issues corrected. Again, comments are welcomed. Vitaliy -- adutux is a simple Linux device driver for ADU boards from Ontrak Control Systems. The adutux driver exposes standard open/close/read/write API's to the user application. Signed-off-by: Vitaliy Ivanov <vitalivanov@gmail.com> Tested-by: Vitaliy Ivanov <vitalivanov@gmail.com> -- diff -uprN -X dontdiff KERNELS/linux-2.4.35.3/Documentation/Configure.help linux-2.4.35.3.build/Documentation/Configure.help --- KERNELS/linux-2.4.35.3/Documentation/Configure.help 2007-09-24 01:02:58.000000000 +0300 +++ linux-2.4.35.3.build/Documentation/Configure.help 2007-10-14 19:19:06.000000000 +0300 @@ -16123,6 +16123,17 @@ CONFIG_USB_RIO500 The module will be called rio500.o. If you want to compile it as a module, say M here and read <file:Documenatation/modules.txt>. +Ontrak Control Systems ADU devices support +CONFIG_USB_ADUTUX + Say Y here if you want to connect ADU 100/200 series devices to your + computer's USB port. + Details at: <http://www.ontrak.net/products.htm#Table%205> + + This code is also available as a module ( = code which can be + inserted in and removed from the running kernel whenever you want). + The module will be called adutux.o. If you want to compile it as + a module, say M here and read <file:Documenatation/modules.txt>. + Auerswald device support CONFIG_USB_AUERSWALD Say Y here if you want to connect an Auerswald USB ISDN Device diff -uprN -X dontdiff KERNELS/linux-2.4.35.3/drivers/usb/adutux.c linux-2.4.35.3.build/drivers/usb/adutux.c --- KERNELS/linux-2.4.35.3/drivers/usb/adutux.c 1970-01-01 03:00:00.000000000 +0300 +++ linux-2.4.35.3.build/drivers/usb/adutux.c 2007-10-16 16:27:41.000000000 +0300 @@ -0,0 +1,1043 ...
OK, I have no objection, but please apply the fixes the le16 problem as suggested by Pete and Greg first. Also, you will probably receive more comments, and/or criticisms from further reviews. This is normal and If this is going to be the commit message, please reduce lines to less than 70 chars, and also enumerate the supportd devices and the one you performed the tests with. It helps a lot when users encounter problems. Also, if you did not (I forgot to check), please ensure that the adutux maintainer in 2.6 is CC'd. Last but not least, please remove the "KERNELS/" path component from your next diff. Thanks, Willy ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
I do not object to the driver (with byte order fixed), although it seems written oddly... E.g. lots of trivial comments, the open count locked by wrong lock. But I do not want to hold the 2.4 version hostage to errors of the mainline. We should've caught them when we merged adutux into Greg's tree. I have to admit I'm skipping reviewing a lot of 2.6. -- Pete ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
I really appreciate your position, Pete. It's fair and motivating for newcomers. I hope Vitalyi will be able to forward-port some fixes and cleanups to 2.6 in the future. Thanks, Willy ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
No Pete. Seems it's my changes. Actually such approach is proposed by usb-skeleton in 2.4. Will rework code to lock it correctly and clean code from trivial comments. Will back to this soon. Vitaliy ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Done. Removed all le16_to_cpu's. Comments are OK. Just a little bit busy on my regular work. So can give It's a driver description you asked for previously. Commit message should look like this probably: "Port of adutux driver from 2.6 kernel for Ontrak ADU boards." Devices list: As I said previously supported devices are: http://www.ontrak.net/products.htm#Table%205 Just to list them here: ADU100, ADU120, ADU130, ADU200, ADU208, ADU218. Testing is performed on ADU200. But command structure is identical for this devices and it is only There is not maintainer for this device in 2.6. Following Pete notes I will rework code and give it for review once again. As I said it's usb-skeleton approach that I reused during the port. Anyway, will get back to this soon. Vitaliy ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Willy & Pete, Reworked code a little. Issue with locks... Now minor_table_mutex is used to protect the minor_table structure and nothing else. It was its first purpose. dev->sem is used to protect device manipulations. It's a normal practice in 2.4. So I leave it this way. I think it's OK, Pete? So the final patch, hope it's final, no?:) P.S. Willy all the details you asked before can be found in my previous mail(description, device list, etc). Signed-off-by: Vitaliy Ivanov <vitalivanov@gmail.com> Tested-by: Vitaliy Ivanov <vitalivanov@gmail.com> -- diff -uprN -X dontdiff linux-2.4.35.3/Documentation/Configure.help linux-2.4.35.3.build/Documentation/Configure.help --- linux-2.4.35.3/Documentation/Configure.help 2007-10-16 20:52:09.000000000 +0300 +++ linux-2.4.35.3.build/Documentation/Configure.help 2007-10-14 19:19:06.000000000 +0300 @@ -16123,6 +16123,17 @@ CONFIG_USB_RIO500 The module will be called rio500.o. If you want to compile it as a module, say M here and read <file:Documenatation/modules.txt>. +Ontrak Control Systems ADU devices support +CONFIG_USB_ADUTUX + Say Y here if you want to connect ADU 100/200 series devices to your + computer's USB port. + Details at: <http://www.ontrak.net/products.htm#Table%205> + + This code is also available as a module ( = code which can be + inserted in and removed from the running kernel whenever you want). + The module will be called adutux.o. If you want to compile it as + a module, say M here and read <file:Documenatation/modules.txt>. + Auerswald device support CONFIG_USB_AUERSWALD Say Y here if you want to connect an Auerswald USB ISDN Device diff -uprN -X dontdiff linux-2.4.35.3/drivers/usb/adutux.c linux-2.4.35.3.build/drivers/usb/adutux.c --- linux-2.4.35.3/drivers/usb/adutux.c 1970-01-01 03:00:00.000000000 +0300 +++ linux-2.4.35.3.build/drivers/usb/adutux.c 2007-10-17 20:57:03.000000000 +0300 @@ -0,0 +1,1040 @@ +/* + * This is a 2.4.* kernel version of adutux driver that + * ...
Hi all, Didn't here anything on this? What is our final decision here? Thanks, Vitaliy ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
It's gotten worse, not better. Apparently, you aren't getting the concept of protecting the open count with a static lock and my explanations are just not vivid enough or something. So I decided to fix it myself. Maybe then the patch in C will explain it better than English. But I didn't have time to do it. Also, there's an outright bug in the latest version. Your purge of the wrong lock was incomplete and so there was an unbalanced up(). But this is moot. So, the version before the latest is borderline acceptable. If Willy wants to take it, it's fine. I'll fix it up later together with 2.6. -- Pete ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Probably I'm not trying to do what you want. I analyzed locks for other usb drivers in 2.4 tree and used same ideas. Static lock minor_table_mutex is used for minor table structure. And dev->sem for dev manipulations and that's why for open_count. If you will simply browse /drivers/usb dir for 2.4 you will see that such approach is widely used there. What's not right? Let's do everything correctly for 2.4. V. ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
On Fri, 19 Oct 2007 20:40:35 +0300, Vitaliy Ivanov <vitalivanov@gmail.com> wrote: Hi, Vitaly, I added you on cc: for the 2.6 cleanup. Please double-check what I'm doing there and use it for your 2.4 version. I hope my intentions The fundamental reason why you cannot lock a free-able structure with an in-structure lock is this. Imagine thread A locks in order to process a disconnect. Thread B wants to open and waits for the lock. Notice that the struct is not open, so thread A frees it. At this point, thread B is using a freed memory. The solution is to lock the instance struct dev with dev->mtx, except for the open count, which is locked by a static lock (I'm ignoring interrupts here, which cannot use mutexes). I'm sorry to say, you're quite right: a number of drivers in 2.4 got it wrong, and some (like adutux) carried it through 2.6.23. -- Pete ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Vitaly, I'm planning on issuing a new 2.4.36 prerelease soon. Have you made any progress on your code after Pete's recommendations ? Thanks, Willy ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Willy, Yep. We just finalized 2.6 changes and now I'm working on 2.4 version. Will get back to you all soon. When are you plan to make prerelease? As always I'm too busy on my regular work and will try to put more time to this. Thanks, Vitaliy ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
I can understand, of course. There's no reason to hurry. If my prerelease is ready before your patch, I'll release it anyway. It's just that the sooner it gets merged, the better for everyone. Thanks, Willy ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
It looks like you misunderstood why a static lock protects open counts. This is done so you do not need to worry about in-structure lock which The dev->sem is entirely unnecessary here. Every time you use open_count, it's protected by minor_table_mutex. The name is a litte unfortunate, feel free to rename it. This is probably a problem in 2.6 as well. I don't know why people keep writing these things. Someone at Ontrak needs to look into it. -- Pete ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
At least in case of RHEL, such backports never were automatic. In any case, RHEL 2.1 and 3 do not receive new drivers anymore. We only do bugfixes if something comes up. Realistically speaking, 2.4 kernels are just too old for anyone to use. So, I think it would be best for Did you verify if this works? We use pre-swapped descriptors in 2.4. The above very clearly is a use-after-free, in case the device was open across a disconnect. Solution: Use minor_table_mutex to lock dev->open_count instead of dev->sem. There's no rule that the lock has to live inside the same structure with members it locks. -- Pete ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Pete, Thanks for your notes. Will check and correct it asap. Vitaliy ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Pete, Yeah. You are right. Found similar issue in adu_release also. It's a problem with 2.6 kernel driver. So, I've got a material to create some fixes in 2.6 driver too. I've reworked the code to avoid this issue. Sending final patch as a reply to Willy's mail. Please check it. Vitaliy ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
That's probably because you tested this on a little-endian machine :) Pete is right, this code is incorrect for 2.4, drop the le16_to_cpu function, the wMaxPacketSize variable is in native-endian form in 2.4 and early 2.6 versions. thanks, greg k-h ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
