[U-Boot] [PATCH] mmc:fsl_esdhc invalidate dcache before read

[U-Boot] [PATCH] mmc:fsl_esdhc invalidate dcache before read

[Peng Fan]

DCIMVAC is upgraded to DCCIMVAC for the individual processor
(Cortex-A7) that the DCIMVAC is executed on.

We should follow the linux dma follow. Before DMA read, first
invalidate dcache then after DMA read, invalidate dcache again.

With the DMA direction DMA_FROM_DEVICE, the dcache need be
invalidated again after the DMA completion. The reason is
that we need explicity make sure the dcache been invalidated
thus to get the DMA'ed memory correctly from the physical memory.
Any cache-line fill during the DMA operations such as the
pre-fetching can cause the DMA coherency issue, thus CPU get the stale data.

Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
Signed-off-by: Ye.Li <B37916@freescale.com>
Signed-off-by: Nitin Garg <nitin.garg@freescale.com>
Signed-off-by: Jason Liu <r64343@freescale.com>
---
 drivers/mmc/fsl_esdhc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index c4719e6..0510bf0 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -341,6 +341,9 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 		err = esdhc_setup_data(mmc, data);
 		if(err)
 			return err;
+
+		if (data->flags & MMC_DATA_READ)
+			check_and_invalidate_dcache_range(cmd, data);
 	}
 
 	/* Figure out the transfer arguments */
@@ -437,6 +440,11 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 			}
 		} while ((irqstat & DATA_COMPLETE) != DATA_COMPLETE);
 
+		/*
+		 * Need invalidate the dcache here again to avoid any
+		 * cache-fill during the DMA operations such as the
+		 * speculative pre-fetching etc.
+		 */
 		if (data->flags & MMC_DATA_READ)
 			check_and_invalidate_dcache_range(cmd, data);
 #endif
-- 
1.8.4

[U-Boot] [PATCH] mmc:fsl_esdhc invalidate dcache before read

[Peng Fan]

Ping. Anyone review/apply this patch?

On Thu, Jun 25, 2015 at 10:32:26AM +0800, Peng Fan wrote:
>DCIMVAC is upgraded to DCCIMVAC for the individual processor
>(Cortex-A7) that the DCIMVAC is executed on.
>
>We should follow the linux dma follow. Before DMA read, first
>invalidate dcache then after DMA read, invalidate dcache again.
>
>With the DMA direction DMA_FROM_DEVICE, the dcache need be
>invalidated again after the DMA completion. The reason is
>that we need explicity make sure the dcache been invalidated
>thus to get the DMA'ed memory correctly from the physical memory.
>Any cache-line fill during the DMA operations such as the
>pre-fetching can cause the DMA coherency issue, thus CPU get the stale data.
>
>Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
>Signed-off-by: Ye.Li <B37916@freescale.com>
>Signed-off-by: Nitin Garg <nitin.garg@freescale.com>
>Signed-off-by: Jason Liu <r64343@freescale.com>
>---
> drivers/mmc/fsl_esdhc.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
>diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
>index c4719e6..0510bf0 100644
>--- a/drivers/mmc/fsl_esdhc.c
>+++ b/drivers/mmc/fsl_esdhc.c
>@@ -341,6 +341,9 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
> 		err = esdhc_setup_data(mmc, data);
> 		if(err)
> 			return err;
>+
>+		if (data->flags & MMC_DATA_READ)
>+			check_and_invalidate_dcache_range(cmd, data);
> 	}
> 
> 	/* Figure out the transfer arguments */
>@@ -437,6 +440,11 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
> 			}
> 		} while ((irqstat & DATA_COMPLETE) != DATA_COMPLETE);
> 
>+		/*
>+		 * Need invalidate the dcache here again to avoid any
>+		 * cache-fill during the DMA operations such as the
>+		 * speculative pre-fetching etc.
>+		 */
> 		if (data->flags & MMC_DATA_READ)
> 			check_and_invalidate_dcache_range(cmd, data);
> #endif
>-- 
>1.8.4
>
>
>_______________________________________________
>U-Boot mailing list
>U-Boot at lists.denx.de
>http://lists.denx.de/mailman/listinfo/u-boot

Regards,
Peng.
-- 

[U-Boot] [PATCH] mmc:fsl_esdhc invalidate dcache before read

[Stefano Babic]

Hi Peng,


I see this patch is now delegated to me. Then, I have no objections to
merge it into u-boot-imx if Pantelis agree.


On 25/06/2015 04:32, Peng Fan wrote:
> DCIMVAC is upgraded to DCCIMVAC for the individual processor
> (Cortex-A7) that the DCIMVAC is executed on.
> 
Can you better explain it ? What are the consequences of using DCCIMVAC
into this driver ?

This driver is not only for i.MX. It runs on PowerPc, too. Do you tested
this patch on PowerPC architecture, too ? Maybe someone else can do it ?

> We should follow the linux dma follow. Before DMA read, first
> invalidate dcache then after DMA read, invalidate dcache again.
> 
> With the DMA direction DMA_FROM_DEVICE, the dcache need be
> invalidated again after the DMA completion. The reason is
> that we need explicity make sure the dcache been invalidated
> thus to get the DMA'ed memory correctly from the physical memory.
> Any cache-line fill during the DMA operations such as the
> pre-fetching can cause the DMA coherency issue, thus CPU get the stale data.
> 
> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
> Signed-off-by: Ye.Li <B37916@freescale.com>
> Signed-off-by: Nitin Garg <nitin.garg@freescale.com>
> Signed-off-by: Jason Liu <r64343@freescale.com>
> ---
>  drivers/mmc/fsl_esdhc.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index c4719e6..0510bf0 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -341,6 +341,9 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  		err = esdhc_setup_data(mmc, data);
>  		if(err)
>  			return err;
> +
> +		if (data->flags & MMC_DATA_READ)
> +			check_and_invalidate_dcache_range(cmd, data);
>  	}
>  
>  	/* Figure out the transfer arguments */
> @@ -437,6 +440,11 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  			}
>  		} while ((irqstat & DATA_COMPLETE) != DATA_COMPLETE);
>  
> +		/*
> +		 * Need invalidate the dcache here again to avoid any
> +		 * cache-fill during the DMA operations such as the
> +		 * speculative pre-fetching etc.
> +		 */
>  		if (data->flags & MMC_DATA_READ)
>  			check_and_invalidate_dcache_range(cmd, data);
>  #endif
> 

From my side it is ok.

Reviewed-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano Babic



-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] mmc:fsl_esdhc invalidate dcache before read

[Peng Fan]

Hi Stefano,

On Fri, Jul 10, 2015 at 09:56:23AM +0200, Stefano Babic wrote:
>Hi Peng,
>
>
>I see this patch is now delegated to me. Then, I have no objections to
>merge it into u-boot-imx if Pantelis agree.
>
>
>On 25/06/2015 04:32, Peng Fan wrote:
>> DCIMVAC is upgraded to DCCIMVAC for the individual processor
>> (Cortex-A7) that the DCIMVAC is executed on.
>> 
>Can you better explain it ? What are the consequences of using DCCIMVAC
>into this driver ?

This is from http://infocenter.arm.com/help/topic/com.arm.doc.ddi0464f/DDI0464F_cortex_a7_mpcore_r0p5_trm.pdf

"
DCIMVAC means Data cache invalidate by MVA to PoC
DCCIMVAC means Data cache clean and invalidate line by MVA to PoC

DCIMVAC is upgraded to DCCIMVAC for the individual processor that the DCIMVAC
is executed on. Additionally, if the DCIMVAC is executed from a Non-secure
state other than Hyp mode without second state write permissions then the
DCIMVAC is upgraded to DCCIMVAC when broadcast to other processors or
broadcast on the ACE interface.
"
See page 4-9, Table 4-8 c7 register summary (continued).

>
>This driver is not only for i.MX. It runs on PowerPc, too. Do you tested
>this patch on PowerPC architecture, too ? Maybe someone else can do it ?

I do not have Powerpc board to test this patch.

York, do you have such a board or is this patch is ok for powerpc?

>
>> We should follow the linux dma follow. Before DMA read, first
>> invalidate dcache then after DMA read, invalidate dcache again.
>> 
>> With the DMA direction DMA_FROM_DEVICE, the dcache need be
>> invalidated again after the DMA completion. The reason is
>> that we need explicity make sure the dcache been invalidated
>> thus to get the DMA'ed memory correctly from the physical memory.
>> Any cache-line fill during the DMA operations such as the
>> pre-fetching can cause the DMA coherency issue, thus CPU get the stale data.
>> 
>> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
>> Signed-off-by: Ye.Li <B37916@freescale.com>
>> Signed-off-by: Nitin Garg <nitin.garg@freescale.com>
>> Signed-off-by: Jason Liu <r64343@freescale.com>
>> ---
>>  drivers/mmc/fsl_esdhc.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
>> index c4719e6..0510bf0 100644
>> --- a/drivers/mmc/fsl_esdhc.c
>> +++ b/drivers/mmc/fsl_esdhc.c
>> @@ -341,6 +341,9 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>>  		err = esdhc_setup_data(mmc, data);
>>  		if(err)
>>  			return err;
>> +
>> +		if (data->flags & MMC_DATA_READ)
>> +			check_and_invalidate_dcache_range(cmd, data);
>>  	}
>>  
>>  	/* Figure out the transfer arguments */
>> @@ -437,6 +440,11 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>>  			}
>>  		} while ((irqstat & DATA_COMPLETE) != DATA_COMPLETE);
>>  
>> +		/*
>> +		 * Need invalidate the dcache here again to avoid any
>> +		 * cache-fill during the DMA operations such as the
>> +		 * speculative pre-fetching etc.
>> +		 */
>>  		if (data->flags & MMC_DATA_READ)
>>  			check_and_invalidate_dcache_range(cmd, data);
>>  #endif
>> 
>
From my side it is ok.
>
>Reviewed-by: Stefano Babic <sbabic@denx.de>
>
>Best regards,
>Stefano Babic
>
>
>
>-- 
>=====================================================================
>DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
>=====================================================================

Regards,
Peng
-- 

[U-Boot] [PATCH] mmc:fsl_esdhc invalidate dcache before read

[Stefano Babic]

Hi Peng,

