Revamped the backlight state machine and added buttonlight controls. Go to the Info/debug menu and test out the new modes for buttonlights on the Gigabeat - ON, OFF, Faint, and flicker - flickers on touchpad and disk reads.

git-svn-id: svn://svn.rockbox.org/rockbox/trunk@11935 a1c6a512-1295-4272-9138-f99709370657

7 files changed, 586 insertions, 49 deletions

diff --git a/apps/debug_menu.c b/apps/debug_menu.c
index 4f2266ff47..3785374338 100644
--- a/apps/debug_menu.c
+++ b/apps/debug_menu.c
@@ -2089,6 +2089,98 @@ static bool dbg_lcd_power_off(void)
     return false;
 }
 
+#include "backlight-target.h"
+
+static bool dbg_buttonlights(void)
+{
+    unsigned short mode_changed = 1, mode = 0;
+    unsigned short which_led = BUTTONLIGHT_LED_ALL;
+    
+    lcd_setmargins(0, 0);
+    for (;;)
+    {
+        int button;
+
+
+        if (mode_changed)
+        {
+            lcd_clear_display();
+            lcd_puts(0, 0, "Button light support");
+            lcd_puts(0, 1, "Press UP for mode change");
+            lcd_puts(0, 2, "Press DOWN for buttonlight selection");
+
+            switch (mode)
+            {
+                case 0: 
+                lcd_puts(1, 3, "Off");
+                __buttonlight_mode(BUTTONLIGHT_OFF);
+                break;
+                
+                case 1: 
+                lcd_puts(1, 3, "On");
+                __buttonlight_mode(BUTTONLIGHT_ON);
+                break;
+                
+                case 2: 
+                lcd_puts(1, 3, "Faint");
+                __buttonlight_mode(BUTTONLIGHT_FAINT);
+                break;
+                
+                case 3: 
+                lcd_puts(1, 3, "Flicker");
+                __buttonlight_mode(BUTTONLIGHT_FLICKER);
+                break;
+                
+            }
+            mode_changed = 0;
+            lcd_update();
+        }
+        
+
+
+        /* does nothing unless in flicker mode */
+        /* the parameter sets the brightness */
+        __buttonlight_flicker(20); 
+        button = get_action(CONTEXT_STD,HZ/5);
+        switch(button)
+        {
+            case ACTION_STD_PREV:
+            mode++;
+            if (mode > 3) mode = 0;
+            mode_changed = 1;
+            break;
+
+            case ACTION_STD_NEXT:
+            if (which_led == BUTTONLIGHT_LED_ALL)
+            {
+                which_led = BUTTONLIGHT_LED_MENU;
+            }
+            else
+            {
+                which_led = BUTTONLIGHT_LED_ALL;
+            }
+
+            __buttonlight_select(which_led);
+
+            break;
+
+                
+            case ACTION_STD_OK:
+            case ACTION_STD_CANCEL:
+            action_signalscreenchange();
+            return false;
+
+            default:
+            sleep(HZ/10);
+            break;
+        }
+    }
+    return false;
+}
+
+
+
+
 #endif
 
 #if defined(HAVE_EEPROM) && !defined(HAVE_EEPROM_SETTINGS)
