[PATCH 0/2] mmc: sdhci-iproc: Improve bcm2835 performance

[PATCH 0/2] mmc: sdhci-iproc: Improve bcm2835 performance

[Stefan Wahren]

The sdhci-iproc waste a lot performance potential on bcm2835 because of
missing capabilities in the platform data. This patch series tries to fix
this.

Raspberry Pi Zero with a class 10 sdhc card

Before:
dd if=/dev/zero conv=fdatasync of=test bs=8k count=10000
81920000 Bytes (82 MB), 11,0044 s, 7,4 MB/s

sudo dd if=/dev/mmcblk0p1 of=/dev/null
62914560 Bytes (63 MB), 5,83784 s, 10,8 MB/s

After:
dd if=/dev/zero conv=fdatasync of=test bs=8k count=10000
81920000 Bytes (82 MB), 9,76938 s, 8,4 MB/s

sudo dd if=/dev/mmcblk0p1 of=/dev/null
62914560 Bytes (63 MB), 4,73651 s, 13,3 MB/s

Raspberry Pi Compute Module

Before:
dd if=/dev/zero conv=fdatasync of=test bs=8k count=10000
81920000 Bytes (82 MB), 27,4257 s, 3,0 MB/s

sudo dd if=/dev/mmcblk0p1 of=/dev/null
66060288 Bytes (66 MB), 21,4365 s, 3,1 MB/s

After:
dd if=/dev/zero conv=fdatasync of=test bs=8k count=10000
81920000 Bytes (82 MB), 7,19661 s, 11,4 MB/s

sudo dd if=/dev/mmcblk0p1 of=/dev/null
66060288 Bytes (66 MB), 4,76453 s, 13,9 MB/s

Any tests with a Raspberry Pi 3 (SD and Wifi over SDIO) are welcome.

Stefan Wahren (2):
  mmc: sdhci-iproc: Apply caps from bcm2835-mmc driver
  mmc: sdhci-iproc: Increase max_blk_size for bcm2835

 drivers/mmc/host/sdhci-iproc.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] mmc: sdhci-iproc: Apply caps from bcm2835-mmc driver

[Stefan Wahren]

Since the mmc module on bcm2835 neither provide a capabilities register nor 
free documentation we must rely on the downstream implementation [1].

So enable the following capabilities for bcm2835:

MMC_CAP_MMC_HIGHSPEED
MMC_CAP_SD_HIGHSPEED
MMC_CAP_DRIVER_TYPE_A
MMC_CAP_DRIVER_TYPE_C

[1] - https://github.com/raspberrypi/linux/blob/rpi-4.4.y/drivers/mmc/host/bcm2835-mmc.c

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/mmc/host/sdhci-iproc.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index d7046d6..30b3fdf 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -211,14 +211,17 @@ static void sdhci_iproc_writeb(struct sdhci_host *host, u8 val, int reg)
 static const struct sdhci_pltfm_data sdhci_bcm2835_pltfm_data = {
 	.quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
 		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
-		  SDHCI_QUIRK_MISSING_CAPS,
+		  SDHCI_QUIRK_MISSING_CAPS |
+		  SDHCI_QUIRK_NO_HISPD_BIT,
 	.ops = &sdhci_iproc_32only_ops,
 };
 
 static const struct sdhci_iproc_data bcm2835_data = {
 	.pdata = &sdhci_bcm2835_pltfm_data,
-	.caps = SDHCI_CAN_VDD_330,
-	.caps1 = 0x00000000,
+	.caps = SDHCI_CAN_VDD_330 |
+		SDHCI_CAN_DO_HISPD,
+	.caps1 = SDHCI_DRIVER_TYPE_A |
+		 SDHCI_DRIVER_TYPE_C,
 	.mmc_caps = 0x00000000,
 };
 
-- 
1.7.9.5

Re: [PATCH 1/2] mmc: sdhci-iproc: Apply caps from bcm2835-mmc driver

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

Stefan Wahren <stefan.wahren@i2se.com> writes:

> Since the mmc module on bcm2835 neither provide a capabilities register nor 
> free documentation we must rely on the downstream implementation [1].

Yeah, caps reg is wired to 0, but it can do high speed.

Since we're VDD_330 only, I'm not sure the driver type flags do
anything, but I'm still good with having consistency.

Reviewed-by: Eric Anholt <eric@anholt.net>

Interested in porting over the DMA support? :)

I also noticed that the emmc wrapper module claims it can run with a
clock from 50-100mhz, and the arasan says it can run at up to 50, 52, or
208mhz depending on SD/MMC mode.  Could we get a boost from running the
module at a higher clock rate?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 1/2] mmc: sdhci-iproc: Apply caps from bcm2835-mmc driver

[Stefan Wahren]

Hi Eric,

> Eric Anholt <eric@anholt.net> hat am 3. Januar 2017 um 19:27 geschrieben:
> 
> 
> Stefan Wahren <stefan.wahren@i2se.com> writes:
> 
> > Since the mmc module on bcm2835 neither provide a capabilities register nor 
> > free documentation we must rely on the downstream implementation [1].
> 
> Yeah, caps reg is wired to 0, but it can do high speed.
> 
> Since we're VDD_330 only, I'm not sure the driver type flags do
> anything, but I'm still good with having consistency.

yes, the driver type flags currently have no benefit.

> 
> Reviewed-by: Eric Anholt <eric@anholt.net>
> 
> Interested in porting over the DMA support? :)

AFAIK the sdhci doesn't support external DMA (required for bcm2835).
So DMA support would result in a new driver, which i'm currently not interested.

But i've prepared a fixup patch series for the sdhost driver i want to send to Gerd.

> 
> I also noticed that the emmc wrapper module claims it can run with a
> clock from 50-100mhz, and the arasan says it can run at up to 50, 52, or
> 208mhz depending on SD/MMC mode.  Could we get a boost from running the
> module at a higher clock rate?

Sorry, i can't follow. Do you mean DDR mode?

Re: [PATCH 1/2] mmc: sdhci-iproc: Apply caps from bcm2835-mmc driver

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]

Stefan Wahren <stefan.wahren@i2se.com> writes:

> Hi Eric,
>
>> Eric Anholt <eric@anholt.net> hat am 3. Januar 2017 um 19:27 geschrieben:
>> 
>> 
>> Stefan Wahren <stefan.wahren@i2se.com> writes:
>> 
>> > Since the mmc module on bcm2835 neither provide a capabilities register nor 
>> > free documentation we must rely on the downstream implementation [1].
>> 
>> Yeah, caps reg is wired to 0, but it can do high speed.
>> 
>> Since we're VDD_330 only, I'm not sure the driver type flags do
>> anything, but I'm still good with having consistency.
>
> yes, the driver type flags currently have no benefit.
>
>> 
>> Reviewed-by: Eric Anholt <eric@anholt.net>
>> 
>> Interested in porting over the DMA support? :)
>
> AFAIK the sdhci doesn't support external DMA (required for bcm2835).
> So DMA support would result in a new driver, which i'm currently not interested.
>
> But i've prepared a fixup patch series for the sdhost driver i want to send to Gerd.
>
>> 
>> I also noticed that the emmc wrapper module claims it can run with a
>> clock from 50-100mhz, and the arasan says it can run at up to 50, 52, or
>> 208mhz depending on SD/MMC mode.  Could we get a boost from running the
>> module at a higher clock rate?
>
> Sorry, i can't follow. Do you mean DDR mode?

Apparently I what I was reading about was SDR104 mode, which also
requires 1.8V.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 1/2] mmc: sdhci-iproc: Apply caps from bcm2835-mmc driver

[Adrian Hunter]

On 30/12/16 17:24, Stefan Wahren wrote:
> Since the mmc module on bcm2835 neither provide a capabilities register nor 
> free documentation we must rely on the downstream implementation [1].
> 
> So enable the following capabilities for bcm2835:
> 
> MMC_CAP_MMC_HIGHSPEED
> MMC_CAP_SD_HIGHSPEED
> MMC_CAP_DRIVER_TYPE_A
> MMC_CAP_DRIVER_TYPE_C
> 
> [1] - https://github.com/raspberrypi/linux/blob/rpi-4.4.y/drivers/mmc/host/bcm2835-mmc.c
> 
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-iproc.c |    9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index d7046d6..30b3fdf 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -211,14 +211,17 @@ static void sdhci_iproc_writeb(struct sdhci_host *host, u8 val, int reg)
>  static const struct sdhci_pltfm_data sdhci_bcm2835_pltfm_data = {
>  	.quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
>  		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> -		  SDHCI_QUIRK_MISSING_CAPS,
> +		  SDHCI_QUIRK_MISSING_CAPS |
> +		  SDHCI_QUIRK_NO_HISPD_BIT,
>  	.ops = &sdhci_iproc_32only_ops,
>  };
>  
>  static const struct sdhci_iproc_data bcm2835_data = {
>  	.pdata = &sdhci_bcm2835_pltfm_data,
> -	.caps = SDHCI_CAN_VDD_330,
> -	.caps1 = 0x00000000,
> +	.caps = SDHCI_CAN_VDD_330 |
> +		SDHCI_CAN_DO_HISPD,
> +	.caps1 = SDHCI_DRIVER_TYPE_A |
> +		 SDHCI_DRIVER_TYPE_C,
>  	.mmc_caps = 0x00000000,
>  };
>  
> 

[PATCH 2/2] mmc: sdhci-iproc: Increase max_blk_size for bcm2835

[Stefan Wahren]

