[PATCH 0/2] OMAP3: PM: More pedantic parameter and error checking in smartreflex

[PATCH 0/2] OMAP3: PM: More pedantic parameter and error checking in smartreflex

[Phil Carmody]

A couple of simple patches to improve error handling in smartreflex.
The first has a practical benefit of avoiding a string-based search
in situtations where the result wouldn't be needed. The second is
simple paranoia.

[PATCH 1/2] OMAP3: PM: Don't do unnecessary searches in omap_sr_vdd*_autocomp_store

[Phil Carmody]

From: Phil Carmody <ext-phil.2.carmody@nokia.com>

When setting to 0, we don't need to do searches for the resource by
its name. In the case where we do search, handle the error condition
cleanly.

Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
---
 arch/arm/mach-omap2/smartreflex.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index ae5c336..ce7d436 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -744,7 +744,6 @@ static ssize_t omap_sr_vdd1_autocomp_store(struct kobject *kobj,
 					struct kobj_attribute *attr,
 					const char *buf, size_t n)
 {
-	u32 current_vdd1opp_no;
 	unsigned short value;
 
 	if (sscanf(buf, "%hu", &value) != 1 || (value > 1)) {
@@ -752,13 +751,14 @@ static ssize_t omap_sr_vdd1_autocomp_store(struct kobject *kobj,
 		return -EINVAL;
 	}
 
-	current_vdd1opp_no = omap_pm_vdd1_get_opp();
-
-	if (value == 0)
+	if (value == 0) {
 		sr_stop_vddautocomap(SR1);
-	else
+	} else {
+		u32 current_vdd1opp_no = omap_pm_vdd1_get_opp();
+		if (IS_ERR_VALUE(current_vdd1opp_no))
+			return -ENODEV;
 		sr_start_vddautocomap(SR1, current_vdd1opp_no);
-
+	}
 	return n;
 }
 
@@ -782,7 +782,6 @@ static ssize_t omap_sr_vdd2_autocomp_store(struct kobject *kobj,
 					struct kobj_attribute *attr,
 					const char *buf, size_t n)
 {
-	u32 current_vdd2opp_no;
 	unsigned short value;
 
 	if (sscanf(buf, "%hu", &value) != 1 || (value > 1)) {
@@ -790,13 +789,14 @@ static ssize_t omap_sr_vdd2_autocomp_store(struct kobject *kobj,
 		return -EINVAL;
 	}
 
-	current_vdd2opp_no = omap_pm_vdd2_get_opp();
-
-	if (value == 0)
+	if (value == 0) {
 		sr_stop_vddautocomap(SR2);
-	else
+	} else {
+		u32 current_vdd2opp_no = omap_pm_vdd2_get_opp();
+		if (IS_ERR_VALUE(current_vdd2opp_no))
+			return -ENODEV;
 		sr_start_vddautocomap(SR2, current_vdd2opp_no);
-
+	}
 	return n;
 }
 
-- 
1.5.4.3

[PATCH 2/2] OMAP3: PM: Early exit on invalid parameters

[Phil Carmody]

From: Phil Carmody <ext-phil.2.carmody@nokia.com>

Currently no clients call with invalid parameters, but as
these are exported functions there ought to be some protection
against what would cause NULL pointer dereferences.

Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
---
 arch/arm/mach-omap2/smartreflex.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index ce7d436..741b51e 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -555,6 +555,8 @@ void sr_start_vddautocomap(int srid, u32 target_opp_no)
 		sr = &sr1;
 	else if (srid == SR2)
 		sr = &sr2;
+	else
+		return;
 
 	if (sr->is_sr_reset == 1) {
 		sr_clk_enable(sr);
@@ -583,6 +585,8 @@ int sr_stop_vddautocomap(int srid)
 		sr = &sr1;
 	else if (srid == SR2)
 		sr = &sr2;
+	else
+		return -EINVAL;
 
 	if (sr->is_autocomp_active == 1) {
 		sr_disable(sr);
@@ -609,6 +613,8 @@ void enable_smartreflex(int srid)
 		sr = &sr1;
 	else if (srid == SR2)
 		sr = &sr2;
+	else
+		return;
 
 	if (sr->is_autocomp_active == 1) {
 		if (sr->is_sr_reset == 1) {
@@ -636,6 +642,8 @@ void disable_smartreflex(int srid)
 		sr = &sr1;
 	else if (srid == SR2)
 		sr = &sr2;
+	else
+		return;
 
 	if (sr->is_autocomp_active == 1) {
 		if (sr->is_sr_reset == 0) {
-- 
1.5.4.3

Re: [PATCH 0/2] OMAP3: PM: More pedantic parameter and error checking in smartreflex

[Kevin Hilman]

Phil Carmody <ext-phil.2.carmody@nokia.com> writes:

> A couple of simple patches to improve error handling in smartreflex.
> The first has a practical benefit of avoiding a string-based search
> in situtations where the result wouldn't be needed. The second is
> simple paranoia.

Thanks, pushing this series today.

Note that I pushed this on top of Rajendra's patch:
"OMAP3: SR: Fix SR driver to check for omap-pm return values"
and had to resolve a couple of conflicts.

Could you please sanity check it?

Thanks,

Kevin

Re: [PATCH 0/2] OMAP3: PM: More pedantic parameter and error checking in smartreflex

[Phil Carmody]

On Wed, 2009-05-13 at 17:10 +0200, ext Kevin Hilman wrote:
> Phil Carmody <ext-phil.2.carmody@nokia.com> writes:
> 
> > A couple of simple patches to improve error handling in smartreflex.
> > The first has a practical benefit of avoiding a string-based search
> > in situtations where the result wouldn't be needed. The second is
> > simple paranoia.
> 
> Thanks, pushing this series today.
> 
> Note that I pushed this on top of Rajendra's patch:
> "OMAP3: SR: Fix SR driver to check for omap-pm return values"
> and had to resolve a couple of conflicts.
> 
> Could you please sanity check it?

Sanity check successful. Many thanks, Kevin.

Phil

Re: [PATCH 0/2] OMAP3: PM: More pedantic parameter and error checking in smartreflex

[Kevin Hilman]

Phil Carmody <ext-phil.2.carmody@nokia.com> writes:

> On Wed, 2009-05-13 at 17:10 +0200, ext Kevin Hilman wrote:
>> Phil Carmody <ext-phil.2.carmody@nokia.com> writes:
>> 
>> > A couple of simple patches to improve error handling in smartreflex.
>> > The first has a practical benefit of avoiding a string-based search
>> > in situtations where the result wouldn't be needed. The second is
>> > simple paranoia.
>> 
>> Thanks, pushing this series today.
>> 
>> Note that I pushed this on top of Rajendra's patch:
>> "OMAP3: SR: Fix SR driver to check for omap-pm return values"
>> and had to resolve a couple of conflicts.
>> 
>> Could you please sanity check it?
>
> Sanity check successful. Many thanks, Kevin.
>

Hmm, you're too fast for me.
Not sure how you tested as I hadn't pushed your changes yet ;)

I just pushed them to my pm branch, but they are not yet sync'd to
tony's tree.  Can you try now.

Kevin

[1] http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git

Re: [PATCH 0/2] OMAP3: PM: More pedantic parameter and error checking in smartreflex

[Phil Carmody]

On Wed, 2009-05-13 at 18:54 +0200, ext Kevin Hilman wrote:
> Phil Carmody <ext-phil.2.carmody@nokia.com> writes:
> 
> > On Wed, 2009-05-13 at 17:10 +0200, ext Kevin Hilman wrote:
> >> Phil Carmody <ext-phil.2.carmody@nokia.com> writes:
> >> 
> >> > A couple of simple patches to improve error handling in smartreflex.
> >> > The first has a practical benefit of avoiding a string-based search
> >> > in situtations where the result wouldn't be needed. The second is
> >> > simple paranoia.
> >> 
> >> Thanks, pushing this series today.
> >> 
> >> Note that I pushed this on top of Rajendra's patch:
> >> "OMAP3: SR: Fix SR driver to check for omap-pm return values"
> >> and had to resolve a couple of conflicts.
> >> 
> >> Could you please sanity check it?
> >
> > Sanity check successful. Many thanks, Kevin.
> 
> Hmm, you're too fast for me.

It's the time zone difference! 

> Not sure how you tested as I hadn't pushed your changes yet ;)
>
> I just pushed them to my pm branch, but they are not yet sync'd to
> tony's tree.  Can you try now.

Freshly pulled, indeed I now see the extent of the rework. I don't know
whether -ENODEV or -EINVAL is better, the former just came to mind first
when I was in the file.

Either way, it looks fine. Thanks again.

Phil