Skip to contentSkip to conversation

ARM: OMAP2+: OPP: Fix to ensure check of right oppdef after bad one

PATCH New Nishanth Menon 3 participants 5 messages May 18, 2012 – Jun 1, 2012

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

Nishanth Menon

author
#0 lore raw

PATCH 1/1 ARM: OMAP2+: OPP: Fix to ensure check of right oppdef after bad one

Commit 9fa2df6b90786301b175e264f5fa9846aba81a65
(ARM: OMAP2+: OPP: allow OPP enumeration to continue if device is not present) makes the logic:

for (i = 0; i < opp_def_size; i++) {
	<snip>
	if (!oh || !oh->od) {
		<snip>
		continue;
	}

<snip>
opp_def++;
}

In short, the moment we hit a "Bad OPP", we end up looping the list comparing against the bad opp definition pointer for the rest of the iteration count. Instead, increment opp_def in the for loop itself and allow continue to be used in code without much thought so that we check the next set of OPP definition pointers :)

Cc Kevin Hilman <[email protected]>
Cc Steve Sakoman <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc linux-omap at vger.kernel.org
Cc linux-arm-kernel at lists.infradead.org
Signed-off-by Nishanth Menon <[email protected]>

---

 arch/arm/mach-omap2/opp.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
arch/arm/mach-omap2/opp.c +1 -2
index de6d464..d8f6dbf 100644
@@ -53,7 +53,7 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def,
 	omap_table_init = 1;
 
 	/* Lets now register with OPP library */
-	for (i = 0; i < opp_def_size; i++) {
+	for (i = 0; i < opp_def_size; i++, opp_def++) {
 		struct omap_hwmod *oh;
 		struct device *dev;
 

@@ -86,7 +86,6 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def,
 					__func__, opp_def->freq,
 					opp_def->hwmod_name, i, r);
 		}
-		opp_def++;
 	}
 
 	return 0;
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/1 ARM: OMAP2+: OPP: Fix to ensure check of right oppdef after bad one

Quoted 19 lines · Nishanth Menon

Commit 9fa2df6b90786301b175e264f5fa9846aba81a65
(ARM: OMAP2+: OPP: allow OPP enumeration to continue if device is not present) makes the logic:

for (i = 0; i < opp_def_size; i++) {
	<snip>
	if (!oh || !oh->od) {
		<snip>
		continue;
	}

<snip>
opp_def++;
}

In short, the moment we hit a "Bad OPP", we end up looping the list comparing against the bad opp definition pointer for the rest of the iteration count. Instead, increment opp_def in the for loop itself and allow continue to be used in code without much thought so that we check the next set of OPP definition pointers :)

Quoted 7 lines
Cc Kevin Hilman <[email protected]>
Cc Steve Sakoman <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc linux-omap at vger.kernel.org
Cc linux-arm-kernel at lists.infradead.org
Signed-off-by Nishanth Menon <[email protected]>

Good catch.

Queuing for my next set of PM fixes for v3.5-rc (branch: for_3.5/fixes/pm-2)

Thanks for the fix!

Kevin

#2 lore raw

PATCH 1/1 ARM: OMAP2+: OPP: Fix to ensure check of right oppdef after bad one

Hi Kevin, Nishanth,

Quoted 27 lines · Kevin Hilman
Quoted 25 lines · Nishanth Menon

Commit 9fa2df6b90786301b175e264f5fa9846aba81a65
(ARM: OMAP2+: OPP: allow OPP enumeration to continue if device is not present) makes the logic:

for (i = 0; i < opp_def_size; i++) {
	<snip>
	if (!oh || !oh->od) {
		<snip>
		continue;
	}

<snip>
opp_def++;
}

In short, the moment we hit a "Bad OPP", we end up looping the list comparing against the bad opp definition pointer for the rest of the iteration count. Instead, increment opp_def in the for loop itself and allow continue to be used in code without much thought so that we check the next set of OPP definition pointers :)

Cc Kevin Hilman <[email protected]>
Cc Steve Sakoman <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc linux-omap at vger.kernel.org
Cc linux-arm-kernel at lists.infradead.org
Quoted 5 lines
Quoted 1 line
Signed-off-by Nishanth Menon <[email protected]>

Good catch.

Queuing for my next set of PM fixes for v3.5-rc (branch: for_3.5/fixes/pm-2)

I think this should also apply for stable, right?
If it should, can you please add a
Cc: stable at vger.kernel.org
?

Thanks

Signature · 2 lines
Regards,
Igor.
author
#3 lore raw

PATCH 1/1 ARM: OMAP2+: OPP: Fix to ensure check of right oppdef after bad one

Quoted 31 lines · Igor Grinberg
Quoted 29 lines · Kevin Hilman
Quoted 26 lines · Nishanth Menon

Commit 9fa2df6b90786301b175e264f5fa9846aba81a65
(ARM: OMAP2+: OPP: allow OPP enumeration to continue if device is not present) makes the logic:
for (i = 0; i < opp_def_size; i++) {
? ? ?<snip>
? ? ?if (!oh || !oh->od) {
? ? ? ? ? ? ?<snip>
? ? ? ? ? ? ?continue;
? ? ?}
<snip>
opp_def++;
}

In short, the moment we hit a "Bad OPP", we end up looping the list comparing against the bad opp definition pointer for the rest of the iteration count. Instead, increment opp_def in the for loop itself and allow continue to be used in code without much thought so that we check the next set of OPP definition pointers :)

Cc Kevin Hilman <[email protected]>
Cc Steve Sakoman <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc linux-omap at vger.kernel.org
Cc linux-arm-kernel at lists.infradead.org
Signed-off-by Nishanth Menon <[email protected]>
Quoted 7 lines
Quoted 3 lines

Good catch.

Queuing for my next set of PM fixes for v3.5-rc (branch: for_3.5/fixes/pm-2)

I think this should also apply for stable, right?
If it should, can you please add a
Cc: stable at vger.kernel.org

I would like to think so, but punting over to Kevin on that decision.

Regards,
Nishanth Menon

#4 lore raw

PATCH 1/1 ARM: OMAP2+: OPP: Fix to ensure check of right oppdef after bad one

In reply to Igor Grinberg
Quoted 32 lines · Igor Grinberg

Hi Kevin, Nishanth,

Quoted 29 lines · Kevin Hilman
Quoted 26 lines · Nishanth Menon

Commit 9fa2df6b90786301b175e264f5fa9846aba81a65
(ARM: OMAP2+: OPP: allow OPP enumeration to continue if device is not present) makes the logic:

for (i = 0; i < opp_def_size; i++) {
	<snip>
	if (!oh || !oh->od) {
		<snip>
		continue;
	}

<snip>
opp_def++;
}

In short, the moment we hit a "Bad OPP", we end up looping the list comparing against the bad opp definition pointer for the rest of the iteration count. Instead, increment opp_def in the for loop itself and allow continue to be used in code without much thought so that we check the next set of OPP definition pointers :)

Cc Kevin Hilman <[email protected]>
Cc Steve Sakoman <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc linux-omap at vger.kernel.org
Cc linux-arm-kernel at lists.infradead.org
Signed-off-by Nishanth Menon <[email protected]>
Quoted 7 lines
Quoted 3 lines

Good catch.

Queuing for my next set of PM fixes for v3.5-rc (branch: for_3.5/fixes/pm-2)

I think this should also apply for stable, right?
If it should, can you please add a
Cc: stable at vger.kernel.org

Yes, added.

Thanks,

Kevin