* [U-Boot] [PATCH RFC] fsl_esdhc: flush cache after non-read operation
@ 2014-03-28 10:15 Hector Palacios
2014-03-28 13:49 ` Fabio Estevam
0 siblings, 1 reply; 6+ messages in thread
From: Hector Palacios @ 2014-03-28 10:15 UTC (permalink / raw)
To: u-boot
Cache was invalidated on the read operation, but it should
also be flushed otherwise.
Signed-off-by: Hector Palacios <hector.palacios@digi.com>
---
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(+)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index e945c0a470ca..5ef575eb0272 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -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(®s->irqstat, -1);
^ permalink raw reply related [flat|nested] 6+ messages in thread* [U-Boot] [PATCH RFC] fsl_esdhc: flush cache after non-read operation
2014-03-28 10:15 [U-Boot] [PATCH RFC] fsl_esdhc: flush cache after non-read operation Hector Palacios
@ 2014-03-28 13:49 ` Fabio Estevam
2014-03-28 14:36 ` Eric Nelson
0 siblings, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2014-03-28 13:49 UTC (permalink / raw)
To: u-boot
On Fri, Mar 28, 2014 at 7:15 AM, Hector Palacios
<hector.palacios@digi.com> wrote:
> Cache was invalidated on the read operation, but it should
> also be flushed otherwise.
>
> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
Thanks, Hector.
Adding Marek on Cc as well.
> ---
>
> 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(+)
>
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index e945c0a470ca..5ef575eb0272 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -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(®s->irqstat, -1);
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 6+ messages in thread* [U-Boot] [PATCH RFC] fsl_esdhc: flush cache after non-read operation
2014-03-28 13:49 ` Fabio Estevam
@ 2014-03-28 14:36 ` Eric Nelson
2014-03-29 3:45 ` Marek Vasut
2014-03-31 8:23 ` Hector Palacios
0 siblings, 2 replies; 6+ messages in thread
From: Eric Nelson @ 2014-03-28 14:36 UTC (permalink / raw)
To: u-boot
Hi Hector,
On 03/28/2014 06:49 AM, Fabio Estevam wrote:
> On Fri, Mar 28, 2014 at 7:15 AM, Hector Palacios
> <hector.palacios@digi.com> wrote:
>> Cache was invalidated on the read operation, but it should
>> also be flushed otherwise.
>>
>> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
>
> Thanks, Hector.
>
> Adding Marek on Cc as well.
>
>
>> ---
>>
>> 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(+)
>>
>> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
>> index e945c0a470ca..5ef575eb0272 100644
>> --- a/drivers/mmc/fsl_esdhc.c
>> +++ b/drivers/mmc/fsl_esdhc.c
>> @@ -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);
>> }
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?
^ permalink raw reply [flat|nested] 6+ messages in thread* [U-Boot] [PATCH RFC] fsl_esdhc: flush cache after non-read operation
2014-03-28 14:36 ` Eric Nelson
@ 2014-03-29 3:45 ` Marek Vasut
2014-03-31 8:23 ` Hector Palacios
1 sibling, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2014-03-29 3:45 UTC (permalink / raw)
To: u-boot
On Friday, March 28, 2014 at 03:36:42 PM, Eric Nelson wrote:
> Hi Hector,
>
> On 03/28/2014 06:49 AM, Fabio Estevam wrote:
> > On Fri, Mar 28, 2014 at 7:15 AM, Hector Palacios
> >
> > <hector.palacios@digi.com> wrote:
> >> Cache was invalidated on the read operation, but it should
> >> also be flushed otherwise.
> >>
> >> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> >
> > Thanks, Hector.
> >
> > Adding Marek on Cc as well.
> >
> >> ---
> >>
> >> 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(+)
> >>
> >> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> >> index e945c0a470ca..5ef575eb0272 100644
> >> --- a/drivers/mmc/fsl_esdhc.c
> >> +++ b/drivers/mmc/fsl_esdhc.c
> >> @@ -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);
> >>
> >> }
>
> Since this comes after the wait for completion, this is
> clearly not the right fix.
ACK
> 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?
^ permalink raw reply [flat|nested] 6+ messages in thread* [U-Boot] [PATCH RFC] fsl_esdhc: flush cache after non-read operation
2014-03-28 14:36 ` Eric Nelson
2014-03-29 3:45 ` Marek Vasut
@ 2014-03-31 8:23 ` Hector Palacios
2014-03-31 8:37 ` Marek Vasut
1 sibling, 1 reply; 6+ messages in thread
From: Hector Palacios @ 2014-03-31 8:23 UTC (permalink / raw)
To: u-boot
Hi,
On 03/28/2014 03:36 PM, Eric Nelson wrote:
> Hi Hector,
>
> On 03/28/2014 06:49 AM, Fabio Estevam wrote:
>> On Fri, Mar 28, 2014 at 7:15 AM, Hector Palacios
>> <hector.palacios@digi.com> wrote:
>>> Cache was invalidated on the read operation, but it should
>>> also be flushed otherwise.
>>>
>>> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
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?
--
H?ctor Palacios
^ permalink raw reply [flat|nested] 6+ messages in thread* [U-Boot] [PATCH RFC] fsl_esdhc: flush cache after non-read operation
2014-03-31 8:23 ` Hector Palacios
@ 2014-03-31 8:37 ` Marek Vasut
0 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2014-03-31 8:37 UTC (permalink / raw)
To: u-boot
On Monday, March 31, 2014 at 10:23:50 AM, Hector Palacios wrote:
> Hi,
>
> On 03/28/2014 03:36 PM, Eric Nelson wrote:
> > Hi Hector,
> >
> > On 03/28/2014 06:49 AM, Fabio Estevam wrote:
> >> On Fri, Mar 28, 2014 at 7:15 AM, Hector Palacios
> >>
> >> <hector.palacios@digi.com> wrote:
> >>> Cache was invalidated on the read operation, but it should
> >>> also be flushed otherwise.
> >>>
> >>> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
>
> 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?
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-31 8:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-28 10:15 [U-Boot] [PATCH RFC] fsl_esdhc: flush cache after non-read operation Hector Palacios
2014-03-28 13:49 ` Fabio Estevam
2014-03-28 14:36 ` Eric Nelson
2014-03-29 3:45 ` Marek Vasut
2014-03-31 8:23 ` Hector Palacios
2014-03-31 8:37 ` Marek Vasut
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox