SUPERH platform development
 help / color / mirror / Atom feed
* Re: [PATCH 3/3] ARM: mach-shmobile: sh7372 A4R support (v4)
From: Guennadi Liakhovetski @ 2011-11-02 10:38 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Linux-sh list, Linux PM list, Linux Kernel, Magnus Damm
In-Reply-To: <201110200015.06277.rjw@sisk.pl>

On Thu, 20 Oct 2011, Rafael J. Wysocki wrote:

> From: Magnus Damm <damm@opensource.se>
> 
> This change adds support for the sh7372 A4R power domain.

This version still breaks I2C (#0) on mackerel. Tested with your 
sh7372-test branch of Oct 18th plus these four patches:

6b29305a PM / Domains: Add default power off governor function (v2)
3d578ae PM / Domains: Add device stop governor function (v2)
6c2cd02 ARM: mach-shmobile: sh7372 A4R support (v4)
9da11f4 ARM: mach-shmobile: sh7372 A3SP support (v4)

Thanks
Guennadi

> 
> The sh7372 A4R hardware power domain contains the
> SH CPU Core and a set of I/O devices including
> multimedia accelerators and I2C controllers.
> 
> One special case about A4R is the INTCS interrupt
> controller that needs to be saved and restored to
> keep working as expected. Also the LCDC hardware
> blocks are in a different hardware power domain
> but have their IRQs routed only through INTCS. So
> as long as LCDCs are active we cannot power down
> INTCS because that would risk losing interrupts.
> 
> Signed-off-by: Magnus Damm <damm@opensource.se>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  arch/arm/mach-shmobile/board-ap4evb.c        |    1 
>  arch/arm/mach-shmobile/board-mackerel.c      |    1 
>  arch/arm/mach-shmobile/include/mach/sh7372.h |    7 +++
>  arch/arm/mach-shmobile/intc-sh7372.c         |   52 ++++++++++++++++++++++++++-
>  arch/arm/mach-shmobile/pm-sh7372.c           |   29 ++++++++++++++-
>  arch/arm/mach-shmobile/setup-sh7372.c        |    8 ++++
>  6 files changed, 96 insertions(+), 2 deletions(-)
> 
> Index: linux/arch/arm/mach-shmobile/board-ap4evb.c
> =================================> --- linux.orig/arch/arm/mach-shmobile/board-ap4evb.c
> +++ linux/arch/arm/mach-shmobile/board-ap4evb.c
> @@ -1412,6 +1412,7 @@ static void __init ap4evb_init(void)
>  	sh7372_add_device_to_domain(&sh7372_a3sp, &sh_mmcif_device);
>  	sh7372_add_device_to_domain(&sh7372_a3sp, &sdhi0_device);
>  	sh7372_add_device_to_domain(&sh7372_a3sp, &sdhi1_device);
> +	sh7372_add_device_to_domain(&sh7372_a4r, &ceu_device);
>  
>  	hdmi_init_pm_clock();
>  	fsi_init_pm_clock();
> Index: linux/arch/arm/mach-shmobile/board-mackerel.c
> =================================> --- linux.orig/arch/arm/mach-shmobile/board-mackerel.c
> +++ linux/arch/arm/mach-shmobile/board-mackerel.c
> @@ -1596,6 +1596,7 @@ static void __init mackerel_init(void)
>  	sh7372_add_device_to_domain(&sh7372_a3sp, &sdhi1_device);
>  #endif
>  	sh7372_add_device_to_domain(&sh7372_a3sp, &sdhi2_device);
> +	sh7372_add_device_to_domain(&sh7372_a4r, &ceu_device);
>  
>  	hdmi_init_pm_clock();
>  	sh7372_pm_init();
> Index: linux/arch/arm/mach-shmobile/include/mach/sh7372.h
> =================================> --- linux.orig/arch/arm/mach-shmobile/include/mach/sh7372.h
> +++ linux/arch/arm/mach-shmobile/include/mach/sh7372.h
> @@ -480,8 +480,11 @@ struct platform_device;
>  struct sh7372_pm_domain {
>  	struct generic_pm_domain genpd;
>  	struct dev_power_governor *gov;
> +	void (*suspend)(void);
> +	void (*resume)(void);
>  	unsigned int bit_shift;
>  	bool no_debug;
> +	bool stay_on;
>  };
>  
>  static inline struct sh7372_pm_domain *to_sh7372_pd(struct generic_pm_domain *d)
> @@ -493,6 +496,7 @@ static inline struct sh7372_pm_domain *t
>  extern struct sh7372_pm_domain sh7372_a4lc;
>  extern struct sh7372_pm_domain sh7372_a4mp;
>  extern struct sh7372_pm_domain sh7372_d4;
> +extern struct sh7372_pm_domain sh7372_a4r;
>  extern struct sh7372_pm_domain sh7372_a3rv;
>  extern struct sh7372_pm_domain sh7372_a3ri;
>  extern struct sh7372_pm_domain sh7372_a3sp;
> @@ -509,4 +513,7 @@ extern void sh7372_pm_add_subdomain(stru
>  #define sh7372_pm_add_subdomain(pd, sd) do { } while(0)
>  #endif /* CONFIG_PM */
>  
> +extern void sh7372_intcs_suspend(void);
> +extern void sh7372_intcs_resume(void);
> +
>  #endif /* __ASM_SH7372_H__ */
> Index: linux/arch/arm/mach-shmobile/intc-sh7372.c
> =================================> --- linux.orig/arch/arm/mach-shmobile/intc-sh7372.c
> +++ linux/arch/arm/mach-shmobile/intc-sh7372.c
> @@ -606,9 +606,16 @@ static void intcs_demux(unsigned int irq
>  	generic_handle_irq(intcs_evt2irq(evtcodeas));
>  }
>  
> +static void __iomem *intcs_ffd2;
> +static void __iomem *intcs_ffd5;
> +
>  void __init sh7372_init_irq(void)
>  {
> -	void __iomem *intevtsa = ioremap_nocache(0xffd20100, PAGE_SIZE);
> +	void __iomem *intevtsa;
> +
> +	intcs_ffd2 = ioremap_nocache(0xffd20000, PAGE_SIZE);
> +	intevtsa = intcs_ffd2 + 0x100;
> +	intcs_ffd5 = ioremap_nocache(0xffd50000, PAGE_SIZE);
>  
>  	register_intc_controller(&intca_desc);
>  	register_intc_controller(&intcs_desc);
> @@ -617,3 +624,46 @@ void __init sh7372_init_irq(void)
>  	irq_set_handler_data(evt2irq(0xf80), (void *)intevtsa);
>  	irq_set_chained_handler(evt2irq(0xf80), intcs_demux);
>  }
> +
> +static unsigned short ffd2[0x200];
> +static unsigned short ffd5[0x100];
> +
> +void sh7372_intcs_suspend(void)
> +{
> +	int k;
> +
> +	for (k = 0x00; k <= 0x30; k += 4)
> +		ffd2[k] = __raw_readw(intcs_ffd2 + k);
> +
> +	for (k = 0x80; k <= 0xb0; k += 4)
> +		ffd2[k] = __raw_readb(intcs_ffd2 + k);
> +
> +	for (k = 0x180; k <= 0x188; k += 4)
> +		ffd2[k] = __raw_readb(intcs_ffd2 + k);
> +
> +	for (k = 0x00; k <= 0x3c; k += 4)
> +		ffd5[k] = __raw_readw(intcs_ffd5 + k);
> +
> +	for (k = 0x80; k <= 0x9c; k += 4)
> +		ffd5[k] = __raw_readb(intcs_ffd5 + k);
> +}
> +
> +void sh7372_intcs_resume(void)
> +{
> +	int k;
> +
> +	for (k = 0x00; k <= 0x30; k += 4)
> +		__raw_writew(ffd2[k], intcs_ffd2 + k);
> +
> +	for (k = 0x80; k <= 0xb0; k += 4)
> +		__raw_writeb(ffd2[k], intcs_ffd2 + k);
> +
> +	for (k = 0x180; k <= 0x188; k += 4)
> +		__raw_writeb(ffd2[k], intcs_ffd2 + k);
> +
> +	for (k = 0x00; k <= 0x3c; k += 4)
> +		__raw_writew(ffd5[k], intcs_ffd5 + k);
> +
> +	for (k = 0x80; k <= 0x9c; k += 4)
> +		__raw_writeb(ffd5[k], intcs_ffd5 + k);
> +}
> Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
> =================================> --- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
> +++ linux/arch/arm/mach-shmobile/pm-sh7372.c
> @@ -44,6 +44,7 @@
>  #define SPDCR 0xe6180008
>  #define SWUCR 0xe6180014
>  #define SBAR 0xe6180020
> +#define WUPRMSK 0xe6180028
>  #define WUPSMSK 0xe618002c
>  #define WUPSMSK2 0xe6180048
>  #define PSTR 0xe6180080
> @@ -80,6 +81,12 @@ static int pd_power_down(struct generic_
>  	struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
>  	unsigned int mask = 1 << sh7372_pd->bit_shift;
>  
> +	if (sh7372_pd->suspend)
> +		sh7372_pd->suspend();
> +
> +	if (sh7372_pd->stay_on)
> +		return 0;
> +
>  	if (__raw_readl(PSTR) & mask) {
>  		unsigned int retry_count;
>  
> @@ -106,6 +113,9 @@ static int pd_power_up(struct generic_pm
>  	unsigned int retry_count;
>  	int ret = 0;
>  
> +	if (sh7372_pd->stay_on)
> +		goto out;
> +
>  	if (__raw_readl(PSTR) & mask)
>  		goto out;
>  
> @@ -122,14 +132,23 @@ static int pd_power_up(struct generic_pm
>  	if (__raw_readl(SWUCR) & mask)
>  		ret = -EIO;
>  
> - out:
>  	if (!sh7372_pd->no_debug)
>  		pr_debug("sh7372 power domain up 0x%08x -> PSTR = 0x%08x\n",
>  			 mask, __raw_readl(PSTR));
>  
> + out:
> +	if (ret = 0 && sh7372_pd->resume)
> +		sh7372_pd->resume();
> +
>  	return ret;
>  }
>  
> +static void sh7372_a4r_suspend(void)
> +{
> +	sh7372_intcs_suspend();
> +	__raw_writel(0x300fffff, WUPRMSK); /* avoid wakeup */
> +}
> +
>  static bool pd_active_wakeup(struct device *dev)
>  {
>  	return true;
> @@ -186,6 +205,14 @@ struct sh7372_pm_domain sh7372_d4 = {
>  	.bit_shift = 3,
>  };
>  
> +struct sh7372_pm_domain sh7372_a4r = {
> +	.bit_shift = 5,
> +	.gov = &sh7372_always_on_gov,
> +	.suspend = sh7372_a4r_suspend,
> +	.resume = sh7372_intcs_resume,
> +	.stay_on = true,
> +};
> +
>  struct sh7372_pm_domain sh7372_a3rv = {
>  	.bit_shift = 6,
>  };
> Index: linux/arch/arm/mach-shmobile/setup-sh7372.c
> =================================> --- linux.orig/arch/arm/mach-shmobile/setup-sh7372.c
> +++ linux/arch/arm/mach-shmobile/setup-sh7372.c
> @@ -991,12 +991,14 @@ void __init sh7372_add_standard_devices(
>  	sh7372_init_pm_domain(&sh7372_a4lc);
>  	sh7372_init_pm_domain(&sh7372_a4mp);
>  	sh7372_init_pm_domain(&sh7372_d4);
> +	sh7372_init_pm_domain(&sh7372_a4r);
>  	sh7372_init_pm_domain(&sh7372_a3rv);
>  	sh7372_init_pm_domain(&sh7372_a3ri);
>  	sh7372_init_pm_domain(&sh7372_a3sg);
>  	sh7372_init_pm_domain(&sh7372_a3sp);
>  
>  	sh7372_pm_add_subdomain(&sh7372_a4lc, &sh7372_a3rv);
> +	sh7372_pm_add_subdomain(&sh7372_a4r, &sh7372_a4lc);
>  
>  	platform_add_devices(sh7372_early_devices,
>  			    ARRAY_SIZE(sh7372_early_devices));
> @@ -1020,6 +1022,12 @@ void __init sh7372_add_standard_devices(
>  	sh7372_add_device_to_domain(&sh7372_a3sp, &dma2_device);
>  	sh7372_add_device_to_domain(&sh7372_a3sp, &usb_dma0_device);
>  	sh7372_add_device_to_domain(&sh7372_a3sp, &usb_dma1_device);
> +	sh7372_add_device_to_domain(&sh7372_a4r, &iic0_device);
> +	sh7372_add_device_to_domain(&sh7372_a4r, &veu0_device);
> +	sh7372_add_device_to_domain(&sh7372_a4r, &veu1_device);
> +	sh7372_add_device_to_domain(&sh7372_a4r, &veu2_device);
> +	sh7372_add_device_to_domain(&sh7372_a4r, &veu3_device);
> +	sh7372_add_device_to_domain(&sh7372_a4r, &jpu_device);
>  }
>  
>  void __init sh7372_add_early_devices(void)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* Re: [PATCH] regulator: Provide dummy supply support
From: Sascha Hauer @ 2011-11-02 10:03 UTC (permalink / raw)
  To: Mark Brown
  Cc: Mike Frysinger, linux-kernel, Liam Girdwood, Linus Walleij,
	Paul Mundt, Tony Lindgren, linux-sh, linux-omap,
	uclinux-dist-devel
In-Reply-To: <20111101182720.GH10029@opensource.wolfsonmicro.com>

On Tue, Nov 01, 2011 at 06:27:21PM +0000, Mark Brown wrote:
> On Tue, Nov 01, 2011 at 01:50:14PM -0400, Mike Frysinger wrote:
> > On Friday 28 October 2011 18:47:57 Sascha Hauer wrote:
> 
> > > > We already have a dummy regulator driver and a fixed voltage regulator
> > > > driver, we shouldn't be adding a third implementation of the same thing.
> > > > Just use the fixed voltage regulator for this.
> 
> > > I explained in my mail why I think that the current implementation of
> > > the dummy regulator is not suitable for things apart from debugging.
> 
> > your complaints seem to be specific to how the dummy regulator gets hooked in 
> > and not in the specific regulator implementation.  so it seems like the right 
> > thing would be to split the kconfig knobs:
> 
> Quite.  Sascha, your mail doesn't refer to the implementation of the
> regulator itself at all.  Nothing in your changelog even mentions that
> you're introducing a new regulator driver.
> 
> I think there's a big abstraction understanding failure here, reading
> your changelog I'm not sure you understand the existing mechainsms that
> are in place.  You say:
> 
> | This patch allows a board to register dummy supplies for devices
> | which need a regulator but which is not software controllable
> | on this board.
> 
> but this is exactly the use case the fixed voltage regulator is there
> for.
> 
> >  config REGULATOR_DUMMY
> > -	bool "Provide a dummy regulator if regulator lookups fail"
> > +	bool "Provide a dummy regulator"
> > +config REGULATOR_DUMMY_FALLBACK
> > +	bool "Fallback to dummy regulator if lookup fails"
> > +	depends on REGULATOR_DUMMY
> 
> As I think I said earlier I'd use the fixed regulator for this, all
> Sascha's actually doing here is adding a wrapper to simplify
> registration of that.

There's one difference between the fixed and the dummy regulator though:
The fixed regulator has a voltage. The same dummy regulator instance can
be used for all devices which do not have a software controllable
regulator. I think the same can be done with the fixed regulator aswell,
but the bogus voltage showing up in the sysfs entry might be confusing
to users.
That's the reason I implemented a (second) dummy regulator driver.
Indeed it's not nice to have two of them.

Another approach to this topic would be to allow a board to explicitely
bind to the existing dummy regulator, like the following (error path
should of course be implemented before applying this)

Sascha

8<------------------------------------

[PATCH] regulator: allow boards to bind to the dummy regulator

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/regulator/core.c          |   19 +++++++++++++++++++
 include/linux/regulator/machine.h |    8 ++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index d8e6a42..a7a38ba 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1046,6 +1046,25 @@ static void unset_regulator_supplies(struct regulator_dev *rdev)
 	}
 }
 
+int regulator_add_dummy_supply(struct regulator_consumer_supply *supply,
+		int num_supplies)
+{
+	int i, ret;
+
+	for (i = 0; i < num_supplies; i++) {
+		ret = set_consumer_device_supply(dummy_regulator_rdev, NULL,
+				supply[i].dev_name, supply[i].supply);
+		if (ret)
+			goto err_out;
+	}
+
+	return 0;
+err_out:
+	/* FIXME: unset device supply */
+	return ret;
+}
+EXPORT_SYMBOL_GPL(regulator_add_dummy_supply);
+
 #define REG_STR_SIZE	64
 
 static struct regulator *create_regulator(struct regulator_dev *rdev,
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index ce3127a..89089cd 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -192,6 +192,8 @@ int regulator_suspend_finish(void);
 #ifdef CONFIG_REGULATOR
 void regulator_has_full_constraints(void);
 void regulator_use_dummy_regulator(void);
+int regulator_add_dummy_supply(struct regulator_consumer_supply *supply,
+		int num_supplies);
 #else
 static inline void regulator_has_full_constraints(void)
 {
@@ -200,6 +202,12 @@ static inline void regulator_has_full_constraints(void)
 static inline void regulator_use_dummy_regulator(void)
 {
 }
+
+static inline int regulator_add_dummy_supply(struct regulator_consumer_supply *supply,
+		int num_supplies)
+{
+	return 0;
+}
 #endif
 
 #endif
-- 
1.7.7

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply related

* Re: [PATCH 1/3] PM / Sleep: Mark devices involved in wakeup signaling
From: Guennadi Liakhovetski @ 2011-11-02  9:29 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Linux-sh list, Linux PM list, Linux Kernel, Magnus Damm
In-Reply-To: <201110200013.56428.rjw@sisk.pl>

Hi Rafael

Just something, that made me wonder:

On Thu, 20 Oct 2011, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The generic PM domains code in drivers/base/power/domain.c has
> to avoid powering off domains that provide power to wakeup devices
> during system suspend.  Currently, however, this only works for
> wakeup devices directly belonging to the given domain and not for
> their children (or the children of their children and so on).
> Thus, if there's a wakeup device whose parent belongs to a power
> domain handled by the generic PM domains code, the domain will be
> powered off during system suspend preventing the device from
> signaling wakeup.
> 
> To address this problem introduce a device flag, power.wakeup_path,
> that will be set during system suspend for all wakeup devices,
> their parents, the parents of their parents and so on.  This way,
> all wakeup paths in the device hierarchy will be marked and the
> generic PM domains code will only need to avoid powering off
> domains containing devices whose power.wakeup_path is set.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/base/power/domain.c |    4 ++--
>  drivers/base/power/main.c   |    8 +++++++-
>  include/linux/pm.h          |    1 +
>  3 files changed, 10 insertions(+), 3 deletions(-)

[snip]

> Index: linux/include/linux/pm.h
> =================================> --- linux.orig/include/linux/pm.h
> +++ linux/include/linux/pm.h
> @@ -452,6 +452,7 @@ struct dev_pm_info {
>  	struct list_head	entry;
>  	struct completion	completion;
>  	struct wakeup_source	*wakeup;
> +	bool			wakeup_path:1;

This is an interesting idea... I'd presume, the compiler is aware, that 
one bit is enough for "bool," so, it should choose an optimal 
implementation by itself? I checked gcc 4.4.1 on ARM - without the 
bitfield notation the compiler just uses one byte in my example. Anyway, 
not a request-for-change, just wondering whether you really were trying to 
(potentially) save a couple of bits here or what was the motivation.

>  #else
>  	unsigned int		should_wakeup:1;
>  #endif

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* Re: [PATCH] regulator: Provide dummy supply support
From: Mark Brown @ 2011-11-01 18:27 UTC (permalink / raw)
  To: Mike Frysinger
  Cc: Sascha Hauer, linux-kernel, Liam Girdwood, Linus Walleij,
	Paul Mundt, Tony Lindgren, linux-sh, linux-omap,
	uclinux-dist-devel
In-Reply-To: <201111011350.17425.vapier@gentoo.org>

On Tue, Nov 01, 2011 at 01:50:14PM -0400, Mike Frysinger wrote:
> On Friday 28 October 2011 18:47:57 Sascha Hauer wrote:

> > > We already have a dummy regulator driver and a fixed voltage regulator
> > > driver, we shouldn't be adding a third implementation of the same thing.
> > > Just use the fixed voltage regulator for this.

> > I explained in my mail why I think that the current implementation of
> > the dummy regulator is not suitable for things apart from debugging.

> your complaints seem to be specific to how the dummy regulator gets hooked in 
> and not in the specific regulator implementation.  so it seems like the right 
> thing would be to split the kconfig knobs:

Quite.  Sascha, your mail doesn't refer to the implementation of the
regulator itself at all.  Nothing in your changelog even mentions that
you're introducing a new regulator driver.

I think there's a big abstraction understanding failure here, reading
your changelog I'm not sure you understand the existing mechainsms that
are in place.  You say:

| This patch allows a board to register dummy supplies for devices
| which need a regulator but which is not software controllable
| on this board.

but this is exactly the use case the fixed voltage regulator is there
for.

>  config REGULATOR_DUMMY
> -	bool "Provide a dummy regulator if regulator lookups fail"
> +	bool "Provide a dummy regulator"
> +config REGULATOR_DUMMY_FALLBACK
> +	bool "Fallback to dummy regulator if lookup fails"
> +	depends on REGULATOR_DUMMY

As I think I said earlier I'd use the fixed regulator for this, all
Sascha's actually doing here is adding a wrapper to simplify
registration of that.

^ permalink raw reply

* Re: [PATCH] regulator: Provide dummy supply support
From: Mike Frysinger @ 2011-11-01 17:50 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Mark Brown, linux-kernel, Liam Girdwood, Linus Walleij,
	Paul Mundt, Tony Lindgren, linux-sh, linux-omap,
	uclinux-dist-devel
In-Reply-To: <20111028224757.GG23421@pengutronix.de>

[-- Attachment #1: Type: Text/Plain, Size: 1206 bytes --]

On Friday 28 October 2011 18:47:57 Sascha Hauer wrote:
> On Fri, Oct 28, 2011 at 11:59:31PM +0200, Mark Brown wrote:
> > On Fri, Oct 28, 2011 at 10:26:58PM +0200, Sascha Hauer wrote:
> > >  drivers/regulator/Makefile        |    2 +-
> > >  drivers/regulator/dummy-supply.c  |   88
> > >  +++++++++++++++++++++++++++++++++++++
> > 
> > We already have a dummy regulator driver and a fixed voltage regulator
> > driver, we shouldn't be adding a third implementation of the same thing.
> > Just use the fixed voltage regulator for this.
> 
> I explained in my mail why I think that the current implementation of
> the dummy regulator is not suitable for things apart from debugging.

your complaints seem to be specific to how the dummy regulator gets hooked in 
and not in the specific regulator implementation.  so it seems like the right 
thing would be to split the kconfig knobs:

 config REGULATOR_DUMMY
-	bool "Provide a dummy regulator if regulator lookups fail"
+	bool "Provide a dummy regulator"
+config REGULATOR_DUMMY_FALLBACK
+	bool "Fallback to dummy regulator if lookup fails"
+	depends on REGULATOR_DUMMY

(and then obviously update the .c files accordingly)
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] sh: modify the asm/sh_eth.h to linux/sh_eth.h in sh7757lcr
From: Paul Mundt @ 2011-11-01  7:00 UTC (permalink / raw)
  To: linux-sh
In-Reply-To: <4E82D17A.7050108@renesas.com>

On Tue, Nov 01, 2011 at 02:54:38PM +0900, Nobuhiro Iwamatsu wrote:
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
> ---
>  arch/sh/boards/board-sh7757lcr.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

On Tue, Nov 01, 2011 at 02:57:01PM +0900, Nobuhiro Iwamatsu wrote:
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
> ---
>  arch/sh/Makefile |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

Both applied, thanks.

^ permalink raw reply

* [PATCH] sh: Add default uImage rule for sh7757lcr
From: Nobuhiro Iwamatsu @ 2011-11-01  5:57 UTC (permalink / raw)
  To: linux-sh

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
---
 arch/sh/Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/sh/Makefile b/arch/sh/Makefile
index 99385d0..3fc0f41 100644
--- a/arch/sh/Makefile
+++ b/arch/sh/Makefile
@@ -80,6 +80,7 @@ defaultimage-$(CONFIG_SH_RSK)			:= uImage
 defaultimage-$(CONFIG_SH_URQUELL)		:= uImage
 defaultimage-$(CONFIG_SH_MIGOR)			:= uImage
 defaultimage-$(CONFIG_SH_AP325RXA)		:= uImage
+defaultimage-$(CONFIG_SH_SH7757LCR)		:= uImage
 defaultimage-$(CONFIG_SH_7724_SOLUTION_ENGINE)	:= uImage
 defaultimage-$(CONFIG_SH_7206_SOLUTION_ENGINE)	:= vmlinux
 defaultimage-$(CONFIG_SH_7619_SOLUTION_ENGINE)	:= vmlinux
-- 
1.7.7


^ permalink raw reply related

* [PATCH] sh: modify the asm/sh_eth.h to linux/sh_eth.h in sh7757lcr
From: Nobuhiro Iwamatsu @ 2011-11-01  5:54 UTC (permalink / raw)
  To: linux-sh
In-Reply-To: <4E82D17A.7050108@renesas.com>

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
---
 arch/sh/boards/board-sh7757lcr.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/sh/boards/board-sh7757lcr.c b/arch/sh/boards/board-sh7757lcr.c
index fa2a208..ec8c84c 100644
--- a/arch/sh/boards/board-sh7757lcr.c
+++ b/arch/sh/boards/board-sh7757lcr.c
@@ -18,8 +18,8 @@
 #include <linux/mmc/host.h>
 #include <linux/mmc/sh_mmcif.h>
 #include <linux/mmc/sh_mobile_sdhi.h>
+#include <linux/sh_eth.h>
 #include <cpu/sh7757.h>
-#include <asm/sh_eth.h>
 #include <asm/heartbeat.h>
 
 static struct resource heartbeat_resource = {
-- 
1.7.7


^ permalink raw reply related

* Re: [PATCH 2/2 v4] net/smsc911x: Add regulator support
From: Mike Frysinger @ 2011-10-31 18:21 UTC (permalink / raw)
  To: Robert Marklund
  Cc: netdev, Steve Glendinning, Mathieu Poirier, Paul Mundt, linux-sh,
	Sascha Hauer, Tony Lindgren, linux-omap, uclinux-dist-devel,
	Linus Walleij
In-Reply-To: <1320064719-14449-1-git-send-email-robert.marklund@stericsson.com>

[-- Attachment #1: Type: Text/Plain, Size: 437 bytes --]

On Monday 31 October 2011 08:38:39 Robert Marklund wrote:
> ChangeLog v3->v4:
> - Remove dual prints and old comment on Mike's request.
> - Split the request_free fucntion on Mike and Sascha request.

would be nice if the enable/disable were split as well ...

>  	iounmap(pdata->ioaddr);
> 
> +	(void)smsc911x_enable_disable_resources(pdev, false);

i don't think the (void) cast is necessary

otherwise looks fine
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [PATCH 2/2 v4] net/smsc911x: Add regulator support
From: Robert Marklund @ 2011-10-31 12:38 UTC (permalink / raw)
  To: netdev, Steve Glendinning
  Cc: Mathieu Poirier, Robert Marklund, Paul Mundt, linux-sh,
	Sascha Hauer, Tony Lindgren, linux-omap, Mike Frysinger,
	uclinux-dist-devel, Linus Walleij

Add some basic regulator support for the power pins, as needed
by the ST-Ericsson Snowball platform that powers up the SMSC911
chip using an external regulator.

Platforms that use regulators and the smsc911x and have no defined
regulator for the smsc911x and claim complete regulator
constraints with no dummy regulators will need to provide it, for
example using a fixed voltage regulator. It appears that this may
affect (apart from Ux500 Snowball) possibly these archs/machines
that from some grep:s appear to define both CONFIG_SMSC911X and
CONFIG_REGULATOR:

- ARM Freescale mx3 and OMAP 2 plus, Raumfeld machines
- Blackfin
- Super-H

Cc: Paul Mundt <lethal@linux-sh.org>
Cc: linux-sh@vger.kernel.org
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Tony Lindgren <tony@atomide.com>
Cc: linux-omap@vger.kernel.org
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: uclinux-dist-devel@blackfin.uclinux.org
Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Signed-off-by: Robert Marklund <robert.marklund@stericsson.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v3->v4:
- Remove dual prints and old comment on Mike's request.
- Split the request_free fucntion on Mike and Sascha request.
ChangeLog v2->v3:
- Use bulk regulators on Mark's request.
- Add Cc-fileds to some possibly affected platforms.
ChangeLog v1->v2:
- Don't check for NULL regulators and error out properly if the
  regulators can't be found. All platforms using the smsc911x
  and the regulator framework simultaneously need to provide some
  kind of regulator for it.
---
 drivers/net/ethernet/smsc/smsc911x.c |  103 ++++++++++++++++++++++++++++++----
 1 files changed, 92 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 8843071..9a2e792 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -44,6 +44,7 @@
 #include <linux/module.h>
 #include <linux/netdevice.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <linux/sched.h>
 #include <linux/timer.h>
 #include <linux/bug.h>
@@ -88,6 +89,8 @@ struct smsc911x_ops {
 				unsigned int *buf, unsigned int wordcount);
 };
 
+#define SMSC911X_NUM_SUPPLIES 2
+
 struct smsc911x_data {
 	void __iomem *ioaddr;
 
@@ -138,6 +141,9 @@ struct smsc911x_data {
 
 	/* register access functions */
 	const struct smsc911x_ops *ops;
+
+	/* regulators */
+	struct regulator_bulk_data supplies[SMSC911X_NUM_SUPPLIES];
 };
 
 /* Easy access to information */
@@ -362,6 +368,68 @@ out:
 	spin_unlock_irqrestore(&pdata->dev_lock, flags);
 }
 
+/*
+ * Enable or disable resources, currently just regulators.
+ */
+static int smsc911x_enable_disable_resources(struct platform_device *pdev,
+					     bool enable)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct smsc911x_data *pdata = netdev_priv(ndev);
+	int ret = 0;
+
+	/* enable/disable regulators */
+	if (enable) {
+		ret = regulator_bulk_enable(ARRAY_SIZE(pdata->supplies),
+				pdata->supplies);
+		if (ret)
+			netdev_err(ndev, "failed to enable regulators %d\n",
+					ret);
+	} else
+		ret = regulator_bulk_disable(ARRAY_SIZE(pdata->supplies),
+				pdata->supplies);
+	return ret;
+}
+
+/*
+ * Request resources, currently just regulators.
+ *
+ * The SMSC911x has two power pins: vddvario and vdd33a, in designs where
+ * these are not always-on we need to request regulators to be turned on
+ * before we can try to access the device registers.
+ */
+static int smsc911x_request_resources(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct smsc911x_data *pdata = netdev_priv(ndev);
+	int ret = 0;
+
+	/* Request regulators */
+	pdata->supplies[0].supply = "vdd33a";
+	pdata->supplies[1].supply = "vddvario";
+	ret = regulator_bulk_get(&pdev->dev,
+			ARRAY_SIZE(pdata->supplies),
+			pdata->supplies);
+	if (ret)
+		netdev_err(ndev, "couldn't get regulators %d\n",
+				ret);
+	return ret;
+}
+
+/*
+ * Free resources, currently just regulators.
+ *
+ */
+static void smsc911x_free_resources(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct smsc911x_data *pdata = netdev_priv(ndev);
+
+	/* Free regulators */
+	regulator_bulk_free(ARRAY_SIZE(pdata->supplies),
+			pdata->supplies);
+}
+
 /* waits for MAC not busy, with timeout.  Only called by smsc911x_mac_read
  * and smsc911x_mac_write, so assumes mac_lock is held */
 static int smsc911x_mac_complete(struct smsc911x_data *pdata)
@@ -2092,6 +2160,9 @@ static int __devexit smsc911x_drv_remove(struct platform_device *pdev)
 
 	iounmap(pdata->ioaddr);
 
+	(void)smsc911x_enable_disable_resources(pdev, false);
+	smsc911x_free_resources(pdev);
+
 	free_netdev(dev);
 
 	return 0;
@@ -2218,10 +2289,20 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
 	pdata->dev = dev;
 	pdata->msg_enable = ((1 << debug) - 1);
 
+	platform_set_drvdata(pdev, dev);
+
+	retval = smsc911x_request_resources(pdev);
+	if (retval)
+		goto out_return_resources;
+
+	retval = smsc911x_enable_disable_resources(pdev, true);
+	if (retval)
+		goto out_disable_resources;
+
 	if (pdata->ioaddr = NULL) {
 		SMSC_WARN(pdata, probe, "Error smsc911x base address invalid");
 		retval = -ENOMEM;
-		goto out_free_netdev_2;
+		goto out_disable_resources;
 	}
 
 	retval = smsc911x_probe_config_dt(&pdata->config, np);
@@ -2233,7 +2314,7 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
 
 	if (retval) {
 		SMSC_WARN(pdata, probe, "Error smsc911x config not found");
-		goto out_unmap_io_3;
+		goto out_disable_resources;
 	}
 
 	/* assume standard, non-shifted, access to HW registers */
@@ -2244,7 +2325,7 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
 
 	retval = smsc911x_init(dev);
 	if (retval < 0)
-		goto out_unmap_io_3;
+		goto out_disable_resources;
 
 	/* configure irq polarity and type before connecting isr */
 	if (pdata->config.irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH)
@@ -2264,15 +2345,13 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
 	if (retval) {
 		SMSC_WARN(pdata, probe,
 			  "Unable to claim requested irq: %d", dev->irq);
-		goto out_unmap_io_3;
+		goto out_free_irq;
 	}
 
-	platform_set_drvdata(pdev, dev);
-
 	retval = register_netdev(dev);
 	if (retval) {
 		SMSC_WARN(pdata, probe, "Error %i registering device", retval);
-		goto out_unset_drvdata_4;
+		goto out_free_irq;
 	} else {
 		SMSC_TRACE(pdata, probe,
 			   "Network interface: \"%s\"", dev->name);
@@ -2321,12 +2400,14 @@ static int __devinit smsc911x_drv_probe(struct platform_device *pdev)
 
 out_unregister_netdev_5:
 	unregister_netdev(dev);
-out_unset_drvdata_4:
-	platform_set_drvdata(pdev, NULL);
+out_free_irq:
 	free_irq(dev->irq, dev);
-out_unmap_io_3:
+out_disable_resources:
+	(void)smsc911x_enable_disable_resources(pdev, false);
+out_return_resources:
+	smsc911x_free_resources(pdev);
+	platform_set_drvdata(pdev, NULL);
 	iounmap(pdata->ioaddr);
-out_free_netdev_2:
 	free_netdev(dev);
 out_release_io_1:
 	release_mem_region(res->start, resource_size(res));
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH v9 2/4] cpuidle: Remove CPUIDLE_FLAG_IGNORE and dev->prepare()
From: Deepthi Dharwar @ 2011-10-31  7:44 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: khilman, venki, ak, len.brown, peterz, rjw, santosh.shilimkar,
	lenb, linux-pm, linux-sh, linux-kernel, linux-acpi, linux-pm,
	linux-omap, linux-arm-kernel
In-Reply-To: <4EAABB07.3000305@linux.intel.com>

On Friday 28 October 2011 07:54 PM, Arjan van de Ven wrote:
> 
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 10/28/2011 3:50 AM, Deepthi Dharwar wrote:
>> The cpuidle_device->prepare() mechanism causes updates to the
>> cpuidle_state[].flags, setting and clearing CPUIDLE_FLAG_IGNORE
>> to tell the governor not to chose a state on a per-cpu basis at
>> run-time. State demotion is now handled by the driver and it returns
>> the actual state entered. Hence, this mechanism is not required.
>> Also this removes per-cpu flags from cpuidle_state enabling
>> it to be made global.
>>
> 
> having worked on some newer platforms....
> this one is really still needed. doing this inside the actual states
> does not work,
> because if the deepest 3 states are invalid, the same (somewhat
> expensive) test would have to be done 3 times,
> and each of the states would have to fail before the 4th one gets chosen.
> that's just not going to work
> 
> (in the state handler you can't know what other state to fall back to,
> and especially not how to enter such a fallback state)
> 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.17 (MingW32)
> 
> iQEcBAEBAgAGBQJOqrsGAAoJEEHdSxh4DVnEu7EH/i5lEJctBAIubJOcZz/tvBFp
> XYmAe/HqNtSXeHOVsJkTf8y4ppE8487exF7xxMik4GRN0CZNCtkyMezqDVu+eDim
> O/UUbScsAc5cSY6mkjOFXLFup+mi1nkRUnAbxXEyTMhWwcbfr2OvcuO7l7TmATML
> hu87P3PVEafEop3q2+uWMc57fFxnNFfEDqRx6N9V+OJKV5dHrRYL4G4E01fYGFLo
> xTR0IW7nB15L0C29zk9uk/Dqow8SoJZA83c7p7AieP5zdntb6p7noIf03qmdp19f
> fulwMwembCHivo+pLO+jAMhKD1T6VYoCyiYW0LHrQ2E07fayBhFJCxlazgKFcl0> =FL6o
> -----END PGP SIGNATURE-----
> 
Hi Arjan,

Thanks for the review. 

We retain the dev->prepare() routine and CPUIDLE_FLAG_IGNORE 
but still allow the dev->enter() to return index ? 
So by retaining it, transition to the idle states
would be much quicker in case one more states are invalid.

Also to note is that in the newer design, we have split the 
cpuidle_state structure. One global struct, cpuidle_states[] that
contains all the state related information including flags, and 
the other cpuidle_device that contain statistics and other data 
that are per-cpu basis. 
So the flags are not per cpu per state basis but 
maintained globally as per state basis. 

So if we have to enable CPUIDLE_FLAG_IGNORE flag in this 
current new design, then I am thinking if we needed to have this 
on a per-cpu basis. If so, then flags have to be moved into cpuidle_device 
struct rather than cpuidle_state struct. 

Is it a good idea to retain these flags as global (part of cpuidle_states) 
or make it per-cpu basis ? 

-Thanks
Deepthi   


^ permalink raw reply

* Re: [PATCH] regulator: Provide dummy supply support
From: Mark Brown @ 2011-10-29 17:42 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-kernel, Liam Girdwood, Linus Walleij, Paul Mundt,
	Tony Lindgren, Mike Frysinger, linux-sh, linux-omap,
	uclinux-dist-devel
In-Reply-To: <20111028224757.GG23421@pengutronix.de>

On Sat, Oct 29, 2011 at 12:47:57AM +0200, Sascha Hauer wrote:

> My main concern with the fixed regulator is that it needs quite much
> boilerplate code just to say that we have no regulator at all for a
> given device. That could also be handled with a helper function which
> registers a fixed regulator and only takes the regulator_consumer_supply
> as an argument. Would that be ok for you?

All you're actually doing in this code is adding a function to register
a new type of regulator which is exactly equivalent to what the existing
regulators provide - there's nothing particularly wrong with the helper
function but defining an entirely new regulator type for it doesn't seem
useful.

^ permalink raw reply

* Re: [PATCH 2/2 v3] net/smsc911x: Add regulator support
From: Mike Frysinger @ 2011-10-28 23:33 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Linus Walleij, netdev, Steve Glendinning, Mathieu Poirer,
	Robert Marklund, Paul Mundt, linux-sh, Tony Lindgren, linux-omap,
	uclinux-dist-devel, Linus Walleij
In-Reply-To: <20111028203353.GE23421@pengutronix.de>

On Fri, Oct 28, 2011 at 22:33, Sascha Hauer wrote:
> On Thu, Oct 27, 2011 at 02:48:11PM +0200, Linus Walleij wrote:
>> +/*
>> + * Request or free resources, currently just regulators.
>> + *
>> + * The SMSC911x has two power pins: vddvario and vdd33a, in designs where
>> + * these are not always-on we need to request regulators to be turned on
>> + * before we can try to access the device registers.
>> + */
>> +static int smsc911x_request_free_resources(struct platform_device *pdev,
>> +             bool request)
>
> I had to look twice at this function name. First I thought "request the
> free resources?", which other resources would you request if not the
> free ones? I think it would be nicer to have two functions instead.
> Just my 2 cents.

i'll add my 2 cents and we'll almost have a nickle.  maybe i'm dense,
but i had to look (more than) twice at both funcs before i could get
my head around what was happening.  no, it's not complicated, but it
is unusual in the kernel world.  either that or i haven't read enough
kernel code to consider this a common paradigm.  hopefully it's the
former ;).
-mike

^ permalink raw reply

* Re: [PATCH] regulator: provide dummy supply support
From: Mike Frysinger @ 2011-10-28 23:22 UTC (permalink / raw)
  To: Mark Brown
  Cc: Sascha Hauer, linux-kernel, Liam Girdwood, Linus Walleij,
	Paul Mundt, Tony Lindgren, linux-sh, linux-omap,
	uclinux-dist-devel
In-Reply-To: <20111028215703.GB30339@opensource.wolfsonmicro.com>

On Fri, Oct 28, 2011 at 23:57, Mark Brown wrote:
> On Fri, Oct 28, 2011 at 10:26:57PM +0200, Sascha Hauer wrote:
>> The following patch allows it for boards to register a dummy supply for
>> devices for which no software controllable regulator is available. The
>> current CONFIG_REGULATOR_DUMMY and (unused) regulator_use_dummy_regulator()
>
> Please don't send cover letters for single patch serieses, if there's
> content it should probably be in the changelog...

sorry for the additional noise, but +1
-mike

^ permalink raw reply

* Re: [PATCH] regulator: Provide dummy supply support
From: Mike Frysinger @ 2011-10-28 23:16 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Mark Brown, linux-kernel, Liam Girdwood, Linus Walleij,
	Paul Mundt, Tony Lindgren, linux-sh, linux-omap,
	uclinux-dist-devel
In-Reply-To: <20111028224757.GG23421@pengutronix.de>

On Sat, Oct 29, 2011 at 00:47, Sascha Hauer wrote:
> On Fri, Oct 28, 2011 at 11:59:31PM +0200, Mark Brown wrote:
>> On Fri, Oct 28, 2011 at 10:26:58PM +0200, Sascha Hauer wrote:
>>
>> >  drivers/regulator/Makefile        |    2 +-
>> >  drivers/regulator/dummy-supply.c  |   88 +++++++++++++++++++++++++++++++++++++
>>
>> We already have a dummy regulator driver and a fixed voltage regulator
>> driver, we shouldn't be adding a third implementation of the same thing.
>> Just use the fixed voltage regulator for this.
>
> I explained in my mail why I think that the current implementation of
> the dummy regulator is not suitable for things apart from debugging.
>
> My main concern with the fixed regulator is that it needs quite much
> boilerplate code just to say that we have no regulator at all for a
> given device. That could also be handled with a helper function which
> registers a fixed regulator and only takes the regulator_consumer_supply
> as an argument. Would that be ok for you?

i think Mark's point is that we have code in dummy.c already to
provide a dummy regulator.  so your needs sounds like it could be
satisfied with some Kconfig/ifdef massaging and the existing
drivers/regulator/dummy.c rather than introducing a completely
parallel file that is always enabled ?
-mike

^ permalink raw reply

* Re: [PATCH] regulator: Provide dummy supply support
From: Sascha Hauer @ 2011-10-28 22:47 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-kernel, Liam Girdwood, Linus Walleij, Paul Mundt,
	Tony Lindgren, Mike Frysinger, linux-sh, linux-omap,
	uclinux-dist-devel
In-Reply-To: <20111028215931.GA30366@opensource.wolfsonmicro.com>

On Fri, Oct 28, 2011 at 11:59:31PM +0200, Mark Brown wrote:
> On Fri, Oct 28, 2011 at 10:26:58PM +0200, Sascha Hauer wrote:
> 
> >  drivers/regulator/Makefile        |    2 +-
> >  drivers/regulator/dummy-supply.c  |   88 +++++++++++++++++++++++++++++++++++++
> 
> We already have a dummy regulator driver and a fixed voltage regulator
> driver, we shouldn't be adding a third implementation of the same thing.
> Just use the fixed voltage regulator for this.

I explained in my mail why I think that the current implementation of
the dummy regulator is not suitable for things apart from debugging.

My main concern with the fixed regulator is that it needs quite much
boilerplate code just to say that we have no regulator at all for a
given device. That could also be handled with a helper function which
registers a fixed regulator and only takes the regulator_consumer_supply
as an argument. Would that be ok for you?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH] regulator: Provide dummy supply support
From: Mark Brown @ 2011-10-28 21:59 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-kernel, Liam Girdwood, Linus Walleij, Paul Mundt,
	Tony Lindgren, Mike Frysinger, linux-sh, linux-omap,
	uclinux-dist-devel
In-Reply-To: <1319833618-25190-2-git-send-email-s.hauer@pengutronix.de>

On Fri, Oct 28, 2011 at 10:26:58PM +0200, Sascha Hauer wrote:

>  drivers/regulator/Makefile        |    2 +-
>  drivers/regulator/dummy-supply.c  |   88 +++++++++++++++++++++++++++++++++++++

We already have a dummy regulator driver and a fixed voltage regulator
driver, we shouldn't be adding a third implementation of the same thing.
Just use the fixed voltage regulator for this.

^ permalink raw reply

* Re: [PATCH] regulator: provide dummy supply support
From: Mark Brown @ 2011-10-28 21:57 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-kernel, Liam Girdwood, Linus Walleij, Paul Mundt,
	Tony Lindgren, Mike Frysinger, linux-sh, linux-omap,
	uclinux-dist-devel
In-Reply-To: <1319833618-25190-1-git-send-email-s.hauer@pengutronix.de>

On Fri, Oct 28, 2011 at 10:26:57PM +0200, Sascha Hauer wrote:

> The following patch allows it for boards to register a dummy supply for
> devices for which no software controllable regulator is available. The
> current CONFIG_REGULATOR_DUMMY and (unused) regulator_use_dummy_regulator()

Please don't send cover letters for single patch serieses, if there's
content it should probably be in the changelog...

^ permalink raw reply

* Re: [PATCH 2/2 v3] net/smsc911x: Add regulator support
From: Sascha Hauer @ 2011-10-28 20:33 UTC (permalink / raw)
  To: Linus Walleij
  Cc: netdev, Steve Glendinning, Mathieu Poirer, Robert Marklund,
	Paul Mundt, linux-sh, Tony Lindgren, linux-omap, Mike Frysinger,
	uclinux-dist-devel, Linus Walleij
In-Reply-To: <1319719691-15799-1-git-send-email-linus.walleij@stericsson.com>

Hi Linus,

On Thu, Oct 27, 2011 at 02:48:11PM +0200, Linus Walleij wrote:
> From: Robert Marklund <robert.marklund@stericsson.com>
> 
> Add some basic regulator support for the power pins, as needed
> by the ST-Ericsson Snowball platform that powers up the SMSC911
> chip using an external regulator.
> 
> Platforms that use regulators and the smsc911x and have no defined
> regulator for the smsc911x and claim complete regulator
> constraints with no dummy regulators will need to provide it, for
> example using a fixed voltage regulator. It appears that this may
> affect (apart from Ux500 Snowball) possibly these archs/machines
> that from some grep:s appear to define both CONFIG_SMSC911X and
> CONFIG_REGULATOR:
> 
> - ARM Freescale mx3 and OMAP 2 plus, Raumfeld machines
> - Blackfin
> - Super-H
> 

...

>  
> +
> +/*
> + * Request or free resources, currently just regulators.
> + *
> + * The SMSC911x has two power pins: vddvario and vdd33a, in designs where
> + * these are not always-on we need to request regulators to be turned on
> + * before we can try to access the device registers.
> + */
> +static int smsc911x_request_free_resources(struct platform_device *pdev,
> +		bool request)

I had to look twice at this function name. First I thought "request the
free resources?", which other resources would you request if not the
free ones? I think it would be nicer to have two functions instead.
Just my 2 cents.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* [PATCH] regulator: Provide dummy supply support
From: Sascha Hauer @ 2011-10-28 20:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: Liam Girdwood, Mark Brown, Linus Walleij, Paul Mundt,
	Tony Lindgren, Mike Frysinger, linux-sh, linux-omap,
	uclinux-dist-devel, Sascha Hauer
In-Reply-To: <1319833618-25190-1-git-send-email-s.hauer@pengutronix.de>

Currently we have CONFIG_REGULATOR_DUMMY which provides a fallback
dummy regulator if none is found. Enabling this option shadows
real regulator_get errors though and can't be used in production.
Also there is regulator_use_dummy_regulator() which has the
same behaviour but can be used during runtime.

This patch allows a board to register dummy supplies for devices
which need a regulator but which is not software controllable
on this board.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/regulator/Makefile        |    2 +-
 drivers/regulator/dummy-supply.c  |   88 +++++++++++++++++++++++++++++++++++++
 include/linux/regulator/machine.h |   18 ++++++++
 3 files changed, 107 insertions(+), 1 deletions(-)
 create mode 100644 drivers/regulator/dummy-supply.c

diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 040d5aa..ff618b9 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -3,7 +3,7 @@
 #
 
 
-obj-$(CONFIG_REGULATOR) += core.o dummy.o
+obj-$(CONFIG_REGULATOR) += core.o dummy.o dummy-supply.o
 obj-$(CONFIG_REGULATOR_FIXED_VOLTAGE) += fixed.o
 obj-$(CONFIG_REGULATOR_VIRTUAL_CONSUMER) += virtual.o
 obj-$(CONFIG_REGULATOR_USERSPACE_CONSUMER) += userspace-consumer.o
diff --git a/drivers/regulator/dummy-supply.c b/drivers/regulator/dummy-supply.c
new file mode 100644
index 0000000..4e62b1f
--- /dev/null
+++ b/drivers/regulator/dummy-supply.c
@@ -0,0 +1,88 @@
+/*
+ * dummy-supply.c
+ *
+ * Copyright 2011 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This is useful for systems with mixed controllable and
+ * non-controllable regulators, as well as for allowing testing on
+ * systems with no controllable regulators.
+ */
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+
+struct dummy_regulator {
+	struct regulator_desc desc;
+	struct regulator_init_data initdata;
+	struct regulator_dev *rdev;
+};
+
+static struct regulator_ops dummy_ops;
+
+/**
+ * regulator_register_dummy - register a dummy supply
+ *
+ * Calling this function will register a dummy regulator
+ * for devices for which no software controllable regulator
+ * is available.
+ */
+struct dummy_regulator *regulator_register_dummy(
+		struct regulator_consumer_supply *__supply,
+		int num_supplies)
+{
+	struct dummy_regulator *dummy;
+	struct regulator_consumer_supply *supply;
+
+	dummy = kzalloc(sizeof(*dummy), GFP_KERNEL);
+	if (!dummy)
+		return NULL;
+
+	dummy->desc.name = "dummy";
+	dummy->desc.id = -1;
+	dummy->desc.type = REGULATOR_VOLTAGE;
+	dummy->desc.owner = THIS_MODULE;
+	dummy->desc.ops = &dummy_ops;
+
+	supply = kzalloc(sizeof(*supply) * num_supplies, GFP_KERNEL);
+	if (!supply)
+		goto err_alloc;
+	memcpy(supply, __supply, sizeof(*supply) * num_supplies);
+
+	dummy->initdata.num_consumer_supplies = num_supplies;
+	dummy->initdata.consumer_supplies = supply,
+
+	dummy->rdev = regulator_register(&dummy->desc, NULL,
+						  &dummy->initdata, NULL);
+	if (!dummy->rdev)
+		goto err_register;
+
+	return dummy;
+
+err_register:
+	kfree(supply);
+err_alloc:
+	kfree(dummy);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(regulator_register_dummy);
+
+/**
+ * regulator_unregister_dummy - unregister a dummy supply
+ *
+ * This function unregisters a supply previously registered
+ * with regulator_register_dummy.
+ */
+void regulator_unregister_dummy(struct dummy_regulator *dummy)
+{
+	regulator_unregister(dummy->rdev);
+	kfree(dummy->initdata.consumer_supplies);
+	kfree(dummy);
+}
+EXPORT_SYMBOL_GPL(regulator_unregister_dummy);
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index ce3127a..13245cb 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -186,12 +186,20 @@ struct regulator_init_data {
 	void *driver_data;	/* core does not touch this */
 };
 
+/**
+ * cookie for regulator_register_dummy
+ */
+struct dummy_regulator;
+
 int regulator_suspend_prepare(suspend_state_t state);
 int regulator_suspend_finish(void);
 
 #ifdef CONFIG_REGULATOR
 void regulator_has_full_constraints(void);
 void regulator_use_dummy_regulator(void);
+struct dummy_regulator *regulator_register_dummy(
+		struct regulator_consumer_supply *, int);
+void regulator_unregister_dummy(struct dummy_regulator *);
 #else
 static inline void regulator_has_full_constraints(void)
 {
@@ -200,6 +208,16 @@ static inline void regulator_has_full_constraints(void)
 static inline void regulator_use_dummy_regulator(void)
 {
 }
+
+static inline struct dummy_regulator *regulator_register_dummy(
+		struct regulator_consumer_supply *, int)
+{
+	return (void *)-1;
+};
+
+void regulator_unregister_dummy(struct dummy_regulator *)
+{
+}
 #endif
 
 #endif
-- 
1.7.7


^ permalink raw reply related

* [PATCH] regulator: provide dummy supply support
From: Sascha Hauer @ 2011-10-28 20:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: Liam Girdwood, Mark Brown, Linus Walleij, Paul Mundt,
	Tony Lindgren, Mike Frysinger, linux-sh, linux-omap,
	uclinux-dist-devel

Hi,

The following patch allows it for boards to register a dummy supply for
devices for which no software controllable regulator is available. The
current CONFIG_REGULATOR_DUMMY and (unused) regulator_use_dummy_regulator()
mechanisms only allow to provide a dummy regulator for *all* devices
for which no regulator is found. This would shadow real errors and thus
is not really an option for production kernels.

This patch may eventually superseed the current all-or-nothing approach.

This came to me while looking over Linus' smsc911x regulator support
patch and so I added all people affected by this patch to Cc.

Sascha


^ permalink raw reply

* Re: [PATCH v9 2/4] cpuidle: Remove CPUIDLE_FLAG_IGNORE and dev->prepare()
From: Arjan van de Ven @ 2011-10-28 14:24 UTC (permalink / raw)
  To: Deepthi Dharwar
  Cc: khilman, venki, ak, len.brown, peterz, linux-pm, linux-kernel,
	linux-acpi, linux-sh, linux-pm, linux-omap, linux-arm-kernel
In-Reply-To: <20111028105020.7520.68014.stgit@localhost6.localdomain6>


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
 
On 10/28/2011 3:50 AM, Deepthi Dharwar wrote:
> The cpuidle_device->prepare() mechanism causes updates to the
> cpuidle_state[].flags, setting and clearing CPUIDLE_FLAG_IGNORE
> to tell the governor not to chose a state on a per-cpu basis at
> run-time. State demotion is now handled by the driver and it returns
> the actual state entered. Hence, this mechanism is not required.
> Also this removes per-cpu flags from cpuidle_state enabling
> it to be made global.
>
 
having worked on some newer platforms....
this one is really still needed. doing this inside the actual states
does not work,
because if the deepest 3 states are invalid, the same (somewhat
expensive) test would have to be done 3 times,
and each of the states would have to fail before the 4th one gets chosen.
that's just not going to work
 
(in the state handler you can't know what other state to fall back to,
and especially not how to enter such a fallback state)
 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
 
iQEcBAEBAgAGBQJOqrsGAAoJEEHdSxh4DVnEu7EH/i5lEJctBAIubJOcZz/tvBFp
XYmAe/HqNtSXeHOVsJkTf8y4ppE8487exF7xxMik4GRN0CZNCtkyMezqDVu+eDim
O/UUbScsAc5cSY6mkjOFXLFup+mi1nkRUnAbxXEyTMhWwcbfr2OvcuO7l7TmATML
hu87P3PVEafEop3q2+uWMc57fFxnNFfEDqRx6N9V+OJKV5dHrRYL4G4E01fYGFLo
xTR0IW7nB15L0C29zk9uk/Dqow8SoJZA83c7p7AieP5zdntb6p7noIf03qmdp19f
fulwMwembCHivo+pLO+jAMhKD1T6VYoCyiYW0LHrQ2E07fayBhFJCxlazgKFcl0=FL6o
-----END PGP SIGNATURE-----


^ permalink raw reply

* Hi
From: lisa hedstrand @ 2011-10-28 11:32 UTC (permalink / raw)
  To: linux-sh

My name is Miss Lisa Please accept my apology if my mode of contacting you will in any way offend you. I am compelled to contact you via this medium because i needed a friend from that part of the world. We will get to know each other in details if my proposition accepted. l am a student in UK but originally from USA.

^ permalink raw reply

* [PATCH v9 0/4] cpuidle: Global registration of idle states with
From: Deepthi Dharwar @ 2011-10-28 10:54 UTC (permalink / raw)
  To: khilman, venki, ak, len.brown, peterz, rjw, santosh.shilimkar,
	arjan, lenb
  Cc: linux-pm, linux-sh, linux-kernel, linux-acpi, linux-pm,
	linux-omap, linux-arm-kernel

Version 6 of this patch series dated 22nd Sept 2011 was 
Acked-by: Arjan van de Ven <arjan@linux.intel.com>
Acked-by: Kevin Hilman <khilman@ti.com> for OMAP specific parts.
Reviewed-by: Kevin Hilman <khilman@ti.com>
Tested-by: Jean Pihet <j-pihet@ti.com>

Hi Len, 
Can you please queue this series for the next mainline merge window by merging
it into your development tree and also linux-next for further testing.

I have also tested it by applying these patches on your ACPI tree hosted @github.

Thanks
-Deepthi

--

The following patch series implements global registration of cpuidle
states, and also has the necessary data structure changes to
accommodate the per-cpu writable members of the cpuidle_states
structure.

This patch series had been in discussion earlier and
following are the links to the previous discussions.

v8 --> https://lkml.org/lkml/2011/10/3/52
v7 --> https://lkml.org/lkml/2011/9/27/74 
v6 --> https://lkml.org/lkml/2011/9/22/58
v5 --> https://lkml.org/lkml/2011/6/6/259
v4 --> https://lkml.org/lkml/2011/4/28/312
v3 --> https://lkml.org/lkml/2011/2/8/73
v2 --> https://lkml.org/lkml/2011/1/13/98
v1 --> https://lkml.org/lkml/2011/3/22/161

Changes from previous version (v8):

   1. Rebased and tested on 3.1

Tests done:

    1. Compile tested for ARM using the following configs: da8xx_omapl_defconfig,
    kirkwood_defconfig, omap2plus_defconfig, at91rm9200_defconfig
      
    2. Boot tested on x86 nehalem with multiple C-states for both intel_idle
    and acpi_idle drivers.

    3. Boot tested on T60p thinkpad that has T2600 cpu with multiple C-states. 
    Also tested the case when there is dynamic changes in C-states 
    AC <-> Battery Power switch.

Brief description of the patches:

Core change in this series is to split the cpuidle_device structure 
into two parts, i.e  global and per-cpu basis. 

The per-cpu pieces are mostly generic statistics that can be independent 
of current running driver. As a result of these changes, there is single 
copy of cpuidle_states structure and single registration done by one 
cpu. The low level driver is free to set per-cpu driver data on
each cpu if needed using the cpuidle_set_statedata() as the case
today. Only in very rare cases asymmetric C-states exist which can be 
handled within the cpuidle driver. Most architectures do not have 
asymmetric C-states.

First two patches in the series facilitate splitting of cpuidle_states
and cpuidle_device structure and next two patches do the actual split,
change the API's and make existing code follow the changed API.

[1/4] - Move the idle residency accounting part from cpuidle.c to
the respective low level drivers, so that the accounting can
be accurately maintained if the driver decides to demote the
chosen (suggested) by the governor.

[2/4] - removes the cpuidle_device()->prepare API since is is not
widely used and the only use case was to allow software
demotion using CPUIDLE_FLAG_IGNORE flag.  Both these
functions can be absorbed within the cpuidle back-end
driver ad hence deprecating the prepare routine and the
CPUIDLE_FLAG_IGNORE flag.

    - Ref: https://lkml.org/lkml/2011/3/25/52

[3/4] - Splits the usage statistics (read/write) part out of
cpuidle_state structure, so that the states can become read
only and hence made global.

[4/4] - Most APIs will now need to pass pointer to both global
cpuidle_driver and per-cpu cpuidle_device structure.

 arch/arm/mach-at91/cpuidle.c          |   41 +++--
 arch/arm/mach-davinci/cpuidle.c       |   51 ++++---
 arch/arm/mach-exynos4/cpuidle.c       |   30 ++--
 arch/arm/mach-kirkwood/cpuidle.c      |   42 +++---
 arch/arm/mach-omap2/cpuidle34xx.c     |  133 +++++++++++------
 arch/sh/kernel/cpu/shmobile/cpuidle.c |   28 ++--
 drivers/acpi/processor_driver.c       |   20 ---
 drivers/acpi/processor_idle.c         |  251 +++++++++++++++++++++++++++------
 drivers/cpuidle/cpuidle.c             |   86 ++++-------
 drivers/cpuidle/driver.c              |   25 +++
 drivers/cpuidle/governors/ladder.c    |   41 ++++-
 drivers/cpuidle/governors/menu.c      |   29 ++--
 drivers/cpuidle/sysfs.c               |   22 ++-
 drivers/idle/intel_idle.c             |  130 +++++++++++++----
 include/acpi/processor.h              |    1 
 include/linux/cpuidle.h               |   52 ++++---
 16 files changed, 650 insertions(+), 332 deletions(-)


-- 

-Deepthi

^ permalink raw reply

* [PATCH v9 1/4] cpuidle: Move dev->last_residency update to driver
From: Deepthi Dharwar @ 2011-10-28 10:54 UTC (permalink / raw)
  To: khilman, venki, ak, len.brown, peterz, rjw, santosh.shilimkar,
	arjan, lenb
  Cc: linux-sh, linux-pm, linux-kernel, linux-acpi, linux-pm,
	linux-omap, linux-arm-kernel
In-Reply-To: <20111028104945.7520.83828.stgit@localhost6.localdomain6>

Cpuidle governor only suggests the state to enter using the
governor->select() interface, but allows the low level driver to
override the recommended state. The actual entered state
may be different because of software or hardware demotion. Software
demotion is done by the back-end cpuidle driver and can be accounted
correctly. Current cpuidle code uses last_state field to capture the
actual state entered and based on that updates the statistics for the
state entered.

Ideally the driver enter routine should update the counters,
and it should return the state actually entered rather than the time
spent there. The generic cpuidle code should simply handle where
the counters live in the sysfs namespace, not updating the counters.

Reference:
https://lkml.org/lkml/2011/3/25/52

Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
Signed-off-by: Trinabh Gupta <g.trinabh@gmail.com>
Tested-by: Jean Pihet <j-pihet@ti.com>
Reviewed-by: Kevin Hilman <khilman@ti.com>
Acked-by: Arjan van de Ven <arjan@linux.intel.com>
Acked-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-at91/cpuidle.c          |   10 +++-
 arch/arm/mach-davinci/cpuidle.c       |    9 +++-
 arch/arm/mach-exynos4/cpuidle.c       |    7 ++-
 arch/arm/mach-kirkwood/cpuidle.c      |   12 ++++-
 arch/arm/mach-omap2/cpuidle34xx.c     |   67 +++++++++++++++++------------
 arch/sh/kernel/cpu/shmobile/cpuidle.c |   12 +++--
 drivers/acpi/processor_idle.c         |   75 ++++++++++++++++++++++-----------
 drivers/cpuidle/cpuidle.c             |   32 +++++++-------
 drivers/cpuidle/governors/ladder.c    |   13 ++++++
 drivers/cpuidle/governors/menu.c      |    7 ++-
 drivers/idle/intel_idle.c             |   12 ++++-
 include/linux/cpuidle.h               |    7 +--
 12 files changed, 164 insertions(+), 99 deletions(-)

diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
index 1cfeac1..4696a0d 100644
--- a/arch/arm/mach-at91/cpuidle.c
+++ b/arch/arm/mach-at91/cpuidle.c
@@ -33,7 +33,7 @@ static struct cpuidle_driver at91_idle_driver = {
 
 /* Actual code that puts the SoC in different idle states */
 static int at91_enter_idle(struct cpuidle_device *dev,
-			       struct cpuidle_state *state)
+			       int index)
 {
 	struct timeval before, after;
 	int idle_time;
@@ -41,10 +41,10 @@ static int at91_enter_idle(struct cpuidle_device *dev,
 
 	local_irq_disable();
 	do_gettimeofday(&before);
-	if (state = &dev->states[0])
+	if (index = 0)
 		/* Wait for interrupt state */
 		cpu_do_idle();
-	else if (state = &dev->states[1]) {
+	else if (index = 1) {
 		asm("b 1f; .align 5; 1:");
 		asm("mcr p15, 0, r0, c7, c10, 4");	/* drain write buffer */
 		saved_lpr = sdram_selfrefresh_enable();
@@ -55,7 +55,9 @@ static int at91_enter_idle(struct cpuidle_device *dev,
 	local_irq_enable();
 	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
 			(after.tv_usec - before.tv_usec);
-	return idle_time;
+
+	dev->last_residency = idle_time;
+	return index;
 }
 
 /* Initialize CPU idle by registering the idle states */
diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index bd59f31..ca8582a 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -78,9 +78,9 @@ static struct davinci_ops davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
 
 /* Actual code that puts the SoC in different idle states */
 static int davinci_enter_idle(struct cpuidle_device *dev,
-						struct cpuidle_state *state)
+						int index)
 {
-	struct davinci_ops *ops = cpuidle_get_statedata(state);
+	struct davinci_ops *ops = cpuidle_get_statedata(&dev->states[index]);
 	struct timeval before, after;
 	int idle_time;
 
@@ -98,7 +98,10 @@ static int davinci_enter_idle(struct cpuidle_device *dev,
 	local_irq_enable();
 	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
 			(after.tv_usec - before.tv_usec);
-	return idle_time;
+
+	dev->last_residency = idle_time;
+
+	return index;
 }
 
 static int __init davinci_cpuidle_probe(struct platform_device *pdev)
diff --git a/arch/arm/mach-exynos4/cpuidle.c b/arch/arm/mach-exynos4/cpuidle.c
index bf7e96f..ea026e7 100644
--- a/arch/arm/mach-exynos4/cpuidle.c
+++ b/arch/arm/mach-exynos4/cpuidle.c
@@ -16,7 +16,7 @@
 #include <asm/proc-fns.h>
 
 static int exynos4_enter_idle(struct cpuidle_device *dev,
-			      struct cpuidle_state *state);
+			      int index);
 
 static struct cpuidle_state exynos4_cpuidle_set[] = {
 	[0] = {
@@ -37,7 +37,7 @@ static struct cpuidle_driver exynos4_idle_driver = {
 };
 
 static int exynos4_enter_idle(struct cpuidle_device *dev,
-			      struct cpuidle_state *state)
+			      int index)
 {
 	struct timeval before, after;
 	int idle_time;
@@ -52,7 +52,8 @@ static int exynos4_enter_idle(struct cpuidle_device *dev,
 	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
 		    (after.tv_usec - before.tv_usec);
 
-	return idle_time;
+	dev->last_residency = idle_time;
+	return index;
 }
 
 static int __init exynos4_init_cpuidle(void)
diff --git a/arch/arm/mach-kirkwood/cpuidle.c b/arch/arm/mach-kirkwood/cpuidle.c
index f68d33f..358dd80 100644
--- a/arch/arm/mach-kirkwood/cpuidle.c
+++ b/arch/arm/mach-kirkwood/cpuidle.c
@@ -32,17 +32,17 @@ static DEFINE_PER_CPU(struct cpuidle_device, kirkwood_cpuidle_device);
 
 /* Actual code that puts the SoC in different idle states */
 static int kirkwood_enter_idle(struct cpuidle_device *dev,
-			       struct cpuidle_state *state)
+			       int index)
 {
 	struct timeval before, after;
 	int idle_time;
 
 	local_irq_disable();
 	do_gettimeofday(&before);
-	if (state = &dev->states[0])
+	if (index = 0)
 		/* Wait for interrupt state */
 		cpu_do_idle();
-	else if (state = &dev->states[1]) {
+	else if (index = 1) {
 		/*
 		 * Following write will put DDR in self refresh.
 		 * Note that we have 256 cycles before DDR puts it
@@ -57,7 +57,11 @@ static int kirkwood_enter_idle(struct cpuidle_device *dev,
 	local_irq_enable();
 	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
 			(after.tv_usec - before.tv_usec);
-	return idle_time;
+
+	/* Update last residency */
+	dev->last_residency = idle_time;
+
+	return index;
 }
 
 /* Initialize CPU idle by registering the idle states */
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 4bf6e6e..58425c7 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -88,17 +88,19 @@ static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
 /**
  * omap3_enter_idle - Programs OMAP3 to enter the specified state
  * @dev: cpuidle device
- * @state: The target state to be programmed
+ * @index: the index of state to be entered
  *
  * Called from the CPUidle framework to program the device to the
  * specified target state selected by the governor.
  */
 static int omap3_enter_idle(struct cpuidle_device *dev,
-			struct cpuidle_state *state)
+				int index)
 {
-	struct omap3_idle_statedata *cx = cpuidle_get_statedata(state);
+	struct omap3_idle_statedata *cx +			cpuidle_get_statedata(&dev->states[index]);
 	struct timespec ts_preidle, ts_postidle, ts_idle;
 	u32 mpu_state = cx->mpu_state, core_state = cx->core_state;
+	int idle_time;
 
 	/* Used to keep track of the total time in idle */
 	getnstimeofday(&ts_preidle);
@@ -113,7 +115,7 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
 		goto return_sleep_time;
 
 	/* Deny idle for C1 */
-	if (state = &dev->states[0]) {
+	if (index = 0) {
 		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
 		pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
 	}
@@ -122,7 +124,7 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
 	omap_sram_idle();
 
 	/* Re-allow idle for C1 */
-	if (state = &dev->states[0]) {
+	if (index = 0) {
 		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
 		pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
 	}
@@ -134,28 +136,35 @@ return_sleep_time:
 	local_irq_enable();
 	local_fiq_enable();
 
-	return ts_idle.tv_nsec / NSEC_PER_USEC + ts_idle.tv_sec * USEC_PER_SEC;
+	idle_time = ts_idle.tv_nsec / NSEC_PER_USEC + ts_idle.tv_sec * \
+								USEC_PER_SEC;
+
+	/* Update cpuidle counters */
+	dev->last_residency = idle_time;
+
+	return index;
 }
 
 /**
  * next_valid_state - Find next valid C-state
  * @dev: cpuidle device
- * @state: Currently selected C-state
+ * @index: Index of currently selected c-state
  *
- * If the current state is valid, it is returned back to the caller.
- * Else, this function searches for a lower c-state which is still
- * valid.
+ * If the state corresponding to index is valid, index is returned back
+ * to the caller. Else, this function searches for a lower c-state which is
+ * still valid (as defined in omap3_power_states[]) and returns its index.
  *
  * A state is valid if the 'valid' field is enabled and
  * if it satisfies the enable_off_mode condition.
  */
-static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
-					      struct cpuidle_state *curr)
+static int next_valid_state(struct cpuidle_device *dev,
+				int index)
 {
-	struct cpuidle_state *next = NULL;
+	struct cpuidle_state *curr = &dev->states[index];
 	struct omap3_idle_statedata *cx = cpuidle_get_statedata(curr);
 	u32 mpu_deepest_state = PWRDM_POWER_RET;
 	u32 core_deepest_state = PWRDM_POWER_RET;
+	int next_index = -1;
 
 	if (enable_off_mode) {
 		mpu_deepest_state = PWRDM_POWER_OFF;
@@ -172,20 +181,20 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
 	if ((cx->valid) &&
 	    (cx->mpu_state >= mpu_deepest_state) &&
 	    (cx->core_state >= core_deepest_state)) {
-		return curr;
+		return index;
 	} else {
 		int idx = OMAP3_NUM_STATES - 1;
 
 		/* Reach the current state starting at highest C-state */
 		for (; idx >= 0; idx--) {
 			if (&dev->states[idx] = curr) {
-				next = &dev->states[idx];
+				next_index = idx;
 				break;
 			}
 		}
 
 		/* Should never hit this condition */
-		WARN_ON(next = NULL);
+		WARN_ON(next_index = -1);
 
 		/*
 		 * Drop to next valid state.
@@ -197,37 +206,39 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
 			if ((cx->valid) &&
 			    (cx->mpu_state >= mpu_deepest_state) &&
 			    (cx->core_state >= core_deepest_state)) {
-				next = &dev->states[idx];
+				next_index = idx;
 				break;
 			}
 		}
 		/*
 		 * C1 is always valid.
-		 * So, no need to check for 'next=NULL' outside this loop.
+		 * So, no need to check for 'next_index = -1' outside
+		 * this loop.
 		 */
 	}
 
-	return next;
+	return next_index;
 }
 
 /**
  * omap3_enter_idle_bm - Checks for any bus activity
  * @dev: cpuidle device
- * @state: The target state to be programmed
+ * @index: array index of target state to be programmed
  *
  * This function checks for any pending activity and then programs
  * the device to the specified or a safer state.
  */
 static int omap3_enter_idle_bm(struct cpuidle_device *dev,
-			       struct cpuidle_state *state)
+			       int index)
 {
-	struct cpuidle_state *new_state;
+	struct cpuidle_state *state = &dev->states[index];
+	int new_state_idx;
 	u32 core_next_state, per_next_state = 0, per_saved_state = 0, cam_state;
 	struct omap3_idle_statedata *cx;
 	int ret;
 
 	if (!omap3_can_sleep()) {
-		new_state = dev->safe_state;
+		new_state_idx = dev->safe_state_index;
 		goto select_state;
 	}
 
@@ -237,7 +248,7 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 	 */
 	cam_state = pwrdm_read_pwrst(cam_pd);
 	if (cam_state = PWRDM_POWER_ON) {
-		new_state = dev->safe_state;
+		new_state_idx = dev->safe_state_index;
 		goto select_state;
 	}
 
@@ -264,11 +275,10 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 	if (per_next_state != per_saved_state)
 		pwrdm_set_next_pwrst(per_pd, per_next_state);
 
-	new_state = next_valid_state(dev, state);
+	new_state_idx = next_valid_state(dev, index);
 
 select_state:
-	dev->last_state = new_state;
-	ret = omap3_enter_idle(dev, new_state);
+	ret = omap3_enter_idle(dev, new_state_idx);
 
 	/* Restore original PER state if it was modified */
 	if (per_next_state != per_saved_state)
@@ -339,11 +349,12 @@ int __init omap3_idle_init(void)
 
 	cpuidle_register_driver(&omap3_idle_driver);
 	dev = &per_cpu(omap3_idle_dev, smp_processor_id());
+	dev->safe_state_index = -1;
 
 	/* C1 . MPU WFI + Core active */
 	cx = _fill_cstate(dev, 0, "MPU ON + CORE ON");
 	(&dev->states[0])->enter = omap3_enter_idle;
-	dev->safe_state = &dev->states[0];
+	dev->safe_state_index = 0;
 	cx->valid = 1;	/* C1 is always valid */
 	cx->mpu_state = PWRDM_POWER_ON;
 	cx->core_state = PWRDM_POWER_ON;
diff --git a/arch/sh/kernel/cpu/shmobile/cpuidle.c b/arch/sh/kernel/cpu/shmobile/cpuidle.c
index e4469e72..7be50d4c 100644
--- a/arch/sh/kernel/cpu/shmobile/cpuidle.c
+++ b/arch/sh/kernel/cpu/shmobile/cpuidle.c
@@ -25,11 +25,11 @@ static unsigned long cpuidle_mode[] = {
 };
 
 static int cpuidle_sleep_enter(struct cpuidle_device *dev,
-			       struct cpuidle_state *state)
+				int index)
 {
 	unsigned long allowed_mode = arch_hwblk_sleep_mode();
 	ktime_t before, after;
-	int requested_state = state - &dev->states[0];
+	int requested_state = index;
 	int allowed_state;
 	int k;
 
@@ -46,11 +46,13 @@ static int cpuidle_sleep_enter(struct cpuidle_device *dev,
 	 */
 	k = min_t(int, allowed_state, requested_state);
 
-	dev->last_state = &dev->states[k];
 	before = ktime_get();
 	sh_mobile_call_standby(cpuidle_mode[k]);
 	after = ktime_get();
-	return ktime_to_ns(ktime_sub(after, before)) >> 10;
+
+	dev->last_residency = (int)ktime_to_ns(ktime_sub(after, before)) >> 10;
+
+	return k;
 }
 
 static struct cpuidle_device cpuidle_dev;
@@ -84,7 +86,7 @@ void sh_mobile_setup_cpuidle(void)
 	state->flags |= CPUIDLE_FLAG_TIME_VALID;
 	state->enter = cpuidle_sleep_enter;
 
-	dev->safe_state = state;
+	dev->safe_state_index = i-1;
 
 	if (sh_mobile_sleep_supported & SUSP_SH_SF) {
 		state = &dev->states[i++];
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 9b88f98..463cc09 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -741,22 +741,24 @@ static inline void acpi_idle_do_entry(struct acpi_processor_cx *cx)
 /**
  * acpi_idle_enter_c1 - enters an ACPI C1 state-type
  * @dev: the target CPU
- * @state: the state data
+ * @index: index of target state
  *
  * This is equivalent to the HALT instruction.
  */
 static int acpi_idle_enter_c1(struct cpuidle_device *dev,
-			      struct cpuidle_state *state)
+				int index)
 {
 	ktime_t  kt1, kt2;
 	s64 idle_time;
 	struct acpi_processor *pr;
+	struct cpuidle_state *state = &dev->states[index];
 	struct acpi_processor_cx *cx = cpuidle_get_statedata(state);
 
 	pr = __this_cpu_read(processors);
+	dev->last_residency = 0;
 
 	if (unlikely(!pr))
-		return 0;
+		return -EINVAL;
 
 	local_irq_disable();
 
@@ -764,7 +766,7 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
 	if (acpi_idle_suspend) {
 		local_irq_enable();
 		cpu_relax();
-		return 0;
+		return -EINVAL;
 	}
 
 	lapic_timer_state_broadcast(pr, cx, 1);
@@ -773,37 +775,46 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
 	kt2 = ktime_get_real();
 	idle_time =  ktime_to_us(ktime_sub(kt2, kt1));
 
+	/* Update device last_residency*/
+	dev->last_residency = (int)idle_time;
+
 	local_irq_enable();
 	cx->usage++;
 	lapic_timer_state_broadcast(pr, cx, 0);
 
-	return idle_time;
+	return index;
 }
 
 /**
  * acpi_idle_enter_simple - enters an ACPI state without BM handling
  * @dev: the target CPU
- * @state: the state data
+ * @index: the index of suggested state
  */
 static int acpi_idle_enter_simple(struct cpuidle_device *dev,
-				  struct cpuidle_state *state)
+				int index)
 {
 	struct acpi_processor *pr;
+	struct cpuidle_state *state = &dev->states[index];
 	struct acpi_processor_cx *cx = cpuidle_get_statedata(state);
 	ktime_t  kt1, kt2;
 	s64 idle_time_ns;
 	s64 idle_time;
 
 	pr = __this_cpu_read(processors);
+	dev->last_residency = 0;
 
 	if (unlikely(!pr))
-		return 0;
-
-	if (acpi_idle_suspend)
-		return(acpi_idle_enter_c1(dev, state));
+		return -EINVAL;
 
 	local_irq_disable();
 
+	if (acpi_idle_suspend) {
+		local_irq_enable();
+		cpu_relax();
+		return -EINVAL;
+	}
+
+
 	if (cx->entry_method != ACPI_CSTATE_FFH) {
 		current_thread_info()->status &= ~TS_POLLING;
 		/*
@@ -815,7 +826,7 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
 		if (unlikely(need_resched())) {
 			current_thread_info()->status |= TS_POLLING;
 			local_irq_enable();
-			return 0;
+			return -EINVAL;
 		}
 	}
 
@@ -837,6 +848,9 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
 	idle_time = idle_time_ns;
 	do_div(idle_time, NSEC_PER_USEC);
 
+	/* Update device last_residency*/
+	dev->last_residency = (int)idle_time;
+
 	/* Tell the scheduler how much we idled: */
 	sched_clock_idle_wakeup_event(idle_time_ns);
 
@@ -848,7 +862,7 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
 
 	lapic_timer_state_broadcast(pr, cx, 0);
 	cx->time += idle_time;
-	return idle_time;
+	return index;
 }
 
 static int c3_cpu_count;
@@ -857,14 +871,15 @@ static DEFINE_RAW_SPINLOCK(c3_lock);
 /**
  * acpi_idle_enter_bm - enters C3 with proper BM handling
  * @dev: the target CPU
- * @state: the state data
+ * @index: the index of suggested state
  *
  * If BM is detected, the deepest non-C3 idle state is entered instead.
  */
 static int acpi_idle_enter_bm(struct cpuidle_device *dev,
-			      struct cpuidle_state *state)
+				int index)
 {
 	struct acpi_processor *pr;
+	struct cpuidle_state *state = &dev->states[index];
 	struct acpi_processor_cx *cx = cpuidle_get_statedata(state);
 	ktime_t  kt1, kt2;
 	s64 idle_time_ns;
@@ -872,22 +887,26 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
 
 
 	pr = __this_cpu_read(processors);
+	dev->last_residency = 0;
 
 	if (unlikely(!pr))
-		return 0;
+		return -EINVAL;
 
-	if (acpi_idle_suspend)
-		return(acpi_idle_enter_c1(dev, state));
+
+	if (acpi_idle_suspend) {
+		cpu_relax();
+		return -EINVAL;
+	}
 
 	if (!cx->bm_sts_skip && acpi_idle_bm_check()) {
-		if (dev->safe_state) {
-			dev->last_state = dev->safe_state;
-			return dev->safe_state->enter(dev, dev->safe_state);
+		if (dev->safe_state_index >= 0) {
+			return dev->states[dev->safe_state_index].enter(dev,
+						dev->safe_state_index);
 		} else {
 			local_irq_disable();
 			acpi_safe_halt();
 			local_irq_enable();
-			return 0;
+			return -EINVAL;
 		}
 	}
 
@@ -904,7 +923,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
 		if (unlikely(need_resched())) {
 			current_thread_info()->status |= TS_POLLING;
 			local_irq_enable();
-			return 0;
+			return -EINVAL;
 		}
 	}
 
@@ -954,6 +973,9 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
 	idle_time = idle_time_ns;
 	do_div(idle_time, NSEC_PER_USEC);
 
+	/* Update device last_residency*/
+	dev->last_residency = (int)idle_time;
+
 	/* Tell the scheduler how much we idled: */
 	sched_clock_idle_wakeup_event(idle_time_ns);
 
@@ -965,7 +987,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
 
 	lapic_timer_state_broadcast(pr, cx, 0);
 	cx->time += idle_time;
-	return idle_time;
+	return index;
 }
 
 struct cpuidle_driver acpi_idle_driver = {
@@ -992,6 +1014,7 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 	}
 
 	dev->cpu = pr->id;
+	dev->safe_state_index = -1;
 	for (i = 0; i < CPUIDLE_STATE_MAX; i++) {
 		dev->states[i].name[0] = '\0';
 		dev->states[i].desc[0] = '\0';
@@ -1027,13 +1050,13 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 				state->flags |= CPUIDLE_FLAG_TIME_VALID;
 
 			state->enter = acpi_idle_enter_c1;
-			dev->safe_state = state;
+			dev->safe_state_index = count;
 			break;
 
 			case ACPI_STATE_C2:
 			state->flags |= CPUIDLE_FLAG_TIME_VALID;
 			state->enter = acpi_idle_enter_simple;
-			dev->safe_state = state;
+			dev->safe_state_index = count;
 			break;
 
 			case ACPI_STATE_C3:
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 0df0141..8faf3a6 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -62,7 +62,7 @@ int cpuidle_idle_call(void)
 {
 	struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
 	struct cpuidle_state *target_state;
-	int next_state;
+	int next_state, entered_state;
 
 	if (off)
 		return -ENODEV;
@@ -102,26 +102,27 @@ int cpuidle_idle_call(void)
 
 	target_state = &dev->states[next_state];
 
-	/* enter the state and update stats */
-	dev->last_state = target_state;
-
 	trace_power_start(POWER_CSTATE, next_state, dev->cpu);
 	trace_cpu_idle(next_state, dev->cpu);
 
-	dev->last_residency = target_state->enter(dev, target_state);
+	entered_state = target_state->enter(dev, next_state);
 
 	trace_power_end(dev->cpu);
 	trace_cpu_idle(PWR_EVENT_EXIT, dev->cpu);
 
-	if (dev->last_state)
-		target_state = dev->last_state;
-
-	target_state->time += (unsigned long long)dev->last_residency;
-	target_state->usage++;
+	if (entered_state >= 0) {
+		/* Update cpuidle counters */
+		/* This can be moved to within driver enter routine
+		 * but that results in multiple copies of same code.
+		 */
+		dev->states[entered_state].time ++				(unsigned long long)dev->last_residency;
+		dev->states[entered_state].usage++;
+	}
 
 	/* give the governor an opportunity to reflect on the outcome */
 	if (cpuidle_curr_governor->reflect)
-		cpuidle_curr_governor->reflect(dev);
+		cpuidle_curr_governor->reflect(dev, entered_state);
 
 	return 0;
 }
@@ -172,11 +173,10 @@ void cpuidle_resume_and_unlock(void)
 EXPORT_SYMBOL_GPL(cpuidle_resume_and_unlock);
 
 #ifdef CONFIG_ARCH_HAS_CPU_RELAX
-static int poll_idle(struct cpuidle_device *dev, struct cpuidle_state *st)
+static int poll_idle(struct cpuidle_device *dev, int index)
 {
 	ktime_t	t1, t2;
 	s64 diff;
-	int ret;
 
 	t1 = ktime_get();
 	local_irq_enable();
@@ -188,8 +188,9 @@ static int poll_idle(struct cpuidle_device *dev, struct cpuidle_state *st)
 	if (diff > INT_MAX)
 		diff = INT_MAX;
 
-	ret = (int) diff;
-	return ret;
+	dev->last_residency = (int) diff;
+
+	return index;
 }
 
 static void poll_idle_init(struct cpuidle_device *dev)
@@ -248,7 +249,6 @@ int cpuidle_enable_device(struct cpuidle_device *dev)
 		dev->states[i].time = 0;
 	}
 	dev->last_residency = 0;
-	dev->last_state = NULL;
 
 	smp_wmb();
 
diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
index f62fde2..78b06d2 100644
--- a/drivers/cpuidle/governors/ladder.c
+++ b/drivers/cpuidle/governors/ladder.c
@@ -153,11 +153,24 @@ static int ladder_enable_device(struct cpuidle_device *dev)
 	return 0;
 }
 
+/**
+ * ladder_reflect - update the correct last_state_idx
+ * @dev: the CPU
+ * @index: the index of actual state entered
+ */
+static void ladder_reflect(struct cpuidle_device *dev, int index)
+{
+	struct ladder_device *ldev = &__get_cpu_var(ladder_devices);
+	if (index > 0)
+		ldev->last_state_idx = index;
+}
+
 static struct cpuidle_governor ladder_governor = {
 	.name =		"ladder",
 	.rating =	10,
 	.enable =	ladder_enable_device,
 	.select =	ladder_select_state,
+	.reflect =	ladder_reflect,
 	.owner =	THIS_MODULE,
 };
 
diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index 3600f19..3c44c53 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -310,14 +310,17 @@ static int menu_select(struct cpuidle_device *dev)
 /**
  * menu_reflect - records that data structures need update
  * @dev: the CPU
+ * @index: the index of actual entered state
  *
  * NOTE: it's important to be fast here because this operation will add to
  *       the overall exit latency.
  */
-static void menu_reflect(struct cpuidle_device *dev)
+static void menu_reflect(struct cpuidle_device *dev, int index)
 {
 	struct menu_device *data = &__get_cpu_var(menu_devices);
-	data->needs_update = 1;
+	data->last_state_idx = index;
+	if (index >= 0)
+		data->needs_update = 1;
 }
 
 /**
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index a46dddf..a1c888d 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -81,7 +81,7 @@ static unsigned int mwait_substates;
 static unsigned int lapic_timer_reliable_states = (1 << 1);	 /* Default to only C1 */
 
 static struct cpuidle_device __percpu *intel_idle_cpuidle_devices;
-static int intel_idle(struct cpuidle_device *dev, struct cpuidle_state *state);
+static int intel_idle(struct cpuidle_device *dev, int index);
 
 static struct cpuidle_state *cpuidle_state_table;
 
@@ -209,12 +209,13 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = {
 /**
  * intel_idle
  * @dev: cpuidle_device
- * @state: cpuidle state
+ * @index: index of cpuidle state
  *
  */
-static int intel_idle(struct cpuidle_device *dev, struct cpuidle_state *state)
+static int intel_idle(struct cpuidle_device *dev, int index)
 {
 	unsigned long ecx = 1; /* break on interrupt flag */
+	struct cpuidle_state *state = &dev->states[index];
 	unsigned long eax = (unsigned long)cpuidle_get_statedata(state);
 	unsigned int cstate;
 	ktime_t kt_before, kt_after;
@@ -256,7 +257,10 @@ static int intel_idle(struct cpuidle_device *dev, struct cpuidle_state *state)
 	if (!(lapic_timer_reliable_states & (1 << (cstate))))
 		clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
 
-	return usec_delta;
+	/* Update cpuidle counters */
+	dev->last_residency = (int)usec_delta;
+
+	return index;
 }
 
 static void __setup_broadcast_timer(void *arg)
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index b51629e..8da811b 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -42,7 +42,7 @@ struct cpuidle_state {
 	unsigned long long	time; /* in US */
 
 	int (*enter)	(struct cpuidle_device *dev,
-			 struct cpuidle_state *state);
+			int index);
 };
 
 /* Idle State Flags */
@@ -87,13 +87,12 @@ struct cpuidle_device {
 	int			state_count;
 	struct cpuidle_state	states[CPUIDLE_STATE_MAX];
 	struct cpuidle_state_kobj *kobjs[CPUIDLE_STATE_MAX];
-	struct cpuidle_state	*last_state;
 
 	struct list_head 	device_list;
 	struct kobject		kobj;
 	struct completion	kobj_unregister;
 	void			*governor_data;
-	struct cpuidle_state	*safe_state;
+	int			safe_state_index;
 
 	int (*prepare)		(struct cpuidle_device *dev);
 };
@@ -169,7 +168,7 @@ struct cpuidle_governor {
 	void (*disable)		(struct cpuidle_device *dev);
 
 	int  (*select)		(struct cpuidle_device *dev);
-	void (*reflect)		(struct cpuidle_device *dev);
+	void (*reflect)		(struct cpuidle_device *dev, int index);
 
 	struct module 		*owner;
 };


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox