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

Dave Martin Dave.Martin at arm.com
Wed Apr 30 03:24:18 PDT 2014


On Wed, Apr 30, 2014 at 04:01:24AM +0100, Abhilash Kesavan wrote:
> Hi Nicolas,
> 
> On Wed, Apr 30, 2014 at 12:19 AM, Nicolas Pitre
> <nicolas.pitre at linaro.org> wrote:
> > 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.

On some SoCs the argument to cmp might not be #1, or the constant required
might not even be the same on all CPUs.  Some SoCs might need to do other
setup too, such as invalidating a cluster-level cache that doesn't power
up into a clean state.

So, the above function is going to be common but it's not really generic.

For such a tiny function, I agree that factoring it may not be that
beneficial.

Cheers
---Dave



More information about the linux-arm-kernel mailing list