| Message ID | 20200403102815.2.Ib6abcb05422a74bc6bc03daa71b15c98c99dbc5d@changeid |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] arm: caches: protect dram_bank_mmu_setup access to bi_dram | expand |
On 4/3/20 10:28 AM, Patrick Delaunay wrote: > Add the new flags DCACHE_DEFAULT_OPTION to define the default > option to use according the compilation flags > CONFIG_SYS_ARM_CACHE_WRITETHROUGH or CONFIG_SYS_ARM_CACHE_WRITEALLOC. Can't you unify these macros into a single Kconfig "select" statement instead , and then just select the matching cache configuration in Kconfig ? Or better yet, can't you extract this info from DT ?
Dear Marek, > From: Marek Vasut <marex at denx.de> > Sent: vendredi 3 avril 2020 23:29 > > On 4/3/20 10:28 AM, Patrick Delaunay wrote: > > Add the new flags DCACHE_DEFAULT_OPTION to define the default option > > to use according the compilation flags > > CONFIG_SYS_ARM_CACHE_WRITETHROUGH or > CONFIG_SYS_ARM_CACHE_WRITEALLOC. > > Can't you unify these macros into a single Kconfig "select" statement instead , > and then just select the matching cache configuration in Kconfig ? Yes I will try, with 2 steps - migrate existing CONFIG_SYS_ARM_CACHE_.... in Kconfig - add new option CONFIG_SYS_ARM_CACHE_OPTION > Or better yet, can't you extract this info from DT ? I don't think.... it is called before device tree parsing Patrick
On 4/8/20 8:16 PM, Patrick DELAUNAY wrote: > Dear Marek, > >> From: Marek Vasut <marex at denx.de> >> Sent: vendredi 3 avril 2020 23:29 >> >> On 4/3/20 10:28 AM, Patrick Delaunay wrote: >>> Add the new flags DCACHE_DEFAULT_OPTION to define the default option >>> to use according the compilation flags >>> CONFIG_SYS_ARM_CACHE_WRITETHROUGH or >> CONFIG_SYS_ARM_CACHE_WRITEALLOC. >> >> Can't you unify these macros into a single Kconfig "select" statement instead , >> and then just select the matching cache configuration in Kconfig ? > > Yes I will try, with 2 steps > - migrate existing CONFIG_SYS_ARM_CACHE_.... in Kconfig > - add new option CONFIG_SYS_ARM_CACHE_OPTION > >> Or better yet, can't you extract this info from DT ? > > I don't think.... it is called before device tree parsing > The FDT access should be set up as one of the first things during U-Boot's boot_init_f , so it should be possible.
Hi > From: Marek Vasut <marex at denx.de> > Sent: mercredi 8 avril 2020 20:18 > > On 4/8/20 8:16 PM, Patrick DELAUNAY wrote: > > Dear Marek, > > > >> From: Marek Vasut <marex at denx.de> > >> Sent: vendredi 3 avril 2020 23:29 > >> > >> On 4/3/20 10:28 AM, Patrick Delaunay wrote: > >>> Add the new flags DCACHE_DEFAULT_OPTION to define the default option > >>> to use according the compilation flags > >>> CONFIG_SYS_ARM_CACHE_WRITETHROUGH or > >> CONFIG_SYS_ARM_CACHE_WRITEALLOC. > >> > >> Can't you unify these macros into a single Kconfig "select" statement > >> instead , and then just select the matching cache configuration in Kconfig ? > > > > Yes I will try, with 2 steps > > - migrate existing CONFIG_SYS_ARM_CACHE_.... in Kconfig > > - add new option CONFIG_SYS_ARM_CACHE_OPTION > > > >> Or better yet, can't you extract this info from DT ? > > > > I don't think.... it is called before device tree parsing > > > > The FDT access should be set up as one of the first things during U-Boot's > boot_init_f , so it should be possible. But I try to activate de dcache to speed-up the device tree parsing.... So the MMU function is now called really early, in arch init. Or I miss something. Patrick
On 4/8/20 9:07 PM, Patrick DELAUNAY wrote: > Hi > >> From: Marek Vasut <marex at denx.de> >> Sent: mercredi 8 avril 2020 20:18 >> >> On 4/8/20 8:16 PM, Patrick DELAUNAY wrote: >>> Dear Marek, >>> >>>> From: Marek Vasut <marex at denx.de> >>>> Sent: vendredi 3 avril 2020 23:29 >>>> >>>> On 4/3/20 10:28 AM, Patrick Delaunay wrote: >>>>> Add the new flags DCACHE_DEFAULT_OPTION to define the default option >>>>> to use according the compilation flags >>>>> CONFIG_SYS_ARM_CACHE_WRITETHROUGH or >>>> CONFIG_SYS_ARM_CACHE_WRITEALLOC. >>>> >>>> Can't you unify these macros into a single Kconfig "select" statement >>>> instead , and then just select the matching cache configuration in Kconfig ? >>> >>> Yes I will try, with 2 steps >>> - migrate existing CONFIG_SYS_ARM_CACHE_.... in Kconfig >>> - add new option CONFIG_SYS_ARM_CACHE_OPTION >>> >>>> Or better yet, can't you extract this info from DT ? >>> >>> I don't think.... it is called before device tree parsing >>> >> >> The FDT access should be set up as one of the first things during U-Boot's >> boot_init_f , so it should be possible. > > But I try to activate de dcache to speed-up the device tree parsing.... > So the MMU function is now called really early, in arch init. > > Or I miss something. Ah, oops, please forget what I said.
Dear Marek, > From: Uboot-stm32 <uboot-stm32-bounces at st-md-mailman.stormreply.com> On > Behalf Of Patrick DELAUNAY > > Dear Marek, > > > From: Marek Vasut <marex at denx.de> > > Sent: vendredi 3 avril 2020 23:29 > > > > On 4/3/20 10:28 AM, Patrick Delaunay wrote: > > > Add the new flags DCACHE_DEFAULT_OPTION to define the default option > > > to use according the compilation flags > > > CONFIG_SYS_ARM_CACHE_WRITETHROUGH or > > CONFIG_SYS_ARM_CACHE_WRITEALLOC. > > > > Can't you unify these macros into a single Kconfig "select" statement > > instead , and then just select the matching cache configuration in Kconfig ? > > Yes I will try, with 2 steps > - migrate existing CONFIG_SYS_ARM_CACHE_.... in Kconfig First step done... I will push it as a separate patchset I think. > - add new option CONFIG_SYS_ARM_CACHE_OPTION In fact it is to difficult to use select because each defines DCACHE_XXX value can have several values they are build according CONFIG_ARM64 / LPAE But, I can't use this define in Kconfig I try : option ARM_OPTION int "option" default DCACHE_WRITETHROUGHT if CONFIG_SYS_ARM_CACHE_WRITETHROUGH default DCACHE_ WRITEALLOC if CONFIG_SYS_ARM_CACHE_ WRITEALLOC default DCACHE_WRITEBACK if CONFIG_SYS_ARM_CACHE_WRITEBACK int and hex is invalid, and string can't be use with "". And I don't found way to build it automatically when option is activated. Any idea ? Regards Patrick
On 4/9/20 12:01 PM, Patrick DELAUNAY wrote: > Dear Marek, Hi, >> From: Uboot-stm32 <uboot-stm32-bounces at st-md-mailman.stormreply.com> On >> Behalf Of Patrick DELAUNAY >> >> Dear Marek, >> >>> From: Marek Vasut <marex at denx.de> >>> Sent: vendredi 3 avril 2020 23:29 >>> >>> On 4/3/20 10:28 AM, Patrick Delaunay wrote: >>>> Add the new flags DCACHE_DEFAULT_OPTION to define the default option >>>> to use according the compilation flags >>>> CONFIG_SYS_ARM_CACHE_WRITETHROUGH or >>> CONFIG_SYS_ARM_CACHE_WRITEALLOC. >>> >>> Can't you unify these macros into a single Kconfig "select" statement >>> instead , and then just select the matching cache configuration in Kconfig ? >> >> Yes I will try, with 2 steps >> - migrate existing CONFIG_SYS_ARM_CACHE_.... in Kconfig > > First step done... > I will push it as a separate patchset I think. > >> - add new option CONFIG_SYS_ARM_CACHE_OPTION > > In fact it is to difficult to use select because each defines > DCACHE_XXX value can have several values > > they are build according CONFIG_ARM64 / LPAE > > But, I can't use this define in Kconfig > > I try : > option ARM_OPTION > int "option" > default DCACHE_WRITETHROUGHT if CONFIG_SYS_ARM_CACHE_WRITETHROUGH > default DCACHE_ WRITEALLOC if CONFIG_SYS_ARM_CACHE_ WRITEALLOC > default DCACHE_WRITEBACK if CONFIG_SYS_ARM_CACHE_WRITEBACK > > int and hex is invalid, and string can't be use with "". > > And I don't found way to build it automatically when option is activated. > > Any idea ? Maybe you can have a select in the Kconfig to set some differently named option, e.g. DCACHE_MODE_WRITE{THROUGH,ALLOC,BACK} and then an ifdef in some header file, e.g. #ifdef CONFIG_DCACHE_MODE_WRITETHROUGH #define ARM_CACHE_MODE DCACHE_WRITETHROUGH ... And then use ARM_CACHE_MODE where you need a value and CONFIG_DCACHE_MODE{...} where you need a config option check. Does this work ?
Dear Marek, > Sent: jeudi 9 avril 2020 12:21 > To: Patrick DELAUNAY <patrick.delaunay at st.com>; u-boot at lists.denx.de > > On 4/9/20 12:01 PM, Patrick DELAUNAY wrote: > > Dear Marek, > > Hi, > > >> From: Uboot-stm32 <uboot-stm32-bounces at st-md-mailman.stormreply.com> > >> On Behalf Of Patrick DELAUNAY > >> > >> Dear Marek, > >> > >>> From: Marek Vasut <marex at denx.de> > >>> Sent: vendredi 3 avril 2020 23:29 > >>> > >>> On 4/3/20 10:28 AM, Patrick Delaunay wrote: > >>>> Add the new flags DCACHE_DEFAULT_OPTION to define the default > >>>> option to use according the compilation flags > >>>> CONFIG_SYS_ARM_CACHE_WRITETHROUGH or > >>> CONFIG_SYS_ARM_CACHE_WRITEALLOC. > >>> > >>> Can't you unify these macros into a single Kconfig "select" > >>> statement instead , and then just select the matching cache configuration in > Kconfig ? > >> > >> Yes I will try, with 2 steps > >> - migrate existing CONFIG_SYS_ARM_CACHE_.... in Kconfig > > > > First step done... > > I will push it as a separate patchset I think. > > > >> - add new option CONFIG_SYS_ARM_CACHE_OPTION > > > > In fact it is to difficult to use select because each defines > > DCACHE_XXX value can have several values > > > > they are build according CONFIG_ARM64 / LPAE > > > > But, I can't use this define in Kconfig > > > > I try : > > option ARM_OPTION > > int "option" > > default DCACHE_WRITETHROUGHT if > CONFIG_SYS_ARM_CACHE_WRITETHROUGH > > default DCACHE_ WRITEALLOC if CONFIG_SYS_ARM_CACHE_ > WRITEALLOC > > default DCACHE_WRITEBACK if > CONFIG_SYS_ARM_CACHE_WRITEBACK > > > > int and hex is invalid, and string can't be use with "". > > > > And I don't found way to build it automatically when option is activated. > > > > Any idea ? > > Maybe you can have a select in the Kconfig to set some differently named option, > e.g. > > DCACHE_MODE_WRITE{THROUGH,ALLOC,BACK} > > and then an ifdef in some header file, e.g. > > #ifdef CONFIG_DCACHE_MODE_WRITETHROUGH > #define ARM_CACHE_MODE DCACHE_WRITETHROUGH ... > > And then use ARM_CACHE_MODE where you need a value and > CONFIG_DCACHE_MODE{...} where you need a config option check. > > Does this work ? I try with string and default (as select is allowed on for bolean or trisate), And I failed :-< I don't found a way to have the de-stringficate the KConfig option to generated the correct define config SYS_ARM_CACHE_POLICY string "Name of the ARM data write cache policy" default WRITEBACK if SYS_ARM_CACHE_WRITEBACK default WRITETHROUGH if SYS_ARM_CACHE_WRITEBACK default WRITEALLOC if SYS_ARM_CACHE_WRITEALLOC #define DCACHE_DEFAULT_OPTION DCACHE_ ## CONFIG_SYS_ARM_CACHE_POLICY => error: ?DCACHE_CONFIG_SYS_ARM_CACHE_POLICY? undeclared (first use in this function); did you mean ?CONFIG_SYS_ARM_CACHE_POLICY?? #define DCACHE_OPTION(s) DCACHE_ ## CONFIG_SYS_ARM_CACHE_POLICY #define DCACHE_DEFAULT_OPTION DCACHE_OPTION(CONFIG_SYS_ARM_CACHE_POLICY) arch/arm/include/asm/system.h:488:26: error: ?DCACHE_? undeclared (first use in this function); did you mean ?DCACHE_OFF?? arch/arm/lib/cache-cp15.c:99:25: error: expected ?)? before string constant The stringification is possible but not the inverse operation (remove the quote)... In my .config, CONFIG_SYS_ARM_CACHE_POLICY="WRITEBACK" Regards Patrick
On 4/9/20 8:06 PM, Patrick DELAUNAY wrote: > Dear Marek, Hi, >> Sent: jeudi 9 avril 2020 12:21 >> To: Patrick DELAUNAY <patrick.delaunay at st.com>; u-boot at lists.denx.de >> >> On 4/9/20 12:01 PM, Patrick DELAUNAY wrote: >>> Dear Marek, >> >> Hi, >> >>>> From: Uboot-stm32 <uboot-stm32-bounces at st-md-mailman.stormreply.com> >>>> On Behalf Of Patrick DELAUNAY >>>> >>>> Dear Marek, >>>> >>>>> From: Marek Vasut <marex at denx.de> >>>>> Sent: vendredi 3 avril 2020 23:29 >>>>> >>>>> On 4/3/20 10:28 AM, Patrick Delaunay wrote: >>>>>> Add the new flags DCACHE_DEFAULT_OPTION to define the default >>>>>> option to use according the compilation flags >>>>>> CONFIG_SYS_ARM_CACHE_WRITETHROUGH or >>>>> CONFIG_SYS_ARM_CACHE_WRITEALLOC. >>>>> >>>>> Can't you unify these macros into a single Kconfig "select" >>>>> statement instead , and then just select the matching cache configuration in >> Kconfig ? >>>> >>>> Yes I will try, with 2 steps >>>> - migrate existing CONFIG_SYS_ARM_CACHE_.... in Kconfig >>> >>> First step done... >>> I will push it as a separate patchset I think. >>> >>>> - add new option CONFIG_SYS_ARM_CACHE_OPTION >>> >>> In fact it is to difficult to use select because each defines >>> DCACHE_XXX value can have several values >>> >>> they are build according CONFIG_ARM64 / LPAE >>> >>> But, I can't use this define in Kconfig >>> >>> I try : >>> option ARM_OPTION >>> int "option" >>> default DCACHE_WRITETHROUGHT if >> CONFIG_SYS_ARM_CACHE_WRITETHROUGH >>> default DCACHE_ WRITEALLOC if CONFIG_SYS_ARM_CACHE_ >> WRITEALLOC >>> default DCACHE_WRITEBACK if >> CONFIG_SYS_ARM_CACHE_WRITEBACK >>> >>> int and hex is invalid, and string can't be use with "". >>> >>> And I don't found way to build it automatically when option is activated. >>> >>> Any idea ? >> >> Maybe you can have a select in the Kconfig to set some differently named option, >> e.g. >> >> DCACHE_MODE_WRITE{THROUGH,ALLOC,BACK} >> >> and then an ifdef in some header file, e.g. >> >> #ifdef CONFIG_DCACHE_MODE_WRITETHROUGH >> #define ARM_CACHE_MODE DCACHE_WRITETHROUGH ... >> >> And then use ARM_CACHE_MODE where you need a value and >> CONFIG_DCACHE_MODE{...} where you need a config option check. >> >> Does this work ? > > I try with string and default (as select is allowed on for bolean or trisate), > And I failed :-< > > I don't found a way to have the de-stringficate the KConfig option > to generated the correct define The result is a boolean , isn't it ? One out of N configs ends up being defined and the rest are not defined. > config SYS_ARM_CACHE_POLICY > string "Name of the ARM data write cache policy" > default WRITEBACK if SYS_ARM_CACHE_WRITEBACK > default WRITETHROUGH if SYS_ARM_CACHE_WRITEBACK > default WRITEALLOC if SYS_ARM_CACHE_WRITEALLOC > > #define DCACHE_DEFAULT_OPTION DCACHE_ ## CONFIG_SYS_ARM_CACHE_POLICY > > => error: ?DCACHE_CONFIG_SYS_ARM_CACHE_POLICY? undeclared (first use in this function); did you mean ?CONFIG_SYS_ARM_CACHE_POLICY?? > > #define DCACHE_OPTION(s) DCACHE_ ## CONFIG_SYS_ARM_CACHE_POLICY > > #define DCACHE_DEFAULT_OPTION DCACHE_OPTION(CONFIG_SYS_ARM_CACHE_POLICY) > > arch/arm/include/asm/system.h:488:26: error: ?DCACHE_? undeclared (first use in this function); did you mean ?DCACHE_OFF?? > arch/arm/lib/cache-cp15.c:99:25: error: expected ?)? before string constant > > The stringification is possible but not the inverse operation (remove the quote)... > > In my .config, CONFIG_SYS_ARM_CACHE_POLICY="WRITEBACK" What about this: choice prompt "Cache policy" default CACHE_WRITEBACK config CACHE_WRITEBACK bool "Writeback" config ... endchoice and then in some header #ifdef CONFIG_CACHE_WRITEBACK #define CONFIF_SYS_ARM_CACHE_WRITEBACK #else ... Would that work ? But I feel I might really be missing the question here.
Hi Marek, > From: Marek Vasut <marex at denx.de> > Sent: vendredi 10 avril 2020 10:14 > > On 4/9/20 8:06 PM, Patrick DELAUNAY wrote: > > Dear Marek, > > Hi, > > >> Sent: jeudi 9 avril 2020 12:21 > >> To: Patrick DELAUNAY <patrick.delaunay at st.com>; u-boot at lists.denx.de > >> > >> On 4/9/20 12:01 PM, Patrick DELAUNAY wrote: > >>> Dear Marek, > >> > >> Hi, > >> > >>>> From: Uboot-stm32 > >>>> <uboot-stm32-bounces at st-md-mailman.stormreply.com> > >>>> On Behalf Of Patrick DELAUNAY > >>>> > >>>> Dear Marek, > >>>> > >>>>> From: Marek Vasut <marex at denx.de> > >>>>> Sent: vendredi 3 avril 2020 23:29 > >>>>> > >>>>> On 4/3/20 10:28 AM, Patrick Delaunay wrote: > >>>>>> Add the new flags DCACHE_DEFAULT_OPTION to define the default > >>>>>> option to use according the compilation flags > >>>>>> CONFIG_SYS_ARM_CACHE_WRITETHROUGH or > >>>>> CONFIG_SYS_ARM_CACHE_WRITEALLOC. > >>>>> > >>>>> Can't you unify these macros into a single Kconfig "select" > >>>>> statement instead , and then just select the matching cache > >>>>> configuration in > >> Kconfig ? > >>>> > >>>> Yes I will try, with 2 steps > >>>> - migrate existing CONFIG_SYS_ARM_CACHE_.... in Kconfig > >>> > >>> First step done... > >>> I will push it as a separate patchset I think. > >>> > >>>> - add new option CONFIG_SYS_ARM_CACHE_OPTION > >>> > >>> In fact it is to difficult to use select because each defines > >>> DCACHE_XXX value can have several values > >>> > >>> they are build according CONFIG_ARM64 / LPAE > >>> > >>> But, I can't use this define in Kconfig > >>> > >>> I try : > >>> option ARM_OPTION > >>> int "option" > >>> default DCACHE_WRITETHROUGHT if > >> CONFIG_SYS_ARM_CACHE_WRITETHROUGH > >>> default DCACHE_ WRITEALLOC if CONFIG_SYS_ARM_CACHE_ > >> WRITEALLOC > >>> default DCACHE_WRITEBACK if > >> CONFIG_SYS_ARM_CACHE_WRITEBACK > >>> > >>> int and hex is invalid, and string can't be use with "". > >>> > >>> And I don't found way to build it automatically when option is activated. > >>> > >>> Any idea ? > >> > >> Maybe you can have a select in the Kconfig to set some differently > >> named option, e.g. > >> > >> DCACHE_MODE_WRITE{THROUGH,ALLOC,BACK} > >> > >> and then an ifdef in some header file, e.g. > >> > >> #ifdef CONFIG_DCACHE_MODE_WRITETHROUGH #define > ARM_CACHE_MODE > >> DCACHE_WRITETHROUGH ... > >> > >> And then use ARM_CACHE_MODE where you need a value and > >> CONFIG_DCACHE_MODE{...} where you need a config option check. > >> > >> Does this work ? > > > > I try with string and default (as select is allowed on for bolean or > > trisate), And I failed :-< > > > > I don't found a way to have the de-stringficate the KConfig option to > > generated the correct define > > The result is a boolean , isn't it ? One out of N configs ends up being defined and > the rest are not defined. > > > config SYS_ARM_CACHE_POLICY > > string "Name of the ARM data write cache policy" > > default WRITEBACK if SYS_ARM_CACHE_WRITEBACK > > default WRITETHROUGH if SYS_ARM_CACHE_WRITEBACK > > default WRITEALLOC if SYS_ARM_CACHE_WRITEALLOC > > > > #define DCACHE_DEFAULT_OPTION DCACHE_ ## > CONFIG_SYS_ARM_CACHE_POLICY > > > > => error: ?DCACHE_CONFIG_SYS_ARM_CACHE_POLICY? undeclared (first > use in this function); did you mean ?CONFIG_SYS_ARM_CACHE_POLICY?? > > > > #define DCACHE_OPTION(s) DCACHE_ ## > CONFIG_SYS_ARM_CACHE_POLICY > > > > #define DCACHE_DEFAULT_OPTION > DCACHE_OPTION(CONFIG_SYS_ARM_CACHE_POLICY) > > > > arch/arm/include/asm/system.h:488:26: error: ?DCACHE_? undeclared (first use > in this function); did you mean ?DCACHE_OFF?? > > arch/arm/lib/cache-cp15.c:99:25: error: expected ?)? before string > > constant > > > > The stringification is possible but not the inverse operation (remove the quote)... > > > > In my .config, CONFIG_SYS_ARM_CACHE_POLICY="WRITEBACK" > > What about this: > > choice > prompt "Cache policy" > default CACHE_WRITEBACK > > config CACHE_WRITEBACK > bool "Writeback" > > config ... > > endchoice > > and then in some header > > #ifdef CONFIG_CACHE_WRITEBACK > #define CONFIF_SYS_ARM_CACHE_WRITEBACK > #else > ... > > Would that work ? Yes, it can work it seems complicated I push v2 for CONFIG_SYS_ARM_CACHE_* migration in Kconfig My proposal becomes: +#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH) +#define DCACHE_DEFAULT_OPTION DCACHE_WRITETHROUGH +#elif defined(CONFIG_SYS_ARM_CACHE_WRITEALLOC) +#define DCACHE_DEFAULT_OPTION DCACHE_WRITEALLOC +#elif defined(CONFIG_SYS_ARM_CACHE_WRITEBACK) +#define DCACHE_DEFAULT_OPTION DCACHE_WRITEBACK +#endif + I think it is is more clear solution. I can use macro magic to build DCACHE_DEFAULT_OPTION +#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH) +#define ARM_CACHE_POLICY WRITETHROUGH +#elif defined(CONFIG_SYS_ARM_CACHE_WRITEALLOC) +#define ARM_CACHE_POLICY WRITEALLOC +#elif defined(CONFIG_SYS_ARM_CACHE_WRITEBACK) +#define ARM_CACHE_POLICY WRITEBACK +#endif +#define _DCACHE_OPTION(policy) DCACHE_ ## policy +#define DCACHE_OPTION(policy) _DCACHE_OPTION(policy) +#define DCACHE_DEFAULT_OPTION DCACHE_OPTION(ARM_CACHE_POLICY) + > > But I feel I might really be missing the question here. I think I will push V2 in one week (I am out of office next week) to wait other feedback. It could be more clear with an updated patchset. Regards
On 4/10/20 4:58 PM, Patrick DELAUNAY wrote: > Hi Marek, > >> From: Marek Vasut <marex at denx.de> >> Sent: vendredi 10 avril 2020 10:14 >> >> On 4/9/20 8:06 PM, Patrick DELAUNAY wrote: >>> Dear Marek, >> >> Hi, >> >>>> Sent: jeudi 9 avril 2020 12:21 >>>> To: Patrick DELAUNAY <patrick.delaunay at st.com>; u-boot at lists.denx.de >>>> >>>> On 4/9/20 12:01 PM, Patrick DELAUNAY wrote: >>>>> Dear Marek, >>>> >>>> Hi, >>>> >>>>>> From: Uboot-stm32 >>>>>> <uboot-stm32-bounces at st-md-mailman.stormreply.com> >>>>>> On Behalf Of Patrick DELAUNAY >>>>>> >>>>>> Dear Marek, >>>>>> >>>>>>> From: Marek Vasut <marex at denx.de> >>>>>>> Sent: vendredi 3 avril 2020 23:29 >>>>>>> >>>>>>> On 4/3/20 10:28 AM, Patrick Delaunay wrote: >>>>>>>> Add the new flags DCACHE_DEFAULT_OPTION to define the default >>>>>>>> option to use according the compilation flags >>>>>>>> CONFIG_SYS_ARM_CACHE_WRITETHROUGH or >>>>>>> CONFIG_SYS_ARM_CACHE_WRITEALLOC. >>>>>>> >>>>>>> Can't you unify these macros into a single Kconfig "select" >>>>>>> statement instead , and then just select the matching cache >>>>>>> configuration in >>>> Kconfig ? >>>>>> >>>>>> Yes I will try, with 2 steps >>>>>> - migrate existing CONFIG_SYS_ARM_CACHE_.... in Kconfig >>>>> >>>>> First step done... >>>>> I will push it as a separate patchset I think. >>>>> >>>>>> - add new option CONFIG_SYS_ARM_CACHE_OPTION >>>>> >>>>> In fact it is to difficult to use select because each defines >>>>> DCACHE_XXX value can have several values >>>>> >>>>> they are build according CONFIG_ARM64 / LPAE >>>>> >>>>> But, I can't use this define in Kconfig >>>>> >>>>> I try : >>>>> option ARM_OPTION >>>>> int "option" >>>>> default DCACHE_WRITETHROUGHT if >>>> CONFIG_SYS_ARM_CACHE_WRITETHROUGH >>>>> default DCACHE_ WRITEALLOC if CONFIG_SYS_ARM_CACHE_ >>>> WRITEALLOC >>>>> default DCACHE_WRITEBACK if >>>> CONFIG_SYS_ARM_CACHE_WRITEBACK >>>>> >>>>> int and hex is invalid, and string can't be use with "". >>>>> >>>>> And I don't found way to build it automatically when option is activated. >>>>> >>>>> Any idea ? >>>> >>>> Maybe you can have a select in the Kconfig to set some differently >>>> named option, e.g. >>>> >>>> DCACHE_MODE_WRITE{THROUGH,ALLOC,BACK} >>>> >>>> and then an ifdef in some header file, e.g. >>>> >>>> #ifdef CONFIG_DCACHE_MODE_WRITETHROUGH #define >> ARM_CACHE_MODE >>>> DCACHE_WRITETHROUGH ... >>>> >>>> And then use ARM_CACHE_MODE where you need a value and >>>> CONFIG_DCACHE_MODE{...} where you need a config option check. >>>> >>>> Does this work ? >>> >>> I try with string and default (as select is allowed on for bolean or >>> trisate), And I failed :-< >>> >>> I don't found a way to have the de-stringficate the KConfig option to >>> generated the correct define >> >> The result is a boolean , isn't it ? One out of N configs ends up being defined and >> the rest are not defined. >> >>> config SYS_ARM_CACHE_POLICY >>> string "Name of the ARM data write cache policy" >>> default WRITEBACK if SYS_ARM_CACHE_WRITEBACK >>> default WRITETHROUGH if SYS_ARM_CACHE_WRITEBACK >>> default WRITEALLOC if SYS_ARM_CACHE_WRITEALLOC >>> >>> #define DCACHE_DEFAULT_OPTION DCACHE_ ## >> CONFIG_SYS_ARM_CACHE_POLICY >>> >>> => error: ?DCACHE_CONFIG_SYS_ARM_CACHE_POLICY? undeclared (first >> use in this function); did you mean ?CONFIG_SYS_ARM_CACHE_POLICY?? >>> >>> #define DCACHE_OPTION(s) DCACHE_ ## >> CONFIG_SYS_ARM_CACHE_POLICY >>> >>> #define DCACHE_DEFAULT_OPTION >> DCACHE_OPTION(CONFIG_SYS_ARM_CACHE_POLICY) >>> >>> arch/arm/include/asm/system.h:488:26: error: ?DCACHE_? undeclared (first use >> in this function); did you mean ?DCACHE_OFF?? >>> arch/arm/lib/cache-cp15.c:99:25: error: expected ?)? before string >>> constant >>> >>> The stringification is possible but not the inverse operation (remove the quote)... >>> >>> In my .config, CONFIG_SYS_ARM_CACHE_POLICY="WRITEBACK" >> >> What about this: >> >> choice >> prompt "Cache policy" >> default CACHE_WRITEBACK >> >> config CACHE_WRITEBACK >> bool "Writeback" >> >> config ... >> >> endchoice >> >> and then in some header >> >> #ifdef CONFIG_CACHE_WRITEBACK >> #define CONFIF_SYS_ARM_CACHE_WRITEBACK >> #else >> ... >> >> Would that work ? > > Yes, it can work it seems complicated > > I push v2 for CONFIG_SYS_ARM_CACHE_* migration in Kconfig > > My proposal becomes: > > +#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH) > +#define DCACHE_DEFAULT_OPTION DCACHE_WRITETHROUGH > +#elif defined(CONFIG_SYS_ARM_CACHE_WRITEALLOC) > +#define DCACHE_DEFAULT_OPTION DCACHE_WRITEALLOC > +#elif defined(CONFIG_SYS_ARM_CACHE_WRITEBACK) > +#define DCACHE_DEFAULT_OPTION DCACHE_WRITEBACK > +#endif > + > > I think it is is more clear solution. > > > I can use macro magic to build DCACHE_DEFAULT_OPTION > > +#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH) > +#define ARM_CACHE_POLICY WRITETHROUGH > +#elif defined(CONFIG_SYS_ARM_CACHE_WRITEALLOC) > +#define ARM_CACHE_POLICY WRITEALLOC > +#elif defined(CONFIG_SYS_ARM_CACHE_WRITEBACK) > +#define ARM_CACHE_POLICY WRITEBACK > +#endif > > +#define _DCACHE_OPTION(policy) DCACHE_ ## policy > +#define DCACHE_OPTION(policy) _DCACHE_OPTION(policy) > +#define DCACHE_DEFAULT_OPTION DCACHE_OPTION(ARM_CACHE_POLICY) > + > >> >> But I feel I might really be missing the question here. > > I think I will push V2 in one week (I am out of office next week) to wait other feedback. > > It could be more clear with an updated patchset. That's a good idea, let's see.
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h index 81ccead112..01ea96e8ad 100644 --- a/arch/arm/include/asm/system.h +++ b/arch/arm/include/asm/system.h @@ -485,6 +485,14 @@ enum dcache_option { }; #endif +#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH) +#define DCACHE_DEFAULT_OPTION DCACHE_WRITETHROUGH +#elif defined(CONFIG_SYS_ARM_CACHE_WRITEALLOC) +#define DCACHE_DEFAULT_OPTION DCACHE_WRITEALLOC +#else +#define DCACHE_DEFAULT_OPTION DCACHE_WRITEBACK +#endif + /* Size of an MMU section */ enum { #ifdef CONFIG_ARMV7_LPAE diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c index 54509f11c3..d15144188b 100644 --- a/arch/arm/lib/cache-cp15.c +++ b/arch/arm/lib/cache-cp15.c @@ -99,15 +99,8 @@ __weak void dram_bank_mmu_setup(int bank) for (i = bd->bi_dram[bank].start >> MMU_SECTION_SHIFT; i < (bd->bi_dram[bank].start >> MMU_SECTION_SHIFT) + (bd->bi_dram[bank].size >> MMU_SECTION_SHIFT); - i++) { -#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH) - set_section_dcache(i, DCACHE_WRITETHROUGH); -#elif defined(CONFIG_SYS_ARM_CACHE_WRITEALLOC) - set_section_dcache(i, DCACHE_WRITEALLOC); -#else - set_section_dcache(i, DCACHE_WRITEBACK); -#endif - } + i++) + set_section_dcache(i, DCACHE_DEFAULT_OPTION); } /* to activate the MMU we need to set up virtual memory: use 1M areas */
Add the new flags DCACHE_DEFAULT_OPTION to define the default option to use according the compilation flags CONFIG_SYS_ARM_CACHE_WRITETHROUGH or CONFIG_SYS_ARM_CACHE_WRITEALLOC. This new compilation flag allows to simplify dram_bank_mmu_setup() and can be used as third parameter (option=dcache option to select) of mmu_set_region_dcache_behaviour function. Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com> --- arch/arm/include/asm/system.h | 8 ++++++++ arch/arm/lib/cache-cp15.c | 11 ++--------- 2 files changed, 10 insertions(+), 9 deletions(-)