Merge tag 'for-linus-4.19' of git://github.com/cminyard/linux-ipmi

Pull IPMI bugfixes from Corey Minyard:
 "A few fixes that came around or after the merge window, except for
  commit cd2315d471f4 ("ipmi: kcs_bmc: don't change device name") which
  is for a driver that very few people use, and those people need the
  change"

* tag 'for-linus-4.19' of git://github.com/cminyard/linux-ipmi:
  ipmi: Fix NULL pointer dereference in ssif_probe
  ipmi: Fix I2C client removal in the SSIF driver
  ipmi: Move BT capabilities detection to the detect call
  ipmi: Rework SMI registration failure
  ipmi: kcs_bmc: don't change device name

5 files changed, 97 insertions, 104 deletions

diff --git a/drivers/char/ipmi/ipmi_bt_sm.c b/drivers/char/ipmi/ipmi_bt_sm.c
index a3397664f800..97d6856c9c0f 100644
--- a/drivers/char/ipmi/ipmi_bt_sm.c
+++ b/drivers/char/ipmi/ipmi_bt_sm.c
@@ -59,8 +59,6 @@ enum bt_states {
         BT_STATE_RESET3,
         BT_STATE_RESTART,
         BT_STATE_PRINTME,
-        BT_STATE_CAPABILITIES_BEGIN,
-        BT_STATE_CAPABILITIES_END,
         BT_STATE_LONG_BUSY      /* BT doesn't get hosed :-) */
 };
 
@@ -86,7 +84,6 @@ struct si_sm_data {
         int             error_retries;  /* end of "common" fields */
         int             nonzero_status; /* hung BMCs stay all 0 */
         enum bt_states  complete;       /* to divert the state machine */
-        int             BT_CAP_outreqs;
         long            BT_CAP_req2rsp;
         int             BT_CAP_retries; /* Recommended retries */
 };
@@ -137,8 +134,6 @@ static char *state2txt(unsigned char state)
         case BT_STATE_RESET3:           return("RESET3");
         case BT_STATE_RESTART:          return("RESTART");
         case BT_STATE_LONG_BUSY:        return("LONG_BUSY");
-        case BT_STATE_CAPABILITIES_BEGIN: return("CAP_BEGIN");
-        case BT_STATE_CAPABILITIES_END: return("CAP_END");
         }
         return("BAD STATE");
 }
@@ -185,7 +180,6 @@ static unsigned int bt_init_data(struct si_sm_data *bt, struct si_sm_io *io)
         bt->complete = BT_STATE_IDLE;   /* end here */
         bt->BT_CAP_req2rsp = BT_NORMAL_TIMEOUT * USEC_PER_SEC;
         bt->BT_CAP_retries = BT_NORMAL_RETRY_LIMIT;
-        /* BT_CAP_outreqs == zero is a flag to read BT Capabilities */
         return 3; /* We claim 3 bytes of space; ought to check SPMI table */
 }
 
