Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Zev Weiss <zweiss@equinix.com>
To:     Andrew Jeffery <andrew@aj.id.au>
CC:     "openipmi-developer@lists.sourceforge.net" 
        <openipmi-developer@lists.sourceforge.net>,
        "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
        "minyard@acm.org" <minyard@acm.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "ryan_chen@aspeedtech.com" <ryan_chen@aspeedtech.com>,
        "tmaimon77@gmail.com" <tmaimon77@gmail.com>,
        "linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>,
        "avifishman70@gmail.com" <avifishman70@gmail.com>,
        "venture@google.com" <venture@google.com>,
        "linus.walleij@linaro.org" <linus.walleij@linaro.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "tali.perry1@gmail.com" <tali.perry1@gmail.com>,
        "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "lee.jones@linaro.org" <lee.jones@linaro.org>,
        "chiawei_wang@aspeedtech.com" <chiawei_wang@aspeedtech.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "benjaminfair@google.com" <benjaminfair@google.com>
Subject: Re: [PATCH v2 11/21] ipmi: kcs_bmc: Split headers into device and
 client
Thread-Topic: [PATCH v2 11/21] ipmi: kcs_bmc: Split headers into device and
 client
Thread-Index: AQHXLPUAcNk3H3d+bUCgCJVA00RN8A==
Date:   Fri, 9 Apr 2021 04:01:21 +0000
Message-ID: <YG/RkB07Wh69/xdu@packtop>
References: <20210319062752.145730-1-andrew@aj.id.au>
 <20210319062752.145730-11-andrew@aj.id.au>
In-Reply-To: <20210319062752.145730-11-andrew@aj.id.au>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <D20D87C509F3CF44A7C5D5F61F40112A@namprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: a660d812-8251-4c18-1d6a-08d8fb0c22e2
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Apr 2021 04:01:21.4959
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 72adb271-2fc7-4afe-a5ee-9de6a59f6bfb
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Tu3D5O17jsz+gy3uBHbBx4o/x2aqD5gDz8Ay6Q6dbhI1nBIBOi+tjfTVg7QIc3m4X++sCS9Tomq0r2zzuaBTuA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR04MB5420
X-Proofpoint-GUID: oyVuhMHmUtrptN4_e7HF9hmqY5UFf0zt
X-Proofpoint-ORIG-GUID: oyVuhMHmUtrptN4_e7HF9hmqY5UFf0zt
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.761
 definitions=2021-04-09_03:2021-04-08,2021-04-09 signatures=0
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501
 lowpriorityscore=0 phishscore=0 impostorscore=0 suspectscore=0
 malwarescore=0 bulkscore=0 adultscore=0 mlxlogscore=999 mlxscore=0
 spamscore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2104060000 definitions=main-2104090026
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Mar 19, 2021 at 01:27:42AM CDT, Andrew Jeffery wrote:
>Strengthen the distinction between code that abstracts the
>implementation of the KCS behaviours (device drivers) and code that
>exploits KCS behaviours (clients). Neither needs to know about the APIs
>required by the other, so provide separate headers.
>
>Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>---
> drivers/char/ipmi/kcs_bmc.c           | 21 ++++++++++-----
> drivers/char/ipmi/kcs_bmc.h           | 30 ++++++++++-----------
> drivers/char/ipmi/kcs_bmc_aspeed.c    | 20 +++++++++-----
> drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 39 ++++++++++++++++++---------
> drivers/char/ipmi/kcs_bmc_client.h    | 29 ++++++++++++++++++++
> drivers/char/ipmi/kcs_bmc_device.h    | 19 +++++++++++++
> drivers/char/ipmi/kcs_bmc_npcm7xx.c   | 20 +++++++++-----
> 7 files changed, 129 insertions(+), 49 deletions(-)
> create mode 100644 drivers/char/ipmi/kcs_bmc_client.h
> create mode 100644 drivers/char/ipmi/kcs_bmc_device.h
>
>diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
>index 709b6bdec165..1046ce2bbefc 100644
>--- a/drivers/char/ipmi/kcs_bmc.c
>+++ b/drivers/char/ipmi/kcs_bmc.c
>@@ -1,46 +1,52 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
>  * Copyright (c) 2015-2018, Intel Corporation.
>+ * Copyright (c) 2021, IBM Corp.
>  */
>
> #include <linux/module.h>
>
> #include "kcs_bmc.h"
>
>+/* Implement both the device and client interfaces here */
>+#include "kcs_bmc_device.h"
>+#include "kcs_bmc_client.h"
>+
>+/* Consumer data access */
>+
> u8 kcs_bmc_read_data(struct kcs_bmc *kcs_bmc)
> {
>-	return kcs_bmc->io_inputb(kcs_bmc, kcs_bmc->ioreg.idr);
>+	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)
> {
>-	kcs_bmc->io_outputb(kcs_bmc, kcs_bmc->ioreg.odr, 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)
> {
>-	return kcs_bmc->io_inputb(kcs_bmc, kcs_bmc->ioreg.str);
>+	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)
> {
>-	kcs_bmc->io_outputb(kcs_bmc, kcs_bmc->ioreg.str, 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)
> {
>-	kcs_bmc->io_updateb(kcs_bmc, kcs_bmc->ioreg.str, mask, val);
>+	kcs_bmc->ops->io_updateb(kcs_bmc, kcs_bmc->ioreg.str, mask, val);
> }
> EXPORT_SYMBOL(kcs_bmc_update_status);
>
>-int kcs_bmc_ipmi_event(struct kcs_bmc *kcs_bmc);
> int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc)
> {
>-	return kcs_bmc_ipmi_event(kcs_bmc);
>+	return kcs_bmc->client.ops->event(&kcs_bmc->client);
> }
> EXPORT_SYMBOL(kcs_bmc_handle_event);
>
>@@ -60,4 +66,5 @@ EXPORT_SYMBOL(kcs_bmc_remove_device);
>
> 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 softwa=
re");
>diff --git a/drivers/char/ipmi/kcs_bmc.h b/drivers/char/ipmi/kcs_bmc.h
>index bf0ae327997f..a1350e567723 100644
>--- a/drivers/char/ipmi/kcs_bmc.h
>+++ b/drivers/char/ipmi/kcs_bmc.h
>@@ -8,6 +8,15 @@
>
> #include <linux/miscdevice.h>
>
>+#include "kcs_bmc_client.h"
>+
>+#define KCS_BMC_EVENT_NONE	0
>+#define KCS_BMC_EVENT_HANDLED	1

Is there a particular reason we're introducing these macros and using an
int return type for kcs_bmc_client_ops.event instead of just having it
be irqreturn_t?  Other event types or outcomes we're anticipating needing
to handle maybe?

>+
>+#define KCS_BMC_STR_OBF		BIT(0)
>+#define KCS_BMC_STR_IBF		BIT(1)
>+#define KCS_BMC_STR_CMD_DAT	BIT(3)

The first two of these macros are used later in the series, but the third
doesn't end up used at all I think?

>+
> /* Different phases of the KCS BMC module.
>  *  KCS_PHASE_IDLE:
>  *            BMC should not be expecting nor sending any data.
>@@ -66,19 +75,21 @@ struct kcs_ioreg {
> 	u32 str;
> };
>
>+struct kcs_bmc_device_ops;
>+
> 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;
>
>-	/* Setup by BMC KCS controller driver */
> 	struct kcs_ioreg ioreg;
>-	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 val);
>
> 	enum kcs_phases phase;
> 	enum kcs_errors error;
>@@ -97,15 +108,4 @@ struct kcs_bmc {
>
> 	struct miscdevice miscdev;
> };
>-
>-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);
>-
>-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);
>-
> #endif /* __KCS_BMC_H__ */
>diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c b/drivers/char/ipmi/kcs_bm=
c_aspeed.c
>index 0416ac78ce68..1b313355b1c8 100644
>--- a/drivers/char/ipmi/kcs_bmc_aspeed.c
>+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
>@@ -21,7 +21,7 @@
> #include <linux/slab.h>
> #include <linux/timer.h>
>
>-#include "kcs_bmc.h"
>+#include "kcs_bmc_device.h"
>
>
> #define DEVICE_NAME     "ast-kcs-bmc"
>@@ -220,14 +220,22 @@ static void aspeed_kcs_enable_channel(struct kcs_bmc=
 *kcs_bmc, bool enable)
> 	}
> }
>
>+static const struct kcs_bmc_device_ops aspeed_kcs_ops =3D {
>+	.io_inputb =3D aspeed_kcs_inb,
>+	.io_outputb =3D aspeed_kcs_outb,
>+	.io_updateb =3D aspeed_kcs_updateb,
>+};
>+
> static irqreturn_t aspeed_kcs_irq(int irq, void *arg)
> {
> 	struct kcs_bmc *kcs_bmc =3D arg;
>+	int rc;
>
>-	if (!kcs_bmc_handle_event(kcs_bmc))
>-		return IRQ_HANDLED;
>+	rc =3D kcs_bmc_handle_event(kcs_bmc);
>+	if (rc < 0)
>+		dev_warn(kcs_bmc->dev, "Failed to service irq: %d\n", rc);
>
>-	return IRQ_NONE;
>+	return rc =3D=3D KCS_BMC_EVENT_HANDLED ? IRQ_HANDLED : IRQ_NONE;
> }
>
> static int aspeed_kcs_config_irq(struct kcs_bmc *kcs_bmc,
>@@ -362,9 +370,7 @@ static int aspeed_kcs_probe(struct platform_device *pd=
ev)
> 	kcs_bmc->dev =3D &pdev->dev;
> 	kcs_bmc->channel =3D channel;
> 	kcs_bmc->ioreg =3D ast_kcs_bmc_ioregs[channel - 1];
>-	kcs_bmc->io_inputb =3D aspeed_kcs_inb;
>-	kcs_bmc->io_outputb =3D aspeed_kcs_outb;
>-	kcs_bmc->io_updateb =3D aspeed_kcs_updateb;
>+	kcs_bmc->ops =3D &aspeed_kcs_ops;
>
> 	priv->map =3D syscon_node_to_regmap(pdev->dev.parent->of_node);
> 	if (IS_ERR(priv->map)) {
>diff --git a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c b/drivers/char/ipmi/kcs=
_bmc_cdev_ipmi.c
>index 0ca71c135a1a..fd852d8abe48 100644
>--- a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
>+++ b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
>@@ -22,7 +22,6 @@
>
> #define KCS_ZERO_DATA     0
>
>-
> /* IPMI 2.0 - Table 9-1, KCS Interface Status Register Bits */
> #define KCS_STATUS_STATE(state) (state << 6)
> #define KCS_STATUS_STATE_MASK   GENMASK(7, 6)
>@@ -179,12 +178,19 @@ static void kcs_bmc_ipmi_handle_cmd(struct kcs_bmc *=
kcs_bmc)
> 	}
> }
>
>-int kcs_bmc_ipmi_event(struct kcs_bmc *kcs_bmc);
>-int kcs_bmc_ipmi_event(struct kcs_bmc *kcs_bmc)
>+static inline struct kcs_bmc *client_to_kcs_bmc(struct kcs_bmc_client *cl=
ient)
> {
>+	return container_of(client, struct kcs_bmc, client);
>+}
>+
>+static int kcs_bmc_ipmi_event(struct kcs_bmc_client *client)
>+{
>+	struct kcs_bmc *kcs_bmc;
> 	unsigned long flags;
>-	int ret =3D -ENODATA;
> 	u8 status;
>+	int ret;
>+
>+	kcs_bmc =3D client_to_kcs_bmc(client);
>
> 	spin_lock_irqsave(&kcs_bmc->lock, flags);
>
>@@ -197,23 +203,28 @@ int kcs_bmc_ipmi_event(struct kcs_bmc *kcs_bmc)
> 		else
> 			kcs_bmc_ipmi_handle_data(kcs_bmc);
>
>-		ret =3D 0;
>+		ret =3D KCS_BMC_EVENT_HANDLED;
>+	} else {
>+		ret =3D KCS_BMC_EVENT_NONE;
> 	}
>
> 	spin_unlock_irqrestore(&kcs_bmc->lock, flags);
>
> 	return ret;
> }
>-EXPORT_SYMBOL(kcs_bmc_ipmi_event);
>
>-static inline struct kcs_bmc *to_kcs_bmc(struct file *filp)
>+static const struct kcs_bmc_client_ops kcs_bmc_ipmi_client_ops =3D {
>+	.event =3D kcs_bmc_ipmi_event,
>+};
>+
>+static inline struct kcs_bmc *file_to_kcs_bmc(struct file *filp)
> {
> 	return container_of(filp->private_data, struct kcs_bmc, miscdev);
> }
>
> static int kcs_bmc_ipmi_open(struct inode *inode, struct file *filp)
> {
>-	struct kcs_bmc *kcs_bmc =3D to_kcs_bmc(filp);
>+	struct kcs_bmc *kcs_bmc =3D file_to_kcs_bmc(filp);
> 	int ret =3D 0;
>
> 	spin_lock_irq(&kcs_bmc->lock);
>@@ -228,7 +239,7 @@ static int kcs_bmc_ipmi_open(struct inode *inode, stru=
ct file *filp)
>
> static __poll_t kcs_bmc_ipmi_poll(struct file *filp, poll_table *wait)
> {
>-	struct kcs_bmc *kcs_bmc =3D to_kcs_bmc(filp);
>+	struct kcs_bmc *kcs_bmc =3D file_to_kcs_bmc(filp);
> 	__poll_t mask =3D 0;
>
> 	poll_wait(filp, &kcs_bmc->queue, wait);
>@@ -244,7 +255,7 @@ static __poll_t kcs_bmc_ipmi_poll(struct file *filp, p=
oll_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 =3D to_kcs_bmc(filp);
>+	struct kcs_bmc *kcs_bmc =3D file_to_kcs_bmc(filp);
> 	bool data_avail;
> 	size_t data_len;
> 	ssize_t ret;
>@@ -306,7 +317,7 @@ static ssize_t kcs_bmc_ipmi_read(struct file *filp, ch=
ar __user *buf,
> static ssize_t kcs_bmc_ipmi_write(struct file *filp, const char __user *b=
uf,
> 			     size_t count, loff_t *ppos)
> {
>-	struct kcs_bmc *kcs_bmc =3D to_kcs_bmc(filp);
>+	struct kcs_bmc *kcs_bmc =3D file_to_kcs_bmc(filp);
> 	ssize_t ret;
>
> 	/* a minimum response size '3' : netfn + cmd + ccode */
>@@ -342,7 +353,7 @@ static ssize_t kcs_bmc_ipmi_write(struct file *filp, c=
onst char __user *buf,
> static long kcs_bmc_ipmi_ioctl(struct file *filp, unsigned int cmd,
> 			  unsigned long arg)
> {
>-	struct kcs_bmc *kcs_bmc =3D to_kcs_bmc(filp);
>+	struct kcs_bmc *kcs_bmc =3D file_to_kcs_bmc(filp);
> 	long ret =3D 0;
>
> 	spin_lock_irq(&kcs_bmc->lock);
>@@ -372,7 +383,7 @@ static long kcs_bmc_ipmi_ioctl(struct file *filp, unsi=
gned int cmd,
>
> static int kcs_bmc_ipmi_release(struct inode *inode, struct file *filp)
> {
>-	struct kcs_bmc *kcs_bmc =3D to_kcs_bmc(filp);
>+	struct kcs_bmc *kcs_bmc =3D file_to_kcs_bmc(filp);
>
> 	spin_lock_irq(&kcs_bmc->lock);
> 	kcs_bmc->running =3D 0;
>@@ -401,6 +412,8 @@ int kcs_bmc_ipmi_attach_cdev(struct kcs_bmc *kcs_bmc)
> 	mutex_init(&kcs_bmc->mutex);
> 	init_waitqueue_head(&kcs_bmc->queue);
>
>+	kcs_bmc->client.dev =3D kcs_bmc;
>+	kcs_bmc->client.ops =3D &kcs_bmc_ipmi_client_ops;
> 	kcs_bmc->data_in =3D devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERN=
EL);
> 	kcs_bmc->data_out =3D devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KER=
NEL);
> 	kcs_bmc->kbuffer =3D devm_kmalloc(kcs_bmc->dev, KCS_MSG_BUFSIZ, GFP_KERN=
EL);
>diff --git a/drivers/char/ipmi/kcs_bmc_client.h b/drivers/char/ipmi/kcs_bm=
c_client.h
>new file mode 100644
>index 000000000000..140631d157d8
>--- /dev/null
>+++ b/drivers/char/ipmi/kcs_bmc_client.h
>@@ -0,0 +1,29 @@
>+/* SPDX-License-Identifier: GPL-2.0 */
>+/* Copyright (c) 2021, IBM Corp. */
>+
>+#ifndef __KCS_BMC_CONSUMER_H__
>+#define __KCS_BMC_CONSUMER_H__
>+
>+#include <linux/list.h>
>+#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;
>+};
>+
>+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);
>+#endif
>diff --git a/drivers/char/ipmi/kcs_bmc_device.h b/drivers/char/ipmi/kcs_bm=
c_device.h
>new file mode 100644
>index 000000000000..33462174516d
>--- /dev/null
>+++ b/drivers/char/ipmi/kcs_bmc_device.h
>@@ -0,0 +1,19 @@
>+/* SPDX-License-Identifier: GPL-2.0 */
>+/* Copyright (c) 2021, IBM Corp. */
>+
>+#ifndef __KCS_BMC_DEVICE_H__
>+#define __KCS_BMC_DEVICE_H__
>+
>+#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);
>+};
>+
>+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);
>+
>+#endif
>diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c b/drivers/char/ipmi/kcs_b=
mc_npcm7xx.c
>index 5d017498dc69..1d21697fc585 100644
>--- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>+++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
>@@ -17,7 +17,7 @@
> #include <linux/regmap.h>
> #include <linux/slab.h>
>
>-#include "kcs_bmc.h"
>+#include "kcs_bmc_device.h"
>
> #define DEVICE_NAME	"npcm-kcs-bmc"
> #define KCS_CHANNEL_MAX	3
>@@ -127,11 +127,13 @@ static void npcm7xx_kcs_enable_channel(struct kcs_bm=
c *kcs_bmc, bool enable)
> static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)
> {
> 	struct kcs_bmc *kcs_bmc =3D arg;
>+	int rc;
>
>-	if (!kcs_bmc_handle_event(kcs_bmc))
>-		return IRQ_HANDLED;
>+	rc =3D kcs_bmc_handle_event(kcs_bmc);
>+	if (rc < 0)
>+		dev_warn(kcs_bmc->dev, "Failed to service irq: %d\n", rc);
>
>-	return IRQ_NONE;
>+	return rc =3D=3D KCS_BMC_EVENT_HANDLED ? IRQ_HANDLED : IRQ_NONE;
> }
>
> static int npcm7xx_kcs_config_irq(struct kcs_bmc *kcs_bmc,
>@@ -148,6 +150,12 @@ static int npcm7xx_kcs_config_irq(struct kcs_bmc *kcs=
_bmc,
> 				dev_name(dev), kcs_bmc);
> }
>
>+static const struct kcs_bmc_device_ops npcm7xx_kcs_ops =3D {
>+	.io_inputb =3D npcm7xx_kcs_inb,
>+	.io_outputb =3D npcm7xx_kcs_outb,
>+	.io_updateb =3D npcm7xx_kcs_updateb,
>+};
>+
> static int npcm7xx_kcs_probe(struct platform_device *pdev)
> {
> 	struct device *dev =3D &pdev->dev;
>@@ -179,9 +187,7 @@ static int npcm7xx_kcs_probe(struct platform_device *p=
dev)
> 	kcs_bmc->ioreg.idr =3D priv->reg->dib;
> 	kcs_bmc->ioreg.odr =3D priv->reg->dob;
> 	kcs_bmc->ioreg.str =3D priv->reg->sts;
>-	kcs_bmc->io_inputb =3D npcm7xx_kcs_inb;
>-	kcs_bmc->io_outputb =3D npcm7xx_kcs_outb;
>-	kcs_bmc->io_updateb =3D npcm7xx_kcs_updateb;
>+	kcs_bmc->ops =3D &npcm7xx_kcs_ops;
>
> 	platform_set_drvdata(pdev, priv);
>
>--=20
>2.27.0
>=