diff mbox

[2/3] ARM: pxa: palmtreo: fix #ifdefs for leds-gpio device

Message ID 1356204719-3317-3-git-send-email-mikedunn@newsguy.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mike Dunn Dec. 22, 2012, 7:31 p.m. UTC
The #ifdefs around the leds-gpio device platform data are erroneous.  Currently
the device is not instantiated on the centro unless CONFIG_MACH_TREO680 is
defined.

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---

The #ifdefs within palmtreo_leds_init() are a little ugly, but I think the only
alternative would be to instantiate the leds-gpio device platform data for both
treo680 and centro, which would require some of the #defines in palmtreo.h to be
moved outside their containing #ifdef CONFIG_MACH_{TREO680,CENTRO}.

 arch/arm/mach-pxa/palmtreo.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

Comments

Marek Vasut Dec. 23, 2012, 3:20 a.m. UTC | #1
Dear Mike Dunn,

> The #ifdefs around the leds-gpio device platform data are erroneous. 
> Currently the device is not instantiated on the centro unless
> CONFIG_MACH_TREO680 is defined.
> 
> Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
> ---
> 
> The #ifdefs within palmtreo_leds_init() are a little ugly, but I think the
> only alternative would be to instantiate the leds-gpio device platform
> data for both treo680 and centro, which would require some of the #defines
> in palmtreo.h to be moved outside their containing #ifdef
> CONFIG_MACH_{TREO680,CENTRO}.

Shall we not just kill all those ifdefs and let GCC decide what to optimize out? 
I think this kind of simple stuff should just be optimized out anyway.

Best regards,
Marek Vasut
Mike Dunn Dec. 23, 2012, 3:10 p.m. UTC | #2
On 12/22/2012 07:20 PM, Marek Vasut wrote:
> Dear Mike Dunn,
> 
>> The #ifdefs around the leds-gpio device platform data are erroneous. 
>> Currently the device is not instantiated on the centro unless
>> CONFIG_MACH_TREO680 is defined.
>>
>> Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
>> ---
>>
>> The #ifdefs within palmtreo_leds_init() are a little ugly, but I think the
>> only alternative would be to instantiate the leds-gpio device platform
>> data for both treo680 and centro, which would require some of the #defines
>> in palmtreo.h to be moved outside their containing #ifdef
>> CONFIG_MACH_{TREO680,CENTRO}.
> 
> Shall we not just kill all those ifdefs and let GCC decide what to optimize out? 
> I think this kind of simple stuff should just be optimized out anyway.


You're suggesting that all the #ifdef CONFIG_MACH_{TREO680,CENTRO} be removed
from palmtreo.h?  Yes, this might be better.  I'll look into it.

Thanks again,
Mike
diff mbox

Patch

diff --git a/arch/arm/mach-pxa/palmtreo.c b/arch/arm/mach-pxa/palmtreo.c
index dcc38a1..cb9cc7b 100644
--- a/arch/arm/mach-pxa/palmtreo.c
+++ b/arch/arm/mach-pxa/palmtreo.c
@@ -357,7 +357,9 @@  static struct gpio_led_platform_data treo680_gpio_led_info = {
 	.leds		= treo680_gpio_leds,
 	.num_leds	= ARRAY_SIZE(treo680_gpio_leds),
 };
+#endif
 
+#ifdef CONFIG_MACH_CENTRO
 static struct gpio_led centro_gpio_leds[] = {
 	{
 		.name			= "centro:vibra:vibra",
@@ -381,25 +383,25 @@  static struct gpio_led_platform_data centro_gpio_led_info = {
 	.leds		= centro_gpio_leds,
 	.num_leds	= ARRAY_SIZE(centro_gpio_leds),
 };
+#endif
 
 static struct platform_device palmtreo_leds = {
 	.name   = "leds-gpio",
 	.id     = -1,
-	.dev    = {
-		.platform_data  = &treo680_gpio_led_info,
-	}
 };
 
 static void __init palmtreo_leds_init(void)
 {
+#ifdef CONFIG_MACH_CENTRO
 	if (machine_is_centro())
 		palmtreo_leds.dev.platform_data = &centro_gpio_led_info;
-
+#endif
+#ifdef CONFIG_MACH_TREO680
+	if (machine_is_treo680())
+		palmtreo_leds.dev.platform_data = &treo680_gpio_led_info;
+#endif
 	platform_device_register(&palmtreo_leds);
 }
-#else
-static inline void palmtreo_leds_init(void) {}
-#endif
 
 /******************************************************************************
  * Machine init