Skip to contentSkip to conversation

OMAP3+: PM: SR: add suspend/resume handlers

PATCH New Felipe Balbi 2 participants 12 messages Oct 10, 2011 – Oct 12, 2011

Subsystems (3)
Arm (32-bit) Arm sub-architectures OMAP2+ support
lore
Share & tools
lore

Felipe Balbi

author
#0 lore raw

PATCH 1/7 OMAP3+: PM: SR: add suspend/resume handlers

From: Nishanth Menon <[email protected]>

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 <[email protected]>
---
 arch/arm/mach-omap2/smartreflex.c |   96 ++++++++++++++++++++++++++++++++++--
 1 files changed, 90 insertions(+), 6 deletions(-)
arch/arm/mach-omap2/smartreflex.c +90 -6
index 34c01a7..af158c0 100644
@@ -23,6 +23,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>

@@ -39,6 +40,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;

@@ -683,6 +685,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__);

@@ -716,6 +723,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__);

@@ -749,6 +761,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__);

@@ -807,14 +824,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;
 }
 

@@ -1001,10 +1020,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,
 	},
 };
Signature · 1 line
1.7.6.396.ge0613
author
#1 lore raw

PATCH 2/7 arm: omap: smartreflex: add missing platform_set_drvdata()

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

Signed-off-by Felipe Balbi <[email protected]>
---
 arch/arm/mach-omap2/smartreflex.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/smartreflex.c +2 -0
index af158c0..55e297e 100644
@@ -855,6 +855,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;
Signature · 1 line
1.7.6.396.ge0613
author
#2 lore raw

PATCH 3/7 arm: omap: smartreflex: use dev_get_drvdata()

In reply to 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 <[email protected]>
---
 arch/arm/mach-omap2/smartreflex.c |   51 ++++++------------------------------
 1 files changed, 9 insertions(+), 42 deletions(-)
arch/arm/mach-omap2/smartreflex.c +9 -42
index 55e297e..357363b 100644
@@ -992,22 +992,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)

@@ -1024,20 +1011,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;

@@ -1045,7 +1022,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();

@@ -1055,20 +1032,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;

@@ -1079,7 +1046,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;
 }
Signature · 1 line
1.7.6.396.ge0613
author
#3 lore raw

PATCH 4/7 arm: omap: smartreflex: move late_initcall() closer to its argument

In reply to Felipe Balbi

no functional changes, trivial patch.

Signed-off-by Felipe Balbi <[email protected]>
---
 arch/arm/mach-omap2/smartreflex.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/smartreflex.c +1 -1
index 357363b..28a24fd 100644
@@ -1085,12 +1085,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");
Signature · 1 line
1.7.6.396.ge0613
author
#4 lore raw

PATCH 5/7 arm: omap: smartreflex: clean ups all over

In reply to 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 <[email protected]>
---
 arch/arm/mach-omap2/smartreflex.c |  217 +++++++++++++++++++++++--------------
 1 files changed, 134 insertions(+), 83 deletions(-)
arch/arm/mach-omap2/smartreflex.c +134 -83
index 28a24fd..6e9eb46 100644
@@ -36,26 +36,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;
 };
 

@@ -73,11 +79,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

@@ -88,14 +92,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;
 

@@ -111,7 +124,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__);

@@ -128,33 +141,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");

@@ -166,6 +185,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);
 

@@ -255,10 +275,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);

@@ -267,7 +287,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);

@@ -288,12 +308,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,

@@ -329,7 +350,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);

@@ -366,7 +387,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",

@@ -398,10 +419,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",

@@ -418,22 +445,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;
 	}
 

@@ -467,10 +496,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",

@@ -488,16 +520,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;
 	}
 

@@ -511,20 +545,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;

@@ -543,10 +583,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",

@@ -600,7 +640,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",

@@ -617,10 +657,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);

@@ -636,7 +683,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",

@@ -674,7 +721,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",

@@ -712,7 +759,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",

@@ -750,7 +797,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",

@@ -794,10 +841,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__);

@@ -811,7 +858,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__);

@@ -842,13 +889,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 *vdd_dbg_dir, *nvalue_dir;
-	struct omap_volt_data *volt_data;
-	int i, ret = 0;
-
+	struct omap_sr		*sr_info;
+	struct omap_sr_data	*pdata = pdev->dev.platform_data;
+	struct resource		*mem;
+	struct resource		*irq;
+	struct dentry		*vdd_dbg_dir;
+	struct dentry		*nvalue_dir;
+	struct omap_volt_data	*volt_data;
+	int			ret = 0;
+	int			i;
+
+	sr_info = kzalloc(sizeof(struct omap_sr), GFP_KERNEL);
 	if (!sr_info) {
 		dev_err(&pdev->dev, "%s: unable to allocate sr_info\n",
 			__func__);

@@ -992,8 +1043,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);

@@ -1011,7 +1062,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;

@@ -1032,7 +1083,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;
Signature · 1 line
1.7.6.396.ge0613
author
#5 lore raw

PATCH 6/7 arm: omap: smartreflex: fix IRQ handling bug

In reply to 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 <[email protected]>
---
 arch/arm/mach-omap2/smartreflex.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/smartreflex.c +1 -1
index 6e9eb46..7bdabfa 100644
@@ -154,7 +154,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);
Signature · 1 line
1.7.6.396.ge0613
#6 lore raw

PATCH 6/7 arm: omap: smartreflex: fix IRQ handling bug

Hello.

Quoted 2 lines · Felipe Balbi

fix a bug which has been on this driver since
it was added by the original commit 984aa6db

Please also specify that commit's summary in parens.

Quoted 3 lines

which would never clear IRQSTATUS bits.

Signed-off-by Felipe Balbi <[email protected]>

WBR, Sergei

author
#7 lore raw

PATCH 6/7 arm: omap: smartreflex: fix IRQ handling bug

Quoted 8 lines · Sergei Shtylyov

Hello.

Quoted 2 lines · Felipe Balbi

fix a bug which has been on this driver since
it was added by the original commit 984aa6db

Please also specify that commit's summary in parens.

sure, no problem

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20111011/7d5743e5/attachment.sig>
author
#8 lore raw

PATCH 7/7 arm: omap: smartreflex: micro-optimization for sanity check

In reply to Felipe Balbi

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

Signed-off-by Felipe Balbi <[email protected]>
---
 arch/arm/mach-omap2/smartreflex.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/smartreflex.c +1 -1
index 7bdabfa..4b0d6a8 100644
@@ -866,7 +866,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;
 	}
Signature · 1 line
1.7.6.396.ge0613
#9 lore raw

PATCH 7/7 arm: omap: smartreflex: micro-optimization for sanity check

Hello.

Quoted 12 lines · Felipe Balbi

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

Signed-off-by Felipe Balbi<[email protected]>
---
  arch/arm/mach-omap2/smartreflex.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/smartreflex.c +0 -0
index 7bdabfa..4b0d6a8 100644

arch/arm/mach-omap2/smartreflex.c static int omap_sr_autocomp_store(void *data, u64 val) · line 870

Quoted 6 lines
arch/arm/mach-omap2/smartreflex.c +1 -1
@@ -866,7 +866,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);

The format specified should be "%llu"?

WBR, Sergei

author
#10 lore raw

PATCH 7/7 arm: omap: smartreflex: micro-optimization for sanity check

Quoted 18 lines · Sergei Shtylyov

Hello.

Quoted 14 lines · Felipe Balbi

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

Signed-off-by Felipe Balbi<[email protected]>
---
 arch/arm/mach-omap2/smartreflex.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/smartreflex.c +0 -0
index 7bdabfa..4b0d6a8 100644
@@ -866,7 +866,7 @@ static int omap_sr_autocomp_store(void *data, u64 val)
 	}
Quoted 6 lines

arch/arm/mach-omap2/smartreflex.c static int omap_sr_autocomp_store(void *data, u64 val) · line 870

Quoted 4 lines
arch/arm/mach-omap2/smartreflex.c +1 -1
 	/* Sanity check */
-	if (val&&  (val != 1)) {
+	if (val>  1) {
 		pr_warning("%s: Invalid argument %lld\n", __func__, val);

The format specified should be "%llu"?

and what does that have to do with $SUBJECT ??

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20111011/f99ff706/attachment-0001.sig>
#11 lore raw

PATCH 7/7 arm: omap: smartreflex: micro-optimization for sanity check

Hello.

Quoted 16 lines · Felipe Balbi
Quoted 16 lines · Felipe Balbi
Quoted 15 lines

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

Signed-off-by Felipe Balbi<[email protected]>
---
  arch/arm/mach-omap2/smartreflex.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/smartreflex.c +0 -0
index 7bdabfa..4b0d6a8 100644
@@ -866,7 +866,7 @@ static int omap_sr_autocomp_store(void *data, u64 val)
  	}
Quoted 8 lines
Quoted 6 lines

arch/arm/mach-omap2/smartreflex.c static int omap_sr_autocomp_store(void *data, u64 val) · line 870

Quoted 4 lines
arch/arm/mach-omap2/smartreflex.c +1 -1
  	/* Sanity check */
-	if (val&&   (val != 1)) {
+	if (val>   1) {
  		pr_warning("%s: Invalid argument %lld\n", __func__, val);

The format specified should be "%llu"?

and what does that have to do with $SUBJECT ??

Well, nothing. Just an idea for another patch I guess.

WBR, Sergei