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

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

[Eric Nelson]

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
V2 fixes checkpatch errors and typecasts as pointed out on the ML.

 drivers/mmc/fsl_esdhc.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index a2f35e3..539d848 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -190,6 +190,10 @@ 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 +253,15 @@ 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 = (unsigned)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 +323,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 V2] i.MX: fsl_esdhc: allow use with cache enabled.

[Andy Fleming]

> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -190,6 +190,10 @@ 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 still won't work. I don't believe this is implemented at all on
the FSL PowerPC parts that use this controller.

At the very least, it needs to be protected by an ifdef.


> ? ? ? ? ? ? ? ?if (wml_value > WML_WR_WML_MAX)
> ? ? ? ? ? ? ? ? ? ? ? ?wml_value = WML_WR_WML_MAX_VAL;
> ? ? ? ? ? ? ? ?if ((esdhc_read32(&regs->prsstat) & PRSSTAT_WPSPL) == 0) {
> @@ -249,7 +253,15 @@ 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) {


This is non-standard formatting in U-Boot.


Andy

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

[Eric Nelson]

Thanks Andy,

On 05/08/2012 03:59 PM, Andy Fleming wrote:
>> --- a/drivers/mmc/fsl_esdhc.c
>> +++ b/drivers/mmc/fsl_esdhc.c
>> @@ -190,6 +190,10 @@ 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 still won't work. I don't believe this is implemented at all on
> the FSL PowerPC parts that use this controller.
>
> At the very least, it needs to be protected by an ifdef.
>

It seems more generally useful to implement a PowerPC cache layer
than to instrument each driver that supports cache.

Do you know how many other peripherals are shared between ARM and PPC
that might be broken by cache operations?

>>                 if (wml_value>  WML_WR_WML_MAX)
>>                         wml_value = WML_WR_WML_MAX_VAL;
>>                 if ((esdhc_read32(&regs->prsstat)&  PRSSTAT_WPSPL) == 0) {
>> @@ -249,7 +253,15 @@ 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) {
>
>
> This is non-standard formatting in U-Boot.
>

Ok. I can address that in a V3 once I know the direction.

Regards,


Eric

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

[Stefano Babic]

On 09/05/2012 01:31, Eric Nelson wrote:
> Thanks Andy,
> 
> On 05/08/2012 03:59 PM, Andy Fleming wrote:
>>> --- a/drivers/mmc/fsl_esdhc.c
>>> +++ b/drivers/mmc/fsl_esdhc.c
>>> @@ -190,6 +190,10 @@ 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 still won't work. I don't believe this is implemented at all on
>> the FSL PowerPC parts that use this controller.
>>
>> At the very least, it needs to be protected by an ifdef.
>>
> 
> It seems more generally useful to implement a PowerPC cache layer
> than to instrument each driver that supports cache.

Some PowerPC (MPC86xx,  arch/powerpc/cpu/mpc86xx/cache.S) have this
layer, some other not. This driver is used by MPC85xx and
flush_dcache_range is not implemented. The reason is that this SOC  uses
its internal snooping mechanism and does not need to explicitely flush
the buffers in the drivers.

The problem with an #ifdef is that it is not very generic - we should
add some #if (defined(CONFIG_MX51) || defined(CONFIG_MX53) || ...
and update the driver for each new SOCs. I think also that maybe the
best way is to add an empty flush_dcache_range() to the MPC85xx, maybe
as weak function.

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 V2] i.MX: fsl_esdhc: allow use with cache enabled.

[Dirk Behme]

On 09.05.2012 07:45, Stefano Babic wrote:
> On 09/05/2012 01:31, Eric Nelson wrote:
>> Thanks Andy,
>>
>> On 05/08/2012 03:59 PM, Andy Fleming wrote:
>>>> --- a/drivers/mmc/fsl_esdhc.c
>>>> +++ b/drivers/mmc/fsl_esdhc.c
>>>> @@ -190,6 +190,10 @@ 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 still won't work. I don't believe this is implemented at all on
>>> the FSL PowerPC parts that use this controller.
>>>
>>> At the very least, it needs to be protected by an ifdef.
>>>
>> It seems more generally useful to implement a PowerPC cache layer
>> than to instrument each driver that supports cache.
> 
> Some PowerPC (MPC86xx,  arch/powerpc/cpu/mpc86xx/cache.S) have this
> layer, some other not. This driver is used by MPC85xx and
> flush_dcache_range is not implemented. The reason is that this SOC  uses
> its internal snooping mechanism and does not need to explicitely flush
> the buffers in the drivers.
> 
> The problem with an #ifdef is that it is not very generic - we should
> add some #if (defined(CONFIG_MX51) || defined(CONFIG_MX53) || ...
> and update the driver for each new SOCs. I think also that maybe the
> best way is to add an empty flush_dcache_range() to the MPC85xx, maybe
> as weak function.

You might have noticed that I'm looking through my list of open patches. 
So: Any news regarding V3 of this patch? ;)

