lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 18 Mar 2015 09:13:27 +0100 From: Daniel Lezcano <daniel.lezcano@...aro.org> To: Stephen Boyd <sboyd@...eaurora.org>, Lorenzo Pieralisi <lorenzo.pieralisi@....com> CC: "rjw@...ysocki.net" <rjw@...ysocki.net>, "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Catalin Marinas <Catalin.Marinas@....com>, "robherring2@...il.com" <robherring2@...il.com>, "arnd@...db.de" <arnd@...db.de>, "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, "lina.iyer@...aro.org" <lina.iyer@...aro.org> Subject: Re: [PATCH 2/6] ARM: cpuidle: Add a cpuidle ops structure to be used for DT On 03/18/2015 02:14 AM, Stephen Boyd wrote: > On 03/17/15 04:29, Lorenzo Pieralisi wrote: >> On Mon, Mar 16, 2015 at 10:08:19PM +0000, Stephen Boyd wrote: >>> On 03/03/15 04:29, Daniel Lezcano wrote: >>>> The code is optimized to use the __init section intensively in order to reduce >>>> the memory footprint after the driver is initialized and unify the function >>>> names with ARM64. >>>> >>>> In order to prevent multiple declarations and the specific cpuidle ops to be >>>> spread across the different headers, a mechanism, similar to the cgroup subsys, >>>> has been introduced. >>>> >>>> A new platform willing to add its cpuidle ops must add an entry in the file >>>> cpuidle_ops.h in the current form: >>>> >>>> #if IS_ENABLED(CONFIG_ARM_FOO_CPUIDLE) >>>> CPUIDLE_OPS(foo) >>>> #endif >>>> >>>> ... and use the variable name in the specific low level code: >>>> >>>> struct cpuidle_ops foo_cpuidle_ops; >>>> >>>> The CPUIDLE_OPS macro will be processed in different way in the cpuidle.c file, >>>> thus allowing to keep untouched the arm cpuidle core code in the future when >>>> a new platform is added. >>> [...] >>>> diff --git a/arch/arm/include/asm/cpuidle_ops.h b/arch/arm/include/asm/cpuidle_ops.h >>>> new file mode 100644 >>>> index 0000000..be0a612 >>>> --- /dev/null >>>> +++ b/arch/arm/include/asm/cpuidle_ops.h >>>> @@ -0,0 +1,3 @@ >>>> +/* >>>> + * List of cpuidle operations >>>> + */ >>>> diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c >>>> index 45969f8..25e9789c 100644 >>>> --- a/arch/arm/kernel/cpuidle.c >>>> +++ b/arch/arm/kernel/cpuidle.c >>>> @@ -10,8 +10,29 @@ >>>> */ >>>> >>>> #include <linux/cpuidle.h> >>>> +#include <linux/of.h> >>>> +#include <linux/of_device.h> >>>> #include <asm/cpuidle.h> >>>> >>>> +#define CPUIDLE_OPS(__x) extern struct cpuidle_ops __x ## _cpuidle_ops; >>>> +#include <asm/cpuidle_ops.h> >>>> +#undef CPUIDLE_OPS >>>> + >>>> +#define CPUIDLE_OPS(__x) __x ## _cpuidle_ops_id, >>>> +enum cpuidle_ops_id { >>>> +#include <asm/cpuidle_ops.h> >>>> + CPUIDLE_OPS_COUNT, >>>> +}; >>>> +#undef CPUIDLE_OPS >>>> + >>>> +#define CPUIDLE_OPS(__x) [__x ## _cpuidle_ops_id ] = &__x ## _cpuidle_ops, >>>> +static struct cpuidle_ops *supported_cpuidle_ops[] __initconst = { >>>> +#include <asm/cpuidle_ops.h> >>>> +}; >>>> +#undef CPUIDLE_OPS >>>> + >>>> +static struct cpuidle_ops cpuidle_ops[NR_CPUS]; >>> Is there any reason why we aren't putting these structures into a linker >>> section like we do for the smp operations structures? >> I think it can be done with an OF_TABLE, it is a bit of shame cpuidle_ops >> should work on UP too otherwise they could have been merged in >> smp_ops to create cpu_ops, like arm64 does. > > We should merge the two and remove the SMP dependency on arm32. I will be happy to do that but right now it would be nice to keep focused on bringing the cpuidle ops first, even if we have a bit of code duplicated, in order to unblock the cpuidle drivers awaiting for this code to be merged. >>> The nice thing about using the linker is it makes it clearer at the >>> location where we define the structure that it's actually used by >>> something. Right now the structures are defined non-static in a file and >>> then we have to know that a CPUIDLE_OPS() define has been made in >>> another architecture specific asm header file so that this macro magic >>> works. The commit text says something about multiple declarations and >>> ops spread across header files, which shouldn't apply if we're using the >>> linker to find these ops and merge them into an array we can iterate over. >> It makes sense, see above for UP vs SMP. I wonder if we can't find >> something to overcome the UP limitation nicely, the init code in >> arch/arm/kernel/devtree.c is identical for smp_ops and cpuidle_ops, >> apart from the CONFIG_SMP ifdeffery. > > It should be possible to replace the arm32 smp_operations structure with > something like the arm64 cpu_operations structure. Yes we would have to > drop the SMP dependency, but that will be ok. It would require some work > to make arm32 and arm64 the same, but for these purposes that isn't > really required as long as we can put the cpu idle hook there. > -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists