[U-Boot] [PATCH] i.MX: fsl_esdhc: allow use with cache enabled.

[U-Boot] [PATCH] i.MX: fsl_esdhc: allow use with cache enabled.

[Eric Nelson]

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
 drivers/mmc/fsl_esdhc.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index a2f35e3..8e4bcef 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -190,6 +190,8 @@ static int esdhc_setup_data(struct mmc *mmc, struct mmc_data *data)
 		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
 		esdhc_write32(&regs->dsaddr, (u32)data->dest);
 	} else {
+                flush_dcache_range((ulong)data->src,(ulong)data->src+data->blocks*data->blocksize);
+
 		if (wml_value > WML_WR_WML_MAX)
 			wml_value = WML_WR_WML_MAX_VAL;
 		if ((esdhc_read32(&regs->prsstat) & PRSSTAT_WPSPL) == 0) {
@@ -249,7 +251,12 @@ static int esdhc_setup_data(struct mmc *mmc, struct mmc_data *data)
 	return 0;
 }
 
-
+static void check_and_invalidate_dcache_range(struct mmc_cmd *cmd, struct mmc_data *data) {
+	unsigned start = data->dest ;
+	unsigned size = roundup(ARCH_DMA_MINALIGN,data->blocks*data->blocksize);
+	unsigned end = start+size ;
+	invalidate_dcache_range(start,end);
+}
 /*
  * Sends a command out on the bus.  Takes the mmc pointer,
  * a command pointer, and an optional data pointer.
@@ -311,6 +318,9 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 	while (!(esdhc_read32(&regs->irqstat) & IRQSTAT_CC))
 		;
 
+	if (data && (data->flags & MMC_DATA_READ))
+		check_and_invalidate_dcache_range(cmd,data);
+
 	irqstat = esdhc_read32(&regs->irqstat);
 	esdhc_write32(&regs->irqstat, irqstat);
 
-- 
1.7.9

[U-Boot] [PATCH] i.MX: fsl_esdhc: allow use with cache enabled.

[Stefano Babic]

On 03/03/2012 23:37, Eric Nelson wrote:
> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> ---

Hi Eric,

I have added Andy in CC as MMC Maintainer

>  drivers/mmc/fsl_esdhc.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index a2f35e3..8e4bcef 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -190,6 +190,8 @@ static int esdhc_setup_data(struct mmc *mmc, struct mmc_data *data)
>  		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
>  		esdhc_write32(&regs->dsaddr, (u32)data->dest);
>  	} else {
> +                flush_dcache_range((ulong)data->src,(ulong)data->src+data->blocks*data->blocksize);

This breaks PowerPC boards.

> +
>  		if (wml_value > WML_WR_WML_MAX)
>  			wml_value = WML_WR_WML_MAX_VAL;
>  		if ((esdhc_read32(&regs->prsstat) & PRSSTAT_WPSPL) == 0) {
> @@ -249,7 +251,12 @@ static int esdhc_setup_data(struct mmc *mmc, struct mmc_data *data)
>  	return 0;
>  }
>  
> -
> +static void check_and_invalidate_dcache_range(struct mmc_cmd *cmd, struct mmc_data *data) {
> +	unsigned start = data->dest ;

A cast is missing, a warning is generated.

> +	unsigned size = roundup(ARCH_DMA_MINALIGN,data->blocks*data->blocksize);
> +	unsigned end = start+size ;
> +	invalidate_dcache_range(start,end)

This also breaks PowerPC boards.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] i.MX: fsl_esdhc: allow use with cache enabled.

[Eric Nelson]

Hi Stefano,

Sorry I let this linger.

On 03/17/2012 03:50 AM, Stefano Babic wrote:
> On 03/03/2012 23:37, Eric Nelson wrote:
>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>> ---
>
> Hi Eric,
>
> I have added Andy in CC as MMC Maintainer
>
>>   drivers/mmc/fsl_esdhc.c |   12 +++++++++++-
>>   1 files changed, 11 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
>> index a2f35e3..8e4bcef 100644
>> --- a/drivers/mmc/fsl_esdhc.c
>> +++ b/drivers/mmc/fsl_esdhc.c
>> @@ -190,6 +190,8 @@ static int esdhc_setup_data(struct mmc *mmc, struct mmc_data *data)
>>   		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
>>   		esdhc_write32(&regs->dsaddr, (u32)data->dest);
>>   	} else {
>> +                flush_dcache_range((ulong)data->src,(ulong)data->src+data->blocks*data->blocksize);
>
> This breaks PowerPC boards.
>

Can you tell me how?

AFAIK, blocksize needs to be a multiple of 512 so this should be cache-safe with
any cacheline size.

>> +
>>   		if (wml_value>  WML_WR_WML_MAX)
>>   			wml_value = WML_WR_WML_MAX_VAL;
>>   		if ((esdhc_read32(&regs->prsstat)&  PRSSTAT_WPSPL) == 0) {
>> @@ -249,7 +251,12 @@ static int esdhc_setup_data(struct mmc *mmc, struct mmc_data *data)
>>   	return 0;
>>   }
>>
>> -
>> +static void check_and_invalidate_dcache_range(struct mmc_cmd *cmd, struct mmc_data *data) {
>> +	unsigned start = data->dest ;
>
> A cast is missing, a warning is generated.
>

Yep. I recognized that right **after** I hit <send>...

If you can give me some guidance on the comment above, I'll address with a V2.

Regards,


Eric

[U-Boot] [PATCH] i.MX: fsl_esdhc: allow use with cache enabled.

[Stefano Babic]

On 24/03/2012 23:36, Eric Nelson wrote:
> Hi Stefano,
> 

Hi Eric,

> Sorry I let this linger.
> 
> On 03/17/2012 03:50 AM, Stefano Babic wrote:
>> On 03/03/2012 23:37, Eric Nelson wrote:
>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>> ---
>>
>> Hi Eric,
>>
>> I have added Andy in CC as MMC Maintainer
>>
>>>   drivers/mmc/fsl_esdhc.c |   12 +++++++++++-
>>>   1 files changed, 11 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
>>> index a2f35e3..8e4bcef 100644
>>> --- a/drivers/mmc/fsl_esdhc.c
>>> +++ b/drivers/mmc/fsl_esdhc.c
>>> @@ -190,6 +190,8 @@ static int esdhc_setup_data(struct mmc *mmc,
>>> struct mmc_data *data)
>>>           esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
>>>           esdhc_write32(&regs->dsaddr, (u32)data->dest);
>>>       } else {
>>> +               
>>> flush_dcache_range((ulong)data->src,(ulong)data->src+data->blocks*data->blocksize);
>>>
>>
>> This breaks PowerPC boards.
>>
> 
> Can you tell me how?


./MAKEALL MPC8536DS
Configuring for MPC8536DS board...
fsl_esdhc.c: In function 'check_and_invalidate_dcache_range':
fsl_esdhc.c:255:19: warning: initialization makes integer from pointer
without a cast [enabled by default]
drivers/mmc/libmmc.o: In function `esdhc_setup_data':
/home/stefano/Projects/imx/u-boot-imx/drivers/mmc/fsl_esdhc.c:193:
undefined reference to `flush_dcache_range'
drivers/mmc/libmmc.o: In function `check_and_invalidate_dcache_range':
/home/stefano/Projects/imx/u-boot-imx/drivers/mmc/fsl_esdhc.c:258:
undefined reference to `invalidate_dcache_range'
make: *** [u-boot] Fehler 1

Main reason is that PowerPC (this is a PowerQuickIII) has a "snooping"
mechanism, and it is enough to set a bit in the SOC to make things working.

Maybe can we add these functions as empty in 85xx ? Or we can add an
intermediate layer in fsl_eshc.h that manage this, such as
fsl_esdhc_flush_dcache_range. This was already done to solve the
endianess problem (PowerPC is big endian).

Andy, what do you think about ?

> 
> AFAIK, blocksize needs to be a multiple of 512 so this should be
> cache-safe with
> any cacheline size.

Yes, I think your changes are correct in the i.MX case.

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
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] i.MX: fsl_esdhc: allow use with cache enabled.

[Dirk Behme]

Hi Eric,

On 25.03.2012 12:32, Stefano Babic wrote:
> On 24/03/2012 23:36, Eric Nelson wrote:
>> Hi Stefano,
>>
>
> Hi Eric,
>
>> Sorry I let this linger.
>>
>> On 03/17/2012 03:50 AM, Stefano Babic wrote:
>>> On 03/03/2012 23:37, Eric Nelson wrote:
>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>>> ---
>>>
>>> Hi Eric,
>>>
>>> I have added Andy in CC as MMC Maintainer
>>>
>>>>    drivers/mmc/fsl_esdhc.c |   12 +++++++++++-
>>>>    1 files changed, 11 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
>>>> index a2f35e3..8e4bcef 100644
>>>> --- a/drivers/mmc/fsl_esdhc.c
>>>> +++ b/drivers/mmc/fsl_esdhc.c
>>>> @@ -190,6 +190,8 @@ static int esdhc_setup_data(struct mmc *mmc,
>>>> struct mmc_data *data)
>>>>            esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
>>>>            esdhc_write32(&regs->dsaddr, (u32)data->dest);
>>>>        } else {
>>>> +
>>>> flush_dcache_range((ulong)data->src,(ulong)data->src+data->blocks*data->blocksize);
>>>>
>>>
>>> This breaks PowerPC boards.
>>>
>>
>> Can you tell me how?
>
>
> ./MAKEALL MPC8536DS
> Configuring for MPC8536DS board...
> fsl_esdhc.c: In function 'check_and_invalidate_dcache_range':
> fsl_esdhc.c:255:19: warning: initialization makes integer from pointer

I get this building it for SabreLite, too. So any plans to update this 
patch?

Many thanks and best regards

Dirk

> without a cast [enabled by default]
> drivers/mmc/libmmc.o: In function `esdhc_setup_data':
> /home/stefano/Projects/imx/u-boot-imx/drivers/mmc/fsl_esdhc.c:193:
> undefined reference to `flush_dcache_range'
> drivers/mmc/libmmc.o: In function `check_and_invalidate_dcache_range':
> /home/stefano/Projects/imx/u-boot-imx/drivers/mmc/fsl_esdhc.c:258:
> undefined reference to `invalidate_dcache_range'
> make: *** [u-boot] Fehler 1
>
> Main reason is that PowerPC (this is a PowerQuickIII) has a "snooping"
> mechanism, and it is enough to set a bit in the SOC to make things working.
>
> Maybe can we add these functions as empty in 85xx ? Or we can add an
> intermediate layer in fsl_eshc.h that manage this, such as
> fsl_esdhc_flush_dcache_range. This was already done to solve the
> endianess problem (PowerPC is big endian).
>
> Andy, what do you think about ?
>
>>
>> AFAIK, blocksize needs to be a multiple of 512 so this should be
>> cache-safe with
>> any cacheline size.
>
> Yes, I think your changes are correct in the i.MX case.
>
> Best regards,
> Stefano
>