[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 at freescale.com>
>>> Signed-off-by: Ye.Li <B37916 at freescale.com>
>>> Signed-off-by: Nitin Garg <nitin.garg at freescale.com>
>>> Signed-off-by: Jason Liu <r64343 at 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 at 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
=====================================================================