mmc: dw_mmc: fix pio mode when internal dmac is enabled

PATCH Applied Heiko Stübner 3 participants 6 messages Aug 3, 2015 – Aug 17, 2015

Subsystems (2)

MMC, SD and SDIO Synopsys DesignWare MMC/SD/SDIO driver

lore

Share & tools

lore

Heiko Stübner

author

2015-08-03 15:04 UTC #0 lore raw

PATCH 1/1 mmc: dw_mmc: fix pio mode when internal dmac is enabled

The dw_mci_init_dma() may decide to not use dma, but pio instead, caused by things like wrong dma settings in the system.

Till now the code dw_mci_init_slot() always assumed that dma is available when CONFIG_MMC_DW_IDMAC was defined, ignoring the host->use_dma var set during dma init.

So when now the dma init failed for whatever reason, the transfer sizes would still be set for dma transfers, especially including the maximum block-count calculated from host->ring_size and resulting in a

[    4.991109] ------------[ cut here ]------------
[    4.991111] kernel BUG at drivers/mmc/core/core.c:256!
[    4.991113] Internal error: Oops - BUG: 0 [#1] SMP ARM

because host->ring_size is 0 in this case and the slot init code uses the wrong code to calculate the values.

Fix this by selecting the correct calculations using the host->use_dma variable instead of the CONFIG_MMC_DW_IDMAC config option.

Signed-off-by Heiko Stuebner <[email protected]>

---
 drivers/mmc/host/dw_mmc.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

drivers/mmc/host/dw_mmc.c +14 ~~-13~~

index 40e9d8e..9ec3521 100644
@@ -2391,19 +2391,20 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 		mmc->max_seg_size = host->pdata->blk_settings->max_seg_size;
 	} else {
 		/* Useful defaults if platform data is unset. */
-#ifdef CONFIG_MMC_DW_IDMAC
-		mmc->max_segs = host->ring_size;
-		mmc->max_blk_size = 65536;
-		mmc->max_seg_size = 0x1000;
-		mmc->max_req_size = mmc->max_seg_size * host->ring_size;
-		mmc->max_blk_count = mmc->max_req_size / 512;
-#else
-		mmc->max_segs = 64;
-		mmc->max_blk_size = 65536; /* BLKSIZ is 16 bits */
-		mmc->max_blk_count = 512;
-		mmc->max_req_size = mmc->max_blk_size * mmc->max_blk_count;
-		mmc->max_seg_size = mmc->max_req_size;
-#endif /* CONFIG_MMC_DW_IDMAC */
+		if (host->use_dma) {
+			mmc->max_segs = host->ring_size;
+			mmc->max_blk_size = 65536;
+			mmc->max_seg_size = 0x1000;
+			mmc->max_req_size = mmc->max_seg_size * host->ring_size;
+			mmc->max_blk_count = mmc->max_req_size / 512;
+		} else {
+			mmc->max_segs = 64;
+			mmc->max_blk_size = 65536; /* BLKSIZ is 16 bits */
+			mmc->max_blk_count = 512;
+			mmc->max_req_size = mmc->max_blk_size *
+					    mmc->max_blk_count;
+			mmc->max_seg_size = mmc->max_req_size;
+		}
 	}
 
 	if (dw_mci_get_cd(mmc))

Signature · 1 line

2.1.4

Jaehoon Chung

maintainer Applied

2015-08-06 01:48 UTC #1 lore raw

Hi, Heiko.

Applied this patch at my dw-mmc tree.
I will request pull on this weekend.
Thanks a lot!

Best Regards,
Jaehoon Chung

Quoted 65 lines · Heiko Stübner

The dw_mci_init_dma() may decide to not use dma, but pio instead, caused by things like wrong dma settings in the system.
Till now the code dw_mci_init_slot() always assumed that dma is available when CONFIG_MMC_DW_IDMAC was defined, ignoring the host->use_dma var set during dma init.
So when now the dma init failed for whatever reason, the transfer sizes would still be set for dma transfers, especially including the maximum block-count calculated from host->ring_size and resulting in a
[    4.991109] ------------[ cut here ]------------
[    4.991111] kernel BUG at drivers/mmc/core/core.c:256!
[    4.991113] Internal error: Oops - BUG: 0 [#1] SMP ARM
because host->ring_size is 0 in this case and the slot init code uses the wrong code to calculate the values.
Fix this by selecting the correct calculations using the host->use_dma variable instead of the CONFIG_MMC_DW_IDMAC config option.
Signed-off-by Heiko Stuebner <[email protected]>
---
 drivers/mmc/host/dw_mmc.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)
