[PATCH v3 24/33] swim: Don't needlessly re-read sectors

[Finn Thain]

floppy_read_sectors() is confusing because the `track' variable seems to
conflate tracks and cylinders. Rename this variable, eliminate a division
operation and adopt suitable integer types.

For readahead to work effectively, small sequential reads should not
require waiting for spindle rotation. Unfortunately, the present algorithm
is very inefficient and does a lot of unnecessary waiting.

E.g. if the device is asked to read sectors 1 thru 16, and if sector 9
happens to be under the heads, the driver will proceed to read sectors 9
thru 18, but discard the results, while it waits for sector 1 to arrive.

If sector 1 couldn't be read on the first attempt and needs a retry, the
driver will proceed to read sectors 2 thru 18, but discard the results,
while it waits for sector 1 to come around again.

In between reading sector 1 and sector 2, the driver needlessly calls
swim_track() and swim_head() again. But what's worse is re-enabling
interrupts after each sector, because on a 68030 system this can result
in a full rotation between sectors (which would be a 200 ms wait).

Floppy drivers usually implement a track cache that can be filled in a
single rotation to solve such problems. But I think there is a simpler
solution.

After stepping the heads, use a sector bitmap to record sectors that were
successfully read from the present track. Read (or retry, if need be) the
requested sectors in whatever sequence they become available. Keep
interrupts disabled until the whole track has passed under the read head.

swim_read_sector() assumes that it can search a whole track by reading a
fixed number of sector headers (essentially, fs->secpertrack) but this
assumes no false sector headers are matched in the sector contents. To
prevent that, call swim_read_sector_data() unconditionally after any
valid sector header is matched.

Fixes: 8852ecd97488 ("m68k: mac - Add SWIM floppy support")
Signed-off-by: Finn Thain <fthain@xxxxxxxxxxxxxx>
---
Changed since v1:
 - Use GENMASK() macro.
 - Use fs->secpertrack instead of hard-coding the high-density value.
 - Don't use a failure counter.
 - Use unsigned integers where appropriate.
 - Call swim_read_sector_data() whenever swim_read_sector_header() is
   successful. A NULL pointer is passed to indicate that no data is to be
   copied into the buffer.

Changed since v2:
 - Loop for fs->secpertrack + 1 iterations because the gap search will
   match the index gap as well as every sector gap.
 - Reinstate the failure counter because that way interrupts aren't
   disabled for too long when a medium error is encountered.
 - Improved commit log text.
---
 drivers/block/swim.c     | 99 ++++++++++++++++++++++------------------
 drivers/block/swim_asm.S | 12 ++++-
 2 files changed, 65 insertions(+), 46 deletions(-)

diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index 9bfda4b07ba9..637810d86e37 100644
--- a/drivers/block/swim.c
+++ b/drivers/block/swim.c
@@ -180,9 +180,9 @@ struct floppy_state {
 	enum media_type	 type;
 	int		 write_protected;
 
-	int		 total_secs;
-	int		 secpercyl;
-	int		 secpertrack;
+	unsigned int	 total_secs;
+	unsigned int	 secpercyl;
+	unsigned int	 secpertrack;
 
 	/* in-use information */
 
@@ -452,68 +452,79 @@ static int floppy_eject(struct floppy_state *fs)
 	return 0;
 }
 
-static inline int swim_read_sector(struct floppy_state *fs,
-				   int side, int track,
-				   int sector, unsigned char *buffer)
+static unsigned int swim_read_sector_range(struct floppy_state *fs,
+					unsigned int side, unsigned int track,
+					unsigned int start, unsigned int count,
+					unsigned char *buffer)
 {
 	struct swim __iomem *base = fs->swd->base;
 	unsigned long flags;
 	struct sector_header header;
-	int ret = -1;
-	short i;
+	unsigned int i, bits = 0;
 
-	swim_track(fs, track);
-	swim_head(base, side);
+	if (count > 0) {
+		count = min(count, fs->secpertrack);
+		bits = GENMASK(count - 1, 0);
+	}
 
 	local_irq_save(flags);
-	for (i = 0; i < 36; i++) {
-		if (swim_read_sector_header(base, &header) ||
-		    swim_read(base, error) || header.track != track ||
-		    header.side != side || header.size != 2)
-			continue;
-		if (header.sector == sector) {
-			/* found */
-
-			ret = swim_read_sector_data(base, buffer);
-			if (swim_read(base, error))
-				ret = -EIO;
+	for (i = 0; i < fs->secpertrack + 1; i++) {
+		if (bits == 0) /* All sectors were read ok */
 			break;
+
+		if (swim_read_sector_header(base, &header) == 0 &&
+		    swim_read(base, error) == 0) {
+			unsigned int offset = header.sector - start;
+			unsigned char *buf = NULL;
+			int len;
+
+			if (header.track == track && header.side == side &&
+			    header.size == 2 && header.sector >= start &&
+			    header.sector < start + count &&
+			    (bits & BIT(offset)))
+				buf = buffer + 512 * offset;
+			len = swim_read_sector_data(base, buf);
+			if (swim_read(base, error) == 0 && buf && len == 512)
+				bits &= ~BIT(offset);
 		}
 	}
 	local_irq_restore(flags);
 
-	return ret;
+	return bits ? ffs(bits) - 1 : count; /* No. of contiguous ok sectors */
 }
 
 static blk_status_t floppy_read_sectors(struct floppy_state *fs,
-			       int req_sector, int sectors_nb,
-			       unsigned char *buffer)
+					unsigned int req_sector,
+					unsigned int sectors_nb,
+					unsigned char *buffer)
 {
 	struct swim __iomem *base = fs->swd->base;
-	int ret;
-	int side, track, sector;
-	int i, try;
-
+	unsigned int try = 0;
 
 	swim_drive(base, fs->location);
 	swim_READY_timeout(base);
 
-	for (i = req_sector; i < req_sector + sectors_nb; i++) {
-		int x;
-		track = i / fs->secpercyl;
-		x = i % fs->secpercyl;
-		side = x / fs->secpertrack;
-		sector = x % fs->secpertrack + 1;
-
-		try = 5;
-		do {
-			ret = swim_read_sector(fs, side, track, sector,
-						buffer);
-			if (try-- == 0)
-				return BLK_STS_IOERR;
-		} while (ret != 512);
-
-		buffer += ret;
+	while (sectors_nb) {
+		unsigned int cyl, x, head, sector, n, n_ok;
+
+		cyl = req_sector / fs->secpercyl;
+		x = req_sector % fs->secpercyl;
+		head = (x >= fs->secpertrack) ? 1 : 0;
+		sector = x % fs->secpertrack;
+		n = min(sectors_nb, fs->secpertrack - sector);
+
+		swim_track(fs, cyl);
+		swim_head(base, head);
+
+		n_ok = swim_read_sector_range(fs, head, cyl, sector + 1, n, buffer);
+		if (n_ok == n)
+			try = 0;
+		else if (++try >= 5)
+			return BLK_STS_IOERR;
+
+		buffer += 512 * n_ok;
+		sectors_nb -= n_ok;
+		req_sector += n_ok;
 	}
 
 	return 0;
diff --git a/drivers/block/swim_asm.S b/drivers/block/swim_asm.S
index e06aadb411a1..73d5ced1abe3 100644
--- a/drivers/block/swim_asm.S
+++ b/drivers/block/swim_asm.S
@@ -198,12 +198,14 @@ read_data_loop:
 	dbne	%d2, read_data_loop
 	beq	data_exit
 	moveq	#max_retry, %d2
-	moveb	%a5@, %a4@+
+	moveb	%a5@, %d3
+	bsr	.Lmaybe_store
 	dbra	%d4, 1f
 	bra	data_crc0
 1:	andb	#.Lhr_fifo_2bytes, %d5
 	beq	read_data_loop
-	moveb	%a5@, %a4@+
+	moveb	%a5@, %d3
+	bsr	.Lmaybe_store
 	dbra	%d4, read_data_loop
 
 	/* read CRC */
@@ -240,3 +242,9 @@ data_crc1:
 data_exit:
 	moveb	#0x18, %a3@(write_mode0 - read_mark)
 	rts
+
+.Lmaybe_store:
+	tstl	%a4
+	beq	9f
+	moveb	%d3, %a4@+
+9:	rts
-- 
2.52.0