Re: [PATCH/RFC 3/2] OMAP: PM noop: implement context loss count for non-omap_devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

a few more minor comments

On Thu, 9 Dec 2010, Kevin Hilman wrote:

> For devices which have not (yet) been converted to use omap_device,
> implement the context loss counter using the "brutal method" as
> originally proposed by Paul Walmsley[1].
> 
> The dummy context loss counter is incremented every time it is
> checked, but only when off-mode is enabled.  When off-mode is
> disabled, the dummy counter stops incrementing.
> 
> Tested on 36xx/Zoom3 using MMC driver, which is currently the
> only in-tree user of this API.
> 
> This patch should be reverted after all devices are converted to using
> omap_device.
> 
> [1] http://marc.info/?l=linux-omap&m=129176260000626&w=2
> 
> Cc: Paul Walmsley <paul@xxxxxxxxx>
> Signed-off-by: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
> ---
>  arch/arm/mach-omap2/pm-debug.c            |    2 ++
>  arch/arm/plat-omap/include/plat/omap-pm.h |    1 +
>  arch/arm/plat-omap/omap-pm-noop.c         |   19 ++++++++++++++++++-
>  3 files changed, 21 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/pm-debug.c b/arch/arm/mach-omap2/pm-debug.c
> index e535082..af66acb 100644
> --- a/arch/arm/mach-omap2/pm-debug.c
> +++ b/arch/arm/mach-omap2/pm-debug.c
> @@ -32,6 +32,7 @@
>  #include "powerdomain.h"
>  #include "clockdomain.h"
>  #include <plat/dmtimer.h>
> +#include <plat/omap-pm.h>
>  
>  #include "cm2xxx_3xxx.h"
>  #include "prm2xxx_3xxx.h"
> @@ -581,6 +582,7 @@ static int option_set(void *data, u64 val)
>  	*option = val;
>  
>  	if (option == &enable_off_mode) {
> +		omap_pm_enable_off_mode(val ? true : false);
>  		if (cpu_is_omap34xx())
>  			omap3_pm_off_mode_enable(val);
>  	}
> diff --git a/arch/arm/plat-omap/include/plat/omap-pm.h b/arch/arm/plat-omap/include/plat/omap-pm.h
> index 9870b4f..cfa5e44 100644
> --- a/arch/arm/plat-omap/include/plat/omap-pm.h
> +++ b/arch/arm/plat-omap/include/plat/omap-pm.h
> @@ -372,5 +372,6 @@ unsigned long omap_pm_cpu_get_freq(void);
>   */
>  int omap_pm_get_dev_context_loss_count(struct device *dev);
>  
> +void omap_pm_enable_off_mode(bool enable);
>  
>  #endif
> diff --git a/arch/arm/plat-omap/omap-pm-noop.c b/arch/arm/plat-omap/omap-pm-noop.c
> index 5a58f97..32f2818 100644
> --- a/arch/arm/plat-omap/omap-pm-noop.c
> +++ b/arch/arm/plat-omap/omap-pm-noop.c
> @@ -30,6 +30,9 @@ struct omap_opp *dsp_opps;
>  struct omap_opp *mpu_opps;
>  struct omap_opp *l3_opps;
>  
> +static bool off_mode_enabled;
> +static u32 dummy_context_loss_counter;
> +
>  /*
>   * Device-driver-originated constraints (via board-*.c files)
>   */
> @@ -287,6 +290,10 @@ unsigned long omap_pm_cpu_get_freq(void)
>  /*
>   * Device context loss tracking
>   */

Please add some basic kerneldoc here -- maybe just document that it's 
meant to be called from the core OMAP PM or PM debug code whenever 
off-mode is enabled or disabled...

> +void omap_pm_enable_off_mode(bool enable)
> +{
> +	off_mode_enabled = enable;
> +}

Personal preference: please split this into two functions, 
omap_pm_{enable,disable}_off_mode(void).  That way, when I read it in 
other code, I don't have to try to remember what the argument means :-)

>  int omap_pm_get_dev_context_loss_count(struct device *dev)
>  {
> @@ -298,7 +305,17 @@ int omap_pm_get_dev_context_loss_count(struct device *dev)
>  		return -EINVAL;
>  	};
>  
> -	count = omap_device_get_context_loss_count(pdev);
> +	if (dev->parent == &omap_device_parent) {
> +		count = omap_device_get_context_loss_count(pdev);
> +	} else {
> +		WARN_ONCE(off_mode_enabled, "using dummy context loss counter, "
> +			  "device %s should be converted to omap_device.",
> +			  __func__, dev_name(dev));
> +		if (off_mode_enabled)
> +			dummy_context_loss_counter++;
> +		count = dummy_context_loss_counter;
> +	}
> +

This part looks fine to me, thanks for working on this stuff.  Once you 
fix the arithmetic rollover stuff, I think it will be good for 2.6.38.


- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux