2009-09-17 12:58:17

by Bart Hartgers

[permalink] [raw]
Subject: New ark3116 driver - how to get included into kernel?

(Sorry for sending an HTML-ized version of this mail before)

Hi All,

I managed to write an improved ark3116 driver after I figured out that
it is just an 16450 UART with some USB glue logic.

The attached files can be compiled outside the kernel tree, and work
for 2.6.31. However, I would like this driver to (eventually) end up
in the kernel tree. In order to get there, who should I sent patches
against what? I've contributed code to the kernel before, but not in
the last 5 or so years, so I am a bit out of touch.

Compared to the old ark3116 driver this one offers the following improvements:
 - cts/rts handshake support
 - break signalling
 - line error detection

Since it is a big step from the previous driver, it made little sense
to modify that one, so I created new driver, named ark316new. Also
this means that both could coexist if the new one doesn't work for
someone.

This driver also includes support for an ark3116-based IrDA dongle,
tested by Ondrej Zary.

Thanks in advance for any input/help.

Groeten,
Bart

>>
>> I also tried to include the IrDA fixes for the old ark3116.c driver
>> that were posted on this list recently, but I could not test if
>> these work, since I don't have the proper hardware.

2009/8/31 Bart Hartgers <[email protected]>
>
> 2009/8/29 Ondrej Zary <[email protected]>:
> > On Sunday 23 August 2009 14:46:16 Bart Hartgers wrote:
> >> Hi All,
> >>
> >> I managed to write an improved ark3116 driver after I figured out that
> >> it is just an 16450 UART with some USB glue logic.
> >>
> >> Compared to the old ark3116 driver it offers the following improvements:
> >> - cts/rts handshake support
> >> - break signalling
> >> - line error detection
> >>
> >> Since it is a big step from the previous driver, it made little sense
> >> to modify that one, so I created new driver, named ark316new. Also
> >> this means that both could coexist if the new one doesn't work for
> >> someone.
> >>
> >> I have no datasheet, so the driver is entirely based on reverse
> >> engineering, and some testing by others would be a good idea.
> >>
> >> I also tried to include the IrDA fixes for the old ark3116.c driver
> >> that were posted on this list recently, but I could not test if
> >> these work, since I don't have the proper hardware.
> >>
> >> Because of this, I would appreciate reports of both success and
> >> failure in using this driver.
> >
> > Tested the driver today with that Gembird UIR-22.
> >
> > It did not compile neither in 2.6.30.1 nor in 2.6.31-rc8 with the following
> > error:
> >
> > ark3116new.c:790: error: unknown field shutdown specified in initializer
> > ark3116new/ark3116new.c:790: warning: initialization from incompatible pointer
> > type
> >
> > Commenting that line produced a module that worked:
> > usb 1-1: new full speed USB device using uhci_hcd and address 2
> > usb 1-1: configuration #0 chosen from 1 choice
> > usb 1-1: config 0 descriptor??
> > usbcore: registered new interface driver usbserial
> > usbserial: USB Serial Driver core
> > USB Serial support registered for ARK3116 RS232/IrDA
> > ark3116new 1-1:0.0: ARK3116 RS232/IrDA converter detected
> > usb 1-1: ark3116new using IrDA mode
> > usb 1-1: ARK3116 RS232/IrDA converter now attached to ttyUSB0
> > usbcore: registered new interface driver ark3116new
> > ark3116new:v0.1:USB ARK3116 serial/IrDA driver
> > usb 1-1: ark3116new don't know how to do software flow control
> > NET: Registered protocol family 23
> >
> > Ran "irattach /dev/ttyUSB0 -s" and then "obexftp -i -l" to list files on Nokia
> > 6230i. It worked fine and file transfer too. So the IrDA mode works.
> >
> > When I unplugged the device, irattach remained running and some error message
> > was written to console every few seconds (sorry, I haven't recorded it).
> > Killing irattach then resulted in oops. But this might be expected as the
> > shutdown function was commented out.
> >
> Hi Ondrej,
>
> Thanks for testing this!
>
> I later discovered that there is a change with respect to
> shutdown-functionality between 2.6.28 (on which I made the driver) and
> 2.6.31. I have to update the driver to 2.6.current anyway.
>
> > I think that this driver should replace current ark3116 driver. Having two
> > drivers for the same hardware is a bad thing.
>
> I agree. However, given the experimental nature of the driver, I would
> prefer some more testing. Then again, the original driver is lacks
> some features, and making ark3116new default would increase its
> exposure ;-).
>
> Groeten,
> Bart
>
> >
> > --
> > Ondrej Zary
> >
>
>
>
> --
> Bart Hartgers - New e-mail: [email protected]



--
Bart Hartgers - New e-mail: [email protected]



--
Bart Hartgers - New e-mail: [email protected]


Attachments:
Makefile.txt (493.00 B)
ark3116new.c (24.41 kB)
Download all attachments

2009-09-18 06:16:06

by Greg KH

[permalink] [raw]
Subject: Re: New ark3116 driver - how to get included into kernel?

On Thu, Sep 17, 2009 at 02:52:29PM +0200, Bart Hartgers wrote:
> (Sorry for sending an HTML-ized version of this mail before)
>
> Hi All,
>
> I managed to write an improved ark3116 driver after I figured out that
> it is just an 16450 UART with some USB glue logic.
>
> The attached files can be compiled outside the kernel tree, and work
> for 2.6.31. However, I would like this driver to (eventually) end up
> in the kernel tree. In order to get there, who should I sent patches
> against what? I've contributed code to the kernel before, but not in
> the last 5 or so years, so I am a bit out of touch.

