staging: nvec: Rewrite the interrupt handler

Rewrite the interrupt handler to use a state machine similar to
that found in the various kernels for the Advent Vega. This also
changes the code to use the new functions introduced in the
previous commits.

This also merges the rewrite sent in August 2011 by Marc
Dietrich, and thus also includes code by him. His original
patch can be found on the mailing list.

Signed-off-by: Julian Andres Klode <jak@jak-linux.org>
Acked-by: Marc Dietrich <marvin24@gmx.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

drivers/staging/nvec/nvec.c

@@ -155,27 +155,81 @@ static void nvec_gpio_set_value(struct nvec_chip *nvec, int value)
 void nvec_write_async(struct nvec_chip *nvec, const 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 = nvec_msg_alloc(nvec);
 	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,
+		const unsigned char *data, short size)
+{
+	struct nvec_msg *msg;
+
+	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");
+
+	msg = nvec->last_sync_msg;
+
+	mutex_unlock(&nvec->sync_write_mutex);
+
+	return 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;
+	long err;
+	struct nvec_msg *msg;
+
+	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);
+		nvec_gpio_set_value(nvec, 0);
+		err = wait_for_completion_interruptible_timeout(
+				&nvec->ec_transfer, msecs_to_jiffies(5000));
+
+		if (err == 0) {
+			dev_warn(nvec->dev, "timeout waiting for ec transfer\n");
+			nvec_gpio_set_value(nvec, 1);
+			msg->pos = 0;
+		}
 
-	if (!list_empty(&nvec->tx_data))
-		gpio_set_value(nvec->gpio, 0);
+		spin_lock_irqsave(&nvec->tx_lock, flags);
+
+		if (err > 0) {
+			list_del_init(&msg->node);
+			nvec_msg_free(nvec, msg);
+		}
+	}
+	spin_unlock_irqrestore(&nvec->tx_lock, flags);
 }
 
 static int parse_msg(struct nvec_chip *nvec, struct nvec_msg *msg)
@@ -197,143 +251,243 @@ 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,
-					const 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;
 
+	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]) {
+		      (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\n");
-			else {
-				kfree(msg->data);
-				kfree(msg);
-			}
+			nvec_msg_free(nvec, msg);
 		}
+		spin_lock_irqsave(&nvec->rx_lock, flags);
 	}
+	spin_unlock_irqrestore(&nvec->rx_lock, flags);
 }
 
+static void nvec_tx_completed(struct nvec_chip *nvec)
+{
+	/* We got an END_TRANS, let's skip this, maybe there's an event */
+	if (nvec->tx->pos != nvec->tx->size) {
+		dev_err(nvec->dev, "premature END_TRANS, resending\n");
+		nvec->tx->pos = 0;
+		nvec_gpio_set_value(nvec, 0);
+	} else {
+		nvec->state = 0;
+	}
+}
+
+static void nvec_rx_completed(struct nvec_chip *nvec)
+{
+	if (nvec->rx->pos != nvec_msg_size(nvec->rx))
+		dev_err(nvec->dev, "RX incomplete: Expected %u bytes, got %u\n",
+			   (uint) nvec_msg_size(nvec->rx),
+			   (uint) nvec->rx->pos);
+
+	spin_lock(&nvec->rx_lock);
+
+	/* 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);
+
+	spin_unlock(&nvec->rx_lock);
+
+	nvec->state = 0;
+
+	if (!nvec_msg_is_event(nvec->rx))
+		complete(&nvec->ec_transfer);
+
+	queue_work(nvec->wq, &nvec->rx_work);
+}
+
+/**
+ * nvec_invalid_flags - Send an error message about invalid flags and jump
+ * @nvec: The nvec device
+ * @status: The status flags
+ * @reset: Whether we shall jump to state 0.
+ */
+static void nvec_invalid_flags(struct nvec_chip *nvec, unsigned int status,
+			       bool reset)
+{
+	dev_err(nvec->dev, "unexpected status flags 0x%02x during state %i\n",
+		status, nvec->state);
+	if (reset)
+		nvec->state = 0;
+}
+
+/**
+ * nvec_tx_set - Set the message to transfer (nvec->tx)
+ */
+static void nvec_tx_set(struct nvec_chip *nvec)
+{
+	spin_lock(&nvec->tx_lock);
+	if (list_empty(&nvec->tx_data)) {
+		dev_err(nvec->dev, "empty tx - sending no-op\n");
+		memcpy(nvec->tx_scratch.data, "\x02\x07\x02", 3);
+		nvec->tx_scratch.size = 3;
+		nvec->tx_scratch.pos = 0;
+		nvec->tx = &nvec->tx_scratch;
+		list_add_tail(&nvec->tx->node, &nvec->tx_data);
+	} else {
+		nvec->tx = list_first_entry(&nvec->tx_data, struct nvec_msg,
+					    node);
+		nvec->tx->pos = 0;
+	}
+	spin_unlock(&nvec->tx_lock);
+
+	dev_dbg(nvec->dev, "Sending message of length %u, command 0x%x\n",
+		(uint)nvec->tx->size, nvec->tx->data[1]);
+}
+
+/**
+ * nvec_interrupt - Interrupt handler
+ * @irq: The IRQ
+ * @dev: The nvec device
+ */
 static irqreturn_t nvec_interrupt(int irq, void *dev)
 {
 	unsigned long status;
-	unsigned long received;
-	unsigned char to_send;
-	struct nvec_msg *msg;
-	struct nvec_chip *nvec = (struct nvec_chip *)dev;
-	void __iomem *base = nvec->base;
+	unsigned int received = 0;
+	unsigned char to_send = 0xff;
+	const unsigned long irq_mask = I2C_SL_IRQ | END_TRANS | RCVD | RNW;
+	struct nvec_chip *nvec = dev;
+	unsigned int state = nvec->state;
 
-	status = readl(base + I2C_SL_STATUS);
+	status = readl(nvec->base + I2C_SL_STATUS);
 
-	if (!(status & I2C_SL_IRQ)) {
-		dev_warn(nvec->dev, "nvec Spurious IRQ\n");
-		goto handled;
+	/* Filter out some errors */
+	if ((status & irq_mask) == 0 && (status & ~irq_mask) != 0) {
+		dev_err(nvec->dev, "unexpected irq mask %lx\n", status);
+		return IRQ_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);
-		}
+	if ((status & I2C_SL_IRQ) == 0) {
+		dev_err(nvec->dev, "Spurious IRQ\n");
 		return IRQ_HANDLED;
-	} else if (status & RNW) {
-		if (status & RCVD)
-			udelay(3);
+	}
 
+	/* The EC did not request a read, so it send us something, read it */
+	if ((status & RNW) == 0) {
+		received = readl(nvec->base + I2C_SL_RCVD);
 		if (status & RCVD)
-			nvec->state = NVEC_WRITE;
+			writel(0, nvec->base + I2C_SL_RCVD);
+	}
 
-		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);
+	if (status == (I2C_SL_IRQ | RCVD))
+		nvec->state = 0;
+
+	switch (nvec->state) {
+	case 0:		/* Verify that its a transfer start, the rest later */
+		if (status != (I2C_SL_IRQ | RCVD))
+			nvec_invalid_flags(nvec, status, false);
+		break;
+	case 1:		/* command byte */
+		if (status != I2C_SL_IRQ) {
+			nvec_invalid_flags(nvec, status, true);
 		} 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 (msg->pos >= msg->size) {
-				list_del_init(&msg->node);
-				kfree(msg->data);
-				kfree(msg);
-				schedule_work(&nvec->tx_work);
-				nvec->state = NVEC_WAIT;
+			nvec->rx = nvec_msg_alloc(nvec);
+			nvec->rx->data[0] = received;
+			nvec->rx->pos = 1;
+			nvec->state = 2;
+		}
+		break;
+	case 2:		/* first byte after command */
+		if (status == (I2C_SL_IRQ | RNW | RCVD)) {
+			udelay(33);
+			if (nvec->rx->data[0] != 0x01) {
+				dev_err(nvec->dev,
+					"Read without prior read command\n");
+				nvec->state = 0;
+				break;
 			}
+			nvec_msg_free(nvec, nvec->rx);
+			nvec->state = 3;
+			nvec_tx_set(nvec);
+			BUG_ON(nvec->tx->size < 1);
+			to_send = nvec->tx->data[0];
+			nvec->tx->pos = 1;
+		} else if (status == (I2C_SL_IRQ)) {
+			BUG_ON(nvec->rx == NULL);
+			nvec->rx->data[1] = received;
+			nvec->rx->pos = 2;
+			nvec->state = 4;
+		} else {
+			nvec_invalid_flags(nvec, status, true);
 		}
-		writel(to_send, base + I2C_SL_RCVD);
-
-		gpio_set_value(nvec->gpio, 1);
-
-		dev_dbg(nvec->dev, "nvec sent %x\n", to_send);
-
-		goto handled;
-	} else {
-		received = readl(base + I2C_SL_RCVD);
-
-		if (status & RCVD) {
-			writel(0, base + I2C_SL_RCVD);
-			goto handled;
+		break;
+	case 3:		/* EC does a block read, we transmit data */
+		if (status & END_TRANS) {
+			nvec_tx_completed(nvec);
+		} else if ((status & RNW) == 0 || (status & RCVD)) {
+			nvec_invalid_flags(nvec, status, true);
+		} else if (nvec->tx && nvec->tx->pos < nvec->tx->size) {
+			to_send = nvec->tx->data[nvec->tx->pos++];
+		} else {
+			dev_err(nvec->dev, "tx buffer underflow on %p (%u > %u)\n",
+				nvec->tx,
+				(uint) (nvec->tx ? nvec->tx->pos : 0),
+				(uint) (nvec->tx ? nvec->tx->size : 0));
+			nvec->state = 0;
 		}
+		break;
+	case 4:		/* EC does some write, we read the data */
+		if ((status & (END_TRANS | RNW)) == END_TRANS)
+			nvec_rx_completed(nvec);
+		else if (status & (RNW | RCVD))
+			nvec_invalid_flags(nvec, status, true);
+		else if (nvec->rx && nvec->rx->pos < NVEC_MSG_SIZE)
+			nvec->rx->data[nvec->rx->pos++] = received;
+		else
+			dev_err(nvec->dev,
+				"RX buffer overflow on %p: "
+				"Trying to write byte %u of %u\n",
+				nvec->rx, nvec->rx->pos, NVEC_MSG_SIZE);
+		break;
+	default:
+		nvec->state = 0;
+	}
 
-		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 we are told that a new transfer starts, verify it */
+	if ((status & (RCVD | RNW)) == RCVD) {
+		if (received != nvec->i2c_addr)
+			dev_err(nvec->dev,
+			"received address 0x%02x, expected 0x%02x\n",
+			received, nvec->i2c_addr);
+		nvec->state = 1;
 	}
-handled:
+
+	/* Send data if requested, but not on end of transmission */
+	if ((status & (RNW | END_TRANS)) == RNW)
+		writel(to_send, nvec->base + I2C_SL_RCVD);
+
+	/* If we have send the first byte */
+	if (status == (I2C_SL_IRQ | RNW | RCVD))
+		nvec_gpio_set_value(nvec, 1);
+
+	dev_dbg(nvec->dev,
+		"Handled: %s 0x%02x, %s 0x%02x in state %u [%s%s%s]\n",
+		(status & RNW) == 0 ? "received" : "R=",
+		received,
+		(status & (RNW | END_TRANS)) ? "sent" : "S=",
+		to_send,
+		state,
+		status & END_TRANS ? " END_TRANS" : "",
+		status & RCVD ? " RCVD" : "",
+		status & RNW ? " RNW" : "");
+
 	return IRQ_HANDLED;
 }
 
@@ -432,6 +586,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->msg_pool[0];
 
 	/* Set the gpio to low when we've got something to say */
 	err = gpio_request(nvec->gpio, "nvec gpio");
@@ -441,11 +596,15 @@ 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);
+	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, 2);
 
 	err = request_irq(nvec->irq, nvec_interrupt, 0, "nvec", nvec);
 	if (err) {
@@ -473,13 +632,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);
+		nvec_msg_free(nvec, msg);
+	}
 
 	ret = mfd_add_devices(nvec->dev, -1, nvec_devices,
 			      ARRAY_SIZE(nvec_devices), base, 0);
