All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches
@ 2009-04-23 12:25 Tejun Heo
  2009-04-23 12:25 ` [PATCH 01/12] block-update-end_cur Tejun Heo
                   ` (12 more replies)
  0 siblings, 13 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-23 12:25 UTC (permalink / raw)
  To: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent

Hello,

This is a RFC patchset.  Please do NOT pull directly from the git
tree.

This patchset contains block lld cleanups and is consisted of the
following 12 patches.

 0001-block-update-end_cur.patch
 0002-block-don-t-init-rq-fields-unnecessarily.patch
 0003-amiflop-ataflop-xd-mg_disk-clean-up-unnecessary-stu.patch
 0004-ps3disk-simplify-request-completion.patch
 0005-sunvdc-kill-vdc_end_request.patch
 0006-ubd-cleanup-completion-path.patch
 0007-ubd-drop-unnecessary-rq-sector-manipulation.patch
 0008-hd-clean-up-request-completion-paths.patch
 0009-swim3-clean-up-request-completion-paths.patch
 0010-swim-clean-up-request-completion-paths.patch
 0011-mg_disk-fold-mg_disk.h-into-mg_disk.c.patch
 0012-mg_disk-clean-up-request-completion-paths.patch

0001 fixes a stupid mistake while implementing blk_request_end_cur()
and 0002 and 0003 are pretty straight forward cleanup.  Please feel
free to apply these three patches.

0004-0012 are cleanup patches for various block low level drivers
mostly focusing on completion paths.  The biggest change is use of
standard block layer mechanisms for partial completion instead of
doing it by manipulating request directly.  All the touched drivers
are only compile tested.

I'm planning on testing hd, xd and um but can't test others.  Can you
guys please verify the following drivers work?

 ps3disk, sunvdc, swim3, swim and mg_disk.

The following git tree can be used for testing.

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git rfc-block-lld-cleanup

Thanks.

--
tejun

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 01/12] block-update-end_cur
  2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
@ 2009-04-23 12:25 ` Tejun Heo
  2009-04-23 12:25 ` [PATCH 02/12] block: don't init rq fields unnecessarily Tejun Heo
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-23 12:25 UTC (permalink / raw)
  To: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent
  Cc: Tejun Heo

---
 include/linux/blkdev.h |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 12c545e..3a5b1bd 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -903,10 +903,14 @@ static inline void blk_end_request_all(struct request *rq, int error)
  *
  * Description:
  *     Complete the current consecutively mapped chunk from @rq.
+ *
+ * Return:
+ *     %false - we are done with this request
+ *     %true  - still buffers pending for this request
  */
-static inline void blk_end_request_cur(struct request *rq, int error)
+static inline bool blk_end_request_cur(struct request *rq, int error)
 {
-	blk_end_request(rq, error, rq->hard_cur_sectors << 9);
+	return blk_end_request(rq, error, rq->hard_cur_sectors << 9);
 }
 
 /**
@@ -952,10 +956,14 @@ static inline void __blk_end_request_all(struct request *rq, int error)
  * Description:
  *     Complete the current consecutively mapped chunk from @rq.  Must
  *     be called with queue lock held.
+ *
+ * Return:
+ *     %false - we are done with this request
+ *     %true  - still buffers pending for this request
  */
-static inline void __blk_end_request_cur(struct request *rq, int error)
+static inline bool __blk_end_request_cur(struct request *rq, int error)
 {
-	__blk_end_request(rq, error, rq->hard_cur_sectors << 9);
+	return __blk_end_request(rq, error, rq->hard_cur_sectors << 9);
 }
 
 extern void blk_complete_request(struct request *);
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 02/12] block: don't init rq fields unnecessarily
  2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
  2009-04-23 12:25 ` [PATCH 01/12] block-update-end_cur Tejun Heo
@ 2009-04-23 12:25 ` Tejun Heo
  2009-04-23 12:25 ` [PATCH 03/12] amiflop,ataflop,xd,mg_disk: clean up unnecessary stuff from block drivers Tejun Heo
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-23 12:25 UTC (permalink / raw)
  To: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent
  Cc: Tejun Heo

blk_get_request() always returns properly zeroed requests.  Don't set
fields to zero/NULL unnecessarily.

[ Impact: cleanup ]

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 block/scsi_ioctl.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index bb9aa43..58cf456 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -500,8 +500,6 @@ static int __blk_send_generic(struct request_queue *q, struct gendisk *bd_disk,
 
 	rq = blk_get_request(q, WRITE, __GFP_WAIT);
 	rq->cmd_type = REQ_TYPE_BLOCK_PC;
-	rq->data_len = 0;
-	rq->extra_len = 0;
 	rq->timeout = BLK_DEFAULT_SG_TIMEOUT;
 	rq->cmd[0] = cmd;
 	rq->cmd[4] = data;
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 03/12] amiflop,ataflop,xd,mg_disk: clean up unnecessary stuff from block drivers
  2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
  2009-04-23 12:25 ` [PATCH 01/12] block-update-end_cur Tejun Heo
  2009-04-23 12:25 ` [PATCH 02/12] block: don't init rq fields unnecessarily Tejun Heo
@ 2009-04-23 12:25 ` Tejun Heo
  2009-04-23 12:25 ` [PATCH 04/12] ps3disk: simplify request completion Tejun Heo
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-23 12:25 UTC (permalink / raw)
  To: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent
  Cc: Tejun Heo

rq_data_dir() can only be READ or WRITE and rq->sector and nr_sectors
are always automatically updated after partial request completion.
Don't worry about rq_data_dir() not being either READ or WRITE or
manually update sector and nr_sectors.

[ Impact: cleanup ]

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jörg Dorchain <joerg@dorchain.net>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: unsik Kim <donari75@gmail.com>
---
 drivers/block/amiflop.c |    7 -------
 drivers/block/ataflop.c |    4 ----
 drivers/block/mg_disk.c |   10 ----------
 drivers/block/xd.c      |    9 ++-------
 4 files changed, 2 insertions(+), 28 deletions(-)

diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
index b99a2a6..8ff95f2 100644
--- a/drivers/block/amiflop.c
+++ b/drivers/block/amiflop.c
@@ -1371,11 +1371,6 @@ static void redo_fd_request(void)
 		       "0x%08lx\n", track, sector, data);
 #endif
 
-		if ((rq_data_dir(CURRENT) != READ) && (rq_data_dir(CURRENT) != WRITE)) {
-			printk(KERN_WARNING "do_fd_request: unknown command\n");
-			__blk_end_request_cur(CURRENT, -EIO);
-			goto repeat;
-		}
 		if (get_track(drive, track) == -1) {
 			__blk_end_request_cur(CURRENT, -EIO);
 			goto repeat;
@@ -1407,8 +1402,6 @@ static void redo_fd_request(void)
 			break;
 		}
 	}
-	CURRENT->nr_sectors -= CURRENT->current_nr_sectors;
-	CURRENT->sector += CURRENT->current_nr_sectors;
 
 	__blk_end_request_cur(CURRENT, 0);
 	goto repeat;
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 44a8702..2506728 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -732,8 +732,6 @@ static void do_fd_action( int drive )
 		    }
 		    else {
 			/* all sectors finished */
-			CURRENT->nr_sectors -= CURRENT->current_nr_sectors;
-			CURRENT->sector += CURRENT->current_nr_sectors;
 			__blk_end_request_cur(CURRENT, 0);
 			redo_fd_request();
 			return;
@@ -1139,8 +1137,6 @@ static void fd_rwsec_done1(int status)
 	}
 	else {
 		/* all sectors finished */
-		CURRENT->nr_sectors -= CURRENT->current_nr_sectors;
-		CURRENT->sector += CURRENT->current_nr_sectors;
 		__blk_end_request_cur(CURRENT, 0);
 		redo_fd_request();
 	}
diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c
index 62acb2e..9d16e2a 100644
--- a/drivers/block/mg_disk.c
+++ b/drivers/block/mg_disk.c
@@ -560,11 +560,6 @@ static void mg_request_poll(struct request_queue *q)
 			case WRITE:
 				mg_write(req);
 				break;
-			default:
-				printk(KERN_WARNING "%s:%d unknown command\n",
-						__func__, __LINE__);
-				__blk_end_request_cur(req, -EIO);
-				break;
 			}
 		}
 	}
@@ -614,11 +609,6 @@ static unsigned int mg_issue_req(struct request *req,
 		outb(MG_CMD_WR_CONF, (unsigned long)host->dev_base +
 				MG_REG_COMMAND);
 		break;
-	default:
-		printk(KERN_WARNING "%s:%d unknown command\n",
-				__func__, __LINE__);
-		__blk_end_request_cur(req, -EIO);
-		break;
 	}
 	return MG_ERR_NONE;
 }
diff --git a/drivers/block/xd.c b/drivers/block/xd.c
index 6f6ad82..14be4c1 100644
--- a/drivers/block/xd.c
+++ b/drivers/block/xd.c
@@ -308,7 +308,6 @@ static void do_xd_request (struct request_queue * q)
 	while ((req = elv_next_request(q)) != NULL) {
 		unsigned block = req->sector;
 		unsigned count = req->nr_sectors;
-		int rw = rq_data_dir(req);
 		XD_INFO *disk = req->rq_disk->private_data;
 		int res = 0;
 		int retry;
@@ -321,13 +320,9 @@ static void do_xd_request (struct request_queue * q)
 			__blk_end_request_cur(req, -EIO);
 			continue;
 		}
-		if (rw != READ && rw != WRITE) {
-			printk("do_xd_request: unknown request\n");
-			__blk_end_request_cur(req, -EIO);
-			continue;
-		}
 		for (retry = 0; (retry < XD_RETRIES) && !res; retry++)
-			res = xd_readwrite(rw, disk, req->buffer, block, count);
+			res = xd_readwrite(rq_data_dir(req), disk, req->buffer,
+					   block, count);
 		/* wrap up, 0 = success, -errno = fail */
 		__blk_end_request_cur(req, res);
 	}
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 04/12] ps3disk: simplify request completion
  2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
                   ` (2 preceding siblings ...)
  2009-04-23 12:25 ` [PATCH 03/12] amiflop,ataflop,xd,mg_disk: clean up unnecessary stuff from block drivers Tejun Heo
@ 2009-04-23 12:25 ` Tejun Heo
  2009-04-23 12:25 ` [PATCH 05/12] sunvdc: kill vdc_end_request() Tejun Heo
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-23 12:25 UTC (permalink / raw)
  To: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent
  Cc: Tejun Heo

ps3disk_interrupt() always completes requests fully but it uses
rq->hard_cur_sectors for FLUSH requests for some reason.  Drop them
and simply use __blk_end_request_all().

[ Impact: cleanup ]

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 drivers/block/ps3disk.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index d23b54b..f6586e4 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -231,7 +231,6 @@ static irqreturn_t ps3disk_interrupt(int irq, void *data)
 	struct request *req;
 	int res, read, error;
 	u64 tag, status;
-	unsigned long num_sectors;
 	const char *op;
 
 	res = lv1_storage_get_async_status(dev->sbd.dev_id, &tag, &status);
@@ -261,11 +260,9 @@ static irqreturn_t ps3disk_interrupt(int irq, void *data)
 	if (req->cmd_type == REQ_TYPE_LINUX_BLOCK &&
 	    req->cmd[0] == REQ_LB_OP_FLUSH) {
 		read = 0;
-		num_sectors = req->hard_cur_sectors;
 		op = "flush";
 	} else {
 		read = !rq_data_dir(req);
-		num_sectors = req->nr_sectors;
 		op = read ? "read" : "write";
 	}
 	if (status) {
@@ -281,7 +278,7 @@ static irqreturn_t ps3disk_interrupt(int irq, void *data)
 	}
 
 	spin_lock(&priv->lock);
-	__blk_end_request(req, error, num_sectors << 9);
+	__blk_end_request_all(req, error);
 	priv->req = NULL;
 	ps3disk_do_request(dev, priv->queue);
 	spin_unlock(&priv->lock);
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 05/12] sunvdc: kill vdc_end_request()
  2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
                   ` (3 preceding siblings ...)
  2009-04-23 12:25 ` [PATCH 04/12] ps3disk: simplify request completion Tejun Heo
@ 2009-04-23 12:25 ` Tejun Heo
  2009-04-23 13:33   ` David Miller
  2009-04-23 12:25 ` [PATCH 06/12] ubd: cleanup completion path Tejun Heo
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 19+ messages in thread
From: Tejun Heo @ 2009-04-23 12:25 UTC (permalink / raw)
  To: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent
  Cc: Tejun Heo

vdc_end_request() is a thin silly wrapper on top of
__blk_end_request().  Kill it.

[ Impact: cleanup ]

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: David S. Miller <davem@davemloft.net>
---
 drivers/block/sunvdc.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/block/sunvdc.c b/drivers/block/sunvdc.c
index 5861e33..f59887c 100644
--- a/drivers/block/sunvdc.c
+++ b/drivers/block/sunvdc.c
@@ -212,11 +212,6 @@ static void vdc_end_special(struct vdc_port *port, struct vio_disk_desc *desc)
 	vdc_finish(&port->vio, -err, WAITING_FOR_GEN_CMD);
 }
 
-static void vdc_end_request(struct request *req, int error, int num_sectors)
-{
-	__blk_end_request(req, error, num_sectors << 9);
-}
-
 static void vdc_end_one(struct vdc_port *port, struct vio_dring_state *dr,
 			unsigned int index)
 {
@@ -239,7 +234,7 @@ static void vdc_end_one(struct vdc_port *port, struct vio_dring_state *dr,
 
 	rqe->req = NULL;
 
-	vdc_end_request(req, (desc->status ? -EIO : 0), desc->size >> 9);
+	__blk_end_request(req, (desc->status ? -EIO : 0), desc->size);
 
 	if (blk_queue_stopped(port->disk->queue))
 		blk_start_queue(port->disk->queue);
@@ -453,7 +448,7 @@ static void do_vdc_request(struct request_queue *q)
 
 		blkdev_dequeue_request(req);
 		if (__send_request(req) < 0)
-			vdc_end_request(req, -EIO, req->hard_nr_sectors);
+			__blk_end_request_all(req, -EIO);
 	}
 }
 
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 06/12] ubd: cleanup completion path
  2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
                   ` (4 preceding siblings ...)
  2009-04-23 12:25 ` [PATCH 05/12] sunvdc: kill vdc_end_request() Tejun Heo
@ 2009-04-23 12:25 ` Tejun Heo
  2009-04-23 12:25 ` [PATCH 07/12] ubd: drop unnecessary rq->sector manipulation Tejun Heo
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-23 12:25 UTC (permalink / raw)
  To: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent
  Cc: Tejun Heo

ubd had its own block request partial completion mechanism, which is
unnecessary as block layer already does it.  Kill ubd_end_request()
and ubd_finish() and replace them with direct call to
blk_end_request().

[ Impact: cleanup ]

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jeff Dike <jdike@linux.intel.com>
---
 arch/um/drivers/ubd_kern.c |   23 +----------------------
 1 files changed, 1 insertions(+), 22 deletions(-)

diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index f934225..36ca9fa 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -451,23 +451,6 @@ static void do_ubd_request(struct request_queue * q);
 
 /* Only changed by ubd_init, which is an initcall. */
 static int thread_fd = -1;
-
-static void ubd_end_request(struct request *req, int bytes, int error)
-{
-	blk_end_request(req, error, bytes);
-}
-
-/* Callable only from interrupt context - otherwise you need to do
- * spin_lock_irq()/spin_lock_irqsave() */
-static inline void ubd_finish(struct request *req, int bytes)
-{
-	if(bytes < 0){
-		ubd_end_request(req, 0, -EIO);
-		return;
-	}
-	ubd_end_request(req, bytes, 0);
-}
-
 static LIST_HEAD(restart);
 
 /* XXX - move this inside ubd_intr. */
@@ -475,7 +458,6 @@ static LIST_HEAD(restart);
 static void ubd_handler(void)
 {
 	struct io_thread_req *req;
-	struct request *rq;
 	struct ubd *ubd;
 	struct list_head *list, *next_ele;
 	unsigned long flags;
@@ -492,10 +474,7 @@ static void ubd_handler(void)
 			return;
 		}
 
-		rq = req->req;
-		rq->nr_sectors -= req->length >> 9;
-		if(rq->nr_sectors == 0)
-			ubd_finish(rq, rq->hard_nr_sectors << 9);
+		blk_end_request(req->req, 0, req->length);
 		kfree(req);
 	}
 	reactivate_fd(thread_fd, UBD_IRQ);
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 07/12] ubd: drop unnecessary rq->sector manipulation
  2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
                   ` (5 preceding siblings ...)
  2009-04-23 12:25 ` [PATCH 06/12] ubd: cleanup completion path Tejun Heo
@ 2009-04-23 12:25 ` Tejun Heo
  2009-04-23 12:25 ` [PATCH 08/12] hd: clean up request completion paths Tejun Heo
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-23 12:25 UTC (permalink / raw)
  To: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent
  Cc: Tejun Heo

ubd curiously updates rq->sector while issuing the request in multiple
pieces.  Don't do it and simply use local copy of sector.

[ Impact: cleanup ]

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jeff Dike <jdike@linux.intel.com>
---
 arch/um/drivers/ubd_kern.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index 36ca9fa..d100af4 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -1222,7 +1222,8 @@ static void do_ubd_request(struct request_queue *q)
 {
 	struct io_thread_req *io_req;
 	struct request *req;
-	int n, last_sectors;
+	sector_t sector;
+	int n;
 
 	while(1){
 		struct ubd *dev = q->queuedata;
@@ -1238,11 +1239,10 @@ static void do_ubd_request(struct request_queue *q)
 		}
 
 		req = dev->request;
-		last_sectors = 0;
+		sector = rq->sector;
 		while(dev->start_sg < dev->end_sg){
 			struct scatterlist *sg = &dev->sg[dev->start_sg];
 
-			req->sector += last_sectors;
 			io_req = kmalloc(sizeof(struct io_thread_req),
 					 GFP_ATOMIC);
 			if(io_req == NULL){
@@ -1251,10 +1251,10 @@ static void do_ubd_request(struct request_queue *q)
 				return;
 			}
 			prepare_request(req, io_req,
-					(unsigned long long) req->sector << 9,
+					(unsigned long long)sector << 9,
 					sg->offset, sg->length, sg_page(sg));
 
-			last_sectors = sg->length >> 9;
+			sector += sg->length >> 9;
 			n = os_write_file(thread_fd, &io_req,
 					  sizeof(struct io_thread_req *));
 			if(n != sizeof(struct io_thread_req *)){
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 08/12] hd: clean up request completion paths
  2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
                   ` (6 preceding siblings ...)
  2009-04-23 12:25 ` [PATCH 07/12] ubd: drop unnecessary rq->sector manipulation Tejun Heo
@ 2009-04-23 12:25 ` Tejun Heo
  2009-04-23 12:25 ` [PATCH 09/12] swim3: " Tejun Heo
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-23 12:25 UTC (permalink / raw)
  To: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent
  Cc: Tejun Heo

hd read/write_intr() functions manually manipulate request to
incrementally complete it, which block layer already supports.  Simply
use block layer completion routines instead of manual partial
completion.

While at it, clear unnecessary elv_next_request() check at the tail of
read_intr().  This also makes read and write_intr() more consistent.

[ Impact: cleanup ]

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 drivers/block/hd.c |   36 ++++++++++++------------------------
 1 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/block/hd.c b/drivers/block/hd.c
index f3dbc96..f3d4fe0 100644
--- a/drivers/block/hd.c
+++ b/drivers/block/hd.c
@@ -452,32 +452,25 @@ static void read_intr(void)
 	bad_rw_intr();
 	hd_request();
 	return;
+
 ok_to_read:
 	req = CURRENT;
 	insw(HD_DATA, req->buffer, 256);
-	req->sector++;
-	req->buffer += 512;
-	req->errors = 0;
-	i = --req->nr_sectors;
-	--req->current_nr_sectors;
 #ifdef DEBUG
 	printk("%s: read: sector %ld, remaining = %ld, buffer=%p\n",
-		req->rq_disk->disk_name, req->sector, req->nr_sectors,
+		req->rq_disk->disk_name, req->sector + 1, req->nr_sectors - 1,
 		req->buffer+512);
 #endif
-	if (req->current_nr_sectors <= 0)
-		__blk_end_request_cur(req, 0);
-	if (i > 0) {
+	if (__blk_end_request(req, 0, 512)) {
 		SET_HANDLER(&read_intr);
 		return;
 	}
+
 	(void) inb_p(HD_STATUS);
 #if (HD_DELAY > 0)
 	last_req = read_timer();
 #endif
-	if (elv_next_request(QUEUE))
-		hd_request();
-	return;
+	hd_request();
 }
 
 static void write_intr(void)
@@ -499,24 +492,19 @@ static void write_intr(void)
 	bad_rw_intr();
 	hd_request();
 	return;
+
 ok_to_write:
-	req->sector++;
-	i = --req->nr_sectors;
-	--req->current_nr_sectors;
-	req->buffer += 512;
-	if (!i || (req->bio && req->current_nr_sectors <= 0))
-		__blk_end_request_cur(req, 0);
-	if (i > 0) {
+	if (__blk_end_request(req, 0, 512)) {
 		SET_HANDLER(&write_intr);
 		outsw(HD_DATA, req->buffer, 256);
 		local_irq_enable();
-	} else {
+		return;
+	}
+
 #if (HD_DELAY > 0)
-		last_req = read_timer();
+	last_req = read_timer();
 #endif
-		hd_request();
-	}
-	return;
+	hd_request();
 }
 
 static void recal_intr(void)
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 09/12] swim3: clean up request completion paths
  2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
                   ` (7 preceding siblings ...)
  2009-04-23 12:25 ` [PATCH 08/12] hd: clean up request completion paths Tejun Heo
@ 2009-04-23 12:25 ` Tejun Heo
  2009-04-23 12:25 ` [PATCH 10/12] swim: " Tejun Heo
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-23 12:25 UTC (permalink / raw)
  To: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent
  Cc: Tejun Heo

swim3 curiously tries to update request parameters before calling
__blk_end_request() when __blk_end_request() will do it anyway, and it
updates request for partial completion manually instead of using
blk_update_request().  Also, it does some spurious checks on rq such
as testing whether rq->sector is negative or current_nr_sectors is
zero right after fetching.

Drop unnecessary stuff and use standard block layer mechanisms.

[ Impact: cleanup ]

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/block/swim3.c |   31 +++++--------------------------
 1 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/block/swim3.c b/drivers/block/swim3.c
index 5904f7b..4248559 100644
--- a/drivers/block/swim3.c
+++ b/drivers/block/swim3.c
@@ -319,14 +319,10 @@ static void start_request(struct floppy_state *fs)
 		       req->errors, req->current_nr_sectors);
 #endif
 
-		if (req->sector < 0 || req->sector >= fs->total_secs) {
+		if (req->sector >= fs->total_secs) {
 			__blk_end_request_cur(req, -EIO);
 			continue;
 		}
-		if (req->current_nr_sectors == 0) {
-			__blk_end_request_cur(req, 0);
-			continue;
-		}
 		if (fs->ejected) {
 			__blk_end_request_cur(req, -EIO);
 			continue;
@@ -593,8 +589,6 @@ static void xfer_timeout(unsigned long data)
 	struct floppy_state *fs = (struct floppy_state *) data;
 	struct swim3 __iomem *sw = fs->swim3;
 	struct dbdma_regs __iomem *dr = fs->dma;
-	struct dbdma_cmd *cp = fs->dma_cmd;
-	unsigned long s;
 	int n;
 
 	fs->timeout_pending = 0;
@@ -605,14 +599,6 @@ static void xfer_timeout(unsigned long data)
 	out_8(&sw->intr_enable, 0);
 	out_8(&sw->control_bic, WRITE_SECTORS | DO_ACTION);
 	out_8(&sw->select, RELAX);
-	if (rq_data_dir(fd_req) == WRITE)
-		++cp;
-	if (ld_le16(&cp->xfer_status) != 0)
-		s = fs->scount - ((ld_le16(&cp->res_count) + 511) >> 9);
-	else
-		s = 0;
-	fd_req->sector += s;
-	fd_req->current_nr_sectors -= s;
 	printk(KERN_ERR "swim3: timeout %sing sector %ld\n",
 	       (rq_data_dir(fd_req)==WRITE? "writ": "read"), (long)fd_req->sector);
 	__blk_end_request_cur(fd_req, -EIO);
@@ -719,9 +705,7 @@ static irqreturn_t swim3_interrupt(int irq, void *dev_id)
 		if (intr & ERROR_INTR) {
 			n = fs->scount - 1 - resid / 512;
 			if (n > 0) {
-				fd_req->sector += n;
-				fd_req->current_nr_sectors -= n;
-				fd_req->buffer += n * 512;
+				blk_update_request(fd_req, 0, n << 9);
 				fs->req_sector += n;
 			}
 			if (fs->retries < 5) {
@@ -745,13 +729,7 @@ static irqreturn_t swim3_interrupt(int irq, void *dev_id)
 				start_request(fs);
 				break;
 			}
-			fd_req->sector += fs->scount;
-			fd_req->current_nr_sectors -= fs->scount;
-			fd_req->buffer += fs->scount * 512;
-			if (fd_req->current_nr_sectors <= 0) {
-				__blk_end_request_cur(fd_req, 0);
-				fs->state = idle;
-			} else {
+			if (__blk_end_request(fd_req, 0, fs->scount << 9)) {
 				fs->req_sector += fs->scount;
 				if (fs->req_sector > fs->secpertrack) {
 					fs->req_sector -= fs->secpertrack;
@@ -761,7 +739,8 @@ static irqreturn_t swim3_interrupt(int irq, void *dev_id)
 					}
 				}
 				act(fs);
-			}
+			} else
+				fs->state = idle;
 		}
 		if (fs->state == idle)
 			start_request(fs);
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 10/12] swim: clean up request completion paths
  2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
                   ` (8 preceding siblings ...)
  2009-04-23 12:25 ` [PATCH 09/12] swim3: " Tejun Heo
@ 2009-04-23 12:25 ` Tejun Heo
  2009-04-23 12:25 ` [PATCH 11/12] mg_disk: fold mg_disk.h into mg_disk.c Tejun Heo
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-23 12:25 UTC (permalink / raw)
  To: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent
  Cc: Tejun Heo

swim curiously tries to update request parameters before calling
__blk_end_request() when __blk_end_request() will do it anyway and
unnecessarily checks whether current_nr_sectors is zero right after
fetching.

Drop unnecessary stuff and use standard block layer mechanisms.

[ Impact: cleanup ]

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Laurent Vivier <Laurent@lvivier.info>
---
 drivers/block/swim.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index 6544a7b..97ef426 100644
--- a/drivers/block/swim.c
+++ b/drivers/block/swim.c
@@ -535,10 +535,6 @@ static void redo_fd_request(struct request_queue *q)
 			__blk_end_request_cur(req, -EIO);
 			continue;
 		}
-		if (req->current_nr_sectors == 0) {
-			__blk_end_request_cur(req, 0);
-			continue;
-		}
 		if (!fs->disk_in) {
 			__blk_end_request_cur(req, -EIO);
 			continue;
@@ -561,9 +557,6 @@ static void redo_fd_request(struct request_queue *q)
 				__blk_end_request_cur(req, -EIO);
 				continue;
 			}
-			req->nr_sectors -= req->current_nr_sectors;
-			req->sector += req->current_nr_sectors;
-			req->buffer += req->current_nr_sectors * 512;
 			__blk_end_request_cur(req, 0);
 			break;
 		}
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 11/12] mg_disk: fold mg_disk.h into mg_disk.c
  2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
                   ` (9 preceding siblings ...)
  2009-04-23 12:25 ` [PATCH 10/12] swim: " Tejun Heo
@ 2009-04-23 12:25 ` Tejun Heo
  2009-04-23 12:25 ` [PATCH 12/12] mg_disk: clean up request completion paths Tejun Heo
  2009-04-24 19:41 ` [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Bartlomiej Zolnierkiewicz
  12 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-23 12:25 UTC (permalink / raw)
  To: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent
  Cc: Tejun Heo

include/linux/mg_disk.h is used only by drivers/block/mg_disk.c.  No
reason to put it in a separate header.  Fold it into mg_disk.c.

[ Impact: cleanup ]

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: unsik Kim <donari75@gmail.com>
---
 drivers/block/mg_disk.c |  186 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/mg_disk.h |  206 -----------------------------------------------
 2 files changed, 185 insertions(+), 207 deletions(-)
 delete mode 100644 include/linux/mg_disk.h

diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c
index 9d16e2a..bb0ff8c 100644
--- a/drivers/block/mg_disk.c
+++ b/drivers/block/mg_disk.c
@@ -22,10 +22,194 @@
 #include <linux/delay.h>
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
-#include <linux/mg_disk.h>
 
 #define MG_RES_SEC (CONFIG_MG_DISK_RES << 1)
 
+/* name for block device */
+#define MG_DISK_NAME "mgd"
+/* name for platform device */
+#define MG_DEV_NAME "mg_disk"
+
+#define MG_DISK_MAJ 0
+#define MG_DISK_MAX_PART 16
+#define MG_SECTOR_SIZE 512
+#define MG_MAX_SECTS 256
+
+/* Register offsets */
+#define MG_BUFF_OFFSET			0x8000
+#define MG_STORAGE_BUFFER_SIZE		0x200
+#define MG_REG_OFFSET			0xC000
+#define MG_REG_FEATURE			(MG_REG_OFFSET + 2)	/* write case */
+#define MG_REG_ERROR			(MG_REG_OFFSET + 2)	/* read case */
+#define MG_REG_SECT_CNT			(MG_REG_OFFSET + 4)
+#define MG_REG_SECT_NUM			(MG_REG_OFFSET + 6)
+#define MG_REG_CYL_LOW			(MG_REG_OFFSET + 8)
+#define MG_REG_CYL_HIGH			(MG_REG_OFFSET + 0xA)
+#define MG_REG_DRV_HEAD			(MG_REG_OFFSET + 0xC)
+#define MG_REG_COMMAND			(MG_REG_OFFSET + 0xE)	/* write case */
+#define MG_REG_STATUS			(MG_REG_OFFSET + 0xE)	/* read  case */
+#define MG_REG_DRV_CTRL			(MG_REG_OFFSET + 0x10)
+#define MG_REG_BURST_CTRL		(MG_REG_OFFSET + 0x12)
+
+/* "Drive Select/Head Register" bit values */
+#define MG_REG_HEAD_MUST_BE_ON		0xA0 /* These 2 bits are always on */
+#define MG_REG_HEAD_DRIVE_MASTER	(0x00 | MG_REG_HEAD_MUST_BE_ON)
+#define MG_REG_HEAD_DRIVE_SLAVE		(0x10 | MG_REG_HEAD_MUST_BE_ON)
+#define MG_REG_HEAD_LBA_MODE		(0x40 | MG_REG_HEAD_MUST_BE_ON)
+
+
+/* "Device Control Register" bit values */
+#define MG_REG_CTRL_INTR_ENABLE			0x0
+#define MG_REG_CTRL_INTR_DISABLE		(0x1<<1)
+#define MG_REG_CTRL_RESET			(0x1<<2)
+#define MG_REG_CTRL_INTR_POLA_ACTIVE_HIGH	0x0
+#define MG_REG_CTRL_INTR_POLA_ACTIVE_LOW	(0x1<<4)
+#define MG_REG_CTRL_DPD_POLA_ACTIVE_LOW		0x0
+#define MG_REG_CTRL_DPD_POLA_ACTIVE_HIGH	(0x1<<5)
+#define MG_REG_CTRL_DPD_DISABLE			0x0
+#define MG_REG_CTRL_DPD_ENABLE			(0x1<<6)
+
+/* Status register bit */
+/* error bit in status register */
+#define MG_REG_STATUS_BIT_ERROR			0x01
+/* corrected error in status register */
+#define MG_REG_STATUS_BIT_CORRECTED_ERROR	0x04
+/* data request bit in status register */
+#define MG_REG_STATUS_BIT_DATA_REQ		0x08
+/* DSC - Drive Seek Complete */
+#define MG_REG_STATUS_BIT_SEEK_DONE		0x10
+/* DWF - Drive Write Fault */
+#define MG_REG_STATUS_BIT_WRITE_FAULT		0x20
+#define MG_REG_STATUS_BIT_READY			0x40
+#define MG_REG_STATUS_BIT_BUSY			0x80
+
+/* handy status */
+#define MG_STAT_READY	(MG_REG_STATUS_BIT_READY | MG_REG_STATUS_BIT_SEEK_DONE)
+#define MG_READY_OK(s)	(((s) & (MG_STAT_READY | \
+				(MG_REG_STATUS_BIT_BUSY | \
+				 MG_REG_STATUS_BIT_WRITE_FAULT | \
+				 MG_REG_STATUS_BIT_ERROR))) == MG_STAT_READY)
+
+/* Error register */
+#define MG_REG_ERR_AMNF		0x01
+#define MG_REG_ERR_ABRT		0x04
+#define MG_REG_ERR_IDNF		0x10
+#define MG_REG_ERR_UNC		0x40
+#define MG_REG_ERR_BBK		0x80
+
+/* error code for others */
+#define MG_ERR_NONE		0
+#define MG_ERR_TIMEOUT		0x100
+#define MG_ERR_INIT_STAT	0x101
+#define MG_ERR_TRANSLATION	0x102
+#define MG_ERR_CTRL_RST		0x103
+#define MG_ERR_INV_STAT		0x104
+#define MG_ERR_RSTOUT		0x105
+
+#define MG_MAX_ERRORS	6	/* Max read/write errors */
+
+/* command */
+#define MG_CMD_RD 0x20
+#define MG_CMD_WR 0x30
+#define MG_CMD_SLEEP 0x99
+#define MG_CMD_WAKEUP 0xC3
+#define MG_CMD_ID 0xEC
+#define MG_CMD_WR_CONF 0x3C
+#define MG_CMD_RD_CONF 0x40
+
+/* operation mode */
+#define MG_OP_CASCADE (1 << 0)
+#define MG_OP_CASCADE_SYNC_RD (1 << 1)
+#define MG_OP_CASCADE_SYNC_WR (1 << 2)
+#define MG_OP_INTERLEAVE (1 << 3)
+
+/* synchronous */
+#define MG_BURST_LAT_4 (3 << 4)
+#define MG_BURST_LAT_5 (4 << 4)
+#define MG_BURST_LAT_6 (5 << 4)
+#define MG_BURST_LAT_7 (6 << 4)
+#define MG_BURST_LAT_8 (7 << 4)
+#define MG_BURST_LEN_4 (1 << 1)
+#define MG_BURST_LEN_8 (2 << 1)
+#define MG_BURST_LEN_16 (3 << 1)
+#define MG_BURST_LEN_32 (4 << 1)
+#define MG_BURST_LEN_CONT (0 << 1)
+
+/* timeout value (unit: ms) */
+#define MG_TMAX_CONF_TO_CMD	1
+#define MG_TMAX_WAIT_RD_DRQ	10
+#define MG_TMAX_WAIT_WR_DRQ	500
+#define MG_TMAX_RST_TO_BUSY	10
+#define MG_TMAX_HDRST_TO_RDY	500
+#define MG_TMAX_SWRST_TO_RDY	500
+#define MG_TMAX_RSTOUT		3000
+
+/* device attribution */
+/* use mflash as boot device */
+#define MG_BOOT_DEV		(1 << 0)
+/* use mflash as storage device */
+#define MG_STORAGE_DEV		(1 << 1)
+/* same as MG_STORAGE_DEV, but bootloader already done reset sequence */
+#define MG_STORAGE_DEV_SKIP_RST	(1 << 2)
+
+#define MG_DEV_MASK (MG_BOOT_DEV | MG_STORAGE_DEV | MG_STORAGE_DEV_SKIP_RST)
+
+/* names of GPIO resource */
+#define MG_RST_PIN	"mg_rst"
+/* except MG_BOOT_DEV, reset-out pin should be assigned */
+#define MG_RSTOUT_PIN	"mg_rstout"
+
+/* private driver data */
+struct mg_drv_data {
+	/* disk resource */
+	u32 use_polling;
+
+	/* device attribution */
+	u32 dev_attr;
+
+	/* internally used */
+	struct mg_host *host;
+};
+
+/* main structure for mflash driver */
+struct mg_host {
+	struct device *dev;
+
+	struct request_queue *breq;
+	spinlock_t lock;
+	struct gendisk *gd;
+
+	struct timer_list timer;
+	void (*mg_do_intr) (struct mg_host *);
+
+	u16 id[ATA_ID_WORDS];
+
+	u16 cyls;
+	u16 heads;
+	u16 sectors;
+	u32 n_sectors;
+	u32 nres_sectors;
+
+	void __iomem *dev_base;
+	unsigned int irq;
+	unsigned int rst;
+	unsigned int rstout;
+
+	u32 major;
+	u32 error;
+};
+
+/*
+ * Debugging macro and defines
+ */
+#undef DO_MG_DEBUG
+#ifdef DO_MG_DEBUG
+#  define MG_DBG(fmt, args...) \
+	printk(KERN_DEBUG "%s:%d "fmt, __func__, __LINE__, ##args)
+#else /* CONFIG_MG_DEBUG */
+#  define MG_DBG(fmt, args...) do { } while (0)
+#endif /* CONFIG_MG_DEBUG */
+
 static void mg_request(struct request_queue *);
 
 static void mg_dump_status(const char *msg, unsigned int stat,
diff --git a/include/linux/mg_disk.h b/include/linux/mg_disk.h
deleted file mode 100644
index 1f76b1e..0000000
--- a/include/linux/mg_disk.h
+++ /dev/null
@@ -1,206 +0,0 @@
-/*
- *  include/linux/mg_disk.c
- *
- *  Support for the mGine m[g]flash IO mode.
- *  Based on legacy hd.c
- *
- * (c) 2008 mGine Co.,LTD
- * (c) 2008 unsik Kim <donari75@gmail.com>
- *
- *  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.
- */
-
-#ifndef __MG_DISK_H__
-#define __MG_DISK_H__
-
-#include <linux/blkdev.h>
-#include <linux/ata.h>
-
-/* name for block device */
-#define MG_DISK_NAME "mgd"
-/* name for platform device */
-#define MG_DEV_NAME "mg_disk"
-
-#define MG_DISK_MAJ 0
-#define MG_DISK_MAX_PART 16
-#define MG_SECTOR_SIZE 512
-#define MG_MAX_SECTS 256
-
-/* Register offsets */
-#define MG_BUFF_OFFSET			0x8000
-#define MG_STORAGE_BUFFER_SIZE		0x200
-#define MG_REG_OFFSET			0xC000
-#define MG_REG_FEATURE			(MG_REG_OFFSET + 2)	/* write case */
-#define MG_REG_ERROR			(MG_REG_OFFSET + 2)	/* read case */
-#define MG_REG_SECT_CNT			(MG_REG_OFFSET + 4)
-#define MG_REG_SECT_NUM			(MG_REG_OFFSET + 6)
-#define MG_REG_CYL_LOW			(MG_REG_OFFSET + 8)
-#define MG_REG_CYL_HIGH			(MG_REG_OFFSET + 0xA)
-#define MG_REG_DRV_HEAD			(MG_REG_OFFSET + 0xC)
-#define MG_REG_COMMAND			(MG_REG_OFFSET + 0xE)	/* write case */
-#define MG_REG_STATUS			(MG_REG_OFFSET + 0xE)	/* read  case */
-#define MG_REG_DRV_CTRL			(MG_REG_OFFSET + 0x10)
-#define MG_REG_BURST_CTRL		(MG_REG_OFFSET + 0x12)
-
-/* "Drive Select/Head Register" bit values */
-#define MG_REG_HEAD_MUST_BE_ON		0xA0 /* These 2 bits are always on */
-#define MG_REG_HEAD_DRIVE_MASTER	(0x00 | MG_REG_HEAD_MUST_BE_ON)
-#define MG_REG_HEAD_DRIVE_SLAVE		(0x10 | MG_REG_HEAD_MUST_BE_ON)
-#define MG_REG_HEAD_LBA_MODE		(0x40 | MG_REG_HEAD_MUST_BE_ON)
-
-
-/* "Device Control Register" bit values */
-#define MG_REG_CTRL_INTR_ENABLE			0x0
-#define MG_REG_CTRL_INTR_DISABLE		(0x1<<1)
-#define MG_REG_CTRL_RESET			(0x1<<2)
-#define MG_REG_CTRL_INTR_POLA_ACTIVE_HIGH	0x0
-#define MG_REG_CTRL_INTR_POLA_ACTIVE_LOW	(0x1<<4)
-#define MG_REG_CTRL_DPD_POLA_ACTIVE_LOW		0x0
-#define MG_REG_CTRL_DPD_POLA_ACTIVE_HIGH	(0x1<<5)
-#define MG_REG_CTRL_DPD_DISABLE			0x0
-#define MG_REG_CTRL_DPD_ENABLE			(0x1<<6)
-
-/* Status register bit */
-/* error bit in status register */
-#define MG_REG_STATUS_BIT_ERROR			0x01
-/* corrected error in status register */
-#define MG_REG_STATUS_BIT_CORRECTED_ERROR	0x04
-/* data request bit in status register */
-#define MG_REG_STATUS_BIT_DATA_REQ		0x08
-/* DSC - Drive Seek Complete */
-#define MG_REG_STATUS_BIT_SEEK_DONE		0x10
-/* DWF - Drive Write Fault */
-#define MG_REG_STATUS_BIT_WRITE_FAULT		0x20
-#define MG_REG_STATUS_BIT_READY			0x40
-#define MG_REG_STATUS_BIT_BUSY			0x80
-
-/* handy status */
-#define MG_STAT_READY	(MG_REG_STATUS_BIT_READY | MG_REG_STATUS_BIT_SEEK_DONE)
-#define MG_READY_OK(s)	(((s) & (MG_STAT_READY | \
-				(MG_REG_STATUS_BIT_BUSY | \
-				 MG_REG_STATUS_BIT_WRITE_FAULT | \
-				 MG_REG_STATUS_BIT_ERROR))) == MG_STAT_READY)
-
-/* Error register */
-#define MG_REG_ERR_AMNF		0x01
-#define MG_REG_ERR_ABRT		0x04
-#define MG_REG_ERR_IDNF		0x10
-#define MG_REG_ERR_UNC		0x40
-#define MG_REG_ERR_BBK		0x80
-
-/* error code for others */
-#define MG_ERR_NONE		0
-#define MG_ERR_TIMEOUT		0x100
-#define MG_ERR_INIT_STAT	0x101
-#define MG_ERR_TRANSLATION	0x102
-#define MG_ERR_CTRL_RST		0x103
-#define MG_ERR_INV_STAT		0x104
-#define MG_ERR_RSTOUT		0x105
-
-#define MG_MAX_ERRORS	6	/* Max read/write errors */
-
-/* command */
-#define MG_CMD_RD 0x20
-#define MG_CMD_WR 0x30
-#define MG_CMD_SLEEP 0x99
-#define MG_CMD_WAKEUP 0xC3
-#define MG_CMD_ID 0xEC
-#define MG_CMD_WR_CONF 0x3C
-#define MG_CMD_RD_CONF 0x40
-
-/* operation mode */
-#define MG_OP_CASCADE (1 << 0)
-#define MG_OP_CASCADE_SYNC_RD (1 << 1)
-#define MG_OP_CASCADE_SYNC_WR (1 << 2)
-#define MG_OP_INTERLEAVE (1 << 3)
-
-/* synchronous */
-#define MG_BURST_LAT_4 (3 << 4)
-#define MG_BURST_LAT_5 (4 << 4)
-#define MG_BURST_LAT_6 (5 << 4)
-#define MG_BURST_LAT_7 (6 << 4)
-#define MG_BURST_LAT_8 (7 << 4)
-#define MG_BURST_LEN_4 (1 << 1)
-#define MG_BURST_LEN_8 (2 << 1)
-#define MG_BURST_LEN_16 (3 << 1)
-#define MG_BURST_LEN_32 (4 << 1)
-#define MG_BURST_LEN_CONT (0 << 1)
-
-/* timeout value (unit: ms) */
-#define MG_TMAX_CONF_TO_CMD	1
-#define MG_TMAX_WAIT_RD_DRQ	10
-#define MG_TMAX_WAIT_WR_DRQ	500
-#define MG_TMAX_RST_TO_BUSY	10
-#define MG_TMAX_HDRST_TO_RDY	500
-#define MG_TMAX_SWRST_TO_RDY	500
-#define MG_TMAX_RSTOUT		3000
-
-/* device attribution */
-/* use mflash as boot device */
-#define MG_BOOT_DEV		(1 << 0)
-/* use mflash as storage device */
-#define MG_STORAGE_DEV		(1 << 1)
-/* same as MG_STORAGE_DEV, but bootloader already done reset sequence */
-#define MG_STORAGE_DEV_SKIP_RST	(1 << 2)
-
-#define MG_DEV_MASK (MG_BOOT_DEV | MG_STORAGE_DEV | MG_STORAGE_DEV_SKIP_RST)
-
-/* names of GPIO resource */
-#define MG_RST_PIN	"mg_rst"
-/* except MG_BOOT_DEV, reset-out pin should be assigned */
-#define MG_RSTOUT_PIN	"mg_rstout"
-
-/* private driver data */
-struct mg_drv_data {
-	/* disk resource */
-	u32 use_polling;
-
-	/* device attribution */
-	u32 dev_attr;
-
-	/* internally used */
-	struct mg_host *host;
-};
-
-/* main structure for mflash driver */
-struct mg_host {
-	struct device *dev;
-
-	struct request_queue *breq;
-	spinlock_t lock;
-	struct gendisk *gd;
-
-	struct timer_list timer;
-	void (*mg_do_intr) (struct mg_host *);
-
-	u16 id[ATA_ID_WORDS];
-
-	u16 cyls;
-	u16 heads;
-	u16 sectors;
-	u32 n_sectors;
-	u32 nres_sectors;
-
-	void __iomem *dev_base;
-	unsigned int irq;
-	unsigned int rst;
-	unsigned int rstout;
-
-	u32 major;
-	u32 error;
-};
-
-/*
- * Debugging macro and defines
- */
-#undef DO_MG_DEBUG
-#ifdef DO_MG_DEBUG
-#  define MG_DBG(fmt, args...) \
-	printk(KERN_DEBUG "%s:%d "fmt, __func__, __LINE__, ##args)
-#else /* CONFIG_MG_DEBUG */
-#  define MG_DBG(fmt, args...) do { } while (0)
-#endif /* CONFIG_MG_DEBUG */
-
-#endif
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 12/12] mg_disk: clean up request completion paths
  2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
                   ` (10 preceding siblings ...)
  2009-04-23 12:25 ` [PATCH 11/12] mg_disk: fold mg_disk.h into mg_disk.c Tejun Heo
@ 2009-04-23 12:25 ` Tejun Heo
  2009-04-24 19:41 ` [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Bartlomiej Zolnierkiewicz
  12 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-23 12:25 UTC (permalink / raw)
  To: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent
  Cc: Tejun Heo

mg_disk implements its own partial completion.  Convert to standard
block layer partial completion.

[ Impact: cleanup ]

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: unsik Kim <donari75@gmail.com>
---
 drivers/block/mg_disk.c |  117 +++++++++++++---------------------------------
 1 files changed, 33 insertions(+), 84 deletions(-)

diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c
index bb0ff8c..95e08c0 100644
--- a/drivers/block/mg_disk.c
+++ b/drivers/block/mg_disk.c
@@ -503,95 +503,68 @@ static unsigned int mg_out(struct mg_host *host,
 
 static void mg_read(struct request *req)
 {
-	u32 remains, j;
+	u32 j;
 	struct mg_host *host = req->rq_disk->private_data;
 
-	remains = req->nr_sectors;
-
 	if (mg_out(host, req->sector, req->nr_sectors, MG_CMD_RD, NULL) !=
 			MG_ERR_NONE)
 		mg_bad_rw_intr(host);
 
 	MG_DBG("requested %d sects (from %ld), buffer=0x%p\n",
-			remains, req->sector, req->buffer);
+	       req->nr_sectors, req->sector, req->buffer);
+
+	do {
+		u16 *buff = (u16 *)req->buffer;
 
-	while (remains) {
 		if (mg_wait(host, MG_REG_STATUS_BIT_DATA_REQ,
 					MG_TMAX_WAIT_RD_DRQ) != MG_ERR_NONE) {
 			mg_bad_rw_intr(host);
 			return;
 		}
-		for (j = 0; j < MG_SECTOR_SIZE >> 1; j++) {
-			*(u16 *)req->buffer =
-				inw((unsigned long)host->dev_base +
-						MG_BUFF_OFFSET + (j << 1));
-			req->buffer += 2;
-		}
-
-		req->sector++;
-		req->errors = 0;
-		remains = --req->nr_sectors;
-		--req->current_nr_sectors;
-
-		if (req->current_nr_sectors <= 0) {
-			MG_DBG("remain : %d sects\n", remains);
-			__blk_end_request_cur(req, 0);
-			if (remains > 0)
-				req = elv_next_request(host->breq);
-		}
+		for (j = 0; j < MG_SECTOR_SIZE >> 1; j++)
+			*buff++ = inw((unsigned long)host->dev_base +
+				      MG_BUFF_OFFSET + (j << 1));
 
 		outb(MG_CMD_RD_CONF, (unsigned long)host->dev_base +
 				MG_REG_COMMAND);
-	}
+	} while (__blk_end_request(req, 0, MG_SECTOR_SIZE));
 }
 
 static void mg_write(struct request *req)
 {
-	u32 remains, j;
+	u32 j;
 	struct mg_host *host = req->rq_disk->private_data;
 
-	remains = req->nr_sectors;
-
 	if (mg_out(host, req->sector, req->nr_sectors, MG_CMD_WR, NULL) !=
 			MG_ERR_NONE) {
 		mg_bad_rw_intr(host);
 		return;
 	}
 
-
 	MG_DBG("requested %d sects (from %ld), buffer=0x%p\n",
-			remains, req->sector, req->buffer);
-	while (remains) {
+	       req->nr_sectors, req->sector, req->buffer);
+
+	do {
+		u16 *buff = (u16 *)req->buffer;
+
 		if (mg_wait(host, MG_REG_STATUS_BIT_DATA_REQ,
 					MG_TMAX_WAIT_WR_DRQ) != MG_ERR_NONE) {
 			mg_bad_rw_intr(host);
 			return;
 		}
-		for (j = 0; j < MG_SECTOR_SIZE >> 1; j++) {
-			outw(*(u16 *)req->buffer,
-					(unsigned long)host->dev_base +
-					MG_BUFF_OFFSET + (j << 1));
-			req->buffer += 2;
-		}
-		req->sector++;
-		remains = --req->nr_sectors;
-		--req->current_nr_sectors;
-
-		if (req->current_nr_sectors <= 0) {
-			MG_DBG("remain : %d sects\n", remains);
-			__blk_end_request_cur(req, 0);
-			if (remains > 0)
-				req = elv_next_request(host->breq);
-		}
+		for (j = 0; j < MG_SECTOR_SIZE >> 1; j++)
+			outw(*buff++, (unsigned long)host->dev_base +
+				      MG_BUFF_OFFSET + (j << 1));
 
 		outb(MG_CMD_WR_CONF, (unsigned long)host->dev_base +
 				MG_REG_COMMAND);
-	}
+	} while (__blk_end_request(req, 0, MG_SECTOR_SIZE));
 }
 
 static void mg_read_intr(struct mg_host *host)
 {
 	u32 i;
+	u16 *buff;
 	struct request *req;
 
 	/* check status */
@@ -612,39 +585,24 @@ static void mg_read_intr(struct mg_host *host)
 ok_to_read:
 	/* get current segment of request */
 	req = elv_next_request(host->breq);
+	buff = (u16 *)req->buffer;
 
 	/* read 1 sector */
-	for (i = 0; i < MG_SECTOR_SIZE >> 1; i++) {
-		*(u16 *)req->buffer =
-			inw((unsigned long)host->dev_base + MG_BUFF_OFFSET +
-					(i << 1));
-		req->buffer += 2;
-	}
+	for (i = 0; i < MG_SECTOR_SIZE >> 1; i++)
+		*buff++ = inw((unsigned long)host->dev_base + MG_BUFF_OFFSET +
+			      (i << 1));
 
-	/* manipulate request */
 	MG_DBG("sector %ld, remaining=%ld, buffer=0x%p\n",
 			req->sector, req->nr_sectors - 1, req->buffer);
 
-	req->sector++;
-	req->errors = 0;
-	i = --req->nr_sectors;
-	--req->current_nr_sectors;
-
-	/* let know if current segment done */
-	if (req->current_nr_sectors <= 0)
-		__blk_end_request_cur(req, 0);
-
-	/* set handler if read remains */
-	if (i > 0) {
-		host->mg_do_intr = mg_read_intr;
-		mod_timer(&host->timer, jiffies + 3 * HZ);
-	}
-
 	/* send read confirm */
 	outb(MG_CMD_RD_CONF, (unsigned long)host->dev_base + MG_REG_COMMAND);
 
-	/* goto next request */
-	if (!i)
+	if (__blk_end_request(req, 0, MG_SECTOR_SIZE)) {
+		/* set handler if read remains */
+		host->mg_do_intr = mg_read_intr;
+		mod_timer(&host->timer, jiffies + 3 * HZ);
+	} else /* goto next request */
 		mg_request(host->breq);
 }
 
@@ -653,6 +611,7 @@ static void mg_write_intr(struct mg_host *host)
 	u32 i, j;
 	u16 *buff;
 	struct request *req;
+	bool rem;
 
 	/* get current segment of request */
 	req = elv_next_request(host->breq);
@@ -673,18 +632,8 @@ static void mg_write_intr(struct mg_host *host)
 	return;
 
 ok_to_write:
-	/* manipulate request */
-	req->sector++;
-	i = --req->nr_sectors;
-	--req->current_nr_sectors;
-	req->buffer += MG_SECTOR_SIZE;
-
-	/* let know if current segment or all done */
-	if (!i || (req->bio && req->current_nr_sectors <= 0))
-		__blk_end_request_cur(req, 0);
-
-	/* write 1 sector and set handler if remains */
-	if (i > 0) {
+	if ((rem = __blk_end_request(req, 0, MG_SECTOR_SIZE))) {
+		/* write 1 sector and set handler if remains */
 		buff = (u16 *)req->buffer;
 		for (j = 0; j < MG_STORAGE_BUFFER_SIZE >> 1; j++) {
 			outw(*buff, (unsigned long)host->dev_base +
@@ -700,7 +649,7 @@ ok_to_write:
 	/* send write confirm */
 	outb(MG_CMD_WR_CONF, (unsigned long)host->dev_base + MG_REG_COMMAND);
 
-	if (!i)
+	if (!rem)
 		mg_request(host->breq);
 }
 
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 05/12] sunvdc: kill vdc_end_request()
  2009-04-23 12:25 ` [PATCH 05/12] sunvdc: kill vdc_end_request() Tejun Heo
@ 2009-04-23 13:33   ` David Miller
  2009-04-28  2:54     ` Tejun Heo
  0 siblings, 1 reply; 19+ messages in thread
From: David Miller @ 2009-04-23 13:33 UTC (permalink / raw)
  To: tj; +Cc: axboe, linux-kernel, joerg, geert, donari75, jdike, benh, Laurent

From: Tejun Heo <tj@kernel.org>
Date: Thu, 23 Apr 2009 21:25:46 +0900

> vdc_end_request() is a thin silly wrapper on top of
> __blk_end_request().  Kill it.
> 
> [ Impact: cleanup ]
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches
  2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
                   ` (11 preceding siblings ...)
  2009-04-23 12:25 ` [PATCH 12/12] mg_disk: clean up request completion paths Tejun Heo
@ 2009-04-24 19:41 ` Bartlomiej Zolnierkiewicz
  2009-04-28  3:44   ` Tejun Heo
  12 siblings, 1 reply; 19+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-04-24 19:41 UTC (permalink / raw)
  To: Tejun Heo
  Cc: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent

On Thursday 23 April 2009 14:25:41 Tejun Heo wrote:
> Hello,
> 
> This is a RFC patchset.  Please do NOT pull directly from the git
> tree.
> 
> This patchset contains block lld cleanups and is consisted of the
> following 12 patches.
> 
>  0001-block-update-end_cur.patch
>  0002-block-don-t-init-rq-fields-unnecessarily.patch
>  0003-amiflop-ataflop-xd-mg_disk-clean-up-unnecessary-stu.patch
>  0004-ps3disk-simplify-request-completion.patch
>  0005-sunvdc-kill-vdc_end_request.patch
>  0006-ubd-cleanup-completion-path.patch
>  0007-ubd-drop-unnecessary-rq-sector-manipulation.patch
>  0008-hd-clean-up-request-completion-paths.patch
>  0009-swim3-clean-up-request-completion-paths.patch
>  0010-swim-clean-up-request-completion-paths.patch
>  0011-mg_disk-fold-mg_disk.h-into-mg_disk.c.patch
>  0012-mg_disk-clean-up-request-completion-paths.patch
> 
> 0001 fixes a stupid mistake while implementing blk_request_end_cur()
> and 0002 and 0003 are pretty straight forward cleanup.  Please feel
> free to apply these three patches.
> 
> 0004-0012 are cleanup patches for various block low level drivers
> mostly focusing on completion paths.  The biggest change is use of
> standard block layer mechanisms for partial completion instead of
> doing it by manipulating request directly.  All the touched drivers
> are only compile tested.
> 
> I'm planning on testing hd, xd and um but can't test others.  Can you
> guys please verify the following drivers work?
> 
>  ps3disk, sunvdc, swim3, swim and mg_disk.
> 
> The following git tree can be used for testing.
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git rfc-block-lld-cleanup

I finally found some time to catch up with your patchsets and everything
looks good so far. :)

BTW while going through mg_disk changes I noticed that this driver may
need the following fix (just a RFC on top of your patchset, feel free to
take it over if unsik acks the change):

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [RFC][PATCH] mg_disk: fix issue with data integrity on error in mg_write()

We cannot acknowledge the sector write before checking its status
(which is done on the next loop iteration) and we also need to do
the final status register check after writing the last sector.

Fix mg_write() to match mg_write_intr() in this regard.

While at it:
- add mg_read_one() and mg_write_one() helpers
- always use MG_SECTOR_SIZE and remove MG_STORAGE_BUFFER_SIZE

Cc: unsik Kim <donari75@gmail.com>
Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/block/mg_disk.c |   87 +++++++++++++++++++++++++-----------------------
 1 file changed, 47 insertions(+), 40 deletions(-)

Index: b/drivers/block/mg_disk.c
===================================================================
--- a/drivers/block/mg_disk.c
+++ b/drivers/block/mg_disk.c
@@ -37,7 +37,6 @@
 
 /* Register offsets */
 #define MG_BUFF_OFFSET			0x8000
-#define MG_STORAGE_BUFFER_SIZE		0x200
 #define MG_REG_OFFSET			0xC000
 #define MG_REG_FEATURE			(MG_REG_OFFSET + 2)	/* write case */
 #define MG_REG_ERROR			(MG_REG_OFFSET + 2)	/* read case */
@@ -501,9 +500,18 @@ static unsigned int mg_out(struct mg_hos
 	return MG_ERR_NONE;
 }
 
+static void mg_read_one(struct mg_host *host, struct request *req)
+{
+	u16 *buff = (u16 *)req->buffer;
+	u32 i;
+
+	for (i = 0; i < MG_SECTOR_SIZE >> 1; i++)
+		*buff++ = inw((unsigned long)host->dev_base + MG_BUFF_OFFSET +
+			      (i << 1));
+}
+
 static void mg_read(struct request *req)
 {
-	u32 j;
 	struct mg_host *host = req->rq_disk->private_data;
 
 	if (mg_out(host, req->sector, req->nr_sectors, MG_CMD_RD, NULL) !=
@@ -514,26 +522,33 @@ static void mg_read(struct request *req)
 	       req->nr_sectors, req->sector, req->buffer);
 
 	do {
-		u16 *buff = (u16 *)req->buffer;
-
 		if (mg_wait(host, MG_REG_STATUS_BIT_DATA_REQ,
 					MG_TMAX_WAIT_RD_DRQ) != MG_ERR_NONE) {
 			mg_bad_rw_intr(host);
 			return;
 		}
-		for (j = 0; j < MG_SECTOR_SIZE >> 1; j++)
-			*buff++ = inw((unsigned long)host->dev_base +
-				      MG_BUFF_OFFSET + (j << 1));
+
+		mg_read_one(host, req);
 
 		outb(MG_CMD_RD_CONF, (unsigned long)host->dev_base +
 				MG_REG_COMMAND);
 	} while (__blk_end_request(req, 0, MG_SECTOR_SIZE));
 }
 
