Message-ID: <e925dc3d-53d3-b656-8c17-470ada66f3f7@linux.com>
Date:   Sun, 8 Mar 2020 16:12:22 +0300
From:   Denis Efremov <efremov@...ux.com>
To:     Willy Tarreau <w@....eu>
Cc:     Jens Axboe <axboe@...nel.dk>, linux-kernel@...r.kernel.org,
        linux-block@...r.kernel.org, Ian Molton <spyro@....com>,
        Russell King <linux@...linux.org.uk>
Subject: Re: [PATCH v2 0/6] floppy: make use of the local/global fdc explicit

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

Powered by blists - more mailing lists