On 4/10/20 4:58 PM, Patrick DELAUNAY wrote: > Hi Marek, > >> From: Marek Vasut <ma...@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.delau...@st.com>; u-boot@lists.denx.de >>>> >>>> On 4/9/20 12:01 PM, Patrick DELAUNAY wrote: >>>>> Dear Marek, >>>> >>>> Hi, >>>> >>>>>> From: Uboot-stm32 >>>>>> <uboot-stm32-boun...@st-md-mailman.stormreply.com> >>>>>> On Behalf Of Patrick DELAUNAY >>>>>> >>>>>> Dear Marek, >>>>>> >>>>>>> From: Marek Vasut <ma...@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.