block/swim: Fixes and improvements - #31
Open
blktests-ci-block-trial[bot] wants to merge 31 commits into
Open
Conversation
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
This fixes multi-sector reads on a Sony drive. The Mitsubishi drive doesn't seem to care either way. Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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> Fixes: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Initialize the error result once only. Update the result only after a successful read. Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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: 8852ecd ("m68k: mac - Add SWIM floppy support") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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")
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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") Signed-off-by: Finn Thain <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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 <fthain@linux-m68k.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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") 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") 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") 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.) Signed-off-by: Finn Thain <fthain@linux-m68k.org>
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: 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. 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. 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. 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. Signed-off-by: Finn Thain <fthain@linux-m68k.org>
No functional changes. 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. Signed-off-by: Finn Thain <fthain@linux-m68k.org>
Define local symbols to give some meaning to anonymous constants. No functional change. 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. Signed-off-by: Finn Thain <fthain@linux-m68k.org>
Author
|
Upstream branch: 248951d |
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: 1
url: http://redsun45:8000/project/linux-block/list/?series=355