ARM: OMAP3: hwmod: fix variant registration and remove SmartReflex from common list

PATCH New Paul Walmsley 3 participants 6 messages Oct 6, 2011

Subsystems (4)

Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP hwmod data

lore

Share & tools

lore

Paul Walmsley

author maintainer

2011-10-06 20:39 UTC #0 lore raw

PATCH 1/1 ARM: OMAP3: hwmod: fix variant registration and remove SmartReflex from common list

Commit d6504acd2125984c61dce24727dd3842d0144015 ("OMAP2+: hwmod:
remove OMAP_CHIP*") tests the inverse condition of what it should be testing for the return value from omap_hwmod_register(). This causes several IP blocks to not be registered on several OMAP3 family devices.

Fixing that bug also unmasked another bug, originally reported by Chase Maupin <[email protected]> and then subsequently by Abhilash K V <[email protected]>, which caused SmartReflex IP blocks to be registered on SoCs that don't support them.

Signed-off-by Paul Walmsley <[email protected]>

Cc Chase Maupin <[email protected]>

Cc Abhilash K V <[email protected]>

---

This fixes a pretty embarrassing "brown paper bag" bug of mine in one of the cleanup patches. Tony, it would be good if you could send this to Arnd ASAP and merge into your cleanup branch. Tested on 3430SDP, 37XX EVM, and OMAP3503.

 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +4 -4

index ab35acb..53b6706 100644
@@ -13,6 +13,8 @@
  *
  * XXX these should be marked initdata for multi-OMAP kernels
  */
+#include <linux/err.h>
+
 #include <plat/omap_hwmod.h>
 #include <mach/irqs.h>
 #include <plat/cpu.h>

@@ -3161,8 +3163,6 @@ static __initdata struct omap_hwmod *omap3xxx_hwmods[] = {
 	&omap3xxx_i2c1_hwmod,
 	&omap3xxx_i2c2_hwmod,
 	&omap3xxx_i2c3_hwmod,
-	&omap34xx_sr1_hwmod,
-	&omap34xx_sr2_hwmod,
 
 	/* gpio class */
 	&omap3xxx_gpio1_hwmod,

@@ -3240,7 +3240,7 @@ int __init omap3xxx_hwmod_init(void)
 
 	/* Register hwmods common to all OMAP3 */
 	r = omap_hwmod_register(omap3xxx_hwmods);
-	if (!r)
+	if (IS_ERR_VALUE(r))
 		return r;
 
 	rev = omap_rev();

@@ -3265,7 +3265,7 @@ int __init omap3xxx_hwmod_init(void)
 	};
 
 	r = omap_hwmod_register(h);
-	if (!r)
+	if (IS_ERR_VALUE(r))
 		return r;
 
 	/*

Signature · 1 line

1.7.6.3

Tony Lindgren

maintainer

2011-10-06 20:43 UTC #1 lore raw

PATCH 1/1 ARM: OMAP3: hwmod: fix variant registration and remove SmartReflex from common list

Quoted 14 lines · Paul Walmsley

Commit d6504acd2125984c61dce24727dd3842d0144015 ("OMAP2+: hwmod:
remove OMAP_CHIP*") tests the inverse condition of what it should be testing for the return value from omap_hwmod_register(). This causes several IP blocks to not be registered on several OMAP3 family devices.
Fixing that bug also unmasked another bug, originally reported by Chase Maupin <[email protected]> and then subsequently by Abhilash K V <[email protected]>, which caused SmartReflex IP blocks to be registered on SoCs that don't support them.
Signed-off-by Paul Walmsley <[email protected]>
Cc Chase Maupin <[email protected]>
Cc Abhilash K V <[email protected]>

Quoted 6 lines

---
This fixes a pretty embarrassing "brown paper bag" bug of mine in one of the cleanup patches. Tony, it would be good if you could send this to Arnd ASAP and merge into your cleanup branch. Tested on 3430SDP, 37XX EVM, and OMAP3503.

OK thanks, applying into fixes-part2 branch.

Regards,

Tony

Russell King - ARM Linux

2011-10-06 22:35 UTC #2 lore raw

PATCH 1/1 ARM: OMAP3: hwmod: fix variant registration and remove SmartReflex from common list

In reply to Paul Walmsley

Quoted 10 lines · Paul Walmsley

arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +1 -1

@@ -3240,7 +3240,7 @@ int __init omap3xxx_hwmod_init(void)
 
 	/* Register hwmods common to all OMAP3 */
 	r = omap_hwmod_register(omap3xxx_hwmods);
-	if (!r)
+	if (IS_ERR_VALUE(r))
 		return r;
 
 	rev = omap_rev();

arch/arm/mach-omap2/omap_hwmod_3xxx_data.c int __init omap3xxx_hwmod_init(void) · line 3269

Quoted 6 lines

arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +1 -1

@@ -3265,7 +3265,7 @@ int __init omap3xxx_hwmod_init(void)
 	};
 
 	r = omap_hwmod_register(h);
-	if (!r)
+	if (IS_ERR_VALUE(r))
 		return r;

What _really_ is the return value meaning of omap_hwmod_register() ?

If it's zero for success, negative for error codes, then don't be clever and try to use IS_ERR_VALUE() for that - just use a standard < 0 test instead.

You only need to use IS_ERR_VALUE() for stuff that may return negative errno values _and_ which may return valid addresses which may be misinterpreted as negative numbers.

Paul Walmsley

author maintainer

2011-10-06 23:19 UTC #3 lore raw

PATCH 1/1 ARM: OMAP3: hwmod: fix variant registration and remove SmartReflex from common list

Quoted 20 lines · Russell King - ARM Linux

Quoted 10 lines · Paul Walmsley
arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +1 -1
@@ -3240,7 +3240,7 @@ int __init omap3xxx_hwmod_init(void)
 
 	/* Register hwmods common to all OMAP3 */
 	r = omap_hwmod_register(omap3xxx_hwmods);
-	if (!r)
+	if (IS_ERR_VALUE(r))
 		return r;
 
 	rev = omap_rev();
arch/arm/mach-omap2/omap_hwmod_3xxx_data.c int __init omap3xxx_hwmod_init(void) · line 3269
Quoted 6 lines
arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +1 -1
@@ -3265,7 +3265,7 @@ int __init omap3xxx_hwmod_init(void)
 	};
 
 	r = omap_hwmod_register(h);
-	if (!r)
+	if (IS_ERR_VALUE(r))
 		return r;
What _really_ is the return value meaning of omap_hwmod_register() ?

Quoted 7 lines

If it's zero for success, negative for error codes, then don't be clever and try to use IS_ERR_VALUE() for that - just use a standard < 0 test instead.
You only need to use IS_ERR_VALUE() for stuff that may return negative errno values _and_ which may return valid addresses which may be misinterpreted as negative numbers.

Thanks, that's a good point - fixed.

- Paul

Paul Walmsley

author maintainer

2011-10-06 23:24 UTC #4 lore raw

PATCH v2 1/1 ARM: OMAP3: hwmod: fix variant registration and remove SmartReflex from common list

In reply to Russell King - ARM Linux

Thanks to Russell King - ARM Linux <[email protected]> for comments on a previous version of the patch.

Signed-off-by Paul Walmsley <[email protected]>

Cc Chase Maupin <[email protected]>

Cc Abhilash K V <[email protected]>

Cc Russell King - ARM Linux <[email protected]>

---

This version incorporates some comments from RMK.

 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +2 -4

index ab35acb..0771dc6 100644
@@ -3161,8 +3161,6 @@ static __initdata struct omap_hwmod *omap3xxx_hwmods[] = {
 	&omap3xxx_i2c1_hwmod,
 	&omap3xxx_i2c2_hwmod,
 	&omap3xxx_i2c3_hwmod,
-	&omap34xx_sr1_hwmod,
-	&omap34xx_sr2_hwmod,
 
 	/* gpio class */
 	&omap3xxx_gpio1_hwmod,

@@ -3240,7 +3238,7 @@ int __init omap3xxx_hwmod_init(void)
 
 	/* Register hwmods common to all OMAP3 */
 	r = omap_hwmod_register(omap3xxx_hwmods);
-	if (!r)
+	if (r < 0)
 		return r;
 
 	rev = omap_rev();

@@ -3265,7 +3263,7 @@ int __init omap3xxx_hwmod_init(void)
 	};
 
 	r = omap_hwmod_register(h);
-	if (!r)
+	if (r < 0)
 		return r;
 
 	/*

Signature · 1 line

1.7.6.3

Tony Lindgren

maintainer

2011-10-06 23:29 UTC #5 lore raw

PATCH v2 1/1 ARM: OMAP3: hwmod: fix variant registration and remove SmartReflex from common list

Quoted 14 lines · Paul Walmsley

Commit d6504acd2125984c61dce24727dd3842d0144015 ("OMAP2+: hwmod:
remove OMAP_CHIP*") tests the inverse condition of what it should be testing for the return value from omap_hwmod_register(). This causes several IP blocks to not be registered on several OMAP3 family devices.
Fixing that bug also unmasked another bug, originally reported by Chase Maupin <[email protected]> and then subsequently by Abhilash K V <[email protected]>, which caused SmartReflex IP blocks to be registered on SoCs that don't support them.
Thanks to Russell King - ARM Linux <[email protected]> for comments on a previous version of the patch.

Quoted 7 lines

Signed-off-by Paul Walmsley <[email protected]>
Cc Chase Maupin <[email protected]>
Cc Abhilash K V <[email protected]>
Cc Russell King - ARM Linux <[email protected]>
---
This version incorporates some comments from RMK.

Thanks, updating in fixes-part2.

Tony