| Message ID | 20200226080732.1913-1-w@1wt.eu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | floppy driver cleanups (deobfuscation) | expand |
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
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
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
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(-)