* [PATCH 00/10] floppy driver cleanups (deobfuscation)
@ 2020-02-24 21:23 Willy Tarreau
2020-02-24 21:23 ` [PATCH 01/10] floppy: cleanup: expand macro FDCS Willy Tarreau
` (11 more replies)
0 siblings, 12 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-24 21:23 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Linus Torvalds, Willy Tarreau
As indicated in commit 2e90ca6 ("floppy: check FDC index for errors
before assigning it") there are some surprising effects in the floppy
driver due to some macros referencing global or local variables while
at first glance being inoffensive.
This patchset aims at removing these macros and replacing all of their
occurrences by the equivalent code. Most of the work was done under
Coccinelle's assistance, and it was verified that the resulting binary
code is exactly the same as the original one.
The aim is not to make the driver prettier, as Linus mentioned it's
already not pretty. It only aims at making potential bugs more visible,
given almost all latest changes to this driver were fixes for out-of-
bounds and similar bugs.
As a side effect, some lines got longer, causing checkpatch to complain
a bit, but I preferred to let it complain as I didn't want to break them
apart as I'm already seeing the trap of going too far here.
The patches are broken by macro (or sets of macros when relevant) so
that each of them remains reviewable.
I can possibly go a bit further in the cleanup but I haven't used
floppies for a few years now and am not interested in doing too much
on this driver by lack of use cases.
Willy Tarreau (10):
floppy: cleanup: expand macro FDCS
floppy: cleanup: expand macro UFDCS
floppy: cleanup: expand macro UDP
floppy: cleanup: expand macro UDRS
floppy: cleanup: expand macro UDRWE
floppy: cleanup: expand macro DP
floppy: cleanup: expand macro DRS
floppy: cleanup: expand macro DRWE
floppy: cleanup: expand the R/W / format command macros
floppy: cleanup: expand the reply_buffer macros
drivers/block/floppy.c | 971 +++++++++++++++++++++++++------------------------
1 file changed, 499 insertions(+), 472 deletions(-)
--
2.9.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-24 21:23 [PATCH 00/10] floppy driver cleanups (deobfuscation) Willy Tarreau
@ 2020-02-24 21:23 ` Willy Tarreau
2020-02-24 21:53 ` Linus Torvalds
2020-02-25 11:37 ` Denis Efremov
2020-02-24 21:23 ` [PATCH 02/10] floppy: cleanup: expand macro UFDCS Willy Tarreau
` (10 subsequent siblings)
11 siblings, 2 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-24 21:23 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Linus Torvalds, Willy Tarreau
Macro FDCS silently uses identifier "fdc" which may be either the
global one or a local one. Let's expand the macro to make this more
obvious.
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/block/floppy.c | 183 ++++++++++++++++++++++++-------------------------
1 file changed, 91 insertions(+), 92 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 8ef65c0..93e0840 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -309,7 +309,6 @@ static bool initialized;
#define DP (&drive_params[current_drive])
#define DRS (&drive_state[current_drive])
#define DRWE (&write_errors[current_drive])
-#define FDCS (&fdc_state[fdc])
#define UDP (&drive_params[drive])
#define UDRS (&drive_state[drive])
@@ -742,11 +741,11 @@ static int disk_change(int drive)
if (time_before(jiffies, UDRS->select_date + UDP->select_delay))
DPRINT("WARNING disk change called early\n");
- if (!(FDCS->dor & (0x10 << UNIT(drive))) ||
- (FDCS->dor & 3) != UNIT(drive) || fdc != FDC(drive)) {
+ if (!(fdc_state[fdc].dor & (0x10 << UNIT(drive))) ||
+ (fdc_state[fdc].dor & 3) != UNIT(drive) || fdc != FDC(drive)) {
DPRINT("probing disk change on unselected drive\n");
DPRINT("drive=%d fdc=%d dor=%x\n", drive, FDC(drive),
- (unsigned int)FDCS->dor);
+ (unsigned int)fdc_state[fdc].dor);
}
debug_dcl(UDP->flags,
@@ -799,10 +798,10 @@ static int set_dor(int fdc, char mask, char data)
unsigned char newdor;
unsigned char olddor;
- if (FDCS->address == -1)
+ if (fdc_state[fdc].address == -1)
return -1;
- olddor = FDCS->dor;
+ olddor = fdc_state[fdc].dor;
newdor = (olddor & mask) | data;
if (newdor != olddor) {
unit = olddor & 0x3;
@@ -812,7 +811,7 @@ static int set_dor(int fdc, char mask, char data)
"calling disk change from set_dor\n");
disk_change(drive);
}
- FDCS->dor = newdor;
+ fdc_state[fdc].dor = newdor;
fd_outb(newdor, FD_DOR);
unit = newdor & 0x3;
@@ -828,8 +827,8 @@ static void twaddle(void)
{
if (DP->select_delay)
return;
- fd_outb(FDCS->dor & ~(0x10 << UNIT(current_drive)), FD_DOR);
- fd_outb(FDCS->dor, FD_DOR);
+ fd_outb(fdc_state[fdc].dor & ~(0x10 << UNIT(current_drive)), FD_DOR);
+ fd_outb(fdc_state[fdc].dor, FD_DOR);
DRS->select_date = jiffies;
}
@@ -841,10 +840,10 @@ static void reset_fdc_info(int mode)
{
int drive;
- FDCS->spec1 = FDCS->spec2 = -1;
- FDCS->need_configure = 1;
- FDCS->perp_mode = 1;
- FDCS->rawcmd = 0;
+ 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;
for (drive = 0; drive < N_DRIVE; drive++)
if (FDC(drive) == fdc && (mode || UDRS->track != NEED_1_RECAL))
UDRS->track = NEED_2_RECAL;
@@ -868,10 +867,10 @@ static void set_fdc(int drive)
#if N_FDC > 1
set_dor(1 - fdc, ~8, 0);
#endif
- if (FDCS->rawcmd == 2)
+ if (fdc_state[fdc].rawcmd == 2)
reset_fdc_info(1);
if (fd_inb(FD_STATUS) != STATUS_READY)
- FDCS->reset = 1;
+ fdc_state[fdc].reset = 1;
}
/* locks the driver */
@@ -924,7 +923,7 @@ static void floppy_off(unsigned int drive)
unsigned long volatile delta;
int fdc = FDC(drive);
- if (!(FDCS->dor & (0x10 << UNIT(drive))))
+ if (!(fdc_state[fdc].dor & (0x10 << UNIT(drive))))
return;
del_timer(motor_off_timer + drive);
@@ -1035,7 +1034,7 @@ static void main_command_interrupt(void)
static int fd_wait_for_completion(unsigned long expires,
void (*function)(void))
{
- if (FDCS->reset) {
+ if (fdc_state[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 */
@@ -1063,13 +1062,13 @@ static void setup_DMA(void)
pr_cont("%x,", raw_cmd->cmd[i]);
pr_cont("\n");
cont->done(0);
- FDCS->reset = 1;
+ fdc_state[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);
- FDCS->reset = 1;
+ fdc_state[fdc].reset = 1;
return;
}
f = claim_dma_lock();
@@ -1077,10 +1076,10 @@ 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, FDCS->address) < 0) {
+ DMA_MODE_READ : DMA_MODE_WRITE, fdc_state[fdc].address) < 0) {
release_dma_lock(f);
cont->done(0);
- FDCS->reset = 1;
+ fdc_state[fdc].reset = 1;
return;
}
release_dma_lock(f);
@@ -1091,7 +1090,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 = FDCS->address;
+ virtual_dma_port = fdc_state[fdc].address;
fd_enable_dma();
release_dma_lock(f);
#endif
@@ -1105,7 +1104,7 @@ static int wait_til_ready(void)
int status;
int counter;
- if (FDCS->reset)
+ if (fdc_state[fdc].reset)
return -1;
for (counter = 0; counter < 10000; counter++) {
status = fd_inb(FD_STATUS);
@@ -1116,7 +1115,7 @@ static int wait_til_ready(void)
DPRINT("Getstatus times out (%x) on fdc %d\n", status, fdc);
show_floppy();
}
- FDCS->reset = 1;
+ fdc_state[fdc].reset = 1;
return -1;
}
@@ -1136,7 +1135,7 @@ static int output_byte(char byte)
output_log_pos = (output_log_pos + 1) % OLOGSIZE;
return 0;
}
- FDCS->reset = 1;
+ fdc_state[fdc].reset = 1;
if (initialized) {
DPRINT("Unable to send byte %x to FDC. Fdc=%x Status=%x\n",
byte, fdc, status);
@@ -1171,7 +1170,7 @@ static int result(void)
fdc, status, i);
show_floppy();
}
- FDCS->reset = 1;
+ fdc_state[fdc].reset = 1;
return -1;
}
@@ -1208,7 +1207,7 @@ static void perpendicular_mode(void)
default:
DPRINT("Invalid data rate for perpendicular mode!\n");
cont->done(0);
- FDCS->reset = 1;
+ fdc_state[fdc].reset = 1;
/*
* convenient way to return to
* redo without too much hassle
@@ -1219,12 +1218,12 @@ static void perpendicular_mode(void)
} else
perp_mode = 0;
- if (FDCS->perp_mode == perp_mode)
+ if (fdc_state[fdc].perp_mode == perp_mode)
return;
- if (FDCS->version >= FDC_82077_ORIG) {
+ if (fdc_state[fdc].version >= FDC_82077_ORIG) {
output_byte(FD_PERPENDICULAR);
output_byte(perp_mode);
- FDCS->perp_mode = perp_mode;
+ fdc_state[fdc].perp_mode = perp_mode;
} else if (perp_mode) {
DPRINT("perpendicular mode not supported by this FDC.\n");
}
@@ -1279,9 +1278,9 @@ static void fdc_specify(void)
int hlt_max_code = 0x7f;
int hut_max_code = 0xf;
- if (FDCS->need_configure && FDCS->version >= FDC_82072A) {
+ if (fdc_state[fdc].need_configure && fdc_state[fdc].version >= FDC_82072A) {
fdc_configure();
- FDCS->need_configure = 0;
+ fdc_state[fdc].need_configure = 0;
}
switch (raw_cmd->rate & 0x03) {
@@ -1290,7 +1289,7 @@ static void fdc_specify(void)
break;
case 1:
dtr = 300;
- if (FDCS->version >= FDC_82078) {
+ if (fdc_state[fdc].version >= FDC_82078) {
/* chose the default rate table, not the one
* where 1 = 2 Mbps */
output_byte(FD_DRIVESPEC);
@@ -1305,7 +1304,7 @@ static void fdc_specify(void)
break;
}
- if (FDCS->version >= FDC_82072) {
+ if (fdc_state[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!) */
@@ -1335,11 +1334,11 @@ static void fdc_specify(void)
spec2 = (hlt << 1) | (use_virtual_dma & 1);
/* If these parameters did not change, just return with success */
- if (FDCS->spec1 != spec1 || FDCS->spec2 != spec2) {
+ if (fdc_state[fdc].spec1 != spec1 || fdc_state[fdc].spec2 != spec2) {
/* Go ahead and set spec1 and spec2 */
output_byte(FD_SPECIFY);
- output_byte(FDCS->spec1 = spec1);
- output_byte(FDCS->spec2 = spec2);
+ output_byte(fdc_state[fdc].spec1 = spec1);
+ output_byte(fdc_state[fdc].spec2 = spec2);
}
} /* fdc_specify */
@@ -1350,7 +1349,7 @@ 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) == FDCS->dtr)
+ if ((raw_cmd->rate & 3) == fdc_state[fdc].dtr)
return 0;
/* Set dtr */
@@ -1361,7 +1360,7 @@ static int fdc_dtr(void)
* enforced after data rate changes before R/W operations.
* Pause 5 msec to avoid trouble. (Needs to be 2 jiffies)
*/
- FDCS->dtr = raw_cmd->rate & 3;
+ fdc_state[fdc].dtr = raw_cmd->rate & 3;
return fd_wait_for_completion(jiffies + 2UL * HZ / 100, floppy_ready);
} /* fdc_dtr */
@@ -1414,7 +1413,7 @@ static int interpret_errors(void)
if (inr != 7) {
DPRINT("-- FDC reply error\n");
- FDCS->reset = 1;
+ fdc_state[fdc].reset = 1;
return 1;
}
@@ -1548,7 +1547,7 @@ static void check_wp(void)
output_byte(FD_GETSTATUS);
output_byte(UNIT(current_drive));
if (result() != 1) {
- FDCS->reset = 1;
+ fdc_state[fdc].reset = 1;
return;
}
clear_bit(FD_VERIFY_BIT, &DRS->flags);
@@ -1625,7 +1624,7 @@ static void recal_interrupt(void)
{
debugt(__func__, "");
if (inr != 2)
- FDCS->reset = 1;
+ fdc_state[fdc].reset = 1;
else if (ST0 & ST0_ECE) {
switch (DRS->track) {
case NEED_1_RECAL:
@@ -1693,7 +1692,7 @@ irqreturn_t floppy_interrupt(int irq, void *dev_id)
release_dma_lock(f);
do_floppy = NULL;
- if (fdc >= N_FDC || FDCS->address == -1) {
+ if (fdc >= N_FDC || fdc_state[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);
@@ -1702,11 +1701,11 @@ irqreturn_t floppy_interrupt(int irq, void *dev_id)
return IRQ_NONE;
}
- FDCS->reset = 0;
+ fdc_state[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 FDCS->reset blocks the
- * emission of the SENSEI's.
+ * emit SENSEI's to clear the interrupt line. And fdc_state[fdc].reset
+ * blocks the emission of the SENSEI's.
* It is OK to emit floppy commands because we are in an interrupt
* handler here, and thus we have to fear no interference of other
* activity.
@@ -1729,7 +1728,7 @@ irqreturn_t floppy_interrupt(int irq, void *dev_id)
inr == 2 && max_sensei);
}
if (!handler) {
- FDCS->reset = 1;
+ fdc_state[fdc].reset = 1;
return IRQ_NONE;
}
schedule_bh(handler);
@@ -1755,7 +1754,7 @@ static void reset_interrupt(void)
{
debugt(__func__, "");
result(); /* get the status ready for set_fdc */
- if (FDCS->reset) {
+ if (fdc_state[fdc].reset) {
pr_info("reset set in interrupt, calling %ps\n", cont->error);
cont->error(); /* a reset just after a reset. BAD! */
}
@@ -1771,7 +1770,7 @@ static void reset_fdc(void)
unsigned long flags;
do_floppy = reset_interrupt;
- FDCS->reset = 0;
+ fdc_state[fdc].reset = 0;
reset_fdc_info(0);
/* Pseudo-DMA may intercept 'reset finished' interrupt. */
@@ -1781,12 +1780,12 @@ static void reset_fdc(void)
fd_disable_dma();
release_dma_lock(flags);
- if (FDCS->version >= FDC_82072A)
- fd_outb(0x80 | (FDCS->dtr & 3), FD_STATUS);
+ if (fdc_state[fdc].version >= FDC_82072A)
+ fd_outb(0x80 | (fdc_state[fdc].dtr & 3), FD_STATUS);
else {
- fd_outb(FDCS->dor & ~0x04, FD_DOR);
+ fd_outb(fdc_state[fdc].dor & ~0x04, FD_DOR);
udelay(FD_RESET_DELAY);
- fd_outb(FDCS->dor, FD_DOR);
+ fd_outb(fdc_state[fdc].dor, FD_DOR);
}
}
@@ -1850,7 +1849,7 @@ static void floppy_shutdown(struct work_struct *arg)
if (initialized)
DPRINT("floppy timeout called\n");
- FDCS->reset = 1;
+ fdc_state[fdc].reset = 1;
if (cont) {
cont->done(0);
cont->redo(); /* this will recall reset when needed */
@@ -1870,7 +1869,7 @@ static int start_motor(void (*function)(void))
mask = 0xfc;
data = UNIT(current_drive);
if (!(raw_cmd->flags & FD_RAW_NO_MOTOR)) {
- if (!(FDCS->dor & (0x10 << UNIT(current_drive)))) {
+ if (!(fdc_state[fdc].dor & (0x10 << UNIT(current_drive)))) {
set_debugt();
/* no read since this drive is running */
DRS->first_read_date = 0;
@@ -1878,7 +1877,7 @@ static int start_motor(void (*function)(void))
DRS->spinup_date = jiffies;
data |= (0x10 << UNIT(current_drive));
}
- } else if (FDCS->dor & (0x10 << UNIT(current_drive)))
+ } else if (fdc_state[fdc].dor & (0x10 << UNIT(current_drive)))
mask &= ~(0x10 << UNIT(current_drive));
/* starts motor and selects floppy */
@@ -1892,7 +1891,7 @@ static int start_motor(void (*function)(void))
static void floppy_ready(void)
{
- if (FDCS->reset) {
+ if (fdc_state[fdc].reset) {
reset_fdc();
return;
}
@@ -1991,7 +1990,7 @@ static int wait_til_done(void (*handler)(void), bool interruptible)
return -EINTR;
}
- if (FDCS->reset)
+ if (fdc_state[fdc].reset)
command_status = FD_COMMAND_ERROR;
if (command_status == FD_COMMAND_OKAY)
ret = 0;
@@ -2060,7 +2059,7 @@ static void bad_flp_intr(void)
if (err_count > DP->max_errors.abort)
cont->done(0);
if (err_count > DP->max_errors.reset)
- FDCS->reset = 1;
+ fdc_state[fdc].reset = 1;
else if (err_count > DP->max_errors.recal)
DRS->track = NEED_2_RECAL;
}
@@ -2967,8 +2966,8 @@ static int user_reset_fdc(int drive, int arg, bool interruptible)
return -EINTR;
if (arg == FD_RESET_ALWAYS)
- FDCS->reset = 1;
- if (FDCS->reset) {
+ fdc_state[fdc].reset = 1;
+ if (fdc_state[fdc].reset) {
cont = &reset_cont;
ret = wait_til_done(reset_fdc, interruptible);
if (ret == -EINTR)
@@ -3179,23 +3178,23 @@ static int raw_cmd_ioctl(int cmd, void __user *param)
int ret2;
int ret;
- if (FDCS->rawcmd <= 1)
- FDCS->rawcmd = 1;
+ if (fdc_state[fdc].rawcmd <= 1)
+ fdc_state[fdc].rawcmd = 1;
for (drive = 0; drive < N_DRIVE; drive++) {
if (FDC(drive) != fdc)
continue;
if (drive == current_drive) {
if (UDRS->fd_ref > 1) {
- FDCS->rawcmd = 2;
+ fdc_state[fdc].rawcmd = 2;
break;
}
} else if (UDRS->fd_ref) {
- FDCS->rawcmd = 2;
+ fdc_state[fdc].rawcmd = 2;
break;
}
}
- if (FDCS->reset)
+ if (fdc_state[fdc].reset)
return -EIO;
ret = raw_cmd_copyin(cmd, param, &my_raw_cmd);
@@ -3209,7 +3208,7 @@ static int raw_cmd_ioctl(int cmd, void __user *param)
ret = wait_til_done(floppy_start, true);
debug_dcl(DP->flags, "calling disk change from raw_cmd ioctl\n");
- if (ret != -EINTR && FDCS->reset)
+ if (ret != -EINTR && fdc_state[fdc].reset)
ret = -EIO;
DRS->track = NO_TRACK;
@@ -4261,7 +4260,7 @@ static char __init get_fdc_version(void)
int r;
output_byte(FD_DUMPREGS); /* 82072 and better know DUMPREGS */
- if (FDCS->reset)
+ if (fdc_state[fdc].reset)
return FDC_NONE;
r = result();
if (r <= 0x00)
@@ -4494,7 +4493,7 @@ static int floppy_resume(struct device *dev)
int fdc;
for (fdc = 0; fdc < N_FDC; fdc++)
- if (FDCS->address != -1)
+ if (fdc_state[fdc].address != -1)
user_reset_fdc(-1, FD_RESET_ALWAYS, false);
return 0;
@@ -4605,15 +4604,15 @@ static int __init do_floppy_init(void)
for (i = 0; i < N_FDC; i++) {
fdc = i;
- memset(FDCS, 0, sizeof(*FDCS));
- FDCS->dtr = -1;
- FDCS->dor = 0x4;
+ memset(&fdc_state[fdc], 0, sizeof(*fdc_state));
+ fdc_state[fdc].dtr = -1;
+ fdc_state[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
- FDCS->version = FDC_82072A;
+ fdc_state[fdc].version = FDC_82072A;
#endif
}
@@ -4656,28 +4655,28 @@ static int __init do_floppy_init(void)
for (i = 0; i < N_FDC; i++) {
fdc = i;
- FDCS->driver_version = FD_DRIVER_VERSION;
+ fdc_state[fdc].driver_version = FD_DRIVER_VERSION;
for (unit = 0; unit < 4; unit++)
- FDCS->track[unit] = 0;
- if (FDCS->address == -1)
+ fdc_state[fdc].track[unit] = 0;
+ if (fdc_state[fdc].address == -1)
continue;
- FDCS->rawcmd = 2;
+ fdc_state[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);
- FDCS->address = -1;
- FDCS->version = FDC_NONE;
+ fdc_state[fdc].address = -1;
+ fdc_state[fdc].version = FDC_NONE;
continue;
}
/* Try to determine the floppy controller type */
- FDCS->version = get_fdc_version();
- if (FDCS->version == FDC_NONE) {
+ fdc_state[fdc].version = get_fdc_version();
+ if (fdc_state[fdc].version == FDC_NONE) {
/* free ioports reserved by floppy_grab_irq_and_dma() */
floppy_release_regions(fdc);
- FDCS->address = -1;
+ fdc_state[fdc].address = -1;
continue;
}
- if (can_use_virtual_dma == 2 && FDCS->version < FDC_82072A)
+ if (can_use_virtual_dma == 2 && fdc_state[fdc].version < FDC_82072A)
can_use_virtual_dma = 0;
have_no_fdc = 0;
@@ -4783,7 +4782,7 @@ static void floppy_release_allocated_regions(int fdc, const struct io_region *p)
{
while (p != io_regions) {
p--;
- release_region(FDCS->address + p->offset, p->size);
+ release_region(fdc_state[fdc].address + p->offset, p->size);
}
}
@@ -4794,10 +4793,10 @@ static int floppy_request_regions(int fdc)
const struct io_region *p;
for (p = io_regions; p < ARRAY_END(io_regions); p++) {
- if (!request_region(FDCS->address + p->offset,
+ if (!request_region(fdc_state[fdc].address + p->offset,
p->size, "floppy")) {
DPRINT("Floppy io-port 0x%04lx in use\n",
- FDCS->address + p->offset);
+ fdc_state[fdc].address + p->offset);
floppy_release_allocated_regions(fdc, p);
return -EBUSY;
}
@@ -4840,23 +4839,23 @@ static int floppy_grab_irq_and_dma(void)
}
for (fdc = 0; fdc < N_FDC; fdc++) {
- if (FDCS->address != -1) {
+ if (fdc_state[fdc].address != -1) {
if (floppy_request_regions(fdc))
goto cleanup;
}
}
for (fdc = 0; fdc < N_FDC; fdc++) {
- if (FDCS->address != -1) {
+ if (fdc_state[fdc].address != -1) {
reset_fdc_info(1);
- fd_outb(FDCS->dor, FD_DOR);
+ fd_outb(fdc_state[fdc].dor, FD_DOR);
}
}
fdc = 0;
set_dor(0, ~0, 8); /* avoid immediate interrupt */
for (fdc = 0; fdc < N_FDC; fdc++)
- if (FDCS->address != -1)
- fd_outb(FDCS->dor, FD_DOR);
+ if (fdc_state[fdc].address != -1)
+ fd_outb(fdc_state[fdc].dor, FD_DOR);
/*
* The driver will try and free resources and relies on us
* to know if they were allocated or not.
@@ -4918,7 +4917,7 @@ static void floppy_release_irq_and_dma(void)
pr_info("work still pending\n");
old_fdc = fdc;
for (fdc = 0; fdc < N_FDC; fdc++)
- if (FDCS->address != -1)
+ if (fdc_state[fdc].address != -1)
floppy_release_regions(fdc);
fdc = old_fdc;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 02/10] floppy: cleanup: expand macro UFDCS
2020-02-24 21:23 [PATCH 00/10] floppy driver cleanups (deobfuscation) Willy Tarreau
2020-02-24 21:23 ` [PATCH 01/10] floppy: cleanup: expand macro FDCS Willy Tarreau
@ 2020-02-24 21:23 ` Willy Tarreau
2020-02-24 21:23 ` [PATCH 03/10] floppy: cleanup: expand macro UDP Willy Tarreau
` (9 subsequent siblings)
11 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-24 21:23 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Linus Torvalds, Willy Tarreau
This macro doesn't bring much value and only slightly obfuscates the
code by silently using local variable "drive", let's expand it.
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/block/floppy.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 93e0840..182148a 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -313,7 +313,6 @@ static bool initialized;
#define UDP (&drive_params[drive])
#define UDRS (&drive_state[drive])
#define UDRWE (&write_errors[drive])
-#define UFDCS (&fdc_state[FDC(drive)])
#define PH_HEAD(floppy, head) (((((floppy)->stretch & 2) >> 1) ^ head) << 2)
#define STRETCH(floppy) ((floppy)->stretch & FD_STRETCH)
@@ -3549,7 +3548,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
case FDRESET:
return user_reset_fdc(drive, (int)param, true);
case FDGETFDCSTAT:
- outparam = UFDCS;
+ outparam = &fdc_state[FDC(drive)];
break;
case FDWERRORCLR:
memset(UDRWE, 0, sizeof(*UDRWE));
@@ -3833,7 +3832,7 @@ static int compat_getfdcstat(int drive,
struct floppy_fdc_state v;
mutex_lock(&floppy_mutex);
- v = *UFDCS;
+ v = fdc_state[FDC(drive)];
mutex_unlock(&floppy_mutex);
memset(&v32, 0, sizeof(struct compat_floppy_fdc_state));
@@ -4062,8 +4061,8 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
buffer_track = -1;
}
- if (UFDCS->rawcmd == 1)
- UFDCS->rawcmd = 2;
+ if (fdc_state[FDC(drive)].rawcmd == 1)
+ fdc_state[FDC(drive)].rawcmd = 2;
if (!(mode & FMODE_NDELAY)) {
if (mode & (FMODE_READ|FMODE_WRITE)) {
--
2.9.0
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 03/10] floppy: cleanup: expand macro UDP
2020-02-24 21:23 [PATCH 00/10] floppy driver cleanups (deobfuscation) Willy Tarreau
2020-02-24 21:23 ` [PATCH 01/10] floppy: cleanup: expand macro FDCS Willy Tarreau
2020-02-24 21:23 ` [PATCH 02/10] floppy: cleanup: expand macro UFDCS Willy Tarreau
@ 2020-02-24 21:23 ` Willy Tarreau
2020-02-24 21:23 ` [PATCH 04/10] floppy: cleanup: expand macro UDRS Willy Tarreau
` (8 subsequent siblings)
11 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-24 21:23 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Linus Torvalds, Willy Tarreau
This macro doesn't bring much value and only slightly obfuscates the
code by silently using local variable "drive", let's expand it.
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/block/floppy.c | 152 +++++++++++++++++++++++++------------------------
1 file changed, 77 insertions(+), 75 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 182148a..8fcedb2 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -310,7 +310,6 @@ static bool initialized;
#define DRS (&drive_state[current_drive])
#define DRWE (&write_errors[current_drive])
-#define UDP (&drive_params[drive])
#define UDRS (&drive_state[drive])
#define UDRWE (&write_errors[drive])
@@ -681,10 +680,10 @@ static void __reschedule_timeout(int drive, const char *message)
delay = 20UL * HZ;
drive = 0;
} else
- delay = UDP->timeout;
+ delay = drive_params[drive].timeout;
mod_delayed_work(floppy_wq, &fd_timeout, delay);
- if (UDP->flags & FD_DEBUG)
+ if (drive_params[drive].flags & FD_DEBUG)
DPRINT("reschedule timeout %s\n", message);
timeout_message = message;
}
@@ -738,7 +737,7 @@ static int disk_change(int drive)
{
int fdc = FDC(drive);
- if (time_before(jiffies, UDRS->select_date + UDP->select_delay))
+ if (time_before(jiffies, UDRS->select_date + drive_params[drive].select_delay))
DPRINT("WARNING disk change called early\n");
if (!(fdc_state[fdc].dor & (0x10 << UNIT(drive))) ||
(fdc_state[fdc].dor & 3) != UNIT(drive) || fdc != FDC(drive)) {
@@ -747,15 +746,16 @@ static int disk_change(int drive)
(unsigned int)fdc_state[fdc].dor);
}
- debug_dcl(UDP->flags,
+ debug_dcl(drive_params[drive].flags,
"checking disk change line for drive %d\n", drive);
- debug_dcl(UDP->flags, "jiffies=%lu\n", jiffies);
- debug_dcl(UDP->flags, "disk change line=%x\n", fd_inb(FD_DIR) & 0x80);
- debug_dcl(UDP->flags, "flags=%lx\n", UDRS->flags);
+ 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);
+ debug_dcl(drive_params[drive].flags, "flags=%lx\n", UDRS->flags);
- if (UDP->flags & FD_BROKEN_DCL)
+ if (drive_params[drive].flags & FD_BROKEN_DCL)
return test_bit(FD_DISK_CHANGED_BIT, &UDRS->flags);
- if ((fd_inb(FD_DIR) ^ UDP->flags) & 0x80) {
+ if ((fd_inb(FD_DIR) ^ drive_params[drive].flags) & 0x80) {
set_bit(FD_VERIFY_BIT, &UDRS->flags);
/* verify write protection */
@@ -764,7 +764,7 @@ static int disk_change(int drive)
/* invalidate its geometry */
if (UDRS->keep_data >= 0) {
- if ((UDP->flags & FTD_MSG) &&
+ if ((drive_params[drive].flags & FTD_MSG) &&
current_type[drive] != NULL)
DPRINT("Disk type is undefined after disk change\n");
current_type[drive] = NULL;
@@ -806,7 +806,7 @@ static int set_dor(int fdc, char mask, char data)
unit = olddor & 0x3;
if (is_selected(olddor, unit) && !is_selected(newdor, unit)) {
drive = REVDRIVE(fdc, unit);
- debug_dcl(UDP->flags,
+ debug_dcl(drive_params[drive].flags,
"calling disk change from set_dor\n");
disk_change(drive);
}
@@ -929,12 +929,12 @@ static void floppy_off(unsigned int drive)
/* make spindle stop in a position which minimizes spinup time
* next time */
- if (UDP->rps) {
+ if (drive_params[drive].rps) {
delta = jiffies - UDRS->first_read_date + HZ -
- UDP->spindown_offset;
- delta = ((delta * UDP->rps) % HZ) / UDP->rps;
+ drive_params[drive].spindown_offset;
+ delta = ((delta * drive_params[drive].rps) % HZ) / drive_params[drive].rps;
motor_off_timer[drive].expires =
- jiffies + UDP->spindown - delta;
+ jiffies + drive_params[drive].spindown - delta;
}
add_timer(motor_off_timer + drive);
}
@@ -956,7 +956,7 @@ static void scandrives(void)
saved_drive = current_drive;
for (i = 0; i < N_DRIVE; i++) {
drive = (saved_drive + i + 1) % N_DRIVE;
- if (UDRS->fd_ref == 0 || UDP->select_delay != 0)
+ if (UDRS->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))) &
@@ -2999,8 +2999,8 @@ static const char *drive_name(int type, int drive)
if (type)
floppy = floppy_type + type;
else {
- if (UDP->native_format)
- floppy = floppy_type + UDP->native_format;
+ if (drive_params[drive].native_format)
+ floppy = floppy_type + drive_params[drive].native_format;
else
return "(null)";
}
@@ -3240,7 +3240,7 @@ static int set_geometry(unsigned int cmd, struct floppy_struct *g,
(int)(g->sect * g->head) <= 0 ||
/* check for zero in F_SECT_PER_TRACK */
(unsigned char)((g->sect << 2) >> FD_SIZECODE(g)) == 0 ||
- g->track <= 0 || g->track > UDP->tracks >> STRETCH(g) ||
+ g->track <= 0 || g->track > drive_params[drive].tracks >> STRETCH(g) ||
/* check if reserved bits are set */
(g->stretch & ~(FD_STRETCH | FD_SWAPSIDES | FD_SECTBASEMASK)) != 0)
return -EINVAL;
@@ -3487,10 +3487,10 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
outparam = &inparam.g;
break;
case FDMSGON:
- UDP->flags |= FTD_MSG;
+ drive_params[drive].flags |= FTD_MSG;
return 0;
case FDMSGOFF:
- UDP->flags &= ~FTD_MSG;
+ drive_params[drive].flags &= ~FTD_MSG;
return 0;
case FDFMTBEG:
if (lock_fdc(drive))
@@ -3514,13 +3514,13 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
return -EINTR;
return invalidate_drive(bdev);
case FDSETEMSGTRESH:
- UDP->max_errors.reporting = (unsigned short)(param & 0x0f);
+ drive_params[drive].max_errors.reporting = (unsigned short)(param & 0x0f);
return 0;
case FDGETMAXERRS:
- outparam = &UDP->max_errors;
+ outparam = &drive_params[drive].max_errors;
break;
case FDSETMAXERRS:
- UDP->max_errors = inparam.max_errors;
+ drive_params[drive].max_errors = inparam.max_errors;
break;
case FDGETDRVTYP:
outparam = drive_name(type, drive);
@@ -3530,10 +3530,10 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
if (!valid_floppy_drive_params(inparam.dp.autodetect,
inparam.dp.native_format))
return -EINVAL;
- *UDP = inparam.dp;
+ drive_params[drive] = inparam.dp;
break;
case FDGETDRVPRM:
- outparam = UDP;
+ outparam = &drive_params[drive];
break;
case FDPOLLDRVSTAT:
if (lock_fdc(drive))
@@ -3730,25 +3730,26 @@ static int compat_setdrvprm(int drive,
if (!valid_floppy_drive_params(v.autodetect, v.native_format))
return -EINVAL;
mutex_lock(&floppy_mutex);
- UDP->cmos = v.cmos;
- UDP->max_dtr = v.max_dtr;
- UDP->hlt = v.hlt;
- UDP->hut = v.hut;
- UDP->srt = v.srt;
- UDP->spinup = v.spinup;
- UDP->spindown = v.spindown;
- UDP->spindown_offset = v.spindown_offset;
- UDP->select_delay = v.select_delay;
- UDP->rps = v.rps;
- UDP->tracks = v.tracks;
- UDP->timeout = v.timeout;
- UDP->interleave_sect = v.interleave_sect;
- UDP->max_errors = v.max_errors;
- UDP->flags = v.flags;
- UDP->read_track = v.read_track;
- memcpy(UDP->autodetect, v.autodetect, sizeof(v.autodetect));
- UDP->checkfreq = v.checkfreq;
- UDP->native_format = v.native_format;
+ drive_params[drive].cmos = v.cmos;
+ drive_params[drive].max_dtr = v.max_dtr;
+ drive_params[drive].hlt = v.hlt;
+ drive_params[drive].hut = v.hut;
+ drive_params[drive].srt = v.srt;
+ drive_params[drive].spinup = v.spinup;
+ drive_params[drive].spindown = v.spindown;
+ drive_params[drive].spindown_offset = v.spindown_offset;
+ drive_params[drive].select_delay = v.select_delay;
+ drive_params[drive].rps = v.rps;
+ drive_params[drive].tracks = v.tracks;
+ drive_params[drive].timeout = v.timeout;
+ drive_params[drive].interleave_sect = v.interleave_sect;
+ drive_params[drive].max_errors = v.max_errors;
+ drive_params[drive].flags = v.flags;
+ drive_params[drive].read_track = v.read_track;
+ memcpy(drive_params[drive].autodetect, v.autodetect,
+ sizeof(v.autodetect));
+ drive_params[drive].checkfreq = v.checkfreq;
+ drive_params[drive].native_format = v.native_format;
mutex_unlock(&floppy_mutex);
return 0;
}
@@ -3760,25 +3761,26 @@ static int compat_getdrvprm(int drive,
memset(&v, 0, sizeof(struct compat_floppy_drive_params));
mutex_lock(&floppy_mutex);
- v.cmos = UDP->cmos;
- v.max_dtr = UDP->max_dtr;
- v.hlt = UDP->hlt;
- v.hut = UDP->hut;
- v.srt = UDP->srt;
- v.spinup = UDP->spinup;
- v.spindown = UDP->spindown;
- v.spindown_offset = UDP->spindown_offset;
- v.select_delay = UDP->select_delay;
- v.rps = UDP->rps;
- v.tracks = UDP->tracks;
- v.timeout = UDP->timeout;
- v.interleave_sect = UDP->interleave_sect;
- v.max_errors = UDP->max_errors;
- v.flags = UDP->flags;
- v.read_track = UDP->read_track;
- memcpy(v.autodetect, UDP->autodetect, sizeof(v.autodetect));
- v.checkfreq = UDP->checkfreq;
- v.native_format = UDP->native_format;
+ v.cmos = drive_params[drive].cmos;
+ v.max_dtr = drive_params[drive].max_dtr;
+ v.hlt = drive_params[drive].hlt;
+ v.hut = drive_params[drive].hut;
+ v.srt = drive_params[drive].srt;
+ v.spinup = drive_params[drive].spinup;
+ v.spindown = drive_params[drive].spindown;
+ v.spindown_offset = drive_params[drive].spindown_offset;
+ v.select_delay = drive_params[drive].select_delay;
+ v.rps = drive_params[drive].rps;
+ v.tracks = drive_params[drive].tracks;
+ v.timeout = drive_params[drive].timeout;
+ v.interleave_sect = drive_params[drive].interleave_sect;
+ v.max_errors = drive_params[drive].max_errors;
+ v.flags = drive_params[drive].flags;
+ v.read_track = drive_params[drive].read_track;
+ memcpy(v.autodetect, drive_params[drive].autodetect,
+ sizeof(v.autodetect));
+ v.checkfreq = drive_params[drive].checkfreq;
+ v.native_format = drive_params[drive].native_format;
mutex_unlock(&floppy_mutex);
if (copy_to_user(arg, &v, sizeof(struct compat_floppy_drive_params)))
@@ -3931,16 +3933,16 @@ static void __init config_types(void)
/* read drive info out of physical CMOS */
drive = 0;
- if (!UDP->cmos)
- UDP->cmos = FLOPPY0_TYPE;
+ if (!drive_params[drive].cmos)
+ drive_params[drive].cmos = FLOPPY0_TYPE;
drive = 1;
- if (!UDP->cmos)
- UDP->cmos = FLOPPY1_TYPE;
+ if (!drive_params[drive].cmos)
+ drive_params[drive].cmos = FLOPPY1_TYPE;
/* FIXME: additional physical CMOS drive detection should go here */
for (drive = 0; drive < N_DRIVE; drive++) {
- unsigned int type = UDP->cmos;
+ unsigned int type = drive_params[drive].cmos;
struct floppy_drive_params *params;
const char *name = NULL;
char temparea[32];
@@ -3970,7 +3972,7 @@ static void __init config_types(void)
pr_cont("%s fd%d is %s", prepend, drive, name);
}
- *UDP = *params;
+ drive_params[drive] = *params;
}
if (has_drive)
@@ -4012,7 +4014,7 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
if (opened_bdev[drive] && opened_bdev[drive] != bdev)
goto out2;
- if (!UDRS->fd_ref && (UDP->flags & FD_BROKEN_DCL)) {
+ if (!UDRS->fd_ref && (drive_params[drive].flags & FD_BROKEN_DCL)) {
set_bit(FD_DISK_CHANGED_BIT, &UDRS->flags);
set_bit(FD_VERIFY_BIT, &UDRS->flags);
}
@@ -4026,7 +4028,7 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
if (!floppy_track_buffer) {
/* if opening an ED drive, reserve a big buffer,
* else reserve a small one */
- if ((UDP->cmos == 6) || (UDP->cmos == 5))
+ if ((drive_params[drive].cmos == 6) || (drive_params[drive].cmos == 5))
try = 64; /* Only 48 actually useful */
else
try = 32; /* Only 24 actually useful */
@@ -4105,7 +4107,7 @@ static unsigned int floppy_check_events(struct gendisk *disk,
test_bit(FD_VERIFY_BIT, &UDRS->flags))
return DISK_EVENT_MEDIA_CHANGE;
- if (time_after(jiffies, UDRS->last_checked + UDP->checkfreq)) {
+ if (time_after(jiffies, UDRS->last_checked + drive_params[drive].checkfreq)) {
if (lock_fdc(drive))
return 0;
poll_drive(false, 0);
@@ -4471,7 +4473,7 @@ static ssize_t floppy_cmos_show(struct device *dev,
int drive;
drive = p->id;
- return sprintf(buf, "%X\n", UDP->cmos);
+ return sprintf(buf, "%X\n", drive_params[drive].cmos);
}
static DEVICE_ATTR(cmos, 0444, floppy_cmos_show, NULL);
--
2.9.0
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 04/10] floppy: cleanup: expand macro UDRS
2020-02-24 21:23 [PATCH 00/10] floppy driver cleanups (deobfuscation) Willy Tarreau
` (2 preceding siblings ...)
2020-02-24 21:23 ` [PATCH 03/10] floppy: cleanup: expand macro UDP Willy Tarreau
@ 2020-02-24 21:23 ` Willy Tarreau
2020-02-24 21:23 ` [PATCH 05/10] floppy: cleanup: expand macro UDRWE Willy Tarreau
` (7 subsequent siblings)
11 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-24 21:23 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Linus Torvalds, Willy Tarreau
This macro doesn't bring much value and only slightly obfuscates the
code by silently using local variable "drive", let's expand it.
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/block/floppy.c | 162 +++++++++++++++++++++++++------------------------
1 file changed, 83 insertions(+), 79 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 8fcedb2..522fbcc 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -310,7 +310,6 @@ static bool initialized;
#define DRS (&drive_state[current_drive])
#define DRWE (&write_errors[current_drive])
-#define UDRS (&drive_state[drive])
#define UDRWE (&write_errors[drive])
#define PH_HEAD(floppy, head) (((((floppy)->stretch & 2) >> 1) ^ head) << 2)
@@ -603,7 +602,7 @@ static unsigned char in_sector_offset; /* offset within physical sector,
static inline bool drive_no_geom(int drive)
{
- return !current_type[drive] && !ITYPE(UDRS->fd_device);
+ return !current_type[drive] && !ITYPE(drive_state[drive].fd_device);
}
#ifndef fd_eject
@@ -737,7 +736,7 @@ static int disk_change(int drive)
{
int fdc = FDC(drive);
- if (time_before(jiffies, UDRS->select_date + drive_params[drive].select_delay))
+ if (time_before(jiffies, drive_state[drive].select_date + drive_params[drive].select_delay))
DPRINT("WARNING disk change called early\n");
if (!(fdc_state[fdc].dor & (0x10 << UNIT(drive))) ||
(fdc_state[fdc].dor & 3) != UNIT(drive) || fdc != FDC(drive)) {
@@ -751,19 +750,22 @@ static int disk_change(int 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);
- debug_dcl(drive_params[drive].flags, "flags=%lx\n", UDRS->flags);
+ 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, &UDRS->flags);
+ return test_bit(FD_DISK_CHANGED_BIT,
+ &drive_state[drive].flags);
if ((fd_inb(FD_DIR) ^ drive_params[drive].flags) & 0x80) {
- set_bit(FD_VERIFY_BIT, &UDRS->flags);
+ set_bit(FD_VERIFY_BIT, &drive_state[drive].flags);
/* verify write protection */
- if (UDRS->maxblock) /* mark it changed */
- set_bit(FD_DISK_CHANGED_BIT, &UDRS->flags);
+ if (drive_state[drive].maxblock) /* mark it changed */
+ set_bit(FD_DISK_CHANGED_BIT,
+ &drive_state[drive].flags);
/* invalidate its geometry */
- if (UDRS->keep_data >= 0) {
+ if (drive_state[drive].keep_data >= 0) {
if ((drive_params[drive].flags & FTD_MSG) &&
current_type[drive] != NULL)
DPRINT("Disk type is undefined after disk change\n");
@@ -773,8 +775,8 @@ static int disk_change(int drive)
return 1;
} else {
- UDRS->last_checked = jiffies;
- clear_bit(FD_DISK_NEWCHANGE_BIT, &UDRS->flags);
+ drive_state[drive].last_checked = jiffies;
+ clear_bit(FD_DISK_NEWCHANGE_BIT, &drive_state[drive].flags);
}
return 0;
}
@@ -816,7 +818,7 @@ static int set_dor(int fdc, char mask, char data)
unit = newdor & 0x3;
if (!is_selected(olddor, unit) && is_selected(newdor, unit)) {
drive = REVDRIVE(fdc, unit);
- UDRS->select_date = jiffies;
+ drive_state[drive].select_date = jiffies;
}
}
return olddor;
@@ -844,8 +846,8 @@ static void reset_fdc_info(int mode)
fdc_state[fdc].perp_mode = 1;
fdc_state[fdc].rawcmd = 0;
for (drive = 0; drive < N_DRIVE; drive++)
- if (FDC(drive) == fdc && (mode || UDRS->track != NEED_1_RECAL))
- UDRS->track = NEED_2_RECAL;
+ if (FDC(drive) == 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. */
@@ -930,7 +932,7 @@ static void floppy_off(unsigned int drive)
/* make spindle stop in a position which minimizes spinup time
* next time */
if (drive_params[drive].rps) {
- delta = jiffies - UDRS->first_read_date + HZ -
+ delta = jiffies - drive_state[drive].first_read_date + HZ -
drive_params[drive].spindown_offset;
delta = ((delta * drive_params[drive].rps) % HZ) / drive_params[drive].rps;
motor_off_timer[drive].expires =
@@ -956,7 +958,7 @@ static void scandrives(void)
saved_drive = current_drive;
for (i = 0; i < N_DRIVE; i++) {
drive = (saved_drive + i + 1) % N_DRIVE;
- if (UDRS->fd_ref == 0 || drive_params[drive].select_delay != 0)
+ 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))) &
@@ -2065,7 +2067,7 @@ static void bad_flp_intr(void)
static void set_floppy(int drive)
{
- int type = ITYPE(UDRS->fd_device);
+ int type = ITYPE(drive_state[drive].fd_device);
if (type)
_floppy = floppy_type + type;
@@ -3183,11 +3185,11 @@ static int raw_cmd_ioctl(int cmd, void __user *param)
if (FDC(drive) != fdc)
continue;
if (drive == current_drive) {
- if (UDRS->fd_ref > 1) {
+ if (drive_state[drive].fd_ref > 1) {
fdc_state[fdc].rawcmd = 2;
break;
}
- } else if (UDRS->fd_ref) {
+ } else if (drive_state[drive].fd_ref) {
fdc_state[fdc].rawcmd = 2;
break;
}
@@ -3405,7 +3407,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
unsigned long param)
{
int drive = (long)bdev->bd_disk->private_data;
- int type = ITYPE(UDRS->fd_device);
+ int type = ITYPE(drive_state[drive].fd_device);
int i;
int ret;
int size;
@@ -3453,7 +3455,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
switch (cmd) {
case FDEJECT:
- if (UDRS->fd_ref != 1)
+ if (drive_state[drive].fd_ref != 1)
/* somebody else has this drive open */
return -EBUSY;
if (lock_fdc(drive))
@@ -3463,8 +3465,8 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
* non-Sparc architectures */
ret = fd_eject(UNIT(drive));
- set_bit(FD_DISK_CHANGED_BIT, &UDRS->flags);
- set_bit(FD_VERIFY_BIT, &UDRS->flags);
+ set_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags);
+ set_bit(FD_VERIFY_BIT, &drive_state[drive].flags);
process_fd_request();
return ret;
case FDCLRPRM:
@@ -3472,7 +3474,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
return -EINTR;
current_type[drive] = NULL;
floppy_sizes[drive] = MAX_DISK_SIZE << 1;
- UDRS->keep_data = 0;
+ drive_state[drive].keep_data = 0;
return invalidate_drive(bdev);
case FDSETPRM:
case FDDEFPRM:
@@ -3497,7 +3499,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
return -EINTR;
if (poll_drive(true, FD_RAW_NEED_DISK) == -EINTR)
return -EINTR;
- ret = UDRS->flags;
+ ret = drive_state[drive].flags;
process_fd_request();
if (ret & FD_VERIFY)
return -ENODEV;
@@ -3505,7 +3507,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
return -EROFS;
return 0;
case FDFMTTRK:
- if (UDRS->fd_ref != 1)
+ if (drive_state[drive].fd_ref != 1)
return -EBUSY;
return do_format(drive, &inparam.f);
case FDFMTEND:
@@ -3543,7 +3545,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
process_fd_request();
/* fall through */
case FDGETDRVSTAT:
- outparam = UDRS;
+ outparam = &drive_state[drive];
break;
case FDRESET:
return user_reset_fdc(drive, (int)param, true);
@@ -3690,7 +3692,7 @@ static int compat_set_geometry(struct block_device *bdev, fmode_t mode, unsigned
mutex_lock(&floppy_mutex);
drive = (long)bdev->bd_disk->private_data;
- type = ITYPE(UDRS->fd_device);
+ type = ITYPE(drive_state[drive].fd_device);
err = set_geometry(cmd == FDSETPRM32 ? FDSETPRM : FDDEFPRM,
&v, drive, type, bdev);
mutex_unlock(&floppy_mutex);
@@ -3706,7 +3708,8 @@ static int compat_get_prm(int drive,
memset(&v, 0, sizeof(v));
mutex_lock(&floppy_mutex);
- err = get_floppy_geometry(drive, ITYPE(UDRS->fd_device), &p);
+ err = get_floppy_geometry(drive, ITYPE(drive_state[drive].fd_device),
+ &p);
if (err) {
mutex_unlock(&floppy_mutex);
return err;
@@ -3803,20 +3806,20 @@ static int compat_getdrvstat(int drive, bool poll,
goto Eintr;
process_fd_request();
}
- v.spinup_date = UDRS->spinup_date;
- v.select_date = UDRS->select_date;
- v.first_read_date = UDRS->first_read_date;
- v.probed_format = UDRS->probed_format;
- v.track = UDRS->track;
- v.maxblock = UDRS->maxblock;
- v.maxtrack = UDRS->maxtrack;
- v.generation = UDRS->generation;
- v.keep_data = UDRS->keep_data;
- v.fd_ref = UDRS->fd_ref;
- v.fd_device = UDRS->fd_device;
- v.last_checked = UDRS->last_checked;
- v.dmabuf = (uintptr_t)UDRS->dmabuf;
- v.bufblocks = UDRS->bufblocks;
+ v.spinup_date = drive_state[drive].spinup_date;
+ v.select_date = drive_state[drive].select_date;
+ v.first_read_date = drive_state[drive].first_read_date;
+ v.probed_format = drive_state[drive].probed_format;
+ v.track = drive_state[drive].track;
+ v.maxblock = drive_state[drive].maxblock;
+ v.maxtrack = drive_state[drive].maxtrack;
+ v.generation = drive_state[drive].generation;
+ v.keep_data = drive_state[drive].keep_data;
+ v.fd_ref = drive_state[drive].fd_ref;
+ v.fd_device = drive_state[drive].fd_device;
+ v.last_checked = drive_state[drive].last_checked;
+ v.dmabuf = (uintptr_t) drive_state[drive].dmabuf;
+ v.bufblocks = drive_state[drive].bufblocks;
mutex_unlock(&floppy_mutex);
if (copy_to_user(arg, &v, sizeof(struct compat_floppy_drive_struct)))
@@ -3985,11 +3988,11 @@ static void floppy_release(struct gendisk *disk, fmode_t mode)
mutex_lock(&floppy_mutex);
mutex_lock(&open_lock);
- if (!UDRS->fd_ref--) {
+ if (!drive_state[drive].fd_ref--) {
DPRINT("floppy_release with fd_ref == 0");
- UDRS->fd_ref = 0;
+ drive_state[drive].fd_ref = 0;
}
- if (!UDRS->fd_ref)
+ if (!drive_state[drive].fd_ref)
opened_bdev[drive] = NULL;
mutex_unlock(&open_lock);
mutex_unlock(&floppy_mutex);
@@ -4010,16 +4013,16 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
mutex_lock(&floppy_mutex);
mutex_lock(&open_lock);
- old_dev = UDRS->fd_device;
+ old_dev = drive_state[drive].fd_device;
if (opened_bdev[drive] && opened_bdev[drive] != bdev)
goto out2;
- if (!UDRS->fd_ref && (drive_params[drive].flags & FD_BROKEN_DCL)) {
- set_bit(FD_DISK_CHANGED_BIT, &UDRS->flags);
- set_bit(FD_VERIFY_BIT, &UDRS->flags);
+ if (!drive_state[drive].fd_ref && (drive_params[drive].flags & FD_BROKEN_DCL)) {
+ set_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags);
+ set_bit(FD_VERIFY_BIT, &drive_state[drive].flags);
}
- UDRS->fd_ref++;
+ drive_state[drive].fd_ref++;
opened_bdev[drive] = bdev;
@@ -4056,7 +4059,7 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
}
new_dev = MINOR(bdev->bd_dev);
- UDRS->fd_device = new_dev;
+ drive_state[drive].fd_device = new_dev;
set_capacity(disks[drive], floppy_sizes[new_dev]);
if (old_dev != -1 && old_dev != new_dev) {
if (buffer_drive == drive)
@@ -4068,26 +4071,27 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
if (!(mode & FMODE_NDELAY)) {
if (mode & (FMODE_READ|FMODE_WRITE)) {
- UDRS->last_checked = 0;
- clear_bit(FD_OPEN_SHOULD_FAIL_BIT, &UDRS->flags);
+ drive_state[drive].last_checked = 0;
+ clear_bit(FD_OPEN_SHOULD_FAIL_BIT,
+ &drive_state[drive].flags);
check_disk_change(bdev);
- if (test_bit(FD_DISK_CHANGED_BIT, &UDRS->flags))
+ if (test_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags))
goto out;
- if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, &UDRS->flags))
+ if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, &drive_state[drive].flags))
goto out;
}
res = -EROFS;
if ((mode & FMODE_WRITE) &&
- !test_bit(FD_DISK_WRITABLE_BIT, &UDRS->flags))
+ !test_bit(FD_DISK_WRITABLE_BIT, &drive_state[drive].flags))
goto out;
}
mutex_unlock(&open_lock);
mutex_unlock(&floppy_mutex);
return 0;
out:
- UDRS->fd_ref--;
+ drive_state[drive].fd_ref--;
- if (!UDRS->fd_ref)
+ if (!drive_state[drive].fd_ref)
opened_bdev[drive] = NULL;
out2:
mutex_unlock(&open_lock);
@@ -4103,19 +4107,19 @@ static unsigned int floppy_check_events(struct gendisk *disk,
{
int drive = (long)disk->private_data;
- if (test_bit(FD_DISK_CHANGED_BIT, &UDRS->flags) ||
- test_bit(FD_VERIFY_BIT, &UDRS->flags))
+ if (test_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags) ||
+ test_bit(FD_VERIFY_BIT, &drive_state[drive].flags))
return DISK_EVENT_MEDIA_CHANGE;
- if (time_after(jiffies, UDRS->last_checked + drive_params[drive].checkfreq)) {
+ if (time_after(jiffies, drive_state[drive].last_checked + drive_params[drive].checkfreq)) {
if (lock_fdc(drive))
return 0;
poll_drive(false, 0);
process_fd_request();
}
- if (test_bit(FD_DISK_CHANGED_BIT, &UDRS->flags) ||
- test_bit(FD_VERIFY_BIT, &UDRS->flags) ||
+ if (test_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags) ||
+ test_bit(FD_VERIFY_BIT, &drive_state[drive].flags) ||
test_bit(drive, &fake_change) ||
drive_no_geom(drive))
return DISK_EVENT_MEDIA_CHANGE;
@@ -4141,7 +4145,7 @@ static void floppy_rb0_cb(struct bio *bio)
if (bio->bi_status) {
pr_info("floppy: error %d while reading block 0\n",
bio->bi_status);
- set_bit(FD_OPEN_SHOULD_FAIL_BIT, &UDRS->flags);
+ set_bit(FD_OPEN_SHOULD_FAIL_BIT, &drive_state[drive].flags);
}
complete(&cbdata->complete);
}
@@ -4198,8 +4202,8 @@ static int floppy_revalidate(struct gendisk *disk)
int cf;
int res = 0;
- if (test_bit(FD_DISK_CHANGED_BIT, &UDRS->flags) ||
- test_bit(FD_VERIFY_BIT, &UDRS->flags) ||
+ if (test_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags) ||
+ test_bit(FD_VERIFY_BIT, &drive_state[drive].flags) ||
test_bit(drive, &fake_change) ||
drive_no_geom(drive)) {
if (WARN(atomic_read(&usage_count) == 0,
@@ -4209,20 +4213,20 @@ static int floppy_revalidate(struct gendisk *disk)
res = lock_fdc(drive);
if (res)
return res;
- cf = (test_bit(FD_DISK_CHANGED_BIT, &UDRS->flags) ||
- test_bit(FD_VERIFY_BIT, &UDRS->flags));
+ cf = (test_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags) ||
+ test_bit(FD_VERIFY_BIT, &drive_state[drive].flags));
if (!(cf || test_bit(drive, &fake_change) || drive_no_geom(drive))) {
process_fd_request(); /*already done by another thread */
return 0;
}
- UDRS->maxblock = 0;
- UDRS->maxtrack = 0;
+ drive_state[drive].maxblock = 0;
+ drive_state[drive].maxtrack = 0;
if (buffer_drive == drive)
buffer_track = -1;
clear_bit(drive, &fake_change);
- clear_bit(FD_DISK_CHANGED_BIT, &UDRS->flags);
+ clear_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags);
if (cf)
- UDRS->generation++;
+ drive_state[drive].generation++;
if (drive_no_geom(drive)) {
/* auto-sensing */
res = __floppy_read_block_0(opened_bdev[drive], drive);
@@ -4232,7 +4236,7 @@ static int floppy_revalidate(struct gendisk *disk)
process_fd_request();
}
}
- set_capacity(disk, floppy_sizes[UDRS->fd_device]);
+ set_capacity(disk, floppy_sizes[drive_state[drive].fd_device]);
return res;
}
@@ -4638,12 +4642,12 @@ static int __init do_floppy_init(void)
/* initialise drive state */
for (drive = 0; drive < N_DRIVE; drive++) {
- memset(UDRS, 0, sizeof(*UDRS));
+ memset(&drive_state[drive], 0, sizeof(drive_state[drive]));
memset(UDRWE, 0, sizeof(*UDRWE));
- set_bit(FD_DISK_NEWCHANGE_BIT, &UDRS->flags);
- set_bit(FD_DISK_CHANGED_BIT, &UDRS->flags);
- set_bit(FD_VERIFY_BIT, &UDRS->flags);
- UDRS->fd_device = -1;
+ set_bit(FD_DISK_NEWCHANGE_BIT, &drive_state[drive].flags);
+ set_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags);
+ set_bit(FD_VERIFY_BIT, &drive_state[drive].flags);
+ drive_state[drive].fd_device = -1;
floppy_track_buffer = NULL;
max_buffer_sectors = 0;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 05/10] floppy: cleanup: expand macro UDRWE
2020-02-24 21:23 [PATCH 00/10] floppy driver cleanups (deobfuscation) Willy Tarreau
` (3 preceding siblings ...)
2020-02-24 21:23 ` [PATCH 04/10] floppy: cleanup: expand macro UDRS Willy Tarreau
@ 2020-02-24 21:23 ` Willy Tarreau
2020-02-24 21:23 ` [PATCH 06/10] floppy: cleanup: expand macro DP Willy Tarreau
` (6 subsequent siblings)
11 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-24 21:23 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Linus Torvalds, Willy Tarreau
This macro doesn't bring much value and only slightly obfuscates the
code by silently using local variable "drive", let's expand it.
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/block/floppy.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 522fbcc..a76a9bb 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -310,8 +310,6 @@ static bool initialized;
#define DRS (&drive_state[current_drive])
#define DRWE (&write_errors[current_drive])
-#define UDRWE (&write_errors[drive])
-
#define PH_HEAD(floppy, head) (((((floppy)->stretch & 2) >> 1) ^ head) << 2)
#define STRETCH(floppy) ((floppy)->stretch & FD_STRETCH)
@@ -3553,10 +3551,10 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
outparam = &fdc_state[FDC(drive)];
break;
case FDWERRORCLR:
- memset(UDRWE, 0, sizeof(*UDRWE));
+ memset(&write_errors[drive], 0, sizeof(write_errors[drive]));
return 0;
case FDWERRORGET:
- outparam = UDRWE;
+ outparam = &write_errors[drive];
break;
case FDRAWCMD:
if (type)
@@ -3867,7 +3865,7 @@ static int compat_werrorget(int drive,
memset(&v32, 0, sizeof(struct compat_floppy_write_errors));
mutex_lock(&floppy_mutex);
- v = *UDRWE;
+ v = write_errors[drive];
mutex_unlock(&floppy_mutex);
v32.write_errors = v.write_errors;
v32.first_error_sector = v.first_error_sector;
@@ -4643,7 +4641,7 @@ static int __init do_floppy_init(void)
/* initialise drive state */
for (drive = 0; drive < N_DRIVE; drive++) {
memset(&drive_state[drive], 0, sizeof(drive_state[drive]));
- memset(UDRWE, 0, sizeof(*UDRWE));
+ memset(&write_errors[drive], 0, sizeof(write_errors[drive]));
set_bit(FD_DISK_NEWCHANGE_BIT, &drive_state[drive].flags);
set_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags);
set_bit(FD_VERIFY_BIT, &drive_state[drive].flags);
--
2.9.0
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 06/10] floppy: cleanup: expand macro DP
2020-02-24 21:23 [PATCH 00/10] floppy driver cleanups (deobfuscation) Willy Tarreau
` (4 preceding siblings ...)
2020-02-24 21:23 ` [PATCH 05/10] floppy: cleanup: expand macro UDRWE Willy Tarreau
@ 2020-02-24 21:23 ` Willy Tarreau
2020-02-24 21:23 ` [PATCH 07/10] floppy: cleanup: expand macro DRS Willy Tarreau
` (5 subsequent siblings)
11 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-24 21:23 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Linus Torvalds, Willy Tarreau
This macro doesn't bring much value and only slightly obfuscates the
code by silently using global variable "current_drive", let's expand it.
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/block/floppy.c | 84 ++++++++++++++++++++++++++++----------------------
1 file changed, 47 insertions(+), 37 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index a76a9bb..7744e42 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -306,7 +306,6 @@ static bool initialized;
/* reverse mapping from unit and fdc to drive */
#define REVDRIVE(fdc, unit) ((unit) + ((fdc) << 2))
-#define DP (&drive_params[current_drive])
#define DRS (&drive_state[current_drive])
#define DRWE (&write_errors[current_drive])
@@ -624,7 +623,7 @@ static inline void set_debugt(void)
static inline void debugt(const char *func, const char *msg)
{
- if (DP->flags & DEBUGT)
+ if (drive_params[current_drive].flags & DEBUGT)
pr_info("%s:%s dtime=%lu\n", func, msg, jiffies - debugtimer);
}
#else
@@ -824,7 +823,7 @@ static int set_dor(int fdc, char mask, char data)
static void twaddle(void)
{
- if (DP->select_delay)
+ 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);
@@ -950,7 +949,7 @@ static void scandrives(void)
int drive;
int saved_drive;
- if (DP->select_delay)
+ if (drive_params[current_drive].select_delay)
return;
saved_drive = current_drive;
@@ -1009,7 +1008,8 @@ static void cancel_activity(void)
* transfer */
static void fd_watchdog(void)
{
- debug_dcl(DP->flags, "calling disk change from watchdog\n");
+ debug_dcl(drive_params[current_drive].flags,
+ "calling disk change from watchdog\n");
if (disk_change(current_drive)) {
DPRINT("disk removed during i/o\n");
@@ -1310,20 +1310,23 @@ static void fdc_specify(void)
}
/* Convert step rate from microseconds to milliseconds and 4 bits */
- srt = 16 - DIV_ROUND_UP(DP->srt * scale_dtr / 1000, NOMINAL_DTR);
+ srt = 16 - DIV_ROUND_UP(drive_params[current_drive].srt * scale_dtr / 1000,
+ NOMINAL_DTR);
if (slow_floppy)
srt = srt / 4;
SUPBOUND(srt, 0xf);
INFBOUND(srt, 0);
- hlt = DIV_ROUND_UP(DP->hlt * scale_dtr / 2, NOMINAL_DTR);
+ hlt = DIV_ROUND_UP(drive_params[current_drive].hlt * scale_dtr / 2,
+ NOMINAL_DTR);
if (hlt < 0x01)
hlt = 0x01;
else if (hlt > 0x7f)
hlt = hlt_max_code;
- hut = DIV_ROUND_UP(DP->hut * scale_dtr / 16, NOMINAL_DTR);
+ hut = DIV_ROUND_UP(drive_params[current_drive].hut * scale_dtr / 16,
+ NOMINAL_DTR);
if (hut < 0x1)
hut = 0x1;
else if (hut > 0xf)
@@ -1430,10 +1433,10 @@ static int interpret_errors(void)
} else if (ST1 & ST1_ND) {
set_bit(FD_NEED_TWADDLE_BIT, &DRS->flags);
} else if (ST1 & ST1_OR) {
- if (DP->flags & FTD_MSG)
+ if (drive_params[current_drive].flags & FTD_MSG)
DPRINT("Over/Underrun - retrying\n");
bad = 0;
- } else if (*errors >= DP->max_errors.reporting) {
+ } else if (*errors >= drive_params[current_drive].max_errors.reporting) {
print_errors();
}
if (ST2 & ST2_WC || ST2 & ST2_BC)
@@ -1471,13 +1474,13 @@ static void setup_rw_floppy(void)
flags |= FD_RAW_INTR;
if ((flags & FD_RAW_SPIN) && !(flags & FD_RAW_NO_MOTOR)) {
- ready_date = DRS->spinup_date + DP->spinup;
+ ready_date = DRS->spinup_date + drive_params[current_drive].spinup;
/* If spinup will take a long time, rerun scandrives
* again just before spinup completion. Beware that
* after scandrives, we must again wait for selection.
*/
- if (time_after(ready_date, jiffies + DP->select_delay)) {
- ready_date -= DP->select_delay;
+ if (time_after(ready_date, jiffies + drive_params[current_drive].select_delay)) {
+ ready_date -= drive_params[current_drive].select_delay;
function = floppy_start;
} else
function = setup_rw_floppy;
@@ -1528,9 +1531,10 @@ static void seek_interrupt(void)
return;
}
if (DRS->track >= 0 && DRS->track != ST1 && !blind_seek) {
- debug_dcl(DP->flags,
+ debug_dcl(drive_params[current_drive].flags,
"clearing NEWCHANGE flag because of effective seek\n");
- debug_dcl(DP->flags, "jiffies=%lu\n", jiffies);
+ debug_dcl(drive_params[current_drive].flags, "jiffies=%lu\n",
+ jiffies);
clear_bit(FD_DISK_NEWCHANGE_BIT, &DRS->flags);
/* effective seek */
DRS->select_date = jiffies;
@@ -1551,9 +1555,10 @@ static void check_wp(void)
}
clear_bit(FD_VERIFY_BIT, &DRS->flags);
clear_bit(FD_NEED_TWADDLE_BIT, &DRS->flags);
- debug_dcl(DP->flags,
+ debug_dcl(drive_params[current_drive].flags,
"checking whether disk is write protected\n");
- debug_dcl(DP->flags, "wp=%x\n", ST3 & 0x40);
+ debug_dcl(drive_params[current_drive].flags, "wp=%x\n",
+ ST3 & 0x40);
if (!(ST3 & 0x40))
set_bit(FD_DISK_WRITABLE_BIT, &DRS->flags);
else
@@ -1567,7 +1572,8 @@ static void seek_floppy(void)
blind_seek = 0;
- debug_dcl(DP->flags, "calling disk change from %s\n", __func__);
+ debug_dcl(drive_params[current_drive].flags,
+ "calling disk change from %s\n", __func__);
if (!test_bit(FD_DISK_NEWCHANGE_BIT, &DRS->flags) &&
disk_change(current_drive) && (raw_cmd->flags & FD_RAW_NEED_DISK)) {
@@ -1591,7 +1597,7 @@ static void seek_floppy(void)
if (raw_cmd->track)
track = raw_cmd->track - 1;
else {
- if (DP->flags & FD_SILENT_DCL_CLEAR) {
+ if (drive_params[current_drive].flags & FD_SILENT_DCL_CLEAR) {
set_dor(fdc, ~(0x10 << UNIT(current_drive)), 0);
blind_seek = 1;
raw_cmd->flags |= FD_RAW_NEED_SEEK;
@@ -1643,7 +1649,7 @@ static void recal_interrupt(void)
* not to move at recalibration is to
* be already at track 0.) Clear the
* new change flag */
- debug_dcl(DP->flags,
+ debug_dcl(drive_params[current_drive].flags,
"clearing NEWCHANGE flag because of second recalibrate\n");
clear_bit(FD_DISK_NEWCHANGE_BIT, &DRS->flags);
@@ -1884,7 +1890,7 @@ static int start_motor(void (*function)(void))
set_dor(fdc, mask, data);
/* wait_for_completion also schedules reset if needed. */
- return fd_wait_for_completion(DRS->select_date + DP->select_delay,
+ return fd_wait_for_completion(DRS->select_date + drive_params[current_drive].select_delay,
function);
}
@@ -1899,9 +1905,10 @@ static void floppy_ready(void)
if (fdc_dtr())
return;
- debug_dcl(DP->flags, "calling disk change from floppy_ready\n");
+ debug_dcl(drive_params[current_drive].flags,
+ "calling disk change from floppy_ready\n");
if (!(raw_cmd->flags & FD_RAW_NO_MOTOR) &&
- disk_change(current_drive) && !DP->select_delay)
+ disk_change(current_drive) && !drive_params[current_drive].select_delay)
twaddle(); /* this clears the dcl on certain
* drive/controller combinations */
@@ -1930,7 +1937,8 @@ static void floppy_start(void)
reschedule_timeout(current_reqD, "floppy start");
scandrives();
- debug_dcl(DP->flags, "setting NEWCHANGE in floppy_start\n");
+ debug_dcl(drive_params[current_drive].flags,
+ "setting NEWCHANGE in floppy_start\n");
set_bit(FD_DISK_NEWCHANGE_BIT, &DRS->flags);
floppy_ready();
}
@@ -2032,11 +2040,11 @@ static int next_valid_format(void)
probed_format = DRS->probed_format;
while (1) {
- if (probed_format >= 8 || !DP->autodetect[probed_format]) {
+ if (probed_format >= 8 || !drive_params[current_drive].autodetect[probed_format]) {
DRS->probed_format = 0;
return 1;
}
- if (floppy_type[DP->autodetect[probed_format]].sect) {
+ if (floppy_type[drive_params[current_drive].autodetect[probed_format]].sect) {
DRS->probed_format = probed_format;
return 0;
}
@@ -2055,11 +2063,11 @@ static void bad_flp_intr(void)
}
err_count = ++(*errors);
INFBOUND(DRWE->badness, err_count);
- if (err_count > DP->max_errors.abort)
+ if (err_count > drive_params[current_drive].max_errors.abort)
cont->done(0);
- if (err_count > DP->max_errors.reset)
+ if (err_count > drive_params[current_drive].max_errors.reset)
fdc_state[fdc].reset = 1;
- else if (err_count > DP->max_errors.recal)
+ else if (err_count > drive_params[current_drive].max_errors.recal)
DRS->track = NEED_2_RECAL;
}
@@ -2189,7 +2197,7 @@ static int do_format(int drive, struct format_descr *tmp_format_req)
set_floppy(drive);
if (!_floppy ||
- _floppy->track > DP->tracks ||
+ _floppy->track > drive_params[current_drive].tracks ||
tmp_format_req->track >= _floppy->track ||
tmp_format_req->head >= _floppy->head ||
(_floppy->sect << 2) % (1 << FD_SIZECODE(_floppy)) ||
@@ -2345,7 +2353,7 @@ static void rw_interrupt(void)
}
if (probing) {
- if (DP->flags & FTD_MSG)
+ if (drive_params[current_drive].flags & FTD_MSG)
DPRINT("Auto-detected floppy type %s in fd%d\n",
_floppy->name, current_drive);
current_type[current_drive] = _floppy;
@@ -2675,9 +2683,9 @@ static int make_raw_rw_request(void)
*/
if (!direct ||
(indirect * 2 > direct * 3 &&
- *errors < DP->max_errors.read_track &&
+ *errors < drive_params[current_drive].max_errors.read_track &&
((!probing ||
- (DP->read_track & (1 << DRS->probed_format)))))) {
+ (drive_params[current_drive].read_track & (1 << DRS->probed_format)))))) {
max_size = blk_rq_sectors(current_req);
} else {
raw_cmd->kernel_data = bio_data(current_req->bio);
@@ -2855,7 +2863,7 @@ static void redo_fd_request(void)
}
}
probing = 1;
- _floppy = floppy_type + DP->autodetect[DRS->probed_format];
+ _floppy = floppy_type + drive_params[current_drive].autodetect[DRS->probed_format];
} else
probing = 0;
errors = &(current_req->error_count);
@@ -2934,7 +2942,8 @@ static int poll_drive(bool interruptible, int flag)
raw_cmd->track = 0;
raw_cmd->cmd_count = 0;
cont = &poll_cont;
- debug_dcl(DP->flags, "setting NEWCHANGE in poll_drive\n");
+ debug_dcl(drive_params[current_drive].flags,
+ "setting NEWCHANGE in poll_drive\n");
set_bit(FD_DISK_NEWCHANGE_BIT, &DRS->flags);
return wait_til_done(floppy_ready, interruptible);
@@ -3205,7 +3214,8 @@ static int raw_cmd_ioctl(int cmd, void __user *param)
raw_cmd = my_raw_cmd;
cont = &raw_cmd_cont;
ret = wait_til_done(floppy_start, true);
- debug_dcl(DP->flags, "calling disk change from raw_cmd ioctl\n");
+ debug_dcl(drive_params[current_drive].flags,
+ "calling disk change from raw_cmd ioctl\n");
if (ret != -EINTR && fdc_state[fdc].reset)
ret = -EIO;
@@ -4386,7 +4396,7 @@ static void __init set_cmos(int *ints, int dummy, int dummy2)
if (current_drive >= 4 && !FDC2)
FDC2 = 0x370;
#endif
- DP->cmos = ints[2];
+ drive_params[current_drive].cmos = ints[2];
DPRINT("setting CMOS code to %d\n", ints[2]);
}
--
2.9.0
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 07/10] floppy: cleanup: expand macro DRS
2020-02-24 21:23 [PATCH 00/10] floppy driver cleanups (deobfuscation) Willy Tarreau
` (5 preceding siblings ...)
2020-02-24 21:23 ` [PATCH 06/10] floppy: cleanup: expand macro DP Willy Tarreau
@ 2020-02-24 21:23 ` Willy Tarreau
2020-02-24 21:23 ` [PATCH 08/10] floppy: cleanup: expand macro DRWE Willy Tarreau
` (4 subsequent siblings)
11 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-24 21:23 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Linus Torvalds, Willy Tarreau
This macro doesn't bring much value and only slightly obfuscates the
code by silently using global variable "current_drive", let's expand it.
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/block/floppy.c | 115 ++++++++++++++++++++++++++-----------------------
1 file changed, 61 insertions(+), 54 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 7744e42..6d4a2e1 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -306,7 +306,6 @@ static bool initialized;
/* reverse mapping from unit and fdc to drive */
#define REVDRIVE(fdc, unit) ((unit) + ((fdc) << 2))
-#define DRS (&drive_state[current_drive])
#define DRWE (&write_errors[current_drive])
#define PH_HEAD(floppy, head) (((((floppy)->stretch & 2) >> 1) ^ head) << 2)
@@ -827,7 +826,7 @@ static void twaddle(void)
return;
fd_outb(fdc_state[fdc].dor & ~(0x10 << UNIT(current_drive)), FD_DOR);
fd_outb(fdc_state[fdc].dor, FD_DOR);
- DRS->select_date = jiffies;
+ drive_state[current_drive].select_date = jiffies;
}
/*
@@ -1427,11 +1426,13 @@ static int interpret_errors(void)
bad = 1;
if (ST1 & ST1_WP) {
DPRINT("Drive is write protected\n");
- clear_bit(FD_DISK_WRITABLE_BIT, &DRS->flags);
+ clear_bit(FD_DISK_WRITABLE_BIT,
+ &drive_state[current_drive].flags);
cont->done(0);
bad = 2;
} else if (ST1 & ST1_ND) {
- set_bit(FD_NEED_TWADDLE_BIT, &DRS->flags);
+ set_bit(FD_NEED_TWADDLE_BIT,
+ &drive_state[current_drive].flags);
} else if (ST1 & ST1_OR) {
if (drive_params[current_drive].flags & FTD_MSG)
DPRINT("Over/Underrun - retrying\n");
@@ -1441,7 +1442,7 @@ static int interpret_errors(void)
}
if (ST2 & ST2_WC || ST2 & ST2_BC)
/* wrong cylinder => recal */
- DRS->track = NEED_2_RECAL;
+ drive_state[current_drive].track = NEED_2_RECAL;
return bad;
case 0x80: /* invalid command given */
DPRINT("Invalid FDC command given!\n");
@@ -1474,7 +1475,7 @@ static void setup_rw_floppy(void)
flags |= FD_RAW_INTR;
if ((flags & FD_RAW_SPIN) && !(flags & FD_RAW_NO_MOTOR)) {
- ready_date = DRS->spinup_date + drive_params[current_drive].spinup;
+ ready_date = drive_state[current_drive].spinup_date + drive_params[current_drive].spinup;
/* If spinup will take a long time, rerun scandrives
* again just before spinup completion. Beware that
* after scandrives, we must again wait for selection.
@@ -1525,27 +1526,28 @@ static void seek_interrupt(void)
debugt(__func__, "");
if (inr != 2 || (ST0 & 0xF8) != 0x20) {
DPRINT("seek failed\n");
- DRS->track = NEED_2_RECAL;
+ drive_state[current_drive].track = NEED_2_RECAL;
cont->error();
cont->redo();
return;
}
- if (DRS->track >= 0 && DRS->track != ST1 && !blind_seek) {
+ if (drive_state[current_drive].track >= 0 && drive_state[current_drive].track != ST1 && !blind_seek) {
debug_dcl(drive_params[current_drive].flags,
"clearing NEWCHANGE flag because of effective seek\n");
debug_dcl(drive_params[current_drive].flags, "jiffies=%lu\n",
jiffies);
- clear_bit(FD_DISK_NEWCHANGE_BIT, &DRS->flags);
+ clear_bit(FD_DISK_NEWCHANGE_BIT,
+ &drive_state[current_drive].flags);
/* effective seek */
- DRS->select_date = jiffies;
+ drive_state[current_drive].select_date = jiffies;
}
- DRS->track = ST1;
+ drive_state[current_drive].track = ST1;
floppy_ready();
}
static void check_wp(void)
{
- if (test_bit(FD_VERIFY_BIT, &DRS->flags)) {
+ if (test_bit(FD_VERIFY_BIT, &drive_state[current_drive].flags)) {
/* check write protection */
output_byte(FD_GETSTATUS);
output_byte(UNIT(current_drive));
@@ -1553,16 +1555,19 @@ static void check_wp(void)
fdc_state[fdc].reset = 1;
return;
}
- clear_bit(FD_VERIFY_BIT, &DRS->flags);
- clear_bit(FD_NEED_TWADDLE_BIT, &DRS->flags);
+ clear_bit(FD_VERIFY_BIT, &drive_state[current_drive].flags);
+ clear_bit(FD_NEED_TWADDLE_BIT,
+ &drive_state[current_drive].flags);
debug_dcl(drive_params[current_drive].flags,
"checking whether disk is write protected\n");
debug_dcl(drive_params[current_drive].flags, "wp=%x\n",
ST3 & 0x40);
if (!(ST3 & 0x40))
- set_bit(FD_DISK_WRITABLE_BIT, &DRS->flags);
+ set_bit(FD_DISK_WRITABLE_BIT,
+ &drive_state[current_drive].flags);
else
- clear_bit(FD_DISK_WRITABLE_BIT, &DRS->flags);
+ clear_bit(FD_DISK_WRITABLE_BIT,
+ &drive_state[current_drive].flags);
}
}
@@ -1575,23 +1580,24 @@ static void seek_floppy(void)
debug_dcl(drive_params[current_drive].flags,
"calling disk change from %s\n", __func__);
- if (!test_bit(FD_DISK_NEWCHANGE_BIT, &DRS->flags) &&
+ if (!test_bit(FD_DISK_NEWCHANGE_BIT, &drive_state[current_drive].flags) &&
disk_change(current_drive) && (raw_cmd->flags & FD_RAW_NEED_DISK)) {
/* the media changed flag should be cleared after the seek.
* If it isn't, this means that there is really no disk in
* the drive.
*/
- set_bit(FD_DISK_CHANGED_BIT, &DRS->flags);
+ set_bit(FD_DISK_CHANGED_BIT,
+ &drive_state[current_drive].flags);
cont->done(0);
cont->redo();
return;
}
- if (DRS->track <= NEED_1_RECAL) {
+ if (drive_state[current_drive].track <= NEED_1_RECAL) {
recalibrate_floppy();
return;
- } else if (test_bit(FD_DISK_NEWCHANGE_BIT, &DRS->flags) &&
+ } else if (test_bit(FD_DISK_NEWCHANGE_BIT, &drive_state[current_drive].flags) &&
(raw_cmd->flags & FD_RAW_NEED_DISK) &&
- (DRS->track <= NO_TRACK || DRS->track == raw_cmd->track)) {
+ (drive_state[current_drive].track <= NO_TRACK || drive_state[current_drive].track == raw_cmd->track)) {
/* we seek to clear the media-changed condition. Does anybody
* know a more elegant way, which works on all drives? */
if (raw_cmd->track)
@@ -1606,7 +1612,7 @@ static void seek_floppy(void)
}
} else {
check_wp();
- if (raw_cmd->track != DRS->track &&
+ if (raw_cmd->track != drive_state[current_drive].track &&
(raw_cmd->flags & FD_RAW_NEED_SEEK))
track = raw_cmd->track;
else {
@@ -1631,7 +1637,7 @@ static void recal_interrupt(void)
if (inr != 2)
fdc_state[fdc].reset = 1;
else if (ST0 & ST0_ECE) {
- switch (DRS->track) {
+ switch (drive_state[current_drive].track) {
case NEED_1_RECAL:
debugt(__func__, "need 1 recal");
/* after a second recalibrate, we still haven't
@@ -1652,8 +1658,9 @@ static void recal_interrupt(void)
debug_dcl(drive_params[current_drive].flags,
"clearing NEWCHANGE flag because of second recalibrate\n");
- clear_bit(FD_DISK_NEWCHANGE_BIT, &DRS->flags);
- DRS->select_date = jiffies;
+ clear_bit(FD_DISK_NEWCHANGE_BIT,
+ &drive_state[current_drive].flags);
+ drive_state[current_drive].select_date = jiffies;
/* fall through */
default:
debugt(__func__, "default");
@@ -1663,11 +1670,11 @@ static void recal_interrupt(void)
* track 0, this might mean that we
* started beyond track 80. Try
* again. */
- DRS->track = NEED_1_RECAL;
+ drive_state[current_drive].track = NEED_1_RECAL;
break;
}
} else
- DRS->track = ST1;
+ drive_state[current_drive].track = ST1;
floppy_ready();
}
@@ -1877,9 +1884,9 @@ static int start_motor(void (*function)(void))
if (!(fdc_state[fdc].dor & (0x10 << UNIT(current_drive)))) {
set_debugt();
/* no read since this drive is running */
- DRS->first_read_date = 0;
+ drive_state[current_drive].first_read_date = 0;
/* note motor start time if motor is not yet running */
- DRS->spinup_date = jiffies;
+ drive_state[current_drive].spinup_date = jiffies;
data |= (0x10 << UNIT(current_drive));
}
} else if (fdc_state[fdc].dor & (0x10 << UNIT(current_drive)))
@@ -1890,7 +1897,7 @@ static int start_motor(void (*function)(void))
set_dor(fdc, mask, data);
/* wait_for_completion also schedules reset if needed. */
- return fd_wait_for_completion(DRS->select_date + drive_params[current_drive].select_delay,
+ return fd_wait_for_completion(drive_state[current_drive].select_date + drive_params[current_drive].select_delay,
function);
}
@@ -1939,7 +1946,7 @@ static void floppy_start(void)
scandrives();
debug_dcl(drive_params[current_drive].flags,
"setting NEWCHANGE in floppy_start\n");
- set_bit(FD_DISK_NEWCHANGE_BIT, &DRS->flags);
+ set_bit(FD_DISK_NEWCHANGE_BIT, &drive_state[current_drive].flags);
floppy_ready();
}
@@ -2038,14 +2045,14 @@ static int next_valid_format(void)
{
int probed_format;
- probed_format = DRS->probed_format;
+ probed_format = drive_state[current_drive].probed_format;
while (1) {
if (probed_format >= 8 || !drive_params[current_drive].autodetect[probed_format]) {
- DRS->probed_format = 0;
+ drive_state[current_drive].probed_format = 0;
return 1;
}
if (floppy_type[drive_params[current_drive].autodetect[probed_format]].sect) {
- DRS->probed_format = probed_format;
+ drive_state[current_drive].probed_format = probed_format;
return 0;
}
probed_format++;
@@ -2057,7 +2064,7 @@ static void bad_flp_intr(void)
int err_count;
if (probing) {
- DRS->probed_format++;
+ drive_state[current_drive].probed_format++;
if (!next_valid_format())
return;
}
@@ -2068,7 +2075,7 @@ static void bad_flp_intr(void)
if (err_count > drive_params[current_drive].max_errors.reset)
fdc_state[fdc].reset = 1;
else if (err_count > drive_params[current_drive].max_errors.recal)
- DRS->track = NEED_2_RECAL;
+ drive_state[current_drive].track = NEED_2_RECAL;
}
static void set_floppy(int drive)
@@ -2259,9 +2266,9 @@ static void request_done(int uptodate)
/* maintain values for invalidation on geometry
* change */
block = current_count_sectors + blk_rq_pos(req);
- INFBOUND(DRS->maxblock, block);
+ INFBOUND(drive_state[current_drive].maxblock, block);
if (block > _floppy->sect)
- DRS->maxtrack = 1;
+ drive_state[current_drive].maxtrack = 1;
floppy_end_request(req, 0);
} else {
@@ -2270,10 +2277,10 @@ static void request_done(int uptodate)
DRWE->write_errors++;
if (DRWE->write_errors == 1) {
DRWE->first_error_sector = blk_rq_pos(req);
- DRWE->first_error_generation = DRS->generation;
+ DRWE->first_error_generation = drive_state[current_drive].generation;
}
DRWE->last_error_sector = blk_rq_pos(req);
- DRWE->last_error_generation = DRS->generation;
+ DRWE->last_error_generation = drive_state[current_drive].generation;
}
floppy_end_request(req, BLK_STS_IOERR);
}
@@ -2294,8 +2301,8 @@ static void rw_interrupt(void)
return;
}
- if (!DRS->first_read_date)
- DRS->first_read_date = jiffies;
+ if (!drive_state[current_drive].first_read_date)
+ drive_state[current_drive].first_read_date = jiffies;
nr_sectors = 0;
ssize = DIV_ROUND_UP(1 << SIZECODE, 4);
@@ -2568,7 +2575,7 @@ static int make_raw_rw_request(void)
HEAD = fsector_t / _floppy->sect;
if (((_floppy->stretch & (FD_SWAPSIDES | FD_SECTBASEMASK)) ||
- test_bit(FD_NEED_TWADDLE_BIT, &DRS->flags)) &&
+ test_bit(FD_NEED_TWADDLE_BIT, &drive_state[current_drive].flags)) &&
fsector_t < _floppy->sect)
max_sector = _floppy->sect;
@@ -2685,7 +2692,7 @@ static int make_raw_rw_request(void)
(indirect * 2 > direct * 3 &&
*errors < drive_params[current_drive].max_errors.read_track &&
((!probing ||
- (drive_params[current_drive].read_track & (1 << DRS->probed_format)))))) {
+ (drive_params[current_drive].read_track & (1 << drive_state[current_drive].probed_format)))))) {
max_size = blk_rq_sectors(current_req);
} else {
raw_cmd->kernel_data = bio_data(current_req->bio);
@@ -2847,14 +2854,14 @@ static void redo_fd_request(void)
disk_change(current_drive);
if (test_bit(current_drive, &fake_change) ||
- test_bit(FD_DISK_CHANGED_BIT, &DRS->flags)) {
+ test_bit(FD_DISK_CHANGED_BIT, &drive_state[current_drive].flags)) {
DPRINT("disk absent or changed during operation\n");
request_done(0);
goto do_request;
}
if (!_floppy) { /* Autodetection */
if (!probing) {
- DRS->probed_format = 0;
+ drive_state[current_drive].probed_format = 0;
if (next_valid_format()) {
DPRINT("no autodetectable formats\n");
_floppy = NULL;
@@ -2863,7 +2870,7 @@ static void redo_fd_request(void)
}
}
probing = 1;
- _floppy = floppy_type + drive_params[current_drive].autodetect[DRS->probed_format];
+ _floppy = floppy_type + drive_params[current_drive].autodetect[drive_state[current_drive].probed_format];
} else
probing = 0;
errors = &(current_req->error_count);
@@ -2873,7 +2880,7 @@ static void redo_fd_request(void)
goto do_request;
}
- if (test_bit(FD_NEED_TWADDLE_BIT, &DRS->flags))
+ if (test_bit(FD_NEED_TWADDLE_BIT, &drive_state[current_drive].flags))
twaddle();
schedule_bh(floppy_start);
debugt(__func__, "queue fd request");
@@ -2944,7 +2951,7 @@ static int poll_drive(bool interruptible, int flag)
cont = &poll_cont;
debug_dcl(drive_params[current_drive].flags,
"setting NEWCHANGE in poll_drive\n");
- set_bit(FD_DISK_NEWCHANGE_BIT, &DRS->flags);
+ set_bit(FD_DISK_NEWCHANGE_BIT, &drive_state[current_drive].flags);
return wait_til_done(floppy_ready, interruptible);
}
@@ -3220,7 +3227,7 @@ static int raw_cmd_ioctl(int cmd, void __user *param)
if (ret != -EINTR && fdc_state[fdc].reset)
ret = -EIO;
- DRS->track = NO_TRACK;
+ drive_state[current_drive].track = NO_TRACK;
ret2 = raw_cmd_copyout(cmd, param, my_raw_cmd);
if (!ret)
@@ -3293,16 +3300,16 @@ static int set_geometry(unsigned int cmd, struct floppy_struct *g,
current_type[drive] = &user_params[drive];
floppy_sizes[drive] = user_params[drive].size;
if (cmd == FDDEFPRM)
- DRS->keep_data = -1;
+ drive_state[current_drive].keep_data = -1;
else
- DRS->keep_data = 1;
+ drive_state[current_drive].keep_data = 1;
/* invalidation. Invalidate only when needed, i.e.
* when there are already sectors in the buffer cache
* whose number will change. This is useful, because
* mtools often changes the geometry of the disk after
* looking at the boot block */
- if (DRS->maxblock > user_params[drive].sect ||
- DRS->maxtrack ||
+ if (drive_state[current_drive].maxblock > user_params[drive].sect ||
+ drive_state[current_drive].maxtrack ||
((user_params[drive].sect ^ oldStretch) &
(FD_SWAPSIDES | FD_SECTBASEMASK)))
invalidate_drive(bdev);
--
2.9.0
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 08/10] floppy: cleanup: expand macro DRWE
2020-02-24 21:23 [PATCH 00/10] floppy driver cleanups (deobfuscation) Willy Tarreau
` (6 preceding siblings ...)
2020-02-24 21:23 ` [PATCH 07/10] floppy: cleanup: expand macro DRS Willy Tarreau
@ 2020-02-24 21:23 ` Willy Tarreau
2020-02-24 21:23 ` [PATCH 09/10] floppy: cleanup: expand the R/W / format command macros Willy Tarreau
` (3 subsequent siblings)
11 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-24 21:23 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Linus Torvalds, Willy Tarreau
This macro doesn't bring much value and only slightly obfuscates the
code by silently using global variable "current_drive", let's expand it.
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/block/floppy.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 6d4a2e1..d771579 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -306,8 +306,6 @@ static bool initialized;
/* reverse mapping from unit and fdc to drive */
#define REVDRIVE(fdc, unit) ((unit) + ((fdc) << 2))
-#define DRWE (&write_errors[current_drive])
-
#define PH_HEAD(floppy, head) (((((floppy)->stretch & 2) >> 1) ^ head) << 2)
#define STRETCH(floppy) ((floppy)->stretch & FD_STRETCH)
@@ -2069,7 +2067,7 @@ static void bad_flp_intr(void)
return;
}
err_count = ++(*errors);
- INFBOUND(DRWE->badness, err_count);
+ INFBOUND(write_errors[current_drive].badness, err_count);
if (err_count > drive_params[current_drive].max_errors.abort)
cont->done(0);
if (err_count > drive_params[current_drive].max_errors.reset)
@@ -2274,13 +2272,13 @@ static void request_done(int uptodate)
} else {
if (rq_data_dir(req) == WRITE) {
/* record write error information */
- DRWE->write_errors++;
- if (DRWE->write_errors == 1) {
- DRWE->first_error_sector = blk_rq_pos(req);
- DRWE->first_error_generation = drive_state[current_drive].generation;
+ write_errors[current_drive].write_errors++;
+ if (write_errors[current_drive].write_errors == 1) {
+ write_errors[current_drive].first_error_sector = blk_rq_pos(req);
+ write_errors[current_drive].first_error_generation = drive_state[current_drive].generation;
}
- DRWE->last_error_sector = blk_rq_pos(req);
- DRWE->last_error_generation = drive_state[current_drive].generation;
+ write_errors[current_drive].last_error_sector = blk_rq_pos(req);
+ write_errors[current_drive].last_error_generation = drive_state[current_drive].generation;
}
floppy_end_request(req, BLK_STS_IOERR);
}
--
2.9.0
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 09/10] floppy: cleanup: expand the R/W / format command macros
2020-02-24 21:23 [PATCH 00/10] floppy driver cleanups (deobfuscation) Willy Tarreau
` (7 preceding siblings ...)
2020-02-24 21:23 ` [PATCH 08/10] floppy: cleanup: expand macro DRWE Willy Tarreau
@ 2020-02-24 21:23 ` Willy Tarreau
2020-02-24 21:23 ` [PATCH 10/10] floppy: cleanup: expand the reply_buffer macros Willy Tarreau
` (2 subsequent siblings)
11 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-24 21:23 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Linus Torvalds, Willy Tarreau
Various macros were used to access raw_cmd for R/W or format commands
without making it obvious that raw_cmd->cmd[] was used. Let's expand
the macros to make this more obvious.
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/block/floppy.c | 194 +++++++++++++++++++++++++------------------------
1 file changed, 98 insertions(+), 96 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index d771579..0d53335 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -309,23 +309,23 @@ static bool initialized;
#define PH_HEAD(floppy, head) (((((floppy)->stretch & 2) >> 1) ^ head) << 2)
#define STRETCH(floppy) ((floppy)->stretch & FD_STRETCH)
-/* read/write */
-#define COMMAND (raw_cmd->cmd[0])
-#define DR_SELECT (raw_cmd->cmd[1])
-#define TRACK (raw_cmd->cmd[2])
-#define HEAD (raw_cmd->cmd[3])
-#define SECTOR (raw_cmd->cmd[4])
-#define SIZECODE (raw_cmd->cmd[5])
-#define SECT_PER_TRACK (raw_cmd->cmd[6])
-#define GAP (raw_cmd->cmd[7])
-#define SIZECODE2 (raw_cmd->cmd[8])
+/* read/write commands */
+#define COMMAND 0
+#define DR_SELECT 1
+#define TRACK 2
+#define HEAD 3
+#define SECTOR 4
+#define SIZECODE 5
+#define SECT_PER_TRACK 6
+#define GAP 7
+#define SIZECODE2 8
#define NR_RW 9
-/* format */
-#define F_SIZECODE (raw_cmd->cmd[2])
-#define F_SECT_PER_TRACK (raw_cmd->cmd[3])
-#define F_GAP (raw_cmd->cmd[4])
-#define F_FILL (raw_cmd->cmd[5])
+/* format commands */
+#define F_SIZECODE 2
+#define F_SECT_PER_TRACK 3
+#define F_GAP 4
+#define F_FILL 5
#define NR_F 6
/*
@@ -2124,28 +2124,28 @@ static void setup_format_params(int track)
FD_RAW_NEED_DISK | FD_RAW_NEED_SEEK);
raw_cmd->rate = _floppy->rate & 0x43;
raw_cmd->cmd_count = NR_F;
- COMMAND = FM_MODE(_floppy, FD_FORMAT);
- DR_SELECT = UNIT(current_drive) + PH_HEAD(_floppy, format_req.head);
- F_SIZECODE = FD_SIZECODE(_floppy);
- F_SECT_PER_TRACK = _floppy->sect << 2 >> F_SIZECODE;
- F_GAP = _floppy->fmt_gap;
- F_FILL = FD_FILL_BYTE;
+ raw_cmd->cmd[COMMAND] = FM_MODE(_floppy, FD_FORMAT);
+ raw_cmd->cmd[DR_SELECT] = UNIT(current_drive) + PH_HEAD(_floppy, format_req.head);
+ raw_cmd->cmd[F_SIZECODE] = FD_SIZECODE(_floppy);
+ raw_cmd->cmd[F_SECT_PER_TRACK] = _floppy->sect << 2 >> raw_cmd->cmd[F_SIZECODE];
+ raw_cmd->cmd[F_GAP] = _floppy->fmt_gap;
+ raw_cmd->cmd[F_FILL] = FD_FILL_BYTE;
raw_cmd->kernel_data = floppy_track_buffer;
- raw_cmd->length = 4 * F_SECT_PER_TRACK;
+ raw_cmd->length = 4 * raw_cmd->cmd[F_SECT_PER_TRACK];
- if (!F_SECT_PER_TRACK)
+ if (!raw_cmd->cmd[F_SECT_PER_TRACK])
return;
/* allow for about 30ms for data transport per track */
- head_shift = (F_SECT_PER_TRACK + 5) / 6;
+ head_shift = (raw_cmd->cmd[F_SECT_PER_TRACK] + 5) / 6;
/* a ``cylinder'' is two tracks plus a little stepping time */
track_shift = 2 * head_shift + 3;
/* position of logical sector 1 on this track */
n = (track_shift * format_req.track + head_shift * format_req.head)
- % F_SECT_PER_TRACK;
+ % raw_cmd->cmd[F_SECT_PER_TRACK];
/* determine interleave */
il = 1;
@@ -2153,27 +2153,27 @@ static void setup_format_params(int track)
il++;
/* initialize field */
- for (count = 0; count < F_SECT_PER_TRACK; ++count) {
+ for (count = 0; count < raw_cmd->cmd[F_SECT_PER_TRACK]; ++count) {
here[count].track = format_req.track;
here[count].head = format_req.head;
here[count].sect = 0;
- here[count].size = F_SIZECODE;
+ here[count].size = raw_cmd->cmd[F_SIZECODE];
}
/* place logical sectors */
- for (count = 1; count <= F_SECT_PER_TRACK; ++count) {
+ for (count = 1; count <= raw_cmd->cmd[F_SECT_PER_TRACK]; ++count) {
here[n].sect = count;
- n = (n + il) % F_SECT_PER_TRACK;
+ n = (n + il) % raw_cmd->cmd[F_SECT_PER_TRACK];
if (here[n].sect) { /* sector busy, find next free sector */
++n;
- if (n >= F_SECT_PER_TRACK) {
- n -= F_SECT_PER_TRACK;
+ if (n >= raw_cmd->cmd[F_SECT_PER_TRACK]) {
+ n -= raw_cmd->cmd[F_SECT_PER_TRACK];
while (here[n].sect)
++n;
}
}
}
if (_floppy->stretch & FD_SECTBASEMASK) {
- for (count = 0; count < F_SECT_PER_TRACK; count++)
+ for (count = 0; count < raw_cmd->cmd[F_SECT_PER_TRACK]; count++)
here[count].sect += FD_SECTBASE(_floppy) - 1;
}
}
@@ -2303,32 +2303,32 @@ static void rw_interrupt(void)
drive_state[current_drive].first_read_date = jiffies;
nr_sectors = 0;
- ssize = DIV_ROUND_UP(1 << SIZECODE, 4);
+ ssize = DIV_ROUND_UP(1 << raw_cmd->cmd[SIZECODE], 4);
if (ST1 & ST1_EOC)
eoc = 1;
else
eoc = 0;
- if (COMMAND & 0x80)
+ if (raw_cmd->cmd[COMMAND] & 0x80)
heads = 2;
else
heads = 1;
- nr_sectors = (((R_TRACK - TRACK) * heads +
- R_HEAD - HEAD) * SECT_PER_TRACK +
- R_SECTOR - SECTOR + eoc) << SIZECODE >> 2;
+ nr_sectors = (((R_TRACK - raw_cmd->cmd[TRACK]) * heads +
+ R_HEAD - raw_cmd->cmd[HEAD]) * raw_cmd->cmd[SECT_PER_TRACK] +
+ R_SECTOR - raw_cmd->cmd[SECTOR] + eoc) << raw_cmd->cmd[SIZECODE] >> 2;
if (nr_sectors / ssize >
DIV_ROUND_UP(in_sector_offset + current_count_sectors, ssize)) {
DPRINT("long rw: %x instead of %lx\n",
nr_sectors, current_count_sectors);
- pr_info("rs=%d s=%d\n", R_SECTOR, SECTOR);
- pr_info("rh=%d h=%d\n", R_HEAD, HEAD);
- pr_info("rt=%d t=%d\n", R_TRACK, TRACK);
+ pr_info("rs=%d s=%d\n", R_SECTOR, raw_cmd->cmd[SECTOR]);
+ pr_info("rh=%d h=%d\n", R_HEAD, raw_cmd->cmd[HEAD]);
+ pr_info("rt=%d t=%d\n", R_TRACK, raw_cmd->cmd[TRACK]);
pr_info("heads=%d eoc=%d\n", heads, eoc);
pr_info("spt=%d st=%d ss=%d\n",
- SECT_PER_TRACK, fsector_t, ssize);
+ raw_cmd->cmd[SECT_PER_TRACK], fsector_t, ssize);
pr_info("in_sector_offset=%d\n", in_sector_offset);
}
@@ -2366,11 +2366,11 @@ static void rw_interrupt(void)
probing = 0;
}
- if (CT(COMMAND) != FD_READ ||
+ if (CT(raw_cmd->cmd[COMMAND]) != FD_READ ||
raw_cmd->kernel_data == bio_data(current_req->bio)) {
/* transfer directly from buffer */
cont->done(1);
- } else if (CT(COMMAND) == FD_READ) {
+ } else if (CT(raw_cmd->cmd[COMMAND]) == FD_READ) {
buffer_track = raw_cmd->track;
buffer_drive = current_drive;
INFBOUND(buffer_max, nr_sectors + fsector_t);
@@ -2429,13 +2429,13 @@ static void copy_buffer(int ssize, int max_sector, int max_sector_2)
min(max_sector, max_sector_2),
blk_rq_sectors(current_req));
- if (current_count_sectors <= 0 && CT(COMMAND) == FD_WRITE &&
+ if (current_count_sectors <= 0 && CT(raw_cmd->cmd[COMMAND]) == FD_WRITE &&
buffer_max > fsector_t + blk_rq_sectors(current_req))
current_count_sectors = min_t(int, buffer_max - fsector_t,
blk_rq_sectors(current_req));
remaining = current_count_sectors << 9;
- if (remaining > blk_rq_bytes(current_req) && CT(COMMAND) == FD_WRITE) {
+ if (remaining > blk_rq_bytes(current_req) && CT(raw_cmd->cmd[COMMAND]) == FD_WRITE) {
DPRINT("in copy buffer\n");
pr_info("current_count_sectors=%ld\n", current_count_sectors);
pr_info("remaining=%d\n", remaining >> 9);
@@ -2470,16 +2470,16 @@ static void copy_buffer(int ssize, int max_sector, int max_sector_2)
fsector_t, buffer_min);
pr_info("current_count_sectors=%ld\n",
current_count_sectors);
- if (CT(COMMAND) == FD_READ)
+ if (CT(raw_cmd->cmd[COMMAND]) == FD_READ)
pr_info("read\n");
- if (CT(COMMAND) == FD_WRITE)
+ if (CT(raw_cmd->cmd[COMMAND]) == FD_WRITE)
pr_info("write\n");
break;
}
if (((unsigned long)buffer) % 512)
DPRINT("%p buffer not aligned\n", buffer);
- if (CT(COMMAND) == FD_READ)
+ if (CT(raw_cmd->cmd[COMMAND]) == FD_READ)
memcpy(buffer, dma_buffer, size);
else
memcpy(dma_buffer, buffer, size);
@@ -2497,7 +2497,7 @@ static void copy_buffer(int ssize, int max_sector, int max_sector_2)
/* work around a bug in pseudo DMA
* (on some FDCs) pseudo DMA does not stop when the CPU stops
* sending data. Hence we need a different way to signal the
- * transfer length: We use SECT_PER_TRACK. Unfortunately, this
+ * transfer length: We use raw_cmd->cmd[SECT_PER_TRACK]. Unfortunately, this
* does not work with MT, hence we can only transfer one head at
* a time
*/
@@ -2506,18 +2506,18 @@ static void virtualdmabug_workaround(void)
int hard_sectors;
int end_sector;
- if (CT(COMMAND) == FD_WRITE) {
- COMMAND &= ~0x80; /* switch off multiple track mode */
+ if (CT(raw_cmd->cmd[COMMAND]) == FD_WRITE) {
+ raw_cmd->cmd[COMMAND] &= ~0x80; /* switch off multiple track mode */
- hard_sectors = raw_cmd->length >> (7 + SIZECODE);
- end_sector = SECTOR + hard_sectors - 1;
- if (end_sector > SECT_PER_TRACK) {
+ hard_sectors = raw_cmd->length >> (7 + raw_cmd->cmd[SIZECODE]);
+ end_sector = raw_cmd->cmd[SECTOR] + hard_sectors - 1;
+ if (end_sector > raw_cmd->cmd[SECT_PER_TRACK]) {
pr_info("too many sectors %d > %d\n",
- end_sector, SECT_PER_TRACK);
+ end_sector, raw_cmd->cmd[SECT_PER_TRACK]);
return;
}
- SECT_PER_TRACK = end_sector;
- /* make sure SECT_PER_TRACK
+ raw_cmd->cmd[SECT_PER_TRACK] = end_sector;
+ /* make sure raw_cmd->cmd[SECT_PER_TRACK]
* points to end of transfer */
}
}
@@ -2550,10 +2550,10 @@ static int make_raw_rw_request(void)
raw_cmd->cmd_count = NR_RW;
if (rq_data_dir(current_req) == READ) {
raw_cmd->flags |= FD_RAW_READ;
- COMMAND = FM_MODE(_floppy, FD_READ);
+ raw_cmd->cmd[COMMAND] = FM_MODE(_floppy, FD_READ);
} else if (rq_data_dir(current_req) == WRITE) {
raw_cmd->flags |= FD_RAW_WRITE;
- COMMAND = FM_MODE(_floppy, FD_WRITE);
+ raw_cmd->cmd[COMMAND] = FM_MODE(_floppy, FD_WRITE);
} else {
DPRINT("%s: unknown command\n", __func__);
return 0;
@@ -2561,16 +2561,16 @@ static int make_raw_rw_request(void)
max_sector = _floppy->sect * _floppy->head;
- TRACK = (int)blk_rq_pos(current_req) / max_sector;
+ raw_cmd->cmd[TRACK] = (int)blk_rq_pos(current_req) / max_sector;
fsector_t = (int)blk_rq_pos(current_req) % max_sector;
- if (_floppy->track && TRACK >= _floppy->track) {
+ if (_floppy->track && raw_cmd->cmd[TRACK] >= _floppy->track) {
if (blk_rq_cur_sectors(current_req) & 1) {
current_count_sectors = 1;
return 1;
} else
return 0;
}
- HEAD = fsector_t / _floppy->sect;
+ raw_cmd->cmd[HEAD] = fsector_t / _floppy->sect;
if (((_floppy->stretch & (FD_SWAPSIDES | FD_SECTBASEMASK)) ||
test_bit(FD_NEED_TWADDLE_BIT, &drive_state[current_drive].flags)) &&
@@ -2578,7 +2578,7 @@ static int make_raw_rw_request(void)
max_sector = _floppy->sect;
/* 2M disks have phantom sectors on the first track */
- if ((_floppy->rate & FD_2M) && (!TRACK) && (!HEAD)) {
+ if ((_floppy->rate & FD_2M) && (!raw_cmd->cmd[TRACK]) && (!raw_cmd->cmd[HEAD])) {
max_sector = 2 * _floppy->sect / 3;
if (fsector_t >= max_sector) {
current_count_sectors =
@@ -2586,23 +2586,24 @@ static int make_raw_rw_request(void)
blk_rq_sectors(current_req));
return 1;
}
- SIZECODE = 2;
+ raw_cmd->cmd[SIZECODE] = 2;
} else
- SIZECODE = FD_SIZECODE(_floppy);
+ raw_cmd->cmd[SIZECODE] = FD_SIZECODE(_floppy);
raw_cmd->rate = _floppy->rate & 0x43;
- if ((_floppy->rate & FD_2M) && (TRACK || HEAD) && raw_cmd->rate == 2)
+ if ((_floppy->rate & FD_2M) &&
+ (raw_cmd->cmd[TRACK] || raw_cmd->cmd[HEAD]) && raw_cmd->rate == 2)
raw_cmd->rate = 1;
- if (SIZECODE)
- SIZECODE2 = 0xff;
+ if (raw_cmd->cmd[SIZECODE])
+ raw_cmd->cmd[SIZECODE2] = 0xff;
else
- SIZECODE2 = 0x80;
- raw_cmd->track = TRACK << STRETCH(_floppy);
- DR_SELECT = UNIT(current_drive) + PH_HEAD(_floppy, HEAD);
- GAP = _floppy->gap;
- ssize = DIV_ROUND_UP(1 << SIZECODE, 4);
- SECT_PER_TRACK = _floppy->sect << 2 >> SIZECODE;
- SECTOR = ((fsector_t % _floppy->sect) << 2 >> SIZECODE) +
+ raw_cmd->cmd[SIZECODE2] = 0x80;
+ raw_cmd->track = raw_cmd->cmd[TRACK] << STRETCH(_floppy);
+ raw_cmd->cmd[DR_SELECT] = UNIT(current_drive) + PH_HEAD(_floppy, raw_cmd->cmd[HEAD]);
+ raw_cmd->cmd[GAP] = _floppy->gap;
+ ssize = DIV_ROUND_UP(1 << raw_cmd->cmd[SIZECODE], 4);
+ raw_cmd->cmd[SECT_PER_TRACK] = _floppy->sect << 2 >> raw_cmd->cmd[SIZECODE];
+ raw_cmd->cmd[SECTOR] = ((fsector_t % _floppy->sect) << 2 >> raw_cmd->cmd[SIZECODE]) +
FD_SECTBASE(_floppy);
/* tracksize describes the size which can be filled up with sectors
@@ -2610,24 +2611,24 @@ static int make_raw_rw_request(void)
*/
tracksize = _floppy->sect - _floppy->sect % ssize;
if (tracksize < _floppy->sect) {
- SECT_PER_TRACK++;
+ raw_cmd->cmd[SECT_PER_TRACK]++;
if (tracksize <= fsector_t % _floppy->sect)
- SECTOR--;
+ raw_cmd->cmd[SECTOR]--;
/* if we are beyond tracksize, fill up using smaller sectors */
while (tracksize <= fsector_t % _floppy->sect) {
while (tracksize + ssize > _floppy->sect) {
- SIZECODE--;
+ raw_cmd->cmd[SIZECODE]--;
ssize >>= 1;
}
- SECTOR++;
- SECT_PER_TRACK++;
+ raw_cmd->cmd[SECTOR]++;
+ raw_cmd->cmd[SECT_PER_TRACK]++;
tracksize += ssize;
}
- max_sector = HEAD * _floppy->sect + tracksize;
- } else if (!TRACK && !HEAD && !(_floppy->rate & FD_2M) && probing) {
+ max_sector = raw_cmd->cmd[HEAD] * _floppy->sect + tracksize;
+ } else if (!raw_cmd->cmd[TRACK] && !raw_cmd->cmd[HEAD] && !(_floppy->rate & FD_2M) && probing) {
max_sector = _floppy->sect;
- } else if (!HEAD && CT(COMMAND) == FD_WRITE) {
+ } else if (!raw_cmd->cmd[HEAD] && CT(raw_cmd->cmd[COMMAND]) == FD_WRITE) {
/* for virtual DMA bug workaround */
max_sector = _floppy->sect;
}
@@ -2639,12 +2640,12 @@ static int make_raw_rw_request(void)
(current_drive == buffer_drive) &&
(fsector_t >= buffer_min) && (fsector_t < buffer_max)) {
/* data already in track buffer */
- if (CT(COMMAND) == FD_READ) {
+ if (CT(raw_cmd->cmd[COMMAND]) == FD_READ) {
copy_buffer(1, max_sector, buffer_max);
return 1;
}
} else if (in_sector_offset || blk_rq_sectors(current_req) < ssize) {
- if (CT(COMMAND) == FD_WRITE) {
+ if (CT(raw_cmd->cmd[COMMAND]) == FD_WRITE) {
unsigned int sectors;
sectors = fsector_t + blk_rq_sectors(current_req);
@@ -2655,7 +2656,7 @@ static int make_raw_rw_request(void)
}
raw_cmd->flags &= ~FD_RAW_WRITE;
raw_cmd->flags |= FD_RAW_READ;
- COMMAND = FM_MODE(_floppy, FD_READ);
+ raw_cmd->cmd[COMMAND] = FM_MODE(_floppy, FD_READ);
} else if ((unsigned long)bio_data(current_req->bio) < MAX_DMA_ADDRESS) {
unsigned long dma_limit;
int direct, indirect;
@@ -2706,7 +2707,7 @@ static int make_raw_rw_request(void)
}
}
- if (CT(COMMAND) == FD_READ)
+ if (CT(raw_cmd->cmd[COMMAND]) == FD_READ)
max_size = max_sector; /* unbounded */
/* claim buffer track if needed */
@@ -2714,7 +2715,7 @@ static int make_raw_rw_request(void)
buffer_drive != current_drive || /* bad drive */
fsector_t > buffer_max ||
fsector_t < buffer_min ||
- ((CT(COMMAND) == FD_READ ||
+ ((CT(raw_cmd->cmd[COMMAND]) == FD_READ ||
(!in_sector_offset && blk_rq_sectors(current_req) >= ssize)) &&
max_sector > 2 * max_buffer_sectors + buffer_min &&
max_size + fsector_t > 2 * max_buffer_sectors + buffer_min)) {
@@ -2726,7 +2727,7 @@ static int make_raw_rw_request(void)
raw_cmd->kernel_data = floppy_track_buffer +
((aligned_sector_t - buffer_min) << 9);
- if (CT(COMMAND) == FD_WRITE) {
+ if (CT(raw_cmd->cmd[COMMAND]) == FD_WRITE) {
/* copy write buffer to track buffer.
* if we get here, we know that the write
* is either aligned or the data already in the buffer
@@ -2748,10 +2749,10 @@ static int make_raw_rw_request(void)
raw_cmd->length <<= 9;
if ((raw_cmd->length < current_count_sectors << 9) ||
(raw_cmd->kernel_data != bio_data(current_req->bio) &&
- CT(COMMAND) == FD_WRITE &&
+ CT(raw_cmd->cmd[COMMAND]) == FD_WRITE &&
(aligned_sector_t + (raw_cmd->length >> 9) > buffer_max ||
aligned_sector_t < buffer_min)) ||
- raw_cmd->length % (128 << SIZECODE) ||
+ raw_cmd->length % (128 << raw_cmd->cmd[SIZECODE]) ||
raw_cmd->length <= 0 || current_count_sectors <= 0) {
DPRINT("fractionary current count b=%lx s=%lx\n",
raw_cmd->length, current_count_sectors);
@@ -2762,9 +2763,10 @@ static int make_raw_rw_request(void)
current_count_sectors);
pr_info("st=%d ast=%d mse=%d msi=%d\n",
fsector_t, aligned_sector_t, max_sector, max_size);
- pr_info("ssize=%x SIZECODE=%d\n", ssize, SIZECODE);
+ pr_info("ssize=%x SIZECODE=%d\n", ssize, raw_cmd->cmd[SIZECODE]);
pr_info("command=%x SECTOR=%d HEAD=%d, TRACK=%d\n",
- COMMAND, SECTOR, HEAD, TRACK);
+ raw_cmd->cmd[COMMAND], raw_cmd->cmd[SECTOR],
+ raw_cmd->cmd[HEAD], raw_cmd->cmd[TRACK]);
pr_info("buffer drive=%d\n", buffer_drive);
pr_info("buffer track=%d\n", buffer_track);
pr_info("buffer_min=%d\n", buffer_min);
@@ -2783,9 +2785,9 @@ static int make_raw_rw_request(void)
fsector_t, buffer_min, raw_cmd->length >> 9);
pr_info("current_count_sectors=%ld\n",
current_count_sectors);
- if (CT(COMMAND) == FD_READ)
+ if (CT(raw_cmd->cmd[COMMAND]) == FD_READ)
pr_info("read\n");
- if (CT(COMMAND) == FD_WRITE)
+ if (CT(raw_cmd->cmd[COMMAND]) == FD_WRITE)
pr_info("write\n");
return 0;
}
@@ -3253,7 +3255,7 @@ static int set_geometry(unsigned int cmd, struct floppy_struct *g,
(int)g->head <= 0 ||
/* check for overflow in max_sector */
(int)(g->sect * g->head) <= 0 ||
- /* check for zero in F_SECT_PER_TRACK */
+ /* check for zero in raw_cmd->cmd[F_SECT_PER_TRACK] */
(unsigned char)((g->sect << 2) >> FD_SIZECODE(g)) == 0 ||
g->track <= 0 || g->track > drive_params[drive].tracks >> STRETCH(g) ||
/* check if reserved bits are set */
--
2.9.0
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 10/10] floppy: cleanup: expand the reply_buffer macros
2020-02-24 21:23 [PATCH 00/10] floppy driver cleanups (deobfuscation) Willy Tarreau
` (8 preceding siblings ...)
2020-02-24 21:23 ` [PATCH 09/10] floppy: cleanup: expand the R/W / format command macros Willy Tarreau
@ 2020-02-24 21:23 ` Willy Tarreau
2020-02-26 8:07 ` [PATCH 11/16] floppy: remove dead code for drives scanning on ARM Willy Tarreau
2020-02-26 14:57 ` [PATCH 00/10] floppy driver cleanups (deobfuscation) Denis Efremov
11 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-24 21:23 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, linux-kernel, linux-block, Linus Torvalds, Willy Tarreau
Several macros were used to access reply_buffer[] at discrete positions
without making it obvious they were relying on this. These ones have
been replaced by their offset in the reply buffer to make these accesses
more obvious.
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
drivers/block/floppy.c | 86 +++++++++++++++++++++++++++-----------------------
1 file changed, 47 insertions(+), 39 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 0d53335..d521899 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -341,14 +341,14 @@ static bool initialized;
#define MAX_REPLIES 16
static unsigned char reply_buffer[MAX_REPLIES];
static int inr; /* size of reply buffer, when called from interrupt */
-#define ST0 (reply_buffer[0])
-#define ST1 (reply_buffer[1])
-#define ST2 (reply_buffer[2])
-#define ST3 (reply_buffer[0]) /* result of GETSTATUS */
-#define R_TRACK (reply_buffer[3])
-#define R_HEAD (reply_buffer[4])
-#define R_SECTOR (reply_buffer[5])
-#define R_SIZECODE (reply_buffer[6])
+#define ST0 0
+#define ST1 1
+#define ST2 2
+#define ST3 0 /* result of GETSTATUS */
+#define R_TRACK 3
+#define R_HEAD 4
+#define R_SECTOR 5
+#define R_SIZECODE 6
#define SEL_DLY (2 * HZ / 100)
@@ -1366,34 +1366,37 @@ static int fdc_dtr(void)
static void tell_sector(void)
{
pr_cont(": track %d, head %d, sector %d, size %d",
- R_TRACK, R_HEAD, R_SECTOR, R_SIZECODE);
+ reply_buffer[R_TRACK], reply_buffer[R_HEAD],
+ reply_buffer[R_SECTOR],
+ reply_buffer[R_SIZECODE]);
} /* tell_sector */
static void print_errors(void)
{
DPRINT("");
- if (ST0 & ST0_ECE) {
+ if (reply_buffer[ST0] & ST0_ECE) {
pr_cont("Recalibrate failed!");
- } else if (ST2 & ST2_CRC) {
+ } else if (reply_buffer[ST2] & ST2_CRC) {
pr_cont("data CRC error");
tell_sector();
- } else if (ST1 & ST1_CRC) {
+ } else if (reply_buffer[ST1] & ST1_CRC) {
pr_cont("CRC error");
tell_sector();
- } else if ((ST1 & (ST1_MAM | ST1_ND)) ||
- (ST2 & ST2_MAM)) {
+ } else if ((reply_buffer[ST1] & (ST1_MAM | ST1_ND)) ||
+ (reply_buffer[ST2] & ST2_MAM)) {
if (!probing) {
pr_cont("sector not found");
tell_sector();
} else
pr_cont("probe failed...");
- } else if (ST2 & ST2_WC) { /* seek error */
+ } else if (reply_buffer[ST2] & ST2_WC) { /* seek error */
pr_cont("wrong cylinder");
- } else if (ST2 & ST2_BC) { /* cylinder marked as bad */
+ } else if (reply_buffer[ST2] & ST2_BC) { /* cylinder marked as bad */
pr_cont("bad cylinder");
} else {
pr_cont("unknown error. ST[0..2] are: 0x%x 0x%x 0x%x",
- ST0, ST1, ST2);
+ reply_buffer[ST0], reply_buffer[ST1],
+ reply_buffer[ST2]);
tell_sector();
}
pr_cont("\n");
@@ -1417,28 +1420,28 @@ static int interpret_errors(void)
}
/* check IC to find cause of interrupt */
- switch (ST0 & ST0_INTR) {
+ switch (reply_buffer[ST0] & ST0_INTR) {
case 0x40: /* error occurred during command execution */
- if (ST1 & ST1_EOC)
+ if (reply_buffer[ST1] & ST1_EOC)
return 0; /* occurs with pseudo-DMA */
bad = 1;
- if (ST1 & ST1_WP) {
+ if (reply_buffer[ST1] & ST1_WP) {
DPRINT("Drive is write protected\n");
clear_bit(FD_DISK_WRITABLE_BIT,
&drive_state[current_drive].flags);
cont->done(0);
bad = 2;
- } else if (ST1 & ST1_ND) {
+ } else if (reply_buffer[ST1] & ST1_ND) {
set_bit(FD_NEED_TWADDLE_BIT,
&drive_state[current_drive].flags);
- } else if (ST1 & ST1_OR) {
+ } else if (reply_buffer[ST1] & ST1_OR) {
if (drive_params[current_drive].flags & FTD_MSG)
DPRINT("Over/Underrun - retrying\n");
bad = 0;
} else if (*errors >= drive_params[current_drive].max_errors.reporting) {
print_errors();
}
- if (ST2 & ST2_WC || ST2 & ST2_BC)
+ if (reply_buffer[ST2] & ST2_WC || reply_buffer[ST2] & ST2_BC)
/* wrong cylinder => recal */
drive_state[current_drive].track = NEED_2_RECAL;
return bad;
@@ -1522,14 +1525,16 @@ static int blind_seek;
static void seek_interrupt(void)
{
debugt(__func__, "");
- if (inr != 2 || (ST0 & 0xF8) != 0x20) {
+ if (inr != 2 || (reply_buffer[ST0] & 0xF8) != 0x20) {
DPRINT("seek failed\n");
drive_state[current_drive].track = NEED_2_RECAL;
cont->error();
cont->redo();
return;
}
- if (drive_state[current_drive].track >= 0 && drive_state[current_drive].track != ST1 && !blind_seek) {
+ if (drive_state[current_drive].track >= 0 &&
+ drive_state[current_drive].track != reply_buffer[ST1] &&
+ !blind_seek) {
debug_dcl(drive_params[current_drive].flags,
"clearing NEWCHANGE flag because of effective seek\n");
debug_dcl(drive_params[current_drive].flags, "jiffies=%lu\n",
@@ -1539,7 +1544,7 @@ static void seek_interrupt(void)
/* effective seek */
drive_state[current_drive].select_date = jiffies;
}
- drive_state[current_drive].track = ST1;
+ drive_state[current_drive].track = reply_buffer[ST1];
floppy_ready();
}
@@ -1559,8 +1564,8 @@ static void check_wp(void)
debug_dcl(drive_params[current_drive].flags,
"checking whether disk is write protected\n");
debug_dcl(drive_params[current_drive].flags, "wp=%x\n",
- ST3 & 0x40);
- if (!(ST3 & 0x40))
+ reply_buffer[ST3] & 0x40);
+ if (!(reply_buffer[ST3] & 0x40))
set_bit(FD_DISK_WRITABLE_BIT,
&drive_state[current_drive].flags);
else
@@ -1634,7 +1639,7 @@ static void recal_interrupt(void)
debugt(__func__, "");
if (inr != 2)
fdc_state[fdc].reset = 1;
- else if (ST0 & ST0_ECE) {
+ else if (reply_buffer[ST0] & ST0_ECE) {
switch (drive_state[current_drive].track) {
case NEED_1_RECAL:
debugt(__func__, "need 1 recal");
@@ -1672,7 +1677,7 @@ static void recal_interrupt(void)
break;
}
} else
- drive_state[current_drive].track = ST1;
+ drive_state[current_drive].track = reply_buffer[ST1];
floppy_ready();
}
@@ -1734,7 +1739,7 @@ irqreturn_t floppy_interrupt(int irq, void *dev_id)
if (do_print)
print_result("sensei", inr);
max_sensei--;
- } while ((ST0 & 0x83) != UNIT(current_drive) &&
+ } while ((reply_buffer[ST0] & 0x83) != UNIT(current_drive) &&
inr == 2 && max_sensei);
}
if (!handler) {
@@ -2292,7 +2297,7 @@ static void rw_interrupt(void)
int heads;
int nr_sectors;
- if (R_HEAD >= 2) {
+ if (reply_buffer[R_HEAD] >= 2) {
/* some Toshiba floppy controllers occasionnally seem to
* return bogus interrupts after read/write operations, which
* can be recognized by a bad head number (>= 2) */
@@ -2305,7 +2310,7 @@ static void rw_interrupt(void)
nr_sectors = 0;
ssize = DIV_ROUND_UP(1 << raw_cmd->cmd[SIZECODE], 4);
- if (ST1 & ST1_EOC)
+ if (reply_buffer[ST1] & ST1_EOC)
eoc = 1;
else
eoc = 0;
@@ -2315,17 +2320,20 @@ static void rw_interrupt(void)
else
heads = 1;
- nr_sectors = (((R_TRACK - raw_cmd->cmd[TRACK]) * heads +
- R_HEAD - raw_cmd->cmd[HEAD]) * raw_cmd->cmd[SECT_PER_TRACK] +
- R_SECTOR - raw_cmd->cmd[SECTOR] + eoc) << raw_cmd->cmd[SIZECODE] >> 2;
+ nr_sectors = (((reply_buffer[R_TRACK] - raw_cmd->cmd[TRACK]) * heads +
+ reply_buffer[R_HEAD] - raw_cmd->cmd[HEAD]) * raw_cmd->cmd[SECT_PER_TRACK] +
+ reply_buffer[R_SECTOR] - raw_cmd->cmd[SECTOR] + eoc) << raw_cmd->cmd[SIZECODE] >> 2;
if (nr_sectors / ssize >
DIV_ROUND_UP(in_sector_offset + current_count_sectors, ssize)) {
DPRINT("long rw: %x instead of %lx\n",
nr_sectors, current_count_sectors);
- pr_info("rs=%d s=%d\n", R_SECTOR, raw_cmd->cmd[SECTOR]);
- pr_info("rh=%d h=%d\n", R_HEAD, raw_cmd->cmd[HEAD]);
- pr_info("rt=%d t=%d\n", R_TRACK, raw_cmd->cmd[TRACK]);
+ pr_info("rs=%d s=%d\n", reply_buffer[R_SECTOR],
+ raw_cmd->cmd[SECTOR]);
+ pr_info("rh=%d h=%d\n", reply_buffer[R_HEAD],
+ raw_cmd->cmd[HEAD]);
+ pr_info("rt=%d t=%d\n", reply_buffer[R_TRACK],
+ raw_cmd->cmd[TRACK]);
pr_info("heads=%d eoc=%d\n", heads, eoc);
pr_info("spt=%d st=%d ss=%d\n",
raw_cmd->cmd[SECT_PER_TRACK], fsector_t, ssize);
--
2.9.0
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-24 21:23 ` [PATCH 01/10] floppy: cleanup: expand macro FDCS Willy Tarreau
@ 2020-02-24 21:53 ` Linus Torvalds
2020-02-24 23:13 ` Denis Efremov
2020-02-25 11:37 ` Denis Efremov
1 sibling, 1 reply; 45+ messages in thread
From: Linus Torvalds @ 2020-02-24 21:53 UTC (permalink / raw)
To: Willy Tarreau
Cc: Denis Efremov, Jens Axboe, Linux Kernel Mailing List, linux-block
On Mon, Feb 24, 2020 at 1:24 PM Willy Tarreau <w@1wt.eu> wrote:
>
> Macro FDCS silently uses identifier "fdc" which may be either the
> global one or a local one. Let's expand the macro to make this more
> obvious.
Hmm. These macro expansions feel wrong to me.
Or rather, they look right as a first step - and it's probably worth
doing this just to have that "exact same code generation" step.
But I think there should be a second step (also with "exact same code
generation") which then renames the driver-global "fdc" index as
"current_fdc".
That way you'll _really_ see when you use the global vs local ones.
The local ones would continue to be just "fdc".
Because with just this patch, I don't think you actually get any more
obvious whether it's the global or local "fdc" index that is used.
So I'd like to see that second step that does the
-static int fdc; /* current fdc */
+static int current_fdc;
change.
We already call the global 'drive' variable 'current_drive', so it
really is 'fdc' that is misnamed and ambiguous because it then has two
different cases: the global 'fdc' and then the various shadowing local
'fdc' variables (or function arguments).
Mind adding that too? Slightly less automatic, I agree, because then
you really do have to disambiguate between the "is this the shadowed
use of a local 'fdc'" case or the "this is the global 'fdc' use" case.
Can coccinelle do that?
Linus
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-24 21:53 ` Linus Torvalds
@ 2020-02-24 23:13 ` Denis Efremov
2020-02-25 3:45 ` Willy Tarreau
0 siblings, 1 reply; 45+ messages in thread
From: Denis Efremov @ 2020-02-24 23:13 UTC (permalink / raw)
To: Linus Torvalds, Willy Tarreau
Cc: Jens Axboe, Linux Kernel Mailing List, linux-block
Hi,
On 2/25/20 12:53 AM, Linus Torvalds wrote:
> So I'd like to see that second step that does the
>
> -static int fdc; /* current fdc */
> +static int current_fdc;
>
> change.
>
> We already call the global 'drive' variable 'current_drive', so it
> really is 'fdc' that is misnamed and ambiguous because it then has two
> different cases: the global 'fdc' and then the various shadowing local
> 'fdc' variables (or function arguments).
>
> Mind adding that too? Slightly less automatic, I agree, because then
> you really do have to disambiguate between the "is this the shadowed
> use of a local 'fdc'" case or the "this is the global 'fdc' use" case.
>
> Can coccinelle do that?
Willy, if you don't want to spend your time with this code anymore I can
prepare patсhes for the second step. I know coccinelle and could try
to automate this transformation. At first sight your patches look
good to me. I will answer to the top email after more accurate review.
Thanks,
Denis
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-24 23:13 ` Denis Efremov
@ 2020-02-25 3:45 ` Willy Tarreau
2020-02-25 7:14 ` Denis Efremov
0 siblings, 1 reply; 45+ messages in thread
From: Willy Tarreau @ 2020-02-25 3:45 UTC (permalink / raw)
To: Denis Efremov
Cc: Linus Torvalds, Jens Axboe, Linux Kernel Mailing List, linux-block
On Tue, Feb 25, 2020 at 02:13:42AM +0300, Denis Efremov wrote:
> On 2/25/20 12:53 AM, Linus Torvalds wrote:
> > So I'd like to see that second step that does the
> >
> > -static int fdc; /* current fdc */
> > +static int current_fdc;
> >
> > change.
> >
> > We already call the global 'drive' variable 'current_drive', so it
> > really is 'fdc' that is misnamed and ambiguous because it then has two
> > different cases: the global 'fdc' and then the various shadowing local
> > 'fdc' variables (or function arguments).
> >
> > Mind adding that too? Slightly less automatic, I agree, because then
> > you really do have to disambiguate between the "is this the shadowed
> > use of a local 'fdc'" case or the "this is the global 'fdc' use" case.
I definitely agree. I first wanted to be sure the patches were acceptable
as a principle, but disambiguating the variables is easy to do now.
> > Can coccinelle do that?
I could do it by hand, I did quite a bit of manual changes and checks
already and the driver is not that long.
> Willy, if you don't want to spend your time with this code anymore I can
> prepare pat?hes for the second step. I know coccinelle and could try
> to automate this transformation. At first sight your patches look
> good to me. I will answer to the top email after more accurate review.
OK, it's as you like. If you think you can do the change quickly, feel
free to do so, otherwise it should not take me more than one hour. In
any case as previously mentioned I still have the hardware in a usable
state if you want me to recheck anything.
Cheers,
Willy
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-25 3:45 ` Willy Tarreau
@ 2020-02-25 7:14 ` Denis Efremov
2020-02-25 14:02 ` Willy Tarreau
0 siblings, 1 reply; 45+ messages in thread
From: Denis Efremov @ 2020-02-25 7:14 UTC (permalink / raw)
To: Willy Tarreau
Cc: Jens Axboe, Linux Kernel Mailing List, linux-block, Linus Torvalds
On 2/25/20 6:45 AM, Willy Tarreau wrote:
> On Tue, Feb 25, 2020 at 02:13:42AM +0300, Denis Efremov wrote:
>> On 2/25/20 12:53 AM, Linus Torvalds wrote:
>>> So I'd like to see that second step that does the
>>>
>>> -static int fdc; /* current fdc */
>>> +static int current_fdc;
>>>
>>> change.
>>>
>>> We already call the global 'drive' variable 'current_drive', so it
>>> really is 'fdc' that is misnamed and ambiguous because it then has two
>>> different cases: the global 'fdc' and then the various shadowing local
>>> 'fdc' variables (or function arguments).
>>>
>>> Mind adding that too? Slightly less automatic, I agree, because then
>>> you really do have to disambiguate between the "is this the shadowed
>>> use of a local 'fdc'" case or the "this is the global 'fdc' use" case.
>
> I definitely agree. I first wanted to be sure the patches were acceptable
> as a principle, but disambiguating the variables is easy to do now.
Ok, I don't want to break in the middle of your changes in this case.
>
>>> Can coccinelle do that?
>
> I could do it by hand, I did quite a bit of manual changes and checks
> already and the driver is not that long.
>
>> Willy, if you don't want to spend your time with this code anymore I can
>> prepare pat?hes for the second step. I know coccinelle and could try
>> to automate this transformation. At first sight your patches look
>> good to me. I will answer to the top email after more accurate review.
>
> OK, it's as you like. If you think you can do the change quickly, feel
> free to do so, otherwise it should not take me more than one hour. In
> any case as previously mentioned I still have the hardware in a usable
> state if you want me to recheck anything.
>
I also have working hardware to test your changes with the previous patch.
However, double check is always welcome if you've got time for that. Please,
send patches on top of these ones.
Thanks,
Denis
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-24 21:23 ` [PATCH 01/10] floppy: cleanup: expand macro FDCS Willy Tarreau
2020-02-24 21:53 ` Linus Torvalds
@ 2020-02-25 11:37 ` Denis Efremov
1 sibling, 0 replies; 45+ messages in thread
From: Denis Efremov @ 2020-02-25 11:37 UTC (permalink / raw)
To: Willy Tarreau; +Cc: Jens Axboe, linux-kernel, linux-block, Linus Torvalds
On 2/25/20 12:23 AM, Willy Tarreau wrote:
> Macro FDCS silently uses identifier "fdc" which may be either the
> global one or a local one. Let's expand the macro to make this more
> obvious.
This patch looks good to me. Just want to leave a note here that
FD_IOPORT macro from include/uapi/linux/fdreg.h also silently uses fdc
global var: #define FD_IOPORT fdc_state[fdc].address
Thanks,
Denis
>
> Signed-off-by: Willy Tarreau <w@1wt.eu>
> ---
> drivers/block/floppy.c | 183 ++++++++++++++++++++++++-------------------------
> 1 file changed, 91 insertions(+), 92 deletions(-)
>
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index 8ef65c0..93e0840 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -309,7 +309,6 @@ static bool initialized;
> #define DP (&drive_params[current_drive])
> #define DRS (&drive_state[current_drive])
> #define DRWE (&write_errors[current_drive])
> -#define FDCS (&fdc_state[fdc])
>
> #define UDP (&drive_params[drive])
> #define UDRS (&drive_state[drive])
> @@ -742,11 +741,11 @@ static int disk_change(int drive)
>
> if (time_before(jiffies, UDRS->select_date + UDP->select_delay))
> DPRINT("WARNING disk change called early\n");
> - if (!(FDCS->dor & (0x10 << UNIT(drive))) ||
> - (FDCS->dor & 3) != UNIT(drive) || fdc != FDC(drive)) {
> + if (!(fdc_state[fdc].dor & (0x10 << UNIT(drive))) ||
> + (fdc_state[fdc].dor & 3) != UNIT(drive) || fdc != FDC(drive)) {
> DPRINT("probing disk change on unselected drive\n");
> DPRINT("drive=%d fdc=%d dor=%x\n", drive, FDC(drive),
> - (unsigned int)FDCS->dor);
> + (unsigned int)fdc_state[fdc].dor);
> }
>
> debug_dcl(UDP->flags,
> @@ -799,10 +798,10 @@ static int set_dor(int fdc, char mask, char data)
> unsigned char newdor;
> unsigned char olddor;
>
> - if (FDCS->address == -1)
> + if (fdc_state[fdc].address == -1)
> return -1;
>
> - olddor = FDCS->dor;
> + olddor = fdc_state[fdc].dor;
> newdor = (olddor & mask) | data;
> if (newdor != olddor) {
> unit = olddor & 0x3;
> @@ -812,7 +811,7 @@ static int set_dor(int fdc, char mask, char data)
> "calling disk change from set_dor\n");
> disk_change(drive);
> }
> - FDCS->dor = newdor;
> + fdc_state[fdc].dor = newdor;
> fd_outb(newdor, FD_DOR);
>
> unit = newdor & 0x3;
> @@ -828,8 +827,8 @@ static void twaddle(void)
> {
> if (DP->select_delay)
> return;
> - fd_outb(FDCS->dor & ~(0x10 << UNIT(current_drive)), FD_DOR);
> - fd_outb(FDCS->dor, FD_DOR);
> + fd_outb(fdc_state[fdc].dor & ~(0x10 << UNIT(current_drive)), FD_DOR);
> + fd_outb(fdc_state[fdc].dor, FD_DOR);
> DRS->select_date = jiffies;
> }
>
> @@ -841,10 +840,10 @@ static void reset_fdc_info(int mode)
> {
> int drive;
>
> - FDCS->spec1 = FDCS->spec2 = -1;
> - FDCS->need_configure = 1;
> - FDCS->perp_mode = 1;
> - FDCS->rawcmd = 0;
> + 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;
> for (drive = 0; drive < N_DRIVE; drive++)
> if (FDC(drive) == fdc && (mode || UDRS->track != NEED_1_RECAL))
> UDRS->track = NEED_2_RECAL;
> @@ -868,10 +867,10 @@ static void set_fdc(int drive)
> #if N_FDC > 1
> set_dor(1 - fdc, ~8, 0);
> #endif
> - if (FDCS->rawcmd == 2)
> + if (fdc_state[fdc].rawcmd == 2)
> reset_fdc_info(1);
> if (fd_inb(FD_STATUS) != STATUS_READY)
> - FDCS->reset = 1;
> + fdc_state[fdc].reset = 1;
> }
>
> /* locks the driver */
> @@ -924,7 +923,7 @@ static void floppy_off(unsigned int drive)
> unsigned long volatile delta;
> int fdc = FDC(drive);
>
> - if (!(FDCS->dor & (0x10 << UNIT(drive))))
> + if (!(fdc_state[fdc].dor & (0x10 << UNIT(drive))))
> return;
>
> del_timer(motor_off_timer + drive);
> @@ -1035,7 +1034,7 @@ static void main_command_interrupt(void)
> static int fd_wait_for_completion(unsigned long expires,
> void (*function)(void))
> {
> - if (FDCS->reset) {
> + if (fdc_state[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 */
> @@ -1063,13 +1062,13 @@ static void setup_DMA(void)
> pr_cont("%x,", raw_cmd->cmd[i]);
> pr_cont("\n");
> cont->done(0);
> - FDCS->reset = 1;
> + fdc_state[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);
> - FDCS->reset = 1;
> + fdc_state[fdc].reset = 1;
> return;
> }
> f = claim_dma_lock();
> @@ -1077,10 +1076,10 @@ 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, FDCS->address) < 0) {
> + DMA_MODE_READ : DMA_MODE_WRITE, fdc_state[fdc].address) < 0) {
> release_dma_lock(f);
> cont->done(0);
> - FDCS->reset = 1;
> + fdc_state[fdc].reset = 1;
> return;
> }
> release_dma_lock(f);
> @@ -1091,7 +1090,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 = FDCS->address;
> + virtual_dma_port = fdc_state[fdc].address;
> fd_enable_dma();
> release_dma_lock(f);
> #endif
> @@ -1105,7 +1104,7 @@ static int wait_til_ready(void)
> int status;
> int counter;
>
> - if (FDCS->reset)
> + if (fdc_state[fdc].reset)
> return -1;
> for (counter = 0; counter < 10000; counter++) {
> status = fd_inb(FD_STATUS);
> @@ -1116,7 +1115,7 @@ static int wait_til_ready(void)
> DPRINT("Getstatus times out (%x) on fdc %d\n", status, fdc);
> show_floppy();
> }
> - FDCS->reset = 1;
> + fdc_state[fdc].reset = 1;
> return -1;
> }
>
> @@ -1136,7 +1135,7 @@ static int output_byte(char byte)
> output_log_pos = (output_log_pos + 1) % OLOGSIZE;
> return 0;
> }
> - FDCS->reset = 1;
> + fdc_state[fdc].reset = 1;
> if (initialized) {
> DPRINT("Unable to send byte %x to FDC. Fdc=%x Status=%x\n",
> byte, fdc, status);
> @@ -1171,7 +1170,7 @@ static int result(void)
> fdc, status, i);
> show_floppy();
> }
> - FDCS->reset = 1;
> + fdc_state[fdc].reset = 1;
> return -1;
> }
>
> @@ -1208,7 +1207,7 @@ static void perpendicular_mode(void)
> default:
> DPRINT("Invalid data rate for perpendicular mode!\n");
> cont->done(0);
> - FDCS->reset = 1;
> + fdc_state[fdc].reset = 1;
> /*
> * convenient way to return to
> * redo without too much hassle
> @@ -1219,12 +1218,12 @@ static void perpendicular_mode(void)
> } else
> perp_mode = 0;
>
> - if (FDCS->perp_mode == perp_mode)
> + if (fdc_state[fdc].perp_mode == perp_mode)
> return;
> - if (FDCS->version >= FDC_82077_ORIG) {
> + if (fdc_state[fdc].version >= FDC_82077_ORIG) {
> output_byte(FD_PERPENDICULAR);
> output_byte(perp_mode);
> - FDCS->perp_mode = perp_mode;
> + fdc_state[fdc].perp_mode = perp_mode;
> } else if (perp_mode) {
> DPRINT("perpendicular mode not supported by this FDC.\n");
> }
> @@ -1279,9 +1278,9 @@ static void fdc_specify(void)
> int hlt_max_code = 0x7f;
> int hut_max_code = 0xf;
>
> - if (FDCS->need_configure && FDCS->version >= FDC_82072A) {
> + if (fdc_state[fdc].need_configure && fdc_state[fdc].version >= FDC_82072A) {
> fdc_configure();
> - FDCS->need_configure = 0;
> + fdc_state[fdc].need_configure = 0;
> }
>
> switch (raw_cmd->rate & 0x03) {
> @@ -1290,7 +1289,7 @@ static void fdc_specify(void)
> break;
> case 1:
> dtr = 300;
> - if (FDCS->version >= FDC_82078) {
> + if (fdc_state[fdc].version >= FDC_82078) {
> /* chose the default rate table, not the one
> * where 1 = 2 Mbps */
> output_byte(FD_DRIVESPEC);
> @@ -1305,7 +1304,7 @@ static void fdc_specify(void)
> break;
> }
>
> - if (FDCS->version >= FDC_82072) {
> + if (fdc_state[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!) */
> @@ -1335,11 +1334,11 @@ static void fdc_specify(void)
> spec2 = (hlt << 1) | (use_virtual_dma & 1);
>
> /* If these parameters did not change, just return with success */
> - if (FDCS->spec1 != spec1 || FDCS->spec2 != spec2) {
> + if (fdc_state[fdc].spec1 != spec1 || fdc_state[fdc].spec2 != spec2) {
> /* Go ahead and set spec1 and spec2 */
> output_byte(FD_SPECIFY);
> - output_byte(FDCS->spec1 = spec1);
> - output_byte(FDCS->spec2 = spec2);
> + output_byte(fdc_state[fdc].spec1 = spec1);
> + output_byte(fdc_state[fdc].spec2 = spec2);
> }
> } /* fdc_specify */
>
> @@ -1350,7 +1349,7 @@ 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) == FDCS->dtr)
> + if ((raw_cmd->rate & 3) == fdc_state[fdc].dtr)
> return 0;
>
> /* Set dtr */
> @@ -1361,7 +1360,7 @@ static int fdc_dtr(void)
> * enforced after data rate changes before R/W operations.
> * Pause 5 msec to avoid trouble. (Needs to be 2 jiffies)
> */
> - FDCS->dtr = raw_cmd->rate & 3;
> + fdc_state[fdc].dtr = raw_cmd->rate & 3;
> return fd_wait_for_completion(jiffies + 2UL * HZ / 100, floppy_ready);
> } /* fdc_dtr */
>
> @@ -1414,7 +1413,7 @@ static int interpret_errors(void)
>
> if (inr != 7) {
> DPRINT("-- FDC reply error\n");
> - FDCS->reset = 1;
> + fdc_state[fdc].reset = 1;
> return 1;
> }
>
> @@ -1548,7 +1547,7 @@ static void check_wp(void)
> output_byte(FD_GETSTATUS);
> output_byte(UNIT(current_drive));
> if (result() != 1) {
> - FDCS->reset = 1;
> + fdc_state[fdc].reset = 1;
> return;
> }
> clear_bit(FD_VERIFY_BIT, &DRS->flags);
> @@ -1625,7 +1624,7 @@ static void recal_interrupt(void)
> {
> debugt(__func__, "");
> if (inr != 2)
> - FDCS->reset = 1;
> + fdc_state[fdc].reset = 1;
> else if (ST0 & ST0_ECE) {
> switch (DRS->track) {
> case NEED_1_RECAL:
> @@ -1693,7 +1692,7 @@ irqreturn_t floppy_interrupt(int irq, void *dev_id)
> release_dma_lock(f);
>
> do_floppy = NULL;
> - if (fdc >= N_FDC || FDCS->address == -1) {
> + if (fdc >= N_FDC || fdc_state[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);
> @@ -1702,11 +1701,11 @@ irqreturn_t floppy_interrupt(int irq, void *dev_id)
> return IRQ_NONE;
> }
>
> - FDCS->reset = 0;
> + fdc_state[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 FDCS->reset blocks the
> - * emission of the SENSEI's.
> + * emit SENSEI's to clear the interrupt line. And fdc_state[fdc].reset
> + * blocks the emission of the SENSEI's.
> * It is OK to emit floppy commands because we are in an interrupt
> * handler here, and thus we have to fear no interference of other
> * activity.
> @@ -1729,7 +1728,7 @@ irqreturn_t floppy_interrupt(int irq, void *dev_id)
> inr == 2 && max_sensei);
> }
> if (!handler) {
> - FDCS->reset = 1;
> + fdc_state[fdc].reset = 1;
> return IRQ_NONE;
> }
> schedule_bh(handler);
> @@ -1755,7 +1754,7 @@ static void reset_interrupt(void)
> {
> debugt(__func__, "");
> result(); /* get the status ready for set_fdc */
> - if (FDCS->reset) {
> + if (fdc_state[fdc].reset) {
> pr_info("reset set in interrupt, calling %ps\n", cont->error);
> cont->error(); /* a reset just after a reset. BAD! */
> }
> @@ -1771,7 +1770,7 @@ static void reset_fdc(void)
> unsigned long flags;
>
> do_floppy = reset_interrupt;
> - FDCS->reset = 0;
> + fdc_state[fdc].reset = 0;
> reset_fdc_info(0);
>
> /* Pseudo-DMA may intercept 'reset finished' interrupt. */
> @@ -1781,12 +1780,12 @@ static void reset_fdc(void)
> fd_disable_dma();
> release_dma_lock(flags);
>
> - if (FDCS->version >= FDC_82072A)
> - fd_outb(0x80 | (FDCS->dtr & 3), FD_STATUS);
> + if (fdc_state[fdc].version >= FDC_82072A)
> + fd_outb(0x80 | (fdc_state[fdc].dtr & 3), FD_STATUS);
> else {
> - fd_outb(FDCS->dor & ~0x04, FD_DOR);
> + fd_outb(fdc_state[fdc].dor & ~0x04, FD_DOR);
> udelay(FD_RESET_DELAY);
> - fd_outb(FDCS->dor, FD_DOR);
> + fd_outb(fdc_state[fdc].dor, FD_DOR);
> }
> }
>
> @@ -1850,7 +1849,7 @@ static void floppy_shutdown(struct work_struct *arg)
>
> if (initialized)
> DPRINT("floppy timeout called\n");
> - FDCS->reset = 1;
> + fdc_state[fdc].reset = 1;
> if (cont) {
> cont->done(0);
> cont->redo(); /* this will recall reset when needed */
> @@ -1870,7 +1869,7 @@ static int start_motor(void (*function)(void))
> mask = 0xfc;
> data = UNIT(current_drive);
> if (!(raw_cmd->flags & FD_RAW_NO_MOTOR)) {
> - if (!(FDCS->dor & (0x10 << UNIT(current_drive)))) {
> + if (!(fdc_state[fdc].dor & (0x10 << UNIT(current_drive)))) {
> set_debugt();
> /* no read since this drive is running */
> DRS->first_read_date = 0;
> @@ -1878,7 +1877,7 @@ static int start_motor(void (*function)(void))
> DRS->spinup_date = jiffies;
> data |= (0x10 << UNIT(current_drive));
> }
> - } else if (FDCS->dor & (0x10 << UNIT(current_drive)))
> + } else if (fdc_state[fdc].dor & (0x10 << UNIT(current_drive)))
> mask &= ~(0x10 << UNIT(current_drive));
>
> /* starts motor and selects floppy */
> @@ -1892,7 +1891,7 @@ static int start_motor(void (*function)(void))
>
> static void floppy_ready(void)
> {
> - if (FDCS->reset) {
> + if (fdc_state[fdc].reset) {
> reset_fdc();
> return;
> }
> @@ -1991,7 +1990,7 @@ static int wait_til_done(void (*handler)(void), bool interruptible)
> return -EINTR;
> }
>
> - if (FDCS->reset)
> + if (fdc_state[fdc].reset)
> command_status = FD_COMMAND_ERROR;
> if (command_status == FD_COMMAND_OKAY)
> ret = 0;
> @@ -2060,7 +2059,7 @@ static void bad_flp_intr(void)
> if (err_count > DP->max_errors.abort)
> cont->done(0);
> if (err_count > DP->max_errors.reset)
> - FDCS->reset = 1;
> + fdc_state[fdc].reset = 1;
> else if (err_count > DP->max_errors.recal)
> DRS->track = NEED_2_RECAL;
> }
> @@ -2967,8 +2966,8 @@ static int user_reset_fdc(int drive, int arg, bool interruptible)
> return -EINTR;
>
> if (arg == FD_RESET_ALWAYS)
> - FDCS->reset = 1;
> - if (FDCS->reset) {
> + fdc_state[fdc].reset = 1;
> + if (fdc_state[fdc].reset) {
> cont = &reset_cont;
> ret = wait_til_done(reset_fdc, interruptible);
> if (ret == -EINTR)
> @@ -3179,23 +3178,23 @@ static int raw_cmd_ioctl(int cmd, void __user *param)
> int ret2;
> int ret;
>
> - if (FDCS->rawcmd <= 1)
> - FDCS->rawcmd = 1;
> + if (fdc_state[fdc].rawcmd <= 1)
> + fdc_state[fdc].rawcmd = 1;
> for (drive = 0; drive < N_DRIVE; drive++) {
> if (FDC(drive) != fdc)
> continue;
> if (drive == current_drive) {
> if (UDRS->fd_ref > 1) {
> - FDCS->rawcmd = 2;
> + fdc_state[fdc].rawcmd = 2;
> break;
> }
> } else if (UDRS->fd_ref) {
> - FDCS->rawcmd = 2;
> + fdc_state[fdc].rawcmd = 2;
> break;
> }
> }
>
> - if (FDCS->reset)
> + if (fdc_state[fdc].reset)
> return -EIO;
>
> ret = raw_cmd_copyin(cmd, param, &my_raw_cmd);
> @@ -3209,7 +3208,7 @@ static int raw_cmd_ioctl(int cmd, void __user *param)
> ret = wait_til_done(floppy_start, true);
> debug_dcl(DP->flags, "calling disk change from raw_cmd ioctl\n");
>
> - if (ret != -EINTR && FDCS->reset)
> + if (ret != -EINTR && fdc_state[fdc].reset)
> ret = -EIO;
>
> DRS->track = NO_TRACK;
> @@ -4261,7 +4260,7 @@ static char __init get_fdc_version(void)
> int r;
>
> output_byte(FD_DUMPREGS); /* 82072 and better know DUMPREGS */
> - if (FDCS->reset)
> + if (fdc_state[fdc].reset)
> return FDC_NONE;
> r = result();
> if (r <= 0x00)
> @@ -4494,7 +4493,7 @@ static int floppy_resume(struct device *dev)
> int fdc;
>
> for (fdc = 0; fdc < N_FDC; fdc++)
> - if (FDCS->address != -1)
> + if (fdc_state[fdc].address != -1)
> user_reset_fdc(-1, FD_RESET_ALWAYS, false);
>
> return 0;
> @@ -4605,15 +4604,15 @@ static int __init do_floppy_init(void)
>
> for (i = 0; i < N_FDC; i++) {
> fdc = i;
> - memset(FDCS, 0, sizeof(*FDCS));
> - FDCS->dtr = -1;
> - FDCS->dor = 0x4;
> + memset(&fdc_state[fdc], 0, sizeof(*fdc_state));
> + fdc_state[fdc].dtr = -1;
> + fdc_state[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
> - FDCS->version = FDC_82072A;
> + fdc_state[fdc].version = FDC_82072A;
> #endif
> }
>
> @@ -4656,28 +4655,28 @@ static int __init do_floppy_init(void)
>
> for (i = 0; i < N_FDC; i++) {
> fdc = i;
> - FDCS->driver_version = FD_DRIVER_VERSION;
> + fdc_state[fdc].driver_version = FD_DRIVER_VERSION;
> for (unit = 0; unit < 4; unit++)
> - FDCS->track[unit] = 0;
> - if (FDCS->address == -1)
> + fdc_state[fdc].track[unit] = 0;
> + if (fdc_state[fdc].address == -1)
> continue;
> - FDCS->rawcmd = 2;
> + fdc_state[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);
> - FDCS->address = -1;
> - FDCS->version = FDC_NONE;
> + fdc_state[fdc].address = -1;
> + fdc_state[fdc].version = FDC_NONE;
> continue;
> }
> /* Try to determine the floppy controller type */
> - FDCS->version = get_fdc_version();
> - if (FDCS->version == FDC_NONE) {
> + fdc_state[fdc].version = get_fdc_version();
> + if (fdc_state[fdc].version == FDC_NONE) {
> /* free ioports reserved by floppy_grab_irq_and_dma() */
> floppy_release_regions(fdc);
> - FDCS->address = -1;
> + fdc_state[fdc].address = -1;
> continue;
> }
> - if (can_use_virtual_dma == 2 && FDCS->version < FDC_82072A)
> + if (can_use_virtual_dma == 2 && fdc_state[fdc].version < FDC_82072A)
> can_use_virtual_dma = 0;
>
> have_no_fdc = 0;
> @@ -4783,7 +4782,7 @@ static void floppy_release_allocated_regions(int fdc, const struct io_region *p)
> {
> while (p != io_regions) {
> p--;
> - release_region(FDCS->address + p->offset, p->size);
> + release_region(fdc_state[fdc].address + p->offset, p->size);
> }
> }
>
> @@ -4794,10 +4793,10 @@ static int floppy_request_regions(int fdc)
> const struct io_region *p;
>
> for (p = io_regions; p < ARRAY_END(io_regions); p++) {
> - if (!request_region(FDCS->address + p->offset,
> + if (!request_region(fdc_state[fdc].address + p->offset,
> p->size, "floppy")) {
> DPRINT("Floppy io-port 0x%04lx in use\n",
> - FDCS->address + p->offset);
> + fdc_state[fdc].address + p->offset);
> floppy_release_allocated_regions(fdc, p);
> return -EBUSY;
> }
> @@ -4840,23 +4839,23 @@ static int floppy_grab_irq_and_dma(void)
> }
>
> for (fdc = 0; fdc < N_FDC; fdc++) {
> - if (FDCS->address != -1) {
> + if (fdc_state[fdc].address != -1) {
> if (floppy_request_regions(fdc))
> goto cleanup;
> }
> }
> for (fdc = 0; fdc < N_FDC; fdc++) {
> - if (FDCS->address != -1) {
> + if (fdc_state[fdc].address != -1) {
> reset_fdc_info(1);
> - fd_outb(FDCS->dor, FD_DOR);
> + fd_outb(fdc_state[fdc].dor, FD_DOR);
> }
> }
> fdc = 0;
> set_dor(0, ~0, 8); /* avoid immediate interrupt */
>
> for (fdc = 0; fdc < N_FDC; fdc++)
> - if (FDCS->address != -1)
> - fd_outb(FDCS->dor, FD_DOR);
> + if (fdc_state[fdc].address != -1)
> + fd_outb(fdc_state[fdc].dor, FD_DOR);
> /*
> * The driver will try and free resources and relies on us
> * to know if they were allocated or not.
> @@ -4918,7 +4917,7 @@ static void floppy_release_irq_and_dma(void)
> pr_info("work still pending\n");
> old_fdc = fdc;
> for (fdc = 0; fdc < N_FDC; fdc++)
> - if (FDCS->address != -1)
> + if (fdc_state[fdc].address != -1)
> floppy_release_regions(fdc);
> fdc = old_fdc;
> }
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-25 7:14 ` Denis Efremov
@ 2020-02-25 14:02 ` Willy Tarreau
2020-02-25 15:22 ` Denis Efremov
0 siblings, 1 reply; 45+ messages in thread
From: Willy Tarreau @ 2020-02-25 14:02 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, Linux Kernel Mailing List, linux-block, Linus Torvalds
On Tue, Feb 25, 2020 at 10:14:40AM +0300, Denis Efremov wrote:
>
>
> On 2/25/20 6:45 AM, Willy Tarreau wrote:
> > On Tue, Feb 25, 2020 at 02:13:42AM +0300, Denis Efremov wrote:
> >> On 2/25/20 12:53 AM, Linus Torvalds wrote:
> >>> So I'd like to see that second step that does the
> >>>
> >>> -static int fdc; /* current fdc */
> >>> +static int current_fdc;
> >>>
> >>> change.
> >>>
> >>> We already call the global 'drive' variable 'current_drive', so it
> >>> really is 'fdc' that is misnamed and ambiguous because it then has two
> >>> different cases: the global 'fdc' and then the various shadowing local
> >>> 'fdc' variables (or function arguments).
> >>>
> >>> Mind adding that too? Slightly less automatic, I agree, because then
> >>> you really do have to disambiguate between the "is this the shadowed
> >>> use of a local 'fdc'" case or the "this is the global 'fdc' use" case.
> >
> > I definitely agree. I first wanted to be sure the patches were acceptable
> > as a principle, but disambiguating the variables is easy to do now.
>
> Ok, I don't want to break in the middle of your changes in this case.
So I started this and discovered the nice joke you were telling me
about regarding FD_IOPORT which references fdc. Then the address
registers FD_STATUS, FD_DATA, FD_DOR, FD_DIR, FD_DCR which are
based on FD_IOPORT also depend on it.
These ones are used by fd_outb() which is arch-dependent, so if we
want to pass a third argument we have to change them all and make sure
not to break them too much.
In addition the FD_* macros defined above are used by x86, and FD_DOR is
also used by arm while all other archs hard-code all the values. ARM also
uses floppy_selects[fdc] and new_dor... I'm starting to feel the trap here!
I also feel a bit concerned that these are exported in uapi with a hard-coded
0x3f0 base address. I'm just not sure how portable all of this is in
the end :-/
Now I'm wondering, how far should we go and how much is it acceptable to
change ? I'd rather not have "#define fdc current_fdc" just so that it
builds, but on the other hand this problem clearly outlights the roots
of the issue, which lies in "fdc" being silently accessed by macros with
nobody noticing!
Willy
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-25 14:02 ` Willy Tarreau
@ 2020-02-25 15:22 ` Denis Efremov
2020-02-25 15:39 ` Denis Efremov
` (2 more replies)
0 siblings, 3 replies; 45+ messages in thread
From: Denis Efremov @ 2020-02-25 15:22 UTC (permalink / raw)
To: Willy Tarreau
Cc: Jens Axboe, Linux Kernel Mailing List, linux-block, Linus Torvalds
On 2/25/20 5:02 PM, Willy Tarreau wrote:
> On Tue, Feb 25, 2020 at 10:14:40AM +0300, Denis Efremov wrote:
>>
>>
>> On 2/25/20 6:45 AM, Willy Tarreau wrote:
>>> On Tue, Feb 25, 2020 at 02:13:42AM +0300, Denis Efremov wrote:
>>>> On 2/25/20 12:53 AM, Linus Torvalds wrote:
>>>>> So I'd like to see that second step that does the
>>>>>
>>>>> -static int fdc; /* current fdc */
>>>>> +static int current_fdc;
>>>>>
>>>>> change.
>>>>>
>>>>> We already call the global 'drive' variable 'current_drive', so it
>>>>> really is 'fdc' that is misnamed and ambiguous because it then has two
>>>>> different cases: the global 'fdc' and then the various shadowing local
>>>>> 'fdc' variables (or function arguments).
>>>>>
>>>>> Mind adding that too? Slightly less automatic, I agree, because then
>>>>> you really do have to disambiguate between the "is this the shadowed
>>>>> use of a local 'fdc'" case or the "this is the global 'fdc' use" case.
>>>
>>> I definitely agree. I first wanted to be sure the patches were acceptable
>>> as a principle, but disambiguating the variables is easy to do now.
>>
>> Ok, I don't want to break in the middle of your changes in this case.
>
> So I started this and discovered the nice joke you were telling me
> about regarding FD_IOPORT which references fdc. Then the address
> registers FD_STATUS, FD_DATA, FD_DOR, FD_DIR, FD_DCR which are
> based on FD_IOPORT also depend on it.
>
> These ones are used by fd_outb() which is arch-dependent, so if we
> want to pass a third argument we have to change them all and make sure
> not to break them too much.
>
> In addition the FD_* macros defined above are used by x86, and FD_DOR is
> also used by arm while all other archs hard-code all the values. ARM also
> uses floppy_selects[fdc] and new_dor... I'm starting to feel the trap here!
> I also feel a bit concerned that these are exported in uapi with a hard-coded
> 0x3f0 base address. I'm just not sure how portable all of this is in
> the end :-/
>
> Now I'm wondering, how far should we go and how much is it acceptable to
> change ? I'd rather not have "#define fdc current_fdc" just so that it
> builds, but on the other hand this problem clearly outlights the roots
> of the issue, which lies in "fdc" being silently accessed by macros with
> nobody noticing!
>
I think that for the first attempt changing will be enough:
-static int fdc; /* current fdc */
+static int current_fdc; /* current fdc */
and
-#define FD_IOPORT fdc_state[fdc].address
+#define FD_IOPORT fdc_state[current_fdc].address
and for fd_setdor in ./arch/arm/include/asm/floppy.h
This already will require to at least test the building on x86,
arm, sparc arches (maybe more).
Just need to leave a note in the commit why it's not easy to
change FD_IOPORT in this case.
I think that small-step and observable patches are the best option
when we change floppy driver.
As for now, I can see that only floppy.c includes fdreg.h file
with define FDPATCHES. If it's true then #define FD_IOPORT 0x3f0
branch is never used and we can try to fix remaining FD_* macro
in the next round.
Denis
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-25 15:22 ` Denis Efremov
@ 2020-02-25 15:39 ` Denis Efremov
2020-02-25 16:12 ` Willy Tarreau
2020-02-25 18:02 ` Willy Tarreau
2020-02-25 18:08 ` Linus Torvalds
2 siblings, 1 reply; 45+ messages in thread
From: Denis Efremov @ 2020-02-25 15:39 UTC (permalink / raw)
To: Willy Tarreau
Cc: Jens Axboe, Linux Kernel Mailing List, linux-block, Linus Torvalds
On 2/25/20 6:22 PM, Denis Efremov wrote:
> As for now, I can see that only floppy.c includes fdreg.h file
> with define FDPATCHES. If it's true then #define FD_IOPORT 0x3f0
> branch is never used and we can try to fix remaining FD_* macro
> in the next round.
Ah, I forgot that fdregs.h is uapi. Thus, we can't simplify FDPATCHES.
Denis
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-25 15:39 ` Denis Efremov
@ 2020-02-25 16:12 ` Willy Tarreau
0 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-25 16:12 UTC (permalink / raw)
To: efremov
Cc: Jens Axboe, Linux Kernel Mailing List, linux-block, Linus Torvalds
On Tue, Feb 25, 2020 at 06:39:05PM +0300, Denis Efremov wrote:
> On 2/25/20 6:22 PM, Denis Efremov wrote:
> > As for now, I can see that only floppy.c includes fdreg.h file
> > with define FDPATCHES. If it's true then #define FD_IOPORT 0x3f0
> > branch is never used and we can try to fix remaining FD_* macro
> > in the next round.
>
> Ah, I forgot that fdregs.h is uapi. Thus, we can't simplify FDPATCHES.
Yep, that's why we can't do it. I also agree with the other change of
fdc->current_fdc in floppy.h, I think it's the most reasonable. And if
it breaks anywhere, it will simply have uncovered new latent bugs
because it will mean that it was using the wrong fdc.
Willy
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-25 15:22 ` Denis Efremov
2020-02-25 15:39 ` Denis Efremov
@ 2020-02-25 18:02 ` Willy Tarreau
2020-02-25 18:08 ` Willy Tarreau
2020-02-25 18:08 ` Linus Torvalds
2 siblings, 1 reply; 45+ messages in thread
From: Willy Tarreau @ 2020-02-25 18:02 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, Linux Kernel Mailing List, linux-block, Linus Torvalds
On Tue, Feb 25, 2020 at 06:22:47PM +0300, Denis Efremov wrote:
> I think that for the first attempt changing will be enough:
> -static int fdc; /* current fdc */
> +static int current_fdc; /* current fdc */
> and
> -#define FD_IOPORT fdc_state[fdc].address
> +#define FD_IOPORT fdc_state[current_fdc].address
> and for fd_setdor in ./arch/arm/include/asm/floppy.h
So after a bit more digging, that should not be correct because:
- disk_change() uses a local "fdc" variable with expectations that
it will be used by fd_inb(FD_DIR)
- set_dor() uses a local fdc argument that's used by
fd_outb(newdor, FD_DOR)
Here we have "fdc" hidden in:
- FD_DOR/FD_DIR (referencing FD_IOPORT) on x86
- fd_outb(), relying on fd_setdor() on ARM
I'm now looking how to change fd_outb() to pass the fdc in argument,
after all it's not that many places and that's exactly what we need.
Maybe afterwards we'll figure that some of them are still wrong :-)
Willy
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-25 18:02 ` Willy Tarreau
@ 2020-02-25 18:08 ` Willy Tarreau
0 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-25 18:08 UTC (permalink / raw)
To: Denis Efremov
Cc: Jens Axboe, Linux Kernel Mailing List, linux-block, Linus Torvalds
On Tue, Feb 25, 2020 at 07:02:19PM +0100, Willy Tarreau wrote:
> On Tue, Feb 25, 2020 at 06:22:47PM +0300, Denis Efremov wrote:
> > I think that for the first attempt changing will be enough:
> > -static int fdc; /* current fdc */
> > +static int current_fdc; /* current fdc */
> > and
> > -#define FD_IOPORT fdc_state[fdc].address
> > +#define FD_IOPORT fdc_state[current_fdc].address
> > and for fd_setdor in ./arch/arm/include/asm/floppy.h
>
> So after a bit more digging, that should not be correct because:
> - disk_change() uses a local "fdc" variable with expectations that
> it will be used by fd_inb(FD_DIR)
>
> - set_dor() uses a local fdc argument that's used by
> fd_outb(newdor, FD_DOR)
>
> Here we have "fdc" hidden in:
> - FD_DOR/FD_DIR (referencing FD_IOPORT) on x86
> - fd_outb(), relying on fd_setdor() on ARM
And in the ARM case, fdc is used to index a two-entries array with exactly
identical values, with N_FDC set to 1 so even there it's pointless... Maybe
I'll get rid of this first.
Willy
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-25 15:22 ` Denis Efremov
2020-02-25 15:39 ` Denis Efremov
2020-02-25 18:02 ` Willy Tarreau
@ 2020-02-25 18:08 ` Linus Torvalds
2020-02-25 18:15 ` Willy Tarreau
2020-02-26 8:18 ` Willy Tarreau
2 siblings, 2 replies; 45+ messages in thread
From: Linus Torvalds @ 2020-02-25 18:08 UTC (permalink / raw)
To: Denis Efremov
Cc: Willy Tarreau, Jens Axboe, Linux Kernel Mailing List, linux-block
On Tue, Feb 25, 2020 at 7:22 AM Denis Efremov <efremov@linux.com> wrote:
>
> I think that for the first attempt changing will be enough:
> -static int fdc; /* current fdc */
> +static int current_fdc; /* current fdc */
> and
> -#define FD_IOPORT fdc_state[fdc].address
> +#define FD_IOPORT fdc_state[current_fdc].address
Please don't do this blindly - ie without verifying that there are no
cases of that "local fdc variable shadowing" issue.
Of course, such a verification might be as easy as "generates exact
same code" rather than looking at every use.
And btw, don't worry too much about this being in an UAPI file. I'm
pretty sure that's because of specialty programs that use the magical
ioctls to do special formatting. They want the special commands
(FD_FORMAT etc), but I don't think they really use the port addresses.
So I think it's in a UAPI file entirely by mistake.
We should at least try moving those bits to the floppy.c file and
remove it from the header file.
For example, doing a Debian code search on "FDPATCHES" doesn't find
any user space hits. Searching for "FD_STATUS" gets a lot of hits, but
thos all seem to be because it's a symbol used by user space programs,
("file descriptor status"), not because those hits actually used the
fdreg.h header file.
So we can remove at least the FD_IOPORT mess from the header file, I bet.
Worst case - if somebody finds some case that uses them, we can put it back.
Linus
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-25 18:08 ` Linus Torvalds
@ 2020-02-25 18:15 ` Willy Tarreau
2020-02-25 18:27 ` Linus Torvalds
2020-02-26 8:18 ` Willy Tarreau
1 sibling, 1 reply; 45+ messages in thread
From: Willy Tarreau @ 2020-02-25 18:15 UTC (permalink / raw)
To: Linus Torvalds
Cc: Denis Efremov, Jens Axboe, Linux Kernel Mailing List, linux-block
On Tue, Feb 25, 2020 at 10:08:51AM -0800, Linus Torvalds wrote:
> On Tue, Feb 25, 2020 at 7:22 AM Denis Efremov <efremov@linux.com> wrote:
> >
> > I think that for the first attempt changing will be enough:
> > -static int fdc; /* current fdc */
> > +static int current_fdc; /* current fdc */
> > and
> > -#define FD_IOPORT fdc_state[fdc].address
> > +#define FD_IOPORT fdc_state[current_fdc].address
>
> Please don't do this blindly - ie without verifying that there are no
> cases of that "local fdc variable shadowing" issue.
That's exactly what I'm doing. In fact I first renamed the variable
and am manually checking all places which do not compile anymore.
Hence the surprizes.
> Of course, such a verification might be as easy as "generates exact
> same code" rather than looking at every use.
That's exactly what I'm doing.
> And btw, don't worry too much about this being in an UAPI file. I'm
> pretty sure that's because of specialty programs that use the magical
> ioctls to do special formatting. They want the special commands
> (FD_FORMAT etc), but I don't think they really use the port addresses.
>
> So I think it's in a UAPI file entirely by mistake.
OK this will help me, thanks for the hint :-)
> We should at least try moving those bits to the floppy.c file and
> remove it from the header file.
Makes sense.
> For example, doing a Debian code search on "FDPATCHES" doesn't find
> any user space hits. Searching for "FD_STATUS" gets a lot of hits, but
> thos all seem to be because it's a symbol used by user space programs,
> ("file descriptor status"), not because those hits actually used the
> fdreg.h header file.
>
> So we can remove at least the FD_IOPORT mess from the header file, I bet.
>
> Worst case - if somebody finds some case that uses them, we can put it back.
I like that. And at least we'll know how they use it (likely without the
dependency on fdc).
Thanks,
Willy
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-25 18:15 ` Willy Tarreau
@ 2020-02-25 18:27 ` Linus Torvalds
0 siblings, 0 replies; 45+ messages in thread
From: Linus Torvalds @ 2020-02-25 18:27 UTC (permalink / raw)
To: Willy Tarreau
Cc: Denis Efremov, Jens Axboe, Linux Kernel Mailing List, linux-block
On Tue, Feb 25, 2020 at 10:15 AM Willy Tarreau <w@1wt.eu> wrote:
>
> On Tue, Feb 25, 2020 at 10:08:51AM -0800, Linus Torvalds wrote:
> >
> > So we can remove at least the FD_IOPORT mess from the header file, I bet.
> >
> > Worst case - if somebody finds some case that uses them, we can put it back.
>
> I like that. And at least we'll know how they use it (likely without the
> dependency on fdc).
Note that the way uapi header files generally got created was by just
moving header files that user space used mechanically. See for example
commit 607ca46e97a1 ("UAPI: (Scripted) Disintegrate include/linux")
which created most of them.
There was no careful vetting of "this is the part that is used by user
space". It was just a "these are the files user space has used".
So it's not really a "the uapi files are set in stone and you can't
change them". Instead, you should think of the uapi files as a big red
blinking warning that says "sure, you can change them, but you need to
be very careful and think about the fact that user space may be
including this thing".
So it's a "think hard about it" rather than a "don't go there".
Of course, usually it's much _simpler_ to just "don't go there" ;)
Linus
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 11/16] floppy: remove dead code for drives scanning on ARM
2020-02-24 21:23 [PATCH 00/10] floppy driver cleanups (deobfuscation) Willy Tarreau
` (9 preceding siblings ...)
2020-02-24 21:23 ` [PATCH 10/10] floppy: cleanup: expand the reply_buffer macros Willy Tarreau
@ 2020-02-26 8:07 ` Willy Tarreau
2020-02-26 8:07 ` [PATCH 12/16] floppy: remove incomplete support for second FDC from ARM code Willy Tarreau
` (5 more replies)
2020-02-26 14:57 ` [PATCH 00/10] floppy driver cleanups (deobfuscation) Denis Efremov
11 siblings, 6 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-26 8:07 UTC (permalink / raw)
To: Denis Efremov, Linus Torvalds
Cc: Jens Axboe, linux-block, linux-kernel, Willy Tarreau
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.
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] 45+ messages in thread
* [PATCH 12/16] floppy: remove incomplete support for second FDC from ARM code
2020-02-26 8:07 ` [PATCH 11/16] floppy: remove dead code for drives scanning on ARM Willy Tarreau
@ 2020-02-26 8:07 ` Willy Tarreau
2020-02-29 16:38 ` Denis Efremov
2020-02-26 8:07 ` [PATCH 13/16] floppy: prepare ARM code to simplify base address separation Willy Tarreau
` (4 subsequent siblings)
5 siblings, 1 reply; 45+ messages in thread
From: Willy Tarreau @ 2020-02-26 8:07 UTC (permalink / raw)
To: Denis Efremov, Linus Torvalds
Cc: Jens Axboe, linux-block, linux-kernel, Willy Tarreau
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.
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
arch/arm/include/asm/floppy.h | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/arm/include/asm/floppy.h b/arch/arm/include/asm/floppy.h
index 4655652..f683953 100644
--- a/arch/arm/include/asm/floppy.h
+++ b/arch/arm/include/asm/floppy.h
@@ -50,17 +50,16 @@ 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] =
+static unsigned char floppy_selects[4] =
{
{ 0x10, 0x21, 0x23, 0x33 },
- { 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 +83,9 @@ 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];
+ floppy_selects[0] ^= floppy_selects[1];
+ floppy_selects[1] ^= floppy_selects[0];
+ floppy_selects[0] ^= floppy_selects[1];
}
#define EXTRA_FLOPPY_PARAMS ,{ "driveswap", &driveswap, NULL, 0, 0 }
--
2.9.0
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 13/16] floppy: prepare ARM code to simplify base address separation
2020-02-26 8:07 ` [PATCH 11/16] floppy: remove dead code for drives scanning on ARM Willy Tarreau
2020-02-26 8:07 ` [PATCH 12/16] floppy: remove incomplete support for second FDC from ARM code Willy Tarreau
@ 2020-02-26 8:07 ` Willy Tarreau
2020-02-26 8:07 ` [PATCH 14/16] floppy: introduce new functions fdc_inb() and fdc_outb() Willy Tarreau
` (3 subsequent siblings)
5 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-26 8:07 UTC (permalink / raw)
To: Denis Efremov, Linus Torvalds
Cc: Jens Axboe, linux-block, linux-kernel, Willy Tarreau
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.
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 f683953..c665136 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))
@@ -55,16 +60,6 @@ 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] 45+ messages in thread
* [PATCH 14/16] floppy: introduce new functions fdc_inb() and fdc_outb()
2020-02-26 8:07 ` [PATCH 11/16] floppy: remove dead code for drives scanning on ARM Willy Tarreau
2020-02-26 8:07 ` [PATCH 12/16] floppy: remove incomplete support for second FDC from ARM code Willy Tarreau
2020-02-26 8:07 ` [PATCH 13/16] floppy: prepare ARM code to simplify base address separation Willy Tarreau
@ 2020-02-26 8:07 ` Willy Tarreau
2020-02-26 8:07 ` [PATCH 15/16] floppy: separate the FDC's base address from its registers Willy Tarreau
` (2 subsequent siblings)
5 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-26 8:07 UTC (permalink / raw)
To: Denis Efremov, Linus Torvalds
Cc: Jens Axboe, linux-block, linux-kernel, Willy Tarreau
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.
---
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] 45+ messages in thread
* [PATCH 15/16] floppy: separate the FDC's base address from its registers
2020-02-26 8:07 ` [PATCH 11/16] floppy: remove dead code for drives scanning on ARM Willy Tarreau
` (2 preceding siblings ...)
2020-02-26 8:07 ` [PATCH 14/16] floppy: introduce new functions fdc_inb() and fdc_outb() Willy Tarreau
@ 2020-02-26 8:07 ` Willy Tarreau
2020-02-26 15:36 ` Denis Efremov
2020-02-26 8:07 ` [PATCH 16/16] floppy: rename the global "fdc" variable to "current_fdc" Willy Tarreau
2020-03-01 8:21 ` [PATCH 11/16] floppy: remove dead code for drives scanning on ARM Denis Efremov
5 siblings, 1 reply; 45+ messages in thread
From: Willy Tarreau @ 2020-02-26 8:07 UTC (permalink / raw)
To: Denis Efremov, Linus Torvalds
Cc: Jens Axboe, linux-block, linux-kernel, Willy Tarreau
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.
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 c665136..4e3fb71 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] 45+ messages in thread
* [PATCH 16/16] floppy: rename the global "fdc" variable to "current_fdc"
2020-02-26 8:07 ` [PATCH 11/16] floppy: remove dead code for drives scanning on ARM Willy Tarreau
` (3 preceding siblings ...)
2020-02-26 8:07 ` [PATCH 15/16] floppy: separate the FDC's base address from its registers Willy Tarreau
@ 2020-02-26 8:07 ` Willy Tarreau
2020-03-01 8:21 ` [PATCH 11/16] floppy: remove dead code for drives scanning on ARM Denis Efremov
5 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-26 8:07 UTC (permalink / raw)
To: Denis Efremov, Linus Torvalds
Cc: Jens Axboe, linux-block, linux-kernel, Willy Tarreau
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.
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] 45+ messages in thread
* Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS
2020-02-25 18:08 ` Linus Torvalds
2020-02-25 18:15 ` Willy Tarreau
@ 2020-02-26 8:18 ` Willy Tarreau
1 sibling, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-26 8:18 UTC (permalink / raw)
To: Linus Torvalds
Cc: Denis Efremov, Jens Axboe, Linux Kernel Mailing List, linux-block
On Tue, Feb 25, 2020 at 10:08:51AM -0800, Linus Torvalds wrote:
> We should at least try moving those bits to the floppy.c file and
> remove it from the header file.
>
> For example, doing a Debian code search on "FDPATCHES" doesn't find
> any user space hits. Searching for "FD_STATUS" gets a lot of hits, but
> thos all seem to be because it's a symbol used by user space programs,
> ("file descriptor status"), not because those hits actually used the
> fdreg.h header file.
>
> So we can remove at least the FD_IOPORT mess from the header file, I bet.
OK so I think this time I managed to get it done after two failed attempts.
I've sent in response to this thread 6 new patches to the series just for
validation (11 to 16), I'll spam relevant people when resending the whole
if we agree on the principle already.
First, still no single byte change in the output code:
willy@wtap:master$ diff -u floppy-{before,after}.s
--- floppy-before.s 2020-02-26 08:59:04.185152450 +0100
+++ floppy-after.s 2020-02-26 08:58:58.253156733 +0100
@@ -1,5 +1,5 @@
-floppy-before.o: file format elf64-x86-64
+floppy-after.o: file format elf64-x86-64
Disassembly of section .text:
Second, I could kill FD_IOPORT entirely. The FD_* macros are now
just the registers offsets. I've added two local functions fdc_inb()
and fdc_outb() which take an fdc and the register, and remap this
to fd_inb() and fd_outb() so that we don't need to fiddle anymore
with "fdc". I had one attempt at propagating that cleanup (base+reg
instead of port) to various archs, it was OK but didn't bring any
visible value in my opinion.
Third, I renamed "fdc" to "current_fdc" and carefully replaced all
"fdc" instances which didn't build with "current_fdc". This revealed
that at many places we iterate over current_fdc just because it was
the required name for the register macro (which used to derive from
FD_IOPORT). So at this point I'm still seeing a lot of possible
cleanups which will produce different binary output but will be quite
more reviewable. The common pattern in floppy.c is :
for (current_fdc = 0; current_fdc < N_FDC; current_fdc++) {
do_something(current_fdc);
}
current_fdc = 0;
or:
for (i = 0; i < N_FDC; i++) {
current_fdc = i;
do_something(current_fdc);
}
current_fdc = 0;
These ones can safely be cleaned up.
I also thought that once done we could have a "current_fdc" being a
struct floppy_fdc_state* instead of an int and directly point to the
correct fdc_state. This way we'll regain a lot of readability in the
code.
Please just tell me what you think and if I should repost a whole
series and/or continue the cleanup.
Thanks,
Willy
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 00/10] floppy driver cleanups (deobfuscation)
2020-02-24 21:23 [PATCH 00/10] floppy driver cleanups (deobfuscation) Willy Tarreau
` (10 preceding siblings ...)
2020-02-26 8:07 ` [PATCH 11/16] floppy: remove dead code for drives scanning on ARM Willy Tarreau
@ 2020-02-26 14:57 ` Denis Efremov
2020-02-26 17:49 ` Linus Torvalds
11 siblings, 1 reply; 45+ messages in thread
From: Denis Efremov @ 2020-02-26 14:57 UTC (permalink / raw)
To: Willy Tarreau; +Cc: Jens Axboe, linux-kernel, linux-block, Linus Torvalds
On 2/25/20 12:23 AM, Willy Tarreau wrote:
> As indicated in commit 2e90ca6 ("floppy: check FDC index for errors
> before assigning it") there are some surprising effects in the floppy
> driver due to some macros referencing global or local variables while
> at first glance being inoffensive.
>
> This patchset aims at removing these macros and replacing all of their
> occurrences by the equivalent code. Most of the work was done under
> Coccinelle's assistance, and it was verified that the resulting binary
> code is exactly the same as the original one.
>
> The aim is not to make the driver prettier, as Linus mentioned it's
> already not pretty. It only aims at making potential bugs more visible,
> given almost all latest changes to this driver were fixes for out-of-
> bounds and similar bugs.
>
> As a side effect, some lines got longer, causing checkpatch to complain
> a bit, but I preferred to let it complain as I didn't want to break them
> apart as I'm already seeing the trap of going too far here.
>
> The patches are broken by macro (or sets of macros when relevant) so
> that each of them remains reviewable.
>
> I can possibly go a bit further in the cleanup but I haven't used
> floppies for a few years now and am not interested in doing too much
> on this driver by lack of use cases.
For patches 1-10.
[x] eye checked the changes
[x] bloat-o-meter and .s diff show no real changes
[x] tested that kernel builds after every patch
[x] floppy targeted fuzzing with kasan+ubsan reveals no *new* issues
(required mainly to test the previous patch)
If Linus has no objections (regarding his review) I would prefer to
accept 1-10 patches rather to resend them again. They seems complete
to me as the first step.
I've placed the patches here:
https://github.com/evdenis/linux-floppy/commits/floppy-next
Thanks,
Denis
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 15/16] floppy: separate the FDC's base address from its registers
2020-02-26 8:07 ` [PATCH 15/16] floppy: separate the FDC's base address from its registers Willy Tarreau
@ 2020-02-26 15:36 ` Denis Efremov
2020-02-26 15:46 ` Willy Tarreau
0 siblings, 1 reply; 45+ messages in thread
From: Denis Efremov @ 2020-02-26 15:36 UTC (permalink / raw)
To: Willy Tarreau, Linus Torvalds; +Cc: Jens Axboe, linux-block, linux-kernel
> 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.
>
> 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.
Just to note for future ref: the mask (7) can be introduced as define
during future clean up of these magic constants.
>
> 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 c665136..4e3fb71 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
Again, just to note: FD_IOPORT (now removed), FDC1, FDC_BASE are pointing to
the same port 0x3f0 in many cases.
And at least in some cases used directly:
$ fgrep --include='*floppy*' -nrie '0x3f0' .
./arch/mips/include/asm/mach-generic/floppy.h:113: return 0x3f0;
./arch/m68k/include/asm/floppy.h:124: return 0x3f0;
./drivers/block/floppy.c:234:static unsigned short virtual_dma_port = 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 */
>
Denis
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 15/16] floppy: separate the FDC's base address from its registers
2020-02-26 15:36 ` Denis Efremov
@ 2020-02-26 15:46 ` Willy Tarreau
0 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-26 15:46 UTC (permalink / raw)
To: Denis Efremov; +Cc: Linus Torvalds, Jens Axboe, linux-block, linux-kernel
On Wed, Feb 26, 2020 at 06:36:52PM +0300, Denis Efremov wrote:
> > 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.
> >
> > 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.
>
> Just to note for future ref: the mask (7) can be introduced as define
> during future clean up of these magic constants.
I'd rather not add it because if we finish to clean up the internal API,
then we can have fd_outb(value, base, reg) and fd_inb(base, reg) where
reg is one of FD_* and base the base address. In this context we don't
need the mask anymore since the register is placed there verbatim.
I do have another earlier patch which did just that, its just that I
attacked the problem from the wrong side, resulting in too many changes
at once for my taste. But I definitely see how we can finish that job
and make everything almost elegant.
Willy
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 00/10] floppy driver cleanups (deobfuscation)
2020-02-26 14:57 ` [PATCH 00/10] floppy driver cleanups (deobfuscation) Denis Efremov
@ 2020-02-26 17:49 ` Linus Torvalds
2020-02-26 18:41 ` Willy Tarreau
2020-02-29 14:13 ` Willy Tarreau
0 siblings, 2 replies; 45+ messages in thread
From: Linus Torvalds @ 2020-02-26 17:49 UTC (permalink / raw)
To: Denis Efremov
Cc: Willy Tarreau, Jens Axboe, Linux Kernel Mailing List, linux-block
On Wed, Feb 26, 2020 at 6:57 AM Denis Efremov <efremov@linux.com> wrote:
>
> If Linus has no objections (regarding his review) I would prefer to
> accept 1-10 patches rather to resend them again. They seems complete
> to me as the first step.
I have no objections, and the patches 11-16 seem to have addressed all
my "I wish.." concerns too (except for the "we should rewrite or
sunset the driver entirely"). Looks fine to me.
Linus
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 00/10] floppy driver cleanups (deobfuscation)
2020-02-26 17:49 ` Linus Torvalds
@ 2020-02-26 18:41 ` Willy Tarreau
2020-02-29 14:13 ` Willy Tarreau
1 sibling, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-02-26 18:41 UTC (permalink / raw)
To: Linus Torvalds
Cc: Denis Efremov, Jens Axboe, Linux Kernel Mailing List, linux-block
On Wed, Feb 26, 2020 at 09:49:05AM -0800, Linus Torvalds wrote:
> On Wed, Feb 26, 2020 at 6:57 AM Denis Efremov <efremov@linux.com> wrote:
> >
> > If Linus has no objections (regarding his review) I would prefer to
> > accept 1-10 patches rather to resend them again. They seems complete
> > to me as the first step.
>
> I have no objections, and the patches 11-16 seem to have addressed all
> my "I wish.." concerns too (except for the "we should rewrite or
> sunset the driver entirely").
Sorry if I broke your dream :-)
> Looks fine to me.
Thanks!
Willy
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 00/10] floppy driver cleanups (deobfuscation)
2020-02-26 17:49 ` Linus Torvalds
2020-02-26 18:41 ` Willy Tarreau
@ 2020-02-29 14:13 ` Willy Tarreau
2020-02-29 15:58 ` Linus Torvalds
1 sibling, 1 reply; 45+ messages in thread
From: Willy Tarreau @ 2020-02-29 14:13 UTC (permalink / raw)
To: Linus Torvalds
Cc: Denis Efremov, Jens Axboe, Linux Kernel Mailing List, linux-block
Hi Linus,
On Wed, Feb 26, 2020 at 09:49:05AM -0800, Linus Torvalds wrote:
> On Wed, Feb 26, 2020 at 6:57 AM Denis Efremov <efremov@linux.com> wrote:
> >
> > If Linus has no objections (regarding his review) I would prefer to
> > accept 1-10 patches rather to resend them again. They seems complete
> > to me as the first step.
>
> I have no objections, and the patches 11-16 seem to have addressed all
> my "I wish.." concerns too (except for the "we should rewrite or
> sunset the driver entirely"). Looks fine to me.
So I continued to work on this cleanup and regardless of the side I
attacked this hill, it progressively stalled once I got closer to
the interrupt and delayed work stuff. I understood the root cause of
the problem, it's actually double:
- single interrupt for multiple FDCs : this means we need to have
some global context to know what we're working with. It is not
completely true because we could very well have a list of pending
operations per FDC to scan on interrupt and update them when the
interrupt strikes and the FDC reports a completion. I noticed that
raw_cmd, raw_buffer, inr, current_req, _floppy, plus a number of
function pointers used to handle delayed work should in fact be
per-FDC if we didn't want to change them at run time ;
- single DMA channel for multiple FDCs : regardless of what could
be done for the interrupt above, we're still stuck with a single
DMA setup at once, which requires to issue reset_fdc() here and
there, making it clear we can't work with multiple FDCs in parallel.
Given the number of places where such global variables are set, I'm
not totally confident in the fact that nowhere we keep a setting
that was assigned for the FDC in the previous request. For example
a spurious (or late) interrupt could slightly affect the fdc_state[]
and maybe even emit FD_SENSEI to the current controller. Also this
comment in floppy_interrupt() made me realize the driver pre-dates
SMP support:
/* It is OK to emit floppy commands because we are in an interrupt
* handler here, and thus we have to fear no interference of other
* activity.
*/
It seems that changing the current FDC is still only protected by
the fdc_busy bit, but that the interrupt handler doesn't check
if anything is expected before touching bits related to current_fdc.
All this made me wonder if we really want to continue to maintain the
support for multiple FDCs. I checked all archs, and only x86, alpha
and powerpc support more than one FDC, 2 to be precise (hence up to
8 drives). I have the impression that if we keep a single FDC we'll
have a cleaner code that doesn't need to change global settings under
us when doing resets or scans. So I don't know if that's something
interesting to pursue.
I also noticed that a lot of global variables are inter-dependent and
should probably be moved to fdc_state[] so that what's per-FDC is now
more explicit, except that this struct is exposed to userland via
the FDGETFDCSTAT ioctl (but that could be changed so that the fdc_state
is just a struct inside a per-fdc larger struct).
At least now I get a better picture of the little ROI felt trying to
clean this, and I don't think that even a complete rewrite as you
suggested would address all the issues at all.
So if you or Denis think there's some value in me continuing to explore
one of these areas, I can continue, otherwise I can simply resend the
last part of my series with the few missing Cc and be done with it.
Willy
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 00/10] floppy driver cleanups (deobfuscation)
2020-02-29 14:13 ` Willy Tarreau
@ 2020-02-29 15:58 ` Linus Torvalds
2020-02-29 23:19 ` Ondrej Zary
0 siblings, 1 reply; 45+ messages in thread
From: Linus Torvalds @ 2020-02-29 15:58 UTC (permalink / raw)
To: Willy Tarreau
Cc: Denis Efremov, Jens Axboe, Linux Kernel Mailing List, linux-block
On Sat, Feb 29, 2020 at 8:14 AM Willy Tarreau <w@1wt.eu> wrote:
>
> So if you or Denis think there's some value in me continuing to explore
> one of these areas, I can continue, otherwise I can simply resend the
> last part of my series with the few missing Cc and be done with it.
It's fine - this driver isn't worth spending a ton of effort on.
The only users are virtualization, and even they are going away
because floppies are so small, and other things have become more
standard anyway (ie USB disk) or easier to emulate (NVMe or whatever).
So I suspect the only reason floppy is used even in that area is just
legacy "we haven't bothered updating to anything better and we have
old scripts and images that work".
Linus
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 12/16] floppy: remove incomplete support for second FDC from ARM code
2020-02-26 8:07 ` [PATCH 12/16] floppy: remove incomplete support for second FDC from ARM code Willy Tarreau
@ 2020-02-29 16:38 ` Denis Efremov
0 siblings, 0 replies; 45+ messages in thread
From: Denis Efremov @ 2020-02-29 16:38 UTC (permalink / raw)
To: Willy Tarreau, Linus Torvalds; +Cc: Jens Axboe, linux-block, linux-kernel
Hi,
On 2/26/20 11:07 AM, Willy Tarreau wrote:
> 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.
>
> Signed-off-by: Willy Tarreau <w@1wt.eu>
> ---
> arch/arm/include/asm/floppy.h | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/include/asm/floppy.h b/arch/arm/include/asm/floppy.h
> index 4655652..f683953 100644
> --- a/arch/arm/include/asm/floppy.h
> +++ b/arch/arm/include/asm/floppy.h
> @@ -50,17 +50,16 @@ 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] =
> +static unsigned char floppy_selects[4] =
> {
> { 0x10, 0x21, 0x23, 0x33 },
> - { 0x10, 0x21, 0x23, 0x33 }
> };
>
You need remove curly braces here, e.g.
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 +83,9 @@ 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];
> + floppy_selects[0] ^= floppy_selects[1];
> + floppy_selects[1] ^= floppy_selects[0];
> + floppy_selects[0] ^= floppy_selects[1];
What do you think about using swap macro (kernel.h) here?
> }
>
> #define EXTRA_FLOPPY_PARAMS ,{ "driveswap", &driveswap, NULL, 0, 0 }
>
Denis
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 00/10] floppy driver cleanups (deobfuscation)
2020-02-29 15:58 ` Linus Torvalds
@ 2020-02-29 23:19 ` Ondrej Zary
2020-03-01 6:46 ` Willy Tarreau
0 siblings, 1 reply; 45+ messages in thread
From: Ondrej Zary @ 2020-02-29 23:19 UTC (permalink / raw)
To: Linus Torvalds
Cc: Willy Tarreau, Denis Efremov, Jens Axboe,
Linux Kernel Mailing List, linux-block
On Saturday 29 February 2020 16:58:11 Linus Torvalds wrote:
> On Sat, Feb 29, 2020 at 8:14 AM Willy Tarreau <w@1wt.eu> wrote:
> >
> > So if you or Denis think there's some value in me continuing to explore
> > one of these areas, I can continue, otherwise I can simply resend the
> > last part of my series with the few missing Cc and be done with it.
>
> It's fine - this driver isn't worth spending a ton of effort on.
>
> The only users are virtualization, and even they are going away
> because floppies are so small, and other things have become more
> standard anyway (ie USB disk) or easier to emulate (NVMe or whatever).
>
> So I suspect the only reason floppy is used even in that area is just
> legacy "we haven't bothered updating to anything better and we have
> old scripts and images that work".
>
> Linus
>
There are real users with real floppy drives out there.
--
Ondrej Zary
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 00/10] floppy driver cleanups (deobfuscation)
2020-02-29 23:19 ` Ondrej Zary
@ 2020-03-01 6:46 ` Willy Tarreau
2020-03-01 17:01 ` Ondrej Zary
0 siblings, 1 reply; 45+ messages in thread
From: Willy Tarreau @ 2020-03-01 6:46 UTC (permalink / raw)
To: Ondrej Zary
Cc: Linus Torvalds, Denis Efremov, Jens Axboe,
Linux Kernel Mailing List, linux-block
On Sun, Mar 01, 2020 at 12:19:14AM +0100, Ondrej Zary wrote:
> On Saturday 29 February 2020 16:58:11 Linus Torvalds wrote:
> > On Sat, Feb 29, 2020 at 8:14 AM Willy Tarreau <w@1wt.eu> wrote:
> > >
> > > So if you or Denis think there's some value in me continuing to explore
> > > one of these areas, I can continue, otherwise I can simply resend the
> > > last part of my series with the few missing Cc and be done with it.
> >
> > It's fine - this driver isn't worth spending a ton of effort on.
> >
> > The only users are virtualization, and even they are going away
> > because floppies are so small, and other things have become more
> > standard anyway (ie USB disk) or easier to emulate (NVMe or whatever).
> >
> > So I suspect the only reason floppy is used even in that area is just
> > legacy "we haven't bothered updating to anything better and we have
> > old scripts and images that work".
> >
> > Linus
> >
>
> There are real users with real floppy drives out there.
OK thanks for the feedback. Then I'll continue the minimum cleanups to
try to focus on maintainability and on the principle of least surprise,
and I'll have a quick look at the possible simplifications brought by
the limitation to one FDC, in case that really helps.
Willy
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 11/16] floppy: remove dead code for drives scanning on ARM
2020-02-26 8:07 ` [PATCH 11/16] floppy: remove dead code for drives scanning on ARM Willy Tarreau
` (4 preceding siblings ...)
2020-02-26 8:07 ` [PATCH 16/16] floppy: rename the global "fdc" variable to "current_fdc" Willy Tarreau
@ 2020-03-01 8:21 ` Denis Efremov
2020-03-01 8:59 ` Willy Tarreau
5 siblings, 1 reply; 45+ messages in thread
From: Denis Efremov @ 2020-03-01 8:21 UTC (permalink / raw)
To: Willy Tarreau, Linus Torvalds; +Cc: Jens Axboe, linux-block, linux-kernel
Hi,
For patches 11-16,
I've checked the building on x86, arm.
x86 shows no difference in floppy.o binary.
Compilation on arm tested (make rpc_defconfig).
I think that macro fd_outb from arm could be turned to
the static inline function, like on mips or m68k arches. However,
it's up to you if you want to keep the changes close to the
original structure.
Please address the warnings and resend the patches:
In file included from drivers/block/floppy.c:251:
./arch/arm/include/asm/floppy.h:60:2: warning: braces around scalar initializer
60 | { 0x10, 0x21, 0x23, 0x33 },
| ^
./arch/arm/include/asm/floppy.h:60:2: note: (near initialization for ‘floppy_selects[0]’)
./arch/arm/include/asm/floppy.h:60:10: warning: excess elements in scalar initializer
60 | { 0x10, 0x21, 0x23, 0x33 },
| ^~~~
./arch/arm/include/asm/floppy.h:60:10: note: (near initialization for ‘floppy_selects[0]’)
./arch/arm/include/asm/floppy.h:60:16: warning: excess elements in scalar initializer
60 | { 0x10, 0x21, 0x23, 0x33 },
| ^~~~
./arch/arm/include/asm/floppy.h:60:16: note: (near initialization for ‘floppy_selects[0]’)
./arch/arm/include/asm/floppy.h:60:22: warning: excess elements in scalar initializer
60 | { 0x10, 0x21, 0x23, 0x33 },
| ^~~~
./arch/arm/include/asm/floppy.h:60:22: note: (near initialization for ‘floppy_selects[0]’)
CC [M] drivers/block/loop.o
drivers/block/floppy.c: In function ‘fdc_outb’:
./arch/arm/include/asm/floppy.h:15:14: warning: suggest parentheses around comparison in operand of ‘&’ [-Wparentheses]
15 | if ((port) & 7 == FD_DOR) { \
| ^
drivers/block/floppy.c:603:2: note: in expansion of macro ‘fd_outb’
603 | fd_outb(value, fdc_state[fdc].address + reg);
Everything else looks good to me. Thanks!
Regards,
Denis
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 11/16] floppy: remove dead code for drives scanning on ARM
2020-03-01 8:21 ` [PATCH 11/16] floppy: remove dead code for drives scanning on ARM Denis Efremov
@ 2020-03-01 8:59 ` Willy Tarreau
0 siblings, 0 replies; 45+ messages in thread
From: Willy Tarreau @ 2020-03-01 8:59 UTC (permalink / raw)
To: Denis Efremov; +Cc: Linus Torvalds, Jens Axboe, linux-block, linux-kernel
Hi Denis,
On Sun, Mar 01, 2020 at 11:21:45AM +0300, Denis Efremov wrote:
> Hi,
>
> For patches 11-16,
>
> I've checked the building on x86, arm.
> x86 shows no difference in floppy.o binary.
Thanks for double-checking.
> Compilation on arm tested (make rpc_defconfig).
>
> I think that macro fd_outb from arm could be turned to
> the static inline function, like on mips or m68k arches. However,
> it's up to you if you want to keep the changes close to the
> original structure.
I wanted to do it as well but wondered if we should focus on limiting
changes or doign an in-depth cleanup. I'll see if I find time for
an extra round of per-arch cleanup, I think it doesn't cost much and
is probably worth being done.
> Please address the warnings and resend the patches:
(...)
Will do in my local patch set and I'll also CC RMK and the few other
persons having touched the ARM part of the driver.
> Everything else looks good to me. Thanks!
Thank you! I've also found an unlocking bug in the driver, when doing
ioctl(FDRESET), if a signal comes, we leave without unlocking. I'll
send a separate patch for this.
Regards,
Willy
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 00/10] floppy driver cleanups (deobfuscation)
2020-03-01 6:46 ` Willy Tarreau
@ 2020-03-01 17:01 ` Ondrej Zary
0 siblings, 0 replies; 45+ messages in thread
From: Ondrej Zary @ 2020-03-01 17:01 UTC (permalink / raw)
To: Willy Tarreau
Cc: Linus Torvalds, Denis Efremov, Jens Axboe,
Linux Kernel Mailing List, linux-block
On Sunday 01 March 2020 07:46:01 Willy Tarreau wrote:
> On Sun, Mar 01, 2020 at 12:19:14AM +0100, Ondrej Zary wrote:
> > On Saturday 29 February 2020 16:58:11 Linus Torvalds wrote:
> > > On Sat, Feb 29, 2020 at 8:14 AM Willy Tarreau <w@1wt.eu> wrote:
> > > > So if you or Denis think there's some value in me continuing to
> > > > explore one of these areas, I can continue, otherwise I can simply
> > > > resend the last part of my series with the few missing Cc and be done
> > > > with it.
> > >
> > > It's fine - this driver isn't worth spending a ton of effort on.
> > >
> > > The only users are virtualization, and even they are going away
> > > because floppies are so small, and other things have become more
> > > standard anyway (ie USB disk) or easier to emulate (NVMe or whatever).
> > >
> > > So I suspect the only reason floppy is used even in that area is just
> > > legacy "we haven't bothered updating to anything better and we have
> > > old scripts and images that work".
> > >
> > > Linus
> >
> > There are real users with real floppy drives out there.
>
> OK thanks for the feedback. Then I'll continue the minimum cleanups to
> try to focus on maintainability and on the principle of least surprise,
> and I'll have a quick look at the possible simplifications brought by
> the limitation to one FDC, in case that really helps.
Thank you very much for the work.
I haven't ever seen a machine with more than single FDC so that case might be
hard to test. There are some ISA FDCs with configurable I/O addresses (maybe
I have one of them somewhere) but they might not work properly together with
on-board super I/O FDCs.
The most common case - one FDC with at most two drives should be enough for
the modern simplified driver.
--
Ondrej Zary
^ permalink raw reply [flat|nested] 45+ messages in thread
end of thread, other threads:[~2020-03-01 17:01 UTC | newest]
Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-24 21:23 [PATCH 00/10] floppy driver cleanups (deobfuscation) Willy Tarreau
2020-02-24 21:23 ` [PATCH 01/10] floppy: cleanup: expand macro FDCS Willy Tarreau
2020-02-24 21:53 ` Linus Torvalds
2020-02-24 23:13 ` Denis Efremov
2020-02-25 3:45 ` Willy Tarreau
2020-02-25 7:14 ` Denis Efremov
2020-02-25 14:02 ` Willy Tarreau
2020-02-25 15:22 ` Denis Efremov
2020-02-25 15:39 ` Denis Efremov
2020-02-25 16:12 ` Willy Tarreau
2020-02-25 18:02 ` Willy Tarreau
2020-02-25 18:08 ` Willy Tarreau
2020-02-25 18:08 ` Linus Torvalds
2020-02-25 18:15 ` Willy Tarreau
2020-02-25 18:27 ` Linus Torvalds
2020-02-26 8:18 ` Willy Tarreau
2020-02-25 11:37 ` Denis Efremov
2020-02-24 21:23 ` [PATCH 02/10] floppy: cleanup: expand macro UFDCS Willy Tarreau
2020-02-24 21:23 ` [PATCH 03/10] floppy: cleanup: expand macro UDP Willy Tarreau
2020-02-24 21:23 ` [PATCH 04/10] floppy: cleanup: expand macro UDRS Willy Tarreau
2020-02-24 21:23 ` [PATCH 05/10] floppy: cleanup: expand macro UDRWE Willy Tarreau
2020-02-24 21:23 ` [PATCH 06/10] floppy: cleanup: expand macro DP Willy Tarreau
2020-02-24 21:23 ` [PATCH 07/10] floppy: cleanup: expand macro DRS Willy Tarreau
2020-02-24 21:23 ` [PATCH 08/10] floppy: cleanup: expand macro DRWE Willy Tarreau
2020-02-24 21:23 ` [PATCH 09/10] floppy: cleanup: expand the R/W / format command macros Willy Tarreau
2020-02-24 21:23 ` [PATCH 10/10] floppy: cleanup: expand the reply_buffer macros Willy Tarreau
2020-02-26 8:07 ` [PATCH 11/16] floppy: remove dead code for drives scanning on ARM Willy Tarreau
2020-02-26 8:07 ` [PATCH 12/16] floppy: remove incomplete support for second FDC from ARM code Willy Tarreau
2020-02-29 16:38 ` Denis Efremov
2020-02-26 8:07 ` [PATCH 13/16] floppy: prepare ARM code to simplify base address separation Willy Tarreau
2020-02-26 8:07 ` [PATCH 14/16] floppy: introduce new functions fdc_inb() and fdc_outb() Willy Tarreau
2020-02-26 8:07 ` [PATCH 15/16] floppy: separate the FDC's base address from its registers Willy Tarreau
2020-02-26 15:36 ` Denis Efremov
2020-02-26 15:46 ` Willy Tarreau
2020-02-26 8:07 ` [PATCH 16/16] floppy: rename the global "fdc" variable to "current_fdc" Willy Tarreau
2020-03-01 8:21 ` [PATCH 11/16] floppy: remove dead code for drives scanning on ARM Denis Efremov
2020-03-01 8:59 ` Willy Tarreau
2020-02-26 14:57 ` [PATCH 00/10] floppy driver cleanups (deobfuscation) Denis Efremov
2020-02-26 17:49 ` Linus Torvalds
2020-02-26 18:41 ` Willy Tarreau
2020-02-29 14:13 ` Willy Tarreau
2020-02-29 15:58 ` Linus Torvalds
2020-02-29 23:19 ` Ondrej Zary
2020-03-01 6:46 ` Willy Tarreau
2020-03-01 17:01 ` Ondrej Zary
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).