1 files changed, 222 insertions, 117 deletions

diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 18e4650c233b..ca9528c4f183 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -27,6 +27,8 @@
  * interface into the I2C driver, I believe.
  */
 
+#define pr_fmt(fmt) "ipmi_ssif: " fmt
+
 #if defined(MODVERSIONS)
 #include <linux/modversions.h>
 #endif
@@ -52,7 +54,6 @@
 #include "ipmi_si_sm.h"
 #include "ipmi_dmi.h"
 
-#define PFX "ipmi_ssif: "
 #define DEVICE_NAME "ipmi_ssif"
 
 #define IPMI_GET_SYSTEM_INTERFACE_CAPABILITIES_CMD	0x57
@@ -60,6 +61,7 @@
 #define	SSIF_IPMI_REQUEST			2
 #define	SSIF_IPMI_MULTI_PART_REQUEST_START	6
 #define	SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE	7
+#define	SSIF_IPMI_MULTI_PART_REQUEST_END	8
 #define	SSIF_IPMI_RESPONSE			3
 #define	SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE	9
 
@@ -181,6 +183,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;
 
@@ -269,6 +273,7 @@ struct ssif_info {
 	/* Info from SSIF cmd */
 	unsigned char max_xmit_msg_size;
 	unsigned char max_recv_msg_size;
+	bool cmd8_works; /* See test_multipart_messages() for details. */
 	unsigned int  multi_support;
 	int           supports_pec;
 
@@ -314,9 +319,8 @@ static void deliver_recv_msg(struct ssif_info *ssif_info,
 {
 	if (msg->rsp_size < 0) {
 		return_hosed_msg(ssif_info, msg);
-		pr_err(PFX
-		       "Malformed message in deliver_recv_msg: rsp_size = %d\n",
-		       msg->rsp_size);
+		pr_err("%s: Malformed message: rsp_size = %d\n",
+		       __func__, msg->rsp_size);
 	} else {
 		ipmi_smi_msg_received(ssif_info->intf, msg);
 	}
@@ -604,8 +608,9 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 			flags = ipmi_ssif_lock_cond(ssif_info, &oflags);
 			ssif_info->waiting_alert = true;
 			ssif_info->rtc_us_timer = SSIF_MSG_USEC;
-			mod_timer(&ssif_info->retry_timer,
-				  jiffies + SSIF_MSG_JIFFIES);
+			if (!ssif_info->stopping)
+				mod_timer(&ssif_info->retry_timer,
+					  jiffies + SSIF_MSG_JIFFIES);
 			ipmi_ssif_unlock_cond(ssif_info, flags);
 			return;
 		}
@@ -650,7 +655,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 		if (len == 0) {
 			result = -EIO;
 			if (ssif_info->ssif_debug & SSIF_DEBUG_MSG)
-				pr_info(PFX "Middle message with no data\n");
+				pr_info("Middle message with no data\n");
 
 			goto continue_op;
 		}
@@ -694,8 +699,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 					   I2C_SMBUS_BLOCK_DATA);
 			if (rv < 0) {
 				if (ssif_info->ssif_debug & SSIF_DEBUG_MSG)
-					pr_info(PFX
-						"Error from ssif_i2c_send\n");
+					pr_info("Error from ssif_i2c_send\n");
 
 				result = -EIO;
 			} else
@@ -713,7 +717,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 
  continue_op:
 	if (ssif_info->ssif_debug & SSIF_DEBUG_STATE)
-		pr_info(PFX "DONE 1: state = %d, result=%d.\n",
+		pr_info("DONE 1: state = %d, result=%d\n",
 			ssif_info->ssif_state, result);
 
 	flags = ipmi_ssif_lock_cond(ssif_info, &oflags);
@@ -747,8 +751,8 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 			 */
 			ssif_info->ssif_state = SSIF_NORMAL;
 			ipmi_ssif_unlock_cond(ssif_info, flags);
-			pr_warn(PFX "Error getting flags: %d %d, %x\n",
-			       result, len, (len >= 3) ? data[2] : 0);
+			pr_warn("Error getting flags: %d %d, %x\n",
+				result, len, (len >= 3) ? data[2] : 0);
 		} else if (data[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2
 			   || data[1] != IPMI_GET_MSG_FLAGS_CMD) {
 			/*
@@ -756,7 +760,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 			 * response to a previous command.
 			 */
 			ipmi_ssif_unlock_cond(ssif_info, flags);
-			pr_warn(PFX "Invalid response getting flags: %x %x\n",
+			pr_warn("Invalid response getting flags: %x %x\n",
 				data[0], data[1]);
 		} else {
 			ssif_inc_stat(ssif_info, flag_fetches);
@@ -769,11 +773,11 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 		/* We cleared the flags. */
 		if ((result < 0) || (len < 3) || (data[2] != 0)) {
 			/* Error clearing flags */
-			pr_warn(PFX "Error clearing flags: %d %d, %x\n",
-			       result, len, (len >= 3) ? data[2] : 0);
+			pr_warn("Error clearing flags: %d %d, %x\n",
+				result, len, (len >= 3) ? data[2] : 0);
 		} else if (data[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2
 			   || data[1] != IPMI_CLEAR_MSG_FLAGS_CMD) {
-			pr_warn(PFX "Invalid response clearing flags: %x %x\n",
+			pr_warn("Invalid response clearing flags: %x %x\n",
 				data[0], data[1]);
 		}
 		ssif_info->ssif_state = SSIF_NORMAL;
@@ -790,7 +794,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 			handle_flags(ssif_info, flags);
 		} else if (msg->rsp[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2
 			   || msg->rsp[1] != IPMI_READ_EVENT_MSG_BUFFER_CMD) {
-			pr_warn(PFX "Invalid response getting events: %x %x\n",
+			pr_warn("Invalid response getting events: %x %x\n",
 				msg->rsp[0], msg->rsp[1]);
 			msg->done(msg);
 			/* Take off the event flag. */
@@ -813,7 +817,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 			handle_flags(ssif_info, flags);
 		} else if (msg->rsp[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2
 			   || msg->rsp[1] != IPMI_GET_MSG_CMD) {
-			pr_warn(PFX "Invalid response clearing flags: %x %x\n",
+			pr_warn("Invalid response clearing flags: %x %x\n",
 				msg->rsp[0], msg->rsp[1]);
 			msg->done(msg);
 
@@ -840,7 +844,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
 		ipmi_ssif_unlock_cond(ssif_info, flags);
 
 	if (ssif_info->ssif_debug & SSIF_DEBUG_STATE)
-		pr_info(PFX "DONE 2: state = %d.\n", ssif_info->ssif_state);
+		pr_info("DONE 2: state = %d.\n", ssif_info->ssif_state);
 }
 
 static void msg_written_handler(struct ssif_info *ssif_info, int result,
@@ -860,8 +864,7 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result,
 			ssif_inc_stat(ssif_info, send_errors);
 
 			if (ssif_info->ssif_debug & SSIF_DEBUG_MSG)
-				pr_info(PFX
-					"Out of retries in msg_written_handler\n");
+				pr_info("%s: Out of retries\n", __func__);
 			msg_done_handler(ssif_info, -EIO, NULL, 0);
 			return;
 		}
@@ -885,32 +888,33 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result,
 		 * in the SSIF_MULTI_n_PART case in the probe function
 		 * for details on the intricacies of this.
 		 */
-		int left;
+		int left, to_write;
 		unsigned char *data_to_send;
+		unsigned char cmd;
 
 		ssif_inc_stat(ssif_info, sent_messages_parts);
 
 		left = ssif_info->multi_len - ssif_info->multi_pos;
-		if (left > 32)
-			left = 32;
+		to_write = left;
+		if (to_write > 32)
+			to_write = 32;
 		/* Length byte. */
-		ssif_info->multi_data[ssif_info->multi_pos] = left;
+		ssif_info->multi_data[ssif_info->multi_pos] = to_write;
 		data_to_send = ssif_info->multi_data + ssif_info->multi_pos;
-		ssif_info->multi_pos += left;
-		if (left < 32)
-			/*
-			 * Write is finished.  Note that we must end
-			 * with a write of less than 32 bytes to
-			 * complete the transaction, even if it is
-			 * zero bytes.
-			 */
+		ssif_info->multi_pos += to_write;
+		cmd = SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE;
+		if (ssif_info->cmd8_works) {
+			if (left == to_write) {
+				cmd = SSIF_IPMI_MULTI_PART_REQUEST_END;
+				ssif_info->multi_data = NULL;
+			}
+		} else if (to_write < 32) {
 			ssif_info->multi_data = NULL;
+		}
 
 		rv = ssif_i2c_send(ssif_info, msg_written_handler,
-				  I2C_SMBUS_WRITE,
-				  SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
-				  data_to_send,
-				  I2C_SMBUS_BLOCK_DATA);
+				   I2C_SMBUS_WRITE, cmd,
+				   data_to_send, I2C_SMBUS_BLOCK_DATA);
 		if (rv < 0) {
 			/* request failed, just return the error. */
 			ssif_inc_stat(ssif_info, send_errors);
@@ -937,8 +941,9 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result,
 			ssif_info->waiting_alert = true;
 			ssif_info->retries_left = SSIF_RECV_RETRIES;
 			ssif_info->rtc_us_timer = SSIF_MSG_PART_USEC;
-			mod_timer(&ssif_info->retry_timer,
-				  jiffies + SSIF_MSG_PART_JIFFIES);
+			if (!ssif_info->stopping)
+				mod_timer(&ssif_info->retry_timer,
+					  jiffies + SSIF_MSG_PART_JIFFIES);
 			ipmi_ssif_unlock_cond(ssif_info, flags);
 		}
 	}
@@ -1041,8 +1046,8 @@ static void sender(void                *send_info,
 
 		ktime_get_real_ts64(&t);
 		pr_info("**Enqueue %02x %02x: %lld.%6.6ld\n",
-		       msg->data[0], msg->data[1],
-		       (long long) t.tv_sec, (long) t.tv_nsec / NSEC_PER_USEC);
+			msg->data[0], msg->data[1],
+			(long long)t.tv_sec, (long)t.tv_nsec / NSEC_PER_USEC);
 	}
 }
 
@@ -1214,18 +1219,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 +1233,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,9 +1242,29 @@ static int ssif_remove(struct i2c_client *client)
 		}
 	}
 
+	kfree(ssif_info);
+
 	return 0;
 }
 
+static int read_response(struct i2c_client *client, unsigned char *resp)
+{
+	int ret = -ENODEV, retry_cnt = SSIF_RECV_RETRIES;
+
+	while (retry_cnt > 0) {
+		ret = i2c_smbus_read_block_data(client, SSIF_IPMI_RESPONSE,
+						resp);
+		if (ret > 0)
+			break;
+		msleep(SSIF_MSG_MSEC);
+		retry_cnt--;
+		if (retry_cnt <= 0)
+			break;
+	}
+
+	return ret;
+}
+
 static int do_cmd(struct i2c_client *client, int len, unsigned char *msg,
 		  int *resp_len, unsigned char *resp)
 {
@@ -1265,26 +1281,16 @@ static int do_cmd(struct i2c_client *client, int len, unsigned char *msg,
 		return -ENODEV;
 	}
 
-	ret = -ENODEV;
-	retry_cnt = SSIF_RECV_RETRIES;
-	while (retry_cnt > 0) {
-		ret = i2c_smbus_read_block_data(client, SSIF_IPMI_RESPONSE,
-						resp);
-		if (ret > 0)
-			break;
-		msleep(SSIF_MSG_MSEC);
-		retry_cnt--;
-		if (retry_cnt <= 0)
-			break;
-	}
-
+	ret = read_response(client, resp);
 	if (ret > 0) {
 		/* Validate that the response is correct. */
 		if (ret < 3 ||
 		    (resp[0] != (msg[0] | (1 << 2))) ||
 		    (resp[1] != msg[1]))
 			ret = -EINVAL;
-		else {
+		else if (ret > IPMI_MAX_MSG_LENGTH) {
+			ret = -E2BIG;
+		} else {
 			*resp_len = ret;
 			ret = 0;
 		}
@@ -1396,6 +1402,121 @@ static int find_slave_address(struct i2c_client *client, int slave_addr)
 	return slave_addr;
 }
 
+static int start_multipart_test(struct i2c_client *client,
+				unsigned char *msg, bool do_middle)
+{
+	int retry_cnt = SSIF_SEND_RETRIES, ret;
+
+retry_write:
+	ret = i2c_smbus_write_block_data(client,
+					 SSIF_IPMI_MULTI_PART_REQUEST_START,
+					 32, msg);
+	if (ret) {
+		retry_cnt--;
+		if (retry_cnt > 0)
+			goto retry_write;
+		dev_err(&client->dev, "Could not write multi-part start, though the BMC said it could handle it.  Just limit sends to one part.\n");
+		return ret;
+	}
+
+	if (!do_middle)
+		return 0;
+
+	ret = i2c_smbus_write_block_data(client,
+					 SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
+					 32, msg + 32);
+	if (ret) {
+		dev_err(&client->dev, "Could not write multi-part middle, though the BMC said it could handle it.  Just limit sends to one part.\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static void test_multipart_messages(struct i2c_client *client,
+				    struct ssif_info *ssif_info,
+				    unsigned char *resp)
+{
+	unsigned char msg[65];
+	int ret;
+	bool do_middle;
+
+	if (ssif_info->max_xmit_msg_size <= 32)
+		return;
+
+	do_middle = ssif_info->max_xmit_msg_size > 63;
+
+	memset(msg, 0, sizeof(msg));
+	msg[0] = IPMI_NETFN_APP_REQUEST << 2;
+	msg[1] = IPMI_GET_DEVICE_ID_CMD;
+
+	/*
+	 * The specification is all messed up dealing with sending
+	 * multi-part messages.  Per what the specification says, it
+	 * is impossible to send a message that is a multiple of 32
+	 * bytes, except for 32 itself.  It talks about a "start"
+	 * transaction (cmd=6) that must be 32 bytes, "middle"
+	 * transaction (cmd=7) that must be 32 bytes, and an "end"
+	 * transaction.  The "end" transaction is shown as cmd=7 in
+	 * the text, but if that's the case there is no way to
+	 * differentiate between a middle and end part except the
+	 * length being less than 32.  But there is a table at the far
+	 * end of the section (that I had never noticed until someone
+	 * pointed it out to me) that mentions it as cmd=8.
+	 *
+	 * After some thought, I think the example is wrong and the
+	 * end transaction should be cmd=8.  But some systems don't
+	 * implement cmd=8, they use a zero-length end transaction,
+	 * even though that violates the SMBus specification.
+	 *
+	 * So, to work around this, this code tests if cmd=8 works.
+	 * If it does, then we use that.  If not, it tests zero-
+	 * byte end transactions.  If that works, good.  If not,
+	 * we only allow 63-byte transactions max.
+	 */
+
+	ret = start_multipart_test(client, msg, do_middle);
+	if (ret)
+		goto out_no_multi_part;
+
+	ret = i2c_smbus_write_block_data(client,
+					 SSIF_IPMI_MULTI_PART_REQUEST_END,
+					 1, msg + 64);
+
+	if (!ret)
+		ret = read_response(client, resp);
+
+	if (ret > 0) {
+		/* End transactions work, we are good. */
+		ssif_info->cmd8_works = true;
+		return;
+	}
+
+	ret = start_multipart_test(client, msg, do_middle);
+	if (ret) {
+		dev_err(&client->dev, "Second multipart test failed.\n");
+		goto out_no_multi_part;
+	}
+
+	ret = i2c_smbus_write_block_data(client,
+					 SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
+					 0, msg + 64);
+	if (!ret)
+		ret = read_response(client, resp);
+	if (ret > 0)
+		/* Zero-size end parts work, use those. */
+		return;
+
+	/* Limit to 63 bytes and use a short middle command to mark the end. */
+	if (ssif_info->max_xmit_msg_size > 63)
+		ssif_info->max_xmit_msg_size = 63;
+	return;
+
+out_no_multi_part:
+	ssif_info->max_xmit_msg_size = 32;
+	return;
+}
+
 /*
  * Global enables we care about.
  */
@@ -1440,9 +1561,9 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 	slave_addr = find_slave_address(client, slave_addr);
 
-	pr_info(PFX "Trying %s-specified SSIF interface at i2c address 0x%x, adapter %s, slave address 0x%x\n",
-	       ipmi_addr_src_to_str(ssif_info->addr_source),
-	       client->addr, client->adapter->name, slave_addr);
+	pr_info("Trying %s-specified SSIF interface at i2c address 0x%x, adapter %s, slave address 0x%x\n",
+		ipmi_addr_src_to_str(ssif_info->addr_source),
+		client->addr, client->adapter->name, slave_addr);
 
 	ssif_info->client = client;
 	i2c_set_clientdata(client, ssif_info);
@@ -1455,7 +1576,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	if (!rv && (len >= 3) && (resp[2] == 0)) {
 		if (len < 7) {
 			if (ssif_dbg_probe)
-				pr_info(PFX "SSIF info too short: %d\n", len);
+				pr_info("SSIF info too short: %d\n", len);
 			goto no_support;
 		}
 
@@ -1482,26 +1603,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 			break;
 
 		case SSIF_MULTI_n_PART:
-			/*
-			 * The specification is rather confusing at
-			 * this point, but I think I understand what
-			 * is meant.  At least I have a workable
-			 * solution.  With multi-part messages, you
-			 * cannot send a message that is a multiple of
-			 * 32-bytes in length, because the start and
-			 * middle messages are 32-bytes and the end
-			 * message must be at least one byte.  You
-			 * can't fudge on an extra byte, that would
-			 * screw up things like fru data writes.  So
-			 * we limit the length to 63 bytes.  That way
-			 * a 32-byte message gets sent as a single
-			 * part.  A larger message will be a 32-byte
-			 * start and the next message is always going
-			 * to be 1-31 bytes in length.  Not ideal, but
-			 * it should work.
-			 */
-			if (ssif_info->max_xmit_msg_size > 63)
-				ssif_info->max_xmit_msg_size = 63;
+			/* We take whatever size given, but do some testing. */
 			break;
 
 		default:
@@ -1511,8 +1613,8 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	} else {
  no_support:
 		/* Assume no multi-part or PEC support */
-		pr_info(PFX "Error fetching SSIF: %d %d %2.2x, your system probably doesn't support this command so using defaults\n",
-		       rv, len, resp[2]);
+		pr_info("Error fetching SSIF: %d %d %2.2x, your system probably doesn't support this command so using defaults\n",
+			rv, len, resp[2]);
 
 		ssif_info->max_xmit_msg_size = 32;
 		ssif_info->max_recv_msg_size = 32;
@@ -1520,13 +1622,15 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		ssif_info->supports_pec = 0;
 	}
 
+	test_multipart_messages(client, ssif_info, resp);
+
 	/* Make sure the NMI timeout is cleared. */
 	msg[0] = IPMI_NETFN_APP_REQUEST << 2;
 	msg[1] = IPMI_CLEAR_MSG_FLAGS_CMD;
 	msg[2] = WDT_PRE_TIMEOUT_INT;
 	rv = do_cmd(client, 3, msg, &len, resp);
 	if (rv || (len < 3) || (resp[2] != 0))
-		pr_warn(PFX "Unable to clear message flags: %d %d %2.2x\n",
+		pr_warn("Unable to clear message flags: %d %d %2.2x\n",
 			rv, len, resp[2]);
 
 	/* Attempt to enable the event buffer. */
@@ -1534,7 +1638,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	msg[1] = IPMI_GET_BMC_GLOBAL_ENABLES_CMD;
 	rv = do_cmd(client, 2, msg, &len, resp);
 	if (rv || (len < 4) || (resp[2] != 0)) {
-		pr_warn(PFX "Error getting global enables: %d %d %2.2x\n",
+		pr_warn("Error getting global enables: %d %d %2.2x\n",
 			rv, len, resp[2]);
 		rv = 0; /* Not fatal */
 		goto found;
@@ -1553,7 +1657,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	msg[2] = ssif_info->global_enables | IPMI_BMC_EVT_MSG_BUFF;
 	rv = do_cmd(client, 3, msg, &len, resp);
 	if (rv || (len < 2)) {
-		pr_warn(PFX "Error setting global enables: %d %d %2.2x\n",
+		pr_warn("Error setting global enables: %d %d %2.2x\n",
 			rv, len, resp[2]);
 		rv = 0; /* Not fatal */
 		goto found;
@@ -1574,7 +1678,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	msg[2] = ssif_info->global_enables | IPMI_BMC_RCV_MSG_INTR;
 	rv = do_cmd(client, 3, msg, &len, resp);
 	if (rv || (len < 2)) {
-		pr_warn(PFX "Error setting global enables: %d %d %2.2x\n",
+		pr_warn("Error setting global enables: %d %d %2.2x\n",
 			rv, len, resp[2]);
 		rv = 0; /* Not fatal */
 		goto found;
@@ -1642,21 +1746,15 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 			       &ssif_info->client->dev,
 			       slave_addr);
 	 if (rv) {
-		pr_err(PFX "Unable to register device: error %d\n", rv);
+		pr_err("Unable to register device: error %d\n", rv);
 		goto out_remove_attr;
 	}
 
  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 +1774,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. */
@@ -1751,7 +1850,7 @@ static void free_ssif_clients(void)
 static unsigned short *ssif_address_list(void)
 {
 	struct ssif_addr_info *info;
-	unsigned int count = 0, i;
+	unsigned int count = 0, i = 0;
 	unsigned short *address_list;
 
 	list_for_each_entry(info, &ssif_infos, link)
@@ -1762,18 +1861,17 @@ static unsigned short *ssif_address_list(void)
 	if (!address_list)
 		return NULL;
 
-	i = 0;
 	list_for_each_entry(info, &ssif_infos, link) {
 		unsigned short addr = info->binfo.addr;
 		int j;
 
 		for (j = 0; j < i; j++) {
 			if (address_list[j] == addr)
-				goto skip_addr;
+				/* Found a dup. */
+				break;
 		}
-		address_list[i] = addr;
-skip_addr:
-		i++;
+		if (j == i) /* Didn't find it in the list. */
+			address_list[i++] = addr;
 	}
 	address_list[i] = I2C_CLIENT_END;
 
@@ -1800,7 +1898,7 @@ static int dmi_ipmi_probe(struct platform_device *pdev)
 
 	rv = device_property_read_u16(&pdev->dev, "i2c-addr", &i2c_addr);
 	if (rv) {
-		dev_warn(&pdev->dev, PFX "No i2c-addr property\n");
+		dev_warn(&pdev->dev, "No i2c-addr property\n");
 		return -ENODEV;
 	}
 
@@ -1849,7 +1947,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);
@@ -1857,12 +1955,18 @@ static int ssif_platform_remove(struct platform_device *dev)
 	return 0;
 }
 
+static const struct platform_device_id ssif_plat_ids[] = {
+    { "dmi-ipmi-ssif", 0 },
+    { }
+};
+
 static struct platform_driver ipmi_driver = {
 	.driver = {
 		.name = DEVICE_NAME,
 	},
 	.probe		= ssif_platform_probe,
 	.remove		= ssif_platform_remove,
+	.id_table       = ssif_plat_ids
 };
 
 static int init_ipmi_ssif(void)
@@ -1881,8 +1985,7 @@ static int init_ipmi_ssif(void)
 				     dbg[i], slave_addrs[i],
 				     SI_HARDCODED, NULL);
 		if (rv)
-			pr_err(PFX
-			       "Couldn't add hardcoded device at addr 0x%x\n",
+			pr_err("Couldn't add hardcoded device at addr 0x%x\n",
 			       addr[i]);
 	}
 
@@ -1893,7 +1996,7 @@ static int init_ipmi_ssif(void)
 	if (ssif_trydmi) {
 		rv = platform_driver_register(&ipmi_driver);
 		if (rv)
-			pr_err(PFX "Unable to register driver: %d\n", rv);
+			pr_err("Unable to register driver: %d\n", rv);
 	}
 
 	ssif_i2c_driver.address_list = ssif_address_list();
@@ -1915,6 +2018,8 @@ static void cleanup_ipmi_ssif(void)
 
 	i2c_del_driver(&ssif_i2c_driver);
 
+	kfree(ssif_i2c_driver.address_list);
+
 	platform_driver_unregister(&ipmi_driver);
 
 	free_ssif_clients();