Re: [PATCH v3 5/6] arm: exynos: Add MCPM call-back functions

[Nicolas Pitre <nicolas.pitre-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]

On Tue, 29 Apr 2014, Abhilash Kesavan wrote:

> >> +/*
> >> + * Enable cluster-level coherency, in preparation for turning on the MMU.
> >> + */
> >> +static void __naked exynos_pm_power_up_setup(unsigned int affinity_level)
> >> +{
> >> +       asm volatile ("\n"
> >> +       "cmp    r0, #1\n"
> >> +       "bxne   lr\n"
> >> +       "b      cci_enable_port_for_self");
> >> +}
> >
> > How many times are we going to duplicate this function before we decide
> > to move it to a common header ?
> I see this being used in arch/arm/mach-vexpress/tc2_pm.c (where I
> copied it from for exynos) and arch/arm/mach-vexpress/dcscb.c. A
> common function named "mcpm_default_power_up_setup" in the mcpm header
> would be acceptable ?

Not necessarily.

First of all, this can't be a static inline as we need a pointer to it.  
And moving this to a header would create multiple instances of the same 
function in a multiplatform build for example.

Furthermore, this can't be called "default_power_up_setup" as this is 
specific to systems with a CCI.

It is true that the code in dcscb_setup.S does the same thing as this 
3-line inline assembly, but the former is heavily commented and may 
serve as an example template for platforms that may require something 
more sophisticated here.

There are other patterns that seem to emerge as more MCPM backends are 
being submitted.  I'm making a list of them and I intend to address such 
duplications after those backends do hit mainline.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html