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.

Regards

Reply via email to