block/swim: Fixes and improvements

Inside Macintosh says, "Be sure that you don't change CA0-CA2 or SEL while LSTRB is high". Unfortunately, those bits do change. The CA0-CA2 outputs become inputs when (LSTRB << 4) | LSTRB gets written to the phase register. Then when LSTRB is driven low, CA0-CA2 get driven high.

This is a problem because the drive interprets a STEP command as an EJECT command when these pins float high. This occurs intermittently, perhaps because interrupts are disabled and the race condition happens to end well. However, when I add code to step the heads with interrupts enabled, the disk always ejects.

Keep the four phase pin directions set to output and hold their levels constant during LSTRB signalling.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 52 ++++++++++++++++++++++----------------------
  1 file changed, 26 insertions(+), 26 deletions(-)

drivers/block/swim.c +26 -26

index 0ccc12a72388..b61e92d0e3e9 100644
@@ -89,35 +89,35 @@ struct iwm {
 
  /* bits in phase register */
 
-#define SEEK_POSITIVE	0x070
-#define SEEK_NEGATIVE	0x074
-#define STEP		0x071
-#define MOTOR_ON	0x072
-#define MOTOR_OFF	0x076
-#define INDEX		0x073
-#define EJECT		0x077
-#define SETMFM		0x171
-#define SETGCR		0x175
+#define SEEK_POSITIVE	0x0F0
+#define SEEK_NEGATIVE	0x0F4
+#define STEP		0x0F1
+#define MOTOR_ON	0x0F2
+#define MOTOR_OFF	0x0F6
+#define INDEX		0x0F3
+#define EJECT		0x0F7
+#define SETMFM		0x1F1
+#define SETGCR		0x1F5
 
-#define RELAX		0x033
-#define LSTRB		0x008
+#define RELAX		0x0F3
+#define LSTRB		0x0F8
 
-#define CA_MASK		0x077
+#define CA_MASK		0x0F7
 
  /* Select values for swim_select and swim_readbit */
 
-#define READ_DATA_0	0x074
-#define ONEMEG_DRIVE	0x075
-#define SINGLE_SIDED	0x076
-#define DRIVE_PRESENT	0x077
-#define DISK_IN		0x170
-#define WRITE_PROT	0x171
-#define TRACK_ZERO	0x172
-#define TACHO		0x173
-#define READ_DATA_1	0x174
-#define GCR_MODE	0x175
-#define SEEK_COMPLETE	0x176
-#define TWOMEG_MEDIA	0x177
+#define READ_DATA_0	0x0F4
+#define ONEMEG_DRIVE	0x0F5
+#define SINGLE_SIDED	0x0F6
+#define DRIVE_PRESENT	0x0F7
+#define DISK_IN		0x1F0
+#define WRITE_PROT	0x1F1
+#define TRACK_ZERO	0x1F2
+#define TACHO		0x1F3
+#define READ_DATA_1	0x1F4
+#define GCR_MODE	0x1F5
+#define SEEK_COMPLETE	0x1F6
+#define TWOMEG_MEDIA	0x1F7
 
  /* Bits in handshake register */
 

@@ -284,9 +284,9 @@ static inline void swim_action(struct swim __iomem *base, int action)
 
  	swim_select(base, action);
  	udelay(1);
-	swim_write(base, phase, (LSTRB<<4) | LSTRB);
+	swim_write(base, phase, action | LSTRB);
  	udelay(1);
-	swim_write(base, phase, (LSTRB<<4) | ((~LSTRB) & 0x0F));
+	swim_write(base, phase, action);
  	udelay(1);
 
  	local_irq_restore(flags);

Le 16/07/2026 à 12:02, Finn Thain a écrit :

Quoted 2 lines

Keep the four phase pin directions set to output and hold their levels constant during LSTRB signalling.

Perhaps you can also add a short comment to describe the changes you made to the defines.

Quoted 6 lines

Le 16/07/2026 à 12:02, Finn Thain a écrit :

Quoted 2 lines

Keep the four phase pin directions set to output and hold their levels constant during LSTRB signalling.

Quoted 3 lines

Perhaps you can also add a short comment to describe the changes you made to the defines.

Can you clarify please? I think what you're asking is, shouldn't the PH3 pin be an input when it's not being used for LSTRB signalling?

It would be nice if we had specs for the relevant FDD models, to show that no FDD ever drives the phase lines. Then I could write something more authoritative. I don't have that information but I do have the chip specs.

The swim3.c driver never sets phase pin direction because the SWIM3 chip doesn't implement that i.e. the four phase pins are permanently outputs. Similary, the programming model for the IWM chip offers no way to use phase lines as inputs, AFAICT. Same for the uPD72070 FDC chip.

I will add some of these observations to the commit log or the commentary in the source code if you think it would help.

Thanks for your review.

Le 21/07/2026 à 07:41, Finn Thain a écrit :

Quoted 12 lines · Laurent Vivier

Quoted 9 lines

Le 16/07/2026 à 12:02, Finn Thain a écrit :

Quoted 2 lines

Keep the four phase pin directions set to output and hold their levels constant during LSTRB signalling.

Perhaps you can also add a short comment to describe the changes you made to the defines.

Quoted 2 lines

Can you clarify please? I think what you're asking is, shouldn't the PH3 pin be an input when it's not being used for LSTRB signalling?

My question is simpler:
You added the directions bits in the defines and it's not clear in the commit message.
For instance, I can see you update all the defines, but at first glance I didn't understand why.

Quoted 14 lines

Le 21/07/2026 à 07:41, Finn Thain a écrit :

Quoted 12 lines · Laurent Vivier

Quoted 9 lines

Le 16/07/2026 à 12:02, Finn Thain a écrit :

Quoted 2 lines

Keep the four phase pin directions set to output and hold their levels constant during LSTRB signalling.

Perhaps you can also add a short comment to describe the changes you made to the defines.

Quoted 9 lines

Quoted 2 lines

Can you clarify please? I think what you're asking is, shouldn't the PH3 pin be an input when it's not being used for LSTRB signalling?

My question is simpler:
You added the directions bits in the defines and it's not clear in the commit message.
For instance, I can see you update all the defines, but at first glance I didn't understand why.

OK -- I see what you mean. It might be more idiomatic and more readable if the the pin direction nybble were defined in a separate macro rather than baked into every macro definition. E.g. #define PHASE_PIN_DIRECTION 0xF0 The writes to the phase register would then get a bitwise OR with this new macro. Would that work for you?

Before turning off the spindle motor, call swim_drive() to select the appropriate drive. Remove the swim_drive() call from swim_add_floppy() because it was already called by swim_floppy_init().

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

drivers/block/swim.c +3 -3

index b61e92d0e3e9..2c52a210f4a6 100644
@@ -679,8 +679,10 @@ static void floppy_release(struct gendisk *disk)
  	else if (fs->ref_count > 0)
  		--fs->ref_count;
 
-	if (fs->ref_count == 0)
+	if (fs->ref_count == 0) {
+		swim_drive(base, fs->location);
  		swim_motor(base, OFF);
+	}
  	mutex_unlock(&swim_mutex);
  }
 

@@ -752,8 +754,6 @@ static int swim_add_floppy(struct swim_priv *swd, enum drive_location location)
 
  	fs->location = location;
 
-	swim_drive(base, location);
-
  	swim_motor(base, OFF);
 
  	fs->type = HD_MEDIA;

According to the ISM spec, the MOTON bit "causes the Enable 1 and Enable 2 signals to be turned on to the drive." It doesn't actually turn on the motor. When selecting a drive, enable MOTON. Disable it upon eject or release. This fixes detection of the Sony FDHD drive.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 18 +++++++++++++++---
  1 file changed, 15 insertions(+), 3 deletions(-)

drivers/block/swim.c +15 -3

index 2c52a210f4a6..2a400e544959 100644
@@ -154,8 +154,9 @@ struct iwm {
  /*----------------------------------------------------------------------------*/
 
  enum drive_location {
-	INTERNAL_DRIVE = 0x02,
-	EXTERNAL_DRIVE = 0x04,
+	NO_DRIVE = 0,
+	INTERNAL_DRIVE = BIT(1),
+	EXTERNAL_DRIVE = BIT(2),
  };
 
  enum media_type {

@@ -311,9 +312,15 @@ static inline void swim_drive(struct swim __iomem *base,
  	if (location == INTERNAL_DRIVE) {
  		swim_write(base, mode0, EXTERNAL_DRIVE); /* clear drive 1 bit */
  		swim_write(base, mode1, INTERNAL_DRIVE); /* set drive 0 bit */
+		swim_write(base, mode1, MOTON);
  	} else if (location == EXTERNAL_DRIVE) {
  		swim_write(base, mode0, INTERNAL_DRIVE); /* clear drive 0 bit */
  		swim_write(base, mode1, EXTERNAL_DRIVE); /* set drive 1 bit */
+		swim_write(base, mode1, MOTON);
+	} else {
+		swim_write(base, mode0, INTERNAL_DRIVE);
+		swim_write(base, mode0, EXTERNAL_DRIVE);
+		swim_write(base, mode0, MOTON);
  	}
  }
 

@@ -447,6 +454,7 @@ static int floppy_eject(struct floppy_state *fs)
  	swim_drive(base, fs->location);
  	swim_motor(base, OFF);
  	swim_eject(base);
+	swim_drive(base, NO_DRIVE);
 
  	fs->disk_in = 0;
  	fs->ejected = 1;

@@ -652,8 +660,10 @@ static int floppy_open(struct gendisk *disk, blk_mode_t mode)
  	else if (fs->ref_count > 0)
  		--fs->ref_count;
 
-	if (fs->ref_count == 0)
+	if (fs->ref_count == 0) {
  		swim_motor(base, OFF);
+		swim_drive(base, NO_DRIVE);
+	}
  	return err;
  }
 

@@ -682,6 +692,7 @@ static void floppy_release(struct gendisk *disk)
  	if (fs->ref_count == 0) {
  		swim_drive(base, fs->location);
  		swim_motor(base, OFF);
+		swim_drive(base, NO_DRIVE);
  	}
  	mutex_unlock(&swim_mutex);
  }

@@ -804,6 +815,7 @@ static int swim_floppy_init(struct swim_priv *swd)
  	if (swim_readbit(base, DRIVE_PRESENT) &&
  	    !swim_readbit(base, ONEMEG_DRIVE))
  		swim_add_floppy(swd, EXTERNAL_DRIVE);
+	swim_drive(base, NO_DRIVE);
 
  	/* register floppy drives */
 

This fixes multi-sector reads on a Sony drive. The Mitsubishi drive doesn't seem to care either way.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 4 ----
  1 file changed, 4 deletions(-)

drivers/block/swim.c +0 -2

index 2a400e544959..f9c59282b81f 100644
@@ -473,8 +473,6 @@ static inline int swim_read_sector(struct floppy_state *fs,
  	short i;
 
  	swim_track(fs, track);
-
-	swim_write(base, mode1, MOTON);
  	swim_head(base, side);
  	swim_write(base, mode0, side);
 

The SWIM spec says, "MOTOREN must be low to switch modes" and "after switching from ISM to IWM, the very first command must be a clear L7".

The ISM spec says, MOTOREN "must not be cleared until after the Action bit is cleared". Perform those operations in the correct sequence.

When switching to ISM mode, the Mode register has to be selected with a particular sequence of bit flips. Set q7 low then q6 low then mtrOff.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 10 +++++++---
  1 file changed, 7 insertions(+), 3 deletions(-)

drivers/block/swim.c +6 -2

index f9c59282b81f..7499ff250ca7 100644
@@ -149,6 +149,7 @@ struct iwm {
  #define ACTION		0x08
  #define WRITE_MODE	0x10
  #define HEDSEL		0x20
+#define ISM_SELECT	0x40
  #define MOTON		0x80
 
  /*----------------------------------------------------------------------------*/

@@ -223,18 +224,21 @@ extern int swim_read_sector_data(struct swim __iomem *base,
  static DEFINE_MUTEX(swim_mutex);
  static inline void set_swim_mode(struct swim __iomem *base, int enable)
  {
-	struct iwm __iomem *iwm_base;
+	struct iwm __iomem *iwm_base = (struct iwm __iomem *)base;
  	unsigned long flags;
 
  	if (!enable) {
-		swim_write(base, mode0, 0xf8);
+		swim_write(base, mode0, ACTION);
+		swim_write(base, mode0, ENBL1 | ENBL2 | MOTON);
+		swim_write(base, mode0, ISM_SELECT);
+		iwm_read(iwm_base, q7L);
  		return;

The original SWIM chip has a 16 byte memory to store parameters which control read/write timing. The SWIM 2 chip retains the last 4 parameters which control write pre-compensation, but omits the first 12. Hence SWIM 2 can perform reads without any parameter memory setup but SWIM 1 cannot.

Configure the SWIM parameter memory with the appropriate values so that SWIM 1 can read too. The parameters used here were observed in SWIM chip memory, using Macsbug, while MacOS was reading from an MFM floppy disk.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 13 +++++++++++++
  1 file changed, 13 insertions(+)

drivers/block/swim.c +13 -0

index 7499ff250ca7..481fb1e63ac1 100644
@@ -796,6 +796,17 @@ static void swim_cleanup_floppy_disk(struct floppy_state *fs)
  	blk_mq_free_tag_set(&fs->tag_set);
  }
 
+static void swim_set_parameters(struct swim __iomem *base)
+{
+	unsigned int i;
+	static const u8 mem[] = { 0x18, 0x41, 0x2e, 0x2e, 0x18, 0x18, 0x1b, 0x1b,
+				  0x2f, 0x2f, 0x19, 0x19, 0x97, 0x1b, 0x57, 0x3b, };
+
+	swim_write(base, mode0, 0); /* reset parameter memory index */
+	for (i = 0; i < 16; ++i)
+		swim_write(base, parameter, mem[i]);
+}
+
  static int swim_floppy_init(struct swim_priv *swd)
  {
  	struct queue_limits lim = {

@@ -805,6 +816,8 @@ static int swim_floppy_init(struct swim_priv *swd)
  	int drive;
  	struct swim __iomem *base = swd->base;
 
+	swim_set_parameters(base);
+
  	/* scan floppy drives */
 
  	swim_drive(base, INTERNAL_DRIVE);

Some models have a 16 MHz FCLK oscillator and others 32 MHz. Put this information into the swim device platform_data so that the driver can correctly enable the clock divider. When this is enabled incorrectly, nothing can be read and failures from the Error Correction Machine are flagged in the error register.

This is chip initialization, so do this in swim_floppy_init() rather than floppy_open(). Drop the udelay() which was apparently copied and pasted from swim3.c, where it relates to interrupts (of which this chip has none).

Cc Joshua Thompson <[email protected]>

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  arch/m68k/mac/config.c | 31 ++++++++++++++++++++++++++++++-
  drivers/block/swim.c   | 11 +++++++----
  2 files changed, 37 insertions(+), 5 deletions(-)

arch/m68k/mac/config.c +30 -1

index c0033f885ed4..dba3de21c3c4 100644
@@ -984,8 +984,37 @@ static int __init mac_platform_init(void)
  			.start = swim_base,
  			.end   = swim_base + 0x1FFF,
  		};
+		unsigned int data = 0;
+
+		/* Models in this list are supposed to run FCLK at 32 MHz */
+		switch (macintosh_config->ident) {
+		case MAC_MODEL_CCL:
+		case MAC_MODEL_CCLII:
+		case MAC_MODEL_LCIII:
+		case MAC_MODEL_Q605:
+		case MAC_MODEL_Q605_ACC:
+		case MAC_MODEL_Q610:
+		case MAC_MODEL_Q630:
+		case MAC_MODEL_Q650:
+		case MAC_MODEL_Q800:
+		case MAC_MODEL_P460:
+		case MAC_MODEL_P475:
+		case MAC_MODEL_P475F:
+		case MAC_MODEL_P520:
+		case MAC_MODEL_P550:
+		case MAC_MODEL_P575:
+		case MAC_MODEL_P588:
+		case MAC_MODEL_TV:
+		case MAC_MODEL_C610:
+		case MAC_MODEL_C650:
+		case MAC_MODEL_PB190:
+		case MAC_MODEL_PB520:
+			data = 1;
+			break;
+		}
 
-		platform_device_register_simple("swim", -1, &swim_rsrc, 1);
+		platform_device_register_resndata(NULL, "swim", -1, &swim_rsrc, 1,
+						  &data, sizeof(data));
  	}
 
  	/*

drivers/block/swim.c +7 -4

index 481fb1e63ac1..c49e584cd663 100644
@@ -628,8 +628,7 @@ static int floppy_open(struct gendisk *disk, blk_mode_t mode)
  		fs->ref_count = -1;
  	else
  		fs->ref_count++;
-	swim_write(base, setup, S_IBM_DRIVE  | S_FCLK_DIV2);
-	udelay(10);
+
  	swim_drive(base, fs->location);
  	swim_motor(base, ON);
  	swim_action(base, SETMFM);

@@ -807,8 +806,10 @@ static void swim_set_parameters(struct swim __iomem *base)
  		swim_write(base, parameter, mem[i]);
  }
 
-static int swim_floppy_init(struct swim_priv *swd)
+static int swim_floppy_init(struct platform_device *pdev)
  {
+	struct swim_priv *swd = platform_get_drvdata(pdev);
+	unsigned int *data = pdev->dev.platform_data;
  	struct queue_limits lim = {
  		.features		= BLK_FEAT_ROTATIONAL,
  	};

@@ -816,6 +817,8 @@ static int swim_floppy_init(struct swim_priv *swd)
  	int drive;
  	struct swim __iomem *base = swd->base;
 
+	swim_write(base, setup, S_IBM_DRIVE | (*data ? S_FCLK_DIV2 : 0));
+
  	swim_set_parameters(base);
 
  	/* scan floppy drives */

@@ -930,7 +933,7 @@ static int swim_probe(struct platform_device *dev)
 
  	swd->base = swim_base;
 
-	ret = swim_floppy_init(swd);
+	ret = swim_floppy_init(dev);
  	if (ret)
  		goto out_kfree;
 

The spindle motor should not be running when a disk is to be inserted. Don't start the motor while the drive is empty.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)

drivers/block/swim.c +4 -2

index c49e584cd663..03a89916d4ed 100644
@@ -630,8 +630,7 @@ static int floppy_open(struct gendisk *disk, blk_mode_t mode)
  		fs->ref_count++;
 
  	swim_drive(base, fs->location);
-	swim_motor(base, ON);
-	swim_action(base, SETMFM);
+
  	if (fs->ejected)
  		setup_medium(fs);
  	if (!fs->disk_in) {

@@ -639,6 +638,9 @@ static int floppy_open(struct gendisk *disk, blk_mode_t mode)
  		goto out;
  	}
 
+	swim_motor(base, ON);
+	swim_action(base, SETMFM);
+
  	set_capacity(fs->disk, fs->total_secs);
 
  	if (mode & BLK_OPEN_NDELAY)

Track zero recalibration can be slow and is normally done only once i.e. during system POST or boot-up. Recalibrate once after the drive is probed rather than every time the device is opened. Don't register the drive if recalibration fails. Move the heads to the edge of the disk before ejecting so that recalibration happens quickly.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 15 +++++----------
  1 file changed, 5 insertions(+), 10 deletions(-)

drivers/block/swim.c +5 -10

index 03a89916d4ed..76f6cfa68432 100644
@@ -412,6 +412,7 @@ static inline int swim_track00(struct swim __iomem *base)
  	if (swim_readbit(base, TRACK_ZERO))
  		return 0;
 
+	pr_err("swim: track zero recalibration failed\n");
  	return -1;
  }
 

@@ -456,6 +457,7 @@ static int floppy_eject(struct floppy_state *fs)
  	struct swim __iomem *base = fs->swd->base;
 
  	swim_drive(base, fs->location);
+	swim_track(fs, 1);
  	swim_motor(base, OFF);
  	swim_eject(base);
  	swim_drive(base, NO_DRIVE);

@@ -596,13 +598,6 @@ static void setup_medium(struct floppy_state *fs)
  		struct floppy_struct *g;
  		fs->disk_in = 1;
  		fs->write_protected = swim_readbit(base, WRITE_PROT);
-
-		if (swim_track00(base))
-			printk(KERN_ERR
-				"SWIM: cannot move floppy head to track 0\n");
-
-		swim_track00(base);
-
  		fs->type = swim_readbit(base, TWOMEG_MEDIA) ?
  			HD_MEDIA : DD_MEDIA;
  		fs->head_number = swim_readbit(base, SINGLE_SIDED) ? 1 : 2;

@@ -610,7 +605,6 @@ static void setup_medium(struct floppy_state *fs)
  		fs->total_secs = g->size;
  		fs->secpercyl = g->head * g->sect;
  		fs->secpertrack = g->sect;
-		fs->track = 0;
  	} else {
  		fs->disk_in = 0;
  	}

@@ -773,6 +767,7 @@ static int swim_add_floppy(struct swim_priv *swd, enum drive_location location)
 
  	fs->ref_count = 0;
  	fs->ejected = 1;
+	fs->track = 0;
 
  	swd->floppy_count++;
 

@@ -827,11 +822,11 @@ static int swim_floppy_init(struct platform_device *pdev)
 
  	swim_drive(base, INTERNAL_DRIVE);
  	if (swim_readbit(base, DRIVE_PRESENT) &&
-	    !swim_readbit(base, ONEMEG_DRIVE))
+	    !swim_readbit(base, ONEMEG_DRIVE) && !swim_track00(base))
  		swim_add_floppy(swd, INTERNAL_DRIVE);
  	swim_drive(base, EXTERNAL_DRIVE);
  	if (swim_readbit(base, DRIVE_PRESENT) &&
-	    !swim_readbit(base, ONEMEG_DRIVE))
+	    !swim_readbit(base, ONEMEG_DRIVE) && !swim_track00(base))
  		swim_add_floppy(swd, EXTERNAL_DRIVE);
  	swim_drive(base, NO_DRIVE);
 

