Skip to contentSkip to conversation

fsl_esdhc: flush cache after non-read operation

RFC PATCH New Hector Palacios 4 participants 6 messages Mar 28, 2014 – Mar 31, 2014 MMC, SD and SDIO

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 fsl_esdhc: flush cache after non-read operation

Cache was invalidated on the read operation, but it should also be flushed otherwise.

---

Notes:
After enabling L2 cache on i.MX6 I found out that many times when running the 'gpt' command to partition a uSD card, the data was not written at all, or was badly written to the media.

This patch seems to solve it but I'm not sure if that's the right place to flush the cache. Could someone please comment?

Thank you.

 drivers/mmc/fsl_esdhc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
drivers/mmc/fsl_esdhc.c +12 -0
index e945c0a470ca..5ef575eb0272 100644
@@ -253,6 +253,16 @@ static int esdhc_setup_data(struct mmc *mmc, struct mmc_data *data)
 	return 0;
 }
 
+static void check_and_flush_dcache_range
+	(struct mmc_cmd *cmd,
+	 struct mmc_data *data) {
+	unsigned start = (unsigned)data->dest ;
+	unsigned size = roundup(ARCH_DMA_MINALIGN,
+				data->blocks*data->blocksize);
+	unsigned end = start+size ;
+	flush_dcache_range(start, end);
+}
+
 static void check_and_invalidate_dcache_range
 	(struct mmc_cmd *cmd,
 	 struct mmc_data *data) {
@@ -401,6 +411,8 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 #endif
 		if (data->flags & MMC_DATA_READ)
 			check_and_invalidate_dcache_range(cmd, data);
+		else
+			check_and_flush_dcache_range(cmd, data);
 	}
 
 	esdhc_write32(&regs->irqstat, -1);
#1 lore raw

RFC PATCH 1/1 fsl_esdhc: flush cache after non-read operation

Quoted 4 lines · Hector Palacios

Cache was invalidated on the read operation, but it should also be flushed otherwise.

Thanks, Hector.

Adding Marek on Cc as well.

Quoted 49 lines

---

Notes:
After enabling L2 cache on i.MX6 I found out that many times when running the 'gpt' command to partition a uSD card, the data was not written at all, or was badly written to the media.

This patch seems to solve it but I'm not sure if that's the right place to flush the cache. Could someone please comment?

Thank you.

 drivers/mmc/fsl_esdhc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
drivers/mmc/fsl_esdhc.c +12 -0
index e945c0a470ca..5ef575eb0272 100644
@@ -253,6 +253,16 @@ static int esdhc_setup_data(struct mmc *mmc, struct mmc_data *data)
        return 0;
 }
 
+static void check_and_flush_dcache_range
+       (struct mmc_cmd *cmd,
+        struct mmc_data *data) {
+       unsigned start = (unsigned)data->dest ;
+       unsigned size = roundup(ARCH_DMA_MINALIGN,
+                               data->blocks*data->blocksize);
+       unsigned end = start+size ;
+       flush_dcache_range(start, end);
+}
+
 static void check_and_invalidate_dcache_range
        (struct mmc_cmd *cmd,
         struct mmc_data *data) {
@@ -401,6 +411,8 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 #endif
                if (data->flags & MMC_DATA_READ)
                        check_and_invalidate_dcache_range(cmd, data);
+               else
+                       check_and_flush_dcache_range(cmd, data);
        }
 
        esdhc_write32(&regs->irqstat, -1);

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#2 lore raw

RFC PATCH 1/1 fsl_esdhc: flush cache after non-read operation

Hi Hector,

Quoted 47 lines · Fabio Estevam
Quoted 4 lines · Hector Palacios

Cache was invalidated on the read operation, but it should also be flushed otherwise.

Thanks, Hector.

Adding Marek on Cc as well.

Quoted 35 lines

---

Notes:
After enabling L2 cache on i.MX6 I found out that many times when running the 'gpt' command to partition a uSD card, the data was not written at all, or was badly written to the media.

This patch seems to solve it but I'm not sure if that's the right place to flush the cache. Could someone please comment?