On 11/07/2015 04:43, Peng Fan wrote:
> Hi Stefano,
> 
> On Fri, Jul 10, 2015 at 09:56:23AM +0200, Stefano Babic wrote:
>> Hi Peng,
>>
>>
>> I see this patch is now delegated to me. Then, I have no objections to
>> merge it into u-boot-imx if Pantelis agree.
>>
>>
>> On 25/06/2015 04:32, Peng Fan wrote:
>>> DCIMVAC is upgraded to DCCIMVAC for the individual processor
>>> (Cortex-A7) that the DCIMVAC is executed on.
>>>
>> Can you better explain it ? What are the consequences of using DCCIMVAC
>> into this driver ?
> 
> This is from http://infocenter.arm.com/help/topic/com.arm.doc.ddi0464f/DDI0464F_cortex_a7_mpcore_r0p5_trm.pdf
> 
> "
> DCIMVAC means Data cache invalidate by MVA to PoC
> DCCIMVAC means Data cache clean and invalidate line by MVA to PoC
> 
> DCIMVAC is upgraded to DCCIMVAC for the individual processor that the DCIMVAC
> is executed on. Additionally, if the DCIMVAC is executed from a Non-secure
> state other than Hyp mode without second state write permissions then the
> DCIMVAC is upgraded to DCCIMVAC when broadcast to other processors or
> broadcast on the ACE interface.
> "
> See page 4-9, Table 4-8 c7 register summary (continued).
> 
>>
>> This driver is not only for i.MX. It runs on PowerPc, too. Do you tested
>> this patch on PowerPC architecture, too ? Maybe someone else can do it ?
> 
> I do not have Powerpc board to test this patch.
> 
> York, do you have such a board or is this patch is ok for powerpc?
> 
>>
>>> We should follow the linux dma follow. Before DMA read, first
>>> invalidate dcache then after DMA read, invalidate dcache again.
>>>
>>> With the DMA direction DMA_FROM_DEVICE, the dcache need be
>>> invalidated again after the DMA completion. The reason is
>>> that we need explicity make sure the dcache been invalidated
>>> thus to get the DMA'ed memory correctly from the physical memory.
>>> Any cache-line fill during the DMA operations such as the
>>> pre-fetching can cause the DMA coherency issue, thus CPU get the stale data.
>>>
>>> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
>>> Signed-off-by: Ye.Li <B37916@freescale.com>
>>> Signed-off-by: Nitin Garg <nitin.garg@freescale.com>
>>> Signed-off-by: Jason Liu <r64343@freescale.com>
>>> ---
>>>  drivers/mmc/fsl_esdhc.c | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
>>> index c4719e6..0510bf0 100644
>>> --- a/drivers/mmc/fsl_esdhc.c
>>> +++ b/drivers/mmc/fsl_esdhc.c
>>> @@ -341,6 +341,9 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>>>  		err = esdhc_setup_data(mmc, data);
>>>  		if(err)
>>>  			return err;
>>> +
>>> +		if (data->flags & MMC_DATA_READ)
>>> +			check_and_invalidate_dcache_range(cmd, data);
>>>  	}
>>>  
>>>  	/* Figure out the transfer arguments */
>>> @@ -437,6 +440,11 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>>>  			}
>>>  		} while ((irqstat & DATA_COMPLETE) != DATA_COMPLETE);
>>>  
>>> +		/*
>>> +		 * Need invalidate the dcache here again to avoid any
>>> +		 * cache-fill during the DMA operations such as the
>>> +		 * speculative pre-fetching etc.
>>> +		 */
>>>  		if (data->flags & MMC_DATA_READ)
>>>  			check_and_invalidate_dcache_range(cmd, data);
>>>  #endif
>>>
>>
>>From my side it is ok.
>>
>> Reviewed-by: Stefano Babic <sbabic@denx.de>
>>

I apply it to u-boot-imx - merging into mainline, we will have more
chances to get it tested on PowerPc.

Applied to u-boot-imx, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] mmc:fsl_esdhc invalidate dcache before read

[York Sun]

On 07/26/2015 03:20 AM, Stefano Babic wrote:
> I apply it to u-boot-imx - merging into mainline, we will have more
> chances to get it tested on PowerPc.
> 
> Applied to u-boot-imx, thanks !
> 

Sorry I didn't see this thread until Stefano CC'ed me. I will see if I can test
it on powerpc boards.

York

[U-Boot] [PATCH] mmc:fsl_esdhc invalidate dcache before read

[York Sun]

On 07/27/2015 09:36 AM, York Sun wrote:
> 
> 
> On 07/26/2015 03:20 AM, Stefano Babic wrote:
>> I apply it to u-boot-imx - merging into mainline, we will have more
>> chances to get it tested on PowerPc.
>>
>> Applied to u-boot-imx, thanks !
>>
> 
> Sorry I didn't see this thread until Stefano CC'ed me. I will see if I can test
> it on powerpc boards.
> 

The change is safe for powerpc. Verified on T4240RDB with SD read/write.

York

[U-Boot] [PATCH] mmc:fsl_esdhc invalidate dcache before read

[Stefano Babic]

On 27/07/2015 20:15, York Sun wrote:
> 
> 
> On 07/27/2015 09:36 AM, York Sun wrote:
>>
>>
>> On 07/26/2015 03:20 AM, Stefano Babic wrote:
>>> I apply it to u-boot-imx - merging into mainline, we will have more
>>> chances to get it tested on PowerPc.
>>>
>>> Applied to u-boot-imx, thanks !
>>>
>>
>> Sorry I didn't see this thread until Stefano CC'ed me. I will see if I can test
>> it on powerpc boards.
>>
> 
> The change is safe for powerpc. Verified on T4240RDB with SD read/write.
> 

Thanks !

Regards,
Stefano


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================