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@xxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