Many thanks

Dirk

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

[Anatolij Gustschin]

Hi Dirk,

On Tue, 15 May 2012 14:58:54 +0200
Dirk Behme <dirk.behme@de.bosch.com> wrote:

> On 09.05.2012 07:45, Stefano Babic wrote:
...
> > The problem with an #ifdef is that it is not very generic - we should
> > add some #if (defined(CONFIG_MX51) || defined(CONFIG_MX53) || ...
> > and update the driver for each new SOCs. I think also that maybe the
> > best way is to add an empty flush_dcache_range() to the MPC85xx, maybe
> > as weak function.
> 
> You might have noticed that I'm looking through my list of open patches. 
> So: Any news regarding V3 of this patch? ;)

Stefano is on vacation; please don't expect any quick reply from him.

Thanks,
Anatolij

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

[Eric Nelson]

On 05/15/2012 06:09 AM, Anatolij Gustschin wrote:
> Hi Dirk,
>
> On Tue, 15 May 2012 14:58:54 +0200
> Dirk Behme<dirk.behme@de.bosch.com>  wrote:
>
>> On 09.05.2012 07:45, Stefano Babic wrote:
> ...
>>> The problem with an #ifdef is that it is not very generic - we should
>>> add some #if (defined(CONFIG_MX51) || defined(CONFIG_MX53) || ...
>>> and update the driver for each new SOCs. I think also that maybe the
>>> best way is to add an empty flush_dcache_range() to the MPC85xx, maybe
>>> as weak function.
>>
>> You might have noticed that I'm looking through my list of open patches.
>> So: Any news regarding V3 of this patch? ;)
>
> Stefano is on vacation; please don't expect any quick reply from him.
>

Vacation? Is that one of those European words I should really know? ;)

I don't have the same excuse. I'm just struggling to find a few hours
to take a stab at this.

I'm without a PPC device and compiler, so I'll need some assistance in
validating things.

Regards,


Eric

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

[Marek Vasut]

Dear Eric Nelson,

> On 05/15/2012 06:09 AM, Anatolij Gustschin wrote:
> > Hi Dirk,
> > 
> > On Tue, 15 May 2012 14:58:54 +0200
> > 
> > Dirk Behme<dirk.behme@de.bosch.com>  wrote:
> >> On 09.05.2012 07:45, Stefano Babic wrote:
> > ...
> > 
> >>> The problem with an #ifdef is that it is not very generic - we should
> >>> add some #if (defined(CONFIG_MX51) || defined(CONFIG_MX53) || ...
> >>> and update the driver for each new SOCs. I think also that maybe the
> >>> best way is to add an empty flush_dcache_range() to the MPC85xx, maybe
> >>> as weak function.
> >> 
> >> You might have noticed that I'm looking through my list of open patches.
> >> So: Any news regarding V3 of this patch? ;)
> > 
> > Stefano is on vacation; please don't expect any quick reply from him.
> 
> Vacation? Is that one of those European words I should really know? ;)
> 
> I don't have the same excuse. I'm just struggling to find a few hours
> to take a stab at this.
> 
> I'm without a PPC device and compiler, so I'll need some assistance in
> validating things.

Well if you put up the tree somewhere, I can run it for you ;-)

> 
> Regards,
> 
> 
> Eric

Best regards,
Marek Vasut

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

[Stefano Babic]

On 15/05/2012 14:58, Dirk Behme wrote:
> On 09.05.2012 07:45, Stefano Babic wrote:
>> On 09/05/2012 01:31, Eric Nelson wrote:
>>> Thanks Andy,
>>>
>>> On 05/08/2012 03:59 PM, Andy Fleming wrote:
>>>>> --- a/drivers/mmc/fsl_esdhc.c
>>>>> +++ b/drivers/mmc/fsl_esdhc.c
>>>>> @@ -190,6 +190,10 @@ 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 still won't work. I don't believe this is implemented at all on
>>>> the FSL PowerPC parts that use this controller.
>>>>
>>>> At the very least, it needs to be protected by an ifdef.
>>>>
>>> It seems more generally useful to implement a PowerPC cache layer
>>> than to instrument each driver that supports cache.
>>
>> Some PowerPC (MPC86xx,  arch/powerpc/cpu/mpc86xx/cache.S) have this
>> layer, some other not. This driver is used by MPC85xx and
>> flush_dcache_range is not implemented. The reason is that this SOC  uses
>> its internal snooping mechanism and does not need to explicitely flush
>> the buffers in the drivers.
>>
>> The problem with an #ifdef is that it is not very generic - we should
>> add some #if (defined(CONFIG_MX51) || defined(CONFIG_MX53) || ...
>> and update the driver for each new SOCs. I think also that maybe the
>> best way is to add an empty flush_dcache_range() to the MPC85xx, maybe
>> as weak function.
> 
> You might have noticed that I'm looking through my list of open patches.
> So: Any news regarding V3 of this patch? ;)

This is my turn to look back if there are some news. Has anyone taken a
look at it ?

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-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

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

[Eric Nelson]

On 06/12/2012 10:07 AM, Stefano Babic wrote:
> On 15/05/2012 14:58, Dirk Behme wrote:
>> On 09.05.2012 07:45, Stefano Babic wrote:
>>> On 09/05/2012 01:31, Eric Nelson wrote:
>>>> Thanks Andy,
>>>>
>>>> On 05/08/2012 03:59 PM, Andy Fleming wrote:
>>>>>> --- a/drivers/mmc/fsl_esdhc.c
>>>>>> +++ b/drivers/mmc/fsl_esdhc.c
>>>>>> @@ -190,6 +190,10 @@ 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 still won't work. I don't believe this is implemented at all on
>>>>> the FSL PowerPC parts that use this controller.
>>>>>
>>>>> At the very least, it needs to be protected by an ifdef.
>>>>>
>>>> It seems more generally useful to implement a PowerPC cache layer
>>>> than to instrument each driver that supports cache.
>>>
>>> Some PowerPC (MPC86xx,  arch/powerpc/cpu/mpc86xx/cache.S) have this
>>> layer, some other not. This driver is used by MPC85xx and
>>> flush_dcache_range is not implemented. The reason is that this SOC  uses
>>> its internal snooping mechanism and does not need to explicitely flush
>>> the buffers in the drivers.
>>>
>>> The problem with an #ifdef is that it is not very generic - we should
>>> add some #if (defined(CONFIG_MX51) || defined(CONFIG_MX53) || ...
>>> and update the driver for each new SOCs. I think also that maybe the
>>> best way is to add an empty flush_dcache_range() to the MPC85xx, maybe
>>> as weak function.
>>
>> You might have noticed that I'm looking through my list of open patches.
>> So: Any news regarding V3 of this patch? ;)
>
> This is my turn to look back if there are some news. Has anyone taken a
> look at it ?
>
> Best regards,
> Stefano Babic
>

Sorry Stefano,

I've been meaning to generate an update but seem to be starving for time.

Some other messages on the list regarding PPC seem to indicate that the
real solution here is a set of cache stubs for PPC, not an SDHC patch though.

I'm not likely to have any time until after FTF next week.

Any chance that a PPC maintainer can chime in here?

Please advise,


Eric

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

[Marek Vasut]

Dear Eric Nelson,

> On 06/12/2012 10:07 AM, Stefano Babic wrote:
> > On 15/05/2012 14:58, Dirk Behme wrote:
> >> On 09.05.2012 07:45, Stefano Babic wrote:
> >>> On 09/05/2012 01:31, Eric Nelson wrote:
> >>>> Thanks Andy,
> >>>> 
> >>>> On 05/08/2012 03:59 PM, Andy Fleming wrote:
> >>>>>> --- a/drivers/mmc/fsl_esdhc.c
> >>>>>> +++ b/drivers/mmc/fsl_esdhc.c
> >>>>>> @@ -190,6 +190,10 @@ 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 still won't work. I don't believe this is implemented at all on
> >>>>> the FSL PowerPC parts that use this controller.
> >>>>> 
> >>>>> At the very least, it needs to be protected by an ifdef.
> >>>> 
> >>>> It seems more generally useful to implement a PowerPC cache layer
> >>>> than to instrument each driver that supports cache.
> >>> 
> >>> Some PowerPC (MPC86xx,  arch/powerpc/cpu/mpc86xx/cache.S) have this
> >>> layer, some other not. This driver is used by MPC85xx and
> >>> flush_dcache_range is not implemented. The reason is that this SOC 
> >>> uses its internal snooping mechanism and does not need to explicitely
> >>> flush the buffers in the drivers.
> >>> 
> >>> The problem with an #ifdef is that it is not very generic - we should
> >>> add some #if (defined(CONFIG_MX51) || defined(CONFIG_MX53) || ...
> >>> and update the driver for each new SOCs. I think also that maybe the
> >>> best way is to add an empty flush_dcache_range() to the MPC85xx, maybe
> >>> as weak function.
> >> 
> >> You might have noticed that I'm looking through my list of open patches.
> >> So: Any news regarding V3 of this patch? ;)
> > 
> > This is my turn to look back if there are some news. Has anyone taken a
> > look at it ?
> > 
> > Best regards,
> > Stefano Babic
> 
> Sorry Stefano,
> 
> I've been meaning to generate an update but seem to be starving for time.
> 
> Some other messages on the list regarding PPC seem to indicate that the
> real solution here is a set of cache stubs for PPC, not an SDHC patch
> though.
> 
> I'm not likely to have any time until after FTF next week.
> 
> Any chance that a PPC maintainer can chime in here?

I think WD applied the cache stub patch already. Can you try now please?

> 
> Please advise,
> 
> 
> Eric

Best regards,
Marek Vasut

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

[Dirk Behme]

