* [PATCH] mmc: dw_mmc: handle data blocks > than 4kB if IDMAC is used
@ 2015-06-25 8:25 Alexey Brodkin
2015-07-01 10:02 ` Alexey Brodkin
2015-07-08 4:14 ` Jaehoon Chung
0 siblings, 2 replies; 9+ messages in thread
From: Alexey Brodkin @ 2015-06-25 8:25 UTC (permalink / raw)
To: linux-mmc
Cc: Alexey Brodkin, Seungwon Jeon, Jaehoon Chung, Ulf Hansson,
arc-linux-dev, linux-kernel
As per DW MobileStorage databook "each descriptor can transfer up to 4kB
of data in chained mode", moreover buffer size that is put in "des1" is
limited to 13 bits, i.e. for example on attempt to
IDMAC_SET_BUFFER1_SIZE(desc, 8192) size value that's effectively written
will be 0.
On the platform with 8kB PAGE_SIZE I see dw_mmc gets data blocks in
SG-list of 8kB size and that leads to unpredictable behavior of the
SD/MMC controller.
In particular on write to FAT partition of SD-card the controller will
stuck in the middle of DMA transaction.
Solution to the problem is simple - we need to pass large (> 4kB) data
buffers to the controller via multiple descriptors. And that's what
that change does.
What's interesting I did try original driver on same platform but
configured with 4kB PAGE_SIZE and may confirm that data blocks passed
in SG-list to dw_mmc never exeed 4kB limit - that explains why nobody
ever faced a problem I did.
Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Seungwon Jeon <tgih.jun@samsung.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: arc-linux-dev@synopsys.com
Cc: linux-kernel@vger.kernel.org
---
drivers/mmc/host/dw_mmc.c | 109 ++++++++++++++++++++++++++++++----------------
1 file changed, 71 insertions(+), 38 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 40e9d8e..e41fb74 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -99,6 +99,9 @@ struct idmac_desc {
__le32 des3; /* buffer 2 physical address */
};
+
+/* Each descriptor can transfer up to 4KB of data in chained mode */
+#define DW_MCI_DESC_DATA_LENGTH 0x1000
#endif /* CONFIG_MMC_DW_IDMAC */
static bool dw_mci_reset(struct dw_mci *host);
@@ -462,66 +465,96 @@ static void dw_mci_idmac_complete_dma(struct dw_mci *host)
static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
unsigned int sg_len)
{
+ unsigned int desc_len;
int i;
if (host->dma_64bit_address == 1) {
- struct idmac_desc_64addr *desc = host->sg_cpu;
+ struct idmac_desc_64addr *desc_first, *desc_last, *desc;
+
+ desc_first = desc_last = desc = host->sg_cpu;
- for (i = 0; i < sg_len; i++, desc++) {
+ for (i = 0; i < sg_len; i++) {
unsigned int length = sg_dma_len(&data->sg[i]);
u64 mem_addr = sg_dma_address(&data->sg[i]);
- /*
- * Set the OWN bit and disable interrupts for this
- * descriptor
- */
- desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC |
- IDMAC_DES0_CH;
- /* Buffer length */
- IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, length);
-
- /* Physical address to DMA to/from */
- desc->des4 = mem_addr & 0xffffffff;
- desc->des5 = mem_addr >> 32;
+ for ( ; length ; desc++) {
+ desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ?
+ length : DW_MCI_DESC_DATA_LENGTH;
+
+ length -= desc_len;
+
+ /*
+ * Set the OWN bit and disable interrupts
+ * for this descriptor
+ */
+ desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC |
+ IDMAC_DES0_CH;
+
+ /* Buffer length */
+ IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, desc_len);
+
+ /* Physical address to DMA to/from */
+ desc->des4 = mem_addr & 0xffffffff;
+ desc->des5 = mem_addr >> 32;
+
+ /* Update physical address for the next desc */
+ mem_addr += desc_len;
+
+ /* Save pointer to the last descriptor */
+ desc_last = desc;
+ }
}
/* Set first descriptor */
- desc = host->sg_cpu;
- desc->des0 |= IDMAC_DES0_FD;
+ desc_first->des0 |= IDMAC_DES0_FD;
/* Set last descriptor */
- desc = host->sg_cpu + (i - 1) *
- sizeof(struct idmac_desc_64addr);
- desc->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC);
- desc->des0 |= IDMAC_DES0_LD;
+ desc_last->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC);
+ desc_last->des0 |= IDMAC_DES0_LD;
} else {
- struct idmac_desc *desc = host->sg_cpu;
+ struct idmac_desc *desc_first, *desc_last, *desc;
+
+ desc_first = desc_last = desc = host->sg_cpu;
- for (i = 0; i < sg_len; i++, desc++) {
+ for (i = 0; i < sg_len; i++) {
unsigned int length = sg_dma_len(&data->sg[i]);
u32 mem_addr = sg_dma_address(&data->sg[i]);
- /*
- * Set the OWN bit and disable interrupts for this
- * descriptor
- */
- desc->des0 = cpu_to_le32(IDMAC_DES0_OWN |
- IDMAC_DES0_DIC | IDMAC_DES0_CH);
- /* Buffer length */
- IDMAC_SET_BUFFER1_SIZE(desc, length);
+ for ( ; length ; desc++) {
+ desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ?
+ length : DW_MCI_DESC_DATA_LENGTH;
+
+ length -= desc_len;
+
+ /*
+ * Set the OWN bit and disable interrupts
+ * for this descriptor
+ */
+ desc->des0 = cpu_to_le32(IDMAC_DES0_OWN |
+ IDMAC_DES0_DIC |
+ IDMAC_DES0_CH);
+
+ /* Buffer length */
+ IDMAC_SET_BUFFER1_SIZE(desc, desc_len);
- /* Physical address to DMA to/from */
- desc->des2 = cpu_to_le32(mem_addr);
+ /* Physical address to DMA to/from */
+ desc->des2 = cpu_to_le32(mem_addr);
+
+ /* Update physical address for the next desc */
+ mem_addr += desc_len;
+
+ /* Save pointer to the last descriptor */
+ desc_last = desc;
+ }
}
/* Set first descriptor */
- desc = host->sg_cpu;
- desc->des0 |= cpu_to_le32(IDMAC_DES0_FD);
+ desc_first->des0 |= cpu_to_le32(IDMAC_DES0_FD);
/* Set last descriptor */
- desc = host->sg_cpu + (i - 1) * sizeof(struct idmac_desc);
- desc->des0 &= cpu_to_le32(~(IDMAC_DES0_CH | IDMAC_DES0_DIC));
- desc->des0 |= cpu_to_le32(IDMAC_DES0_LD);
+ desc_last->des0 &= cpu_to_le32(~(IDMAC_DES0_CH |
+ IDMAC_DES0_DIC));
+ desc_last->des0 |= cpu_to_le32(IDMAC_DES0_LD);
}
wmb();
@@ -2394,7 +2427,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
#ifdef CONFIG_MMC_DW_IDMAC
mmc->max_segs = host->ring_size;
mmc->max_blk_size = 65536;
- mmc->max_seg_size = 0x1000;
+ mmc->max_seg_size = DW_MCI_DESC_DATA_LENGTH;
mmc->max_req_size = mmc->max_seg_size * host->ring_size;
mmc->max_blk_count = mmc->max_req_size / 512;
#else
--
2.4.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] mmc: dw_mmc: handle data blocks > than 4kB if IDMAC is used
2015-06-25 8:25 [PATCH] mmc: dw_mmc: handle data blocks > than 4kB if IDMAC is used Alexey Brodkin
@ 2015-07-01 10:02 ` Alexey Brodkin
2015-07-01 10:13 ` Jaehoon Chung
2015-07-01 10:15 ` Vineet Gupta
2015-07-08 4:14 ` Jaehoon Chung
1 sibling, 2 replies; 9+ messages in thread
From: Alexey Brodkin @ 2015-07-01 10:02 UTC (permalink / raw)
To: ulf.hansson, jh80.chung
Cc: linux-kernel, linux-mmc, tgih.jun, arc-linux-dev, Vineet Gupta
Hi Jaehoon, Seungwon, Ulf,
On Thu, 2015-06-25 at 11:25 +0300, Alexey Brodkin wrote:
> As per DW MobileStorage databook "each descriptor can transfer up to
> 4kB
> of data in chained mode", moreover buffer size that is put in "des1"
> is
> limited to 13 bits, i.e. for example on attempt to
> IDMAC_SET_BUFFER1_SIZE(desc, 8192) size value that's effectively
> written
> will be 0.
>
> On the platform with 8kB PAGE_SIZE I see dw_mmc gets data blocks in
> SG-list of 8kB size and that leads to unpredictable behavior of the
> SD/MMC controller.
>
> In particular on write to FAT partition of SD-card the controller
> will
> stuck in the middle of DMA transaction.
>
> Solution to the problem is simple - we need to pass large (> 4kB)
> data
> buffers to the controller via multiple descriptors. And that's what
> that change does.
>
> What's interesting I did try original driver on same platform but
> configured with 4kB PAGE_SIZE and may confirm that data blocks passed
> in SG-list to dw_mmc never exeed 4kB limit - that explains why nobody
> ever faced a problem I did.
>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Seungwon Jeon <tgih.jun@samsung.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: arc-linux-dev@synopsys.com
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/mmc/host/dw_mmc.c | 109 ++++++++++++++++++++++++++++++------
> ----------
I'm wondering if there're any comments on that patch or if it could be
applied?
It fixes a real problem on systems on 4K PAGE_SIZE so would be good to
have it in upstream. In particular this is the case with ARC AXS board
which made its way in upstream kernel recently.
Regards,
Alexey
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mmc: dw_mmc: handle data blocks > than 4kB if IDMAC is used
2015-07-01 10:02 ` Alexey Brodkin
@ 2015-07-01 10:13 ` Jaehoon Chung
2015-07-06 7:12 ` Alexey Brodkin
2015-07-01 10:15 ` Vineet Gupta
1 sibling, 1 reply; 9+ messages in thread
From: Jaehoon Chung @ 2015-07-01 10:13 UTC (permalink / raw)
To: Alexey Brodkin, ulf.hansson, jh80.chung
Cc: linux-kernel, linux-mmc, tgih.jun, arc-linux-dev, Vineet Gupta
Hi, Alexey.
Sorry for reviewing late. i missed this patch.
I will check your patch and reply at mailing.
Thanks a lot for noticing this.
Best Regards,
Jaehoon Chung
On 07/01/2015 07:02 PM, Alexey Brodkin wrote:
> Hi Jaehoon, Seungwon, Ulf,
>
> On Thu, 2015-06-25 at 11:25 +0300, Alexey Brodkin wrote:
>> As per DW MobileStorage databook "each descriptor can transfer up to
>> 4kB
>> of data in chained mode", moreover buffer size that is put in "des1"
>> is
>> limited to 13 bits, i.e. for example on attempt to
>> IDMAC_SET_BUFFER1_SIZE(desc, 8192) size value that's effectively
>> written
>> will be 0.
>>
>> On the platform with 8kB PAGE_SIZE I see dw_mmc gets data blocks in
>> SG-list of 8kB size and that leads to unpredictable behavior of the
>> SD/MMC controller.
>>
>> In particular on write to FAT partition of SD-card the controller
>> will
>> stuck in the middle of DMA transaction.
>>
>> Solution to the problem is simple - we need to pass large (> 4kB)
>> data
>> buffers to the controller via multiple descriptors. And that's what
>> that change does.
>>
>> What's interesting I did try original driver on same platform but
>> configured with 4kB PAGE_SIZE and may confirm that data blocks passed
>> in SG-list to dw_mmc never exeed 4kB limit - that explains why nobody
>> ever faced a problem I did.
>>
>> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
>> Cc: Seungwon Jeon <tgih.jun@samsung.com>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> Cc: arc-linux-dev@synopsys.com
>> Cc: linux-kernel@vger.kernel.org
>> ---
>> drivers/mmc/host/dw_mmc.c | 109 ++++++++++++++++++++++++++++++------
>> ----------
>
> I'm wondering if there're any comments on that patch or if it could be
> applied?
>
> It fixes a real problem on systems on 4K PAGE_SIZE so would be good to
> have it in upstream. In particular this is the case with ARC AXS board
> which made its way in upstream kernel recently.
>
> Regards,
> Alexey--
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mmc: dw_mmc: handle data blocks > than 4kB if IDMAC is used
2015-07-01 10:02 ` Alexey Brodkin
2015-07-01 10:13 ` Jaehoon Chung
@ 2015-07-01 10:15 ` Vineet Gupta
1 sibling, 0 replies; 9+ messages in thread
From: Vineet Gupta @ 2015-07-01 10:15 UTC (permalink / raw)
To: Alexey Brodkin, ulf.hansson, jh80.chung
Cc: linux-kernel, linux-mmc, tgih.jun, arc-linux-dev, Vineet Gupta
On Wednesday 01 July 2015 03:32 PM, Alexey Brodkin wrote:
> Hi Jaehoon, Seungwon, Ulf,
>
> On Thu, 2015-06-25 at 11:25 +0300, Alexey Brodkin wrote:
>> > As per DW MobileStorage databook "each descriptor can transfer up to
>> > 4kB
>> > of data in chained mode", moreover buffer size that is put in "des1"
>> > is
>> > limited to 13 bits, i.e. for example on attempt to
>> > IDMAC_SET_BUFFER1_SIZE(desc, 8192) size value that's effectively
>> > written
>> > will be 0.
>> >
>> > On the platform with 8kB PAGE_SIZE I see dw_mmc gets data blocks in
>> > SG-list of 8kB size and that leads to unpredictable behavior of the
>> > SD/MMC controller.
>> >
>> > In particular on write to FAT partition of SD-card the controller
>> > will
>> > stuck in the middle of DMA transaction.
>> >
>> > Solution to the problem is simple - we need to pass large (> 4kB)
>> > data
>> > buffers to the controller via multiple descriptors. And that's what
>> > that change does.
>> >
>> > What's interesting I did try original driver on same platform but
>> > configured with 4kB PAGE_SIZE and may confirm that data blocks passed
>> > in SG-list to dw_mmc never exeed 4kB limit - that explains why nobody
>> > ever faced a problem I did.
>> >
>> > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
>> > Cc: Seungwon Jeon <tgih.jun@samsung.com>
>> > Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> > Cc: arc-linux-dev@synopsys.com
>> > Cc: linux-kernel@vger.kernel.org
>> > ---
>> > drivers/mmc/host/dw_mmc.c | 109 ++++++++++++++++++++++++++++++------
>> > ----------
> I'm wondering if there're any comments on that patch or if it could be
> applied?
>
> It fixes a real problem on systems on 4K PAGE_SIZE so would be good to
s/4K/8k
Alexey fat-fingered it seems. ARC cores by default have 8k MMU page size and we
saw this issue on ARC SDP boards !
-Vineet
> have it in upstream. In particular this is the case with ARC AXS board
> which made its way in upstream kernel recently.
>
> Regards,
> Alexey--
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mmc: dw_mmc: handle data blocks > than 4kB if IDMAC is used
2015-07-01 10:13 ` Jaehoon Chung
@ 2015-07-06 7:12 ` Alexey Brodkin
0 siblings, 0 replies; 9+ messages in thread
From: Alexey Brodkin @ 2015-07-06 7:12 UTC (permalink / raw)
To: jh80.chung
Cc: ulf.hansson, linux-kernel, linux-mmc, tgih.jun, arc-linux-dev,
Vineet.Gupta1
Hi Jaehoon,
On Wed, 2015-07-01 at 19:13 +0900, Jaehoon Chung wrote:
> Hi, Alexey.
>
> Sorry for reviewing late. i missed this patch.
> I will check your patch and reply at mailing.
> Thanks a lot for noticing this.
>
> Best Regards,
> Jaehoon Chung
Please treat this as a polite reminder, would be nice to get some
comments on that patch.
-Alexey
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mmc: dw_mmc: handle data blocks > than 4kB if IDMAC is used
2015-06-25 8:25 [PATCH] mmc: dw_mmc: handle data blocks > than 4kB if IDMAC is used Alexey Brodkin
2015-07-01 10:02 ` Alexey Brodkin
@ 2015-07-08 4:14 ` Jaehoon Chung
2015-07-08 8:45 ` Alexey Brodkin
1 sibling, 1 reply; 9+ messages in thread
From: Jaehoon Chung @ 2015-07-08 4:14 UTC (permalink / raw)
To: Alexey Brodkin, linux-mmc
Cc: Seungwon Jeon, Jaehoon Chung, Ulf Hansson, arc-linux-dev, linux-kernel
Hi, Alexey.
On 06/25/2015 05:25 PM, Alexey Brodkin wrote:
> As per DW MobileStorage databook "each descriptor can transfer up to 4kB
> of data in chained mode", moreover buffer size that is put in "des1" is
> limited to 13 bits, i.e. for example on attempt to
> IDMAC_SET_BUFFER1_SIZE(desc, 8192) size value that's effectively written
> will be 0.
>
> On the platform with 8kB PAGE_SIZE I see dw_mmc gets data blocks in
> SG-list of 8kB size and that leads to unpredictable behavior of the
> SD/MMC controller.
I didn't see your problem, since i didn't test with 8K PAGE_SIZE.
But I think your patch is reasonable.
As possible, I want to know in more detail what unpredictable behavior.
(Just stuck behavior?)
>
> In particular on write to FAT partition of SD-card the controller will
> stuck in the middle of DMA transaction.
>
> Solution to the problem is simple - we need to pass large (> 4kB) data
> buffers to the controller via multiple descriptors. And that's what
> that change does.
>
> What's interesting I did try original driver on same platform but
> configured with 4kB PAGE_SIZE and may confirm that data blocks passed
> in SG-list to dw_mmc never exeed 4kB limit - that explains why nobody
> ever faced a problem I did.
>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Seungwon Jeon <tgih.jun@samsung.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: arc-linux-dev@synopsys.com
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/mmc/host/dw_mmc.c | 109 ++++++++++++++++++++++++++++++----------------
> 1 file changed, 71 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 40e9d8e..e41fb74 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -99,6 +99,9 @@ struct idmac_desc {
>
> __le32 des3; /* buffer 2 physical address */
> };
> +
> +/* Each descriptor can transfer up to 4KB of data in chained mode */
> +#define DW_MCI_DESC_DATA_LENGTH 0x1000
> #endif /* CONFIG_MMC_DW_IDMAC */
>
> static bool dw_mci_reset(struct dw_mci *host);
> @@ -462,66 +465,96 @@ static void dw_mci_idmac_complete_dma(struct dw_mci *host)
> static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
> unsigned int sg_len)
> {
> + unsigned int desc_len;
> int i;
> if (host->dma_64bit_address == 1) {
> - struct idmac_desc_64addr *desc = host->sg_cpu;
> + struct idmac_desc_64addr *desc_first, *desc_last, *desc;
Why it needs desc_first?
> +
> + desc_first = desc_last = desc = host->sg_cpu;
>
> - for (i = 0; i < sg_len; i++, desc++) {
> + for (i = 0; i < sg_len; i++) {
> unsigned int length = sg_dma_len(&data->sg[i]);
> u64 mem_addr = sg_dma_address(&data->sg[i]);
>
> - /*
> - * Set the OWN bit and disable interrupts for this
> - * descriptor
> - */
> - desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC |
> - IDMAC_DES0_CH;
> - /* Buffer length */
> - IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, length);
> -
> - /* Physical address to DMA to/from */
> - desc->des4 = mem_addr & 0xffffffff;
> - desc->des5 = mem_addr >> 32;
> + for ( ; length ; desc++) {
Is there no infinite loop case?
Best Regards,
Jaehoon Chung
> + desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ?
> + length : DW_MCI_DESC_DATA_LENGTH;
> +
> + length -= desc_len;
> +
> + /*
> + * Set the OWN bit and disable interrupts
> + * for this descriptor
> + */
> + desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC |
> + IDMAC_DES0_CH;
> +
> + /* Buffer length */
> + IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, desc_len);
> +
> + /* Physical address to DMA to/from */
> + desc->des4 = mem_addr & 0xffffffff;
> + desc->des5 = mem_addr >> 32;
> +
> + /* Update physical address for the next desc */
> + mem_addr += desc_len;
> +
> + /* Save pointer to the last descriptor */
> + desc_last = desc;
> + }
> }
>
> /* Set first descriptor */
> - desc = host->sg_cpu;
> - desc->des0 |= IDMAC_DES0_FD;
> + desc_first->des0 |= IDMAC_DES0_FD;
>
> /* Set last descriptor */
> - desc = host->sg_cpu + (i - 1) *
> - sizeof(struct idmac_desc_64addr);
> - desc->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC);
> - desc->des0 |= IDMAC_DES0_LD;
> + desc_last->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC);
> + desc_last->des0 |= IDMAC_DES0_LD;
>
> } else {
> - struct idmac_desc *desc = host->sg_cpu;
> + struct idmac_desc *desc_first, *desc_last, *desc;
> +
> + desc_first = desc_last = desc = host->sg_cpu;
>
> - for (i = 0; i < sg_len; i++, desc++) {
> + for (i = 0; i < sg_len; i++) {
> unsigned int length = sg_dma_len(&data->sg[i]);
> u32 mem_addr = sg_dma_address(&data->sg[i]);
>
> - /*
> - * Set the OWN bit and disable interrupts for this
> - * descriptor
> - */
> - desc->des0 = cpu_to_le32(IDMAC_DES0_OWN |
> - IDMAC_DES0_DIC | IDMAC_DES0_CH);
> - /* Buffer length */
> - IDMAC_SET_BUFFER1_SIZE(desc, length);
> + for ( ; length ; desc++) {
> + desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ?
> + length : DW_MCI_DESC_DATA_LENGTH;
> +
> + length -= desc_len;
> +
> + /*
> + * Set the OWN bit and disable interrupts
> + * for this descriptor
> + */
> + desc->des0 = cpu_to_le32(IDMAC_DES0_OWN |
> + IDMAC_DES0_DIC |
> + IDMAC_DES0_CH);
> +
> + /* Buffer length */
> + IDMAC_SET_BUFFER1_SIZE(desc, desc_len);
>
> - /* Physical address to DMA to/from */
> - desc->des2 = cpu_to_le32(mem_addr);
> + /* Physical address to DMA to/from */
> + desc->des2 = cpu_to_le32(mem_addr);
> +
> + /* Update physical address for the next desc */
> + mem_addr += desc_len;
> +
> + /* Save pointer to the last descriptor */
> + desc_last = desc;
> + }
> }
>
> /* Set first descriptor */
> - desc = host->sg_cpu;
> - desc->des0 |= cpu_to_le32(IDMAC_DES0_FD);
> + desc_first->des0 |= cpu_to_le32(IDMAC_DES0_FD);
>
> /* Set last descriptor */
> - desc = host->sg_cpu + (i - 1) * sizeof(struct idmac_desc);
> - desc->des0 &= cpu_to_le32(~(IDMAC_DES0_CH | IDMAC_DES0_DIC));
> - desc->des0 |= cpu_to_le32(IDMAC_DES0_LD);
> + desc_last->des0 &= cpu_to_le32(~(IDMAC_DES0_CH |
> + IDMAC_DES0_DIC));
> + desc_last->des0 |= cpu_to_le32(IDMAC_DES0_LD);
> }
>
> wmb();
> @@ -2394,7 +2427,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> #ifdef CONFIG_MMC_DW_IDMAC
> mmc->max_segs = host->ring_size;
> mmc->max_blk_size = 65536;
> - mmc->max_seg_size = 0x1000;
> + mmc->max_seg_size = DW_MCI_DESC_DATA_LENGTH;
> mmc->max_req_size = mmc->max_seg_size * host->ring_size;
> mmc->max_blk_count = mmc->max_req_size / 512;
> #else
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mmc: dw_mmc: handle data blocks > than 4kB if IDMAC is used
2015-07-08 4:14 ` Jaehoon Chung
@ 2015-07-08 8:45 ` Alexey Brodkin
2015-07-09 13:04 ` Alexey Brodkin
0 siblings, 1 reply; 9+ messages in thread
From: Alexey Brodkin @ 2015-07-08 8:45 UTC (permalink / raw)
To: jh80.chung; +Cc: ulf.hansson, linux-kernel, linux-mmc, tgih.jun, arc-linux-dev
Hi Jaehoon,
On Wed, 2015-07-08 at 13:14 +0900, Jaehoon Chung wrote:
> Hi, Alexey.
>
> On 06/25/2015 05:25 PM, Alexey Brodkin wrote:
> > As per DW MobileStorage databook "each descriptor can transfer up to 4kB
> > of data in chained mode", moreover buffer size that is put in "des1" is
> > limited to 13 bits, i.e. for example on attempt to
> > IDMAC_SET_BUFFER1_SIZE(desc, 8192) size value that's effectively written
> > will be 0.
> >
> > On the platform with 8kB PAGE_SIZE I see dw_mmc gets data blocks in
> > SG-list of 8kB size and that leads to unpredictable behavior of the
> > SD/MMC controller.
>
> I didn't see your problem, since i didn't test with 8K PAGE_SIZE.
> But I think your patch is reasonable.
> As possible, I want to know in more detail what unpredictable behavior.
> (Just stuck behavior?)
Please find below my observations from before the fix.
I noticed that some simple operations (especially reads of large files from FAT partitions)
lead to dw_mmc being unresponsive, see below and example:
---------------------------------->8------------------------------
$ mkdir /sd1
$ mount /dev/mmcblk0p1 /sd1
FAT-fs (mmcblk0p1): Volume was not properly unmounted. Some data may be corrupt. Please run fsck.
[ARCLinux]$ ls -lah /sd1
total 7252
drwxr-xr-x 8 root root 16.0K Dec 31 16:00 .
drwxrwxrwt 16 root root 380 Dec 31 16:03 ..
-rwxr-xr-x 1 root root 241 Dec 18 2014 boot.scr
-rwxr-xr-x 1 root root 44.3K Dec 18 2014 script.bin
-rwxr-xr-x 1 root root 7.0M Jan 13 2015 uImage
[ARCLinux]$ md5sum /sd1/uImage
---------------------------------->8------------------------------
At this point nothing was happening for a long time, so I pressed Ctrl-C and
run another "ls" that worked perfectly fine on the previous step (see above).
But that time "ls" didn't work, instead I saw:
---------------------------------->8------------------------------
$ mkdir /sd2
$ mount /dev/mmcblk0p2 /sd2
$ ls -lah /sd2
INFO: task ls:104 blocked for more than 10 seconds.
Not tainted 3.18.10-01062-g89ecf3c-dirty #1
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
ls D 8020e79c 0 104 84 0x00000004
Stack Trace:
__switch_to+0x0/0x98
__schedule+0x1d0/0x494
io_schedule+0x42/0x6c
bit_wait_io+0x1e/0x40
__wait_on_bit+0x86/0xac
out_of_line_wait_on_bit+0x48/0x58
ext4_bread+0x68/0x7c
__ext4_read_dirblock+0x32/0x320
htree_dirblock_to_tree+0x4a/0x174
ext4_htree_fill_tree+0x76/0x1e0
ext4_readdir+0x5e6/0x86c
iterate_dir+0x80/0xf4
SyS_getdents64+0x64/0xd4
ret_from_system_call+0x0/0x4
INFO: task ls:104 blocked for more than 10 seconds.
Not tainted 3.18.10-01062-g89ecf3c-dirty #1
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
ls D 8020e79c 0 104 84 0x00000004
Stack Trace:
__switch_to+0x0/0x98
__schedule+0x1d0/0x494
io_schedule+0x42/0x6c
bit_wait_io+0x1e/0x40
__wait_on_bit+0x86/0xac
out_of_line_wait_on_bit+0x48/0x58
ext4_bread+0x68/0x7c
__ext4_read_dirblock+0x32/0x320
htree_dirblock_to_tree+0x4a/0x174
ext4_htree_fill_tree+0x76/0x1e0
ext4_readdir+0x5e6/0x86c
iterate_dir+0x80/0xf4
SyS_getdents64+0x64/0xd4
ret_from_system_call+0x0/0x4
INFO: task ls:104 blocked for more than 10 seconds.
Not tainted 3.18.10-01062-g89ecf3c-dirty #1
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
ls D 8020e79c 0 104 84 0x00000004
Stack Trace:
__switch_to+0x0/0x98
__schedule+0x1d0/0x494
io_schedule+0x42/0x6c
bit_wait_io+0x1e/0x40
__wait_on_bit+0x86/0xac
out_of_line_wait_on_bit+0x48/0x58
ext4_bread+0x68/0x7c
__ext4_read_dirblock+0x32/0x320
htree_dirblock_to_tree+0x4a/0x174
ext4_htree_fill_tree+0x76/0x1e0
ext4_readdir+0x5e6/0x86c
iterate_dir+0x80/0xf4
SyS_getdents64+0x64/0xd4
ret_from_system_call+0x0/0x4
INFO: task ls:104 blocked for more than 10 seconds.
Not tainted 3.18.10-01062-g89ecf3c-dirty #1
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
ls D 8020e79c 0 104 84 0x00000004
Stack Trace:
__switch_to+0x0/0x98
__schedule+0x1d0/0x494
io_schedule+0x42/0x6c
bit_wait_io+0x1e/0x40
__wait_on_bit+0x86/0xac
out_of_line_wait_on_bit+0x48/0x58
ext4_bread+0x68/0x7c
__ext4_read_dirblock+0x32/0x320
htree_dirblock_to_tree+0x4a/0x174
ext4_htree_fill_tree+0x76/0x1e0
ext4_readdir+0x5e6/0x86c
iterate_dir+0x80/0xf4
SyS_getdents64+0x64/0xd4
ret_from_system_call+0x0/0x4
---------------------------------->8------------------------------
Seeing that problem I started to check what data is being sent to MMC controller
and pretty quickly found-out that sometimes value 8192 is written in the first
13 bits of DES1 that in case of IDMAC_SET_BUFFER1_SIZE macro usage effectively
writes 0. That was a clean misuse of MMC controller (it gets buffer descriptor
that points to zero-sized buffer). Once I fixed that flaw my initial problem
went away.
Let me know if that description makes sense to you.
-Alexey
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mmc: dw_mmc: handle data blocks > than 4kB if IDMAC is used
2015-07-08 8:45 ` Alexey Brodkin
@ 2015-07-09 13:04 ` Alexey Brodkin
2015-07-09 14:44 ` Jaehoon Chung
0 siblings, 1 reply; 9+ messages in thread
From: Alexey Brodkin @ 2015-07-09 13:04 UTC (permalink / raw)
To: jh80.chung; +Cc: ulf.hansson, linux-kernel, linux-mmc, tgih.jun, arc-linux-dev
Hi Jaehoon,
On Wed, 2015-07-08 at 11:45 +0300, Alexey Brodkin wrote:
>
> Let me know if that description makes sense to you.
I'm wondering if you had a chance to look at my comments and if
those comments make sense or you need more input from me.
-Alexey
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mmc: dw_mmc: handle data blocks > than 4kB if IDMAC is used
2015-07-09 13:04 ` Alexey Brodkin
@ 2015-07-09 14:44 ` Jaehoon Chung
0 siblings, 0 replies; 9+ messages in thread
From: Jaehoon Chung @ 2015-07-09 14:44 UTC (permalink / raw)
To: Alexey Brodkin, jh80.chung
Cc: ulf.hansson, linux-kernel, linux-mmc, tgih.jun, arc-linux-dev
Hi, Alexey.
On 07/09/2015 10:04 PM, Alexey Brodkin wrote:
> Hi Jaehoon,
>
> On Wed, 2015-07-08 at 11:45 +0300, Alexey Brodkin wrote:
>>
>> Let me know if that description makes sense to you.
>
> I'm wondering if you had a chance to look at my comments and if
> those comments make sense or you need more input from me.
As my understanding, it makes sense.
If page size should be changed bigger than 4K, internal dma should not be working fine.
(Especially your case..Page size is 8K.)
I will pick this patch at my dw-mmc tree.
Thanks for your effort and pointing out! :)
Best Regards,
Jaehoon Chung
>
> -Alexey--
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-07-09 14:45 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-25 8:25 [PATCH] mmc: dw_mmc: handle data blocks > than 4kB if IDMAC is used Alexey Brodkin
2015-07-01 10:02 ` Alexey Brodkin
2015-07-01 10:13 ` Jaehoon Chung
2015-07-06 7:12 ` Alexey Brodkin
2015-07-01 10:15 ` Vineet Gupta
2015-07-08 4:14 ` Jaehoon Chung
2015-07-08 8:45 ` Alexey Brodkin
2015-07-09 13:04 ` Alexey Brodkin
2015-07-09 14:44 ` Jaehoon Chung
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).