On Mon, 4 Jan 2010 12:49:34 -0800
Erik Gilling <konk...@google.com> wrote:

> 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 driver is indeed way cleaner than the one we use in OpenEZX, and
with some quite limited changes it can be made to work with EZX
platform baseband processor too, see the attached proof-of-concept
patch; basically all it needed were sequence numbers and ACK packets
support, and adjusted dlci-tty mapping. It kind of works but it still
needs some more work.

> >> 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.
>

Erik, would you at least accept patches? I also found some small
defects.

If we collaborate on this driver, what should we consider as the
"upstream" repository where to get the latest version of it, on which we
can base our modifications? Is the aforementioned android git repo ok?

I'd also like to discuss the dlci-tty mapping: can we get completely rid
of it assuming an implicit 1:1 mapping? And, how are you opening
channel 0 with the current ts27010mux driver?

Thanks in advance,
   Antonio Ospite

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
Make ts27010mux driver work with EZX baseband processor

ts27010mux is a TS 07.10/27.010 mux kernel driver for Motorola Milestone
developed by Erik Gilling.

It  can be found 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

In order to check out the code:
  git clone git://android.git.kernel.org/kernel/omap.git
  git checkout remotes/origin/android-omap-2.6.29-eclair
  ls -l drivers/misc/ts27010mux

This patch adds sequence numbers, ACK packets, and adjusts the dli-tty
mapping to work with the baseband processor used in Motorola EZX
platform.

Antonio Ospite
http://ao2.it

diff -pruN __mux/ts27010mux//ts0710.h drivers/char/ts27010mux//ts0710.h
--- __mux/ts27010mux//ts0710.h	2010-02-14 12:17:25.000000000 +0100
+++ drivers/char/ts27010mux//ts0710.h	2010-02-20 19:32:42.000000000 +0100
@@ -53,7 +53,7 @@
 #define FLAG_SIZE 2
 
 #define TS0710_MAX_HDR_SIZE 5
-#define DEF_TS0710_MTU 512
+#define DEF_TS0710_MTU 256
 
 #define TS0710_BASIC_FLAG 0xF9
 
diff -pruN __mux/ts27010mux//ts27010_mux.c drivers/char/ts27010mux//ts27010_mux.c
--- __mux/ts27010mux//ts27010_mux.c	2010-02-17 18:33:44.000000000 +0100
+++ drivers/char/ts27010mux//ts27010_mux.c	2010-02-20 20:18:06.000000000 +0100
@@ -94,10 +94,12 @@
 #define CMDTAG 0x55
 #define DATATAG 0xAA
 
