[PATCH 0/2] Driver for Ricoh cardreader

[PATCH 0/2] Driver for Ricoh cardreader

[Maxim Levitsky]

Hi,

Here is the driver for the reader I have.
Plus one fix for hangs on unclean removal introduced my changes in block core.

This is just writen and debbuged, but works very well now.
I don't know if possible, but I would like to see that in 2.6.36, because
this is new driver, so no posiibility of regressions.

Best regards,
	Maxim Levitsky

[PATCH 1/2] MEMSTICK: fix hangs on unexpected device removal in mspro_blk

[Maxim Levitsky]

Now that del_gendisk syncs, we better start rejecting requests right away.

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/memstick/core/mspro_block.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/memstick/core/mspro_block.c b/drivers/memstick/core/mspro_block.c
index 8327e24..8d27c13 100644
--- a/drivers/memstick/core/mspro_block.c
+++ b/drivers/memstick/core/mspro_block.c
@@ -1330,13 +1330,14 @@ static void mspro_block_remove(struct memstick_dev *card)
 	struct mspro_block_data *msb = memstick_get_drvdata(card);
 	unsigned long flags;
 
-	del_gendisk(msb->disk);
-	dev_dbg(&card->dev, "mspro block remove\n");
 	spin_lock_irqsave(&msb->q_lock, flags);
 	msb->eject = 1;
 	blk_start_queue(msb->queue);
 	spin_unlock_irqrestore(&msb->q_lock, flags);
 
+	del_gendisk(msb->disk);
+	dev_dbg(&card->dev, "mspro block remove\n");
+
 	blk_cleanup_queue(msb->queue);
 	msb->queue = NULL;
 
-- 
1.7.0.4

[PATCH 2/2] MEMSTICK: Add driver for Ricoh R5C592 Card reader.

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 MAINTAINERS                    |    6 +
 drivers/memstick/host/Kconfig  |   13 +
 drivers/memstick/host/Makefile |    1 +
 drivers/memstick/host/r592.c   |  920 ++++++++++++++++++++++++++++++++++++++++
 drivers/memstick/host/r592.h   |  178 ++++++++
 5 files changed, 1118 insertions(+), 0 deletions(-)
 create mode 100644 drivers/memstick/host/r592.c
 create mode 100644 drivers/memstick/host/r592.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 88fcb7c..7ebdd32 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4822,6 +4822,12 @@ S:	Maintained
 F:	drivers/mtd/nand/r822.c
 F:	drivers/mtd/nand/r822.h
 
+RICOH R5C592 MEMORYSTICK DRIVER
+M:	Maxim Levitsky <maximlevitsky@gmail.com>
+S:	Maintained
+F:	drivers/memstick/host/r592.c
+F:	drivers/memstick/host/r592.h
+
 RISCOM8 DRIVER
 S:	Orphan
 F:	Documentation/serial/riscom8.txt
diff --git a/drivers/memstick/host/Kconfig b/drivers/memstick/host/Kconfig
index 4ce5c8d..7022a2a 100644
--- a/drivers/memstick/host/Kconfig
+++ b/drivers/memstick/host/Kconfig
@@ -30,3 +30,16 @@ config MEMSTICK_JMICRON_38X
 
           To compile this driver as a module, choose M here: the
 	  module will be called jmb38x_ms.
+
+config MEMSTICK_R592
+	tristate "Ricoh R5C592 MemoryStick interface support (EXPERIMENTAL)"
+	depends on EXPERIMENTAL && PCI
+
+	help
+	  Say Y here if you want to be able to access MemoryStick cards with
+	  the Ricoh R5C592 MemoryStick card reader (which is part of 5 in one
+		multifunction reader)
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called r592.
+
diff --git a/drivers/memstick/host/Makefile b/drivers/memstick/host/Makefile
index 12530e4..ad63c16 100644
--- a/drivers/memstick/host/Makefile
+++ b/drivers/memstick/host/Makefile
@@ -8,3 +8,4 @@ endif
 
 obj-$(CONFIG_MEMSTICK_TIFM_MS)		+= tifm_ms.o
 obj-$(CONFIG_MEMSTICK_JMICRON_38X)	+= jmb38x_ms.o
+obj-$(CONFIG_MEMSTICK_R592)		+= r592.o
diff --git a/drivers/memstick/host/r592.c b/drivers/memstick/host/r592.c
new file mode 100644
index 0000000..bff5bc3
--- /dev/null
+++ b/drivers/memstick/host/r592.c
@@ -0,0 +1,920 @@
+/*
+ * Copyright (C) 2010 - Maxim Levitsky
+ * driver for Ricoh memstick readers
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/freezer.h>
+#include <linux/jiffies.h>
+#include <linux/interrupt.h>
+#include <linux/pci.h>
+#include <linux/pci_ids.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/kthread.h>
+#include <linux/sched.h>
+#include <linux/highmem.h>
+#include <asm/byteorder.h>
+#include "r592.h"
+
+static int enable_dma = 1;
+static int debug;
+
+static char *tpc_names[] = {
+	"MS_TPC_READ_MG_STATUS",
+	"MS_TPC_READ_LONG_DATA",
+	"MS_TPC_READ_SHORT_DATA",
+	"MS_TPC_READ_REG",
+	"MS_TPC_READ_QUAD_DATA",
+	"MS_INVALID",
+	"MS_TPC_GET_INT",
+	"MS_TPC_SET_RW_REG_ADRS",
+	"MS_TPC_EX_SET_CMD",
+	"MS_TPC_WRITE_QUAD_DATA",
+	"MS_TPC_WRITE_REG",
+	"MS_TPC_WRITE_SHORT_DATA",
+	"MS_TPC_WRITE_LONG_DATA",
+	"MS_TPC_SET_CMD",
+};
+
+
+static char *dbg_tpc_name(int tpc)
+{
+	return tpc_names[tpc-1];
+}
+
+/* Read a register*/
+static inline u32 r592_read_reg(struct r592_device *dev, int address)
+{
+	u32 value = le32_to_cpu(readl(dev->mmio + address));
+	dbg_reg("reg #%02d == 0x%08x", address, value);
+	return value;
+}
+
+/* Write a register */
+static inline void r592_write_reg(struct r592_device *dev,
+							int address, u32 value)
+{
+	dbg_reg("reg #%02d <- 0x%08x", address, value);
+	writel(cpu_to_le32(value), dev->mmio + address);
+	mmiowb();
+}
+
+/* Reads a big endian DWORD register */
+static inline u32 r592_read_reg_be(struct r592_device *dev, int address)
+{
+	return be32_to_cpu(r592_read_reg(dev, address));
+}
+
+/* Writes a big endian DWORD register */
+static inline void r592_write_reg_be(struct r592_device *dev,
+							int address, u32 value)
+{
+	return r592_write_reg(dev, address, cpu_to_be32(value));
+}
+
+/* Set specific bits in a register (little endian)*/
+static inline void r592_set_reg_mask(struct r592_device *dev,
+							int address, u32 mask)
+{
+	u32 reg = readl(dev->mmio + address);
+	dbg_reg("reg #%02d |= 0x%08x (old =0x%08x)", address, mask, reg);
+	writel(cpu_to_le32(reg | mask) , dev->mmio + address);
+	mmiowb();
+}
+
+/* Clear specific bits in a register (little endian)*/
+static inline void r592_clear_reg_mask(struct r592_device *dev,
+						int address, u32 mask)
+{
+	u32 reg = readl(dev->mmio + address);
+	dbg_reg("reg #%02d &= 0x%08x (old = 0x%08x, mask = 0x%08x)",
+						address, ~mask, reg, mask);
+	writel(cpu_to_le32(reg & ~mask), dev->mmio + address);
+	mmiowb();
+}
+
+/* Wait till specific bits are set/clear in a register */
+/* Note that this is intended for waits that take very little time */
+static inline int r592_reg_wait(struct r592_device *dev, int address,
+					u32 mask, u32 wanted_mask)
+{
+	unsigned long timeout = jiffies + msecs_to_jiffies(2000);
+
+	if ((r592_read_reg(dev, address) & mask) == wanted_mask)
+		return 0;
+
+	while (time_before(jiffies, timeout)) {
+		if ((r592_read_reg(dev, address) & mask) == wanted_mask)
+			return 0;
+		cpu_relax();
+	}
+	return -ETIME;
+}
+
+
+/* Enable/disable device */
+/* A common part of initialization between serial and parallel mode */
+/* This is not enough to actually use the device */
+static void r592_enable_device(struct r592_device *dev, bool enable)
+{
+	u32 reg = enable ? 3 : 0;
+	dbg("%sabling the device", enable ? "en" : "dis");
+	r592_write_reg(dev, R592_REG32, reg);
+
+	/* These error bits aren't used.
+		Still they are usefull for debug */
+	r592_clear_reg_mask(dev, R592_REG_MSC,
+		R592_REG_MSC_FIFO_USER_ORN | R592_REG_MSC_FIFO_MISMATH);
+}
+
+
+/* Set serial/parallel mode */
+static void r592_set_mode(struct r592_device *dev, bool parallel_mode)
+{
+	r592_set_reg_mask(dev, R592_IO, R592_IO_RESET);
+	msleep(100);
+
+	if (!parallel_mode) {
+		dbg("enabling serial mode");
+		r592_write_reg(dev, R592_REG36, 1);
+	} else {
+		dbg("enabling parallel mode");
+		r592_set_reg_mask(dev, R592_REG32, R592_REG32_20);
+
+		r592_clear_reg_mask(dev, R592_IO,
+			R592_IO_SERIAL1 | R592_IO_SERIAL2);
+
+		r592_write_reg(dev, R592_REG36, 3);
+	}
+}
+
+static void r592_reset(struct r592_device *dev)
+{
+	dbg("resetting device due to error");
+	/* TODO: extend 'card' interface to add a reset there */
+	/*dev->host->card->reset(dev->host->card);*/
+}
+
+
+/* Tests if there is an CRC error */
+static int r592_test_io_error(struct r592_device *dev)
+{
+	if (r592_read_reg(dev, R592_STATUS) &
+		(R592_STATUS_SEND_ERR | R592_STATUS_RECV_ERR)) {
+		dbg("got CRC error");
+		return -EILSEQ;
+	}
+	return 0;
+}
+
+/* Wait for empty fifo. Mostly precation */
+static int r592_wait_fifo_empty(struct r592_device *dev)
+{
+	/* Ensure fifo is ready */
+	int error = r592_reg_wait(dev, R592_REG_MSC,
+		R592_REG_MSC_FIFO_EMPTY, R592_REG_MSC_FIFO_EMPTY);
+
+	if (error) {
+		dbg("wait for FIFO empty failed");
+		return error;
+	}
+
+	return 0;
+}
+
+/* Activates the DMA transfer from to FIFO */
+static void r592_start_dma(struct r592_device *dev, bool is_write)
+{
+	unsigned long flags;
+	u32 reg;
+	spin_lock_irqsave(&dev->irq_lock, flags);
+
+	/* Ack interrupts (just in case) + enable them */
+	r592_clear_reg_mask(dev, R592_REG_MSC, DMA_IRQ_ACK_MASK);
+	r592_set_reg_mask(dev, R592_REG_MSC, DMA_IRQ_EN_MASK);
+
+	/* Set DMA address */
+	r592_write_reg(dev, R592_FIFO_DMA, sg_dma_address(&dev->req->sg));
+
+	/* Enable the DMA */
+	reg = r592_read_reg(dev, R592_FIFO_DMA_SETTINGS);
+	reg |= R592_FIFO_DMA_SETTINGS_EN;
+
+	if (!is_write)
+		reg |= R592_FIFO_DMA_SETTINGS_DIR;
+	else
+		reg &= ~R592_FIFO_DMA_SETTINGS_DIR;
+	r592_write_reg(dev, R592_FIFO_DMA_SETTINGS, reg);
+
+	spin_unlock_irqrestore(&dev->irq_lock, flags);
+}
+
+/* Cleanups DMA related settings */
+static void r592_stop_dma(struct r592_device *dev, int error)
+{
+	r592_clear_reg_mask(dev, R592_FIFO_DMA_SETTINGS,
+		R592_FIFO_DMA_SETTINGS_EN);
+
+	r592_write_reg(dev, R592_FIFO_DMA,
+			dev->scrath_dma_page_physical_address);
+
+	r592_clear_reg_mask(dev, R592_REG_MSC, DMA_IRQ_EN_MASK);
+	r592_clear_reg_mask(dev, R592_REG_MSC, DMA_IRQ_ACK_MASK);
+	dev->dma_error = error;
+}
+
+/* Test if hardware supports DMA */
+static void r592_check_dma(struct r592_device *dev)
+{
+	dev->dma_capable = enable_dma &&
+		(r592_read_reg(dev, R592_FIFO_DMA_SETTINGS) &
+			R592_FIFO_DMA_SETTINGS_CAP);
+}
+
+/* Transfers fifo contents in/out using DMA */
+static int r592_transfer_fifo_dma(struct r592_device *dev)
+{
+	int len, sg_count;
+	bool is_write;
+
+	if (!dev->dma_capable || !dev->req->long_data)
+		return -EINVAL;
+
+	len = dev->req->sg.length;
+	is_write = dev->req->data_dir == WRITE;
+
+	if (len != R592_LFIFO_SIZE)
+		return -EINVAL;
+
+	dbg_verbose("doing dma transfer");
+
+	dev->dma_error = 0;
+	INIT_COMPLETION(dev->dma_done);
+
+	/* TODO: hidden assumption about nenth beeing always 1 */
+	sg_count = dma_map_sg(&dev->pci_dev->dev, &dev->req->sg, 1, is_write ?
+		PCI_DMA_TODEVICE : PCI_DMA_FROMDEVICE);
+
+	if (sg_count != 1 ||
+			(sg_dma_len(&dev->req->sg) < dev->req->sg.length)) {
+		dbg("problem in dma_map_sg");
+		return -EIO;
+	}
+
+	r592_start_dma(dev, is_write);
+
+	/* Wait for DMA completion */
+	if (!wait_for_completion_timeout(
+			&dev->dma_done, msecs_to_jiffies(1000))) {
+		message("DMA timeout");
+		r592_stop_dma(dev, -ETIMEDOUT);
+	}
+
+	/* Cleanup */
+	return dev->dma_error;
+}
+
+/* writes the FIFO in 4 byte chunks
+   if lenght isn't 4 byte aligned, it spills remaining bytes into 'spill' buffer
+   it also accepts spill from former write
+   to flush a write, just call this with null  buffer */
+static void r592_write_fifo_pio(struct r592_device *dev,
+	struct dword_buffer *spill, unsigned char *buffer, int len)
+{
+	unsigned char dword[4] = {0};
+
+	if (!buffer) {
+		buffer = dword;
+		len = sizeof(dword);
+	}
+
+	/* flush spill from former write */
+	while (spill->len < 4 && len) {
+		spill->dword[spill->len++] = *buffer++;
+		len--;
+	}
+
+	if (spill->len == 4) {
+		r592_write_reg_be(dev, R592_FIFO_PIO, *(u32 *)spill->dword);
+		spill->len = 0;
+	}
+
+	if (!len)
+		return;
+
+	/* write full dwords */
+	while (len >= 4) {
+		r592_write_reg_be(
+			dev, R592_FIFO_PIO, *(u32 *)buffer);
+		buffer += 4;
+		len -= 4;
+	}
+
+	/* put remaining bytes to the spill */
+	while (len--)
+		spill->dword[spill->len++] = *buffer++;
+}
+
+/* Read a fifo in 4 bytes chunks.
+   if input doesn't fit the buffer, it places bytes of last dword in spill
+   buffer, so that they don't get lost
+   on last read, just throw these away
+*/
+static void r592_read_fifo_pio(struct r592_device *dev,
+		struct dword_buffer *spill, unsigned char *buffer, int len)
+{
+	int i;
+	u32 tmp;
+
+	for (i = 0 ; len && i < spill->len ; i++) {
+		*buffer++ = spill->dword[i];
+		len--;
+		spill->len--;
+	}
+
+	if (spill->len) {
+		memmove(spill->dword, spill->dword + i, spill->len);
+		return;
+	}
+
+	while (len >= 4) {
+		*(u32 *)buffer = r592_read_reg_be(
+					dev, R592_FIFO_PIO);
+		buffer += 4;
+		len -= 4;
+	}
+
+	if (!len)
+		return;
+
+	tmp = r592_read_reg_be(dev, R592_FIFO_PIO);
+
+	spill->len = 4;
+	while (len--) {
+		*buffer++ = tmp & 0xFF;
+		tmp >>= 8;
+		spill->len--;
+	}
+
+	for (i = 0 ; i < spill->len ; i++) {
+		spill->dword[i] = tmp & 0xFF;
+		tmp >>= 8;
+	}
+
+	return;
+}
+
+/* Transfers actual data using PIO. */
+static int r592_transfer_fifo_pio(struct r592_device *dev)
+{
+	unsigned char *buffer;
+	struct dword_buffer spill = {{0}, 0};
+	unsigned long flags;
+
+	bool is_write = dev->req->tpc >= MS_TPC_SET_RW_REG_ADRS;
+	int p_len, p_offset, len, offset;
+	struct page *page;
+
+
+	if (!dev->req) {
+		dbg("pio transfer called without request!");
+		return -EINVAL;
+	}
+
+	if (!dev->req->long_data) {
+		if (is_write) {
+			r592_write_fifo_pio(dev, &spill, dev->req->data,
+							dev->req->data_len);
+			r592_write_fifo_pio(dev, &spill, NULL, 0);
+		} else
+			r592_read_fifo_pio(dev, &spill, dev->req->data,
+							dev->req->data_len);
+		return 0;
+	}
+
+	spin_lock_irqsave(&dev->atomic_lock, flags);
+
+	len = dev->req->sg.length;
+	offset = dev->req->sg.offset;
+
+	while (len) {
+
+		p_offset = offset_in_page(offset);
+		p_len = min((int)(PAGE_SIZE - p_offset), len);
+
+		page = nth_page(sg_page(&dev->req->sg), offset >> PAGE_SHIFT);
+		buffer = kmap_atomic(page, KM_BIO_SRC_IRQ) + p_offset;
+
+
+		/* Do the transfer fifo<->memory*/
+		if (is_write)
+			r592_write_fifo_pio(dev, &spill, buffer, p_len);
+		else
+			r592_read_fifo_pio(dev, &spill, buffer, p_len);
+
+		kunmap_atomic(buffer - p_offset, KM_BIO_SRC_IRQ);
+
+		offset += p_len;
+		len -= p_len;
+
+	}
+
+	if (is_write)
+		r592_write_fifo_pio(dev, &spill, NULL, 0);
+
+	spin_unlock_irqrestore(&dev->atomic_lock, flags);
+	return 0;
+}
+
+
+/* Executes one TPC (data is read/written from small or large fifo) */
+static void r592_process_request(struct r592_device *dev)
+{
+	bool is_write = dev->req->tpc >= MS_TPC_SET_RW_REG_ADRS;
+	int len, error;
+	u32 status, reg;
+
+	if (!dev->req) {
+		dbg("r592_process_request with no request!");
+		return;
+	}
+
+	len = dev->req->long_data ?
+		dev->req->sg.length : dev->req->data_len;
+
+	/* Ensure that FIFO can hold the input data */
+	WARN_ON(len > R592_LFIFO_SIZE);
+	if (len > R592_LFIFO_SIZE) {
+		error = -EINVAL;
+		goto out;
+	}
+
+	dbg("executing %s LEN=%d", dbg_tpc_name(dev->req->tpc), len);
+
+
+	/* Set IO direction */
+	if (is_write)
+		r592_set_reg_mask(dev, R592_IO, R592_IO_DIRECTION);
+	else
+		r592_clear_reg_mask(dev, R592_IO, R592_IO_DIRECTION);
+
+
+	error = r592_wait_fifo_empty(dev);
+	if (error)
+		goto out;
+
+	/* Transfer write data */
+	if (is_write) {
+		error = r592_transfer_fifo_dma(dev);
+		if (error)
+			error = r592_transfer_fifo_pio(dev);
+	}
+
+	if (error)
+		goto out;
+
+	/* Trigger the TPC */
+	reg = (len << R592_TPC_EXEC_LEN_SHIFT) |
+		(dev->req->tpc << R592_TPC_EXEC_TPC_SHIFT) |
+			R592_TPC_EXEC_BIG_FIFO;
+
+	r592_write_reg(dev, R592_TPC_EXEC, reg);
+
+	/* Wait for TPC completion */
+	status = R592_STATUS_RDY;
+	if (dev->req->need_card_int)
+		status |= R592_STATUS_CED;
+
+	error = r592_reg_wait(dev, R592_STATUS, status, status);
+	if (error) {
+		dbg("card didn't respond");
+		goto out;
+	}
+
+	/* Test IO errors */
+	error = r592_test_io_error(dev);
+	if (error)
+		goto out;
+
+	/* Read data from FIFO */
+	if (!is_write) {
+		error = r592_transfer_fifo_dma(dev);
+
+		if (error)
+			error = r592_transfer_fifo_pio(dev);
+	}
+
+	/* read INT reg. This can be shortened with shifts, but that way
+		its more readable */
+	if (dev->parallel_mode && dev->req->need_card_int) {
+
+		dev->req->int_reg = 0;
+		status = r592_read_reg(dev, R592_STATUS);
+
+		if (status & R592_STATUS_P_CMDNACK)
+			dev->req->int_reg |= MEMSTICK_INT_CMDNAK;
+
+		if (status & R592_STATUS_P_BREQ)
+			dev->req->int_reg |= MEMSTICK_INT_BREQ;
+
+		if (status & R592_STATUS_P_INTERR)
+			dev->req->int_reg |= MEMSTICK_INT_ERR;
+
+		if (status & R592_STATUS_P_CED)
+			dev->req->int_reg |= MEMSTICK_INT_CED;
+	}
+out:
+	dev->req->error = error;
+	r592_clear_reg_mask(dev, R592_REG_MSC, R592_REG_MSC_LED);
+	return;
+}
+
+/* Main request processing thread */
+static int r592_process_thread(void *data)
+{
+	int error;
+	struct r592_device *dev = (struct r592_device *)data;
+
+	while (!kthread_should_stop()) {
+
+		try_to_freeze();
+		set_current_state(TASK_INTERRUPTIBLE);
+		error = memstick_next_req(dev->host, &dev->req);
+
+		if (error) {
+
+			if (error == -EAGAIN) {
+				dbg_verbose("IO: end of data, sleeping now");
+			} else {
+				dbg("IO: unknown error from "
+					"memstick_next_req %d", error);
+			}
+
+			schedule();
+		} else {
+			set_current_state(TASK_RUNNING);
+			dbg_verbose("IO: got request, processing it");
+			r592_process_request(dev);
+			if (dev->req->error)
+				r592_reset(dev);
+		}
+	}
+
+	return 0;
+}
+
+/* Reprogram chip to detect change in card state */
+/* eg, if card is detected, arm it to detect removal, and vice versa */
+static void r592_update_card_detect(struct r592_device *dev, bool enable)
+{
+	u32 reg = r592_read_reg(dev, R592_REG_MSC);
+	bool card_detected = reg & R592_REG_MSC_PRSNT;
+
+	dbg("update card detect. Card state: %s", card_detected ?
+		"present" : "absent");
+
+	reg &= ~(R592_REG_MSC_IRQ_REMOVE | R592_REG_MSC_IRQ_INSERT);
+
+	if (enable) {
+		if (card_detected)
+			reg |= (R592_REG_MSC_IRQ_REMOVE << 16);
+		else
+			reg |= (R592_REG_MSC_IRQ_INSERT << 16);
+	}
+
+	r592_write_reg(dev, R592_REG_MSC, reg);
+}
+
+/* Timer routine that fires 1 second after last card detection event, */
+static void r592_detect_timer(long unsigned int data)
+{
+	struct r592_device *dev = (struct r592_device *)data;
+	r592_update_card_detect(dev, true);
+	memstick_detect_change(dev->host);
+}
+
+/* Interrupt handler */
+static irqreturn_t r592_irq(int irq, void *data)
+{
+	struct r592_device *dev = (struct r592_device *)data;
+	irqreturn_t ret = IRQ_NONE;
+	u32 reg;
+	u16 irq_enable, irq_status;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->irq_lock, flags);
+
+	if (dev->insuspend)
+		goto unlock;
+
+	reg = r592_read_reg(dev, R592_REG_MSC);
+	irq_enable = reg >> 16;
+	irq_status = reg & 0xFFFF;
+
+	/* Ack the interrupts */
+	reg &= ~irq_status;
+	r592_write_reg(dev, R592_REG_MSC, reg);
+
+	/* Get the IRQ status minus bits that aren't enabled */
+	irq_status & (irq_enable);
+
+	/* Due to limitation of memstick core, we don't look at bits that
+		indicate that card was removed/inserted and/or present */
+	if (irq_status & (R592_REG_MSC_IRQ_INSERT | R592_REG_MSC_IRQ_REMOVE)) {
+
+		ret = IRQ_HANDLED;
+
+		message("IRQ: card %s", irq_status & R592_REG_MSC_IRQ_INSERT ?
+			"added" : "removed");
+
+		/*  Ack the interrupt */
+		r592_clear_reg_mask(dev, R592_REG_MSC, R592_REG_MSC_IRQ_INSERT |
+			R592_REG_MSC_IRQ_REMOVE);
+
+		mod_timer(&dev->detect_timer, jiffies + msecs_to_jiffies(500));
+		goto unlock;
+	}
+
+	if (irq_status &
+		(R592_REG_MSC_FIFO_DMA_DONE | R592_REG_MSC_FIFO_DMA_ERR)) {
+		ret = IRQ_HANDLED;
+
+		if (irq_status & R592_REG_MSC_FIFO_DMA_ERR) {
+			dbg("IRQ: DMA error");
+			r592_stop_dma(dev, -EIO);
+		} else {
+			dbg_verbose("IRQ: dma done");
+			r592_stop_dma(dev, 0);
+		}
+
+		complete(&dev->dma_done);
+		goto unlock;
+	}
+
+
+	dbg("got unhandled IRQ status = %x", reg);
+unlock:
+	spin_unlock_irqrestore(&dev->irq_lock, flags);
+	return ret;
+}
+
+/* External inteface: set settings */
+static int r592_set_param(struct memstick_host *host,
+			       enum memstick_param param,
+			       int value)
+{
+	struct r592_device *dev = memstick_priv(host);
+
+	switch (param) {
+	case MEMSTICK_POWER:
+		switch (value) {
+		case MEMSTICK_POWER_ON:
+			r592_enable_device(dev, true);
+			break;
+		case MEMSTICK_POWER_OFF:
+			r592_enable_device(dev, false);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case MEMSTICK_INTERFACE:
+		switch (value) {
+		case MEMSTICK_SERIAL:
+			r592_set_mode(dev, 0);
+			dev->parallel_mode = 0;
+			dev->host->caps &= ~MEMSTICK_CAP_AUTO_GET_INT;
+			break;
+		case MEMSTICK_PAR4:
+			r592_set_mode(dev, 1);
+			dev->host->caps |= MEMSTICK_CAP_AUTO_GET_INT;
+			dev->parallel_mode = 1;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/* External interface: submit requests */
+static void r592_submit_req(struct memstick_host *host)
+{
+	struct r592_device *dev = memstick_priv(host);
+
+	dbg("new request from user");
+	wake_up_process(dev->io_thread);
+	dbg("woke IO thread...");
+}
+
+
+static const struct pci_device_id r592_pci_id_tbl[] = {
+
+	{ PCI_VDEVICE(RICOH, 0x0592), },
+	{ },
+};
+
+/* Main entry */
+int r592_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	int error = -ENOMEM;
+	struct memstick_host *host;
+	struct r592_device *dev;
+
+	/* Allocate memory */
+	host = memstick_alloc_host(sizeof(struct r592_device), &pdev->dev);
+	if (!host)
+		goto error1;
+
+	dev = memstick_priv(host);
+	dev->host = host;
+	dev->pci_dev = pdev;
+	pci_set_drvdata(pdev, dev);
+
+
+	/* pci initialization */
+	error = pci_enable_device(pdev);
+	if (error)
+		goto error2;
+
+	pci_set_master(pdev);
+	error = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
+	if (error)
+		goto error3;
+
+	error = pci_request_regions(pdev, DRV_NAME);
+	if (error)
+		goto error3;
+
+	dev->mmio = pci_ioremap_bar(pdev, 0);
+	if (!dev->mmio)
+		goto error4;
+
+
+	dev->irq = pdev->irq;
+	spin_lock_init(&dev->irq_lock);
+	spin_lock_init(&dev->atomic_lock);
+	init_completion(&dev->dma_done);
+	setup_timer(&dev->detect_timer,
+		r592_detect_timer, (long unsigned int)dev);
+
+	/* Host initialization */
+	host->caps = MEMSTICK_CAP_PAR4;
+	host->request = r592_submit_req;
+	host->set_param = r592_set_param;
+	r592_check_dma(dev);
+
+
+	dev->io_thread = kthread_run(r592_process_thread, dev, "r592");
+	if (IS_ERR(dev->io_thread)) {
+		error = PTR_ERR(dev->io_thread);
+		goto error5;
+	}
+
+	/* This is just a precation, so don't fail */
+	dev->scrath_dma_page = dma_alloc_from_coherent(pdev->dev, PAGE_SIZE,
+		&dev->scrath_dma_page_physical_address, &error);
+
+	if (error) {
+		dev->scrath_dma_page = NULL;
+		dev->scrath_dma_page_physical_address = 0;
+	}
+	r592_stop_dma(dev , 0);
+
+
+	if (request_irq(dev->irq, &r592_irq, IRQF_SHARED,
+			  DRV_NAME, dev))
+		goto error6;
+
+	r592_update_card_detect(dev, true);
+	if (memstick_add_host(host))
+		goto error7;
+
+	message("driver succesfully loaded");
+	return 0;
+error7:
+	free_irq(dev->irq, dev);
+error6:
+	if (dev->scrath_dma_page)
+		dma_free_coherent(&pdev->dev, PAGE_SIZE, dev->scrath_dma_page,
+				dev->scrath_dma_page_physical_address);
+	kthread_stop(dev->io_thread);
+error5:
+	iounmap(dev->mmio);
+error4:
+	pci_release_regions(pdev);
+error3:
+	pci_disable_device(pdev);
+error2:
+	memstick_free_host(host);
+error1:
+	return error;
+}
+
+
+void r592_remove(struct pci_dev *pdev)
+{
+	int error = 0;
+	struct r592_device *dev = pci_get_drvdata(pdev);
+
+	/* Stop the processing thread.
+	That ensures that we don't take more requests */
+	kthread_stop(dev->io_thread);
+
+	while (!error && dev->req) {
+		dev->req->error = -ETIME;
+		error = memstick_next_req(dev->host, &dev->req);
+	}
+	memstick_remove_host(dev->host);
+
+	free_irq(dev->irq, dev);
+	iounmap(dev->mmio);
+	pci_release_regions(pdev);
+	pci_disable_device(pdev);
+	memstick_free_host(dev->host);
+
+	if (dev->scrath_dma_page)
+		dma_free_coherent(&pdev->dev, PAGE_SIZE, dev->scrath_dma_page,
+				dev->scrath_dma_page_physical_address);
+
+}
+
+int r592_suspend(struct device *core_dev)
+{
+	struct pci_dev *pdev = to_pci_dev(core_dev);
+	struct r592_device *dev = pci_get_drvdata(pdev);
+	unsigned long flags;
+
+	memstick_suspend_host(dev->host);
+
+	spin_lock_irqsave(&dev->irq_lock, flags);
+	dev->insuspend = 1;
+	spin_unlock_irqrestore(&dev->irq_lock, flags);
+	synchronize_irq(dev->irq);
+
+	r592_stop_dma(dev, -ETIME);
+	r592_update_card_detect(dev, false);
+	del_timer_sync(&dev->detect_timer);
+	return 0;
+}
+
+int r592_resume(struct device *core_dev)
+{
+	unsigned long flags;
+	struct pci_dev *pdev = to_pci_dev(core_dev);
+	struct r592_device *dev = pci_get_drvdata(pdev);
+
+	spin_lock_irqsave(&dev->irq_lock, flags);
+	dev->insuspend = 0;
+	spin_unlock_irqrestore(&dev->irq_lock, flags);
+	r592_update_card_detect(dev, true);
+
+	memstick_resume_host(dev->host);
+	return 0;
+}
+
+
+SIMPLE_DEV_PM_OPS(r592_pm_ops, r592_suspend, r592_resume);
+MODULE_DEVICE_TABLE(pci, r592_pci_id_tbl);
+
+
+static struct pci_driver r852_pci_driver = {
+	.name		= DRV_NAME,
+	.id_table	= r592_pci_id_tbl,
+	.probe		= r592_probe,
+	.remove		= r592_remove,
+	.driver.pm	= &r592_pm_ops,
+};
+
+static __init int r592_module_init(void)
+{
+	return pci_register_driver(&r852_pci_driver);
+}
+
+static void __exit r592_module_exit(void)
+{
+	pci_unregister_driver(&r852_pci_driver);
+}
+
+module_init(r592_module_init);
+module_exit(r592_module_exit);
+
+
+module_param(enable_dma, bool, S_IRUGO);
+MODULE_PARM_DESC(enable_dma, "Enable usage of the DMA (default)");
+module_param(debug, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(debug, "Debug level (0-2)");
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Maxim Levitsky <maximlevitsky@gmail.com>");
+MODULE_DESCRIPTION("Ricoh R5C592 Memstick/Memstick PRO card reader driver");
diff --git a/drivers/memstick/host/r592.h b/drivers/memstick/host/r592.h
new file mode 100644
index 0000000..05c8da9
--- /dev/null
+++ b/drivers/memstick/host/r592.h
@@ -0,0 +1,178 @@
+/*
+ * Copyright (C) 2010 - Maxim Levitsky
+ * driver for Ricoh memstick readers
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/memstick.h>
+#include <linux/spinlock.h>
+#include <linux/interrupt.h>
+#include <linux/workqueue.h>
+#include <linux/ctype.h>
+
+
+
+/* write to this reg (number,len) triggers TPC execution */
+#define R592_TPC_EXEC		0x00
+#define R592_TPC_EXEC_LEN_SHIFT	16		/* Bits 16..25 are TPC len */
+#define R592_TPC_EXEC_BIG_FIFO	(1 << 26)	/* If bit 26 is set, large fifo is used (reg 48) */
+#define R592_TPC_EXEC_TPC_SHIFT	28		/* Bits 28..31 are the TPC number */
+
+
+/* Window for small TPC fifo (big endian)*/
+/* reads and writes always are done in  8 byte chunks */
+/* Not used in driver, because large fifo does better job */
+#define R592_SFIFO		0x08
+
+
+/* Status register (ms int, small fifo, IO)*/
+#define R592_STATUS		0x10
+						/* Parallel INT bits */
+#define R592_STATUS_P_CMDNACK	(1 << 16)	/* INT reg: NACK (parallel mode) */
+#define R592_STATUS_P_BREQ	(1 << 17)	/* INT reg: card ready (parallel mode)*/
+#define R592_STATUS_P_INTERR	(1 << 18)	/* INT reg: int error (parallel mode)*/
+#define R592_STATUS_P_CED	(1 << 19)	/* INT reg: command done (parallel mode) */
+
+						/* Fifo status */
+#define R592_STATUS_SFIFO_FULL	(1 << 20)	/* Small Fifo almost full (last chunk is written) */
+#define R592_STATUS_SFIFO_EMPTY  (1 << 21)	/* Small Fifo empty */
+
+						/* Error detection via CRC */
+#define R592_STATUS_SEND_ERR	(1 << 24)	/* Send failed */
+#define R592_STATUS_RECV_ERR	(1 << 25)	/* Recieve failed */
+
+						/* Card state */
+#define R592_STATUS_RDY		(1 << 28)	/* RDY signal recieved */
+#define R592_STATUS_CED		(1 << 29)	/* INT: Command done (serial mode)*/
+#define R592_STATUS_SFIFO_INPUT	(1 << 30)	/* Small fifo recieved data*/
+
+
+#define R592_SFIFO_SIZE		32		/* total size of small fifo is 32 bytes */
+#define R592_SFIFO_PACKET	8		/* packet size of small fifo */
+
+/* IO control */
+#define R592_IO			0x18
+						/* 5 */
+#define	R592_IO_16		(1 << 16)	/* Set by default, can be cleared */
+#define	R592_IO_18		(1 << 18)	/* Set by default, can be cleared */
+						/* 5 */
+#define	R592_IO_SERIAL1		(1 << 20)	/* Set by default, can be cleared, (cleared on parallel) */
+#define	R592_IO_22		(1 << 22)	/* Set by default, can be cleared */
+						/* 4 or 5*/
+#define R592_IO_DIRECTION	(1 << 24)	/* TPC direction (1 write 0 read) */
+#define	R592_IO_26		(1 << 26)	/* Set by default, can be cleared */
+						/* 4 8, or C*/
+#define	R592_IO_SERIAL2		(1 << 30)	/* Set by default, can be cleared (cleared on parallel), serial doesn't work if unset */
+#define R592_IO_RESET		(1 << 31)	/* Reset, sets defaults*/
+
+
+/* Unknown register 1 (enables internal blocks ???)*/
+#define R592_REG32		0x20		/* bits 0-7 writeable */
+#define R592_REG32_0		(1 << 0)	/* set on start, cleared on stop - must be set*/
+#define R592_REG32_1		(1 << 1)	/* set on start, cleared on stop - must be set*/
+#define R592_REG32_3		(1 << 3)	/* must be clear */
+#define R592_REG32_20		(1 << 5)	/* set before switch to parallel */
+
+/* Unknown register (clock control???)*/
+#define R592_REG36		0x24
+#define R592_REG36_0		(1 << 0)	/* set on init, never reset (enable clock engine???)*/
+#define R592_REG36_1		(1 << 1)	/* set after switch to parallel (fast clock??)*/
+
+
+/* IRQ,card detection,large fifo (first word irq status, second enable) */
+/* IRQs are ACKed by clearing the bits */
+#define R592_REG_MSC			0x28
+#define R592_REG_MSC_PRSNT		(1 << 1)	/* card present (only status)*/
+#define R592_REG_MSC_IRQ_INSERT		(1 << 8)	/* detect insert / card insered */
+#define R592_REG_MSC_IRQ_REMOVE		(1 << 9)	/* detect removal / card removed */
+#define R592_REG_MSC_FIFO_EMPTY		(1 << 10)	/* fifo is empty */
+#define R592_REG_MSC_FIFO_DMA_DONE	(1 << 11)	/* dma enable / dma done */
+
+#define R592_REG_MSC_FIFO_USER_ORN	(1 << 12)	/* set if software reads empty fifo (if R592_REG_MSC_FIFO_EMPTY is set) */
+#define R592_REG_MSC_FIFO_MISMATH	(1 << 13)	/* set if amount of data in fifo doesn't match amount in TPC */
+#define R592_REG_MSC_FIFO_DMA_ERR	(1 << 14)	/* IO failure */
+#define R592_REG_MSC_LED		(1 << 15)	/* clear to turn led off (only status)*/
+
+#define DMA_IRQ_ACK_MASK (R592_REG_MSC_FIFO_DMA_DONE | R592_REG_MSC_FIFO_DMA_ERR)
+#define DMA_IRQ_EN_MASK (DMA_IRQ_ACK_MASK << 16)
+
+/* DMA address for large FIFO read/writes*/
+#define R592_FIFO_DMA		0x2C
+
+/* PIO access to large FIFO (512 bytes) (big endian)*/
+#define R592_FIFO_PIO		0x30
+#define R592_LFIFO_SIZE		512		/* large fifo size */
+
+
+/* large FIFO DMA settings */
+#define R592_FIFO_DMA_SETTINGS		0x34
+#define R592_FIFO_DMA_SETTINGS_EN	(1 << 0)	/* DMA enabled */
+#define R592_FIFO_DMA_SETTINGS_DIR	(1 << 1)	/* Dma direction (1 read, 0 write) */
+#define R592_FIFO_DMA_SETTINGS_CAP	(1 << 24)	/* Dma is aviable */
+
+
+/* Maybe just an delay */
+/* Bits 17..19 are just number */
+/* bit 16 is set, then bit 20 is waited */
+/* time to wait is about 50 spins * 2 ^ (bits 17..19) */
+/* seems to be possible just to ignore */
+#define R592_REG56		0x38
+#define R592_REG56_START	(1 << 16)	/* Start bit */
+#define R592_REG56_WAIT		(1 << 20)	/* HW set this after the delay */
+
+/* Scrach register, written (0xABCD00) when error happens - not used*/
+#define R592_REG_DEBUG		0x3C
+
+struct r592_device {
+	struct pci_dev *pci_dev;
+	struct memstick_host	*host;		/* host backpointer */
+	struct memstick_request *req;		/* current request */
+
+	/* Registers, IRQ */
+	void __iomem *mmio;
+	int irq;
+	int insuspend;
+	spinlock_t irq_lock;
+	struct timer_list detect_timer;
+
+
+	struct task_struct *io_thread;
+	bool parallel_mode;
+
+
+	/* DMA area */
+	int dma_capable;
+	int dma_error;
+	struct completion dma_done;
+	void *scrath_dma_page;
+	u32 scrath_dma_page_physical_address;
+	spinlock_t atomic_lock;	/* used to enter atomic mode only in PIO */
+};
+
+
+struct dword_buffer {
+	unsigned char dword[4];
+	unsigned char len;
+};
+
+
+#define DRV_NAME "r592"
+
+
+#define dbg(format, ...) \
+	if (debug) \
+		printk(KERN_DEBUG DRV_NAME ": " format "\n", ## __VA_ARGS__)
+
+#define dbg_verbose(format, ...) \
+	if (debug > 1) \
+		printk(KERN_DEBUG DRV_NAME ": " format "\n", ## __VA_ARGS__)
+
+#define dbg_reg(format, ...) \
+	if (debug > 2) \
+		printk(KERN_DEBUG DRV_NAME ": " format "\n", ## __VA_ARGS__)
+
+#define message(format, ...) \
+	printk(KERN_INFO DRV_NAME ": " format "\n", ## __VA_ARGS__)
-- 
1.7.0.4

Re: [PATCH 1/2] MEMSTICK: fix hangs on unexpected device removal in mspro_blk

[Alex Dubov]

--- On Tue, 3/8/10, Maxim Levitsky <maximlevitsky@gmail.com> wrote:

> Now that del_gendisk syncs, we better
> start rejecting requests right away.


I don't quite see why this change is needed. My understanding is, user
accessible interface should be marked as removed as early, as possible.


> 
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> ---
>  drivers/memstick/core/mspro_block.c |    5
> +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/memstick/core/mspro_block.c
> b/drivers/memstick/core/mspro_block.c
> index 8327e24..8d27c13 100644
> --- a/drivers/memstick/core/mspro_block.c
> +++ b/drivers/memstick/core/mspro_block.c
> @@ -1330,13 +1330,14 @@ static void
> mspro_block_remove(struct memstick_dev *card)
>      struct mspro_block_data *msb =
> memstick_get_drvdata(card);
>      unsigned long flags;
>  
> -    del_gendisk(msb->disk);
> -    dev_dbg(&card->dev, "mspro block
> remove\n");
>      spin_lock_irqsave(&msb->q_lock,
> flags);
>      msb->eject = 1;
>      blk_start_queue(msb->queue);
>     
> spin_unlock_irqrestore(&msb->q_lock, flags);
>  
> +    del_gendisk(msb->disk);
> +    dev_dbg(&card->dev, "mspro block
> remove\n");
> +
>      blk_cleanup_queue(msb->queue);
>      msb->queue = NULL;
>  
> -- 
> 1.7.0.4
> 
> 


      

Re: [PATCH 2/2] MEMSTICK: Add driver for Ricoh R5C592 Card reader.

[Alex Dubov]

I see two immediate problems with this patch:

1. On cosmetic level, custom debug macros should not be employed. Device
core already have this functionality (dynamic debug levels and such). Please,
use dev_dbg and friends for print-outs.

2. On a structural level, I'd rather prefer host drivers to not start their
own threads. If you look at both current host implementations, they operate
in callback fashion. Apart from saving some resources, this reduces the
amount of problems encountered during suspend/resume and shutdown.



      

Re: [PATCH 2/2] MEMSTICK: Add driver for Ricoh R5C592 Card reader.

[Maxim Levitsky]

On Wed, 2010-08-04 at 00:57 -0700, Alex Dubov wrote: 
> I see two immediate problems with this patch:
> 
> 1. On cosmetic level, custom debug macros should not be employed. Device
> core already have this functionality (dynamic debug levels and such). Please,
> use dev_dbg and friends for print-outs.
This allows much easier control for debug.
Single module parameter is enough to adjust it.
This helps me help users.
(Eg, kernel compilation is out of question)


> 
> 2. On a structural level, I'd rather prefer host drivers to not start their
> own threads. If you look at both current host implementations, they operate
> in callback fashion. Apart from saving some resources, this reduces the
> amount of problems encountered during suspend/resume and shutdown.
This isn't possible.
Hardware doesn't support interrupts on memstick bus changes, it only
supports DMA done from/to internal FIFO, and DMA it only possible for
512 byte TPCs.


Best regards,
Maxim Levitsky

Re: [PATCH 1/2] MEMSTICK: fix hangs on unexpected device removal in mspro_blk

[Maxim Levitsky]

On Wed, 2010-08-04 at 00:50 -0700, Alex Dubov wrote: 
> --- On Tue, 3/8/10, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> 
> > Now that del_gendisk syncs, we better
> > start rejecting requests right away.
> 
> 
> I don't quite see why this change is needed. My understanding is, user
> accessible interface should be marked as removed as early, as possible.

The problem here is that del_gendisk, syncs the device.
This is new change, made after you did your drivers.

I have this problem on jMicron too (which otherwise works fine).


PS:

I have a copy of your ms_block.c.
I would would be very happy if you share with me, what problems does it
still have (besides need of trivial port for changes in block system,
because I want to push it upstream too.

I have MS DUO 64M to test it against.


Best regards,
Maxim Levitsky

Re: [PATCH 2/2] MEMSTICK: Add driver for Ricoh R5C592 Card reader.

[Maxim Levitsky]

On Wed, 2010-08-04 at 19:48 +0300, Maxim Levitsky wrote: 
> On Wed, 2010-08-04 at 00:57 -0700, Alex Dubov wrote: 
> > I see two immediate problems with this patch:
> > 
> > 1. On cosmetic level, custom debug macros should not be employed. Device
> > core already have this functionality (dynamic debug levels and such). Please,
> > use dev_dbg and friends for print-outs.
> This allows much easier control for debug.
> Single module parameter is enough to adjust it.
> This helps me help users.
> (Eg, kernel compilation is out of question)
> 
> 
> > 
> > 2. On a structural level, I'd rather prefer host drivers to not start their
> > own threads. If you look at both current host implementations, they operate
> > in callback fashion. Apart from saving some resources, this reduces the
> > amount of problems encountered during suspend/resume and shutdown.
> This isn't possible.
> Hardware doesn't support interrupts on memstick bus changes, it only
> supports DMA done from/to internal FIFO, and DMA it only possible for
> 512 byte TPCs.
> 


Another question.

I see that current code ignores MEMSTICK_CAP_AUTO_GET_INT
Instread mspro_blk.c enables this capability for parallel mode, assuming
that hw supports it. Its true in my case, but might not be true in other
cases.
I think I should fix that, right?

Also I see that you bath TPC_READ_LONG_DATA/TPC_READ_LONG_DATA
Does that mean that every HW sector is larger that 512?
If so, you are doing copy on write, right?
I have small caching in my sm_ftl of last sector. It helps performance a
lot.


Also I want to clarify that the only kind of interrupts supported by hw
(besides usual card detection interrupt), is DMA done interrupt.
Thats why I have to use thread.
Doing polling in r592_submit_req (which runs in atomic context is just
cruel).
Besides, under moderate IO load, the IO thread doesn't sleep, thus there
is no overhead of wake/sleep.


Best regards,
Maxim Levitsky

Re: [PATCH 1/2] MEMSTICK: fix hangs on unexpected device removal in mspro_blk

[Maxim Levitsky]

On Wed, 2010-08-04 at 19:53 +0300, Maxim Levitsky wrote: 
> On Wed, 2010-08-04 at 00:50 -0700, Alex Dubov wrote: 
> > --- On Tue, 3/8/10, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> > 
> > > Now that del_gendisk syncs, we better
> > > start rejecting requests right away.
> > 
> > 
> > I don't quite see why this change is needed. My understanding is, user
> > accessible interface should be marked as removed as early, as possible.
> 
> The problem here is that del_gendisk, syncs the device.
> This is new change, made after you did your drivers.
> 
> I have this problem on jMicron too (which otherwise works fine).
The problem is that card check thread explicitly calls ->stop before
removing the device.
In case of mspro_blk.c that stops the request queue.
Attempt to call del_gendisk with stopped request queue hangs due to
syncing.


> 
> 
> PS:
> 
> I have a copy of your ms_block.c.
> I would would be very happy if you share with me, what problems does it
> still have (besides need of trivial port for changes in block system,
> because I want to push it upstream too.
> 
> I have MS DUO 64M to test it against.
> 
> 
Best regards,
	Maxim Levitsky

Re: [PATCH 1/2] MEMSTICK: fix hangs on unexpected device removal in mspro_blk

[Alex Dubov]

> From: Maxim Levitsky <maximlevitsky@gmail.com>
> Subject: Re: [PATCH 1/2] MEMSTICK: fix hangs on unexpected device removal in mspro_blk
> > > 
> > > > Now that del_gendisk syncs, we better
> > > > start rejecting requests right away.
> > > 
> > > 
> > > I don't quite see why this change is needed. My
> understanding is, user
> > > accessible interface should be marked as removed
> as early, as possible.
> > 
> > The problem here is that del_gendisk, syncs the
> device.
> > This is new change, made after you did your drivers.
> > 
> > I have this problem on jMicron too (which otherwise
> works fine).
> The problem is that card check thread explicitly calls
> ->stop before
> removing the device.
> In case of mspro_blk.c that stops the request queue.
> Attempt to call del_gendisk with stopped request queue
> hangs due to
> syncing.

Well, ok. Sounds good.


> > 
> > I have a copy of your ms_block.c.
> > I would would be very happy if you share with me, what
> problems does it
> > still have (besides need of trivial port for changes
> in block system,
> > because I want to push it upstream too.
> > 
> > I have MS DUO 64M to test it against.
> > 

I've got two rather different implementations of ms_block, if I remember
correctly. Both suffer from random data corruptions, thanks to my
inexplicable desire to write the state machine as tightly, as possible.
Only the later one does the spec mandated geometry correctly - I actually
wrote the first version before I've seen the spec for the first time.



      

Re: [PATCH 1/2] MEMSTICK: fix hangs on unexpected device removal in mspro_blk

[Maxim Levitsky]

On Thu, 2010-08-05 at 01:43 -0700, Alex Dubov wrote: 
> > From: Maxim Levitsky <maximlevitsky@gmail.com>
> > Subject: Re: [PATCH 1/2] MEMSTICK: fix hangs on unexpected device removal in mspro_blk
> > > > 
> > > > > Now that del_gendisk syncs, we better
> > > > > start rejecting requests right away.
> > > > 
> > > > 
> > > > I don't quite see why this change is needed. My
> > understanding is, user
> > > > accessible interface should be marked as removed
> > as early, as possible.
> > > 
> > > The problem here is that del_gendisk, syncs the
> > device.
> > > This is new change, made after you did your drivers.
> > > 
> > > I have this problem on jMicron too (which otherwise
> > works fine).
> > The problem is that card check thread explicitly calls
> > ->stop before
> > removing the device.
> > In case of mspro_blk.c that stops the request queue.
> > Attempt to call del_gendisk with stopped request queue
> > hangs due to
> > syncing.
> 
> Well, ok. Sounds good.
> 
> 
> > > 
> > > I have a copy of your ms_block.c.
> > > I would would be very happy if you share with me, what
> > problems does it
> > > still have (besides need of trivial port for changes
> > in block system,
> > > because I want to push it upstream too.
> > > 
> > > I have MS DUO 64M to test it against.
> > > 
> 
> I've got two rather different implementations of ms_block, if I remember
> correctly. Both suffer from random data corruptions, thanks to my
> inexplicable desire to write the state machine as tightly, as possible.
> Only the later one does the spec mandated geometry correctly - I actually
> wrote the first version before I've seen the spec for the first time.

By 'spec mandated geometry' you mean the fact the card needs to be
broken into zones, somewhat like XD?

Best regards,
Maxim Levitsky

Re: [PATCH 1/2] MEMSTICK: fix hangs on unexpected device removal in mspro_blk

[Alex Dubov]

> 
> By 'spec mandated geometry' you mean the fact the card
> needs to be
> broken into zones, somewhat like XD?
> 

Yes, and in fancy fashion.

First zone should have at most 494 useful logical blocks, other zones - 496.
16 physical blocks per zone are considered local spares, 2 first blocks in
zone 1 must be identical and contain factory information.