[U-Boot] [PATCH] mx53: use CONFIG_SYS_L2CACHE_OFF in config file

[Jason Liu]

Hi, stefano,

2011/7/28 Stefano Babic <sbabic at denx.de>:
> On 07/28/2011 07:56 AM, Jason Liu wrote:
>> Hi, Stefano,
>
> Hi Jason,
>
>>>
>>> It's used before, if you checkout tag: v2011.06
>>>
>>> #ifndef CONFIG_L2_OFF
>>>        /* turn off L2 cache */
>>>        l2_cache_disable();
>>>        /* invalidate L2 cache also */
>>>        invalidate_dcache(get_device_type());
>>> #endif
>>>        i = 0;
>>>        /* mem barrier to sync up things */
>>>        asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i));
>>>
>>> #ifndef CONFIG_L2_OFF
>>>        l2_cache_enable();
>>> #endif
>
> However, as far as I can understand, L2 cache is disabled until
> explicitely enabled.

Look at the meaning from the name X_OFF, if we define it, it tells
people that we want to disable it explicitly. If not define it, it underlying
tells that we want to enable it, right? So, if we remove this define L2_OFF
from board config file, it may cause confuse since we don't want to
enable it now.

>
>>>
>>>
>>> I still think we need explicitly disable L2 cache first
>
> I am still checking this point. My concern is to understand if in the
> current code we need to do something or not. If there is no code to
> enable L2 cache (I have not found), there should be no need to disable
> it. After a reset, L2 cache is disabled, am I right ?
> Why do you think we have to explicitely disable it ? Am I missing
> something ?

Currently, MX5 will disable L2 after reset. some points as above.

>
>>>. If it only
>>> omap related, I don't think we need
>>> CONFIG_SYS_L2CACHE_OFF for the global u-boot. If you grep the
>>> CONFIG_SYS_L2CACHE_OFF
>>> under include/configs, you will see a lot of define other than omap platform.
>
> Nevertheless we have to decide if we need it for the MX5. If we manage
> enable/disable L2 Cache for the MX5, we need it, else not. As I can see
> in the actual code, we do not manage L2 Cache.

Yes, currently, we don't manage L2.
But, I don't know whether the common code will be changed in the future,
if we don't disable L2 explicitly, it will enable L2 by default just
like d-cache.

>
>>> It's due to it enable dcache by default if not define
>>> CONFIG_SYS_DCACHE_OFF explicitly.
>>> mxc_fec driver need be fixed or re-write to consider cache safe. I
>>> agree to disable d-cache first.
>>
>> Do I need submit patch to disable D-CACHE first?
>
> let's see if we agree on the following points:
>
> - CONFIG_L2_OFF is obsolete, as you pointed out, and must be removed
>
> - CONFIG_SYS_L2CACHE_OFF is needed for MX5 if we have code to enable /
> disable it. Let me know if you agree that L2 cache is disabled after a
> reset. Then we do not need this define until we explicitely enable it as
> default, as now for D cache.
>
> - because of D Cache issues in the driver, it is better to disable D
> Cache for MX5 processors.
>
> I think, if we agree on these point, we can manage the changes in a
> single patch (changes are made on the same files, that is the
> configuration file for the boards), and it is enough to add a useful
> comment to explain what we do.

OK, after get agreement, I will do it soon with single patch.

>
>>  And another issue
>> for imx51 is that
>> mmc command does not work correctly sometimes such as saveenv. Do you
>> notice that?
>
> I admit I have not tested recently and I have not noted this issue. Last
> time was after some changes in MMC code.
>
> I am sure we get the same issues for D Cache as for the FEC. In fact,
> for powerpc the fsl_esdhc.c enables cache snooping and does not need to
> invalidate buffers. We have no counterpart for MX5.

Yes, correct.

Jason

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