Take a look at the file, Documentation/SubmittingPatches, it should
describe what you need to do.

> Compared to the old ark3116 driver this one offers the following improvements:
> ?- cts/rts handshake support
> ?- break signalling
> ?- line error detection

Why can't you just send patches adding support for these features to the
existing driver? It shouldn't be that much different between the two
versions, right?

That's the preferred method, I'd like to not drop the existing one if at
all possible.

thanks,

greg k-h

2009-09-18 12:15:56

by Bart Hartgers

[permalink] [raw]
Subject: Re: New ark3116 driver - how to get included into kernel?

Hi Greg,

Thanks for your reply.

2009/9/18 Greg KH <[email protected]>:
> On Thu, Sep 17, 2009 at 02:52:29PM +0200, Bart Hartgers wrote:
>> (Sorry for sending an HTML-ized version of this mail before)
>>
>> Hi All,
>>
>> I managed to write an improved ark3116 driver after I figured out that
>> it is just an 16450 UART with some USB glue logic.
>>
>> The attached files can be compiled outside the kernel tree, and work
>> for 2.6.31. However, I would like this driver to (eventually) end up
>> in the kernel tree. In order to get there, who should I sent patches
>> against what? I've contributed code to the kernel before, but not in
>> the last 5 or so years, so I am a bit out of touch.
>
> Take a look at the file, Documentation/SubmittingPatches, it should
> describe what you need to do.
>
Thanks. But the question I had was more that I didn't know where to
put a new driver. In drivers/usb/serial, or perhaps in
drivers/staging. Anyway, if we are going to replace the existing
driver, it is obvious what the patch should be against.

>> Compared to the old ark3116 driver this one offers the following improvements:
>>  - cts/rts handshake support
>>  - break signalling
>>  - line error detection
>
> Why can't you just send patches adding support for these features to the
> existing driver?  It shouldn't be that much different between the two
> versions, right?

The difference is actually quite significant. The old driver is pretty
much a dumb parameterized replay of the windows usb-snoops. The new
driver actually "understands" the hardware. That's why I made a
completely new driver in the first place. A diff between the two is
ore or less the same as a complete replacing. I could try to minimize
the differences, but I would be surprised if more than 30% of the
lines will be shared, and most of those will be red tape, not actual
code. The patch will be hard to read anyhow.

>
> That's the preferred method, I'd like to not drop the existing one if at
> all possible.
>

Do you think it is worth the effort to minimize the diff, or should I
just replace ark3116.c by ark3116new.c?

Groeten,
Bart

> thanks,
>
> greg k-h
>



--
Bart Hartgers - New e-mail: [email protected]

2009-09-18 12:54:24

by Oliver Neukum

[permalink] [raw]
Subject: Re: New ark3116 driver - how to get included into kernel?

Am Freitag, 18. September 2009 14:15:57 schrieb Bart Hartgers:
> > That's the preferred method, I'd like to not drop the existing one if at
> > all possible.
>
> Do you think it is worth the effort to minimize the diff, or should I
> just replace ark3116.c by ark3116new.c?

The important part is not the size but the clarity of the diff.

Regards
Oliver

2009-09-18 16:21:37

by Greg KH

[permalink] [raw]
Subject: Re: New ark3116 driver - how to get included into kernel?

On Fri, Sep 18, 2009 at 02:15:57PM +0200, Bart Hartgers wrote:
> Hi Greg,
>
> Thanks for your reply.
>
> 2009/9/18 Greg KH <[email protected]>:
> > On Thu, Sep 17, 2009 at 02:52:29PM +0200, Bart Hartgers wrote:
> >> (Sorry for sending an HTML-ized version of this mail before)
> >>
> >> Hi All,
> >>
> >> I managed to write an improved ark3116 driver after I figured out that
> >> it is just an 16450 UART with some USB glue logic.
> >>
> >> The attached files can be compiled outside the kernel tree, and work
> >> for 2.6.31. However, I would like this driver to (eventually) end up
> >> in the kernel tree. In order to get there, who should I sent patches
> >> against what? I've contributed code to the kernel before, but not in
> >> the last 5 or so years, so I am a bit out of touch.
> >
> > Take a look at the file, Documentation/SubmittingPatches, it should
> > describe what you need to do.
> >
> Thanks. But the question I had was more that I didn't know where to
> put a new driver. In drivers/usb/serial, or perhaps in
> drivers/staging. Anyway, if we are going to replace the existing
> driver, it is obvious what the patch should be against.

Yes, please work on fixing up the existing driver, that's the proper way
to do it.

> >> Compared to the old ark3116 driver this one offers the following improvements:
> >> ?- cts/rts handshake support
> >> ?- break signalling
> >> ?- line error detection
> >
> > Why can't you just send patches adding support for these features to the
> > existing driver? ?It shouldn't be that much different between the two
> > versions, right?
>
> The difference is actually quite significant. The old driver is pretty
> much a dumb parameterized replay of the windows usb-snoops. The new
> driver actually "understands" the hardware. That's why I made a
> completely new driver in the first place. A diff between the two is
> ore or less the same as a complete replacing. I could try to minimize
> the differences, but I would be surprised if more than 30% of the
> lines will be shared, and most of those will be red tape, not actual
> code. The patch will be hard to read anyhow.

Then work on converting the driver over incrementally, one step at a
time, so that the patches are readable and understandable :)

> > That's the preferred method, I'd like to not drop the existing one if at
> > all possible.
> >
>
> Do you think it is worth the effort to minimize the diff, or should I
> just replace ark3116.c by ark3116new.c?

Well, I will not take such a patch, so I would work on the incremental
change version instead.

thanks,

greg k-h