Remove card_enable_monitoring() and use a mutex instead. The card_enable_monitoring() method actually didn't eliminate the possible race conditions it was meant to fix.

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

apps/main.c

@@ -342,6 +342,7 @@ static void init(void)
     sb_skin_init();
     viewportmanager_init();
 
+    disk_init_subsystem();
     storage_init();
     settings_reset();
     settings_load(SETTINGS_ALL);
@@ -506,6 +507,8 @@ static void init(void)
     }
 #endif
 
+
+    disk_init_subsystem();
     CHART(">storage_init");
     rc = storage_init();
     CHART("<storage_init");

firmware/common/disk.c

@@ -19,11 +19,11 @@
  *
  ****************************************************************************/
 #include <stdio.h>
+#include "kernel.h"
 #include "storage.h"
 #include "debug.h"
 #include "fat.h"
 #ifdef HAVE_HOTSWAP
-#include "sdmmc.h" /* for card_enable_monitoring() */
 #include "dir.h" /* for release_dirs() */
 #include "file.h" /* for release_files() */
 #endif
@@ -60,12 +60,13 @@ static const unsigned char fat_partition_types[] = {
 
 static struct partinfo part[NUM_DRIVES*4]; /* space for 4 partitions on 2 drives */
 static int vol_drive[NUM_VOLUMES]; /* mounted to which drive (-1 if none) */
+static struct mutex disk_mutex;
 
 #ifdef MAX_LOG_SECTOR_SIZE
 int disk_sector_multiplier = 1;
 #endif
 
-struct partinfo* disk_init(IF_MD_NONVOID(int drive))
+static struct partinfo* disk_init(IF_MD_NONVOID(int drive))
 {
     int i;
     unsigned char sector[SECTOR_SIZE];
@@ -113,13 +114,18 @@ struct partinfo* disk_partinfo(int partition)
     return &part[partition];
 }
 
+void disk_init_subsystem(void)
+{
+   mutex_init(&disk_mutex);
+}
+
 int disk_mount_all(void)
 {
     int mounted=0;
     int i;
     
 #ifdef HAVE_HOTSWAP
-    card_enable_monitoring(false);
+    mutex_lock(&disk_mutex);
 #endif
 
     fat_init(); /* reset all mounted partitions */
@@ -139,9 +145,8 @@ int disk_mount_all(void)
 #endif
 
 #ifdef HAVE_HOTSWAP
-    card_enable_monitoring(true);
+    mutex_unlock(&disk_mutex);
 #endif
-
     return mounted;
 }
 
@@ -160,11 +165,21 @@ static int get_free_volume(void)
 int disk_mount(int drive)
 {
     int mounted = 0; /* reset partition-on-drive flag */
-    int volume = get_free_volume();
+    int volume;
-    struct partinfo* pinfo = disk_init(IF_MD(drive));
+    struct partinfo* pinfo;
+
+#ifdef HAVE_HOTSWAP
+    mutex_lock(&disk_mutex);
+#endif
+
+    volume = get_free_volume();
+    pinfo = disk_init(IF_MD(drive));
 
     if (pinfo == NULL)
     {
+#ifdef HAVE_HOTSWAP
+        mutex_unlock(&disk_mutex);
+#endif
         return 0;
     }
 #if defined(TOSHIBA_GIGABEAT_S)
@@ -214,6 +229,9 @@ int disk_mount(int drive)
             vol_drive[volume] = drive; /* remember the drive for this volume */
         }
     }
+#ifdef HAVE_HOTSWAP
+    mutex_unlock(&disk_mutex);
+#endif
     return mounted;
 }
 
@@ -222,6 +240,7 @@ int disk_unmount(int drive)
 {
     int unmounted = 0;
     int i;
+    mutex_lock(&disk_mutex);
     for (i=0; i<NUM_VOLUMES; i++)
     {
         if (vol_drive[i] == drive)
@@ -233,6 +252,7 @@ int disk_unmount(int drive)
             fat_unmount(i, false);
         }
     }
