All of lore.kernel.org
 help / color / mirror / Atom feed
From: Felipe Balbi <balbi@ti.com>
To: Felipe Balbi <balbi@ti.com>
Cc: Andrew Lunn <andrew@lunn.ch>, Tony Lindgren <tony@atomide.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Daniel Walker <dwalker@fifo99.com>,
	Lennert Buytenhek <kernel@wantstofly.org>,
	Kevin Hilman <khilman@ti.com>, Kukjin Kim <kgene.kim@samsung.com>,
	Bryan Huntsman <bryanh@codeaurora.org>,
	Russell King <rmk+kernel@arm.linux.org.uk>,
	David Brown <davidb@codeaurora.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Ben Dooks <ben-linux@fluff.org>,
	Alexander Schulz <alex@shark-linux.de>,
	Linux OMAP Mailing List <linux-omap@vger.kernel.org>,
	Andrew Victor <linux@maxim.org.za>,
	Linux ARM Kernel Mailing List
	<linux-arm-kernel@lists.infradead.org>,
	Paul Walmsley <paul@pwsan.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Santosh Shilimkar <santosh.shilimkar@ti.com>,
	Sascha Hauer <kernel@peng>
Subject: Re: [PATCH] arm: sched: stop sched_clock() during suspend
Date: Mon, 22 Oct 2012 15:07:22 +0300	[thread overview]
Message-ID: <20121022120722.GV14033@arwen.pp.htv.fi> (raw)
In-Reply-To: <1350906877-19410-1-git-send-email-balbi@ti.com>


[-- Attachment #1.1: Type: text/plain, Size: 3182 bytes --]

Hi,

On Mon, Oct 22, 2012 at 02:54:37PM +0300, Felipe Balbi wrote:
> The scheduler imposes a requirement to sched_clock()
> which is to stop the clock during suspend, if we don't
> do that IRQ threads will be rescheduled in the future
> which might cause transfers to timeout depending on
> how driver is written.
> 
> This became an issue on OMAP when we converted omap-i2c.c
> to use threaded IRQs, it turned out that depending on how
> much time we spent on suspend, the I2C IRQ thread would
> end up being rescheduled so far in the future that I2C
> transfers would timeout and, because omap_hsmmc depends
> on an I2C-connected device to detect if an MMC card is
> inserted in the slot, our rootfs would just vanish.
> 
> arch/arm/kernel/sched_clock.c already had an optional
> implementation (sched_clock_needs_suspend()) which would
> handle scheduler's requirement properly, what this patch
> does is simply to make that implementation non-optional.
> 
> This has been tested with beagleboard XM (OMAP3630) and
> pandaboard rev A3 (OMAP4430). Suspend to RAM is now working
> after this patch.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>

just adding more guys to Cc. Please add more if I have missed someone.

> ---
>  arch/arm/include/asm/sched_clock.h |  2 --
>  arch/arm/kernel/sched_clock.c      | 18 ++++--------------
>  2 files changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/include/asm/sched_clock.h b/arch/arm/include/asm/sched_clock.h
> index 05b8e82..e3f7572 100644
> --- a/arch/arm/include/asm/sched_clock.h
> +++ b/arch/arm/include/asm/sched_clock.h
> @@ -10,7 +10,5 @@
>  
>  extern void sched_clock_postinit(void);
>  extern void setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate);
> -extern void setup_sched_clock_needs_suspend(u32 (*read)(void), int bits,
> -		unsigned long rate);
>  
>  #endif
> diff --git a/arch/arm/kernel/sched_clock.c b/arch/arm/kernel/sched_clock.c
> index e21bac2..fc6692e 100644
> --- a/arch/arm/kernel/sched_clock.c
> +++ b/arch/arm/kernel/sched_clock.c
> @@ -107,13 +107,6 @@ static void sched_clock_poll(unsigned long wrap_ticks)
>  	update_sched_clock();
>  }
>  
> -void __init setup_sched_clock_needs_suspend(u32 (*read)(void), int bits,
> -		unsigned long rate)
> -{
> -	setup_sched_clock(read, bits, rate);
> -	cd.needs_suspend = true;
> -}
> -
>  void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
>  {
>  	unsigned long r, w;
> @@ -189,18 +182,15 @@ void __init sched_clock_postinit(void)
>  static int sched_clock_suspend(void)
>  {
>  	sched_clock_poll(sched_clock_timer.data);
> -	if (cd.needs_suspend)
> -		cd.suspended = true;
> +	cd.suspended = true;
>  	return 0;
>  }
>  
>  static void sched_clock_resume(void)
>  {
> -	if (cd.needs_suspend) {
> -		cd.epoch_cyc = read_sched_clock();
> -		cd.epoch_cyc_copy = cd.epoch_cyc;
> -		cd.suspended = false;
> -	}
> +	cd.epoch_cyc = read_sched_clock();
> +	cd.epoch_cyc_copy = cd.epoch_cyc;
> +	cd.suspended = false;
>  }
>  
>  static struct syscore_ops sched_clock_ops = {
> -- 
> 1.8.0.rc0
> 

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: balbi@ti.com (Felipe Balbi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm: sched: stop sched_clock() during suspend
Date: Mon, 22 Oct 2012 15:07:22 +0300	[thread overview]
Message-ID: <20121022120722.GV14033@arwen.pp.htv.fi> (raw)
In-Reply-To: <1350906877-19410-1-git-send-email-balbi@ti.com>

Hi,

On Mon, Oct 22, 2012 at 02:54:37PM +0300, Felipe Balbi wrote:
> The scheduler imposes a requirement to sched_clock()
> which is to stop the clock during suspend, if we don't
> do that IRQ threads will be rescheduled in the future
> which might cause transfers to timeout depending on
> how driver is written.
> 
> This became an issue on OMAP when we converted omap-i2c.c
> to use threaded IRQs, it turned out that depending on how
> much time we spent on suspend, the I2C IRQ thread would
> end up being rescheduled so far in the future that I2C
> transfers would timeout and, because omap_hsmmc depends
> on an I2C-connected device to detect if an MMC card is
> inserted in the slot, our rootfs would just vanish.
> 
> arch/arm/kernel/sched_clock.c already had an optional
> implementation (sched_clock_needs_suspend()) which would
> handle scheduler's requirement properly, what this patch
> does is simply to make that implementation non-optional.
> 
> This has been tested with beagleboard XM (OMAP3630) and
> pandaboard rev A3 (OMAP4430). Suspend to RAM is now working
> after this patch.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>

just adding more guys to Cc. Please add more if I have missed someone.

> ---
>  arch/arm/include/asm/sched_clock.h |  2 --
>  arch/arm/kernel/sched_clock.c      | 18 ++++--------------
>  2 files changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/include/asm/sched_clock.h b/arch/arm/include/asm/sched_clock.h
> index 05b8e82..e3f7572 100644
> --- a/arch/arm/include/asm/sched_clock.h
> +++ b/arch/arm/include/asm/sched_clock.h
> @@ -10,7 +10,5 @@
>  
>  extern void sched_clock_postinit(void);
>  extern void setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate);
> -extern void setup_sched_clock_needs_suspend(u32 (*read)(void), int bits,
> -		unsigned long rate);
>  
>  #endif
> diff --git a/arch/arm/kernel/sched_clock.c b/arch/arm/kernel/sched_clock.c
> index e21bac2..fc6692e 100644
> --- a/arch/arm/kernel/sched_clock.c
> +++ b/arch/arm/kernel/sched_clock.c
> @@ -107,13 +107,6 @@ static void sched_clock_poll(unsigned long wrap_ticks)
>  	update_sched_clock();
>  }
>  
> -void __init setup_sched_clock_needs_suspend(u32 (*read)(void), int bits,
> -		unsigned long rate)
> -{
> -	setup_sched_clock(read, bits, rate);
> -	cd.needs_suspend = true;
> -}
> -
>  void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
>  {
>  	unsigned long r, w;
> @@ -189,18 +182,15 @@ void __init sched_clock_postinit(void)
>  static int sched_clock_suspend(void)
>  {
>  	sched_clock_poll(sched_clock_timer.data);
> -	if (cd.needs_suspend)
> -		cd.suspended = true;
> +	cd.suspended = true;
>  	return 0;
>  }
>  
>  static void sched_clock_resume(void)
>  {
> -	if (cd.needs_suspend) {
> -		cd.epoch_cyc = read_sched_clock();
> -		cd.epoch_cyc_copy = cd.epoch_cyc;
> -		cd.suspended = false;
> -	}
> +	cd.epoch_cyc = read_sched_clock();
> +	cd.epoch_cyc_copy = cd.epoch_cyc;
> +	cd.suspended = false;
>  }
>  
>  static struct syscore_ops sched_clock_ops = {
> -- 
> 1.8.0.rc0
> 

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121022/01b6c272/attachment.sig>

WARNING: multiple messages have this Message-ID (diff)
From: Felipe Balbi <balbi@ti.com>
To: Felipe Balbi <balbi@ti.com>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>,
	Linux ARM Kernel Mailing List 
	<linux-arm-kernel@lists.infradead.org>,
	Linux OMAP Mailing List <linux-omap@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Kevin Hilman <khilman@ti.com>, Paul Walmsley <paul@pwsan.com>,
	Tony Lindgren <tony@atomide.com>,
	Santosh Shilimkar <santosh.shilimkar@ti.com>,
	Arnd Bergmann <arnd@arndb.de>, Olof Johansson <olof@lixom.net>,
	Kukjin Kim <kgene.kim@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Ben Dooks <ben-linux@fluff.org>,
	Alexander Schulz <alex@shark-linux.de>,
	Lennert Buytenhek <kernel@wantstofly.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Andrew Victor <linux@maxim.org.za>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Andrew Lunn <andrew@lunn.ch>,
	Sascha Hauer <kernel@pengutronix.de>,
	David Brown <davidb@codeaurora.org>,
	Daniel Walker <dwalker@fifo99.com>,
	Bryan Huntsman <bryanh@codeaurora.org>
Subject: Re: [PATCH] arm: sched: stop sched_clock() during suspend
Date: Mon, 22 Oct 2012 15:07:22 +0300	[thread overview]
Message-ID: <20121022120722.GV14033@arwen.pp.htv.fi> (raw)
In-Reply-To: <1350906877-19410-1-git-send-email-balbi@ti.com>

[-- Attachment #1: Type: text/plain, Size: 3182 bytes --]

Hi,

On Mon, Oct 22, 2012 at 02:54:37PM +0300, Felipe Balbi wrote:
> The scheduler imposes a requirement to sched_clock()
> which is to stop the clock during suspend, if we don't
> do that IRQ threads will be rescheduled in the future
> which might cause transfers to timeout depending on
> how driver is written.
> 
> This became an issue on OMAP when we converted omap-i2c.c
> to use threaded IRQs, it turned out that depending on how
> much time we spent on suspend, the I2C IRQ thread would
> end up being rescheduled so far in the future that I2C
> transfers would timeout and, because omap_hsmmc depends
> on an I2C-connected device to detect if an MMC card is
> inserted in the slot, our rootfs would just vanish.
> 
> arch/arm/kernel/sched_clock.c already had an optional
> implementation (sched_clock_needs_suspend()) which would
> handle scheduler's requirement properly, what this patch
> does is simply to make that implementation non-optional.
> 
> This has been tested with beagleboard XM (OMAP3630) and
> pandaboard rev A3 (OMAP4430). Suspend to RAM is now working
> after this patch.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>

just adding more guys to Cc. Please add more if I have missed someone.

> ---
>  arch/arm/include/asm/sched_clock.h |  2 --
>  arch/arm/kernel/sched_clock.c      | 18 ++++--------------
>  2 files changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/include/asm/sched_clock.h b/arch/arm/include/asm/sched_clock.h
> index 05b8e82..e3f7572 100644
> --- a/arch/arm/include/asm/sched_clock.h
> +++ b/arch/arm/include/asm/sched_clock.h
> @@ -10,7 +10,5 @@
>  
>  extern void sched_clock_postinit(void);
>  extern void setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate);
> -extern void setup_sched_clock_needs_suspend(u32 (*read)(void), int bits,
> -		unsigned long rate);
>  
>  #endif
> diff --git a/arch/arm/kernel/sched_clock.c b/arch/arm/kernel/sched_clock.c
> index e21bac2..fc6692e 100644
> --- a/arch/arm/kernel/sched_clock.c
> +++ b/arch/arm/kernel/sched_clock.c
> @@ -107,13 +107,6 @@ static void sched_clock_poll(unsigned long wrap_ticks)
>  	update_sched_clock();
>  }
>  
> -void __init setup_sched_clock_needs_suspend(u32 (*read)(void), int bits,
> -		unsigned long rate)
> -{
> -	setup_sched_clock(read, bits, rate);
> -	cd.needs_suspend = true;
> -}
> -
>  void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
>  {
>  	unsigned long r, w;
> @@ -189,18 +182,15 @@ void __init sched_clock_postinit(void)
>  static int sched_clock_suspend(void)
>  {
>  	sched_clock_poll(sched_clock_timer.data);
> -	if (cd.needs_suspend)
> -		cd.suspended = true;
> +	cd.suspended = true;
>  	return 0;
>  }
>  
>  static void sched_clock_resume(void)
>  {
> -	if (cd.needs_suspend) {
> -		cd.epoch_cyc = read_sched_clock();
> -		cd.epoch_cyc_copy = cd.epoch_cyc;
> -		cd.suspended = false;
> -	}
> +	cd.epoch_cyc = read_sched_clock();
> +	cd.epoch_cyc_copy = cd.epoch_cyc;
> +	cd.suspended = false;
>  }
>  
>  static struct syscore_ops sched_clock_ops = {
> -- 
> 1.8.0.rc0
> 

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

  reply	other threads:[~2012-10-22 12:07 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-22 11:54 [PATCH] arm: sched: stop sched_clock() during suspend Felipe Balbi
2012-10-22 11:54 ` Felipe Balbi
2012-10-22 12:07 ` Felipe Balbi [this message]
2012-10-22 12:07   ` Felipe Balbi
2012-10-22 12:07   ` Felipe Balbi
2012-10-22 17:05 ` Kevin Hilman
2012-10-22 17:05   ` Kevin Hilman
2012-10-22 17:05   ` Kevin Hilman
2012-10-22 22:28   ` Linus Walleij
2012-10-22 22:28     ` Linus Walleij
2012-10-22 22:28     ` Linus Walleij
2012-10-23  9:22     ` Russell King - ARM Linux
2012-10-23  9:22       ` Russell King - ARM Linux
2012-10-23 14:17       ` Kevin Hilman
2012-10-23 14:17         ` Kevin Hilman
2012-10-23 16:03         ` Russell King - ARM Linux
2012-10-23 16:03           ` Russell King - ARM Linux
2012-10-23 17:24           ` Felipe Balbi
2012-10-23 17:24             ` Felipe Balbi
2012-10-23  5:57   ` [PATCH v2] " Felipe Balbi
2012-10-23  5:57     ` Felipe Balbi
2012-10-23 14:14     ` Kevin Hilman
2012-10-23 14:14       ` Kevin Hilman
2012-10-23 10:11 ` [PATCH] " Linus Walleij
2012-10-23 10:11   ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121022120722.GV14033@arwen.pp.htv.fi \
    --to=balbi@ti.com \
    --cc=alex@shark-linux.de \
    --cc=andrew@lunn.ch \
    --cc=arnd@arndb.de \
    --cc=ben-linux@fluff.org \
    --cc=bryanh@codeaurora.org \
    --cc=davidb@codeaurora.org \
    --cc=dwalker@fifo99.com \
    --cc=kernel@peng \
    --cc=kernel@wantstofly.org \
    --cc=kgene.kim@samsung.com \
    --cc=khilman@ti.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux@maxim.org.za \
    --cc=paul@pwsan.com \
    --cc=rmk+kernel@arm.linux.org.uk \
    --cc=santosh.shilimkar@ti.com \
    --cc=swarren@wwwdotorg.org \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.