@@ -513,6 +673,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;

drivers/staging/nvec/nvec.h

@@ -23,39 +23,36 @@
 /* NVEC_POOL_SIZE - Size of the pool in &struct nvec_msg */
 #define NVEC_POOL_SIZE	64
 
-typedef enum {
+/*
+ * NVEC_MSG_SIZE - Maximum size of the data field of &struct nvec_msg.
+ *
+ * A message must store up to a SMBus block operation which consists of
+ * one command byte, one count byte, and up to 32 payload bytes = 34
+ * byte.
+ */
+#define NVEC_MSG_SIZE	34
+
+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[NVEC_MSG_SIZE];
 	unsigned short size;
 	unsigned short pos;
-	struct list_head node;
 	atomic_t used;
 };
 
@@ -77,24 +74,35 @@ 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 msg_pool[NVEC_POOL_SIZE];
+	struct nvec_msg *rx;
+
+	struct nvec_msg *tx;
+	struct nvec_msg tx_scratch;
+	struct completion ec_transfer;
+
+	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;
+
+	int state;
 };
 
 extern void nvec_write_async(struct nvec_chip *nvec, const unsigned char *data,
 			     short size);
 
+extern struct nvec_msg *nvec_write_sync(struct nvec_chip *nvec,
+					const unsigned char *data, short size);
+
 extern int nvec_register_notifier(struct nvec_chip *nvec,
 				  struct notifier_block *nb,
 				  unsigned int events);
@@ -103,10 +111,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)

drivers/staging/nvec/nvec_kbd.c

@@ -41,7 +41,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)