[PATCH v2 23/32] 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 is a 3 ms wait).

Floppy drivers usually implement a track cache that can be filled in a
single rotation to solve problems like these. 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)
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 found in the sector contents. To
prevent that, call swim_read_sector_data() unconditionally after a valid
sector header is found.

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.
---
 drivers/block/swim.c     | 96 ++++++++++++++++++++++------------------
 drivers/block/swim_asm.S | 12 ++++-
 2 files changed, 62 insertions(+), 46 deletions(-)

diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index 38b1643f3702..e35918bdf9d2 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,76 @@ 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 < 5 * fs->secpertrack; 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;
-
 
 	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)
+			return BLK_STS_IOERR;
+
+		buffer += 512 * n;
+		sectors_nb -= n;
+		req_sector += n;
 	}
 
 	return 0;
diff --git a/drivers/block/swim_asm.S b/drivers/block/swim_asm.S
index 9a7d7466e846..10f4c42ee3ab 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