Re: [PATCH 1/6 v2] mmc: core: Delete bounce buffer Kconfig option

2017-05-19 Thread Linus Walleij
On Fri, May 19, 2017 at 10:30 AM, Ulf Hansson  wrote:

> Thanks, the *series* applied for next! (Responding to patch1 as
> couldn't find the cover-letter for v2).

Awesome, and just to make sure you will not be bored in the weekend
I just sent a sequel series expanding the use of per-request datas
to move more host locking and congestion out of the way.

Yours,
Linus Walleij


Re: [PATCH 1/6 v2] mmc: core: Delete bounce buffer Kconfig option

2017-05-19 Thread Ulf Hansson
On 18 May 2017 at 11:29, Linus Walleij  wrote:
> This option is activated by all multiplatform configs and what
> not so we almost always have it turned on, and the memory it
> saves is negligible, even more so moving forward. The actual
> bounce buffer only gets allocated only when used, the only
> thing the ifdefs are saving is a little bit of code.
>
> It is highly improper to have this as a Kconfig option that
> get turned on by Kconfig, make this a pure runtime-thing and
> let the host decide whether we use bounce buffers. We add a
> new property "disable_bounce" to the host struct.
>
> Notice that mmc_queue_calc_bouncesz() already disables the
> bounce buffers if host->max_segs != 1, so any arch that has a
> maximum number of segments higher than 1 will have bounce
> buffers disabled.
>
> The option CONFIG_MMC_BLOCK_BOUNCE is default y so the
> majority of platforms in the kernel already have it on, and
> it then gets turned off at runtime since most of these have
> a host->max_segs > 1. The few exceptions that have
> host->max_segs == 1 and still turn off the bounce buffering
> are those that disable it in their defconfig.
>
> Those are the following:
>
> arch/arm/configs/colibri_pxa300_defconfig
> arch/arm/configs/zeus_defconfig
> - Uses MMC_PXA, drivers/mmc/host/pxamci.c
> - Sets host->max_segs = NR_SG, which is 1
> - This needs its bounce buffer deactivated so we set
>   host->disable_bounce to true in the host driver
>
> arch/arm/configs/davinci_all_defconfig
> - Uses MMC_DAVINCI, drivers/mmc/host/davinci_mmc.c
> - This driver sets host->max_segs to MAX_NR_SG, which is 16
> - That means this driver anyways disabled bounce buffers
> - No special action needed for this platform
>
> arch/arm/configs/lpc32xx_defconfig
> arch/arm/configs/nhk8815_defconfig
> arch/arm/configs/u300_defconfig
> - Uses MMC_ARMMMCI, drivers/mmc/host/mmci.[c|h]
> - This driver by default sets host->max_segs to NR_SG,
>   which is 128, unless a DMA engine is used, and in that case
>   the number of segments are also > 1
> - That means this driver already disables bounce buffers
> - No special action needed for these platforms
>
> arch/arm/configs/sama5_defconfig
> - Uses MMC_SDHCI, MMC_SDHCI_PLTFM, MMC_SDHCI_OF_AT91, MMC_ATMELMCI
> - Uses drivers/mmc/host/sdhci.c
> - Normally sets host->max_segs to SDHCI_MAX_SEGS which is 128 and
>   thus disables bounce buffers
> - Sets host->max_segs to 1 if SDHCI_USE_SDMA is set
> - SDHCI_USE_SDMA is only set by SDHCI on PCI adapers
> - That means that for this platform bounce buffers are already
>   disabled at runtime
> - No special action needed for this platform
>
> arch/blackfin/configs/CM-BF533_defconfig
> arch/blackfin/configs/CM-BF537E_defconfig
> - Uses MMC_SPI (a simple MMC card connected on SPI pins)
> - Uses drivers/mmc/host/mmc_spi.c
> - Sets host->max_segs to MMC_SPI_BLOCKSATONCE which is 128
> - That means this platform already disables bounce buffers at
>   runtime
> - No special action needed for these platforms
>
> arch/mips/configs/cavium_octeon_defconfig
> - Uses MMC_CAVIUM_OCTEON, drivers/mmc/host/cavium.c
> - Sets host->max_segs to 16 or 1
> - Setting host->disable_bounce to be sure for the 1 case
>
> arch/mips/configs/qi_lb60_defconfig
> - Uses MMC_JZ4740, drivers/mmc/host/jz4740_mmc.c
> - This sets host->max_segs to 128 so bounce buffers are
>   already runtime disabled
> - No action needed for this platform
>
> It would be interesting to come up with a list of the platforms
> that actually end up using bounce buffers. I have not been
> able to infer such a list, but it occurs when
> host->max_segs == 1 and the bounce buffering is not explicitly
> disabled.
>
> Signed-off-by: Linus Walleij 

Thanks, the *series* applied for next! (Responding to patch1 as
couldn't find the cover-letter for v2).

Kind regards
Uffe

> ---
> ChangeLog v1->v2:
> - Instead of adding a new bool "disable_bounce" we use the host
>   caps variable, reuse the free bit 21 to indicate that bounce
>   buffers should be disabled on the host.
> ---
>  drivers/mmc/core/Kconfig  | 18 --
>  drivers/mmc/core/queue.c  | 15 +--
>  drivers/mmc/host/cavium.c |  4 +++-
>  drivers/mmc/host/pxamci.c |  6 +-
>  include/linux/mmc/host.h  |  1 +
>  5 files changed, 10 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
> index fc1ecdaaa9ca..42e89060cd41 100644
> --- a/drivers/mmc/core/Kconfig
> +++ b/drivers/mmc/core/Kconfig
> @@ -61,24 +61,6 @@ config MMC_BLOCK_MINORS
>
>   If unsure, say 8 here.
>
> -config MMC_BLOCK_BOUNCE
> -   bool "Use bounce buffer for simple hosts"
> -   depends on MMC_BLOCK
> -   default y
> -   help
> - SD/MMC is a high latency protocol where it is crucial to
> - send large requests in order to get high performance. Many
> - controllers, however, are restricted to continuous memory
> - (i.e. they can't do scatter-gather), something the kernel
> -