Thank you.

  drivers/mmc/fsl_esdhc.c | 12 ++++++++++++
  1 file changed, 12 insertions(+)
drivers/mmc/fsl_esdhc.c +10 -0
index e945c0a470ca..5ef575eb0272 100644
@@ -253,6 +253,16 @@ static int esdhc_setup_data(struct mmc *mmc, struct mmc_data *data)
         return 0;
  }
 
+static void check_and_flush_dcache_range
+       (struct mmc_cmd *cmd,
+        struct mmc_data *data) {
+       unsigned start = (unsigned)data->dest ;
+       unsigned size = roundup(ARCH_DMA_MINALIGN,
+                               data->blocks*data->blocksize);
+       unsigned end = start+size ;
+       flush_dcache_range(start, end);
+}
+
  static void check_and_invalidate_dcache_range
         (struct mmc_cmd *cmd,
Quoted 8 lines

drivers/mmc/fsl_esdhc.c esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) · line 416

Quoted 8 lines
drivers/mmc/fsl_esdhc.c +2 -0
          struct mmc_data *data) {
@@ -401,6 +411,8 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
  #endif
                 if (data->flags & MMC_DATA_READ)
                         check_and_invalidate_dcache_range(cmd, data);
+               else
+                       check_and_flush_dcache_range(cmd, data);
         }

Since this comes after the wait for completion, this is clearly not the right fix.

If this patch is fixing the problem, the issue must be somewhere else.

Can you verify that the call to flush_dcache_range() in the esdhc_setup_data routine is being called prior to esdhc_send_command?

#3 lore raw

RFC PATCH 1/1 fsl_esdhc: flush cache after non-read operation

Quoted 67 lines · Eric Nelson

Hi Hector,

Quoted 64 lines · Fabio Estevam

On Fri, Mar 28, 2014 at 7:15 AM, Hector Palacios

Quoted 4 lines · <[email protected]> wrote:

Cache was invalidated on the read operation, but it should also be flushed otherwise.

Thanks, Hector.

Adding Marek on Cc as well.

Quoted 52 lines

---

Notes:
After enabling L2 cache on i.MX6 I found out that many times when running the 'gpt' command to partition a uSD card, the data was not written at all, or was badly written to the media.

This patch seems to solve it but I'm not sure if that's the right place to flush the cache. Could someone please comment?

Thank you.

  drivers/mmc/fsl_esdhc.c | 12 ++++++++++++
  1 file changed, 12 insertions(+)
drivers/mmc/fsl_esdhc.c +10 -0
index e945c0a470ca..5ef575eb0272 100644
@@ -253,6 +253,16 @@ static int esdhc_setup_data(struct mmc *mmc, struct
 mmc_data *data)
 
         return 0;
 
  }
 
+static void check_and_flush_dcache_range
+       (struct mmc_cmd *cmd,
+        struct mmc_data *data) {
+       unsigned start = (unsigned)data->dest ;
+       unsigned size = roundup(ARCH_DMA_MINALIGN,
+                               data->blocks*data->blocksize);
+       unsigned end = start+size ;
+       flush_dcache_range(start, end);
+}
+

static void check_and_invalidate_dcache_range

(struct mmc_cmd *cmd,

struct mmc_data *data) {

drivers/mmc/fsl_esdhc.c +0 -0
@@ -401,6 +411,8 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 struct mmc_data *data)
 
  #endif
 
                 if (data->flags & MMC_DATA_READ)
 
                         check_and_invalidate_dcache_range(cmd, data);
Quoted 7 lines
Quoted 4 lines

drivers/mmc/fsl_esdhc.c esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)

Quoted 4 lines
drivers/mmc/fsl_esdhc.c +2 -0
+               else
+                       check_and_flush_dcache_range(cmd, data);
 
         }

Since this comes after the wait for completion, this is clearly not the right fix.

ACK

Quoted 4 lines

If this patch is fixing the problem, the issue must be somewhere else.

Can you verify that the call to flush_dcache_range() in the esdhc_setup_data routine is being called prior to esdhc_send_command?