@@ -451,7 +445,7 @@ static enum si_sm_result error_recovery(struct si_sm_data *bt,
 
 static enum si_sm_result bt_event(struct si_sm_data *bt, long time)
 {
-        unsigned char status, BT_CAP[8];
+        unsigned char status;
         static enum bt_states last_printed = BT_STATE_PRINTME;
         int i;
 
@@ -504,12 +498,6 @@ static enum si_sm_result bt_event(struct si_sm_data *bt, long time)
                 if (status & BT_H_BUSY)         /* clear a leftover H_BUSY */
                         BT_CONTROL(BT_H_BUSY);
 
-                bt->timeout = bt->BT_CAP_req2rsp;
-
-                /* Read BT capabilities if it hasn't been done yet */
-                if (!bt->BT_CAP_outreqs)
-                        BT_STATE_CHANGE(BT_STATE_CAPABILITIES_BEGIN,
-                                        SI_SM_CALL_WITHOUT_DELAY);
                 BT_SI_SM_RETURN(SI_SM_IDLE);
 
         case BT_STATE_XACTION_START:
@@ -614,37 +602,6 @@ static enum si_sm_result bt_event(struct si_sm_data *bt, long time)
                 BT_STATE_CHANGE(BT_STATE_XACTION_START,
                                 SI_SM_CALL_WITH_DELAY);
 
-        /*
-         * Get BT Capabilities, using timing of upper level state machine.
-         * Set outreqs to prevent infinite loop on timeout.
-         */
-        case BT_STATE_CAPABILITIES_BEGIN:
-                bt->BT_CAP_outreqs = 1;
-                {
-                        unsigned char GetBT_CAP[] = { 0x18, 0x36 };
-                        bt->state = BT_STATE_IDLE;
-                        bt_start_transaction(bt, GetBT_CAP, sizeof(GetBT_CAP));
-                }
-                bt->complete = BT_STATE_CAPABILITIES_END;
-                BT_STATE_CHANGE(BT_STATE_XACTION_START,
-                                SI_SM_CALL_WITH_DELAY);
-
-        case BT_STATE_CAPABILITIES_END:
-                i = bt_get_result(bt, BT_CAP, sizeof(BT_CAP));
-                bt_init_data(bt, bt->io);
-                if ((i == 8) && !BT_CAP[2]) {
-                        bt->BT_CAP_outreqs = BT_CAP[3];
-                        bt->BT_CAP_req2rsp = BT_CAP[6] * USEC_PER_SEC;
-                        bt->BT_CAP_retries = BT_CAP[7];
-                } else
-                        printk(KERN_WARNING "IPMI BT: using default values\n");
-                if (!bt->BT_CAP_outreqs)
-                        bt->BT_CAP_outreqs = 1;
-                printk(KERN_WARNING "IPMI BT: req2rsp=%ld secs retries=%d\n",
-                        bt->BT_CAP_req2rsp / USEC_PER_SEC, bt->BT_CAP_retries);
-                bt->timeout = bt->BT_CAP_req2rsp;
-                return SI_SM_CALL_WITHOUT_DELAY;
-
         default:        /* should never occur */
                 return error_recovery(bt,
                                       status,
@@ -655,6 +612,11 @@ static enum si_sm_result bt_event(struct si_sm_data *bt, long time)
 
 static int bt_detect(struct si_sm_data *bt)
 {
+        unsigned char GetBT_CAP[] = { 0x18, 0x36 };
+        unsigned char BT_CAP[8];
+        enum si_sm_result smi_result;
+        int rv;
+
         /*
          * It's impossible for the BT status and interrupt registers to be
          * all 1's, (assuming a properly functioning, self-initialized BMC)
@@ -665,6 +627,48 @@ static int bt_detect(struct si_sm_data *bt)
         if ((BT_STATUS == 0xFF) && (BT_INTMASK_R == 0xFF))
                 return 1;
         reset_flags(bt);
+
+        /*
+         * Try getting the BT capabilities here.
+         */
+        rv = bt_start_transaction(bt, GetBT_CAP, sizeof(GetBT_CAP));
+        if (rv) {
+                dev_warn(bt->io->dev,
+                         "Can't start capabilities transaction: %d\n", rv);
+                goto out_no_bt_cap;
+        }
+
+        smi_result = SI_SM_CALL_WITHOUT_DELAY;
+        for (;;) {
+                if (smi_result == SI_SM_CALL_WITH_DELAY ||
+                    smi_result == SI_SM_CALL_WITH_TICK_DELAY) {
+                        schedule_timeout_uninterruptible(1);
+                        smi_result = bt_event(bt, jiffies_to_usecs(1));
+                } else if (smi_result == SI_SM_CALL_WITHOUT_DELAY) {
+                        smi_result = bt_event(bt, 0);
+                } else
+                        break;
+        }
+
+        rv = bt_get_result(bt, BT_CAP, sizeof(BT_CAP));
+        bt_init_data(bt, bt->io);
+        if (rv < 8) {
+                dev_warn(bt->io->dev, "bt cap response too short: %d\n", rv);
+                goto out_no_bt_cap;
+        }
+
+        if (BT_CAP[2]) {
+                dev_warn(bt->io->dev, "Error fetching bt cap: %x\n", BT_CAP[2]);
+out_no_bt_cap:
+                dev_warn(bt->io->dev, "using default values\n");
+        } else {
+                bt->BT_CAP_req2rsp = BT_CAP[6] * USEC_PER_SEC;
+                bt->BT_CAP_retries = BT_CAP[7];
+        }
+
+        dev_info(bt->io->dev, "req2rsp=%ld secs retries=%d\n",
+                 bt->BT_CAP_req2rsp / USEC_PER_SEC, bt->BT_CAP_retries);
+
         return 0;
 }
 
diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 51832b8a2c62..7fc9612070a1 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -3381,39 +3381,45 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
 
         rv = handlers->start_processing(send_info, intf);
         if (rv)
-                goto out;
+                goto out_err;
 
         rv = __bmc_get_device_id(intf, NULL, &id, NULL, NULL, i);
         if (rv) {
                 dev_err(si_dev, "Unable to get the device id: %d\n", rv);
-                goto out;
+                goto out_err_started;
         }
 
         mutex_lock(&intf->bmc_reg_mutex);
         rv = __scan_channels(intf, &id);
         mutex_unlock(&intf->bmc_reg_mutex);
+        if (rv)
+                goto out_err_bmc_reg;
 
- out:
-        if (rv) {
-                ipmi_bmc_unregister(intf);
-                list_del_rcu(&intf->link);
-                mutex_unlock(&ipmi_interfaces_mutex);
-                synchronize_srcu(&ipmi_interfaces_srcu);
-                cleanup_srcu_struct(&intf->users_srcu);
-                kref_put(&intf->refcount, intf_free);
-        } else {
-                /*
-                 * Keep memory order straight for RCU readers.  Make
-                 * sure everything else is committed to memory before
-                 * setting intf_num to mark the interface valid.
-                 */
-                smp_wmb();
-                intf->intf_num = i;
-                mutex_unlock(&ipmi_interfaces_mutex);
+        /*
+         * Keep memory order straight for RCU readers.  Make
+         * sure everything else is committed to memory before
+         * setting intf_num to mark the interface valid.
+         */
+        smp_wmb();
+        intf->intf_num = i;
+        mutex_unlock(&ipmi_interfaces_mutex);
 
-                /* After this point the interface is legal to use. */
-                call_smi_watchers(i, intf->si_dev);
-        }
+        /* After this point the interface is legal to use. */
+        call_smi_watchers(i, intf->si_dev);
+
+        return 0;
+
+ out_err_bmc_reg:
+        ipmi_bmc_unregister(intf);
+ out_err_started:
+        if (intf->handlers->shutdown)
+                intf->handlers->shutdown(intf->send_info);
+ out_err:
+        list_del_rcu(&intf->link);
+        mutex_unlock(&ipmi_interfaces_mutex);
+        synchronize_srcu(&ipmi_interfaces_srcu);
+        cleanup_srcu_struct(&intf->users_srcu);
+        kref_put(&intf->refcount, intf_free);
 
         return rv;
 }
@@ -3504,7 +3510,8 @@ void ipmi_unregister_smi(struct ipmi_smi *intf)
         }
         srcu_read_unlock(&intf->users_srcu, index);
 
-        intf->handlers->shutdown(intf->send_info);
+        if (intf->handlers->shutdown)
+                intf->handlers->shutdown(intf->send_info);
 
         cleanup_smi_msgs(intf);
 
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 90ec010bffbd..5faa917df1b6 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -2083,18 +2083,9 @@ static int try_smi_init(struct smi_info *new_smi)
                  si_to_str[new_smi->io.si_type]);
 
         WARN_ON(new_smi->io.dev->init_name != NULL);
-        kfree(init_name);
-
-        return 0;
-
-out_err:
-        if (new_smi->intf) {
-                ipmi_unregister_smi(new_smi->intf);
-                new_smi->intf = NULL;
-        }
 
+ out_err:
         kfree(init_name);
-
         return rv;
 }
 
@@ -2227,6 +2218,8 @@ static void shutdown_smi(void *send_info)
 
         kfree(smi_info->si_sm);
         smi_info->si_sm = NULL;
+
+        smi_info->intf = NULL;
 }
 
 /*
@@ -2240,10 +2233,8 @@ static void cleanup_one_si(struct smi_info *smi_info)
 
         list_del(&smi_info->link);
 
-        if (smi_info->intf) {
+        if (smi_info->intf)
                 ipmi_unregister_smi(smi_info->intf);
-                smi_info->intf = NULL;
-        }
 
         if (smi_info->pdev) {
                 if (smi_info->pdev_registered)
diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 18e4650c233b..29e67a80fb20 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -181,6 +181,8 @@ struct ssif_addr_info {
         struct device *dev;
         struct i2c_client *client;
 
+        struct i2c_client *added_client;
+
         struct mutex clients_mutex;
         struct list_head clients;
 
@@ -1214,18 +1216,11 @@ static void shutdown_ssif(void *send_info)
                 complete(&ssif_info->wake_thread);
                 kthread_stop(ssif_info->thread);
         }
-
-        /*
-         * No message can be outstanding now, we have removed the
-         * upper layer and it permitted us to do so.
-         */
-        kfree(ssif_info);
 }
 
 static int ssif_remove(struct i2c_client *client)
 {
         struct ssif_info *ssif_info = i2c_get_clientdata(client);
-        struct ipmi_smi *intf;
         struct ssif_addr_info *addr_info;
 
         if (!ssif_info)
@@ -1235,9 +1230,7 @@ static int ssif_remove(struct i2c_client *client)
          * After this point, we won't deliver anything asychronously
          * to the message handler.  We can unregister ourself.
          */
-        intf = ssif_info->intf;
-        ssif_info->intf = NULL;
-        ipmi_unregister_smi(intf);
+        ipmi_unregister_smi(ssif_info->intf);
 
         list_for_each_entry(addr_info, &ssif_infos, link) {
                 if (addr_info->client == client) {
@@ -1246,6 +1239,8 @@ static int ssif_remove(struct i2c_client *client)
                 }
         }
 
+        kfree(ssif_info);
+
         return 0;
 }
 
