2008-06-06 14:13:03

by oakad

[permalink] [raw]
Subject: Some fixes to memstick code


These are some fixes and improvements to sony memorystick code.


2008-06-06 14:09:21

by oakad

[permalink] [raw]
Subject: [PATCH 2/3] memstick: add "start" and "stop" methods to memstick device

From: Alex Dubov <[email protected]>

In some cases it may be desirable to ensure that associated driver is not going
to access the media in some period of time. "start" and "stop" methods are
provided therefore to allow it.

Signed-off-by: Alex Dubov <[email protected]>
---
drivers/memstick/core/memstick.c | 11 ++++++++---
drivers/memstick/core/mspro_block.c | 33 +++++++++++++++++++++++++++++++++
include/linux/memstick.h | 4 ++++
3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c
index 3c7d9a7..7162f77 100644
--- a/drivers/memstick/core/memstick.c
+++ b/drivers/memstick/core/memstick.c
@@ -433,8 +433,11 @@ static void memstick_check(struct work_struct *work)

dev_dbg(&host->dev, "memstick_check started\n");
mutex_lock(&host->lock);
- if (!host->card)
- memstick_power_on(host);
+ if (!host->card) {
+ if (memstick_power_on(host))
+ goto out_power_off;
+ } else
+ host->card->stop(host->card);

card = memstick_alloc_card(host);

@@ -452,7 +455,8 @@ static void memstick_check(struct work_struct *work)
|| !(host->card->check(host->card))) {
device_unregister(&host->card->dev);
host->card = NULL;
- }
+ } else
+ host->card->start(host->card);
}

if (!host->card) {
@@ -465,6 +469,7 @@ static void memstick_check(struct work_struct *work)
kfree(card);
}

+out_power_off:
if (!host->card)
host->set_param(host, MEMSTICK_POWER, MEMSTICK_POWER_OFF);

diff --git a/drivers/memstick/core/mspro_block.c b/drivers/memstick/core/mspro_block.c
index 477d0fb..004ac4d 100644
--- a/drivers/memstick/core/mspro_block.c
+++ b/drivers/memstick/core/mspro_block.c
@@ -752,6 +752,37 @@ static int mspro_block_has_request(struct mspro_block_data *msb)
return rc;
}

+static void mspro_block_stop(struct memstick_dev *card)
+{
+ struct mspro_block_data *msb = memstick_get_drvdata(card);
+ int rc = 0;
+ unsigned long flags;
+
+ while (1) {
+ spin_lock_irqsave(&msb->q_lock, flags);
+ if (!msb->has_request) {
+ blk_stop_queue(msb->queue);
+ rc = 1;
+ }
+ spin_unlock_irqrestore(&msb->q_lock, flags);
+
+ if (rc)
+ break;
+
+ wait_for_completion(&card->mrq_complete);
+ }
+}
+
+static void mspro_block_start(struct memstick_dev *card)
+{
+ struct mspro_block_data *msb = memstick_get_drvdata(card);
+ unsigned long flags;
+
+ spin_lock_irqsave(&msb->q_lock, flags);
+ blk_start_queue(msb->queue);
+ spin_unlock_irqrestore(&msb->q_lock, flags);
+}
+
static int mspro_block_queue_thread(void *data)
{
struct memstick_dev *card = data;
@@ -1272,6 +1303,8 @@ static int mspro_block_probe(struct memstick_dev *card)
rc = mspro_block_init_disk(card);
if (!rc) {
card->check = mspro_block_check_card;
+ card->stop = mspro_block_stop;
+ card->start = mspro_block_start;
return 0;
}

diff --git a/include/linux/memstick.h b/include/linux/memstick.h
index 2fe599c..a9f998a 100644
--- a/include/linux/memstick.h
+++ b/include/linux/memstick.h
@@ -263,6 +263,10 @@ struct memstick_dev {
/* Get next request from the media driver. */
int (*next_request)(struct memstick_dev *card,
struct memstick_request **mrq);
+ /* Tell the media driver to stop doing things */
+ void (*stop)(struct memstick_dev *card);
+ /* Allow the media driver to continue */
+ void (*start)(struct memstick_dev *card);

struct device dev;
};
--
1.5.3.6

2008-06-06 14:09:35

by oakad

[permalink] [raw]
Subject: [PATCH 3/3] memstick: use fully asynchronous request processing

From: Alex Dubov <[email protected]>

Instead of using a separate thread to pump requests from block layer queue to
memstick, do so inline, utilizing the callback design of the memstick.

Signed-off-by: Alex Dubov <[email protected]>
---
drivers/memstick/core/memstick.c | 7 +-
drivers/memstick/core/mspro_block.c | 346 ++++++++++++++++-------------------
drivers/memstick/host/jmb38x_ms.c | 35 +++-
drivers/memstick/host/tifm_ms.c | 49 +++--
4 files changed, 219 insertions(+), 218 deletions(-)

diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c
index 7162f77..a380050 100644
--- a/drivers/memstick/core/memstick.c
+++ b/drivers/memstick/core/memstick.c
@@ -249,8 +249,11 @@ EXPORT_SYMBOL(memstick_next_req);
*/
void memstick_new_req(struct memstick_host *host)
{
- host->retries = cmd_retries;
- host->request(host);
+ if (host->card) {
+ host->retries = cmd_retries;
+ INIT_COMPLETION(host->card->mrq_complete);
+ host->request(host);
+ }
}
EXPORT_SYMBOL(memstick_new_req);

diff --git a/drivers/memstick/core/mspro_block.c b/drivers/memstick/core/mspro_block.c
index 004ac4d..069919e 100644
--- a/drivers/memstick/core/mspro_block.c
+++ b/drivers/memstick/core/mspro_block.c
@@ -136,9 +136,8 @@ struct mspro_block_data {
unsigned int caps;
struct gendisk *disk;
struct request_queue *queue;
+ struct request *block_req;
spinlock_t q_lock;
- wait_queue_head_t q_wait;
- struct task_struct *q_thread;

unsigned short page_size;
unsigned short cylinders;
@@ -147,9 +146,10 @@ struct mspro_block_data {

unsigned char system;
unsigned char read_only:1,
- active:1,
+ eject:1,
has_request:1,
- data_dir:1;
+ data_dir:1,
+ active:1;
unsigned char transfer_cmd;

int (*mrq_handler)(struct memstick_dev *card,
@@ -160,12 +160,14 @@ struct mspro_block_data {
struct scatterlist req_sg[MSPRO_BLOCK_MAX_SEGS];
unsigned int seg_count;
unsigned int current_seg;
- unsigned short current_page;
+ unsigned int current_page;
};

static DEFINE_IDR(mspro_block_disk_idr);
static DEFINE_MUTEX(mspro_block_disk_lock);

+static int mspro_block_complete_req(struct memstick_dev *card, int error);
+
/*** Block device ***/

static int mspro_block_bd_open(struct inode *inode, struct file *filp)
@@ -197,8 +199,10 @@ static int mspro_block_disk_release(struct gendisk *disk)

mutex_lock(&mspro_block_disk_lock);

- if (msb->usage_count) {
- msb->usage_count--;
+ if (msb) {
+ if (msb->usage_count)
+ msb->usage_count--;
+
if (!msb->usage_count) {
kfree(msb);
disk->private_data = NULL;
@@ -523,11 +527,13 @@ static int h_mspro_block_req_init(struct memstick_dev *card,
static int h_mspro_block_default(struct memstick_dev *card,
struct memstick_request **mrq)
{
- complete(&card->mrq_complete);
- if (!(*mrq)->error)
- return -EAGAIN;
- else
- return (*mrq)->error;
+ return mspro_block_complete_req(card, (*mrq)->error);
+}
+
+static int h_mspro_block_default_bad(struct memstick_dev *card,
+ struct memstick_request **mrq)
+{
+ return -ENXIO;
}

static int h_mspro_block_get_ro(struct memstick_dev *card,
@@ -535,44 +541,30 @@ static int h_mspro_block_get_ro(struct memstick_dev *card,
{
struct mspro_block_data *msb = memstick_get_drvdata(card);

- if ((*mrq)->error) {
- complete(&card->mrq_complete);
- return (*mrq)->error;
+ if (!(*mrq)->error) {
+ if ((*mrq)->data[offsetof(struct ms_status_register, status0)]
+ & MEMSTICK_STATUS0_WP)
+ msb->read_only = 1;
+ else
+ msb->read_only = 0;
}

- if ((*mrq)->data[offsetof(struct ms_status_register, status0)]
- & MEMSTICK_STATUS0_WP)
- msb->read_only = 1;
- else
- msb->read_only = 0;
-
- complete(&card->mrq_complete);
- return -EAGAIN;
+ return mspro_block_complete_req(card, (*mrq)->error);
}

static int h_mspro_block_wait_for_ced(struct memstick_dev *card,
struct memstick_request **mrq)
{
- if ((*mrq)->error) {
- complete(&card->mrq_complete);
- return (*mrq)->error;
- }
-
dev_dbg(&card->dev, "wait for ced: value %x\n", (*mrq)->data[0]);

- if ((*mrq)->data[0] & (MEMSTICK_INT_CMDNAK | MEMSTICK_INT_ERR)) {
- card->current_mrq.error = -EFAULT;
- complete(&card->mrq_complete);
- return card->current_mrq.error;
+ if (!(*mrq)->error) {
+ if ((*mrq)->data[0] & (MEMSTICK_INT_CMDNAK | MEMSTICK_INT_ERR))
+ (*mrq)->error = -EFAULT;
+ else if (!((*mrq)->data[0] & MEMSTICK_INT_CED))
+ return 0;
}

- if (!((*mrq)->data[0] & MEMSTICK_INT_CED))
- return 0;
- else {
- card->current_mrq.error = 0;
- complete(&card->mrq_complete);
- return -EAGAIN;
- }
+ return mspro_block_complete_req(card, (*mrq)->error);
}

static int h_mspro_block_transfer_data(struct memstick_dev *card,
@@ -583,10 +575,8 @@ static int h_mspro_block_transfer_data(struct memstick_dev *card,
struct scatterlist t_sg = { 0 };
size_t t_offset;

- if ((*mrq)->error) {
- complete(&card->mrq_complete);
- return (*mrq)->error;
- }
+ if ((*mrq)->error)
+ return mspro_block_complete_req(card, (*mrq)->error);

switch ((*mrq)->tpc) {
case MS_TPC_WRITE_REG:
@@ -617,8 +607,8 @@ has_int_reg:

if (msb->current_seg == msb->seg_count) {
if (t_val & MEMSTICK_INT_CED) {
- complete(&card->mrq_complete);
- return -EAGAIN;
+ return mspro_block_complete_req(card,
+ 0);
} else {
card->next_request
= h_mspro_block_wait_for_ced;
@@ -666,90 +656,120 @@ has_int_reg:

/*** Data transfer ***/

-static void mspro_block_process_request(struct memstick_dev *card,
- struct request *req)
+static int mspro_block_issue_req(struct memstick_dev *card, int chunk)
{
struct mspro_block_data *msb = memstick_get_drvdata(card);
- struct mspro_param_register param;
- int rc, chunk, cnt;
- unsigned short page_count;
sector_t t_sec;
- unsigned long flags;
+ unsigned int count;
+ struct mspro_param_register param;

- do {
- page_count = 0;
+try_again:
+ while (chunk) {
+ msb->current_page = 0;
msb->current_seg = 0;
- msb->seg_count = blk_rq_map_sg(req->q, req, msb->req_sg);
+ msb->seg_count = blk_rq_map_sg(msb->block_req->q,
+ msb->block_req,
+ msb->req_sg);

- if (msb->seg_count) {
- msb->current_page = 0;
- for (rc = 0; rc < msb->seg_count; rc++)
- page_count += msb->req_sg[rc].length
- / msb->page_size;
-
- t_sec = req->sector;
- sector_div(t_sec, msb->page_size >> 9);
- param.system = msb->system;
- param.data_count = cpu_to_be16(page_count);
- param.data_address = cpu_to_be32((uint32_t)t_sec);
- param.tpc_param = 0;
-
- msb->data_dir = rq_data_dir(req);
- msb->transfer_cmd = msb->data_dir == READ
- ? MSPRO_CMD_READ_DATA
- : MSPRO_CMD_WRITE_DATA;
-
- dev_dbg(&card->dev, "data transfer: cmd %x, "
- "lba %x, count %x\n", msb->transfer_cmd,
- be32_to_cpu(param.data_address),
- page_count);
-
- card->next_request = h_mspro_block_req_init;
- msb->mrq_handler = h_mspro_block_transfer_data;
- memstick_init_req(&card->current_mrq, MS_TPC_WRITE_REG,
- &param, sizeof(param));
- memstick_new_req(card->host);
- wait_for_completion(&card->mrq_complete);
- rc = card->current_mrq.error;
+ if (!msb->seg_count) {
+ chunk = __blk_end_request(msb->block_req, -ENOMEM,
+ blk_rq_cur_bytes(msb->block_req));
+ continue;
+ }

- if (rc || (card->current_mrq.tpc == MSPRO_CMD_STOP)) {
- for (cnt = 0; cnt < msb->current_seg; cnt++)
- page_count += msb->req_sg[cnt].length
- / msb->page_size;
-
- if (msb->current_page)
- page_count += msb->current_page - 1;
-
- if (page_count && (msb->data_dir == READ))
- rc = msb->page_size * page_count;
- else
- rc = -EIO;
- } else
- rc = msb->page_size * page_count;
- } else
- rc = -EFAULT;
+ t_sec = msb->block_req->sector << 9;
+ sector_div(t_sec, msb->page_size);

- spin_lock_irqsave(&msb->q_lock, flags);
- if (rc >= 0)
- chunk = __blk_end_request(req, 0, rc);
- else
- chunk = __blk_end_request(req, rc, 0);
+ count = msb->block_req->nr_sectors << 9;
+ count /= msb->page_size;

- dev_dbg(&card->dev, "end chunk %d, %d\n", rc, chunk);
- spin_unlock_irqrestore(&msb->q_lock, flags);
- } while (chunk);
+ param.system = msb->system;
+ param.data_count = cpu_to_be16(count);
+ param.data_address = cpu_to_be32((uint32_t)t_sec);
+ param.tpc_param = 0;
+
+ msb->data_dir = rq_data_dir(msb->block_req);
+ msb->transfer_cmd = msb->data_dir == READ
+ ? MSPRO_CMD_READ_DATA
+ : MSPRO_CMD_WRITE_DATA;
+
+ dev_dbg(&card->dev, "data transfer: cmd %x, "
+ "lba %x, count %x\n", msb->transfer_cmd,
+ be32_to_cpu(param.data_address), count);
+
+ card->next_request = h_mspro_block_req_init;
+ msb->mrq_handler = h_mspro_block_transfer_data;
+ memstick_init_req(&card->current_mrq, MS_TPC_WRITE_REG,
+ &param, sizeof(param));
+ memstick_new_req(card->host);
+ return 0;
+ }
+
+ dev_dbg(&card->dev, "elv_next\n");
+ msb->block_req = elv_next_request(msb->queue);
+ if (!msb->block_req) {
+ dev_dbg(&card->dev, "issue end\n");
+ return -EAGAIN;
+ }
+
+ dev_dbg(&card->dev, "trying again\n");
+ chunk = 1;
+ goto try_again;
}

-static int mspro_block_has_request(struct mspro_block_data *msb)
+static int mspro_block_complete_req(struct memstick_dev *card, int error)
{
- int rc = 0;
- unsigned long flags;
+ struct mspro_block_data *msb = memstick_get_drvdata(card);
+ int chunk, cnt;
+ unsigned int t_len = 0;
+ unsigned int flags;

spin_lock_irqsave(&msb->q_lock, flags);
- if (kthread_should_stop() || msb->has_request)
- rc = 1;
+ dev_dbg(&card->dev, "complete %d, %d\n", msb->has_request ? 1 : 0,
+ error);
+
+ if (msb->has_request) {
+ /* Nothing to do - not really an error */
+ if (error == -EAGAIN)
+ error = 0;
+
+ if (error || (card->current_mrq.tpc == MSPRO_CMD_STOP)) {
+ if (msb->data_dir == READ) {
+ for (cnt = 0; cnt < msb->current_seg; cnt++)
+ t_len += msb->req_sg[cnt].length
+ / msb->page_size;
+
+ if (msb->current_page)
+ t_len += msb->current_page - 1;
+
+ t_len *= msb->page_size;
+ }
+ } else
+ t_len = msb->block_req->nr_sectors << 9;
+
+ dev_dbg(&card->dev, "transferred %x (%d)\n", t_len, error);
+
+ if (error && !t_len)
+ t_len = blk_rq_cur_bytes(msb->block_req);
+
+ chunk = __blk_end_request(msb->block_req, error, t_len);
+
+ error = mspro_block_issue_req(card, chunk);
+
+ if (!error)
+ goto out;
+ else
+ msb->has_request = 0;
+ } else {
+ if (!error)
+ error = -EAGAIN;
+ }
+
+ card->next_request = h_mspro_block_default_bad;
+ complete_all(&card->mrq_complete);
+out:
spin_unlock_irqrestore(&msb->q_lock, flags);
- return rc;
+ return error;
}

static void mspro_block_stop(struct memstick_dev *card)
@@ -783,54 +803,37 @@ static void mspro_block_start(struct memstick_dev *card)
spin_unlock_irqrestore(&msb->q_lock, flags);
}

-static int mspro_block_queue_thread(void *data)
+static int mspro_block_prepare_req(struct request_queue *q, struct request *req)
{
- struct memstick_dev *card = data;
- struct memstick_host *host = card->host;
- struct mspro_block_data *msb = memstick_get_drvdata(card);
- struct request *req;
- unsigned long flags;
+ if (!blk_fs_request(req) && !blk_pc_request(req)) {
+ blk_dump_rq_flags(req, "MSPro unsupported request");
+ return BLKPREP_KILL;
+ }

- while (1) {
- wait_event(msb->q_wait, mspro_block_has_request(msb));
- dev_dbg(&card->dev, "thread iter\n");
+ req->cmd_flags |= REQ_DONTPREP;

- spin_lock_irqsave(&msb->q_lock, flags);
- req = elv_next_request(msb->queue);
- dev_dbg(&card->dev, "next req %p\n", req);
- if (!req) {
- msb->has_request = 0;
- if (kthread_should_stop()) {
- spin_unlock_irqrestore(&msb->q_lock, flags);
- break;
- }
- } else
- msb->has_request = 1;
- spin_unlock_irqrestore(&msb->q_lock, flags);
-
- if (req) {
- mutex_lock(&host->lock);
- mspro_block_process_request(card, req);
- mutex_unlock(&host->lock);
- }
- }
- dev_dbg(&card->dev, "thread finished\n");
- return 0;
+ return BLKPREP_OK;
}

-static void mspro_block_request(struct request_queue *q)
+static void mspro_block_submit_req(struct request_queue *q)
{
struct memstick_dev *card = q->queuedata;
struct mspro_block_data *msb = memstick_get_drvdata(card);
struct request *req = NULL;

- if (msb->q_thread) {
- msb->has_request = 1;
- wake_up_all(&msb->q_wait);
- } else {
+ if (msb->has_request)
+ return;
+
+ if (msb->eject) {
while ((req = elv_next_request(q)) != NULL)
end_queued_request(req, -ENODEV);
+
+ return;
}
+
+ msb->has_request = 1;
+ if (mspro_block_issue_req(card, 0))
+ msb->has_request = 0;
}

/*** Initialization ***/
@@ -1200,16 +1203,14 @@ static int mspro_block_init_disk(struct memstick_dev *card)
goto out_release_id;
}

- spin_lock_init(&msb->q_lock);
- init_waitqueue_head(&msb->q_wait);
-
- msb->queue = blk_init_queue(mspro_block_request, &msb->q_lock);
+ msb->queue = blk_init_queue(mspro_block_submit_req, &msb->q_lock);
if (!msb->queue) {
rc = -ENOMEM;
goto out_put_disk;
}

msb->queue->queuedata = card;
+ blk_queue_prep_rq(msb->queue, mspro_block_prepare_req);

blk_queue_bounce_limit(msb->queue, limit);
blk_queue_max_sectors(msb->queue, MSPRO_BLOCK_MAX_PAGES);
@@ -1235,14 +1236,8 @@ static int mspro_block_init_disk(struct memstick_dev *card)
capacity *= msb->page_size >> 9;
set_capacity(msb->disk, capacity);
dev_dbg(&card->dev, "capacity set %ld\n", capacity);
- msb->q_thread = kthread_run(mspro_block_queue_thread, card,
- DRIVER_NAME"d");
- if (IS_ERR(msb->q_thread))
- goto out_put_disk;

- mutex_unlock(&host->lock);
add_disk(msb->disk);
- mutex_lock(&host->lock);
msb->active = 1;
return 0;

@@ -1290,6 +1285,7 @@ static int mspro_block_probe(struct memstick_dev *card)
return -ENOMEM;
memstick_set_drvdata(card, msb);
msb->card = card;
+ spin_lock_init(&msb->q_lock);

rc = mspro_block_init_card(card);

@@ -1319,26 +1315,17 @@ out_free:
static void mspro_block_remove(struct memstick_dev *card)
{
struct mspro_block_data *msb = memstick_get_drvdata(card);
- struct task_struct *q_thread = NULL;
unsigned long flags;

del_gendisk(msb->disk);
dev_dbg(&card->dev, "mspro block remove\n");
spin_lock_irqsave(&msb->q_lock, flags);
- q_thread = msb->q_thread;
- msb->q_thread = NULL;
- msb->active = 0;
+ msb->eject = 1;
+ blk_start_queue(msb->queue);
spin_unlock_irqrestore(&msb->q_lock, flags);

- if (q_thread) {
- mutex_unlock(&card->host->lock);
- kthread_stop(q_thread);
- mutex_lock(&card->host->lock);
- }
-
- dev_dbg(&card->dev, "queue thread stopped\n");
-
blk_cleanup_queue(msb->queue);
+ msb->queue = NULL;

sysfs_remove_group(&card->dev.kobj, &msb->attr_group);

@@ -1355,19 +1342,13 @@ static void mspro_block_remove(struct memstick_dev *card)
static int mspro_block_suspend(struct memstick_dev *card, pm_message_t state)
{
struct mspro_block_data *msb = memstick_get_drvdata(card);
- struct task_struct *q_thread = NULL;
unsigned long flags;

spin_lock_irqsave(&msb->q_lock, flags);
- q_thread = msb->q_thread;
- msb->q_thread = NULL;
- msb->active = 0;
blk_stop_queue(msb->queue);
+ msb->active = 0;
spin_unlock_irqrestore(&msb->q_lock, flags);

- if (q_thread)
- kthread_stop(q_thread);
-
return 0;
}

@@ -1406,14 +1387,7 @@ static int mspro_block_resume(struct memstick_dev *card)
if (memcmp(s_attr->data, r_attr->data, s_attr->size))
break;

- memstick_set_drvdata(card, msb);
- msb->q_thread = kthread_run(mspro_block_queue_thread,
- card, DRIVER_NAME"d");
- if (IS_ERR(msb->q_thread))
- msb->q_thread = NULL;
- else
- msb->active = 1;
-
+ msb->active = 1;
break;
}
}
diff --git a/drivers/memstick/host/jmb38x_ms.c b/drivers/memstick/host/jmb38x_ms.c
index 10be6f3..a9f98ea 100644
--- a/drivers/memstick/host/jmb38x_ms.c
+++ b/drivers/memstick/host/jmb38x_ms.c
@@ -50,6 +50,7 @@ struct jmb38x_ms_host {
struct jmb38x_ms *chip;
void __iomem *addr;
spinlock_t lock;
+ struct tasklet_struct notify;
int id;
char host_id[DEVICE_ID_SIZE];
int irq;
@@ -590,25 +591,35 @@ static void jmb38x_ms_abort(unsigned long data)
spin_unlock_irqrestore(&host->lock, flags);
}

-static void jmb38x_ms_request(struct memstick_host *msh)
+static void jmb38x_ms_req_tasklet(unsigned long data)
{
+ struct memstick_host *msh = (struct memstick_host *)data;
struct jmb38x_ms_host *host = memstick_priv(msh);
unsigned long flags;
int rc;

spin_lock_irqsave(&host->lock, flags);
- if (host->req) {
- spin_unlock_irqrestore(&host->lock, flags);
- BUG();
- return;
+ if (!host->req) {
+ do {
+ rc = memstick_next_req(msh, &host->req);
+ dev_dbg(&host->chip->pdev->dev, "tasklet req %d\n", rc);
+ } while (!rc && jmb38x_ms_issue_cmd(msh));
}
-
- do {
- rc = memstick_next_req(msh, &host->req);
- } while (!rc && jmb38x_ms_issue_cmd(msh));
spin_unlock_irqrestore(&host->lock, flags);
}

+static void jmb38x_ms_dummy_submit(struct memstick_host *msh)
+{
+ return;
+}
+
+static void jmb38x_ms_submit_req(struct memstick_host *msh)
+{
+ struct jmb38x_ms_host *host = memstick_priv(msh);
+
+ tasklet_schedule(&host->notify);
+}
+
static int jmb38x_ms_reset(struct jmb38x_ms_host *host)
{
int cnt;
@@ -816,7 +827,9 @@ static struct memstick_host *jmb38x_ms_alloc_host(struct jmb38x_ms *jm, int cnt)
host->id);
host->irq = jm->pdev->irq;
host->timeout_jiffies = msecs_to_jiffies(1000);
- msh->request = jmb38x_ms_request;
+
+ tasklet_init(&host->notify, jmb38x_ms_req_tasklet, (unsigned long)msh);
+ msh->request = jmb38x_ms_submit_req;
msh->set_param = jmb38x_ms_set_param;

msh->caps = MEMSTICK_CAP_PAR4 | MEMSTICK_CAP_PAR8;
@@ -928,6 +941,8 @@ static void jmb38x_ms_remove(struct pci_dev *dev)

host = memstick_priv(jm->hosts[cnt]);

+ jm->hosts[cnt]->request = jmb38x_ms_dummy_submit;
+ tasklet_kill(&host->notify);
writel(0, host->addr + INT_SIGNAL_ENABLE);
writel(0, host->addr + INT_STATUS_ENABLE);
mmiowb();
diff --git a/drivers/memstick/host/tifm_ms.c b/drivers/memstick/host/tifm_ms.c
index 1445876..d32d6ad 100644
--- a/drivers/memstick/host/tifm_ms.c
+++ b/drivers/memstick/host/tifm_ms.c
@@ -71,6 +71,7 @@ struct tifm_ms {
struct tifm_dev *dev;
struct timer_list timer;
struct memstick_request *req;
+ struct tasklet_struct notify;
unsigned int mode_mask;
unsigned int block_pos;
unsigned long timeout_jiffies;
@@ -455,40 +456,45 @@ static void tifm_ms_card_event(struct tifm_dev *sock)
return;
}

-static void tifm_ms_request(struct memstick_host *msh)
+static void tifm_ms_req_tasklet(unsigned long data)
{
+ struct memstick_host *msh = (struct memstick_host *)data;
struct tifm_ms *host = memstick_priv(msh);
struct tifm_dev *sock = host->dev;
unsigned long flags;
int rc;

spin_lock_irqsave(&sock->lock, flags);
- if (host->req) {
- printk(KERN_ERR "%s : unfinished request detected\n",
- sock->dev.bus_id);
- spin_unlock_irqrestore(&sock->lock, flags);
- tifm_eject(host->dev);
- return;
- }
+ if (!host->req) {
+ if (host->eject) {
+ do {
+ rc = memstick_next_req(msh, &host->req);
+ if (!rc)
+ host->req->error = -ETIME;
+ } while (!rc);
+ spin_unlock_irqrestore(&sock->lock, flags);
+ return;
+ }

- if (host->eject) {
do {
rc = memstick_next_req(msh, &host->req);
- if (!rc)
- host->req->error = -ETIME;
- } while (!rc);
- spin_unlock_irqrestore(&sock->lock, flags);
- return;
+ } while (!rc && tifm_ms_issue_cmd(host));
}
-
- do {
- rc = memstick_next_req(msh, &host->req);
- } while (!rc && tifm_ms_issue_cmd(host));
-
spin_unlock_irqrestore(&sock->lock, flags);
+}
+
+static void tifm_ms_dummy_submit(struct memstick_host *msh)
+{
return;
}

+static void tifm_ms_submit_req(struct memstick_host *msh)
+{
+ struct tifm_ms *host = memstick_priv(msh);
+
+ tasklet_schedule(&host->notify);
+}
+
static int tifm_ms_set_param(struct memstick_host *msh,
enum memstick_param param,
int value)
@@ -569,8 +575,9 @@ static int tifm_ms_probe(struct tifm_dev *sock)
host->timeout_jiffies = msecs_to_jiffies(1000);

setup_timer(&host->timer, tifm_ms_abort, (unsigned long)host);
+ tasklet_init(&host->notify, tifm_ms_req_tasklet, (unsigned long)msh);

- msh->request = tifm_ms_request;
+ msh->request = tifm_ms_submit_req;
msh->set_param = tifm_ms_set_param;
sock->card_event = tifm_ms_card_event;
sock->data_event = tifm_ms_data_event;
@@ -592,6 +599,8 @@ static void tifm_ms_remove(struct tifm_dev *sock)
int rc = 0;
unsigned long flags;

+ msh->request = tifm_ms_dummy_submit;
+ tasklet_kill(&host->notify);
spin_lock_irqsave(&sock->lock, flags);
host->eject = 1;
if (host->req) {
--
1.5.3.6

2008-06-06 14:13:22

by oakad

[permalink] [raw]
Subject: [PATCH 1/3] memstick: allow "set_param" method to return an error code

From: Alex Dubov <[email protected]>

Some controllers (Jmicron, for instance) can report temporal failure condition
during power-on. It is desirable to account for this using a return value of
"set_param" device method. The return value can also be handy to distinguish
between supported and unsupported device parameters in run time.

Signed-off-by: Alex Dubov <[email protected]>
---
drivers/memstick/core/memstick.c | 18 +++++++---
drivers/memstick/host/jmb38x_ms.c | 69 ++++++++++++++++++++++++++----------
drivers/memstick/host/tifm_ms.c | 17 ++++-----
include/linux/memstick.h | 2 +-
4 files changed, 72 insertions(+), 34 deletions(-)

diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c
index 61b98c3..3c7d9a7 100644
--- a/drivers/memstick/core/memstick.c
+++ b/drivers/memstick/core/memstick.c
@@ -415,10 +415,14 @@ err_out:
return NULL;
}

-static void memstick_power_on(struct memstick_host *host)
+static int memstick_power_on(struct memstick_host *host)
{
- host->set_param(host, MEMSTICK_POWER, MEMSTICK_POWER_ON);
- host->set_param(host, MEMSTICK_INTERFACE, MEMSTICK_SERIAL);
+ int rc = host->set_param(host, MEMSTICK_POWER, MEMSTICK_POWER_ON);
+
+ if (!rc)
+ rc = host->set_param(host, MEMSTICK_INTERFACE, MEMSTICK_SERIAL);
+
+ return rc;
}

static void memstick_check(struct work_struct *work)
@@ -573,11 +577,15 @@ EXPORT_SYMBOL(memstick_suspend_host);
*/
void memstick_resume_host(struct memstick_host *host)
{
+ int rc = 0;
+
mutex_lock(&host->lock);
if (host->card)
- memstick_power_on(host);
+ rc = memstick_power_on(host);
mutex_unlock(&host->lock);
- memstick_detect_change(host);
+
+ if (!rc)
+ memstick_detect_change(host);
}
EXPORT_SYMBOL(memstick_resume_host);

diff --git a/drivers/memstick/host/jmb38x_ms.c b/drivers/memstick/host/jmb38x_ms.c
index a054668..10be6f3 100644
--- a/drivers/memstick/host/jmb38x_ms.c
+++ b/drivers/memstick/host/jmb38x_ms.c
@@ -609,36 +609,68 @@ static void jmb38x_ms_request(struct memstick_host *msh)
spin_unlock_irqrestore(&host->lock, flags);
}

-static void jmb38x_ms_reset(struct jmb38x_ms_host *host)
+static int jmb38x_ms_reset(struct jmb38x_ms_host *host)
{
- unsigned int host_ctl = readl(host->addr + HOST_CONTROL);
+ int cnt;

- writel(HOST_CONTROL_RESET_REQ, host->addr + HOST_CONTROL);
+ writel(HOST_CONTROL_RESET_REQ | HOST_CONTROL_CLOCK_EN
+ | readl(host->addr + HOST_CONTROL),
+ host->addr + HOST_CONTROL);
+ mmiowb();
+
+ for (cnt = 0; cnt < 20; ++cnt) {
+ if (!(HOST_CONTROL_RESET_REQ
+ & readl(host->addr + HOST_CONTROL)))
+ goto reset_next;

- while (HOST_CONTROL_RESET_REQ
- & (host_ctl = readl(host->addr + HOST_CONTROL))) {
ndelay(20);
- dev_dbg(&host->chip->pdev->dev, "reset %08x\n", host_ctl);
}
+ dev_dbg(&host->chip->pdev->dev, "reset_req timeout\n");
+ return -EIO;

- writel(HOST_CONTROL_RESET, host->addr + HOST_CONTROL);
+reset_next:
+ writel(HOST_CONTROL_RESET | HOST_CONTROL_CLOCK_EN
+ | readl(host->addr + HOST_CONTROL),
+ host->addr + HOST_CONTROL);
+ mmiowb();
+
+ for (cnt = 0; cnt < 20; ++cnt) {
+ if (!(HOST_CONTROL_RESET
+ & readl(host->addr + HOST_CONTROL)))
+ goto reset_ok;
+
+ ndelay(20);
+ }
+ dev_dbg(&host->chip->pdev->dev, "reset timeout\n");
+ return -EIO;
+
+reset_ok:
mmiowb();
writel(INT_STATUS_ALL, host->addr + INT_SIGNAL_ENABLE);
writel(INT_STATUS_ALL, host->addr + INT_STATUS_ENABLE);
+ return 0;
}

-static void jmb38x_ms_set_param(struct memstick_host *msh,
- enum memstick_param param,
- int value)
+static int jmb38x_ms_set_param(struct memstick_host *msh,
+ enum memstick_param param,
+ int value)
{
struct jmb38x_ms_host *host = memstick_priv(msh);
unsigned int host_ctl = readl(host->addr + HOST_CONTROL);
unsigned int clock_ctl = CLOCK_CONTROL_40MHZ, clock_delay = 0;
+ int rc = 0;

- switch (param) {
+ switch(param) {
case MEMSTICK_POWER:
if (value == MEMSTICK_POWER_ON) {
- jmb38x_ms_reset(host);
+ rc = jmb38x_ms_reset(host);
+ if (rc)
+ return rc;
+
+ host_ctl = 7;
+ host_ctl |= HOST_CONTROL_POWER_EN
+ | HOST_CONTROL_CLOCK_EN;
+ writel(host_ctl, host->addr + HOST_CONTROL);

writel(host->id ? PAD_PU_PD_ON_MS_SOCK1
: PAD_PU_PD_ON_MS_SOCK0,
@@ -647,11 +679,7 @@ static void jmb38x_ms_set_param(struct memstick_host *msh,
writel(PAD_OUTPUT_ENABLE_MS,
host->addr + PAD_OUTPUT_ENABLE);

- host_ctl = 7;
- host_ctl |= HOST_CONTROL_POWER_EN
- | HOST_CONTROL_CLOCK_EN;
- writel(host_ctl, host->addr + HOST_CONTROL);
-
+ msleep(10);
dev_dbg(&host->chip->pdev->dev, "power on\n");
} else if (value == MEMSTICK_POWER_OFF) {
host_ctl &= ~(HOST_CONTROL_POWER_EN
@@ -660,7 +688,8 @@ static void jmb38x_ms_set_param(struct memstick_host *msh,
writel(0, host->addr + PAD_OUTPUT_ENABLE);
writel(PAD_PU_PD_OFF, host->addr + PAD_PU_PD);
dev_dbg(&host->chip->pdev->dev, "power off\n");
- }
+ } else
+ return -EINVAL;
break;
case MEMSTICK_INTERFACE:
host_ctl &= ~(3 << HOST_CONTROL_IF_SHIFT);
@@ -686,12 +715,14 @@ static void jmb38x_ms_set_param(struct memstick_host *msh,
host_ctl &= ~HOST_CONTROL_REI;
clock_ctl = CLOCK_CONTROL_60MHZ;
clock_delay = 0;
- }
+ } else
+ return -EINVAL;
writel(host_ctl, host->addr + HOST_CONTROL);
writel(clock_ctl, host->addr + CLOCK_CONTROL);
writel(clock_delay, host->addr + CLOCK_DELAY);
break;
};
+ return 0;
}

#ifdef CONFIG_PM
diff --git a/drivers/memstick/host/tifm_ms.c b/drivers/memstick/host/tifm_ms.c
index 8577de4..1445876 100644
--- a/drivers/memstick/host/tifm_ms.c
+++ b/drivers/memstick/host/tifm_ms.c
@@ -489,15 +489,12 @@ static void tifm_ms_request(struct memstick_host *msh)
return;
}

-static void tifm_ms_set_param(struct memstick_host *msh,
- enum memstick_param param,
- int value)
+static int tifm_ms_set_param(struct memstick_host *msh,
+ enum memstick_param param,
+ int value)
{
struct tifm_ms *host = memstick_priv(msh);
struct tifm_dev *sock = host->dev;
- unsigned long flags;
-
- spin_lock_irqsave(&sock->lock, flags);

switch (param) {
case MEMSTICK_POWER:
@@ -512,7 +509,8 @@ static void tifm_ms_set_param(struct memstick_host *msh,
writel(TIFM_MS_SYS_FCLR | TIFM_MS_SYS_INTCLR,
sock->addr + SOCK_MS_SYSTEM);
writel(0xffffffff, sock->addr + SOCK_MS_STATUS);
- }
+ } else
+ return -EINVAL;
break;
case MEMSTICK_INTERFACE:
if (value == MEMSTICK_SERIAL) {
@@ -525,11 +523,12 @@ static void tifm_ms_set_param(struct memstick_host *msh,
writel(TIFM_CTRL_FAST_CLK
| readl(sock->addr + SOCK_CONTROL),
sock->addr + SOCK_CONTROL);
- }
+ } else
+ return -EINVAL;
break;
};

- spin_unlock_irqrestore(&sock->lock, flags);
+ return 0;
}

static void tifm_ms_abort(unsigned long data)
diff --git a/include/linux/memstick.h b/include/linux/memstick.h
index 37a5cdb..2fe599c 100644
--- a/include/linux/memstick.h
+++ b/include/linux/memstick.h
@@ -284,7 +284,7 @@ struct memstick_host {
/* Notify the host that some requests are pending. */
void (*request)(struct memstick_host *host);
/* Set host IO parameters (power, clock, etc). */
- void (*set_param)(struct memstick_host *host,
+ int (*set_param)(struct memstick_host *host,
enum memstick_param param,
int value);
unsigned long private[0] ____cacheline_aligned;
--
1.5.3.6