[PATCH RFC] block - ataflop.c: fix breakage introduced at blk-mq refactoring

Michael Schmitz
2021-10-18T22:22:05Z

Refactoring of the Atari floppy driver when converting to blk-mq
has broken the state machine in not-so-subtle ways:

finish_fdc() must be called when operations on the floppy device
have completed. This is crucial in order to relase the ST-DMA
lock, which protects against concurrent access to the ST-DMA
controller by other drivers (some DMA related, most just related
to device register access - broken beyond compare, I know).

When rewriting the drivers' old do_request() function, the fact
that finish_fdc() was called only when all queued requests had
completed appears to have been overlooked. Instead, the new
request function calls finish_fdc() immediately after the last
request has been queued. finish_fdc() executes a dummy seek after
most requests, and this overwrites the state machine's interrupt
hander that was set up to wait for completion of the read/write
request just prior. To make matters worse, finish_fdc() is called
before device interrupts are re-enabled, making certain that the
read/write interupt is missed.

Shifting the finish_fdc() call into the read/write request completion
handler ensures the driver waits for the request to actually complete.

Testing this change, I've only ever seen single sector requests with the
'last' flag set. If there is a way to send requests to the driver
without that flag set, I'd appreciate a hint. As it now stands,
the driver won't release the ST-DMA lock on requests that don't have
this flag set, but won't accept further requests because the attempt
to acquire the already-held lock once more will fail.

Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
Fixes: 6ec3938cff95f (ataflop: convert to blk-mq)
CC: linux-block@vger.kernel.org
CC: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
---
drivers/block/ataflop.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 96e2abe34a72..95469b4a8f78 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -653,8 +653,10 @@ static inline void copy_buffer(void *from, void *to)
*p2++ = *p1++;
}

+/* finish_fdc() handling */

-
+static void (*fdc_finish_action)( void ) = NULL;
+

/* General Interrupt Handling */

@@ -1228,6 +1230,12 @@ static void fd_rwsec_done1(int status)
}
else {
/* all sectors finished */
+ void (*handler)( void );
+
+ handler = xchg(&fdc_finish_action, NULL);
+ if (handler)
+ handler();
+
fd_end_request_cur(BLK_STS_OK);
}
return;
@@ -1391,6 +1399,11 @@ static void finish_fdc_done( int dummy )
DPRINT(("finish_fdc() finished\n"));
}

+static void queue_finish_fdc( void )
+{
+ fdc_finish_action = finish_fdc;
+}
+
/* The detection of disk changes is a dark chapter in Atari history :-(
* Because the "Drive ready" signal isn't present in the Atari
* hardware, one has to rely on the "Write Protect". This works fine,
@@ -1491,6 +1504,8 @@ static blk_status_t ataflop_queue_rq(struct blk_mq_hw_ctx *hctx,
int drive = floppy - unit;
int type = floppy->type;

+ DPRINT(("Queue request: drive %d type %d\n", drive, type));
+
spin_lock_irq(&ataflop_lock);
if (fd_request) {
spin_unlock_irq(&ataflop_lock);
@@ -1551,7 +1566,7 @@ static blk_status_t ataflop_queue_rq(struct blk_mq_hw_ctx *hctx,
do_fd_action( drive );

if (bd->last)
- finish_fdc();
+ queue_finish_fdc();
atari_enable_irq( IRQ_MFP_FDC );

out:
--
2.17.1