@@ -1648,15 +1643,9 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
  out:
         if (rv) {
-                /*
-                 * Note that if addr_info->client is assigned, we
-                 * leave it.  The i2c client hangs around even if we
-                 * return a failure here, and the failure here is not
-                 * propagated back to the i2c code.  This seems to be
-                 * design intent, strange as it may be.  But if we
-                 * don't leave it, ssif_platform_remove will not remove
-                 * the client like it should.
-                 */
+                if (addr_info)
+                        addr_info->client = NULL;
+
                 dev_err(&client->dev, "Unable to start IPMI SSIF: %d\n", rv);
                 kfree(ssif_info);
         }
@@ -1676,7 +1665,8 @@ static int ssif_adapter_handler(struct device *adev, void *opaque)
         if (adev->type != &i2c_adapter_type)
                 return 0;
 
-        i2c_new_device(to_i2c_adapter(adev), &addr_info->binfo);
+        addr_info->added_client = i2c_new_device(to_i2c_adapter(adev),
+                                                 &addr_info->binfo);
 
         if (!addr_info->adapter_name)
                 return 1; /* Only try the first I2C adapter by default. */
@@ -1849,7 +1839,7 @@ static int ssif_platform_remove(struct platform_device *dev)
                 return 0;
 
         mutex_lock(&ssif_infos_mutex);
-        i2c_unregister_device(addr_info->client);
+        i2c_unregister_device(addr_info->added_client);
 
         list_del(&addr_info->link);
         kfree(addr_info);
diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
index bb882ab161fe..e6124bd548df 100644
--- a/drivers/char/ipmi/kcs_bmc.c
+++ b/drivers/char/ipmi/kcs_bmc.c
@@ -16,6 +16,8 @@
 
 #include "kcs_bmc.h"
 
+#define DEVICE_NAME "ipmi-kcs"
+
 #define KCS_MSG_BUFSIZ    1000
 
 #define KCS_ZERO_DATA     0
@@ -429,8 +431,6 @@ struct kcs_bmc *kcs_bmc_alloc(struct device *dev, int sizeof_priv, u32 channel)
         if (!kcs_bmc)
                 return NULL;
 
-        dev_set_name(dev, "ipmi-kcs%u", channel);
-
         spin_lock_init(&kcs_bmc->lock);
         kcs_bmc->channel = channel;
 
@@ -444,7 +444,8 @@ struct kcs_bmc *kcs_bmc_alloc(struct device *dev, int sizeof_priv, u32 channel)
                 return NULL;
 
         kcs_bmc->miscdev.minor = MISC_DYNAMIC_MINOR;
-        kcs_bmc->miscdev.name = dev_name(dev);
+        kcs_bmc->miscdev.name = devm_kasprintf(dev, GFP_KERNEL, "%s%u",
+                                               DEVICE_NAME, channel);
         kcs_bmc->miscdev.fops = &kcs_bmc_fops;
 
         return kcs_bmc;