-static const u8 tty2dlci[NR_MUXS] =
-    { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
-static const u8 iscmdtty[NR_MUXS] =
-    { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 };
+#define ACK 0x4F
+
+static const u8 tty2dlci[NR_MUXS+1] =
+    { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
+static const u8 iscmdtty[NR_MUXS+1] =
+    { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 };
 
 struct dlci_tty {
 	const u8 cmdtty;
@@ -105,7 +107,7 @@ struct dlci_tty {
 };
 
 static const struct dlci_tty dlci2tty[] = {
-	{0, 0},	/* DLCI 0 */
+	//{0, 0},	/* DLCI 0 */
 	{0, 0},				/* DLCI 1 */
 	{1, 1},				/* DLCI 2 */
 	{2, 2},				/* DLCI 3 */
@@ -126,6 +128,7 @@ static const struct dlci_tty dlci2tty[]
 
 enum recv_state {
 	RECV_STATE_IDLE,
+	RECV_STATE_BP_SLIDE_SEQ,
 	RECV_STATE_ADDR,
 	RECV_STATE_CONTROL,
 	RECV_STATE_LEN,
@@ -247,7 +250,7 @@ static void ts27010_debugstr(const char
 
 static int ts0710_valid_dlci(u8 dlci)
 {
-	if ((dlci < TS0710_MAX_CHN) && (dlci > 0))
+	if ((dlci < TS0710_MAX_CHN)) // && (dlci >= 0))
 		return 1;
 	else
 		return 0;
@@ -349,7 +352,7 @@ static int ts0710_pkt_send(struct ts0710
 	u8 *d;
 	int len;
 	int header_len;
-	int res;
+	int res = -1;
 
 	if (pkt->h.length.ea == 1) {
 		len = pkt->h.length.len;
@@ -370,7 +373,9 @@ static int ts0710_pkt_send(struct ts0710
 	ts27010_debughex(DBG_VERBOSE, "ts27010: > ",
 			 data, TS0710_FRAME_SIZE(len));
 
-	res = ts27010_ldisc_send(ts27010mux_tty, data, TS0710_FRAME_SIZE(len));
+
+	if (ts27010_mux_active())
+		res = ts27010_ldisc_send(ts27010mux_tty, data, TS0710_FRAME_SIZE(len));
 
 	if (res < 0) {
 		pr_err("ts27010: pkt write error %d\n", res);
@@ -432,7 +437,7 @@ static void ts0710_upon_disconnect(void)
 static int ts27010_send_cmd(struct ts0710_con *ts0710, u8 dlci, u8 cmd)
 {
 	u8 frame[TS0710_FRAME_SIZE(0)];
-	ts0710_pkt_set_header(frame, 0, 1, MCC_RSP, dlci, SET_PF(cmd));
+	ts0710_pkt_set_header(frame, 0, 1, MCC_CMD, dlci, SET_PF(cmd));
 	return ts0710_pkt_send(ts0710, frame);
 }
 
@@ -473,6 +478,18 @@ static int ts27010_send_uih(struct ts071
 	return ts0710_pkt_send(ts0710, frame);
 }
 
+static int ts27010_send_ack(struct ts0710_con *ts0710, u8 seq)
+{
+	u8 frame[TS0710_FRAME_SIZE(1)];
+
+	ts_debug(DBG_CMD,
+		 "ts27010: sending ACK packet to DLCI %d\n", 0);
+	ts0710_pkt_set_header(frame, 1, 1, MCC_CMD, CTRL_CHAN, ACK);
+	*(u8 *)ts0710_pkt_data(frame) = seq;
+	return ts0710_pkt_send(ts0710, frame);
+}
+
+
 static void ts27010_mcc_set_header(u8 *frame, int len, int cr, int cmd)
 {
 	struct mcc_short_frame *mcc_pkt;
@@ -1320,6 +1337,7 @@ void ts27010_mux_recv(struct ts27010_rin
 	int state = RECV_STATE_IDLE;
 	int consume_idx = -1;
 	int data_idx = 0;
+	static u8 seq = 0;
 	u8 addr = 0;
 	u8 control = 0;
 	int len = 0;
@@ -1334,10 +1352,22 @@ void ts27010_mux_recv(struct ts27010_rin
 		case RECV_STATE_IDLE:
 			if (c == TS0710_BASIC_FLAG) {
 				fcs = ts0710_crc_start();
+				state = RECV_STATE_BP_SLIDE_SEQ;
+			} else {
+				consume_idx = i;
+			}
+			break;
+
+		case RECV_STATE_BP_SLIDE_SEQ:
+			if (c == seq) {
+				fcs = ts0710_crc_calc(fcs, c);
 				state = RECV_STATE_ADDR;
 			} else {
+				pr_warning(
+					"ts27010: invalid seq %d, expected: %d. Drop msg.\n", c, seq);
 				consume_idx = i;
 			}
+
 			break;
 
 		case RECV_STATE_ADDR:
@@ -1393,6 +1423,15 @@ void ts27010_mux_recv(struct ts27010_rin
 
 		case RECV_STATE_END:
 			if (c == TS0710_BASIC_FLAG && ts0710_crc_check(fcs)) {
+				seq++;
+				if (seq >= 4)
+					seq = 0;
+
+				ts27010_send_ack(&ts0710_connection, seq);
+
+				ts27010_debugrbufhex(DBG_VERBOSE, "ts27010: < ",
+							rbuf, 0, count);
+
 				ts27010_handle_frame(rbuf, addr, control,
 						      data_idx, len);
 			} else {

Attachment: pgpwFsxvXlxDK.pgp
Description: PGP signature

Reply via email to