kill compile warning on twl4030-power.c

PATCH In review Felipe Balbi 3 participants 11 messages Sep 15, 2008 – Sep 23, 2008

lore

Share & tools

lore

Felipe Balbi

author

2008-09-15 15:12 UTC #0 lore raw

PATCH 1/1 kill compile warning on twl4030-power.c

From: Felipe Balbi <[email protected]>

This part of the patch was left out on previous merge. Here's a fix for that.

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

---

drivers/i2c/chips/twl4030-power.c +0 -1

index 04a13aa..afc19cd 100644
@@ -215,7 +215,6 @@ static int __init twl4030_write_script(u8 address, struct triton_ins *script,
 static int __init config_sleep_wake_sequence(void)
 {
        int err = 0;
-       u8 data;
 
        /*
         * CLKREQ is pulled high on the 2430SDP, therefore, we need to take

Signature · 1 line

balbi

Peter 'p2' De Schrijver

2008-09-15 15:55 UTC #1 lore raw

Quoted 5 lines · ext Felipe Balbi

From: Felipe Balbi <[email protected]>
This part of the patch was left out on previous merge. Here's a fix for that.

I doubt this works if you compile for SDP or labrador. OTOH the code for that bit looks suspicious :)

Cheers,

Peter.

Signature · 1 line

goa is a state of mind

Felipe Balbi

author

2008-09-15 16:21 UTC #2 lore raw

Quoted 9 lines · On Mon, Sep 15, 2008 at 06:55:12PM +0300, Peter 'p2' De Schrijver wrote:

Quoted 5 lines · ext Felipe Balbi
From: Felipe Balbi <[email protected]>
This part of the patch was left out on previous merge. Here's a fix for that.
I doubt this works if you compile for SDP or labrador. OTOH the code for that bit looks suspicious :)

Hmmm... that's true. That variable is only used on 3430sdp and 3430 labrador. How about this version:

drivers/i2c/chips/twl4030-power.c +9 -8

index 04a13aa..b8723e8 100644
@@ -245,14 +245,15 @@ static int __init config_sleep_wake_sequence(void)
        err |= twl4030_write_script(0x2F, sleep_off_seq,
                                        ARRAY_SIZE(sleep_off_seq));
 
-#if defined(CONFIG_MACH_OMAP_3430SDP) || defined(CONFIG_MACH_OMAP_3430LABRADOR)
-       /* Disabling AC charger effect on sleep-active transitions */
-       err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
-                                       R_CFG_P1_TRANSITION);
-       data &= 0x0;
-       err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, data ,
-                                       R_CFG_P1_TRANSITION);
-#endif
+       if (machine_is_omap_3430sdp()) {
+               /* Disabling AC charger effect on sleep-active transitions */
+               err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
+                               R_CFG_P1_TRANSITION);
+               data &= 0x0;
+               err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, data ,
+                               R_CFG_P1_TRANSITION);
+       }
+
        /* P1/P2/P3 LVL_WAKEUP should be on LEVEL */
        err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, LVL_WAKEUP,
                                        R_P1_SW_EVENTS);

It's weird that a grep for LABRADOR on arch/arm does not return anything. Are we missing some patches ??

Signature · 1 line

balbi

Felipe Balbi

author

2008-09-15 16:23 UTC #3 lore raw

Quoted 8 lines · Felipe Balbi

Quoted 7 lines · On Mon, Sep 15, 2008 at 06:55:12PM +0300, Peter 'p2' De Schrijver wrote:
Quoted 5 lines · ext Felipe Balbi
From: Felipe Balbi <[email protected]>
This part of the patch was left out on previous merge. Here's a fix for that.

Quoted 5 lines

Quoted 2 lines
I doubt this works if you compile for SDP or labrador. OTOH the code for that bit looks suspicious :)
Hmmm... that's true. That variable is only used on 3430sdp and 3430 labrador. How about this version:

and I missed the #include <asm/mach-types.h>

Quoted 28 lines