+    mutex_unlock(&disk_mutex);
 
     return unmounted;
 }

firmware/drivers/ata_mmc.c

@@ -93,7 +93,6 @@ static long last_disk_activity = -1;
 static struct mutex mmc_mutex;
 
 #ifdef HAVE_HOTSWAP
-static bool mmc_monitor_enabled = true;
 static long mmc_stack[((DEFAULT_STACK_SIZE*2) + 0x800)/sizeof(long)];
 #else
 static long mmc_stack[(DEFAULT_STACK_SIZE*2)/sizeof(long)];
@@ -130,7 +129,9 @@ static tCardInfo card_info[2];
 static int current_card = 0;
 #endif
 static bool last_mmc_status = false;
-static int countdown = HZ/3; /* for mmc switch debouncing */
+static int countdown = -1; /* for mmc switch debouncing. -1 because the 
+                              countdown should not happen if the card
+                              is inserted at boot */
 static bool usb_activity;    /* monitoring the USB bridge */
 static long last_usb_activity;
 
@@ -807,13 +808,6 @@ static void mmc_thread(void)
     }
 }
 
-#ifdef HAVE_HOTSWAP
-void mmc_enable_monitoring(bool on)
-{
-    mmc_monitor_enabled = on;
-}
-#endif
-
 bool mmc_detect(void)
 {
     return (adc_read(ADC_MMC_SWITCH) < 0x200);
@@ -846,9 +840,6 @@ bool mmc_usb_active(int delayticks)
 static void mmc_tick(void)
 {
     bool current_status;
-#ifndef HAVE_HOTSWAP
-    const bool mmc_monitor_enabled = true;
-#endif
 
     if (new_mmc_circuit)
         /* USB bridge activity is 0 on idle, ~527 on active */
@@ -860,33 +851,30 @@ static void mmc_tick(void)
         last_usb_activity = current_tick;
     usb_activity = current_status;
 
-    if (mmc_monitor_enabled)
+    current_status = mmc_detect();
+    /* Only report when the status has changed */
+    if (current_status != last_mmc_status)
     {
-        current_status = mmc_detect();
+        last_mmc_status = current_status;
-        /* Only report when the status has changed */
+        countdown = HZ/3;
-        if (current_status != last_mmc_status)
+    }
-        {
+    else
-            last_mmc_status = current_status;
+    {
-            countdown = HZ/3;
+        /* Count down until it gets negative */
-        }
+        if (countdown >= 0)
-        else
+            countdown--;
-        {
-            /* Count down until it gets negative */
-            if (countdown >= 0)
-                countdown--;
 
-            if (countdown == 0)
+        if (countdown == 0)
+        {
+            if (current_status)
             {
-                if (current_status)
+                queue_broadcast(SYS_HOTSWAP_INSERTED, 0);
-                {
+            }
-                    queue_broadcast(SYS_HOTSWAP_INSERTED, 0);
+            else
-                }
+            {
-                else
+                queue_broadcast(SYS_HOTSWAP_EXTRACTED, 0);
-                {
+                mmc_status = MMC_UNTOUCHED;
-                    queue_broadcast(SYS_HOTSWAP_EXTRACTED, 0);
+                card_info[1].initialized = false;
-                    mmc_status = MMC_UNTOUCHED;
-                    card_info[1].initialized = false;
-                }
             }
         }
     }

firmware/export/disk.h

@@ -35,8 +35,9 @@ struct partinfo {
 #define PARTITION_TYPE_OS2_HIDDEN_C_DRIVE   0x84
 
 /* returns a pointer to an array of 8 partinfo structs */
-struct partinfo* disk_init(IF_MD_NONVOID(int drive));
 struct partinfo* disk_partinfo(int partition);
+
+void disk_init_subsystem(void); /* Initialises mutexes */
 int disk_mount_all(void); /* returns the # of successful mounts */
 int disk_mount(int drive);
 int disk_unmount(int drive);

firmware/export/sd.h

@@ -48,7 +48,6 @@ void sd_get_info(IF_MD2(int drive,) struct storage_info *info);
 #ifdef HAVE_HOTSWAP
 bool sd_removable(IF_MV_NONVOID(int drive));
 bool sd_present(IF_MV_NONVOID(int drive));
-void card_enable_monitoring_target(bool on);
 #endif
 
 long sd_last_disk_activity(void);

firmware/export/sdmmc.h

@@ -57,17 +57,12 @@ typedef struct
 tCardInfo *card_get_info_target(int card_no);
 void sd_parse_csd(tCardInfo *card);
 
-#ifdef HAVE_HOTSWAP
-#define card_enable_monitoring card_enable_monitoring_target
-#endif
-
 #else /* STORAGE_MMC */
 
 #include "ata_mmc.h"
 #define card_get_info          mmc_card_info
 tCardInfo *mmc_card_info(int card_no);
 #define card_touched           mmc_touched
-#define card_enable_monitoring mmc_enable_monitoring
 
 #endif
 

firmware/target/arm/as3525/sd-as3525.c

@@ -508,6 +508,8 @@ static void init_pl180_controller(const int drive)
     GPIOA_IS &= ~EXT_SD_BITS;
     /* detect both raising and falling edges */
     GPIOA_IBE |= EXT_SD_BITS;
+    /* enable the card detect interrupt */
+    GPIOA_IE |= EXT_SD_BITS;
 
 #else
     VIC_INT_ENABLE = INTERRUPT_NAND;
@@ -922,16 +924,6 @@ tCardInfo *card_get_info_target(int card_no)
     return &card_info[card_no];
 }
 
-#ifdef HAVE_HOTSWAP
-void card_enable_monitoring_target(bool on)
-{
-    if (on) /* enable interrupt */
-        GPIOA_IE |= EXT_SD_BITS;
-    else    /* disable interrupt */
-        GPIOA_IE &= ~EXT_SD_BITS;
-}
-#endif /* HAVE_HOTSWAP */
-
 #endif /* !BOOTLOADER */
 
 #ifdef CONFIG_STORAGE_MULTI

firmware/target/arm/as3525/sd-as3525v2.c

@@ -731,6 +731,9 @@ int sd_init(void)
     GPIOA_IS &= ~EXT_SD_BITS;
     /* detect both raising and falling edges */
     GPIOA_IBE |= EXT_SD_BITS;
+    /* enable the card detect interrupt */
+    GPIOA_IE |= EXT_SD_BITS;
+
     /* Configure XPD for SD-MCI interface */
     CCU_IO |= (1<<2);
 #endif
@@ -989,14 +992,6 @@ void sd_gpioa_isr(void)
     /* acknowledge interrupt */
     GPIOA_IC = EXT_SD_BITS;
 }
-
-void card_enable_monitoring_target(bool on)
-{
-    if (on) /* enable interrupt */
-        GPIOA_IE |= EXT_SD_BITS;
-    else    /* disable interrupt */
-        GPIOA_IE &= ~EXT_SD_BITS;
-}
 #endif /* HAVE_HOTSWAP */
 
 #ifdef CONFIG_STORAGE_MULTI

firmware/target/arm/ata-sd-pp.c

@@ -1190,27 +1190,6 @@ void sd_enable(bool on)
     }
 }
 
-#ifdef HAVE_HOTSWAP
-void card_enable_monitoring_target(bool on)
-{
-    if (on)
-    {
-#ifdef SANSA_E200
-        GPIO_SET_BITWISE(GPIOA_INT_EN, 0x80);
-#elif defined(SANSA_C200)
-        GPIO_SET_BITWISE(GPIOL_INT_EN, 0x08);
-#endif
-    }
-    else
-    {
-#ifdef SANSA_E200
-        GPIO_CLEAR_BITWISE(GPIOA_INT_EN, 0x80);
-#elif defined(SANSA_C200)
-        GPIO_CLEAR_BITWISE(GPIOL_INT_EN, 0x08);
-#endif
-    }
-}
-#endif
 
 int sd_init(void)
 {
@@ -1275,6 +1254,9 @@ int sd_init(void)
         GPIOA_INT_LEV = (0x80 << 8) | (~GPIOA_INPUT_VAL & 0x80);
 
         GPIOA_INT_CLR = 0x80;
+
+        /* enable the card detect interrupt */
+        GPIO_SET_BITWISE(GPIOA_INT_EN, 0x80);
 #elif defined SANSA_C200
         CPU_INT_EN = HI_MASK;
         CPU_HI_INT_EN = GPIO2_MASK;
@@ -1282,6 +1264,9 @@ int sd_init(void)
         GPIOL_INT_LEV = (0x08 << 8) | (~GPIOL_INPUT_VAL & 0x08);
 
         GPIOL_INT_CLR = 0x08;
+    
+        /* enable the card detect interrupt */
+        GPIO_SET_BITWISE(GPIOL_INT_EN, 0x08);
 #endif
 #endif
     }

