Re: [linux-usb-devel] Kingsun KS-959 USB IrDA dongle - release?candidate for submission (try 1)
On Sat, 21 Jul 2007 02:04:34 +0300, Samuel Ortiz [EMAIL PROTECTED] wrote:
On Sat, Jul 21, 2007 at 12:04:27AM +0200, Oliver Neukum wrote:
+struct ks959_speedparams {
+ __le32 baudrate;/* baud rate, little endian */
+ unsigned int data_bits : 2; /* data bits - 5 (5..8) */
+ unsigned int : 1;
+ unsigned int stop_bits : 1;
+ unsigned int parity_enable : 1;
+ unsigned int parity_type : 1;
+ unsigned int : 1;
+ unsigned int reset : 1;
+ __u8 reserved[3];
+} __attribute__ ((packed));
The attribute is not needed.
The driver uses sizeof(struct ks959_speedparams) in a couple places in
the code. If you don't pack it, the size will vary with different
architectures since it's not 32 bits aligned, so I think this attribute
is correct.
Oliver is correct, everything is perfectly aligned and the attribute
is unnecessary.
However the question is moot, since the bit fields vary in order.
Bit fields must not be used, and shifts and masks must be used instead.
-- 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
Re: [linux-usb-devel] Kingsun KS-959 USB IrDA dongle - release candidate for submission (try 1)
On Sat, 21 Jul 2007 02:21:55 +0300, Samuel Ortiz [EMAIL PROTECTED] wrote:
+/* Callback transmission routine */
+static void ks959_speed_irq(struct urb *urb)
+{
+ /* unlink, shutdown, unplug, other nasties */
+ if (urb-status != 0) {
+ err(ks959_speed_irq: urb asynchronously failed - %d,
urb-status);
Here, shouldn't we call unlink_urb() if depending on the status value
(in the -EINPROGRESS at least) ?
If a CPU is executing a callback, the URB is unlinked by definition.
So, I see no opportunity to invoke unlink_urb _here_ as you said.
-- 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
Re: [linux-usb-devel] Kingsun KS-959 USB IrDA dongle - release candidate for submission (try 1)
Greg KH escribio': On Thu, Jul 19, 2007 at 11:36:55AM -0500, Alex Villac??s Lasso wrote: At last I managed to fix the problem with transfers with the dongle. It happens that this dongle does not support payloads larger than 256 bytes, so the driver has to manually fragment anything bigger. The version attached in the tarball is the first version I consider worthy to submit for inclusion in the kernel. Please review (and test, if you can) as much as you like, before sending this to Samuel Ortiz, who maintains the IrDA subsystem. There is both the version to compile out-of-kernel (Makefile and .c file) as well as the version to apply to the kernel itself (on top of clean 2.6.22). Can you create a real patch that is inline so that we can read and review the patch? Take a look at Documentation/SubmittingPatches for more details on how to do this. thanks, greg k-h No problem. I was just trying to protect the patch against character encoding corruption. That is what happened with the previous patch. The patch is supposed to be UTF-8 encoded - please tell me if I should use any other character encoding, or if you see encoding corruption at the start of the source file. This version also includes a fix for an endianness bug I spotted on the speed change payload. -- This patch adds support for the KingSun KS-959 USB IrDA dongle. This dongle does not follow the usb-irda specification, so it needs its own special driver. First, it uses control URBs for data transfer, instead of bulk or interrupt transfers; the only interrupt endpoint exposed seems to be a dummy to prevent the interface from being rejected. Second, it uses obfuscation and padding at the USB traffic level, for no apparent reason other than to make reverse engineering harder (full details on obfuscation in comments at beginning of source). Although it is advertised as a 4 Mbps FIR dongle, it apparently loses packets at speeds greater than 57600 bps. On plugin, this dongle reports vendor and device IDs: 0x07d0:0x4959 . The Windows driver that is used normally to control this dongle has a filename of KS-959.SYS . Signed-off-by: Alex Villaci's Lasso [EMAIL PROTECTED] diff -urN linux-2.6.22-orig/drivers/net/irda/Kconfig linux-2.6.22/drivers/net/irda/Kconfig --- linux-2.6.22-orig/drivers/net/irda/Kconfig 2007-07-20 10:20:31.0 -0500 +++ linux-2.6.22/drivers/net/irda/Kconfig 2007-07-20 10:20:54.0 -0500 @@ -155,6 +155,20 @@ To compile it as a module, choose M here: the module will be called kingsun-sir. +config KS959_DONGLE + tristate KingSun KS-959 IrDA-USB dongle + depends on IRDA USB EXPERIMENTAL + help + Say Y or M here if you want to build support for the KingSun KS-959 + IrDA-USB bridge device driver. + + This USB bridge does not conform to the IrDA-USB device class + specification, and therefore needs its own specific driver. This dongle + supports SIR speeds only (9600 through 57600 bps). + + To compile it as a module, choose M here: the module will be called + ks959-sir. + comment Old SIR device drivers config IRPORT_SIR diff -urN linux-2.6.22-orig/drivers/net/irda/ks959-sir.c linux-2.6.22/drivers/net/irda/ks959-sir.c --- linux-2.6.22-orig/drivers/net/irda/ks959-sir.c 1969-12-31 19:00:00.0 -0500 +++ linux-2.6.22/drivers/net/irda/ks959-sir.c 2007-07-20 10:21:23.0 -0500 @@ -0,0 +1,909 @@ +/* +* +* Filename: ks959-sir.c +* Version: 0.1.2 +* Description: Irda KingSun KS-959 USB Dongle +* Status: Experimental +* Author: Alex Villaci's Lasso [EMAIL PROTECTED] +* with help from Domen Puncer [EMAIL PROTECTED] +* +* Based on stir4200, mcs7780, kingsun-sir drivers. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. +* +*/ + +/* + * Following is my most current (2007-07-17) understanding of how the Kingsun KS-959 + * dongle is supposed to work. This information was deduced by reverse-engineering + * and examining the USB traffic captured with USBSnoopy from the WinXP driver. + * Feel free to update here as more of the dongle is known. + * + * My most sincere thanks must go to Domen Puncer [EMAIL PROTECTED] for + * invaluable help in cracking the obfuscation and
Re: [linux-usb-devel] Kingsun KS-959 USB IrDA dongle - release candidate for submission (try 1)
On Thu, Jul 19, 2007 at 11:36:55AM -0500, Alex Villac??s Lasso wrote: At last I managed to fix the problem with transfers with the dongle. It happens that this dongle does not support payloads larger than 256 bytes, so the driver has to manually fragment anything bigger. The version attached in the tarball is the first version I consider worthy to submit for inclusion in the kernel. Please review (and test, if you can) as much as you like, before sending this to Samuel Ortiz, who maintains the IrDA subsystem. There is both the version to compile out-of-kernel (Makefile and .c file) as well as the version to apply to the kernel itself (on top of clean 2.6.22). Can you create a real patch that is inline so that we can read and review the patch? Take a look at Documentation/SubmittingPatches for more details on how to do this. thanks, greg k-h - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ 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] Kingsun KS-959 USB IrDA dongle - release candidate for submission (try 1)
On Thu, Jul 19, 2007 at 08:21:14PM +0200, Domen Puncer wrote: On 19/07/07 11:36 -0500, Alex Villac??s Lasso wrote: At last I managed to fix the problem with transfers with the dongle. It happens that this dongle does not support payloads larger than 256 bytes, so the driver has to manually fragment anything bigger. The version attached in the tarball is the first version I consider worthy to submit for inclusion in the kernel. Please review (and test, if you can) as much as you like, before sending this to Samuel Ortiz, who maintains the IrDA subsystem. There is both the version to compile out-of-kernel (Makefile and .c file) as well as the version to apply to the kernel itself (on top of clean 2.6.22). Unfortunately I don't have the dongle here, so I can't test. It would be great to get it into mainline, while the window is still open though. stand-alone drivers are pretty easy to add at almost any time in the merge window, don't worry :) thanks, greg k-h - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ 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] Kingsun KS-959 USB IrDA dongle - release candidate for submission (try 1)
Hi Alex, Sorry I went silent earlier. (I had a very busy period and for some time was away from home where I left the dongle, now just got back and re-set up the environment). On 20/07/07, Alex Villacís Lasso [EMAIL PROTECTED] wrote: [...] This patch adds support for the KingSun KS-959 USB IrDA dongle. This dongle does not follow the usb-irda specification, so it needs its own special driver. First, it uses control URBs for data transfer, instead of bulk or interrupt transfers; the only interrupt endpoint exposed seems to be a dummy to prevent the interface from being rejected. Second, it uses obfuscation and padding at the USB traffic level, for no apparent reason other than to make reverse engineering harder (full details on obfuscation in comments at beginning of source). Although it is advertised as a 4 Mbps FIR dongle, it apparently loses packets at speeds greater than 57600 bps. Congratuations on figuring out the protocol! I did a quick test, after manually unwrapping the patch, and it seems to work perfectly, although my test was not very exhaustive. Great job. I remember I was also surprised at the fact that the interrupt endpoint was a dummy and that the endpoint zero was abused in this way when I looked at the qemu logs a long time ago. Regards - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ 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] Kingsun KS-959 USB IrDA dongle - release candidate for submission (try 1)
Greg KH escribió: On Thu, Jul 19, 2007 at 11:36:55AM -0500, Alex Villac??s Lasso wrote: At last I managed to fix the problem with transfers with the dongle. It happens that this dongle does not support payloads larger than 256 bytes, so the driver has to manually fragment anything bigger. The version attached in the tarball is the first version I consider worthy to submit for inclusion in the kernel. Please review (and test, if you can) as much as you like, before sending this to Samuel Ortiz, who maintains the IrDA subsystem. There is both the version to compile out-of-kernel (Makefile and .c file) as well as the version to apply to the kernel itself (on top of clean 2.6.22). Can you create a real patch that is inline so that we can read and review the patch? Take a look at Documentation/SubmittingPatches for more details on how to do this. thanks, greg k-h Trying again, since previous attempt is wrapped. I was just trying to protect the patch against character encoding corruption. That is what happened with the previous patch. The patch is supposed to be UTF-8 encoded - please tell me if I should use any other character encoding, or if you see encoding corruption at the start of the source file. This version also includes a fix for an endianness bug I spotted on the speed change payload. -- This patch adds support for the KingSun KS-959 USB IrDA dongle. This dongle does not follow the usb-irda specification, so it needs its own special driver. First, it uses control URBs for data transfer, instead of bulk or interrupt transfers; the only interrupt endpoint exposed seems to be a dummy to prevent the interface from being rejected. Second, it uses obfuscation and padding at the USB traffic level, for no apparent reason other than to make reverse engineering harder (full details on obfuscation in comments at beginning of source). Although it is advertised as a 4 Mbps FIR dongle, it apparently loses packets at speeds greater than 57600 bps. On plugin, this dongle reports vendor and device IDs: 0x07d0:0x4959 . The Windows driver that is used normally to control this dongle has a filename of KS-959.SYS . Signed-off-by: Alex Villacís Lasso [EMAIL PROTECTED] diff -urN linux-2.6.22-orig/drivers/net/irda/Kconfig linux-2.6.22/drivers/net/irda/Kconfig --- linux-2.6.22-orig/drivers/net/irda/Kconfig 2007-07-20 10:20:31.0 -0500 +++ linux-2.6.22/drivers/net/irda/Kconfig 2007-07-20 10:20:54.0 -0500 @@ -155,6 +155,20 @@ To compile it as a module, choose M here: the module will be called kingsun-sir. +config KS959_DONGLE + tristate KingSun KS-959 IrDA-USB dongle + depends on IRDA USB EXPERIMENTAL + help + Say Y or M here if you want to build support for the KingSun KS-959 + IrDA-USB bridge device driver. + + This USB bridge does not conform to the IrDA-USB device class + specification, and therefore needs its own specific driver. This dongle + supports SIR speeds only (9600 through 57600 bps). + + To compile it as a module, choose M here: the module will be called + ks959-sir. + comment Old SIR device drivers config IRPORT_SIR diff -urN linux-2.6.22-orig/drivers/net/irda/ks959-sir.c linux-2.6.22/drivers/net/irda/ks959-sir.c --- linux-2.6.22-orig/drivers/net/irda/ks959-sir.c 1969-12-31 19:00:00.0 -0500 +++ linux-2.6.22/drivers/net/irda/ks959-sir.c 2007-07-20 10:21:23.0 -0500 @@ -0,0 +1,909 @@ +/* +* +* Filename: ks959-sir.c +* Version: 0.1.2 +* Description: Irda KingSun KS-959 USB Dongle +* Status:Experimental +* Author:Alex Villacís Lasso [EMAIL PROTECTED] +* with help from Domen Puncer [EMAIL PROTECTED] +* +* Based on stir4200, mcs7780, kingsun-sir drivers. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. +* +*/ + +/* + * Following is my most current (2007-07-17) understanding of how the Kingsun KS-959 + * dongle is supposed to work. This information was deduced by reverse-engineering + * and examining the USB
Re: [linux-usb-devel] Kingsun KS-959 USB IrDA dongle - release candidate for submission (try 1)
On Fri, Jul 20, 2007 at 05:02:29PM -0700, Pete Zaitcev wrote:
On Sat, 21 Jul 2007 02:21:55 +0300, Samuel Ortiz [EMAIL PROTECTED] wrote:
+/* Callback transmission routine */
+static void ks959_speed_irq(struct urb *urb)
+{
+ /* unlink, shutdown, unplug, other nasties */
+ if (urb-status != 0) {
+ err(ks959_speed_irq: urb asynchronously failed - %d,
urb-status);
Here, shouldn't we call unlink_urb() if depending on the status value
(in the -EINPROGRESS at least) ?
If a CPU is executing a callback, the URB is unlinked by definition.
Ok, that's what I wasn't sure about, if the callback was supposed to do
the unlink or not.
Cheers,
Samuel.
So, I see no opportunity to invoke unlink_urb _here_ as you said.
-- Pete
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
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] Kingsun KS-959 USB IrDA dongle - release candidate for submission (try 1)
At last I managed to fix the problem with transfers with the dongle. It happens that this dongle does not support payloads larger than 256 bytes, so the driver has to manually fragment anything bigger. The version attached in the tarball is the first version I consider worthy to submit for inclusion in the kernel. Please review (and test, if you can) as much as you like, before sending this to Samuel Ortiz, who maintains the IrDA subsystem. There is both the version to compile out-of-kernel (Makefile and .c file) as well as the version to apply to the kernel itself (on top of clean 2.6.22). Alex Villacís Lasso -- perl -e '$x=2.4;print sprintf(%.0f + %.0f = %.0f\n,$x,$x,$x+$x);' ks-959.tar.bz2 Description: BZip2 compressed data - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/___ 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] Kingsun KS-959 USB IrDA dongle - release candidate for submission (try 1)
On 19/07/07 11:36 -0500, Alex Villacís Lasso wrote:
At last I managed to fix the problem with transfers with the dongle. It
happens that this dongle does not support payloads larger than 256
bytes, so the driver has to manually fragment anything bigger.
The version attached in the tarball is the first version I consider
worthy to submit for inclusion in the kernel. Please review (and test,
if you can) as much as you like, before sending this to Samuel Ortiz,
who maintains the IrDA subsystem. There is both the version to compile
out-of-kernel (Makefile and .c file) as well as the version to apply to
the kernel itself (on top of clean 2.6.22).
Unfortunately I don't have the dongle here, so I can't test.
It would be great to get it into mainline, while the window is still
open though.
Some minor comments/nits:
334 /* unlink, shutdown, unplug, other nasties */
335 if (urb-status != 0) {
336 err(ks959_send_irq: urb asynchronously failed - %d,
urb-status);
337 }
no return? it is in _rcv_irq
460 kingsun-receiving = (kingsun-rx_unwrap_buff.state !=
OUTSIDE_FRAME) ? 1 : 0;
just kingsun-receiving = kingsun-rx_unwrap_buff.state != OUTSIDE_FRAME;
would do the trick, but I don't really care.
721 kingsun-rx_buf = (__u8 *)kmalloc(KINGSUN_RCV_FIFO_SIZE,
GFP_KERNEL);
no need to cast (void *).
Alex Villacís Lasso
--
perl -e '$x=2.4;print sprintf(%.0f + %.0f = %.0f\n,$x,$x,$x+$x);'
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
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] Kingsun KS-959 USB IrDA dongle - release candidate for submission (try 1)
Hello guys, I do have the device here,
kernel 2.6.17.13
I'll start testing as soon as I get home from work ;)
br
On Thu 19 Jul 2007 13:21, Domen Puncer wrote:
On 19/07/07 11:36 -0500, Alex Villacís Lasso wrote:
At last I managed to fix the problem with transfers with the dongle. It
happens that this dongle does not support payloads larger than 256
bytes, so the driver has to manually fragment anything bigger.
The version attached in the tarball is the first version I consider
worthy to submit for inclusion in the kernel. Please review (and test,
if you can) as much as you like, before sending this to Samuel Ortiz,
who maintains the IrDA subsystem. There is both the version to compile
out-of-kernel (Makefile and .c file) as well as the version to apply to
the kernel itself (on top of clean 2.6.22).
Unfortunately I don't have the dongle here, so I can't test.
It would be great to get it into mainline, while the window is still
open though.
Some minor comments/nits:
334 /* unlink, shutdown, unplug, other nasties */
335 if (urb-status != 0) {
336 err(ks959_send_irq: urb asynchronously failed - %d,
urb-status); 337 }
no return? it is in _rcv_irq
460 kingsun-receiving = (kingsun-rx_unwrap_buff.state !=
OUTSIDE_FRAME) ? 1 : 0; just kingsun-receiving =
kingsun-rx_unwrap_buff.state != OUTSIDE_FRAME; would do the trick, but I
don't really care.
721 kingsun-rx_buf = (__u8 *)kmalloc(KINGSUN_RCV_FIFO_SIZE,
GFP_KERNEL); no need to cast (void *).
Alex Villacís Lasso
--
perl -e '$x=2.4;print sprintf(%.0f + %.0f = %.0f\n,$x,$x,$x+$x);'
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