drivers/mmc/host/dw_mmc.c +14 ~~-13~~
index 40e9d8e..9ec3521 100644
@@ -2391,19 +2391,20 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 		mmc->max_seg_size = host->pdata->blk_settings->max_seg_size;
 	} else {
 		/* Useful defaults if platform data is unset. */
-#ifdef CONFIG_MMC_DW_IDMAC
-		mmc->max_segs = host->ring_size;
-		mmc->max_blk_size = 65536;
-		mmc->max_seg_size = 0x1000;
-		mmc->max_req_size = mmc->max_seg_size * host->ring_size;
-		mmc->max_blk_count = mmc->max_req_size / 512;
-#else
-		mmc->max_segs = 64;
-		mmc->max_blk_size = 65536; /* BLKSIZ is 16 bits */
-		mmc->max_blk_count = 512;
-		mmc->max_req_size = mmc->max_blk_size * mmc->max_blk_count;
-		mmc->max_seg_size = mmc->max_req_size;
-#endif /* CONFIG_MMC_DW_IDMAC */
+		if (host->use_dma) {
+			mmc->max_segs = host->ring_size;
+			mmc->max_blk_size = 65536;
+			mmc->max_seg_size = 0x1000;
+			mmc->max_req_size = mmc->max_seg_size * host->ring_size;
+			mmc->max_blk_count = mmc->max_req_size / 512;
+		} else {
+			mmc->max_segs = 64;
+			mmc->max_blk_size = 65536; /* BLKSIZ is 16 bits */
+			mmc->max_blk_count = 512;
+			mmc->max_req_size = mmc->max_blk_size *
+					    mmc->max_blk_count;
+			mmc->max_seg_size = mmc->max_req_size;
+		}
 	}
 
 	if (dw_mci_get_cd(mmc))

Ulf Hansson

2015-08-17 10:16 UTC #2 lore raw

In reply to Heiko Stübner

Quoted 39 lines · Heiko Stübner

The dw_mci_init_dma() may decide to not use dma, but pio instead, caused by things like wrong dma settings in the system.
Till now the code dw_mci_init_slot() always assumed that dma is available when CONFIG_MMC_DW_IDMAC was defined, ignoring the host->use_dma var set during dma init.
So when now the dma init failed for whatever reason, the transfer sizes would still be set for dma transfers, especially including the maximum block-count calculated from host->ring_size and resulting in a
[    4.991109] ------------[ cut here ]------------
[    4.991111] kernel BUG at drivers/mmc/core/core.c:256!
[    4.991113] Internal error: Oops - BUG: 0 [#1] SMP ARM
because host->ring_size is 0 in this case and the slot init code uses the wrong code to calculate the values.
Fix this by selecting the correct calculations using the host->use_dma variable instead of the CONFIG_MMC_DW_IDMAC config option.
Signed-off-by Heiko Stuebner <[email protected]>
---
 drivers/mmc/host/dw_mmc.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)
drivers/mmc/host/dw_mmc.c +0 -5
index 40e9d8e..9ec3521 100644
@@ -2391,19 +2391,20 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
                mmc->max_seg_size = host->pdata->blk_settings->max_seg_size;
        } else {
                /* Useful defaults if platform data is unset. */
-#ifdef CONFIG_MMC_DW_IDMAC
-               mmc->max_segs = host->ring_size;
-               mmc->max_blk_size = 65536;
-               mmc->max_seg_size = 0x1000;
-               mmc->max_req_size = mmc->max_seg_size * host->ring_size;

drivers/mmc/host/dw_mmc.c static int dw_mci_init_slot(struct dw_mci *host, unsigned int id) · line 2395

Quoted 10 lines

drivers/mmc/host/dw_mmc.c +2 -8

-               mmc->max_blk_count = mmc->max_req_size / 512;
-#else
-               mmc->max_segs = 64;
-               mmc->max_blk_size = 65536; /* BLKSIZ is 16 bits */
-               mmc->max_blk_count = 512;
-               mmc->max_req_size = mmc->max_blk_size * mmc->max_blk_count;
-               mmc->max_seg_size = mmc->max_req_size;
-#endif /* CONFIG_MMC_DW_IDMAC */
+               if (host->use_dma) {
+                       mmc->max_segs = host->ring_size;

I expect this may cause a compiler error since host->ring_size is only available in the struct dw_mci *host when CONFIG_MMC_DW_IDMAC is set.

I have already pulled in this patch from Jaehoon's pull request. Perhaps I should only amend the patch and change the host->ring_size to be always available no matter if CONFIG_MMC_DW_IDMAC is set or not?

Quoted 16 lines

drivers/mmc/host/dw_mmc.c +12 -1

+                       mmc->max_blk_size = 65536;
+                       mmc->max_seg_size = 0x1000;
+                       mmc->max_req_size = mmc->max_seg_size * host->ring_size;
+                       mmc->max_blk_count = mmc->max_req_size / 512;
+               } else {
+                       mmc->max_segs = 64;
+                       mmc->max_blk_size = 65536; /* BLKSIZ is 16 bits */
+                       mmc->max_blk_count = 512;
+                       mmc->max_req_size = mmc->max_blk_size *
+                                           mmc->max_blk_count;
+                       mmc->max_seg_size = mmc->max_req_size;
+               }
        }
 
        if (dw_mci_get_cd(mmc))
--

Quoted 3 lines

2.1.4

Kind regards
Uffe

Jaehoon Chung

maintainer

2015-08-17 10:21 UTC #3 lore raw

Hi, Ulf.

Quoted 54 lines · Ulf Hansson

Quoted 39 lines · Heiko Stübner
The dw_mci_init_dma() may decide to not use dma, but pio instead, caused by things like wrong dma settings in the system.
Till now the code dw_mci_init_slot() always assumed that dma is available when CONFIG_MMC_DW_IDMAC was defined, ignoring the host->use_dma var set during dma init.
So when now the dma init failed for whatever reason, the transfer sizes would still be set for dma transfers, especially including the maximum block-count calculated from host->ring_size and resulting in a
[    4.991109] ------------[ cut here ]------------
[    4.991111] kernel BUG at drivers/mmc/core/core.c:256!
[    4.991113] Internal error: Oops - BUG: 0 [#1] SMP ARM
because host->ring_size is 0 in this case and the slot init code uses the wrong code to calculate the values.
Fix this by selecting the correct calculations using the host->use_dma variable instead of the CONFIG_MMC_DW_IDMAC config option.
Signed-off-by Heiko Stuebner <[email protected]>
---
 drivers/mmc/host/dw_mmc.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)
