[PATCH/RESEND #3 0/7] SmartReflex Patches

[PATCH/RESEND #3 0/7] SmartReflex Patches

[Felipe Balbi]

Hi Tony,

here's a bunch of patches to smartreflex which have been pending
for months.

Compile tested with omap2plus_defconfig + CONFIG_OMAP_SMARTREFLEX +
CONFIG_OMAP_SMARTREFLEX_CLASS3.

Felipe Balbi (6):
  arm: omap: smartreflex: add missing platform_set_drvdata()
  arm: omap: smartreflex: use dev_get_drvdata()
  arm: omap: smartreflex: move late_initcall() closer to its argument
  arm: omap: smartreflex: clean ups all over
  arm: omap: smartreflex: fix IRQ handling bug
  arm: omap: smartreflex: micro-optimization for sanity check

Nishanth Menon (1):
  OMAP3+: PM: SR: add suspend/resume handlers

 arch/arm/mach-omap2/smartreflex.c |  309 ++++++++++++++++++++++++------------
 1 files changed, 206 insertions(+), 103 deletions(-)

-- 
1.7.8.rc0

[PATCH/RESEND #3 1/7] OMAP3+: PM: SR: add suspend/resume handlers

[Felipe Balbi]

From: Nishanth Menon <nm@ti.com>

SmartReflex should be disabled while entering low power mode due to
the following reasons:
a) SmartReflex values are not defined for retention voltage.
b) With SmartReflex enabled, if the CPU enters low power state, FSM
   will try to bump the voltage to current OPP's voltage for which
   it has entered low power state, causing power loss and potential
   unknown states for the SoC.
Since we are sure to attempt entering the lowest possible power state
during suspend operation, SmartReflex needs to be disabled for the
voltage domains in suspend path before achieving auto retention voltage
on the device.

Traditionally, this has been done with interrupts disabled as part of
the common code which handles the idle sequence. Instead, by using the
fact that we have to disable SmartReflex for sure during suspend
operations, we can opportunistically disable SmartReflex in device
standard pm ops, instead of disabling it as part of the code which
executes with interrupts disabled and slave CPU{s} shutdown. This
allows the system to do other parallel activities(such as suspending
other devices in the system using slave CPU{s}) and save the time
required to achieve suspend and resume from suspended state as a
sequential activity.

However, by being opportunistic as described above, we also increase
the likelihood of SmartReflex library access functions being invoked in
parallel contexts *after* SmartReflex driver's suspend handler (during
suspend operation) or *before* resume handler (during resume operation)
have been invoked (Example: DVFS for dependent devices, cpufreq for
MPU etc.). We prevent this by using a flag to reject the callers in
the duration where SmartReflex has been disabled.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/smartreflex.c |   96 ++++++++++++++++++++++++++++++++++--
 1 files changed, 90 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index 6a4f683..5763a71 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -24,6 +24,7 @@
 #include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
+#include <linux/pm.h>
 #include <linux/pm_runtime.h>
 
 #include <plat/common.h>
@@ -40,6 +41,7 @@ struct omap_sr {
 	int				ip_type;
 	int				nvalue_count;
 	bool				autocomp_active;
+	bool				is_suspended;
 	u32				clk_length;
 	u32				err_weight;
 	u32				err_minlimit;
@@ -685,6 +687,11 @@ void omap_sr_enable(struct voltagedomain *voltdm)
 	if (!sr->autocomp_active)
 		return;
 
+	if (sr->is_suspended) {
+		dev_dbg(&sr->pdev->dev, "%s: in suspended state\n", __func__);
+		return;
+	}
+
 	if (!sr_class || !(sr_class->enable) || !(sr_class->configure)) {
 		dev_warn(&sr->pdev->dev, "%s: smartreflex class driver not"
 			"registered\n", __func__);
@@ -718,6 +725,11 @@ void omap_sr_disable(struct voltagedomain *voltdm)
 	if (!sr->autocomp_active)
 		return;
 
+	if (sr->is_suspended) {
+		dev_dbg(&sr->pdev->dev, "%s: in suspended state\n", __func__);
+		return;
+	}
+
 	if (!sr_class || !(sr_class->disable)) {
 		dev_warn(&sr->pdev->dev, "%s: smartreflex class driver not"
 			"registered\n", __func__);
@@ -751,6 +763,11 @@ void omap_sr_disable_reset_volt(struct voltagedomain *voltdm)
 	if (!sr->autocomp_active)
 		return;
 
+	if (sr->is_suspended) {
+		dev_dbg(&sr->pdev->dev, "%s: in suspended state\n", __func__);
+		return;
+	}
+
 	if (!sr_class || !(sr_class->disable)) {
 		dev_warn(&sr->pdev->dev, "%s: smartreflex class driver not"
 			"registered\n", __func__);
@@ -809,14 +826,16 @@ static int omap_sr_autocomp_store(void *data, u64 val)
 		return -EINVAL;
 	}
 
-	/* control enable/disable only if there is a delta in value */
-	if (sr_info->autocomp_active != val) {
-		if (!val)
-			sr_stop_vddautocomp(sr_info);
-		else
-			sr_start_vddautocomp(sr_info);
+	if (sr_info->is_suspended) {
+		pr_warning("%s: in suspended state\n", __func__);
+		return -EBUSY;
 	}
 
+	if (!val)
+		sr_stop_vddautocomp(sr_info);
+	else
+		sr_start_vddautocomp(sr_info);
+
 	return 0;
 }
 
@@ -1011,10 +1030,75 @@ static int __devexit omap_sr_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int omap_sr_suspend(struct device *dev)
+{
+	struct omap_sr_data *pdata;
+	struct omap_sr *sr_info;
+
+	pdata = dev_get_platdata(dev);
+	if (!pdata) {
+		dev_err(dev, "%s: platform data missing\n", __func__);
+		return -EINVAL;
+	}
+
+	sr_info = _sr_lookup(pdata->voltdm);
+	if (IS_ERR(sr_info)) {
+		dev_warn(dev, "%s: omap_sr struct not found\n", __func__);
+		return -EINVAL;
+	}
+
+	if (!sr_info->autocomp_active)
+		return 0;
+
+	if (sr_info->is_suspended)
+		return 0;
+
+	omap_sr_disable_reset_volt(pdata->voltdm);
+	sr_info->is_suspended = true;
+	/* Flag the same info to the other CPUs */
+	smp_wmb();
+
+	return 0;
+}
+
+static int omap_sr_resume(struct device *dev)
+{
+	struct omap_sr_data *pdata;
+	struct omap_sr *sr_info;
+
+	pdata = dev_get_platdata(dev);
+	if (!pdata) {
+		dev_err(dev, "%s: platform data missing\n", __func__);
+		return -EINVAL;
+	}
+
+	sr_info = _sr_lookup(pdata->voltdm);
+	if (IS_ERR(sr_info)) {
+		dev_warn(dev, "%s: omap_sr struct not found\n", __func__);
+		return -EINVAL;
+	}
+
+	if (!sr_info->autocomp_active)
+		return 0;
+
+	if (!sr_info->is_suspended)
+		return 0;
+
+	sr_info->is_suspended = false;
+	/* Flag the same info to the other CPUs */
+	smp_wmb();
+	omap_sr_enable(pdata->voltdm);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(omap_sr_dev_pm_ops, omap_sr_suspend, omap_sr_resume);
+
 static struct platform_driver smartreflex_driver = {
 	.remove         = omap_sr_remove,
 	.driver		= {
 		.name	= "smartreflex",
+		.pm	= &omap_sr_dev_pm_ops,
 	},
 };
 
-- 
1.7.8.rc0

[PATCH/RESEND #3 2/7] arm: omap: smartreflex: add missing platform_set_drvdata()

[Felipe Balbi]

that's very useful to fetch the correct struct sr_info
from PM handlers.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/smartreflex.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index 5763a71..443591b 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -858,6 +858,8 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	platform_set_drvdata(pdev, sr_info);
+
 	if (!pdata) {
 		dev_err(&pdev->dev, "%s: platform data missing\n", __func__);
 		ret = -EINVAL;
-- 
1.7.8.rc0

[PATCH/RESEND #3 3/7] arm: omap: smartreflex: use dev_get_drvdata()

[Felipe Balbi]

When we need to fetch struct omap_sr on PM
handlers, there's no need to access platform_data.

Instead, we can use dev_get_drvdata(dev) like
other drivers do.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/smartreflex.c |   51 ++++++------------------------------
 1 files changed, 9 insertions(+), 42 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index 443591b..bb3bfc0 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -1002,22 +1002,9 @@ err_free_devinfo:
 
 static int __devexit omap_sr_remove(struct platform_device *pdev)
 {
-	struct omap_sr_data *pdata = pdev->dev.platform_data;
-	struct omap_sr *sr_info;
+	struct omap_sr *sr_info = platform_get_drvdata(pdev);
 	struct resource *mem;
 
-	if (!pdata) {
-		dev_err(&pdev->dev, "%s: platform data missing\n", __func__);
-		return -EINVAL;
-	}
-
-	sr_info = _sr_lookup(pdata->voltdm);
-	if (IS_ERR(sr_info)) {
-		dev_warn(&pdev->dev, "%s: omap_sr struct not found\n",
-			__func__);
-		return -EINVAL;
-	}
-
 	if (sr_info->autocomp_active)
 		sr_stop_vddautocomp(sr_info);
 	if (sr_info->dbg_dir)
@@ -1034,20 +1021,10 @@ static int __devexit omap_sr_remove(struct platform_device *pdev)
 
 static int omap_sr_suspend(struct device *dev)
 {
-	struct omap_sr_data *pdata;
-	struct omap_sr *sr_info;
+	struct omap_sr *sr_info = dev_get_drvdata(dev);
 
-	pdata = dev_get_platdata(dev);
-	if (!pdata) {
-		dev_err(dev, "%s: platform data missing\n", __func__);
-		return -EINVAL;
-	}
-
-	sr_info = _sr_lookup(pdata->voltdm);
-	if (IS_ERR(sr_info)) {
-		dev_warn(dev, "%s: omap_sr struct not found\n", __func__);
-		return -EINVAL;
-	}
+	if (!sr_info)
+		return 0;
 
 	if (!sr_info->autocomp_active)
 		return 0;
@@ -1055,7 +1032,7 @@ static int omap_sr_suspend(struct device *dev)
 	if (sr_info->is_suspended)
 		return 0;
 
-	omap_sr_disable_reset_volt(pdata->voltdm);
+	omap_sr_disable_reset_volt(sr_info->voltdm);
 	sr_info->is_suspended = true;
 	/* Flag the same info to the other CPUs */
 	smp_wmb();
@@ -1065,20 +1042,10 @@ static int omap_sr_suspend(struct device *dev)
 
 static int omap_sr_resume(struct device *dev)
 {
-	struct omap_sr_data *pdata;
-	struct omap_sr *sr_info;
+	struct omap_sr *sr_info = dev_get_drvdata(dev);
 
-	pdata = dev_get_platdata(dev);
-	if (!pdata) {
-		dev_err(dev, "%s: platform data missing\n", __func__);
-		return -EINVAL;
-	}
-
-	sr_info = _sr_lookup(pdata->voltdm);
-	if (IS_ERR(sr_info)) {
-		dev_warn(dev, "%s: omap_sr struct not found\n", __func__);
-		return -EINVAL;
-	}
+	if (!sr_info)
+		return 0;
 
 	if (!sr_info->autocomp_active)
 		return 0;
@@ -1089,7 +1056,7 @@ static int omap_sr_resume(struct device *dev)
 	sr_info->is_suspended = false;
 	/* Flag the same info to the other CPUs */
 	smp_wmb();
-	omap_sr_enable(pdata->voltdm);
+	omap_sr_enable(sr_info->voltdm);
 
 	return 0;
 }
-- 
1.7.8.rc0

[PATCH/RESEND #3 4/7] arm: omap: smartreflex: move late_initcall() closer to its argument

[Felipe Balbi]

no functional changes, trivial patch.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/smartreflex.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index bb3bfc0..97375d2 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -1095,12 +1095,12 @@ static int __init sr_init(void)
 
 	return 0;
 }
+late_initcall(sr_init);
 
 static void __exit sr_exit(void)
 {
 	platform_driver_unregister(&smartreflex_driver);
 }
-late_initcall(sr_init);
 module_exit(sr_exit);
 
 MODULE_DESCRIPTION("OMAP Smartreflex Driver");
-- 
1.7.8.rc0

[PATCH/RESEND #3 5/7] arm: omap: smartreflex: clean ups all over

[Felipe Balbi]

There are no functional changes here, only
misc cleanups in general: tabifying variable
declarations, converting if {} else if {} else {}
into switch statements, etc.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/smartreflex.c |  218 +++++++++++++++++++++++--------------
 1 files changed, 134 insertions(+), 84 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index 97375d2..eb88b36 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -37,26 +37,32 @@
 #define SR_DISABLE_TIMEOUT	200
 
 struct omap_sr {
-	int				srid;
-	int				ip_type;
+	struct list_head		node;
+	struct platform_device		*pdev;
+	struct omap_sr_nvalue_table	*nvalue_table;
+	struct voltagedomain		*voltdm;
+
+	unsigned int			irq;
+
 	int				nvalue_count;
-	bool				autocomp_active;
-	bool				is_suspended;
-	u32				clk_length;
-	u32				err_weight;
+	int				ip_type;
+	int				srid;
+
+	u32				senn_avgweight;
+	u32				senp_avgweight;
 	u32				err_minlimit;
 	u32				err_maxlimit;
+	u32				clk_length;
+	u32				err_weight;
 	u32				accum_data;
-	u32				senn_avgweight;
-	u32				senp_avgweight;
 	u32				senp_mod;
 	u32				senn_mod;
-	unsigned int			irq;
+
+	bool				autocomp_active;
+	bool				is_suspended;
+
 	void __iomem			*base;
-	struct platform_device		*pdev;
-	struct list_head		node;
-	struct omap_sr_nvalue_table	*nvalue_table;
-	struct voltagedomain		*voltdm;
+
 	struct dentry			*dbg_dir;
 };
 
@@ -75,11 +81,9 @@ static inline void sr_write_reg(struct omap_sr *sr, unsigned offset, u32 value)
 static inline void sr_modify_reg(struct omap_sr *sr, unsigned offset, u32 mask,
 					u32 value)
 {
-	u32 reg_val;
-	u32 errconfig_offs = 0, errconfig_mask = 0;
-
-	reg_val = __raw_readl(sr->base + offset);
-	reg_val &= ~mask;
+	u32		reg_val;
+	u32		errconfig_offs = 0;
+	u32		errconfig_mask = 0;
 
 	/*
 	 * Smartreflex error config register is special as it contains
@@ -90,14 +94,23 @@ static inline void sr_modify_reg(struct omap_sr *sr, unsigned offset, u32 mask,
 	 * if they are currently set, but does allow the caller to write
 	 * those bits.
 	 */
-	if (sr->ip_type == SR_TYPE_V1) {
+	switch (sr->ip_type) {
+	case SR_TYPE_V1:
 		errconfig_offs = ERRCONFIG_V1;
 		errconfig_mask = ERRCONFIG_STATUS_V1_MASK;
-	} else if (sr->ip_type == SR_TYPE_V2) {
+		break;
+	case SR_TYPE_V2:
 		errconfig_offs = ERRCONFIG_V2;
 		errconfig_mask = ERRCONFIG_VPBOUNDINTST_V2;
+		break;
+	default: /* should never happen */
+		dev_err(&sr->pdev->dev, "UNKNOWN IP type %d\n", sr->ip_type);
+		return;
 	}
 
+	reg_val = __raw_readl(sr->base + offset);
+	reg_val &= ~mask;
+
 	if (offset == errconfig_offs)
 		reg_val &= ~errconfig_mask;
 
@@ -113,7 +126,7 @@ static inline u32 sr_read_reg(struct omap_sr *sr, unsigned offset)
 
 static struct omap_sr *_sr_lookup(struct voltagedomain *voltdm)
 {
-	struct omap_sr *sr_info;
+	struct omap_sr	*sr_info;
 
 	if (!voltdm) {
 		pr_err("%s: Null voltage domain passed!\n", __func__);
@@ -130,33 +143,39 @@ static struct omap_sr *_sr_lookup(struct voltagedomain *voltdm)
 
 static irqreturn_t sr_interrupt(int irq, void *data)
 {
-	struct omap_sr *sr_info = (struct omap_sr *)data;
-	u32 status = 0;
+	struct omap_sr	*sr_info = data;
+	u32		status = 0;
 
-	if (sr_info->ip_type == SR_TYPE_V1) {
+	switch (sr_info->ip_type) {
+	case SR_TYPE_V1:
 		/* Read the status bits */
 		status = sr_read_reg(sr_info, ERRCONFIG_V1);
 
 		/* Clear them by writing back */
 		sr_write_reg(sr_info, ERRCONFIG_V1, status);
-	} else if (sr_info->ip_type == SR_TYPE_V2) {
+		break;
+	case SR_TYPE_V2:
 		/* Read the status bits */
 		sr_read_reg(sr_info, IRQSTATUS);
 
 		/* Clear them by writing back */
 		sr_write_reg(sr_info, IRQSTATUS, status);
-	}
 
-	if (sr_class->notify)
-		sr_class->notify(sr_info->voltdm, status);
+		if (sr_class->notify)
+			sr_class->notify(sr_info->voltdm, status);
+		break;
+	default: /* should never happen */
+		dev_err(&sr_info->pdev->dev, "UNKNOWN IP type %d\n", sr_info->ip_type);
+		return IRQ_NONE;
+	}
 
 	return IRQ_HANDLED;
 }
 
 static void sr_set_clk_length(struct omap_sr *sr)
 {
-	struct clk *sys_ck;
-	u32 sys_clk_speed;
+	struct clk	*sys_ck;
+	u32		sys_clk_speed;
 
 	if (cpu_is_omap34xx())
 		sys_ck = clk_get(NULL, "sys_ck");
@@ -168,6 +187,7 @@ static void sr_set_clk_length(struct omap_sr *sr)
 			__func__);
 		return;
 	}
+
 	sys_clk_speed = clk_get_rate(sys_ck);
 	clk_put(sys_ck);
 
@@ -257,10 +277,10 @@ static void sr_stop_vddautocomp(struct omap_sr *sr)
  */
 static int sr_late_init(struct omap_sr *sr_info)
 {
-	char *name;
-	struct omap_sr_data *pdata = sr_info->pdev->dev.platform_data;
-	struct resource *mem;
-	int ret = 0;
+	struct omap_sr_data	*pdata = sr_info->pdev->dev.platform_data;
+	struct			resource *mem;
+	int			ret = 0;
+	char			*name;
 
 	if (sr_class->notify && sr_class->notify_flags && sr_info->irq) {
 		name = kasprintf(GFP_KERNEL, "sr_%s", sr_info->voltdm->name);
@@ -269,7 +289,7 @@ static int sr_late_init(struct omap_sr *sr_info)
 			goto error;
 		}
 		ret = request_irq(sr_info->irq, sr_interrupt,
-				0, name, (void *)sr_info);
+				0, name, sr_info);
 		if (ret)
 			goto error;
 		disable_irq(sr_info->irq);
@@ -290,12 +310,13 @@ error:
 		"not function as desired\n", __func__);
 	kfree(name);
 	kfree(sr_info);
+
 	return ret;
 }
 
 static void sr_v1_disable(struct omap_sr *sr)
 {
-	int timeout = 0;
+	int		timeout = 0;
 
 	/* Enable MCUDisableAcknowledge interrupt */
 	sr_modify_reg(sr, ERRCONFIG_V1,
@@ -331,7 +352,7 @@ static void sr_v1_disable(struct omap_sr *sr)
 
 static void sr_v2_disable(struct omap_sr *sr)
 {
-	int timeout = 0;
+	int		timeout = 0;
 
 	/* Enable MCUDisableAcknowledge interrupt */
 	sr_write_reg(sr, IRQENABLE_SET, IRQENABLE_MCUDISABLEACKINT);
@@ -368,7 +389,7 @@ static void sr_v2_disable(struct omap_sr *sr)
 
 static u32 sr_retrieve_nvalue(struct omap_sr *sr, u32 efuse_offs)
 {
-	int i;
+	int		i;
 
 	if (!sr->nvalue_table) {
 		dev_warn(&sr->pdev->dev, "%s: Missing ntarget value table\n",
@@ -400,10 +421,16 @@ static u32 sr_retrieve_nvalue(struct omap_sr *sr, u32 efuse_offs)
  */
 int sr_configure_errgen(struct voltagedomain *voltdm)
 {
-	u32 sr_config, sr_errconfig, errconfig_offs, vpboundint_en;
-	u32 vpboundint_st, senp_en = 0, senn_en = 0;
-	u8 senp_shift, senn_shift;
-	struct omap_sr *sr = _sr_lookup(voltdm);
+	struct omap_sr		*sr = _sr_lookup(voltdm);
+	u32			sr_config;
+	u32			sr_errconfig;
+	u32			errconfig_offs;
+	u32			vpboundint_en;
+	u32			vpboundint_st;
+	u32			senp_en = 0;
+	u32			senn_en = 0;
+	u8			senp_shift;
+	u8			senn_shift;
 
 	if (IS_ERR(sr)) {
 		pr_warning("%s: omap_sr struct for sr_%s not found\n",
@@ -420,22 +447,24 @@ int sr_configure_errgen(struct voltagedomain *voltdm)
 	sr_config = (sr->clk_length << SRCONFIG_SRCLKLENGTH_SHIFT) |
 		SRCONFIG_SENENABLE | SRCONFIG_ERRGEN_EN;
 
-	if (sr->ip_type == SR_TYPE_V1) {
+	switch (sr->ip_type) {
+	case SR_TYPE_V1:
 		sr_config |= SRCONFIG_DELAYCTRL;
 		senn_shift = SRCONFIG_SENNENABLE_V1_SHIFT;
 		senp_shift = SRCONFIG_SENPENABLE_V1_SHIFT;
 		errconfig_offs = ERRCONFIG_V1;
 		vpboundint_en = ERRCONFIG_VPBOUNDINTEN_V1;
 		vpboundint_st = ERRCONFIG_VPBOUNDINTST_V1;
-	} else if (sr->ip_type == SR_TYPE_V2) {
+		break;
+	case SR_TYPE_V2:
 		senn_shift = SRCONFIG_SENNENABLE_V2_SHIFT;
 		senp_shift = SRCONFIG_SENPENABLE_V2_SHIFT;
 		errconfig_offs = ERRCONFIG_V2;
 		vpboundint_en = ERRCONFIG_VPBOUNDINTEN_V2;
 		vpboundint_st = ERRCONFIG_VPBOUNDINTST_V2;
-	} else {
-		dev_err(&sr->pdev->dev, "%s: Trying to Configure smartreflex"
-			"module without specifying the ip\n", __func__);
+		break;
+	default:
+		dev_err(&sr->pdev->dev, "UNKNOWN IP type %d\n", sr->ip_type);
 		return -EINVAL;
 	}
 
@@ -469,10 +498,13 @@ int sr_configure_errgen(struct voltagedomain *voltdm)
  */
 int sr_configure_minmax(struct voltagedomain *voltdm)
 {
-	u32 sr_config, sr_avgwt;
-	u32 senp_en = 0, senn_en = 0;
-	u8 senp_shift, senn_shift;
-	struct omap_sr *sr = _sr_lookup(voltdm);
+	struct omap_sr		*sr = _sr_lookup(voltdm);
+	u32			sr_config;
+	u32			sr_avgwt;
+	u32			senp_en = 0;
+	u32			senn_en = 0;
+	u8			senp_shift;
+	u8			senn_shift;
 
 	if (IS_ERR(sr)) {
 		pr_warning("%s: omap_sr struct for sr_%s not found\n",
@@ -490,16 +522,18 @@ int sr_configure_minmax(struct voltagedomain *voltdm)
 		SRCONFIG_SENENABLE |
 		(sr->accum_data << SRCONFIG_ACCUMDATA_SHIFT);
 
-	if (sr->ip_type == SR_TYPE_V1) {
+	switch (sr->ip_type) {
+	case SR_TYPE_V1:
 		sr_config |= SRCONFIG_DELAYCTRL;
 		senn_shift = SRCONFIG_SENNENABLE_V1_SHIFT;
 		senp_shift = SRCONFIG_SENPENABLE_V1_SHIFT;
-	} else if (sr->ip_type == SR_TYPE_V2) {
+		break;
+	case SR_TYPE_V2:
 		senn_shift = SRCONFIG_SENNENABLE_V2_SHIFT;
 		senp_shift = SRCONFIG_SENPENABLE_V2_SHIFT;
-	} else {
-		dev_err(&sr->pdev->dev, "%s: Trying to Configure smartreflex"
-			"module without specifying the ip\n", __func__);
+		break;
+	default:
+		dev_err(&sr->pdev->dev, "UNKNOWN IP type %d\n", sr->ip_type);
 		return -EINVAL;
 	}
 
@@ -513,20 +547,26 @@ int sr_configure_minmax(struct voltagedomain *voltdm)
 	 * Enabling the interrupts if MINMAXAVG module is used.
 	 * TODO: check if all the interrupts are mandatory
 	 */
-	if (sr->ip_type == SR_TYPE_V1) {
+	switch (sr->ip_type) {
+	case SR_TYPE_V1:
 		sr_modify_reg(sr, ERRCONFIG_V1,
 			(ERRCONFIG_MCUACCUMINTEN | ERRCONFIG_MCUVALIDINTEN |
 			ERRCONFIG_MCUBOUNDINTEN),
 			(ERRCONFIG_MCUACCUMINTEN | ERRCONFIG_MCUACCUMINTST |
 			 ERRCONFIG_MCUVALIDINTEN | ERRCONFIG_MCUVALIDINTST |
 			 ERRCONFIG_MCUBOUNDINTEN | ERRCONFIG_MCUBOUNDINTST));
-	} else if (sr->ip_type == SR_TYPE_V2) {
+		break;
+	case SR_TYPE_V2:
 		sr_write_reg(sr, IRQSTATUS,
 			IRQSTATUS_MCUACCUMINT | IRQSTATUS_MCVALIDINT |
 			IRQSTATUS_MCBOUNDSINT | IRQSTATUS_MCUDISABLEACKINT);
 		sr_write_reg(sr, IRQENABLE_SET,
 			IRQENABLE_MCUACCUMINT | IRQENABLE_MCUVALIDINT |
 			IRQENABLE_MCUBOUNDSINT | IRQENABLE_MCUDISABLEACKINT);
+		break;
+	default:
+		dev_err(&sr->pdev->dev, "UNKNOWN IP type %d\n", sr->ip_type);
+		return -EINVAL;
 	}
 
 	return 0;
@@ -545,10 +585,10 @@ int sr_configure_minmax(struct voltagedomain *voltdm)
  */
 int sr_enable(struct voltagedomain *voltdm, unsigned long volt)
 {
-	u32 nvalue_reciprocal;
-	struct omap_volt_data *volt_data;
-	struct omap_sr *sr = _sr_lookup(voltdm);
-	int ret;
+	struct omap_sr		*sr = _sr_lookup(voltdm);
+	struct omap_volt_data	*volt_data;
+	u32			nvalue_reciprocal;
+	int			ret;
 
 	if (IS_ERR(sr)) {
 		pr_warning("%s: omap_sr struct for sr_%s not found\n",
@@ -602,7 +642,7 @@ int sr_enable(struct voltagedomain *voltdm, unsigned long volt)
  */
 void sr_disable(struct voltagedomain *voltdm)
 {
-	struct omap_sr *sr = _sr_lookup(voltdm);
+	struct omap_sr		*sr = _sr_lookup(voltdm);
 
 	if (IS_ERR(sr)) {
 		pr_warning("%s: omap_sr struct for sr_%s not found\n",
@@ -619,10 +659,17 @@ void sr_disable(struct voltagedomain *voltdm)
 	 * disable the clocks.
 	 */
 	if (sr_read_reg(sr, SRCONFIG) & SRCONFIG_SRENABLE) {
-		if (sr->ip_type == SR_TYPE_V1)
+		switch (sr->ip_type) {
+		case SR_TYPE_V1:
 			sr_v1_disable(sr);
-		else if (sr->ip_type == SR_TYPE_V2)
+			break;
+		case SR_TYPE_V2:
 			sr_v2_disable(sr);
+			break;
+		default:
+			dev_err(&sr->pdev->dev, "UNKNOWN IP type %d\n",
+					sr->ip_type);
+		}
 	}
 
 	pm_runtime_put_sync_suspend(&sr->pdev->dev);
@@ -638,7 +685,7 @@ void sr_disable(struct voltagedomain *voltdm)
  */
 int sr_register_class(struct omap_sr_class_data *class_data)
 {
-	struct omap_sr *sr_info;
+	struct omap_sr		*sr_info;
 
 	if (!class_data) {
 		pr_warning("%s:, Smartreflex class data passed is NULL\n",
@@ -676,7 +723,7 @@ int sr_register_class(struct omap_sr_class_data *class_data)
  */
 void omap_sr_enable(struct voltagedomain *voltdm)
 {
-	struct omap_sr *sr = _sr_lookup(voltdm);
+	struct omap_sr		*sr = _sr_lookup(voltdm);
 
 	if (IS_ERR(sr)) {
 		pr_warning("%s: omap_sr struct for sr_%s not found\n",
@@ -714,7 +761,7 @@ void omap_sr_enable(struct voltagedomain *voltdm)
  */
 void omap_sr_disable(struct voltagedomain *voltdm)
 {
-	struct omap_sr *sr = _sr_lookup(voltdm);
+	struct omap_sr		*sr = _sr_lookup(voltdm);
 
 	if (IS_ERR(sr)) {
 		pr_warning("%s: omap_sr struct for sr_%s not found\n",
@@ -752,7 +799,7 @@ void omap_sr_disable(struct voltagedomain *voltdm)
  */
 void omap_sr_disable_reset_volt(struct voltagedomain *voltdm)
 {
-	struct omap_sr *sr = _sr_lookup(voltdm);
+	struct omap_sr		*sr = _sr_lookup(voltdm);
 
 	if (IS_ERR(sr)) {
 		pr_warning("%s: omap_sr struct for sr_%s not found\n",
@@ -796,10 +843,10 @@ void omap_sr_register_pmic(struct omap_sr_pmic_data *pmic_data)
 	sr_pmic_data = pmic_data;
 }
 
-/* PM Debug Fs enteries to enable disable smartreflex. */
+/* PM Debug Fs entries to enable disable smartreflex. */
 static int omap_sr_autocomp_show(void *data, u64 *val)
 {
-	struct omap_sr *sr_info = (struct omap_sr *) data;
+	struct omap_sr		*sr_info = data;
 
 	if (!sr_info) {
 		pr_warning("%s: omap_sr struct not found\n", __func__);
@@ -813,7 +860,7 @@ static int omap_sr_autocomp_show(void *data, u64 *val)
 
 static int omap_sr_autocomp_store(void *data, u64 val)
 {
-	struct omap_sr *sr_info = (struct omap_sr *) data;
+	struct omap_sr		*sr_info = data;
 
 	if (!sr_info) {
 		pr_warning("%s: omap_sr struct not found\n", __func__);
@@ -844,14 +891,17 @@ DEFINE_SIMPLE_ATTRIBUTE(pm_sr_fops, omap_sr_autocomp_show,
 
 static int __init omap_sr_probe(struct platform_device *pdev)
 {
-	struct omap_sr *sr_info = kzalloc(sizeof(struct omap_sr), GFP_KERNEL);
-	struct omap_sr_data *pdata = pdev->dev.platform_data;
-	struct resource *mem, *irq;
-	struct dentry *nvalue_dir;
-	struct omap_volt_data *volt_data;
-	int i, ret = 0;
-	char *name;
-
+	struct omap_sr		*sr_info;
+	struct omap_sr_data	*pdata = pdev->dev.platform_data;
+	struct resource		*mem;
+	struct resource		*irq;
+	struct dentry		*nvalue_dir;
+	struct omap_volt_data	*volt_data;
+	int			ret = 0;
+	int			i;
+	char			*name;
+
+	sr_info = kzalloc(sizeof(*sr_info), GFP_KERNEL);
 	if (!sr_info) {
 		dev_err(&pdev->dev, "%s: unable to allocate sr_info\n",
 			__func__);
@@ -1002,8 +1052,8 @@ err_free_devinfo:
 
 static int __devexit omap_sr_remove(struct platform_device *pdev)
 {
-	struct omap_sr *sr_info = platform_get_drvdata(pdev);
-	struct resource *mem;
+	struct omap_sr		*sr_info = platform_get_drvdata(pdev);
+	struct resource		*mem;
 
 	if (sr_info->autocomp_active)
 		sr_stop_vddautocomp(sr_info);
@@ -1021,7 +1071,7 @@ static int __devexit omap_sr_remove(struct platform_device *pdev)
 
 static int omap_sr_suspend(struct device *dev)
 {
-	struct omap_sr *sr_info = dev_get_drvdata(dev);
+	struct omap_sr		*sr_info = dev_get_drvdata(dev);
 
 	if (!sr_info)
 		return 0;
@@ -1042,7 +1092,7 @@ static int omap_sr_suspend(struct device *dev)
 
 static int omap_sr_resume(struct device *dev)
 {
-	struct omap_sr *sr_info = dev_get_drvdata(dev);
+	struct omap_sr		*sr_info = dev_get_drvdata(dev);
 
 	if (!sr_info)
 		return 0;
-- 
1.7.8.rc0

[PATCH/RESEND #3 6/7] arm: omap: smartreflex: fix IRQ handling bug

[Felipe Balbi]

fix a bug which has been on this driver since
it was added by the original commit 984aa6db
which would never clear IRQSTATUS bits.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/smartreflex.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index eb88b36..a5399df 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -156,7 +156,7 @@ static irqreturn_t sr_interrupt(int irq, void *data)
 		break;
 	case SR_TYPE_V2:
 		/* Read the status bits */
-		sr_read_reg(sr_info, IRQSTATUS);
+		status = sr_read_reg(sr_info, IRQSTATUS);
 
 		/* Clear them by writing back */
 		sr_write_reg(sr_info, IRQSTATUS, status);
-- 
1.7.8.rc0

[PATCH/RESEND #3 7/7] arm: omap: smartreflex: micro-optimization for sanity check

[Felipe Balbi]

val && (val != 1) == val > 1

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/smartreflex.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index a5399df..9fee277 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -868,7 +868,7 @@ static int omap_sr_autocomp_store(void *data, u64 val)
 	}
 
 	/* Sanity check */
-	if (val && (val != 1)) {
+	if (val > 1) {
 		pr_warning("%s: Invalid argument %lld\n", __func__, val);
 		return -EINVAL;
 	}
-- 
1.7.8.rc0

[PATCH/RESEND #3 5/7] arm: omap: smartreflex: clean ups all over

[Valdis.Kletnieks at vt.edu]

On Tue, 15 Nov 2011 10:46:03 +0200, Felipe Balbi said:

> -	int				srid;
> -	int				ip_type;
> +	struct list_head		node;
> +	struct platform_device		*pdev;
> +	struct omap_sr_nvalue_table	*nvalue_table;
> +	struct voltagedomain		*voltdm;

As long as you're de-tabifying the function names that caused the problem...

>  	int				nvalue_count;
> -	bool				autocomp_active;
> -	bool				is_suspended;

Why are you leaveing the rest of the struct tabified out to East Podunk?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20111115/e07b4607/attachment.sig>

[PATCH/RESEND #3 5/7] arm: omap: smartreflex: clean ups all over

[Felipe Balbi]

On Tue, Nov 15, 2011 at 07:39:51PM -0500, Valdis.Kletnieks at vt.edu wrote:
> On Tue, 15 Nov 2011 10:46:03 +0200, Felipe Balbi said:
> 
> > -	int				srid;
> > -	int				ip_type;
> > +	struct list_head		node;
> > +	struct platform_device		*pdev;
> > +	struct omap_sr_nvalue_table	*nvalue_table;
> > +	struct voltagedomain		*voltdm;
> 
> As long as you're de-tabifying the function names that caused the problem...
> 
> >  	int				nvalue_count;
> > -	bool				autocomp_active;
> > -	bool				is_suspended;
> 
> Why are you leaveing the rest of the struct tabified out to East Podunk?

de-tabifying ? I tabified the entire thing... Everything is aligned.

-- 
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/20111116/f611e4aa/attachment.sig>

[PATCH/RESEND #3 1/7] OMAP3+: PM: SR: add suspend/resume handlers

[Kevin Hilman]

Felipe Balbi <balbi@ti.com> writes:

> From: Nishanth Menon <nm@ti.com>
>
> SmartReflex should be disabled while entering low power mode due to
> the following reasons:

[...]

Nishanth, in the end, didn't you decide to drop this patch?

Kevin

[PATCH/RESEND #3 6/7] arm: omap: smartreflex: fix IRQ handling bug

[Kevin Hilman]

Felipe Balbi <balbi@ti.com> writes:

F> fix a bug which has been on this driver since
> it was added by the original commit 984aa6db
> which would never clear IRQSTATUS bits.
>
> Signed-off-by: Felipe Balbi <balbi@ti.com>

Queuing this fix for v3.2-rc (branch: for_3.2/fixes/pm)

Kevin

[PATCH/RESEND #3 0/7] SmartReflex Patches

[Kevin Hilman]

+Jean

Felipe Balbi <balbi@ti.com> writes:

> here's a bunch of patches to smartreflex which have been pending
> for months.

The SmartReflex code is going through a major rework by Jean so it can be
converted into a real driver and moved under drivers/*.

I'll leave it to Jean to incorporate these changes into his rework as he
sees fit, although I've queued the IRQ fix for v3.2-rc.

Kevin

[PATCH/RESEND #3 6/7] arm: omap: smartreflex: fix IRQ handling bug

[Felipe Balbi]

On Wed, Nov 16, 2011 at 04:06:08PM -0800, Kevin Hilman wrote:
> Felipe Balbi <balbi@ti.com> writes:
> 
> F> fix a bug which has been on this driver since
> > it was added by the original commit 984aa6db
> > which would never clear IRQSTATUS bits.
> >
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> 
> Queuing this fix for v3.2-rc (branch: for_3.2/fixes/pm)

should it go to stable too ??

-- 
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/20111117/340ef8ba/attachment.sig>

[PATCH/RESEND #3 1/7] OMAP3+: PM: SR: add suspend/resume handlers

[Menon, Nishanth]

On Wed, Nov 16, 2011 at 18:02, Kevin Hilman <khilman@ti.com> wrote:
> Felipe Balbi <balbi@ti.com> writes:
>
>> From: Nishanth Menon <nm@ti.com>
>>
>> SmartReflex should be disabled while entering low power mode due to
>> the following reasons:
> [...]
>
> Nishanth, in the end, didn't you decide to drop this patch?
>

Yes, I did eventually, once we implemented DVFS for GPU, Ducati, HSI,
and other drivers, there was no real way to ensure sequence of suspend
sequencing even after moving this to suspend_noirq. some of the other
reasons:

if I disabled Smartreflex and went to Nominal voltage on MPU, and say
MPU was at "performance" mode of 1.5GHz or so, thermal scenarios got
worse due on hot corner samples - these tend to have higher leakage
and thermal characteristics tend to be more pronounced. The option of
throttling frequency down while suspend was not really a good option
in comparison to switching off smart reflex in the last possible
moment - in pmxxx.c

Regards,
Nishanth Menon

[PATCH/RESEND #3 1/7] OMAP3+: PM: SR: add suspend/resume handlers

[Kevin Hilman]

"Menon, Nishanth" <nm@ti.com> writes:

> On Wed, Nov 16, 2011 at 18:02, Kevin Hilman <khilman@ti.com> wrote:
>> Felipe Balbi <balbi@ti.com> writes:
>>
>>> From: Nishanth Menon <nm@ti.com>
>>>
>>> SmartReflex should be disabled while entering low power mode due to
>>> the following reasons:
>> [...]
>>
>> Nishanth, in the end, didn't you decide to drop this patch?
>>
>
> Yes, I did eventually, once we implemented DVFS for GPU, Ducati, HSI,
> and other drivers, there was no real way to ensure sequence of suspend
> sequencing even after moving this to suspend_noirq. some of the other
> reasons:

OK, thanks for the clarification.

Kevin