diff mbox series

[v2,14/21] ipmi: kcs_bmc: Allow clients to control KCS IRQ state

Message ID 20210319062752.145730-14-andrew@aj.id.au
State Superseded
Headers show
Series [v2,01/21] dt-bindings: aspeed-lpc: Remove LPC partitioning | expand

Commit Message

Andrew Jeffery March 19, 2021, 6:27 a.m. UTC
Add a mechanism for controlling whether the client associated with a
KCS device will receive Input Buffer Full (IBF) and Output Buffer Empty
(OBE) events. This enables an abstract implementation of poll() for KCS
devices.

A wart in the implementation is that the ASPEED KCS devices don't
support an OBE interrupt for the BMC. Instead we pretend it has one by
polling the status register waiting for the Output Buffer Full (OBF) bit
to clear, and generating an event when OBE is observed.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 drivers/char/ipmi/kcs_bmc.c         |   6 ++
 drivers/char/ipmi/kcs_bmc.h         |   3 +
 drivers/char/ipmi/kcs_bmc_aspeed.c  | 150 ++++++++++++++++++----------
 drivers/char/ipmi/kcs_bmc_client.h  |   2 +
 drivers/char/ipmi/kcs_bmc_device.h  |   1 +
 drivers/char/ipmi/kcs_bmc_npcm7xx.c |  25 ++++-
 6 files changed, 130 insertions(+), 57 deletions(-)

Comments

Zev Weiss April 9, 2021, 4:37 a.m. UTC | #1
On Fri, Mar 19, 2021 at 01:27:45AM CDT, Andrew Jeffery wrote:
>Add a mechanism for controlling whether the client associated with a
>KCS device will receive Input Buffer Full (IBF) and Output Buffer Empty
>(OBE) events. This enables an abstract implementation of poll() for KCS
>devices.
>
>A wart in the implementation is that the ASPEED KCS devices don't
>support an OBE interrupt for the BMC. Instead we pretend it has one by
>polling the status register waiting for the Output Buffer Full (OBF) bit
>to clear, and generating an event when OBE is observed.
>
>Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>---
> drivers/char/ipmi/kcs_bmc.c         |   6 ++
> drivers/char/ipmi/kcs_bmc.h         |   3 +
> drivers/char/ipmi/kcs_bmc_aspeed.c  | 150 ++++++++++++++++++----------
> drivers/char/ipmi/kcs_bmc_client.h  |   2 +
> drivers/char/ipmi/kcs_bmc_device.h  |   1 +
> drivers/char/ipmi/kcs_bmc_npcm7xx.c |  25 ++++-
> 6 files changed, 130 insertions(+), 57 deletions(-)
>
>diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
>index 694db6ee2a92..05bbb72418b2 100644
>--- a/drivers/char/ipmi/kcs_bmc.c
>+++ b/drivers/char/ipmi/kcs_bmc.c
>@@ -184,6 +184,12 @@ int kcs_bmc_unregister_cdev(struct kcs_bmc_cdev *cdev)
> }
> EXPORT_SYMBOL(kcs_bmc_unregister_cdev);
>
>+void kcs_bmc_update_event_mask(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 events)
>+{
>+	kcs_bmc->ops->irq_mask_update(kcs_bmc, mask, events);
>+}
>+EXPORT_SYMBOL(kcs_bmc_update_event_mask);
>+
> MODULE_LICENSE("GPL v2");
> MODULE_AUTHOR("Haiyue Wang <haiyue.wang@linux.intel.com>");
> MODULE_AUTHOR("Andrew Jeffery <andrew@aj.id.au>");
>diff --git a/drivers/char/ipmi/kcs_bmc.h b/drivers/char/ipmi/kcs_bmc.h
>index 5deb9a0b8e60..11fff935218c 100644
>--- a/drivers/char/ipmi/kcs_bmc.h
>+++ b/drivers/char/ipmi/kcs_bmc.h
>@@ -11,6 +11,9 @@
> #define KCS_BMC_EVENT_NONE	0
> #define KCS_BMC_EVENT_HANDLED	1
>
>+#define KCS_BMC_EVENT_TYPE_OBE	BIT(0)
>+#define KCS_BMC_EVENT_TYPE_IBF	BIT(1)
>+
> #define KCS_BMC_STR_OBF		BIT(0)
> #define KCS_BMC_STR_IBF		BIT(1)
> #define KCS_BMC_STR_CMD_DAT	BIT(3)
>diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c b/drivers/char/ipmi/kcs_bmc_aspeed.c
>index 6f26e7366c0b..5f26471c038c 100644
>--- a/drivers/char/ipmi/kcs_bmc_aspeed.c
>+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
>@@ -60,10 +60,18 @@
> #define LPC_ODR4             0x118
> #define LPC_STR4             0x11C
>
>+#define OBE_POLL_PERIOD	     (HZ / 2)
>+
> struct aspeed_kcs_bmc {
> 	struct kcs_bmc_device kcs_bmc;
>
> 	struct regmap *map;
>+
>+	struct {
>+		spinlock_t lock;
>+		bool remove;
>+		struct timer_list timer;
>+	} obe;
> };
>
> struct aspeed_kcs_of_ops {
>@@ -159,68 +167,89 @@ static void aspeed_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enabl
>
> 	switch (kcs_bmc->channel) {
> 	case 1:
>-		if (enable) {
>-			regmap_update_bits(priv->map, LPC_HICR2,
>-					LPC_HICR2_IBFIF1, LPC_HICR2_IBFIF1);
>-			regmap_update_bits(priv->map, LPC_HICR0,
>-					LPC_HICR0_LPC1E, LPC_HICR0_LPC1E);
>-		} else {
>-			regmap_update_bits(priv->map, LPC_HICR0,
>-					LPC_HICR0_LPC1E, 0);
>-			regmap_update_bits(priv->map, LPC_HICR2,
>-					LPC_HICR2_IBFIF1, 0);
>-		}
>-		break;
>-
>+		regmap_update_bits(priv->map, LPC_HICR0, LPC_HICR0_LPC1E, enable * LPC_HICR0_LPC1E);
>+		return;
> 	case 2:
>-		if (enable) {
>-			regmap_update_bits(priv->map, LPC_HICR2,
>-					LPC_HICR2_IBFIF2, LPC_HICR2_IBFIF2);
>-			regmap_update_bits(priv->map, LPC_HICR0,
>-					LPC_HICR0_LPC2E, LPC_HICR0_LPC2E);
>-		} else {
>-			regmap_update_bits(priv->map, LPC_HICR0,
>-					LPC_HICR0_LPC2E, 0);
>-			regmap_update_bits(priv->map, LPC_HICR2,
>-					LPC_HICR2_IBFIF2, 0);
>-		}
>-		break;
>-
>+		regmap_update_bits(priv->map, LPC_HICR0, LPC_HICR0_LPC2E, enable * LPC_HICR0_LPC2E);
>+		return;
> 	case 3:
>-		if (enable) {
>-			regmap_update_bits(priv->map, LPC_HICR2,
>-					LPC_HICR2_IBFIF3, LPC_HICR2_IBFIF3);
>-			regmap_update_bits(priv->map, LPC_HICR0,
>-					LPC_HICR0_LPC3E, LPC_HICR0_LPC3E);
>-			regmap_update_bits(priv->map, LPC_HICR4,
>-					LPC_HICR4_KCSENBL, LPC_HICR4_KCSENBL);
>-		} else {
>-			regmap_update_bits(priv->map, LPC_HICR0,
>-					LPC_HICR0_LPC3E, 0);
>-			regmap_update_bits(priv->map, LPC_HICR4,
>-					LPC_HICR4_KCSENBL, 0);
>-			regmap_update_bits(priv->map, LPC_HICR2,
>-					LPC_HICR2_IBFIF3, 0);
>-		}
>-		break;
>-
>+		regmap_update_bits(priv->map, LPC_HICR0, LPC_HICR0_LPC3E, enable * LPC_HICR0_LPC3E);
>+		regmap_update_bits(priv->map, LPC_HICR4,
>+				   LPC_HICR4_KCSENBL, enable * LPC_HICR4_KCSENBL);
>+		return;
> 	case 4:
>-		if (enable)
>-			regmap_update_bits(priv->map, LPC_HICRB,
>-					LPC_HICRB_IBFIF4 | LPC_HICRB_LPC4E,
>-					LPC_HICRB_IBFIF4 | LPC_HICRB_LPC4E);
>+		regmap_update_bits(priv->map, LPC_HICRB, LPC_HICRB_LPC4E, enable * LPC_HICRB_LPC4E);
>+		return;
>+	default:
>+		pr_warn("%s: Unsupported channel: %d", __func__, kcs_bmc->channel);
>+		return;
>+	}
>+}
>+
>+static void aspeed_kcs_check_obe(struct timer_list *timer)
>+{
>+	struct aspeed_kcs_bmc *priv = container_of(timer, struct aspeed_kcs_bmc, obe.timer);
>+	unsigned long flags;
>+	u8 str;
>+
>+	spin_lock_irqsave(&priv->obe.lock, flags);
>+	if (priv->obe.remove) {
>+		spin_unlock_irqrestore(&priv->obe.lock, flags);
>+		return;
>+	}
>+
>+	str = aspeed_kcs_inb(&priv->kcs_bmc, priv->kcs_bmc.ioreg.str);
>+	if (str & KCS_BMC_STR_OBF) {
>+		mod_timer(timer, jiffies + OBE_POLL_PERIOD);
>+		spin_unlock_irqrestore(&priv->obe.lock, flags);
>+		return;
>+	}
>+	spin_unlock_irqrestore(&priv->obe.lock, flags);
>+
>+	kcs_bmc_handle_event(&priv->kcs_bmc);
>+}
>+
>+static void aspeed_kcs_irq_mask_update(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 state)
>+{
>+	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
>+
>+	/* We don't have an OBE IRQ, emulate it */
>+	if (KCS_BMC_EVENT_TYPE_OBE & mask) {
>+		if (KCS_BMC_EVENT_TYPE_OBE & state)
>+			mod_timer(&priv->obe.timer, jiffies + OBE_POLL_PERIOD);
> 		else
>-			regmap_update_bits(priv->map, LPC_HICRB,
>-					LPC_HICRB_IBFIF4 | LPC_HICRB_LPC4E,
>-					0);
>-		break;
>+			del_timer(&priv->obe.timer);
>+	}
>
>-	default:
>-		break;
>+	if (KCS_BMC_EVENT_TYPE_IBF & mask) {
>+		const bool enable = !!(KCS_BMC_EVENT_TYPE_IBF & state);

Totally superficial nitpick: the operand ordering for the bitmask tests
in this function seem a bit inconsistent with what I think is the usual
style of 'variable & BITMASK_MACRO' (reminiscent of "yoda
conditionals").

>+
>+		switch (kcs_bmc->channel) {
>+		case 1:
>+			regmap_update_bits(priv->map, LPC_HICR2, LPC_HICR2_IBFIF1,
>+					   enable * LPC_HICR2_IBFIF1);
>+			return;
>+		case 2:
>+			regmap_update_bits(priv->map, LPC_HICR2, LPC_HICR2_IBFIF2,
>+					   enable * LPC_HICR2_IBFIF2);
>+			return;
>+		case 3:
>+			regmap_update_bits(priv->map, LPC_HICR2, LPC_HICR2_IBFIF3,
>+					   enable * LPC_HICR2_IBFIF3);
>+			return;
>+		case 4:
>+			regmap_update_bits(priv->map, LPC_HICRB, LPC_HICRB_IBFIF4,
>+					   enable * LPC_HICRB_IBFIF4);
>+			return;
>+		default:
>+			pr_warn("%s: Unsupported channel: %d", __func__, kcs_bmc->channel);
>+			return;
>+		}
> 	}
> }
>
> static const struct kcs_bmc_device_ops aspeed_kcs_ops = {
>+	.irq_mask_update = aspeed_kcs_irq_mask_update,
> 	.io_inputb = aspeed_kcs_inb,
> 	.io_outputb = aspeed_kcs_outb,
> 	.io_updateb = aspeed_kcs_updateb,
>@@ -378,6 +407,10 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
> 		return -ENODEV;
> 	}
>
>+	spin_lock_init(&priv->obe.lock);
>+	priv->obe.remove = false;
>+	timer_setup(&priv->obe.timer, aspeed_kcs_check_obe, 0);
>+
> 	aspeed_kcs_set_address(kcs_bmc, addr);
>
> 	rc = aspeed_kcs_config_irq(kcs_bmc, pdev);
>@@ -386,6 +419,8 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
>
> 	platform_set_drvdata(pdev, priv);
>
>+	aspeed_kcs_irq_mask_update(kcs_bmc, (KCS_BMC_EVENT_TYPE_IBF | KCS_BMC_EVENT_TYPE_OBE),
>+				   KCS_BMC_EVENT_TYPE_IBF);
> 	aspeed_kcs_enable_channel(kcs_bmc, true);
>
> 	rc = kcs_bmc_add_device(&priv->kcs_bmc);
>@@ -404,6 +439,15 @@ static int aspeed_kcs_remove(struct platform_device *pdev)
>
> 	kcs_bmc_remove_device(kcs_bmc);
>
>+	aspeed_kcs_enable_channel(kcs_bmc, false);
>+	aspeed_kcs_irq_mask_update(kcs_bmc, (KCS_BMC_EVENT_TYPE_IBF | KCS_BMC_EVENT_TYPE_OBE), 0);
>+
>+	/* Make sure it's proper dead */
>+	spin_lock_irq(&priv->obe.lock);
>+	priv->obe.remove = true;
>+	spin_unlock_irq(&priv->obe.lock);
>+	del_timer_sync(&priv->obe.timer);
>+
> 	return 0;
> }
>
>diff --git a/drivers/char/ipmi/kcs_bmc_client.h b/drivers/char/ipmi/kcs_bmc_client.h
>index d0a7404ff584..456796da33de 100644
>--- a/drivers/char/ipmi/kcs_bmc_client.h
>+++ b/drivers/char/ipmi/kcs_bmc_client.h
>@@ -37,6 +37,8 @@ int kcs_bmc_unregister_cdev(struct kcs_bmc_cdev *cdev);
> 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);
>
>+void kcs_bmc_update_event_mask(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 events);
>+
> 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);
>diff --git a/drivers/char/ipmi/kcs_bmc_device.h b/drivers/char/ipmi/kcs_bmc_device.h
>index 57b7174b2bac..f1ca8912496a 100644
>--- a/drivers/char/ipmi/kcs_bmc_device.h
>+++ b/drivers/char/ipmi/kcs_bmc_device.h
>@@ -7,6 +7,7 @@
> #include "kcs_bmc.h"
>
> struct kcs_bmc_device_ops {
>+	void (*irq_mask_update)(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 enable);
> 	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);
>diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>index dce93ec895fc..c2032728a03d 100644
>--- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>+++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>@@ -38,6 +38,7 @@
> #define KCS2CTL		0x2A
> #define KCS3CTL		0x3C
> #define    KCS_CTL_IBFIE	BIT(0)
>+#define    KCS_CTL_OBEIE	BIT(0)
>
> #define KCS1IE		0x1C
> #define KCS2IE		0x2E
>@@ -117,13 +118,23 @@ static void npcm7xx_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enab
> {
> 	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
>
>-	regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE,
>-			   enable ? KCS_CTL_IBFIE : 0);
>-
> 	regmap_update_bits(priv->map, priv->reg->ie, KCS_IE_IRQE | KCS_IE_HIRQE,
> 			   enable ? KCS_IE_IRQE | KCS_IE_HIRQE : 0);
> }
>
>+static void npcm7xx_kcs_irq_mask_update(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 state)
>+{
>+	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
>+
>+	if (KCS_BMC_EVENT_TYPE_OBE & mask)
>+		regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_OBEIE,
>+				   !!(KCS_BMC_EVENT_TYPE_OBE & state) * KCS_CTL_OBEIE);
>+
>+	if (KCS_BMC_EVENT_TYPE_IBF & mask)
>+		regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE,
>+				   !!(KCS_BMC_EVENT_TYPE_IBF & state) * KCS_CTL_IBFIE);

Same operand ordering thing here...

>+}
>+
> static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)
> {
> 	struct kcs_bmc_device *kcs_bmc = arg;
>@@ -151,6 +162,7 @@ static int npcm7xx_kcs_config_irq(struct kcs_bmc_device *kcs_bmc,
> }
>
> static const struct kcs_bmc_device_ops npcm7xx_kcs_ops = {
>+	.irq_mask_update = npcm7xx_kcs_irq_mask_update,
> 	.io_inputb = npcm7xx_kcs_inb,
> 	.io_outputb = npcm7xx_kcs_outb,
> 	.io_updateb = npcm7xx_kcs_updateb,
>@@ -191,11 +203,13 @@ static int npcm7xx_kcs_probe(struct platform_device *pdev)
>
> 	platform_set_drvdata(pdev, priv);
>
>-	npcm7xx_kcs_enable_channel(kcs_bmc, true);
> 	rc = npcm7xx_kcs_config_irq(kcs_bmc, pdev);
> 	if (rc)
> 		return rc;
>
>+	npcm7xx_kcs_irq_mask_update(kcs_bmc, (KCS_BMC_EVENT_TYPE_IBF | KCS_BMC_EVENT_TYPE_OBE),
>+				    KCS_BMC_EVENT_TYPE_IBF);
>+	npcm7xx_kcs_enable_channel(kcs_bmc, true);
>
> 	pr_info("channel=%u idr=0x%x odr=0x%x str=0x%x\n",
> 		chan,
>@@ -211,6 +225,9 @@ static int npcm7xx_kcs_remove(struct platform_device *pdev)
>
> 	kcs_bmc_remove_device(kcs_bmc);
>
>+	npcm7xx_kcs_enable_channel(kcs_bmc, false);
>+	npcm7xx_kcs_irq_mask_update(kcs_bmc, (KCS_BMC_EVENT_TYPE_IBF | KCS_BMC_EVENT_TYPE_OBE), 0);
>+
> 	return 0;
> }
>
>-- 
>2.27.0
>
Andrew Jeffery April 9, 2021, 6:39 a.m. UTC | #2
On Fri, 9 Apr 2021, at 14:07, Zev Weiss wrote:
> On Fri, Mar 19, 2021 at 01:27:45AM CDT, Andrew Jeffery wrote:
> >Add a mechanism for controlling whether the client associated with a
> >KCS device will receive Input Buffer Full (IBF) and Output Buffer Empty
> >(OBE) events. This enables an abstract implementation of poll() for KCS
> >devices.
> >
> >A wart in the implementation is that the ASPEED KCS devices don't
> >support an OBE interrupt for the BMC. Instead we pretend it has one by
> >polling the status register waiting for the Output Buffer Full (OBF) bit
> >to clear, and generating an event when OBE is observed.
> >
> >Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> >---
> > drivers/char/ipmi/kcs_bmc.c         |   6 ++
> > drivers/char/ipmi/kcs_bmc.h         |   3 +
> > drivers/char/ipmi/kcs_bmc_aspeed.c  | 150 ++++++++++++++++++----------
> > drivers/char/ipmi/kcs_bmc_client.h  |   2 +
> > drivers/char/ipmi/kcs_bmc_device.h  |   1 +
> > drivers/char/ipmi/kcs_bmc_npcm7xx.c |  25 ++++-
> > 6 files changed, 130 insertions(+), 57 deletions(-)
> >
> >diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
> >index 694db6ee2a92..05bbb72418b2 100644
> >--- a/drivers/char/ipmi/kcs_bmc.c
> >+++ b/drivers/char/ipmi/kcs_bmc.c
> >@@ -184,6 +184,12 @@ int kcs_bmc_unregister_cdev(struct kcs_bmc_cdev *cdev)
> > }
> > EXPORT_SYMBOL(kcs_bmc_unregister_cdev);
> >
> >+void kcs_bmc_update_event_mask(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 events)
> >+{
> >+	kcs_bmc->ops->irq_mask_update(kcs_bmc, mask, events);
> >+}
> >+EXPORT_SYMBOL(kcs_bmc_update_event_mask);
> >+
> > MODULE_LICENSE("GPL v2");
> > MODULE_AUTHOR("Haiyue Wang <haiyue.wang@linux.intel.com>");
> > MODULE_AUTHOR("Andrew Jeffery <andrew@aj.id.au>");
> >diff --git a/drivers/char/ipmi/kcs_bmc.h b/drivers/char/ipmi/kcs_bmc.h
> >index 5deb9a0b8e60..11fff935218c 100644
> >--- a/drivers/char/ipmi/kcs_bmc.h
> >+++ b/drivers/char/ipmi/kcs_bmc.h
> >@@ -11,6 +11,9 @@
> > #define KCS_BMC_EVENT_NONE	0
> > #define KCS_BMC_EVENT_HANDLED	1
> >
> >+#define KCS_BMC_EVENT_TYPE_OBE	BIT(0)
> >+#define KCS_BMC_EVENT_TYPE_IBF	BIT(1)
> >+
> > #define KCS_BMC_STR_OBF		BIT(0)
> > #define KCS_BMC_STR_IBF		BIT(1)
> > #define KCS_BMC_STR_CMD_DAT	BIT(3)
> >diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c b/drivers/char/ipmi/kcs_bmc_aspeed.c
> >index 6f26e7366c0b..5f26471c038c 100644
> >--- a/drivers/char/ipmi/kcs_bmc_aspeed.c
> >+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
> >@@ -60,10 +60,18 @@
> > #define LPC_ODR4             0x118
> > #define LPC_STR4             0x11C
> >
> >+#define OBE_POLL_PERIOD	     (HZ / 2)
> >+
> > struct aspeed_kcs_bmc {
> > 	struct kcs_bmc_device kcs_bmc;
> >
> > 	struct regmap *map;
> >+
> >+	struct {
> >+		spinlock_t lock;
> >+		bool remove;
> >+		struct timer_list timer;
> >+	} obe;
> > };
> >
> > struct aspeed_kcs_of_ops {
> >@@ -159,68 +167,89 @@ static void aspeed_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enabl
> >
> > 	switch (kcs_bmc->channel) {
> > 	case 1:
> >-		if (enable) {
> >-			regmap_update_bits(priv->map, LPC_HICR2,
> >-					LPC_HICR2_IBFIF1, LPC_HICR2_IBFIF1);
> >-			regmap_update_bits(priv->map, LPC_HICR0,
> >-					LPC_HICR0_LPC1E, LPC_HICR0_LPC1E);
> >-		} else {
> >-			regmap_update_bits(priv->map, LPC_HICR0,
> >-					LPC_HICR0_LPC1E, 0);
> >-			regmap_update_bits(priv->map, LPC_HICR2,
> >-					LPC_HICR2_IBFIF1, 0);
> >-		}
> >-		break;
> >-
> >+		regmap_update_bits(priv->map, LPC_HICR0, LPC_HICR0_LPC1E, enable * LPC_HICR0_LPC1E);
> >+		return;
> > 	case 2:
> >-		if (enable) {
> >-			regmap_update_bits(priv->map, LPC_HICR2,
> >-					LPC_HICR2_IBFIF2, LPC_HICR2_IBFIF2);
> >-			regmap_update_bits(priv->map, LPC_HICR0,
> >-					LPC_HICR0_LPC2E, LPC_HICR0_LPC2E);
> >-		} else {
> >-			regmap_update_bits(priv->map, LPC_HICR0,
> >-					LPC_HICR0_LPC2E, 0);
> >-			regmap_update_bits(priv->map, LPC_HICR2,
> >-					LPC_HICR2_IBFIF2, 0);
> >-		}
> >-		break;
> >-
> >+		regmap_update_bits(priv->map, LPC_HICR0, LPC_HICR0_LPC2E, enable * LPC_HICR0_LPC2E);
> >+		return;
> > 	case 3:
> >-		if (enable) {
> >-			regmap_update_bits(priv->map, LPC_HICR2,
> >-					LPC_HICR2_IBFIF3, LPC_HICR2_IBFIF3);
> >-			regmap_update_bits(priv->map, LPC_HICR0,
> >-					LPC_HICR0_LPC3E, LPC_HICR0_LPC3E);
> >-			regmap_update_bits(priv->map, LPC_HICR4,
> >-					LPC_HICR4_KCSENBL, LPC_HICR4_KCSENBL);
> >-		} else {
> >-			regmap_update_bits(priv->map, LPC_HICR0,
> >-					LPC_HICR0_LPC3E, 0);
> >-			regmap_update_bits(priv->map, LPC_HICR4,
> >-					LPC_HICR4_KCSENBL, 0);
> >-			regmap_update_bits(priv->map, LPC_HICR2,
> >-					LPC_HICR2_IBFIF3, 0);
> >-		}
> >-		break;
> >-
> >+		regmap_update_bits(priv->map, LPC_HICR0, LPC_HICR0_LPC3E, enable * LPC_HICR0_LPC3E);
> >+		regmap_update_bits(priv->map, LPC_HICR4,
> >+				   LPC_HICR4_KCSENBL, enable * LPC_HICR4_KCSENBL);
> >+		return;
> > 	case 4:
> >-		if (enable)
> >-			regmap_update_bits(priv->map, LPC_HICRB,
> >-					LPC_HICRB_IBFIF4 | LPC_HICRB_LPC4E,
> >-					LPC_HICRB_IBFIF4 | LPC_HICRB_LPC4E);
> >+		regmap_update_bits(priv->map, LPC_HICRB, LPC_HICRB_LPC4E, enable * LPC_HICRB_LPC4E);
> >+		return;
> >+	default:
> >+		pr_warn("%s: Unsupported channel: %d", __func__, kcs_bmc->channel);
> >+		return;
> >+	}
> >+}
> >+
> >+static void aspeed_kcs_check_obe(struct timer_list *timer)
> >+{
> >+	struct aspeed_kcs_bmc *priv = container_of(timer, struct aspeed_kcs_bmc, obe.timer);
> >+	unsigned long flags;
> >+	u8 str;
> >+
> >+	spin_lock_irqsave(&priv->obe.lock, flags);
> >+	if (priv->obe.remove) {
> >+		spin_unlock_irqrestore(&priv->obe.lock, flags);
> >+		return;
> >+	}
> >+
> >+	str = aspeed_kcs_inb(&priv->kcs_bmc, priv->kcs_bmc.ioreg.str);
> >+	if (str & KCS_BMC_STR_OBF) {
> >+		mod_timer(timer, jiffies + OBE_POLL_PERIOD);
> >+		spin_unlock_irqrestore(&priv->obe.lock, flags);
> >+		return;
> >+	}
> >+	spin_unlock_irqrestore(&priv->obe.lock, flags);
> >+
> >+	kcs_bmc_handle_event(&priv->kcs_bmc);
> >+}
> >+
> >+static void aspeed_kcs_irq_mask_update(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 state)
> >+{
> >+	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
> >+
> >+	/* We don't have an OBE IRQ, emulate it */
> >+	if (KCS_BMC_EVENT_TYPE_OBE & mask) {
> >+		if (KCS_BMC_EVENT_TYPE_OBE & state)
> >+			mod_timer(&priv->obe.timer, jiffies + OBE_POLL_PERIOD);
> > 		else
> >-			regmap_update_bits(priv->map, LPC_HICRB,
> >-					LPC_HICRB_IBFIF4 | LPC_HICRB_LPC4E,
> >-					0);
> >-		break;
> >+			del_timer(&priv->obe.timer);
> >+	}
> >
> >-	default:
> >-		break;
> >+	if (KCS_BMC_EVENT_TYPE_IBF & mask) {
> >+		const bool enable = !!(KCS_BMC_EVENT_TYPE_IBF & state);
> 
> Totally superficial nitpick: the operand ordering for the bitmask tests
> in this function seem a bit inconsistent with what I think is the usual
> style of 'variable & BITMASK_MACRO' (reminiscent of "yoda
> conditionals").

Haha, I'm not sure what possessed me here.

> 
> >+
> >+		switch (kcs_bmc->channel) {
> >+		case 1:
> >+			regmap_update_bits(priv->map, LPC_HICR2, LPC_HICR2_IBFIF1,
> >+					   enable * LPC_HICR2_IBFIF1);
> >+			return;
> >+		case 2:
> >+			regmap_update_bits(priv->map, LPC_HICR2, LPC_HICR2_IBFIF2,
> >+					   enable * LPC_HICR2_IBFIF2);
> >+			return;
> >+		case 3:
> >+			regmap_update_bits(priv->map, LPC_HICR2, LPC_HICR2_IBFIF3,
> >+					   enable * LPC_HICR2_IBFIF3);
> >+			return;
> >+		case 4:
> >+			regmap_update_bits(priv->map, LPC_HICRB, LPC_HICRB_IBFIF4,
> >+					   enable * LPC_HICRB_IBFIF4);
> >+			return;
> >+		default:
> >+			pr_warn("%s: Unsupported channel: %d", __func__, kcs_bmc->channel);
> >+			return;
> >+		}
> > 	}
> > }
> >
> > static const struct kcs_bmc_device_ops aspeed_kcs_ops = {
> >+	.irq_mask_update = aspeed_kcs_irq_mask_update,
> > 	.io_inputb = aspeed_kcs_inb,
> > 	.io_outputb = aspeed_kcs_outb,
> > 	.io_updateb = aspeed_kcs_updateb,
> >@@ -378,6 +407,10 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
> > 		return -ENODEV;
> > 	}
> >
> >+	spin_lock_init(&priv->obe.lock);
> >+	priv->obe.remove = false;
> >+	timer_setup(&priv->obe.timer, aspeed_kcs_check_obe, 0);
> >+
> > 	aspeed_kcs_set_address(kcs_bmc, addr);
> >
> > 	rc = aspeed_kcs_config_irq(kcs_bmc, pdev);
> >@@ -386,6 +419,8 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
> >
> > 	platform_set_drvdata(pdev, priv);
> >
> >+	aspeed_kcs_irq_mask_update(kcs_bmc, (KCS_BMC_EVENT_TYPE_IBF | KCS_BMC_EVENT_TYPE_OBE),
> >+				   KCS_BMC_EVENT_TYPE_IBF);
> > 	aspeed_kcs_enable_channel(kcs_bmc, true);
> >
> > 	rc = kcs_bmc_add_device(&priv->kcs_bmc);
> >@@ -404,6 +439,15 @@ static int aspeed_kcs_remove(struct platform_device *pdev)
> >
> > 	kcs_bmc_remove_device(kcs_bmc);
> >
> >+	aspeed_kcs_enable_channel(kcs_bmc, false);
> >+	aspeed_kcs_irq_mask_update(kcs_bmc, (KCS_BMC_EVENT_TYPE_IBF | KCS_BMC_EVENT_TYPE_OBE), 0);
> >+
> >+	/* Make sure it's proper dead */
> >+	spin_lock_irq(&priv->obe.lock);
> >+	priv->obe.remove = true;
> >+	spin_unlock_irq(&priv->obe.lock);
> >+	del_timer_sync(&priv->obe.timer);
> >+
> > 	return 0;
> > }
> >
> >diff --git a/drivers/char/ipmi/kcs_bmc_client.h b/drivers/char/ipmi/kcs_bmc_client.h
> >index d0a7404ff584..456796da33de 100644
> >--- a/drivers/char/ipmi/kcs_bmc_client.h
> >+++ b/drivers/char/ipmi/kcs_bmc_client.h
> >@@ -37,6 +37,8 @@ int kcs_bmc_unregister_cdev(struct kcs_bmc_cdev *cdev);
> > 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);
> >
> >+void kcs_bmc_update_event_mask(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 events);
> >+
> > 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);
> >diff --git a/drivers/char/ipmi/kcs_bmc_device.h b/drivers/char/ipmi/kcs_bmc_device.h
> >index 57b7174b2bac..f1ca8912496a 100644
> >--- a/drivers/char/ipmi/kcs_bmc_device.h
> >+++ b/drivers/char/ipmi/kcs_bmc_device.h
> >@@ -7,6 +7,7 @@
> > #include "kcs_bmc.h"
> >
> > struct kcs_bmc_device_ops {
> >+	void (*irq_mask_update)(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 enable);
> > 	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);
> >diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
> >index dce93ec895fc..c2032728a03d 100644
> >--- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
> >+++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
> >@@ -38,6 +38,7 @@
> > #define KCS2CTL		0x2A
> > #define KCS3CTL		0x3C
> > #define    KCS_CTL_IBFIE	BIT(0)
> >+#define    KCS_CTL_OBEIE	BIT(0)
> >
> > #define KCS1IE		0x1C
> > #define KCS2IE		0x2E
> >@@ -117,13 +118,23 @@ static void npcm7xx_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enab
> > {
> > 	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
> >
> >-	regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE,
> >-			   enable ? KCS_CTL_IBFIE : 0);
> >-
> > 	regmap_update_bits(priv->map, priv->reg->ie, KCS_IE_IRQE | KCS_IE_HIRQE,
> > 			   enable ? KCS_IE_IRQE | KCS_IE_HIRQE : 0);
> > }
> >
> >+static void npcm7xx_kcs_irq_mask_update(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 state)
> >+{
> >+	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
> >+
> >+	if (KCS_BMC_EVENT_TYPE_OBE & mask)
> >+		regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_OBEIE,
> >+				   !!(KCS_BMC_EVENT_TYPE_OBE & state) * KCS_CTL_OBEIE);
> >+
> >+	if (KCS_BMC_EVENT_TYPE_IBF & mask)
> >+		regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE,
> >+				   !!(KCS_BMC_EVENT_TYPE_IBF & state) * KCS_CTL_IBFIE);
> 
> Same operand ordering thing here...

Ack.

Andrew
diff mbox series

Patch

diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
index 694db6ee2a92..05bbb72418b2 100644
--- a/drivers/char/ipmi/kcs_bmc.c
+++ b/drivers/char/ipmi/kcs_bmc.c
@@ -184,6 +184,12 @@  int kcs_bmc_unregister_cdev(struct kcs_bmc_cdev *cdev)
 }
 EXPORT_SYMBOL(kcs_bmc_unregister_cdev);
 
+void kcs_bmc_update_event_mask(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 events)
+{
+	kcs_bmc->ops->irq_mask_update(kcs_bmc, mask, events);
+}
+EXPORT_SYMBOL(kcs_bmc_update_event_mask);
+
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Haiyue Wang <haiyue.wang@linux.intel.com>");
 MODULE_AUTHOR("Andrew Jeffery <andrew@aj.id.au>");
diff --git a/drivers/char/ipmi/kcs_bmc.h b/drivers/char/ipmi/kcs_bmc.h
index 5deb9a0b8e60..11fff935218c 100644
--- a/drivers/char/ipmi/kcs_bmc.h
+++ b/drivers/char/ipmi/kcs_bmc.h
@@ -11,6 +11,9 @@ 
 #define KCS_BMC_EVENT_NONE	0
 #define KCS_BMC_EVENT_HANDLED	1
 
+#define KCS_BMC_EVENT_TYPE_OBE	BIT(0)
+#define KCS_BMC_EVENT_TYPE_IBF	BIT(1)
+
 #define KCS_BMC_STR_OBF		BIT(0)
 #define KCS_BMC_STR_IBF		BIT(1)
 #define KCS_BMC_STR_CMD_DAT	BIT(3)
diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c b/drivers/char/ipmi/kcs_bmc_aspeed.c
index 6f26e7366c0b..5f26471c038c 100644
--- a/drivers/char/ipmi/kcs_bmc_aspeed.c
+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
@@ -60,10 +60,18 @@ 
 #define LPC_ODR4             0x118
 #define LPC_STR4             0x11C
 
+#define OBE_POLL_PERIOD	     (HZ / 2)
+
 struct aspeed_kcs_bmc {
 	struct kcs_bmc_device kcs_bmc;
 
 	struct regmap *map;
+
+	struct {
+		spinlock_t lock;
+		bool remove;
+		struct timer_list timer;
+	} obe;
 };
 
 struct aspeed_kcs_of_ops {
@@ -159,68 +167,89 @@  static void aspeed_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enabl
 
 	switch (kcs_bmc->channel) {
 	case 1:
-		if (enable) {
-			regmap_update_bits(priv->map, LPC_HICR2,
-					LPC_HICR2_IBFIF1, LPC_HICR2_IBFIF1);
-			regmap_update_bits(priv->map, LPC_HICR0,
-					LPC_HICR0_LPC1E, LPC_HICR0_LPC1E);
-		} else {
-			regmap_update_bits(priv->map, LPC_HICR0,
-					LPC_HICR0_LPC1E, 0);
-			regmap_update_bits(priv->map, LPC_HICR2,
-					LPC_HICR2_IBFIF1, 0);
-		}
-		break;
-
+		regmap_update_bits(priv->map, LPC_HICR0, LPC_HICR0_LPC1E, enable * LPC_HICR0_LPC1E);
+		return;
 	case 2:
-		if (enable) {
-			regmap_update_bits(priv->map, LPC_HICR2,
-					LPC_HICR2_IBFIF2, LPC_HICR2_IBFIF2);
-			regmap_update_bits(priv->map, LPC_HICR0,
-					LPC_HICR0_LPC2E, LPC_HICR0_LPC2E);
-		} else {
-			regmap_update_bits(priv->map, LPC_HICR0,
-					LPC_HICR0_LPC2E, 0);
-			regmap_update_bits(priv->map, LPC_HICR2,
-					LPC_HICR2_IBFIF2, 0);
-		}
-		break;
-
+		regmap_update_bits(priv->map, LPC_HICR0, LPC_HICR0_LPC2E, enable * LPC_HICR0_LPC2E);
+		return;
 	case 3:
-		if (enable) {
-			regmap_update_bits(priv->map, LPC_HICR2,
-					LPC_HICR2_IBFIF3, LPC_HICR2_IBFIF3);
-			regmap_update_bits(priv->map, LPC_HICR0,
-					LPC_HICR0_LPC3E, LPC_HICR0_LPC3E);
-			regmap_update_bits(priv->map, LPC_HICR4,
-					LPC_HICR4_KCSENBL, LPC_HICR4_KCSENBL);
-		} else {
-			regmap_update_bits(priv->map, LPC_HICR0,
-					LPC_HICR0_LPC3E, 0);
-			regmap_update_bits(priv->map, LPC_HICR4,
-					LPC_HICR4_KCSENBL, 0);
-			regmap_update_bits(priv->map, LPC_HICR2,
-					LPC_HICR2_IBFIF3, 0);
-		}
-		break;
-
+		regmap_update_bits(priv->map, LPC_HICR0, LPC_HICR0_LPC3E, enable * LPC_HICR0_LPC3E);
+		regmap_update_bits(priv->map, LPC_HICR4,
+				   LPC_HICR4_KCSENBL, enable * LPC_HICR4_KCSENBL);
+		return;
 	case 4:
-		if (enable)
-			regmap_update_bits(priv->map, LPC_HICRB,
-					LPC_HICRB_IBFIF4 | LPC_HICRB_LPC4E,
-					LPC_HICRB_IBFIF4 | LPC_HICRB_LPC4E);
+		regmap_update_bits(priv->map, LPC_HICRB, LPC_HICRB_LPC4E, enable * LPC_HICRB_LPC4E);
+		return;
+	default:
+		pr_warn("%s: Unsupported channel: %d", __func__, kcs_bmc->channel);
+		return;
+	}
+}
+
+static void aspeed_kcs_check_obe(struct timer_list *timer)
+{
+	struct aspeed_kcs_bmc *priv = container_of(timer, struct aspeed_kcs_bmc, obe.timer);
+	unsigned long flags;
+	u8 str;
+
+	spin_lock_irqsave(&priv->obe.lock, flags);
+	if (priv->obe.remove) {
+		spin_unlock_irqrestore(&priv->obe.lock, flags);
+		return;
+	}
+
+	str = aspeed_kcs_inb(&priv->kcs_bmc, priv->kcs_bmc.ioreg.str);
+	if (str & KCS_BMC_STR_OBF) {
+		mod_timer(timer, jiffies + OBE_POLL_PERIOD);
+		spin_unlock_irqrestore(&priv->obe.lock, flags);
+		return;
+	}
+	spin_unlock_irqrestore(&priv->obe.lock, flags);
+
+	kcs_bmc_handle_event(&priv->kcs_bmc);
+}
+
+static void aspeed_kcs_irq_mask_update(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 state)
+{
+	struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
+
+	/* We don't have an OBE IRQ, emulate it */
+	if (KCS_BMC_EVENT_TYPE_OBE & mask) {
+		if (KCS_BMC_EVENT_TYPE_OBE & state)
+			mod_timer(&priv->obe.timer, jiffies + OBE_POLL_PERIOD);
 		else
-			regmap_update_bits(priv->map, LPC_HICRB,
-					LPC_HICRB_IBFIF4 | LPC_HICRB_LPC4E,
-					0);
-		break;
+			del_timer(&priv->obe.timer);
+	}
 
-	default:
-		break;
+	if (KCS_BMC_EVENT_TYPE_IBF & mask) {
+		const bool enable = !!(KCS_BMC_EVENT_TYPE_IBF & state);
+
+		switch (kcs_bmc->channel) {
+		case 1:
+			regmap_update_bits(priv->map, LPC_HICR2, LPC_HICR2_IBFIF1,
+					   enable * LPC_HICR2_IBFIF1);
+			return;
+		case 2:
+			regmap_update_bits(priv->map, LPC_HICR2, LPC_HICR2_IBFIF2,
+					   enable * LPC_HICR2_IBFIF2);
+			return;
+		case 3:
+			regmap_update_bits(priv->map, LPC_HICR2, LPC_HICR2_IBFIF3,
+					   enable * LPC_HICR2_IBFIF3);
+			return;
+		case 4:
+			regmap_update_bits(priv->map, LPC_HICRB, LPC_HICRB_IBFIF4,
+					   enable * LPC_HICRB_IBFIF4);
+			return;
+		default:
+			pr_warn("%s: Unsupported channel: %d", __func__, kcs_bmc->channel);
+			return;
+		}
 	}
 }
 
 static const struct kcs_bmc_device_ops aspeed_kcs_ops = {
+	.irq_mask_update = aspeed_kcs_irq_mask_update,
 	.io_inputb = aspeed_kcs_inb,
 	.io_outputb = aspeed_kcs_outb,
 	.io_updateb = aspeed_kcs_updateb,
@@ -378,6 +407,10 @@  static int aspeed_kcs_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	spin_lock_init(&priv->obe.lock);
+	priv->obe.remove = false;
+	timer_setup(&priv->obe.timer, aspeed_kcs_check_obe, 0);
+
 	aspeed_kcs_set_address(kcs_bmc, addr);
 
 	rc = aspeed_kcs_config_irq(kcs_bmc, pdev);
@@ -386,6 +419,8 @@  static int aspeed_kcs_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, priv);
 
+	aspeed_kcs_irq_mask_update(kcs_bmc, (KCS_BMC_EVENT_TYPE_IBF | KCS_BMC_EVENT_TYPE_OBE),
+				   KCS_BMC_EVENT_TYPE_IBF);
 	aspeed_kcs_enable_channel(kcs_bmc, true);
 
 	rc = kcs_bmc_add_device(&priv->kcs_bmc);
@@ -404,6 +439,15 @@  static int aspeed_kcs_remove(struct platform_device *pdev)
 
 	kcs_bmc_remove_device(kcs_bmc);
 
+	aspeed_kcs_enable_channel(kcs_bmc, false);
+	aspeed_kcs_irq_mask_update(kcs_bmc, (KCS_BMC_EVENT_TYPE_IBF | KCS_BMC_EVENT_TYPE_OBE), 0);
+
+	/* Make sure it's proper dead */
+	spin_lock_irq(&priv->obe.lock);
+	priv->obe.remove = true;
+	spin_unlock_irq(&priv->obe.lock);
+	del_timer_sync(&priv->obe.timer);
+
 	return 0;
 }
 
diff --git a/drivers/char/ipmi/kcs_bmc_client.h b/drivers/char/ipmi/kcs_bmc_client.h
index d0a7404ff584..456796da33de 100644
--- a/drivers/char/ipmi/kcs_bmc_client.h
+++ b/drivers/char/ipmi/kcs_bmc_client.h
@@ -37,6 +37,8 @@  int kcs_bmc_unregister_cdev(struct kcs_bmc_cdev *cdev);
 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);
 