drivers/mmc/host/dw_mmc.c +0 -5
index 40e9d8e..9ec3521 100644
@@ -2391,19 +2391,20 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
                mmc->max_seg_size = host->pdata->blk_settings->max_seg_size;
        } else {
                /* Useful defaults if platform data is unset. */
-#ifdef CONFIG_MMC_DW_IDMAC
-               mmc->max_segs = host->ring_size;
-               mmc->max_blk_size = 65536;
-               mmc->max_seg_size = 0x1000;
-               mmc->max_req_size = mmc->max_seg_size * host->ring_size;
drivers/mmc/host/dw_mmc.c static int dw_mci_init_slot(struct dw_mci *host, unsigned int id) · line 2395
Quoted 10 lines
drivers/mmc/host/dw_mmc.c +2 -8
-               mmc->max_blk_count = mmc->max_req_size / 512;
-#else
-               mmc->max_segs = 64;
-               mmc->max_blk_size = 65536; /* BLKSIZ is 16 bits */
-               mmc->max_blk_count = 512;
-               mmc->max_req_size = mmc->max_blk_size * mmc->max_blk_count;
-               mmc->max_seg_size = mmc->max_req_size;
-#endif /* CONFIG_MMC_DW_IDMAC */
+               if (host->use_dma) {
+                       mmc->max_segs = host->ring_size;
I expect this may cause a compiler error since host->ring_size is only available in the struct dw_mci *host when CONFIG_MMC_DW_IDMAC is set.

Quoted 3 lines

I have already pulled in this patch from Jaehoon's pull request. Perhaps I should only amend the patch and change the host->ring_size to be always available no matter if CONFIG_MMC_DW_IDMAC is set or not?

Sorry for this. if you can, i think good that CONFIG_MMC_DW_IDMAC is removed at struct dw_mci. Could you amend it?
If you want to get patch, i will send patch at now.

Best Regards,
Jaehoon Chung

Quoted 24 lines

Quoted 17 lines

drivers/mmc/host/dw_mmc.c +12 -1

+                       mmc->max_blk_size = 65536;
+                       mmc->max_seg_size = 0x1000;
+                       mmc->max_req_size = mmc->max_seg_size * host->ring_size;
+                       mmc->max_blk_count = mmc->max_req_size / 512;
+               } else {
+                       mmc->max_segs = 64;
+                       mmc->max_blk_size = 65536; /* BLKSIZ is 16 bits */
+                       mmc->max_blk_count = 512;
+                       mmc->max_req_size = mmc->max_blk_size *
+                                           mmc->max_blk_count;
+                       mmc->max_seg_size = mmc->max_req_size;
+               }
        }
 
        if (dw_mci_get_cd(mmc))
--

Quoted 3 lines

2.1.4

Kind regards
Uffe

Ulf Hansson

2015-08-17 10:34 UTC #4 lore raw

[...]

Quoted 8 lines

Quoted 8 lines
drivers/mmc/host/dw_mmc.c static int dw_mci_init_slot(struct dw_mci *host, unsigned int id) · line 2395
Quoted 4 lines
drivers/mmc/host/dw_mmc.c +2 -2
-               mmc->max_seg_size = mmc->max_req_size;
-#endif /* CONFIG_MMC_DW_IDMAC */
+               if (host->use_dma) {
+                       mmc->max_segs = host->ring_size;
I expect this may cause a compiler error since host->ring_size is only available in the struct dw_mci *host when CONFIG_MMC_DW_IDMAC is set.

Quoted 7 lines

Quoted 3 lines
I have already pulled in this patch from Jaehoon's pull request. Perhaps I should only amend the patch and change the host->ring_size to be always available no matter if CONFIG_MMC_DW_IDMAC is set or not?
Sorry for this. if you can, i think good that CONFIG_MMC_DW_IDMAC is removed at struct dw_mci. Could you amend it?
If you want to get patch, i will send patch at now.

No worries, I amend the patch myself.

Kind regards
Uffe

Heiko Stuebner

author

2015-08-17 10:37 UTC #5 lore raw

Am Montag, 17. August 2015, 12:34:09 schrieb Ulf Hansson:

Quoted 14 lines

[...]
Quoted 12 lines
Quoted 11 lines
drivers/mmc/host/dw_mmc.c static int dw_mci_init_slot(struct dw_mci *host, unsigned int id) · line 2395
Quoted 4 lines
drivers/mmc/host/dw_mmc.c +2 -2
-               mmc->max_seg_size = mmc->max_req_size;
-#endif /* CONFIG_MMC_DW_IDMAC */
+               if (host->use_dma) {
+                       mmc->max_segs = host->ring_size;
I expect this may cause a compiler error since host->ring_size is only available in the struct dw_mci *host when CONFIG_MMC_DW_IDMAC is set.
I have already pulled in this patch from Jaehoon's pull request. Perhaps I should only amend the patch and change the host->ring_size to be always available no matter if CONFIG_MMC_DW_IDMAC is set or not?

Quoted 5 lines

Quoted 3 lines
Sorry for this. if you can, i think good that CONFIG_MMC_DW_IDMAC is removed at struct dw_mci. Could you amend it?
If you want to get patch, i will send patch at now.
No worries, I amend the patch myself.

thanks and sorry for missing that bit.

Heiko