On 12.06.2012 19:23, Marek Vasut wrote:
> Dear Eric Nelson,
> 
>> On 06/12/2012 10:07 AM, Stefano Babic wrote:
>>> On 15/05/2012 14:58, Dirk Behme wrote:
>>>> On 09.05.2012 07:45, Stefano Babic wrote:
>>>>> On 09/05/2012 01:31, Eric Nelson wrote:
>>>>>> Thanks Andy,
>>>>>>
>>>>>> On 05/08/2012 03:59 PM, Andy Fleming wrote:
>>>>>>>> --- a/drivers/mmc/fsl_esdhc.c
>>>>>>>> +++ b/drivers/mmc/fsl_esdhc.c
>>>>>>>> @@ -190,6 +190,10 @@ 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 still won't work. I don't believe this is implemented at all on
>>>>>>> the FSL PowerPC parts that use this controller.
>>>>>>>
>>>>>>> At the very least, it needs to be protected by an ifdef.
>>>>>> It seems more generally useful to implement a PowerPC cache layer
>>>>>> than to instrument each driver that supports cache.
>>>>> Some PowerPC (MPC86xx,  arch/powerpc/cpu/mpc86xx/cache.S) have this
>>>>> layer, some other not. This driver is used by MPC85xx and
>>>>> flush_dcache_range is not implemented. The reason is that this SOC 
>>>>> uses its internal snooping mechanism and does not need to explicitely
>>>>> flush the buffers in the drivers.
>>>>>
>>>>> The problem with an #ifdef is that it is not very generic - we should
>>>>> add some #if (defined(CONFIG_MX51) || defined(CONFIG_MX53) || ...
>>>>> and update the driver for each new SOCs. I think also that maybe the
>>>>> best way is to add an empty flush_dcache_range() to the MPC85xx, maybe
>>>>> as weak function.
>>>> You might have noticed that I'm looking through my list of open patches.
>>>> So: Any news regarding V3 of this patch? ;)
>>> This is my turn to look back if there are some news. Has anyone taken a
>>> look at it ?
>>>
>>> Best regards,
>>> Stefano Babic
>> Sorry Stefano,
>>
>> I've been meaning to generate an update but seem to be starving for time.
>>
>> Some other messages on the list regarding PPC seem to indicate that the
>> real solution here is a set of cache stubs for PPC, not an SDHC patch
>> though.
>>
>> I'm not likely to have any time until after FTF next week.
>>
>> Any chance that a PPC maintainer can chime in here?
> 
> I think WD applied the cache stub patch already. Can you try now please?

Ping ;)

Thanks

Dirk

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

[Marek Vasut]

Dear Dirk Behme,

[...]

> >> I'm not likely to have any time until after FTF next week.
> >> 
> >> Any chance that a PPC maintainer can chime in here?
> > 
> > I think WD applied the cache stub patch already. Can you try now please?
> 
> Ping ;)
> 
> Thanks

Adding our human interactions expert into CC :)

Best regards,
Marek Vasut

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

[Marek Vasut]

Dear Dirk Behme,

[...]

> >> I'm not likely to have any time until after FTF next week.
> >> 
> >> Any chance that a PPC maintainer can chime in here?
> > 
> > I think WD applied the cache stub patch already. Can you try now please?
> 
> Ping ;)
> 
> Thanks
> 
> Dirk

I just noticed this with latest mainline:
Configuring for MPC8569MDS_ATM - Board: MPC8569MDS, Options: ATM
make: *** [u-boot] Error 1
powerpc-linux-size: './u-boot': No such file
drivers/mmc/libmmc.o: In function `esdhc_setup_data':
/home/marex/testing/u-boot.git/drivers/mmc/fsl_esdhc.c:193: undefined reference 
to `flush_dcache_range'
drivers/mmc/libmmc.o: In function `check_and_invalidate_dcache_range':
/home/marex/testing/u-boot.git/drivers/mmc/fsl_esdhc.c:263: undefined reference 
to `invalidate_dcache_range'
make: *** [u-boot] Error 1
Configuring for MPC8569MDS_NAND - Board: MPC8569MDS, Options: NAND
make: *** [u-boot] Error 1
powerpc-linux-size: './u-boot': No such file
drivers/mmc/libmmc.o: In function `esdhc_setup_data':
/home/marex/testing/u-boot.git/drivers/mmc/fsl_esdhc.c:193: undefined reference 
to `flush_dcache_range'
drivers/mmc/libmmc.o: In function `check_and_invalidate_dcache_range':
/home/marex/testing/u-boot.git/drivers/mmc/fsl_esdhc.c:263: undefined reference 
to `invalidate_dcache_range'
make: *** [u-boot] Error 1

