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 <[email protected]>
Cc: Seungwon Jeon <[email protected]>
Cc: Jaehoon Chung <[email protected]>
Cc: Ulf Hansson <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
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
Hi Jaehoon, Seungwon, Ulf,
On Thu, 2015-06-25 at 11:25 +-0300, Alexey Brodkin wrote:
+AD4- As per DW MobileStorage databook +ACI-each descriptor can transfer up to
+AD4- 4kB
+AD4- of data in chained mode+ACI-, moreover buffer size that is put in +ACI-des1+ACI-
+AD4- is
+AD4- limited to 13 bits, i.e. for example on attempt to
+AD4- IDMAC+AF8-SET+AF8-BUFFER1+AF8-SIZE(desc, 8192) size value that's effectively
+AD4- written
+AD4- will be 0.
+AD4-
+AD4- On the platform with 8kB PAGE+AF8-SIZE I see dw+AF8-mmc gets data blocks in
+AD4- SG-list of 8kB size and that leads to unpredictable behavior of the
+AD4- SD/MMC controller.
+AD4-
+AD4- In particular on write to FAT partition of SD-card the controller
+AD4- will
+AD4- stuck in the middle of DMA transaction.
+AD4-
+AD4- Solution to the problem is simple - we need to pass large (+AD4- 4kB)
+AD4- data
+AD4- buffers to the controller via multiple descriptors. And that's what
+AD4- that change does.
+AD4-
+AD4- What's interesting I did try original driver on same platform but
+AD4- configured with 4kB PAGE+AF8-SIZE and may confirm that data blocks passed
+AD4- in SG-list to dw+AF8-mmc never exeed 4kB limit - that explains why nobody
+AD4- ever faced a problem I did.
+AD4-
+AD4- Signed-off-by: Alexey Brodkin +ADw-abrodkin+AEA-synopsys.com+AD4-
+AD4- Cc: Seungwon Jeon +ADw-tgih.jun+AEA-samsung.com+AD4-
+AD4- Cc: Jaehoon Chung +ADw-jh80.chung+AEA-samsung.com+AD4-
+AD4- Cc: Ulf Hansson +ADw-ulf.hansson+AEA-linaro.org+AD4-
+AD4- Cc: arc-linux-dev+AEA-synopsys.com
+AD4- Cc: linux-kernel+AEA-vger.kernel.org
+AD4- ---
+AD4- drivers/mmc/host/dw+AF8-mmc.c +AHw- 109 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-------
+AD4- ----------
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+AF8-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-
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 <[email protected]>
>> Cc: Seungwon Jeon <[email protected]>
>> Cc: Jaehoon Chung <[email protected]>
>> Cc: Ulf Hansson <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> ---
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
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 <[email protected]>
>> > Cc: Seungwon Jeon <[email protected]>
>> > Cc: Jaehoon Chung <[email protected]>
>> > Cc: Ulf Hansson <[email protected]>
>> > Cc: [email protected]
>> > Cc: [email protected]
>> > ---
>> > 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--
Hi Jaehoon,
On Wed, 2015-07-01 at 19:13 +-0900, Jaehoon Chung wrote:
+AD4- Hi, Alexey.
+AD4-
+AD4- Sorry for reviewing late. i missed this patch.
+AD4- I will check your patch and reply at mailing.
+AD4- Thanks a lot for noticing this.
+AD4-
+AD4- Best Regards,
+AD4- Jaehoon Chung
Please treat this as a polite reminder, would be nice to get some
comments on that patch.
-Alexey-
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 <[email protected]>
> Cc: Seungwon Jeon <[email protected]>
> Cc: Jaehoon Chung <[email protected]>
> Cc: Ulf Hansson <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> 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
>
Hi Jaehoon,
On Wed, 2015-07-08 at 13:14 +-0900, Jaehoon Chung wrote:
+AD4- Hi, Alexey.
+AD4-
+AD4- On 06/25/2015 05:25 PM, Alexey Brodkin wrote:
+AD4- +AD4- As per DW MobileStorage databook +ACI-each descriptor can transfer up to 4kB
+AD4- +AD4- of data in chained mode+ACI-, moreover buffer size that is put in +ACI-des1+ACI- is
+AD4- +AD4- limited to 13 bits, i.e. for example on attempt to
+AD4- +AD4- IDMAC+AF8-SET+AF8-BUFFER1+AF8-SIZE(desc, 8192) size value that's effectively written
+AD4- +AD4- will be 0.
+AD4- +AD4-
+AD4- +AD4- On the platform with 8kB PAGE+AF8-SIZE I see dw+AF8-mmc gets data blocks in
+AD4- +AD4- SG-list of 8kB size and that leads to unpredictable behavior of the
+AD4- +AD4- SD/MMC controller.
+AD4-
+AD4- I didn't see your problem, since i didn't test with 8K PAGE+AF8-SIZE.
+AD4- But I think your patch is reasonable.
+AD4- As possible, I want to know in more detail what unpredictable behavior.
+AD4- (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+AF8-mmc being unresponsive, see below and example:
----------------------------------+AD4-8------------------------------
+ACQ- mkdir /sd1
+ACQ- mount /dev/mmcblk0p1 /sd1
FAT-fs (mmcblk0p1): Volume was not properly unmounted. Some data may be corrupt. Please run fsck.
+AFs-ARCLinux+AF0AJA- 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
+AFs-ARCLinux+AF0AJA- md5sum /sd1/uImage
----------------------------------+AD4-8------------------------------
At this point nothing was happening for a long time, so I pressed Ctrl-C and
run another +ACI-ls+ACI- that worked perfectly fine on the previous step (see above).
But that time +ACI-ls+ACI- didn't work, instead I saw:
----------------------------------+AD4-8------------------------------
+ACQ- mkdir /sd2
+ACQ- mount /dev/mmcblk0p2 /sd2
+ACQ- ls -lah /sd2
INFO: task ls:104 blocked for more than 10 seconds.
Not tainted 3.18.10-01062-g89ecf3c-dirty +ACM-1
+ACI-echo 0 +AD4- /proc/sys/kernel/hung+AF8-task+AF8-timeout+AF8-secs+ACI- disables this message.
ls D 8020e79c 0 104 84 0x00000004
Stack Trace:
+AF8AXw-switch+AF8-to+-0x0/0x98
+AF8AXw-schedule+-0x1d0/0x494
io+AF8-schedule+-0x42/0x6c
bit+AF8-wait+AF8-io+-0x1e/0x40
+AF8AXw-wait+AF8-on+AF8-bit+-0x86/0xac
out+AF8-of+AF8-line+AF8-wait+AF8-on+AF8-bit+-0x48/0x58
ext4+AF8-bread+-0x68/0x7c
+AF8AXw-ext4+AF8-read+AF8-dirblock+-0x32/0x320
htree+AF8-dirblock+AF8-to+AF8-tree+-0x4a/0x174
ext4+AF8-htree+AF8-fill+AF8-tree+-0x76/0x1e0
ext4+AF8-readdir+-0x5e6/0x86c
iterate+AF8-dir+-0x80/0xf4
SyS+AF8-getdents64+-0x64/0xd4
ret+AF8-from+AF8-system+AF8-call+-0x0/0x4
INFO: task ls:104 blocked for more than 10 seconds.
Not tainted 3.18.10-01062-g89ecf3c-dirty +ACM-1
+ACI-echo 0 +AD4- /proc/sys/kernel/hung+AF8-task+AF8-timeout+AF8-secs+ACI- disables this message.
ls D 8020e79c 0 104 84 0x00000004
Stack Trace:
+AF8AXw-switch+AF8-to+-0x0/0x98
+AF8AXw-schedule+-0x1d0/0x494
io+AF8-schedule+-0x42/0x6c
bit+AF8-wait+AF8-io+-0x1e/0x40
+AF8AXw-wait+AF8-on+AF8-bit+-0x86/0xac
out+AF8-of+AF8-line+AF8-wait+AF8-on+AF8-bit+-0x48/0x58
ext4+AF8-bread+-0x68/0x7c
+AF8AXw-ext4+AF8-read+AF8-dirblock+-0x32/0x320
htree+AF8-dirblock+AF8-to+AF8-tree+-0x4a/0x174
ext4+AF8-htree+AF8-fill+AF8-tree+-0x76/0x1e0
ext4+AF8-readdir+-0x5e6/0x86c
iterate+AF8-dir+-0x80/0xf4
SyS+AF8-getdents64+-0x64/0xd4
ret+AF8-from+AF8-system+AF8-call+-0x0/0x4
INFO: task ls:104 blocked for more than 10 seconds.
Not tainted 3.18.10-01062-g89ecf3c-dirty +ACM-1
+ACI-echo 0 +AD4- /proc/sys/kernel/hung+AF8-task+AF8-timeout+AF8-secs+ACI- disables this message.
ls D 8020e79c 0 104 84 0x00000004
Stack Trace:
+AF8AXw-switch+AF8-to+-0x0/0x98
+AF8AXw-schedule+-0x1d0/0x494
io+AF8-schedule+-0x42/0x6c
bit+AF8-wait+AF8-io+-0x1e/0x40
+AF8AXw-wait+AF8-on+AF8-bit+-0x86/0xac
out+AF8-of+AF8-line+AF8-wait+AF8-on+AF8-bit+-0x48/0x58
ext4+AF8-bread+-0x68/0x7c
+AF8AXw-ext4+AF8-read+AF8-dirblock+-0x32/0x320
htree+AF8-dirblock+AF8-to+AF8-tree+-0x4a/0x174
ext4+AF8-htree+AF8-fill+AF8-tree+-0x76/0x1e0
ext4+AF8-readdir+-0x5e6/0x86c
iterate+AF8-dir+-0x80/0xf4
SyS+AF8-getdents64+-0x64/0xd4
ret+AF8-from+AF8-system+AF8-call+-0x0/0x4
INFO: task ls:104 blocked for more than 10 seconds.
Not tainted 3.18.10-01062-g89ecf3c-dirty +ACM-1
+ACI-echo 0 +AD4- /proc/sys/kernel/hung+AF8-task+AF8-timeout+AF8-secs+ACI- disables this message.
ls D 8020e79c 0 104 84 0x00000004
Stack Trace:
+AF8AXw-switch+AF8-to+-0x0/0x98
+AF8AXw-schedule+-0x1d0/0x494
io+AF8-schedule+-0x42/0x6c
bit+AF8-wait+AF8-io+-0x1e/0x40
+AF8AXw-wait+AF8-on+AF8-bit+-0x86/0xac
out+AF8-of+AF8-line+AF8-wait+AF8-on+AF8-bit+-0x48/0x58
ext4+AF8-bread+-0x68/0x7c
+AF8AXw-ext4+AF8-read+AF8-dirblock+-0x32/0x320
htree+AF8-dirblock+AF8-to+AF8-tree+-0x4a/0x174
ext4+AF8-htree+AF8-fill+AF8-tree+-0x76/0x1e0
ext4+AF8-readdir+-0x5e6/0x86c
iterate+AF8-dir+-0x80/0xf4
SyS+AF8-getdents64+-0x64/0xd4
ret+AF8-from+AF8-system+AF8-call+-0x0/0x4
----------------------------------+AD4-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+AF8-SET+AF8-BUFFER1+AF8-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-
Hi Jaehoon,
On Wed, 2015-07-08 at 11:45 +-0300, Alexey Brodkin wrote:
+AD4-
+AD4- 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-
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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>