* [PATCH 3/3] ARM: SAMSUNG: Add idle support
@ 2010-01-11 9:44 Kukjin Kim
2010-01-11 10:02 ` Russell King - ARM Linux
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Kukjin Kim @ 2010-01-11 9:44 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds common idle functionality for all Samsung SoC's.
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
---
arch/arm/plat-samsung/include/mach/idle.h | 18 +++++++++++++
arch/arm/plat-samsung/include/mach/system.h | 36 +++++++++++++++++++++++++++
2 files changed, 54 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/plat-samsung/include/mach/idle.h
create mode 100644 arch/arm/plat-samsung/include/mach/system.h
diff --git a/arch/arm/plat-samsung/include/mach/idle.h b/arch/arm/plat-samsung/include/mach/idle.h
new file mode 100644
index 0000000..f3b5034
--- /dev/null
+++ b/arch/arm/plat-samsung/include/mach/idle.h
@@ -0,0 +1,18 @@
+/* linux/arch/arm/plat-samsung/include/mach/idle.h
+ *
+ * Copyright (c) 2009 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com/
+ *
+ * Idle support
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#ifndef __ASM_PLAT_IDLE_H
+#define __ASM_PLAT_IDLE_H __FILE__
+
+extern void (*s3c_idle_fn)(void);
+
+#endif /* __ASM_PLAT_IDLE_H */
diff --git a/arch/arm/plat-samsung/include/mach/system.h b/arch/arm/plat-samsung/include/mach/system.h
new file mode 100644
index 0000000..5377792
--- /dev/null
+++ b/arch/arm/plat-samsung/include/mach/system.h
@@ -0,0 +1,36 @@
+/* linux/arch/arm/plat-samsung/include/mach/idle.h
+ *
+ * Copyright (c) 2009 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com/
+ *
+ * Idle support
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#ifndef __ASM_PLAT_SYSTEM_H
+#define __ASM_PLAT_SYSTEM_H __FILE__
+
+void (*s3c_idle_fn)(void);
+
+static void s3c_default_idle(void)
+{
+ /* nothing here yet */
+}
+
+static void arch_idle(void)
+{
+ if (s3c_idle_fn != NULL)
+ (s3c_idle_fn)();
+ else
+ s3c_default_idle();
+}
+
+static void arch_reset(char mode, const char *cmd)
+{
+ /* nothing here yet */
+}
+
+#endif /* __ASM_PLAT_SYSTEM_H */
--
1.6.2.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/3] ARM: SAMSUNG: Add idle support
2010-01-11 9:44 [PATCH 3/3] ARM: SAMSUNG: Add idle support Kukjin Kim
@ 2010-01-11 10:02 ` Russell King - ARM Linux
2010-01-12 0:06 ` Ben Dooks
2010-01-11 10:35 ` Mark Brown
2010-01-13 0:12 ` Ben Dooks
2 siblings, 1 reply; 9+ messages in thread
From: Russell King - ARM Linux @ 2010-01-11 10:02 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jan 11, 2010 at 06:44:32PM +0900, Kukjin Kim wrote:
> This patch adds common idle functionality for all Samsung SoC's.
NAK. Please use the existing support already provided. There's no point
in double-indirection - it just makes the code harder to follow.
Just set pm_idle to your idle function.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] ARM: SAMSUNG: Add idle support
2010-01-11 10:02 ` Russell King - ARM Linux
@ 2010-01-12 0:06 ` Ben Dooks
2010-01-12 10:12 ` Russell King - ARM Linux
0 siblings, 1 reply; 9+ messages in thread
From: Ben Dooks @ 2010-01-12 0:06 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jan 11, 2010 at 10:02:54AM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 11, 2010 at 06:44:32PM +0900, Kukjin Kim wrote:
> > This patch adds common idle functionality for all Samsung SoC's.
>
> NAK. Please use the existing support already provided. There's no point
> in double-indirection - it just makes the code harder to follow.
>
> Just set pm_idle to your idle function.
Hmm, the default_idle() function that pm_idle is set to at init time does
more than what arch_idle() does. We'd end up having to copy the logic in
default_idle() into each of our idle routines. How difficult would it be
to change the callers of pm_idle() to check whether it actually needs
calling and to re-enable the interrupt state after calling it?
We did this originally as before we issue the WFI co-processor instruction
we need to set the sleep mode. A number of the Samsung SoC have the WFI
line on the cpu routed via a mux to various functions, such as IDLE or
much deeper sleep modes such as the SUSPEND state.
I've no data on whether the write is needed before each sleep or not, so
my preference would be to ensure that this mux is setup before each call,
but it might be possible to simply ensure that it is setup correctly on
init and after suspend/resume.
--
Ben
Q: What's a light-year?
A: One-third less calories than a regular year.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] ARM: SAMSUNG: Add idle support
2010-01-12 0:06 ` Ben Dooks
@ 2010-01-12 10:12 ` Russell King - ARM Linux
2010-01-12 14:18 ` Ben Dooks
0 siblings, 1 reply; 9+ messages in thread
From: Russell King - ARM Linux @ 2010-01-12 10:12 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 12, 2010 at 12:06:06AM +0000, Ben Dooks wrote:
> On Mon, Jan 11, 2010 at 10:02:54AM +0000, Russell King - ARM Linux wrote:
> > On Mon, Jan 11, 2010 at 06:44:32PM +0900, Kukjin Kim wrote:
> > > This patch adds common idle functionality for all Samsung SoC's.
> >
> > NAK. Please use the existing support already provided. There's no point
> > in double-indirection - it just makes the code harder to follow.
> >
> > Just set pm_idle to your idle function.
>
> Hmm, the default_idle() function that pm_idle is set to at init time does
> more than what arch_idle() does. We'd end up having to copy the logic in
> default_idle() into each of our idle routines. How difficult would it be
> to change the callers of pm_idle() to check whether it actually needs
> calling and to re-enable the interrupt state after calling it?
Not possible without changing all the other architectures - pm_idle() is
the standard interface for hooking into the idle code, and is used by
things like cpuidle, APM, ACPI, etc.
Please use the standard cross-architecture pm_idle() hook if you want to
dynamically override the default idle code.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] ARM: SAMSUNG: Add idle support
2010-01-12 10:12 ` Russell King - ARM Linux
@ 2010-01-12 14:18 ` Ben Dooks
2010-01-12 14:47 ` Russell King - ARM Linux
0 siblings, 1 reply; 9+ messages in thread
From: Ben Dooks @ 2010-01-12 14:18 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 12, 2010 at 10:12:26AM +0000, Russell King - ARM Linux wrote:
> On Tue, Jan 12, 2010 at 12:06:06AM +0000, Ben Dooks wrote:
> > On Mon, Jan 11, 2010 at 10:02:54AM +0000, Russell King - ARM Linux wrote:
> > > On Mon, Jan 11, 2010 at 06:44:32PM +0900, Kukjin Kim wrote:
> > > > This patch adds common idle functionality for all Samsung SoC's.
> > >
> > > NAK. Please use the existing support already provided. There's no point
> > > in double-indirection - it just makes the code harder to follow.
> > >
> > > Just set pm_idle to your idle function.
> >
> > Hmm, the default_idle() function that pm_idle is set to at init time does
> > more than what arch_idle() does. We'd end up having to copy the logic in
> > default_idle() into each of our idle routines. How difficult would it be
> > to change the callers of pm_idle() to check whether it actually needs
> > calling and to re-enable the interrupt state after calling it?
>
> Not possible without changing all the other architectures - pm_idle() is
> the standard interface for hooking into the idle code, and is used by
> things like cpuidle, APM, ACPI, etc.
>
> Please use the standard cross-architecture pm_idle() hook if you want to
> dynamically override the default idle code.
So basically i've got to copy the default_idle() and the necessary cpu code
for each of the cpus that are supported? I'd much rather just do it once
and have a pointer to the bits that change thanks.
--
Ben
Q: What's a light-year?
A: One-third less calories than a regular year.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] ARM: SAMSUNG: Add idle support
2010-01-12 14:18 ` Ben Dooks
@ 2010-01-12 14:47 ` Russell King - ARM Linux
0 siblings, 0 replies; 9+ messages in thread
From: Russell King - ARM Linux @ 2010-01-12 14:47 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 12, 2010 at 02:18:53PM +0000, Ben Dooks wrote:
> So basically i've got to copy the default_idle() and the necessary cpu code
> for each of the cpus that are supported? I'd much rather just do it once
> and have a pointer to the bits that change thanks.
No - default_idle is just:
static void default_idle(void)
{
if (!need_resched())
arch_idle();
local_irq_enable();
}
Rather than indirecting arch_idle(), hook into pm_idle instead, and have:
if (!need_resched()) {
your_idle_code_which_may_be_a_call_to_cpu_idle();
}
local_irq_enable();
in each of your idle methods instead, completely ignoring arch_idle().
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] ARM: SAMSUNG: Add idle support
2010-01-11 9:44 [PATCH 3/3] ARM: SAMSUNG: Add idle support Kukjin Kim
2010-01-11 10:02 ` Russell King - ARM Linux
@ 2010-01-11 10:35 ` Mark Brown
2010-01-13 0:12 ` Ben Dooks
2 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2010-01-11 10:35 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jan 11, 2010 at 06:44:32PM +0900, Kukjin Kim wrote:
> diff --git a/arch/arm/plat-samsung/include/mach/system.h b/arch/arm/plat-samsung/include/mach/system.h
> new file mode 100644
> index 0000000..5377792
> --- /dev/null
> +++ b/arch/arm/plat-samsung/include/mach/system.h
> @@ -0,0 +1,36 @@
...
> +void (*s3c_idle_fn)(void);
This should include idle.h to ensure consistency and the entire file
looks like it ought to be a .c file rather than a .h since it includes
function and variable definitions.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] ARM: SAMSUNG: Add idle support
2010-01-11 9:44 [PATCH 3/3] ARM: SAMSUNG: Add idle support Kukjin Kim
2010-01-11 10:02 ` Russell King - ARM Linux
2010-01-11 10:35 ` Mark Brown
@ 2010-01-13 0:12 ` Ben Dooks
2010-01-13 23:51 ` Kukjin Kim
2 siblings, 1 reply; 9+ messages in thread
From: Ben Dooks @ 2010-01-13 0:12 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jan 11, 2010 at 06:44:32PM +0900, Kukjin Kim wrote:
> This patch adds common idle functionality for all Samsung SoC's.
Kukjin, for expediency please do as Russell suggested and hook the pm_idle
call for the s5p6440 case.
> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> ---
> arch/arm/plat-samsung/include/mach/idle.h | 18 +++++++++++++
> arch/arm/plat-samsung/include/mach/system.h | 36 +++++++++++++++++++++++++++
> 2 files changed, 54 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/plat-samsung/include/mach/idle.h
> create mode 100644 arch/arm/plat-samsung/include/mach/system.h
>
> diff --git a/arch/arm/plat-samsung/include/mach/idle.h b/arch/arm/plat-samsung/include/mach/idle.h
> new file mode 100644
> index 0000000..f3b5034
> --- /dev/null
> +++ b/arch/arm/plat-samsung/include/mach/idle.h
> @@ -0,0 +1,18 @@
> +/* linux/arch/arm/plat-samsung/include/mach/idle.h
> + *
> + * Copyright (c) 2009 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com/
> + *
> + * Idle support
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> +*/
> +
> +#ifndef __ASM_PLAT_IDLE_H
> +#define __ASM_PLAT_IDLE_H __FILE__
> +
> +extern void (*s3c_idle_fn)(void);
> +
> +#endif /* __ASM_PLAT_IDLE_H */
> diff --git a/arch/arm/plat-samsung/include/mach/system.h b/arch/arm/plat-samsung/include/mach/system.h
> new file mode 100644
> index 0000000..5377792
> --- /dev/null
> +++ b/arch/arm/plat-samsung/include/mach/system.h
> @@ -0,0 +1,36 @@
> +/* linux/arch/arm/plat-samsung/include/mach/idle.h
> + *
> + * Copyright (c) 2009 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com/
> + *
> + * Idle support
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> +*/
> +
> +#ifndef __ASM_PLAT_SYSTEM_H
> +#define __ASM_PLAT_SYSTEM_H __FILE__
> +
> +void (*s3c_idle_fn)(void);
> +
> +static void s3c_default_idle(void)
> +{
> + /* nothing here yet */
> +}
> +
> +static void arch_idle(void)
> +{
> + if (s3c_idle_fn != NULL)
> + (s3c_idle_fn)();
> + else
> + s3c_default_idle();
> +}
> +
> +static void arch_reset(char mode, const char *cmd)
> +{
> + /* nothing here yet */
> +}
> +
> +#endif /* __ASM_PLAT_SYSTEM_H */
> --
> 1.6.2.5
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
--
Ben
Q: What's a light-year?
A: One-third less calories than a regular year.
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 3/3] ARM: SAMSUNG: Add idle support
2010-01-13 0:12 ` Ben Dooks
@ 2010-01-13 23:51 ` Kukjin Kim
0 siblings, 0 replies; 9+ messages in thread
From: Kukjin Kim @ 2010-01-13 23:51 UTC (permalink / raw)
To: linux-arm-kernel
Ben Dooks wrote:
> On Mon, Jan 11, 2010 at 06:44:32PM +0900, Kukjin Kim wrote:
> > This patch adds common idle functionality for all Samsung SoC's.
>
> Kukjin, for expediency please do as Russell suggested and hook the pm_idle
> call for the s5p6440 case.
Ok, I will do it.
Thanks.
> > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> > ---
> > arch/arm/plat-samsung/include/mach/idle.h | 18 +++++++++++++
> > arch/arm/plat-samsung/include/mach/system.h | 36
> +++++++++++++++++++++++++++
> > 2 files changed, 54 insertions(+), 0 deletions(-)
> > create mode 100644 arch/arm/plat-samsung/include/mach/idle.h
> > create mode 100644 arch/arm/plat-samsung/include/mach/system.h
> >
> > diff --git a/arch/arm/plat-samsung/include/mach/idle.h b/arch/arm/plat-
> samsung/include/mach/idle.h
> > new file mode 100644
> > index 0000000..f3b5034
> > --- /dev/null
> > +++ b/arch/arm/plat-samsung/include/mach/idle.h
> > @@ -0,0 +1,18 @@
> > +/* linux/arch/arm/plat-samsung/include/mach/idle.h
> > + *
> > + * Copyright (c) 2009 Samsung Electronics Co., Ltd.
> > + * http://www.samsung.com/
> > + *
> > + * Idle support
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > +*/
> > +
> > +#ifndef __ASM_PLAT_IDLE_H
> > +#define __ASM_PLAT_IDLE_H __FILE__
> > +
> > +extern void (*s3c_idle_fn)(void);
> > +
> > +#endif /* __ASM_PLAT_IDLE_H */
> > diff --git a/arch/arm/plat-samsung/include/mach/system.h b/arch/arm/plat-
> samsung/include/mach/system.h
> > new file mode 100644
> > index 0000000..5377792
> > --- /dev/null
> > +++ b/arch/arm/plat-samsung/include/mach/system.h
> > @@ -0,0 +1,36 @@
> > +/* linux/arch/arm/plat-samsung/include/mach/idle.h
> > + *
> > + * Copyright (c) 2009 Samsung Electronics Co., Ltd.
> > + * http://www.samsung.com/
> > + *
> > + * Idle support
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > +*/
> > +
> > +#ifndef __ASM_PLAT_SYSTEM_H
> > +#define __ASM_PLAT_SYSTEM_H __FILE__
> > +
> > +void (*s3c_idle_fn)(void);
> > +
> > +static void s3c_default_idle(void)
> > +{
> > + /* nothing here yet */
> > +}
> > +
> > +static void arch_idle(void)
> > +{
> > + if (s3c_idle_fn != NULL)
> > + (s3c_idle_fn)();
> > + else
> > + s3c_default_idle();
> > +}
> > +
> > +static void arch_reset(char mode, const char *cmd)
> > +{
> > + /* nothing here yet */
> > +}
> > +
> > +#endif /* __ASM_PLAT_SYSTEM_H */
> > --
> > 1.6.2.5
> >
> >
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
System LSI Division, SAMSUNG ELECTRONICS CO., LTD.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-01-13 23:51 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-11 9:44 [PATCH 3/3] ARM: SAMSUNG: Add idle support Kukjin Kim
2010-01-11 10:02 ` Russell King - ARM Linux
2010-01-12 0:06 ` Ben Dooks
2010-01-12 10:12 ` Russell King - ARM Linux
2010-01-12 14:18 ` Ben Dooks
2010-01-12 14:47 ` Russell King - ARM Linux
2010-01-11 10:35 ` Mark Brown
2010-01-13 0:12 ` Ben Dooks
2010-01-13 23:51 ` Kukjin Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).