Can this be caused by this patch?

Best regards,
Marek Vasut

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

[Dirk Behme]

On 20.07.2012 04:32, Marek Vasut wrote:
> Dear Dirk Behme,
> 
> [...]
> 
>>>> I'm not likely to have any time until after FTF next week.
>>>>
>>>> Any chance that a PPC maintainer can chime in here?
>>> I think WD applied the cache stub patch already. Can you try now please?
>> Ping ;)
>>
>> Thanks
>>
>> Dirk
> 
> I just noticed this with latest mainline:
> Configuring for MPC8569MDS_ATM - Board: MPC8569MDS, Options: ATM
> make: *** [u-boot] Error 1
> powerpc-linux-size: './u-boot': No such file
> drivers/mmc/libmmc.o: In function `esdhc_setup_data':
> /home/marex/testing/u-boot.git/drivers/mmc/fsl_esdhc.c:193: undefined reference 
> to `flush_dcache_range'
> drivers/mmc/libmmc.o: In function `check_and_invalidate_dcache_range':
> /home/marex/testing/u-boot.git/drivers/mmc/fsl_esdhc.c:263: undefined reference 
> to `invalidate_dcache_range'
> make: *** [u-boot] Error 1
> Configuring for MPC8569MDS_NAND - Board: MPC8569MDS, Options: NAND
> make: *** [u-boot] Error 1
> powerpc-linux-size: './u-boot': No such file
> drivers/mmc/libmmc.o: In function `esdhc_setup_data':
> /home/marex/testing/u-boot.git/drivers/mmc/fsl_esdhc.c:193: undefined reference 
> to `flush_dcache_range'
> drivers/mmc/libmmc.o: In function `check_and_invalidate_dcache_range':
> /home/marex/testing/u-boot.git/drivers/mmc/fsl_esdhc.c:263: undefined reference 
> to `invalidate_dcache_range'
> make: *** [u-boot] Error 1
> 
> Can this be caused by this patch?

Yes, I think so.

Reading the logs, to my understanding the conclusion in

http://lists.denx.de/pipermail/u-boot/2012-May/124036.html

was to "add an empty flush_dcache_range() to the MPC85xx, maybe as weak 
function".

Then

http://lists.denx.de/pipermail/u-boot/2012-June/126097.html

mentions "WD applied the cache stub patch already".

So do we have this "cache stub patch" somewhere which fixes the issue 
mentioned above? I'm not sure I've seen it ...

Best regards

Dirk

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

[Stefano Babic]

On 20/07/2012 07:35, Dirk Behme wrote:

> Then
> 
> http://lists.denx.de/pipermail/u-boot/2012-June/126097.html
> 
> mentions "WD applied the cache stub patch already".
> 
> So do we have this "cache stub patch" somewhere which fixes the issue
> mentioned above? I'm not sure I've seen it ...

It is done, but I see it is compiled only if USB is activated. We need a
fix like:


diff --git a/arch/powerpc/cpu/mpc85xx/Makefile
b/arch/powerpc/cpu/mpc85xx/Makefile
index 7d65aa7..34f6c54 100644
--- a/arch/powerpc/cpu/mpc85xx/Makefile
+++ b/arch/powerpc/cpu/mpc85xx/Makefile
@@ -131,7 +131,7 @@ COBJS       += tlb.o
 COBJS  += traps.o

 # Stub implementations of cache management functions for USB
-COBJS-$(CONFIG_USB_EHCI) += cache.o
+COBJS += cache.o

 SRCS   := $(START:.o=.S) $(SOBJS:.o=.S) $(COBJS:.o=.c)
 OBJS   := $(addprefix $(obj),$(SOBJS) $(COBJS))

I send a proper patch.

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
=====================================================================