+static void mg_write_one(struct mg_host *host, struct request *req)
+{
+	u16 *buff = (u16 *)req->buffer;
+	u32 i;
+
+	for (i = 0; i < MG_SECTOR_SIZE >> 1; i++)
+		outw(*buff++, (unsigned long)host->dev_base + MG_BUFF_OFFSET +
+		     (i << 1));
+}
+
 static void mg_write(struct request *req)
 {
-	u32 j;
 	struct mg_host *host = req->rq_disk->private_data;
+	bool rem;
 
 	if (mg_out(host, req->sector, req->nr_sectors, MG_CMD_WR, NULL) !=
 			MG_ERR_NONE) {
@@ -544,28 +559,37 @@ static void mg_write(struct request *req
 	MG_DBG("requested %d sects (from %ld), buffer=0x%p\n",
 	       req->nr_sectors, req->sector, req->buffer);
 
-	do {
-		u16 *buff = (u16 *)req->buffer;
+	if (mg_wait(host, MG_REG_STATUS_BIT_DATA_REQ,
+			MG_TMAX_WAIT_WR_DRQ) != MG_ERR_NONE) {
+		mg_bad_rw_intr(host);
+		return;
+	}
 
-		if (mg_wait(host, MG_REG_STATUS_BIT_DATA_REQ,
-					MG_TMAX_WAIT_WR_DRQ) != MG_ERR_NONE) {
+	mg_write_one(host, req);
+
+	outb(MG_CMD_WR_CONF, (unsigned long)host->dev_base + MG_REG_COMMAND);
+
+	do {
+		if ((req->nr_sectors > 1) &&
+		     mg_wait(host, MG_REG_STATUS_BIT_DATA_REQ,
+				MG_TMAX_WAIT_WR_DRQ) != MG_ERR_NONE) {
 			mg_bad_rw_intr(host);
 			return;
 		}
-		for (j = 0; j < MG_SECTOR_SIZE >> 1; j++)
-			outw(*buff++, (unsigned long)host->dev_base +
-				      MG_BUFF_OFFSET + (j << 1));
+
+		rem = __blk_end_request(req, 0, MG_SECTOR_SIZE);
+		if (rem)
+			mg_write_one(host, req);
 
 		outb(MG_CMD_WR_CONF, (unsigned long)host->dev_base +
-				MG_REG_COMMAND);
-	} while (__blk_end_request(req, 0, MG_SECTOR_SIZE));
+			MG_REG_COMMAND);
+	} while (rem);
 }
 
 static void mg_read_intr(struct mg_host *host)
 {
-	u32 i;
-	u16 *buff;
 	struct request *req;
+	u32 i;
 
 	/* check status */
 	do {
@@ -585,12 +609,9 @@ static void mg_read_intr(struct mg_host 
 ok_to_read:
 	/* get current segment of request */
 	req = elv_next_request(host->breq);
-	buff = (u16 *)req->buffer;
 
 	/* read 1 sector */
-	for (i = 0; i < MG_SECTOR_SIZE >> 1; i++)
-		*buff++ = inw((unsigned long)host->dev_base + MG_BUFF_OFFSET +
-			      (i << 1));
+	mg_read_one(host, req);
 
 	MG_DBG("sector %ld, remaining=%ld, buffer=0x%p\n",
 			req->sector, req->nr_sectors - 1, req->buffer);
@@ -608,9 +629,8 @@ ok_to_read:
 
 static void mg_write_intr(struct mg_host *host)
 {
-	u32 i, j;
-	u16 *buff;
 	struct request *req;
+	u32 i;
 	bool rem;
 
 	/* get current segment of request */
@@ -634,12 +654,7 @@ static void mg_write_intr(struct mg_host
 ok_to_write:
 	if ((rem = __blk_end_request(req, 0, MG_SECTOR_SIZE))) {
 		/* write 1 sector and set handler if remains */
-		buff = (u16 *)req->buffer;
-		for (j = 0; j < MG_STORAGE_BUFFER_SIZE >> 1; j++) {
-			outw(*buff, (unsigned long)host->dev_base +
-					MG_BUFF_OFFSET + (j << 1));
-			buff++;
-		}
+		mg_write_one(host, req);
 		MG_DBG("sector %ld, remaining=%ld, buffer=0x%p\n",
 				req->sector, req->nr_sectors, req->buffer);
 		host->mg_do_intr = mg_write_intr;
@@ -703,9 +718,6 @@ static unsigned int mg_issue_req(struct 
 		unsigned int sect_num,
 		unsigned int sect_cnt)
 {
-	u16 *buff;
-	u32 i;
-
 	switch (rq_data_dir(req)) {
 	case READ:
 		if (mg_out(host, sect_num, sect_cnt, MG_CMD_RD, &mg_read_intr)
@@ -732,12 +744,7 @@ static unsigned int mg_issue_req(struct 
 			mg_bad_rw_intr(host);
 			return host->error;
 		}
-		buff = (u16 *)req->buffer;
-		for (i = 0; i < MG_SECTOR_SIZE >> 1; i++) {
-			outw(*buff, (unsigned long)host->dev_base +
-					MG_BUFF_OFFSET + (i << 1));
-			buff++;
-		}
+		mg_write_one(host, req);
 		mod_timer(&host->timer, jiffies + 3 * HZ);
 		outb(MG_CMD_WR_CONF, (unsigned long)host->dev_base +
 				MG_REG_COMMAND);

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 05/12] sunvdc: kill vdc_end_request()
  2009-04-23 13:33   ` David Miller
@ 2009-04-28  2:54     ` Tejun Heo
  0 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-28  2:54 UTC (permalink / raw)
  To: David Miller
  Cc: axboe, linux-kernel, joerg, geert, donari75, jdike, benh, Laurent

David Miller wrote:
> From: Tejun Heo <tj@kernel.org>
> Date: Thu, 23 Apr 2009 21:25:46 +0900
> 
>> vdc_end_request() is a thin silly wrapper on top of
>> __blk_end_request().  Kill it.
>>
>> [ Impact: cleanup ]
>>
>> Signed-off-by: Tejun Heo <tj@kernel.org>
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Thanks.  Acked-by added to the patch.

-- 
tejun

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches
  2009-04-24 19:41 ` [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Bartlomiej Zolnierkiewicz
@ 2009-04-28  3:44   ` Tejun Heo
  2009-04-28 18:38     ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 19+ messages in thread
From: Tejun Heo @ 2009-04-28  3:44 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent

Hello, Bartlomiej.

Bartlomiej Zolnierkiewicz wrote:
> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Subject: [RFC][PATCH] mg_disk: fix issue with data integrity on error in mg_write()
> 
> We cannot acknowledge the sector write before checking its status
> (which is done on the next loop iteration) and we also need to do
> the final status register check after writing the last sector.
> 
> Fix mg_write() to match mg_write_intr() in this regard.
> 
> While at it:
> - add mg_read_one() and mg_write_one() helpers
> - always use MG_SECTOR_SIZE and remove MG_STORAGE_BUFFER_SIZE
> 
> Cc: unsik Kim <donari75@gmail.com>
> Cc: Tejun Heo <tj@kernel.org>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

I folded other three patches into my series.  This one looks good to me
too but unlike others this one actually changes how the driver
interacts with the device so I think it would be better to wait for
unsik's ack on this one (I'm pushing out others).

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches
  2009-04-28  3:44   ` Tejun Heo
@ 2009-04-28 18:38     ` Bartlomiej Zolnierkiewicz
  2009-04-28 22:36       ` Tejun Heo
  0 siblings, 1 reply; 19+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-04-28 18:38 UTC (permalink / raw)
  To: Tejun Heo
  Cc: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent

On Tuesday 28 April 2009 05:44:07 Tejun Heo wrote:
> Hello, Bartlomiej.
> 
> Bartlomiej Zolnierkiewicz wrote:
> > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > Subject: [RFC][PATCH] mg_disk: fix issue with data integrity on error in mg_write()
> > 
> > We cannot acknowledge the sector write before checking its status
> > (which is done on the next loop iteration) and we also need to do
> > the final status register check after writing the last sector.
> > 
> > Fix mg_write() to match mg_write_intr() in this regard.
> > 
> > While at it:
> > - add mg_read_one() and mg_write_one() helpers
> > - always use MG_SECTOR_SIZE and remove MG_STORAGE_BUFFER_SIZE
> > 
> > Cc: unsik Kim <donari75@gmail.com>
> > Cc: Tejun Heo <tj@kernel.org>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> 
> I folded other three patches into my series.  This one looks good to me
> too but unlike others this one actually changes how the driver
> interacts with the device so I think it would be better to wait for
> unsik's ack on this one (I'm pushing out others).

Fine with me and thanks for handling it.  BTW do you still have version this
patch re-synced on top of other changes around?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches
  2009-04-28 18:38     ` Bartlomiej Zolnierkiewicz
@ 2009-04-28 22:36       ` Tejun Heo
  0 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-04-28 22:36 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: axboe, linux-kernel, joerg, geert, donari75, davem, jdike, benh, Laurent

Bartlomiej Zolnierkiewicz wrote:
> Fine with me and thanks for handling it.  BTW do you still have version this
> patch re-synced on top of other changes around?

Yeap, inlined below.

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: mg_disk: fix issue with data integrity on error in mg_write()

We cannot acknowledge the sector write before checking its status
(which is done on the next loop iteration) and we also need to do
the final status register check after writing the last sector.

Fix mg_write() to match mg_write_intr() in this regard.

While at it:
- add mg_read_one() and mg_write_one() helpers
- always use MG_SECTOR_SIZE and remove MG_STORAGE_BUFFER_SIZE

Cc: unsik Kim <donari75@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/block/mg_disk.c |   85 ++++++++++++++++++++++++++----------------------
 1 file changed, 47 insertions(+), 38 deletions(-)

Index: block/drivers/block/mg_disk.c
===================================================================
--- block.orig/drivers/block/mg_disk.c
+++ block/drivers/block/mg_disk.c
@@ -37,7 +37,6 @@
 
 /* Register offsets */
 #define MG_BUFF_OFFSET			0x8000
-#define MG_STORAGE_BUFFER_SIZE		0x200
 #define MG_REG_OFFSET			0xC000
 #define MG_REG_FEATURE			(MG_REG_OFFSET + 2)	/* write case */
 #define MG_REG_ERROR			(MG_REG_OFFSET + 2)	/* read case */
@@ -488,9 +487,18 @@ static unsigned int mg_out(struct mg_hos
 	return MG_ERR_NONE;
 }
 
+static void mg_read_one(struct mg_host *host, struct request *req)
+{
+	u16 *buff = (u16 *)req->buffer;
+	u32 i;
+
+	for (i = 0; i < MG_SECTOR_SIZE >> 1; i++)
+		*buff++ = inw((unsigned long)host->dev_base + MG_BUFF_OFFSET +
+			      (i << 1));
+}
+
 static void mg_read(struct request *req)
 {
-	u32 j;
 	struct mg_host *host = req->rq_disk->private_data;
 
 	if (mg_out(host, req->sector, req->nr_sectors, MG_CMD_RD, NULL) !=
@@ -501,26 +509,33 @@ static void mg_read(struct request *req)
 	       req->nr_sectors, req->sector, req->buffer);
 
 	do {
-		u16 *buff = (u16 *)req->buffer;
-
 		if (mg_wait(host, ATA_DRQ,
 			    MG_TMAX_WAIT_RD_DRQ) != MG_ERR_NONE) {
 			mg_bad_rw_intr(host);
 			return;
 		}
-		for (j = 0; j < MG_SECTOR_SIZE >> 1; j++)
-			*buff++ = inw((unsigned long)host->dev_base +
-				      MG_BUFF_OFFSET + (j << 1));
+
+		mg_read_one(host, req);
 
 		outb(MG_CMD_RD_CONF, (unsigned long)host->dev_base +
 				MG_REG_COMMAND);
 	} while (__blk_end_request(req, 0, MG_SECTOR_SIZE));
 }
 
+static void mg_write_one(struct mg_host *host, struct request *req)
+{
+	u16 *buff = (u16 *)req->buffer;
+	u32 i;
+
+	for (i = 0; i < MG_SECTOR_SIZE >> 1; i++)
+		outw(*buff++, (unsigned long)host->dev_base + MG_BUFF_OFFSET +
+		     (i << 1));
+}
+
 static void mg_write(struct request *req)
 {
-	u32 j;
 	struct mg_host *host = req->rq_disk->private_data;
+	bool rem;
 
 	if (mg_out(host, req->sector, req->nr_sectors, MG_CMD_WR, NULL) !=
 			MG_ERR_NONE) {
@@ -531,20 +546,31 @@ static void mg_write(struct request *req
 	MG_DBG("requested %d sects (from %ld), buffer=0x%p\n",
 	       req->nr_sectors, req->sector, req->buffer);
 
-	do {
-		u16 *buff = (u16 *)req->buffer;
+	if (mg_wait(host, MG_REG_STATUS_BIT_DATA_REQ,
+		    MG_TMAX_WAIT_WR_DRQ) != MG_ERR_NONE) {
+		mg_bad_rw_intr(host);
+		return;
+	}
+
+	mg_write_one(host, req);
+
+	outb(MG_CMD_WR_CONF, (unsigned long)host->dev_base + MG_REG_COMMAND);
 
-	if (mg_wait(host, ATA_DRQ, MG_TMAX_WAIT_WR_DRQ) != MG_ERR_NONE) {
+	do {
+		if ((req->nr_sectors > 1) &&
+		     mg_wait(host, MG_REG_STATUS_BIT_DATA_REQ,
+			     MG_TMAX_WAIT_WR_DRQ) != MG_ERR_NONE) {
 			mg_bad_rw_intr(host);
 			return;
 		}
-		for (j = 0; j < MG_SECTOR_SIZE >> 1; j++)
-			outw(*buff++, (unsigned long)host->dev_base +
-				      MG_BUFF_OFFSET + (j << 1));
 
-		outb(MG_CMD_WR_CONF, (unsigned long)host->dev_base +
-				MG_REG_COMMAND);
-	} while (__blk_end_request(req, 0, MG_SECTOR_SIZE));
+		rem = __blk_end_request(req, 0, MG_SECTOR_SIZE);
+		if (rem)
+			mg_write_one(host, req);
+
+		outb(MG_CMD_WR_CONF,
+		     (unsigned long)host->dev_base + MG_REG_COMMAND);
+	} while (rem);
 }
 
 static void mg_read_intr(struct mg_host *host)
@@ -571,12 +597,9 @@ static void mg_read_intr(struct mg_host
 ok_to_read:
 	/* get current segment of request */
 	req = elv_next_request(host->breq);
-	buff = (u16 *)req->buffer;
 
 	/* read 1 sector */
-	for (i = 0; i < MG_SECTOR_SIZE >> 1; i++)
-		*buff++ = inw((unsigned long)host->dev_base + MG_BUFF_OFFSET +
-			      (i << 1));
+	mg_read_one(host, req);
 
 	MG_DBG("sector %ld, remaining=%ld, buffer=0x%p\n",
 			req->sector, req->nr_sectors - 1, req->buffer);
@@ -594,9 +617,8 @@ ok_to_read:
 
 static void mg_write_intr(struct mg_host *host)
 {
-	u32 i, j;
-	u16 *buff;
 	struct request *req;
+	u32 i;
 	bool rem;
 
 	/* get current segment of request */
@@ -620,12 +642,7 @@ static void mg_write_intr(struct mg_host
 ok_to_write:
 	if ((rem = __blk_end_request(req, 0, MG_SECTOR_SIZE))) {
 		/* write 1 sector and set handler if remains */
-		buff = (u16 *)req->buffer;
-		for (j = 0; j < MG_STORAGE_BUFFER_SIZE >> 1; j++) {
-			outw(*buff, (unsigned long)host->dev_base +
-					MG_BUFF_OFFSET + (j << 1));
-			buff++;
-		}
+		mg_write_one(host, req);
 		MG_DBG("sector %ld, remaining=%ld, buffer=0x%p\n",
 				req->sector, req->nr_sectors, req->buffer);
 		host->mg_do_intr = mg_write_intr;
@@ -689,9 +706,6 @@ static unsigned int mg_issue_req(struct
 		unsigned int sect_num,
 		unsigned int sect_cnt)
 {
-	u16 *buff;
-	u32 i;
-
 	switch (rq_data_dir(req)) {
 	case READ:
 		if (mg_out(host, sect_num, sect_cnt, MG_CMD_RD, &mg_read_intr)
@@ -715,12 +729,7 @@ static unsigned int mg_issue_req(struct
 			mg_bad_rw_intr(host);
 			return host->error;
 		}
-		buff = (u16 *)req->buffer;
-		for (i = 0; i < MG_SECTOR_SIZE >> 1; i++) {
-			outw(*buff, (unsigned long)host->dev_base +
-					MG_BUFF_OFFSET + (i << 1));
-			buff++;
-		}
+		mg_write_one(host, req);
 		mod_timer(&host->timer, jiffies + 3 * HZ);
 		outb(MG_CMD_WR_CONF, (unsigned long)host->dev_base +
 				MG_REG_COMMAND);

-- 
tejun

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2009-04-28 22:36 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-23 12:25 [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Tejun Heo
2009-04-23 12:25 ` [PATCH 01/12] block-update-end_cur Tejun Heo
2009-04-23 12:25 ` [PATCH 02/12] block: don't init rq fields unnecessarily Tejun Heo
2009-04-23 12:25 ` [PATCH 03/12] amiflop,ataflop,xd,mg_disk: clean up unnecessary stuff from block drivers Tejun Heo
2009-04-23 12:25 ` [PATCH 04/12] ps3disk: simplify request completion Tejun Heo
2009-04-23 12:25 ` [PATCH 05/12] sunvdc: kill vdc_end_request() Tejun Heo
2009-04-23 13:33   ` David Miller
2009-04-28  2:54     ` Tejun Heo
2009-04-23 12:25 ` [PATCH 06/12] ubd: cleanup completion path Tejun Heo
2009-04-23 12:25 ` [PATCH 07/12] ubd: drop unnecessary rq->sector manipulation Tejun Heo
2009-04-23 12:25 ` [PATCH 08/12] hd: clean up request completion paths Tejun Heo
2009-04-23 12:25 ` [PATCH 09/12] swim3: " Tejun Heo
2009-04-23 12:25 ` [PATCH 10/12] swim: " Tejun Heo
2009-04-23 12:25 ` [PATCH 11/12] mg_disk: fold mg_disk.h into mg_disk.c Tejun Heo
2009-04-23 12:25 ` [PATCH 12/12] mg_disk: clean up request completion paths Tejun Heo
2009-04-24 19:41 ` [RFC PATCHSET linux-2.6-block#for-2.6.31] block: lld cleanup patches Bartlomiej Zolnierkiewicz
2009-04-28  3:44   ` Tejun Heo
2009-04-28 18:38     ` Bartlomiej Zolnierkiewicz
2009-04-28 22:36       ` Tejun Heo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.