[RFC] WIP-patches on sllin

[RFC] WIP-patches on sllin

[Alexander Stein]

Hello,

I'm currently working with sllin (LIN based on socketcan) from
git://rtime.felk.cvut.cz/linux-lin.git / https://rtime.felk.cvut.cz/gitweb/linux-lin.git/shortlog.
Up to now master is working pretty well and I fixed a bug for slave node
(patch 5). I'm currently working on LIN slave task on slave node which was
unimplemented up to now.
This first version uses the slave cache for message sending when requested,
but so it is no longer possible to configure the length for specific message
IDs which are only recweived. The LIN_CACHE_RESPONSE bit is used up to now
to configure the data length (DLC) of receptions in slave mode. This doesn't
work anymore when slave responses to configured IDs.
Are there any comments to distinguish between configured for sending and
configured for reception only? I tried to avoid addig a new bit for that, but
didn't find any idea for that.
I still would acknowledge comments on the other patches.

Best regards,
Alexander

[PATCH 1/6] sllin: Replace pr_* with netdev_* where approriate

[Alexander Stein]

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 70 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index 58e59e4..c1cdcd1 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -319,7 +319,7 @@ static void sllin_write_wakeup(struct tty_struct *tty)
 	} while (unlikely(test_bit(SLF_TXBUFF_RQ, &sl->flags)));
 
 	if ((remains > 0) && (actual >= 0)) {
-		pr_debug("sllin: sllin_write_wakeup sent %d, remains %d, waiting\n",
+		netdev_dbg(sl->dev, "sllin_write_wakeup sent %d, remains %d, waiting\n",
 			sl->tx_cnt, sl->tx_lim - sl->tx_cnt);
 		return;
 	}
@@ -328,7 +328,7 @@ static void sllin_write_wakeup(struct tty_struct *tty)
 	set_bit(SLF_TXEVENT, &sl->flags);
 	wake_up(&sl->kwt_wq);
 
-	pr_debug("sllin: sllin_write_wakeup sent %d, wakeup\n", sl->tx_cnt);
+	netdev_dbg(sl->dev, "sllin_write_wakeup sent %d, wakeup\n", sl->tx_cnt);
 }
 
 /**
@@ -347,11 +347,11 @@ static netdev_tx_t sll_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	spin_lock(&sl->lock);
 	if (!netif_running(dev))  {
-		pr_warn("%s: xmit: iface is down\n", dev->name);
+		netdev_warn(sl->dev, "xmit: iface is down\n");
 		goto err_out_unlock;
 	}
 	if (sl->tty == NULL) {
-		pr_warn("%s: xmit: no tty device connected\n", dev->name);
+		netdev_warn(sl->dev, "xmit: no tty device connected\n");
 		goto err_out_unlock;
 	}
 
@@ -405,7 +405,7 @@ static int sll_open(struct net_device *dev)
 {
 	struct sllin *sl = netdev_priv(dev);
 
-	pr_debug("sllin: %s() invoked\n", __func__);
+	netdev_dbg(sl->dev, "%s() invoked\n", __func__);
 
 	if (sl->tty == NULL)
 		return -ENODEV;
@@ -457,7 +457,7 @@ static void sllin_master_receive_buf(struct tty_struct *tty,
 	/* Read the characters out of the buffer */
 	while (count--) {
 		if (fp && *fp++) {
-			pr_debug("sllin: sllin_receive_buf char 0x%02x ignored "
+			netdev_dbg(sl->dev, "sllin_master_receive_buf char 0x%02x ignored "
 				"due marker 0x%02x, flags 0x%lx\n",
 				*cp, *(fp-1), sl->flags);
 
@@ -472,13 +472,13 @@ static void sllin_master_receive_buf(struct tty_struct *tty,
 #ifndef BREAK_BY_BAUD
 		/* We didn't receive Break character -- fake it! */
 		if ((sl->rx_cnt == SLLIN_BUFF_BREAK) && (*cp == 0x55)) {
-			pr_debug("sllin: LIN_RX[%d]: 0x00\n", sl->rx_cnt);
+			netdev_dbg(sl->dev, "LIN_RX[%d]: 0x00\n", sl->rx_cnt);
 			sl->rx_buff[sl->rx_cnt++] = 0x00;
 		}
 #endif /* BREAK_BY_BAUD */
 
 		if (sl->rx_cnt < SLLIN_BUFF_LEN) {
-			pr_debug("sllin: LIN_RX[%d]: 0x%02x\n", sl->rx_cnt, *cp);
+			netdev_dbg(sl->dev, "LIN_RX[%d]: 0x%02x\n", sl->rx_cnt, *cp);
 			sl->rx_buff[sl->rx_cnt++] = *cp++;
 		}
 	}
@@ -487,9 +487,9 @@ static void sllin_master_receive_buf(struct tty_struct *tty,
 	if (sl->rx_cnt >= sl->rx_expect) {
 		set_bit(SLF_RXEVENT, &sl->flags);
 		wake_up(&sl->kwt_wq);
-		pr_debug("sllin: sllin_receive_buf count %d, wakeup\n",	sl->rx_cnt);
+		netdev_dbg(sl->dev, "sllin_receive_buf count %d, wakeup\n", sl->rx_cnt);
 	} else {
-		pr_debug("sllin: sllin_receive_buf count %d, waiting\n", sl->rx_cnt);
+		netdev_dbg(sl->dev, "sllin_receive_buf count %d, waiting\n", sl->rx_cnt);
 	}
 }
 
@@ -505,7 +505,7 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 	/* Read the characters out of the buffer */
 	while (count--) {
 		if (fp && *fp++) {
-			pr_debug("sllin: sllin_receive_buf char 0x%02x ignored "
+			netdev_dbg(sl->dev, "sllin_slave_receive_buf char 0x%02x ignored "
 				"due marker 0x%02x, flags 0x%lx\n",
 				*cp, *(fp-1), sl->flags);
 
@@ -517,7 +517,7 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 		}
 
 		if (sl->rx_cnt < SLLIN_BUFF_LEN) {
-			pr_debug("sllin: LIN_RX[%d]: 0x%02x\n", sl->rx_cnt, *cp);
+			netdev_dbg(sl->dev, "LIN_RX[%d]: 0x%02x\n", sl->rx_cnt, *cp);
 
 			/* We did not receive break (0x00) character */
 			if ((sl->rx_cnt == SLLIN_BUFF_BREAK) && (*cp == 0x55)) {
@@ -547,6 +547,7 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 			sce = &sl->linfr_cache[lin_id];
 
 			spin_lock_irqsave(&sl->linfr_lock, flags);
+
 			/* Is the length of data set in frame cache? */
 			if (sce->frame_fl & LIN_CACHE_RESPONSE) {
 				sl->rx_expect += sce->dlc;
@@ -558,6 +559,7 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 			spin_unlock_irqrestore(&sl->linfr_lock, flags);
 
 			sl->header_received = true;
+
 			sll_send_rtr(sl);
 			continue;
 		}
@@ -568,7 +570,7 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 			((sl->rx_len_unknown == true) && (count == 0)))) {
 
 			sll_bump(sl);
-			pr_debug("sllin: Received LIN header & LIN response. "
+			netdev_dbg(sl->dev, "Received LIN header & LIN response. "
 					"rx_cnt = %u, rx_expect = %u\n", sl->rx_cnt,
 					sl->rx_expect);
 
@@ -585,7 +587,7 @@ static void sllin_receive_buf(struct tty_struct *tty,
 			      const unsigned char *cp, char *fp, int count)
 {
 	struct sllin *sl = (struct sllin *) tty->disc_data;
-	pr_debug("sllin: sllin_receive_buf invoked, count = %u\n", count);
+	netdev_dbg(sl->dev, "sllin_receive_buf invoked, count = %u\n", count);
 
 	if (!sl || sl->magic != SLLIN_MAGIC || !netif_running(sl->dev))
 		return;
@@ -643,7 +645,7 @@ static int sllin_configure_frame_cache(struct sllin *sl, struct can_frame *cf)
 		return -1;
 
 	sce = &sl->linfr_cache[cf->can_id & LIN_ID_MASK];
-	pr_debug("sllin: Setting frame cache with EFF CAN frame. LIN ID = %d\n",
+	netdev_dbg(sl->dev, "Setting frame cache with EFF CAN frame. LIN ID = %d\n",
 		cf->can_id & LIN_ID_MASK);
 
 	spin_lock_irqsave(&sl->linfr_lock, flags);
@@ -776,7 +778,7 @@ int sllin_send_tx_buff(struct sllin *sl)
 			sl->tx_cnt += res;
 		}
 
-		pr_debug("sllin: sllin_send_tx_buff sent %d, remains %d\n",
+		netdev_dbg(sl->dev, "sllin_send_tx_buff sent %d, remains %d\n",
 				sl->tx_cnt, remains);
 
 		clear_bit(SLF_TXBUFF_INPR, &sl->flags);
@@ -851,7 +853,7 @@ int sllin_send_break(struct sllin *sl)
 
 	sl->tx_cnt = SLLIN_BUFF_SYNC;
 
-	pr_debug("sllin: Break sent.\n");
+	netdev_dbg(sl->dev, "Break sent.\n");
 	set_bit(SLF_RXEVENT, &sl->flags);
 	wake_up(&sl->kwt_wq);
 
@@ -922,7 +924,7 @@ int sllin_kwthread(void *ptr)
 	int tx_bytes = 0; /* Used for Network statistics */
 
 
-	pr_debug("sllin: sllin_kwthread started.\n");
+	netdev_dbg(sl->dev, "sllin_kwthread started.\n");
 	sched_setscheduler(current, SCHED_FIFO, &schparam);
 
 	clear_bit(SLF_ERROR, &sl->flags);
@@ -952,14 +954,14 @@ int sllin_kwthread(void *ptr)
 				&& test_bit(SLF_MSGEVENT, &sl->flags)));
 
 		if (test_and_clear_bit(SLF_RXEVENT, &sl->flags)) {
-			pr_debug("sllin: sllin_kthread RXEVENT\n");
+			netdev_dbg(sl->dev, "sllin_kthread RXEVENT\n");
 		}
 
 		if (test_and_clear_bit(SLF_ERROR, &sl->flags)) {
 			unsigned long usleep_range_min;
 			unsigned long usleep_range_max;
 			hrtimer_cancel(&sl->rx_timer);
-			pr_debug("sllin: sllin_kthread ERROR\n");
+			netdev_dbg(sl->dev, "sllin_kthread ERROR\n");
 
 			if (sl->lin_state != SLSTATE_IDLE)
 				sllin_report_error(sl, LIN_ERR_FRAMING);
@@ -973,11 +975,11 @@ int sllin_kwthread(void *ptr)
 		}
 
 		if (test_and_clear_bit(SLF_TXEVENT, &sl->flags)) {
-			pr_debug("sllin: sllin_kthread TXEVENT\n");
+			netdev_dbg(sl->dev, "sllin_kthread TXEVENT\n");
 		}
 
 		if (test_and_clear_bit(SLF_TMOUTEVENT, &sl->flags)) {
-			pr_debug("sllin: sllin_kthread TMOUTEVENT\n");
+			netdev_dbg(sl->dev, "sllin_kthread TMOUTEVENT\n");
 			sllin_reset_buffs(sl);
 
 			sl->lin_state = SLSTATE_IDLE;
@@ -995,7 +997,7 @@ int sllin_kwthread(void *ptr)
 				unsigned long flags;
 				struct sllin_conf_entry *sce;
 
-				pr_debug("sllin: %s: RTR SFF CAN frame, ID = %x\n",
+				netdev_dbg(sl->dev, "%s: RTR SFF CAN frame, ID = %x\n",
 					__func__, cf->can_id & LIN_ID_MASK);
 
 				sce = &sl->linfr_cache[cf->can_id & LIN_ID_MASK];
@@ -1005,7 +1007,7 @@ int sllin_kwthread(void *ptr)
 				if ((sce->frame_fl & LIN_CACHE_RESPONSE)
 					&& (sce->dlc > 0)) {
 
-					pr_debug("sllin: Sending LIN response from linfr_cache\n");
+					netdev_dbg(sl->dev, "Sending LIN response from linfr_cache\n");
 
 					lin_data = sce->data;
 					lin_dlc = sce->dlc;
@@ -1020,7 +1022,7 @@ int sllin_kwthread(void *ptr)
 				spin_unlock_irqrestore(&sl->linfr_lock, flags);
 
 			} else { /* SFF NON-RTR CAN frame -> LIN header + LIN response */
-				pr_debug("sllin: %s: NON-RTR SFF CAN frame, ID = %x\n",
+				netdev_dbg(sl->dev, "%s: NON-RTR SFF CAN frame, ID = %x\n",
 					__func__, (int)cf->can_id & LIN_ID_MASK);
 
 				lin_data = cf->data;
@@ -1092,7 +1094,7 @@ slstate_response_wait:
 				lin_buff = (sl->lin_master) ? sl->tx_buff : sl->rx_buff;
 				if (cf->can_id == (lin_buff[SLLIN_BUFF_ID] & LIN_ID_MASK)) {
 					hrtimer_cancel(&sl->rx_timer);
-					pr_debug("sllin: received LIN response in a CAN frame.\n");
+					netdev_dbg(sl->dev, "received LIN response in a CAN frame.\n");
 					if (sllin_setup_msg(sl, SLLIN_STPMSG_RESPONLY,
 						cf->can_id & LIN_ID_MASK,
 						cf->data, cf->can_dlc) != -1) {
@@ -1125,15 +1127,15 @@ slstate_response_wait:
 				continue;
 
 			hrtimer_cancel(&sl->rx_timer);
-			pr_debug("sllin: response received ID %d len %d\n",
+			netdev_dbg(sl->dev, "response received ID %d len %d\n",
 				sl->rx_buff[SLLIN_BUFF_ID], sl->rx_cnt - SLLIN_BUFF_DATA - 1);
 
 			if (sllin_rx_validate(sl) == -1) {
-				pr_debug("sllin: RX validation failed.\n");
+				netdev_dbg(sl->dev, "RX validation failed.\n");
 				sllin_report_error(sl, LIN_ERR_CHECKSUM);
 			} else {
 				/* Send CAN non-RTR frame with data */
-				pr_debug("sllin: sending NON-RTR CAN frame with LIN payload.");
+				netdev_dbg(sl->dev, "sending NON-RTR CAN frame with LIN payload.");
 				sll_bump(sl); /* send packet to the network layer */
 			}
 
@@ -1148,7 +1150,7 @@ slstate_response_sent:
 
 			hrtimer_cancel(&sl->rx_timer);
 			sll_bump(sl); /* send packet to the network layer */
-			pr_debug("sllin: response sent ID %d len %d\n",
+			netdev_dbg(sl->dev, "response sent ID %d len %d\n",
 				sl->rx_buff[SLLIN_BUFF_ID], sl->rx_cnt - SLLIN_BUFF_DATA - 1);
 
 			sl->id_to_send = false;
@@ -1158,7 +1160,7 @@ slstate_response_sent:
 	}
 
 	hrtimer_cancel(&sl->rx_timer);
-	pr_debug("sllin: sllin_kwthread stopped.\n");
+	netdev_dbg(sl->dev, "sllin_kwthread stopped.\n");
 
 	return 0;
 }
@@ -1253,6 +1255,7 @@ static int sllin_open(struct tty_struct *tty)
 {
 	struct sllin *sl;
 	int err;
+
 	pr_debug("sllin: %s() invoked\n", __func__);
 
 	if (!capable(CAP_NET_ADMIN))
@@ -1290,12 +1293,10 @@ static int sllin_open(struct tty_struct *tty)
 	if (!test_bit(SLF_INUSE, &sl->flags)) {
 		/* Perform the low-level SLLIN initialization. */
 		sl->lin_master = master;
-#ifdef DEBUG
 		if (master)
 			pr_debug("sllin: Configured as MASTER\n");
 		else
 			pr_debug("sllin: Configured as SLAVE\n");
-#endif
 
 		sllin_reset_buffs(sl);
 
@@ -1492,8 +1493,7 @@ static void __exit sllin_exit(void)
 
 		sl = netdev_priv(dev);
 		if (sl->tty) {
-			pr_err("%s: tty discipline still running\n",
-			       dev->name);
+			netdev_dbg(sl->dev, "tty discipline still running\n");
 			/* Intentionally leak the control block. */
 			dev->destructor = NULL;
 		}
-- 
1.8.3.2

[PATCH 2/6] sllin: Make local functions static

[Alexander Stein]

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index c1cdcd1..cf3cd75 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -609,7 +609,7 @@ static void sllin_receive_buf(struct tty_struct *tty,
  * @sl:
  * @err: Error flag to be sent.
  */
-void sllin_report_error(struct sllin *sl, int err)
+static void sllin_report_error(struct sllin *sl, int err)
 {
 	switch (err) {
 	case LIN_ERR_CHECKSUM:
@@ -694,7 +694,7 @@ static inline unsigned sllin_checksum(unsigned char *data, int length, int enhan
 #define SLLIN_STPMSG_CHCKSUM_CLS	(1 << 1)
 #define SLLIN_STPMSG_CHCKSUM_ENH	(1 << 2)
 
-int sllin_setup_msg(struct sllin *sl, int mode, int id,
+static int sllin_setup_msg(struct sllin *sl, int mode, int id,
 		unsigned char *data, int len)
 {
 	if (id > LIN_ID_MASK)
@@ -736,7 +736,7 @@ static void sllin_reset_buffs(struct sllin *sl)
 	sl->data_to_send = false;
 }
 
-int sllin_send_tx_buff(struct sllin *sl)
+static int sllin_send_tx_buff(struct sllin *sl)
 {
 	struct tty_struct *tty = sl->tty;
 	int remains;
@@ -795,7 +795,7 @@ error_in_write:
 }
 
 #ifdef BREAK_BY_BAUD
-int sllin_send_break(struct sllin *sl)
+static int sllin_send_break(struct sllin *sl)
 {
 	struct tty_struct *tty = sl->tty;
 	unsigned long break_baud;
@@ -820,7 +820,7 @@ int sllin_send_break(struct sllin *sl)
 }
 #else /* BREAK_BY_BAUD */
 
-int sllin_send_break(struct sllin *sl)
+static int sllin_send_break(struct sllin *sl)
 {
 	struct tty_struct *tty = sl->tty;
 	int retval;
@@ -916,7 +916,7 @@ static int sllin_rx_validate(struct sllin *sl)
  *  sllin_kwthread - kernel worker thread
  *****************************************/
 
-int sllin_kwthread(void *ptr)
+static int sllin_kwthread(void *ptr)
 {
 	struct sllin *sl = (struct sllin *)ptr;
 	struct tty_struct *tty = sl->tty;
-- 
1.8.3.2

[PATCH 3/6] sllin: slave: don't forget to wait for the checksum

[Alexander Stein]

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index cf3cd75..b97219e 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -550,7 +550,7 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 
 			/* Is the length of data set in frame cache? */
 			if (sce->frame_fl & LIN_CACHE_RESPONSE) {
-				sl->rx_expect += sce->dlc;
+				sl->rx_expect += sce->dlc + 1; /* + checksum */
 				sl->rx_len_unknown = false;
 			} else {
 				sl->rx_expect += SLLIN_DATA_MAX + 1; /* + checksum */
-- 
1.8.3.2

[PATCH 4/6] sllin: reorder functions

[Alexander Stein]

Needed for further commits

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 288 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 144 insertions(+), 144 deletions(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index b97219e..e8aca08 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -494,111 +494,6 @@ static void sllin_master_receive_buf(struct tty_struct *tty,
 }
 
 
-static void sllin_slave_receive_buf(struct tty_struct *tty,
-			      const unsigned char *cp, char *fp, int count)
-{
-	struct sllin *sl = (struct sllin *) tty->disc_data;
-	int lin_id;
-	struct sllin_conf_entry *sce;
-
-
-	/* Read the characters out of the buffer */
-	while (count--) {
-		if (fp && *fp++) {
-			netdev_dbg(sl->dev, "sllin_slave_receive_buf char 0x%02x ignored "
-				"due marker 0x%02x, flags 0x%lx\n",
-				*cp, *(fp-1), sl->flags);
-
-			/* Received Break */
-			sl->rx_cnt = 0;
-			sl->rx_expect = SLLIN_BUFF_ID + 1;
-			sl->rx_len_unknown = false; /* We do know exact length of the header */
-			sl->header_received = false;
-		}
-
-		if (sl->rx_cnt < SLLIN_BUFF_LEN) {
-			netdev_dbg(sl->dev, "LIN_RX[%d]: 0x%02x\n", sl->rx_cnt, *cp);
-
-			/* We did not receive break (0x00) character */
-			if ((sl->rx_cnt == SLLIN_BUFF_BREAK) && (*cp == 0x55)) {
-				sl->rx_buff[sl->rx_cnt++] = 0x00;
-			}
-
-			if (sl->rx_cnt == SLLIN_BUFF_SYNC) {
-				/* 'Duplicated' break character -- ignore */
-				if (*cp == 0x00) {
-					cp++;
-					continue;
-				}
-
-				/* Wrong sync character */
-				if (*cp != 0x55)
-					break;
-			}
-
-			sl->rx_buff[sl->rx_cnt++] = *cp++;
-		}
-
-		/* Header received */
-		if ((sl->header_received == false) && (sl->rx_cnt >= (SLLIN_BUFF_ID + 1))) {
-			unsigned long flags;
-
-			lin_id = sl->rx_buff[SLLIN_BUFF_ID] & LIN_ID_MASK;
-			sce = &sl->linfr_cache[lin_id];
-
-			spin_lock_irqsave(&sl->linfr_lock, flags);
-
-			/* Is the length of data set in frame cache? */
-			if (sce->frame_fl & LIN_CACHE_RESPONSE) {
-				sl->rx_expect += sce->dlc + 1; /* + checksum */
-				sl->rx_len_unknown = false;
-			} else {
-				sl->rx_expect += SLLIN_DATA_MAX + 1; /* + checksum */
-				sl->rx_len_unknown = true;
-			}
-			spin_unlock_irqrestore(&sl->linfr_lock, flags);
-
-			sl->header_received = true;
-
-			sll_send_rtr(sl);
-			continue;
-		}
-
-		/* Response received */
-		if ((sl->header_received == true) &&
-			((sl->rx_cnt >= sl->rx_expect) ||
-			((sl->rx_len_unknown == true) && (count == 0)))) {
-
-			sll_bump(sl);
-			netdev_dbg(sl->dev, "Received LIN header & LIN response. "
-					"rx_cnt = %u, rx_expect = %u\n", sl->rx_cnt,
-					sl->rx_expect);
-
-			/* Prepare for reception of new header */
-			sl->rx_cnt = 0;
-			sl->rx_expect = SLLIN_BUFF_ID + 1;
-			sl->rx_len_unknown = false; /* We do know exact length of the header */
-			sl->header_received = false;
-		}
-	}
-}
-
-static void sllin_receive_buf(struct tty_struct *tty,
-			      const unsigned char *cp, char *fp, int count)
-{
-	struct sllin *sl = (struct sllin *) tty->disc_data;
-	netdev_dbg(sl->dev, "sllin_receive_buf invoked, count = %u\n", count);
-
-	if (!sl || sl->magic != SLLIN_MAGIC || !netif_running(sl->dev))
-		return;
-
-	if (sl->lin_master)
-		sllin_master_receive_buf(tty, cp, fp, count);
-	else
-		sllin_slave_receive_buf(tty, cp, fp, count);
-
-}
-
 /*****************************************
  *  sllin message helper routines
  *****************************************/
@@ -736,6 +631,150 @@ static void sllin_reset_buffs(struct sllin *sl)
 	sl->data_to_send = false;
 }
 
+/**
+ * sllin_rx_validate() -- Validate received frame, i,e. check checksum
+ *
+ * @sl:
+ */
+static int sllin_rx_validate(struct sllin *sl)
+{
+	unsigned long flags;
+	int actual_id;
+	int ext_chcks_fl;
+	int lin_dlc;
+	unsigned char rec_chcksm = sl->rx_buff[sl->rx_cnt - 1];
+	struct sllin_conf_entry *sce;
+
+	actual_id = sl->rx_buff[SLLIN_BUFF_ID] & LIN_ID_MASK;
+	sce = &sl->linfr_cache[actual_id];
+
+	spin_lock_irqsave(&sl->linfr_lock, flags);
+	lin_dlc = sce->dlc;
+	ext_chcks_fl = sce->frame_fl & LIN_CHECKSUM_EXTENDED;
+	spin_unlock_irqrestore(&sl->linfr_lock, flags);
+
+	if (sllin_checksum(sl->rx_buff, sl->rx_cnt - 1, ext_chcks_fl) !=
+		rec_chcksm) {
+
+		/* Type of checksum is configured for particular frame */
+		if (lin_dlc > 0) {
+			return -1;
+		} else {
+			if (sllin_checksum(sl->rx_buff,	sl->rx_cnt - 1,
+				!ext_chcks_fl) != rec_chcksm) {
+				return -1;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static void sllin_slave_receive_buf(struct tty_struct *tty,
+			      const unsigned char *cp, char *fp, int count)
+{
+	struct sllin *sl = (struct sllin *) tty->disc_data;
+	int lin_id;
+	struct sllin_conf_entry *sce;
+
+
+	/* Read the characters out of the buffer */
+	while (count--) {
+		if (fp && *fp++) {
+			netdev_dbg(sl->dev, "sllin_slave_receive_buf char 0x%02x ignored "
+				"due marker 0x%02x, flags 0x%lx\n",
+				*cp, *(fp-1), sl->flags);
+
+			/* Received Break */
+			sl->rx_cnt = 0;
+			sl->rx_expect = SLLIN_BUFF_ID + 1;
+			sl->rx_len_unknown = false; /* We do know exact length of the header */
+			sl->header_received = false;
+		}
+
+		if (sl->rx_cnt < SLLIN_BUFF_LEN) {
+			netdev_dbg(sl->dev, "LIN_RX[%d]: 0x%02x\n", sl->rx_cnt, *cp);
+
+			/* We did not receive break (0x00) character */
+			if ((sl->rx_cnt == SLLIN_BUFF_BREAK) && (*cp == 0x55)) {
+				sl->rx_buff[sl->rx_cnt++] = 0x00;
+			}
+
+			if (sl->rx_cnt == SLLIN_BUFF_SYNC) {
+				/* 'Duplicated' break character -- ignore */
+				if (*cp == 0x00) {
+					cp++;
+					continue;
+				}
+
+				/* Wrong sync character */
+				if (*cp != 0x55)
+					break;
+			}
+
+			sl->rx_buff[sl->rx_cnt++] = *cp++;
+		}
+
+		/* Header received */
+		if ((sl->header_received == false) && (sl->rx_cnt >= (SLLIN_BUFF_ID + 1))) {
+			unsigned long flags;
+
+			lin_id = sl->rx_buff[SLLIN_BUFF_ID] & LIN_ID_MASK;
+			sce = &sl->linfr_cache[lin_id];
+
+			spin_lock_irqsave(&sl->linfr_lock, flags);
+
+			/* Is the length of data set in frame cache? */
+			if (sce->frame_fl & LIN_CACHE_RESPONSE) {
+				sl->rx_expect += sce->dlc + 1; /* + checksum */
+				sl->rx_len_unknown = false;
+			} else {
+				sl->rx_expect += SLLIN_DATA_MAX + 1; /* + checksum */
+				sl->rx_len_unknown = true;
+			}
+			spin_unlock_irqrestore(&sl->linfr_lock, flags);
+
+			sl->header_received = true;
+
+			sll_send_rtr(sl);
+			continue;
+		}
+
+		/* Response received */
+		if ((sl->header_received == true) &&
+			((sl->rx_cnt >= sl->rx_expect) ||
+			((sl->rx_len_unknown == true) && (count == 0)))) {
+
+			sll_bump(sl);
+			netdev_dbg(sl->dev, "Received LIN header & LIN response. "
+					"rx_cnt = %u, rx_expect = %u\n", sl->rx_cnt,
+					sl->rx_expect);
+
+			/* Prepare for reception of new header */
+			sl->rx_cnt = 0;
+			sl->rx_expect = SLLIN_BUFF_ID + 1;
+			sl->rx_len_unknown = false; /* We do know exact length of the header */
+			sl->header_received = false;
+		}
+	}
+}
+
+static void sllin_receive_buf(struct tty_struct *tty,
+			      const unsigned char *cp, char *fp, int count)
+{
+	struct sllin *sl = (struct sllin *) tty->disc_data;
+	netdev_dbg(sl->dev, "sllin_receive_buf invoked, count = %u\n", count);
+
+	if (!sl || sl->magic != SLLIN_MAGIC || !netif_running(sl->dev))
+		return;
+
+	if (sl->lin_master)
+		sllin_master_receive_buf(tty, cp, fp, count);
+	else
+		sllin_slave_receive_buf(tty, cp, fp, count);
+
+}
+
 static int sllin_send_tx_buff(struct sllin *sl)
 {
 	struct tty_struct *tty = sl->tty;
@@ -873,45 +912,6 @@ static enum hrtimer_restart sllin_rx_timeout_handler(struct hrtimer *hrtimer)
 	return HRTIMER_NORESTART;
 }
 
-/**
- * sllin_rx_validate() -- Validate received frame, i,e. check checksum
- *
- * @sl:
- */
-static int sllin_rx_validate(struct sllin *sl)
-{
-	unsigned long flags;
-	int actual_id;
-	int ext_chcks_fl;
-	int lin_dlc;
-	unsigned char rec_chcksm = sl->rx_buff[sl->rx_cnt - 1];
-	struct sllin_conf_entry *sce;
-
-	actual_id = sl->rx_buff[SLLIN_BUFF_ID] & LIN_ID_MASK;
-	sce = &sl->linfr_cache[actual_id];
-
-	spin_lock_irqsave(&sl->linfr_lock, flags);
-	lin_dlc = sce->dlc;
-	ext_chcks_fl = sce->frame_fl & LIN_CHECKSUM_EXTENDED;
-	spin_unlock_irqrestore(&sl->linfr_lock, flags);
-
-	if (sllin_checksum(sl->rx_buff, sl->rx_cnt - 1, ext_chcks_fl) !=
-		rec_chcksm) {
-
-		/* Type of checksum is configured for particular frame */
-		if (lin_dlc > 0) {
-			return -1;
-		} else {
-			if (sllin_checksum(sl->rx_buff,	sl->rx_cnt - 1,
-				!ext_chcks_fl) != rec_chcksm) {
-				return -1;
-			}
-		}
-	}
-
-	return 0;
-}
-
 /*****************************************
  *  sllin_kwthread - kernel worker thread
  *****************************************/
-- 
1.8.3.2

[PATCH 5/6] sllin: If length is unknown read until timer occurs or break is received

[Alexander Stein]

If the length is unknown stop waiting for new data until the tty rx queue
is empty once is not working on fast systems where sllin_receive_buf is
called for each byte received. Instead wait up to 8 bytes, when the
timeout occurs or when a new break is received.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 54 ++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 10 deletions(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index e8aca08..36f23a5 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -670,6 +670,23 @@ static int sllin_rx_validate(struct sllin *sl)
 	return 0;
 }
 
+static void sllin_slave_finish_rx_msg(struct sllin *sl)
+{
+	if (sllin_rx_validate(sl) == -1) {
+		netdev_dbg(sl->dev, "sllin: RX validation failed.\n");
+		sllin_report_error(sl, LIN_ERR_CHECKSUM);
+	} else {
+		/* Send CAN non-RTR frame with data */
+		netdev_dbg(sl->dev, "sllin: sending NON-RTR CAN frame with LIN payload.");
+		sll_bump(sl); /* send packet to the network layer */
+	}
+	/* Prepare for reception of new header */
+	sl->rx_cnt = 0;
+	sl->rx_expect = SLLIN_BUFF_ID + 1;
+	sl->rx_len_unknown = false; /* We do know exact length of the header */
+	sl->header_received = false;
+}
+
 static void sllin_slave_receive_buf(struct tty_struct *tty,
 			      const unsigned char *cp, char *fp, int count)
 {
@@ -681,6 +698,20 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 	/* Read the characters out of the buffer */
 	while (count--) {
 		if (fp && *fp++) {
+			/*
+			 * If we don't know the length of the current message
+			 * we received the break of the next message.
+			 * Evaluate the previous one before continuing
+			 */
+			if (sl->rx_len_unknown == true)
+			{
+				hrtimer_cancel(&sl->rx_timer);
+				sllin_slave_finish_rx_msg(sl);
+
+				set_bit(SLF_RXEVENT, &sl->flags);
+				wake_up(&sl->kwt_wq);
+			}
+
 			netdev_dbg(sl->dev, "sllin_slave_receive_buf char 0x%02x ignored "
 				"due marker 0x%02x, flags 0x%lx\n",
 				*cp, *(fp-1), sl->flags);
@@ -736,25 +767,23 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 
 			sl->header_received = true;
 
+			hrtimer_start(&sl->rx_timer,
+				ktime_add(ktime_get(), sl->rx_timer_timeout),
+				HRTIMER_MODE_ABS);
 			sll_send_rtr(sl);
 			continue;
 		}
 
 		/* Response received */
 		if ((sl->header_received == true) &&
-			((sl->rx_cnt >= sl->rx_expect) ||
-			((sl->rx_len_unknown == true) && (count == 0)))) {
+			((sl->rx_cnt >= sl->rx_expect))) {
 
-			sll_bump(sl);
+			hrtimer_cancel(&sl->rx_timer);
 			netdev_dbg(sl->dev, "Received LIN header & LIN response. "
 					"rx_cnt = %u, rx_expect = %u\n", sl->rx_cnt,
 					sl->rx_expect);
+			sllin_slave_finish_rx_msg(sl);
 
-			/* Prepare for reception of new header */
-			sl->rx_cnt = 0;
-			sl->rx_expect = SLLIN_BUFF_ID + 1;
-			sl->rx_len_unknown = false; /* We do know exact length of the header */
-			sl->header_received = false;
 		}
 	}
 }
@@ -905,8 +934,13 @@ static enum hrtimer_restart sllin_rx_timeout_handler(struct hrtimer *hrtimer)
 {
 	struct sllin *sl = container_of(hrtimer, struct sllin, rx_timer);
 
-	sllin_report_error(sl, LIN_ERR_RX_TIMEOUT);
-	set_bit(SLF_TMOUTEVENT, &sl->flags);
+	if (sl->lin_master) {
+		sllin_report_error(sl, LIN_ERR_RX_TIMEOUT);
+		set_bit(SLF_TMOUTEVENT, &sl->flags);
+	} else {
+		sllin_slave_finish_rx_msg(sl);
+		set_bit(SLF_RXEVENT, &sl->flags);
+	}
 	wake_up(&sl->kwt_wq);
 
 	return HRTIMER_NORESTART;
-- 
1.8.3.2

[PATCH 6/6] sllin: Add first support for LIN slave task on slave node

[Alexander Stein]

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 3 deletions(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index 36f23a5..3e020b1 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -113,6 +113,7 @@ enum slstate {
 	SLSTATE_RESPONSE_WAIT_BUS, /* Wait for response from LIN bus
 				only (CAN frames from network stack
 				are not processed in this moment) */
+	SLSTATE_ID_RECEIVED,
 	SLSTATE_RESPONSE_SENT,
 };
 
@@ -755,10 +756,13 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 
 			spin_lock_irqsave(&sl->linfr_lock, flags);
 
+			sl->lin_state = SLSTATE_ID_RECEIVED;
 			/* Is the length of data set in frame cache? */
 			if (sce->frame_fl & LIN_CACHE_RESPONSE) {
 				sl->rx_expect += sce->dlc + 1; /* + checksum */
 				sl->rx_len_unknown = false;
+				set_bit(SLF_MSGEVENT, &sl->flags);
+				wake_up(&sl->kwt_wq);
 			} else {
 				sl->rx_expect += SLLIN_DATA_MAX + 1; /* + checksum */
 				sl->rx_len_unknown = true;
@@ -784,6 +788,8 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 					sl->rx_expect);
 			sllin_slave_finish_rx_msg(sl);
 
+			set_bit(SLF_RXEVENT, &sl->flags);
+			wake_up(&sl->kwt_wq);
 		}
 	}
 }
@@ -956,7 +962,9 @@ static int sllin_kwthread(void *ptr)
 	struct tty_struct *tty = sl->tty;
 	struct sched_param schparam = { .sched_priority = 40 };
 	int tx_bytes = 0; /* Used for Network statistics */
-
+	unsigned long flags;
+	int lin_id;
+	struct sllin_conf_entry *sce;
 
 	netdev_dbg(sl->dev, "sllin_kwthread started.\n");
 	sched_setscheduler(current, SCHED_FIFO, &schparam);
@@ -984,7 +992,8 @@ static int sllin_kwthread(void *ptr)
 			test_bit(SLF_TMOUTEVENT, &sl->flags) ||
 			test_bit(SLF_ERROR, &sl->flags) ||
 			(((sl->lin_state == SLSTATE_IDLE) ||
-				(sl->lin_state == SLSTATE_RESPONSE_WAIT))
+				(sl->lin_state == SLSTATE_RESPONSE_WAIT) ||
+				(sl->lin_state == SLSTATE_ID_RECEIVED))
 				&& test_bit(SLF_MSGEVENT, &sl->flags)));
 
 		if (test_and_clear_bit(SLF_RXEVENT, &sl->flags)) {
@@ -1028,7 +1037,6 @@ static int sllin_kwthread(void *ptr)
 
 			/* SFF RTR CAN frame -> LIN header */
 			if (cf->can_id & CAN_RTR_FLAG) {
-				unsigned long flags;
 				struct sllin_conf_entry *sce;
 
 				netdev_dbg(sl->dev, "%s: RTR SFF CAN frame, ID = %x\n",
@@ -1177,6 +1185,56 @@ slstate_response_wait:
 			sl->lin_state = SLSTATE_IDLE;
 			break;
 
+		case SLSTATE_ID_RECEIVED:
+			lin_id = sl->rx_buff[SLLIN_BUFF_ID] & LIN_ID_MASK;
+			sce = &sl->linfr_cache[lin_id];
+			spin_lock_irqsave(&sl->linfr_lock, flags);
+
+			if ((sce->frame_fl & LIN_CACHE_RESPONSE)
+					&& (sce->dlc > 0)
+					&& (test_bit(SLF_MSGEVENT, &sl->flags))) {
+				int mode;
+
+				netdev_dbg(sl->dev, "Sending LIN response from linfr_cache\n");
+
+				lin_data = sce->data;
+				lin_dlc = sce->dlc;
+				if (lin_dlc > SLLIN_DATA_MAX)
+					lin_dlc = SLLIN_DATA_MAX;
+				memcpy(lin_data_buff, lin_data, lin_dlc);
+				lin_data = lin_data_buff;
+				tx_bytes = lin_dlc;
+
+				mode = SLLIN_STPMSG_RESPONLY;
+				if (sl->rx_buff[SLLIN_BUFF_ID] & LIN_CHECKSUM_EXTENDED)
+					mode |= SLLIN_STPMSG_CHCKSUM_ENH;
+
+				if (sllin_setup_msg(sl, mode, lin_id & LIN_ID_MASK,
+					lin_data, lin_dlc) != -1) {
+
+					sl->rx_expect = sl->tx_lim;
+					sl->data_to_send = true;
+					sl->dev->stats.tx_packets++;
+					sl->dev->stats.tx_bytes += tx_bytes;
+					sl->resp_len_known = true;
+
+					if (!sl->lin_master) {
+						sl->tx_cnt = SLLIN_BUFF_DATA;
+					}
+					sllin_send_tx_buff(sl);
+				}
+
+				clear_bit(SLF_MSGEVENT, &sl->flags);
+				kfree_skb(sl->tx_req_skb);
+				netif_wake_queue(sl->dev);
+				hrtimer_start(&sl->rx_timer,
+					ktime_add(ktime_get(), sl->rx_timer_timeout),
+					HRTIMER_MODE_ABS);
+			}
+			spin_unlock_irqrestore(&sl->linfr_lock, flags);
+			sl->lin_state = SLSTATE_IDLE;
+			break;
+
 		case SLSTATE_RESPONSE_SENT:
 slstate_response_sent:
 			if (sl->rx_cnt < sl->tx_lim)
-- 
1.8.3.2

Re: [RFC] WIP-patches on sllin

[Pavel Pisa]

Hello Alexander,

On Tuesday 10 of December 2013 18:16:11 Alexander Stein wrote:
> I'm currently working with sllin (LIN based on socketcan) from
> git://rtime.felk.cvut.cz/linux-lin.git /
> https://rtime.felk.cvut.cz/gitweb/linux-lin.git/shortlog. Up to now master
> is working pretty well and I fixed a bug for slave node (patch 5). I'm
> currently working on LIN slave task on slave node which was unimplemented
> up to now.

first I want to express my happines that our work is
getting tested/used. This allows to check if concept fits needs
of more users and if it is confirmed we can start to speak
about move to the mainline (linux/drivers/staging or net/can)

I have pushed next patches to our repository

  sllin: Replace pr_* with netdev_* where approriate
  sllin: Make local functions static
  sllin: slave: don't forget to wait for the checksum

I would like to think and discuss little about the rest

> This first version uses the slave cache for message sending when requested,
> but so it is no longer possible to configure the length for specific
> message IDs which are only recweived. The LIN_CACHE_RESPONSE bit is used up
> to now to configure the data length (DLC) of receptions in slave mode. This
> doesn't work anymore when slave responses to configured IDs.
> Are there any comments to distinguish between configured for sending and
> configured for reception only? I tried to avoid addig a new bit for that,
> but didn't find any idea for that.
> I still would acknowledge comments on the other patches.

There was much more rich set of the flags in original code design

  /* Save configuration for particular LIN ID */
  #define LIN_ID_CONF		(1 <<  LIN_CANFR_FLAGS_OFFS)
  /* Publisher of particular LIN response is SLLIN Master */
  #define LIN_SRC_MASTER		(1 << (LIN_CANFR_FLAGS_OFFS + 1))
  #define LIN_SRC_SLAVE		(1 << (LIN_CANFR_FLAGS_OFFS + 2))
  #define LIN_SLAVE_LOCAL		(1 << (LIN_CANFR_FLAGS_OFFS + 3))
  #define LIN_SLAVE_REMOTE	(1 << (LIN_CANFR_FLAGS_OFFS + 4))
  #define LIN_LOC_SLAVE_CACHE	(1 << (LIN_CANFR_FLAGS_OFFS + 5))
  #define LIN_CHECKSUM_EXTENDED	(1 << (LIN_CANFR_FLAGS_OFFS + 6))
 
  #define LIN_ERR_RX_TIMEOUT	(1 << (LIN_CANFR_FLAGS_OFFS + 7))
  #define LIN_ERR_CHECKSUM	(1 << (LIN_CANFR_FLAGS_OFFS + 8))
  #define LIN_ERR_FRAMING		(1 << (LIN_CANFR_FLAGS_OFFS + 9))

but it was reduced to make life easier for users in one of the
final project clenup commit

  sllin: Flags clean up

and it seems that the set was oversimplified. One option is to
consider each cache location as setup when sl->linfr_cache[lin_id].dlc != 0
or we need to reintroduce back concept of LIN_LOC_SLAVE_CACHE.
I am not sure if zero length messages are disallowed by standard.
But if they are then dlc!=0 is reasonable choice. My personal
is many flags but I tend to too generic solutions usually as you
can in original flags support.

...

It has still one drawback if we decide to store received data
for IDs with unknown length in the frame cache for later retrieval
by local application. This can be resolved by re-addition of another
flag.

Without the flag, I propose next chages


diff --git a/sllin/sllin.c b/sllin/sllin.c
index b97219e..12dce59 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -549,7 +549,7 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
                        spin_lock_irqsave(&sl->linfr_lock, flags);

                        /* Is the length of data set in frame cache? */
-                       if (sce->frame_fl & LIN_CACHE_RESPONSE) {
+                       if (sce->dlc > 0) {
                                sl->rx_expect += sce->dlc + 1; /* + checksum */
                                sl->rx_len_unknown = false;
                        } else {


And appropriate change in the user space configuration utility

diff --git a/lin_config/src/sllin_config.c b/lin_config/src/sllin_config.c
index dc704d8..757046b 100644
--- a/lin_config/src/sllin_config.c
+++ b/lin_config/src/sllin_config.c
@@ -123,7 +123,6 @@ int sllin_cache_config(struct linc_lin_state *linc_lin_state,
        }

        for (i = 0; i < 0x3F; i++) {
-               if (linc_lin_state->frame_entry[i].status == 1) { /* Is active */
                        frame.can_dlc = linc_lin_state->frame_entry[i].data_len;
                        frame.can_id = i; /* LIN ID */
                        frame.data[0] = linc_lin_state->frame_entry[i].data[0]; /* Data */
@@ -134,13 +133,14 @@ int sllin_cache_config(struct linc_lin_state *linc_lin_state,
                        frame.data[5] = linc_lin_state->frame_entry[i].data[5]; /* Data */
                        frame.data[6] = linc_lin_state->frame_entry[i].data[6]; /* Data */
                        frame.data[7] = linc_lin_state->frame_entry[i].data[7]; /* Data */
-
-                       frame.can_id |= LIN_CTRL_FRAME | LIN_CACHE_RESPONSE;
+                       frame.can_id |= LIN_CTRL_FRAME;
+               if (linc_lin_state->frame_entry[i].status == 1) { /* Is active */
+                       frame.can_id |= LIN_CACHE_RESPONSE;
+               }
                        ret = write(s, &frame, sizeof(frame));
                        printf("configuring frame cache; ret = %d\n", ret);
                        //if (ret ...)
                        //read_response(tty);
-               }
        }

        close(s);

The use of dlc == 0 allows still to disable entry and I propose clear unconfigured
entries to do full reconfiguration when tool is called.

If you agree with above solution or propose to use new flag I prepare and commit
the patch.

To your additional patches, I need a little refresh of my information
to review slave functionality and there is a must for communication
with UART maintainers because without at least minor changes in FIFO
control it cannot work reliably.

Some comments

  sllin: reorder functions
       generally I am not opposed to the change but there are things to consider
          initial order tries keep lower level UART manipulation functions first
          to allow future extending the code to part specific to UART/TTY and more
          generic LIN code reusable even with LIN direct hardware support.
          But I agree that things has got mixed already due to checking for some
          LIN specific data in generic UART part and UART processing considerations
          in LIN protocol specific code.

  sllin: If length is unknown read until timer occurs or break is received
       probably the best thing to do in general. The use of Rx FIFO timeouts
       of some UARTS can be used there. It worked on PC for us but it is
       not robust. I would like to analyze little more the code to ensure
       that locking stay correct after your change.

  sllin: Add first support for LIN slave task on slave node
      it is possible that we do not need worker thread for the slave at all.
      The data can be prepared in UART callback processing. I believe
      we have it working that way at least in one of code versions.
      But slave was declared not to be implemented in the last version.
      On the other hand, use of the worker thread is more polite to other
      kernel services. There has been attempt for minimal locking between
      UART specific part and the worker thread because UART callbacks
      are invoked in many different even curious contexts according to
      serial port HW and kernel - RT in IRQ threads, vanilla usually BH
      but may be even hard IRQ for some arch, worker threads for USB devices
      etc.

Anyway thanks for your contribution and I hope that we move project
at least a little forward.

Best wishes,

                Pavel

-- 
                Pavel Pisa
    e-mail:     pisa@cmp.felk.cvut.cz
    www:        http://cmp.felk.cvut.cz/~pisa
    university: http://dce.fel.cvut.cz/

               

Re: [RFC] WIP-patches on sllin

[Alexander Stein]

Hello pavel,

On Friday 13 December 2013 02:00:18, Pavel Pisa wrote:
> On Tuesday 10 of December 2013 18:16:11 Alexander Stein wrote:
> > I'm currently working with sllin (LIN based on socketcan) from
> > git://rtime.felk.cvut.cz/linux-lin.git /
> > https://rtime.felk.cvut.cz/gitweb/linux-lin.git/shortlog. Up to now master
> > is working pretty well and I fixed a bug for slave node (patch 5). I'm
> > currently working on LIN slave task on slave node which was unimplemented
> > up to now.
> 
> first I want to express my happines that our work is
> getting tested/used. This allows to check if concept fits needs
> of more users and if it is confirmed we can start to speak
> about move to the mainline (linux/drivers/staging or net/can)

I think staging is currently the only acceptable place for sllin. But I don't know to handle 
> #define N_SLLIN			25
in that case. I guess even for staging, this line discipline number must be reserved.

> I have pushed next patches to our repository
> 
>   sllin: Replace pr_* with netdev_* where approriate
>   sllin: Make local functions static
>   sllin: slave: don't forget to wait for the checksum

Thanks for taking them.

> I would like to think and discuss little about the rest
> 
> > This first version uses the slave cache for message sending when requested,
> > but so it is no longer possible to configure the length for specific
> > message IDs which are only recweived. The LIN_CACHE_RESPONSE bit is used up
> > to now to configure the data length (DLC) of receptions in slave mode. This
> > doesn't work anymore when slave responses to configured IDs.
> > Are there any comments to distinguish between configured for sending and
> > configured for reception only? I tried to avoid addig a new bit for that,
> > but didn't find any idea for that.
> > I still would acknowledge comments on the other patches.
> 
> There was much more rich set of the flags in original code design
> 
>   /* Save configuration for particular LIN ID */
>   #define LIN_ID_CONF		(1 <<  LIN_CANFR_FLAGS_OFFS)
>   /* Publisher of particular LIN response is SLLIN Master */
>   #define LIN_SRC_MASTER		(1 << (LIN_CANFR_FLAGS_OFFS + 1))
>   #define LIN_SRC_SLAVE		(1 << (LIN_CANFR_FLAGS_OFFS + 2))
>   #define LIN_SLAVE_LOCAL		(1 << (LIN_CANFR_FLAGS_OFFS + 3))
>   #define LIN_SLAVE_REMOTE	(1 << (LIN_CANFR_FLAGS_OFFS + 4))
>   #define LIN_LOC_SLAVE_CACHE	(1 << (LIN_CANFR_FLAGS_OFFS + 5))
>   #define LIN_CHECKSUM_EXTENDED	(1 << (LIN_CANFR_FLAGS_OFFS + 6))
>  
>   #define LIN_ERR_RX_TIMEOUT	(1 << (LIN_CANFR_FLAGS_OFFS + 7))
>   #define LIN_ERR_CHECKSUM	(1 << (LIN_CANFR_FLAGS_OFFS + 8))
>   #define LIN_ERR_FRAMING		(1 << (LIN_CANFR_FLAGS_OFFS + 9))
> 
> but it was reduced to make life easier for users in one of the
> final project clenup commit
> 
>   sllin: Flags clean up
> 
> and it seems that the set was oversimplified. One option is to
> consider each cache location as setup when sl->linfr_cache[lin_id].dlc != 0
> or we need to reintroduce back concept of LIN_LOC_SLAVE_CACHE.
> I am not sure if zero length messages are disallowed by standard.
> But if they are then dlc!=0 is reasonable choice. My personal
> is many flags but I tend to too generic solutions usually as you
> can in original flags support.

I've reread LIN-Spec_2-2A.pdf section "2.3.1.4 Data" where the first sentence states:
> "A frame carries between one and eight bytes of data."
So a dlc of 0 is invalid. This is the reason I picked that as an indicator for known length or not.

> It has still one drawback if we decide to store received data
> for IDs with unknown length in the frame cache for later retrieval
> by local application. This can be resolved by re-addition of another
> flag.

Is storing in the cache for later retrieval actual necessary? Sure, you currently need a socket for LIN frame reception.

> Without the flag, I propose next chages
> 
> 
> diff --git a/sllin/sllin.c b/sllin/sllin.c
> index b97219e..12dce59 100644
> --- a/sllin/sllin.c
> +++ b/sllin/sllin.c
> @@ -549,7 +549,7 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
>                         spin_lock_irqsave(&sl->linfr_lock, flags);
> 
>                         /* Is the length of data set in frame cache? */
> -                       if (sce->frame_fl & LIN_CACHE_RESPONSE) {
> +                       if (sce->dlc > 0) {
>                                 sl->rx_expect += sce->dlc + 1; /* + checksum */
>                                 sl->rx_len_unknown = false;
>                         } else {

I also created this one.

> And appropriate change in the user space configuration utility
> 
> diff --git a/lin_config/src/sllin_config.c b/lin_config/src/sllin_config.c
> index dc704d8..757046b 100644
> --- a/lin_config/src/sllin_config.c
> +++ b/lin_config/src/sllin_config.c
> @@ -123,7 +123,6 @@ int sllin_cache_config(struct linc_lin_state *linc_lin_state,
>         }
> 
>         for (i = 0; i < 0x3F; i++) {
> -               if (linc_lin_state->frame_entry[i].status == 1) { /* Is active */
>                         frame.can_dlc = linc_lin_state->frame_entry[i].data_len;
>                         frame.can_id = i; /* LIN ID */
>                         frame.data[0] = linc_lin_state->frame_entry[i].data[0]; /* Data */
> @@ -134,13 +133,14 @@ int sllin_cache_config(struct linc_lin_state *linc_lin_state,
>                         frame.data[5] = linc_lin_state->frame_entry[i].data[5]; /* Data */
>                         frame.data[6] = linc_lin_state->frame_entry[i].data[6]; /* Data */
>                         frame.data[7] = linc_lin_state->frame_entry[i].data[7]; /* Data */
> -
> -                       frame.can_id |= LIN_CTRL_FRAME | LIN_CACHE_RESPONSE;
> +                       frame.can_id |= LIN_CTRL_FRAME;
> +               if (linc_lin_state->frame_entry[i].status == 1) { /* Is active */
> +                       frame.can_id |= LIN_CACHE_RESPONSE;
> +               }
>                         ret = write(s, &frame, sizeof(frame));
>                         printf("configuring frame cache; ret = %d\n", ret);
>                         //if (ret ...)
>                         //read_response(tty);
> -               }
>         }
> 
>         close(s);
> 
> The use of dlc == 0 allows still to disable entry and I propose clear unconfigured
> entries to do full reconfiguration when tool is called.
> 
> If you agree with above solution or propose to use new flag I prepare and commit
> the patch.

I don't use sllin_config, so I can't comment much on that. But from your comments it sound reasonable.

> To your additional patches, I need a little refresh of my information
> to review slave functionality and there is a must for communication
> with UART maintainers because without at least minor changes in FIFO
> control it cannot work reliably.

Yep, I'm aware of that. sllin in slave currently required an FIFO Rx (or even DMA) interrupt when a single character is received. Luckily the driver I'm currently working on (imx.c) does that out-of-the-box.

> Some comments
> 
>   sllin: reorder functions
>        generally I am not opposed to the change but there are things to consider
>           initial order tries keep lower level UART manipulation functions first
>           to allow future extending the code to part specific to UART/TTY and more
>           generic LIN code reusable even with LIN direct hardware support.
>           But I agree that things has got mixed already due to checking for some
>           LIN specific data in generic UART part and UART processing considerations
>           in LIN protocol specific code.

Putting function together which do some related stuff is fine. I reordered them to avoid additional declarations at the beginning, like many drivers do. But I'm fine with keeping the "old" order.

>   sllin: If length is unknown read until timer occurs or break is received
>        probably the best thing to do in general. The use of Rx FIFO timeouts
>        of some UARTS can be used there. It worked on PC for us but it is
>        not robust. I would like to analyze little more the code to ensure
>        that locking stay correct after your change.
> 
>   sllin: Add first support for LIN slave task on slave node
>       it is possible that we do not need worker thread for the slave at all.
>       The data can be prepared in UART callback processing. I believe
>       we have it working that way at least in one of code versions.
>       But slave was declared not to be implemented in the last version.
>       On the other hand, use of the worker thread is more polite to other
>       kernel services. There has been attempt for minimal locking between
>       UART specific part and the worker thread because UART callbacks
>       are invoked in many different even curious contexts according to
>       serial port HW and kernel - RT in IRQ threads, vanilla usually BH
>       but may be even hard IRQ for some arch, worker threads for USB devices
>       etc.

My patchset has enlarged meanwhile which fixes some problems I noticed or even adds more features.

> Anyway thanks for your contribution and I hope that we move project
> at least a little forward.

As I'm currently not working on sllin I think I'll send my current patchset (based on your new master). It is not cleaned up 8e.g. patches fixing things added in other patches), but it LIN slave worked pretty well with that. Mh, maybe I should get a github/gitorious account...
Oh, I started "creating" lindump (obviously copied from candump) for easier LIN frame reading. Not perfect and lots of candump leftovers, but fits my current needs and "works for me" :)

Best regards,
Alexander
-- 
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
Am Windrad 2
08468 Heinsdorfergrund
Tel.: 03765 38600-1156
Fax: 03765 38600-4100
Email: alexander.stein@systec-electronic.com
Website: www.systec-electronic.com
 
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Chemnitz, HRB 28082

[RFC v2] WIP-patches on sllin

[Alexander Stein]

This patchset is rebased on new master and adds some patches fixing bugs or
adding features.

Best regards,
Alexander

[PATCH 01/11] sllin: reorder functions

[Alexander Stein]

Needed for further commits

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 288 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 144 insertions(+), 144 deletions(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index b97219e..e8aca08 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -494,111 +494,6 @@ static void sllin_master_receive_buf(struct tty_struct *tty,
 }
 
 
-static void sllin_slave_receive_buf(struct tty_struct *tty,
-			      const unsigned char *cp, char *fp, int count)
-{
-	struct sllin *sl = (struct sllin *) tty->disc_data;
-	int lin_id;
-	struct sllin_conf_entry *sce;
-
-
-	/* Read the characters out of the buffer */
-	while (count--) {
-		if (fp && *fp++) {
-			netdev_dbg(sl->dev, "sllin_slave_receive_buf char 0x%02x ignored "
-				"due marker 0x%02x, flags 0x%lx\n",
-				*cp, *(fp-1), sl->flags);
-
-			/* Received Break */
-			sl->rx_cnt = 0;
-			sl->rx_expect = SLLIN_BUFF_ID + 1;
-			sl->rx_len_unknown = false; /* We do know exact length of the header */
-			sl->header_received = false;
-		}
-
-		if (sl->rx_cnt < SLLIN_BUFF_LEN) {
-			netdev_dbg(sl->dev, "LIN_RX[%d]: 0x%02x\n", sl->rx_cnt, *cp);
-
-			/* We did not receive break (0x00) character */
-			if ((sl->rx_cnt == SLLIN_BUFF_BREAK) && (*cp == 0x55)) {
-				sl->rx_buff[sl->rx_cnt++] = 0x00;
-			}
-
-			if (sl->rx_cnt == SLLIN_BUFF_SYNC) {
-				/* 'Duplicated' break character -- ignore */
-				if (*cp == 0x00) {
-					cp++;
-					continue;
-				}
-
-				/* Wrong sync character */
-				if (*cp != 0x55)
-					break;
-			}
-
-			sl->rx_buff[sl->rx_cnt++] = *cp++;
-		}
-
-		/* Header received */
-		if ((sl->header_received == false) && (sl->rx_cnt >= (SLLIN_BUFF_ID + 1))) {
-			unsigned long flags;
-
-			lin_id = sl->rx_buff[SLLIN_BUFF_ID] & LIN_ID_MASK;
-			sce = &sl->linfr_cache[lin_id];
-
-			spin_lock_irqsave(&sl->linfr_lock, flags);
-
-			/* Is the length of data set in frame cache? */
-			if (sce->frame_fl & LIN_CACHE_RESPONSE) {
-				sl->rx_expect += sce->dlc + 1; /* + checksum */
-				sl->rx_len_unknown = false;
-			} else {
-				sl->rx_expect += SLLIN_DATA_MAX + 1; /* + checksum */
-				sl->rx_len_unknown = true;
-			}
-			spin_unlock_irqrestore(&sl->linfr_lock, flags);
-
-			sl->header_received = true;
-
-			sll_send_rtr(sl);
-			continue;
-		}
-
-		/* Response received */
-		if ((sl->header_received == true) &&
-			((sl->rx_cnt >= sl->rx_expect) ||
-			((sl->rx_len_unknown == true) && (count == 0)))) {
-
-			sll_bump(sl);
-			netdev_dbg(sl->dev, "Received LIN header & LIN response. "
-					"rx_cnt = %u, rx_expect = %u\n", sl->rx_cnt,
-					sl->rx_expect);
-
-			/* Prepare for reception of new header */
-			sl->rx_cnt = 0;
-			sl->rx_expect = SLLIN_BUFF_ID + 1;
-			sl->rx_len_unknown = false; /* We do know exact length of the header */
-			sl->header_received = false;
-		}
-	}
-}
-
-static void sllin_receive_buf(struct tty_struct *tty,
-			      const unsigned char *cp, char *fp, int count)
-{
-	struct sllin *sl = (struct sllin *) tty->disc_data;
-	netdev_dbg(sl->dev, "sllin_receive_buf invoked, count = %u\n", count);
-
-	if (!sl || sl->magic != SLLIN_MAGIC || !netif_running(sl->dev))
-		return;
-
-	if (sl->lin_master)
-		sllin_master_receive_buf(tty, cp, fp, count);
-	else
-		sllin_slave_receive_buf(tty, cp, fp, count);
-
-}
-
 /*****************************************
  *  sllin message helper routines
  *****************************************/
@@ -736,6 +631,150 @@ static void sllin_reset_buffs(struct sllin *sl)
 	sl->data_to_send = false;
 }
 
+/**
+ * sllin_rx_validate() -- Validate received frame, i,e. check checksum
+ *
+ * @sl:
+ */
+static int sllin_rx_validate(struct sllin *sl)
+{
+	unsigned long flags;
+	int actual_id;
+	int ext_chcks_fl;
+	int lin_dlc;
+	unsigned char rec_chcksm = sl->rx_buff[sl->rx_cnt - 1];
+	struct sllin_conf_entry *sce;
+
+	actual_id = sl->rx_buff[SLLIN_BUFF_ID] & LIN_ID_MASK;
+	sce = &sl->linfr_cache[actual_id];
+
+	spin_lock_irqsave(&sl->linfr_lock, flags);
+	lin_dlc = sce->dlc;
+	ext_chcks_fl = sce->frame_fl & LIN_CHECKSUM_EXTENDED;
+	spin_unlock_irqrestore(&sl->linfr_lock, flags);
+
+	if (sllin_checksum(sl->rx_buff, sl->rx_cnt - 1, ext_chcks_fl) !=
+		rec_chcksm) {
+
+		/* Type of checksum is configured for particular frame */
+		if (lin_dlc > 0) {
+			return -1;
+		} else {
+			if (sllin_checksum(sl->rx_buff,	sl->rx_cnt - 1,
+				!ext_chcks_fl) != rec_chcksm) {
+				return -1;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static void sllin_slave_receive_buf(struct tty_struct *tty,
+			      const unsigned char *cp, char *fp, int count)
+{
+	struct sllin *sl = (struct sllin *) tty->disc_data;
+	int lin_id;
+	struct sllin_conf_entry *sce;
+
+
+	/* Read the characters out of the buffer */
+	while (count--) {
+		if (fp && *fp++) {
+			netdev_dbg(sl->dev, "sllin_slave_receive_buf char 0x%02x ignored "
+				"due marker 0x%02x, flags 0x%lx\n",
+				*cp, *(fp-1), sl->flags);
+
+			/* Received Break */
+			sl->rx_cnt = 0;
+			sl->rx_expect = SLLIN_BUFF_ID + 1;
+			sl->rx_len_unknown = false; /* We do know exact length of the header */
+			sl->header_received = false;
+		}
+
+		if (sl->rx_cnt < SLLIN_BUFF_LEN) {
+			netdev_dbg(sl->dev, "LIN_RX[%d]: 0x%02x\n", sl->rx_cnt, *cp);
+
+			/* We did not receive break (0x00) character */
+			if ((sl->rx_cnt == SLLIN_BUFF_BREAK) && (*cp == 0x55)) {
+				sl->rx_buff[sl->rx_cnt++] = 0x00;
+			}
+
+			if (sl->rx_cnt == SLLIN_BUFF_SYNC) {
+				/* 'Duplicated' break character -- ignore */
+				if (*cp == 0x00) {
+					cp++;
+					continue;
+				}
+
+				/* Wrong sync character */
+				if (*cp != 0x55)
+					break;
+			}
+
+			sl->rx_buff[sl->rx_cnt++] = *cp++;
+		}
+
+		/* Header received */
+		if ((sl->header_received == false) && (sl->rx_cnt >= (SLLIN_BUFF_ID + 1))) {
+			unsigned long flags;
+
+			lin_id = sl->rx_buff[SLLIN_BUFF_ID] & LIN_ID_MASK;
+			sce = &sl->linfr_cache[lin_id];
+
+			spin_lock_irqsave(&sl->linfr_lock, flags);
+
+			/* Is the length of data set in frame cache? */
+			if (sce->frame_fl & LIN_CACHE_RESPONSE) {
+				sl->rx_expect += sce->dlc + 1; /* + checksum */
+				sl->rx_len_unknown = false;
+			} else {
+				sl->rx_expect += SLLIN_DATA_MAX + 1; /* + checksum */
+				sl->rx_len_unknown = true;
+			}
+			spin_unlock_irqrestore(&sl->linfr_lock, flags);
+
+			sl->header_received = true;
+
+			sll_send_rtr(sl);
+			continue;
+		}
+
+		/* Response received */
+		if ((sl->header_received == true) &&
+			((sl->rx_cnt >= sl->rx_expect) ||
+			((sl->rx_len_unknown == true) && (count == 0)))) {
+
+			sll_bump(sl);
+			netdev_dbg(sl->dev, "Received LIN header & LIN response. "
+					"rx_cnt = %u, rx_expect = %u\n", sl->rx_cnt,
+					sl->rx_expect);
+
+			/* Prepare for reception of new header */
+			sl->rx_cnt = 0;
+			sl->rx_expect = SLLIN_BUFF_ID + 1;
+			sl->rx_len_unknown = false; /* We do know exact length of the header */
+			sl->header_received = false;
+		}
+	}
+}
+
+static void sllin_receive_buf(struct tty_struct *tty,
+			      const unsigned char *cp, char *fp, int count)
+{
+	struct sllin *sl = (struct sllin *) tty->disc_data;
+	netdev_dbg(sl->dev, "sllin_receive_buf invoked, count = %u\n", count);
+
+	if (!sl || sl->magic != SLLIN_MAGIC || !netif_running(sl->dev))
+		return;
+
+	if (sl->lin_master)
+		sllin_master_receive_buf(tty, cp, fp, count);
+	else
+		sllin_slave_receive_buf(tty, cp, fp, count);
+
+}
+
 static int sllin_send_tx_buff(struct sllin *sl)
 {
 	struct tty_struct *tty = sl->tty;
@@ -873,45 +912,6 @@ static enum hrtimer_restart sllin_rx_timeout_handler(struct hrtimer *hrtimer)
 	return HRTIMER_NORESTART;
 }
 
-/**
- * sllin_rx_validate() -- Validate received frame, i,e. check checksum
- *
- * @sl:
- */
-static int sllin_rx_validate(struct sllin *sl)
-{
-	unsigned long flags;
-	int actual_id;
-	int ext_chcks_fl;
-	int lin_dlc;
-	unsigned char rec_chcksm = sl->rx_buff[sl->rx_cnt - 1];
-	struct sllin_conf_entry *sce;
-
-	actual_id = sl->rx_buff[SLLIN_BUFF_ID] & LIN_ID_MASK;
-	sce = &sl->linfr_cache[actual_id];
-
-	spin_lock_irqsave(&sl->linfr_lock, flags);
-	lin_dlc = sce->dlc;
-	ext_chcks_fl = sce->frame_fl & LIN_CHECKSUM_EXTENDED;
-	spin_unlock_irqrestore(&sl->linfr_lock, flags);
-
-	if (sllin_checksum(sl->rx_buff, sl->rx_cnt - 1, ext_chcks_fl) !=
-		rec_chcksm) {
-
-		/* Type of checksum is configured for particular frame */
-		if (lin_dlc > 0) {
-			return -1;
-		} else {
-			if (sllin_checksum(sl->rx_buff,	sl->rx_cnt - 1,
-				!ext_chcks_fl) != rec_chcksm) {
-				return -1;
-			}
-		}
-	}
-
-	return 0;
-}
-
 /*****************************************
  *  sllin_kwthread - kernel worker thread
  *****************************************/
-- 
1.8.3.2

[PATCH 02/11] sllin: If length is unknown read until timer occurs or break is received

[Alexander Stein]

If the length is unknown stop waiting for new data until the tty rx queue
is empty once is not working on fast systems where sllin_receive_buf is
called for each byte received. Instead wait up to 8 bytes, when the
timeout occurs or when a new break is received.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 54 ++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 10 deletions(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index e8aca08..36f23a5 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -670,6 +670,23 @@ static int sllin_rx_validate(struct sllin *sl)
 	return 0;
 }
 
+static void sllin_slave_finish_rx_msg(struct sllin *sl)
+{
+	if (sllin_rx_validate(sl) == -1) {
+		netdev_dbg(sl->dev, "sllin: RX validation failed.\n");
+		sllin_report_error(sl, LIN_ERR_CHECKSUM);
+	} else {
+		/* Send CAN non-RTR frame with data */
+		netdev_dbg(sl->dev, "sllin: sending NON-RTR CAN frame with LIN payload.");
+		sll_bump(sl); /* send packet to the network layer */
+	}
+	/* Prepare for reception of new header */
+	sl->rx_cnt = 0;
+	sl->rx_expect = SLLIN_BUFF_ID + 1;
+	sl->rx_len_unknown = false; /* We do know exact length of the header */
+	sl->header_received = false;
+}
+
 static void sllin_slave_receive_buf(struct tty_struct *tty,
 			      const unsigned char *cp, char *fp, int count)
 {
@@ -681,6 +698,20 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 	/* Read the characters out of the buffer */
 	while (count--) {
 		if (fp && *fp++) {
+			/*
+			 * If we don't know the length of the current message
+			 * we received the break of the next message.
+			 * Evaluate the previous one before continuing
+			 */
+			if (sl->rx_len_unknown == true)
+			{
+				hrtimer_cancel(&sl->rx_timer);
+				sllin_slave_finish_rx_msg(sl);
+
+				set_bit(SLF_RXEVENT, &sl->flags);
+				wake_up(&sl->kwt_wq);
+			}
+
 			netdev_dbg(sl->dev, "sllin_slave_receive_buf char 0x%02x ignored "
 				"due marker 0x%02x, flags 0x%lx\n",
 				*cp, *(fp-1), sl->flags);
@@ -736,25 +767,23 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 
 			sl->header_received = true;
 
+			hrtimer_start(&sl->rx_timer,
+				ktime_add(ktime_get(), sl->rx_timer_timeout),
+				HRTIMER_MODE_ABS);
 			sll_send_rtr(sl);
 			continue;
 		}
 
 		/* Response received */
 		if ((sl->header_received == true) &&
-			((sl->rx_cnt >= sl->rx_expect) ||
-			((sl->rx_len_unknown == true) && (count == 0)))) {
+			((sl->rx_cnt >= sl->rx_expect))) {
 
-			sll_bump(sl);
+			hrtimer_cancel(&sl->rx_timer);
 			netdev_dbg(sl->dev, "Received LIN header & LIN response. "
 					"rx_cnt = %u, rx_expect = %u\n", sl->rx_cnt,
 					sl->rx_expect);
+			sllin_slave_finish_rx_msg(sl);
 
-			/* Prepare for reception of new header */
-			sl->rx_cnt = 0;
-			sl->rx_expect = SLLIN_BUFF_ID + 1;
-			sl->rx_len_unknown = false; /* We do know exact length of the header */
-			sl->header_received = false;
 		}
 	}
 }
@@ -905,8 +934,13 @@ static enum hrtimer_restart sllin_rx_timeout_handler(struct hrtimer *hrtimer)
 {
 	struct sllin *sl = container_of(hrtimer, struct sllin, rx_timer);
 
-	sllin_report_error(sl, LIN_ERR_RX_TIMEOUT);
-	set_bit(SLF_TMOUTEVENT, &sl->flags);
+	if (sl->lin_master) {
+		sllin_report_error(sl, LIN_ERR_RX_TIMEOUT);
+		set_bit(SLF_TMOUTEVENT, &sl->flags);
+	} else {
+		sllin_slave_finish_rx_msg(sl);
+		set_bit(SLF_RXEVENT, &sl->flags);
+	}
 	wake_up(&sl->kwt_wq);
 
 	return HRTIMER_NORESTART;
-- 
1.8.3.2

[PATCH 03/11] sllin: Add first support for LIN slave task on slave node

[Alexander Stein]

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 3 deletions(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index 36f23a5..3e020b1 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -113,6 +113,7 @@ enum slstate {
 	SLSTATE_RESPONSE_WAIT_BUS, /* Wait for response from LIN bus
 				only (CAN frames from network stack
 				are not processed in this moment) */
+	SLSTATE_ID_RECEIVED,
 	SLSTATE_RESPONSE_SENT,
 };
 
@@ -755,10 +756,13 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 
 			spin_lock_irqsave(&sl->linfr_lock, flags);
 
+			sl->lin_state = SLSTATE_ID_RECEIVED;
 			/* Is the length of data set in frame cache? */
 			if (sce->frame_fl & LIN_CACHE_RESPONSE) {
 				sl->rx_expect += sce->dlc + 1; /* + checksum */
 				sl->rx_len_unknown = false;
+				set_bit(SLF_MSGEVENT, &sl->flags);
+				wake_up(&sl->kwt_wq);
 			} else {
 				sl->rx_expect += SLLIN_DATA_MAX + 1; /* + checksum */
 				sl->rx_len_unknown = true;
@@ -784,6 +788,8 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 					sl->rx_expect);
 			sllin_slave_finish_rx_msg(sl);
 
+			set_bit(SLF_RXEVENT, &sl->flags);
+			wake_up(&sl->kwt_wq);
 		}
 	}
 }
@@ -956,7 +962,9 @@ static int sllin_kwthread(void *ptr)
 	struct tty_struct *tty = sl->tty;
 	struct sched_param schparam = { .sched_priority = 40 };
 	int tx_bytes = 0; /* Used for Network statistics */
-
+	unsigned long flags;
+	int lin_id;
+	struct sllin_conf_entry *sce;
 
 	netdev_dbg(sl->dev, "sllin_kwthread started.\n");
 	sched_setscheduler(current, SCHED_FIFO, &schparam);
@@ -984,7 +992,8 @@ static int sllin_kwthread(void *ptr)
 			test_bit(SLF_TMOUTEVENT, &sl->flags) ||
 			test_bit(SLF_ERROR, &sl->flags) ||
 			(((sl->lin_state == SLSTATE_IDLE) ||
-				(sl->lin_state == SLSTATE_RESPONSE_WAIT))
+				(sl->lin_state == SLSTATE_RESPONSE_WAIT) ||
+				(sl->lin_state == SLSTATE_ID_RECEIVED))
 				&& test_bit(SLF_MSGEVENT, &sl->flags)));
 
 		if (test_and_clear_bit(SLF_RXEVENT, &sl->flags)) {
@@ -1028,7 +1037,6 @@ static int sllin_kwthread(void *ptr)
 
 			/* SFF RTR CAN frame -> LIN header */
 			if (cf->can_id & CAN_RTR_FLAG) {
-				unsigned long flags;
 				struct sllin_conf_entry *sce;
 
 				netdev_dbg(sl->dev, "%s: RTR SFF CAN frame, ID = %x\n",
@@ -1177,6 +1185,56 @@ slstate_response_wait:
 			sl->lin_state = SLSTATE_IDLE;
 			break;
 
+		case SLSTATE_ID_RECEIVED:
+			lin_id = sl->rx_buff[SLLIN_BUFF_ID] & LIN_ID_MASK;
+			sce = &sl->linfr_cache[lin_id];
+			spin_lock_irqsave(&sl->linfr_lock, flags);
+
+			if ((sce->frame_fl & LIN_CACHE_RESPONSE)
+					&& (sce->dlc > 0)
+					&& (test_bit(SLF_MSGEVENT, &sl->flags))) {
+				int mode;
+
+				netdev_dbg(sl->dev, "Sending LIN response from linfr_cache\n");
+
+				lin_data = sce->data;
+				lin_dlc = sce->dlc;
+				if (lin_dlc > SLLIN_DATA_MAX)
+					lin_dlc = SLLIN_DATA_MAX;
+				memcpy(lin_data_buff, lin_data, lin_dlc);
+				lin_data = lin_data_buff;
+				tx_bytes = lin_dlc;
+
+				mode = SLLIN_STPMSG_RESPONLY;
+				if (sl->rx_buff[SLLIN_BUFF_ID] & LIN_CHECKSUM_EXTENDED)
+					mode |= SLLIN_STPMSG_CHCKSUM_ENH;
+
+				if (sllin_setup_msg(sl, mode, lin_id & LIN_ID_MASK,
+					lin_data, lin_dlc) != -1) {
+
+					sl->rx_expect = sl->tx_lim;
+					sl->data_to_send = true;
+					sl->dev->stats.tx_packets++;
+					sl->dev->stats.tx_bytes += tx_bytes;
+					sl->resp_len_known = true;
+
+					if (!sl->lin_master) {
+						sl->tx_cnt = SLLIN_BUFF_DATA;
+					}
+					sllin_send_tx_buff(sl);
+				}
+
+				clear_bit(SLF_MSGEVENT, &sl->flags);
+				kfree_skb(sl->tx_req_skb);
+				netif_wake_queue(sl->dev);
+				hrtimer_start(&sl->rx_timer,
+					ktime_add(ktime_get(), sl->rx_timer_timeout),
+					HRTIMER_MODE_ABS);
+			}
+			spin_unlock_irqrestore(&sl->linfr_lock, flags);
+			sl->lin_state = SLSTATE_IDLE;
+			break;
+
 		case SLSTATE_RESPONSE_SENT:
 slstate_response_sent:
 			if (sl->rx_cnt < sl->tx_lim)
-- 
1.8.3.2

[PATCH 04/11] sllin: Add version dependent access to termios in tty_struct

[Alexander Stein]

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index 3e020b1..e05564c 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -62,6 +62,7 @@
 #include <linux/can.h>
 #include <linux/kthread.h>
 #include <linux/hrtimer.h>
+#include <linux/version.h>
 #include "linux/lin_bus.h"
 
 /* Should be in include/linux/tty.h */
@@ -206,22 +207,31 @@ const unsigned char sllin_id_parity_table[] = {
  */
 static int sltty_change_speed(struct tty_struct *tty, unsigned speed)
 {
-	struct ktermios old_termios;
+	struct ktermios old_termios, termios;
 	int cflag;
 
 	mutex_lock(&tty->termios_mutex);
 
-	old_termios = *(tty->termios);
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 7, 0)
+	old_termios = termios = *(tty->termios);
+#else
+	old_termios = termios = tty->termios;
+#endif
 
 	cflag = CS8 | CREAD | CLOCAL | HUPCL;
 	cflag &= ~(CBAUD | CIBAUD);
 	cflag |= BOTHER;
-	tty->termios->c_cflag = cflag;
-	tty->termios->c_oflag = 0;
-	tty->termios->c_lflag = 0;
+	termios.c_cflag = cflag;
+	termios.c_oflag = 0;
+	termios.c_lflag = 0;
 
 	/* Enable interrupt when UART-Break or Framing error received */
-	tty->termios->c_iflag = BRKINT | INPCK;
+	termios.c_iflag = BRKINT | INPCK;
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 7, 0)
+	*(tty->termios) = termios;
+#else
+	tty->termios = termios;
+#endif
 
 	tty_encode_baud_rate(tty, speed, speed);
 
-- 
1.8.3.2

[PATCH 05/11] sllin: Fix checksum mode decision

[Alexander Stein]

rx_buff[SLLIN_BUFF_ID] only holds LIN ID + parity bits, but never internal
bits like LIN_CHECKSUM_EXTENDED.
Use correct value from internal cache entry.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index e05564c..487b72a 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -1216,7 +1216,7 @@ slstate_response_wait:
 				tx_bytes = lin_dlc;
 
 				mode = SLLIN_STPMSG_RESPONLY;
-				if (sl->rx_buff[SLLIN_BUFF_ID] & LIN_CHECKSUM_EXTENDED)
+				if (sce->frame_fl & LIN_CHECKSUM_EXTENDED)
 					mode |= SLLIN_STPMSG_CHCKSUM_ENH;
 
 				if (sllin_setup_msg(sl, mode, lin_id & LIN_ID_MASK,
-- 
1.8.3.2

[PATCH 06/11] sllin: dlc in cache > 0 is the expected LIN frame length

[Alexander Stein]

A cache entry should not be set to LIN_CACHE_RESPONSE if there is nothing
to reply from this node. A dlc set > 0 configures the expected length.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index 487b72a..c5e1749 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -768,7 +768,7 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 
 			sl->lin_state = SLSTATE_ID_RECEIVED;
 			/* Is the length of data set in frame cache? */
-			if (sce->frame_fl & LIN_CACHE_RESPONSE) {
+			if (sce->dlc > 0) {
 				sl->rx_expect += sce->dlc + 1; /* + checksum */
 				sl->rx_len_unknown = false;
 				set_bit(SLF_MSGEVENT, &sl->flags);
-- 
1.8.3.2

[PATCH 07/11] sllin: Do not set SLF_MSGEVENT flag

[Alexander Stein]

This is only used if there is some Tx CAN frame.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index c5e1749..457788c 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -771,7 +771,6 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 			if (sce->dlc > 0) {
 				sl->rx_expect += sce->dlc + 1; /* + checksum */
 				sl->rx_len_unknown = false;
-				set_bit(SLF_MSGEVENT, &sl->flags);
 				wake_up(&sl->kwt_wq);
 			} else {
 				sl->rx_expect += SLLIN_DATA_MAX + 1; /* + checksum */
@@ -1001,9 +1000,9 @@ static int sllin_kwthread(void *ptr)
 			test_bit(SLF_TXEVENT, &sl->flags) ||
 			test_bit(SLF_TMOUTEVENT, &sl->flags) ||
 			test_bit(SLF_ERROR, &sl->flags) ||
+			(sl->lin_state == SLSTATE_ID_RECEIVED) ||
 			(((sl->lin_state == SLSTATE_IDLE) ||
-				(sl->lin_state == SLSTATE_RESPONSE_WAIT) ||
-				(sl->lin_state == SLSTATE_ID_RECEIVED))
+				(sl->lin_state == SLSTATE_RESPONSE_WAIT))
 				&& test_bit(SLF_MSGEVENT, &sl->flags)));
 
 		if (test_and_clear_bit(SLF_RXEVENT, &sl->flags)) {
@@ -1201,8 +1200,7 @@ slstate_response_wait:
 			spin_lock_irqsave(&sl->linfr_lock, flags);
 
 			if ((sce->frame_fl & LIN_CACHE_RESPONSE)
-					&& (sce->dlc > 0)
-					&& (test_bit(SLF_MSGEVENT, &sl->flags))) {
+					&& (sce->dlc > 0)) {
 				int mode;
 
 				netdev_dbg(sl->dev, "Sending LIN response from linfr_cache\n");
@@ -1234,7 +1232,6 @@ slstate_response_wait:
 					sllin_send_tx_buff(sl);
 				}
 
-				clear_bit(SLF_MSGEVENT, &sl->flags);
 				kfree_skb(sl->tx_req_skb);
 				netif_wake_queue(sl->dev);
 				hrtimer_start(&sl->rx_timer,
-- 
1.8.3.2

[PATCH 08/11] sllin: signal timeout if there are less than configured characters

[Alexander Stein]

We will hit the timeout if there is a larger length configured than
actually received or none at all. Signal this to userspace.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index 457788c..ffbdf01 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -949,7 +949,16 @@ static enum hrtimer_restart sllin_rx_timeout_handler(struct hrtimer *hrtimer)
 {
 	struct sllin *sl = container_of(hrtimer, struct sllin, rx_timer);
 
-	if (sl->lin_master) {
+	/*
+	 * Signal timeout when:
+	 * master: We did not receive as much characters as expected
+	 * slave: * we did not receive any data bytes at all
+	 *        * we know the length and didn't receive enough
+	 */
+	if ((sl->lin_master) ||
+			(sl->rx_cnt <= SLLIN_BUFF_DATA) ||
+			((!sl->rx_len_unknown) &&
+			(sl->rx_cnt < sl->rx_expect))) {
 		sllin_report_error(sl, LIN_ERR_RX_TIMEOUT);
 		set_bit(SLF_TMOUTEVENT, &sl->flags);
 	} else {
-- 
1.8.3.2

[PATCH 09/11] sllin: Don't fiddle with netdev queue and skb in slave receive mode

[Alexander Stein]

The queue and stored skb is only used for master mode.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index ffbdf01..c922b84 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -1241,8 +1241,6 @@ slstate_response_wait:
 					sllin_send_tx_buff(sl);
 				}
 
-				kfree_skb(sl->tx_req_skb);
-				netif_wake_queue(sl->dev);
 				hrtimer_start(&sl->rx_timer,
 					ktime_add(ktime_get(), sl->rx_timer_timeout),
 					HRTIMER_MODE_ABS);
-- 
1.8.3.2

[PATCH 10/11] sllin: Add LIN ID to timeout frame

[Alexander Stein]

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index c922b84..61c8134 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -517,6 +517,9 @@ static void sllin_master_receive_buf(struct tty_struct *tty,
  */
 static void sllin_report_error(struct sllin *sl, int err)
 {
+	unsigned char *lin_buff;
+	int lin_id;
+
 	switch (err) {
 	case LIN_ERR_CHECKSUM:
 		sl->dev->stats.rx_crc_errors++;
@@ -531,7 +534,9 @@ static void sllin_report_error(struct sllin *sl, int err)
 		break;
 	}
 
-	sllin_send_canfr(sl, 0 | CAN_EFF_FLAG |
+	lin_buff = (sl->lin_master) ? sl->tx_buff : sl->rx_buff;
+	lin_id = lin_buff[SLLIN_BUFF_ID] & LIN_ID_MASK;
+	sllin_send_canfr(sl, lin_id | CAN_EFF_FLAG |
 		(err & ~LIN_ID_MASK), NULL, 0);
 }
 
-- 
1.8.3.2

[PATCH 11/11] sllin: Evaluate message upon break only if received at least a LIN ID

[Alexander Stein]

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 sllin/sllin.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/sllin/sllin.c b/sllin/sllin.c
index 61c8134..8d7ad69 100644
--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -716,10 +716,12 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 		if (fp && *fp++) {
 			/*
 			 * If we don't know the length of the current message
-			 * we received the break of the next message.
-			 * Evaluate the previous one before continuing
+			 * and received at least the LIN ID, we received here
+			 * the break of the next message.
+			 * Evaluate the previous one before continuing.
 			 */
-			if (sl->rx_len_unknown == true)
+			if ((sl->rx_len_unknown == true) &&
+				(sl->rx_cnt >= SLLIN_BUFF_ID))
 			{
 				hrtimer_cancel(&sl->rx_timer);
 				sllin_slave_finish_rx_msg(sl);
-- 
1.8.3.2