On Fri, Jul 14, 2017 at 03:33:32AM +0300, Artturi Alm wrote:
> On Fri, Jul 14, 2017 at 03:28:25AM +0300, Artturi Alm wrote:
> > Hi,
> > 
> > i'm having hard time choosing, between this diff, and the one that
> > renames it to for what it does, which would likely incur cleanup
> > elsewhere. current is just wrong.. but i'm open to be taught on this
> > matter anyway.
> > 
> > -Artturi
> > 
> 
> Wonders of manually recreating a quick diff by hand above.. fixed:
> 
> diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c
> index c91108e7066..fd5385043fc 100644
> --- a/sys/arch/arm/arm/cpufunc.c
> +++ b/sys/arch/arm/arm/cpufunc.c
> @@ -291,8 +291,8 @@ armv7_dcache_wbinv_all()
>               for (ways = 0; ways < nways; ways++) {
>                       word = wayval | setval | lvl;
>  
> -                     /* Clean D cache SE with Set/Index */
> -                     __asm volatile("mcr     p15, 0, %0, c7, c10, 2"
> +                     /* Data cache clean and invalidate by set/way */
> +                     __asm volatile("mcr     p15, 0, %0, c7, c14, 2\n"
>                           : : "r" (word));
>                       wayval += wayincr;
>               }

Guessing now, that this is just yet another case still prevalent in cvs
tree, where things are the way they are, and need no cleaning/fixing,
because it was written for !MULTIPROCESSOR + does work as is for GENERIC.

As a side note w/some relevancy to this, and more to the "sysreg.h use
in C". This is what i ended up using in some of my branches where i
don't restrict myself from anything, and i find it useful while cleaning:

diff --git a/sys/arch/arm/include/sysreg.h b/sys/arch/arm/include/sysreg.h
index c2aab7d6667..5d48912494a 100644
--- a/sys/arch/arm/include/sysreg.h
+++ b/sys/arch/arm/include/sysreg.h
@@ -269,4 +269,8 @@
  */
 #define CP15_CBAR(rr)          p15, 4, rr, c15, c0, 0 /* Configuration Base 
Address Register */
 
+#define        _CP_REG_STR(...)        #__VA_ARGS__
+#define        _IA_MRC_RD(x)           "mrc    " _CP_REG_STR(x) "\n"
+#define        _IA_MCR_WR(x)           "mcr    " _CP_REG_STR(x) "\n"
+
 #endif /* !MACHINE_SYSREG_H */


that does look like below in use, i think the mcr/mrc was not enough, so
went w/longer and more verbose name with some duplicity being _X_RealOP_OP,
where RealOP is enough to reveal _OP already...

u_int
cpu_id(void)
{
        u_int _midr;
        asm volatile(_IA_MRC_RD(CP15_MIDR(%0)) : "=r"(_midr));
        return _midr;
}

void
cpu_set_tpidrprw(void *_tpidrprw)
{
        asm volatile(_IA_MCR_WR(CP15_TPIDRPRW(%0)) :: "r"(_tpidrprw));
}

now im still not suggesting this anymore, just wanted to note why i hit
the naming issue of non-invalidating _wbinv_all() again.
-Artturi

Reply via email to