#include "edac_core.h"
 #include "edac_module.h"
 
-/* lock to memory controller's control array 'edac_device_list' */
+/* lock for the list: 'edac_device_list', manipulation of this list
+ * is protected by the 'device_ctls_mutex' lock
+ */
 static DEFINE_MUTEX(device_ctls_mutex);
 static struct list_head edac_device_list = LIST_HEAD_INIT(edac_device_list);
 
 /*
  * edac_device_workq_function
  *     performs the operation scheduled by a workq request
+ *
+ *     this workq is embedded within an edac_device_ctl_info
+ *     structure, that needs to be polled for possible error events.
+ *
+ *     This operation is to acquire the list mutex lock
+ *     (thus preventing insertation or deletion)
+ *     and then call the device's poll function IFF this device is
+ *     running polled and there is a poll function defined.
  */
 static void edac_device_workq_function(struct work_struct *work_req)
 {
 
        mutex_unlock(&device_ctls_mutex);
 
-       /* Reschedule */
-       queue_delayed_work(edac_workqueue, &edac_dev->work, edac_dev->delay);
+       /* Reschedule the workq for the next time period to start again
+        * if the number of msec is for 1 sec, then adjust to the next
+        * whole one second to save timers fireing all over the period
+        * between integral seconds
+        */
+       if (edac_dev->poll_msec == 1000)
+               queue_delayed_work(edac_workqueue, &edac_dev->work,
+                               round_jiffies(edac_dev->delay));
+       else
+               queue_delayed_work(edac_workqueue, &edac_dev->work,
+                               edac_dev->delay);
 }
 
 /*
 {
        debugf0("%s()\n", __func__);
 
+       /* take the arg 'msec' and set it into the control structure
+        * to used in the time period calculation
+        * then calc the number of jiffies that represents
+        */
        edac_dev->poll_msec = msec;
-       edac_dev->delay = msecs_to_jiffies(msec);       /* Calc delay jiffies */
+       edac_dev->delay = msecs_to_jiffies(msec);
 
        INIT_DELAYED_WORK(&edac_dev->work, edac_device_workq_function);
-       queue_delayed_work(edac_workqueue, &edac_dev->work, edac_dev->delay);
+
+       /* optimize here for the 1 second case, which will be normal value, to
+        * fire ON the 1 second time event. This helps reduce all sorts of
+        * timers firing on sub-second basis, while they are happy
+        * to fire together on the 1 second exactly
+        */
+       if (edac_dev->poll_msec == 1000)
+               queue_delayed_work(edac_workqueue, &edac_dev->work,
+                               round_jiffies(edac_dev->delay));
+       else
+               queue_delayed_work(edac_workqueue, &edac_dev->work,
+                               edac_dev->delay);
 }
 
 /*
 
 /*
  * edac_device_reset_delay_period
+ *
+ *     need to stop any outstanding workq queued up at this time
+ *     because we will be resetting the sleep time.
+ *     Then restart the workq on the new delay
  */
-
 void edac_device_reset_delay_period(struct edac_device_ctl_info *edac_dev,
                                        unsigned long value)
 {
-       mutex_lock(&device_ctls_mutex);
-
-       /* cancel the current workq request */
+       /* cancel the current workq request, without the mutex lock */
        edac_device_workq_teardown(edac_dev);
 
+       /* acquire the mutex before doing the workq setup */
+       mutex_lock(&device_ctls_mutex);
+
        /* restart the workq request, with new delay value */
        edac_device_workq_setup(edac_dev, value);
 
 
 
        mutex_lock(&mem_ctls_mutex);
 
+       /* if this control struct has movd to offline state, we are done */
+       if (mci->op_state == OP_OFFLINE) {
+               mutex_unlock(&mem_ctls_mutex);
+               return;
+       }
+
        /* Only poll controllers that are running polled and have a check */
        if (edac_mc_assert_error_check_and_clear() && (mci->edac_check != NULL))
                mci->edac_check(mci);
  * edac_mc_workq_setup
  *     initialize a workq item for this mci
  *     passing in the new delay period in msec
+ *
+ *     locking model:
+ *
+ *             called with the mem_ctls_mutex held
  */
-void edac_mc_workq_setup(struct mem_ctl_info *mci, unsigned msec)
+static void edac_mc_workq_setup(struct mem_ctl_info *mci, unsigned msec)
 {
        debugf0("%s()\n", __func__);
 
+       /* if this instance is not in the POLL state, then simply return */
+       if (mci->op_state != OP_RUNNING_POLL)
+               return;
+
        INIT_DELAYED_WORK(&mci->work, edac_mc_workq_function);
        queue_delayed_work(edac_workqueue, &mci->work, msecs_to_jiffies(msec));
 }
 /*
  * edac_mc_workq_teardown
  *     stop the workq processing on this mci
+ *
+ *     locking model:
+ *
+ *             called WITHOUT lock held
  */
-void edac_mc_workq_teardown(struct mem_ctl_info *mci)
+static void edac_mc_workq_teardown(struct mem_ctl_info *mci)
 {
        int status;
 
-       status = cancel_delayed_work(&mci->work);
-       if (status == 0) {
-               /* workq instance might be running, wait for it */
-               flush_workqueue(edac_workqueue);
+       /* if not running POLL, leave now */
+       if (mci->op_state == OP_RUNNING_POLL) {
+               status = cancel_delayed_work(&mci->work);
+               if (status == 0) {
+                       debugf0("%s() not canceled, flush the queue\n",
+                               __func__);
+
+                       /* workq instance might be running, wait for it */
+                       flush_workqueue(edac_workqueue);
+               }
        }
 }
 
 /*
  * edac_reset_delay_period
  */
-
-void edac_reset_delay_period(struct mem_ctl_info *mci, unsigned long value)
+static void edac_reset_delay_period(struct mem_ctl_info *mci, unsigned long value)
 {
-       mutex_lock(&mem_ctls_mutex);
-
        /* cancel the current workq request */
        edac_mc_workq_teardown(mci);
 
+       /* lock the list of devices for the new setup */
+       mutex_lock(&mem_ctls_mutex);
+
        /* restart the workq request, with new delay value */
        edac_mc_workq_setup(mci, value);
 
 /* Return 0 on success, 1 on failure.
  * Before calling this function, caller must
  * assign a unique value to mci->mc_idx.
+ *
+ *     locking model:
+ *
+ *             called with the mem_ctls_mutex lock held
  */
 static int add_mc_to_global_list(struct mem_ctl_info *mci)
 {
 
        insert_before = &mc_devices;
 
-       if (unlikely((p = find_mci_by_dev(mci->dev)) != NULL))
+       p = find_mci_by_dev(mci->dev);
+       if (unlikely(p != NULL))
                goto fail0;
 
        list_for_each(item, &mc_devices) {
        }
 
        /* Report action taken */
-       edac_mc_printk(mci, KERN_INFO, "Giving out device to %s %s: DEV %s\n",
-               mci->mod_name, mci->ctl_name, dev_name(mci));
+       edac_mc_printk(mci, KERN_INFO, "Giving out device to '%s' '%s':"
+               " DEV %s\n", mci->mod_name, mci->ctl_name, dev_name(mci));
 
        mutex_unlock(&mem_ctls_mutex);
        return 0;
 {
        struct mem_ctl_info *mci;
 
-       debugf0("MC: %s()\n", __func__);
+       debugf0("%s()\n", __func__);
+
        mutex_lock(&mem_ctls_mutex);
 
-       if ((mci = find_mci_by_dev(dev)) == NULL) {
+       /* find the requested mci struct in the global list */
+       mci = find_mci_by_dev(dev);
+       if (mci == NULL) {
                mutex_unlock(&mem_ctls_mutex);
                return NULL;
        }
        /* marking MCI offline */
        mci->op_state = OP_OFFLINE;
 
-       /* flush workq processes */
-       edac_mc_workq_teardown(mci);
-
-       edac_remove_sysfs_mci_device(mci);
        del_mc_from_global_list(mci);
        mutex_unlock(&mem_ctls_mutex);
+
+       /* flush workq processes and remove sysfs */
+       edac_mc_workq_teardown(mci);
+       edac_remove_sysfs_mci_device(mci);
+
        edac_printk(KERN_INFO, EDAC_MC,
                "Removed device %d for %s %s: DEV %s\n", mci->mc_idx,
                mci->mod_name, mci->ctl_name, dev_name(mci));
+
        return mci;
 }
 EXPORT_SYMBOL_GPL(edac_mc_del_mc);