According to the BCM2835 datasheet the maximum block size for the
eMMC module is restricted to the internal data FIFO which is 1024 byte.
But this is still an improvement to the default of 512 byte.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/mmc/host/sdhci-iproc.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index 30b3fdf..3275d49 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -218,7 +218,9 @@ static void sdhci_iproc_writeb(struct sdhci_host *host, u8 val, int reg)
 
 static const struct sdhci_iproc_data bcm2835_data = {
 	.pdata = &sdhci_bcm2835_pltfm_data,
-	.caps = SDHCI_CAN_VDD_330 |
+	.caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT)
+			& SDHCI_MAX_BLOCK_MASK) |
+		SDHCI_CAN_VDD_330 |
 		SDHCI_CAN_DO_HISPD,
 	.caps1 = SDHCI_DRIVER_TYPE_A |
 		 SDHCI_DRIVER_TYPE_C,
-- 
1.7.9.5

Re: [PATCH 2/2] mmc: sdhci-iproc: Increase max_blk_size for bcm2835

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 340 bytes --]

Stefan Wahren <stefan.wahren@i2se.com> writes:

> According to the BCM2835 datasheet the maximum block size for the
> eMMC module is restricted to the internal data FIFO which is 1024 byte.
> But this is still an improvement to the default of 512 byte.

Confirmed that the internal FIFOs are 1k.

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 2/2] mmc: sdhci-iproc: Increase max_blk_size for bcm2835

[Scott Branden]

On 17-01-03 10:04 AM, Eric Anholt wrote:
> Stefan Wahren <stefan.wahren@i2se.com> writes:
>
>> According to the BCM2835 datasheet the maximum block size for the
>> eMMC module is restricted to the internal data FIFO which is 1024 byte.
>> But this is still an improvement to the default of 512 byte.
>
> Confirmed that the internal FIFOs are 1k.
>
> Reviewed-by: Eric Anholt <eric@anholt.net>
>
Acked-by: Scott Branden <scott.branden@broadcom.com>

Re: [PATCH 2/2] mmc: sdhci-iproc: Increase max_blk_size for bcm2835

[Adrian Hunter]

On 30/12/16 17:24, Stefan Wahren wrote:
> According to the BCM2835 datasheet the maximum block size for the
> eMMC module is restricted to the internal data FIFO which is 1024 byte.
> But this is still an improvement to the default of 512 byte.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-iproc.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index 30b3fdf..3275d49 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -218,7 +218,9 @@ static void sdhci_iproc_writeb(struct sdhci_host *host, u8 val, int reg)
>  
>  static const struct sdhci_iproc_data bcm2835_data = {
>  	.pdata = &sdhci_bcm2835_pltfm_data,
> -	.caps = SDHCI_CAN_VDD_330 |
> +	.caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT)
> +			& SDHCI_MAX_BLOCK_MASK) |
> +		SDHCI_CAN_VDD_330 |
>  		SDHCI_CAN_DO_HISPD,
>  	.caps1 = SDHCI_DRIVER_TYPE_A |
>  		 SDHCI_DRIVER_TYPE_C,
> 

Re: [PATCH 0/2] mmc: sdhci-iproc: Improve bcm2835 performance

[Ulf Hansson]

On 30 December 2016 at 16:24, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> The sdhci-iproc waste a lot performance potential on bcm2835 because of
> missing capabilities in the platform data. This patch series tries to fix
> this.
>
> Raspberry Pi Zero with a class 10 sdhc card
>
> Before:
> dd if=/dev/zero conv=fdatasync of=test bs=8k count=10000
> 81920000 Bytes (82 MB), 11,0044 s, 7,4 MB/s
>
> sudo dd if=/dev/mmcblk0p1 of=/dev/null
> 62914560 Bytes (63 MB), 5,83784 s, 10,8 MB/s
>
> After:
> dd if=/dev/zero conv=fdatasync of=test bs=8k count=10000
> 81920000 Bytes (82 MB), 9,76938 s, 8,4 MB/s
>
> sudo dd if=/dev/mmcblk0p1 of=/dev/null
> 62914560 Bytes (63 MB), 4,73651 s, 13,3 MB/s
>
> Raspberry Pi Compute Module
>
> Before:
> dd if=/dev/zero conv=fdatasync of=test bs=8k count=10000
> 81920000 Bytes (82 MB), 27,4257 s, 3,0 MB/s
>
> sudo dd if=/dev/mmcblk0p1 of=/dev/null
> 66060288 Bytes (66 MB), 21,4365 s, 3,1 MB/s
>
> After:
> dd if=/dev/zero conv=fdatasync of=test bs=8k count=10000
> 81920000 Bytes (82 MB), 7,19661 s, 11,4 MB/s
>
> sudo dd if=/dev/mmcblk0p1 of=/dev/null
> 66060288 Bytes (66 MB), 4,76453 s, 13,9 MB/s
>
> Any tests with a Raspberry Pi 3 (SD and Wifi over SDIO) are welcome.
>
> Stefan Wahren (2):
>   mmc: sdhci-iproc: Apply caps from bcm2835-mmc driver
>   mmc: sdhci-iproc: Increase max_blk_size for bcm2835
>
>  drivers/mmc/host/sdhci-iproc.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> --
> 1.7.9.5
>

Thanks, applied for next!

Kind regards
Uffe