block: IBM RamSan 70/80 driver fixes

This patch includes the following driver fixes for the
IBM RamSan 70/80 driver:

o Changed the creg_ctrl lock from a mutex to a spinlock.
o Added a count check for ioctl calls.
o Removed unnecessary casting of void pointers.
o Made every function static that needed to be.
o Added comments to explain things more thoroughly.

Signed-off-by: Philip J Kelleher <pjk1939@linux.vnet.ibm.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

7 files changed, 69 insertions, 63 deletions

diff --git a/drivers/block/rsxx/config.c b/drivers/block/rsxx/config.c
index c8829cd4db11..a295e7e9ee41 100644
--- a/drivers/block/rsxx/config.c
+++ b/drivers/block/rsxx/config.c
@@ -31,7 +31,7 @@
 
 static void initialize_config(void *config)
 {
-        struct rsxx_card_cfg *cfg = (struct rsxx_card_cfg *) config;
+        struct rsxx_card_cfg *cfg = config;
 
         cfg->hdr.version = RSXX_CFG_VERSION;
 
@@ -97,7 +97,7 @@ static void config_data_cpu_to_le(struct rsxx_card_cfg *cfg)
 
 
 /*----------------- Config Operations ------------------*/
-int rsxx_save_config(struct rsxx_cardinfo *card)
+static int rsxx_save_config(struct rsxx_cardinfo *card)
 {
         struct rsxx_card_cfg cfg;
         int st;
diff --git a/drivers/block/rsxx/core.c b/drivers/block/rsxx/core.c
index 83dadbee0375..e5162487686a 100644
--- a/drivers/block/rsxx/core.c
+++ b/drivers/block/rsxx/core.c
@@ -102,9 +102,9 @@ void rsxx_disable_ier_and_isr(struct rsxx_cardinfo *card,
         iowrite32(card->ier_mask, card->regmap + IER);
 }
 
-irqreturn_t rsxx_isr(int irq, void *pdata)
+static irqreturn_t rsxx_isr(int irq, void *pdata)
 {
-        struct rsxx_cardinfo *card = (struct rsxx_cardinfo *) pdata;
+        struct rsxx_cardinfo *card = pdata;
         unsigned int isr;
         int handled = 0;
         int reread_isr;
@@ -161,6 +161,17 @@ irqreturn_t rsxx_isr(int irq, void *pdata)
 }
 
 /*----------------- Card Event Handler -------------------*/
+static char *rsxx_card_state_to_str(unsigned int state)
+{
+        static char *state_strings[] = {
+                "Unknown", "Shutdown", "Starting", "Formatting",
+                "Uninitialized", "Good", "Shutting Down",
+                "Fault", "Read Only Fault", "dStroying"
+        };
+
+        return state_strings[ffs(state)];
+}
+
 static void card_state_change(struct rsxx_cardinfo *card,
                               unsigned int new_state)
 {
@@ -251,18 +262,6 @@ static void card_event_handler(struct work_struct *work)
                 rsxx_read_hw_log(card);
 }
 
-
-char *rsxx_card_state_to_str(unsigned int state)
-{
-        static char *state_strings[] = {
-                "Unknown", "Shutdown", "Starting", "Formatting",
-                "Uninitialized", "Good", "Shutting Down",
-                "Fault", "Read Only Fault", "dStroying"
-        };
-
-        return state_strings[ffs(state)];
-}
-
 /*----------------- Card Operations -------------------*/
 static int card_shutdown(struct rsxx_cardinfo *card)
 {
@@ -323,7 +322,6 @@ static int rsxx_pci_probe(struct pci_dev *dev,
                                         const struct pci_device_id *id)
 {
         struct rsxx_cardinfo *card;
-        unsigned long flags;
         int st;
 
         dev_info(&dev->dev, "PCI-Flash SSD discovered\n");
@@ -386,9 +384,9 @@ static int rsxx_pci_probe(struct pci_dev *dev,
         spin_lock_init(&card->irq_lock);
         card->halt = 0;
 
-        spin_lock_irqsave(&card->irq_lock, flags);
+        spin_lock_irq(&card->irq_lock);
         rsxx_disable_ier_and_isr(card, CR_INTR_ALL);
-        spin_unlock_irqrestore(&card->irq_lock, flags);
+        spin_unlock_irq(&card->irq_lock);
 
         if (!force_legacy) {
                 st = pci_enable_msi(dev);
@@ -408,9 +406,9 @@ static int rsxx_pci_probe(struct pci_dev *dev,
         /************* Setup Processor Command Interface *************/
         rsxx_creg_setup(card);
 
-        spin_lock_irqsave(&card->irq_lock, flags);
+        spin_lock_irq(&card->irq_lock);
         rsxx_enable_ier_and_isr(card, CR_INTR_CREG);
-        spin_unlock_irqrestore(&card->irq_lock, flags);
+        spin_unlock_irq(&card->irq_lock);
 
         st = rsxx_compatibility_check(card);
         if (st) {
@@ -463,9 +461,9 @@ static int rsxx_pci_probe(struct pci_dev *dev,
          * we can enable the event interrupt(it kicks off actions in
          * those layers so we couldn't enable it right away.)
          */
-        spin_lock_irqsave(&card->irq_lock, flags);
+        spin_lock_irq(&card->irq_lock);
         rsxx_enable_ier_and_isr(card, CR_INTR_EVENT);
-        spin_unlock_irqrestore(&card->irq_lock, flags);
+        spin_unlock_irq(&card->irq_lock);
 
         if (card->state == CARD_STATE_SHUTDOWN) {
                 st = rsxx_issue_card_cmd(card, CARD_CMD_STARTUP);
@@ -487,9 +485,9 @@ failed_create_dev:
         rsxx_dma_destroy(card);
 failed_dma_setup:
 failed_compatiblity_check:
-        spin_lock_irqsave(&card->irq_lock, flags);
+        spin_lock_irq(&card->irq_lock);
         rsxx_disable_ier_and_isr(card, CR_INTR_ALL);
-        spin_unlock_irqrestore(&card->irq_lock, flags);
+        spin_unlock_irq(&card->irq_lock);
         free_irq(dev->irq, card);
         if (!force_legacy)
                 pci_disable_msi(dev);
diff --git a/drivers/block/rsxx/cregs.c b/drivers/block/rsxx/cregs.c
index a31fd727e804..80bbe639fccd 100644
--- a/drivers/block/rsxx/cregs.c
+++ b/drivers/block/rsxx/cregs.c
@@ -107,10 +107,10 @@ static struct creg_cmd *pop_active_cmd(struct rsxx_cardinfo *card)
          * Spin lock is needed because this can be called in atomic/interrupt
          * context.
          */
-        spin_lock_bh(&card->creg_ctrl.pop_lock);
+        spin_lock_bh(&card->creg_ctrl.lock);
         cmd = card->creg_ctrl.active_cmd;
         card->creg_ctrl.active_cmd = NULL;
-        spin_unlock_bh(&card->creg_ctrl.pop_lock);
+        spin_unlock_bh(&card->creg_ctrl.lock);
 
         return cmd;
 }
@@ -126,7 +126,11 @@ static void creg_issue_cmd(struct rsxx_cardinfo *card, struct creg_cmd *cmd)
                                           cmd->buf, cmd->stream);
         }
 
-        /* Data copy must complete before initiating the command. */
+        /*
+         * Data copy must complete before initiating the command. This is
+         * needed for weakly ordered processors (i.e. PowerPC), so that all
+         * neccessary registers are written before we kick the hardware.
+         */
         wmb();
 
         /* Setting the valid bit will kick off the command. */
@@ -192,11 +196,11 @@ static int creg_queue_cmd(struct rsxx_cardinfo *card,
         cmd->cb_private = cb_private;
         cmd->status     = 0;
 
-        mutex_lock(&card->creg_ctrl.lock);
+        spin_lock(&card->creg_ctrl.lock);
         list_add_tail(&cmd->list, &card->creg_ctrl.queue);
         card->creg_ctrl.q_depth++;
         creg_kick_queue(card);
-        mutex_unlock(&card->creg_ctrl.lock);
+        spin_unlock(&card->creg_ctrl.lock);
 
         return 0;
 }
@@ -219,10 +223,11 @@ static void creg_cmd_timed_out(unsigned long data)
 
         kmem_cache_free(creg_cmd_pool, cmd);
 
-        spin_lock(&card->creg_ctrl.pop_lock);
+
+        spin_lock(&card->creg_ctrl.lock);
         card->creg_ctrl.active = 0;
         creg_kick_queue(card);
-        spin_unlock(&card->creg_ctrl.pop_lock);
+        spin_unlock(&card->creg_ctrl.lock);
 }
 
 
@@ -291,10 +296,10 @@ creg_done:
 
         kmem_cache_free(creg_cmd_pool, cmd);
 
-        mutex_lock(&card->creg_ctrl.lock);
+        spin_lock(&card->creg_ctrl.lock);
         card->creg_ctrl.active = 0;
         creg_kick_queue(card);
-        mutex_unlock(&card->creg_ctrl.lock);
+        spin_unlock(&card->creg_ctrl.lock);
 }
 
 static void creg_reset(struct rsxx_cardinfo *card)
@@ -303,6 +308,10 @@ static void creg_reset(struct rsxx_cardinfo *card)
         struct creg_cmd *tmp;
         unsigned long flags;
 
+        /*
+         * mutex_trylock is used here because if reset_lock is taken then a
+         * reset is already happening. So, we can just go ahead and return.
+         */
         if (!mutex_trylock(&card->creg_ctrl.reset_lock))
                 return;
 
@@ -315,7 +324,7 @@ static void creg_reset(struct rsxx_cardinfo *card)
                 "Resetting creg interface for recovery\n");
 
         /* Cancel outstanding commands */
-        mutex_lock(&card->creg_ctrl.lock);
+        spin_lock(&card->creg_ctrl.lock);
         list_for_each_entry_safe(cmd, tmp, &card->creg_ctrl.queue, list) {
                 list_del(&cmd->list);
                 card->creg_ctrl.q_depth--;
@@ -336,7 +345,7 @@ static void creg_reset(struct rsxx_cardinfo *card)
 
                 card->creg_ctrl.active = 0;
         }
-        mutex_unlock(&card->creg_ctrl.lock);
+        spin_unlock(&card->creg_ctrl.lock);
 
         card->creg_ctrl.reset = 0;
         spin_lock_irqsave(&card->irq_lock, flags);
@@ -359,7 +368,7 @@ static void creg_cmd_done_cb(struct rsxx_cardinfo *card,
 {
         struct creg_completion *cmd_completion;
 
-        cmd_completion = (struct creg_completion *)cmd->cb_private;
+        cmd_completion = cmd->cb_private;
         BUG_ON(!cmd_completion);
 
         cmd_completion->st = st;
@@ -380,7 +389,6 @@ static int __issue_creg_rw(struct rsxx_cardinfo *card,
         unsigned long timeout;
         int st;
 
-        INIT_COMPLETION(cmd_done);
         completion.cmd_done = &cmd_done;
         completion.st = 0;
         completion.creg_status = 0;
@@ -390,8 +398,13 @@ static int __issue_creg_rw(struct rsxx_cardinfo *card,
         if (st)
                 return st;
 
+        /*
+         * This timeout is neccessary for unresponsive hardware. The additional
+         * 20 seconds to used to guarantee that each cregs requests has time to
+         * complete.
+         */
         timeout = msecs_to_jiffies((CREG_TIMEOUT_MSEC *
-                                    card->creg_ctrl.q_depth) + 20000);
+                                card->creg_ctrl.q_depth) + 20000);
 
         /*
          * The creg interface is guaranteed to complete. It has a timeout
@@ -443,7 +456,7 @@ static int issue_creg_rw(struct rsxx_cardinfo *card,
                 if (st)
                         return st;
 
-                data   = (void *)((char *)data + xfer);
+                data   = (char *)data + xfer;
                 addr  += xfer;
                 size8 -= xfer;
         } while (size8);
@@ -558,9 +571,9 @@ static void hw_log_msg(struct rsxx_cardinfo *card, const char *str, int len)
 }
 
 /*
- * The substrncpy() function copies to string(up to count bytes) point to by src
- * (including the terminating '\0' character) to dest. Returns the number of
- * bytes copied to dest.
+ * The substrncpy function copies the src string (which includes the
+ * terminating '\0' character), up to the count into the dest pointer.
+ * Returns the number of bytes copied to dest.
  */
 static int substrncpy(char *dest, const char *src, int count)
 {
@@ -657,6 +670,9 @@ int rsxx_reg_access(struct rsxx_cardinfo *card,
         if (st)
                 return -EFAULT;
 
+        if (cmd.cnt > RSXX_MAX_REG_CNT)
+                return -EFAULT;
+
         st = issue_reg_cmd(card, &cmd, read);
         if (st)
                 return st;
@@ -682,8 +698,7 @@ int rsxx_creg_setup(struct rsxx_cardinfo *card)
         INIT_WORK(&card->creg_ctrl.done_work, creg_cmd_done);
         mutex_init(&card->creg_ctrl.reset_lock);
         INIT_LIST_HEAD(&card->creg_ctrl.queue);
-        mutex_init(&card->creg_ctrl.lock);
-        spin_lock_init(&card->creg_ctrl.pop_lock);
+        spin_lock_init(&card->creg_ctrl.lock);
         setup_timer(&card->creg_ctrl.cmd_timer, creg_cmd_timed_out,
                     (unsigned long) card);
 
@@ -697,7 +712,7 @@ void rsxx_creg_destroy(struct rsxx_cardinfo *card)
         int cnt = 0;
 
         /* Cancel outstanding commands */
-        mutex_lock(&card->creg_ctrl.lock);
+        spin_lock(&card->creg_ctrl.lock);
         list_for_each_entry_safe(cmd, tmp, &card->creg_ctrl.queue, list) {
                 list_del(&cmd->list);
                 if (cmd->cb)
@@ -722,7 +737,7 @@ void rsxx_creg_destroy(struct rsxx_cardinfo *card)
                         "Canceled active creg command\n");
                 kmem_cache_free(creg_cmd_pool, cmd);
         }
-        mutex_unlock(&card->creg_ctrl.lock);
+        spin_unlock(&card->creg_ctrl.lock);
 
         cancel_work_sync(&card->creg_ctrl.done_work);
 }
diff --git a/drivers/block/rsxx/dev.c b/drivers/block/rsxx/dev.c
index 96df053e0ed4..4346d17d2949 100644
--- a/drivers/block/rsxx/dev.c
+++ b/drivers/block/rsxx/dev.c
@@ -149,7 +149,7 @@ static void bio_dma_done_cb(struct rsxx_cardinfo *card,
                             void *cb_data,
                             unsigned int error)
 {
-        struct rsxx_bio_meta *meta = (struct rsxx_bio_meta *)cb_data;
+        struct rsxx_bio_meta *meta = cb_data;
 
         if (error)
                 atomic_set(&meta->error, 1);
diff --git a/drivers/block/rsxx/dma.c b/drivers/block/rsxx/dma.c
index 872f50358624..63176e67662f 100644
--- a/drivers/block/rsxx/dma.c
+++ b/drivers/block/rsxx/dma.c
@@ -102,7 +102,7 @@ struct dma_tracker_list {
 
 
 /*----------------- Misc Utility Functions -------------------*/
-unsigned int rsxx_addr8_to_laddr(u64 addr8, struct rsxx_cardinfo *card)
+static unsigned int rsxx_addr8_to_laddr(u64 addr8, struct rsxx_cardinfo *card)
 {
         unsigned long long tgt_addr8;
 
@@ -113,7 +113,7 @@ unsigned int rsxx_addr8_to_laddr(u64 addr8, struct rsxx_cardinfo *card)
         return tgt_addr8;
 }
 
-unsigned int rsxx_get_dma_tgt(struct rsxx_cardinfo *card, u64 addr8)
+static unsigned int rsxx_get_dma_tgt(struct rsxx_cardinfo *card, u64 addr8)
 {
         unsigned int tgt;
 
@@ -757,7 +757,7 @@ static int rsxx_dma_ctrl_init(struct pci_dev *dev,
         INIT_LIST_HEAD(&ctrl->queue);
 
         setup_timer(&ctrl->activity_timer, dma_engine_stalled,
-                    (unsigned long)ctrl);
+                                        (unsigned long)ctrl);
 
         ctrl->issue_wq = alloc_ordered_workqueue(DRIVER_NAME"_issue", 0);
         if (!ctrl->issue_wq)
@@ -803,7 +803,7 @@ static int rsxx_dma_ctrl_init(struct pci_dev *dev,
         return 0;
 }
 
-int rsxx_dma_stripe_setup(struct rsxx_cardinfo *card,
+static int rsxx_dma_stripe_setup(struct rsxx_cardinfo *card,
                               unsigned int stripe_size8)
 {
         if (!is_power_of_2(stripe_size8)) {
@@ -834,7 +834,7 @@ int rsxx_dma_stripe_setup(struct rsxx_cardinfo *card,
         return 0;
 }
 
-int rsxx_dma_configure(struct rsxx_cardinfo *card)
+static int rsxx_dma_configure(struct rsxx_cardinfo *card)
 {
         u32 intr_coal;
 
diff --git a/drivers/block/rsxx/rsxx.h b/drivers/block/rsxx/rsxx.h
index 9581e136a042..2e50b65902b7 100644
--- a/drivers/block/rsxx/rsxx.h
+++ b/drivers/block/rsxx/rsxx.h
@@ -35,6 +35,8 @@ struct rsxx_reg_access {
         __u32 data[8];
 };
 
+#define RSXX_MAX_REG_CNT        (8 * (sizeof(__u32)))
+
 #define RSXX_IOC_MAGIC 'r'
 
 #define RSXX_GETREG _IOWR(RSXX_IOC_MAGIC, 0x20, struct rsxx_reg_access)
diff --git a/drivers/block/rsxx/rsxx_priv.h b/drivers/block/rsxx/rsxx_priv.h
index 3887e496f54b..a1ac907d8f4c 100644
--- a/drivers/block/rsxx/rsxx_priv.h
+++ b/drivers/block/rsxx/rsxx_priv.h
@@ -127,7 +127,7 @@ struct rsxx_cardinfo {
 
         /* Embedded CPU Communication */
         struct {
-                struct mutex            lock;
+                spinlock_t              lock;
                 bool                    active;
                 struct creg_cmd         *active_cmd;
                 struct work_struct      done_work;
@@ -141,7 +141,6 @@ struct rsxx_cardinfo {
                 } creg_stats;
                 struct timer_list       cmd_timer;
                 struct mutex            reset_lock;
-                spinlock_t              pop_lock;
                 int                     reset;
         } creg_ctrl;
 
@@ -336,7 +335,6 @@ static inline unsigned int CREG_DATA(int N)
 
 /***** config.c *****/
 int rsxx_load_config(struct rsxx_cardinfo *card);
-int rsxx_save_config(struct rsxx_cardinfo *card);
 
 /***** core.c *****/
 void rsxx_enable_ier(struct rsxx_cardinfo *card, unsigned int intr);
@@ -345,8 +343,6 @@ void rsxx_enable_ier_and_isr(struct rsxx_cardinfo *card,
                                  unsigned int intr);
 void rsxx_disable_ier_and_isr(struct rsxx_cardinfo *card,
                                   unsigned int intr);
-char *rsxx_card_state_to_str(unsigned int state);
-irqreturn_t rsxx_isr(int irq, void *pdata);
 
 /***** dev.c *****/
 int rsxx_attach_dev(struct rsxx_cardinfo *card);
@@ -364,16 +360,11 @@ int rsxx_dma_setup(struct rsxx_cardinfo *card);
 void rsxx_dma_destroy(struct rsxx_cardinfo *card);
 int rsxx_dma_init(void);
 void rsxx_dma_cleanup(void);
-int rsxx_dma_configure(struct rsxx_cardinfo *card);
 int rsxx_dma_queue_bio(struct rsxx_cardinfo *card,
                            struct bio *bio,
                            atomic_t *n_dmas,
                            rsxx_dma_cb cb,
                            void *cb_data);
-int rsxx_dma_stripe_setup(struct rsxx_cardinfo *card,
-                              unsigned int stripe_size8);
-unsigned int rsxx_get_dma_tgt(struct rsxx_cardinfo *card, u64 addr8);
-unsigned int rsxx_addr8_to_laddr(u64 addr8, struct rsxx_cardinfo *card);
 
 /***** cregs.c *****/
 int rsxx_creg_write(struct rsxx_cardinfo *card, u32 addr,