The UPD72070 spec indicates that the track zero sensor can take 3 ms to stabilize following a STEP command so add a call to msleep(). Remove the duplicate swim_readbit() call as there's no need for that once the sensor signal has stabilized.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 9 ++++-----
  1 file changed, 4 insertions(+), 5 deletions(-)

drivers/block/swim.c +4 -5

index 76f6cfa68432..9755b86d4dd4 100644
@@ -402,16 +402,15 @@ static inline int swim_track00(struct swim __iomem *base)
  	for (try = 0; try < 100; try++) {
 
  		swim_select(base, RELAX);
+		msleep(3);
+
  		if (swim_readbit(base, TRACK_ZERO))
-			break;
+			return 0;
 
  		if (swim_step(base))
-			return -1;
+			break;
  	}
 
-	if (swim_readbit(base, TRACK_ZERO))
-		return 0;
-
  	pr_err("swim: track zero recalibration failed\n");
  	return -1;
  }

When polling the FIFO for a mark byte in the sector header, don't return zero if the timeout counter has expired, return an error code.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim_asm.S | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

drivers/block/swim_asm.S +1 -2

index 3d7a2d87595a..208622ac4aeb 100644
@@ -79,7 +79,7 @@ wait_addr_mark_byte:
 
  	tstb	%a2@
  	dbmi	%d2, wait_addr_mark_byte
-	bpl	header_exit
+	bpl	signal_nonyb
 
  	moveb	%a3@, %d3
  	cmpb	%a0@+, %d3

@@ -136,7 +136,6 @@ crc1:	tstb	%a2@
 
  	tstb	%a3@(read_error - read_mark)
 
-header_exit:
  	moveq	#0, %d0
  	moveb	#0x18, %a3@(write_mode0 - read_mark)
  	rts

Initialize the error result once only. Update the result only after a successful read.

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim_asm.S | 11 +++--------
  1 file changed, 3 insertions(+), 8 deletions(-)

drivers/block/swim_asm.S +3 -8

index 208622ac4aeb..31fc63b074dc 100644
@@ -47,6 +47,7 @@ swim_read_sector_header:
  	link	%a6, #0
  	moveml	%d1-%d5/%a0-%a4,%sp@-
  	movel	%a6@(0x0c), %a4
+	moveq	#-1, %d0
  	bsr	mfm_read_addrmark
  	moveml	%sp@+, %d1-%d5/%a0-%a4
  	unlk	%a6

@@ -61,7 +62,6 @@ mfm_read_addrmark:
  	movel	%a6@(0x08), %a3
  	lea	%a3@(read_handshake), %a2
  	lea	%a3@(read_mark), %a3
-	moveq	#-1, %d0
  	movew	#seek_time, %d2
 
  wait_header_init:

@@ -137,10 +137,7 @@ crc1:	tstb	%a2@
  	tstb	%a3@(read_error - read_mark)
 
  	moveq	#0, %d0
-	moveb	#0x18, %a3@(write_mode0 - read_mark)
-	rts
  signal_nonyb:
-	moveq	#-1, %d0
  	moveb	#0x18, %a3@(write_mode0 - read_mark)
  	rts
 

@@ -149,6 +146,7 @@ swim_read_sector_data:
  	link	%a6, #0
  	moveml	%d1-%d5/%a0-%a5,%sp@-
  	movel	%a6@(0x0c), %a4
+	moveq	#-1, %d0
  	bsr	mfm_read_data
  	moveml	%sp@+, %d1-%d5/%a0-%a5
  	unlk	%a6

@@ -228,15 +226,12 @@ data_crc1:
 
  	tstb	%a3@(read_error - read_mark)
 
-	moveb	#0x18, %a3@(write_mode0 - read_mark)
-
  	/* return number of bytes read */
 
  	movel	#sector_size, %d0
  	addw	#1, %d4
  	subl	%d4, %d0
-	rts
+
  data_exit:
  	moveb	#0x18, %a3@(write_mode0 - read_mark)
-	moveq	#-1, %d0
  	rts

After reading either the sector header or sector data, examine that flag in the handshake register which holds the result of the CRC calculation. CRC validation has to take place with the last byte still in the FIFO. This flag can't be checked by the caller because by then all bytes will have been retrieved from the FIFO. Return an error code when appropriate.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim_asm.S | 16 ++++++++++++----
  1 file changed, 12 insertions(+), 4 deletions(-)

drivers/block/swim_asm.S +12 -4

index 31fc63b074dc..f40e19e48021 100644
@@ -42,6 +42,8 @@
  	.equ	max_retry, 40
  	.equ	sector_size, 512
 
+	.equ	.Lhr_crc_error,		0x02
+
  	.global swim_read_sector_header
  swim_read_sector_header:
  	link	%a6, #0

@@ -128,12 +130,15 @@ crc0:	tstb	%a2@
 
  	moveq	#max_retry, %d2
 
-crc1:	tstb	%a2@
+crc1:	moveb	%a2@, %d5
  	dbmi	%d2, crc1
  	bpl	signal_nonyb
 
  	moveb	%a3@, %a4@(o_crc1)
 
+	andb	#.Lhr_crc_error, %d5
+	bne	signal_nonyb
+
  	tstb	%a3@(read_error - read_mark)
 
  	moveq	#0, %d0

@@ -212,17 +217,20 @@ data_crc0:
  	dbmi	%d2, data_crc0
  	bpl	data_exit
 
-	moveb	%a3@, %d5
+	moveb	%a3@, %d2
 
  	moveq	#max_retry, %d2
 
  data_crc1:
 
-	tstb	%a2@
+	moveb	%a2@, %d5
  	dbmi	%d2, data_crc1
  	bpl	data_exit
 
-	moveb	%a3@, %d5
+	moveb	%a3@, %d2
+
+	andb	#.Lhr_crc_error, %d5
+	bne	data_exit
 
  	tstb	%a3@(read_error - read_mark)
 

Clear the error register only once before a sector read operation. Don't clear it afterwards -- the caller needs it. Check the error register in swim_read_sector() and return the appropriate error when necessary. Fully validate the sector header. Don't terminate the search loop early just because an erroneous sector header showed up.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c     | 13 +++++++------
  drivers/block/swim_asm.S |  8 --------
  2 files changed, 7 insertions(+), 14 deletions(-)

drivers/block/swim.c +7 -6

index 9755b86d4dd4..e2a8c4ba4e39 100644
@@ -483,20 +483,21 @@ static inline int swim_read_sector(struct floppy_state *fs,
 
  	local_irq_save(flags);
  	for (i = 0; i < 36; i++) {
-		ret = swim_read_sector_header(base, &header);
-		if (!ret && (header.sector == sector)) {
+		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;
  			break;
  		}
  	}
  	local_irq_restore(flags);
 
-	if ((header.side != side)  || (header.track != track) ||
-	     (header.sector != sector))
-		return 0;
-
  	return ret;
  }
 

drivers/block/swim_asm.S +0 -8

index f40e19e48021..1d394ee930d6 100644
@@ -67,7 +67,6 @@ mfm_read_addrmark:
  	movew	#seek_time, %d2
 
  wait_header_init:
-	tstb	%a3@(read_error - read_mark)
  	moveb	#0x18, %a3@(write_mode0 - read_mark)
  	moveb	#0x01, %a3@(write_mode1 - read_mark)
  	moveb	#0x01, %a3@(write_mode0 - read_mark)

