RE: [PATCH 08/10] ARM: OMAP5/DRA7: PM: cpuidle MPU CSWR support
From: Daniel Lezcano [daniel.lezc...@linaro.org]
Sent: Wednesday, September 17, 2014 8:22 PM
To: Shilimkar, Santosh; Menon, Nishanth; Tony Lindgren; Kristo, Tero; Paul
Walmsley
Cc: Kevin Hilman; linux-arm-ker...@lists.infradead.org;
linux-o...@vger.kernel.org; linux-kernel@vger.kernel.org; J, KEERTHY; Benoît
Cousson
Subject: Re: [PATCH 08/10] ARM: OMAP5/DRA7: PM: cpuidle MPU CSWR support
On 09/17/2014 04:20 PM, Shilimkar, Santosh wrote:
> Sorry for the format. Emailing from webmail.
>
[ ... ]
>> + cx->mpu_state_vote++;
>> + if (cx->mpu_state_vote == num_online_cpus()) {
>> + pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state);
>> + omap_set_pwrdm_state(mpu_pd, cx->mpu_state);
>> + }
>> + raw_spin_unlock_irqrestore(_lock, flag);
>> +
>> + omap4_enter_lowpower(dev->cpu, cx->cpu_state);
>> +
>> + raw_spin_lock_irqsave(_lock, flag);
>> + if (cx->mpu_state_vote == num_online_cpus())
>> + omap_set_pwrdm_state(mpu_pd, PWRDM_POWER_ON);
>> + cx->mpu_state_vote--;
>> + raw_spin_unlock_irqrestore(_lock, flag);
>
> I am not sure that will work. What happens if a cpu exits idle and then
> re-enter idle immediately ?
>
> [Santosh] It works and that case is already taken care. CPU exist the idle
> and then votes
> out for cluster state and if it reenters with the right targeted state, the
> cluster state would
> be picked.
It isn't possible to have one cpu disabling the coherency, while the
other one is looking for a lock ? Or eg. cpu0 is on WFI then cpu1 is the
last entering idle. While cpu1 is entering 'lowpower', cpu0 exits the
wfi check the state vote and set the power domain on. In the meantime
cpu1 disables the coherency and cpu0 decrease the vote and release the
lock. Could be possible there is a very small racy window here ?
[Santosh] The coherency isn't disable by CPU. Thats actually taken care by
hardware. CPU takes it own power domain down and takes itself out of
coherency. The Coherency is always ON as long as there is a CPU ON
and SMP bit on that CPU is enabled.
The scenario, you mentioned can never happen on this hardware thanks
to the inbuilt smart hardware.
If you have more questions, lets discuss. I am around here at connect. ;-)
Regards,
Santosh--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 08/10] ARM: OMAP5/DRA7: PM: cpuidle MPU CSWR support
Sorry for the format. Emailing from webmail.
From: Daniel Lezcano [daniel.lezc...@linaro.org]
Sent: Wednesday, September 17, 2014 2:49 PM
To: Menon, Nishanth; Shilimkar, Santosh; Tony Lindgren; Kristo, Tero; Paul
Walmsley
Cc: Kevin Hilman; linux-arm-ker...@lists.infradead.org;
linux-o...@vger.kernel.org; linux-kernel@vger.kernel.org; J, KEERTHY; Benoît
Cousson
Subject: Re: [PATCH 08/10] ARM: OMAP5/DRA7: PM: cpuidle MPU CSWR support
On 08/22/2014 07:02 AM, Nishanth Menon wrote:
> From: Santosh Shilimkar
>
> Add OMAP5/DRA74/72 CPUIDLE support.
>
> This patch adds MPUSS low power states in cpuidle.
>
> C1 - CPU0 WFI + CPU1 WFI + MPU ON
> C2 - CPU0 RET + CPU1 RET + MPU CSWR
>
> Tested on DRA74/72-EVM for C1 and C2 states.
>
> NOTE: DRA7 does not do voltage scaling as part of retention transition
> and has Mercury which speeds up transition paths - Latency numbers are
> based on measurements done by toggling GPIOs.
>
> Signed-off-by: Santosh Shilimkar
> [ j-keer...@ti.com rework on 3.14]
> Signed-off-by: Keerthy
> [n...@ti.com: updates based on profiling, OMAP5 squashed]
> Signed-off-by: Nishanth Menon
> ---
> arch/arm/mach-omap2/cpuidle44xx.c | 82
> -
> arch/arm/mach-omap2/pm44xx.c |2 +-
> 2 files changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
> b/arch/arm/mach-omap2/cpuidle44xx.c
> index 2498ab0..8ad4f44 100644
> --- a/arch/arm/mach-omap2/cpuidle44xx.c
> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
> @@ -22,6 +22,7 @@
> #include "common.h"
> #include "pm.h"
> #include "prm.h"
> +#include "soc.h"
> #include "clockdomain.h"
>
> #define MAX_CPUS2
> @@ -31,6 +32,7 @@ struct idle_statedata {
> u32 cpu_state;
> u32 mpu_logic_state;
> u32 mpu_state;
> + u32 mpu_state_vote;
> };
>
> static struct idle_statedata omap4_idle_data[] = {
> @@ -51,12 +53,26 @@ static struct idle_statedata omap4_idle_data[] = {
> },
> };
>
> +static struct idle_statedata dra7_idle_data[] = {
> + {
> + .cpu_state = PWRDM_POWER_ON,
> + .mpu_state = PWRDM_POWER_ON,
> + .mpu_logic_state = PWRDM_POWER_ON,
> + },
> + {
> + .cpu_state = PWRDM_POWER_RET,
> + .mpu_state = PWRDM_POWER_RET,
> + .mpu_logic_state = PWRDM_POWER_RET,
> + },
> +};
> +
> static struct powerdomain *mpu_pd, *cpu_pd[MAX_CPUS];
> static struct clockdomain *cpu_clkdm[MAX_CPUS];
>
> static atomic_t abort_barrier;
> static bool cpu_done[MAX_CPUS];
> static struct idle_statedata *state_ptr = _idle_data[0];
> +static DEFINE_RAW_SPINLOCK(mpu_lock);
>
> /* Private functions */
>
> @@ -78,6 +94,32 @@ static int omap_enter_idle_simple(struct cpuidle_device
> *dev,
> return index;
> }
>
> +static int omap_enter_idle_smp(struct cpuidle_device *dev,
> +struct cpuidle_driver *drv,
> +int index)
> +{
> + struct idle_statedata *cx = state_ptr + index;
> + unsigned long flag;
> +
> + raw_spin_lock_irqsave(_lock, flag);
Why do you need this spin_lock_irqsave ? Aren't the local irqs already
disabled ?
[Santosh] Actually at one point of time before the idle consolidation the local
irq disable was inside the idle drivers. Now with that moved to core layer,
I think plain spin_lock/unlock() should work.
> + cx->mpu_state_vote++;
> + if (cx->mpu_state_vote == num_online_cpus()) {
> + pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state);
> + omap_set_pwrdm_state(mpu_pd, cx->mpu_state);
> + }
> + raw_spin_unlock_irqrestore(_lock, flag);
> +
> + omap4_enter_lowpower(dev->cpu, cx->cpu_state);
> +
> + raw_spin_lock_irqsave(_lock, flag);
> + if (cx->mpu_state_vote == num_online_cpus())
> + omap_set_pwrdm_state(mpu_pd, PWRDM_POWER_ON);
> + cx->mpu_state_vote--;
> + raw_spin_unlock_irqrestore(_lock, flag);
I am not sure that will work. What happens if a cpu exits idle and then
re-enter idle immediately ?
[Santosh] It works and that case is already taken care. CPU exist the idle and
then votes
out for cluster state and if it reenters with the right targeted state, the
cluster state would
be picked.
Could you try a long run of this little program:
https://git.linaro.org/power/pm-qa.git/blob/HEAD:/cpuidle/cpuidle_killer.c
[Santosh] I am sure there will not be any issue with the long run test case
here.
Lets see if Nishant sees anything otherwise
Regards,
Santosh--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 08/10] ARM: OMAP5/DRA7: PM: cpuidle MPU CSWR support
Sorry for the format. Emailing from webmail.
From: Daniel Lezcano [daniel.lezc...@linaro.org]
Sent: Wednesday, September 17, 2014 2:49 PM
To: Menon, Nishanth; Shilimkar, Santosh; Tony Lindgren; Kristo, Tero; Paul
Walmsley
Cc: Kevin Hilman; linux-arm-ker...@lists.infradead.org;
linux-o...@vger.kernel.org; linux-kernel@vger.kernel.org; J, KEERTHY; Benoît
Cousson
Subject: Re: [PATCH 08/10] ARM: OMAP5/DRA7: PM: cpuidle MPU CSWR support
On 08/22/2014 07:02 AM, Nishanth Menon wrote:
From: Santosh Shilimkar santosh.shilim...@ti.com
Add OMAP5/DRA74/72 CPUIDLE support.
This patch adds MPUSS low power states in cpuidle.
C1 - CPU0 WFI + CPU1 WFI + MPU ON
C2 - CPU0 RET + CPU1 RET + MPU CSWR
Tested on DRA74/72-EVM for C1 and C2 states.
NOTE: DRA7 does not do voltage scaling as part of retention transition
and has Mercury which speeds up transition paths - Latency numbers are
based on measurements done by toggling GPIOs.
Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
[ j-keer...@ti.com rework on 3.14]
Signed-off-by: Keerthy j-keer...@ti.com
[n...@ti.com: updates based on profiling, OMAP5 squashed]
Signed-off-by: Nishanth Menon n...@ti.com
---
arch/arm/mach-omap2/cpuidle44xx.c | 82
-
arch/arm/mach-omap2/pm44xx.c |2 +-
2 files changed, 82 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
b/arch/arm/mach-omap2/cpuidle44xx.c
index 2498ab0..8ad4f44 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -22,6 +22,7 @@
#include common.h
#include pm.h
#include prm.h
+#include soc.h
#include clockdomain.h
#define MAX_CPUS2
@@ -31,6 +32,7 @@ struct idle_statedata {
u32 cpu_state;
u32 mpu_logic_state;
u32 mpu_state;
+ u32 mpu_state_vote;
};
static struct idle_statedata omap4_idle_data[] = {
@@ -51,12 +53,26 @@ static struct idle_statedata omap4_idle_data[] = {
},
};
+static struct idle_statedata dra7_idle_data[] = {
+ {
+ .cpu_state = PWRDM_POWER_ON,
+ .mpu_state = PWRDM_POWER_ON,
+ .mpu_logic_state = PWRDM_POWER_ON,
+ },
+ {
+ .cpu_state = PWRDM_POWER_RET,
+ .mpu_state = PWRDM_POWER_RET,
+ .mpu_logic_state = PWRDM_POWER_RET,
+ },
+};
+
static struct powerdomain *mpu_pd, *cpu_pd[MAX_CPUS];
static struct clockdomain *cpu_clkdm[MAX_CPUS];
static atomic_t abort_barrier;
static bool cpu_done[MAX_CPUS];
static struct idle_statedata *state_ptr = omap4_idle_data[0];
+static DEFINE_RAW_SPINLOCK(mpu_lock);
/* Private functions */
@@ -78,6 +94,32 @@ static int omap_enter_idle_simple(struct cpuidle_device
*dev,
return index;
}
+static int omap_enter_idle_smp(struct cpuidle_device *dev,
+struct cpuidle_driver *drv,
+int index)
+{
+ struct idle_statedata *cx = state_ptr + index;
+ unsigned long flag;
+
+ raw_spin_lock_irqsave(mpu_lock, flag);
Why do you need this spin_lock_irqsave ? Aren't the local irqs already
disabled ?
[Santosh] Actually at one point of time before the idle consolidation the local
irq disable was inside the idle drivers. Now with that moved to core layer,
I think plain spin_lock/unlock() should work.
+ cx-mpu_state_vote++;
+ if (cx-mpu_state_vote == num_online_cpus()) {
+ pwrdm_set_logic_retst(mpu_pd, cx-mpu_logic_state);
+ omap_set_pwrdm_state(mpu_pd, cx-mpu_state);
+ }
+ raw_spin_unlock_irqrestore(mpu_lock, flag);
+
+ omap4_enter_lowpower(dev-cpu, cx-cpu_state);
+
+ raw_spin_lock_irqsave(mpu_lock, flag);
+ if (cx-mpu_state_vote == num_online_cpus())
+ omap_set_pwrdm_state(mpu_pd, PWRDM_POWER_ON);
+ cx-mpu_state_vote--;
+ raw_spin_unlock_irqrestore(mpu_lock, flag);
I am not sure that will work. What happens if a cpu exits idle and then
re-enter idle immediately ?
[Santosh] It works and that case is already taken care. CPU exist the idle and
then votes
out for cluster state and if it reenters with the right targeted state, the
cluster state would
be picked.
Could you try a long run of this little program:
https://git.linaro.org/power/pm-qa.git/blob/HEAD:/cpuidle/cpuidle_killer.c
[Santosh] I am sure there will not be any issue with the long run test case
here.
Lets see if Nishant sees anything otherwise
Regards,
Santosh--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 08/10] ARM: OMAP5/DRA7: PM: cpuidle MPU CSWR support
From: Daniel Lezcano [daniel.lezc...@linaro.org]
Sent: Wednesday, September 17, 2014 8:22 PM
To: Shilimkar, Santosh; Menon, Nishanth; Tony Lindgren; Kristo, Tero; Paul
Walmsley
Cc: Kevin Hilman; linux-arm-ker...@lists.infradead.org;
linux-o...@vger.kernel.org; linux-kernel@vger.kernel.org; J, KEERTHY; Benoît
Cousson
Subject: Re: [PATCH 08/10] ARM: OMAP5/DRA7: PM: cpuidle MPU CSWR support
On 09/17/2014 04:20 PM, Shilimkar, Santosh wrote:
Sorry for the format. Emailing from webmail.
[ ... ]
+ cx-mpu_state_vote++;
+ if (cx-mpu_state_vote == num_online_cpus()) {
+ pwrdm_set_logic_retst(mpu_pd, cx-mpu_logic_state);
+ omap_set_pwrdm_state(mpu_pd, cx-mpu_state);
+ }
+ raw_spin_unlock_irqrestore(mpu_lock, flag);
+
+ omap4_enter_lowpower(dev-cpu, cx-cpu_state);
+
+ raw_spin_lock_irqsave(mpu_lock, flag);
+ if (cx-mpu_state_vote == num_online_cpus())
+ omap_set_pwrdm_state(mpu_pd, PWRDM_POWER_ON);
+ cx-mpu_state_vote--;
+ raw_spin_unlock_irqrestore(mpu_lock, flag);
I am not sure that will work. What happens if a cpu exits idle and then
re-enter idle immediately ?
[Santosh] It works and that case is already taken care. CPU exist the idle
and then votes
out for cluster state and if it reenters with the right targeted state, the
cluster state would
be picked.
It isn't possible to have one cpu disabling the coherency, while the
other one is looking for a lock ? Or eg. cpu0 is on WFI then cpu1 is the
last entering idle. While cpu1 is entering 'lowpower', cpu0 exits the
wfi check the state vote and set the power domain on. In the meantime
cpu1 disables the coherency and cpu0 decrease the vote and release the
lock. Could be possible there is a very small racy window here ?
[Santosh] The coherency isn't disable by CPU. Thats actually taken care by
hardware. CPU takes it own power domain down and takes itself out of
coherency. The Coherency is always ON as long as there is a CPU ON
and SMP bit on that CPU is enabled.
The scenario, you mentioned can never happen on this hardware thanks
to the inbuilt smart hardware.
If you have more questions, lets discuss. I am around here at connect. ;-)
Regards,
Santosh--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 0/2] ARM: OMAP: dmtimers improvements.
Sorry for top posting Probably we should move the dmtimer to drivers/misc or create drivers/timer/ This has been pending for quite some time now Tony, what you say ? From: linux-arm-kernel [linux-arm-kernel-boun...@lists.infradead.org] on behalf of NeilBrown [ne...@suse.de] Sent: Thursday, October 24, 2013 2:57 AM To: Tony Lindgren; Russell King Cc: Belisko Marek; linux-o...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Dr. H. Nikolaus Schaller Subject: [PATCH 0/2] ARM: OMAP: dmtimers improvements. I would like to use omap dmtimer for a PWM driver (for the backlight on my display). But there are two difficulties. Unfortunately Jon Hunter (who has helped me with dmtimers before) seems to have disappeared. So I guess I get to do it myself :-) Following two patches make dmtimers more usable. 1/ allow the counter to be set even when the timer isn't counting. 2/ move the include file to a more useful location Any help you can provide in getting these fixes upstream would be appreciated. Thanks, NeilBrown --- NeilBrown (2): ARM: OMAP2+: dmtimer: allow counter to be set at any time. ARM: OMAP: move dmtimer.h from plat-omap/include/plat to include/linux arch/arm/mach-omap1/pm.c |2 arch/arm/mach-omap1/timer.c|2 arch/arm/mach-omap2/omap_hwmod_2420_data.c |2 arch/arm/mach-omap2/omap_hwmod_2430_data.c |2 arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c |2 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |2 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |2 arch/arm/mach-omap2/omap_hwmod_54xx_data.c |2 arch/arm/mach-omap2/omap_hwmod_7xx_data.c |2 arch/arm/mach-omap2/timer.c|2 arch/arm/plat-omap/dmtimer.c | 11 - arch/arm/plat-omap/include/plat/dmtimer.h | 415 drivers/media/rc/ir-rx51.c |2 drivers/pwm/pwm-omap.c |2 drivers/staging/tidspbridge/core/dsp-clock.c |2 include/linux/omap-dmtimer.h | 415 16 files changed, 434 insertions(+), 433 deletions(-) delete mode 100644 arch/arm/plat-omap/include/plat/dmtimer.h create mode 100644 include/linux/omap-dmtimer.h -- Signature ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 0/2] ARM: OMAP: dmtimers improvements.
Sorry for top posting Probably we should move the dmtimer to drivers/misc or create drivers/timer/ This has been pending for quite some time now Tony, what you say ? From: linux-arm-kernel [linux-arm-kernel-boun...@lists.infradead.org] on behalf of NeilBrown [ne...@suse.de] Sent: Thursday, October 24, 2013 2:57 AM To: Tony Lindgren; Russell King Cc: Belisko Marek; linux-o...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Dr. H. Nikolaus Schaller Subject: [PATCH 0/2] ARM: OMAP: dmtimers improvements. I would like to use omap dmtimer for a PWM driver (for the backlight on my display). But there are two difficulties. Unfortunately Jon Hunter (who has helped me with dmtimers before) seems to have disappeared. So I guess I get to do it myself :-) Following two patches make dmtimers more usable. 1/ allow the counter to be set even when the timer isn't counting. 2/ move the include file to a more useful location Any help you can provide in getting these fixes upstream would be appreciated. Thanks, NeilBrown --- NeilBrown (2): ARM: OMAP2+: dmtimer: allow counter to be set at any time. ARM: OMAP: move dmtimer.h from plat-omap/include/plat to include/linux arch/arm/mach-omap1/pm.c |2 arch/arm/mach-omap1/timer.c|2 arch/arm/mach-omap2/omap_hwmod_2420_data.c |2 arch/arm/mach-omap2/omap_hwmod_2430_data.c |2 arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c |2 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |2 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |2 arch/arm/mach-omap2/omap_hwmod_54xx_data.c |2 arch/arm/mach-omap2/omap_hwmod_7xx_data.c |2 arch/arm/mach-omap2/timer.c|2 arch/arm/plat-omap/dmtimer.c | 11 - arch/arm/plat-omap/include/plat/dmtimer.h | 415 drivers/media/rc/ir-rx51.c |2 drivers/pwm/pwm-omap.c |2 drivers/staging/tidspbridge/core/dsp-clock.c |2 include/linux/omap-dmtimer.h | 415 16 files changed, 434 insertions(+), 433 deletions(-) delete mode 100644 arch/arm/plat-omap/include/plat/dmtimer.h create mode 100644 include/linux/omap-dmtimer.h -- Signature ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] ARM: keystone: drop "select HAVE_SCHED_CLOCK"
Sorry for top posting. Thanks for the patch. I will pick this up.
Regards,
Santosh
From: Paul Bolle [pebo...@tiscali.nl]
Sent: Sunday, July 14, 2013 6:38 AM
To: Russell King
Cc: Olof Johansson; Shilimkar, Santosh; linux-arm-ker...@lists.infradead.org;
linux-kernel@vger.kernel.org
Subject: [PATCH] ARM: keystone: drop "select HAVE_SCHED_CLOCK"
The Kconfig symbol HAVE_SCHED_CLOCK got removed in v3.4,
with commit 6905a65879b5 ("ARM: Make the sched_clock
framework mandatory"). But a select statement for it popped up again
through commit 828989ad87af ("ARM: keystone: Add minimal TI Keystone
platform support"). Drop that statement, as it is useless.
Signed-off-by: Paul Bolle
---
Not tested.
arch/arm/mach-keystone/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/mach-keystone/Kconfig b/arch/arm/mach-keystone/Kconfig
index 51a50e9..366d1a3 100644
--- a/arch/arm/mach-keystone/Kconfig
+++ b/arch/arm/mach-keystone/Kconfig
@@ -7,7 +7,6 @@ config ARCH_KEYSTONE
select HAVE_SMP
select CLKSRC_MMIO
select GENERIC_CLOCKEVENTS
- select HAVE_SCHED_CLOCK
select ARCH_WANT_OPTIONAL_GPIOLIB
select ARM_ERRATA_798181 if SMP
help
--
1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] ARM: keystone: drop select HAVE_SCHED_CLOCK
Sorry for top posting. Thanks for the patch. I will pick this up. Regards, Santosh From: Paul Bolle [pebo...@tiscali.nl] Sent: Sunday, July 14, 2013 6:38 AM To: Russell King Cc: Olof Johansson; Shilimkar, Santosh; linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org Subject: [PATCH] ARM: keystone: drop select HAVE_SCHED_CLOCK The Kconfig symbol HAVE_SCHED_CLOCK got removed in v3.4, with commit 6905a65879b5 (ARM: Make the sched_clock framework mandatory). But a select statement for it popped up again through commit 828989ad87af (ARM: keystone: Add minimal TI Keystone platform support). Drop that statement, as it is useless. Signed-off-by: Paul Bolle pebo...@tiscali.nl --- Not tested. arch/arm/mach-keystone/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/arm/mach-keystone/Kconfig b/arch/arm/mach-keystone/Kconfig index 51a50e9..366d1a3 100644 --- a/arch/arm/mach-keystone/Kconfig +++ b/arch/arm/mach-keystone/Kconfig @@ -7,7 +7,6 @@ config ARCH_KEYSTONE select HAVE_SMP select CLKSRC_MMIO select GENERIC_CLOCKEVENTS - select HAVE_SCHED_CLOCK select ARCH_WANT_OPTIONAL_GPIOLIB select ARM_ERRATA_798181 if SMP help -- 1.8.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] ARM: OMAP5: select SCU
Sorry for top posting. Can you just add static inlines functions in header file and #ifdef it for OMAP5. Regards, Santosh From: Stehle, Vincent Sent: Thursday, May 16, 2013 7:59 PM To: Shilimkar, Santosh Cc: Vincent Stehlé; Tony Lindgren; linux-o...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] ARM: OMAP5: select SCU So the exact commit breaking OMAP5 link for me is: 883a106 ARM: default machine descriptor for multiplatform The breakage seems to be a side effect of not selecting ARCH_VEXPRESS any more, which causes HAVE_ARM_SCU to not be selected any more, too, when compiling only for OMAP5. IMHO this only reveals a deeper issue. Probably the ideal fix would be to remove all references to the scu_ functions for OMAP5, but those seem to exist in files and functions, common to OMAP4 and 5 (sleep44xx.S, omap-smp.c). How would you deal with that, please? Best regards, V. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] ARM: OMAP5: select SCU
Sorry for top posting. Can you just add static inlines functions in header file and #ifdef it for OMAP5. Regards, Santosh From: Stehle, Vincent Sent: Thursday, May 16, 2013 7:59 PM To: Shilimkar, Santosh Cc: Vincent Stehlé; Tony Lindgren; linux-o...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] ARM: OMAP5: select SCU So the exact commit breaking OMAP5 link for me is: 883a106 ARM: default machine descriptor for multiplatform The breakage seems to be a side effect of not selecting ARCH_VEXPRESS any more, which causes HAVE_ARM_SCU to not be selected any more, too, when compiling only for OMAP5. IMHO this only reveals a deeper issue. Probably the ideal fix would be to remove all references to the scu_ functions for OMAP5, but those seem to exist in files and functions, common to OMAP4 and 5 (sleep44xx.S, omap-smp.c). How would you deal with that, please? Best regards, V. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memory: emif: Add ifdef CONFIG_DEBUG_FS guard for emif_debugfs_[init|exit]
Axel, On Tue, Sep 25, 2012 at 9:24 AM, Axel Lin wrote: > > Add ifdef CONFIG_DEBUG_FS guard for emif_debugfs_[init|exit], and adds > stub > functions for the case CONFIG_DEBUG_FS is not set. > > When CONFIG_DEBUG_FS is enabled, debugfs_create_dir and > debugfs_create_file > return NULL on failure, fix it. > > Signed-off-by: Axel Lin > --- Patch looks good to me. Thanks for the fix. Acked-by : Santosh Shilimkar Greg, Will you able to pick this fix ? Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memory: emif: Add ifdef CONFIG_DEBUG_FS guard for emif_debugfs_[init|exit]
Axel, On Tue, Sep 25, 2012 at 9:24 AM, Axel Lin axel@ingics.com wrote: Add ifdef CONFIG_DEBUG_FS guard for emif_debugfs_[init|exit], and adds stub functions for the case CONFIG_DEBUG_FS is not set. When CONFIG_DEBUG_FS is enabled, debugfs_create_dir and debugfs_create_file return NULL on failure, fix it. Signed-off-by: Axel Lin axel@ingics.com --- Patch looks good to me. Thanks for the fix. Acked-by : Santosh Shilimkar santosh.shilim...@ti.com Greg, Will you able to pick this fix ? Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: rcu self-detected stall messages on OMAP3, 4 boards
On Sun, Sep 23, 2012 at 3:29 AM, Paul E. McKenney wrote: > On Sat, Sep 22, 2012 at 01:10:43PM -0700, Paul E. McKenney wrote: >> On Sat, Sep 22, 2012 at 06:42:08PM +, Paul Walmsley wrote: >> > On Fri, 21 Sep 2012, Paul E. McKenney wrote: [...] > > And here is a patch. I am still having trouble reproducing the problem, > but figured that I should avoid serializing things. > > Thanx, Paul > > > > b/kernel/rcutree.c |4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > rcu: Fix day-one dyntick-idle stall-warning bug > > Each grace period is supposed to have at least one callback waiting > for that grace period to complete. However, if CONFIG_NO_HZ=n, an > extra callback-free grace period is no big problem -- it will chew up > a tiny bit of CPU time, but it will complete normally. In contrast, > CONFIG_NO_HZ=y kernels have the potential for all the CPUs to go to > sleep indefinitely, in turn indefinitely delaying completion of the > callback-free grace period. Given that nothing is waiting on this grace > period, this is also not a problem. > > Unless RCU CPU stall warnings are also enabled, as they are in recent > kernels. In this case, if a CPU wakes up after at least one minute > of inactivity, an RCU CPU stall warning will result. The reason that > no one noticed until quite recently is that most systems have enough > OS noise that they will never remain absolutely idle for a full minute. > But there are some embedded systems with cut-down userspace configurations > that get into this mode quite easily. > > All this begs the question of exactly how a callback-free grace period > gets started in the first place. This can happen due to the fact that > CPUs do not necessarily agree on which grace period is in progress. > If a CPU still believes that the grace period that just completed is > still ongoing, it will believe that it has callbacks that need to wait > for another grace period, never mind the fact that the grace period > that they were waiting for just completed. This CPU can therefore > erroneously decide to start a new grace period. > > Once this CPU notices that the earlier grace period completed, it will > invoke its callbacks. It then won't have any callbacks left. If no > other CPU has any callbacks, we now have a callback-free grace period. > > This commit therefore makes CPUs check more carefully before starting a > new grace period. This new check relies on an array of tail pointers > into each CPU's list of callbacks. If the CPU is up to date on which > grace periods have completed, it checks to see if any callbacks follow > the RCU_DONE_TAIL segment, otherwise it checks to see if any callbacks > follow the RCU_WAIT_TAIL segment. The reason that this works is that > the RCU_WAIT_TAIL segment will be promoted to the RCU_DONE_TAIL segment > as soon as the CPU figures out that the old grace period has ended. > > This change is to cpu_needs_another_gp(), which is called in a number > of places. The only one that really matters is in rcu_start_gp(), where > the root rcu_node structure's ->lock is held, which prevents any > other CPU from starting or completing a grace period, so that the > comparison that determines whether the CPU is missing the completion > of a grace period is stable. > > Signed-off-by: Paul E. McKenney > Signed-off-by: Paul E. McKenney > As already confirmed by Paul W and others, I too no longer see the rcu dumps any more with above patch. Thanks a lot for the fix. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: rcu self-detected stall messages on OMAP3, 4 boards
On Sun, Sep 23, 2012 at 3:29 AM, Paul E. McKenney paul...@linux.vnet.ibm.com wrote: On Sat, Sep 22, 2012 at 01:10:43PM -0700, Paul E. McKenney wrote: On Sat, Sep 22, 2012 at 06:42:08PM +, Paul Walmsley wrote: On Fri, 21 Sep 2012, Paul E. McKenney wrote: [...] And here is a patch. I am still having trouble reproducing the problem, but figured that I should avoid serializing things. Thanx, Paul b/kernel/rcutree.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) rcu: Fix day-one dyntick-idle stall-warning bug Each grace period is supposed to have at least one callback waiting for that grace period to complete. However, if CONFIG_NO_HZ=n, an extra callback-free grace period is no big problem -- it will chew up a tiny bit of CPU time, but it will complete normally. In contrast, CONFIG_NO_HZ=y kernels have the potential for all the CPUs to go to sleep indefinitely, in turn indefinitely delaying completion of the callback-free grace period. Given that nothing is waiting on this grace period, this is also not a problem. Unless RCU CPU stall warnings are also enabled, as they are in recent kernels. In this case, if a CPU wakes up after at least one minute of inactivity, an RCU CPU stall warning will result. The reason that no one noticed until quite recently is that most systems have enough OS noise that they will never remain absolutely idle for a full minute. But there are some embedded systems with cut-down userspace configurations that get into this mode quite easily. All this begs the question of exactly how a callback-free grace period gets started in the first place. This can happen due to the fact that CPUs do not necessarily agree on which grace period is in progress. If a CPU still believes that the grace period that just completed is still ongoing, it will believe that it has callbacks that need to wait for another grace period, never mind the fact that the grace period that they were waiting for just completed. This CPU can therefore erroneously decide to start a new grace period. Once this CPU notices that the earlier grace period completed, it will invoke its callbacks. It then won't have any callbacks left. If no other CPU has any callbacks, we now have a callback-free grace period. This commit therefore makes CPUs check more carefully before starting a new grace period. This new check relies on an array of tail pointers into each CPU's list of callbacks. If the CPU is up to date on which grace periods have completed, it checks to see if any callbacks follow the RCU_DONE_TAIL segment, otherwise it checks to see if any callbacks follow the RCU_WAIT_TAIL segment. The reason that this works is that the RCU_WAIT_TAIL segment will be promoted to the RCU_DONE_TAIL segment as soon as the CPU figures out that the old grace period has ended. This change is to cpu_needs_another_gp(), which is called in a number of places. The only one that really matters is in rcu_start_gp(), where the root rcu_node structure's -lock is held, which prevents any other CPU from starting or completing a grace period, so that the comparison that determines whether the CPU is missing the completion of a grace period is stable. Signed-off-by: Paul E. McKenney paul.mcken...@linaro.org Signed-off-by: Paul E. McKenney paul...@linux.vnet.ibm.com As already confirmed by Paul W and others, I too no longer see the rcu dumps any more with above patch. Thanks a lot for the fix. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] drivers: bus: Move the OMAP interconnect driver to drivers/bus/
On Thu, Sep 20, 2012 at 12:27 AM, Arnd Bergmann wrote: > On Monday 17 September 2012, Tony Lindgren wrote: >> * Santosh Shilimkar [120914 02:21]: >> > OMAP interconnect drivers are used for the interconnect error handling. >> > Since they are bus driver, lets move it to newly created drivers/bus. >> > >> > Cc: Arnd Bergmann >> > Cc: Tony Lindgren >> > Tested-by: Lokesh Vutla >> > Signed-off-by: Santosh Shilimkar >> > --- >> > Patch just moves OMAP interconnect drivers as is to the newly created >> > driver/bus/* directory. Patch is generated against >> > "arm-soc/drivers/ocp2scp" >> > tree and test on all OMAP boards. >> >> Great, looks like this should not conflict with other >> omap patches queued, so Arnd should probably take this into >> the bus branch: >> >> Acked-by: Tony Lindgren > > It turns out that the patch actually did conflict and we now have a broken > omap2plus_defconfig. The patch below seems to fix it, but please verify > that this makes sense. > > Signed-off-by: Arnd Bergmann > Looks correct. Acked-by: Santosh Shilimkar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] drivers: bus: Move the OMAP interconnect driver to drivers/bus/
On Wed, Sep 19, 2012 at 8:28 PM, Arnd Bergmann wrote: > On Monday 17 September 2012, Tony Lindgren wrote: >> * Santosh Shilimkar [120914 02:21]: >> > OMAP interconnect drivers are used for the interconnect error handling. >> > Since they are bus driver, lets move it to newly created drivers/bus. >> > >> > Cc: Arnd Bergmann >> > Cc: Tony Lindgren >> > Tested-by: Lokesh Vutla >> > Signed-off-by: Santosh Shilimkar >> > --- >> > Patch just moves OMAP interconnect drivers as is to the newly created >> > driver/bus/* directory. Patch is generated against >> > "arm-soc/drivers/ocp2scp" >> > tree and test on all OMAP boards. >> >> Great, looks like this should not conflict with other >> omap patches queued, so Arnd should probably take this into >> the bus branch: >> >> Acked-by: Tony Lindgren > > Ok, added to the branch. In the future please Cc both Olof and me when > submitting > patches for inclusion. > Thanks Arnd. Sorry I missed to Cc Olof on the patch. Will take care of this in future. Regards santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] drivers: bus: Move the OMAP interconnect driver to drivers/bus/
On Wed, Sep 19, 2012 at 8:28 PM, Arnd Bergmann a...@arndb.de wrote: On Monday 17 September 2012, Tony Lindgren wrote: * Santosh Shilimkar santosh.shilim...@ti.com [120914 02:21]: OMAP interconnect drivers are used for the interconnect error handling. Since they are bus driver, lets move it to newly created drivers/bus. Cc: Arnd Bergmann a...@arndb.de Cc: Tony Lindgren t...@atomide.com Tested-by: Lokesh Vutla lokeshvu...@ti.com Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com --- Patch just moves OMAP interconnect drivers as is to the newly created driver/bus/* directory. Patch is generated against arm-soc/drivers/ocp2scp tree and test on all OMAP boards. Great, looks like this should not conflict with other omap patches queued, so Arnd should probably take this into the bus branch: Acked-by: Tony Lindgren t...@atomide.com Ok, added to the branch. In the future please Cc both Olof and me when submitting patches for inclusion. Thanks Arnd. Sorry I missed to Cc Olof on the patch. Will take care of this in future. Regards santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] drivers: bus: Move the OMAP interconnect driver to drivers/bus/
On Thu, Sep 20, 2012 at 12:27 AM, Arnd Bergmann a...@arndb.de wrote: On Monday 17 September 2012, Tony Lindgren wrote: * Santosh Shilimkar santosh.shilim...@ti.com [120914 02:21]: OMAP interconnect drivers are used for the interconnect error handling. Since they are bus driver, lets move it to newly created drivers/bus. Cc: Arnd Bergmann a...@arndb.de Cc: Tony Lindgren t...@atomide.com Tested-by: Lokesh Vutla lokeshvu...@ti.com Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com --- Patch just moves OMAP interconnect drivers as is to the newly created driver/bus/* directory. Patch is generated against arm-soc/drivers/ocp2scp tree and test on all OMAP boards. Great, looks like this should not conflict with other omap patches queued, so Arnd should probably take this into the bus branch: Acked-by: Tony Lindgren t...@atomide.com It turns out that the patch actually did conflict and we now have a broken omap2plus_defconfig. The patch below seems to fix it, but please verify that this makes sense. Signed-off-by: Arnd Bergmann a...@arndb.de Looks correct. Acked-by: Santosh Shilimkar santosh.shilim...@ti.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: suspend: use flush range instead of flush all
On Wed, Sep 12, 2012 at 3:16 PM, Russell King - ARM Linux
wrote:
> On Wed, Sep 12, 2012 at 02:40:45PM +0530, Shilimkar, Santosh wrote:
>> On Wed, Sep 12, 2012 at 2:24 PM, Russell King - ARM Linux
>> wrote:
>> > On Wed, Sep 12, 2012 at 01:13:33PM +0530, Shilimkar, Santosh wrote:
>> >> On Wed, Sep 12, 2012 at 12:48 PM, wzch wrote:
>> >> > void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32 *save_ptr)
>> >> > {
>> >> > + u32 *ptr_orig = ptr;
>> >> > *save_ptr = virt_to_phys(ptr);
>> >> >
>> >> > /* This must correspond to the LDM in cpu_resume() assembly */
>> >> > @@ -26,7 +27,8 @@ void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp,
>> >> > u32 *save_ptr)
>> >> >
>> >> > cpu_do_suspend(ptr);
>> >> >
>> >> > - flush_cache_all();
>> >> Lorenzo's patch was limiting above flush to local cache (LOUs) instead
>> >> of dropping it completely.
>> >
>> > Err, that is wrong. Normally, when CPUs go into suspend, the L1 cache is
>> > lost entirely. This is the only flush which many CPUs see of the L1
>> > cache.
>> >
>> > So removing this flush _will_ break suspend to RAM on existing CPUs.
>>
>> As mentioned, keeping that flush till inner shareability domain(L1) should be
>> enough. In fact if that part gets pushed down to the finisher() which any
>> way needs to take care of the cache maintenance, we can get rid of
>> completely.
>
> It is difficult to call the cache maintenance functions from assembly.
> Why not have the generic code do the inner shareability flush, and then
> leave the responsibility for any further cache maintenance caused by the
> actions of the finisher to the finisher to deal with - as it is now.
>
> That way we end up with more generic code, and don't go back to the
> rediculous situation where we had everyone implementing this crap in
> their own broken way time and time again in their platform code.
Fully agree with you.
Leaving the local cache flush should be better choice and that
is the additional bit Lorenzo's patch(yet to be posted) is doing on top
of the $subject patch. If the platform has special need like secure
cache maintenance, they can take care of that additionally in the
finisher.
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: suspend: use flush range instead of flush all
On Wed, Sep 12, 2012 at 2:28 PM, Lorenzo Pieralisi
wrote:
> On Wed, Sep 12, 2012 at 08:43:33AM +0100, Shilimkar, Santosh wrote:
>> + Lorenzo,
>>
>> On Wed, Sep 12, 2012 at 12:48 PM, wzch wrote:
>> > From: Wenzeng Chen
>> >
>> > In cpu suspend function __cpu_suspend_save(), we save cp15 registers,
>> > resume function, sp and suspend_pgd, then flush the data to DDR, but
>> > no need to flush all D cache, only need to flush range.
>> >
>> > Change-Id: I591a1fde929f3f987c69306b601843ed975d3e41
>> You should drop above.
>>
>> > Signed-off-by: Wenzeng Chen
>> > ---
>> Lorenzo and myself discussed about the above expensive flush and he
>> is planning to post a similar patch but with small difference.
>>
>> > arch/arm/kernel/suspend.c |4 +++-
>> > 1 files changed, 3 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/arch/arm/kernel/suspend.c b/arch/arm/kernel/suspend.c
>> > index 1794cc3..bb582d8 100644
>> > --- a/arch/arm/kernel/suspend.c
>> > +++ b/arch/arm/kernel/suspend.c
>> > @@ -17,6 +17,7 @@ extern void cpu_resume_mmu(void);
>> > */
>> > void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32 *save_ptr)
>> > {
>> > + u32 *ptr_orig = ptr;
>> > *save_ptr = virt_to_phys(ptr);
>> >
>> > /* This must correspond to the LDM in cpu_resume() assembly */
>> > @@ -26,7 +27,8 @@ void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32
>> > *save_ptr)
>> >
>> > cpu_do_suspend(ptr);
>> >
>> > - flush_cache_all();
>> Lorenzo's patch was limiting above flush to local cache (LOUs) instead
>> of dropping
>> it completely.
>
> Because if we remove it completely we have to make sure that every given
> suspend finisher calls flush_cache_all(), hence from my viewpoint this
> patch is incomplete. Either we remove it, and add it to ALL suspend
> finisher (or just make sure it is there) or we leave it but it should use
> the new LoUIS API we are going to add.
>
Yep.
>>
>> > + __cpuc_flush_dcache_area((void *)ptr_orig, ptrsz);
>> > + __cpuc_flush_dcache_area((void *)save_ptr, sizeof(*save_ptr));
>> > outer_clean_range(*save_ptr, *save_ptr + ptrsz);
>> > outer_clean_range(virt_to_phys(save_ptr),
>> > virt_to_phys(save_ptr) + sizeof(*save_ptr));
>>
>> Just thinking bit more, even in case we drop the flush_cache_all()
>> completely, it should be safe since the suspend_finisher() takes
>> care of the cache maintenance already.
>
> We already discussed this. Fine by me, but we have to make sure it is
> called on all suspend finishers in the mainline.
>
I agree. As mentioned in reply to Russell, am ok to limit this
flush to LoUIS to start with.
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: suspend: use flush range instead of flush all
On Wed, Sep 12, 2012 at 2:24 PM, Russell King - ARM Linux
wrote:
> On Wed, Sep 12, 2012 at 01:13:33PM +0530, Shilimkar, Santosh wrote:
>> On Wed, Sep 12, 2012 at 12:48 PM, wzch wrote:
>> > void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32 *save_ptr)
>> > {
>> > + u32 *ptr_orig = ptr;
>> > *save_ptr = virt_to_phys(ptr);
>> >
>> > /* This must correspond to the LDM in cpu_resume() assembly */
>> > @@ -26,7 +27,8 @@ void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32
>> > *save_ptr)
>> >
>> > cpu_do_suspend(ptr);
>> >
>> > - flush_cache_all();
>> Lorenzo's patch was limiting above flush to local cache (LOUs) instead
>> of dropping it completely.
>
> Err, that is wrong. Normally, when CPUs go into suspend, the L1 cache is
> lost entirely. This is the only flush which many CPUs see of the L1
> cache.
>
> So removing this flush _will_ break suspend to RAM on existing CPUs.
As mentioned, keeping that flush till inner shareability domain(L1) should be
enough. In fact if that part gets pushed down to the finisher() which any
way needs to take care of the cache maintenance, we can get rid of completely.
At least limiting the flush to local cache instead of all cache levels should
be fixed.
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: suspend: use flush range instead of flush all
+ Lorenzo,
On Wed, Sep 12, 2012 at 12:48 PM, wzch wrote:
> From: Wenzeng Chen
>
> In cpu suspend function __cpu_suspend_save(), we save cp15 registers,
> resume function, sp and suspend_pgd, then flush the data to DDR, but
> no need to flush all D cache, only need to flush range.
>
> Change-Id: I591a1fde929f3f987c69306b601843ed975d3e41
You should drop above.
> Signed-off-by: Wenzeng Chen
> ---
Lorenzo and myself discussed about the above expensive flush and he
is planning to post a similar patch but with small difference.
> arch/arm/kernel/suspend.c |4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/kernel/suspend.c b/arch/arm/kernel/suspend.c
> index 1794cc3..bb582d8 100644
> --- a/arch/arm/kernel/suspend.c
> +++ b/arch/arm/kernel/suspend.c
> @@ -17,6 +17,7 @@ extern void cpu_resume_mmu(void);
> */
> void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32 *save_ptr)
> {
> + u32 *ptr_orig = ptr;
> *save_ptr = virt_to_phys(ptr);
>
> /* This must correspond to the LDM in cpu_resume() assembly */
> @@ -26,7 +27,8 @@ void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32
> *save_ptr)
>
> cpu_do_suspend(ptr);
>
> - flush_cache_all();
Lorenzo's patch was limiting above flush to local cache (LOUs) instead
of dropping
it completely.
> + __cpuc_flush_dcache_area((void *)ptr_orig, ptrsz);
> + __cpuc_flush_dcache_area((void *)save_ptr, sizeof(*save_ptr));
> outer_clean_range(*save_ptr, *save_ptr + ptrsz);
> outer_clean_range(virt_to_phys(save_ptr),
> virt_to_phys(save_ptr) + sizeof(*save_ptr));
Just thinking bit more, even in case we drop the flush_cache_all()
completely, it should be safe since the suspend_finisher() takes
care of the cache maintenance already.
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: suspend: use flush range instead of flush all
+ Lorenzo,
On Wed, Sep 12, 2012 at 12:48 PM, wzch w...@marvell.com wrote:
From: Wenzeng Chen w...@marvell.com
In cpu suspend function __cpu_suspend_save(), we save cp15 registers,
resume function, sp and suspend_pgd, then flush the data to DDR, but
no need to flush all D cache, only need to flush range.
Change-Id: I591a1fde929f3f987c69306b601843ed975d3e41
You should drop above.
Signed-off-by: Wenzeng Chen w...@marvell.com
---
Lorenzo and myself discussed about the above expensive flush and he
is planning to post a similar patch but with small difference.
arch/arm/kernel/suspend.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/arch/arm/kernel/suspend.c b/arch/arm/kernel/suspend.c
index 1794cc3..bb582d8 100644
--- a/arch/arm/kernel/suspend.c
+++ b/arch/arm/kernel/suspend.c
@@ -17,6 +17,7 @@ extern void cpu_resume_mmu(void);
*/
void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32 *save_ptr)
{
+ u32 *ptr_orig = ptr;
*save_ptr = virt_to_phys(ptr);
/* This must correspond to the LDM in cpu_resume() assembly */
@@ -26,7 +27,8 @@ void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32
*save_ptr)
cpu_do_suspend(ptr);
- flush_cache_all();
Lorenzo's patch was limiting above flush to local cache (LOUs) instead
of dropping
it completely.
+ __cpuc_flush_dcache_area((void *)ptr_orig, ptrsz);
+ __cpuc_flush_dcache_area((void *)save_ptr, sizeof(*save_ptr));
outer_clean_range(*save_ptr, *save_ptr + ptrsz);
outer_clean_range(virt_to_phys(save_ptr),
virt_to_phys(save_ptr) + sizeof(*save_ptr));
Just thinking bit more, even in case we drop the flush_cache_all()
completely, it should be safe since the suspend_finisher() takes
care of the cache maintenance already.
Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: suspend: use flush range instead of flush all
On Wed, Sep 12, 2012 at 2:24 PM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
On Wed, Sep 12, 2012 at 01:13:33PM +0530, Shilimkar, Santosh wrote:
On Wed, Sep 12, 2012 at 12:48 PM, wzch w...@marvell.com wrote:
void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32 *save_ptr)
{
+ u32 *ptr_orig = ptr;
*save_ptr = virt_to_phys(ptr);
/* This must correspond to the LDM in cpu_resume() assembly */
@@ -26,7 +27,8 @@ void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32
*save_ptr)
cpu_do_suspend(ptr);
- flush_cache_all();
Lorenzo's patch was limiting above flush to local cache (LOUs) instead
of dropping it completely.
Err, that is wrong. Normally, when CPUs go into suspend, the L1 cache is
lost entirely. This is the only flush which many CPUs see of the L1
cache.
So removing this flush _will_ break suspend to RAM on existing CPUs.
As mentioned, keeping that flush till inner shareability domain(L1) should be
enough. In fact if that part gets pushed down to the finisher() which any
way needs to take care of the cache maintenance, we can get rid of completely.
At least limiting the flush to local cache instead of all cache levels should
be fixed.
Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: suspend: use flush range instead of flush all
On Wed, Sep 12, 2012 at 2:28 PM, Lorenzo Pieralisi
lorenzo.pieral...@arm.com wrote:
On Wed, Sep 12, 2012 at 08:43:33AM +0100, Shilimkar, Santosh wrote:
+ Lorenzo,
On Wed, Sep 12, 2012 at 12:48 PM, wzch w...@marvell.com wrote:
From: Wenzeng Chen w...@marvell.com
In cpu suspend function __cpu_suspend_save(), we save cp15 registers,
resume function, sp and suspend_pgd, then flush the data to DDR, but
no need to flush all D cache, only need to flush range.
Change-Id: I591a1fde929f3f987c69306b601843ed975d3e41
You should drop above.
Signed-off-by: Wenzeng Chen w...@marvell.com
---
Lorenzo and myself discussed about the above expensive flush and he
is planning to post a similar patch but with small difference.
arch/arm/kernel/suspend.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/arch/arm/kernel/suspend.c b/arch/arm/kernel/suspend.c
index 1794cc3..bb582d8 100644
--- a/arch/arm/kernel/suspend.c
+++ b/arch/arm/kernel/suspend.c
@@ -17,6 +17,7 @@ extern void cpu_resume_mmu(void);
*/
void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32 *save_ptr)
{
+ u32 *ptr_orig = ptr;
*save_ptr = virt_to_phys(ptr);
/* This must correspond to the LDM in cpu_resume() assembly */
@@ -26,7 +27,8 @@ void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32
*save_ptr)
cpu_do_suspend(ptr);
- flush_cache_all();
Lorenzo's patch was limiting above flush to local cache (LOUs) instead
of dropping
it completely.
Because if we remove it completely we have to make sure that every given
suspend finisher calls flush_cache_all(), hence from my viewpoint this
patch is incomplete. Either we remove it, and add it to ALL suspend
finisher (or just make sure it is there) or we leave it but it should use
the new LoUIS API we are going to add.
Yep.
+ __cpuc_flush_dcache_area((void *)ptr_orig, ptrsz);
+ __cpuc_flush_dcache_area((void *)save_ptr, sizeof(*save_ptr));
outer_clean_range(*save_ptr, *save_ptr + ptrsz);
outer_clean_range(virt_to_phys(save_ptr),
virt_to_phys(save_ptr) + sizeof(*save_ptr));
Just thinking bit more, even in case we drop the flush_cache_all()
completely, it should be safe since the suspend_finisher() takes
care of the cache maintenance already.
We already discussed this. Fine by me, but we have to make sure it is
called on all suspend finishers in the mainline.
I agree. As mentioned in reply to Russell, am ok to limit this
flush to LoUIS to start with.
Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: suspend: use flush range instead of flush all
On Wed, Sep 12, 2012 at 3:16 PM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
On Wed, Sep 12, 2012 at 02:40:45PM +0530, Shilimkar, Santosh wrote:
On Wed, Sep 12, 2012 at 2:24 PM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
On Wed, Sep 12, 2012 at 01:13:33PM +0530, Shilimkar, Santosh wrote:
On Wed, Sep 12, 2012 at 12:48 PM, wzch w...@marvell.com wrote:
void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32 *save_ptr)
{
+ u32 *ptr_orig = ptr;
*save_ptr = virt_to_phys(ptr);
/* This must correspond to the LDM in cpu_resume() assembly */
@@ -26,7 +27,8 @@ void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp,
u32 *save_ptr)
cpu_do_suspend(ptr);
- flush_cache_all();
Lorenzo's patch was limiting above flush to local cache (LOUs) instead
of dropping it completely.
Err, that is wrong. Normally, when CPUs go into suspend, the L1 cache is
lost entirely. This is the only flush which many CPUs see of the L1
cache.
So removing this flush _will_ break suspend to RAM on existing CPUs.
As mentioned, keeping that flush till inner shareability domain(L1) should be
enough. In fact if that part gets pushed down to the finisher() which any
way needs to take care of the cache maintenance, we can get rid of
completely.
It is difficult to call the cache maintenance functions from assembly.
Why not have the generic code do the inner shareability flush, and then
leave the responsibility for any further cache maintenance caused by the
actions of the finisher to the finisher to deal with - as it is now.
That way we end up with more generic code, and don't go back to the
rediculous situation where we had everyone implementing this crap in
their own broken way time and time again in their platform code.
Fully agree with you.
Leaving the local cache flush should be better choice and that
is the additional bit Lorenzo's patch(yet to be posted) is doing on top
of the $subject patch. If the platform has special need like secure
cache maintenance, they can take care of that additionally in the
finisher.
Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/16] ARM: omap: move platform_data definitions
On Tue, Sep 11, 2012 at 7:47 PM, Arnd Bergmann wrote: > On Tuesday 11 September 2012, Shilimkar, Santosh wrote: >> Just curious to know how you came with some of the above header names ? >> >> plat/mcbsp ---> dsp-mcbsp.h >> There is no connection of DSP with McBSP. "omap-mcbsp.h" would been a >> better name. > > This one was a mistake on my side, as Tony and Peter already pointed out. > How about asoc-mcbsp.h or asoc-omap-mcbsp.h? > After re-reading the cover-letter and your below response, 'asoc-omap-mcbsp.h' seems to be fine. >> plat/nand.h --> /mtd-nand-omap2.h >> plat/onenand.h --> /mtd-onenand-omap2.h >> May be "omap-nand.h" and "omap-onenand.h" >> >> plat/mcspi.h-->spi-omap2-mcspi.h >> May be "omap-spi.h" >> > > As I wrote in the introductory mail, I tried to always prefix the file > names with the subsystem, followed by the name of the driver, in order > to standardize on just one set of rules. > Just read that now. > The drivers implementing the three headers above are: > > drivers/mtd/nand/omap2.c > drivers/mtd/onenand/omap2.c > drivers/spi/spi-omap2-mcspi.c > > so these all seem appropriate. > Sorry I missed the subsystem prefix point. These names seems to be fine then. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/16] ARM: omap: move platform_data definitions
Arnd, On Tue, Sep 11, 2012 at 6:32 PM, Arnd Bergmann wrote: > Platform data for device drivers should be defined in > include/linux/platform_data/*.h, not in the architecture > and platform specific directories. > > This moves such data out of the omap include directories > > Signed-off-by: Arnd Bergmann [...] > 81 files changed, 87 insertions(+), 87 deletions(-) > rename arch/arm/plat-omap/include/plat/mcbsp.h => > include/linux/platform_data/dsp-mcbsp.h (100%) > rename arch/arm/plat-omap/include/plat/dsp.h => > include/linux/platform_data/dsp-omap.h (100%) > rename arch/arm/plat-omap/include/plat/keypad.h => > include/linux/platform_data/keypad-omap.h (100%) > rename arch/arm/plat-omap/include/plat/lcd_mipid.h => > include/linux/platform_data/lcd-mipid.h (100%) > rename arch/arm/plat-omap/include/plat/nand.h => > include/linux/platform_data/mtd-nand-omap2.h (100%) > rename arch/arm/plat-omap/include/plat/onenand.h => > include/linux/platform_data/mtd-onenand-omap2.h (100%) > rename arch/arm/plat-omap/include/plat/remoteproc.h => > include/linux/platform_data/remoteproc-omap.h (100%) > rename arch/arm/plat-omap/include/plat/voltage.h => > include/linux/platform_data/smartreflex-omap.h (100%) > rename arch/arm/plat-omap/include/plat/mcspi.h => > include/linux/platform_data/spi-omap2-mcspi.h (100%) > Just curious to know how you came with some of the above header names ? plat/mcbsp ---> dsp-mcbsp.h There is no connection of DSP with McBSP. "omap-mcbsp.h" would been a better name. plat/nand.h --> /mtd-nand-omap2.h plat/onenand.h --> /mtd-onenand-omap2.h May be "omap-nand.h" and "omap-onenand.h" plat/mcspi.h-->spi-omap2-mcspi.h May be "omap-spi.h" Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/16] ARM: omap: move platform_data definitions
Arnd, On Tue, Sep 11, 2012 at 6:32 PM, Arnd Bergmann a...@arndb.de wrote: Platform data for device drivers should be defined in include/linux/platform_data/*.h, not in the architecture and platform specific directories. This moves such data out of the omap include directories Signed-off-by: Arnd Bergmann a...@arndb.de [...] 81 files changed, 87 insertions(+), 87 deletions(-) rename arch/arm/plat-omap/include/plat/mcbsp.h = include/linux/platform_data/dsp-mcbsp.h (100%) rename arch/arm/plat-omap/include/plat/dsp.h = include/linux/platform_data/dsp-omap.h (100%) rename arch/arm/plat-omap/include/plat/keypad.h = include/linux/platform_data/keypad-omap.h (100%) rename arch/arm/plat-omap/include/plat/lcd_mipid.h = include/linux/platform_data/lcd-mipid.h (100%) rename arch/arm/plat-omap/include/plat/nand.h = include/linux/platform_data/mtd-nand-omap2.h (100%) rename arch/arm/plat-omap/include/plat/onenand.h = include/linux/platform_data/mtd-onenand-omap2.h (100%) rename arch/arm/plat-omap/include/plat/remoteproc.h = include/linux/platform_data/remoteproc-omap.h (100%) rename arch/arm/plat-omap/include/plat/voltage.h = include/linux/platform_data/smartreflex-omap.h (100%) rename arch/arm/plat-omap/include/plat/mcspi.h = include/linux/platform_data/spi-omap2-mcspi.h (100%) Just curious to know how you came with some of the above header names ? plat/mcbsp --- dsp-mcbsp.h There is no connection of DSP with McBSP. omap-mcbsp.h would been a better name. plat/nand.h -- /mtd-nand-omap2.h plat/onenand.h -- /mtd-onenand-omap2.h May be omap-nand.h and omap-onenand.h plat/mcspi.h--spi-omap2-mcspi.h May be omap-spi.h Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/16] ARM: omap: move platform_data definitions
On Tue, Sep 11, 2012 at 7:47 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 11 September 2012, Shilimkar, Santosh wrote: Just curious to know how you came with some of the above header names ? plat/mcbsp --- dsp-mcbsp.h There is no connection of DSP with McBSP. omap-mcbsp.h would been a better name. This one was a mistake on my side, as Tony and Peter already pointed out. How about asoc-mcbsp.h or asoc-omap-mcbsp.h? After re-reading the cover-letter and your below response, 'asoc-omap-mcbsp.h' seems to be fine. plat/nand.h -- /mtd-nand-omap2.h plat/onenand.h -- /mtd-onenand-omap2.h May be omap-nand.h and omap-onenand.h plat/mcspi.h--spi-omap2-mcspi.h May be omap-spi.h As I wrote in the introductory mail, I tried to always prefix the file names with the subsystem, followed by the name of the driver, in order to standardize on just one set of rules. Just read that now. The drivers implementing the three headers above are: drivers/mtd/nand/omap2.c drivers/mtd/onenand/omap2.c drivers/spi/spi-omap2-mcspi.c so these all seem appropriate. Sorry I missed the subsystem prefix point. These names seems to be fine then. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Seeking clarity on IRQCHIP_MASK_ON_SUSPEND
Thomas, On Mon, Sep 10, 2012 at 3:58 PM, Thomas Gleixner wrote: > On Mon, 10 Sep 2012, NeilBrown wrote: >> >> The IRQCHIP_MASK_ON_SUSPEND flag seems to be hard to use correctly, so either >> I'm understanding it wrongly, or it could be made easier to use. >> If the first case, I'm hoping that some improvement to documentation might >> result. If the second, then maybe we can fix the code. > ... >> Is anyone able to give a definitive answer on this? Should >> IRQCHIP_MASK_ON_SUSPEND be removed? > > The whole point of IRQCHIP_MASK_ON_SUSPEND is to deal with hardware > designed by geniuses. > > Most SoCs have a way to mark the interrupts which serve as a wake up > source as such. All other interrupts are magically "masked" on entry > to suspend. > Just to support this, IRQCHIP_MASK_ON_SUSPEND does work with quite a few ARM platforms too. OMAP already uses it in mainline and I have seen patches for U500 and Tegra SOCs. Most of these usages are with IRQ chips who doesn't have any driver run time PM supported and the IRQ CHIP itself is shutdown when the CPU/CPU cluster gets power down. So as far as functionality concerned with the flag, it does what it suppose to do. > Now there is hardware which is missing such a control, so we need to > mask the non wakeup interrupts right before going into suspend. > > That's what IRQCHIP_MASK_ON_SUSPEND does. Not more, not less. See > commit d209a699a0b for more ugly details. > > You might be looking for a different functionality. Can you explain > what you need? > Neil's email came from a discussion on the usage of this flag for OMAP GPIO irqchip which I proposed. With the flag, when the lazy check_irq routine is called, the GPIO driver is runtime suspended and hence the late mask/unmask calls take abort(clocks are already gated). GPIO IRQCHIP is secondary IRQCHIP connected to 1 interrupt line per bank(32 GPIOs) to the primary interrupt controller IRQCHIP. Hope this gives bit more clarity to the discussed problem. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Seeking clarity on IRQCHIP_MASK_ON_SUSPEND
Thomas, On Mon, Sep 10, 2012 at 3:58 PM, Thomas Gleixner t...@linutronix.de wrote: On Mon, 10 Sep 2012, NeilBrown wrote: The IRQCHIP_MASK_ON_SUSPEND flag seems to be hard to use correctly, so either I'm understanding it wrongly, or it could be made easier to use. If the first case, I'm hoping that some improvement to documentation might result. If the second, then maybe we can fix the code. ... Is anyone able to give a definitive answer on this? Should IRQCHIP_MASK_ON_SUSPEND be removed? The whole point of IRQCHIP_MASK_ON_SUSPEND is to deal with hardware designed by geniuses. Most SoCs have a way to mark the interrupts which serve as a wake up source as such. All other interrupts are magically masked on entry to suspend. Just to support this, IRQCHIP_MASK_ON_SUSPEND does work with quite a few ARM platforms too. OMAP already uses it in mainline and I have seen patches for U500 and Tegra SOCs. Most of these usages are with IRQ chips who doesn't have any driver run time PM supported and the IRQ CHIP itself is shutdown when the CPU/CPU cluster gets power down. So as far as functionality concerned with the flag, it does what it suppose to do. Now there is hardware which is missing such a control, so we need to mask the non wakeup interrupts right before going into suspend. That's what IRQCHIP_MASK_ON_SUSPEND does. Not more, not less. See commit d209a699a0b for more ugly details. You might be looking for a different functionality. Can you explain what you need? Neil's email came from a discussion on the usage of this flag for OMAP GPIO irqchip which I proposed. With the flag, when the lazy check_irq routine is called, the GPIO driver is runtime suspended and hence the late mask/unmask calls take abort(clocks are already gated). GPIO IRQCHIP is secondary IRQCHIP connected to 1 interrupt line per bank(32 GPIOs) to the primary interrupt controller IRQCHIP. Hope this gives bit more clarity to the discussed problem. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 00/31] AArch64 Linux kernel port
On Sat, Sep 8, 2012 at 7:29 PM, Nicolas Pitre wrote: > On Sat, 8 Sep 2012, Santosh Shilimkar wrote: > >> Mostly I was looking at the series from SOC boot and CPU PM >> point of view and boot part seems to just fine. >> >> As per discussion at LPC, I have gone through the SMC >> proposal which ARM has published. In general the boot part >> with SMC seems to be doable and can be standardized across SOCs. >> The part which will be conflicting is the CPU power management. >> That seems to be the harder one and the document is at too >> infancy stage from the details point of view. Some bits about >> save, restore are related to switcher kind of architecture, which >> may not be the requirement for all the SoCs. > > I've reviewed an earlier draft of that document. Although the examples > in the latest document appear to be geared towards switcher usage, my > suggestions to the ARM folks was to take into account the CPU hotplug > scenario instead. The provided examples remained switcher centric, but > the API in the published document is now much more generic than it used > to be. I think it should cover all usage scenarios now. > Thanks for the background Nicolas. Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 28/31] arm64: Generic timers support
On Fri, Sep 7, 2012 at 9:57 PM, Catalin Marinas wrote: > From: Marc Zyngier > > This patch adds support for the ARM generic timers with A64 instructions > for accessing the timer registers. It uses the physical counter as the > clock source and the virtual counter as sched_clock. > > The timer frequency can be specified via DT or read from the CNTFRQ_EL0 > register. The physical counter is also accessible from user space > allowing fast gettimeofday() implementation. > > Signed-off-by: Marc Zyngier > Signed-off-by: Will Deacon > Signed-off-by: Catalin Marinas > Acked-by: Tony Lindgren > --- Thanks for C3STOP update. Acked-by: Santosh Shilimkar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
On Sat, Sep 8, 2012 at 3:07 AM, Kevin Hilman wrote: > Hi Neil, > > NeilBrown writes: > >> On Thu, 6 Sep 2012 11:18:09 +0530 "Shilimkar, Santosh" >> wrote: >> >>> On Thu, Sep 6, 2012 at 8:35 AM, NeilBrown wrote: >>> > On Mon, 3 Sep 2012 22:59:06 -0700 "Shilimkar, Santosh" >>> > wrote: >> >>> >> After thinking bit more on this, the problem seems to be coming >>> >> mainly because the gpio device is runtime suspended bit early than >>> >> it should be. Similar issue seen with i2c driver as well. The i2c issue >>> >> was discussed with Rafael at LPC last week. The idea is to move >>> >> the pm_runtime_enable/disable() calls entirely up to the >>> >> _late/_early stage of device suspend/resume. >>> >> Will update this thread once I have further update. >>> > >>> > This won't be late enough. IRQCHIP_MASK_ON_SUSPEND takes effect after all >>> > the _late callbacks have been called. >>> > I, too, spoke to Rafael about this in San Diego. He seemed to agree with >>> > me >>> > that the interrupt needs to be masked in the ->suspend callback. any >>> > later >>> > is too late. >>> > >>> Thanks for information about your discussion. Will wait for the patch then. >>> >>> Regards >>> santosh >> >> I already sent a patch - that is what started this thread :-) >> >> I include it below. >> You said "The patch doesn't seems to be correct" but didn't expand on why. >> Do you still think it is not correct? I wouldn't be surprised if there is >> some case that it doesn't handle quite right, but it seems right to me. >> >> Thanks, >> NeilBrown >> >> >> From: NeilBrown >> Subject: [PATCH] OMAP GPIO - don't wake from suspend unless requested. >> >> Current kernel will wake from suspend on an event on any active >> GPIO even if enable_irq_wake() wasn't called. >> >> There are two reasons that the hardware wake-enable bit should be set: >> >> 1/ while non-suspended the CPU might go into a deep sleep (off_mode) >> in which the wake-enable bit is needed for an interrupt to be >> recognised. >> 2/ while suspended the GPIO interrupt should wake from suspend if and >>only if irq_wake as been enabled. >> >> The code currently doesn't keep these two reasons separate so they get >> confused and sometimes the wakeup flags is set incorrectly. >> >> This patch reverts: >> commit 9c4ed9e6c01e7a8bd9079da8267e1f03cb4761fc >> gpio/omap: remove suspend/resume callbacks >> and >> commit 0aa2727399c0b78225021413022c164cb99fbc5e >> gpio/omap: remove suspend_wakeup field from struct gpio_bank >> >> and makes some minor changes so that we have separate flags for "GPIO >> should wake from deep idle" and "GPIO should wake from suspend". >> >> With this patch, the GPIO from my touch screen doesn't wake my device >> any more, which is what I want. > > I think the direction is right here. We never should've separated the > handling of idle vs suspend wakeups. However, I have a few > questions/doubts below... > I thought irq_set_wake() is suspend only wakeup functionality. In idle, the driver IRQs are not disabled/masked so there no need of any special wakeup calls for idle. More ever patch is adding the suspend hooks for wakeups. I have no objection on the subject patch, but the suspend wakeup facility is easy enough to implement for IRQCHIPS and that is what I was proposing it. Infact the mask on suspend patch almost works and it fails only because the GPIO driver is suspended earlier than the IRQ framework expect it to be. Anyways I step back here since the proposed patch already fixes the issue seen. Assuming the IRQCHIP mask on suspend doesn't seems to work well with drivers as Neil mentioned, the $subject patch seems to be the right option. Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
On Sat, Sep 8, 2012 at 3:07 AM, Kevin Hilman khil...@deeprootsystems.com wrote: Hi Neil, NeilBrown ne...@suse.de writes: On Thu, 6 Sep 2012 11:18:09 +0530 Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Thu, Sep 6, 2012 at 8:35 AM, NeilBrown ne...@suse.de wrote: On Mon, 3 Sep 2012 22:59:06 -0700 Shilimkar, Santosh santosh.shilim...@ti.com wrote: After thinking bit more on this, the problem seems to be coming mainly because the gpio device is runtime suspended bit early than it should be. Similar issue seen with i2c driver as well. The i2c issue was discussed with Rafael at LPC last week. The idea is to move the pm_runtime_enable/disable() calls entirely up to the _late/_early stage of device suspend/resume. Will update this thread once I have further update. This won't be late enough. IRQCHIP_MASK_ON_SUSPEND takes effect after all the _late callbacks have been called. I, too, spoke to Rafael about this in San Diego. He seemed to agree with me that the interrupt needs to be masked in the -suspend callback. any later is too late. Thanks for information about your discussion. Will wait for the patch then. Regards santosh I already sent a patch - that is what started this thread :-) I include it below. You said The patch doesn't seems to be correct but didn't expand on why. Do you still think it is not correct? I wouldn't be surprised if there is some case that it doesn't handle quite right, but it seems right to me. Thanks, NeilBrown From: NeilBrown ne...@suse.de Subject: [PATCH] OMAP GPIO - don't wake from suspend unless requested. Current kernel will wake from suspend on an event on any active GPIO even if enable_irq_wake() wasn't called. There are two reasons that the hardware wake-enable bit should be set: 1/ while non-suspended the CPU might go into a deep sleep (off_mode) in which the wake-enable bit is needed for an interrupt to be recognised. 2/ while suspended the GPIO interrupt should wake from suspend if and only if irq_wake as been enabled. The code currently doesn't keep these two reasons separate so they get confused and sometimes the wakeup flags is set incorrectly. This patch reverts: commit 9c4ed9e6c01e7a8bd9079da8267e1f03cb4761fc gpio/omap: remove suspend/resume callbacks and commit 0aa2727399c0b78225021413022c164cb99fbc5e gpio/omap: remove suspend_wakeup field from struct gpio_bank and makes some minor changes so that we have separate flags for GPIO should wake from deep idle and GPIO should wake from suspend. With this patch, the GPIO from my touch screen doesn't wake my device any more, which is what I want. I think the direction is right here. We never should've separated the handling of idle vs suspend wakeups. However, I have a few questions/doubts below... I thought irq_set_wake() is suspend only wakeup functionality. In idle, the driver IRQs are not disabled/masked so there no need of any special wakeup calls for idle. More ever patch is adding the suspend hooks for wakeups. I have no objection on the subject patch, but the suspend wakeup facility is easy enough to implement for IRQCHIPS and that is what I was proposing it. Infact the mask on suspend patch almost works and it fails only because the GPIO driver is suspended earlier than the IRQ framework expect it to be. Anyways I step back here since the proposed patch already fixes the issue seen. Assuming the IRQCHIP mask on suspend doesn't seems to work well with drivers as Neil mentioned, the $subject patch seems to be the right option. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 28/31] arm64: Generic timers support
On Fri, Sep 7, 2012 at 9:57 PM, Catalin Marinas catalin.mari...@arm.com wrote: From: Marc Zyngier marc.zyng...@arm.com This patch adds support for the ARM generic timers with A64 instructions for accessing the timer registers. It uses the physical counter as the clock source and the virtual counter as sched_clock. The timer frequency can be specified via DT or read from the CNTFRQ_EL0 register. The physical counter is also accessible from user space allowing fast gettimeofday() implementation. Signed-off-by: Marc Zyngier marc.zyng...@arm.com Signed-off-by: Will Deacon will.dea...@arm.com Signed-off-by: Catalin Marinas catalin.mari...@arm.com Acked-by: Tony Lindgren t...@atomide.com --- Thanks for C3STOP update. Acked-by: Santosh Shilimkar santosh.shilim...@ti.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 00/31] AArch64 Linux kernel port
On Sat, Sep 8, 2012 at 7:29 PM, Nicolas Pitre n...@fluxnic.net wrote: On Sat, 8 Sep 2012, Santosh Shilimkar wrote: Mostly I was looking at the series from SOC boot and CPU PM point of view and boot part seems to just fine. As per discussion at LPC, I have gone through the SMC proposal which ARM has published. In general the boot part with SMC seems to be doable and can be standardized across SOCs. The part which will be conflicting is the CPU power management. That seems to be the harder one and the document is at too infancy stage from the details point of view. Some bits about save, restore are related to switcher kind of architecture, which may not be the requirement for all the SoCs. I've reviewed an earlier draft of that document. Although the examples in the latest document appear to be geared towards switcher usage, my suggestions to the ARM folks was to take into account the CPU hotplug scenario instead. The provided examples remained switcher centric, but the API in the published document is now much more generic than it used to be. I think it should cover all usage scenarios now. Thanks for the background Nicolas. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
On Thu, Sep 6, 2012 at 1:21 PM, NeilBrown wrote: > On Thu, 6 Sep 2012 12:57:46 +0530 "Shilimkar, Santosh" > wrote: > >> On Thu, Sep 6, 2012 at 12:32 PM, NeilBrown wrote: >> > On Thu, 6 Sep 2012 11:18:09 +0530 "Shilimkar, Santosh" >> > wrote: >> > >> >> On Thu, Sep 6, 2012 at 8:35 AM, NeilBrown wrote: >> >> > On Mon, 3 Sep 2012 22:59:06 -0700 "Shilimkar, Santosh" >> >> > wrote: >> > >> >> >> After thinking bit more on this, the problem seems to be coming >> >> >> mainly because the gpio device is runtime suspended bit early than >> >> >> it should be. Similar issue seen with i2c driver as well. The i2c issue >> >> >> was discussed with Rafael at LPC last week. The idea is to move >> >> >> the pm_runtime_enable/disable() calls entirely up to the >> >> >> _late/_early stage of device suspend/resume. >> >> >> Will update this thread once I have further update. >> >> > >> >> > This won't be late enough. IRQCHIP_MASK_ON_SUSPEND takes effect after >> >> > all >> >> > the _late callbacks have been called. >> >> > I, too, spoke to Rafael about this in San Diego. He seemed to agree >> >> > with me >> >> > that the interrupt needs to be masked in the ->suspend callback. any >> >> > later >> >> > is too late. >> >> > >> >> Thanks for information about your discussion. Will wait for the patch >> >> then. >> >> >> >> Regards >> >> santosh >> > >> > I already sent a patch - that is what started this thread :-) >> > >> > I include it below. >> > You said "The patch doesn't seems to be correct" but didn't expand on why. >> > Do you still think it is not correct? I wouldn't be surprised if there is >> > some case that it doesn't handle quite right, but it seems right to me. >> > >> Sorry I though you were talking about moving the "Checking wakeup interrupts" >> bit early as discussed on the follow up of alternate suggested patch to make >> use of IRQCHIP_MASK_ON_SUSPEND. >> >> I think we need to fix the issue seen with ' IRQCHIP_MASK_ON_SUSPEND' >> patch. That is at least the expected way to manage suspend and wakeup >> irq masks for a IRQCHIP. > > That is what I thought at first too. But when talking to Rafael he said that > IRQCHIP_MASK_ON_SUSPEND was intended mainly for clock interrupts. For other > less fundamental interrupts, doing the mask/unmask in suspend/resume > callbacks is sufficient and simpler... and actually works. > That is not the how I undetand IRQCHIP_MASK_ON_SUSPEND use. I thought it can be used for any IRQ chip for masking or setting wakeup on interrupts lines managed by that chip for suspend. May be I am wrong. > IRQCHIP_MASK_ON_SUSPEND is currently used by precisely two drivers: > >arch/arm/mach-omap2/omap-wakeupgen.c > and >drivers/mfd/pm8xxx-irq.c > > which suggests that it isn't widely used and quite possibly doesn't actually > work in general. I have seen lot more platforms use in downstream kernels. Also seen recently patches on the linux-arm list for couple of platforms. > > Maybe we need to start a new thread and pester Rafael or Thomas Gleixner > to either explain what is intended for this case, or to fix > IRQCHIP_MASK_ON_SUSPEND so that it can be used in general. > Sounds a good idea. Since you already had discussion with Rafael, probably you can describe the issue better. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
On Thu, Sep 6, 2012 at 12:32 PM, NeilBrown wrote: > On Thu, 6 Sep 2012 11:18:09 +0530 "Shilimkar, Santosh" > wrote: > >> On Thu, Sep 6, 2012 at 8:35 AM, NeilBrown wrote: >> > On Mon, 3 Sep 2012 22:59:06 -0700 "Shilimkar, Santosh" >> > wrote: > >> >> After thinking bit more on this, the problem seems to be coming >> >> mainly because the gpio device is runtime suspended bit early than >> >> it should be. Similar issue seen with i2c driver as well. The i2c issue >> >> was discussed with Rafael at LPC last week. The idea is to move >> >> the pm_runtime_enable/disable() calls entirely up to the >> >> _late/_early stage of device suspend/resume. >> >> Will update this thread once I have further update. >> > >> > This won't be late enough. IRQCHIP_MASK_ON_SUSPEND takes effect after all >> > the _late callbacks have been called. >> > I, too, spoke to Rafael about this in San Diego. He seemed to agree with >> > me >> > that the interrupt needs to be masked in the ->suspend callback. any later >> > is too late. >> > >> Thanks for information about your discussion. Will wait for the patch then. >> >> Regards >> santosh > > I already sent a patch - that is what started this thread :-) > > I include it below. > You said "The patch doesn't seems to be correct" but didn't expand on why. > Do you still think it is not correct? I wouldn't be surprised if there is > some case that it doesn't handle quite right, but it seems right to me. > Sorry I though you were talking about moving the "Checking wakeup interrupts" bit early as discussed on the follow up of alternate suggested patch to make use of IRQCHIP_MASK_ON_SUSPEND. I think we need to fix the issue seen with ' IRQCHIP_MASK_ON_SUSPEND' patch. That is at least the expected way to manage suspend and wakeup irq masks for a IRQCHIP. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
On Thu, Sep 6, 2012 at 12:32 PM, NeilBrown ne...@suse.de wrote: On Thu, 6 Sep 2012 11:18:09 +0530 Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Thu, Sep 6, 2012 at 8:35 AM, NeilBrown ne...@suse.de wrote: On Mon, 3 Sep 2012 22:59:06 -0700 Shilimkar, Santosh santosh.shilim...@ti.com wrote: After thinking bit more on this, the problem seems to be coming mainly because the gpio device is runtime suspended bit early than it should be. Similar issue seen with i2c driver as well. The i2c issue was discussed with Rafael at LPC last week. The idea is to move the pm_runtime_enable/disable() calls entirely up to the _late/_early stage of device suspend/resume. Will update this thread once I have further update. This won't be late enough. IRQCHIP_MASK_ON_SUSPEND takes effect after all the _late callbacks have been called. I, too, spoke to Rafael about this in San Diego. He seemed to agree with me that the interrupt needs to be masked in the -suspend callback. any later is too late. Thanks for information about your discussion. Will wait for the patch then. Regards santosh I already sent a patch - that is what started this thread :-) I include it below. You said The patch doesn't seems to be correct but didn't expand on why. Do you still think it is not correct? I wouldn't be surprised if there is some case that it doesn't handle quite right, but it seems right to me. Sorry I though you were talking about moving the Checking wakeup interrupts bit early as discussed on the follow up of alternate suggested patch to make use of IRQCHIP_MASK_ON_SUSPEND. I think we need to fix the issue seen with ' IRQCHIP_MASK_ON_SUSPEND' patch. That is at least the expected way to manage suspend and wakeup irq masks for a IRQCHIP. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
On Thu, Sep 6, 2012 at 1:21 PM, NeilBrown ne...@suse.de wrote: On Thu, 6 Sep 2012 12:57:46 +0530 Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Thu, Sep 6, 2012 at 12:32 PM, NeilBrown ne...@suse.de wrote: On Thu, 6 Sep 2012 11:18:09 +0530 Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Thu, Sep 6, 2012 at 8:35 AM, NeilBrown ne...@suse.de wrote: On Mon, 3 Sep 2012 22:59:06 -0700 Shilimkar, Santosh santosh.shilim...@ti.com wrote: After thinking bit more on this, the problem seems to be coming mainly because the gpio device is runtime suspended bit early than it should be. Similar issue seen with i2c driver as well. The i2c issue was discussed with Rafael at LPC last week. The idea is to move the pm_runtime_enable/disable() calls entirely up to the _late/_early stage of device suspend/resume. Will update this thread once I have further update. This won't be late enough. IRQCHIP_MASK_ON_SUSPEND takes effect after all the _late callbacks have been called. I, too, spoke to Rafael about this in San Diego. He seemed to agree with me that the interrupt needs to be masked in the -suspend callback. any later is too late. Thanks for information about your discussion. Will wait for the patch then. Regards santosh I already sent a patch - that is what started this thread :-) I include it below. You said The patch doesn't seems to be correct but didn't expand on why. Do you still think it is not correct? I wouldn't be surprised if there is some case that it doesn't handle quite right, but it seems right to me. Sorry I though you were talking about moving the Checking wakeup interrupts bit early as discussed on the follow up of alternate suggested patch to make use of IRQCHIP_MASK_ON_SUSPEND. I think we need to fix the issue seen with ' IRQCHIP_MASK_ON_SUSPEND' patch. That is at least the expected way to manage suspend and wakeup irq masks for a IRQCHIP. That is what I thought at first too. But when talking to Rafael he said that IRQCHIP_MASK_ON_SUSPEND was intended mainly for clock interrupts. For other less fundamental interrupts, doing the mask/unmask in suspend/resume callbacks is sufficient and simpler... and actually works. That is not the how I undetand IRQCHIP_MASK_ON_SUSPEND use. I thought it can be used for any IRQ chip for masking or setting wakeup on interrupts lines managed by that chip for suspend. May be I am wrong. IRQCHIP_MASK_ON_SUSPEND is currently used by precisely two drivers: arch/arm/mach-omap2/omap-wakeupgen.c and drivers/mfd/pm8xxx-irq.c which suggests that it isn't widely used and quite possibly doesn't actually work in general. I have seen lot more platforms use in downstream kernels. Also seen recently patches on the linux-arm list for couple of platforms. Maybe we need to start a new thread and pester Rafael or Thomas Gleixner to either explain what is intended for this case, or to fix IRQCHIP_MASK_ON_SUSPEND so that it can be used in general. Sounds a good idea. Since you already had discussion with Rafael, probably you can describe the issue better. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
On Thu, Sep 6, 2012 at 8:35 AM, NeilBrown wrote:
> On Mon, 3 Sep 2012 22:59:06 -0700 "Shilimkar, Santosh"
> wrote:
>
>> On Sun, Aug 26, 2012 at 6:29 PM, Shilimkar, Santosh
>> wrote:
>> > On Sun, Aug 26, 2012 at 3:53 PM, NeilBrown wrote:
>> >>
>> >> On Sun, 26 Aug 2012 09:47:50 +0530 "Shilimkar, Santosh"
>> >> wrote:
>> >>
>> >> > + Jon,
>> >> >
>> >> > On Sat, Aug 25, 2012 at 5:14 PM, NeilBrown wrote:
>> >> > >
>> >> > >
>> >> > >
>> >> > > Current kernel will wake from suspend on an event on any active
>> >> > > GPIO even if enable_irq_wake() wasn't called.
>> >> > >
>> >> > > There are two reasons that the hardware wake-enable bit should be set:
>> >> > >
>> >> > > 1/ while non-suspended the CPU might go into a deep sleep (off_mode)
>> >> > > in which the wake-enable bit is needed for an interrupt to be
>> >> > > recognised.
>> >> > > 2/ while suspended the GPIO interrupt should wake from suspend if and
>> >> > >only if irq_wake as been enabled.
>> >> > >
>> >> > > The code currently doesn't keep these two reasons separate so they get
>> >> > > confused and sometimes the wakeup flags is set incorrectly.
>> >> > >
>> >> > > This patch reverts:
>> >> > > commit 9c4ed9e6c01e7a8bd9079da8267e1f03cb4761fc
>> >> > > gpio/omap: remove suspend/resume callbacks
>> >> > > and
>> >> > > commit 0aa2727399c0b78225021413022c164cb99fbc5e
>> >> > > gpio/omap: remove suspend_wakeup field from struct gpio_bank
>> >> > >
>> >> > > and makes some minor changes so that we have separate flags for "GPIO
>> >> > > should wake from deep idle" and "GPIO should wake from suspend".
>> >> > >
>> >> > > With this patch, the GPIO from my touch screen doesn't wake my device
>> >> > > any more, which is what I want.
>> >> > >
>> >> > > Cc: Kevin Hilman
>> >> > > Cc: Tony Lindgren
>> >> > > Cc: Santosh Shilimkar
>> >> > > Cc: Cousson, Benoit
>> >> > > Cc: Grant Likely
>> >> > > Cc: Tarun Kanti DebBarma
>> >> > > Cc: Felipe Balbi
>> >> > > Cc: Govindraj.R
>> >> > >
>> >> > > Signed-off-by: NeilBrown
>> >> > >
>> >> > The patch doesn't seems to be correct. At least the 2/ gets
>> >> > fixed with a proper IRQCHIP flag. Can you try the patch at
>> >> > end of the email and see if it helps ? Am attaching it in case
>> >> > mailer damages it.
>> >> >
>> >> > Regards
>> >> > Santosh
>> >> >
>> >> > >From b8a38fc75e046f6462610e26c47c620cad850c24 Mon Sep 17 00:00:00 2001
>> >> > From: Santosh Shilimkar
>> >> > Date: Sun, 26 Aug 2012 09:39:51 +0530
>> >> > Subject: [PATCH] gpio: omap: Set IRQCHIP_MASK_ON_SUSPEND to mask all
>> >> > non-wakeup gpio wakeups.
>> >> >
>> >> > Set the irq chip flag IRQCHIP_MASK_ON_SUSPEND to cause the irq pm code
>> >> > to mask all non-wake gpios in suspend, which will ensure the wakeup
>> >> > enable
>> >> > bit is not set on non-wake gpios.
>> >> >
>> >> > Signed-off-by: Santosh Shilimkar
>> >> > ---
>> >> > drivers/gpio/gpio-omap.c |1 +
>> >> > 1 file changed, 1 insertion(+)
>> >> >
>> >> > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> >> > index e6efd77..50b4c18 100644
>> >> > --- a/drivers/gpio/gpio-omap.c
>> >> > +++ b/drivers/gpio/gpio-omap.c
>> >> > @@ -779,6 +779,7 @@ static struct irq_chip gpio_irq_chip = {
>> >> > .irq_unmask = gpio_unmask_irq,
>> >> > .irq_set_type = gpio_irq_type,
>> >> > .irq_set_wake = gpio_wake_enable,
>> >> > + .flags = IRQCHIP_MASK_ON_SUSPEND;
>> >> > };
>> >> >
>> >> >
>> >> > /*--
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
On Thu, Sep 6, 2012 at 8:35 AM, NeilBrown ne...@suse.de wrote:
On Mon, 3 Sep 2012 22:59:06 -0700 Shilimkar, Santosh
santosh.shilim...@ti.com wrote:
On Sun, Aug 26, 2012 at 6:29 PM, Shilimkar, Santosh
santosh.shilim...@ti.com wrote:
On Sun, Aug 26, 2012 at 3:53 PM, NeilBrown ne...@suse.de wrote:
On Sun, 26 Aug 2012 09:47:50 +0530 Shilimkar, Santosh
santosh.shilim...@ti.com wrote:
+ Jon,
On Sat, Aug 25, 2012 at 5:14 PM, NeilBrown ne...@suse.de wrote:
Current kernel will wake from suspend on an event on any active
GPIO even if enable_irq_wake() wasn't called.
There are two reasons that the hardware wake-enable bit should be set:
1/ while non-suspended the CPU might go into a deep sleep (off_mode)
in which the wake-enable bit is needed for an interrupt to be
recognised.
2/ while suspended the GPIO interrupt should wake from suspend if and
only if irq_wake as been enabled.
The code currently doesn't keep these two reasons separate so they get
confused and sometimes the wakeup flags is set incorrectly.
This patch reverts:
commit 9c4ed9e6c01e7a8bd9079da8267e1f03cb4761fc
gpio/omap: remove suspend/resume callbacks
and
commit 0aa2727399c0b78225021413022c164cb99fbc5e
gpio/omap: remove suspend_wakeup field from struct gpio_bank
and makes some minor changes so that we have separate flags for GPIO
should wake from deep idle and GPIO should wake from suspend.
With this patch, the GPIO from my touch screen doesn't wake my device
any more, which is what I want.
Cc: Kevin Hilman khil...@ti.com
Cc: Tony Lindgren t...@atomide.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Cousson, Benoit b-cous...@ti.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Tarun Kanti DebBarma tarun.ka...@ti.com
Cc: Felipe Balbi ba...@ti.com
Cc: Govindraj.R govindraj.r...@ti.com
Signed-off-by: NeilBrown ne...@suse.de
The patch doesn't seems to be correct. At least the 2/ gets
fixed with a proper IRQCHIP flag. Can you try the patch at
end of the email and see if it helps ? Am attaching it in case
mailer damages it.
Regards
Santosh
From b8a38fc75e046f6462610e26c47c620cad850c24 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar santosh.shilim...@ti.com
Date: Sun, 26 Aug 2012 09:39:51 +0530
Subject: [PATCH] gpio: omap: Set IRQCHIP_MASK_ON_SUSPEND to mask all
non-wakeup gpio wakeups.
Set the irq chip flag IRQCHIP_MASK_ON_SUSPEND to cause the irq pm code
to mask all non-wake gpios in suspend, which will ensure the wakeup
enable
bit is not set on non-wake gpios.
Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
---
drivers/gpio/gpio-omap.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index e6efd77..50b4c18 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -779,6 +779,7 @@ static struct irq_chip gpio_irq_chip = {
.irq_unmask = gpio_unmask_irq,
.irq_set_type = gpio_irq_type,
.irq_set_wake = gpio_wake_enable,
+ .flags = IRQCHIP_MASK_ON_SUSPEND;
};
/*-*/
No obvious damage, unless the mailer is responsible or the ';' at the end
of
the line, rather than ',' :-)
:-) That was typo.
The approach makes sense, but does actually work. Should be fixable
though.
When I try this I get:
[ 158.114440] Checking wakeup interrupts
[ 158.118408] Unhandled fault: external abort on non-linefetch (0x1028)
at 0xfb054040
[ 158.126403] Internal error: : 1028 [#1] PREEMPT ARM
[ 158.131500] Modules linked in: ipv6 g_ether hso libertas_sdio libertas
cfg80211
[ 158.139190] CPU: 0Not tainted (3.5.0-gta04-debug+ #2)
[ 158.144927] PC is at _set_gpio_triggering+0x38/0x258
[ 158.150115] LR is at gpio_mask_irq+0xac/0xc0
[ 158.154602] pc : [c01d24a0]lr : [c01d2f68]psr: 6193
[ 158.154602] sp : db521e90 ip : 0011 fp : beeecc2c
[ 158.166595] r10: c05c8ebc r9 : daa5a858 r8 : 0003
[ 158.172027] r7 : a193 r6 : r5 : fb054000 r4 : ded44e18
[ 158.178863] r3 : 0001 r2 : r1 : ded30340 r0 : 0040
[ 158.185668] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM
Segment use
so it looks like runtime PM has turned off the iclk to the GPIO module so
that
when we try to tell it to change settings, it is no longer listening to
us.
From the crash logs it appears like that.
The Checking wakeup interrupts function happens very late in the suspend
cycle, after all the suspend_late and suspend_noirq functions have run.
Maybe it needs to be moved earlier...
No it shouldn't be moved and it is that point for lot many good
reasons. Ofcourse
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
On Sun, Aug 26, 2012 at 6:29 PM, Shilimkar, Santosh
wrote:
> On Sun, Aug 26, 2012 at 3:53 PM, NeilBrown wrote:
>>
>> On Sun, 26 Aug 2012 09:47:50 +0530 "Shilimkar, Santosh"
>> wrote:
>>
>> > + Jon,
>> >
>> > On Sat, Aug 25, 2012 at 5:14 PM, NeilBrown wrote:
>> > >
>> > >
>> > >
>> > > Current kernel will wake from suspend on an event on any active
>> > > GPIO even if enable_irq_wake() wasn't called.
>> > >
>> > > There are two reasons that the hardware wake-enable bit should be set:
>> > >
>> > > 1/ while non-suspended the CPU might go into a deep sleep (off_mode)
>> > > in which the wake-enable bit is needed for an interrupt to be
>> > > recognised.
>> > > 2/ while suspended the GPIO interrupt should wake from suspend if and
>> > >only if irq_wake as been enabled.
>> > >
>> > > The code currently doesn't keep these two reasons separate so they get
>> > > confused and sometimes the wakeup flags is set incorrectly.
>> > >
>> > > This patch reverts:
>> > > commit 9c4ed9e6c01e7a8bd9079da8267e1f03cb4761fc
>> > > gpio/omap: remove suspend/resume callbacks
>> > > and
>> > > commit 0aa2727399c0b78225021413022c164cb99fbc5e
>> > > gpio/omap: remove suspend_wakeup field from struct gpio_bank
>> > >
>> > > and makes some minor changes so that we have separate flags for "GPIO
>> > > should wake from deep idle" and "GPIO should wake from suspend".
>> > >
>> > > With this patch, the GPIO from my touch screen doesn't wake my device
>> > > any more, which is what I want.
>> > >
>> > > Cc: Kevin Hilman
>> > > Cc: Tony Lindgren
>> > > Cc: Santosh Shilimkar
>> > > Cc: Cousson, Benoit
>> > > Cc: Grant Likely
>> > > Cc: Tarun Kanti DebBarma
>> > > Cc: Felipe Balbi
>> > > Cc: Govindraj.R
>> > >
>> > > Signed-off-by: NeilBrown
>> > >
>> > The patch doesn't seems to be correct. At least the 2/ gets
>> > fixed with a proper IRQCHIP flag. Can you try the patch at
>> > end of the email and see if it helps ? Am attaching it in case
>> > mailer damages it.
>> >
>> > Regards
>> > Santosh
>> >
>> > >From b8a38fc75e046f6462610e26c47c620cad850c24 Mon Sep 17 00:00:00 2001
>> > From: Santosh Shilimkar
>> > Date: Sun, 26 Aug 2012 09:39:51 +0530
>> > Subject: [PATCH] gpio: omap: Set IRQCHIP_MASK_ON_SUSPEND to mask all
>> > non-wakeup gpio wakeups.
>> >
>> > Set the irq chip flag IRQCHIP_MASK_ON_SUSPEND to cause the irq pm code
>> > to mask all non-wake gpios in suspend, which will ensure the wakeup
>> > enable
>> > bit is not set on non-wake gpios.
>> >
>> > Signed-off-by: Santosh Shilimkar
>> > ---
>> > drivers/gpio/gpio-omap.c |1 +
>> > 1 file changed, 1 insertion(+)
>> >
>> > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> > index e6efd77..50b4c18 100644
>> > --- a/drivers/gpio/gpio-omap.c
>> > +++ b/drivers/gpio/gpio-omap.c
>> > @@ -779,6 +779,7 @@ static struct irq_chip gpio_irq_chip = {
>> > .irq_unmask = gpio_unmask_irq,
>> > .irq_set_type = gpio_irq_type,
>> > .irq_set_wake = gpio_wake_enable,
>> > + .flags = IRQCHIP_MASK_ON_SUSPEND;
>> > };
>> >
>> >
>> > /*-*/
>>
>>
>> No obvious damage, unless the mailer is responsible or the ';' at the end
>> of
>> the line, rather than ',' :-)
>>
> :-) That was typo.
>
>> The approach makes sense, but does actually work. Should be fixable
>> though.
>>
>> When I try this I get:
>>
>>
>>
>> [ 158.114440] Checking wakeup interrupts
>> [ 158.118408] Unhandled fault: external abort on non-linefetch (0x1028)
>> at 0xfb054040
>> [ 158.126403] Internal error: : 1028 [#1] PREEMPT ARM
>> [ 158.131500] Modules linked in: ipv6 g_ether hso libertas_sdio libertas
>> cfg80211
>> [ 158.139190] CPU: 0Not tainted (3.5.0-gta04-debug+ #2)
>> [ 158.144927] PC is at _set_gpio_triggering+0x38/0x258
>> [ 158.150115] LR is at gpio_mask_irq+0xac/0xc0
>> [ 158.154602] pc : []lr : []psr: 619
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
On Sun, Aug 26, 2012 at 6:29 PM, Shilimkar, Santosh
santosh.shilim...@ti.com wrote:
On Sun, Aug 26, 2012 at 3:53 PM, NeilBrown ne...@suse.de wrote:
On Sun, 26 Aug 2012 09:47:50 +0530 Shilimkar, Santosh
santosh.shilim...@ti.com wrote:
+ Jon,
On Sat, Aug 25, 2012 at 5:14 PM, NeilBrown ne...@suse.de wrote:
Current kernel will wake from suspend on an event on any active
GPIO even if enable_irq_wake() wasn't called.
There are two reasons that the hardware wake-enable bit should be set:
1/ while non-suspended the CPU might go into a deep sleep (off_mode)
in which the wake-enable bit is needed for an interrupt to be
recognised.
2/ while suspended the GPIO interrupt should wake from suspend if and
only if irq_wake as been enabled.
The code currently doesn't keep these two reasons separate so they get
confused and sometimes the wakeup flags is set incorrectly.
This patch reverts:
commit 9c4ed9e6c01e7a8bd9079da8267e1f03cb4761fc
gpio/omap: remove suspend/resume callbacks
and
commit 0aa2727399c0b78225021413022c164cb99fbc5e
gpio/omap: remove suspend_wakeup field from struct gpio_bank
and makes some minor changes so that we have separate flags for GPIO
should wake from deep idle and GPIO should wake from suspend.
With this patch, the GPIO from my touch screen doesn't wake my device
any more, which is what I want.
Cc: Kevin Hilman khil...@ti.com
Cc: Tony Lindgren t...@atomide.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Cousson, Benoit b-cous...@ti.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Tarun Kanti DebBarma tarun.ka...@ti.com
Cc: Felipe Balbi ba...@ti.com
Cc: Govindraj.R govindraj.r...@ti.com
Signed-off-by: NeilBrown ne...@suse.de
The patch doesn't seems to be correct. At least the 2/ gets
fixed with a proper IRQCHIP flag. Can you try the patch at
end of the email and see if it helps ? Am attaching it in case
mailer damages it.
Regards
Santosh
From b8a38fc75e046f6462610e26c47c620cad850c24 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar santosh.shilim...@ti.com
Date: Sun, 26 Aug 2012 09:39:51 +0530
Subject: [PATCH] gpio: omap: Set IRQCHIP_MASK_ON_SUSPEND to mask all
non-wakeup gpio wakeups.
Set the irq chip flag IRQCHIP_MASK_ON_SUSPEND to cause the irq pm code
to mask all non-wake gpios in suspend, which will ensure the wakeup
enable
bit is not set on non-wake gpios.
Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
---
drivers/gpio/gpio-omap.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index e6efd77..50b4c18 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -779,6 +779,7 @@ static struct irq_chip gpio_irq_chip = {
.irq_unmask = gpio_unmask_irq,
.irq_set_type = gpio_irq_type,
.irq_set_wake = gpio_wake_enable,
+ .flags = IRQCHIP_MASK_ON_SUSPEND;
};
/*-*/
No obvious damage, unless the mailer is responsible or the ';' at the end
of
the line, rather than ',' :-)
:-) That was typo.
The approach makes sense, but does actually work. Should be fixable
though.
When I try this I get:
[ 158.114440] Checking wakeup interrupts
[ 158.118408] Unhandled fault: external abort on non-linefetch (0x1028)
at 0xfb054040
[ 158.126403] Internal error: : 1028 [#1] PREEMPT ARM
[ 158.131500] Modules linked in: ipv6 g_ether hso libertas_sdio libertas
cfg80211
[ 158.139190] CPU: 0Not tainted (3.5.0-gta04-debug+ #2)
[ 158.144927] PC is at _set_gpio_triggering+0x38/0x258
[ 158.150115] LR is at gpio_mask_irq+0xac/0xc0
[ 158.154602] pc : [c01d24a0]lr : [c01d2f68]psr: 6193
[ 158.154602] sp : db521e90 ip : 0011 fp : beeecc2c
[ 158.166595] r10: c05c8ebc r9 : daa5a858 r8 : 0003
[ 158.172027] r7 : a193 r6 : r5 : fb054000 r4 : ded44e18
[ 158.178863] r3 : 0001 r2 : r1 : ded30340 r0 : 0040
[ 158.185668] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM
Segment use
so it looks like runtime PM has turned off the iclk to the GPIO module so
that
when we try to tell it to change settings, it is no longer listening to
us.
From the crash logs it appears like that.
The Checking wakeup interrupts function happens very late in the suspend
cycle, after all the suspend_late and suspend_noirq functions have run.
Maybe it needs to be moved earlier...
No it shouldn't be moved and it is that point for lot many good
reasons. Ofcourse
this omap gpio driver crash needs to be addressed. Need to think bit
more on this
issue.
After thinking bit more on this, the problem seems to be coming
mainly because the gpio device is runtime suspended bit early than
it should be. Similar issue seen with i2c driver as well
Re: [PATCH v2 6/9] misc: Generic on-chip SRAM allocation driver
On Fri, Aug 31, 2012 at 2:37 AM, Jan Lübbe wrote: > > On Fri, 2012-08-31 at 11:27 +0200, Philipp Zabel wrote: > > This driver requests and remaps a memory region as configured in the > > device tree. It serves memory from this region via the genalloc API. > > > > Other drivers can retrieve the genalloc pool from a phandle pointing > > to this drivers' device node in the device tree. > > > > Signed-off-by: Philipp Zabel > > --- > > drivers/misc/Kconfig |8 > > drivers/misc/Makefile |1 + > > drivers/misc/sram.c | 105 > > + > > 3 files changed, 114 insertions(+) > > create mode 100644 drivers/misc/sram.c > > We now have drivers/memory, which seems to be a good place for this. > drivers/memory is created for Memory controller device drivers. SRAM is just pool of memory and should belong to some other place. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 6/9] misc: Generic on-chip SRAM allocation driver
On Fri, Aug 31, 2012 at 2:37 AM, Jan Lübbe j...@pengutronix.de wrote: On Fri, 2012-08-31 at 11:27 +0200, Philipp Zabel wrote: This driver requests and remaps a memory region as configured in the device tree. It serves memory from this region via the genalloc API. Other drivers can retrieve the genalloc pool from a phandle pointing to this drivers' device node in the device tree. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/misc/Kconfig |8 drivers/misc/Makefile |1 + drivers/misc/sram.c | 105 + 3 files changed, 114 insertions(+) create mode 100644 drivers/misc/sram.c We now have drivers/memory, which seems to be a good place for this. drivers/memory is created for Memory controller device drivers. SRAM is just pool of memory and should belong to some other place. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH - 2/2] arm: disable caches based on config option
On Thu, Aug 30, 2012 at 7:43 AM, Murali Karicheri wrote: > CONFIG_CPU_ICACHE_DISABLE and CONFIG_CPU_DCACHE_DISABLE are available > to allow disabling of D-Cache and I-Cache. However the compressed version > of head.S currently doesn't check these variables. This patch will fix > this issue and is written similar to the uncompressed version of head.S > > Signed-off-by: Murali Karicheri > --- Acked-by: Santosh Shilimkar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH - 2/2] arm: disable caches based on config option
On Thu, Aug 30, 2012 at 7:43 AM, Murali Karicheri m-kariche...@ti.com wrote: CONFIG_CPU_ICACHE_DISABLE and CONFIG_CPU_DCACHE_DISABLE are available to allow disabling of D-Cache and I-Cache. However the compressed version of head.S currently doesn't check these variables. This patch will fix this issue and is written similar to the uncompressed version of head.S Signed-off-by: Murali Karicheri m-kariche...@ti.com --- Acked-by: Santosh Shilimkar santosh.shilim...@ti.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
On Sun, Aug 26, 2012 at 3:53 PM, NeilBrown wrote:
>
> On Sun, 26 Aug 2012 09:47:50 +0530 "Shilimkar, Santosh"
> wrote:
>
> > + Jon,
> >
> > On Sat, Aug 25, 2012 at 5:14 PM, NeilBrown wrote:
> > >
> > >
> > >
> > > Current kernel will wake from suspend on an event on any active
> > > GPIO even if enable_irq_wake() wasn't called.
> > >
> > > There are two reasons that the hardware wake-enable bit should be set:
> > >
> > > 1/ while non-suspended the CPU might go into a deep sleep (off_mode)
> > > in which the wake-enable bit is needed for an interrupt to be
> > > recognised.
> > > 2/ while suspended the GPIO interrupt should wake from suspend if and
> > >only if irq_wake as been enabled.
> > >
> > > The code currently doesn't keep these two reasons separate so they get
> > > confused and sometimes the wakeup flags is set incorrectly.
> > >
> > > This patch reverts:
> > > commit 9c4ed9e6c01e7a8bd9079da8267e1f03cb4761fc
> > > gpio/omap: remove suspend/resume callbacks
> > > and
> > > commit 0aa2727399c0b78225021413022c164cb99fbc5e
> > > gpio/omap: remove suspend_wakeup field from struct gpio_bank
> > >
> > > and makes some minor changes so that we have separate flags for "GPIO
> > > should wake from deep idle" and "GPIO should wake from suspend".
> > >
> > > With this patch, the GPIO from my touch screen doesn't wake my device
> > > any more, which is what I want.
> > >
> > > Cc: Kevin Hilman
> > > Cc: Tony Lindgren
> > > Cc: Santosh Shilimkar
> > > Cc: Cousson, Benoit
> > > Cc: Grant Likely
> > > Cc: Tarun Kanti DebBarma
> > > Cc: Felipe Balbi
> > > Cc: Govindraj.R
> > >
> > > Signed-off-by: NeilBrown
> > >
> > The patch doesn't seems to be correct. At least the 2/ gets
> > fixed with a proper IRQCHIP flag. Can you try the patch at
> > end of the email and see if it helps ? Am attaching it in case
> > mailer damages it.
> >
> > Regards
> > Santosh
> >
> > >From b8a38fc75e046f6462610e26c47c620cad850c24 Mon Sep 17 00:00:00 2001
> > From: Santosh Shilimkar
> > Date: Sun, 26 Aug 2012 09:39:51 +0530
> > Subject: [PATCH] gpio: omap: Set IRQCHIP_MASK_ON_SUSPEND to mask all
> > non-wakeup gpio wakeups.
> >
> > Set the irq chip flag IRQCHIP_MASK_ON_SUSPEND to cause the irq pm code
> > to mask all non-wake gpios in suspend, which will ensure the wakeup
> > enable
> > bit is not set on non-wake gpios.
> >
> > Signed-off-by: Santosh Shilimkar
> > ---
> > drivers/gpio/gpio-omap.c |1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> > index e6efd77..50b4c18 100644
> > --- a/drivers/gpio/gpio-omap.c
> > +++ b/drivers/gpio/gpio-omap.c
> > @@ -779,6 +779,7 @@ static struct irq_chip gpio_irq_chip = {
> > .irq_unmask = gpio_unmask_irq,
> > .irq_set_type = gpio_irq_type,
> > .irq_set_wake = gpio_wake_enable,
> > + .flags = IRQCHIP_MASK_ON_SUSPEND;
> > };
> >
> >
> > /*-*/
>
>
> No obvious damage, unless the mailer is responsible or the ';' at the end
> of
> the line, rather than ',' :-)
>
:-) That was typo.
> The approach makes sense, but does actually work. Should be fixable
> though.
>
> When I try this I get:
>
>
>
> [ 158.114440] Checking wakeup interrupts
> [ 158.118408] Unhandled fault: external abort on non-linefetch (0x1028)
> at 0xfb054040
> [ 158.126403] Internal error: : 1028 [#1] PREEMPT ARM
> [ 158.131500] Modules linked in: ipv6 g_ether hso libertas_sdio libertas
> cfg80211
> [ 158.139190] CPU: 0Not tainted (3.5.0-gta04-debug+ #2)
> [ 158.144927] PC is at _set_gpio_triggering+0x38/0x258
> [ 158.150115] LR is at gpio_mask_irq+0xac/0xc0
> [ 158.154602] pc : []lr : []psr: 6193
> [ 158.154602] sp : db521e90 ip : 0011 fp : beeecc2c
> [ 158.166595] r10: c05c8ebc r9 : daa5a858 r8 : 0003
> [ 158.172027] r7 : a193 r6 : r5 : fb054000 r4 : ded44e18
> [ 158.178863] r3 : 0001 r2 : r1 : ded30340 r0 : 0040
> [ 158.185668] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM
> Segment use
>
> so it looks like runtime PM has turned off the iclk to the GPIO module so
> that
> when we try to tell it to change settings, it is no longer listening to
> us.
>From the crash logs it appears like that.
> The "Checking wakeup interrupts" function happens very late in the suspend
> cycle, after all the suspend_late and suspend_noirq functions have run.
> Maybe it needs to be moved earlier...
>
No it shouldn't be moved and it is that point for lot many good
reasons. Ofcourse
this omap gpio driver crash needs to be addressed. Need to think bit
more on this
issue.
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
On Sun, Aug 26, 2012 at 3:53 PM, NeilBrown ne...@suse.de wrote:
On Sun, 26 Aug 2012 09:47:50 +0530 Shilimkar, Santosh
santosh.shilim...@ti.com wrote:
+ Jon,
On Sat, Aug 25, 2012 at 5:14 PM, NeilBrown ne...@suse.de wrote:
Current kernel will wake from suspend on an event on any active
GPIO even if enable_irq_wake() wasn't called.
There are two reasons that the hardware wake-enable bit should be set:
1/ while non-suspended the CPU might go into a deep sleep (off_mode)
in which the wake-enable bit is needed for an interrupt to be
recognised.
2/ while suspended the GPIO interrupt should wake from suspend if and
only if irq_wake as been enabled.
The code currently doesn't keep these two reasons separate so they get
confused and sometimes the wakeup flags is set incorrectly.
This patch reverts:
commit 9c4ed9e6c01e7a8bd9079da8267e1f03cb4761fc
gpio/omap: remove suspend/resume callbacks
and
commit 0aa2727399c0b78225021413022c164cb99fbc5e
gpio/omap: remove suspend_wakeup field from struct gpio_bank
and makes some minor changes so that we have separate flags for GPIO
should wake from deep idle and GPIO should wake from suspend.
With this patch, the GPIO from my touch screen doesn't wake my device
any more, which is what I want.
Cc: Kevin Hilman khil...@ti.com
Cc: Tony Lindgren t...@atomide.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Cousson, Benoit b-cous...@ti.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Tarun Kanti DebBarma tarun.ka...@ti.com
Cc: Felipe Balbi ba...@ti.com
Cc: Govindraj.R govindraj.r...@ti.com
Signed-off-by: NeilBrown ne...@suse.de
The patch doesn't seems to be correct. At least the 2/ gets
fixed with a proper IRQCHIP flag. Can you try the patch at
end of the email and see if it helps ? Am attaching it in case
mailer damages it.
Regards
Santosh
From b8a38fc75e046f6462610e26c47c620cad850c24 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar santosh.shilim...@ti.com
Date: Sun, 26 Aug 2012 09:39:51 +0530
Subject: [PATCH] gpio: omap: Set IRQCHIP_MASK_ON_SUSPEND to mask all
non-wakeup gpio wakeups.
Set the irq chip flag IRQCHIP_MASK_ON_SUSPEND to cause the irq pm code
to mask all non-wake gpios in suspend, which will ensure the wakeup
enable
bit is not set on non-wake gpios.
Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
---
drivers/gpio/gpio-omap.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index e6efd77..50b4c18 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -779,6 +779,7 @@ static struct irq_chip gpio_irq_chip = {
.irq_unmask = gpio_unmask_irq,
.irq_set_type = gpio_irq_type,
.irq_set_wake = gpio_wake_enable,
+ .flags = IRQCHIP_MASK_ON_SUSPEND;
};
/*-*/
No obvious damage, unless the mailer is responsible or the ';' at the end
of
the line, rather than ',' :-)
:-) That was typo.
The approach makes sense, but does actually work. Should be fixable
though.
When I try this I get:
[ 158.114440] Checking wakeup interrupts
[ 158.118408] Unhandled fault: external abort on non-linefetch (0x1028)
at 0xfb054040
[ 158.126403] Internal error: : 1028 [#1] PREEMPT ARM
[ 158.131500] Modules linked in: ipv6 g_ether hso libertas_sdio libertas
cfg80211
[ 158.139190] CPU: 0Not tainted (3.5.0-gta04-debug+ #2)
[ 158.144927] PC is at _set_gpio_triggering+0x38/0x258
[ 158.150115] LR is at gpio_mask_irq+0xac/0xc0
[ 158.154602] pc : [c01d24a0]lr : [c01d2f68]psr: 6193
[ 158.154602] sp : db521e90 ip : 0011 fp : beeecc2c
[ 158.166595] r10: c05c8ebc r9 : daa5a858 r8 : 0003
[ 158.172027] r7 : a193 r6 : r5 : fb054000 r4 : ded44e18
[ 158.178863] r3 : 0001 r2 : r1 : ded30340 r0 : 0040
[ 158.185668] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM
Segment use
so it looks like runtime PM has turned off the iclk to the GPIO module so
that
when we try to tell it to change settings, it is no longer listening to
us.
From the crash logs it appears like that.
The Checking wakeup interrupts function happens very late in the suspend
cycle, after all the suspend_late and suspend_noirq functions have run.
Maybe it needs to be moved earlier...
No it shouldn't be moved and it is that point for lot many good
reasons. Ofcourse
this omap gpio driver crash needs to be addressed. Need to think bit
more on this
issue.
Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
+ Jon,
On Sat, Aug 25, 2012 at 5:14 PM, NeilBrown wrote:
>
>
>
> Current kernel will wake from suspend on an event on any active
> GPIO even if enable_irq_wake() wasn't called.
>
> There are two reasons that the hardware wake-enable bit should be set:
>
> 1/ while non-suspended the CPU might go into a deep sleep (off_mode)
> in which the wake-enable bit is needed for an interrupt to be
> recognised.
> 2/ while suspended the GPIO interrupt should wake from suspend if and
>only if irq_wake as been enabled.
>
> The code currently doesn't keep these two reasons separate so they get
> confused and sometimes the wakeup flags is set incorrectly.
>
> This patch reverts:
> commit 9c4ed9e6c01e7a8bd9079da8267e1f03cb4761fc
> gpio/omap: remove suspend/resume callbacks
> and
> commit 0aa2727399c0b78225021413022c164cb99fbc5e
> gpio/omap: remove suspend_wakeup field from struct gpio_bank
>
> and makes some minor changes so that we have separate flags for "GPIO
> should wake from deep idle" and "GPIO should wake from suspend".
>
> With this patch, the GPIO from my touch screen doesn't wake my device
> any more, which is what I want.
>
> Cc: Kevin Hilman
> Cc: Tony Lindgren
> Cc: Santosh Shilimkar
> Cc: Cousson, Benoit
> Cc: Grant Likely
> Cc: Tarun Kanti DebBarma
> Cc: Felipe Balbi
> Cc: Govindraj.R
>
> Signed-off-by: NeilBrown
>
The patch doesn't seems to be correct. At least the 2/ gets
fixed with a proper IRQCHIP flag. Can you try the patch at
end of the email and see if it helps ? Am attaching it in case
mailer damages it.
Regards
Santosh
>From b8a38fc75e046f6462610e26c47c620cad850c24 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar
Date: Sun, 26 Aug 2012 09:39:51 +0530
Subject: [PATCH] gpio: omap: Set IRQCHIP_MASK_ON_SUSPEND to mask all
non-wakeup gpio wakeups.
Set the irq chip flag IRQCHIP_MASK_ON_SUSPEND to cause the irq pm code
to mask all non-wake gpios in suspend, which will ensure the wakeup enable
bit is not set on non-wake gpios.
Signed-off-by: Santosh Shilimkar
---
drivers/gpio/gpio-omap.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index e6efd77..50b4c18 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -779,6 +779,7 @@ static struct irq_chip gpio_irq_chip = {
.irq_unmask = gpio_unmask_irq,
.irq_set_type = gpio_irq_type,
.irq_set_wake = gpio_wake_enable,
+ .flags = IRQCHIP_MASK_ON_SUSPEND;
};
/*-*/
--
1.7.9.5
0001-gpio-omap-Set-IRQCHIP_MASK_ON_SUSPEND-to-mask-all-no.patch
Description: Binary data
Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
+ Jon,
On Sat, Aug 25, 2012 at 5:14 PM, NeilBrown ne...@suse.de wrote:
Current kernel will wake from suspend on an event on any active
GPIO even if enable_irq_wake() wasn't called.
There are two reasons that the hardware wake-enable bit should be set:
1/ while non-suspended the CPU might go into a deep sleep (off_mode)
in which the wake-enable bit is needed for an interrupt to be
recognised.
2/ while suspended the GPIO interrupt should wake from suspend if and
only if irq_wake as been enabled.
The code currently doesn't keep these two reasons separate so they get
confused and sometimes the wakeup flags is set incorrectly.
This patch reverts:
commit 9c4ed9e6c01e7a8bd9079da8267e1f03cb4761fc
gpio/omap: remove suspend/resume callbacks
and
commit 0aa2727399c0b78225021413022c164cb99fbc5e
gpio/omap: remove suspend_wakeup field from struct gpio_bank
and makes some minor changes so that we have separate flags for GPIO
should wake from deep idle and GPIO should wake from suspend.
With this patch, the GPIO from my touch screen doesn't wake my device
any more, which is what I want.
Cc: Kevin Hilman khil...@ti.com
Cc: Tony Lindgren t...@atomide.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Cousson, Benoit b-cous...@ti.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Tarun Kanti DebBarma tarun.ka...@ti.com
Cc: Felipe Balbi ba...@ti.com
Cc: Govindraj.R govindraj.r...@ti.com
Signed-off-by: NeilBrown ne...@suse.de
The patch doesn't seems to be correct. At least the 2/ gets
fixed with a proper IRQCHIP flag. Can you try the patch at
end of the email and see if it helps ? Am attaching it in case
mailer damages it.
Regards
Santosh
From b8a38fc75e046f6462610e26c47c620cad850c24 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar santosh.shilim...@ti.com
Date: Sun, 26 Aug 2012 09:39:51 +0530
Subject: [PATCH] gpio: omap: Set IRQCHIP_MASK_ON_SUSPEND to mask all
non-wakeup gpio wakeups.
Set the irq chip flag IRQCHIP_MASK_ON_SUSPEND to cause the irq pm code
to mask all non-wake gpios in suspend, which will ensure the wakeup enable
bit is not set on non-wake gpios.
Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
---
drivers/gpio/gpio-omap.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index e6efd77..50b4c18 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -779,6 +779,7 @@ static struct irq_chip gpio_irq_chip = {
.irq_unmask = gpio_unmask_irq,
.irq_set_type = gpio_irq_type,
.irq_set_wake = gpio_wake_enable,
+ .flags = IRQCHIP_MASK_ON_SUSPEND;
};
/*-*/
--
1.7.9.5
0001-gpio-omap-Set-IRQCHIP_MASK_ON_SUSPEND-to-mask-all-no.patch
Description: Binary data
Re: [PATCH 2/6] ARM: omap: allow building omap44xx without SMP
On Thu, Aug 23, 2012 at 5:42 PM, Arnd Bergmann wrote: > > On Thursday 23 August 2012, Shilimkar, Santosh wrote: > > I see your point but alternate patch is pushing down the fix to the low > > level driver and that means you end up patching more drivers when they > > use COUPLE idle infrastructure. That was the only reason I was thinking > > of suppressing the error at the source. > > > > Since it is just for the random builds and actually doesn't impact the > > real > > functionality as such, I am fine with your proposed patch too. > > Ok. It would be nice of course to test if this actually works on > uniprocessor > configurations. > Have tested the patch and it does boot with UP build on OMAP. Acked-tested-by: Santosh Shilimkar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] ARM: omap: allow building omap44xx without SMP
On Wed, Aug 22, 2012 at 10:52 PM, Arnd Bergmann wrote: > > On Wednesday 22 August 2012, Shilimkar, Santosh wrote: > > > Was just thinking whether we should just take care of it at > > core cpuidle level itself. Will below be enough to kill the build > > error what you mentioned in the change log ? > > > > diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c > > index 2c9bf26..df34534 100644 > > --- a/drivers/cpuidle/coupled.c > > +++ b/drivers/cpuidle/coupled.c > > @@ -314,7 +314,9 @@ static void cpuidle_coupled_poke(int cpu) > > struct call_single_data *csd = _cpu(cpuidle_coupled_poke_cb, > > cpu); > > > > if (!cpumask_test_and_set_cpu(cpu, _coupled_poked_mask)) > > +#ifdef CONFIG_SMP > > __smp_call_function_single(cpu, csd, 0); > > +#endif > > } > > > > That would work, but isn't the entire concept of the cpuidle-coupled > driver > dependent on SMP? If this driver makes no sense on UP, I think we should > not attempt to build it. > I see your point but alternate patch is pushing down the fix to the low level driver and that means you end up patching more drivers when they use COUPLE idle infrastructure. That was the only reason I was thinking of suppressing the error at the source. Since it is just for the random builds and actually doesn't impact the real functionality as such, I am fine with your proposed patch too. Regards santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] ARM: omap: allow building omap44xx without SMP
On Wed, Aug 22, 2012 at 10:52 PM, Arnd Bergmann a...@arndb.de wrote: On Wednesday 22 August 2012, Shilimkar, Santosh wrote: Was just thinking whether we should just take care of it at core cpuidle level itself. Will below be enough to kill the build error what you mentioned in the change log ? diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c index 2c9bf26..df34534 100644 --- a/drivers/cpuidle/coupled.c +++ b/drivers/cpuidle/coupled.c @@ -314,7 +314,9 @@ static void cpuidle_coupled_poke(int cpu) struct call_single_data *csd = per_cpu(cpuidle_coupled_poke_cb, cpu); if (!cpumask_test_and_set_cpu(cpu, cpuidle_coupled_poked_mask)) +#ifdef CONFIG_SMP __smp_call_function_single(cpu, csd, 0); +#endif } That would work, but isn't the entire concept of the cpuidle-coupled driver dependent on SMP? If this driver makes no sense on UP, I think we should not attempt to build it. I see your point but alternate patch is pushing down the fix to the low level driver and that means you end up patching more drivers when they use COUPLE idle infrastructure. That was the only reason I was thinking of suppressing the error at the source. Since it is just for the random builds and actually doesn't impact the real functionality as such, I am fine with your proposed patch too. Regards santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] ARM: omap: allow building omap44xx without SMP
On Thu, Aug 23, 2012 at 5:42 PM, Arnd Bergmann a...@arndb.de wrote: On Thursday 23 August 2012, Shilimkar, Santosh wrote: I see your point but alternate patch is pushing down the fix to the low level driver and that means you end up patching more drivers when they use COUPLE idle infrastructure. That was the only reason I was thinking of suppressing the error at the source. Since it is just for the random builds and actually doesn't impact the real functionality as such, I am fine with your proposed patch too. Ok. It would be nice of course to test if this actually works on uniprocessor configurations. Have tested the patch and it does boot with UP build on OMAP. Acked-tested-by: Santosh Shilimkar santosh.shilim...@ti.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] ARM: omap: allow building omap44xx without SMP
On Wed, Aug 22, 2012 at 8:43 PM, Arnd Bergmann wrote:
> The new omap4 cpuidle implementation currently requires
> ARCH_NEEDS_CPU_IDLE_COUPLED, which only works on SMP.
>
> This patch makes it possible to build a non-SMP kernel
> for that platform. This is not normally desired for
> end-users but can be useful for testing.
>
> Without this patch, building rand-0y2jSKT results in:
>
> drivers/cpuidle/coupled.c: In function 'cpuidle_coupled_poke':
> drivers/cpuidle/coupled.c:317:3: error: implicit declaration of function
> '__smp_call_function_single' [-Werror=implicit-function-declaration]
>
> It's not clear if this patch is the best solution for
> the problem at hand. I have made sure that we can now
> build the kernel in all configurations, but that does
> not mean it will actually work on an OMAP44xx.
>
Am not sure either about the subject patch.
> Signed-off-by: Arnd Bergmann
> Cc: Santosh Shilimkar
> Cc: Kevin Hilman
> Cc: Tony Lindgren
> ---
> arch/arm/mach-omap2/Kconfig |2 +-
> arch/arm/mach-omap2/cpuidle44xx.c |3 ++-
> include/linux/cpuidle.h |4
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index dd2db02..66a8be3 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -62,7 +62,7 @@ config ARCH_OMAP4
> select PM_OPP if PM
> select USB_ARCH_HAS_EHCI if USB_SUPPORT
> select ARM_CPU_SUSPEND if PM
> - select ARCH_NEEDS_CPU_IDLE_COUPLED
> + select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP
>
> config SOC_OMAP5
> bool "TI OMAP5"
> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
> b/arch/arm/mach-omap2/cpuidle44xx.c
> index ee05e19..288bee6 100644
> --- a/arch/arm/mach-omap2/cpuidle44xx.c
> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
> @@ -238,8 +238,9 @@ int __init omap4_idle_init(void)
> for_each_cpu(cpu_id, cpu_online_mask) {
> dev = _cpu(omap4_idle_dev, cpu_id);
> dev->cpu = cpu_id;
> +#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
> dev->coupled_cpus = *cpu_online_mask;
> -
> +#endif
Was just thinking whether we should just take care of it at
core cpuidle level itself. Will below be enough to kill the build
error what you mentioned in the change log ?
diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
index 2c9bf26..df34534 100644
--- a/drivers/cpuidle/coupled.c
+++ b/drivers/cpuidle/coupled.c
@@ -314,7 +314,9 @@ static void cpuidle_coupled_poke(int cpu)
struct call_single_data *csd = _cpu(cpuidle_coupled_poke_cb, cpu);
if (!cpumask_test_and_set_cpu(cpu, _coupled_poked_mask))
+#ifdef CONFIG_SMP
__smp_call_function_single(cpu, csd, 0);
+#endif
}
/**
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] ARM: omap: allow building omap44xx without SMP
On Wed, Aug 22, 2012 at 8:43 PM, Arnd Bergmann a...@arndb.de wrote:
The new omap4 cpuidle implementation currently requires
ARCH_NEEDS_CPU_IDLE_COUPLED, which only works on SMP.
This patch makes it possible to build a non-SMP kernel
for that platform. This is not normally desired for
end-users but can be useful for testing.
Without this patch, building rand-0y2jSKT results in:
drivers/cpuidle/coupled.c: In function 'cpuidle_coupled_poke':
drivers/cpuidle/coupled.c:317:3: error: implicit declaration of function
'__smp_call_function_single' [-Werror=implicit-function-declaration]
It's not clear if this patch is the best solution for
the problem at hand. I have made sure that we can now
build the kernel in all configurations, but that does
not mean it will actually work on an OMAP44xx.
Am not sure either about the subject patch.
Signed-off-by: Arnd Bergmann a...@arndb.de
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Kevin Hilman khil...@ti.com
Cc: Tony Lindgren t...@atomide.com
---
arch/arm/mach-omap2/Kconfig |2 +-
arch/arm/mach-omap2/cpuidle44xx.c |3 ++-
include/linux/cpuidle.h |4
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index dd2db02..66a8be3 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -62,7 +62,7 @@ config ARCH_OMAP4
select PM_OPP if PM
select USB_ARCH_HAS_EHCI if USB_SUPPORT
select ARM_CPU_SUSPEND if PM
- select ARCH_NEEDS_CPU_IDLE_COUPLED
+ select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP
config SOC_OMAP5
bool TI OMAP5
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
b/arch/arm/mach-omap2/cpuidle44xx.c
index ee05e19..288bee6 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -238,8 +238,9 @@ int __init omap4_idle_init(void)
for_each_cpu(cpu_id, cpu_online_mask) {
dev = per_cpu(omap4_idle_dev, cpu_id);
dev-cpu = cpu_id;
+#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
dev-coupled_cpus = *cpu_online_mask;
-
+#endif
Was just thinking whether we should just take care of it at
core cpuidle level itself. Will below be enough to kill the build
error what you mentioned in the change log ?
diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
index 2c9bf26..df34534 100644
--- a/drivers/cpuidle/coupled.c
+++ b/drivers/cpuidle/coupled.c
@@ -314,7 +314,9 @@ static void cpuidle_coupled_poke(int cpu)
struct call_single_data *csd = per_cpu(cpuidle_coupled_poke_cb, cpu);
if (!cpumask_test_and_set_cpu(cpu, cpuidle_coupled_poked_mask))
+#ifdef CONFIG_SMP
__smp_call_function_single(cpu, csd, 0);
+#endif
}
/**
Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/PATCH 10/13] serial: omap: stick to put_autosuspend
On Tue, Aug 21, 2012 at 4:27 PM, Felipe Balbi wrote: > On Tue, Aug 21, 2012 at 04:12:11PM +0530, Shilimkar, Santosh wrote: >> On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi wrote: >> > Everytime we're done using our TTY, we want >> > the pm timer to be reinitilized. By sticking >> > to pm_runtime_pm_autosuspend() we make sure >> > that this will always be the case. >> > >> > Signed-off-by: Felipe Balbi >> > --- >> > drivers/tty/serial/omap-serial.c | 33 ++--- >> > 1 file changed, 22 insertions(+), 11 deletions(-) >> > >> > diff --git a/drivers/tty/serial/omap-serial.c >> > b/drivers/tty/serial/omap-serial.c >> > index 6ea24c5..458d77c 100644 >> > --- a/drivers/tty/serial/omap-serial.c >> > +++ b/drivers/tty/serial/omap-serial.c >> > @@ -164,7 +164,8 @@ static void serial_omap_enable_ms(struct uart_port >> > *port) >> > pm_runtime_get_sync(up->dev); >> > up->ier |= UART_IER_MSI; >> > serial_out(up, UART_IER, up->ier); >> > - pm_runtime_put(up->dev); >> > + pm_runtime_mark_last_busy(up->dev); >> > + pm_runtime_put_autosuspend(up->dev); >> > } >> > >> Can you please expand the change-log a bit ? >> Didn't follow the time re-init part completely. > > It's really just a micro-optimization. The thing is: > > if I call pm_runtime_put(), I will not reinitialize the pm timer to > whatever timeout value I used. This means that pm_runtime_put() could > actually execute right away (if timer was about to expire when I called > pm_runtime_put()). While this wouldn't cause any issues, it's better to > reinitialize the timer and make sure if there's another > read/write/set_termios/whatever coming right after this, UART is still > powered up. > > I mean, it's really just trying to avoid context save & restore when > UART is still under heavy usage. > > Does it make sense ? It does. Would be good to add the above description in the change-log. Thanks for clarification. Regars Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/PATCH 00/13] OMAP UART patches
On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi wrote: > Hi guys, > > here's a series of cleanup patches to the OMAP serial > driver. A later series could be made re-implementing > DMA using the DMA Engine API. Note that for RX DMA > we could be using RX Timeout IRQ as a hint that we better > use PIO instead ;-) > > All patches were tested on my pandaboard, but I'd really > like to receive Tested-by on other platforms. > > After this goes in, I'll probably try to get UART wakeup > working again and only after that look at DMA. > > cheers > > Felipe Balbi (13): > serial: omap: define and use to_uart_omap_port() > serial: omap: always return IRQ_HANDLED > serial: omap: define helpers for pdata function pointers > serial: omap: don't access the platform_device > serial: omap: drop DMA support > serial: add OMAP-specific defines > serial: omap: simplify IRQ handling > serial: omap: refactor receive_chars() into rdi/rlsi handlers > serial: omap: move THRE check to transmit_chars() > serial: omap: stick to put_autosuspend > serial: omap: set dev->drvdata before enabling pm_runtime > serial: omap: drop unnecessary check from remove > serial: omap: make sure to suspend device before remove > Apart from that one question on last patch, rest of the clean-up is really good. Nice work. FWIW, Acked-by: Santosh Shilimkar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/PATCH 10/13] serial: omap: stick to put_autosuspend
On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi wrote: > Everytime we're done using our TTY, we want > the pm timer to be reinitilized. By sticking > to pm_runtime_pm_autosuspend() we make sure > that this will always be the case. > > Signed-off-by: Felipe Balbi > --- > drivers/tty/serial/omap-serial.c | 33 ++--- > 1 file changed, 22 insertions(+), 11 deletions(-) > > diff --git a/drivers/tty/serial/omap-serial.c > b/drivers/tty/serial/omap-serial.c > index 6ea24c5..458d77c 100644 > --- a/drivers/tty/serial/omap-serial.c > +++ b/drivers/tty/serial/omap-serial.c > @@ -164,7 +164,8 @@ static void serial_omap_enable_ms(struct uart_port *port) > pm_runtime_get_sync(up->dev); > up->ier |= UART_IER_MSI; > serial_out(up, UART_IER, up->ier); > - pm_runtime_put(up->dev); > + pm_runtime_mark_last_busy(up->dev); > + pm_runtime_put_autosuspend(up->dev); > } > Can you please expand the change-log a bit ? Didn't follow the time re-init part completely. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/PATCH 05/13] serial: omap: drop DMA support
On Tue, Aug 21, 2012 at 3:50 PM, Felipe Balbi wrote: > On Tue, Aug 21, 2012 at 03:14:19PM +0530, Shilimkar, Santosh wrote: >> On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi wrote: >> > The current support is known to be broken and >> > a later patch will come re-adding it using >> > dma engine API. >> > >> > Signed-off-by: Felipe Balbi >> > --- >> Thanks Felipe !! > > no problem. > >> One less driver now towards OMAP DMA >> engine conversion. > > indeed :-) I'll take a closer look into rx timeout IRQ, but it looks > like we can use it to kick dma only for "big" transfers... need to play > with it for a while first, though. > Yep. The RX path with DMA is bit of difficult part to manage for UART. >> FWIW, >> Acked-by: Santosh Shilimkar > > is this Ack for this patch only or the entire series ?? > Two more patches to review and then I will do it for full series on top of the cover-letter :-) Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/PATCH 05/13] serial: omap: drop DMA support
On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi wrote: > The current support is known to be broken and > a later patch will come re-adding it using > dma engine API. > > Signed-off-by: Felipe Balbi > --- Thanks Felipe !! One less driver now towards OMAP DMA engine conversion. FWIW, Acked-by: Santosh Shilimkar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/PATCH 05/13] serial: omap: drop DMA support
On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi ba...@ti.com wrote: The current support is known to be broken and a later patch will come re-adding it using dma engine API. Signed-off-by: Felipe Balbi ba...@ti.com --- Thanks Felipe !! One less driver now towards OMAP DMA engine conversion. FWIW, Acked-by: Santosh Shilimkar santosh.shilim...@ti.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/PATCH 05/13] serial: omap: drop DMA support
On Tue, Aug 21, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: On Tue, Aug 21, 2012 at 03:14:19PM +0530, Shilimkar, Santosh wrote: On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi ba...@ti.com wrote: The current support is known to be broken and a later patch will come re-adding it using dma engine API. Signed-off-by: Felipe Balbi ba...@ti.com --- Thanks Felipe !! no problem. One less driver now towards OMAP DMA engine conversion. indeed :-) I'll take a closer look into rx timeout IRQ, but it looks like we can use it to kick dma only for big transfers... need to play with it for a while first, though. Yep. The RX path with DMA is bit of difficult part to manage for UART. FWIW, Acked-by: Santosh Shilimkar santosh.shilim...@ti.com is this Ack for this patch only or the entire series ?? Two more patches to review and then I will do it for full series on top of the cover-letter :-) Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/PATCH 10/13] serial: omap: stick to put_autosuspend
On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi ba...@ti.com wrote: Everytime we're done using our TTY, we want the pm timer to be reinitilized. By sticking to pm_runtime_pm_autosuspend() we make sure that this will always be the case. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/tty/serial/omap-serial.c | 33 ++--- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c index 6ea24c5..458d77c 100644 --- a/drivers/tty/serial/omap-serial.c +++ b/drivers/tty/serial/omap-serial.c @@ -164,7 +164,8 @@ static void serial_omap_enable_ms(struct uart_port *port) pm_runtime_get_sync(up-dev); up-ier |= UART_IER_MSI; serial_out(up, UART_IER, up-ier); - pm_runtime_put(up-dev); + pm_runtime_mark_last_busy(up-dev); + pm_runtime_put_autosuspend(up-dev); } Can you please expand the change-log a bit ? Didn't follow the time re-init part completely. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/PATCH 00/13] OMAP UART patches
On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi ba...@ti.com wrote: Hi guys, here's a series of cleanup patches to the OMAP serial driver. A later series could be made re-implementing DMA using the DMA Engine API. Note that for RX DMA we could be using RX Timeout IRQ as a hint that we better use PIO instead ;-) All patches were tested on my pandaboard, but I'd really like to receive Tested-by on other platforms. After this goes in, I'll probably try to get UART wakeup working again and only after that look at DMA. cheers Felipe Balbi (13): serial: omap: define and use to_uart_omap_port() serial: omap: always return IRQ_HANDLED serial: omap: define helpers for pdata function pointers serial: omap: don't access the platform_device serial: omap: drop DMA support serial: add OMAP-specific defines serial: omap: simplify IRQ handling serial: omap: refactor receive_chars() into rdi/rlsi handlers serial: omap: move THRE check to transmit_chars() serial: omap: stick to put_autosuspend serial: omap: set dev-drvdata before enabling pm_runtime serial: omap: drop unnecessary check from remove serial: omap: make sure to suspend device before remove Apart from that one question on last patch, rest of the clean-up is really good. Nice work. FWIW, Acked-by: Santosh Shilimkar santosh.shilim...@ti.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/PATCH 10/13] serial: omap: stick to put_autosuspend
On Tue, Aug 21, 2012 at 4:27 PM, Felipe Balbi ba...@ti.com wrote: On Tue, Aug 21, 2012 at 04:12:11PM +0530, Shilimkar, Santosh wrote: On Tue, Aug 21, 2012 at 2:45 PM, Felipe Balbi ba...@ti.com wrote: Everytime we're done using our TTY, we want the pm timer to be reinitilized. By sticking to pm_runtime_pm_autosuspend() we make sure that this will always be the case. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/tty/serial/omap-serial.c | 33 ++--- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c index 6ea24c5..458d77c 100644 --- a/drivers/tty/serial/omap-serial.c +++ b/drivers/tty/serial/omap-serial.c @@ -164,7 +164,8 @@ static void serial_omap_enable_ms(struct uart_port *port) pm_runtime_get_sync(up-dev); up-ier |= UART_IER_MSI; serial_out(up, UART_IER, up-ier); - pm_runtime_put(up-dev); + pm_runtime_mark_last_busy(up-dev); + pm_runtime_put_autosuspend(up-dev); } Can you please expand the change-log a bit ? Didn't follow the time re-init part completely. It's really just a micro-optimization. The thing is: if I call pm_runtime_put(), I will not reinitialize the pm timer to whatever timeout value I used. This means that pm_runtime_put() could actually execute right away (if timer was about to expire when I called pm_runtime_put()). While this wouldn't cause any issues, it's better to reinitialize the timer and make sure if there's another read/write/set_termios/whatever coming right after this, UART is still powered up. I mean, it's really just trying to avoid context save restore when UART is still under heavy usage. Does it make sense ? It does. Would be good to add the above description in the change-log. Thanks for clarification. Regars Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 05/31] arm64: MMU initialisation
On Fri, Aug 17, 2012 at 3:45 PM, Catalin Marinas
wrote:
> On Fri, Aug 17, 2012 at 11:06:11AM +0100, Santosh Shilimkar wrote:
>> On Tuesday 14 August 2012 11:22 PM, Catalin Marinas wrote:
>> > This patch contains the initialisation of the memory blocks, MMU
>> > attributes and the memory map. Only five memory types are defined:
>> > Device nGnRnE (equivalent to Strongly Ordered), Device nGnRE (classic
>> > Device memory), Device GRE, Normal Non-cacheable and Normal Cacheable.
>> > Cache policies are supported via the memory attributes register
>> > (MAIR_EL1) and only affect the Normal Cacheable mappings.
>> >
>> > This patch also adds the SPARSEMEM_VMEMMAP initialisation.
>> >
>> > Signed-off-by: Will Deacon
>> > Signed-off-by: Catalin Marinas
>> > ---
>>
>> Whats the difference between Device nGnRE and Device GRE ?
>> Sorry, I haven't gone through the specs yet and hence the
>> question.
>
> G - gathering (multiple reads/writes into one)
> R - reordering (reads/writes)
> E - early acknowledgement (the write may have not hit the device before
> the instruction returns).
>
> The 'n' in front just negates the meaning.
>
> So the Device memory as we know it on ARMv7 is equivalent to nGnRE. The
> Strongly Ordered is nGnRnE. GRE is pretty much like Normal Non-cacheable
> memory but with Device mapping, so there are restrictions on unaligned
> accesses.
>
Thanks for explaining it so clearly.
>> > +#ifdef CONFIG_ZONE_DMA32
>> > + /* 4GB maximum for 32-bit only capable devices */
>> > + max_dma32 = min(max, MAX_DMA32_PFN);
>> > + zone_size[ZONE_DMA32] = max_dma32 - min;
>> > +#endif
>>
>> Do you see need of supporting DMA32 on arm64 SOCs ?
>
> I've got some questions from partners but those devices may just be
> hidden behind an iommu. For now I left it in.
>
ok.
>> > +static struct cachepolicy cache_policies[] __initdata = {
>> > + {
>> > + .policy = "uncached",
>> > + .mair = 0x44, /* inner, outer
>> > non-cacheable */
>> > + .tcr= TCR_IRGN_NC | TCR_ORGN_NC,
>> > + }, {
>> > + .policy = "writethrough",
>> > + .mair = 0xaa, /* inner, outer
>> > write-through, read-allocate */
>> > + .tcr= TCR_IRGN_WT | TCR_ORGN_WT,
>>
>> Is WT supported on arm64?
>> On the recent ARMv7 processors, I think WT wasn't supported.
>
> All of WB, WA, WT are just architectural hints. A CPU implementation may
> or may not ignore them but with Linux we try to follow the architecture
> rather than specific implementations.
>
Agree.
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 09/31] arm64: Cache maintenance routines
On Fri, Aug 17, 2012 at 3:37 PM, Catalin Marinas wrote: > On Fri, Aug 17, 2012 at 10:57:20AM +0100, Santosh Shilimkar wrote: >> On Tuesday 14 August 2012 11:22 PM, Catalin Marinas wrote: >> > +ENTRY(__cpuc_flush_dcache_all) >> >> We have discussed the need of cache maintenance by >> level kind of API for ARMv7 (A15). >> >> Shouldn't we add such API for arm64 as well ? > > Yes, at some point we'll probably add but we'll discuss it again when > actually needed. I wouldn't define new API now that's not used by any > AArch64 code. > The patches are already on the list for ARMv7 and just waiting to merge two approaches. I agree once it is getting merged for ARMv7 and ARMv8 port can be updated. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 02/31] arm64: Kernel booting and initialisation
On Fri, Aug 17, 2012 at 3:35 PM, Catalin Marinas
wrote:
>
> On Fri, Aug 17, 2012 at 10:41:10AM +0100, Santosh Shilimkar wrote:
> > On Tuesday 14 August 2012 11:22 PM, Catalin Marinas wrote:
> > > +The boot loader is expected to enter the kernel on each CPU in the
> > > +following manner:
> > > +
> > > +- The primary CPU must jump directly to the first instruction of the
> > > + kernel image. The device tree blob passed by this CPU must contain
> > > + for each CPU node:
> > > +
> > > +1. An 'enable-method' property. Currently, the only supported
> > > value
> > > + for this field is the string "spin-table".
> > > +
> > > +2. A 'cpu-release-addr' property identifying a 64-bit,
> > > + zero-initialised memory location.
> > > +
> > > + It is expected that the bootloader will generate these device tree
> > > + properties and insert them into the blob prior to kernel entry.
> > > +
> > > +- Any secondary CPUs must spin outside of the kernel in a reserved
> > > area
> > > + of memory (communicated to the kernel by a /memreserve/ region in
> > > the
> > > + device tree) polling their cpu-release-addr location, which must be
> > > + contained in the reserved region. A wfe instruction may be
> > > inserted
> > > + to reduce the overhead of the busy-loop and a sev will be issued by
> > > + the primary CPU. When a read of the location pointed to by the
> > > + cpu-release-addr returns a non-zero value, the CPU must jump
> > > directly
> > > + to this value.
> >
> > So you expect all the secondary CPUs to be in wakeup state and probably
> > looping in WFE for a signal from kernel to boot. There is one issue
> > with this requirement though. For large CPU system, you need to reset
> > all the CPUs and hit this waiting loop. This will lead to large inrush
> > current need at bootup which may be not be supported. To avoid this
> > issue, secondary CPUs are kept in OFF state and then they are woken
> > up from kernel one by one whenever they need to be brought into the
> > system. This requirement should be considered.
>
> I agree, this part will be extended. That's one method that we currently
> support and suitable to the model.
>
> The better method is the SMC standardisation that Charles Garcia-Tobin
> has written (to be made available soon) and was presented at the last
> Linaro Connect in HK. Given that the CPU power is usually controlled by
> the secure side, we'll ask for an SMC to be issued for waking up
> secondary CPUs, so it's up to the secure firmware to write the correct
> hardware registers.
>
Thanks for the information. SMC standardization would indeed help
to overcome some of these. Will wait for that information before
next set of questions.
> > > --- /dev/null
> > > +++ b/arch/arm64/kernel/head.S
> > [..]
> > > + /*
> > > +* DO NOT MODIFY. Image header expected by Linux boot-loaders.
> > > +*/
> > > + b stext // branch to kernel start,
> > > magic
> > > + .long 0 // reserved
> > > + .quad TEXT_OFFSET // Image load offset from
> > > start of RAM
> > > + .quad 0 // reserved
> > > + .quad 0 // reserved
> > > +
> >
> > Minor nit. Avoid C++ commenting style "//" here and rest of the patch.
>
> That's not C++ comment style, it's the *official* assembly comment style
> for AArch64 ('@' is no longer supported).
>
Ok. Thanks for clarifying.
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 02/31] arm64: Kernel booting and initialisation
On Fri, Aug 17, 2012 at 3:35 PM, Catalin Marinas
catalin.mari...@arm.com wrote:
On Fri, Aug 17, 2012 at 10:41:10AM +0100, Santosh Shilimkar wrote:
On Tuesday 14 August 2012 11:22 PM, Catalin Marinas wrote:
+The boot loader is expected to enter the kernel on each CPU in the
+following manner:
+
+- The primary CPU must jump directly to the first instruction of the
+ kernel image. The device tree blob passed by this CPU must contain
+ for each CPU node:
+
+1. An 'enable-method' property. Currently, the only supported
value
+ for this field is the string spin-table.
+
+2. A 'cpu-release-addr' property identifying a 64-bit,
+ zero-initialised memory location.
+
+ It is expected that the bootloader will generate these device tree
+ properties and insert them into the blob prior to kernel entry.
+
+- Any secondary CPUs must spin outside of the kernel in a reserved
area
+ of memory (communicated to the kernel by a /memreserve/ region in
the
+ device tree) polling their cpu-release-addr location, which must be
+ contained in the reserved region. A wfe instruction may be
inserted
+ to reduce the overhead of the busy-loop and a sev will be issued by
+ the primary CPU. When a read of the location pointed to by the
+ cpu-release-addr returns a non-zero value, the CPU must jump
directly
+ to this value.
So you expect all the secondary CPUs to be in wakeup state and probably
looping in WFE for a signal from kernel to boot. There is one issue
with this requirement though. For large CPU system, you need to reset
all the CPUs and hit this waiting loop. This will lead to large inrush
current need at bootup which may be not be supported. To avoid this
issue, secondary CPUs are kept in OFF state and then they are woken
up from kernel one by one whenever they need to be brought into the
system. This requirement should be considered.
I agree, this part will be extended. That's one method that we currently
support and suitable to the model.
The better method is the SMC standardisation that Charles Garcia-Tobin
has written (to be made available soon) and was presented at the last
Linaro Connect in HK. Given that the CPU power is usually controlled by
the secure side, we'll ask for an SMC to be issued for waking up
secondary CPUs, so it's up to the secure firmware to write the correct
hardware registers.
Thanks for the information. SMC standardization would indeed help
to overcome some of these. Will wait for that information before
next set of questions.
--- /dev/null
+++ b/arch/arm64/kernel/head.S
[..]
+ /*
+* DO NOT MODIFY. Image header expected by Linux boot-loaders.
+*/
+ b stext // branch to kernel start,
magic
+ .long 0 // reserved
+ .quad TEXT_OFFSET // Image load offset from
start of RAM
+ .quad 0 // reserved
+ .quad 0 // reserved
+
Minor nit. Avoid C++ commenting style // here and rest of the patch.
That's not C++ comment style, it's the *official* assembly comment style
for AArch64 ('@' is no longer supported).
Ok. Thanks for clarifying.
Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 09/31] arm64: Cache maintenance routines
On Fri, Aug 17, 2012 at 3:37 PM, Catalin Marinas catalin.mari...@arm.com wrote: On Fri, Aug 17, 2012 at 10:57:20AM +0100, Santosh Shilimkar wrote: On Tuesday 14 August 2012 11:22 PM, Catalin Marinas wrote: +ENTRY(__cpuc_flush_dcache_all) We have discussed the need of cache maintenance by level kind of API for ARMv7 (A15). Shouldn't we add such API for arm64 as well ? Yes, at some point we'll probably add but we'll discuss it again when actually needed. I wouldn't define new API now that's not used by any AArch64 code. The patches are already on the list for ARMv7 and just waiting to merge two approaches. I agree once it is getting merged for ARMv7 and ARMv8 port can be updated. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 05/31] arm64: MMU initialisation
On Fri, Aug 17, 2012 at 3:45 PM, Catalin Marinas
catalin.mari...@arm.com wrote:
On Fri, Aug 17, 2012 at 11:06:11AM +0100, Santosh Shilimkar wrote:
On Tuesday 14 August 2012 11:22 PM, Catalin Marinas wrote:
This patch contains the initialisation of the memory blocks, MMU
attributes and the memory map. Only five memory types are defined:
Device nGnRnE (equivalent to Strongly Ordered), Device nGnRE (classic
Device memory), Device GRE, Normal Non-cacheable and Normal Cacheable.
Cache policies are supported via the memory attributes register
(MAIR_EL1) and only affect the Normal Cacheable mappings.
This patch also adds the SPARSEMEM_VMEMMAP initialisation.
Signed-off-by: Will Deaconwill.dea...@arm.com
Signed-off-by: Catalin Marinascatalin.mari...@arm.com
---
Whats the difference between Device nGnRE and Device GRE ?
Sorry, I haven't gone through the specs yet and hence the
question.
G - gathering (multiple reads/writes into one)
R - reordering (reads/writes)
E - early acknowledgement (the write may have not hit the device before
the instruction returns).
The 'n' in front just negates the meaning.
So the Device memory as we know it on ARMv7 is equivalent to nGnRE. The
Strongly Ordered is nGnRnE. GRE is pretty much like Normal Non-cacheable
memory but with Device mapping, so there are restrictions on unaligned
accesses.
Thanks for explaining it so clearly.
+#ifdef CONFIG_ZONE_DMA32
+ /* 4GB maximum for 32-bit only capable devices */
+ max_dma32 = min(max, MAX_DMA32_PFN);
+ zone_size[ZONE_DMA32] = max_dma32 - min;
+#endif
Do you see need of supporting DMA32 on arm64 SOCs ?
I've got some questions from partners but those devices may just be
hidden behind an iommu. For now I left it in.
ok.
+static struct cachepolicy cache_policies[] __initdata = {
+ {
+ .policy = uncached,
+ .mair = 0x44, /* inner, outer
non-cacheable */
+ .tcr= TCR_IRGN_NC | TCR_ORGN_NC,
+ }, {
+ .policy = writethrough,
+ .mair = 0xaa, /* inner, outer
write-through, read-allocate */
+ .tcr= TCR_IRGN_WT | TCR_ORGN_WT,
Is WT supported on arm64?
On the recent ARMv7 processors, I think WT wasn't supported.
All of WB, WA, WT are just architectural hints. A CPU implementation may
or may not ignore them but with Linux we try to follow the architecture
rather than specific implementations.
Agree.
Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [discussion]sched: a rough proposal to enable power saving in scheduler
On Thu, Aug 16, 2012 at 12:23 PM, preeti wrote: > > Hi everyone, > > From what I have understood so far,I try to summarise pin pointed > differences between the performance and power policies as found > relevant to the scheduler-load balancing mechanism.Any thoughts? > > *Performance policy*: > > Q1.Who triggers load_balance? > Load balance is triggered when a cpu is found to be idle.(Pull mechanism) > > Q2.How is load_balance handled? > When triggered,the load is looked to be pulled from its sched domain. > First the sched groups in the domain the cpu belongs to is queried > followed by the runqueues in the busiest group.then the tasks are moved. > > This course of action is found analogous to the performance policy because: > > 1.First the idle cpu initiates the pull action > 2.The busiest cpu hands over the load to this cpu.A person who can > handle any work is querying as to who cannot handle more work. > > *Power policy*: > > So how is power policy different? As Peter says,'pack more than spread > more'. > > Q1.Who triggers load balance? > It is the cpu which cannot handle more work.Idle cpu is left to remain > idle.(Push mechanism) > > Q2.How is load_balance handled? > First the least busy runqueue,from within the sched_group that the busy > cpu belongs to is queried.if none exist,ie all the runqueues are equally > busy then move on to the other sched groups. > > Here again the 'least busy' policy should be applied,first at > group level then at the runqueue level. > > This course of action is found analogous to the power policy because as > much as possible busy and capable cpus within a small range try to > handle the existing load. > Not to complicate the power policy scheme but always *packing* may not be the best approach for all CPU packages. As mentioned, packing ensures that least number of power domains are in use and effectively reduce the active power consumption on paper but there are few considerations which might conflict with this assumption. -- Many architectures get best power saving when the entire CPU cluster or SD is idle. Intel folks already mentioned this and also extended this concept for attached memory with the CPU domain from self refresh point of view. This is true for the CPUs who have very little active leakage and hence "race to idle" would be better so that cluster can hit the deeper C-state to save more power. -- Spreading vs Packing actually can be made OPP(CPU operating point) dependent. Some of the mobile workload and power numbers measured in the past shown that when CPU operating at lower OPP(considering the load is less), packing would be the best option to have higher opportunity for cluster to idle where as while operating at higher operating point(assuming the higher CPU load and possibly more threads), a spread with race to idle in mind might be beneficial. Of-course this is going to be bit messy since the CPUFreq and scheduler needs to be linked. -- May be this is already possible but for architectures like big.LITTLE, the power consumption and active leakage can be significantly different across big and little CPU packages. Meaning the big CPU cluster or SD might be more power efficient with packing where as Little CPU cluster would be power efficient with spreading. Hence the possible need of per SD configurability. Ofcourse all of this can be done step by step starting with most simple power policy as stated by Peter. Regards Santosh been used whenever possible in sd. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [discussion]sched: a rough proposal to enable power saving in scheduler
On Thu, Aug 16, 2012 at 12:23 PM, preeti pre...@linux.vnet.ibm.com wrote: Hi everyone, From what I have understood so far,I try to summarise pin pointed differences between the performance and power policies as found relevant to the scheduler-load balancing mechanism.Any thoughts? *Performance policy*: Q1.Who triggers load_balance? Load balance is triggered when a cpu is found to be idle.(Pull mechanism) Q2.How is load_balance handled? When triggered,the load is looked to be pulled from its sched domain. First the sched groups in the domain the cpu belongs to is queried followed by the runqueues in the busiest group.then the tasks are moved. This course of action is found analogous to the performance policy because: 1.First the idle cpu initiates the pull action 2.The busiest cpu hands over the load to this cpu.A person who can handle any work is querying as to who cannot handle more work. *Power policy*: So how is power policy different? As Peter says,'pack more than spread more'. Q1.Who triggers load balance? It is the cpu which cannot handle more work.Idle cpu is left to remain idle.(Push mechanism) Q2.How is load_balance handled? First the least busy runqueue,from within the sched_group that the busy cpu belongs to is queried.if none exist,ie all the runqueues are equally busy then move on to the other sched groups. Here again the 'least busy' policy should be applied,first at group level then at the runqueue level. This course of action is found analogous to the power policy because as much as possible busy and capable cpus within a small range try to handle the existing load. Not to complicate the power policy scheme but always *packing* may not be the best approach for all CPU packages. As mentioned, packing ensures that least number of power domains are in use and effectively reduce the active power consumption on paper but there are few considerations which might conflict with this assumption. -- Many architectures get best power saving when the entire CPU cluster or SD is idle. Intel folks already mentioned this and also extended this concept for attached memory with the CPU domain from self refresh point of view. This is true for the CPUs who have very little active leakage and hence race to idle would be better so that cluster can hit the deeper C-state to save more power. -- Spreading vs Packing actually can be made OPP(CPU operating point) dependent. Some of the mobile workload and power numbers measured in the past shown that when CPU operating at lower OPP(considering the load is less), packing would be the best option to have higher opportunity for cluster to idle where as while operating at higher operating point(assuming the higher CPU load and possibly more threads), a spread with race to idle in mind might be beneficial. Of-course this is going to be bit messy since the CPUFreq and scheduler needs to be linked. -- May be this is already possible but for architectures like big.LITTLE, the power consumption and active leakage can be significantly different across big and little CPU packages. Meaning the big CPU cluster or SD might be more power efficient with packing where as Little CPU cluster would be power efficient with spreading. Hence the possible need of per SD configurability. Ofcourse all of this can be done step by step starting with most simple power policy as stated by Peter. Regards Santosh been used whenever possible in sd. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 0/4] Add device tree data for omap5
Sourav, On Mon, Aug 13, 2012 at 3:35 PM, Sourav Poddar wrote: > > The following patch series add i2c support for omap5. > As well as enable I2C based devices like pressure and temperature > through device tree. Also add onchip keypad dts data. > > Cc: Benoit Cousson > Cc: Felipe Balbi > Cc: Santosh Shilimkar > > Sourav Poddar (4): > arm/dts: omap5-evm: Add I2C support > arm/dts: omap5-evm: Add tmp102 sensor support > arm/dts: omap5-evm: Add keypad data > arm/dts: omap5-evm: Add bmp085 sensor support > > arch/arm/boot/dts/omap5-evm.dts | 30 + > arch/arm/boot/dts/omap5.dtsi| 40 > +++ > 2 files changed, 70 insertions(+), 0 deletions(-) > The entire series looks fine to me. Acked-by: Santosh Shilimkar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 0/4] Add device tree data for omap5
Sourav, On Mon, Aug 13, 2012 at 3:35 PM, Sourav Poddar sourav.pod...@ti.com wrote: The following patch series add i2c support for omap5. As well as enable I2C based devices like pressure and temperature through device tree. Also add onchip keypad dts data. Cc: Benoit Cousson b-cous...@ti.com Cc: Felipe Balbi ba...@ti.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Sourav Poddar (4): arm/dts: omap5-evm: Add I2C support arm/dts: omap5-evm: Add tmp102 sensor support arm/dts: omap5-evm: Add keypad data arm/dts: omap5-evm: Add bmp085 sensor support arch/arm/boot/dts/omap5-evm.dts | 30 + arch/arm/boot/dts/omap5.dtsi| 40 +++ 2 files changed, 70 insertions(+), 0 deletions(-) The entire series looks fine to me. Acked-by: Santosh Shilimkar santosh.shilim...@ti.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] gpio/omap: add *remove* callback in platform_driver
On Fri, Aug 10, 2012 at 11:38 AM, DebBarma, Tarun Kanti wrote: > > On Thu, Aug 9, 2012 at 8:28 PM, Kevin Hilman wrote: > > "DebBarma, Tarun Kanti" writes: > > > >> On Wed, Aug 8, 2012 at 10:40 PM, Kevin Hilman wrote: > >>> Tarun Kanti DebBarma writes: > >>> > Add *remove* callback so that necessary cleanup operations are > performed when device is unregistered. The device is deleted > from the list and associated clock handle is released by > calling clk_put() and irq descriptor is released using the > irq_free_desc() api. > > Signed-off-by: Tarun Kanti DebBarma > Reported-by: Paul Walmsley > Reviewed-by: Jon Hunter > Cc: Kevin Hilman > Cc: Rajendra Nayak > Cc: Santosh Shilimkar > Cc: Cousson, Benoit > Cc: Paul Walmsley > --- > v2: > Baseline: > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git > Commit: 0d7614f09c1ebdbaa1599a5aba7593f147bf96ee (Linux 3.6-rc1) > > (1) Use irq_free_descs() instead of irq_free_desc(). > Besides, irq_free_desc() was using wrong parameter, > irq_base, instead of bank->irq. > (2) irq_free_descs() moved outside spin_lock/unlock_*() > in order to avoid exception warnings. > > (3) pm_runtime_disable() added so that bind can happen successfully > > Test Detail: > Step 1: Unbind gpio.5 device in order to invoke the *remove* > callback. > #echo "omap_gpio.5" > sys/bus/platform/drivers/omap_gpio/unbind > > Step 2: Bind gpio.5 device and confirm probe() for the device > succeeds. > #echo "omap_gpio.5" > sys/bus/platform/drivers/omap_gpio/bind > > Step 3: Execute read/write GPIO test case. > >>> > >>> What happens when GPIOs are in use (requested)? > >> If I try to unbind a currently active GPIO bank then I see an exception > >> after the irq descriptor is freed by the remove. I believe this is > >> expected > >> because interrupt raised by the system would not be handled. > > > > ... and the GPIO is still configured to trigger interrupts. > Right! > > > > > The point is that there is lots to cleanup/unconfigure properly for this > > to work properly. > As far as I can think of, all active gpio requests done either in > board files or through DT > have to be freed. But if this is done then when we bind() the device > again initialization > done in omap_gpio_probe() would not restore the board/DT related > configuration. > So the point is are we supposed to do all these cleanup in *remove* > callback? > If yes, how do we manage board level gpio usage? More and more I think of the .remove() for GPIO, the interface not seems to make sense. Being an infrastructure driver which can be used by many client drivers like Ethernet, MMC card detect, sensors etc etc. And hence it can not be made a loadable module. So I am in favor of dropping the $subject patch completely unless and until we need it genuinely. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] gpio/omap: add *remove* callback in platform_driver
On Fri, Aug 10, 2012 at 11:38 AM, DebBarma, Tarun Kanti tarun.ka...@ti.com wrote: On Thu, Aug 9, 2012 at 8:28 PM, Kevin Hilman khil...@ti.com wrote: DebBarma, Tarun Kanti tarun.ka...@ti.com writes: On Wed, Aug 8, 2012 at 10:40 PM, Kevin Hilman khil...@ti.com wrote: Tarun Kanti DebBarma tarun.ka...@ti.com writes: Add *remove* callback so that necessary cleanup operations are performed when device is unregistered. The device is deleted from the list and associated clock handle is released by calling clk_put() and irq descriptor is released using the irq_free_desc() api. Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com Reported-by: Paul Walmsley p...@pwsan.com Reviewed-by: Jon Hunter jon-hun...@ti.com Cc: Kevin Hilman khil...@ti.com Cc: Rajendra Nayak rna...@ti.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Cousson, Benoit b-cous...@ti.com Cc: Paul Walmsley p...@pwsan.com --- v2: Baseline: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Commit: 0d7614f09c1ebdbaa1599a5aba7593f147bf96ee (Linux 3.6-rc1) (1) Use irq_free_descs() instead of irq_free_desc(). Besides, irq_free_desc() was using wrong parameter, irq_base, instead of bank-irq. (2) irq_free_descs() moved outside spin_lock/unlock_*() in order to avoid exception warnings. (3) pm_runtime_disable() added so that bind can happen successfully Test Detail: Step 1: Unbind gpio.5 device in order to invoke the *remove* callback. #echo omap_gpio.5 sys/bus/platform/drivers/omap_gpio/unbind Step 2: Bind gpio.5 device and confirm probe() for the device succeeds. #echo omap_gpio.5 sys/bus/platform/drivers/omap_gpio/bind Step 3: Execute read/write GPIO test case. What happens when GPIOs are in use (requested)? If I try to unbind a currently active GPIO bank then I see an exception after the irq descriptor is freed by the remove. I believe this is expected because interrupt raised by the system would not be handled. ... and the GPIO is still configured to trigger interrupts. Right! The point is that there is lots to cleanup/unconfigure properly for this to work properly. As far as I can think of, all active gpio requests done either in board files or through DT have to be freed. But if this is done then when we bind() the device again initialization done in omap_gpio_probe() would not restore the board/DT related configuration. So the point is are we supposed to do all these cleanup in *remove* callback? If yes, how do we manage board level gpio usage? More and more I think of the .remove() for GPIO, the interface not seems to make sense. Being an infrastructure driver which can be used by many client drivers like Ethernet, MMC card detect, sensors etc etc. And hence it can not be made a loadable module. So I am in favor of dropping the $subject patch completely unless and until we need it genuinely. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] gpio/omap: add *remove* callback in platform_driver
On Wed, Aug 8, 2012 at 7:28 PM, Tarun Kanti DebBarma wrote: > Add *remove* callback so that necessary cleanup operations are > performed when device is unregistered. The device is deleted > from the list and associated clock handle is released by > calling clk_put() and irq descriptor is released using the > irq_free_desc() api. > > Signed-off-by: Tarun Kanti DebBarma > Reported-by: Paul Walmsley > Reviewed-by: Jon Hunter > Cc: Kevin Hilman > Cc: Rajendra Nayak > Cc: Santosh Shilimkar > Cc: Cousson, Benoit > Cc: Paul Walmsley > --- > v2: > Baseline: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git > Commit: 0d7614f09c1ebdbaa1599a5aba7593f147bf96ee (Linux 3.6-rc1) > > (1) Use irq_free_descs() instead of irq_free_desc(). > Besides, irq_free_desc() was using wrong parameter, > irq_base, instead of bank->irq. > (2) irq_free_descs() moved outside spin_lock/unlock_*() > in order to avoid exception warnings. > > (3) pm_runtime_disable() added so that bind can happen successfully > > Test Detail: > Step 1: Unbind gpio.5 device in order to invoke the *remove* callback. > #echo "omap_gpio.5" > sys/bus/platform/drivers/omap_gpio/unbind > > Step 2: Bind gpio.5 device and confirm probe() for the device succeeds. > #echo "omap_gpio.5" > sys/bus/platform/drivers/omap_gpio/bind > > Step 3: Execute read/write GPIO test case. > Thanks details about test. Whe you to "Unbind->bind", do you see that PM is not broken. In other words, can you also test and ensure that the OMAP3 suspend and CPUIDLE are not broken because of this patch. PER and CORE domains should transition to low power states. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] gpio/omap: add *remove* callback in platform_driver
On Wed, Aug 8, 2012 at 7:28 PM, Tarun Kanti DebBarma tarun.ka...@ti.com wrote: Add *remove* callback so that necessary cleanup operations are performed when device is unregistered. The device is deleted from the list and associated clock handle is released by calling clk_put() and irq descriptor is released using the irq_free_desc() api. Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com Reported-by: Paul Walmsley p...@pwsan.com Reviewed-by: Jon Hunter jon-hun...@ti.com Cc: Kevin Hilman khil...@ti.com Cc: Rajendra Nayak rna...@ti.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Cousson, Benoit b-cous...@ti.com Cc: Paul Walmsley p...@pwsan.com --- v2: Baseline: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Commit: 0d7614f09c1ebdbaa1599a5aba7593f147bf96ee (Linux 3.6-rc1) (1) Use irq_free_descs() instead of irq_free_desc(). Besides, irq_free_desc() was using wrong parameter, irq_base, instead of bank-irq. (2) irq_free_descs() moved outside spin_lock/unlock_*() in order to avoid exception warnings. (3) pm_runtime_disable() added so that bind can happen successfully Test Detail: Step 1: Unbind gpio.5 device in order to invoke the *remove* callback. #echo omap_gpio.5 sys/bus/platform/drivers/omap_gpio/unbind Step 2: Bind gpio.5 device and confirm probe() for the device succeeds. #echo omap_gpio.5 sys/bus/platform/drivers/omap_gpio/bind Step 3: Execute read/write GPIO test case. Thanks details about test. Whe you to Unbind-bind, do you see that PM is not broken. In other words, can you also test and ensure that the OMAP3 suspend and CPUIDLE are not broken because of this patch. PER and CORE domains should transition to low power states. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GPIO] Crashed when not using
On Tue, Jul 31, 2012 at 6:26 PM, Poddar, Sourav wrote:
> Hi Santosh,
>
> On Tue, Jul 31, 2012 at 6:02 PM, Shilimkar, Santosh
> wrote:
>> On Tue, Jul 31, 2012 at 8:52 AM, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Jul 31, 2012 at 10:23:16AM +0530, Poddar, Sourav wrote:
>>> > >>>> The device tree data for acquiring the above GPIO interrupt line
>>> > >>>> looks
>>> > >>>> like this.
>>> > >>>>
>>> > >>>> +++ linux-omap-storage/arch/arm/boot/dts/omap5-evm.dts 2012-07-30
>>> > >>>> 14:11:08.931694001 +0530
>>> > >>>> @@ -42,7 +42,8 @@
>>> > >>>> tsl2771@39 {
>>> > >>>> compatible = "taos,tsl2771";
>>> > >>>> reg = <0x39>;
>>> > >>>> +interrupt-parent = <>;
>>> > >>>> +interrupts = <21>; /* gpio line 149 */
>>> > >>>> };
>>> > >>>> };
>>> > >>>>
>>> > >>>> Note: using "gpio_request_one" in the driver solves the issue.
>>> > >>>> Is using this api in the driver required?
>>> > >>>> Any pointer on the above crash?
>>> > >>>
>>> > >> Hi Tarun,
>>> > >>> Any user/client driver of GPIO is supposed to go through
>>> > >>> gpio_request() API so that module clock
>>> > >>> is enabled correctly. Overriding of APIs would put the power
>>> > >>> management state machine in jeopardy.
>>> > >>> --
>>> > >> I tried putting "pm_runtime_get_sync" in gpio_irq_type api where the
>>> > >> kernel
>>> > >> is crashing and the crash is no longer observed. So indeed, its about
>>> > >> enabling clocks.
>>> > >>
>>> > >> One doubt: Can't we put runtime apis in "gpio_irq_type" and eliminate
>>> > >> the use of
>>> > >> "gpio_request_one"??
>>> > >
>>> > > No.
>>> > >
>>> > > You must use the GPIO requiest/free APIs to tell the GPIO core that
>>> > > the GPIO line is in use.
>>> > >
>>> > Thanks for this confirmation.
>>> > > Why do you want to avoid using gpio_request/gpio_free?
>>> > >
>>> > I was assuming that DT based gpio IRQ registration will automatically
>>> > take care of
>>> > the above APIs. But since that is not the case(as mentioned by
>>> > santosh), we need to use the
>>> > gpio_request/free apis.
>>>
>>> Hang on for a while, let's try to get to the bottom of this debate first
>>> ;-)
>>>
>>> We have a canonical way of passing IRQ numbers to drivers through DT and
>>> that is the "interrupts" attribute. It shouldn't matter if that IRQ pin
>>> is connected to a real IRQ line or through a GPIO controller. In both
>>> cases we should use the "interrupts" attribute.
>>>
>>> If DT core doesn't allocate the GPIO for us then how does this work:
>>>
>>> (omap4-sdp.dts)
>>>
>>> 127 {
>>> 128 eth@0 {
>>> 129 compatible = "ks8851";
>>> 130 spi-max-frequency = <2400>;
>>> 131 reg = <0>;
>>> 132 interrupt-parent = <>;
>>> 133 interrupts = <2>; /* gpio line 34 */
>>> 134 vdd-supply = <_eth>;
>>> 135 };
>>> 136 };
>>>
>>>
>>> There's no gpio request on the driver:
>>>
>>> $ git grep -e gpio_request drivers/net/ethernet/micrel/ks8851.c
>>> $
>>>
>>> Since Benoit was the one who added that to the dts file (commit
>>> e7c64db9), I assume he tested his patch before posting, so again I ask -
>>> How does that work and why doesn't this work for Sourav's tsl2771
>>> controller ?
>>>
>>> This is either a regression on drivers/of, or commit e7c64db9 is also
>>> broken...
>>>
>>> Benoit, do you know how should this work ?
>>>
>> I had a discussion with Benoit on this. In fact there is a way to actually
>> trigger the GPIO request.
>>
>>> 132 interrupt-parent = <>;
>>> 133 interrupts = <2>; /* gpio line 34 */
>>
>> As above you can see, GPIO2 bank and 2nd line.
>> And then it will make use of gpio_irq chip properties
>> to probe the GPIO line.
>>
>> Saurabh can try this out for his use case.
>>
> I am using the above properties only and seeing the issue.
>
There seeems to be an issue with the GPIO DT probing. The Ethernet
works because there is another GPIO line from BANK2 is probed and since
BANK2 shares the clock, you won't see any issue.
We are looking at fixing the issue. Will keep the thread posted.
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GPIO] Crashed when not using
On Tue, Jul 31, 2012 at 6:26 PM, Poddar, Sourav sourav.pod...@ti.com wrote:
Hi Santosh,
On Tue, Jul 31, 2012 at 6:02 PM, Shilimkar, Santosh
santosh.shilim...@ti.com wrote:
On Tue, Jul 31, 2012 at 8:52 AM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Tue, Jul 31, 2012 at 10:23:16AM +0530, Poddar, Sourav wrote:
The device tree data for acquiring the above GPIO interrupt line
looks
like this.
+++ linux-omap-storage/arch/arm/boot/dts/omap5-evm.dts 2012-07-30
14:11:08.931694001 +0530
@@ -42,7 +42,8 @@
tsl2771@39 {
compatible = taos,tsl2771;
reg = 0x39;
+interrupt-parent = gpio5;
+interrupts = 21; /* gpio line 149 */
};
};
Note: using gpio_request_one in the driver solves the issue.
Is using this api in the driver required?
Any pointer on the above crash?
Hi Tarun,
Any user/client driver of GPIO is supposed to go through
gpio_request() API so that module clock
is enabled correctly. Overriding of APIs would put the power
management state machine in jeopardy.
--
I tried putting pm_runtime_get_sync in gpio_irq_type api where the
kernel
is crashing and the crash is no longer observed. So indeed, its about
enabling clocks.
One doubt: Can't we put runtime apis in gpio_irq_type and eliminate
the use of
gpio_request_one??
No.
You must use the GPIO requiest/free APIs to tell the GPIO core that
the GPIO line is in use.
Thanks for this confirmation.
Why do you want to avoid using gpio_request/gpio_free?
I was assuming that DT based gpio IRQ registration will automatically
take care of
the above APIs. But since that is not the case(as mentioned by
santosh), we need to use the
gpio_request/free apis.
Hang on for a while, let's try to get to the bottom of this debate first
;-)
We have a canonical way of passing IRQ numbers to drivers through DT and
that is the interrupts attribute. It shouldn't matter if that IRQ pin
is connected to a real IRQ line or through a GPIO controller. In both
cases we should use the interrupts attribute.
If DT core doesn't allocate the GPIO for us then how does this work:
(omap4-sdp.dts)
127 mcspi1 {
128 eth@0 {
129 compatible = ks8851;
130 spi-max-frequency = 2400;
131 reg = 0;
132 interrupt-parent = gpio2;
133 interrupts = 2; /* gpio line 34 */
134 vdd-supply = vdd_eth;
135 };
136 };
There's no gpio request on the driver:
$ git grep -e gpio_request drivers/net/ethernet/micrel/ks8851.c
$
Since Benoit was the one who added that to the dts file (commit
e7c64db9), I assume he tested his patch before posting, so again I ask -
How does that work and why doesn't this work for Sourav's tsl2771
controller ?
This is either a regression on drivers/of, or commit e7c64db9 is also
broken...
Benoit, do you know how should this work ?
I had a discussion with Benoit on this. In fact there is a way to actually
trigger the GPIO request.
132 interrupt-parent = gpio2;
133 interrupts = 2; /* gpio line 34 */
As above you can see, GPIO2 bank and 2nd line.
And then it will make use of gpio_irq chip properties
to probe the GPIO line.
Saurabh can try this out for his use case.
I am using the above properties only and seeing the issue.
There seeems to be an issue with the GPIO DT probing. The Ethernet
works because there is another GPIO line from BANK2 is probed and since
BANK2 shares the clock, you won't see any issue.
We are looking at fixing the issue. Will keep the thread posted.
Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GPIO] Crashed when not using
On Tue, Jul 31, 2012 at 8:52 AM, Felipe Balbi wrote:
>
> Hi,
>
> On Tue, Jul 31, 2012 at 10:23:16AM +0530, Poddar, Sourav wrote:
> > The device tree data for acquiring the above GPIO interrupt line
> > looks
> > like this.
> >
> > +++ linux-omap-storage/arch/arm/boot/dts/omap5-evm.dts 2012-07-30
> > 14:11:08.931694001 +0530
> > @@ -42,7 +42,8 @@
> > tsl2771@39 {
> > compatible = "taos,tsl2771";
> > reg = <0x39>;
> > +interrupt-parent = <>;
> > +interrupts = <21>; /* gpio line 149 */
> > };
> > };
> >
> > Note: using "gpio_request_one" in the driver solves the issue.
> > Is using this api in the driver required?
> > Any pointer on the above crash?
> > >>>
> > >> Hi Tarun,
> > >>> Any user/client driver of GPIO is supposed to go through
> > >>> gpio_request() API so that module clock
> > >>> is enabled correctly. Overriding of APIs would put the power
> > >>> management state machine in jeopardy.
> > >>> --
> > >> I tried putting "pm_runtime_get_sync" in gpio_irq_type api where the
> > >> kernel
> > >> is crashing and the crash is no longer observed. So indeed, its about
> > >> enabling clocks.
> > >>
> > >> One doubt: Can't we put runtime apis in "gpio_irq_type" and eliminate
> > >> the use of
> > >> "gpio_request_one"??
> > >
> > > No.
> > >
> > > You must use the GPIO requiest/free APIs to tell the GPIO core that
> > > the GPIO line is in use.
> > >
> > Thanks for this confirmation.
> > > Why do you want to avoid using gpio_request/gpio_free?
> > >
> > I was assuming that DT based gpio IRQ registration will automatically
> > take care of
> > the above APIs. But since that is not the case(as mentioned by
> > santosh), we need to use the
> > gpio_request/free apis.
>
> Hang on for a while, let's try to get to the bottom of this debate first
> ;-)
>
> We have a canonical way of passing IRQ numbers to drivers through DT and
> that is the "interrupts" attribute. It shouldn't matter if that IRQ pin
> is connected to a real IRQ line or through a GPIO controller. In both
> cases we should use the "interrupts" attribute.
>
> If DT core doesn't allocate the GPIO for us then how does this work:
>
> (omap4-sdp.dts)
>
> 127 {
> 128 eth@0 {
> 129 compatible = "ks8851";
> 130 spi-max-frequency = <2400>;
> 131 reg = <0>;
> 132 interrupt-parent = <>;
> 133 interrupts = <2>; /* gpio line 34 */
> 134 vdd-supply = <_eth>;
> 135 };
> 136 };
>
>
> There's no gpio request on the driver:
>
> $ git grep -e gpio_request drivers/net/ethernet/micrel/ks8851.c
> $
>
> Since Benoit was the one who added that to the dts file (commit
> e7c64db9), I assume he tested his patch before posting, so again I ask -
> How does that work and why doesn't this work for Sourav's tsl2771
> controller ?
>
> This is either a regression on drivers/of, or commit e7c64db9 is also
> broken...
>
> Benoit, do you know how should this work ?
>
I had a discussion with Benoit on this. In fact there is a way to actually
trigger the GPIO request.
> 132 interrupt-parent = <>;
> 133 interrupts = <2>; /* gpio line 34 */
As above you can see, GPIO2 bank and 2nd line.
And then it will make use of gpio_irq chip properties
to probe the GPIO line.
Saurabh can try this out for his use case.
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GPIO] Crashed when not using
On Tue, Jul 31, 2012 at 8:52 AM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Tue, Jul 31, 2012 at 10:23:16AM +0530, Poddar, Sourav wrote:
The device tree data for acquiring the above GPIO interrupt line
looks
like this.
+++ linux-omap-storage/arch/arm/boot/dts/omap5-evm.dts 2012-07-30
14:11:08.931694001 +0530
@@ -42,7 +42,8 @@
tsl2771@39 {
compatible = taos,tsl2771;
reg = 0x39;
+interrupt-parent = gpio5;
+interrupts = 21; /* gpio line 149 */
};
};
Note: using gpio_request_one in the driver solves the issue.
Is using this api in the driver required?
Any pointer on the above crash?
Hi Tarun,
Any user/client driver of GPIO is supposed to go through
gpio_request() API so that module clock
is enabled correctly. Overriding of APIs would put the power
management state machine in jeopardy.
--
I tried putting pm_runtime_get_sync in gpio_irq_type api where the
kernel
is crashing and the crash is no longer observed. So indeed, its about
enabling clocks.
One doubt: Can't we put runtime apis in gpio_irq_type and eliminate
the use of
gpio_request_one??
No.
You must use the GPIO requiest/free APIs to tell the GPIO core that
the GPIO line is in use.
Thanks for this confirmation.
Why do you want to avoid using gpio_request/gpio_free?
I was assuming that DT based gpio IRQ registration will automatically
take care of
the above APIs. But since that is not the case(as mentioned by
santosh), we need to use the
gpio_request/free apis.
Hang on for a while, let's try to get to the bottom of this debate first
;-)
We have a canonical way of passing IRQ numbers to drivers through DT and
that is the interrupts attribute. It shouldn't matter if that IRQ pin
is connected to a real IRQ line or through a GPIO controller. In both
cases we should use the interrupts attribute.
If DT core doesn't allocate the GPIO for us then how does this work:
(omap4-sdp.dts)
127 mcspi1 {
128 eth@0 {
129 compatible = ks8851;
130 spi-max-frequency = 2400;
131 reg = 0;
132 interrupt-parent = gpio2;
133 interrupts = 2; /* gpio line 34 */
134 vdd-supply = vdd_eth;
135 };
136 };
There's no gpio request on the driver:
$ git grep -e gpio_request drivers/net/ethernet/micrel/ks8851.c
$
Since Benoit was the one who added that to the dts file (commit
e7c64db9), I assume he tested his patch before posting, so again I ask -
How does that work and why doesn't this work for Sourav's tsl2771
controller ?
This is either a regression on drivers/of, or commit e7c64db9 is also
broken...
Benoit, do you know how should this work ?
I had a discussion with Benoit on this. In fact there is a way to actually
trigger the GPIO request.
132 interrupt-parent = gpio2;
133 interrupts = 2; /* gpio line 34 */
As above you can see, GPIO2 bank and 2nd line.
And then it will make use of gpio_irq chip properties
to probe the GPIO line.
Saurabh can try this out for his use case.
Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GPIO] Crashed when not using
On Mon, Jul 30, 2012 at 10:36 PM, Kevin Hilman wrote: > > "Poddar, Sourav" writes: > > > On Mon, Jul 30, 2012 at 3:04 PM, DebBarma, Tarun Kanti > > wrote: > >> Sourav, > >> > >> On Mon, Jul 30, 2012 at 2:13 PM, Poddar, Sourav > >> wrote: > >>> Hi All, > >>> > >>> I tried using gpio as an interrupt line for my driver > >>> (drivers/staging/iio/light/tsl2x7x_core.c) for omap5. > >>> The interrupt line number was directly passed to the driver using > >>> device tree. But what I observed > >>> is the following crash.. > >>> > >>> > >>> [1.599273] mousedev: PS/2 mouse device common for all mice > >>> [1.607513] i2c /dev entries driver > >>> [1.613739] Driver for 1-wire Dallas network protocol. > >>> [1.622650] usbcore: registered new interface driver usbhid > >>> [1.628540] usbhid: USB HID core driver > >>> [1.633728] Unhandled fault: imprecise external abort (0x1406) at > >>> 0x > >>> [1.641113] Internal error: : 1406 [#1] SMP ARM > >>> [1.645874] Modules linked in: > >>> [1.649078] CPU: 0Not tainted (3.5.0-02045-g0b474d6-dirty > >>> #415) > >>> [1.655761] PC is at _set_gpio_triggering+0x44/0x264 > >>> [1.660980] LR is at gpio_irq_type+0xb8/0x160 > >>> [1.665527] pc : []lr : []psr: 6093 > >>> [1.665527] sp : dc851df0 ip : c07a8f00 fp : > >>> [1.677581] r10: 0081 r9 : r8 : dc8ffc10 > >>> [1.683074] r7 : 2093 r6 : r5 : 0001 r4 : > >>> fa05b000 > >>> [1.689910] r3 : dc8ffc10 r2 : 0002 r1 : 0002 r0 : > >>> 0140 > >>> [1.696746] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM > >>> Segment kernel > >>> [1.704528] Control: 10c53c7d Table: 8000406a DAC: 0017 > >>> [1.710540] Process swapper/0 (pid: 1, stack limit = 0xdc8502f8) > >>> [1.716857] Stack: (0xdc851df0 to 0xdc852000) > >>> [1.721405] 1de0: 0002 > >>> c0731280 dc8ffc6c 2093 > >>> [1.730010] 1e00: c02dc224 c0731280 dc8d4980 c07a8e50 > >>> 0141 0002 c00a37b8 > >>> [1.738586] 1e20: 2002 c0731280 dc8d4980 0141 c07312d4 > >>> c07312b4 4013 c00a3d7c > >>> [1.747161] 1e40: 0141 c06728c0 c0090578 c247f000 > >>> c00a2d5c c0412258 0141 > >>> [1.755737] 1e60: c0731280 c0537818 dc8d4980 c00a3fe4 c07bf6a0 > >>> 2002 c071bed8 c247f000 > >>> [1.764312] 1e80: dc9df000 0004 c07bf6a0 dc9df020 c247f3c0 > >>> c04ea344 c06728c0 c247f000 > >>> [1.772888] 1ea0: 09090578 c07bf6a0 dc9df004 dc9df000 > >>> c04ea194 0091 c071bed8 > >>> [1.781494] 1ec0: c06f89f4 c03e13c0 c03e1314 dc9df020 c0d1afd8 > >>> dc9df054 c07bf60c c03272ac > >>> [1.790069] 1ee0: dc9df020 c07bf60c dc9df054 0091 > >>> c03274c0 c07bf60c dc851f08 > >>> [1.798645] 1f00: c032742c c0325b10 dc8f26a8 dc9ef790 > >>> c07bf60c c07bcea8 c243d6c0 > >>> [1.807220] 1f20: c0326240 c0672958 c02bce08 c07bf5e4 > >>> c07bf60c c03e12a0 > >>> [1.815795] 1f40: 0091 c071bed8 c06f89f4 c0327ab0 c07bf5e4 > >>> 0007 c07c8c40 > >>> [1.824401] 1f60: 0091 c03e1738 dc85 0007 > >>> c0008648 c112c9f0 > >>> [1.832977] 1f80: c06a6d58 c06f89f4 0001 6013 c06694c8 > >>> 0006 0006 > >>> [1.841552] 1fa0: 6013 c0700a20 0007 c07c8c40 c06d020c > >>> 0091 c071bed8 c0700a28 > >>> [1.850128] 1fc0: c06d0380 0006 0006 c06d020c > >>> c06d028c > >>> [1.858703] 1fe0: c001548c 0013 > >>> c001548c 3f3f3f3f 3f3f3f3f > >>> [1.867309] [] (_set_gpio_triggering+0x44/0x264) from > >>> [] (gpio_irq_type+0x0/0x160) > >>> [1.877075] [] (gpio_irq_type+0x0/0x160) from > >>> [<2002>] (0x2002) > >>> [1.884552] Code: e3120008 11816006 01c66001 e7846000 (e593c0fc) > >>> [1.890960] [ cut here ] > >>> [1.895812] WARNING: at arch/arm/mach-omap2/omap_l3_noc.c:113 > >>> l3_interrupt_handler+0x184/0x1bc() > >>> [1.905029] L3 custom error: MASTER:MPU TARGET:L4 PER2 > >>> [1.910430] Modules linked in: > >>> [1.913635] [] (unwind_backtrace+0x0/0xf4) from > >>> [] (warn_slowpath_common+0x4c/0x64) > >>> [1.923492] [] (warn_slowpath_common+0x4c/0x64) from > >>> [] (warn_slowpath_fmt+0x30/0x40) > >>> [1.933563] [] (warn_slowpath_fmt+0x30/0x40) from > >>> [] (l3_interrupt_handler+0x184/0x1bc) > >>> [1.943786] [] (l3_interrupt_handler+0x184/0x1bc) from > >>> [] (handle_irq_event_percpu+0x64/0x24c) > >>> [1.954650] [] (handle_irq_event_percpu+0x64/0x24c) from > >>> [] (handle_irq_event+0x3c/0x5c) > >>> [1.964965] [] (handle_irq_event+0x3c/0x5c) from > >>> [] (handle_fasteoi_irq+0x98/0x13c) > >>> [1.974822] [] (handle_fasteoi_irq+0x98/0x13c) from > >>> [] (generic_handle_irq+0x34/0x44) > >>> [1.984863] [] (generic_handle_irq+0x34/0x44) from > >>> [] (handle_IRQ+0x4c/0xac)
Re: [GPIO] Crashed when not using
On Mon, Jul 30, 2012 at 10:36 PM, Kevin Hilman khil...@ti.com wrote:
Poddar, Sourav sourav.pod...@ti.com writes:
On Mon, Jul 30, 2012 at 3:04 PM, DebBarma, Tarun Kanti
tarun.ka...@ti.com wrote:
Sourav,
On Mon, Jul 30, 2012 at 2:13 PM, Poddar, Sourav sourav.pod...@ti.com
wrote:
Hi All,
I tried using gpio as an interrupt line for my driver
(drivers/staging/iio/light/tsl2x7x_core.c) for omap5.
The interrupt line number was directly passed to the driver using
device tree. But what I observed
is the following crash..
[1.599273] mousedev: PS/2 mouse device common for all mice
[1.607513] i2c /dev entries driver
[1.613739] Driver for 1-wire Dallas network protocol.
[1.622650] usbcore: registered new interface driver usbhid
[1.628540] usbhid: USB HID core driver
[1.633728] Unhandled fault: imprecise external abort (0x1406) at
0x
[1.641113] Internal error: : 1406 [#1] SMP ARM
[1.645874] Modules linked in:
[1.649078] CPU: 0Not tainted (3.5.0-02045-g0b474d6-dirty
#415)
[1.655761] PC is at _set_gpio_triggering+0x44/0x264
[1.660980] LR is at gpio_irq_type+0xb8/0x160
[1.665527] pc : [c02dbb68]lr : [c02dc2dc]psr: 6093
[1.665527] sp : dc851df0 ip : c07a8f00 fp :
[1.677581] r10: 0081 r9 : r8 : dc8ffc10
[1.683074] r7 : 2093 r6 : r5 : 0001 r4 :
fa05b000
[1.689910] r3 : dc8ffc10 r2 : 0002 r1 : 0002 r0 :
0140
[1.696746] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM
Segment kernel
[1.704528] Control: 10c53c7d Table: 8000406a DAC: 0017
[1.710540] Process swapper/0 (pid: 1, stack limit = 0xdc8502f8)
[1.716857] Stack: (0xdc851df0 to 0xdc852000)
[1.721405] 1de0: 0002
c0731280 dc8ffc6c 2093
[1.730010] 1e00: c02dc224 c0731280 dc8d4980 c07a8e50
0141 0002 c00a37b8
[1.738586] 1e20: 2002 c0731280 dc8d4980 0141 c07312d4
c07312b4 4013 c00a3d7c
[1.747161] 1e40: 0141 c06728c0 c0090578 c247f000
c00a2d5c c0412258 0141
[1.755737] 1e60: c0731280 c0537818 dc8d4980 c00a3fe4 c07bf6a0
2002 c071bed8 c247f000
[1.764312] 1e80: dc9df000 0004 c07bf6a0 dc9df020 c247f3c0
c04ea344 c06728c0 c247f000
[1.772888] 1ea0: 09090578 c07bf6a0 dc9df004 dc9df000
c04ea194 0091 c071bed8
[1.781494] 1ec0: c06f89f4 c03e13c0 c03e1314 dc9df020 c0d1afd8
dc9df054 c07bf60c c03272ac
[1.790069] 1ee0: dc9df020 c07bf60c dc9df054 0091
c03274c0 c07bf60c dc851f08
[1.798645] 1f00: c032742c c0325b10 dc8f26a8 dc9ef790
c07bf60c c07bcea8 c243d6c0
[1.807220] 1f20: c0326240 c0672958 c02bce08 c07bf5e4
c07bf60c c03e12a0
[1.815795] 1f40: 0091 c071bed8 c06f89f4 c0327ab0 c07bf5e4
0007 c07c8c40
[1.824401] 1f60: 0091 c03e1738 dc85 0007
c0008648 c112c9f0
[1.832977] 1f80: c06a6d58 c06f89f4 0001 6013 c06694c8
0006 0006
[1.841552] 1fa0: 6013 c0700a20 0007 c07c8c40 c06d020c
0091 c071bed8 c0700a28
[1.850128] 1fc0: c06d0380 0006 0006 c06d020c
c06d028c
[1.858703] 1fe0: c001548c 0013
c001548c 3f3f3f3f 3f3f3f3f
[1.867309] [c02dbb68] (_set_gpio_triggering+0x44/0x264) from
[c02dc224] (gpio_irq_type+0x0/0x160)
[1.877075] [c02dc224] (gpio_irq_type+0x0/0x160) from
[2002] (0x2002)
[1.884552] Code: e3120008 11816006 01c66001 e7846000 (e593c0fc)
[1.890960] [ cut here ]
[1.895812] WARNING: at arch/arm/mach-omap2/omap_l3_noc.c:113
l3_interrupt_handler+0x184/0x1bc()
[1.905029] L3 custom error: MASTER:MPU TARGET:L4 PER2
[1.910430] Modules linked in:
[1.913635] [c001bd5c] (unwind_backtrace+0x0/0xf4) from
[c00414c4] (warn_slowpath_common+0x4c/0x64)
[1.923492] [c00414c4] (warn_slowpath_common+0x4c/0x64) from
[c0041570] (warn_slowpath_fmt+0x30/0x40)
[1.933563] [c0041570] (warn_slowpath_fmt+0x30/0x40) from
[c0035de4] (l3_interrupt_handler+0x184/0x1bc)
[1.943786] [c0035de4] (l3_interrupt_handler+0x184/0x1bc) from
[c00a28b8] (handle_irq_event_percpu+0x64/0x24c)
[1.954650] [c00a28b8] (handle_irq_event_percpu+0x64/0x24c) from
[c00a2adc] (handle_irq_event+0x3c/0x5c)
[1.964965] [c00a2adc] (handle_irq_event+0x3c/0x5c) from
[c00a56a8] (handle_fasteoi_irq+0x98/0x13c)
[1.974822] [c00a56a8] (handle_fasteoi_irq+0x98/0x13c) from
[c00a235c] (generic_handle_irq+0x34/0x44)
[1.984863] [c00a235c] (generic_handle_irq+0x34/0x44) from
[c00153a0] (handle_IRQ+0x4c/0xac)
[1.994110] [c00153a0] (handle_IRQ+0x4c/0xac) from [c0008480]
(gic_handle_irq+0x2c/0x60)
[2.002960] [c0008480] (gic_handle_irq+0x2c/0x60) from
Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier
On Thu, Jul 26, 2012 at 9:25 AM, Shilimkar, Santosh wrote: > On Wed, Jul 25, 2012 at 11:20 PM, Colin Cross wrote: >> The cpu hotplug notifier gets called in both atomic and non-atomic >> contexts, it is not always safe to lock a mutex. Filter out all events >> except the six necessary ones, which are all sleepable, before taking >> the mutex. >> >> Signed-off-by: Colin Cross >> --- > Agree. > Have you observed any lock-up ? > Colin explained me about cause of the issue in an off-list discussion. Thought of updating the thread in case some one wants to reproduce the issue. You get a warning during cpu hotplug in suspend if you turn on sleeping while atomic debugging option in kernel build and the patch fixes it. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier
On Wed, Jul 25, 2012 at 11:20 PM, Colin Cross wrote: > The cpu hotplug notifier gets called in both atomic and non-atomic > contexts, it is not always safe to lock a mutex. Filter out all events > except the six necessary ones, which are all sleepable, before taking > the mutex. > > Signed-off-by: Colin Cross > --- Agree. Have you observed any lock-up ? For that patch itself, Acked-by: Santosh Shilimkar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier
On Wed, Jul 25, 2012 at 11:20 PM, Colin Cross ccr...@android.com wrote: The cpu hotplug notifier gets called in both atomic and non-atomic contexts, it is not always safe to lock a mutex. Filter out all events except the six necessary ones, which are all sleepable, before taking the mutex. Signed-off-by: Colin Cross ccr...@android.com --- Agree. Have you observed any lock-up ? For that patch itself, Acked-by: Santosh Shilimkar santosh.shilim...@ti.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier
On Thu, Jul 26, 2012 at 9:25 AM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Wed, Jul 25, 2012 at 11:20 PM, Colin Cross ccr...@android.com wrote: The cpu hotplug notifier gets called in both atomic and non-atomic contexts, it is not always safe to lock a mutex. Filter out all events except the six necessary ones, which are all sleepable, before taking the mutex. Signed-off-by: Colin Cross ccr...@android.com --- Agree. Have you observed any lock-up ? Colin explained me about cause of the issue in an off-list discussion. Thought of updating the thread in case some one wants to reproduce the issue. You get a warning during cpu hotplug in suspend if you turn on sleeping while atomic debugging option in kernel build and the patch fixes it. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2] cpufreq: Fix sysfs deadlock with concurrent hotplug/frequency switch
On Sat, Jul 21, 2012 at 1:21 AM, Rafael J. Wysocki wrote:
> On Friday, July 20, 2012, Stephen Boyd wrote:
>> Running one program that continuously hotplugs and replugs a cpu
>> concurrently with another program that continuously writes to the
>> scaling_setspeed node eventually deadlocks with:
>>
>> =
>> [ INFO: possible recursive locking detected ]
>> 3.4.0 #37 Tainted: GW
>> -
>> filemonkey/122 is trying to acquire lock:
>> (s_active#13){.+}, at: [] sysfs_remove_dir+0x9c/0xb4
>>
>> but task is already holding lock:
>> (s_active#13){.+}, at: [] sysfs_write_file+0xe8/0x140
>>
>> other info that might help us debug this:
>> Possible unsafe locking scenario:
>>
>>CPU0
>>
>> lock(s_active#13);
>> lock(s_active#13);
>>
>> *** DEADLOCK ***
>>
>> May be due to missing lock nesting notation
>>
>> 2 locks held by filemonkey/122:
>> #0: (>mutex){+.+.+.}, at: [] sysfs_write_file+0x28/0x140
>> #1: (s_active#13){.+}, at: [] sysfs_write_file+0xe8/0x140
>>
>> stack backtrace:
>> [] (unwind_backtrace+0x0/0x120) from []
>> (validate_chain+0x6f8/0x1054)
>> [] (validate_chain+0x6f8/0x1054) from []
>> (__lock_acquire+0x81c/0x8d8)
>> [] (__lock_acquire+0x81c/0x8d8) from []
>> (lock_acquire+0x18c/0x1e8)
>> [] (lock_acquire+0x18c/0x1e8) from []
>> (sysfs_addrm_finish+0xd0/0x180)
>> [] (sysfs_addrm_finish+0xd0/0x180) from []
>> (sysfs_remove_dir+0x9c/0xb4)
>> [] (sysfs_remove_dir+0x9c/0xb4) from []
>> (kobject_del+0x10/0x38)
>> [] (kobject_del+0x10/0x38) from []
>> (kobject_release+0xf0/0x194)
>> [] (kobject_release+0xf0/0x194) from []
>> (cpufreq_cpu_put+0xc/0x24)
>> [] (cpufreq_cpu_put+0xc/0x24) from [] (store+0x6c/0x74)
>> [] (store+0x6c/0x74) from []
>> (sysfs_write_file+0x10c/0x140)
>> [] (sysfs_write_file+0x10c/0x140) from []
>> (vfs_write+0xb0/0x128)
>> [] (vfs_write+0xb0/0x128) from [] (sys_write+0x3c/0x68)
>> [] (sys_write+0x3c/0x68) from []
>> (ret_fast_syscall+0x0/0x3c)
>>
>> This is because store() in cpufreq.c indirectly calls
>> kobject_get() via cpufreq_cpu_get() and is the last one to call
>> kobject_put() via cpufreq_cpu_put(). Sysfs code should not call
>> kobject_get() or kobject_put() directly (see the comment around
>> sysfs_schedule_callback() for more information).
>>
>> Fix this deadlock by introducing two new functions:
>>
>> struct cpufreq_policy *cpufreq_cpu_get_sysfs(unsigned int cpu)
>> void cpufreq_cpu_put_sysfs(struct cpufreq_policy *data)
>>
>> which do the same thing as cpufreq_cpu_{get,put}() but don't call
>> kobject functions.
>>
>> To easily trigger this deadlock you can insert an msleep() with a
>> reasonably large value right after the fail label at the bottom
>> of the store() function in cpufreq.c and then write
>> scaling_setspeed in one task and offline the cpu in another. The
>> first task will hang and be detected by the hung task detector.
>>
>> Signed-off-by: Stephen Boyd
>
> Thanks, applied to the pm-cpufreq branch of the linux-pm.git tree, will be
> pushed for v3.6.
>
Should this fix go to stable as well ?
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHSET] workqueue: reimplement CPU hotplug to keep idle workers
Rafael, On Sat, Jul 21, 2012 at 1:14 AM, Rafael J. Wysocki wrote: > On Friday, July 20, 2012, Tejun Heo wrote: >> Hello, >> >> On Fri, Jul 20, 2012 at 08:22:30PM +0200, Peter Zijlstra wrote: >> > I really think people who use hotplug at high frequencies are on drugs >> > and doing it wrong. >> >> I don't know. It does make some sense. It's not like we have any >> other mechanism to keep some processors completely quiesient, which >> could make a noticeable difference from powersaving POV compared to >> mostly idle. Rafael, can you please chime in and explain how / where >> / how freqeuntly / etc CPU hotplug is used for powersaving? > > Well, there are use cases I'm not really familiar with. > > Pretty much the only use case I'm sufficiently familiar with is > suspend/hibernate where we unplug all of the nonboot CPUs at one point. > > The other use cases, which I don't really think are entirely valid, > are on some ARM platforms where CPUs are unplugged instead of being put into > C-states or equivalent (because we don't have a good mechanism for handling > multiprocessor C-states; there's a set of patches for that waiting for > the merge window in the Len's tree). I'm hoping to get rid of those > use cases in future entirely. > Not sure if you are talking about couple idle series waiting in Len's tree for the merge. That series actually trying add infrastructure for the hardwares where CPU's need co-ordination which is needed on few ARM hardwares to get into deeper CPU cluster C-states. It is indeed true that without that approach, cpu-hotplug was used to overcome the ordering issue I guess Exynos, Tegra and OMAP are the few ARM architectures I know who has been using the couple cpuidle infrastructure. Regards Santosh Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHSET] workqueue: reimplement CPU hotplug to keep idle workers
Rafael, On Sat, Jul 21, 2012 at 1:14 AM, Rafael J. Wysocki r...@sisk.pl wrote: On Friday, July 20, 2012, Tejun Heo wrote: Hello, On Fri, Jul 20, 2012 at 08:22:30PM +0200, Peter Zijlstra wrote: I really think people who use hotplug at high frequencies are on drugs and doing it wrong. I don't know. It does make some sense. It's not like we have any other mechanism to keep some processors completely quiesient, which could make a noticeable difference from powersaving POV compared to mostly idle. Rafael, can you please chime in and explain how / where / how freqeuntly / etc CPU hotplug is used for powersaving? Well, there are use cases I'm not really familiar with. Pretty much the only use case I'm sufficiently familiar with is suspend/hibernate where we unplug all of the nonboot CPUs at one point. The other use cases, which I don't really think are entirely valid, are on some ARM platforms where CPUs are unplugged instead of being put into C-states or equivalent (because we don't have a good mechanism for handling multiprocessor C-states; there's a set of patches for that waiting for the merge window in the Len's tree). I'm hoping to get rid of those use cases in future entirely. Not sure if you are talking about couple idle series waiting in Len's tree for the merge. That series actually trying add infrastructure for the hardwares where CPU's need co-ordination which is needed on few ARM hardwares to get into deeper CPU cluster C-states. It is indeed true that without that approach, cpu-hotplug was used to overcome the ordering issue I guess Exynos, Tegra and OMAP are the few ARM architectures I know who has been using the couple cpuidle infrastructure. Regards Santosh Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2] cpufreq: Fix sysfs deadlock with concurrent hotplug/frequency switch
On Sat, Jul 21, 2012 at 1:21 AM, Rafael J. Wysocki r...@sisk.pl wrote:
On Friday, July 20, 2012, Stephen Boyd wrote:
Running one program that continuously hotplugs and replugs a cpu
concurrently with another program that continuously writes to the
scaling_setspeed node eventually deadlocks with:
=
[ INFO: possible recursive locking detected ]
3.4.0 #37 Tainted: GW
-
filemonkey/122 is trying to acquire lock:
(s_active#13){.+}, at: [c01a3d28] sysfs_remove_dir+0x9c/0xb4
but task is already holding lock:
(s_active#13){.+}, at: [c01a22f0] sysfs_write_file+0xe8/0x140
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
lock(s_active#13);
lock(s_active#13);
*** DEADLOCK ***
May be due to missing lock nesting notation
2 locks held by filemonkey/122:
#0: (buffer-mutex){+.+.+.}, at: [c01a2230] sysfs_write_file+0x28/0x140
#1: (s_active#13){.+}, at: [c01a22f0] sysfs_write_file+0xe8/0x140
stack backtrace:
[c0014fcc] (unwind_backtrace+0x0/0x120) from [c00ca600]
(validate_chain+0x6f8/0x1054)
[c00ca600] (validate_chain+0x6f8/0x1054) from [c00cb778]
(__lock_acquire+0x81c/0x8d8)
[c00cb778] (__lock_acquire+0x81c/0x8d8) from [c00cb9c0]
(lock_acquire+0x18c/0x1e8)
[c00cb9c0] (lock_acquire+0x18c/0x1e8) from [c01a3ba8]
(sysfs_addrm_finish+0xd0/0x180)
[c01a3ba8] (sysfs_addrm_finish+0xd0/0x180) from [c01a3d28]
(sysfs_remove_dir+0x9c/0xb4)
[c01a3d28] (sysfs_remove_dir+0x9c/0xb4) from [c02d0e5c]
(kobject_del+0x10/0x38)
[c02d0e5c] (kobject_del+0x10/0x38) from [c02d0f74]
(kobject_release+0xf0/0x194)
[c02d0f74] (kobject_release+0xf0/0x194) from [c0565a98]
(cpufreq_cpu_put+0xc/0x24)
[c0565a98] (cpufreq_cpu_put+0xc/0x24) from [c05683f0] (store+0x6c/0x74)
[c05683f0] (store+0x6c/0x74) from [c01a2314]
(sysfs_write_file+0x10c/0x140)
[c01a2314] (sysfs_write_file+0x10c/0x140) from [c014af44]
(vfs_write+0xb0/0x128)
[c014af44] (vfs_write+0xb0/0x128) from [c014b06c] (sys_write+0x3c/0x68)
[c014b06c] (sys_write+0x3c/0x68) from [c000e0e0]
(ret_fast_syscall+0x0/0x3c)
This is because store() in cpufreq.c indirectly calls
kobject_get() via cpufreq_cpu_get() and is the last one to call
kobject_put() via cpufreq_cpu_put(). Sysfs code should not call
kobject_get() or kobject_put() directly (see the comment around
sysfs_schedule_callback() for more information).
Fix this deadlock by introducing two new functions:
struct cpufreq_policy *cpufreq_cpu_get_sysfs(unsigned int cpu)
void cpufreq_cpu_put_sysfs(struct cpufreq_policy *data)
which do the same thing as cpufreq_cpu_{get,put}() but don't call
kobject functions.
To easily trigger this deadlock you can insert an msleep() with a
reasonably large value right after the fail label at the bottom
of the store() function in cpufreq.c and then write
scaling_setspeed in one task and offline the cpu in another. The
first task will hang and be detected by the hung task detector.
Signed-off-by: Stephen Boyd sb...@codeaurora.org
Thanks, applied to the pm-cpufreq branch of the linux-pm.git tree, will be
pushed for v3.6.
Should this fix go to stable as well ?
Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] gpio/omap: add *remove* callback in platform_driver
On Mon, Jul 16, 2012 at 10:40 PM, Kevin Hilman wrote: > > Linus Walleij writes: > > > On Thu, Jul 12, 2012 at 7:48 PM, Kevin Hilman wrote: > > > >> In the case of OMAP GPIO, unless it's an obvious fix, I would recommend > >> you wait at least until you see some acks/tested tags from any of > >> > >> - Santosh Shilimkar > >> - Rajendra Nayak > >> - Benoit Cousson > >> > >> or Tony, Paul or myself. > > > > Instead of trying to store this information in my and Grants brains and > > us forgetting it, what about patching MAINTAINERS to reflect the fact > > instead? That's better I think, plus I use that file a lot. > > > >> For major series, I have been collecting/queueing them for Grant after > >> ensuring they have been well reviewed and well tested (although I am > >> eagerly hoping to hand off this role to someone else.) > > > > Listing it under your GIT tree in MAINTAINERS for this driver will make > > this work better I think. > > > > One path for OMAP GPIO patches, simple. It's obviously the > > ambiguity that cause the trouble. Then you can also decide > > on each cycle whether to send these to GPIO or ARM SoC > > etc. > > Yeah, I understand the process, but I've been avoiding doing that > because, well, I don't want the job. I have been trying to get someone > else at TI to maintain this driver, but have not been successful. > > So, until that happens, feel free to queue up the patch below. > > Santosh, note that I've put you as co-maintainer of this driver. > No problem Kevin. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] gpio/omap: add *remove* callback in platform_driver
On Mon, Jul 16, 2012 at 10:40 PM, Kevin Hilman khil...@ti.com wrote: Linus Walleij linus.wall...@linaro.org writes: On Thu, Jul 12, 2012 at 7:48 PM, Kevin Hilman khil...@ti.com wrote: In the case of OMAP GPIO, unless it's an obvious fix, I would recommend you wait at least until you see some acks/tested tags from any of - Santosh Shilimkar santosh.shilim...@ti.com - Rajendra Nayak rna...@ti.com - Benoit Cousson b-cous...@ti.com or Tony, Paul or myself. Instead of trying to store this information in my and Grants brains and us forgetting it, what about patching MAINTAINERS to reflect the fact instead? That's better I think, plus I use that file a lot. For major series, I have been collecting/queueing them for Grant after ensuring they have been well reviewed and well tested (although I am eagerly hoping to hand off this role to someone else.) Listing it under your GIT tree in MAINTAINERS for this driver will make this work better I think. One path for OMAP GPIO patches, simple. It's obviously the ambiguity that cause the trouble. Then you can also decide on each cycle whether to send these to GPIO or ARM SoC etc. Yeah, I understand the process, but I've been avoiding doing that because, well, I don't want the job. I have been trying to get someone else at TI to maintain this driver, but have not been successful. So, until that happens, feel free to queue up the patch below. Santosh, note that I've put you as co-maintainer of this driver. No problem Kevin. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
