| Message ID | 20200301195555.11154-1-w@1wt.eu (mailing list archive) |
|---|---|
| Headers | show |
| Series | floppy: make use of the local/global fdc explicit | expand |
On Sun, 2020-03-01 at 20:55 +0100, Willy Tarreau wrote: > This is an update to the first minimal cleanup of the floppy driver in > order to make use of the FDC number explicit so as to avoid bugs like > the one fixed by 2e90ca68 ("floppy: check FDC index for errors before > assigning it"). Thanks Willy. trivia: you could style check the patches using checkpatch.
On Sun, Mar 01, 2020 at 12:33:25PM -0800, Joe Perches wrote: > On Sun, 2020-03-01 at 20:55 +0100, Willy Tarreau wrote: > > This is an update to the first minimal cleanup of the floppy driver in > > order to make use of the FDC number explicit so as to avoid bugs like > > the one fixed by 2e90ca68 ("floppy: check FDC index for errors before > > assigning it"). > > Thanks Willy. > > trivia: you could style check the patches using checkpatch. Oh yes, sorry about this, I did it in the first series but forgot to do it again. What checkpatch complains about is essentially lines which slightly exceed 80 characters due to the macro expansions, while I tried to limit the changes to the strict minimum. I guess that's OK to keep future fixes easier to backport. Thanks, Willy
Hi, On 3/1/20 10:55 PM, Willy Tarreau wrote: > This is an update to the first minimal cleanup of the floppy driver in > order to make use of the FDC number explicit so as to avoid bugs like > the one fixed by 2e90ca68 ("floppy: check FDC index for errors before > assigning it"). > > The purpose of this patchset is to rename the "fdc" global variable to > "current_fdc" as Linus suggested and adjust the macros which rely on it > depending on their context. > > The most problematic part at this step are the FD_* macros derived > from FD_IOPORT, itself referencing the fdc to get its base address. > These are exclusively used by fd_outb() and fd_inb(). However on ARM > FD_DOR is also used to compare the register based on the port, hence > a small change in the ARM specific code to only check the register > without relying on this hidden memory access. > > In order to avoid touching the fd_outb() and fd_inb() macros/functions > on all supported architectures, a new set of fdc_outb()/fdc_inb() > functions was added to the driver to call the former after adding > the register to the FDC's base address. > > There are still opportunities for more cleanup, though it's uncertain > they're welcome in this old driver : > - the base address and register can be passed separately to fd_outb() > and fd_inb() in order to simplify register retrieval in some archs; > > - a dozen of functions in the driver implicitly depend on current_fdc > while passing it as an argument makes the driver a bit more readable > but that represents less than half of the code and doesn't address > all the readability concerns; > > - a test was done to limit support to a single FDC, but after these > cleanups it doesn't provide any significant benefit in terms of code > readability. > > These patches are to be applied on top of Denis' floppy-next branch. > > v2: > - CC arch maintainers in ARM patches > - fixed issues after Denis' review: > - extra braces in floppy.h in declaration of floppy_selects[] > - missing parenthesis in fd_outb() macro to silence a warning > - used the swap() macro in driveswap() > Willy Tarreau (6): Applied, thanks! https://github.com/evdenis/linux-floppy/commits/floppy-next Ian, Russell, I hope you don't mind if these patches will go through the single tree. If you have any comments, please, write. Tested: [x] Eye-checked the changes [x] Checked that kernel builds after every commit on x86, arm (rpc_defconfig) [x] Checked that there is no binary difference on x86 > floppy: remove dead code for drives scanning on ARM [x] Checked that there is no dead code left unnoticed > floppy: remove incomplete support for second FDC from ARM code > floppy: prepare ARM code to simplify base address separation > floppy: introduce new functions fdc_inb() and fdc_outb() > floppy: separate the FDC's base address from its registers > floppy: rename the global "fdc" variable to "current_fdc" > > arch/arm/include/asm/floppy.h | 88 ++----------- > drivers/block/floppy.c | 284 ++++++++++++++++++++++-------------------- > include/uapi/linux/fdreg.h | 18 +-- > 3 files changed, 168 insertions(+), 222 deletions(-) > Denis
This is an update to the first minimal cleanup of the floppy driver in order to make use of the FDC number explicit so as to avoid bugs like the one fixed by 2e90ca68 ("floppy: check FDC index for errors before assigning it"). The purpose of this patchset is to rename the "fdc" global variable to "current_fdc" as Linus suggested and adjust the macros which rely on it depending on their context. The most problematic part at this step are the FD_* macros derived from FD_IOPORT, itself referencing the fdc to get its base address. These are exclusively used by fd_outb() and fd_inb(). However on ARM FD_DOR is also used to compare the register based on the port, hence a small change in the ARM specific code to only check the register without relying on this hidden memory access. In order to avoid touching the fd_outb() and fd_inb() macros/functions on all supported architectures, a new set of fdc_outb()/fdc_inb() functions was added to the driver to call the former after adding the register to the FDC's base address. There are still opportunities for more cleanup, though it's uncertain they're welcome in this old driver : - the base address and register can be passed separately to fd_outb() and fd_inb() in order to simplify register retrieval in some archs; - a dozen of functions in the driver implicitly depend on current_fdc while passing it as an argument makes the driver a bit more readable but that represents less than half of the code and doesn't address all the readability concerns; - a test was done to limit support to a single FDC, but after these cleanups it doesn't provide any significant benefit in terms of code readability. These patches are to be applied on top of Denis' floppy-next branch. v2: - CC arch maintainers in ARM patches - fixed issues after Denis' review: - extra braces in floppy.h in declaration of floppy_selects[] - missing parenthesis in fd_outb() macro to silence a warning - used the swap() macro in driveswap() Willy Tarreau (6): floppy: remove dead code for drives scanning on ARM floppy: remove incomplete support for second FDC from ARM code floppy: prepare ARM code to simplify base address separation floppy: introduce new functions fdc_inb() and fdc_outb() floppy: separate the FDC's base address from its registers floppy: rename the global "fdc" variable to "current_fdc" arch/arm/include/asm/floppy.h | 88 ++----------- drivers/block/floppy.c | 284 ++++++++++++++++++++++-------------------- include/uapi/linux/fdreg.h | 18 +-- 3 files changed, 168 insertions(+), 222 deletions(-)