@@ -139,8 +138,6 @@ crc1:	moveb	%a2@, %d5
  	andb	#.Lhr_crc_error, %d5
  	bne	signal_nonyb
 
-	tstb	%a3@(read_error - read_mark)
-
  	moveq	#0, %d0
  signal_nonyb:
  	moveb	#0x18, %a3@(write_mode0 - read_mark)

@@ -165,7 +162,6 @@ mfm_read_data:
  	movew	#seek_time, %d2
 
  wait_data_init:
-	tstb	%a3@(read_error - read_mark)
  	moveb	#0x18, %a3@(write_mode0 - read_mark)
  	moveb	#0x01, %a3@(write_mode1 - read_mark)
  	moveb	#0x01, %a3@(write_mode0 - read_mark)

@@ -190,8 +186,6 @@ wait_data_mark_byte:
 
  	/* read data */
 
-	tstb	%a3@(read_error - read_mark)
-
  	movel	#sector_size-1, %d4		/* sector size */
  read_new_data:
  	movew	#max_retry, %d2

@@ -232,8 +226,6 @@ data_crc1:
  	andb	#.Lhr_crc_error, %d5
  	bne	data_exit
 
-	tstb	%a3@(read_error - read_mark)
-
  	/* return number of bytes read */
 
  	movel	#sector_size, %d0

If an unexpected mark byte were to be read from the data register, an error would be flagged. But no error gets flagged when such a byte is read from the mark register, which is misleading. Always use the data register except when a mark byte is expected.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim_asm.S | 21 +++++++++++----------
  1 file changed, 11 insertions(+), 10 deletions(-)

drivers/block/swim_asm.S +11 -10

index 1d394ee930d6..5caeba3a75d8 100644
@@ -47,11 +47,11 @@
  	.global swim_read_sector_header
  swim_read_sector_header:
  	link	%a6, #0
-	moveml	%d1-%d5/%a0-%a4,%sp@-
+	moveml	%d1-%d5/%a0-%a5,%sp@-
  	movel	%a6@(0x0c), %a4
  	moveq	#-1, %d0
  	bsr	mfm_read_addrmark
-	moveml	%sp@+, %d1-%d5/%a0-%a4
+	moveml	%sp@+, %d1-%d5/%a0-%a5
  	unlk	%a6
  	rts
 

@@ -63,6 +63,7 @@ sector_data_mark:
  mfm_read_addrmark:
  	movel	%a6@(0x08), %a3
  	lea	%a3@(read_handshake), %a2
+	lea	%a3@(read_data), %a5
  	lea	%a3@(read_mark), %a3
  	movew	#seek_time, %d2
 

@@ -93,7 +94,7 @@ amark0:	tstb	%a2@
  	dbmi	%d2, amark0
  	bpl	signal_nonyb
 
-	moveb	%a3@, %a4@(o_track)
+	moveb	%a5@, %a4@(o_track)
 
  	moveq	#max_retry, %d2
 

@@ -101,7 +102,7 @@ amark1:	tstb	%a2@
  	dbmi	%d2, amark1
  	bpl	signal_nonyb
 
-	moveb	%a3@, %a4@(o_side)
+	moveb	%a5@, %a4@(o_side)
 
  	moveq	#max_retry, %d2
 

@@ -109,7 +110,7 @@ amark2:	tstb	%a2@
  	dbmi	%d2, amark2
  	bpl	signal_nonyb
 
-	moveb	%a3@, %a4@(o_sector)
+	moveb	%a5@, %a4@(o_sector)
 
  	moveq	#max_retry, %d2
 

@@ -117,7 +118,7 @@ amark3:	tstb	%a2@
  	dbmi	%d2, amark3
  	bpl	signal_nonyb
 
-	moveb	%a3@, %a4@(o_size)
+	moveb	%a5@, %a4@(o_size)
 
  	moveq	#max_retry, %d2
 

@@ -125,7 +126,7 @@ crc0:	tstb	%a2@
  	dbmi	%d2, crc0
  	bpl	signal_nonyb
 
-	moveb	%a3@, %a4@(o_crc0)
+	moveb	%a5@, %a4@(o_crc0)
 
  	moveq	#max_retry, %d2
 

@@ -133,7 +134,7 @@ crc1:	moveb	%a2@, %d5
  	dbmi	%d2, crc1
  	bpl	signal_nonyb
 
-	moveb	%a3@, %a4@(o_crc1)
+	moveb	%a5@, %a4@(o_crc1)
 
  	andb	#.Lhr_crc_error, %d5
  	bne	signal_nonyb

@@ -211,7 +212,7 @@ data_crc0:
  	dbmi	%d2, data_crc0
  	bpl	data_exit
 
-	moveb	%a3@, %d2
+	moveb	%a5@, %d2
 
  	moveq	#max_retry, %d2
 

@@ -221,7 +222,7 @@ data_crc1:
  	dbmi	%d2, data_crc1
  	bpl	data_exit
 
-	moveb	%a3@, %d2
+	moveb	%a5@, %d2
 
  	andb	#.Lhr_crc_error, %d5
  	bne	data_exit

The effect of this bug can be observed as swim_read_sector_data() inexplicably returning -5, or an error flag indicating that a mark byte was read from the data register, or other odd behviour.

When copying bytes from the chip FIFO to the read buffer, the driver keeps count of the remaining buffer space using register %d4. A counter in register %d2 serves as a timeout. The driver polls (%a2), the handshake register, until flags indicate that byte(s) have arrived in the FIFO.

         movel   #sector_size-1, %d4
read_new_data:
         movew   #max_retry, %d2
read_data_loop:
         moveb   %a2@, %d5
         andb    #0xc0, %d5
         dbne    %d2, read_data_loop
         beq     data_exit
         moveb   %a5@, %a4@+
         andb    #0x40, %d5
         dbne    %d4, read_new_data
         beq     exit_loop

Note that the exit_loop branch depends upon a flag in the handshake register and not on the remaining buffer space. Hence there may be no branch to exit_loop after %d4 is decremented to -1 (i.e. full buffer).

moveb %a5@, %a4@+
dbra %d4, read_new_data
exit_loop:

Here is a second decrement of %d4 which can now reach -2. But the buffer bounds check is a comparison with -1, which is now ineffective. Hence the loop will continue copying until %d2 eventually reaches -1.

Fix this bug by terminating the loop as soon as %d4 or %d2 reach -1. Reset the timeout whenever a byte is copied.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim_asm.S | 15 +++++++++------
  1 file changed, 9 insertions(+), 6 deletions(-)

drivers/block/swim_asm.S +9 -6

index 5caeba3a75d8..ae767da671e3 100644
@@ -43,6 +43,8 @@
  	.equ	sector_size, 512
 
  	.equ	.Lhr_crc_error,		0x02
+	.equ	.Lhr_fifo_2bytes,	0x40
+	.equ	.Lhr_fifo_1byte,	0x80
 
  	.global swim_read_sector_header
  swim_read_sector_header:

@@ -192,16 +194,17 @@ read_new_data:
  	movew	#max_retry, %d2
  read_data_loop:
  	moveb	%a2@, %d5