firmware/target/arm/s3c2440/sd-s3c2440.c

@@ -270,6 +270,11 @@ static void init_sdi_controller(const int card_no)
     
     /* Card Detect input */
     S3C2440_GPIO_CONFIG (GPGCON, 8, GPIO_INPUT);
+    /* enable external irq 8-23 on the internal interrupt controller */
+    INTMSK &= ~1<<5;
+    /* enable GPG8 IRQ on the external interrupt controller */
+    EINTMASK &= ~(1<<16);
+    
     
     /* Write Protect input */
     S3C2440_GPIO_CONFIG (GPHCON, 8, GPIO_INPUT);
@@ -523,22 +528,6 @@ static int sd1_oneshot_callback(struct timeout *tmo)
     return 0;
 }
 
-void card_enable_monitoring_target(bool on)
-{
-    if (on)
-    {   /* enable external irq 8-23 on the internal interrupt controller */
-        INTMSK &= ~1<<5;
-        /* enable GPG8 IRQ on the external interrupt controller */
-        EINTMASK &= ~(1<<16);
-    }
-    else
-    {
-        /* mask internal and external IRQs */
-        INTMSK |=  1<<5;
-        EINTMASK |= (1<<16);
-    }
-}
-
 void EINT8_23(void)
 {
     static struct timeout sd1_oneshot;

firmware/target/arm/tcc780x/sd-tcc780x.c

@@ -214,18 +214,6 @@ static inline bool card_detect_target(void)
 #endif
 }
 
-void card_enable_monitoring_target(bool on)
-{
-    if (on)
-    {
-        IEN |= EXT0_IRQ_MASK;
-    }
-    else
-    {
-        IEN &= ~EXT0_IRQ_MASK;
-    }
-}
-
 static int sd1_oneshot_callback(struct timeout *tmo)
 {
     (void)tmo;
@@ -786,6 +774,7 @@ int sd_init(void)
         /* Configure interrupts for the card slot */
         TMODE &= ~EXT0_IRQ_MASK; /* edge-triggered */
         TMODEA |= EXT0_IRQ_MASK; /* trigger on both edges */
+        IEN |= EXT0_IRQ_MASK; /* enable the interrupt */
 #endif
     }
 

firmware/target/mips/ingenic_jz47xx/ata-sd-jz4740.c

@@ -1412,14 +1412,6 @@ bool sd_removable(IF_MV_NONVOID(int drive))
     return true;
 }
 
-void card_enable_monitoring_target(bool on)
-{
-    if(on)
-        sd_gpio_setup_irq(card_detect_target());
-    else
-        __gpio_mask_irq(MMC_CD_PIN);
-}
-
 static int sd_oneshot_callback(struct timeout *tmo)
 {
     (void)tmo;