drivers/i2c/chips/twl4030-power.c +9 -8

index 04a13aa..b8723e8 100644
@@ -245,14 +245,15 @@ static int __init config_sleep_wake_sequence(void)
        err |= twl4030_write_script(0x2F, sleep_off_seq,
                                        ARRAY_SIZE(sleep_off_seq));
 
-#if defined(CONFIG_MACH_OMAP_3430SDP) || defined(CONFIG_MACH_OMAP_3430LABRADOR)
-       /* Disabling AC charger effect on sleep-active transitions */
-       err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
-                                       R_CFG_P1_TRANSITION);
-       data &= 0x0;
-       err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, data ,
-                                       R_CFG_P1_TRANSITION);
-#endif
+       if (machine_is_omap_3430sdp()) {
+               /* Disabling AC charger effect on sleep-active transitions */
+               err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
+                               R_CFG_P1_TRANSITION);
+               data &= 0x0;
+               err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, data ,
+                               R_CFG_P1_TRANSITION);
+       }
+
        /* P1/P2/P3 LVL_WAKEUP should be on LEVEL */
        err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, LVL_WAKEUP,
                                        R_P1_SW_EVENTS);

Signature · 1 line

balbi

Peter 'p2' De Schrijver

2008-09-15 16:24 UTC #4 lore raw

In reply to Felipe Balbi

Hi,

Quoted 11 lines

Quoted 2 lines
I doubt this works if you compile for SDP or labrador. OTOH the code for that bit looks suspicious :)
Hmmm... that's true. That variable is only used on 3430sdp and 3430 labrador. How about this version:
drivers/i2c/chips/twl4030-power.c +0 -0
index 04a13aa..b8723e8 100644

drivers/i2c/chips/twl4030-power.c static int __init config_sleep_wake_sequence(void) · line 252

Quoted 8 lines

drivers/i2c/chips/twl4030-power.c +0 -5

@@ -245,14 +245,15 @@ static int __init config_sleep_wake_sequence(void)
        err |= twl4030_write_script(0x2F, sleep_off_seq,
                                        ARRAY_SIZE(sleep_off_seq));
 
-#if defined(CONFIG_MACH_OMAP_3430SDP) || defined(CONFIG_MACH_OMAP_3430LABRADOR)
-       /* Disabling AC charger effect on sleep-active transitions */
-       err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
-                                       R_CFG_P1_TRANSITION);
-       data &= 0x0;

This does not make sense :)

Quoted 16 lines

drivers/i2c/chips/twl4030-power.c +9 -3

-       err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, data ,
-                                       R_CFG_P1_TRANSITION);
-#endif
+       if (machine_is_omap_3430sdp()) {
+               /* Disabling AC charger effect on sleep-active transitions */
+               err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
+                               R_CFG_P1_TRANSITION);
+               data &= 0x0;
+               err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, data ,
+                               R_CFG_P1_TRANSITION);
+       }
+
        /* P1/P2/P3 LVL_WAKEUP should be on LEVEL */
        err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, LVL_WAKEUP,
                                        R_P1_SW_EVENTS);

Quoted 3 lines

It's weird that a grep for LABRADOR on arch/arm does not return anything. Are we missing some patches ??

Hmm. Good question :)

Cheers,

Peter.

Signature · 1 line

goa is a state of mind

Felipe Balbi

author

2008-09-15 16:36 UTC #5 lore raw

Quoted 16 lines · On Mon, Sep 15, 2008 at 07:24:21PM +0300, Peter 'p2' De Schrijver wrote:

Hi,
Quoted 14 lines
Quoted 2 lines
I doubt this works if you compile for SDP or labrador. OTOH the code for that bit looks suspicious :)
Hmmm... that's true. That variable is only used on 3430sdp and 3430 labrador. How about this version:
drivers/i2c/chips/twl4030-power.c +0 -0
index 04a13aa..b8723e8 100644
@@ -245,14 +245,15 @@ static int __init config_sleep_wake_sequence(void)
        err |= twl4030_write_script(0x2F, sleep_off_seq,
                                        ARRAY_SIZE(sleep_off_seq));
 

Quoted 7 lines

drivers/i2c/chips/twl4030-power.c static int __init config_sleep_wake_sequence(void) · line 252

Quoted 5 lines

drivers/i2c/chips/twl4030-power.c +0 -5

-#if defined(CONFIG_MACH_OMAP_3430SDP) || defined(CONFIG_MACH_OMAP_3430LABRADOR)
-       /* Disabling AC charger effect on sleep-active transitions */
-       err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
-                                       R_CFG_P1_TRANSITION);
-       data &= 0x0;

This does not make sense :)

Why not? That data variable is only used on 3430SDP and LABRADOR anyways. Any other build will keep generating "unused variable" warnings and that's annoying :-p

Signature · 1 line

balbi

Peter 'p2' De Schrijver

2008-09-15 17:30 UTC #6 lore raw

Quoted 11 lines

Quoted 7 lines
drivers/i2c/chips/twl4030-power.c static int __init config_sleep_wake_sequence(void) · line 252
Quoted 5 lines
drivers/i2c/chips/twl4030-power.c +0 -5
-#if defined(CONFIG_MACH_OMAP_3430SDP) || defined(CONFIG_MACH_OMAP_3430LABRADOR)
-       /* Disabling AC charger effect on sleep-active transitions */
-       err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
-                                       R_CFG_P1_TRANSITION);
-       data &= 0x0;
This does not make sense :)
Why not? That data variable is only used on 3430SDP and LABRADOR anyways. Any other build will keep generating "unused variable" warnings and that's annoying :-p

Because a logical and with 0 will always result in 0. So the read is useless. So the original code is probably wrong.

Cheers,

Peter.

Signature · 1 line

goa is a state of mind

Felipe Balbi

author

2008-09-15 17:34 UTC #7 lore raw

Quoted 6 lines · On Mon, Sep 15, 2008 at 08:30:20PM +0300, Peter 'p2' De Schrijver wrote:

Quoted 6 lines

Quoted 5 lines

drivers/i2c/chips/twl4030-power.c +0 -5

-#if defined(CONFIG_MACH_OMAP_3430SDP) || defined(CONFIG_MACH_OMAP_3430LABRADOR)
-       /* Disabling AC charger effect on sleep-active transitions */
-       err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
-                                       R_CFG_P1_TRANSITION);
-       data &= 0x0;

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
This does not make sense :)
Why not? That data variable is only used on 3430SDP and LABRADOR anyways. Any other build will keep generating "unused variable" warnings and that's annoying :-p
Because a logical and with 0 will always result in 0. So the read is useless. So the original code is probably wrong.

ehehe, but that's not what I was changing :-p

Signature · 1 line

balbi

Felipe Balbi

author

2008-09-15 18:23 UTC #8 lore raw

In reply to Peter 'p2' De Schrijver

Quoted 6 lines · On Mon, Sep 15, 2008 at 08:30:20PM +0300, Peter 'p2' De Schrijver wrote:

Quoted 6 lines

Quoted 5 lines

drivers/i2c/chips/twl4030-power.c +0 -5

-#if defined(CONFIG_MACH_OMAP_3430SDP) || defined(CONFIG_MACH_OMAP_3430LABRADOR)
-       /* Disabling AC charger effect on sleep-active transitions */
-       err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
-                                       R_CFG_P1_TRANSITION);
-       data &= 0x0;

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
This does not make sense :)
Why not? That data variable is only used on 3430SDP and LABRADOR anyways. Any other build will keep generating "unused variable" warnings and that's annoying :-p
Because a logical and with 0 will always result in 0. So the read is useless. So the original code is probably wrong.

Maybe you meant data &= ~(1 << 0); ???

Signature · 1 line

balbi

Peter 'p2' De Schrijver

2008-09-16 12:29 UTC #9 lore raw

PATCH 1/1 Fix build for beagleboard

Signed-off-by Peter 'p2' De Schrijver <[email protected]>

---
 drivers/i2c/chips/twl4030-power.c |   25 +++++++++++++++----------
 1 files changed, 15 insertions(+), 10 deletions(-)

drivers/i2c/chips/twl4030-power.c +10 -9

index 195c3c4..ea8f447 100644
@@ -212,7 +216,6 @@ static int __init twl4030_write_script(u8 address, struct triton_ins *script,
 static int __init config_sleep_wake_sequence(void)
 {
 	int err = 0;
-	u8 data;
 
 	/*
 	 * CLKREQ is pulled high on the 2430SDP, therefore, we need to take

@@ -242,14 +245,16 @@ static int __init config_sleep_wake_sequence(void)
 	err |= twl4030_write_script(0x2F, sleep_off_seq,
 					ARRAY_SIZE(sleep_off_seq));
 
-#if defined(CONFIG_MACH_OMAP_3430SDP) || defined(CONFIG_MACH_OMAP_3430LABRADOR)
-	/* Disabling AC charger effect on sleep-active transitions */
-	err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
-					R_CFG_P1_TRANSITION);
-	data &= 0x0;
-	err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, data ,
-					R_CFG_P1_TRANSITION);
-#endif
+	if (machine_is_omap_3430sdp() || machine_is_omap_ldp()) {
+		u8 data;
+		/* Disabling AC charger effect on sleep-active transitions */
+		err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
+						R_CFG_P1_TRANSITION);
+		data &= ~(1<<1);
+		err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, data ,
+						R_CFG_P1_TRANSITION);
+	}
+
 	/* P1/P2/P3 LVL_WAKEUP should be on LEVEL */
 	err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, LVL_WAKEUP,
 					R_P1_SW_EVENTS);

Signature · 1 line

1.5.6.3

Tony Lindgren

2008-09-23 10:54 UTC #10 lore raw

Re: [PATCH] Fix build for beagleboard

* Peter 'p2' De Schrijver <[email protected]> [080916 15:30]:

Pushing.

Tony

Quoted 50 lines

Signed-off-by Peter 'p2' De Schrijver <[email protected]>

---
 drivers/i2c/chips/twl4030-power.c |   25 +++++++++++++++----------
 1 files changed, 15 insertions(+), 10 deletions(-)

drivers/i2c/chips/twl4030-power.c +10 -9

index 195c3c4..ea8f447 100644
@@ -212,7 +216,6 @@ static int __init twl4030_write_script(u8 address, struct triton_ins *script,
 static int __init config_sleep_wake_sequence(void)
 {
 	int err = 0;
-	u8 data;
 
 	/*
 	 * CLKREQ is pulled high on the 2430SDP, therefore, we need to take

@@ -242,14 +245,16 @@ static int __init config_sleep_wake_sequence(void)
 	err |= twl4030_write_script(0x2F, sleep_off_seq,
 					ARRAY_SIZE(sleep_off_seq));
 
-#if defined(CONFIG_MACH_OMAP_3430SDP) || defined(CONFIG_MACH_OMAP_3430LABRADOR)
-	/* Disabling AC charger effect on sleep-active transitions */
-	err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
-					R_CFG_P1_TRANSITION);
-	data &= 0x0;
-	err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, data ,
-					R_CFG_P1_TRANSITION);
-#endif
+	if (machine_is_omap_3430sdp() || machine_is_omap_ldp()) {
+		u8 data;
+		/* Disabling AC charger effect on sleep-active transitions */
+		err |= twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &data,
+						R_CFG_P1_TRANSITION);
+		data &= ~(1<<1);
+		err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, data ,
+						R_CFG_P1_TRANSITION);
+	}
+
 	/* P1/P2/P3 LVL_WAKEUP should be on LEVEL */
 	err |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, LVL_WAKEUP,
 					R_P1_SW_EVENTS);

Signature · 6 lines

1.5.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html