Dear Dirk, 2009/9/5 Dirk Behme <dirk.be...@googlemail.com>
> Minkyu Kang wrote: > >> Dear, Dirk >> >> 2009/9/4 Dirk Behme <dirk.be...@googlemail.com> >> >> Kyungmin Park wrote: >>> >>>> On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behme<dirk.be...@googlemail.com> >>>> >>> wrote: >>> >>>> Kyungmin Park wrote: >>>>> >>>> ... >>> >>>> + if (get_device_type() != 0xC100) { >>>>>>>> >>>>>>> Hmm, what is this "0xC100" ? >>>>>>> >>>>>> Now we got two cpu, s5pc100 and s5pc110. In case of s5pc100 we don't >>>>>> need to turn off l2 cache. but s5pc110 needs it. >>>>>> So first check the device type, actually cpu type. then determine turn >>>>>> off l2 cache or not. >>>>>> >>>>> "0xC100" is the device type of s5pc100 then? So it should be >>>>> >>>>> if (get_device_type() != S5PC100_DEVICE) >>>>> >>>>> ? I hear some people crying "please use macro" ;) >>>>> >>>> Agreed. DONT_NEED_CACHE_FLUSH? >>>> >>>> But I don't like this selection here. When we get additional similar >>>>> >>>> SoCs, >>> >>>> we will end with something like >>>>> >>>>> if (get_device_type() != 0xC100) || >>>>> (get_device_type() != FOO) || >>>>> (get_device_type() != BAR)) || >>>>> ... { >>>>> >>>>> modifying each time cpu/arm_cortexa8/cpu.c. >>>>> >>>>> I would like more that we are able to compile the functionality based >>>>> on >>>>> >>>> the >>> >>>> config file we use for compilation. E.g. provide emtpy >>>>> >>>> l2_cache_disable(); >>> >>>> function for SoCs that don't need it, but have functionality behind it >>>>> >>>> where >>> >>>> needed. >>>>> >>>>> With above patch, this would then become something like >>>>> >>>>> cpu/arm_cortexa8/s5pcxxx/dcache.S >>>>> >>>>> -> Implements invalidate_dcache() (or implement a Cortex A8 generic one >>>>> >>>> in >>> >>>> cpu/arm_cortexa8/cache.S) >>>>> >>>>> cpu/arm_cortexa8/s5pcxxx/cache_110.S >>>>> >>>>> -> Implements l2_cache_enable()/disable() >>>>> >>>>> cpu/arm_cortexa8/s5pcxxx/cache_100.S >>>>> >>>>> -> Implements *empty* l2_cache_enable()/disable() >>>>> >>>>> In cpu/arm_cortexa8/s5pcxxx/Makefile you then could have >>>>> >>>>> SOBJS-y += dcache.o >>>>> SOBJS-$(CONFIG_S5PC100) += cache_100.o >>>>> SOBJS-$(CONFIG_S5PC110) += cache_110.o >>>>> >>>>> What do you think about this? >>>>> >>>>> Basically agreed, of course we can think weak attribute but now we >>>> have to support both cpu simultaneously. >>>> with this reason. we check the device_type at here. >>>> >>> What's about having this check in SoC specific code instead of Cortex >>> A8 generic code, then? >>> >>> E.g apply patch >>> >>> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html >>> >>> and then create >>> >>> cpu/arm_cortexa8/s5pcxxx/cache.S >>> >>> with >>> >>> invalidate_dcache() { >>> if (get_device_type() == S5PC100_DEVICE) >>> return(); >>> ... >>> >>> l2_cache_enable() { >>> if (get_device_type() == S5PC100_DEVICE) >>> return(); >>> ... >>> >>> etc. >>> >>> That is, have the SoC dependent part in SoC specific directory/file. >>> >>> Best regards >>> >>> Dirk >>> >>> >>> >>> _______________________________________________ >>> U-Boot mailing list >>> U-Boot@lists.denx.de >>> http://lists.denx.de/mailman/listinfo/u-boot >>> >>> >> I know about the discussion of this issue between you and Jean-Christophe. >> but it is gone without resolving. >> So, I want to make issue again. >> anyway,, >> >> Actually, we don't need the function of get_device_type() >> I think that function is omap specific function.. isn't it? >> but.. because of current code already use that function, I had to use that >> function >> If you have plan to move the cache routines into SoC, >> I think you can remove the argument for device_type. (check device type in >> omap3's cache routines) >> >> And I want to remove CONFIG_L2_OFF also. >> We can know this through device type or soc type. >> How about make new function? >> e.g l2_off() or need_cache_flush() etc, >> >> Please rework for removing dependency of omap3 soc first. >> > > Just to clarify: It's my understanding that this is already done by > > http://lists.denx.de/pipermail/u-boot/2009-August/058492.html > > Do you agree? That is, when this patch is applied, then Samsung can go on. > Correct? > > If not correct, what is missing in above patch? > > Best regards > > Dirk > > > I have two request. 1. I want to replace CONFIG_L2_OFF define to other function for example.. if (need_cache_flush()) { /* or !l2_off() */ /* turn off L2 cache */ l2_cache_disable(); /* invalidate L2 cache also */ v7_flush_dcache_all(get_device_type()); i = 0; /* mem barrier to sync up things */ asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); l2_cache_enable(); } 2. I don't want to use get_device_type() function (just call like this: v7_flush_decahe_all() ) How you think? thanks Minkyu Kang -- from. prom. www.promsoft.net
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot