diff mbox series

[v2,12/21] ipmi: kcs_bmc: Strip private client data from struct kcs_bmc

Message ID 20210319062752.145730-12-andrew@aj.id.au
State New
Headers show
Series ipmi: Allow raw access to KCS devices | expand

Commit Message

Andrew Jeffery March 19, 2021, 6:27 a.m. UTC
Move all client-private data out of `struct kcs_bmc` into the KCS client
implementation.

With this change the KCS BMC core code now only concerns itself with
abstract `struct kcs_bmc` and `struct kcs_bmc_client` types, achieving
expected separation of concerns. Further, the change clears the path for
implementation of alternative userspace interfaces.

The chardev data-structures are rearranged in the same manner applied to
the KCS device driver data-structures in an earlier patch - `struct
kcs_bmc_client` is embedded in the client's private data and we exploit
container_of() to translate as required.

Finally, now that it is free of client data, `struct kcs_bmc` is renamed
to `struct kcs_bmc_device` to contrast `struct kcs_bmc_client`.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 drivers/char/ipmi/kcs_bmc.c           |  68 +++-
 drivers/char/ipmi/kcs_bmc.h           |  86 +-----
 drivers/char/ipmi/kcs_bmc_aspeed.c    |  22 +-
 drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 428 ++++++++++++++++----------
 drivers/char/ipmi/kcs_bmc_client.h    |  28 +-
 drivers/char/ipmi/kcs_bmc_device.h    |  12 +-
 drivers/char/ipmi/kcs_bmc_npcm7xx.c   |  20 +-
 7 files changed, 368 insertions(+), 296 deletions(-)

Comments

