On Sun, Dec 27, 2009 at 6:39 AM, Michael 'Mickey' Lauer
<mic...@vanille-media.de> wrote:
> Am Sonntag, den 27.12.2009, 15:21 +0100 schrieb Antonio Ospite:
>> Timothy Meade (tmzt) reported that there is some code from OpenEZX[0]
>> which has been used as a base for the TS 07.10/27.010 mux driver for
>> Motorola Milestone[1] (which is a GSM Droid, Droid is also addressed
>> with the codename Sholes, IIUC). You can find some evidence here:
>>
>> http://android.git.kernel.org/?p=kernel/omap.git;a=blob;f=drivers/misc/ts27010mux/ts27010_mux.c;h=e95b7c71f43257f835aa65afffe6dec074c442c1;hb=android-omap-2.6.29-eclair
>
> Good find. Unfortunately it still looks pretty baseband specific,
> I would have welcomed a driver that supports the standardized protocols
> instead.

This driver is almost a complete rewrite of the driver from Motorola.
That driver had significant code-rot, race conditions, and possible
buffer overflows. The driver was primarily written to the TS 27.010
spec.  There are a a few bits that are specific to the Motorola
baseband.  Also, flow control was left unimplemented.

>> The code they used as a base is an old version of "our" mux driver
>> (which, in turn, was based on the one used by Motorola in EZX), which
>> was still using an explicit mapping between DLCIs and TTYs, Ilya Petrov
>> has done a great work simplifying this in our current driver, see:
>>
>> http://git.openezx.org/openezx.git?a=blob;f=drivers/char/ts0710_mux.c;h=e7fbf6df6543d877b3da60c6bbd15cbf17716d29;hb=refs/heads/ezx/current
>
> This looks amazingly clean compared to the previous mess. Is this up and
> running for production code? Which device node is it exported via? I'd
> like to base fsogsmd's Freescale Neptune plugin around this new
> interface.

A quick look through the source gives me concerns about race
conditions.  There are no synchronization primitives in the code.
Lines like:

if (st->state == OUT_OF_PACKET) {
        st->state = INSIDE_PACKET;

are problematic without synchronization.

Also the code has quite a lot of global state which is generally discouraged.

>> It would be great if our code and the one from Motorola could be
>> kept in sync somehow, that would also ease mainline submission a lot
>> in the long run.
>
> You realize this is a pipe dream, right? ;) Oh well, hope dies last.

The time I'll have to work on our code is highly dependent on which
basebands are chosen for future projects we work on.  My personal
opinion is that this code doesn't really need to be in the kernel.  In
Android's case it could be integrated into the radio interface layer
in user space.  The kernel TTY layer is not well understood and nobody
wants to maintain it.  Just take a look at the first line of
Documentation/serial/tty.txt: "Your guide to the ancient and twisted
locking policies of the tty layer and
the warped logic behind them. Beware all ye who read on."

Cheers,
    Erik




-Erik

Reply via email to