block/swim: Fixes and improvements - #1147
Open
blktests-ci[bot] wants to merge 33 commits into
Open
Conversation
Author
|
Upstream branch: fd923b3 |
blktests-ci
Bot
force-pushed
the
linus-master_base
branch
from
August 17, 2026 14:10
fdba928 to
60442a3
Compare
Author
|
Upstream branch: 8d3ae59 |
blktests-ci
Bot
force-pushed
the
series/1146886=>linus-master
branch
from
August 17, 2026 14:27
0b988d9 to
6f18c63
Compare
blktests-ci
Bot
force-pushed
the
linus-master_base
branch
from
August 19, 2026 05:25
60442a3 to
3df366e
Compare
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 LSTRB is driven low (with CA0-CA2 bits set). 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. Introduce the PHASE_PIN_DIRECTION macro to separate the pin configuration from the logic level changes. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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(). Reviewed-by: Laurent Vivier <laurent@vivier.eu> Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
This fixes multi-sector reads on a Sony drive. The Mitsubishi drive doesn't seem to care either way. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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 <funaho@jurai.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu> Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
The spindle motor should not be running when a disk is to be inserted. Don't start the motor while the drive is empty. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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. Park the heads before ejecting. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
Initialize the error result once only. Update the result only after a successful read. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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: 8852ecd ("m68k: mac - Add SWIM floppy support")
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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 e3896d7 ("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 <osandov@fb.com> Fixes: e3896d7 ("swim: convert to blk-mq") Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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.) Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
The ISM chip does an automatic MFM gap/sync search when the Action bit is first set. That search may stop at any of a) post-index gap, b) address field gap or c) data field gap. To find the next sector header, the driver need not search at all. It only has to validate the mark bytes. Once the sector address mark has been validated, swim_read_sector_data() is called to read the sector contents. Between the sector address and data fields lies an intra-sector gap followed by a data field mark. After this mark is validated, the 512-byte data area is read into the IO request buffer. Problem is, if any byte in the data field mark is mis-read, the driver searches the whole sector and then reaches the data field mark in the following sector. The wrong sector is then read into the buffer, and swim_read_sector_data() returns success. The request is silently corrupted. The existing limit on polling loop iterations does constrain the search distance but is inherently tied to CPU speed. This is probably the reason why corruption was only observed on a 68030 system. Discontinue the mark search when the mark bytes fail validation. Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
For better readability, initialize the swd backpointer along with the other floppy_state struct members. No functional change. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
These documents relate to the IWM, ISM, SWIM 1, 2, 3 and associated disk drives. No functional change. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
Also remove the horizontal rule at the end of the macro definitions as it doesn't any add value, IMO. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
No functional changes. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
Define a SEL_MASK macro to name the anonymous constant. Define STEPPING rather than re-use STEP because the latter is a command bit macro (see also GCR_MODE vs. SETGCR). No functional change, just better readability. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
Define local symbols to give some meaning to anonymous constants. No functional change. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
These symbols aren't used outside of this file so use local ones. No functional change. Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
Author
|
Upstream branch: bd5f485 |
blktests-ci
Bot
force-pushed
the
series/1146886=>linus-master
branch
from
August 19, 2026 05:30
6f18c63 to
a690cd5
Compare
blktests-ci
Bot
force-pushed
the
linus-master_base
branch
from
August 25, 2026 03:28
3df366e to
5dd70cb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull request for series with
subject: block/swim: Fixes and improvements
version: 2
url: https://patchwork.kernel.org/project/linux-block/list/?series=1146886