Zev Weiss April 9, 2021, 4:07 a.m. UTC | #1
On Fri, Mar 19, 2021 at 01:27:43AM CDT, Andrew Jeffery wrote:
>Move all client-private data out of `struct kcs_bmc` into the KCS client
>implementation.
>
>With this change the KCS BMC core code now only concerns itself with
>abstract `struct kcs_bmc` and `struct kcs_bmc_client` types, achieving
>expected separation of concerns. Further, the change clears the path for
>implementation of alternative userspace interfaces.
>
>The chardev data-structures are rearranged in the same manner applied to
>the KCS device driver data-structures in an earlier patch - `struct
>kcs_bmc_client` is embedded in the client's private data and we exploit
>container_of() to translate as required.
>
>Finally, now that it is free of client data, `struct kcs_bmc` is renamed
>to `struct kcs_bmc_device` to contrast `struct kcs_bmc_client`.
>
>Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>---
> drivers/char/ipmi/kcs_bmc.c           |  68 +++-
> drivers/char/ipmi/kcs_bmc.h           |  86 +-----
> drivers/char/ipmi/kcs_bmc_aspeed.c    |  22 +-
> drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 428 ++++++++++++++++----------
> drivers/char/ipmi/kcs_bmc_client.h    |  28 +-
> drivers/char/ipmi/kcs_bmc_device.h    |  12 +-
> drivers/char/ipmi/kcs_bmc_npcm7xx.c   |  20 +-
> 7 files changed, 368 insertions(+), 296 deletions(-)
>
>diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
>index 1046ce2bbefc..266ebec71d6f 100644
>--- a/drivers/char/ipmi/kcs_bmc.c
>+++ b/drivers/char/ipmi/kcs_bmc.c
>@@ -4,6 +4,7 @@
>  * Copyright (c) 2021, IBM Corp.
>  */
>
>+#include <linux/device.h>
> #include <linux/module.h>
>
> #include "kcs_bmc.h"
>@@ -14,51 +15,96 @@
>
> /* Consumer data access */
>
>-u8 kcs_bmc_read_data(struct kcs_bmc *kcs_bmc)
>+u8 kcs_bmc_read_data(struct kcs_bmc_device *kcs_bmc)
> {
> 	return kcs_bmc->ops->io_inputb(kcs_bmc, kcs_bmc->ioreg.idr);
> }
> EXPORT_SYMBOL(kcs_bmc_read_data);
>
>-void kcs_bmc_write_data(struct kcs_bmc *kcs_bmc, u8 data)
>+void kcs_bmc_write_data(struct kcs_bmc_device *kcs_bmc, u8 data)
> {
> 	kcs_bmc->ops->io_outputb(kcs_bmc, kcs_bmc->ioreg.odr, data);
> }
> EXPORT_SYMBOL(kcs_bmc_write_data);
>
>-u8 kcs_bmc_read_status(struct kcs_bmc *kcs_bmc)
>+u8 kcs_bmc_read_status(struct kcs_bmc_device *kcs_bmc)
> {
> 	return kcs_bmc->ops->io_inputb(kcs_bmc, kcs_bmc->ioreg.str);
> }
> EXPORT_SYMBOL(kcs_bmc_read_status);
>
>-void kcs_bmc_write_status(struct kcs_bmc *kcs_bmc, u8 data)
>+void kcs_bmc_write_status(struct kcs_bmc_device *kcs_bmc, u8 data)
> {
> 	kcs_bmc->ops->io_outputb(kcs_bmc, kcs_bmc->ioreg.str, data);
> }
> EXPORT_SYMBOL(kcs_bmc_write_status);
>
>-void kcs_bmc_update_status(struct kcs_bmc *kcs_bmc, u8 mask, u8 val)
>+void kcs_bmc_update_status(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 val)
> {
> 	kcs_bmc->ops->io_updateb(kcs_bmc, kcs_bmc->ioreg.str, mask, val);
> }
> EXPORT_SYMBOL(kcs_bmc_update_status);
>
>-int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc)
>+int kcs_bmc_handle_event(struct kcs_bmc_device *kcs_bmc)
> {
>-	return kcs_bmc->client.ops->event(&kcs_bmc->client);
>+	struct kcs_bmc_client *client;
>+	int rc;
>+
>+	spin_lock(&kcs_bmc->lock);
>+	client = kcs_bmc->client;
>+	if (client) {
>+		rc = client->ops->event(client);
>+	} else {
>+		u8 status;
>+
>+		status = kcs_bmc_read_status(kcs_bmc);
>+		if (status & KCS_BMC_STR_IBF) {
>+			/* Ack the event by reading the data */
>+			kcs_bmc_read_data(kcs_bmc);
>+			rc = KCS_BMC_EVENT_HANDLED;
>+		} else {
>+			rc = KCS_BMC_EVENT_NONE;
>+		}
>+	}
>+	spin_unlock(&kcs_bmc->lock);
>+
>+	return rc;
> }
> EXPORT_SYMBOL(kcs_bmc_handle_event);
>
>-int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc *kcs_bmc);
>-int kcs_bmc_add_device(struct kcs_bmc *kcs_bmc)
>+int kcs_bmc_enable_device(struct kcs_bmc_device *kcs_bmc, struct kcs_bmc_client *client)
>+{
>+	int rc;
>+
>+	spin_lock_irq(&kcs_bmc->lock);
>+	if (kcs_bmc->client) {
>+		rc = -EBUSY;
>+	} else {
>+		kcs_bmc->client = client;
>+		rc = 0;
>+	}
>+	spin_unlock_irq(&kcs_bmc->lock);
>+
>+	return rc;
>+}
>+EXPORT_SYMBOL(kcs_bmc_enable_device);
>+
>+void kcs_bmc_disable_device(struct kcs_bmc_device *kcs_bmc, struct kcs_bmc_client *client)
>+{
>+	spin_lock_irq(&kcs_bmc->lock);
>+	if (client == kcs_bmc->client)
>+		kcs_bmc->client = NULL;

Is there any situation in which a non-matching client could be passed in
here?  Might we consider issuing a warning of some sort or returning an
error to the caller if so?

>+	spin_unlock_irq(&kcs_bmc->lock);
>+}
>+EXPORT_SYMBOL(kcs_bmc_disable_device);
>+
>+int kcs_bmc_add_device(struct kcs_bmc_device *kcs_bmc)
> {
> 	return kcs_bmc_ipmi_attach_cdev(kcs_bmc);
> }
> EXPORT_SYMBOL(kcs_bmc_add_device);
>
>-int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc *kcs_bmc);
>-int kcs_bmc_remove_device(struct kcs_bmc *kcs_bmc)
>+int kcs_bmc_remove_device(struct kcs_bmc_device *kcs_bmc)
> {
> 	return kcs_bmc_ipmi_detach_cdev(kcs_bmc);
> }
>diff --git a/drivers/char/ipmi/kcs_bmc.h b/drivers/char/ipmi/kcs_bmc.h
>index a1350e567723..3f266740c759 100644
>--- a/drivers/char/ipmi/kcs_bmc.h
>+++ b/drivers/char/ipmi/kcs_bmc.h
>@@ -6,9 +6,7 @@
> #ifndef __KCS_BMC_H__
> #define __KCS_BMC_H__
>
>-#include <linux/miscdevice.h>
>-
>-#include "kcs_bmc_client.h"
>+#include <linux/list.h>
>
> #define KCS_BMC_EVENT_NONE	0
> #define KCS_BMC_EVENT_HANDLED	1
>@@ -17,53 +15,6 @@
> #define KCS_BMC_STR_IBF		BIT(1)
> #define KCS_BMC_STR_CMD_DAT	BIT(3)
>
>-/* Different phases of the KCS BMC module.
>- *  KCS_PHASE_IDLE:
>- *            BMC should not be expecting nor sending any data.
>- *  KCS_PHASE_WRITE_START:
>- *            BMC is receiving a WRITE_START command from system software.
>- *  KCS_PHASE_WRITE_DATA:
>- *            BMC is receiving a data byte from system software.
>- *  KCS_PHASE_WRITE_END_CMD:
>- *            BMC is waiting a last data byte from system software.
>- *  KCS_PHASE_WRITE_DONE:
>- *            BMC has received the whole request from system software.
>- *  KCS_PHASE_WAIT_READ:
>- *            BMC is waiting the response from the upper IPMI service.
>- *  KCS_PHASE_READ:
>- *            BMC is transferring the response to system software.
>- *  KCS_PHASE_ABORT_ERROR1:
>- *            BMC is waiting error status request from system software.
>- *  KCS_PHASE_ABORT_ERROR2:
>- *            BMC is waiting for idle status afer error from system software.
>- *  KCS_PHASE_ERROR:
>- *            BMC has detected a protocol violation at the interface level.
>- */
>-enum kcs_phases {
>-	KCS_PHASE_IDLE,
>-
>-	KCS_PHASE_WRITE_START,
>-	KCS_PHASE_WRITE_DATA,
>-	KCS_PHASE_WRITE_END_CMD,
>-	KCS_PHASE_WRITE_DONE,
>-
>-	KCS_PHASE_WAIT_READ,
>-	KCS_PHASE_READ,
>-
>-	KCS_PHASE_ABORT_ERROR1,
>-	KCS_PHASE_ABORT_ERROR2,
>-	KCS_PHASE_ERROR
>-};
>-
>-/* IPMI 2.0 - Table 9-4, KCS Interface Status Codes */
>-enum kcs_errors {
>-	KCS_NO_ERROR                = 0x00,
>-	KCS_ABORTED_BY_COMMAND      = 0x01,
>-	KCS_ILLEGAL_CONTROL_CODE    = 0x02,
>-	KCS_LENGTH_ERROR            = 0x06,
>-	KCS_UNSPECIFIED_ERROR       = 0xFF
>-};
>-
> /* IPMI 2.0 - 9.5, KCS Interface Registers
>  * @idr: Input Data Register
>  * @odr: Output Data Register
>@@ -76,36 +27,23 @@ struct kcs_ioreg {
> };
>
> struct kcs_bmc_device_ops;
>+struct kcs_bmc_client;
>+
>+struct kcs_bmc_device {
>+	struct list_head entry;
>
>-struct kcs_bmc {
> 	struct device *dev;
>-
>-	const struct kcs_bmc_device_ops *ops;
>-
>-	struct kcs_bmc_client client;
>-
>-	spinlock_t lock;
>-
> 	u32 channel;
>-	int running;
>
> 	struct kcs_ioreg ioreg;
>
>-	enum kcs_phases phase;
>-	enum kcs_errors error;
>+	const struct kcs_bmc_device_ops *ops;
>
>-	wait_queue_head_t queue;
>-	bool data_in_avail;
>-	int  data_in_idx;
>-	u8  *data_in;
>-
>-	int  data_out_idx;
>-	int  data_out_len;
>-	u8  *data_out;
>-
>-	struct mutex mutex;
>-	u8 *kbuffer;
>-
>-	struct miscdevice miscdev;
>+	spinlock_t lock;
>+	struct kcs_bmc_client *client;
> };
>+
>+/* Temporary exports while refactoring */
>+int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc_device *kcs_bmc);
>+int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc_device *kcs_bmc);
> #endif /* __KCS_BMC_H__ */
>diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c b/drivers/char/ipmi/kcs_bmc_aspeed.c
>index 1b313355b1c8..6f26e7366c0b 100644
>--- a/drivers/char/ipmi/kcs_bmc_aspeed.c
>+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
>@@ -61,7 +61,7 @@
> #define LPC_STR4             0x11C
>
> struct aspeed_kcs_bmc {
>-	struct kcs_bmc kcs_bmc;
>+	struct kcs_bmc_device kcs_bmc;
>
> 	struct regmap *map;
> };
>@@ -71,12 +71,12 @@ struct aspeed_kcs_of_ops {
> 	int (*get_io_address)(struct platform_device *pdev);
> };
>
>-static inline struct aspeed_kcs_bmc *to_aspeed_kcs_bmc(struct kcs_bmc *kcs_bmc)
>+static inline struct aspeed_kcs_bmc *to_aspeed_kcs_bmc(struct kcs_bmc_device *kcs_bmc)
> {
> 	return container_of(kcs_bmc, struct aspeed_kcs_bmc, kcs_bmc);
> }
>
>-static u8 aspeed_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
>+static u8 aspeed_kcs_inb(struct kcs_bmc_device *kcs_bmc, u32 reg)
> {
> 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
> 	u32 val = 0;
>@@ -88,7 +88,7 @@ static u8 aspeed_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
> 	return rc == 0 ? (u8) val : 0;
> }
>
>-static void aspeed_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
>+static void aspeed_kcs_outb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 data)
> {
> 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
> 	int rc;
>@@ -97,7 +97,7 @@ static void aspeed_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
> 	WARN(rc != 0, "regmap_write() failed: %d\n", rc);
> }
>
>-static void aspeed_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 val)
>+static void aspeed_kcs_updateb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 mask, u8 val)
> {
> 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
> 	int rc;
>@@ -119,7 +119,7 @@ static void aspeed_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 val
>  *     C. KCS4
>  *        D / C : CA4h / CA5h
>  */
>-static void aspeed_kcs_set_address(struct kcs_bmc *kcs_bmc, u16 addr)
>+static void aspeed_kcs_set_address(struct kcs_bmc_device *kcs_bmc, u16 addr)
> {
> 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
>
>@@ -153,7 +153,7 @@ static void aspeed_kcs_set_address(struct kcs_bmc *kcs_bmc, u16 addr)
> 	}
> }
>
>-static void aspeed_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable)
>+static void aspeed_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enable)
> {
> 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
>
>@@ -228,7 +228,7 @@ static const struct kcs_bmc_device_ops aspeed_kcs_ops = {
>
> static irqreturn_t aspeed_kcs_irq(int irq, void *arg)
> {
>-	struct kcs_bmc *kcs_bmc = arg;
>+	struct kcs_bmc_device *kcs_bmc = arg;
> 	int rc;
>
> 	rc = kcs_bmc_handle_event(kcs_bmc);
>@@ -238,7 +238,7 @@ static irqreturn_t aspeed_kcs_irq(int irq, void *arg)
> 	return rc == KCS_BMC_EVENT_HANDLED ? IRQ_HANDLED : IRQ_NONE;
> }
>
>-static int aspeed_kcs_config_irq(struct kcs_bmc *kcs_bmc,
>+static int aspeed_kcs_config_irq(struct kcs_bmc_device *kcs_bmc,
> 			struct platform_device *pdev)
> {
> 	struct device *dev = &pdev->dev;
>@@ -338,8 +338,8 @@ static int aspeed_kcs_of_v2_get_io_address(struct platform_device *pdev)
> static int aspeed_kcs_probe(struct platform_device *pdev)
> {
> 	const struct aspeed_kcs_of_ops *ops;
>+	struct kcs_bmc_device *kcs_bmc;
> 	struct aspeed_kcs_bmc *priv;
>-	struct kcs_bmc *kcs_bmc;
> 	struct device_node *np;
> 	int rc, channel, addr;
>
>@@ -400,7 +400,7 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
> static int aspeed_kcs_remove(struct platform_device *pdev)
> {
> 	struct aspeed_kcs_bmc *priv = platform_get_drvdata(pdev);
>-	struct kcs_bmc *kcs_bmc = &priv->kcs_bmc;
>+	struct kcs_bmc_device *kcs_bmc = &priv->kcs_bmc;
>
> 	kcs_bmc_remove_device(kcs_bmc);
>
>diff --git a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
>index fd852d8abe48..58c42e76483d 100644
>--- a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
>+++ b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
>@@ -8,13 +8,88 @@
> #include <linux/errno.h>
> #include <linux/io.h>
> #include <linux/ipmi_bmc.h>
>+#include <linux/list.h>
>+#include <linux/miscdevice.h>
> #include <linux/module.h>
>+#include <linux/mutex.h>
> #include <linux/platform_device.h>
> #include <linux/poll.h>
> #include <linux/sched.h>
> #include <linux/slab.h>
>
>-#include "kcs_bmc.h"
>+#include "kcs_bmc_client.h"
>+
>+/* Different phases of the KCS BMC module.
>+ *  KCS_PHASE_IDLE:
>+ *            BMC should not be expecting nor sending any data.
>+ *  KCS_PHASE_WRITE_START:
>+ *            BMC is receiving a WRITE_START command from system software.
>+ *  KCS_PHASE_WRITE_DATA:
>+ *            BMC is receiving a data byte from system software.
>+ *  KCS_PHASE_WRITE_END_CMD:
>+ *            BMC is waiting a last data byte from system software.
>+ *  KCS_PHASE_WRITE_DONE:
>+ *            BMC has received the whole request from system software.
>+ *  KCS_PHASE_WAIT_READ:
>+ *            BMC is waiting the response from the upper IPMI service.
>+ *  KCS_PHASE_READ:
>+ *            BMC is transferring the response to system software.
>+ *  KCS_PHASE_ABORT_ERROR1:
>+ *            BMC is waiting error status request from system software.
>+ *  KCS_PHASE_ABORT_ERROR2:
>+ *            BMC is waiting for idle status afer error from system software.
>+ *  KCS_PHASE_ERROR:
>+ *            BMC has detected a protocol violation at the interface level.
>+ */
>+enum kcs_ipmi_phases {
>+	KCS_PHASE_IDLE,
>+
>+	KCS_PHASE_WRITE_START,
>+	KCS_PHASE_WRITE_DATA,
>+	KCS_PHASE_WRITE_END_CMD,
>+	KCS_PHASE_WRITE_DONE,
>+
>+	KCS_PHASE_WAIT_READ,
>+	KCS_PHASE_READ,
>+
>+	KCS_PHASE_ABORT_ERROR1,
>+	KCS_PHASE_ABORT_ERROR2,
>+	KCS_PHASE_ERROR
>+};
>+
>+/* IPMI 2.0 - Table 9-4, KCS Interface Status Codes */
>+enum kcs_ipmi_errors {
>+	KCS_NO_ERROR                = 0x00,
>+	KCS_ABORTED_BY_COMMAND      = 0x01,
>+	KCS_ILLEGAL_CONTROL_CODE    = 0x02,
>+	KCS_LENGTH_ERROR            = 0x06,
>+	KCS_UNSPECIFIED_ERROR       = 0xFF
>+};
>+
>+struct kcs_bmc_ipmi {
>+	struct list_head entry;
>+
>+	struct kcs_bmc_client client;
>+
>+	spinlock_t lock;
>+
>+	enum kcs_ipmi_phases phase;
>+	enum kcs_ipmi_errors error;
>+
>+	wait_queue_head_t queue;
>+	bool data_in_avail;
>+	int  data_in_idx;
>+	u8  *data_in;
>+
>+	int  data_out_idx;
>+	int  data_out_len;
>+	u8  *data_out;
>+
>+	struct mutex mutex;
>+	u8 *kbuffer;
>+
>+	struct miscdevice miscdev;
>+};
>
> #define DEVICE_NAME "ipmi-kcs"
>
>@@ -44,171 +119,169 @@ enum kcs_states {
> #define KCS_CMD_WRITE_END         0x62
> #define KCS_CMD_READ_BYTE         0x68
>
>-static inline void set_state(struct kcs_bmc *kcs_bmc, u8 state)
>+static inline void set_state(struct kcs_bmc_ipmi *priv, u8 state)
> {
>-	kcs_bmc_update_status(kcs_bmc, KCS_STATUS_STATE_MASK,
>-					KCS_STATUS_STATE(state));
>+	kcs_bmc_update_status(priv->client.dev, KCS_STATUS_STATE_MASK, KCS_STATUS_STATE(state));
> }
>
>-static void kcs_bmc_ipmi_force_abort(struct kcs_bmc *kcs_bmc)
>+static void kcs_bmc_ipmi_force_abort(struct kcs_bmc_ipmi *priv)
> {
>-	set_state(kcs_bmc, ERROR_STATE);
>-	kcs_bmc_read_data(kcs_bmc);
>-	kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
>+	set_state(priv, ERROR_STATE);
>+	kcs_bmc_read_data(priv->client.dev);
>+	kcs_bmc_write_data(priv->client.dev, KCS_ZERO_DATA);
>
>-	kcs_bmc->phase = KCS_PHASE_ERROR;
>-	kcs_bmc->data_in_avail = false;
>-	kcs_bmc->data_in_idx = 0;
>+	priv->phase = KCS_PHASE_ERROR;
>+	priv->data_in_avail = false;
>+	priv->data_in_idx = 0;
> }
>
>-static void kcs_bmc_ipmi_handle_data(struct kcs_bmc *kcs_bmc)
>+static void kcs_bmc_ipmi_handle_data(struct kcs_bmc_ipmi *priv)
> {
>+	struct kcs_bmc_device *dev;
> 	u8 data;
>
>-	switch (kcs_bmc->phase) {
>+	dev = priv->client.dev;
>+
>+	switch (priv->phase) {
> 	case KCS_PHASE_WRITE_START:
>-		kcs_bmc->phase = KCS_PHASE_WRITE_DATA;
>+		priv->phase = KCS_PHASE_WRITE_DATA;
> 		fallthrough;
>
> 	case KCS_PHASE_WRITE_DATA:
>-		if (kcs_bmc->data_in_idx < KCS_MSG_BUFSIZ) {
>-			set_state(kcs_bmc, WRITE_STATE);
>-			kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
>-			kcs_bmc->data_in[kcs_bmc->data_in_idx++] =
>-						kcs_bmc_read_data(kcs_bmc);
>+		if (priv->data_in_idx < KCS_MSG_BUFSIZ) {
>+			set_state(priv, WRITE_STATE);
>+			kcs_bmc_write_data(dev, KCS_ZERO_DATA);
>+			priv->data_in[priv->data_in_idx++] = kcs_bmc_read_data(dev);
> 		} else {
>-			kcs_bmc_ipmi_force_abort(kcs_bmc);
>-			kcs_bmc->error = KCS_LENGTH_ERROR;
>+			kcs_bmc_ipmi_force_abort(priv);
>+			priv->error = KCS_LENGTH_ERROR;
> 		}
> 		break;
>
> 	case KCS_PHASE_WRITE_END_CMD:
>-		if (kcs_bmc->data_in_idx < KCS_MSG_BUFSIZ) {
>-			set_state(kcs_bmc, READ_STATE);
>-			kcs_bmc->data_in[kcs_bmc->data_in_idx++] =
>-						kcs_bmc_read_data(kcs_bmc);
>-			kcs_bmc->phase = KCS_PHASE_WRITE_DONE;
>-			kcs_bmc->data_in_avail = true;
>-			wake_up_interruptible(&kcs_bmc->queue);
>+		if (priv->data_in_idx < KCS_MSG_BUFSIZ) {
>+			set_state(priv, READ_STATE);
>+			priv->data_in[priv->data_in_idx++] = kcs_bmc_read_data(dev);
>+			priv->phase = KCS_PHASE_WRITE_DONE;
>+			priv->data_in_avail = true;
>+			wake_up_interruptible(&priv->queue);
> 		} else {
>-			kcs_bmc_ipmi_force_abort(kcs_bmc);
>-			kcs_bmc->error = KCS_LENGTH_ERROR;
>+			kcs_bmc_ipmi_force_abort(priv);
>+			priv->error = KCS_LENGTH_ERROR;
> 		}
> 		break;
>
> 	case KCS_PHASE_READ:
>-		if (kcs_bmc->data_out_idx == kcs_bmc->data_out_len)
>-			set_state(kcs_bmc, IDLE_STATE);
>+		if (priv->data_out_idx == priv->data_out_len)
>+			set_state(priv, IDLE_STATE);
>
>-		data = kcs_bmc_read_data(kcs_bmc);
>+		data = kcs_bmc_read_data(dev);
> 		if (data != KCS_CMD_READ_BYTE) {
>-			set_state(kcs_bmc, ERROR_STATE);
>-			kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
>+			set_state(priv, ERROR_STATE);
>+			kcs_bmc_write_data(dev, KCS_ZERO_DATA);
> 			break;
> 		}
>
>-		if (kcs_bmc->data_out_idx == kcs_bmc->data_out_len) {
>-			kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
>-			kcs_bmc->phase = KCS_PHASE_IDLE;
>+		if (priv->data_out_idx == priv->data_out_len) {
>+			kcs_bmc_write_data(dev, KCS_ZERO_DATA);
>+			priv->phase = KCS_PHASE_IDLE;
> 			break;
> 		}
>
>-		kcs_bmc_write_data(kcs_bmc,
>-			kcs_bmc->data_out[kcs_bmc->data_out_idx++]);
>+		kcs_bmc_write_data(dev, priv->data_out[priv->data_out_idx++]);
> 		break;
>
> 	case KCS_PHASE_ABORT_ERROR1:
>-		set_state(kcs_bmc, READ_STATE);
>-		kcs_bmc_read_data(kcs_bmc);
>-		kcs_bmc_write_data(kcs_bmc, kcs_bmc->error);
>-		kcs_bmc->phase = KCS_PHASE_ABORT_ERROR2;
>+		set_state(priv, READ_STATE);
>+		kcs_bmc_read_data(dev);
>+		kcs_bmc_write_data(dev, priv->error);
>+		priv->phase = KCS_PHASE_ABORT_ERROR2;
> 		break;
>
> 	case KCS_PHASE_ABORT_ERROR2:
>-		set_state(kcs_bmc, IDLE_STATE);
>-		kcs_bmc_read_data(kcs_bmc);
>-		kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
>-		kcs_bmc->phase = KCS_PHASE_IDLE;
>+		set_state(priv, IDLE_STATE);
>+		kcs_bmc_read_data(dev);
>+		kcs_bmc_write_data(dev, KCS_ZERO_DATA);
>+		priv->phase = KCS_PHASE_IDLE;
> 		break;
>
> 	default:
>-		kcs_bmc_ipmi_force_abort(kcs_bmc);
>+		kcs_bmc_ipmi_force_abort(priv);
> 		break;
> 	}
> }
>
>-static void kcs_bmc_ipmi_handle_cmd(struct kcs_bmc *kcs_bmc)
>+static void kcs_bmc_ipmi_handle_cmd(struct kcs_bmc_ipmi *priv)
> {
> 	u8 cmd;
>
>-	set_state(kcs_bmc, WRITE_STATE);
>-	kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
>+	set_state(priv, WRITE_STATE);
>+	kcs_bmc_write_data(priv->client.dev, KCS_ZERO_DATA);
>
>-	cmd = kcs_bmc_read_data(kcs_bmc);
>+	cmd = kcs_bmc_read_data(priv->client.dev);
> 	switch (cmd) {
> 	case KCS_CMD_WRITE_START:
>-		kcs_bmc->phase = KCS_PHASE_WRITE_START;
>-		kcs_bmc->error = KCS_NO_ERROR;
>-		kcs_bmc->data_in_avail = false;
>-		kcs_bmc->data_in_idx = 0;
>+		priv->phase = KCS_PHASE_WRITE_START;
>+		priv->error = KCS_NO_ERROR;
>+		priv->data_in_avail = false;
>+		priv->data_in_idx = 0;
> 		break;
>
> 	case KCS_CMD_WRITE_END:
>-		if (kcs_bmc->phase != KCS_PHASE_WRITE_DATA) {
>-			kcs_bmc_ipmi_force_abort(kcs_bmc);
>+		if (priv->phase != KCS_PHASE_WRITE_DATA) {
>+			kcs_bmc_ipmi_force_abort(priv);
> 			break;
> 		}
>
>-		kcs_bmc->phase = KCS_PHASE_WRITE_END_CMD;
>+		priv->phase = KCS_PHASE_WRITE_END_CMD;
> 		break;
>
> 	case KCS_CMD_GET_STATUS_ABORT:
>-		if (kcs_bmc->error == KCS_NO_ERROR)
>-			kcs_bmc->error = KCS_ABORTED_BY_COMMAND;
>+		if (priv->error == KCS_NO_ERROR)
>+			priv->error = KCS_ABORTED_BY_COMMAND;
>
>-		kcs_bmc->phase = KCS_PHASE_ABORT_ERROR1;
>-		kcs_bmc->data_in_avail = false;
>-		kcs_bmc->data_in_idx = 0;
>+		priv->phase = KCS_PHASE_ABORT_ERROR1;
>+		priv->data_in_avail = false;
>+		priv->data_in_idx = 0;
> 		break;
>
> 	default:
>-		kcs_bmc_ipmi_force_abort(kcs_bmc);
>-		kcs_bmc->error = KCS_ILLEGAL_CONTROL_CODE;
>+		kcs_bmc_ipmi_force_abort(priv);
>+		priv->error = KCS_ILLEGAL_CONTROL_CODE;
> 		break;
> 	}
> }
>
>-static inline struct kcs_bmc *client_to_kcs_bmc(struct kcs_bmc_client *client)
>+static inline struct kcs_bmc_ipmi *client_to_kcs_bmc_ipmi(struct kcs_bmc_client *client)
> {
>-	return container_of(client, struct kcs_bmc, client);
>+	return container_of(client, struct kcs_bmc_ipmi, client);
> }
>
> static int kcs_bmc_ipmi_event(struct kcs_bmc_client *client)
> {
>-	struct kcs_bmc *kcs_bmc;
>-	unsigned long flags;
>+	struct kcs_bmc_ipmi *priv;
> 	u8 status;
> 	int ret;
>
>-	kcs_bmc = client_to_kcs_bmc(client);
>+	priv = client_to_kcs_bmc_ipmi(client);
>+	if (!priv)
>+		return KCS_BMC_EVENT_NONE;
>
>-	spin_lock_irqsave(&kcs_bmc->lock, flags);
>+	spin_lock(&priv->lock);
>
>-	status = kcs_bmc_read_status(kcs_bmc);
>+	status = kcs_bmc_read_status(client->dev);
> 	if (status & KCS_STATUS_IBF) {
>-		if (!kcs_bmc->running)
>-			kcs_bmc_ipmi_force_abort(kcs_bmc);
>-		else if (status & KCS_STATUS_CMD_DAT)
>-			kcs_bmc_ipmi_handle_cmd(kcs_bmc);
>+		if (status & KCS_STATUS_CMD_DAT)
>+			kcs_bmc_ipmi_handle_cmd(priv);
> 		else
>-			kcs_bmc_ipmi_handle_data(kcs_bmc);
>+			kcs_bmc_ipmi_handle_data(priv);
>
> 		ret = KCS_BMC_EVENT_HANDLED;
> 	} else {
> 		ret = KCS_BMC_EVENT_NONE;
> 	}
>
>-	spin_unlock_irqrestore(&kcs_bmc->lock, flags);
>+	spin_unlock(&priv->lock);
>
> 	return ret;
> }
>@@ -217,37 +290,29 @@ static const struct kcs_bmc_client_ops kcs_bmc_ipmi_client_ops = {
> 	.event = kcs_bmc_ipmi_event,
> };
>
>-static inline struct kcs_bmc *file_to_kcs_bmc(struct file *filp)
>+static inline struct kcs_bmc_ipmi *to_kcs_bmc(struct file *filp)
> {
>-	return container_of(filp->private_data, struct kcs_bmc, miscdev);
>+	return container_of(filp->private_data, struct kcs_bmc_ipmi, miscdev);
> }
>
> static int kcs_bmc_ipmi_open(struct inode *inode, struct file *filp)
> {
>-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
>-	int ret = 0;
>+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
>
>-	spin_lock_irq(&kcs_bmc->lock);
>-	if (!kcs_bmc->running)
>-		kcs_bmc->running = 1;
>-	else
>-		ret = -EBUSY;
>-	spin_unlock_irq(&kcs_bmc->lock);
>-
>-	return ret;
>+	return kcs_bmc_enable_device(priv->client.dev, &priv->client);
> }
>
> static __poll_t kcs_bmc_ipmi_poll(struct file *filp, poll_table *wait)
> {
>-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
>+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
> 	__poll_t mask = 0;
>
>-	poll_wait(filp, &kcs_bmc->queue, wait);
>+	poll_wait(filp, &priv->queue, wait);
>
>-	spin_lock_irq(&kcs_bmc->lock);
>-	if (kcs_bmc->data_in_avail)
>+	spin_lock_irq(&priv->lock);
>+	if (priv->data_in_avail)
> 		mask |= EPOLLIN;
>-	spin_unlock_irq(&kcs_bmc->lock);
>+	spin_unlock_irq(&priv->lock);
>
> 	return mask;
> }
>@@ -255,24 +320,24 @@ static __poll_t kcs_bmc_ipmi_poll(struct file *filp, poll_table *wait)
> static ssize_t kcs_bmc_ipmi_read(struct file *filp, char __user *buf,
> 			    size_t count, loff_t *ppos)
> {
>-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
>+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
> 	bool data_avail;
> 	size_t data_len;
> 	ssize_t ret;
>
> 	if (!(filp->f_flags & O_NONBLOCK))
>-		wait_event_interruptible(kcs_bmc->queue,
>-					 kcs_bmc->data_in_avail);
>+		wait_event_interruptible(priv->queue,
>+					 priv->data_in_avail);
>
>-	mutex_lock(&kcs_bmc->mutex);
>+	mutex_lock(&priv->mutex);
>
>-	spin_lock_irq(&kcs_bmc->lock);
>-	data_avail = kcs_bmc->data_in_avail;
>+	spin_lock_irq(&priv->lock);
>+	data_avail = priv->data_in_avail;
> 	if (data_avail) {
>-		data_len = kcs_bmc->data_in_idx;
>-		memcpy(kcs_bmc->kbuffer, kcs_bmc->data_in, data_len);
>+		data_len = priv->data_in_idx;
>+		memcpy(priv->kbuffer, priv->data_in, data_len);
> 	}
>-	spin_unlock_irq(&kcs_bmc->lock);
>+	spin_unlock_irq(&priv->lock);
>
> 	if (!data_avail) {
> 		ret = -EAGAIN;
>@@ -281,35 +346,35 @@ static ssize_t kcs_bmc_ipmi_read(struct file *filp, char __user *buf,
>
> 	if (count < data_len) {
> 		pr_err("channel=%u with too large data : %zu\n",
>-			kcs_bmc->channel, data_len);
>+			priv->client.dev->channel, data_len);
>
>-		spin_lock_irq(&kcs_bmc->lock);
>-		kcs_bmc_ipmi_force_abort(kcs_bmc);
>-		spin_unlock_irq(&kcs_bmc->lock);
>+		spin_lock_irq(&priv->lock);
>+		kcs_bmc_ipmi_force_abort(priv);
>+		spin_unlock_irq(&priv->lock);
>
> 		ret = -EOVERFLOW;
> 		goto out_unlock;
> 	}
>
>-	if (copy_to_user(buf, kcs_bmc->kbuffer, data_len)) {
>+	if (copy_to_user(buf, priv->kbuffer, data_len)) {
> 		ret = -EFAULT;
> 		goto out_unlock;
> 	}
>
> 	ret = data_len;
>
>-	spin_lock_irq(&kcs_bmc->lock);
>-	if (kcs_bmc->phase == KCS_PHASE_WRITE_DONE) {
>-		kcs_bmc->phase = KCS_PHASE_WAIT_READ;
>-		kcs_bmc->data_in_avail = false;
>-		kcs_bmc->data_in_idx = 0;
>+	spin_lock_irq(&priv->lock);
>+	if (priv->phase == KCS_PHASE_WRITE_DONE) {
>+		priv->phase = KCS_PHASE_WAIT_READ;
>+		priv->data_in_avail = false;
>+		priv->data_in_idx = 0;
> 	} else {
> 		ret = -EAGAIN;
> 	}
>-	spin_unlock_irq(&kcs_bmc->lock);
>+	spin_unlock_irq(&priv->lock);
>
> out_unlock:
>-	mutex_unlock(&kcs_bmc->mutex);
>+	mutex_unlock(&priv->mutex);
>
> 	return ret;
> }
>@@ -317,35 +382,35 @@ static ssize_t kcs_bmc_ipmi_read(struct file *filp, char __user *buf,
> static ssize_t kcs_bmc_ipmi_write(struct file *filp, const char __user *buf,
> 			     size_t count, loff_t *ppos)
> {
>-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
>+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
> 	ssize_t ret;
>
> 	/* a minimum response size '3' : netfn + cmd + ccode */
> 	if (count < 3 || count > KCS_MSG_BUFSIZ)
> 		return -EINVAL;
>
>-	mutex_lock(&kcs_bmc->mutex);
>+	mutex_lock(&priv->mutex);
>
>-	if (copy_from_user(kcs_bmc->kbuffer, buf, count)) {
>+	if (copy_from_user(priv->kbuffer, buf, count)) {
> 		ret = -EFAULT;
> 		goto out_unlock;
> 	}
>
>-	spin_lock_irq(&kcs_bmc->lock);
>-	if (kcs_bmc->phase == KCS_PHASE_WAIT_READ) {
>-		kcs_bmc->phase = KCS_PHASE_READ;
>-		kcs_bmc->data_out_idx = 1;
>-		kcs_bmc->data_out_len = count;
>-		memcpy(kcs_bmc->data_out, kcs_bmc->kbuffer, count);
>-		kcs_bmc_write_data(kcs_bmc, kcs_bmc->data_out[0]);
>+	spin_lock_irq(&priv->lock);
>+	if (priv->phase == KCS_PHASE_WAIT_READ) {
>+		priv->phase = KCS_PHASE_READ;
>+		priv->data_out_idx = 1;
>+		priv->data_out_len = count;
>+		memcpy(priv->data_out, priv->kbuffer, count);
>+		kcs_bmc_write_data(priv->client.dev, priv->data_out[0]);
> 		ret = count;
> 	} else {
> 		ret = -EINVAL;
> 	}
>-	spin_unlock_irq(&kcs_bmc->lock);
>+	spin_unlock_irq(&priv->lock);
>
> out_unlock:
>-	mutex_unlock(&kcs_bmc->mutex);
>+	mutex_unlock(&priv->mutex);
>
> 	return ret;
> }
>@@ -353,22 +418,22 @@ static ssize_t kcs_bmc_ipmi_write(struct file *filp, const char __user *buf,
> static long kcs_bmc_ipmi_ioctl(struct file *filp, unsigned int cmd,
> 			  unsigned long arg)
> {
>-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
>+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
> 	long ret = 0;
>
>-	spin_lock_irq(&kcs_bmc->lock);
>+	spin_lock_irq(&priv->lock);
>
> 	switch (cmd) {
> 	case IPMI_BMC_IOCTL_SET_SMS_ATN:
>-		kcs_bmc_update_status(kcs_bmc, KCS_STATUS_SMS_ATN, KCS_STATUS_SMS_ATN);
>+		kcs_bmc_update_status(priv->client.dev, KCS_STATUS_SMS_ATN, KCS_STATUS_SMS_ATN);
> 		break;
>
> 	case IPMI_BMC_IOCTL_CLEAR_SMS_ATN:
>-		kcs_bmc_update_status(kcs_bmc, KCS_STATUS_SMS_ATN, 0);
>+		kcs_bmc_update_status(priv->client.dev, KCS_STATUS_SMS_ATN, 0);
> 		break;
>
> 	case IPMI_BMC_IOCTL_FORCE_ABORT:
>-		kcs_bmc_ipmi_force_abort(kcs_bmc);
>+		kcs_bmc_ipmi_force_abort(priv);
> 		break;
>
> 	default:
>@@ -376,19 +441,17 @@ static long kcs_bmc_ipmi_ioctl(struct file *filp, unsigned int cmd,
> 		break;
> 	}
>
>-	spin_unlock_irq(&kcs_bmc->lock);
>+	spin_unlock_irq(&priv->lock);
>
> 	return ret;
> }
>
> static int kcs_bmc_ipmi_release(struct inode *inode, struct file *filp)
> {
>-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
>+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
>
>-	spin_lock_irq(&kcs_bmc->lock);
>-	kcs_bmc->running = 0;
>-	kcs_bmc_ipmi_force_abort(kcs_bmc);
>-	spin_unlock_irq(&kcs_bmc->lock);
>+	kcs_bmc_ipmi_force_abort(priv);
>+	kcs_bmc_disable_device(priv->client.dev, &priv->client);
>
> 	return 0;
> }
>@@ -403,56 +466,78 @@ static const struct file_operations kcs_bmc_ipmi_fops = {
> 	.unlocked_ioctl = kcs_bmc_ipmi_ioctl,
> };
>
>-int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc *kcs_bmc);
>-int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc *kcs_bmc)
>+static DEFINE_SPINLOCK(kcs_bmc_ipmi_instances_lock);
>+static LIST_HEAD(kcs_bmc_ipmi_instances);
>+
>+int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc_device *kcs_bmc);
>+int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc_device *kcs_bmc)
> {
>+	struct kcs_bmc_ipmi *priv;
> 	int rc;
>
>-	spin_lock_init(&kcs_bmc->lock);
>-	mutex_init(&kcs_bmc->mutex);
>-	init_waitqueue_head(&kcs_bmc->queue);
>-
>-	kcs_bmc->client.dev = kcs_bmc;
>-	kcs_bmc->client.ops = &kcs_bmc_ipmi_client_ops;
>-	kcs_bmc->data_in = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
>-	kcs_bmc->data_out = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
>-	kcs_bmc->kbuffer = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
>-
>-	kcs_bmc->miscdev.minor = MISC_DYNAMIC_MINOR;
>-	kcs_bmc->miscdev.name = devm_kasprintf(kcs_bmc->dev, GFP_KERNEL, "%s%u",
>-					       DEVICE_NAME, kcs_bmc->channel);
>-	if (!kcs_bmc->data_in || !kcs_bmc->data_out || !kcs_bmc->kbuffer ||
>-	    !kcs_bmc->miscdev.name)
>+	priv = devm_kzalloc(kcs_bmc->dev, sizeof(*priv), GFP_KERNEL);
>+	if (!priv)
> 		return -ENOMEM;
>
>-	kcs_bmc->miscdev.fops = &kcs_bmc_ipmi_fops;
>+	spin_lock_init(&priv->lock);
>+	mutex_init(&priv->mutex);
>
>-	rc = misc_register(&kcs_bmc->miscdev);
>+	init_waitqueue_head(&priv->queue);
>+
>+	priv->client.dev = kcs_bmc;
>+	priv->client.ops = &kcs_bmc_ipmi_client_ops;
>+	priv->data_in = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
>+	priv->data_out = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
>+	priv->kbuffer = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
>+
>+	priv->miscdev.minor = MISC_DYNAMIC_MINOR;
>+	priv->miscdev.name = devm_kasprintf(kcs_bmc->dev, GFP_KERNEL, "%s%u", DEVICE_NAME,
>+					   kcs_bmc->channel);
>+	if (!priv->data_in || !priv->data_out || !priv->kbuffer || !priv->miscdev.name)
>+		return -EINVAL;
>+
>+	priv->miscdev.fops = &kcs_bmc_ipmi_fops;
>+
>+	rc = misc_register(&priv->miscdev);
> 	if (rc) {
> 		dev_err(kcs_bmc->dev, "Unable to register device: %d\n", rc);
> 		return rc;
> 	}
>
>+	spin_lock_irq(&kcs_bmc_ipmi_instances_lock);
>+	list_add(&priv->entry, &kcs_bmc_ipmi_instances);
>+	spin_unlock_irq(&kcs_bmc_ipmi_instances_lock);
>+
> 	dev_info(kcs_bmc->dev, "Initialised IPMI client for channel %d", kcs_bmc->channel);
>
> 	return 0;
> }
> EXPORT_SYMBOL(kcs_bmc_ipmi_attach_cdev);
>
>-int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc *kcs_bmc);
>-int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc *kcs_bmc)
>+int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc_device *kcs_bmc);
>+int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc_device *kcs_bmc)
> {
>-	misc_deregister(&kcs_bmc->miscdev);
>+	struct kcs_bmc_ipmi *priv = NULL, *pos;
>
>-	spin_lock_irq(&kcs_bmc->lock);
>-	kcs_bmc->running = 0;
>-	kcs_bmc_ipmi_force_abort(kcs_bmc);
>-	spin_unlock_irq(&kcs_bmc->lock);
>+	spin_lock_irq(&kcs_bmc_ipmi_instances_lock);
>+	list_for_each_entry(pos, &kcs_bmc_ipmi_instances, entry) {
>+		if (pos->client.dev == kcs_bmc) {
>+			priv = pos;
>+			list_del(&pos->entry);
>+			break;
>+		}
>+	}
>+	spin_unlock_irq(&kcs_bmc_ipmi_instances_lock);
>
>-	devm_kfree(kcs_bmc->dev, kcs_bmc->kbuffer);
>-	devm_kfree(kcs_bmc->dev, kcs_bmc->data_out);
>-	devm_kfree(kcs_bmc->dev, kcs_bmc->data_in);
>-	devm_kfree(kcs_bmc->dev, kcs_bmc);
>+	if (!priv)
>+		return 0;

Would -ENOENT or something be appropriate here if the thing we're trying
to detach isn't found?

>+
>+	misc_deregister(&priv->miscdev);
>+	kcs_bmc_disable_device(priv->client.dev, &priv->client);
>+	devm_kfree(kcs_bmc->dev, priv->kbuffer);
>+	devm_kfree(kcs_bmc->dev, priv->data_out);
>+	devm_kfree(kcs_bmc->dev, priv->data_in);
>+	devm_kfree(kcs_bmc->dev, priv);
>
> 	return 0;
> }
>@@ -460,4 +545,5 @@ EXPORT_SYMBOL(kcs_bmc_ipmi_detach_cdev);
>
> MODULE_LICENSE("GPL v2");
> MODULE_AUTHOR("Haiyue Wang <haiyue.wang@linux.intel.com>");
>+MODULE_AUTHOR("Andrew Jeffery <andrew@aj.id.au>");
> MODULE_DESCRIPTION("KCS BMC to handle the IPMI request from system software");
>diff --git a/drivers/char/ipmi/kcs_bmc_client.h b/drivers/char/ipmi/kcs_bmc_client.h
>index 140631d157d8..2dd710f4b4aa 100644
>--- a/drivers/char/ipmi/kcs_bmc_client.h
>+++ b/drivers/char/ipmi/kcs_bmc_client.h
>@@ -8,22 +8,24 @@
> #include <linux/notifier.h>
> #include <stdbool.h>
>
>-struct kcs_bmc;
>-struct kcs_bmc_client_ops;
>-
>-struct kcs_bmc_client {
>-	const struct kcs_bmc_client_ops *ops;
>-
>-	struct kcs_bmc *dev;
>-};
>+#include "kcs_bmc.h"
>
> struct kcs_bmc_client_ops {
> 	int (*event)(struct kcs_bmc_client *client);
> };
>
>-u8 kcs_bmc_read_data(struct kcs_bmc *kcs_bmc);
>-void kcs_bmc_write_data(struct kcs_bmc *kcs_bmc, u8 data);
>-u8 kcs_bmc_read_status(struct kcs_bmc *kcs_bmc);
>-void kcs_bmc_write_status(struct kcs_bmc *kcs_bmc, u8 data);
>-void kcs_bmc_update_status(struct kcs_bmc *kcs_bmc, u8 mask, u8 val);
>+struct kcs_bmc_client {
>+	const struct kcs_bmc_client_ops *ops;
>+
>+	struct kcs_bmc_device *dev;
>+};
>+
>+int kcs_bmc_enable_device(struct kcs_bmc_device *kcs_bmc, struct kcs_bmc_client *client);
>+void kcs_bmc_disable_device(struct kcs_bmc_device *kcs_bmc, struct kcs_bmc_client *client);
>+
>+u8 kcs_bmc_read_data(struct kcs_bmc_device *kcs_bmc);
>+void kcs_bmc_write_data(struct kcs_bmc_device *kcs_bmc, u8 data);
>+u8 kcs_bmc_read_status(struct kcs_bmc_device *kcs_bmc);
>+void kcs_bmc_write_status(struct kcs_bmc_device *kcs_bmc, u8 data);
>+void kcs_bmc_update_status(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 val);
> #endif
>diff --git a/drivers/char/ipmi/kcs_bmc_device.h b/drivers/char/ipmi/kcs_bmc_device.h
>index 33462174516d..57b7174b2bac 100644
>--- a/drivers/char/ipmi/kcs_bmc_device.h
>+++ b/drivers/char/ipmi/kcs_bmc_device.h
>@@ -7,13 +7,13 @@
> #include "kcs_bmc.h"
>
> struct kcs_bmc_device_ops {
>-	u8 (*io_inputb)(struct kcs_bmc *kcs_bmc, u32 reg);
>-	void (*io_outputb)(struct kcs_bmc *kcs_bmc, u32 reg, u8 b);
>-	void (*io_updateb)(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 b);
>+	u8 (*io_inputb)(struct kcs_bmc_device *kcs_bmc, u32 reg);
>+	void (*io_outputb)(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 b);
>+	void (*io_updateb)(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 mask, u8 b);
> };
>
>-int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc);
>-int kcs_bmc_add_device(struct kcs_bmc *kcs_bmc);
>-int kcs_bmc_remove_device(struct kcs_bmc *kcs_bmc);
>+int kcs_bmc_handle_event(struct kcs_bmc_device *kcs_bmc);
>+int kcs_bmc_add_device(struct kcs_bmc_device *kcs_bmc);
>+int kcs_bmc_remove_device(struct kcs_bmc_device *kcs_bmc);
>
> #endif
>diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>index 1d21697fc585..dce93ec895fc 100644
>--- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>+++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>@@ -65,7 +65,7 @@ struct npcm7xx_kcs_reg {
> };
>
> struct npcm7xx_kcs_bmc {
>-	struct kcs_bmc kcs_bmc;
>+	struct kcs_bmc_device kcs_bmc;
>
> 	struct regmap *map;
>
>@@ -78,12 +78,12 @@ static const struct npcm7xx_kcs_reg npcm7xx_kcs_reg_tbl[KCS_CHANNEL_MAX] = {
> 	{ .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL, .ie = KCS3IE },
> };
>
>-static inline struct npcm7xx_kcs_bmc *to_npcm7xx_kcs_bmc(struct kcs_bmc *kcs_bmc)
>+static inline struct npcm7xx_kcs_bmc *to_npcm7xx_kcs_bmc(struct kcs_bmc_device *kcs_bmc)
> {
> 	return container_of(kcs_bmc, struct npcm7xx_kcs_bmc, kcs_bmc);
> }
>
>-static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
>+static u8 npcm7xx_kcs_inb(struct kcs_bmc_device *kcs_bmc, u32 reg)
> {
> 	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
> 	u32 val = 0;
>@@ -95,7 +95,7 @@ static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
> 	return rc == 0 ? (u8)val : 0;
> }
>
>-static void npcm7xx_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
>+static void npcm7xx_kcs_outb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 data)
> {
> 	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
> 	int rc;
>@@ -104,7 +104,7 @@ static void npcm7xx_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
> 	WARN(rc != 0, "regmap_write() failed: %d\n", rc);
> }
>
>-static void npcm7xx_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 data)
>+static void npcm7xx_kcs_updateb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 mask, u8 data)
> {
> 	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
> 	int rc;
>@@ -113,7 +113,7 @@ static void npcm7xx_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 da
> 	WARN(rc != 0, "regmap_update_bits() failed: %d\n", rc);
> }
>
>-static void npcm7xx_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable)
>+static void npcm7xx_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enable)
> {
> 	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
>
>@@ -126,7 +126,7 @@ static void npcm7xx_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable)
>
> static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)
> {
>-	struct kcs_bmc *kcs_bmc = arg;
>+	struct kcs_bmc_device *kcs_bmc = arg;
> 	int rc;
>
> 	rc = kcs_bmc_handle_event(kcs_bmc);
>@@ -136,7 +136,7 @@ static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)
> 	return rc == KCS_BMC_EVENT_HANDLED ? IRQ_HANDLED : IRQ_NONE;
> }
>
>-static int npcm7xx_kcs_config_irq(struct kcs_bmc *kcs_bmc,
>+static int npcm7xx_kcs_config_irq(struct kcs_bmc_device *kcs_bmc,
> 				  struct platform_device *pdev)
> {
> 	struct device *dev = &pdev->dev;
>@@ -160,7 +160,7 @@ static int npcm7xx_kcs_probe(struct platform_device *pdev)
> {
> 	struct device *dev = &pdev->dev;
> 	struct npcm7xx_kcs_bmc *priv;
>-	struct kcs_bmc *kcs_bmc;
>+	struct kcs_bmc_device *kcs_bmc;
> 	u32 chan;
> 	int rc;
>
>@@ -207,7 +207,7 @@ static int npcm7xx_kcs_probe(struct platform_device *pdev)
> static int npcm7xx_kcs_remove(struct platform_device *pdev)
> {
> 	struct npcm7xx_kcs_bmc *priv = platform_get_drvdata(pdev);
>-	struct kcs_bmc *kcs_bmc = &priv->kcs_bmc;
>+	struct kcs_bmc_device *kcs_bmc = &priv->kcs_bmc;
>
> 	kcs_bmc_remove_device(kcs_bmc);
>
>-- 
>2.27.0
>
Andrew Jeffery April 9, 2021, 6:15 a.m. UTC | #2
On Fri, 9 Apr 2021, at 13:37, Zev Weiss wrote:
> On Fri, Mar 19, 2021 at 01:27:43AM CDT, Andrew Jeffery wrote:
> >Move all client-private data out of `struct kcs_bmc` into the KCS client
> >implementation.
> >
> >With this change the KCS BMC core code now only concerns itself with
> >abstract `struct kcs_bmc` and `struct kcs_bmc_client` types, achieving
> >expected separation of concerns. Further, the change clears the path for
> >implementation of alternative userspace interfaces.
> >
> >The chardev data-structures are rearranged in the same manner applied to
> >the KCS device driver data-structures in an earlier patch - `struct
> >kcs_bmc_client` is embedded in the client's private data and we exploit
> >container_of() to translate as required.
> >
> >Finally, now that it is free of client data, `struct kcs_bmc` is renamed
> >to `struct kcs_bmc_device` to contrast `struct kcs_bmc_client`.
> >
> >Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> >---
> > drivers/char/ipmi/kcs_bmc.c           |  68 +++-
> > drivers/char/ipmi/kcs_bmc.h           |  86 +-----
> > drivers/char/ipmi/kcs_bmc_aspeed.c    |  22 +-
> > drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 428 ++++++++++++++++----------
> > drivers/char/ipmi/kcs_bmc_client.h    |  28 +-
> > drivers/char/ipmi/kcs_bmc_device.h    |  12 +-
> > drivers/char/ipmi/kcs_bmc_npcm7xx.c   |  20 +-
> > 7 files changed, 368 insertions(+), 296 deletions(-)
> >
> >diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
> >index 1046ce2bbefc..266ebec71d6f 100644
> >--- a/drivers/char/ipmi/kcs_bmc.c
> >+++ b/drivers/char/ipmi/kcs_bmc.c
> >@@ -4,6 +4,7 @@
> >  * Copyright (c) 2021, IBM Corp.
> >  */
> >
> >+#include <linux/device.h>
> > #include <linux/module.h>
> >
> > #include "kcs_bmc.h"
> >@@ -14,51 +15,96 @@
> >
> > /* Consumer data access */
> >
> >-u8 kcs_bmc_read_data(struct kcs_bmc *kcs_bmc)
> >+u8 kcs_bmc_read_data(struct kcs_bmc_device *kcs_bmc)
> > {
> > 	return kcs_bmc->ops->io_inputb(kcs_bmc, kcs_bmc->ioreg.idr);
> > }
> > EXPORT_SYMBOL(kcs_bmc_read_data);
> >
> >-void kcs_bmc_write_data(struct kcs_bmc *kcs_bmc, u8 data)
> >+void kcs_bmc_write_data(struct kcs_bmc_device *kcs_bmc, u8 data)
> > {
> > 	kcs_bmc->ops->io_outputb(kcs_bmc, kcs_bmc->ioreg.odr, data);
> > }
> > EXPORT_SYMBOL(kcs_bmc_write_data);
> >
> >-u8 kcs_bmc_read_status(struct kcs_bmc *kcs_bmc)
> >+u8 kcs_bmc_read_status(struct kcs_bmc_device *kcs_bmc)
> > {
> > 	return kcs_bmc->ops->io_inputb(kcs_bmc, kcs_bmc->ioreg.str);
> > }
> > EXPORT_SYMBOL(kcs_bmc_read_status);
> >
> >-void kcs_bmc_write_status(struct kcs_bmc *kcs_bmc, u8 data)
> >+void kcs_bmc_write_status(struct kcs_bmc_device *kcs_bmc, u8 data)
> > {
> > 	kcs_bmc->ops->io_outputb(kcs_bmc, kcs_bmc->ioreg.str, data);
> > }
> > EXPORT_SYMBOL(kcs_bmc_write_status);
> >
> >-void kcs_bmc_update_status(struct kcs_bmc *kcs_bmc, u8 mask, u8 val)
> >+void kcs_bmc_update_status(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 val)
> > {
> > 	kcs_bmc->ops->io_updateb(kcs_bmc, kcs_bmc->ioreg.str, mask, val);
> > }
> > EXPORT_SYMBOL(kcs_bmc_update_status);
> >
> >-int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc)
> >+int kcs_bmc_handle_event(struct kcs_bmc_device *kcs_bmc)
> > {
> >-	return kcs_bmc->client.ops->event(&kcs_bmc->client);
> >+	struct kcs_bmc_client *client;
> >+	int rc;
> >+
> >+	spin_lock(&kcs_bmc->lock);
> >+	client = kcs_bmc->client;
> >+	if (client) {
> >+		rc = client->ops->event(client);
> >+	} else {
> >+		u8 status;
> >+
> >+		status = kcs_bmc_read_status(kcs_bmc);
> >+		if (status & KCS_BMC_STR_IBF) {
> >+			/* Ack the event by reading the data */
> >+			kcs_bmc_read_data(kcs_bmc);
> >+			rc = KCS_BMC_EVENT_HANDLED;
> >+		} else {
> >+			rc = KCS_BMC_EVENT_NONE;
> >+		}
> >+	}
> >+	spin_unlock(&kcs_bmc->lock);
> >+
> >+	return rc;
> > }
> > EXPORT_SYMBOL(kcs_bmc_handle_event);
> >
> >-int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc *kcs_bmc);
> >-int kcs_bmc_add_device(struct kcs_bmc *kcs_bmc)
> >+int kcs_bmc_enable_device(struct kcs_bmc_device *kcs_bmc, struct kcs_bmc_client *client)
> >+{
> >+	int rc;
> >+
> >+	spin_lock_irq(&kcs_bmc->lock);
> >+	if (kcs_bmc->client) {
> >+		rc = -EBUSY;
> >+	} else {
> >+		kcs_bmc->client = client;
> >+		rc = 0;
> >+	}
> >+	spin_unlock_irq(&kcs_bmc->lock);
> >+
> >+	return rc;
> >+}
> >+EXPORT_SYMBOL(kcs_bmc_enable_device);
> >+
> >+void kcs_bmc_disable_device(struct kcs_bmc_device *kcs_bmc, struct kcs_bmc_client *client)
> >+{
> >+	spin_lock_irq(&kcs_bmc->lock);
> >+	if (client == kcs_bmc->client)
> >+		kcs_bmc->client = NULL;
> 
> Is there any situation in which a non-matching client could be passed in
> here?

Yes; this allows the chardev modules to just unconditionally call 
kcs_bmc_disable_device() without having to duplicate the state tracking 
done here.

>  Might we consider issuing a warning of some sort or returning an
> error to the caller if so?

No, see above.

> 
> >+	spin_unlock_irq(&kcs_bmc->lock);
> >+}
> >+EXPORT_SYMBOL(kcs_bmc_disable_device);
> >+
> >+int kcs_bmc_add_device(struct kcs_bmc_device *kcs_bmc)
> > {
> > 	return kcs_bmc_ipmi_attach_cdev(kcs_bmc);
> > }
> > EXPORT_SYMBOL(kcs_bmc_add_device);
> >
> >-int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc *kcs_bmc);
> >-int kcs_bmc_remove_device(struct kcs_bmc *kcs_bmc)
> >+int kcs_bmc_remove_device(struct kcs_bmc_device *kcs_bmc)
> > {
> > 	return kcs_bmc_ipmi_detach_cdev(kcs_bmc);
> > }
> >diff --git a/drivers/char/ipmi/kcs_bmc.h b/drivers/char/ipmi/kcs_bmc.h
> >index a1350e567723..3f266740c759 100644
> >--- a/drivers/char/ipmi/kcs_bmc.h
> >+++ b/drivers/char/ipmi/kcs_bmc.h
> >@@ -6,9 +6,7 @@
> > #ifndef __KCS_BMC_H__
> > #define __KCS_BMC_H__
> >
> >-#include <linux/miscdevice.h>
> >-
> >-#include "kcs_bmc_client.h"
> >+#include <linux/list.h>
> >
> > #define KCS_BMC_EVENT_NONE	0
> > #define KCS_BMC_EVENT_HANDLED	1
> >@@ -17,53 +15,6 @@
> > #define KCS_BMC_STR_IBF		BIT(1)
> > #define KCS_BMC_STR_CMD_DAT	BIT(3)
> >
> >-/* Different phases of the KCS BMC module.
> >- *  KCS_PHASE_IDLE:
> >- *            BMC should not be expecting nor sending any data.
> >- *  KCS_PHASE_WRITE_START:
> >- *            BMC is receiving a WRITE_START command from system software.
> >- *  KCS_PHASE_WRITE_DATA:
> >- *            BMC is receiving a data byte from system software.
> >- *  KCS_PHASE_WRITE_END_CMD:
> >- *            BMC is waiting a last data byte from system software.
> >- *  KCS_PHASE_WRITE_DONE:
> >- *            BMC has received the whole request from system software.
> >- *  KCS_PHASE_WAIT_READ:
> >- *            BMC is waiting the response from the upper IPMI service.
> >- *  KCS_PHASE_READ:
> >- *            BMC is transferring the response to system software.
> >- *  KCS_PHASE_ABORT_ERROR1:
> >- *            BMC is waiting error status request from system software.
> >- *  KCS_PHASE_ABORT_ERROR2:
> >- *            BMC is waiting for idle status afer error from system software.
> >- *  KCS_PHASE_ERROR:
> >- *            BMC has detected a protocol violation at the interface level.
> >- */
> >-enum kcs_phases {
> >-	KCS_PHASE_IDLE,
> >-
> >-	KCS_PHASE_WRITE_START,
> >-	KCS_PHASE_WRITE_DATA,
> >-	KCS_PHASE_WRITE_END_CMD,
> >-	KCS_PHASE_WRITE_DONE,
> >-
> >-	KCS_PHASE_WAIT_READ,
> >-	KCS_PHASE_READ,
> >-
> >-	KCS_PHASE_ABORT_ERROR1,
> >-	KCS_PHASE_ABORT_ERROR2,
> >-	KCS_PHASE_ERROR
> >-};
> >-
> >-/* IPMI 2.0 - Table 9-4, KCS Interface Status Codes */
> >-enum kcs_errors {
> >-	KCS_NO_ERROR                = 0x00,
> >-	KCS_ABORTED_BY_COMMAND      = 0x01,
> >-	KCS_ILLEGAL_CONTROL_CODE    = 0x02,
> >-	KCS_LENGTH_ERROR            = 0x06,
> >-	KCS_UNSPECIFIED_ERROR       = 0xFF
> >-};
> >-
> > /* IPMI 2.0 - 9.5, KCS Interface Registers
> >  * @idr: Input Data Register
> >  * @odr: Output Data Register
> >@@ -76,36 +27,23 @@ struct kcs_ioreg {
> > };
> >
> > struct kcs_bmc_device_ops;
> >+struct kcs_bmc_client;
> >+
> >+struct kcs_bmc_device {
> >+	struct list_head entry;
> >
> >-struct kcs_bmc {
> > 	struct device *dev;
> >-
> >-	const struct kcs_bmc_device_ops *ops;
> >-
> >-	struct kcs_bmc_client client;
> >-
> >-	spinlock_t lock;
> >-
> > 	u32 channel;
> >-	int running;
> >
> > 	struct kcs_ioreg ioreg;
> >
> >-	enum kcs_phases phase;
> >-	enum kcs_errors error;
> >+	const struct kcs_bmc_device_ops *ops;
> >
> >-	wait_queue_head_t queue;
> >-	bool data_in_avail;
> >-	int  data_in_idx;
> >-	u8  *data_in;
> >-
> >-	int  data_out_idx;
> >-	int  data_out_len;
> >-	u8  *data_out;
> >-
> >-	struct mutex mutex;
> >-	u8 *kbuffer;
> >-
> >-	struct miscdevice miscdev;
> >+	spinlock_t lock;
> >+	struct kcs_bmc_client *client;
> > };
> >+
> >+/* Temporary exports while refactoring */
> >+int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc_device *kcs_bmc);
> >+int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc_device *kcs_bmc);
> > #endif /* __KCS_BMC_H__ */
> >diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c b/drivers/char/ipmi/kcs_bmc_aspeed.c
> >index 1b313355b1c8..6f26e7366c0b 100644
> >--- a/drivers/char/ipmi/kcs_bmc_aspeed.c
> >+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
> >@@ -61,7 +61,7 @@
> > #define LPC_STR4             0x11C
> >
> > struct aspeed_kcs_bmc {
> >-	struct kcs_bmc kcs_bmc;
> >+	struct kcs_bmc_device kcs_bmc;
> >
> > 	struct regmap *map;
> > };
> >@@ -71,12 +71,12 @@ struct aspeed_kcs_of_ops {
> > 	int (*get_io_address)(struct platform_device *pdev);
> > };
> >
> >-static inline struct aspeed_kcs_bmc *to_aspeed_kcs_bmc(struct kcs_bmc *kcs_bmc)
> >+static inline struct aspeed_kcs_bmc *to_aspeed_kcs_bmc(struct kcs_bmc_device *kcs_bmc)
> > {
> > 	return container_of(kcs_bmc, struct aspeed_kcs_bmc, kcs_bmc);
> > }
> >
> >-static u8 aspeed_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
> >+static u8 aspeed_kcs_inb(struct kcs_bmc_device *kcs_bmc, u32 reg)
> > {
> > 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
> > 	u32 val = 0;
> >@@ -88,7 +88,7 @@ static u8 aspeed_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
> > 	return rc == 0 ? (u8) val : 0;
> > }
> >
> >-static void aspeed_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
> >+static void aspeed_kcs_outb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 data)
> > {
> > 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
> > 	int rc;
> >@@ -97,7 +97,7 @@ static void aspeed_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
> > 	WARN(rc != 0, "regmap_write() failed: %d\n", rc);
> > }
> >
> >-static void aspeed_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 val)
> >+static void aspeed_kcs_updateb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 mask, u8 val)
> > {
> > 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
> > 	int rc;
> >@@ -119,7 +119,7 @@ static void aspeed_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 val
> >  *     C. KCS4
> >  *        D / C : CA4h / CA5h
> >  */
> >-static void aspeed_kcs_set_address(struct kcs_bmc *kcs_bmc, u16 addr)
> >+static void aspeed_kcs_set_address(struct kcs_bmc_device *kcs_bmc, u16 addr)
> > {
> > 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
> >
> >@@ -153,7 +153,7 @@ static void aspeed_kcs_set_address(struct kcs_bmc *kcs_bmc, u16 addr)
> > 	}
> > }
> >
> >-static void aspeed_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable)
> >+static void aspeed_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enable)
> > {
> > 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
> >
> >@@ -228,7 +228,7 @@ static const struct kcs_bmc_device_ops aspeed_kcs_ops = {
> >
> > static irqreturn_t aspeed_kcs_irq(int irq, void *arg)
> > {
> >-	struct kcs_bmc *kcs_bmc = arg;
> >+	struct kcs_bmc_device *kcs_bmc = arg;
> > 	int rc;
> >
> > 	rc = kcs_bmc_handle_event(kcs_bmc);
> >@@ -238,7 +238,7 @@ static irqreturn_t aspeed_kcs_irq(int irq, void *arg)
> > 	return rc == KCS_BMC_EVENT_HANDLED ? IRQ_HANDLED : IRQ_NONE;
> > }
> >
> >-static int aspeed_kcs_config_irq(struct kcs_bmc *kcs_bmc,
> >+static int aspeed_kcs_config_irq(struct kcs_bmc_device *kcs_bmc,
> > 			struct platform_device *pdev)
> > {
> > 	struct device *dev = &pdev->dev;
> >@@ -338,8 +338,8 @@ static int aspeed_kcs_of_v2_get_io_address(struct platform_device *pdev)
> > static int aspeed_kcs_probe(struct platform_device *pdev)
> > {
> > 	const struct aspeed_kcs_of_ops *ops;
> >+	struct kcs_bmc_device *kcs_bmc;
> > 	struct aspeed_kcs_bmc *priv;
> >-	struct kcs_bmc *kcs_bmc;
> > 	struct device_node *np;
> > 	int rc, channel, addr;
> >
> >@@ -400,7 +400,7 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
> > static int aspeed_kcs_remove(struct platform_device *pdev)
> > {
> > 	struct aspeed_kcs_bmc *priv = platform_get_drvdata(pdev);
> >-	struct kcs_bmc *kcs_bmc = &priv->kcs_bmc;
> >+	struct kcs_bmc_device *kcs_bmc = &priv->kcs_bmc;
> >
> > 	kcs_bmc_remove_device(kcs_bmc);
> >
> >diff --git a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
> >index fd852d8abe48..58c42e76483d 100644
> >--- a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
> >+++ b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
> >@@ -8,13 +8,88 @@
> > #include <linux/errno.h>
> > #include <linux/io.h>
> > #include <linux/ipmi_bmc.h>
> >+#include <linux/list.h>
> >+#include <linux/miscdevice.h>
> > #include <linux/module.h>
> >+#include <linux/mutex.h>
> > #include <linux/platform_device.h>
> > #include <linux/poll.h>
> > #include <linux/sched.h>
> > #include <linux/slab.h>
> >
> >-#include "kcs_bmc.h"
> >+#include "kcs_bmc_client.h"
> >+
> >+/* Different phases of the KCS BMC module.
> >+ *  KCS_PHASE_IDLE:
> >+ *            BMC should not be expecting nor sending any data.
> >+ *  KCS_PHASE_WRITE_START:
> >+ *            BMC is receiving a WRITE_START command from system software.
> >+ *  KCS_PHASE_WRITE_DATA:
> >+ *            BMC is receiving a data byte from system software.
> >+ *  KCS_PHASE_WRITE_END_CMD:
> >+ *            BMC is waiting a last data byte from system software.
> >+ *  KCS_PHASE_WRITE_DONE:
> >+ *            BMC has received the whole request from system software.
> >+ *  KCS_PHASE_WAIT_READ:
> >+ *            BMC is waiting the response from the upper IPMI service.
> >+ *  KCS_PHASE_READ:
> >+ *            BMC is transferring the response to system software.
> >+ *  KCS_PHASE_ABORT_ERROR1:
> >+ *            BMC is waiting error status request from system software.
> >+ *  KCS_PHASE_ABORT_ERROR2:
> >+ *            BMC is waiting for idle status afer error from system software.
> >+ *  KCS_PHASE_ERROR:
> >+ *            BMC has detected a protocol violation at the interface level.
> >+ */
> >+enum kcs_ipmi_phases {
> >+	KCS_PHASE_IDLE,
> >+
> >+	KCS_PHASE_WRITE_START,
> >+	KCS_PHASE_WRITE_DATA,
> >+	KCS_PHASE_WRITE_END_CMD,
> >+	KCS_PHASE_WRITE_DONE,
> >+
> >+	KCS_PHASE_WAIT_READ,
> >+	KCS_PHASE_READ,
> >+
> >+	KCS_PHASE_ABORT_ERROR1,
> >+	KCS_PHASE_ABORT_ERROR2,
> >+	KCS_PHASE_ERROR
> >+};
> >+
> >+/* IPMI 2.0 - Table 9-4, KCS Interface Status Codes */
> >+enum kcs_ipmi_errors {
> >+	KCS_NO_ERROR                = 0x00,
> >+	KCS_ABORTED_BY_COMMAND      = 0x01,
> >+	KCS_ILLEGAL_CONTROL_CODE    = 0x02,
> >+	KCS_LENGTH_ERROR            = 0x06,
> >+	KCS_UNSPECIFIED_ERROR       = 0xFF
> >+};
> >+
> >+struct kcs_bmc_ipmi {
> >+	struct list_head entry;
> >+
> >+	struct kcs_bmc_client client;
> >+
> >+	spinlock_t lock;
> >+
> >+	enum kcs_ipmi_phases phase;
> >+	enum kcs_ipmi_errors error;
> >+
> >+	wait_queue_head_t queue;
> >+	bool data_in_avail;
> >+	int  data_in_idx;
> >+	u8  *data_in;
> >+
> >+	int  data_out_idx;
> >+	int  data_out_len;
> >+	u8  *data_out;
> >+
> >+	struct mutex mutex;
> >+	u8 *kbuffer;
> >+
> >+	struct miscdevice miscdev;
> >+};
> >
> > #define DEVICE_NAME "ipmi-kcs"
> >
> >@@ -44,171 +119,169 @@ enum kcs_states {
> > #define KCS_CMD_WRITE_END         0x62
> > #define KCS_CMD_READ_BYTE         0x68
> >
> >-static inline void set_state(struct kcs_bmc *kcs_bmc, u8 state)
> >+static inline void set_state(struct kcs_bmc_ipmi *priv, u8 state)
> > {
> >-	kcs_bmc_update_status(kcs_bmc, KCS_STATUS_STATE_MASK,
> >-					KCS_STATUS_STATE(state));
> >+	kcs_bmc_update_status(priv->client.dev, KCS_STATUS_STATE_MASK, KCS_STATUS_STATE(state));
> > }
> >
> >-static void kcs_bmc_ipmi_force_abort(struct kcs_bmc *kcs_bmc)
> >+static void kcs_bmc_ipmi_force_abort(struct kcs_bmc_ipmi *priv)
> > {
> >-	set_state(kcs_bmc, ERROR_STATE);
> >-	kcs_bmc_read_data(kcs_bmc);
> >-	kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
> >+	set_state(priv, ERROR_STATE);
> >+	kcs_bmc_read_data(priv->client.dev);
> >+	kcs_bmc_write_data(priv->client.dev, KCS_ZERO_DATA);
> >
> >-	kcs_bmc->phase = KCS_PHASE_ERROR;
> >-	kcs_bmc->data_in_avail = false;
> >-	kcs_bmc->data_in_idx = 0;
> >+	priv->phase = KCS_PHASE_ERROR;
> >+	priv->data_in_avail = false;
> >+	priv->data_in_idx = 0;
> > }
> >
> >-static void kcs_bmc_ipmi_handle_data(struct kcs_bmc *kcs_bmc)
> >+static void kcs_bmc_ipmi_handle_data(struct kcs_bmc_ipmi *priv)
> > {
> >+	struct kcs_bmc_device *dev;
> > 	u8 data;
> >
> >-	switch (kcs_bmc->phase) {
> >+	dev = priv->client.dev;
> >+
> >+	switch (priv->phase) {
> > 	case KCS_PHASE_WRITE_START:
> >-		kcs_bmc->phase = KCS_PHASE_WRITE_DATA;
> >+		priv->phase = KCS_PHASE_WRITE_DATA;
> > 		fallthrough;
> >
> > 	case KCS_PHASE_WRITE_DATA:
> >-		if (kcs_bmc->data_in_idx < KCS_MSG_BUFSIZ) {
> >-			set_state(kcs_bmc, WRITE_STATE);
> >-			kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
> >-			kcs_bmc->data_in[kcs_bmc->data_in_idx++] =
> >-						kcs_bmc_read_data(kcs_bmc);
> >+		if (priv->data_in_idx < KCS_MSG_BUFSIZ) {
> >+			set_state(priv, WRITE_STATE);
> >+			kcs_bmc_write_data(dev, KCS_ZERO_DATA);
> >+			priv->data_in[priv->data_in_idx++] = kcs_bmc_read_data(dev);
> > 		} else {
> >-			kcs_bmc_ipmi_force_abort(kcs_bmc);
> >-			kcs_bmc->error = KCS_LENGTH_ERROR;
> >+			kcs_bmc_ipmi_force_abort(priv);
> >+			priv->error = KCS_LENGTH_ERROR;
> > 		}
> > 		break;
> >
> > 	case KCS_PHASE_WRITE_END_CMD:
> >-		if (kcs_bmc->data_in_idx < KCS_MSG_BUFSIZ) {
> >-			set_state(kcs_bmc, READ_STATE);
> >-			kcs_bmc->data_in[kcs_bmc->data_in_idx++] =
> >-						kcs_bmc_read_data(kcs_bmc);
> >-			kcs_bmc->phase = KCS_PHASE_WRITE_DONE;
> >-			kcs_bmc->data_in_avail = true;
> >-			wake_up_interruptible(&kcs_bmc->queue);
> >+		if (priv->data_in_idx < KCS_MSG_BUFSIZ) {
> >+			set_state(priv, READ_STATE);
> >+			priv->data_in[priv->data_in_idx++] = kcs_bmc_read_data(dev);
> >+			priv->phase = KCS_PHASE_WRITE_DONE;
> >+			priv->data_in_avail = true;
> >+			wake_up_interruptible(&priv->queue);
> > 		} else {
> >-			kcs_bmc_ipmi_force_abort(kcs_bmc);
> >-			kcs_bmc->error = KCS_LENGTH_ERROR;
> >+			kcs_bmc_ipmi_force_abort(priv);
> >+			priv->error = KCS_LENGTH_ERROR;
> > 		}
> > 		break;
> >
> > 	case KCS_PHASE_READ:
> >-		if (kcs_bmc->data_out_idx == kcs_bmc->data_out_len)
> >-			set_state(kcs_bmc, IDLE_STATE);
> >+		if (priv->data_out_idx == priv->data_out_len)
> >+			set_state(priv, IDLE_STATE);
> >
> >-		data = kcs_bmc_read_data(kcs_bmc);
> >+		data = kcs_bmc_read_data(dev);
> > 		if (data != KCS_CMD_READ_BYTE) {
> >-			set_state(kcs_bmc, ERROR_STATE);
> >-			kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
> >+			set_state(priv, ERROR_STATE);
> >+			kcs_bmc_write_data(dev, KCS_ZERO_DATA);
> > 			break;
> > 		}
> >
> >-		if (kcs_bmc->data_out_idx == kcs_bmc->data_out_len) {
> >-			kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
> >-			kcs_bmc->phase = KCS_PHASE_IDLE;
> >+		if (priv->data_out_idx == priv->data_out_len) {
> >+			kcs_bmc_write_data(dev, KCS_ZERO_DATA);
> >+			priv->phase = KCS_PHASE_IDLE;
> > 			break;
> > 		}
> >
> >-		kcs_bmc_write_data(kcs_bmc,
> >-			kcs_bmc->data_out[kcs_bmc->data_out_idx++]);
> >+		kcs_bmc_write_data(dev, priv->data_out[priv->data_out_idx++]);
> > 		break;
> >
> > 	case KCS_PHASE_ABORT_ERROR1:
> >-		set_state(kcs_bmc, READ_STATE);
> >-		kcs_bmc_read_data(kcs_bmc);
> >-		kcs_bmc_write_data(kcs_bmc, kcs_bmc->error);
> >-		kcs_bmc->phase = KCS_PHASE_ABORT_ERROR2;
> >+		set_state(priv, READ_STATE);
> >+		kcs_bmc_read_data(dev);
> >+		kcs_bmc_write_data(dev, priv->error);
> >+		priv->phase = KCS_PHASE_ABORT_ERROR2;
> > 		break;
> >
> > 	case KCS_PHASE_ABORT_ERROR2:
> >-		set_state(kcs_bmc, IDLE_STATE);
> >-		kcs_bmc_read_data(kcs_bmc);
> >-		kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
> >-		kcs_bmc->phase = KCS_PHASE_IDLE;
> >+		set_state(priv, IDLE_STATE);
> >+		kcs_bmc_read_data(dev);
> >+		kcs_bmc_write_data(dev, KCS_ZERO_DATA);
> >+		priv->phase = KCS_PHASE_IDLE;
> > 		break;
> >
> > 	default:
> >-		kcs_bmc_ipmi_force_abort(kcs_bmc);
> >+		kcs_bmc_ipmi_force_abort(priv);
> > 		break;
> > 	}
> > }
> >
> >-static void kcs_bmc_ipmi_handle_cmd(struct kcs_bmc *kcs_bmc)
> >+static void kcs_bmc_ipmi_handle_cmd(struct kcs_bmc_ipmi *priv)
> > {
> > 	u8 cmd;
> >
> >-	set_state(kcs_bmc, WRITE_STATE);
> >-	kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
> >+	set_state(priv, WRITE_STATE);
> >+	kcs_bmc_write_data(priv->client.dev, KCS_ZERO_DATA);
> >
> >-	cmd = kcs_bmc_read_data(kcs_bmc);
> >+	cmd = kcs_bmc_read_data(priv->client.dev);
> > 	switch (cmd) {
> > 	case KCS_CMD_WRITE_START:
> >-		kcs_bmc->phase = KCS_PHASE_WRITE_START;
> >-		kcs_bmc->error = KCS_NO_ERROR;
> >-		kcs_bmc->data_in_avail = false;
> >-		kcs_bmc->data_in_idx = 0;
> >+		priv->phase = KCS_PHASE_WRITE_START;
> >+		priv->error = KCS_NO_ERROR;
> >+		priv->data_in_avail = false;
> >+		priv->data_in_idx = 0;
> > 		break;
> >
> > 	case KCS_CMD_WRITE_END:
> >-		if (kcs_bmc->phase != KCS_PHASE_WRITE_DATA) {
> >-			kcs_bmc_ipmi_force_abort(kcs_bmc);
> >+		if (priv->phase != KCS_PHASE_WRITE_DATA) {
> >+			kcs_bmc_ipmi_force_abort(priv);
> > 			break;
> > 		}
> >
> >-		kcs_bmc->phase = KCS_PHASE_WRITE_END_CMD;
> >+		priv->phase = KCS_PHASE_WRITE_END_CMD;
> > 		break;
> >
> > 	case KCS_CMD_GET_STATUS_ABORT:
> >-		if (kcs_bmc->error == KCS_NO_ERROR)
> >-			kcs_bmc->error = KCS_ABORTED_BY_COMMAND;
> >+		if (priv->error == KCS_NO_ERROR)
> >+			priv->error = KCS_ABORTED_BY_COMMAND;
> >
> >-		kcs_bmc->phase = KCS_PHASE_ABORT_ERROR1;
> >-		kcs_bmc->data_in_avail = false;
> >-		kcs_bmc->data_in_idx = 0;
> >+		priv->phase = KCS_PHASE_ABORT_ERROR1;
> >+		priv->data_in_avail = false;
> >+		priv->data_in_idx = 0;
> > 		break;
> >
> > 	default:
> >-		kcs_bmc_ipmi_force_abort(kcs_bmc);
> >-		kcs_bmc->error = KCS_ILLEGAL_CONTROL_CODE;
> >+		kcs_bmc_ipmi_force_abort(priv);
> >+		priv->error = KCS_ILLEGAL_CONTROL_CODE;
> > 		break;
> > 	}
> > }
> >
> >-static inline struct kcs_bmc *client_to_kcs_bmc(struct kcs_bmc_client *client)
> >+static inline struct kcs_bmc_ipmi *client_to_kcs_bmc_ipmi(struct kcs_bmc_client *client)
> > {
> >-	return container_of(client, struct kcs_bmc, client);
> >+	return container_of(client, struct kcs_bmc_ipmi, client);
> > }
> >
> > static int kcs_bmc_ipmi_event(struct kcs_bmc_client *client)
> > {
> >-	struct kcs_bmc *kcs_bmc;
> >-	unsigned long flags;
> >+	struct kcs_bmc_ipmi *priv;
> > 	u8 status;
> > 	int ret;
> >
> >-	kcs_bmc = client_to_kcs_bmc(client);
> >+	priv = client_to_kcs_bmc_ipmi(client);
> >+	if (!priv)
> >+		return KCS_BMC_EVENT_NONE;
> >
> >-	spin_lock_irqsave(&kcs_bmc->lock, flags);
> >+	spin_lock(&priv->lock);
> >
> >-	status = kcs_bmc_read_status(kcs_bmc);
> >+	status = kcs_bmc_read_status(client->dev);
> > 	if (status & KCS_STATUS_IBF) {
> >-		if (!kcs_bmc->running)
> >-			kcs_bmc_ipmi_force_abort(kcs_bmc);
> >-		else if (status & KCS_STATUS_CMD_DAT)
> >-			kcs_bmc_ipmi_handle_cmd(kcs_bmc);
> >+		if (status & KCS_STATUS_CMD_DAT)
> >+			kcs_bmc_ipmi_handle_cmd(priv);
> > 		else
> >-			kcs_bmc_ipmi_handle_data(kcs_bmc);
> >+			kcs_bmc_ipmi_handle_data(priv);
> >
> > 		ret = KCS_BMC_EVENT_HANDLED;
> > 	} else {
> > 		ret = KCS_BMC_EVENT_NONE;
> > 	}
> >
> >-	spin_unlock_irqrestore(&kcs_bmc->lock, flags);
> >+	spin_unlock(&priv->lock);
> >
> > 	return ret;
> > }
> >@@ -217,37 +290,29 @@ static const struct kcs_bmc_client_ops kcs_bmc_ipmi_client_ops = {
> > 	.event = kcs_bmc_ipmi_event,
> > };
> >
> >-static inline struct kcs_bmc *file_to_kcs_bmc(struct file *filp)
> >+static inline struct kcs_bmc_ipmi *to_kcs_bmc(struct file *filp)
> > {
> >-	return container_of(filp->private_data, struct kcs_bmc, miscdev);
> >+	return container_of(filp->private_data, struct kcs_bmc_ipmi, miscdev);
> > }
> >
> > static int kcs_bmc_ipmi_open(struct inode *inode, struct file *filp)
> > {
> >-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
> >-	int ret = 0;
> >+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
> >
> >-	spin_lock_irq(&kcs_bmc->lock);
> >-	if (!kcs_bmc->running)
> >-		kcs_bmc->running = 1;
> >-	else
> >-		ret = -EBUSY;
> >-	spin_unlock_irq(&kcs_bmc->lock);
> >-
> >-	return ret;
> >+	return kcs_bmc_enable_device(priv->client.dev, &priv->client);
> > }
> >
> > static __poll_t kcs_bmc_ipmi_poll(struct file *filp, poll_table *wait)
> > {
> >-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
> >+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
> > 	__poll_t mask = 0;
> >
> >-	poll_wait(filp, &kcs_bmc->queue, wait);
> >+	poll_wait(filp, &priv->queue, wait);
> >
> >-	spin_lock_irq(&kcs_bmc->lock);
> >-	if (kcs_bmc->data_in_avail)
> >+	spin_lock_irq(&priv->lock);
> >+	if (priv->data_in_avail)
> > 		mask |= EPOLLIN;
> >-	spin_unlock_irq(&kcs_bmc->lock);
> >+	spin_unlock_irq(&priv->lock);
> >
> > 	return mask;
> > }
> >@@ -255,24 +320,24 @@ static __poll_t kcs_bmc_ipmi_poll(struct file *filp, poll_table *wait)
> > static ssize_t kcs_bmc_ipmi_read(struct file *filp, char __user *buf,
> > 			    size_t count, loff_t *ppos)
> > {
> >-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
> >+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
> > 	bool data_avail;
> > 	size_t data_len;
> > 	ssize_t ret;
> >
> > 	if (!(filp->f_flags & O_NONBLOCK))
> >-		wait_event_interruptible(kcs_bmc->queue,
> >-					 kcs_bmc->data_in_avail);
> >+		wait_event_interruptible(priv->queue,
> >+					 priv->data_in_avail);
> >
> >-	mutex_lock(&kcs_bmc->mutex);
> >+	mutex_lock(&priv->mutex);
> >
> >-	spin_lock_irq(&kcs_bmc->lock);
> >-	data_avail = kcs_bmc->data_in_avail;
> >+	spin_lock_irq(&priv->lock);
> >+	data_avail = priv->data_in_avail;
> > 	if (data_avail) {
> >-		data_len = kcs_bmc->data_in_idx;
> >-		memcpy(kcs_bmc->kbuffer, kcs_bmc->data_in, data_len);
> >+		data_len = priv->data_in_idx;
> >+		memcpy(priv->kbuffer, priv->data_in, data_len);
> > 	}
> >-	spin_unlock_irq(&kcs_bmc->lock);
> >+	spin_unlock_irq(&priv->lock);
> >
> > 	if (!data_avail) {
> > 		ret = -EAGAIN;
> >@@ -281,35 +346,35 @@ static ssize_t kcs_bmc_ipmi_read(struct file *filp, char __user *buf,
> >
> > 	if (count < data_len) {
> > 		pr_err("channel=%u with too large data : %zu\n",
> >-			kcs_bmc->channel, data_len);
> >+			priv->client.dev->channel, data_len);
> >
> >-		spin_lock_irq(&kcs_bmc->lock);
> >-		kcs_bmc_ipmi_force_abort(kcs_bmc);
> >-		spin_unlock_irq(&kcs_bmc->lock);
> >+		spin_lock_irq(&priv->lock);
> >+		kcs_bmc_ipmi_force_abort(priv);
> >+		spin_unlock_irq(&priv->lock);
> >
> > 		ret = -EOVERFLOW;
> > 		goto out_unlock;
> > 	}
> >
> >-	if (copy_to_user(buf, kcs_bmc->kbuffer, data_len)) {
> >+	if (copy_to_user(buf, priv->kbuffer, data_len)) {
> > 		ret = -EFAULT;
> > 		goto out_unlock;
> > 	}
> >
> > 	ret = data_len;
> >
> >-	spin_lock_irq(&kcs_bmc->lock);
> >-	if (kcs_bmc->phase == KCS_PHASE_WRITE_DONE) {
> >-		kcs_bmc->phase = KCS_PHASE_WAIT_READ;
> >-		kcs_bmc->data_in_avail = false;
> >-		kcs_bmc->data_in_idx = 0;
> >+	spin_lock_irq(&priv->lock);
> >+	if (priv->phase == KCS_PHASE_WRITE_DONE) {
> >+		priv->phase = KCS_PHASE_WAIT_READ;
> >+		priv->data_in_avail = false;
> >+		priv->data_in_idx = 0;
> > 	} else {
> > 		ret = -EAGAIN;
> > 	}
> >-	spin_unlock_irq(&kcs_bmc->lock);
> >+	spin_unlock_irq(&priv->lock);
> >
> > out_unlock:
> >-	mutex_unlock(&kcs_bmc->mutex);
> >+	mutex_unlock(&priv->mutex);
> >
> > 	return ret;
> > }
> >@@ -317,35 +382,35 @@ static ssize_t kcs_bmc_ipmi_read(struct file *filp, char __user *buf,
> > static ssize_t kcs_bmc_ipmi_write(struct file *filp, const char __user *buf,
> > 			     size_t count, loff_t *ppos)
> > {
> >-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
> >+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
> > 	ssize_t ret;
> >
> > 	/* a minimum response size '3' : netfn + cmd + ccode */
> > 	if (count < 3 || count > KCS_MSG_BUFSIZ)
> > 		return -EINVAL;
> >
> >-	mutex_lock(&kcs_bmc->mutex);
> >+	mutex_lock(&priv->mutex);
> >
> >-	if (copy_from_user(kcs_bmc->kbuffer, buf, count)) {
> >+	if (copy_from_user(priv->kbuffer, buf, count)) {
> > 		ret = -EFAULT;
> > 		goto out_unlock;
> > 	}
> >
> >-	spin_lock_irq(&kcs_bmc->lock);
> >-	if (kcs_bmc->phase == KCS_PHASE_WAIT_READ) {
> >-		kcs_bmc->phase = KCS_PHASE_READ;
> >-		kcs_bmc->data_out_idx = 1;
> >-		kcs_bmc->data_out_len = count;
> >-		memcpy(kcs_bmc->data_out, kcs_bmc->kbuffer, count);
> >-		kcs_bmc_write_data(kcs_bmc, kcs_bmc->data_out[0]);
> >+	spin_lock_irq(&priv->lock);
> >+	if (priv->phase == KCS_PHASE_WAIT_READ) {
> >+		priv->phase = KCS_PHASE_READ;
> >+		priv->data_out_idx = 1;
> >+		priv->data_out_len = count;
> >+		memcpy(priv->data_out, priv->kbuffer, count);
> >+		kcs_bmc_write_data(priv->client.dev, priv->data_out[0]);
> > 		ret = count;
> > 	} else {
> > 		ret = -EINVAL;
> > 	}
> >-	spin_unlock_irq(&kcs_bmc->lock);
> >+	spin_unlock_irq(&priv->lock);
> >
> > out_unlock:
> >-	mutex_unlock(&kcs_bmc->mutex);
> >+	mutex_unlock(&priv->mutex);
> >
> > 	return ret;
> > }
> >@@ -353,22 +418,22 @@ static ssize_t kcs_bmc_ipmi_write(struct file *filp, const char __user *buf,
> > static long kcs_bmc_ipmi_ioctl(struct file *filp, unsigned int cmd,
> > 			  unsigned long arg)
> > {
> >-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
> >+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
> > 	long ret = 0;
> >
> >-	spin_lock_irq(&kcs_bmc->lock);
> >+	spin_lock_irq(&priv->lock);
> >
> > 	switch (cmd) {
> > 	case IPMI_BMC_IOCTL_SET_SMS_ATN:
> >-		kcs_bmc_update_status(kcs_bmc, KCS_STATUS_SMS_ATN, KCS_STATUS_SMS_ATN);
> >+		kcs_bmc_update_status(priv->client.dev, KCS_STATUS_SMS_ATN, KCS_STATUS_SMS_ATN);
> > 		break;
> >
> > 	case IPMI_BMC_IOCTL_CLEAR_SMS_ATN:
> >-		kcs_bmc_update_status(kcs_bmc, KCS_STATUS_SMS_ATN, 0);
> >+		kcs_bmc_update_status(priv->client.dev, KCS_STATUS_SMS_ATN, 0);
> > 		break;
> >
> > 	case IPMI_BMC_IOCTL_FORCE_ABORT:
> >-		kcs_bmc_ipmi_force_abort(kcs_bmc);
> >+		kcs_bmc_ipmi_force_abort(priv);
> > 		break;
> >
> > 	default:
> >@@ -376,19 +441,17 @@ static long kcs_bmc_ipmi_ioctl(struct file *filp, unsigned int cmd,
> > 		break;
> > 	}
> >
> >-	spin_unlock_irq(&kcs_bmc->lock);
> >+	spin_unlock_irq(&priv->lock);
> >
> > 	return ret;
> > }
> >
> > static int kcs_bmc_ipmi_release(struct inode *inode, struct file *filp)
> > {
> >-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
> >+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
> >
> >-	spin_lock_irq(&kcs_bmc->lock);
> >-	kcs_bmc->running = 0;
> >-	kcs_bmc_ipmi_force_abort(kcs_bmc);
> >-	spin_unlock_irq(&kcs_bmc->lock);
> >+	kcs_bmc_ipmi_force_abort(priv);
> >+	kcs_bmc_disable_device(priv->client.dev, &priv->client);
> >
> > 	return 0;
> > }
> >@@ -403,56 +466,78 @@ static const struct file_operations kcs_bmc_ipmi_fops = {
> > 	.unlocked_ioctl = kcs_bmc_ipmi_ioctl,
> > };
> >
> >-int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc *kcs_bmc);
> >-int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc *kcs_bmc)
> >+static DEFINE_SPINLOCK(kcs_bmc_ipmi_instances_lock);
> >+static LIST_HEAD(kcs_bmc_ipmi_instances);
> >+
> >+int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc_device *kcs_bmc);
> >+int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc_device *kcs_bmc)
> > {
> >+	struct kcs_bmc_ipmi *priv;
> > 	int rc;
> >
> >-	spin_lock_init(&kcs_bmc->lock);
> >-	mutex_init(&kcs_bmc->mutex);
> >-	init_waitqueue_head(&kcs_bmc->queue);
> >-
> >-	kcs_bmc->client.dev = kcs_bmc;
> >-	kcs_bmc->client.ops = &kcs_bmc_ipmi_client_ops;
> >-	kcs_bmc->data_in = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
> >-	kcs_bmc->data_out = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
> >-	kcs_bmc->kbuffer = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
> >-
> >-	kcs_bmc->miscdev.minor = MISC_DYNAMIC_MINOR;
> >-	kcs_bmc->miscdev.name = devm_kasprintf(kcs_bmc->dev, GFP_KERNEL, "%s%u",
> >-					       DEVICE_NAME, kcs_bmc->channel);
> >-	if (!kcs_bmc->data_in || !kcs_bmc->data_out || !kcs_bmc->kbuffer ||
> >-	    !kcs_bmc->miscdev.name)
> >+	priv = devm_kzalloc(kcs_bmc->dev, sizeof(*priv), GFP_KERNEL);
> >+	if (!priv)
> > 		return -ENOMEM;
> >
> >-	kcs_bmc->miscdev.fops = &kcs_bmc_ipmi_fops;
> >+	spin_lock_init(&priv->lock);
> >+	mutex_init(&priv->mutex);
> >
> >-	rc = misc_register(&kcs_bmc->miscdev);
> >+	init_waitqueue_head(&priv->queue);
> >+
> >+	priv->client.dev = kcs_bmc;
> >+	priv->client.ops = &kcs_bmc_ipmi_client_ops;
> >+	priv->data_in = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
> >+	priv->data_out = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
> >+	priv->kbuffer = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
> >+
> >+	priv->miscdev.minor = MISC_DYNAMIC_MINOR;
> >+	priv->miscdev.name = devm_kasprintf(kcs_bmc->dev, GFP_KERNEL, "%s%u", DEVICE_NAME,
> >+					   kcs_bmc->channel);
> >+	if (!priv->data_in || !priv->data_out || !priv->kbuffer || !priv->miscdev.name)
> >+		return -EINVAL;
> >+
> >+	priv->miscdev.fops = &kcs_bmc_ipmi_fops;
> >+
> >+	rc = misc_register(&priv->miscdev);
> > 	if (rc) {
> > 		dev_err(kcs_bmc->dev, "Unable to register device: %d\n", rc);
> > 		return rc;
> > 	}
> >
> >+	spin_lock_irq(&kcs_bmc_ipmi_instances_lock);
> >+	list_add(&priv->entry, &kcs_bmc_ipmi_instances);
> >+	spin_unlock_irq(&kcs_bmc_ipmi_instances_lock);
> >+
> > 	dev_info(kcs_bmc->dev, "Initialised IPMI client for channel %d", kcs_bmc->channel);
> >
> > 	return 0;
> > }
> > EXPORT_SYMBOL(kcs_bmc_ipmi_attach_cdev);
> >
> >-int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc *kcs_bmc);
> >-int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc *kcs_bmc)
> >+int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc_device *kcs_bmc);
> >+int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc_device *kcs_bmc)
> > {
> >-	misc_deregister(&kcs_bmc->miscdev);
> >+	struct kcs_bmc_ipmi *priv = NULL, *pos;
> >
> >-	spin_lock_irq(&kcs_bmc->lock);
> >-	kcs_bmc->running = 0;
> >-	kcs_bmc_ipmi_force_abort(kcs_bmc);
> >-	spin_unlock_irq(&kcs_bmc->lock);
> >+	spin_lock_irq(&kcs_bmc_ipmi_instances_lock);
> >+	list_for_each_entry(pos, &kcs_bmc_ipmi_instances, entry) {
> >+		if (pos->client.dev == kcs_bmc) {
> >+			priv = pos;
> >+			list_del(&pos->entry);
> >+			break;
> >+		}
> >+	}
> >+	spin_unlock_irq(&kcs_bmc_ipmi_instances_lock);
> >
> >-	devm_kfree(kcs_bmc->dev, kcs_bmc->kbuffer);
> >-	devm_kfree(kcs_bmc->dev, kcs_bmc->data_out);
> >-	devm_kfree(kcs_bmc->dev, kcs_bmc->data_in);
> >-	devm_kfree(kcs_bmc->dev, kcs_bmc);
> >+	if (!priv)
> >+		return 0;
> 
> Would -ENOENT or something be appropriate here if the thing we're trying
> to detach isn't found?

Maybe -ENODEV. Let me think about this.

Cheers,

Andrew
diff mbox series

Patch

diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
index 1046ce2bbefc..266ebec71d6f 100644
--- a/drivers/char/ipmi/kcs_bmc.c
+++ b/drivers/char/ipmi/kcs_bmc.c
@@ -4,6 +4,7 @@ 
  * Copyright (c) 2021, IBM Corp.
  */
 
+#include <linux/device.h>
 #include <linux/module.h>
 
 #include "kcs_bmc.h"
@@ -14,51 +15,96 @@ 
 
 /* Consumer data access */
 
-u8 kcs_bmc_read_data(struct kcs_bmc *kcs_bmc)
+u8 kcs_bmc_read_data(struct kcs_bmc_device *kcs_bmc)
 {
 	return kcs_bmc->ops->io_inputb(kcs_bmc, kcs_bmc->ioreg.idr);
 }
 EXPORT_SYMBOL(kcs_bmc_read_data);
 
-void kcs_bmc_write_data(struct kcs_bmc *kcs_bmc, u8 data)
+void kcs_bmc_write_data(struct kcs_bmc_device *kcs_bmc, u8 data)
 {
 	kcs_bmc->ops->io_outputb(kcs_bmc, kcs_bmc->ioreg.odr, data);
 }
 EXPORT_SYMBOL(kcs_bmc_write_data);
 
-u8 kcs_bmc_read_status(struct kcs_bmc *kcs_bmc)
+u8 kcs_bmc_read_status(struct kcs_bmc_device *kcs_bmc)
 {
 	return kcs_bmc->ops->io_inputb(kcs_bmc, kcs_bmc->ioreg.str);
 }
 EXPORT_SYMBOL(kcs_bmc_read_status);
 
-void kcs_bmc_write_status(struct kcs_bmc *kcs_bmc, u8 data)
+void kcs_bmc_write_status(struct kcs_bmc_device *kcs_bmc, u8 data)
 {
 	kcs_bmc->ops->io_outputb(kcs_bmc, kcs_bmc->ioreg.str, data);
 }
 EXPORT_SYMBOL(kcs_bmc_write_status);
 
-void kcs_bmc_update_status(struct kcs_bmc *kcs_bmc, u8 mask, u8 val)
+void kcs_bmc_update_status(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 val)
 {
 	kcs_bmc->ops->io_updateb(kcs_bmc, kcs_bmc->ioreg.str, mask, val);
 }
 EXPORT_SYMBOL(kcs_bmc_update_status);
 
-int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc)
+int kcs_bmc_handle_event(struct kcs_bmc_device *kcs_bmc)
 {
-	return kcs_bmc->client.ops->event(&kcs_bmc->client);
+	struct kcs_bmc_client *client;
+	int rc;
+
+	spin_lock(&kcs_bmc->lock);
+	client = kcs_bmc->client;
+	if (client) {
+		rc = client->ops->event(client);
+	} else {
+		u8 status;
+
+		status = kcs_bmc_read_status(kcs_bmc);
+		if (status & KCS_BMC_STR_IBF) {
+			/* Ack the event by reading the data */
+			kcs_bmc_read_data(kcs_bmc);
+			rc = KCS_BMC_EVENT_HANDLED;
+		} else {
+			rc = KCS_BMC_EVENT_NONE;
+		}
+	}
+	spin_unlock(&kcs_bmc->lock);
+
+	return rc;
 }
 EXPORT_SYMBOL(kcs_bmc_handle_event);
 
-int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc *kcs_bmc);
-int kcs_bmc_add_device(struct kcs_bmc *kcs_bmc)
+int kcs_bmc_enable_device(struct kcs_bmc_device *kcs_bmc, struct kcs_bmc_client *client)
+{
+	int rc;
+
+	spin_lock_irq(&kcs_bmc->lock);
+	if (kcs_bmc->client) {
+		rc = -EBUSY;
+	} else {
+		kcs_bmc->client = client;
+		rc = 0;
+	}
+	spin_unlock_irq(&kcs_bmc->lock);
+
+	return rc;
+}
+EXPORT_SYMBOL(kcs_bmc_enable_device);
+
+void kcs_bmc_disable_device(struct kcs_bmc_device *kcs_bmc, struct kcs_bmc_client *client)
+{
+	spin_lock_irq(&kcs_bmc->lock);
+	if (client == kcs_bmc->client)
+		kcs_bmc->client = NULL;
+	spin_unlock_irq(&kcs_bmc->lock);
+}
+EXPORT_SYMBOL(kcs_bmc_disable_device);
+
+int kcs_bmc_add_device(struct kcs_bmc_device *kcs_bmc)
 {
 	return kcs_bmc_ipmi_attach_cdev(kcs_bmc);
 }
 EXPORT_SYMBOL(kcs_bmc_add_device);
 
-int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc *kcs_bmc);
-int kcs_bmc_remove_device(struct kcs_bmc *kcs_bmc)
+int kcs_bmc_remove_device(struct kcs_bmc_device *kcs_bmc)
 {
 	return kcs_bmc_ipmi_detach_cdev(kcs_bmc);
 }
diff --git a/drivers/char/ipmi/kcs_bmc.h b/drivers/char/ipmi/kcs_bmc.h
index a1350e567723..3f266740c759 100644
--- a/drivers/char/ipmi/kcs_bmc.h
+++ b/drivers/char/ipmi/kcs_bmc.h
@@ -6,9 +6,7 @@ 
 #ifndef __KCS_BMC_H__
 #define __KCS_BMC_H__
 
-#include <linux/miscdevice.h>
-
-#include "kcs_bmc_client.h"
+#include <linux/list.h>
 
 #define KCS_BMC_EVENT_NONE	0
 #define KCS_BMC_EVENT_HANDLED	1
@@ -17,53 +15,6 @@ 
 #define KCS_BMC_STR_IBF		BIT(1)
 #define KCS_BMC_STR_CMD_DAT	BIT(3)
 
-/* Different phases of the KCS BMC module.
- *  KCS_PHASE_IDLE:
- *            BMC should not be expecting nor sending any data.
- *  KCS_PHASE_WRITE_START:
- *            BMC is receiving a WRITE_START command from system software.
- *  KCS_PHASE_WRITE_DATA:
- *            BMC is receiving a data byte from system software.
- *  KCS_PHASE_WRITE_END_CMD:
- *            BMC is waiting a last data byte from system software.
- *  KCS_PHASE_WRITE_DONE:
- *            BMC has received the whole request from system software.
- *  KCS_PHASE_WAIT_READ:
- *            BMC is waiting the response from the upper IPMI service.
- *  KCS_PHASE_READ:
- *            BMC is transferring the response to system software.
- *  KCS_PHASE_ABORT_ERROR1:
- *            BMC is waiting error status request from system software.
- *  KCS_PHASE_ABORT_ERROR2:
- *            BMC is waiting for idle status afer error from system software.
- *  KCS_PHASE_ERROR:
- *            BMC has detected a protocol violation at the interface level.
- */
-enum kcs_phases {
-	KCS_PHASE_IDLE,
-
-	KCS_PHASE_WRITE_START,
-	KCS_PHASE_WRITE_DATA,
-	KCS_PHASE_WRITE_END_CMD,
-	KCS_PHASE_WRITE_DONE,
-
-	KCS_PHASE_WAIT_READ,
-	KCS_PHASE_READ,
-
-	KCS_PHASE_ABORT_ERROR1,
-	KCS_PHASE_ABORT_ERROR2,
-	KCS_PHASE_ERROR
-};
-
-/* IPMI 2.0 - Table 9-4, KCS Interface Status Codes */
-enum kcs_errors {
-	KCS_NO_ERROR                = 0x00,
-	KCS_ABORTED_BY_COMMAND      = 0x01,
-	KCS_ILLEGAL_CONTROL_CODE    = 0x02,
-	KCS_LENGTH_ERROR            = 0x06,
-	KCS_UNSPECIFIED_ERROR       = 0xFF
-};
-
 /* IPMI 2.0 - 9.5, KCS Interface Registers
  * @idr: Input Data Register
  * @odr: Output Data Register
@@ -76,36 +27,23 @@  struct kcs_ioreg {
 };
 
 struct kcs_bmc_device_ops;
+struct kcs_bmc_client;
+
+struct kcs_bmc_device {
+	struct list_head entry;
 
-struct kcs_bmc {
 	struct device *dev;
-
-	const struct kcs_bmc_device_ops *ops;
-
-	struct kcs_bmc_client client;
-
-	spinlock_t lock;
-
 	u32 channel;
-	int running;
 
 	struct kcs_ioreg ioreg;
 
-	enum kcs_phases phase;
-	enum kcs_errors error;
+	const struct kcs_bmc_device_ops *ops;
 
-	wait_queue_head_t queue;
-	bool data_in_avail;
-	int  data_in_idx;
-	u8  *data_in;
-
-	int  data_out_idx;
-	int  data_out_len;
-	u8  *data_out;
-
-	struct mutex mutex;
-	u8 *kbuffer;
-
-	struct miscdevice miscdev;
+	spinlock_t lock;
+	struct kcs_bmc_client *client;
 };
+
+/* Temporary exports while refactoring */
+int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc_device *kcs_bmc);
+int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc_device *kcs_bmc);
 #endif /* __KCS_BMC_H__ */
diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c b/drivers/char/ipmi/kcs_bmc_aspeed.c
index 1b313355b1c8..6f26e7366c0b 100644
--- a/drivers/char/ipmi/kcs_bmc_aspeed.c
+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
@@ -61,7 +61,7 @@ 
 #define LPC_STR4             0x11C
 
 struct aspeed_kcs_bmc {
-	struct kcs_bmc kcs_bmc;
+	struct kcs_bmc_device kcs_bmc;
 
 	struct regmap *map;
 };
@@ -71,12 +71,12 @@  struct aspeed_kcs_of_ops {
 	int (*get_io_address)(struct platform_device *pdev);
 };
 
-static inline struct aspeed_kcs_bmc *to_aspeed_kcs_bmc(struct kcs_bmc *kcs_bmc)
+static inline struct aspeed_kcs_bmc *to_aspeed_kcs_bmc(struct kcs_bmc_device *kcs_bmc)
 {
 	return container_of(kcs_bmc, struct aspeed_kcs_bmc, kcs_bmc);
 }
 
-static u8 aspeed_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
+static u8 aspeed_kcs_inb(struct kcs_bmc_device *kcs_bmc, u32 reg)
 {
 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
 	u32 val = 0;
@@ -88,7 +88,7 @@  static u8 aspeed_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
 	return rc == 0 ? (u8) val : 0;
 }
 
-static void aspeed_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
+static void aspeed_kcs_outb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 data)
 {
 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
 	int rc;
@@ -97,7 +97,7 @@  static void aspeed_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
 	WARN(rc != 0, "regmap_write() failed: %d\n", rc);
 }
 
-static void aspeed_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 val)
+static void aspeed_kcs_updateb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 mask, u8 val)
 {
 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
 	int rc;
@@ -119,7 +119,7 @@  static void aspeed_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 val
  *     C. KCS4
  *        D / C : CA4h / CA5h
  */
-static void aspeed_kcs_set_address(struct kcs_bmc *kcs_bmc, u16 addr)
+static void aspeed_kcs_set_address(struct kcs_bmc_device *kcs_bmc, u16 addr)
 {
 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
 
@@ -153,7 +153,7 @@  static void aspeed_kcs_set_address(struct kcs_bmc *kcs_bmc, u16 addr)
 	}
 }
 
-static void aspeed_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable)
+static void aspeed_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enable)
 {
 	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
 
@@ -228,7 +228,7 @@  static const struct kcs_bmc_device_ops aspeed_kcs_ops = {
 
 static irqreturn_t aspeed_kcs_irq(int irq, void *arg)
 {
-	struct kcs_bmc *kcs_bmc = arg;
+	struct kcs_bmc_device *kcs_bmc = arg;
 	int rc;
 
 	rc = kcs_bmc_handle_event(kcs_bmc);
@@ -238,7 +238,7 @@  static irqreturn_t aspeed_kcs_irq(int irq, void *arg)
 	return rc == KCS_BMC_EVENT_HANDLED ? IRQ_HANDLED : IRQ_NONE;
 }
 
-static int aspeed_kcs_config_irq(struct kcs_bmc *kcs_bmc,
+static int aspeed_kcs_config_irq(struct kcs_bmc_device *kcs_bmc,
 			struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -338,8 +338,8 @@  static int aspeed_kcs_of_v2_get_io_address(struct platform_device *pdev)
 static int aspeed_kcs_probe(struct platform_device *pdev)
 {
 	const struct aspeed_kcs_of_ops *ops;
+	struct kcs_bmc_device *kcs_bmc;
 	struct aspeed_kcs_bmc *priv;
-	struct kcs_bmc *kcs_bmc;
 	struct device_node *np;
 	int rc, channel, addr;
 
@@ -400,7 +400,7 @@  static int aspeed_kcs_probe(struct platform_device *pdev)
 static int aspeed_kcs_remove(struct platform_device *pdev)
 {
 	struct aspeed_kcs_bmc *priv = platform_get_drvdata(pdev);
-	struct kcs_bmc *kcs_bmc = &priv->kcs_bmc;
+	struct kcs_bmc_device *kcs_bmc = &priv->kcs_bmc;
 
 	kcs_bmc_remove_device(kcs_bmc);
 
diff --git a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
index fd852d8abe48..58c42e76483d 100644
--- a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
+++ b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
@@ -8,13 +8,88 @@ 
 #include <linux/errno.h>
 #include <linux/io.h>
 #include <linux/ipmi_bmc.h>
+#include <linux/list.h>
+#include <linux/miscdevice.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/poll.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
 
-#include "kcs_bmc.h"
+#include "kcs_bmc_client.h"
+
+/* Different phases of the KCS BMC module.
+ *  KCS_PHASE_IDLE:
+ *            BMC should not be expecting nor sending any data.
+ *  KCS_PHASE_WRITE_START:
+ *            BMC is receiving a WRITE_START command from system software.
+ *  KCS_PHASE_WRITE_DATA:
+ *            BMC is receiving a data byte from system software.
+ *  KCS_PHASE_WRITE_END_CMD:
+ *            BMC is waiting a last data byte from system software.
+ *  KCS_PHASE_WRITE_DONE:
+ *            BMC has received the whole request from system software.
+ *  KCS_PHASE_WAIT_READ:
+ *            BMC is waiting the response from the upper IPMI service.
+ *  KCS_PHASE_READ:
+ *            BMC is transferring the response to system software.
+ *  KCS_PHASE_ABORT_ERROR1:
+ *            BMC is waiting error status request from system software.
+ *  KCS_PHASE_ABORT_ERROR2:
+ *            BMC is waiting for idle status afer error from system software.
+ *  KCS_PHASE_ERROR:
+ *            BMC has detected a protocol violation at the interface level.
+ */
+enum kcs_ipmi_phases {
+	KCS_PHASE_IDLE,
+
+	KCS_PHASE_WRITE_START,
+	KCS_PHASE_WRITE_DATA,
+	KCS_PHASE_WRITE_END_CMD,
+	KCS_PHASE_WRITE_DONE,
+
+	KCS_PHASE_WAIT_READ,
+	KCS_PHASE_READ,
+
+	KCS_PHASE_ABORT_ERROR1,
+	KCS_PHASE_ABORT_ERROR2,
+	KCS_PHASE_ERROR
+};
+
+/* IPMI 2.0 - Table 9-4, KCS Interface Status Codes */
+enum kcs_ipmi_errors {
+	KCS_NO_ERROR                = 0x00,
+	KCS_ABORTED_BY_COMMAND      = 0x01,
+	KCS_ILLEGAL_CONTROL_CODE    = 0x02,
+	KCS_LENGTH_ERROR            = 0x06,
+	KCS_UNSPECIFIED_ERROR       = 0xFF
+};
+
+struct kcs_bmc_ipmi {
+	struct list_head entry;
+
+	struct kcs_bmc_client client;
+
+	spinlock_t lock;
+
+	enum kcs_ipmi_phases phase;
+	enum kcs_ipmi_errors error;
+
+	wait_queue_head_t queue;
+	bool data_in_avail;
+	int  data_in_idx;
+	u8  *data_in;
+
+	int  data_out_idx;
+	int  data_out_len;
+	u8  *data_out;
+
+	struct mutex mutex;
+	u8 *kbuffer;
+
+	struct miscdevice miscdev;
+};
 
 #define DEVICE_NAME "ipmi-kcs"
 
@@ -44,171 +119,169 @@  enum kcs_states {
 #define KCS_CMD_WRITE_END         0x62
 #define KCS_CMD_READ_BYTE         0x68
 
-static inline void set_state(struct kcs_bmc *kcs_bmc, u8 state)
+static inline void set_state(struct kcs_bmc_ipmi *priv, u8 state)
 {
-	kcs_bmc_update_status(kcs_bmc, KCS_STATUS_STATE_MASK,
-					KCS_STATUS_STATE(state));
+	kcs_bmc_update_status(priv->client.dev, KCS_STATUS_STATE_MASK, KCS_STATUS_STATE(state));
 }
 
-static void kcs_bmc_ipmi_force_abort(struct kcs_bmc *kcs_bmc)
+static void kcs_bmc_ipmi_force_abort(struct kcs_bmc_ipmi *priv)
 {
-	set_state(kcs_bmc, ERROR_STATE);
-	kcs_bmc_read_data(kcs_bmc);
-	kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
+	set_state(priv, ERROR_STATE);
+	kcs_bmc_read_data(priv->client.dev);
+	kcs_bmc_write_data(priv->client.dev, KCS_ZERO_DATA);
 
-	kcs_bmc->phase = KCS_PHASE_ERROR;
-	kcs_bmc->data_in_avail = false;
-	kcs_bmc->data_in_idx = 0;
+	priv->phase = KCS_PHASE_ERROR;
+	priv->data_in_avail = false;
+	priv->data_in_idx = 0;
 }
 
-static void kcs_bmc_ipmi_handle_data(struct kcs_bmc *kcs_bmc)
+static void kcs_bmc_ipmi_handle_data(struct kcs_bmc_ipmi *priv)
 {
+	struct kcs_bmc_device *dev;
 	u8 data;
 
-	switch (kcs_bmc->phase) {
+	dev = priv->client.dev;
+
+	switch (priv->phase) {
 	case KCS_PHASE_WRITE_START:
-		kcs_bmc->phase = KCS_PHASE_WRITE_DATA;
+		priv->phase = KCS_PHASE_WRITE_DATA;
 		fallthrough;
 
 	case KCS_PHASE_WRITE_DATA:
-		if (kcs_bmc->data_in_idx < KCS_MSG_BUFSIZ) {
-			set_state(kcs_bmc, WRITE_STATE);
-			kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
-			kcs_bmc->data_in[kcs_bmc->data_in_idx++] =
-						kcs_bmc_read_data(kcs_bmc);
+		if (priv->data_in_idx < KCS_MSG_BUFSIZ) {
+			set_state(priv, WRITE_STATE);
+			kcs_bmc_write_data(dev, KCS_ZERO_DATA);
+			priv->data_in[priv->data_in_idx++] = kcs_bmc_read_data(dev);
 		} else {
-			kcs_bmc_ipmi_force_abort(kcs_bmc);
-			kcs_bmc->error = KCS_LENGTH_ERROR;
+			kcs_bmc_ipmi_force_abort(priv);
+			priv->error = KCS_LENGTH_ERROR;
 		}
 		break;
 
 	case KCS_PHASE_WRITE_END_CMD:
-		if (kcs_bmc->data_in_idx < KCS_MSG_BUFSIZ) {
-			set_state(kcs_bmc, READ_STATE);
-			kcs_bmc->data_in[kcs_bmc->data_in_idx++] =
-						kcs_bmc_read_data(kcs_bmc);
-			kcs_bmc->phase = KCS_PHASE_WRITE_DONE;
-			kcs_bmc->data_in_avail = true;
-			wake_up_interruptible(&kcs_bmc->queue);
+		if (priv->data_in_idx < KCS_MSG_BUFSIZ) {
+			set_state(priv, READ_STATE);
+			priv->data_in[priv->data_in_idx++] = kcs_bmc_read_data(dev);
+			priv->phase = KCS_PHASE_WRITE_DONE;
+			priv->data_in_avail = true;
+			wake_up_interruptible(&priv->queue);
 		} else {
-			kcs_bmc_ipmi_force_abort(kcs_bmc);
-			kcs_bmc->error = KCS_LENGTH_ERROR;
+			kcs_bmc_ipmi_force_abort(priv);
+			priv->error = KCS_LENGTH_ERROR;
 		}
 		break;
 
 	case KCS_PHASE_READ:
-		if (kcs_bmc->data_out_idx == kcs_bmc->data_out_len)
-			set_state(kcs_bmc, IDLE_STATE);
+		if (priv->data_out_idx == priv->data_out_len)
+			set_state(priv, IDLE_STATE);
 
-		data = kcs_bmc_read_data(kcs_bmc);
+		data = kcs_bmc_read_data(dev);
 		if (data != KCS_CMD_READ_BYTE) {
-			set_state(kcs_bmc, ERROR_STATE);
-			kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
+			set_state(priv, ERROR_STATE);
+			kcs_bmc_write_data(dev, KCS_ZERO_DATA);
 			break;
 		}
 
-		if (kcs_bmc->data_out_idx == kcs_bmc->data_out_len) {
-			kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
-			kcs_bmc->phase = KCS_PHASE_IDLE;
+		if (priv->data_out_idx == priv->data_out_len) {
+			kcs_bmc_write_data(dev, KCS_ZERO_DATA);
+			priv->phase = KCS_PHASE_IDLE;
 			break;
 		}
 
-		kcs_bmc_write_data(kcs_bmc,
-			kcs_bmc->data_out[kcs_bmc->data_out_idx++]);
+		kcs_bmc_write_data(dev, priv->data_out[priv->data_out_idx++]);
 		break;
 
 	case KCS_PHASE_ABORT_ERROR1:
-		set_state(kcs_bmc, READ_STATE);
-		kcs_bmc_read_data(kcs_bmc);
-		kcs_bmc_write_data(kcs_bmc, kcs_bmc->error);
-		kcs_bmc->phase = KCS_PHASE_ABORT_ERROR2;
+		set_state(priv, READ_STATE);
+		kcs_bmc_read_data(dev);
+		kcs_bmc_write_data(dev, priv->error);
+		priv->phase = KCS_PHASE_ABORT_ERROR2;
 		break;
 
 	case KCS_PHASE_ABORT_ERROR2:
-		set_state(kcs_bmc, IDLE_STATE);
-		kcs_bmc_read_data(kcs_bmc);
-		kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
-		kcs_bmc->phase = KCS_PHASE_IDLE;
+		set_state(priv, IDLE_STATE);
+		kcs_bmc_read_data(dev);
+		kcs_bmc_write_data(dev, KCS_ZERO_DATA);
+		priv->phase = KCS_PHASE_IDLE;
 		break;
 
 	default:
-		kcs_bmc_ipmi_force_abort(kcs_bmc);
+		kcs_bmc_ipmi_force_abort(priv);
 		break;
 	}
 }
 
-static void kcs_bmc_ipmi_handle_cmd(struct kcs_bmc *kcs_bmc)
+static void kcs_bmc_ipmi_handle_cmd(struct kcs_bmc_ipmi *priv)
 {
 	u8 cmd;
 
-	set_state(kcs_bmc, WRITE_STATE);
-	kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
+	set_state(priv, WRITE_STATE);
+	kcs_bmc_write_data(priv->client.dev, KCS_ZERO_DATA);
 
-	cmd = kcs_bmc_read_data(kcs_bmc);
+	cmd = kcs_bmc_read_data(priv->client.dev);
 	switch (cmd) {
 	case KCS_CMD_WRITE_START:
-		kcs_bmc->phase = KCS_PHASE_WRITE_START;
-		kcs_bmc->error = KCS_NO_ERROR;
-		kcs_bmc->data_in_avail = false;
-		kcs_bmc->data_in_idx = 0;
+		priv->phase = KCS_PHASE_WRITE_START;
+		priv->error = KCS_NO_ERROR;
+		priv->data_in_avail = false;
+		priv->data_in_idx = 0;
 		break;
 
 	case KCS_CMD_WRITE_END:
-		if (kcs_bmc->phase != KCS_PHASE_WRITE_DATA) {
-			kcs_bmc_ipmi_force_abort(kcs_bmc);
+		if (priv->phase != KCS_PHASE_WRITE_DATA) {
+			kcs_bmc_ipmi_force_abort(priv);
 			break;
 		}
 
-		kcs_bmc->phase = KCS_PHASE_WRITE_END_CMD;
+		priv->phase = KCS_PHASE_WRITE_END_CMD;
 		break;
 
 	case KCS_CMD_GET_STATUS_ABORT:
-		if (kcs_bmc->error == KCS_NO_ERROR)
-			kcs_bmc->error = KCS_ABORTED_BY_COMMAND;
+		if (priv->error == KCS_NO_ERROR)
+			priv->error = KCS_ABORTED_BY_COMMAND;
 
-		kcs_bmc->phase = KCS_PHASE_ABORT_ERROR1;
-		kcs_bmc->data_in_avail = false;
-		kcs_bmc->data_in_idx = 0;
+		priv->phase = KCS_PHASE_ABORT_ERROR1;
+		priv->data_in_avail = false;
+		priv->data_in_idx = 0;
 		break;
 
 	default:
-		kcs_bmc_ipmi_force_abort(kcs_bmc);
-		kcs_bmc->error = KCS_ILLEGAL_CONTROL_CODE;
+		kcs_bmc_ipmi_force_abort(priv);
+		priv->error = KCS_ILLEGAL_CONTROL_CODE;
 		break;
 	}
 }
 
-static inline struct kcs_bmc *client_to_kcs_bmc(struct kcs_bmc_client *client)
+static inline struct kcs_bmc_ipmi *client_to_kcs_bmc_ipmi(struct kcs_bmc_client *client)
 {
-	return container_of(client, struct kcs_bmc, client);
+	return container_of(client, struct kcs_bmc_ipmi, client);
 }
 
 static int kcs_bmc_ipmi_event(struct kcs_bmc_client *client)
 {
-	struct kcs_bmc *kcs_bmc;
-	unsigned long flags;
+	struct kcs_bmc_ipmi *priv;
 	u8 status;
 	int ret;
 
-	kcs_bmc = client_to_kcs_bmc(client);
+	priv = client_to_kcs_bmc_ipmi(client);
+	if (!priv)
+		return KCS_BMC_EVENT_NONE;
 
-	spin_lock_irqsave(&kcs_bmc->lock, flags);
+	spin_lock(&priv->lock);
 
-	status = kcs_bmc_read_status(kcs_bmc);
+	status = kcs_bmc_read_status(client->dev);
 	if (status & KCS_STATUS_IBF) {
-		if (!kcs_bmc->running)
-			kcs_bmc_ipmi_force_abort(kcs_bmc);
-		else if (status & KCS_STATUS_CMD_DAT)
-			kcs_bmc_ipmi_handle_cmd(kcs_bmc);
+		if (status & KCS_STATUS_CMD_DAT)
+			kcs_bmc_ipmi_handle_cmd(priv);
 		else
-			kcs_bmc_ipmi_handle_data(kcs_bmc);
+			kcs_bmc_ipmi_handle_data(priv);
 
 		ret = KCS_BMC_EVENT_HANDLED;
 	} else {
 		ret = KCS_BMC_EVENT_NONE;
 	}
 
-	spin_unlock_irqrestore(&kcs_bmc->lock, flags);
+	spin_unlock(&priv->lock);
 
 	return ret;
 }
@@ -217,37 +290,29 @@  static const struct kcs_bmc_client_ops kcs_bmc_ipmi_client_ops = {
 	.event = kcs_bmc_ipmi_event,
 };
 
-static inline struct kcs_bmc *file_to_kcs_bmc(struct file *filp)
+static inline struct kcs_bmc_ipmi *to_kcs_bmc(struct file *filp)
 {
-	return container_of(filp->private_data, struct kcs_bmc, miscdev);
+	return container_of(filp->private_data, struct kcs_bmc_ipmi, miscdev);
 }
 
 static int kcs_bmc_ipmi_open(struct inode *inode, struct file *filp)
 {
-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
-	int ret = 0;
+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
 
-	spin_lock_irq(&kcs_bmc->lock);
-	if (!kcs_bmc->running)
-		kcs_bmc->running = 1;
-	else
-		ret = -EBUSY;
-	spin_unlock_irq(&kcs_bmc->lock);
-
-	return ret;
+	return kcs_bmc_enable_device(priv->client.dev, &priv->client);
 }
 
 static __poll_t kcs_bmc_ipmi_poll(struct file *filp, poll_table *wait)
 {
-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
 	__poll_t mask = 0;
 
-	poll_wait(filp, &kcs_bmc->queue, wait);
+	poll_wait(filp, &priv->queue, wait);
 
-	spin_lock_irq(&kcs_bmc->lock);
-	if (kcs_bmc->data_in_avail)
+	spin_lock_irq(&priv->lock);
+	if (priv->data_in_avail)
 		mask |= EPOLLIN;
-	spin_unlock_irq(&kcs_bmc->lock);
+	spin_unlock_irq(&priv->lock);
 
 	return mask;
 }
@@ -255,24 +320,24 @@  static __poll_t kcs_bmc_ipmi_poll(struct file *filp, poll_table *wait)
 static ssize_t kcs_bmc_ipmi_read(struct file *filp, char __user *buf,
 			    size_t count, loff_t *ppos)
 {
-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
 	bool data_avail;
 	size_t data_len;
 	ssize_t ret;
 
 	if (!(filp->f_flags & O_NONBLOCK))
-		wait_event_interruptible(kcs_bmc->queue,
-					 kcs_bmc->data_in_avail);
+		wait_event_interruptible(priv->queue,
+					 priv->data_in_avail);
 
-	mutex_lock(&kcs_bmc->mutex);
+	mutex_lock(&priv->mutex);
 
-	spin_lock_irq(&kcs_bmc->lock);
-	data_avail = kcs_bmc->data_in_avail;
+	spin_lock_irq(&priv->lock);
+	data_avail = priv->data_in_avail;
 	if (data_avail) {
-		data_len = kcs_bmc->data_in_idx;
-		memcpy(kcs_bmc->kbuffer, kcs_bmc->data_in, data_len);
+		data_len = priv->data_in_idx;
+		memcpy(priv->kbuffer, priv->data_in, data_len);
 	}
-	spin_unlock_irq(&kcs_bmc->lock);
+	spin_unlock_irq(&priv->lock);
 
 	if (!data_avail) {
 		ret = -EAGAIN;
@@ -281,35 +346,35 @@  static ssize_t kcs_bmc_ipmi_read(struct file *filp, char __user *buf,
 
 	if (count < data_len) {
 		pr_err("channel=%u with too large data : %zu\n",
-			kcs_bmc->channel, data_len);
+			priv->client.dev->channel, data_len);
 
-		spin_lock_irq(&kcs_bmc->lock);
-		kcs_bmc_ipmi_force_abort(kcs_bmc);
-		spin_unlock_irq(&kcs_bmc->lock);
+		spin_lock_irq(&priv->lock);
+		kcs_bmc_ipmi_force_abort(priv);
+		spin_unlock_irq(&priv->lock);
 
 		ret = -EOVERFLOW;
 		goto out_unlock;
 	}
 
-	if (copy_to_user(buf, kcs_bmc->kbuffer, data_len)) {
+	if (copy_to_user(buf, priv->kbuffer, data_len)) {
 		ret = -EFAULT;
 		goto out_unlock;
 	}
 
 	ret = data_len;
 
-	spin_lock_irq(&kcs_bmc->lock);
-	if (kcs_bmc->phase == KCS_PHASE_WRITE_DONE) {
-		kcs_bmc->phase = KCS_PHASE_WAIT_READ;
-		kcs_bmc->data_in_avail = false;
-		kcs_bmc->data_in_idx = 0;
+	spin_lock_irq(&priv->lock);
+	if (priv->phase == KCS_PHASE_WRITE_DONE) {
+		priv->phase = KCS_PHASE_WAIT_READ;
+		priv->data_in_avail = false;
+		priv->data_in_idx = 0;
 	} else {
 		ret = -EAGAIN;
 	}
-	spin_unlock_irq(&kcs_bmc->lock);
+	spin_unlock_irq(&priv->lock);
 
 out_unlock:
-	mutex_unlock(&kcs_bmc->mutex);
+	mutex_unlock(&priv->mutex);
 
 	return ret;
 }
@@ -317,35 +382,35 @@  static ssize_t kcs_bmc_ipmi_read(struct file *filp, char __user *buf,
 static ssize_t kcs_bmc_ipmi_write(struct file *filp, const char __user *buf,
 			     size_t count, loff_t *ppos)
 {
-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
 	ssize_t ret;
 
 	/* a minimum response size '3' : netfn + cmd + ccode */
 	if (count < 3 || count > KCS_MSG_BUFSIZ)
 		return -EINVAL;
 
-	mutex_lock(&kcs_bmc->mutex);
+	mutex_lock(&priv->mutex);
 
-	if (copy_from_user(kcs_bmc->kbuffer, buf, count)) {
+	if (copy_from_user(priv->kbuffer, buf, count)) {
 		ret = -EFAULT;
 		goto out_unlock;
 	}
 
-	spin_lock_irq(&kcs_bmc->lock);
-	if (kcs_bmc->phase == KCS_PHASE_WAIT_READ) {
-		kcs_bmc->phase = KCS_PHASE_READ;
-		kcs_bmc->data_out_idx = 1;
-		kcs_bmc->data_out_len = count;
-		memcpy(kcs_bmc->data_out, kcs_bmc->kbuffer, count);
-		kcs_bmc_write_data(kcs_bmc, kcs_bmc->data_out[0]);
+	spin_lock_irq(&priv->lock);
+	if (priv->phase == KCS_PHASE_WAIT_READ) {
+		priv->phase = KCS_PHASE_READ;
+		priv->data_out_idx = 1;
+		priv->data_out_len = count;
+		memcpy(priv->data_out, priv->kbuffer, count);
+		kcs_bmc_write_data(priv->client.dev, priv->data_out[0]);
 		ret = count;
 	} else {
 		ret = -EINVAL;
 	}
-	spin_unlock_irq(&kcs_bmc->lock);
+	spin_unlock_irq(&priv->lock);
 
 out_unlock:
-	mutex_unlock(&kcs_bmc->mutex);
+	mutex_unlock(&priv->mutex);
 
 	return ret;
 }
@@ -353,22 +418,22 @@  static ssize_t kcs_bmc_ipmi_write(struct file *filp, const char __user *buf,
 static long kcs_bmc_ipmi_ioctl(struct file *filp, unsigned int cmd,
 			  unsigned long arg)
 {
-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
 	long ret = 0;
 
-	spin_lock_irq(&kcs_bmc->lock);
+	spin_lock_irq(&priv->lock);
 
 	switch (cmd) {
 	case IPMI_BMC_IOCTL_SET_SMS_ATN:
-		kcs_bmc_update_status(kcs_bmc, KCS_STATUS_SMS_ATN, KCS_STATUS_SMS_ATN);
+		kcs_bmc_update_status(priv->client.dev, KCS_STATUS_SMS_ATN, KCS_STATUS_SMS_ATN);
 		break;
 
 	case IPMI_BMC_IOCTL_CLEAR_SMS_ATN:
-		kcs_bmc_update_status(kcs_bmc, KCS_STATUS_SMS_ATN, 0);
+		kcs_bmc_update_status(priv->client.dev, KCS_STATUS_SMS_ATN, 0);
 		break;
 
 	case IPMI_BMC_IOCTL_FORCE_ABORT:
-		kcs_bmc_ipmi_force_abort(kcs_bmc);
+		kcs_bmc_ipmi_force_abort(priv);
 		break;
 
 	default:
@@ -376,19 +441,17 @@  static long kcs_bmc_ipmi_ioctl(struct file *filp, unsigned int cmd,
 		break;
 	}
 
-	spin_unlock_irq(&kcs_bmc->lock);
+	spin_unlock_irq(&priv->lock);
 
 	return ret;
 }
 
 static int kcs_bmc_ipmi_release(struct inode *inode, struct file *filp)
 {
-	struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
+	struct kcs_bmc_ipmi *priv = to_kcs_bmc(filp);
 
-	spin_lock_irq(&kcs_bmc->lock);
-	kcs_bmc->running = 0;
-	kcs_bmc_ipmi_force_abort(kcs_bmc);
-	spin_unlock_irq(&kcs_bmc->lock);
+	kcs_bmc_ipmi_force_abort(priv);
+	kcs_bmc_disable_device(priv->client.dev, &priv->client);
 
 	return 0;
 }
@@ -403,56 +466,78 @@  static const struct file_operations kcs_bmc_ipmi_fops = {
 	.unlocked_ioctl = kcs_bmc_ipmi_ioctl,
 };
 
-int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc *kcs_bmc);
-int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc *kcs_bmc)
+static DEFINE_SPINLOCK(kcs_bmc_ipmi_instances_lock);
+static LIST_HEAD(kcs_bmc_ipmi_instances);
+
+int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc_device *kcs_bmc);
+int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc_device *kcs_bmc)
 {
+	struct kcs_bmc_ipmi *priv;
 	int rc;
 
-	spin_lock_init(&kcs_bmc->lock);
-	mutex_init(&kcs_bmc->mutex);
-	init_waitqueue_head(&kcs_bmc->queue);
-
-	kcs_bmc->client.dev = kcs_bmc;
-	kcs_bmc->client.ops = &kcs_bmc_ipmi_client_ops;
-	kcs_bmc->data_in = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
-	kcs_bmc->data_out = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
-	kcs_bmc->kbuffer = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
-
-	kcs_bmc->miscdev.minor = MISC_DYNAMIC_MINOR;
-	kcs_bmc->miscdev.name = devm_kasprintf(kcs_bmc->dev, GFP_KERNEL, "%s%u",
-					       DEVICE_NAME, kcs_bmc->channel);
-	if (!kcs_bmc->data_in || !kcs_bmc->data_out || !kcs_bmc->kbuffer ||
-	    !kcs_bmc->miscdev.name)
+	priv = devm_kzalloc(kcs_bmc->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
 		return -ENOMEM;
 
-	kcs_bmc->miscdev.fops = &kcs_bmc_ipmi_fops;
+	spin_lock_init(&priv->lock);
+	mutex_init(&priv->mutex);
 
-	rc = misc_register(&kcs_bmc->miscdev);
+	init_waitqueue_head(&priv->queue);
+
+	priv->client.dev = kcs_bmc;
+	priv->client.ops = &kcs_bmc_ipmi_client_ops;
+	priv->data_in = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
+	priv->data_out = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
+	priv->kbuffer = devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
+
+	priv->miscdev.minor = MISC_DYNAMIC_MINOR;
+	priv->miscdev.name = devm_kasprintf(kcs_bmc->dev, GFP_KERNEL, "%s%u", DEVICE_NAME,
+					   kcs_bmc->channel);
+	if (!priv->data_in || !priv->data_out || !priv->kbuffer || !priv->miscdev.name)
+		return -EINVAL;
+
+	priv->miscdev.fops = &kcs_bmc_ipmi_fops;
+
+	rc = misc_register(&priv->miscdev);
 	if (rc) {
 		dev_err(kcs_bmc->dev, "Unable to register device: %d\n", rc);
 		return rc;
 	}
 
+	spin_lock_irq(&kcs_bmc_ipmi_instances_lock);
+	list_add(&priv->entry, &kcs_bmc_ipmi_instances);
+	spin_unlock_irq(&kcs_bmc_ipmi_instances_lock);
+
 	dev_info(kcs_bmc->dev, "Initialised IPMI client for channel %d", kcs_bmc->channel);
 
 	return 0;
 }
 EXPORT_SYMBOL(kcs_bmc_ipmi_attach_cdev);
 
-int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc *kcs_bmc);
-int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc *kcs_bmc)
+int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc_device *kcs_bmc);
+int kcs_bmc_ipmi_detach_cdev(struct kcs_bmc_device *kcs_bmc)
 {
-	misc_deregister(&kcs_bmc->miscdev);
+	struct kcs_bmc_ipmi *priv = NULL, *pos;
 
-	spin_lock_irq(&kcs_bmc->lock);
-	kcs_bmc->running = 0;
-	kcs_bmc_ipmi_force_abort(kcs_bmc);
-	spin_unlock_irq(&kcs_bmc->lock);
+	spin_lock_irq(&kcs_bmc_ipmi_instances_lock);
+	list_for_each_entry(pos, &kcs_bmc_ipmi_instances, entry) {
+		if (pos->client.dev == kcs_bmc) {
+			priv = pos;
+			list_del(&pos->entry);
+			break;
+		}
+	}
+	spin_unlock_irq(&kcs_bmc_ipmi_instances_lock);
 
-	devm_kfree(kcs_bmc->dev, kcs_bmc->kbuffer);
-	devm_kfree(kcs_bmc->dev, kcs_bmc->data_out);
-	devm_kfree(kcs_bmc->dev, kcs_bmc->data_in);
-	devm_kfree(kcs_bmc->dev, kcs_bmc);
+	if (!priv)
+		return 0;
+
+	misc_deregister(&priv->miscdev);
+	kcs_bmc_disable_device(priv->client.dev, &priv->client);
+	devm_kfree(kcs_bmc->dev, priv->kbuffer);
+	devm_kfree(kcs_bmc->dev, priv->data_out);
+	devm_kfree(kcs_bmc->dev, priv->data_in);
+	devm_kfree(kcs_bmc->dev, priv);
 
 	return 0;
 }
@@ -460,4 +545,5 @@  EXPORT_SYMBOL(kcs_bmc_ipmi_detach_cdev);
 
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Haiyue Wang <haiyue.wang@linux.intel.com>");
+MODULE_AUTHOR("Andrew Jeffery <andrew@aj.id.au>");
 MODULE_DESCRIPTION("KCS BMC to handle the IPMI request from system software");
diff --git a/drivers/char/ipmi/kcs_bmc_client.h b/drivers/char/ipmi/kcs_bmc_client.h
index 140631d157d8..2dd710f4b4aa 100644
--- a/drivers/char/ipmi/kcs_bmc_client.h
+++ b/drivers/char/ipmi/kcs_bmc_client.h
@@ -8,22 +8,24 @@ 
 #include <linux/notifier.h>
 #include <stdbool.h>
 
-struct kcs_bmc;
-struct kcs_bmc_client_ops;
-
-struct kcs_bmc_client {
-	const struct kcs_bmc_client_ops *ops;
-
-	struct kcs_bmc *dev;
-};
+#include "kcs_bmc.h"
 
 struct kcs_bmc_client_ops {
 	int (*event)(struct kcs_bmc_client *client);
 };
 
-u8 kcs_bmc_read_data(struct kcs_bmc *kcs_bmc);
-void kcs_bmc_write_data(struct kcs_bmc *kcs_bmc, u8 data);
-u8 kcs_bmc_read_status(struct kcs_bmc *kcs_bmc);
-void kcs_bmc_write_status(struct kcs_bmc *kcs_bmc, u8 data);
-void kcs_bmc_update_status(struct kcs_bmc *kcs_bmc, u8 mask, u8 val);
+struct kcs_bmc_client {
+	const struct kcs_bmc_client_ops *ops;
+
+	struct kcs_bmc_device *dev;
+};
+
+int kcs_bmc_enable_device(struct kcs_bmc_device *kcs_bmc, struct kcs_bmc_client *client);
+void kcs_bmc_disable_device(struct kcs_bmc_device *kcs_bmc, struct kcs_bmc_client *client);
+
+u8 kcs_bmc_read_data(struct kcs_bmc_device *kcs_bmc);
+void kcs_bmc_write_data(struct kcs_bmc_device *kcs_bmc, u8 data);
+u8 kcs_bmc_read_status(struct kcs_bmc_device *kcs_bmc);
+void kcs_bmc_write_status(struct kcs_bmc_device *kcs_bmc, u8 data);
+void kcs_bmc_update_status(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 val);
 #endif
diff --git a/drivers/char/ipmi/kcs_bmc_device.h b/drivers/char/ipmi/kcs_bmc_device.h
index 33462174516d..57b7174b2bac 100644
--- a/drivers/char/ipmi/kcs_bmc_device.h
+++ b/drivers/char/ipmi/kcs_bmc_device.h
@@ -7,13 +7,13 @@ 
 #include "kcs_bmc.h"
 
 struct kcs_bmc_device_ops {
-	u8 (*io_inputb)(struct kcs_bmc *kcs_bmc, u32 reg);
-	void (*io_outputb)(struct kcs_bmc *kcs_bmc, u32 reg, u8 b);
-	void (*io_updateb)(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 b);
+	u8 (*io_inputb)(struct kcs_bmc_device *kcs_bmc, u32 reg);
+	void (*io_outputb)(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 b);
+	void (*io_updateb)(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 mask, u8 b);
 };
 
-int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc);
-int kcs_bmc_add_device(struct kcs_bmc *kcs_bmc);
-int kcs_bmc_remove_device(struct kcs_bmc *kcs_bmc);
+int kcs_bmc_handle_event(struct kcs_bmc_device *kcs_bmc);
+int kcs_bmc_add_device(struct kcs_bmc_device *kcs_bmc);
+int kcs_bmc_remove_device(struct kcs_bmc_device *kcs_bmc);
 
 #endif
diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
index 1d21697fc585..dce93ec895fc 100644
--- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
+++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
@@ -65,7 +65,7 @@  struct npcm7xx_kcs_reg {
 };
 
 struct npcm7xx_kcs_bmc {
-	struct kcs_bmc kcs_bmc;
+	struct kcs_bmc_device kcs_bmc;
 
 	struct regmap *map;
 
@@ -78,12 +78,12 @@  static const struct npcm7xx_kcs_reg npcm7xx_kcs_reg_tbl[KCS_CHANNEL_MAX] = {
 	{ .sts = KCS3ST, .dob = KCS3DO, .dib = KCS3DI, .ctl = KCS3CTL, .ie = KCS3IE },
 };
 
-static inline struct npcm7xx_kcs_bmc *to_npcm7xx_kcs_bmc(struct kcs_bmc *kcs_bmc)
+static inline struct npcm7xx_kcs_bmc *to_npcm7xx_kcs_bmc(struct kcs_bmc_device *kcs_bmc)
 {
 	return container_of(kcs_bmc, struct npcm7xx_kcs_bmc, kcs_bmc);
 }
 
-static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
+static u8 npcm7xx_kcs_inb(struct kcs_bmc_device *kcs_bmc, u32 reg)
 {
 	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
 	u32 val = 0;
@@ -95,7 +95,7 @@  static u8 npcm7xx_kcs_inb(struct kcs_bmc *kcs_bmc, u32 reg)
 	return rc == 0 ? (u8)val : 0;
 }
 
-static void npcm7xx_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
+static void npcm7xx_kcs_outb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 data)
 {
 	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
 	int rc;
@@ -104,7 +104,7 @@  static void npcm7xx_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
 	WARN(rc != 0, "regmap_write() failed: %d\n", rc);
 }
 
-static void npcm7xx_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 data)
+static void npcm7xx_kcs_updateb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 mask, u8 data)
 {
 	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
 	int rc;
@@ -113,7 +113,7 @@  static void npcm7xx_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 da
 	WARN(rc != 0, "regmap_update_bits() failed: %d\n", rc);
 }
 
-static void npcm7xx_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable)
+static void npcm7xx_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enable)
 {
 	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
 
@@ -126,7 +126,7 @@  static void npcm7xx_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable)
 
 static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)
 {
-	struct kcs_bmc *kcs_bmc = arg;
+	struct kcs_bmc_device *kcs_bmc = arg;
 	int rc;
 
 	rc = kcs_bmc_handle_event(kcs_bmc);
@@ -136,7 +136,7 @@  static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)
 	return rc == KCS_BMC_EVENT_HANDLED ? IRQ_HANDLED : IRQ_NONE;
 }
 
-static int npcm7xx_kcs_config_irq(struct kcs_bmc *kcs_bmc,
+static int npcm7xx_kcs_config_irq(struct kcs_bmc_device *kcs_bmc,
 				  struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -160,7 +160,7 @@  static int npcm7xx_kcs_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct npcm7xx_kcs_bmc *priv;
-	struct kcs_bmc *kcs_bmc;
+	struct kcs_bmc_device *kcs_bmc;
 	u32 chan;
 	int rc;
 
@@ -207,7 +207,7 @@  static int npcm7xx_kcs_probe(struct platform_device *pdev)
 static int npcm7xx_kcs_remove(struct platform_device *pdev)
 {
 	struct npcm7xx_kcs_bmc *priv = platform_get_drvdata(pdev);
-	struct kcs_bmc *kcs_bmc = &priv->kcs_bmc;
+	struct kcs_bmc_device *kcs_bmc = &priv->kcs_bmc;
 
 	kcs_bmc_remove_device(kcs_bmc);