* [PATCH v2 0/6] floppy: make use of the local/global fdc explicit
@ 2020-03-01 19:55 Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 1/6] floppy: remove dead code for drives scanning on ARM Willy Tarreau
` (7 more replies)
0 siblings, 8 replies; 10+ messages in thread
From: Willy Tarreau @ 2020-03-01 19:55 UTC (permalink / raw)
To: Denis Efremov; +Cc: Jens Axboe, linux-kernel, linux-block, Willy Tarreau
This is an update to the first minimal cleanup of the floppy driver in
order to make use of the FDC number explicit so as to avoid bugs like
the one fixed by 2e90ca68 ("floppy: check FDC index for errors before
assigning it").
The purpose of this patchset is to rename the "fdc" global variable to
"current_fdc" as Linus suggested and adjust the macros which rely on it
depending on their context.
The most problematic part at this step are the FD_* macros derived
from FD_IOPORT, itself referencing the fdc to get its base address.
These are exclusively used by fd_outb() and fd_inb(). However on ARM
FD_DOR is also used to compare the register based on the port, hence
a small change in the ARM specific code to only check the register
without relying on this hidden memory access.
In order to avoid touching the fd_outb() and fd_inb() macros/functions
on all supported architectures, a new set of fdc_outb()/fdc_inb()
functions was added to the driver to call the former after adding
the register to the FDC's base address.
There are still opportunities for more cleanup, though it's uncertain
they're welcome in this old driver :
- the base address and register can be passed separately to fd_outb()
and fd_inb() in order to simplify register retrieval in some archs;
- a dozen of functions in the driver implicitly depend on current_fdc
while passing it as an argument makes the driver a bit more readable
but that represents less than half of the code and doesn't address
all the readability concerns;
- a test was done to limit support to a single FDC, but after these
cleanups it doesn't provide any significant benefit in terms of code
readability.
These patches are to be applied on top of Denis' floppy-next branch.
v2:
- CC arch maintainers in ARM patches
- fixed issues after Denis' review:
- extra braces in floppy.h in declaration of floppy_selects[]
- missing parenthesis in fd_outb() macro to silence a warning
- used the swap() macro in driveswap()
Willy Tarreau (6):
floppy: remove dead code for drives scanning on ARM
floppy: remove incomplete support for second FDC from ARM code
floppy: prepare ARM code to simplify base address separation
floppy: introduce new functions fdc_inb() and fdc_outb()
floppy: separate the FDC's base address from its registers
floppy: rename the global "fdc" variable to "current_fdc"
arch/arm/include/asm/floppy.h | 88 ++-----------
drivers/block/floppy.c | 284 ++++++++++++++++++++++--------------------
include/uapi/linux/fdreg.h | 18 +--
3 files changed, 168 insertions(+), 222 deletions(-)
--
2.9.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/6] floppy: remove dead code for drives scanning on ARM
2020-03-01 19:55 [PATCH v2 0/6] floppy: make use of the local/global fdc explicit Willy Tarreau
@ 2020-03-01 19:55 ` Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 2/6] floppy: remove incomplete support for second FDC from ARM code Willy Tarreau
` (6 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Willy Tarreau @ 2020-03-01 19:55 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Willy Tarreau, Ian Molton,
Russell King, Linus Torvalds
On ARM, function fd_scandrives pre-dates Git era, is #ifed 0 out, not
used, and cannot even compile since it references an fdc variable that's
not declared anywhere (supposed to be the global one that we're turning
to current_fdc apparently).
There was also an ifdefde out include of mach/floppy.h that does not
exist anymore either. Let's get rid of them since they complicate the
fixing of the driver.
Cc: Ian Molton <spyro@f2s.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
arch/arm/include/asm/floppy.h | 51 -------------------------------------------
1 file changed, 51 deletions(-)
diff --git a/arch/arm/include/asm/floppy.h b/arch/arm/include/asm/floppy.h
index f4fe4d0..4655652 100644
--- a/arch/arm/include/asm/floppy.h
+++ b/arch/arm/include/asm/floppy.h
@@ -8,9 +8,6 @@
*/
#ifndef __ASM_ARM_FLOPPY_H
#define __ASM_ARM_FLOPPY_H
-#if 0
-#include <mach/floppy.h>
-#endif
#define fd_outb(val,port) \
do { \
@@ -69,54 +66,6 @@ do { \
outb(new_dor, FD_DOR); \
} while (0)
-/*
- * Someday, we'll automatically detect which drives are present...
- */
-static inline void fd_scandrives (void)
-{
-#if 0
- int floppy, drive_count;
-
- fd_disable_irq();
- raw_cmd = &default_raw_cmd;
- raw_cmd->flags = FD_RAW_SPIN | FD_RAW_NEED_SEEK;
- raw_cmd->track = 0;
- raw_cmd->rate = ?;
- drive_count = 0;
- for (floppy = 0; floppy < 4; floppy ++) {
- current_drive = drive_count;
- /*
- * Turn on floppy motor
- */
- if (start_motor(redo_fd_request))
- continue;
- /*
- * Set up FDC
- */
- fdc_specify();
- /*
- * Tell FDC to recalibrate
- */
- output_byte(FD_RECALIBRATE);
- LAST_OUT(UNIT(floppy));
- /* wait for command to complete */
- if (!successful) {
- int i;
- for (i = drive_count; i < 3; i--)
- floppy_selects[fdc][i] = floppy_selects[fdc][i + 1];
- floppy_selects[fdc][3] = 0;
- floppy -= 1;
- } else
- drive_count++;
- }
-#else
- floppy_selects[0][0] = 0x10;
- floppy_selects[0][1] = 0x21;
- floppy_selects[0][2] = 0x23;
- floppy_selects[0][3] = 0x33;
-#endif
-}
-
#define FDC1 (0x3f0)
#define FLOPPY0_TYPE 4
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/6] floppy: remove incomplete support for second FDC from ARM code
2020-03-01 19:55 [PATCH v2 0/6] floppy: make use of the local/global fdc explicit Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 1/6] floppy: remove dead code for drives scanning on ARM Willy Tarreau
@ 2020-03-01 19:55 ` Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 3/6] floppy: prepare ARM code to simplify base address separation Willy Tarreau
` (5 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Willy Tarreau @ 2020-03-01 19:55 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Willy Tarreau, Ian Molton,
Russell King, Linus Torvalds
The ARM code was written with the apparent hope to one day support
a second FDC except that the code was incomplete and only touches
the first one, which is also reflected by N_FDC==1. However this
made its fd_outb() macro artificially depend on the global or local
"fdc" variable.
Let's get rid of this and make it explicit it doesn't rely on this
variable anymore.
Cc: Ian Molton <spyro@f2s.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
arch/arm/include/asm/floppy.h | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/arch/arm/include/asm/floppy.h b/arch/arm/include/asm/floppy.h
index 4655652..7e58979 100644
--- a/arch/arm/include/asm/floppy.h
+++ b/arch/arm/include/asm/floppy.h
@@ -50,17 +50,13 @@ static inline int fd_dma_setup(void *data, unsigned int length,
* to a non-zero track, and then restoring it to track 0. If an error occurs,
* then there is no floppy drive present. [to be put back in again]
*/
-static unsigned char floppy_selects[2][4] =
-{
- { 0x10, 0x21, 0x23, 0x33 },
- { 0x10, 0x21, 0x23, 0x33 }
-};
+static unsigned char floppy_selects[4] = { 0x10, 0x21, 0x23, 0x33 };
#define fd_setdor(dor) \
do { \
int new_dor = (dor); \
if (new_dor & 0xf0) \
- new_dor = (new_dor & 0x0c) | floppy_selects[fdc][new_dor & 3]; \
+ new_dor = (new_dor & 0x0c) | floppy_selects[new_dor & 3]; \
else \
new_dor &= 0x0c; \
outb(new_dor, FD_DOR); \
@@ -84,9 +80,7 @@ do { \
*/
static void driveswap(int *ints, int dummy, int dummy2)
{
- floppy_selects[0][0] ^= floppy_selects[0][1];
- floppy_selects[0][1] ^= floppy_selects[0][0];
- floppy_selects[0][0] ^= floppy_selects[0][1];
+ swap(floppy_selects[0], floppy_selects[1]);
}
#define EXTRA_FLOPPY_PARAMS ,{ "driveswap", &driveswap, NULL, 0, 0 }
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/6] floppy: prepare ARM code to simplify base address separation
2020-03-01 19:55 [PATCH v2 0/6] floppy: make use of the local/global fdc explicit Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 1/6] floppy: remove dead code for drives scanning on ARM Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 2/6] floppy: remove incomplete support for second FDC from ARM code Willy Tarreau
@ 2020-03-01 19:55 ` Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 4/6] floppy: introduce new functions fdc_inb() and fdc_outb() Willy Tarreau
` (4 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Willy Tarreau @ 2020-03-01 19:55 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Willy Tarreau, Ian Molton,
Russell King, Linus Torvalds
The fd_outb() macro on ARM relies on a special fd_setdor() macro when
the register is FD_DOR and both will need to be changed to accept a
separate base address. Let's just remerge them to simplify the change
and make this code more easily reviewable.
Cc: Ian Molton <spyro@f2s.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
arch/arm/include/asm/floppy.h | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/arch/arm/include/asm/floppy.h b/arch/arm/include/asm/floppy.h
index 7e58979..34ebd86 100644
--- a/arch/arm/include/asm/floppy.h
+++ b/arch/arm/include/asm/floppy.h
@@ -9,12 +9,17 @@
#ifndef __ASM_ARM_FLOPPY_H
#define __ASM_ARM_FLOPPY_H
-#define fd_outb(val,port) \
- do { \
- if ((port) == (u32)FD_DOR) \
- fd_setdor((val)); \
- else \
- outb((val),(port)); \
+#define fd_outb(val,port) \
+ do { \
+ int new_val = (val); \
+ if ((port) == (u32)FD_DOR) { \
+ if (new_val & 0xf0) \
+ new_val = (new_val & 0x0c) | \
+ floppy_selects[new_val & 3]; \
+ else \
+ new_val &= 0x0c; \
+ } \
+ outb(new_val, (port)); \
} while(0)
#define fd_inb(port) inb((port))
@@ -52,16 +57,6 @@ static inline int fd_dma_setup(void *data, unsigned int length,
*/
static unsigned char floppy_selects[4] = { 0x10, 0x21, 0x23, 0x33 };
-#define fd_setdor(dor) \
-do { \
- int new_dor = (dor); \
- if (new_dor & 0xf0) \
- new_dor = (new_dor & 0x0c) | floppy_selects[new_dor & 3]; \
- else \
- new_dor &= 0x0c; \
- outb(new_dor, FD_DOR); \
-} while (0)
-
#define FDC1 (0x3f0)
#define FLOPPY0_TYPE 4
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 4/6] floppy: introduce new functions fdc_inb() and fdc_outb()
2020-03-01 19:55 [PATCH v2 0/6] floppy: make use of the local/global fdc explicit Willy Tarreau
` (2 preceding siblings ...)
2020-03-01 19:55 ` [PATCH v2 3/6] floppy: prepare ARM code to simplify base address separation Willy Tarreau
@ 2020-03-01 19:55 ` Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 5/6] floppy: separate the FDC's base address from its registers Willy Tarreau
` (3 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Willy Tarreau @ 2020-03-01 19:55 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Willy Tarreau, Linus Torvalds
These two functions replace fd_inb() and fd_outb() in that they take
the FDC in argument. This will ease the separation of the base address
and the port everywhere the code is used.
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/block/floppy.c | 42 ++++++++++++++++++++++++++----------------
1 file changed, 26 insertions(+), 16 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index d521899..250a451 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -594,6 +594,16 @@ static unsigned char fsector_t; /* sector in track */
static unsigned char in_sector_offset; /* offset within physical sector,
* expressed in units of 512 bytes */
+static inline unsigned char fdc_inb(int fdc, unsigned long addr)
+{
+ return fd_inb(addr);
+}
+
+static inline void fdc_outb(unsigned char value, int fdc, unsigned long addr)
+{
+ fd_outb(value, addr);
+}
+
static inline bool drive_no_geom(int drive)
{
return !current_type[drive] && !ITYPE(drive_state[drive].fd_device);
@@ -743,14 +753,14 @@ static int disk_change(int drive)
"checking disk change line for drive %d\n", drive);
debug_dcl(drive_params[drive].flags, "jiffies=%lu\n", jiffies);
debug_dcl(drive_params[drive].flags, "disk change line=%x\n",
- fd_inb(FD_DIR) & 0x80);
+ fdc_inb(fdc, FD_DIR) & 0x80);
debug_dcl(drive_params[drive].flags, "flags=%lx\n",
drive_state[drive].flags);
if (drive_params[drive].flags & FD_BROKEN_DCL)
return test_bit(FD_DISK_CHANGED_BIT,
&drive_state[drive].flags);
- if ((fd_inb(FD_DIR) ^ drive_params[drive].flags) & 0x80) {
+ if ((fdc_inb(fdc, FD_DIR) ^ drive_params[drive].flags) & 0x80) {
set_bit(FD_VERIFY_BIT, &drive_state[drive].flags);
/* verify write protection */
@@ -807,7 +817,7 @@ static int set_dor(int fdc, char mask, char data)
disk_change(drive);
}
fdc_state[fdc].dor = newdor;
- fd_outb(newdor, FD_DOR);
+ fdc_outb(newdor, fdc, FD_DOR);
unit = newdor & 0x3;
if (!is_selected(olddor, unit) && is_selected(newdor, unit)) {
@@ -822,8 +832,8 @@ static void twaddle(void)
{
if (drive_params[current_drive].select_delay)
return;
- fd_outb(fdc_state[fdc].dor & ~(0x10 << UNIT(current_drive)), FD_DOR);
- fd_outb(fdc_state[fdc].dor, FD_DOR);
+ fdc_outb(fdc_state[fdc].dor & ~(0x10 << UNIT(current_drive)), fdc, FD_DOR);
+ fdc_outb(fdc_state[fdc].dor, fdc, FD_DOR);
drive_state[current_drive].select_date = jiffies;
}
@@ -864,7 +874,7 @@ static void set_fdc(int drive)
#endif
if (fdc_state[fdc].rawcmd == 2)
reset_fdc_info(1);
- if (fd_inb(FD_STATUS) != STATUS_READY)
+ if (fdc_inb(fdc, FD_STATUS) != STATUS_READY)
fdc_state[fdc].reset = 1;
}
@@ -1103,7 +1113,7 @@ static int wait_til_ready(void)
if (fdc_state[fdc].reset)
return -1;
for (counter = 0; counter < 10000; counter++) {
- status = fd_inb(FD_STATUS);
+ status = fdc_inb(fdc, FD_STATUS);
if (status & STATUS_READY)
return status;
}
@@ -1124,7 +1134,7 @@ static int output_byte(char byte)
return -1;
if (is_ready_state(status)) {
- fd_outb(byte, FD_DATA);
+ fdc_outb(byte, fdc, FD_DATA);
output_log[output_log_pos].data = byte;
output_log[output_log_pos].status = status;
output_log[output_log_pos].jiffies = jiffies;
@@ -1157,7 +1167,7 @@ static int result(void)
return i;
}
if (status == (STATUS_DIR | STATUS_READY | STATUS_BUSY))
- reply_buffer[i] = fd_inb(FD_DATA);
+ reply_buffer[i] = fdc_inb(fdc, FD_DATA);
else
break;
}
@@ -1352,7 +1362,7 @@ static int fdc_dtr(void)
return 0;
/* Set dtr */
- fd_outb(raw_cmd->rate & 3, FD_DCR);
+ fdc_outb(raw_cmd->rate & 3, fdc, FD_DCR);
/* TODO: some FDC/drive combinations (C&T 82C711 with TEAC 1.2MB)
* need a stabilization period of several milliseconds to be
@@ -1796,11 +1806,11 @@ static void reset_fdc(void)
release_dma_lock(flags);
if (fdc_state[fdc].version >= FDC_82072A)
- fd_outb(0x80 | (fdc_state[fdc].dtr & 3), FD_STATUS);
+ fdc_outb(0x80 | (fdc_state[fdc].dtr & 3), fdc, FD_STATUS);
else {
- fd_outb(fdc_state[fdc].dor & ~0x04, FD_DOR);
+ fdc_outb(fdc_state[fdc].dor & ~0x04, fdc, FD_DOR);
udelay(FD_RESET_DELAY);
- fd_outb(fdc_state[fdc].dor, FD_DOR);
+ fdc_outb(fdc_state[fdc].dor, fdc, FD_DOR);
}
}
@@ -1827,7 +1837,7 @@ static void show_floppy(void)
print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1,
reply_buffer, resultsize, true);
- pr_info("status=%x\n", fd_inb(FD_STATUS));
+ pr_info("status=%x\n", fdc_inb(fdc, FD_STATUS));
pr_info("fdc_busy=%lu\n", fdc_busy);
if (do_floppy)
pr_info("do_floppy=%ps\n", do_floppy);
@@ -4875,7 +4885,7 @@ static int floppy_grab_irq_and_dma(void)
for (fdc = 0; fdc < N_FDC; fdc++) {
if (fdc_state[fdc].address != -1) {
reset_fdc_info(1);
- fd_outb(fdc_state[fdc].dor, FD_DOR);
+ fdc_outb(fdc_state[fdc].dor, fdc, FD_DOR);
}
}
fdc = 0;
@@ -4883,7 +4893,7 @@ static int floppy_grab_irq_and_dma(void)
for (fdc = 0; fdc < N_FDC; fdc++)
if (fdc_state[fdc].address != -1)
- fd_outb(fdc_state[fdc].dor, FD_DOR);
+ fdc_outb(fdc_state[fdc].dor, fdc, FD_DOR);
/*
* The driver will try and free resources and relies on us
* to know if they were allocated or not.
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 5/6] floppy: separate the FDC's base address from its registers
2020-03-01 19:55 [PATCH v2 0/6] floppy: make use of the local/global fdc explicit Willy Tarreau
` (3 preceding siblings ...)
2020-03-01 19:55 ` [PATCH v2 4/6] floppy: introduce new functions fdc_inb() and fdc_outb() Willy Tarreau
@ 2020-03-01 19:55 ` Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 6/6] floppy: rename the global "fdc" variable to "current_fdc" Willy Tarreau
` (2 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Willy Tarreau @ 2020-03-01 19:55 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Willy Tarreau, Ian Molton,
Russell King, Linus Torvalds
FDC registers FD_STATUS, FD_DATA, FD_DOR, FD_DIR and FD_DCR used to be
defined relative to FD_IOPORT, which is the FDC's base address, itself
a macro depending on the "fdc" local or global variable.
This patch changes this so that the register macros above now only
reference the address offset, and that the FDC's address is explicitly
passed in each call to fd_inb() and fd_outb(), thus removing the macro.
With this change there is no more implicit usage of the local/global
"fdc" variable.
One place in the ARM code used to check if the port was equal to FD_DOR,
this was changed to testing the register by applying a mask to the port,
as was already done in the sparc code.
There are still occurrences of fd_inb() and fd_outb() in the PARISC
code and these ones remain unaffected since they already used to work
with a base address and a register offset.
The sparc, m68k and parisc code could now be slightly cleaned up to
benefit from the macro definitions above instead of the equivalent
hard-coded values.
Cc: Ian Molton <spyro@f2s.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
arch/arm/include/asm/floppy.h | 2 +-
drivers/block/floppy.c | 9 ++++-----
include/uapi/linux/fdreg.h | 18 +++++-------------
3 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/arch/arm/include/asm/floppy.h b/arch/arm/include/asm/floppy.h
index 34ebd86..79fa327 100644
--- a/arch/arm/include/asm/floppy.h
+++ b/arch/arm/include/asm/floppy.h
@@ -12,7 +12,7 @@
#define fd_outb(val,port) \
do { \
int new_val = (val); \
- if ((port) == (u32)FD_DOR) { \
+ if (((port) & 7) == FD_DOR) { \
if (new_val & 0xf0) \
new_val = (new_val & 0x0c) | \
floppy_selects[new_val & 3]; \
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 250a451..4e43a7e 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -171,7 +171,6 @@ static int print_unex = 1;
#include <linux/kernel.h>
#include <linux/timer.h>
#include <linux/workqueue.h>
-#define FDPATCHES
#include <linux/fdreg.h>
#include <linux/fd.h>
#include <linux/hdreg.h>
@@ -594,14 +593,14 @@ static unsigned char fsector_t; /* sector in track */
static unsigned char in_sector_offset; /* offset within physical sector,
* expressed in units of 512 bytes */
-static inline unsigned char fdc_inb(int fdc, unsigned long addr)
+static inline unsigned char fdc_inb(int fdc, int reg)
{
- return fd_inb(addr);
+ return fd_inb(fdc_state[fdc].address + reg);
}
-static inline void fdc_outb(unsigned char value, int fdc, unsigned long addr)
+static inline void fdc_outb(unsigned char value, int fdc, int reg)
{
- fd_outb(value, addr);
+ fd_outb(value, fdc_state[fdc].address + reg);
}
static inline bool drive_no_geom(int drive)
diff --git a/include/uapi/linux/fdreg.h b/include/uapi/linux/fdreg.h
index 5e2981d..1318881 100644
--- a/include/uapi/linux/fdreg.h
+++ b/include/uapi/linux/fdreg.h
@@ -7,26 +7,18 @@
* Handbook", Sanches and Canton.
*/
-#ifdef FDPATCHES
-#define FD_IOPORT fdc_state[fdc].address
-#else
-/* It would be a lot saner just to force fdc_state[fdc].address to always
- be set ! FIXME */
-#define FD_IOPORT 0x3f0
-#endif
-
/* Fd controller regs. S&C, about page 340 */
-#define FD_STATUS (4 + FD_IOPORT )
-#define FD_DATA (5 + FD_IOPORT )
+#define FD_STATUS 4
+#define FD_DATA 5
/* Digital Output Register */
-#define FD_DOR (2 + FD_IOPORT )
+#define FD_DOR 2
/* Digital Input Register (read) */
-#define FD_DIR (7 + FD_IOPORT )
+#define FD_DIR 7
/* Diskette Control Register (write)*/
-#define FD_DCR (7 + FD_IOPORT )
+#define FD_DCR 7
/* Bits of main status register */
#define STATUS_BUSYMASK 0x0F /* drive busy mask */
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 6/6] floppy: rename the global "fdc" variable to "current_fdc"
2020-03-01 19:55 [PATCH v2 0/6] floppy: make use of the local/global fdc explicit Willy Tarreau
` (4 preceding siblings ...)
2020-03-01 19:55 ` [PATCH v2 5/6] floppy: separate the FDC's base address from its registers Willy Tarreau
@ 2020-03-01 19:55 ` Willy Tarreau
2020-03-01 20:33 ` [PATCH v2 0/6] floppy: make use of the local/global fdc explicit Joe Perches
2020-03-08 13:12 ` Denis Efremov
7 siblings, 0 replies; 10+ messages in thread
From: Willy Tarreau @ 2020-03-01 19:55 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Willy Tarreau, Linus Torvalds
This is done in order to remove the confusion that arises at some places
in the code where local variables or arguments shadow the global variable.
It is already visible that some places are a bit awkward and iterate over
the global variable, for the sole reason that they used to rely on it being
named "fdc" in order to get the correct address when using FD_DOR. These
ones are easy to spot by searching for "for (current_fdc...".
Some more cleanup is definitely possible. For example
"fdc_state[current_fdc].somefield" is used all over the code and would
probably be better with "fdc_state->somefield" with fdc_state being set
when current_fdc is assigned. This would require to pass the pointer to
the current state instead of the current_fdc to the I/O functions.
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/block/floppy.c | 267 +++++++++++++++++++++++++------------------------
1 file changed, 137 insertions(+), 130 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 4e43a7e..c3daa64 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -582,7 +582,7 @@ static int buffer_max = -1;
/* fdc related variables, should end up in a struct */
static struct floppy_fdc_state fdc_state[N_FDC];
-static int fdc; /* current fdc */
+static int current_fdc; /* current fdc */
static struct workqueue_struct *floppy_wq;
@@ -831,8 +831,9 @@ static void twaddle(void)
{
if (drive_params[current_drive].select_delay)
return;
- fdc_outb(fdc_state[fdc].dor & ~(0x10 << UNIT(current_drive)), fdc, FD_DOR);
- fdc_outb(fdc_state[fdc].dor, fdc, FD_DOR);
+ fdc_outb(fdc_state[current_fdc].dor & ~(0x10 << UNIT(current_drive)),
+ current_fdc, FD_DOR);
+ fdc_outb(fdc_state[current_fdc].dor, current_fdc, FD_DOR);
drive_state[current_drive].select_date = jiffies;
}
@@ -844,19 +845,20 @@ static void reset_fdc_info(int mode)
{
int drive;
- fdc_state[fdc].spec1 = fdc_state[fdc].spec2 = -1;
- fdc_state[fdc].need_configure = 1;
- fdc_state[fdc].perp_mode = 1;
- fdc_state[fdc].rawcmd = 0;
+ fdc_state[current_fdc].spec1 = fdc_state[current_fdc].spec2 = -1;
+ fdc_state[current_fdc].need_configure = 1;
+ fdc_state[current_fdc].perp_mode = 1;
+ fdc_state[current_fdc].rawcmd = 0;
for (drive = 0; drive < N_DRIVE; drive++)
- if (FDC(drive) == fdc && (mode || drive_state[drive].track != NEED_1_RECAL))
+ if (FDC(drive) == current_fdc &&
+ (mode || drive_state[drive].track != NEED_1_RECAL))
drive_state[drive].track = NEED_2_RECAL;
}
/* selects the fdc and drive, and enables the fdc's input/dma. */
static void set_fdc(int drive)
{
- unsigned int new_fdc = fdc;
+ unsigned int new_fdc = current_fdc;
if (drive >= 0 && drive < N_DRIVE) {
new_fdc = FDC(drive);
@@ -866,15 +868,15 @@ static void set_fdc(int drive)
pr_info("bad fdc value\n");
return;
}
- fdc = new_fdc;
- set_dor(fdc, ~0, 8);
+ current_fdc = new_fdc;
+ set_dor(current_fdc, ~0, 8);
#if N_FDC > 1
- set_dor(1 - fdc, ~8, 0);
+ set_dor(1 - current_fdc, ~8, 0);
#endif
- if (fdc_state[fdc].rawcmd == 2)
+ if (fdc_state[current_fdc].rawcmd == 2)
reset_fdc_info(1);
- if (fdc_inb(fdc, FD_STATUS) != STATUS_READY)
- fdc_state[fdc].reset = 1;
+ if (fdc_inb(current_fdc, FD_STATUS) != STATUS_READY)
+ fdc_state[current_fdc].reset = 1;
}
/* locks the driver */
@@ -964,11 +966,11 @@ static void scandrives(void)
if (drive_state[drive].fd_ref == 0 || drive_params[drive].select_delay != 0)
continue; /* skip closed drives */
set_fdc(drive);
- if (!(set_dor(fdc, ~3, UNIT(drive) | (0x10 << UNIT(drive))) &
+ if (!(set_dor(current_fdc, ~3, UNIT(drive) | (0x10 << UNIT(drive))) &
(0x10 << UNIT(drive))))
/* switch the motor off again, if it was off to
* begin with */
- set_dor(fdc, ~(0x10 << UNIT(drive)), 0);
+ set_dor(current_fdc, ~(0x10 << UNIT(drive)), 0);
}
set_fdc(saved_drive);
}
@@ -1039,7 +1041,7 @@ static void main_command_interrupt(void)
static int fd_wait_for_completion(unsigned long expires,
void (*function)(void))
{
- if (fdc_state[fdc].reset) {
+ if (fdc_state[current_fdc].reset) {
reset_fdc(); /* do the reset during sleep to win time
* if we don't need to sleep, it's a good
* occasion anyways */
@@ -1067,13 +1069,13 @@ static void setup_DMA(void)
pr_cont("%x,", raw_cmd->cmd[i]);
pr_cont("\n");
cont->done(0);
- fdc_state[fdc].reset = 1;
+ fdc_state[current_fdc].reset = 1;
return;
}
if (((unsigned long)raw_cmd->kernel_data) % 512) {
pr_info("non aligned address: %p\n", raw_cmd->kernel_data);
cont->done(0);
- fdc_state[fdc].reset = 1;
+ fdc_state[current_fdc].reset = 1;
return;
}
f = claim_dma_lock();
@@ -1081,10 +1083,11 @@ static void setup_DMA(void)
#ifdef fd_dma_setup
if (fd_dma_setup(raw_cmd->kernel_data, raw_cmd->length,
(raw_cmd->flags & FD_RAW_READ) ?
- DMA_MODE_READ : DMA_MODE_WRITE, fdc_state[fdc].address) < 0) {
+ DMA_MODE_READ : DMA_MODE_WRITE,
+ fdc_state[current_fdc].address) < 0) {
release_dma_lock(f);
cont->done(0);
- fdc_state[fdc].reset = 1;
+ fdc_state[current_fdc].reset = 1;
return;
}
release_dma_lock(f);
@@ -1095,7 +1098,7 @@ static void setup_DMA(void)
DMA_MODE_READ : DMA_MODE_WRITE);
fd_set_dma_addr(raw_cmd->kernel_data);
fd_set_dma_count(raw_cmd->length);
- virtual_dma_port = fdc_state[fdc].address;
+ virtual_dma_port = fdc_state[current_fdc].address;
fd_enable_dma();
release_dma_lock(f);
#endif
@@ -1109,18 +1112,18 @@ static int wait_til_ready(void)
int status;
int counter;
- if (fdc_state[fdc].reset)
+ if (fdc_state[current_fdc].reset)
return -1;
for (counter = 0; counter < 10000; counter++) {
- status = fdc_inb(fdc, FD_STATUS);
+ status = fdc_inb(current_fdc, FD_STATUS);
if (status & STATUS_READY)
return status;
}
if (initialized) {
- DPRINT("Getstatus times out (%x) on fdc %d\n", status, fdc);
+ DPRINT("Getstatus times out (%x) on fdc %d\n", status, current_fdc);
show_floppy();
}
- fdc_state[fdc].reset = 1;
+ fdc_state[current_fdc].reset = 1;
return -1;
}
@@ -1133,17 +1136,17 @@ static int output_byte(char byte)
return -1;
if (is_ready_state(status)) {
- fdc_outb(byte, fdc, FD_DATA);
+ fdc_outb(byte, current_fdc, FD_DATA);
output_log[output_log_pos].data = byte;
output_log[output_log_pos].status = status;
output_log[output_log_pos].jiffies = jiffies;
output_log_pos = (output_log_pos + 1) % OLOGSIZE;
return 0;
}
- fdc_state[fdc].reset = 1;
+ fdc_state[current_fdc].reset = 1;
if (initialized) {
DPRINT("Unable to send byte %x to FDC. Fdc=%x Status=%x\n",
- byte, fdc, status);
+ byte, current_fdc, status);
show_floppy();
}
return -1;
@@ -1166,16 +1169,16 @@ static int result(void)
return i;
}
if (status == (STATUS_DIR | STATUS_READY | STATUS_BUSY))
- reply_buffer[i] = fdc_inb(fdc, FD_DATA);
+ reply_buffer[i] = fdc_inb(current_fdc, FD_DATA);
else
break;
}
if (initialized) {
DPRINT("get result error. Fdc=%d Last status=%x Read bytes=%d\n",
- fdc, status, i);
+ current_fdc, status, i);
show_floppy();
}
- fdc_state[fdc].reset = 1;
+ fdc_state[current_fdc].reset = 1;
return -1;
}
@@ -1212,7 +1215,7 @@ static void perpendicular_mode(void)
default:
DPRINT("Invalid data rate for perpendicular mode!\n");
cont->done(0);
- fdc_state[fdc].reset = 1;
+ fdc_state[current_fdc].reset = 1;
/*
* convenient way to return to
* redo without too much hassle
@@ -1223,12 +1226,12 @@ static void perpendicular_mode(void)
} else
perp_mode = 0;
- if (fdc_state[fdc].perp_mode == perp_mode)
+ if (fdc_state[current_fdc].perp_mode == perp_mode)
return;
- if (fdc_state[fdc].version >= FDC_82077_ORIG) {
+ if (fdc_state[current_fdc].version >= FDC_82077_ORIG) {
output_byte(FD_PERPENDICULAR);
output_byte(perp_mode);
- fdc_state[fdc].perp_mode = perp_mode;
+ fdc_state[current_fdc].perp_mode = perp_mode;
} else if (perp_mode) {
DPRINT("perpendicular mode not supported by this FDC.\n");
}
@@ -1283,9 +1286,10 @@ static void fdc_specify(void)
int hlt_max_code = 0x7f;
int hut_max_code = 0xf;
- if (fdc_state[fdc].need_configure && fdc_state[fdc].version >= FDC_82072A) {
+ if (fdc_state[current_fdc].need_configure &&
+ fdc_state[current_fdc].version >= FDC_82072A) {
fdc_configure();
- fdc_state[fdc].need_configure = 0;
+ fdc_state[current_fdc].need_configure = 0;
}
switch (raw_cmd->rate & 0x03) {
@@ -1294,7 +1298,7 @@ static void fdc_specify(void)
break;
case 1:
dtr = 300;
- if (fdc_state[fdc].version >= FDC_82078) {
+ if (fdc_state[current_fdc].version >= FDC_82078) {
/* chose the default rate table, not the one
* where 1 = 2 Mbps */
output_byte(FD_DRIVESPEC);
@@ -1309,7 +1313,7 @@ static void fdc_specify(void)
break;
}
- if (fdc_state[fdc].version >= FDC_82072) {
+ if (fdc_state[current_fdc].version >= FDC_82072) {
scale_dtr = dtr;
hlt_max_code = 0x00; /* 0==256msec*dtr0/dtr (not linear!) */
hut_max_code = 0x0; /* 0==256msec*dtr0/dtr (not linear!) */
@@ -1342,11 +1346,12 @@ static void fdc_specify(void)
spec2 = (hlt << 1) | (use_virtual_dma & 1);
/* If these parameters did not change, just return with success */
- if (fdc_state[fdc].spec1 != spec1 || fdc_state[fdc].spec2 != spec2) {
+ if (fdc_state[current_fdc].spec1 != spec1 ||
+ fdc_state[current_fdc].spec2 != spec2) {
/* Go ahead and set spec1 and spec2 */
output_byte(FD_SPECIFY);
- output_byte(fdc_state[fdc].spec1 = spec1);
- output_byte(fdc_state[fdc].spec2 = spec2);
+ output_byte(fdc_state[current_fdc].spec1 = spec1);
+ output_byte(fdc_state[current_fdc].spec2 = spec2);
}
} /* fdc_specify */
@@ -1357,18 +1362,18 @@ static void fdc_specify(void)
static int fdc_dtr(void)
{
/* If data rate not already set to desired value, set it. */
- if ((raw_cmd->rate & 3) == fdc_state[fdc].dtr)
+ if ((raw_cmd->rate & 3) == fdc_state[current_fdc].dtr)
return 0;
/* Set dtr */
- fdc_outb(raw_cmd->rate & 3, fdc, FD_DCR);
+ fdc_outb(raw_cmd->rate & 3, current_fdc, FD_DCR);
/* TODO: some FDC/drive combinations (C&T 82C711 with TEAC 1.2MB)
* need a stabilization period of several milliseconds to be
* enforced after data rate changes before R/W operations.
* Pause 5 msec to avoid trouble. (Needs to be 2 jiffies)
*/
- fdc_state[fdc].dtr = raw_cmd->rate & 3;
+ fdc_state[current_fdc].dtr = raw_cmd->rate & 3;
return fd_wait_for_completion(jiffies + 2UL * HZ / 100, floppy_ready);
} /* fdc_dtr */
@@ -1424,7 +1429,7 @@ static int interpret_errors(void)
if (inr != 7) {
DPRINT("-- FDC reply error\n");
- fdc_state[fdc].reset = 1;
+ fdc_state[current_fdc].reset = 1;
return 1;
}
@@ -1564,7 +1569,7 @@ static void check_wp(void)
output_byte(FD_GETSTATUS);
output_byte(UNIT(current_drive));
if (result() != 1) {
- fdc_state[fdc].reset = 1;
+ fdc_state[current_fdc].reset = 1;
return;
}
clear_bit(FD_VERIFY_BIT, &drive_state[current_drive].flags);
@@ -1616,7 +1621,7 @@ static void seek_floppy(void)
track = raw_cmd->track - 1;
else {
if (drive_params[current_drive].flags & FD_SILENT_DCL_CLEAR) {
- set_dor(fdc, ~(0x10 << UNIT(current_drive)), 0);
+ set_dor(current_fdc, ~(0x10 << UNIT(current_drive)), 0);
blind_seek = 1;
raw_cmd->flags |= FD_RAW_NEED_SEEK;
}
@@ -1647,7 +1652,7 @@ static void recal_interrupt(void)
{
debugt(__func__, "");
if (inr != 2)
- fdc_state[fdc].reset = 1;
+ fdc_state[current_fdc].reset = 1;
else if (reply_buffer[ST0] & ST0_ECE) {
switch (drive_state[current_drive].track) {
case NEED_1_RECAL:
@@ -1716,16 +1721,16 @@ irqreturn_t floppy_interrupt(int irq, void *dev_id)
release_dma_lock(f);
do_floppy = NULL;
- if (fdc >= N_FDC || fdc_state[fdc].address == -1) {
+ if (current_fdc >= N_FDC || fdc_state[current_fdc].address == -1) {
/* we don't even know which FDC is the culprit */
pr_info("DOR0=%x\n", fdc_state[0].dor);
- pr_info("floppy interrupt on bizarre fdc %d\n", fdc);
+ pr_info("floppy interrupt on bizarre fdc %d\n", current_fdc);
pr_info("handler=%ps\n", handler);
is_alive(__func__, "bizarre fdc");
return IRQ_NONE;
}
- fdc_state[fdc].reset = 0;
+ fdc_state[current_fdc].reset = 0;
/* We have to clear the reset flag here, because apparently on boxes
* with level triggered interrupts (PS/2, Sparc, ...), it is needed to
* emit SENSEI's to clear the interrupt line. And fdc_state[fdc].reset
@@ -1752,7 +1757,7 @@ irqreturn_t floppy_interrupt(int irq, void *dev_id)
inr == 2 && max_sensei);
}
if (!handler) {
- fdc_state[fdc].reset = 1;
+ fdc_state[current_fdc].reset = 1;
return IRQ_NONE;
}
schedule_bh(handler);
@@ -1778,7 +1783,7 @@ static void reset_interrupt(void)
{
debugt(__func__, "");
result(); /* get the status ready for set_fdc */
- if (fdc_state[fdc].reset) {
+ if (fdc_state[current_fdc].reset) {
pr_info("reset set in interrupt, calling %ps\n", cont->error);
cont->error(); /* a reset just after a reset. BAD! */
}
@@ -1794,7 +1799,7 @@ static void reset_fdc(void)
unsigned long flags;
do_floppy = reset_interrupt;
- fdc_state[fdc].reset = 0;
+ fdc_state[current_fdc].reset = 0;
reset_fdc_info(0);
/* Pseudo-DMA may intercept 'reset finished' interrupt. */
@@ -1804,12 +1809,13 @@ static void reset_fdc(void)
fd_disable_dma();
release_dma_lock(flags);
- if (fdc_state[fdc].version >= FDC_82072A)
- fdc_outb(0x80 | (fdc_state[fdc].dtr & 3), fdc, FD_STATUS);
+ if (fdc_state[current_fdc].version >= FDC_82072A)
+ fdc_outb(0x80 | (fdc_state[current_fdc].dtr & 3),
+ current_fdc, FD_STATUS);
else {
- fdc_outb(fdc_state[fdc].dor & ~0x04, fdc, FD_DOR);
+ fdc_outb(fdc_state[current_fdc].dor & ~0x04, current_fdc, FD_DOR);
udelay(FD_RESET_DELAY);
- fdc_outb(fdc_state[fdc].dor, fdc, FD_DOR);
+ fdc_outb(fdc_state[current_fdc].dor, current_fdc, FD_DOR);
}
}
@@ -1836,7 +1842,7 @@ static void show_floppy(void)
print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1,
reply_buffer, resultsize, true);
- pr_info("status=%x\n", fdc_inb(fdc, FD_STATUS));
+ pr_info("status=%x\n", fdc_inb(current_fdc, FD_STATUS));
pr_info("fdc_busy=%lu\n", fdc_busy);
if (do_floppy)
pr_info("do_floppy=%ps\n", do_floppy);
@@ -1873,7 +1879,7 @@ static void floppy_shutdown(struct work_struct *arg)
if (initialized)
DPRINT("floppy timeout called\n");
- fdc_state[fdc].reset = 1;
+ fdc_state[current_fdc].reset = 1;
if (cont) {
cont->done(0);
cont->redo(); /* this will recall reset when needed */
@@ -1893,7 +1899,7 @@ static int start_motor(void (*function)(void))
mask = 0xfc;
data = UNIT(current_drive);
if (!(raw_cmd->flags & FD_RAW_NO_MOTOR)) {
- if (!(fdc_state[fdc].dor & (0x10 << UNIT(current_drive)))) {
+ if (!(fdc_state[current_fdc].dor & (0x10 << UNIT(current_drive)))) {
set_debugt();
/* no read since this drive is running */
drive_state[current_drive].first_read_date = 0;
@@ -1901,12 +1907,12 @@ static int start_motor(void (*function)(void))
drive_state[current_drive].spinup_date = jiffies;
data |= (0x10 << UNIT(current_drive));
}
- } else if (fdc_state[fdc].dor & (0x10 << UNIT(current_drive)))
+ } else if (fdc_state[current_fdc].dor & (0x10 << UNIT(current_drive)))
mask &= ~(0x10 << UNIT(current_drive));
/* starts motor and selects floppy */
del_timer(motor_off_timer + current_drive);
- set_dor(fdc, mask, data);
+ set_dor(current_fdc, mask, data);
/* wait_for_completion also schedules reset if needed. */
return fd_wait_for_completion(drive_state[current_drive].select_date + drive_params[current_drive].select_delay,
@@ -1915,7 +1921,7 @@ static int start_motor(void (*function)(void))
static void floppy_ready(void)
{
- if (fdc_state[fdc].reset) {
+ if (fdc_state[current_fdc].reset) {
reset_fdc();
return;
}
@@ -2016,7 +2022,7 @@ static int wait_til_done(void (*handler)(void), bool interruptible)
return -EINTR;
}
- if (fdc_state[fdc].reset)
+ if (fdc_state[current_fdc].reset)
command_status = FD_COMMAND_ERROR;
if (command_status == FD_COMMAND_OKAY)
ret = 0;
@@ -2085,7 +2091,7 @@ static void bad_flp_intr(void)
if (err_count > drive_params[current_drive].max_errors.abort)
cont->done(0);
if (err_count > drive_params[current_drive].max_errors.reset)
- fdc_state[fdc].reset = 1;
+ fdc_state[current_fdc].reset = 1;
else if (err_count > drive_params[current_drive].max_errors.recal)
drive_state[current_drive].track = NEED_2_RECAL;
}
@@ -2998,8 +3004,8 @@ static int user_reset_fdc(int drive, int arg, bool interruptible)
return -EINTR;
if (arg == FD_RESET_ALWAYS)
- fdc_state[fdc].reset = 1;
- if (fdc_state[fdc].reset) {
+ fdc_state[current_fdc].reset = 1;
+ if (fdc_state[current_fdc].reset) {
cont = &reset_cont;
ret = wait_til_done(reset_fdc, interruptible);
if (ret == -EINTR)
@@ -3210,23 +3216,23 @@ static int raw_cmd_ioctl(int cmd, void __user *param)
int ret2;
int ret;
- if (fdc_state[fdc].rawcmd <= 1)
- fdc_state[fdc].rawcmd = 1;
+ if (fdc_state[current_fdc].rawcmd <= 1)
+ fdc_state[current_fdc].rawcmd = 1;
for (drive = 0; drive < N_DRIVE; drive++) {
- if (FDC(drive) != fdc)
+ if (FDC(drive) != current_fdc)
continue;
if (drive == current_drive) {
if (drive_state[drive].fd_ref > 1) {
- fdc_state[fdc].rawcmd = 2;
+ fdc_state[current_fdc].rawcmd = 2;
break;
}
} else if (drive_state[drive].fd_ref) {
- fdc_state[fdc].rawcmd = 2;
+ fdc_state[current_fdc].rawcmd = 2;
break;
}
}
- if (fdc_state[fdc].reset)
+ if (fdc_state[current_fdc].reset)
return -EIO;
ret = raw_cmd_copyin(cmd, param, &my_raw_cmd);
@@ -3241,7 +3247,7 @@ static int raw_cmd_ioctl(int cmd, void __user *param)
debug_dcl(drive_params[current_drive].flags,
"calling disk change from raw_cmd ioctl\n");
- if (ret != -EINTR && fdc_state[fdc].reset)
+ if (ret != -EINTR && fdc_state[current_fdc].reset)
ret = -EIO;
drive_state[current_drive].track = NO_TRACK;
@@ -4297,23 +4303,23 @@ static char __init get_fdc_version(void)
int r;
output_byte(FD_DUMPREGS); /* 82072 and better know DUMPREGS */
- if (fdc_state[fdc].reset)
+ if (fdc_state[current_fdc].reset)
return FDC_NONE;
r = result();
if (r <= 0x00)
return FDC_NONE; /* No FDC present ??? */
if ((r == 1) && (reply_buffer[0] == 0x80)) {
- pr_info("FDC %d is an 8272A\n", fdc);
+ pr_info("FDC %d is an 8272A\n", current_fdc);
return FDC_8272A; /* 8272a/765 don't know DUMPREGS */
}
if (r != 10) {
pr_info("FDC %d init: DUMPREGS: unexpected return of %d bytes.\n",
- fdc, r);
+ current_fdc, r);
return FDC_UNKNOWN;
}
if (!fdc_configure()) {
- pr_info("FDC %d is an 82072\n", fdc);
+ pr_info("FDC %d is an 82072\n", current_fdc);
return FDC_82072; /* 82072 doesn't know CONFIGURE */
}
@@ -4321,50 +4327,50 @@ static char __init get_fdc_version(void)
if (need_more_output() == MORE_OUTPUT) {
output_byte(0);
} else {
- pr_info("FDC %d is an 82072A\n", fdc);
+ pr_info("FDC %d is an 82072A\n", current_fdc);
return FDC_82072A; /* 82072A as found on Sparcs. */
}
output_byte(FD_UNLOCK);
r = result();
if ((r == 1) && (reply_buffer[0] == 0x80)) {
- pr_info("FDC %d is a pre-1991 82077\n", fdc);
+ pr_info("FDC %d is a pre-1991 82077\n", current_fdc);
return FDC_82077_ORIG; /* Pre-1991 82077, doesn't know
* LOCK/UNLOCK */
}
if ((r != 1) || (reply_buffer[0] != 0x00)) {
pr_info("FDC %d init: UNLOCK: unexpected return of %d bytes.\n",
- fdc, r);
+ current_fdc, r);
return FDC_UNKNOWN;
}
output_byte(FD_PARTID);
r = result();
if (r != 1) {
pr_info("FDC %d init: PARTID: unexpected return of %d bytes.\n",
- fdc, r);
+ current_fdc, r);
return FDC_UNKNOWN;
}
if (reply_buffer[0] == 0x80) {
- pr_info("FDC %d is a post-1991 82077\n", fdc);
+ pr_info("FDC %d is a post-1991 82077\n", current_fdc);
return FDC_82077; /* Revised 82077AA passes all the tests */
}
switch (reply_buffer[0] >> 5) {
case 0x0:
/* Either a 82078-1 or a 82078SL running at 5Volt */
- pr_info("FDC %d is an 82078.\n", fdc);
+ pr_info("FDC %d is an 82078.\n", current_fdc);
return FDC_82078;
case 0x1:
- pr_info("FDC %d is a 44pin 82078\n", fdc);
+ pr_info("FDC %d is a 44pin 82078\n", current_fdc);
return FDC_82078;
case 0x2:
- pr_info("FDC %d is a S82078B\n", fdc);
+ pr_info("FDC %d is a S82078B\n", current_fdc);
return FDC_S82078B;
case 0x3:
- pr_info("FDC %d is a National Semiconductor PC87306\n", fdc);
+ pr_info("FDC %d is a National Semiconductor PC87306\n", current_fdc);
return FDC_87306;
default:
pr_info("FDC %d init: 82078 variant with unknown PARTID=%d.\n",
- fdc, reply_buffer[0] >> 5);
+ current_fdc, reply_buffer[0] >> 5);
return FDC_82078_UNKN;
}
} /* get_fdc_version */
@@ -4640,16 +4646,16 @@ static int __init do_floppy_init(void)
config_types();
for (i = 0; i < N_FDC; i++) {
- fdc = i;
- memset(&fdc_state[fdc], 0, sizeof(*fdc_state));
- fdc_state[fdc].dtr = -1;
- fdc_state[fdc].dor = 0x4;
+ current_fdc = i;
+ memset(&fdc_state[current_fdc], 0, sizeof(*fdc_state));
+ fdc_state[current_fdc].dtr = -1;
+ fdc_state[current_fdc].dor = 0x4;
#if defined(__sparc__) || defined(__mc68000__)
/*sparcs/sun3x don't have a DOR reset which we can fall back on to */
#ifdef __mc68000__
if (MACH_IS_SUN3X)
#endif
- fdc_state[fdc].version = FDC_82072A;
+ fdc_state[current_fdc].version = FDC_82072A;
#endif
}
@@ -4664,7 +4670,7 @@ static int __init do_floppy_init(void)
fdc_state[1].address = FDC2;
#endif
- fdc = 0; /* reset fdc in case of unexpected interrupt */
+ current_fdc = 0; /* reset fdc in case of unexpected interrupt */
err = floppy_grab_irq_and_dma();
if (err) {
cancel_delayed_work(&fd_timeout);
@@ -4691,29 +4697,30 @@ static int __init do_floppy_init(void)
msleep(10);
for (i = 0; i < N_FDC; i++) {
- fdc = i;
- fdc_state[fdc].driver_version = FD_DRIVER_VERSION;
+ current_fdc = i;
+ fdc_state[current_fdc].driver_version = FD_DRIVER_VERSION;
for (unit = 0; unit < 4; unit++)
- fdc_state[fdc].track[unit] = 0;
- if (fdc_state[fdc].address == -1)
+ fdc_state[current_fdc].track[unit] = 0;
+ if (fdc_state[current_fdc].address == -1)
continue;
- fdc_state[fdc].rawcmd = 2;
+ fdc_state[current_fdc].rawcmd = 2;
if (user_reset_fdc(-1, FD_RESET_ALWAYS, false)) {
/* free ioports reserved by floppy_grab_irq_and_dma() */
- floppy_release_regions(fdc);
- fdc_state[fdc].address = -1;
- fdc_state[fdc].version = FDC_NONE;
+ floppy_release_regions(current_fdc);
+ fdc_state[current_fdc].address = -1;
+ fdc_state[current_fdc].version = FDC_NONE;
continue;
}
/* Try to determine the floppy controller type */
- fdc_state[fdc].version = get_fdc_version();
- if (fdc_state[fdc].version == FDC_NONE) {
+ fdc_state[current_fdc].version = get_fdc_version();
+ if (fdc_state[current_fdc].version == FDC_NONE) {
/* free ioports reserved by floppy_grab_irq_and_dma() */
- floppy_release_regions(fdc);
- fdc_state[fdc].address = -1;
+ floppy_release_regions(current_fdc);
+ fdc_state[current_fdc].address = -1;
continue;
}
- if (can_use_virtual_dma == 2 && fdc_state[fdc].version < FDC_82072A)
+ if (can_use_virtual_dma == 2 &&
+ fdc_state[current_fdc].version < FDC_82072A)
can_use_virtual_dma = 0;
have_no_fdc = 0;
@@ -4723,7 +4730,7 @@ static int __init do_floppy_init(void)
*/
user_reset_fdc(-1, FD_RESET_ALWAYS, false);
}
- fdc = 0;
+ current_fdc = 0;
cancel_delayed_work(&fd_timeout);
current_drive = 0;
initialized = true;
@@ -4875,36 +4882,36 @@ static int floppy_grab_irq_and_dma(void)
}
}
- for (fdc = 0; fdc < N_FDC; fdc++) {
- if (fdc_state[fdc].address != -1) {
- if (floppy_request_regions(fdc))
+ for (current_fdc = 0; current_fdc < N_FDC; current_fdc++) {
+ if (fdc_state[current_fdc].address != -1) {
+ if (floppy_request_regions(current_fdc))
goto cleanup;
}
}
- for (fdc = 0; fdc < N_FDC; fdc++) {
- if (fdc_state[fdc].address != -1) {
+ for (current_fdc = 0; current_fdc < N_FDC; current_fdc++) {
+ if (fdc_state[current_fdc].address != -1) {
reset_fdc_info(1);
- fdc_outb(fdc_state[fdc].dor, fdc, FD_DOR);
+ fdc_outb(fdc_state[current_fdc].dor, current_fdc, FD_DOR);
}
}
- fdc = 0;
+ current_fdc = 0;
set_dor(0, ~0, 8); /* avoid immediate interrupt */
- for (fdc = 0; fdc < N_FDC; fdc++)
- if (fdc_state[fdc].address != -1)
- fdc_outb(fdc_state[fdc].dor, fdc, FD_DOR);
+ for (current_fdc = 0; current_fdc < N_FDC; current_fdc++)
+ if (fdc_state[current_fdc].address != -1)
+ fdc_outb(fdc_state[current_fdc].dor, current_fdc, FD_DOR);
/*
* The driver will try and free resources and relies on us
* to know if they were allocated or not.
*/
- fdc = 0;
+ current_fdc = 0;
irqdma_allocated = 1;
return 0;
cleanup:
fd_free_irq();
fd_free_dma();
- while (--fdc >= 0)
- floppy_release_regions(fdc);
+ while (--current_fdc >= 0)
+ floppy_release_regions(current_fdc);
atomic_dec(&usage_count);
return -1;
}
@@ -4952,11 +4959,11 @@ static void floppy_release_irq_and_dma(void)
pr_info("auxiliary floppy timer still active\n");
if (work_pending(&floppy_work))
pr_info("work still pending\n");
- old_fdc = fdc;
- for (fdc = 0; fdc < N_FDC; fdc++)
- if (fdc_state[fdc].address != -1)
- floppy_release_regions(fdc);
- fdc = old_fdc;
+ old_fdc = current_fdc;
+ for (current_fdc = 0; current_fdc < N_FDC; current_fdc++)
+ if (fdc_state[current_fdc].address != -1)
+ floppy_release_regions(current_fdc);
+ current_fdc = old_fdc;
}
#ifdef MODULE
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/6] floppy: make use of the local/global fdc explicit
2020-03-01 19:55 [PATCH v2 0/6] floppy: make use of the local/global fdc explicit Willy Tarreau
` (5 preceding siblings ...)
2020-03-01 19:55 ` [PATCH v2 6/6] floppy: rename the global "fdc" variable to "current_fdc" Willy Tarreau
@ 2020-03-01 20:33 ` Joe Perches
2020-03-01 21:20 ` Willy Tarreau
2020-03-08 13:12 ` Denis Efremov
7 siblings, 1 reply; 10+ messages in thread
From: Joe Perches @ 2020-03-01 20:33 UTC (permalink / raw)
To: Willy Tarreau, Denis Efremov; +Cc: Jens Axboe, linux-kernel, linux-block
On Sun, 2020-03-01 at 20:55 +0100, Willy Tarreau wrote:
> This is an update to the first minimal cleanup of the floppy driver in
> order to make use of the FDC number explicit so as to avoid bugs like
> the one fixed by 2e90ca68 ("floppy: check FDC index for errors before
> assigning it").
Thanks Willy.
trivia: you could style check the patches using checkpatch.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/6] floppy: make use of the local/global fdc explicit
2020-03-01 20:33 ` [PATCH v2 0/6] floppy: make use of the local/global fdc explicit Joe Perches
@ 2020-03-01 21:20 ` Willy Tarreau
0 siblings, 0 replies; 10+ messages in thread
From: Willy Tarreau @ 2020-03-01 21:20 UTC (permalink / raw)
To: Joe Perches; +Cc: Denis Efremov, Jens Axboe, linux-kernel, linux-block
On Sun, Mar 01, 2020 at 12:33:25PM -0800, Joe Perches wrote:
> On Sun, 2020-03-01 at 20:55 +0100, Willy Tarreau wrote:
> > This is an update to the first minimal cleanup of the floppy driver in
> > order to make use of the FDC number explicit so as to avoid bugs like
> > the one fixed by 2e90ca68 ("floppy: check FDC index for errors before
> > assigning it").
>
> Thanks Willy.
>
> trivia: you could style check the patches using checkpatch.
Oh yes, sorry about this, I did it in the first series but forgot to
do it again. What checkpatch complains about is essentially lines which
slightly exceed 80 characters due to the macro expansions, while I tried
to limit the changes to the strict minimum. I guess that's OK to keep
future fixes easier to backport.
Thanks,
Willy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/6] floppy: make use of the local/global fdc explicit
2020-03-01 19:55 [PATCH v2 0/6] floppy: make use of the local/global fdc explicit Willy Tarreau
` (6 preceding siblings ...)
2020-03-01 20:33 ` [PATCH v2 0/6] floppy: make use of the local/global fdc explicit Joe Perches
@ 2020-03-08 13:12 ` Denis Efremov
7 siblings, 0 replies; 10+ messages in thread
From: Denis Efremov @ 2020-03-08 13:12 UTC (permalink / raw)
To: Willy Tarreau
Cc: Jens Axboe, linux-kernel, linux-block, Ian Molton, Russell King
Hi,
On 3/1/20 10:55 PM, Willy Tarreau wrote:
> This is an update to the first minimal cleanup of the floppy driver in
> order to make use of the FDC number explicit so as to avoid bugs like
> the one fixed by 2e90ca68 ("floppy: check FDC index for errors before
> assigning it").
>
> The purpose of this patchset is to rename the "fdc" global variable to
> "current_fdc" as Linus suggested and adjust the macros which rely on it
> depending on their context.
>
> The most problematic part at this step are the FD_* macros derived
> from FD_IOPORT, itself referencing the fdc to get its base address.
> These are exclusively used by fd_outb() and fd_inb(). However on ARM
> FD_DOR is also used to compare the register based on the port, hence
> a small change in the ARM specific code to only check the register
> without relying on this hidden memory access.
>
> In order to avoid touching the fd_outb() and fd_inb() macros/functions
> on all supported architectures, a new set of fdc_outb()/fdc_inb()
> functions was added to the driver to call the former after adding
> the register to the FDC's base address.
>
> There are still opportunities for more cleanup, though it's uncertain
> they're welcome in this old driver :
> - the base address and register can be passed separately to fd_outb()
> and fd_inb() in order to simplify register retrieval in some archs;
>
> - a dozen of functions in the driver implicitly depend on current_fdc
> while passing it as an argument makes the driver a bit more readable
> but that represents less than half of the code and doesn't address
> all the readability concerns;
>
> - a test was done to limit support to a single FDC, but after these
> cleanups it doesn't provide any significant benefit in terms of code
> readability.
>
> These patches are to be applied on top of Denis' floppy-next branch.
>
> v2:
> - CC arch maintainers in ARM patches
> - fixed issues after Denis' review:
> - extra braces in floppy.h in declaration of floppy_selects[]
> - missing parenthesis in fd_outb() macro to silence a warning
> - used the swap() macro in driveswap()
> Willy Tarreau (6):
Applied, thanks!
https://github.com/evdenis/linux-floppy/commits/floppy-next
Ian, Russell, I hope you don't mind if these patches will go through
the single tree. If you have any comments, please, write.
Tested:
[x] Eye-checked the changes
[x] Checked that kernel builds after every commit on x86, arm (rpc_defconfig)
[x] Checked that there is no binary difference on x86
> floppy: remove dead code for drives scanning on ARM
[x] Checked that there is no dead code left unnoticed
> floppy: remove incomplete support for second FDC from ARM code
> floppy: prepare ARM code to simplify base address separation
> floppy: introduce new functions fdc_inb() and fdc_outb()
> floppy: separate the FDC's base address from its registers
> floppy: rename the global "fdc" variable to "current_fdc"
>
> arch/arm/include/asm/floppy.h | 88 ++-----------
> drivers/block/floppy.c | 284 ++++++++++++++++++++++--------------------
> include/uapi/linux/fdreg.h | 18 +--
> 3 files changed, 168 insertions(+), 222 deletions(-)
>
Denis
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-03-08 13:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-01 19:55 [PATCH v2 0/6] floppy: make use of the local/global fdc explicit Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 1/6] floppy: remove dead code for drives scanning on ARM Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 2/6] floppy: remove incomplete support for second FDC from ARM code Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 3/6] floppy: prepare ARM code to simplify base address separation Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 4/6] floppy: introduce new functions fdc_inb() and fdc_outb() Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 5/6] floppy: separate the FDC's base address from its registers Willy Tarreau
2020-03-01 19:55 ` [PATCH v2 6/6] floppy: rename the global "fdc" variable to "current_fdc" Willy Tarreau
2020-03-01 20:33 ` [PATCH v2 0/6] floppy: make use of the local/global fdc explicit Joe Perches
2020-03-01 21:20 ` Willy Tarreau
2020-03-08 13:12 ` Denis Efremov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).