@@ -2141,6 +2233,8 @@ bool debug_menu(void)
     static const struct menu_item items[] = {
 #if defined(TOSHIBA_GIGABEAT_F) && !defined(SIMULATOR)
         { "LCD Power Off", dbg_lcd_power_off },
+        { "Button Light modes", dbg_buttonlights },
+            
 #endif
 #if CONFIG_CPU == SH7034 || defined(CPU_COLDFIRE)
         { "Dump ROM contents", dbg_save_roms },
diff --git a/firmware/target/arm/gigabeat/meg-fx/ata-meg-fx.c b/firmware/target/arm/gigabeat/meg-fx/ata-meg-fx.c
index d098d83de0..5e0fd8429c 100644
--- a/firmware/target/arm/gigabeat/meg-fx/ata-meg-fx.c
+++ b/firmware/target/arm/gigabeat/meg-fx/ata-meg-fx.c
@@ -26,6 +26,7 @@
 #include "pcf50606.h"
 #include "ata-target.h"
 #include "mmu-meg-fx.h"
+#include "backlight-target.h"
 
 void ata_reset(void)
 {
@@ -54,6 +55,8 @@ void ata_device_init(void)
 
 void copy_read_sectors(unsigned char* buf, int wordcount)
 {
+    __buttonlight_flicker(DEFAULT_BRIGHTNESS_SETTING);
+    
     /* Unaligned transfer - slow copy */
     if ( (unsigned long)buf & 1)
     {   /* not 16-bit aligned, copy byte by byte */
diff --git a/firmware/target/arm/gigabeat/meg-fx/backlight-meg-fx.c b/firmware/target/arm/gigabeat/meg-fx/backlight-meg-fx.c
index 12a5b55a3c..173d2ce38b 100644
--- a/firmware/target/arm/gigabeat/meg-fx/backlight-meg-fx.c
+++ b/firmware/target/arm/gigabeat/meg-fx/backlight-meg-fx.c
@@ -24,45 +24,100 @@
 #include "lcd.h"
 #include "sc606-meg-fx.h"
 
+#define FLICKER_PERIOD 15
+#define BUTTONLIGHT_MENU (SC606_LED_B1)
+#define BUTTONLIGHT_ALL  (SC606_LED_B1 | SC606_LED_B2 | SC606_LED_C1 | SC606_LED_C2)
 
-static void backlight_fade_service(void);
+static void led_control_service(void);
 static unsigned short backlight_brightness;
-static unsigned short backlight_fading;
 static unsigned short backlight_current;
 static unsigned short backlight_target;
 static unsigned short time_til_fade;
 static unsigned short fade_interval;
+static unsigned char backlight_leds; 
+
+
+static enum backlight_states
+{
+    BACKLIGHT_CONTROL_IDLE,
+    BACKLIGHT_CONTROL_OFF,
+    BACKLIGHT_CONTROL_ON,
+    BACKLIGHT_CONTROL_SET,
+    BACKLIGHT_CONTROL_FADE_OFF,
+    BACKLIGHT_CONTROL_FADE_ON,
+    BACKLIGHT_CONTROL_FADE_ON_FROM_OFF
+} backlight_control;
+
+
+
+enum buttonlight_states
+{
+    /* turn button lights off */
+    BUTTONLIGHT_MODE_OFF_ENTRY,
+    BUTTONLIGHT_MODE_OFF,
+
+    /* turns button lights on to same brightness as backlight */
+    BUTTONLIGHT_MODE_ON_ENTRY,
+    BUTTONLIGHT_MODE_ON,
+
+    /* turns button lights on to minimum */
+    BUTTONLIGHT_MODE_FAINT_ENTRY,
+    BUTTONLIGHT_MODE_FAINT,
+
+    /* allows button lights to flicker when triggered */
+    BUTTONLIGHT_MODE_FLICKER_ENTRY,
+    BUTTONLIGHT_MODE_FLICKER,
+    BUTTONLIGHT_MODE_FLICKERING,
+
+    /* button lights glow */
+    BUTTONLIGHT_MODE_GLOW_ENTRY,
+    BUTTONLIGHT_MODE_GLOW,
+
+    /* internal use only */
+    BUTTONLIGHT_HELPER_SET,
+    BUTTONLIGHT_HELPER_SET_FINAL,
+};
 
 
-static int confval = SC606_LOW_FREQ;
+
+static char buttonlight_leds; 
+static unsigned short buttonlight_setting;
+static unsigned short buttonlight_current;
+static unsigned char buttonlight_selected;
+static enum buttonlight_states buttonlight_state;
+static enum buttonlight_states buttonlight_saved_state;
+static unsigned short buttonlight_flickering;
+static unsigned short buttonlight_flicker_now;
+static unsigned short buttonlight_flicker_brightness;
+
 
 
 
 void __backlight_init(void)
 {
-    backlight_fading = false;
+    backlight_control = BACKLIGHT_CONTROL_IDLE;
+
+    backlight_current = DEFAULT_BRIGHTNESS_SETTING;
 
-    /* current is from settings */
-    backlight_current = 50;
+    buttonlight_state = BUTTONLIGHT_MODE_OFF;
+
+    buttonlight_selected = 0x04;
     
-    /* put the fade tick on the list */
-    tick_add_task(backlight_fade_service);
+    /* put the led control on the tick list */
+    tick_add_task(led_control_service);
 }
 
 
 
 void __backlight_on(void)
 {
-    confval |= (SC606_LED_A1 | SC606_LED_A2);
-    sc606_write(SC606_REG_CONF, confval);
+    backlight_control = BACKLIGHT_CONTROL_ON;
 }
 
 
-
 void __backlight_off(void)
 {
-    confval &= ~(SC606_LED_A1 | SC606_LED_A2);
-    sc606_write(SC606_REG_CONF, confval);
+    backlight_control = BACKLIGHT_CONTROL_OFF;
 }
 
 
@@ -71,46 +126,350 @@ void __backlight_off(void)
 void __backlight_set_brightness(int brightness)
 {
     /* stop the interrupt from messing us up */
-    backlight_fading = false;
+    backlight_control = BACKLIGHT_CONTROL_IDLE;
 
-    backlight_brightness = brightness;
+    backlight_brightness = brightness + 1;
 
     /* only set the brightness if it is different from the current */
     if (backlight_brightness != backlight_current)
     {
-        backlight_target = brightness;
-        fade_interval = time_til_fade = 1;
-        backlight_fading = true;
+        backlight_control = BACKLIGHT_CONTROL_SET;
     }
 }
 
 
 
-static void backlight_fade_service(void)
+/* only works if the buttonlight mode is set to flicker */
+void __buttonlight_flicker(unsigned short brightness)
 {
-    if (!backlight_fading || --time_til_fade) return;
-            
-    if (backlight_target > backlight_current)
+    /* clip the setting */ 
+    if (brightness > MAX_BRIGHTNESS_SETTING)
     {
-        backlight_current++;
+        brightness = MAX_BRIGHTNESS_SETTING;
     }
-    else
+
+    /* add one because we subtract later for full range */
+    buttonlight_flicker_brightness = brightness + 1;
+    buttonlight_flicker_now = 1;
+}
+
+
+
+/* select which LEDs light up 
+ * The only pleasing combinations are: only menu/power or all LEDs
+ */
+void __buttonlight_select(enum buttonlight_selection selection)
+{
+    switch (selection)
     {
-        backlight_current--;
+        default:
+        case BUTTONLIGHT_LED_MENU:
+        buttonlight_selected = BUTTONLIGHT_MENU;
+        break;
+        
+        case BUTTONLIGHT_LED_ALL:
+        buttonlight_selected = BUTTONLIGHT_ALL;
+        break;
     }
+}
         
-    /* The SC606 LED driver can set the brightness in 64 steps */
-    sc606_write(SC606_REG_A, backlight_current);
 
 
-    /* have we hit the target? */
-    if (backlight_current == backlight_target)
+/* map the mode from the command into the state machine entries */
+void __buttonlight_mode(enum buttonlight_mode mode)
+{
+    switch (mode)
     {
-        backlight_fading = false;
+        case BUTTONLIGHT_OFF:
+        buttonlight_state = BUTTONLIGHT_MODE_OFF_ENTRY;
+        break;
+
+        case BUTTONLIGHT_ON:
+        buttonlight_state = BUTTONLIGHT_MODE_ON_ENTRY;
+        break;
+        
+        case BUTTONLIGHT_FAINT:
+        buttonlight_state = BUTTONLIGHT_MODE_FAINT_ENTRY;
+        break;
+        
+        case BUTTONLIGHT_FLICKER:
+        buttonlight_state = BUTTONLIGHT_MODE_FLICKER_ENTRY;
+        break;
+
+        default:
+        return; /* unknown mode */
     }
-    else
+
+    
+}
+
+
+
+/* 
+ * The button lights have 'modes' of operation. Each mode must setup and
+ * execute its own operation - taking care that this is all done in an ISR.
+ * 
+ */
+
+
+
+/* led_control_service runs in interrupt context - be brief! 
+ * This service is called once per interrupt timer tick - 100 times a second. 
+ *
+ * There should be at most only one i2c operation per call - if more are need
+ *  the calls should be spread across calls.
+ *
+ * Putting all led servicing in one thread means that we wont step on any
+ * i2c operations - they are all serialized here in the ISR tick. It also
+ * insures that we get called at equal timing for good visual effect.
+ *
+ * The buttonlight service runs only after all backlight services have finished.
+ * Fading the buttonlights is possible, but not recommended because of the
+ * additional calls needed during the ISR
+ */
+static void led_control_service(void)
+{
+    switch (backlight_control)
     {
-        time_til_fade = fade_interval;
+        case BACKLIGHT_CONTROL_IDLE: 
+        switch (buttonlight_state)
+        {
+            /* Buttonlight mode: OFF */
+            case BUTTONLIGHT_MODE_OFF_ENTRY: 
+            if (buttonlight_current)
+            {
+                buttonlight_leds = 0x00;
+                sc606_write(SC606_REG_CONF, backlight_leds);
+                buttonlight_current = 0;
+            }                
+            buttonlight_state = BUTTONLIGHT_MODE_OFF;
+            break;
+
+            case BUTTONLIGHT_MODE_OFF: 
+            break;
+
+                
+            /* Buttonlight mode: ON */            
+            case BUTTONLIGHT_MODE_ON_ENTRY:
+            case BUTTONLIGHT_MODE_ON: 
+            if (buttonlight_current != backlight_brightness ||
+                (buttonlight_leds != buttonlight_selected))
+            {
+                buttonlight_leds = buttonlight_selected;
+                sc606_write(SC606_REG_CONF, backlight_leds | buttonlight_leds);
+
+                /* temporary save for the next mode - then to do settings */
+                buttonlight_setting = backlight_brightness;
+                buttonlight_saved_state = BUTTONLIGHT_MODE_ON;
+                buttonlight_state = BUTTONLIGHT_HELPER_SET;
+            }
+            break;            
+
+
+            /* Buttonlight mode: Faint */            
+            case BUTTONLIGHT_MODE_FAINT_ENTRY:
+            if (buttonlight_current != 1)
+            {
+                /* need to turn on the backlight? */
+                if (buttonlight_current == 0)
+                {
+                    buttonlight_leds = buttonlight_selected;
+                    sc606_write(SC606_REG_CONF, backlight_leds | buttonlight_leds);
+                }
+
+                /* temporary save for the next mode - then to do settings */
+                buttonlight_setting = 1;
+                buttonlight_saved_state = BUTTONLIGHT_MODE_FAINT;
+                buttonlight_state = BUTTONLIGHT_HELPER_SET;
+            }
+            break;
+
+            case BUTTONLIGHT_MODE_FAINT: 
+            /* watch for change in led selction */
+            if (buttonlight_leds != buttonlight_selected)
+            {
+                buttonlight_leds = buttonlight_selected;
+                sc606_write(SC606_REG_CONF, backlight_leds | buttonlight_leds);
+            }
+
+            break;
+
+
+            /* Buttonlight mode: FLICKER */            
+            case BUTTONLIGHT_MODE_FLICKER_ENTRY:
+            /* already on? turn it off */
+            if (buttonlight_current) 
+            {
+                buttonlight_leds = 0x00;
+                sc606_write(SC606_REG_CONF, backlight_leds);
+                buttonlight_current = 0;
+            }                
+
+            /* set the brightness if not already set */            
+            if (buttonlight_current != buttonlight_flicker_brightness)
+            {
+                /* temporary save for the next mode - then to do settings */
+                buttonlight_setting = buttonlight_flicker_brightness;
+                buttonlight_saved_state = BUTTONLIGHT_MODE_FLICKER;
+                buttonlight_state = BUTTONLIGHT_HELPER_SET;
+            }
+            break;
+
+
+            case BUTTONLIGHT_MODE_FLICKER:
+            /* wait for the foreground to trigger flickering */
+            if (buttonlight_flicker_now)
+            {
+                /* turn them on */
+                buttonlight_leds = buttonlight_selected;
+                buttonlight_current = buttonlight_setting;
+                sc606_write(SC606_REG_CONF, backlight_leds | buttonlight_leds);
+
+                /* reset the trigger and go flicker the LEDs */
+                buttonlight_flicker_now = 0;
+                buttonlight_flickering = FLICKER_PERIOD;
+                buttonlight_state = BUTTONLIGHT_MODE_FLICKERING;
+            }
+            break;
+            
+
+            case BUTTONLIGHT_MODE_FLICKERING:
+            /* flicker the LEDs for as long as we get triggered */
+            if (buttonlight_flickering)
+            {
+                /* turn the leds off if they are on */
+                if (buttonlight_current)
+                {
+                    buttonlight_leds = 0x00;
+                    sc606_write(SC606_REG_CONF, backlight_leds);
+                    buttonlight_current = 0;
+                }
+
+                buttonlight_flickering--;
+            }
+            else
+            {
+                /* is flickering triggered again? */
+                if (!buttonlight_flicker_now)
+                {
+                    /* completed a cycle - no new triggers - go back and wait */
+                    buttonlight_state = BUTTONLIGHT_MODE_FLICKER;
+                }
+                else
+                {
+                    /* reset flickering */
+                    buttonlight_flicker_now = 0;
+                    buttonlight_flickering = FLICKER_PERIOD;
+                    
+                    /* turn buttonlights on */
+                    buttonlight_leds = buttonlight_selected;
+                    buttonlight_current = buttonlight_setting;
+                    sc606_write(SC606_REG_CONF, backlight_leds | buttonlight_leds);
+                }
+            }
+            break;
+
+            /* set the brightness for the buttonlights - takes 2 passes */
+            case BUTTONLIGHT_HELPER_SET:
+            sc606_write(SC606_REG_B, buttonlight_setting-1);
+            buttonlight_state = BUTTONLIGHT_HELPER_SET_FINAL;
+            break;
+
+            case BUTTONLIGHT_HELPER_SET_FINAL:
+            sc606_write(SC606_REG_C, buttonlight_setting-1);
+            buttonlight_current = buttonlight_setting;
+            buttonlight_state = buttonlight_saved_state;
+            break;
+
+            default:
+            break;
+            
+        }
+        break;
+
+
+        case BACKLIGHT_CONTROL_FADE_ON_FROM_OFF:
+        backlight_leds = 0x03;
+        sc606_write(SC606_REG_CONF, 0x03 | buttonlight_leds);
+        backlight_control = BACKLIGHT_CONTROL_FADE_ON;
+        break;
+
+
+        case BACKLIGHT_CONTROL_OFF:
+        backlight_current = 0;
+        backlight_leds = 0x00;
+        sc606_write(SC606_REG_CONF, buttonlight_leds);
+        backlight_control = BACKLIGHT_CONTROL_IDLE;
+        break;
+
+           
+        case BACKLIGHT_CONTROL_ON:
+        backlight_leds = 0x03;
+        sc606_write(SC606_REG_CONF, 0x03 | buttonlight_leds);
+        backlight_current = backlight_brightness;
+        backlight_control = BACKLIGHT_CONTROL_IDLE;
+        break;
+
+
+        case BACKLIGHT_CONTROL_SET:
+        /* The SC606 LED driver can set the brightness in 64 steps */
+        sc606_write(SC606_REG_A, backlight_brightness-1);
+
+        /* if we were turned off - turn the backlight on */
+        if (backlight_current) 
+        {
+            backlight_current = backlight_brightness;
+            backlight_control = BACKLIGHT_CONTROL_IDLE;
+        }
+        else
+        {
+            backlight_control = BACKLIGHT_CONTROL_ON;
+        }
+        break;
+        
+
+        case BACKLIGHT_CONTROL_FADE_ON:
+        if (--time_til_fade) return;
+            
+        /* The SC606 LED driver can set the brightness in 64 steps */
+        sc606_write(SC606_REG_A, backlight_current++);
+
+        /* have we hit the target? */
+        if (backlight_current == backlight_target)
+        {
+            backlight_control = BACKLIGHT_CONTROL_IDLE;
+        }
+        else
+        {
+            time_til_fade = fade_interval;
+        }
+        break;
+        
+
+        case BACKLIGHT_CONTROL_FADE_OFF:
+        if (--time_til_fade) return;
+            
+        /* The SC606 LED driver can set the brightness in 64 steps */
+        sc606_write(SC606_REG_A, --backlight_current);
+
+        /* have we hit the target? */
+        if (backlight_current == backlight_target)
+        {
+            if (backlight_current)
+            {
+                backlight_control = BACKLIGHT_CONTROL_IDLE;
+            }
+            else
+            {
+                backlight_control = BACKLIGHT_CONTROL_OFF;
+            }
+                
+        }
+        else
+        {
+            time_til_fade = fade_interval;
+        }
+        break;
     }
 
 }
@@ -121,10 +480,10 @@ static void backlight_fade_service(void)
 
 void __backlight_dim(bool dim_now)
 {
-    int target;
+    unsigned short target;
     
     /* dont let the interrupt tick happen */
-    backlight_fading = false;
+    backlight_control = BACKLIGHT_CONTROL_IDLE;
 
     target = (dim_now == true) ? 0 : backlight_brightness;
     
@@ -135,17 +494,52 @@ void __backlight_dim(bool dim_now)
         
         if (backlight_current > backlight_target) 
         {
-            fade_interval = 4;
+            time_til_fade = fade_interval = 4;
+            backlight_control = BACKLIGHT_CONTROL_FADE_OFF;
         }
         else
         {
-            fade_interval = 1;
+            time_til_fade = fade_interval = 1;
+            if (backlight_current)
+            {
+                backlight_control = BACKLIGHT_CONTROL_FADE_ON;
+            }
+            else
+            {
+                backlight_control = BACKLIGHT_CONTROL_FADE_ON_FROM_OFF;
+            }
         }
-
-        /* let the tick work */
-        time_til_fade = fade_interval;
-        backlight_fading = true;
     }
             
 }
 
+
+
+
+#define BACKLIGHT_BUTTONS_OFF       0
+#define BACKLIGHT_BUTTONS_ON        1
+#define BACKLIGHT_BUTTONS_FAINT     2
+#define BACKLIGHT_BUTTONS_FOLLOW    3
+
+
+void __backlight_buttons(int value)
+{
+    switch (value)
+    {
+        default:
+        case BACKLIGHT_BUTTONS_OFF:
+        break;
+
+        case BACKLIGHT_BUTTONS_ON:
+        break;
+        
+        case BACKLIGHT_BUTTONS_FAINT:
+        break;
+        
+        case BACKLIGHT_BUTTONS_FOLLOW:
+        break;
+        
+    }
+
+}
+
diff --git a/firmware/target/arm/gigabeat/meg-fx/backlight-target.h b/firmware/target/arm/gigabeat/meg-fx/backlight-target.h
index c53d00d8de..67a055449c 100644
--- a/firmware/target/arm/gigabeat/meg-fx/backlight-target.h
+++ b/firmware/target/arm/gigabeat/meg-fx/backlight-target.h
@@ -19,11 +19,55 @@
 #ifndef BACKLIGHT_TARGET_H
 #define BACKLIGHT_TARGET_H
 
+
+/* select the led */
+enum buttonlight_selection
+{
+    /* all leds */
+    BUTTONLIGHT_LED_ALL,
+
+    /* only the menu/power led (two buttons for one LED) */
+    BUTTONLIGHT_LED_MENU
+};
+
+
+/* Use these to set the buttonlight mode */
+enum buttonlight_mode
+{
+    /* ON follows the setting of the backlight - same brightness */
+    BUTTONLIGHT_ON,
+
+    /* buttonlights always off */
+    BUTTONLIGHT_OFF,
+
+    /* buttonlights always on but set at lowest brightness */
+    BUTTONLIGHT_FAINT,
+
+    /* buttonlights flicker when triggered */
+    BUTTONLIGHT_FLICKER,
+};
+
+
+/* call to flicker the button lights */
+void __buttonlight_flicker(unsigned short brightness);
+
+
+/* only use the XX__ENTRY when setting the mode */
+void __buttonlight_mode(enum buttonlight_mode mode);
+
+
+/* select which led to use on the button lights. Other combinations are
+ * possible, but don't look very good.
+ */
+void __buttonlight_select(enum buttonlight_selection selection);
+
+
 void __backlight_init(void);
 void __backlight_on(void);
 void __backlight_off(void);
 void __backlight_set_brightness(int val);
 
+/* true: backlight fades off - false: backlight fades on */
 void __backlight_dim(bool dim);
 
 #endif
diff --git a/firmware/target/arm/gigabeat/meg-fx/button-meg-fx.c b/firmware/target/arm/gigabeat/meg-fx/button-meg-fx.c
index b5f18a264b..4ba6ce23f7 100644
--- a/firmware/target/arm/gigabeat/meg-fx/button-meg-fx.c
+++ b/firmware/target/arm/gigabeat/meg-fx/button-meg-fx.c
@@ -122,6 +122,8 @@ int button_read_device(void)
 
         if (touchpad & (1 << 3))
             btn |= BUTTON_SELECT;
+
+        __buttonlight_flicker(DEFAULT_BRIGHTNESS_SETTING/2);
     }
     
     return btn;
diff --git a/firmware/target/arm/gigabeat/meg-fx/sc606-meg-fx.c b/firmware/target/arm/gigabeat/meg-fx/sc606-meg-fx.c
index 654095c2f5..6ca64a0437 100644
--- a/firmware/target/arm/gigabeat/meg-fx/sc606-meg-fx.c
+++ b/firmware/target/arm/gigabeat/meg-fx/sc606-meg-fx.c
@@ -151,7 +151,7 @@ static unsigned char sc606_i2c_inb(void)
 /* returns number of acks that were bad */
 int sc606_write(unsigned char reg, unsigned char data)
 {
-    int x = 0;
+    int x;
 
     sc606_i2c_start();
     
@@ -178,7 +178,7 @@ int sc606_write(unsigned char reg, unsigned char data)
 
 int sc606_read(unsigned char reg, unsigned char* data)
 {
-    int x = 0;
+    int x;
 
     sc606_i2c_start();
     sc606_i2c_outb(SLAVE_ADDRESS);
diff --git a/firmware/target/arm/gigabeat/meg-fx/sc606-meg-fx.h b/firmware/target/arm/gigabeat/meg-fx/sc606-meg-fx.h
index 2637c067e0..df7a654b36 100644
--- a/firmware/target/arm/gigabeat/meg-fx/sc606-meg-fx.h
+++ b/firmware/target/arm/gigabeat/meg-fx/sc606-meg-fx.h
@@ -13,14 +13,14 @@
 #define SC606_REG_C      2
 #define SC606_REG_CONF   3
 
-#define SC606_LED_A1     1 << 0
-#define SC606_LED_A2     1 << 1
-#define SC606_LED_B1     1 << 2
-#define SC606_LED_B2     1 << 3
-#define SC606_LED_C1     1 << 4
-#define SC606_LED_C2     1 << 5
+#define SC606_LED_A1     (1 << 0)
+#define SC606_LED_A2     (1 << 1)
+#define SC606_LED_B1     (1 << 2)
+#define SC606_LED_B2     (1 << 3)
+#define SC606_LED_C1     (1 << 4)
+#define SC606_LED_C2     (1 << 5)
 
-#define SC606_LOW_FREQ   1 << 6
+#define SC606_LOW_FREQ   (1 << 6)
 
 int sc606_write(unsigned char reg, unsigned char data);