+void kcs_bmc_update_event_mask(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 events);
+
 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);
diff --git a/drivers/char/ipmi/kcs_bmc_device.h b/drivers/char/ipmi/kcs_bmc_device.h
index 57b7174b2bac..f1ca8912496a 100644
--- a/drivers/char/ipmi/kcs_bmc_device.h
+++ b/drivers/char/ipmi/kcs_bmc_device.h
@@ -7,6 +7,7 @@ 
 #include "kcs_bmc.h"
 
 struct kcs_bmc_device_ops {
+	void (*irq_mask_update)(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 enable);
 	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);
diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
index dce93ec895fc..c2032728a03d 100644
--- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
+++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
@@ -38,6 +38,7 @@ 
 #define KCS2CTL		0x2A
 #define KCS3CTL		0x3C
 #define    KCS_CTL_IBFIE	BIT(0)
+#define    KCS_CTL_OBEIE	BIT(0)
 
 #define KCS1IE		0x1C
 #define KCS2IE		0x2E
@@ -117,13 +118,23 @@  static void npcm7xx_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enab
 {
 	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
 
-	regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE,
-			   enable ? KCS_CTL_IBFIE : 0);
-
 	regmap_update_bits(priv->map, priv->reg->ie, KCS_IE_IRQE | KCS_IE_HIRQE,
 			   enable ? KCS_IE_IRQE | KCS_IE_HIRQE : 0);
 }
 
+static void npcm7xx_kcs_irq_mask_update(struct kcs_bmc_device *kcs_bmc, u8 mask, u8 state)
+{
+	struct npcm7xx_kcs_bmc *priv = to_npcm7xx_kcs_bmc(kcs_bmc);
+
+	if (KCS_BMC_EVENT_TYPE_OBE & mask)
+		regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_OBEIE,
+				   !!(KCS_BMC_EVENT_TYPE_OBE & state) * KCS_CTL_OBEIE);
+
+	if (KCS_BMC_EVENT_TYPE_IBF & mask)
+		regmap_update_bits(priv->map, priv->reg->ctl, KCS_CTL_IBFIE,
+				   !!(KCS_BMC_EVENT_TYPE_IBF & state) * KCS_CTL_IBFIE);
+}
+
 static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)
 {
 	struct kcs_bmc_device *kcs_bmc = arg;
@@ -151,6 +162,7 @@  static int npcm7xx_kcs_config_irq(struct kcs_bmc_device *kcs_bmc,
 }
 
 static const struct kcs_bmc_device_ops npcm7xx_kcs_ops = {
+	.irq_mask_update = npcm7xx_kcs_irq_mask_update,
 	.io_inputb = npcm7xx_kcs_inb,
 	.io_outputb = npcm7xx_kcs_outb,
 	.io_updateb = npcm7xx_kcs_updateb,
@@ -191,11 +203,13 @@  static int npcm7xx_kcs_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, priv);
 
-	npcm7xx_kcs_enable_channel(kcs_bmc, true);
 	rc = npcm7xx_kcs_config_irq(kcs_bmc, pdev);
 	if (rc)
 		return rc;
 
+	npcm7xx_kcs_irq_mask_update(kcs_bmc, (KCS_BMC_EVENT_TYPE_IBF | KCS_BMC_EVENT_TYPE_OBE),
+				    KCS_BMC_EVENT_TYPE_IBF);
+	npcm7xx_kcs_enable_channel(kcs_bmc, true);
 
 	pr_info("channel=%u idr=0x%x odr=0x%x str=0x%x\n",
 		chan,
@@ -211,6 +225,9 @@  static int npcm7xx_kcs_remove(struct platform_device *pdev)
 
 	kcs_bmc_remove_device(kcs_bmc);
 
+	npcm7xx_kcs_enable_channel(kcs_bmc, false);
+	npcm7xx_kcs_irq_mask_update(kcs_bmc, (KCS_BMC_EVENT_TYPE_IBF | KCS_BMC_EVENT_TYPE_OBE), 0);
+
 	return 0;
 }