-	andb	#0xc0, %d5
+	andb	#(.Lhr_fifo_2bytes + .Lhr_fifo_1byte), %d5
+	beq	1f
+	movew	#max_retry, %d2
+	moveb	%a5@, %a4@+
+	dbra	%d4, 1f
+	bra	data_crc0
+1:	andb	#.Lhr_fifo_2bytes, %d5
  	dbne	%d2, read_data_loop
  	beq	data_exit
  	moveb	%a5@, %a4@+
-	andb	#0x40, %d5
-	dbne	%d4, read_new_data
-	beq	exit_loop
-	moveb	%a5@, %a4@+
  	dbra	%d4, read_new_data
-exit_loop:
 
  	/* read CRC */
 

These drives are slow: completing a request can take hundreds of milliseconds. Delays are managed by disabling interrupts judiciously and sleeping opportunistically.

As of commit e3896d77b702 ("swim: convert to blk-mq"), a spinlock is taken in irq mode as soon as a request is issued. That lock is held for the duration of the request. Hence the driver sleeps while holding the lock which is forbidden.

Adopt BLK_MQ_F_BLOCKING and remove the spinlock. Use a mutex to serialize requests from the two request queues. (The chip cannot simultaneously process requests on both internal and external drive.)

Cc Omar Sandoval <[email protected]>

Fixes e3896d77b702 ("swim: convert to blk-mq")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 11 +++--------
  1 file changed, 3 insertions(+), 8 deletions(-)

drivers/block/swim.c +3 -8

index e2a8c4ba4e39..6f93ebaaffb7 100644
@@ -211,7 +211,6 @@ enum head {
 
  struct swim_priv {
  	struct swim __iomem *base;
-	spinlock_t lock;
  	int floppy_count;
  	struct floppy_state unit[FD_MAX_UNIT];
  };

@@ -537,12 +536,10 @@ static blk_status_t swim_queue_rq(struct blk_mq_hw_ctx *hctx,
  				  const struct blk_mq_queue_data *bd)
  {
  	struct floppy_state *fs = hctx->queue->queuedata;
-	struct swim_priv *swd = fs->swd;
  	struct request *req = bd->rq;
  	blk_status_t err;
 
-	if (!spin_trylock_irq(&swd->lock))
-		return BLK_STS_DEV_RESOURCE;
+	mutex_lock(&swim_mutex);
 
  	blk_mq_start_request(req);
 

@@ -560,7 +557,7 @@ static blk_status_t swim_queue_rq(struct blk_mq_hw_ctx *hctx,
 
  	err = BLK_STS_OK;
  out:
-	spin_unlock_irq(&swd->lock);
+	mutex_unlock(&swim_mutex);
  	return err;
 
  }

@@ -839,11 +836,9 @@ static int swim_floppy_init(struct platform_device *pdev)
  		return -EBUSY;
  	}
 
-	spin_lock_init(&swd->lock);
-
  	for (drive = 0; drive < swd->floppy_count; drive++) {
  		err = blk_mq_alloc_sq_tag_set(&swd->unit[drive].tag_set,
-				&swim_mq_ops, 2, 0);
+				&swim_mq_ops, 2, BLK_MQ_F_BLOCKING);
  		if (err)
  			goto exit_put_disks;
 

Wherever we have a swim_select() or swim_readbit() call there is an implicit RELAX. That means the caller doesn't have to do it. Remove the redundant code.

BTW, Inside Macintosh says, "Be sure [...] that CA0 and CA1 are set high before changing SEL." Hence the RELAX found in swim_select(). The SwimIII driver in mkLinux also has that. But the swim3.c driver in Linux is odd:
it scatters RELAX actions around as though SEL was not actually under its control... In anycase, swim.c really does control SEL so there's no need for that here.

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 8 +-------
  1 file changed, 1 insertion(+), 7 deletions(-)

drivers/block/swim.c +1 -7

index 6f93ebaaffb7..b4bf96a788db 100644
@@ -336,7 +336,6 @@ static inline void swim_motor(struct swim __iomem *base,
  		swim_action(base, MOTOR_ON);
 
  		for (i = 0; i < 2*HZ; i++) {
-			swim_select(base, RELAX);
  			if (swim_readbit(base, MOTOR_ON))
  				break;
  			set_current_state(TASK_INTERRUPTIBLE);

@@ -344,7 +343,7 @@ static inline void swim_motor(struct swim __iomem *base,
  		}
  	} else if (action == OFF) {
  		swim_action(base, MOTOR_OFF);
-		swim_select(base, RELAX);
+		swim_write(base, phase, RELAX);
  	}
  }
 

@@ -355,13 +354,11 @@ static inline void swim_eject(struct swim __iomem *base)
  	swim_action(base, EJECT);
 
  	for (i = 0; i < 2*HZ; i++) {
-		swim_select(base, RELAX);
  		if (!swim_readbit(base, DISK_IN))
  			break;
  		set_current_state(TASK_INTERRUPTIBLE);
  		schedule_timeout(1);
  	}
-	swim_select(base, RELAX);
  }
 
  static inline void swim_head(struct swim __iomem *base, enum head head)

@@ -385,7 +382,6 @@ static inline int swim_step(struct swim __iomem *base)
  		set_current_state(TASK_INTERRUPTIBLE);
  		schedule_timeout(1);
 
-		swim_select(base, RELAX);
  		if (!swim_readbit(base, STEP))
  			return 0;
  	}

@@ -399,8 +395,6 @@ static inline int swim_track00(struct swim __iomem *base)
  	swim_action(base, SEEK_NEGATIVE);
 
  	for (try = 0; try < 100; try++) {
-
-		swim_select(base, RELAX);
  		msleep(3);
 
  		if (swim_readbit(base, TRACK_ZERO))

Replace duplicated polling loops with poll_timeout_us(). Change the interruptible sleep to uninterruptible because signal delivery shouldn't be allowed to shorten delays required by the drive hardware.

Change the timeout for the !STEP transition to 20 ms in accordance with the maximum interval required by the UPD72070 spec. The existing 1 second timeout is impractical considering the number of steps in a typical seek.

Change the return type of swim_readbit() to bool because that way the bit names make sense i.e. the reader doesn't have to remember to invert the active-low logic used for drive signals.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 42 +++++++++++-------------------------------
  1 file changed, 11 insertions(+), 31 deletions(-)

drivers/block/swim.c +10 -22

index b4bf96a788db..c10ad1266c3b 100644
@@ -19,6 +19,7 @@
  #include <linux/major.h>
  #include <linux/mutex.h>
  #include <linux/hdreg.h>
+#include <linux/iopoll.h>
  #include <linux/kernel.h>
  #include <linux/delay.h>
  #include <linux/platform_device.h>

@@ -296,7 +297,7 @@ static inline void swim_action(struct swim __iomem *base, int action)
  	local_irq_restore(flags);
  }
 
-static inline int swim_readbit(struct swim __iomem *base, int bit)
+static inline bool swim_readbit(struct swim __iomem *base, int bit)
  {
  	int stat;
 

@@ -309,6 +310,12 @@ static inline int swim_readbit(struct swim __iomem *base, int bit)
  	return (stat & SENSE) == 0;
  }
 