#4 lore raw

RFC PATCH 1/1 fsl_esdhc: flush cache after non-read operation

In reply to Eric Nelson

Hi,

Quoted 9 lines · Eric Nelson

Hi Hector,

Quoted 6 lines · Fabio Estevam
Quoted 4 lines · Hector Palacios

Cache was invalidated on the read operation, but it should also be flushed otherwise.

After further testing it looks like I misinterpreted the results:
First, please disregard the patch as it does not fix anything.
Second, 'mmc part' command seems to be returning cached data after I use 'gpt' command to partition the uSD card. I can reproduce it as follows (consider mmc 1 is my uSD card):

1. Write random data to corrupt the partition table => mmc dev 1
=> mmc write $loadaddr 0 30
2. Check partition table is corrupt
=> mmc part (shows error invalid GPT)
3. Soft reset the target
4. Write a correct partition table

	=> mmc dev 1
	=> gpt write mmc 1 "..."

5. Read back partition table
=> mmc part

At this point 'mmc part' returns again an incorrect partition table. However, if after a while I do an 'mmc rescan' or a soft reset and rerun the 'mmc part' command, it will show the correct partition table was written.

The partition table is read during mmc_init():

int test_part_efi(block_dev_desc_t * dev_desc)

{
	ALLOC_CACHE_ALIGN_BUFFER_PAD(legacy_mbr, legacymbr, 1, dev_desc->blksz);

	/* Read legacy MBR from block 0 and validate it */
	if ((dev_desc->block_read(dev_desc->dev, 0, 1, (ulong *)legacymbr) != 1)
		|| (is_pmbr_valid(legacymbr) != 1)) {
		return -1;
	}
	return 0;
}

Could it be that the read partition table is cached so that after writing it with 'gpt', reading it again returns cached data instead of physical data, just written?

Signature · 1 line
H?ctor Palacios
#5 lore raw

RFC PATCH 1/1 fsl_esdhc: flush cache after non-read operation

Quoted 53 lines · Hector Palacios

Hi,

Quoted 10 lines · Eric Nelson

Hi Hector,

Quoted 7 lines · Fabio Estevam

On Fri, Mar 28, 2014 at 7:15 AM, Hector Palacios

Quoted 4 lines · <[email protected]> wrote:

Cache was invalidated on the read operation, but it should also be flushed otherwise.

After further testing it looks like I misinterpreted the results:
First, please disregard the patch as it does not fix anything. Second, 'mmc part' command seems to be returning cached data after I use 'gpt' command to partition the uSD card. I can reproduce it as follows (consider mmc 1 is my uSD card):

1. Write random data to corrupt the partition table => mmc dev 1
=> mmc write $loadaddr 0 30
2. Check partition table is corrupt
=> mmc part (shows error invalid GPT)
3. Soft reset the target
4. Write a correct partition table

	=> mmc dev 1
	=> gpt write mmc 1 "..."

5. Read back partition table
=> mmc part

At this point 'mmc part' returns again an incorrect partition table. However, if after a while I do an 'mmc rescan' or a soft reset and rerun the 'mmc part' command, it will show the correct partition table was written.

The partition table is read during mmc_init():

int test_part_efi(block_dev_desc_t * dev_desc)

{
	ALLOC_CACHE_ALIGN_BUFFER_PAD(legacy_mbr, legacymbr, 1, dev_desc->blksz);

	/* Read legacy MBR from block 0 and validate it */
	if ((dev_desc->block_read(dev_desc->dev, 0, 1, (ulong *)legacymbr) != 1)

		|| (is_pmbr_valid(legacymbr) != 1)) {

		return -1;
	}
	return 0;
}
Quoted 3 lines

Could it be that the read partition table is cached so that after writing it with 'gpt', reading it again returns cached data instead of physical data, just written?

You mean cached as in "in data cache of the CPU" ? You can test that quite easily, add CONFIG_CMD_CACHE into your board config and then use "dcache off" before running your testcase. You can try "dcache on" and retry the testcase. See if that makes some difference.

Best regards,
Marek Vasut