ac100 team mailing list archive

[Marc Dietrich <marvin24@xxxxxx>]

This is a rather large patch. The older code wasn't stable enough
in all cases as it confused the EC during high system load. The new
code uses a better state machine which is quiet similar to the
original code from NVIDIA.

In short, this introduces:
	- new ISR state machine for protocol handling
	- a ring buffer for received messages
	  (this avoids the use of malloc in the ISR)
	- the list operations are now protected by spinlocks to
	  handle concurrent access in the bottom half
	- a "nvec" workqueue. The EC is very sensitive regarding
	  correct timing. In the past, system events often caused
	  slow responses resulting in a protocol timeout. The new code
	  avoids this by using its own workqueue.

Things to do:

* the ring buffer code is not finished yet, e.g. the received link
list is just a pointer to the ring buffer array, while it should
be a list of its own.

* error handling is also not finished yet, e.g. during ring buffer
overrun the ec needs to be reseted.

* the locking needs a review.

* the sync_write should return an error code instead of the received
message.

* other things I've forgotten now

Signed-off-by: Marc Dietrich <marvin24@xxxxxx>
---
 drivers/staging/nvec/nvec.c     |  332 ++++++++++++++++++++++++++-------------
 drivers/staging/nvec/nvec.h     |   56 ++++---
 drivers/staging/nvec/nvec_kbd.c |    2 +-
 3 files changed, 255 insertions(+), 135 deletions(-)

diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index aed751a..0ccc2b0 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -93,29 +93,73 @@ static int nvec_status_notifier(struct notifier_block *nb,
 
 void nvec_write_async(struct nvec_chip *nvec, unsigned char *data, short size)
 {
-	struct nvec_msg *msg = kzalloc(sizeof(struct nvec_msg), GFP_NOWAIT);
+	struct nvec_msg *msg;
+	unsigned long flags;
 
-	msg->data = kzalloc(size, GFP_NOWAIT);
+	msg = kzalloc(sizeof(struct nvec_msg), GFP_ATOMIC);
 	msg->data[0] = size;
 	memcpy(msg->data + 1, data, size);
 	msg->size = size + 1;
-	msg->pos = 0;
-	INIT_LIST_HEAD(&msg->node);
 
+	spin_lock_irqsave(&nvec->tx_lock, flags);
 	list_add_tail(&msg->node, &nvec->tx_data);
+	spin_unlock_irqrestore(&nvec->tx_lock, flags);
 
-	gpio_set_value(nvec->gpio, 0);
+	queue_work(nvec->wq, &nvec->tx_work);
 }
-
 EXPORT_SYMBOL(nvec_write_async);
 
+struct nvec_msg *nvec_write_sync(struct nvec_chip *nvec,
+		unsigned char *data, short size)
+{
+	mutex_lock(&nvec->sync_write_mutex);
+
+	nvec->sync_write_pending = (data[1] << 8) + data[0];
+	nvec_write_async(nvec, data, size);
+
+	dev_dbg(nvec->dev, "nvec_sync_write: 0x%04x\n",
+					nvec->sync_write_pending);
+	if (!(wait_for_completion_timeout(&nvec->sync_write,
+				msecs_to_jiffies(2000)))) {
+		dev_warn(nvec->dev, "timeout waiting for sync write to complete\n");
+		mutex_unlock(&nvec->sync_write_mutex);
+		return NULL;
+	}
+
+	dev_dbg(nvec->dev, "nvec_sync_write: pong!\n");
+
+	mutex_unlock(&nvec->sync_write_mutex);
+
+	return nvec->last_sync_msg;
+}
+EXPORT_SYMBOL(nvec_write_sync);
+
+/* TX worker */
 static void nvec_request_master(struct work_struct *work)
 {
 	struct nvec_chip *nvec = container_of(work, struct nvec_chip, tx_work);
+	unsigned long flags;
+	struct nvec_msg *msg;
 
-	if (!list_empty(&nvec->tx_data)) {
+	mutex_lock(&nvec->async_write_mutex);
+	spin_lock_irqsave(&nvec->tx_lock, flags);
+	while (!list_empty(&nvec->tx_data)) {
+		msg = list_first_entry(&nvec->tx_data, struct nvec_msg, node);
+		spin_unlock_irqrestore(&nvec->tx_lock, flags);
 		gpio_set_value(nvec->gpio, 0);
+		if (!(wait_for_completion_interruptible_timeout(
+		      &nvec->ec_transfer, msecs_to_jiffies(5000)))) {
+			dev_warn(nvec->dev, "timeout waiting for ec transfer\n");
+			gpio_set_value(nvec->gpio, 1);
+			msg->pos = 0;
+		} else {
+			list_del_init(&msg->node);
+			kfree(msg);
+		}
+		spin_lock_irqsave(&nvec->tx_lock, flags);
 	}
+	spin_unlock_irqrestore(&nvec->tx_lock, flags);
+	mutex_unlock(&nvec->async_write_mutex);
 }
 
 static int parse_msg(struct nvec_chip *nvec, struct nvec_msg *msg)
@@ -141,144 +185,209 @@ static int parse_msg(struct nvec_chip *nvec, struct nvec_msg *msg)
 	return 0;
 }
 
-static struct nvec_msg *nvec_write_sync(struct nvec_chip *nvec,
-					unsigned char *data, short size)
-{
-	down(&nvec->sync_write_mutex);
-
-	nvec->sync_write_pending = (data[1] << 8) + data[0];
-	nvec_write_async(nvec, data, size);
-
-	dev_dbg(nvec->dev, "nvec_sync_write: 0x%04x\n",
-		nvec->sync_write_pending);
-	wait_for_completion(&nvec->sync_write);
-	dev_dbg(nvec->dev, "nvec_sync_write: pong!\n");
-
-	up(&nvec->sync_write_mutex);
-
-	return nvec->last_sync_msg;
-}
-
 /* RX worker */
 static void nvec_dispatch(struct work_struct *work)
 {
 	struct nvec_chip *nvec = container_of(work, struct nvec_chip, rx_work);
+	unsigned long flags;
 	struct nvec_msg *msg;
 
+	mutex_lock(&nvec->dispatch_mutex);
+	spin_lock_irqsave(&nvec->rx_lock, flags);
 	while (!list_empty(&nvec->rx_data)) {
 		msg = list_first_entry(&nvec->rx_data, struct nvec_msg, node);
 		list_del_init(&msg->node);
+		spin_unlock_irqrestore(&nvec->rx_lock, flags);
 
-		if (nvec->sync_write_pending ==
-		    (msg->data[2] << 8) + msg->data[0]) {
+		if (nvec->sync_write_pending == 
+		      (msg->data[2] << 8) + msg->data[0]) {
 			dev_dbg(nvec->dev, "sync write completed!\n");
 			nvec->sync_write_pending = 0;
 			nvec->last_sync_msg = msg;
 			complete(&nvec->sync_write);
 		} else {
 			parse_msg(nvec, msg);
-			if ((!msg) || (!msg->data))
-				dev_warn(nvec->dev,
-					 "attempt access zero pointer");
-			else {
-				kfree(msg->data);
-				kfree(msg);
-			}
+			msg->used = 0;
 		}
+		spin_lock_irqsave(&nvec->rx_lock, flags);
 	}
+	spin_unlock_irqrestore(&nvec->rx_lock, flags);
+	mutex_unlock(&nvec->dispatch_mutex);
 }
 
 static irqreturn_t nvec_interrupt(int irq, void *dev)
 {
 	unsigned long status;
-	unsigned long received;
-	unsigned char to_send;
-	struct nvec_msg *msg;
+	unsigned long received = 0;
+	unsigned char to_send = 0;
+	unsigned long irq_mask = I2C_SL_IRQ | END_TRANS | RCVD | RNW;
+	unsigned long flags;
+	unsigned long dtime;
+	int valid_proto = 0;
+	int end_trans = 0;
+	struct timespec start_time, end_time, diff_time;
 	struct nvec_chip *nvec = (struct nvec_chip *)dev;
 	void __iomem *base = nvec->base;
 
+	getnstimeofday(&start_time);
+
 	status = readl(base + I2C_SL_STATUS);
 
-	if (!(status & I2C_SL_IRQ)) {
-		dev_warn(nvec->dev, "nvec Spurious IRQ\n");
+	if (!(status & irq_mask) && !((status & ~irq_mask) == 0)) {
+		dev_warn(nvec->dev, "unexpected irq mask %lx\n", status);
 		goto handled;
 	}
-	if (status & END_TRANS && !(status & RCVD)) {
-		nvec->state = NVEC_WAIT;
-		if (nvec->rx->size > 1) {
-			list_add_tail(&nvec->rx->node, &nvec->rx_data);
-			schedule_work(&nvec->rx_work);
-		} else {
-			kfree(nvec->rx->data);
-			kfree(nvec->rx);
-		}
-		return IRQ_HANDLED;
-	} else if (status & RNW) {
-		if (status & RCVD)
-			udelay(3);
 
-		if (status & RCVD) {
-			nvec->state = NVEC_WRITE;
-		} else {
-/*                      dev_dbg(nvec->dev, "Read comm cont !\n"); */
-		}
+	if ((status & I2C_SL_IRQ) == 0) {
+		dev_warn(nvec->dev, "Spurious IRQ\n");
+		goto handled;
+	}
+
+	/* just ET (but not ET with new comm [0x1c] !) */
+	if ((status & END_TRANS) && !(status & RCVD))
+		goto handled;
+
+	if (status & RNW) { /* ec reads from slave, status = 0x0a, 0x0e */
+		spin_lock_irqsave(&nvec->tx_lock, flags);
 		if (list_empty(&nvec->tx_data)) {
-			dev_err(nvec->dev, "nvec empty tx - sending no-op\n");
-			to_send = 0x8a;
-			nvec_write_async(nvec, "\x07\x02", 2);
+			dev_err(nvec->dev, "empty tx - sending no-op\n");
+
+			nvec->tx_scratch.data[0] = 4;
+			nvec->tx_scratch.data[1] = 0x8a;
+			nvec->tx_scratch.data[2] = 0x02;
+			nvec->tx_scratch.data[3] = 0x07;
+			nvec->tx_scratch.data[4] = 0x02;
+
+			nvec->tx_scratch.size = 5;
+			nvec->tx_scratch.pos = 0;
+			nvec->tx = &nvec->tx_scratch;
+			list_add_tail(&nvec->tx->node, &nvec->tx_data);
+			spin_unlock_irqrestore(&nvec->tx_lock, flags);
+			valid_proto = 1;
 		} else {
-			msg =
-			    list_first_entry(&nvec->tx_data, struct nvec_msg,
-					     node);
-			if (msg->pos < msg->size) {
-				to_send = msg->data[msg->pos];
-				msg->pos++;
-			} else {
-				dev_err(nvec->dev, "nvec crap! %d\n",
-					msg->size);
-				to_send = 0x01;
+			if (status & RCVD) {	/* 0x0e, new transfer */
+				nvec->tx = list_first_entry(&nvec->tx_data, struct nvec_msg, node);
+				spin_unlock_irqrestore(&nvec->tx_lock, flags);
+				/* Work around for AP20 New Slave Hw Bug.
+				   ((1000 / 80) / 2) + 1 = 33 */
+				getnstimeofday(&end_time);
+				diff_time = timespec_sub(end_time, start_time);
+				dtime = timespec_to_ns(&diff_time);
+				if (dtime < 33000)
+					ndelay(33000 - dtime);
+				else
+					dev_warn(nvec->dev, "isr time: %llu nsec\n", timespec_to_ns(&diff_time));
+
+				if (!nvec->rx)
+					dev_warn(nvec->dev, "no rx buffer available");
+				else if ((nvec->rx->pos == 1) && (nvec->rx->data[0] == 1)) {
+					valid_proto = 1;
+				} else {
+					dev_warn(nvec->dev, "new transaction "
+						"during send (pos: %d) - trying to retransmit!\n", nvec->tx->pos);
+					nvec->tx->pos = 0;
+				}
+			} else {	/* 0x0a, transfer continues */
+				spin_unlock_irqrestore(&nvec->tx_lock, flags);
+				if (nvec->tx != list_first_entry(&nvec->tx_data, struct nvec_msg, node)) {
+					dev_warn(nvec->dev, "tx buffer corrupted");
+				}
+				if ((nvec->tx->pos >= 1) && (nvec->tx->pos < nvec->tx->size)) {
+					valid_proto = 1;
+				}
 			}
+		}
 
-			if (msg->pos >= msg->size) {
-				list_del_init(&msg->node);
-				kfree(msg->data);
-				kfree(msg);
-				schedule_work(&nvec->tx_work);
-				nvec->state = NVEC_WAIT;
-			}
+		if (!valid_proto) {
+			dev_err(nvec->dev, "invalid protocol (sta:%lx, pos:%d, size: %d)\n", status, nvec->tx->pos, nvec->tx->size);
+			to_send = 0xff;
+			nvec->tx->pos = 0;
+		} else {
+			to_send = nvec->tx->data[nvec->tx->pos++];
 		}
+
 		writel(to_send, base + I2C_SL_RCVD);
 
-		gpio_set_value(nvec->gpio, 1);
+		if ((status & RCVD) && valid_proto) {
+			gpio_set_value(nvec->gpio, 1);
+		}
 
-		dev_dbg(nvec->dev, "nvec sent %x\n", to_send);
+		if (nvec->tx->pos == nvec->tx->size) {
+			complete(&nvec->ec_transfer);
+		}
 
 		goto handled;
-	} else {
-		received = readl(base + I2C_SL_RCVD);
+	} else { /* 0x0c, 0x08, 0x1c */
+		if (nvec->rx) {
+			if (status & RCVD) {
+				local_irq_save(flags);
+				received = readl(base + I2C_SL_RCVD);
+				writel(0, base + I2C_SL_RCVD);
+				local_irq_restore(flags);
+			} else
+				received = readl(base + I2C_SL_RCVD);
+
+			if (status & RCVD) { /* new transaction, 0x0c, 0x1c */
+				nvec->rx->pos = 0;
+				nvec->rx->size = 0;
+				nvec->rx->used = 1;
+				if (!(received == nvec->i2c_addr))
+					dev_warn(nvec->dev, "unexpected response from new slave");
+			} else if (nvec->rx->pos == 0) {   /* first byte of new transaction */
+				nvec->rx->data[nvec->rx->pos++] = received;
+				nvec->ev_type = (received & 0x80) >> 7; /* Event or Req/Res */
+				nvec->ev_len = (received & 0x60) >> 5;  /* Event length */
+			} else {			/* transaction continues */
+				if (nvec->rx->pos < MAX_PKT_SIZE)
+					nvec->rx->data[nvec->rx->pos++] = received;
+				if ((nvec->ev_len == NVEC_VAR_SIZE) || (nvec->ev_type == 0)) { /* variable write from master */
+					end_trans = 0;
+					switch (nvec->rx->pos) {
+					case 1:
+						nvec->rx->pos = 0;
+						break;
+					case 2:
+						if ((received == 0) || (received > MAX_PKT_SIZE))
+							nvec->rx->pos = 0;
+						break;
+					default:
+						if (nvec->rx->pos == 2 + nvec->rx->data[1])
+							end_trans = 1;
+					}
+				} else if (nvec->ev_len == NVEC_2BYTES)	/* 2 byte event */
+					end_trans = (nvec->rx->pos == 2);
+				else if (nvec->ev_len == NVEC_3BYTES)	/* 3 byte event */
+					end_trans = (nvec->rx->pos == 3);
+				else
+					dev_err(nvec->dev, "grap!\n");
+			}
 
-		if (status & RCVD) {
-			writel(0, base + I2C_SL_RCVD);
-			goto handled;
+		} else {
+			/* FIXME: implement NACK here ! */
+			received = readl(base + I2C_SL_RCVD);
+			dev_err(nvec->dev, "no rx buffer available!\n");
 		}
 
-		if (nvec->state == NVEC_WAIT) {
-			nvec->state = NVEC_READ;
-			msg = kzalloc(sizeof(struct nvec_msg), GFP_NOWAIT);
-			msg->data = kzalloc(32, GFP_NOWAIT);
-			INIT_LIST_HEAD(&msg->node);
-			nvec->rx = msg;
-		} else
-			msg = nvec->rx;
-
-		BUG_ON(msg->pos > 32);
-
-		msg->data[msg->pos] = received;
-		msg->pos++;
-		msg->size = msg->pos;
-		dev_dbg(nvec->dev, "Got %02lx from Master (pos: %d)!\n",
-			received, msg->pos);
+		if (end_trans) {
+			spin_lock_irqsave(&nvec->rx_lock, flags);
+			/* add the received data to the work list
+			   and move the ring buffer pointer to the next entry */
+			list_add_tail(&nvec->rx->node, &nvec->rx_data);
+			nvec->rx_pos++;
+			nvec->rx_pos &= RX_BUF_MASK;
+			WARN_ON(nvec->rx_buffer[nvec->rx_pos].used == 1);
+			if (nvec->rx_buffer[nvec->rx_pos].used) {
+				dev_err(nvec->dev, "next buffer full!");
+			}
+			nvec->rx->used = 0;
+			nvec->rx = &nvec->rx_buffer[nvec->rx_pos];
+			spin_unlock_irqrestore(&nvec->rx_lock, flags);
+
+			/* only complete on responses */
+			queue_work(nvec->wq, &nvec->rx_work);
+		}
 	}
+
 handled:
 	return IRQ_HANDLED;
 }
@@ -378,6 +487,7 @@ static int __devinit tegra_nvec_probe(struct platform_device *pdev)
 	nvec->base = base;
 	nvec->irq = res->start;
 	nvec->i2c_clk = i2c_clk;
+	nvec->rx = &nvec->rx_buffer[0];
 
 	/* Set the gpio to low when we've got something to say */
 	err = gpio_request(nvec->gpio, "nvec gpio");
@@ -387,11 +497,17 @@ static int __devinit tegra_nvec_probe(struct platform_device *pdev)
 	ATOMIC_INIT_NOTIFIER_HEAD(&nvec->notifier_list);
 
 	init_completion(&nvec->sync_write);
-	sema_init(&nvec->sync_write_mutex, 1);
-	INIT_LIST_HEAD(&nvec->tx_data);
+	init_completion(&nvec->ec_transfer);
+	mutex_init(&nvec->sync_write_mutex);
+	mutex_init(&nvec->async_write_mutex);
+	mutex_init(&nvec->dispatch_mutex);
+	spin_lock_init(&nvec->tx_lock);
+	spin_lock_init(&nvec->rx_lock);
 	INIT_LIST_HEAD(&nvec->rx_data);
+	INIT_LIST_HEAD(&nvec->tx_data);
 	INIT_WORK(&nvec->rx_work, nvec_dispatch);
 	INIT_WORK(&nvec->tx_work, nvec_request_master);
+	nvec->wq = alloc_workqueue("nvec", WQ_NON_REENTRANT, 1);
 
 	err = request_irq(nvec->irq, nvec_interrupt, 0, "nvec", nvec);
 	if (err) {
@@ -419,13 +535,14 @@ static int __devinit tegra_nvec_probe(struct platform_device *pdev)
 
 	/* Get Firmware Version */
 	msg = nvec_write_sync(nvec, EC_GET_FIRMWARE_VERSION,
-			      sizeof(EC_GET_FIRMWARE_VERSION));
+		sizeof(EC_GET_FIRMWARE_VERSION));
 
-	dev_warn(nvec->dev, "ec firmware version %02x.%02x.%02x / %02x\n",
-		 msg->data[4], msg->data[5], msg->data[6], msg->data[7]);
+	if (msg) {
+		dev_warn(nvec->dev, "ec firmware version %02x.%02x.%02x / %02x\n",
+			msg->data[4], msg->data[5], msg->data[6], msg->data[7]);
 
-	kfree(msg->data);
-	kfree(msg);
+		msg->used = 0;
+	}
 
 	ret = mfd_add_devices(nvec->dev, -1, nvec_devices,
 			      ARRAY_SIZE(nvec_devices), base, 0);
@@ -459,6 +576,7 @@ static int __devexit tegra_nvec_remove(struct platform_device *pdev)
 	free_irq(nvec->irq, &nvec_interrupt);
 	iounmap(nvec->base);
 	gpio_free(nvec->gpio);
+	destroy_workqueue(nvec->wq);
 	kfree(nvec);
 
 	return 0;
diff --git a/drivers/staging/nvec/nvec.h b/drivers/staging/nvec/nvec.h
index 27f1b9d..31774af 100644
--- a/drivers/staging/nvec/nvec.h
+++ b/drivers/staging/nvec/nvec.h
@@ -18,39 +18,33 @@
 
 #include <linux/semaphore.h>
 
-typedef enum {
+#define RX_BUF_ORDER	4
+#define RX_BUF_SIZE	(1 << RX_BUF_ORDER)
+#define RX_BUF_MASK	(RX_BUF_SIZE - 1)
+#define MAX_PKT_SIZE	200
+
+enum {
 	NVEC_2BYTES,
 	NVEC_3BYTES,
-	NVEC_VAR_SIZE
-} nvec_size;
-
-typedef enum {
-	NOT_REALLY,
-	YES,
-	NOT_AT_ALL,
-} how_care;
+	NVEC_VAR_SIZE,
+};
 
-typedef enum {
+enum {
 	NVEC_SYS = 1,
 	NVEC_BAT,
 	NVEC_KBD = 5,
 	NVEC_PS2,
 	NVEC_CNTL,
 	NVEC_KB_EVT = 0x80,
-	NVEC_PS2_EVT
-} nvec_event;
-
-typedef enum {
-	NVEC_WAIT,
-	NVEC_READ,
-	NVEC_WRITE
-} nvec_state;
+	NVEC_PS2_EVT,
+};
 
 struct nvec_msg {
-	unsigned char *data;
+	struct list_head node;
+	unsigned char data[MAX_PKT_SIZE];
 	unsigned short size;
 	unsigned short pos;
-	struct list_head node;
+	unsigned short used;
 };
 
 struct nvec_subdev {
@@ -71,15 +65,27 @@ struct nvec_chip {
 	int i2c_addr;
 	void __iomem *base;
 	struct clk *i2c_clk;
-	nvec_state state;
 	struct atomic_notifier_head notifier_list;
 	struct list_head rx_data, tx_data;
 	struct notifier_block nvec_status_notifier;
 	struct work_struct rx_work, tx_work;
-	struct nvec_msg *rx, *tx;
+	struct workqueue_struct *wq;
+	struct nvec_msg *rx;
+	struct nvec_msg rx_buffer[RX_BUF_SIZE];
+	/* points to the position in rx buffer */
+	int rx_pos;
+	int ev_len, ev_type;
+
+	struct nvec_msg *tx;
+	struct nvec_msg tx_scratch;
+	struct completion ec_transfer;
+
+	struct mutex async_write_mutex;
+	struct mutex dispatch_mutex;
+	spinlock_t tx_lock, rx_lock;
 
 	/* sync write stuff */
-	struct semaphore sync_write_mutex;
+	struct mutex sync_write_mutex;
 	struct completion sync_write;
 	u16 sync_write_pending;
 	struct nvec_msg *last_sync_msg;
@@ -96,10 +102,6 @@ extern int nvec_unregister_notifier(struct device *dev,
 				    struct notifier_block *nb,
 				    unsigned int events);
 
-const char *nvec_send_msg(unsigned char *src, unsigned char *dst_size,
-			  how_care care_resp,
-			  void (*rt_handler) (unsigned char *data));
-
 #define I2C_CNFG			0x00
 #define I2C_CNFG_PACKET_MODE_EN		(1<<10)
 #define I2C_CNFG_NEW_MASTER_SFM		(1<<11)
diff --git a/drivers/staging/nvec/nvec_kbd.c b/drivers/staging/nvec/nvec_kbd.c
index 36f8060..aae4494 100644
--- a/drivers/staging/nvec/nvec_kbd.c
+++ b/drivers/staging/nvec/nvec_kbd.c
@@ -40,7 +40,7 @@ static int nvec_keys_notifier(struct notifier_block *nb,
 	unsigned char *msg = (unsigned char *)data;
 
 	if (event_type == NVEC_KB_EVT) {
-		nvec_size _size = (msg[0] & (3 << 5)) >> 5;
+		int _size = (msg[0] & (3 << 5)) >> 5;
 
 /* power on/off button */
 		if (_size == NVEC_VAR_SIZE)
-- 
1.7.4.1