+#define swim_readbit_timeout(base, bit, val, timeout_us) \
+	poll_timeout_us(, swim_readbit(base, bit) == val, 1000, timeout_us, false)
+
+#define swim_readbit_timeout_atomic(base, bit, val, timeout_us) \
+	poll_timeout_us_atomic(, swim_readbit(base, bit) == val, 1, timeout_us, false)
+
  static inline void swim_drive(struct swim __iomem *base,
  			      enum drive_location location)
  {

@@ -331,16 +338,8 @@ static inline void swim_motor(struct swim __iomem *base,
  			      enum motor_action action)
  {
  	if (action == ON) {
-		int i;
-
  		swim_action(base, MOTOR_ON);
-
-		for (i = 0; i < 2*HZ; i++) {
-			if (swim_readbit(base, MOTOR_ON))
-				break;
-			set_current_state(TASK_INTERRUPTIBLE);
-			schedule_timeout(1);
-		}
+		swim_readbit_timeout(base, MOTOR_ON, true, 2000 * 1000);
  	} else if (action == OFF) {
  		swim_action(base, MOTOR_OFF);
  		swim_write(base, phase, RELAX);

@@ -349,16 +348,8 @@ static inline void swim_motor(struct swim __iomem *base,
 
  static inline void swim_eject(struct swim __iomem *base)
  {
-	int i;
-
  	swim_action(base, EJECT);
-
-	for (i = 0; i < 2*HZ; i++) {
-		if (!swim_readbit(base, DISK_IN))
-			break;
-		set_current_state(TASK_INTERRUPTIBLE);
-		schedule_timeout(1);
-	}
+	swim_readbit_timeout(base, DISK_IN, false, 2000 * 1000);
  }
 
  static inline void swim_head(struct swim __iomem *base, enum head head)

@@ -373,19 +364,8 @@ static inline void swim_head(struct swim __iomem *base, enum head head)
 
  static inline int swim_step(struct swim __iomem *base)
  {
-	int wait;
-
  	swim_action(base, STEP);
-

drivers/block/swim.c +0 -0

  }
 
  static inline int swim_track00(struct swim __iomem *base)

Quoted 13 lines

drivers/block/swim.c +0 -7

@@ -373,19 +364,8 @@ static inline void swim_head(struct swim __iomem *base,
 enum head head)
 
  static inline int swim_step(struct swim __iomem *base)
  {
-	int wait;
-
  	swim_action(base, STEP);
-
-	for (wait = 0; wait < HZ; wait++) {
-
-		set_current_state(TASK_INTERRUPTIBLE);
-		schedule_timeout(1);

drivers/block/swim.c static inline void swim_head(struct swim __iomem *base, enum head head) · line 368

Quoted 6 lines

drivers/block/swim.c +1 -5

-
-		if (!swim_readbit(base, STEP))
-			return 0;
-	}
-	return -1;
+	return swim_readbit_timeout_atomic(base, STEP, false, 20 * 1000);

I don't understand why we need _atomic() here.

The drive provides a readiness signal that has to be tested before certain commands are issued to the drive. Rename the SEEK_COMPLETE flag as READY because that's how it's known in the documentation as well as the mkLinux source code.

Poll for that signal after stepping the heads and also after switching to MFM mode, as that's what mkLinux does. Check for readiness when stepping because testing shows that some drives require this.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 29 ++++++++++++++++++++++-------
  1 file changed, 22 insertions(+), 7 deletions(-)

drivers/block/swim.c +22 -7

index c10ad1266c3b..77d7ac3765ae 100644
@@ -117,7 +117,7 @@ struct iwm {
  #define TACHO		0x1F3
  #define READ_DATA_1	0x1F4
  #define GCR_MODE	0x1F5
-#define SEEK_COMPLETE	0x1F6
+#define READY		0x1F6
  #define TWOMEG_MEDIA	0x1F7
 
  /* Bits in handshake register */

@@ -316,6 +316,14 @@ static inline bool swim_readbit(struct swim __iomem *base, int bit)
  #define swim_readbit_timeout_atomic(base, bit, val, timeout_us) \
  	poll_timeout_us_atomic(, swim_readbit(base, bit) == val, 1, timeout_us, false)
 
+#define swim_READY_timeout(base) \
+({ \
+	int ret = swim_readbit_timeout(base, READY, true, 1000 * 1000); \
+	if (ret) \
+		printk(KERN_DEBUG "%s: drive not ready\n", __func__); \
+	ret; \
+})
+
  static inline void swim_drive(struct swim __iomem *base,
  			      enum drive_location location)
  {

@@ -354,8 +362,6 @@ static inline void swim_eject(struct swim __iomem *base)
 
  static inline void swim_head(struct swim __iomem *base, enum head head)
  {
-	/* wait drive is ready */
-
  	if (head == UPPER_HEAD)
  		swim_select(base, READ_DATA_1);
  	else if (head == LOWER_HEAD)

@@ -390,20 +396,25 @@ static inline int swim_track00(struct swim __iomem *base)
 
  static inline int swim_seek(struct swim __iomem *base, int step)
  {
-	if (step == 0)
-		return 0;
-
  	if (step < 0) {
  		swim_action(base, SEEK_NEGATIVE);
  		step = -step;
-	} else
+	} else if (step > 0)
  		swim_action(base, SEEK_POSITIVE);
 
+	swim_READY_timeout(base);
+
+	if (step == 0)
+		return 0;
+
  	for ( ; step > 0; step--) {
  		if (swim_step(base))
  			return -1;
  	}
 
+	msleep(30);
+	swim_READY_timeout(base);
+
  	return 0;
  }
 

@@ -485,6 +496,8 @@ static blk_status_t floppy_read_sectors(struct floppy_state *fs,
 
 
  	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;

@@ -605,6 +618,8 @@ static int floppy_open(struct gendisk *disk, blk_mode_t mode)
 
  	swim_motor(base, ON);
  	swim_action(base, SETMFM);
+	msleep(30);
+	swim_READY_timeout(base);
 
  	set_capacity(fs->disk, fs->total_secs);
 

AFAIK, timing requirements for the various FDHD drive mechanisms aren't well documented. But we do have the UPD72070 spec and secondary sources like swim3.c and mkLinux source code. This patch is needed to satisfy the requirements in the UPD72070 spec and follows mkLinux.

Change the LSTRB pulse to 2 microseconds, because this is what mkLinux does. Inside Macintosh says, "Hold LSTRB high for at least one usec but not more than one msec".

When a disk is ejected, pause before de-asserting /ENBL. Wait 150 us after the STEP command for valid signalling. Pause for 1 us after setting the step direction before sending the STEP command.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)

drivers/block/swim.c +5 -1

index 77d7ac3765ae..44fe7a79059c 100644
@@ -290,7 +290,7 @@ static inline void swim_action(struct swim __iomem *base, int action)
  	swim_select(base, action);
  	udelay(1);
  	swim_write(base, phase, action | LSTRB);
-	udelay(1);
+	udelay(2);
  	swim_write(base, phase, action);
  	udelay(1);
 

@@ -340,6 +340,7 @@ static inline void swim_drive(struct swim __iomem *base,
  		swim_write(base, mode0, EXTERNAL_DRIVE);
  		swim_write(base, mode0, MOTON);
  	}
+	udelay(1);
  }
 
  static inline void swim_motor(struct swim __iomem *base,

@@ -358,6 +359,7 @@ static inline void swim_eject(struct swim __iomem *base)
  {
  	swim_action(base, EJECT);
  	swim_readbit_timeout(base, DISK_IN, false, 2000 * 1000);
+	msleep(1);
  }
 
  static inline void swim_head(struct swim __iomem *base, enum head head)

@@ -371,6 +373,7 @@ static inline void swim_head(struct swim __iomem *base, enum head head)
  static inline int swim_step(struct swim __iomem *base)
  {
  	swim_action(base, STEP);
+	udelay(150);
  	return swim_readbit_timeout_atomic(base, STEP, false, 20 * 1000);
  }
 

@@ -401,6 +404,7 @@ static inline int swim_seek(struct swim __iomem *base, int step)
  		step = -step;
  	} else if (step > 0)
  		swim_action(base, SEEK_POSITIVE);
+	udelay(1);
 
  	swim_READY_timeout(base);
 

This write has no effect so remove it. (If side == 0 then no mode bit gets cleared. If side == 1 then mode bit 0 gets cleared but that's pointless because that bit is already clear here.)

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 1 -
  1 file changed, 1 deletion(-)

drivers/block/swim.c +0 -1

index 44fe7a79059c..0bab7ed3daa8 100644
@@ -467,7 +467,6 @@ static inline int swim_read_sector(struct floppy_state *fs,
 
  	swim_track(fs, track);
  	swim_head(base, side);
-	swim_write(base, mode0, side);
 
  	local_irq_save(flags);
  	for (i = 0; i < 36; i++) {

floppy_read_sectors() is confusing because variable names seem to conflate tracks and cylinders. Rename the track 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 18, 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 that, on a 68030 system, re-enabling interrupts after each sector read can result in a full rotation between sectors (that is a 3 ms wait).

Floppy drivers often implement a track cache, that can be filled in a single rotation, to solve problems like these. But I think this solution is much simpler.

For each request, use a sector bitmap to keep a record of sectors read successfully. Read or retry, as necessary, the requested sectors in whatever sequence they arrive in. Keep interrupts disabled until the whole track has passed under the read head.

Fixes 8852ecd97488 ("m68k: mac - Add SWIM floppy support")

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 91 +++++++++++++++++++++++++-------------------
  1 file changed, 52 insertions(+), 39 deletions(-)

drivers/block/swim.c +38 -39

index 0bab7ed3daa8..dae401e649d7 100644
@@ -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 */
 

@@ -455,68 +455,81 @@ 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;
+	int i, bits = 0;
 
-	swim_track(fs, track);
-	swim_head(base, side);
+	for (i = 0; i < 18; i++) { /* A track has at most 18 sectors */
+		if (i >= count)
+			break;
+		bits |= BIT(i);
+	}
 
  	local_irq_save(flags);
  	for (i = 0; i < 36; i++) {
+		if (bits == 0) /* All count sectors were read ok */
+			break;
+
  		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;
-			break;
+		if (header.sector >= start && header.sector < start + count) {
+			unsigned int offset = header.sector - start;
+			int ret;
+
+			if ((bits & BIT(offset)) == 0)
+				continue; /* This sector was already read ok */
+
+			ret = swim_read_sector_data(base, buffer + 512 * offset);
+			if (ret == 512 && swim_read(base, error) == 0)
+				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 failures = 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, ret;
+
+		cyl = req_sector / fs->secpercyl;
+		x = req_sector % fs->secpercyl;
+		head = (x >= fs->secpertrack) ? 1 : 0;
+		sector = x % fs->secpertrack;

drivers/block/swim.c +4 -0

+
+		buffer += 512 * ret;
+		sectors_nb -= ret;
+		req_sector += ret;
  	}
 
  	return 0;

Quoted 32 lines

   swim_drive(base, fs->location);
   swim_READY_timeout(base);
  -	for (i = req_sector; i < req_sector + sectors_nb; i++) {

drivers/block/swim.c +13 -15

-		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, ret;
+
+		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);
+
+		ret = swim_read_sector_range(fs, head, cyl, sector + 1, n,

buffer);

drivers/block/swim.c static int floppy_eject(struct floppy_state *fs) · line 528

Quoted 4 lines

drivers/block/swim.c +4 -0

+		if (ret != n)
+			++failures;
+		if (failures >= 5)
+			return BLK_STS_IOERR;

Old code failed after 5 retries on the same sector, new code fails after 5 incomplete ranges anywhere in it, even if earlier errors were recovered and request made progress.

Is this what you want?

If the compiler made these functions "as fast as possible" that wouldn't actually help because they involve slow mechanical operations. Remove pointless inline function specifiers.

Signed-off-by Finn Thain <[email protected]>

---
  drivers/block/swim.c | 20 ++++++++++----------
  1 file changed, 10 insertions(+), 10 deletions(-)

drivers/block/swim.c +10 -10

index dae401e649d7..4679f554fec5 100644
@@ -222,7 +222,7 @@ extern int swim_read_sector_data(struct swim __iomem *base,
  				 unsigned char *data);
 
  static DEFINE_MUTEX(swim_mutex);
-static inline void set_swim_mode(struct swim __iomem *base, int enable)
+static void set_swim_mode(struct swim __iomem *base, int enable)
  {
  	struct iwm __iomem *iwm_base = (struct iwm __iomem *)base;
  	unsigned long flags;

@@ -250,7 +250,7 @@ static inline void set_swim_mode(struct swim __iomem *base, int enable)
  	local_irq_restore(flags);
  }
 
-static inline int get_swim_mode(struct swim __iomem *base)
+static int get_swim_mode(struct swim __iomem *base)
  {
  	unsigned long flags;
 

@@ -324,7 +324,7 @@ static inline bool swim_readbit(struct swim __iomem *base, int bit)
  	ret; \
  })
 
-static inline void swim_drive(struct swim __iomem *base,
+static void swim_drive(struct swim __iomem *base,
  			      enum drive_location location)
  {
  	if (location == INTERNAL_DRIVE) {

@@ -343,8 +343,8 @@ static inline void swim_drive(struct swim __iomem *base,
  	udelay(1);
  }
 
-static inline void swim_motor(struct swim __iomem *base,
-			      enum motor_action action)
+static void swim_motor(struct swim __iomem *base,
+		       enum motor_action action)
  {
  	if (action == ON) {
  		swim_action(base, MOTOR_ON);

@@ -355,7 +355,7 @@ static inline void swim_motor(struct swim __iomem *base,
  	}
  }
 
-static inline void swim_eject(struct swim __iomem *base)
+static void swim_eject(struct swim __iomem *base)
  {
  	swim_action(base, EJECT);
  	swim_readbit_timeout(base, DISK_IN, false, 2000 * 1000);

@@ -370,14 +370,14 @@ static inline void swim_head(struct swim __iomem *base, enum head head)
  		swim_select(base, READ_DATA_0);
  }
 
-static inline int swim_step(struct swim __iomem *base)
+static int swim_step(struct swim __iomem *base)
  {
  	swim_action(base, STEP);
  	udelay(150);
  	return swim_readbit_timeout_atomic(base, STEP, false, 20 * 1000);
  }
 
-static inline int swim_track00(struct swim __iomem *base)
+static int swim_track00(struct swim __iomem *base)
  {
  	int try;
 

@@ -397,7 +397,7 @@ static inline int swim_track00(struct swim __iomem *base)
  	return -1;
  }
 
-static inline int swim_seek(struct swim __iomem *base, int step)
+static int swim_seek(struct swim __iomem *base, int step)
  {
  	if (step < 0) {
  		swim_action(base, SEEK_NEGATIVE);

@@ -422,7 +422,7 @@ static inline int swim_seek(struct swim __iomem *base, int step)
  	return 0;
  }
 
-static inline int swim_track(struct floppy_state *fs,  int track)
+static int swim_track(struct floppy_state *fs,  int track)
  {
  	struct swim __iomem *base = fs->swd->base;
  	int ret;