2021-06-08 23:42:04

by Andrew Jeffery

[permalink] [raw]
Subject: [PATCH v4 00/16] ipmi: Allow raw access to KCS devices


Hello,

This is the 4th spin of the series refactoring the keyboard-controller-style
device drivers in the IPMI subsystem.

v3 can be found at:

https://lore.kernel.org/lkml/[email protected]/

v4:

* Makes kcs_bmc_add_device() return an error if no client successfully
initialised with respect to the binding of the device driver
* Retains the existing single-open semantics (v3 allowed multiple-open)
* Fixes the OBE macro for the NPCM7xx KCS driver
* Cleans up Yoda-style masks (mask constant on the LHS rather than RHS)
* Cleans up includes in kcs_bmc_client.h
* Adds some comments to the SerIO adapter to clarify object lifetimes

Previously:

Changes in v3:

* The series was rebased onto v5.13-rc1
* v5.13-rc1 includes Chiawei's patches reworking the LPC devicetree bindings,
so they're no-longer required in the series.
* After some discussion with Arnd[1] and investigating the serio subsystem,
I've replaced the "raw" KCS driver (patch 16/21 in v2) with a serio adaptor
(patch 11/16 in this series). The adaptor allows us to take advantage of the
existing chardevs provided by serio.

[1] https://lore.kernel.org/linux-arm-kernel/[email protected]/

Changes in v2 include:

* A rebase onto v5.12-rc2
* Incorporation of off-list feedback on SerIRQ configuration from
Chiawei
* Further validation on hardware for ASPEED KCS devices 2, 3 and 4
* Lifting the existing single-open constraint of the IPMI chardev
* Fixes addressing Rob's feedback on the conversion of the ASPEED KCS
binding to dt-schema
* Fixes addressing Rob's feedback on the new aspeed,lpc-interrupts
property definition for the ASPEED KCS binding

Please test and review!

Andrew

Andrew Jeffery (16):
ipmi: kcs_bmc_aspeed: Use of match data to extract KCS properties
ipmi: kcs_bmc: Make status update atomic
ipmi: kcs_bmc: Rename {read,write}_{status,data}() functions
ipmi: kcs_bmc: Split out kcs_bmc_cdev_ipmi
ipmi: kcs_bmc: Turn the driver data-structures inside-out
ipmi: kcs_bmc: Split headers into device and client
ipmi: kcs_bmc: Strip private client data from struct kcs_bmc
ipmi: kcs_bmc: Decouple the IPMI chardev from the core
ipmi: kcs_bmc: Allow clients to control KCS IRQ state
ipmi: kcs_bmc: Enable IBF on open
ipmi: kcs_bmc: Add serio adaptor
dt-bindings: ipmi: Convert ASPEED KCS binding to schema
dt-bindings: ipmi: Add optional SerIRQ property to ASPEED KCS devices
ipmi: kcs_bmc_aspeed: Implement KCS SerIRQ configuration
ipmi: kcs_bmc_aspeed: Fix IBFIE typo from datasheet
ipmi: kcs_bmc_aspeed: Optionally apply status address

.../bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml | 106 +++
.../bindings/ipmi/aspeed-kcs-bmc.txt | 33 -
drivers/char/ipmi/Kconfig | 27 +
drivers/char/ipmi/Makefile | 2 +
drivers/char/ipmi/kcs_bmc.c | 523 ++++-----------
drivers/char/ipmi/kcs_bmc.h | 92 +--
drivers/char/ipmi/kcs_bmc_aspeed.c | 633 +++++++++++++-----
drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 568 ++++++++++++++++
drivers/char/ipmi/kcs_bmc_client.h | 45 ++
drivers/char/ipmi/kcs_bmc_device.h | 22 +
drivers/char/ipmi/kcs_bmc_npcm7xx.c | 92 ++-
drivers/char/ipmi/kcs_bmc_serio.c | 157 +++++
12 files changed, 1594 insertions(+), 706 deletions(-)
create mode 100644 Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml
delete mode 100644 Documentation/devicetree/bindings/ipmi/aspeed-kcs-bmc.txt
create mode 100644 drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
create mode 100644 drivers/char/ipmi/kcs_bmc_client.h
create mode 100644 drivers/char/ipmi/kcs_bmc_device.h
create mode 100644 drivers/char/ipmi/kcs_bmc_serio.c

--
2.30.2


2021-06-08 23:42:52

by Andrew Jeffery

[permalink] [raw]
Subject: [PATCH v4 03/16] ipmi: kcs_bmc: Rename {read,write}_{status,data}() functions

Rename the functions in preparation for separating the IPMI chardev out
from the KCS BMC core.

Signed-off-by: Andrew Jeffery <[email protected]>
Reviewed-by: Zev Weiss <[email protected]>
---
drivers/char/ipmi/kcs_bmc.c | 52 ++++++++++++++++++-------------------
1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
index 58fb1a7bd50d..c4336c1f2d6d 100644
--- a/drivers/char/ipmi/kcs_bmc.c
+++ b/drivers/char/ipmi/kcs_bmc.c
@@ -45,42 +45,42 @@ enum kcs_states {
#define KCS_CMD_WRITE_END 0x62
#define KCS_CMD_READ_BYTE 0x68

-static inline u8 read_data(struct kcs_bmc *kcs_bmc)
+static inline u8 kcs_bmc_read_data(struct kcs_bmc *kcs_bmc)
{
return kcs_bmc->io_inputb(kcs_bmc, kcs_bmc->ioreg.idr);
}

-static inline void write_data(struct kcs_bmc *kcs_bmc, u8 data)
+static inline void kcs_bmc_write_data(struct kcs_bmc *kcs_bmc, u8 data)
{
kcs_bmc->io_outputb(kcs_bmc, kcs_bmc->ioreg.odr, data);
}

-static inline u8 read_status(struct kcs_bmc *kcs_bmc)
+static inline u8 kcs_bmc_read_status(struct kcs_bmc *kcs_bmc)
{
return kcs_bmc->io_inputb(kcs_bmc, kcs_bmc->ioreg.str);
}

-static inline void write_status(struct kcs_bmc *kcs_bmc, u8 data)
+static inline void kcs_bmc_write_status(struct kcs_bmc *kcs_bmc, u8 data)
{
kcs_bmc->io_outputb(kcs_bmc, kcs_bmc->ioreg.str, data);
}

-static void update_status_bits(struct kcs_bmc *kcs_bmc, u8 mask, u8 val)
+static 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);
}

static inline void set_state(struct kcs_bmc *kcs_bmc, u8 state)
{
- update_status_bits(kcs_bmc, KCS_STATUS_STATE_MASK,
+ kcs_bmc_update_status(kcs_bmc, KCS_STATUS_STATE_MASK,
KCS_STATUS_STATE(state));
}

static void kcs_force_abort(struct kcs_bmc *kcs_bmc)
{
set_state(kcs_bmc, ERROR_STATE);
- read_data(kcs_bmc);
- write_data(kcs_bmc, KCS_ZERO_DATA);
+ kcs_bmc_read_data(kcs_bmc);
+ kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);

kcs_bmc->phase = KCS_PHASE_ERROR;
kcs_bmc->data_in_avail = false;
@@ -99,9 +99,9 @@ static void kcs_bmc_handle_data(struct kcs_bmc *kcs_bmc)
case KCS_PHASE_WRITE_DATA:
if (kcs_bmc->data_in_idx < KCS_MSG_BUFSIZ) {
set_state(kcs_bmc, WRITE_STATE);
- write_data(kcs_bmc, KCS_ZERO_DATA);
+ kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
kcs_bmc->data_in[kcs_bmc->data_in_idx++] =
- read_data(kcs_bmc);
+ kcs_bmc_read_data(kcs_bmc);
} else {
kcs_force_abort(kcs_bmc);
kcs_bmc->error = KCS_LENGTH_ERROR;
@@ -112,7 +112,7 @@ static void kcs_bmc_handle_data(struct kcs_bmc *kcs_bmc)
if (kcs_bmc->data_in_idx < KCS_MSG_BUFSIZ) {
set_state(kcs_bmc, READ_STATE);
kcs_bmc->data_in[kcs_bmc->data_in_idx++] =
- read_data(kcs_bmc);
+ 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);
@@ -126,34 +126,34 @@ static void kcs_bmc_handle_data(struct kcs_bmc *kcs_bmc)
if (kcs_bmc->data_out_idx == kcs_bmc->data_out_len)
set_state(kcs_bmc, IDLE_STATE);

- data = read_data(kcs_bmc);
+ data = kcs_bmc_read_data(kcs_bmc);
if (data != KCS_CMD_READ_BYTE) {
set_state(kcs_bmc, ERROR_STATE);
- write_data(kcs_bmc, KCS_ZERO_DATA);
+ kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
break;
}

if (kcs_bmc->data_out_idx == kcs_bmc->data_out_len) {
- write_data(kcs_bmc, KCS_ZERO_DATA);
+ kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
kcs_bmc->phase = KCS_PHASE_IDLE;
break;
}

- write_data(kcs_bmc,
+ kcs_bmc_write_data(kcs_bmc,
kcs_bmc->data_out[kcs_bmc->data_out_idx++]);
break;

case KCS_PHASE_ABORT_ERROR1:
set_state(kcs_bmc, READ_STATE);
- read_data(kcs_bmc);
- write_data(kcs_bmc, kcs_bmc->error);
+ kcs_bmc_read_data(kcs_bmc);
+ kcs_bmc_write_data(kcs_bmc, kcs_bmc->error);
kcs_bmc->phase = KCS_PHASE_ABORT_ERROR2;
break;

case KCS_PHASE_ABORT_ERROR2:
set_state(kcs_bmc, IDLE_STATE);
- read_data(kcs_bmc);
- write_data(kcs_bmc, KCS_ZERO_DATA);
+ kcs_bmc_read_data(kcs_bmc);
+ kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
kcs_bmc->phase = KCS_PHASE_IDLE;
break;

@@ -168,9 +168,9 @@ static void kcs_bmc_handle_cmd(struct kcs_bmc *kcs_bmc)
u8 cmd;

set_state(kcs_bmc, WRITE_STATE);
- write_data(kcs_bmc, KCS_ZERO_DATA);
+ kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);

- cmd = read_data(kcs_bmc);
+ cmd = kcs_bmc_read_data(kcs_bmc);
switch (cmd) {
case KCS_CMD_WRITE_START:
kcs_bmc->phase = KCS_PHASE_WRITE_START;
@@ -212,7 +212,7 @@ int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc)

spin_lock_irqsave(&kcs_bmc->lock, flags);

- status = read_status(kcs_bmc);
+ status = kcs_bmc_read_status(kcs_bmc);
if (status & KCS_STATUS_IBF) {
if (!kcs_bmc->running)
kcs_force_abort(kcs_bmc);
@@ -350,7 +350,7 @@ static ssize_t kcs_bmc_write(struct file *filp, const char __user *buf,
kcs_bmc->data_out_idx = 1;
kcs_bmc->data_out_len = count;
memcpy(kcs_bmc->data_out, kcs_bmc->kbuffer, count);
- write_data(kcs_bmc, kcs_bmc->data_out[0]);
+ kcs_bmc_write_data(kcs_bmc, kcs_bmc->data_out[0]);
ret = count;
} else {
ret = -EINVAL;
@@ -373,13 +373,11 @@ static long kcs_bmc_ioctl(struct file *filp, unsigned int cmd,

switch (cmd) {
case IPMI_BMC_IOCTL_SET_SMS_ATN:
- update_status_bits(kcs_bmc, KCS_STATUS_SMS_ATN,
- KCS_STATUS_SMS_ATN);
+ kcs_bmc_update_status(kcs_bmc, KCS_STATUS_SMS_ATN, KCS_STATUS_SMS_ATN);
break;

case IPMI_BMC_IOCTL_CLEAR_SMS_ATN:
- update_status_bits(kcs_bmc, KCS_STATUS_SMS_ATN,
- 0);
+ kcs_bmc_update_status(kcs_bmc, KCS_STATUS_SMS_ATN, 0);
break;

case IPMI_BMC_IOCTL_FORCE_ABORT:
--
2.30.2

2021-06-08 23:43:49

by Andrew Jeffery

[permalink] [raw]
Subject: [PATCH v4 11/16] ipmi: kcs_bmc: Add serio adaptor

kcs_bmc_serio acts as a bridge between the KCS drivers in the IPMI
subsystem and the existing userspace interfaces available through the
serio subsystem. This is useful when userspace would like to make use of
the BMC KCS devices for purposes that aren't IPMI.

Signed-off-by: Andrew Jeffery <[email protected]>
---
drivers/char/ipmi/Kconfig | 14 +++
drivers/char/ipmi/Makefile | 1 +
drivers/char/ipmi/kcs_bmc_serio.c | 157 ++++++++++++++++++++++++++++++
3 files changed, 172 insertions(+)
create mode 100644 drivers/char/ipmi/kcs_bmc_serio.c

diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index bc5f81899b62..249b31197eea 100644
--- a/drivers/char/ipmi/Kconfig
+++ b/drivers/char/ipmi/Kconfig
@@ -137,6 +137,20 @@ config IPMI_KCS_BMC_CDEV_IPMI
This support is also available as a module. The module will be
called kcs_bmc_cdev_ipmi.

+config IPMI_KCS_BMC_SERIO
+ depends on IPMI_KCS_BMC && SERIO
+ tristate "SerIO adaptor for BMC KCS devices"
+ help
+ Adapts the BMC KCS device for the SerIO subsystem. This allows users
+ to take advantage of userspace interfaces provided by SerIO where
+ appropriate.
+
+ Say YES if you wish to expose KCS devices on the BMC via SerIO
+ interfaces.
+
+ This support is also available as a module. The module will be
+ called kcs_bmc_serio.
+
config ASPEED_BT_IPMI_BMC
depends on ARCH_ASPEED || COMPILE_TEST
depends on REGMAP && REGMAP_MMIO && MFD_SYSCON
diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
index fcfa676afddb..84f47d18007f 100644
--- a/drivers/char/ipmi/Makefile
+++ b/drivers/char/ipmi/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_IPMI_POWERNV) += ipmi_powernv.o
obj-$(CONFIG_IPMI_WATCHDOG) += ipmi_watchdog.o
obj-$(CONFIG_IPMI_POWEROFF) += ipmi_poweroff.o
obj-$(CONFIG_IPMI_KCS_BMC) += kcs_bmc.o
+obj-$(CONFIG_IPMI_KCS_BMC_SERIO) += kcs_bmc_serio.o
obj-$(CONFIG_IPMI_KCS_BMC_CDEV_IPMI) += kcs_bmc_cdev_ipmi.o
obj-$(CONFIG_ASPEED_BT_IPMI_BMC) += bt-bmc.o
obj-$(CONFIG_ASPEED_KCS_IPMI_BMC) += kcs_bmc_aspeed.o
diff --git a/drivers/char/ipmi/kcs_bmc_serio.c b/drivers/char/ipmi/kcs_bmc_serio.c
new file mode 100644
index 000000000000..7948cabde50b
--- /dev/null
+++ b/drivers/char/ipmi/kcs_bmc_serio.c
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright (c) 2021 IBM Corp. */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/sched/signal.h>
+#include <linux/serio.h>
+#include <linux/slab.h>
+
+#include "kcs_bmc_client.h"
+
+struct kcs_bmc_serio {
+ struct list_head entry;
+
+ struct kcs_bmc_client client;
+ struct serio *port;
+
+ spinlock_t lock;
+};
+
+static inline struct kcs_bmc_serio *client_to_kcs_bmc_serio(struct kcs_bmc_client *client)
+{
+ return container_of(client, struct kcs_bmc_serio, client);
+}
+
+static irqreturn_t kcs_bmc_serio_event(struct kcs_bmc_client *client)
+{
+ struct kcs_bmc_serio *priv;
+ u8 handled = IRQ_NONE;
+ u8 status;
+
+ priv = client_to_kcs_bmc_serio(client);
+
+ spin_lock(&priv->lock);
+
+ status = kcs_bmc_read_status(client->dev);
+
+ if (status & KCS_BMC_STR_IBF)
+ handled = serio_interrupt(priv->port, kcs_bmc_read_data(client->dev), 0);
+
+ spin_unlock(&priv->lock);
+
+ return handled;
+}
+
+static const struct kcs_bmc_client_ops kcs_bmc_serio_client_ops = {
+ .event = kcs_bmc_serio_event,
+};
+
+static int kcs_bmc_serio_open(struct serio *port)
+{
+ struct kcs_bmc_serio *priv = port->port_data;
+
+ return kcs_bmc_enable_device(priv->client.dev, &priv->client);
+}
+
+static void kcs_bmc_serio_close(struct serio *port)
+{
+ struct kcs_bmc_serio *priv = port->port_data;
+
+ kcs_bmc_disable_device(priv->client.dev, &priv->client);
+}
+
+static DEFINE_SPINLOCK(kcs_bmc_serio_instances_lock);
+static LIST_HEAD(kcs_bmc_serio_instances);
+
+static int kcs_bmc_serio_add_device(struct kcs_bmc_device *kcs_bmc)
+{
+ struct kcs_bmc_serio *priv;
+ struct serio *port;
+
+ priv = devm_kzalloc(kcs_bmc->dev, sizeof(*priv), GFP_KERNEL);
+
+ /* Use kzalloc() as the allocation is cleaned up with kfree() via serio_unregister_port() */
+ port = kzalloc(sizeof(*port), GFP_KERNEL);
+ if (!(priv && port))
+ return -ENOMEM;
+
+ port->id.type = SERIO_8042;
+ port->open = kcs_bmc_serio_open;
+ port->close = kcs_bmc_serio_close;
+ port->port_data = priv;
+ port->dev.parent = kcs_bmc->dev;
+
+ spin_lock_init(&priv->lock);
+ priv->port = port;
+ priv->client.dev = kcs_bmc;
+ priv->client.ops = &kcs_bmc_serio_client_ops;
+
+ spin_lock_irq(&kcs_bmc_serio_instances_lock);
+ list_add(&priv->entry, &kcs_bmc_serio_instances);
+ spin_unlock_irq(&kcs_bmc_serio_instances_lock);
+
+ serio_register_port(port);
+
+ dev_info(kcs_bmc->dev, "Initialised serio client for channel %d", kcs_bmc->channel);
+
+ return 0;
+}
+
+static int kcs_bmc_serio_remove_device(struct kcs_bmc_device *kcs_bmc)
+{
+ struct kcs_bmc_serio *priv = NULL, *pos;
+
+ spin_lock_irq(&kcs_bmc_serio_instances_lock);
+ list_for_each_entry(pos, &kcs_bmc_serio_instances, entry) {
+ if (pos->client.dev == kcs_bmc) {
+ priv = pos;
+ list_del(&pos->entry);
+ break;
+ }
+ }
+ spin_unlock_irq(&kcs_bmc_serio_instances_lock);
+
+ if (!priv)
+ return -ENODEV;
+
+ /* kfree()s priv->port via put_device() */
+ serio_unregister_port(priv->port);
+
+ /* Ensure the IBF IRQ is disabled if we were the active client */
+ kcs_bmc_disable_device(kcs_bmc, &priv->client);
+
+ devm_kfree(priv->client.dev->dev, priv);
+
+ return 0;
+}
+
+static const struct kcs_bmc_driver_ops kcs_bmc_serio_driver_ops = {
+ .add_device = kcs_bmc_serio_add_device,
+ .remove_device = kcs_bmc_serio_remove_device,
+};
+
+static struct kcs_bmc_driver kcs_bmc_serio_driver = {
+ .ops = &kcs_bmc_serio_driver_ops,
+};
+
+static int kcs_bmc_serio_init(void)
+{
+ kcs_bmc_register_driver(&kcs_bmc_serio_driver);
+
+ return 0;
+}
+module_init(kcs_bmc_serio_init);
+
+static void kcs_bmc_serio_exit(void)
+{
+ kcs_bmc_unregister_driver(&kcs_bmc_serio_driver);
+}
+module_exit(kcs_bmc_serio_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Andrew Jeffery <[email protected]>");
+MODULE_DESCRIPTION("Adapter driver for serio access to BMC KCS devices");
--
2.30.2

2021-06-08 23:45:37

by Andrew Jeffery

[permalink] [raw]
Subject: [PATCH v4 06/16] ipmi: kcs_bmc: Split headers into device and client

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 <[email protected]>
---
drivers/char/ipmi/kcs_bmc.c | 23 ++++++++++------
drivers/char/ipmi/kcs_bmc.h | 27 +++++++++----------
drivers/char/ipmi/kcs_bmc_aspeed.c | 17 ++++++------
drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 39 ++++++++++++++++++---------
drivers/char/ipmi/kcs_bmc_client.h | 27 +++++++++++++++++++
drivers/char/ipmi/kcs_bmc_device.h | 19 +++++++++++++
drivers/char/ipmi/kcs_bmc_npcm7xx.c | 17 ++++++------
7 files changed, 117 insertions(+), 52 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 07bb6747f29a..c347cf6f9337 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)
+irqreturn_t 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);

@@ -62,4 +68,5 @@ EXPORT_SYMBOL(kcs_bmc_remove_device);

MODULE_LICENSE("GPL v2");
MODULE_AUTHOR("Haiyue Wang <[email protected]>");
+MODULE_AUTHOR("Andrew Jeffery <[email protected]>");
MODULE_DESCRIPTION("KCS BMC to handle the IPMI request from system software");
diff --git a/drivers/char/ipmi/kcs_bmc.h b/drivers/char/ipmi/kcs_bmc.h
index f3ed89e7da98..f42843d240ed 100644
--- a/drivers/char/ipmi/kcs_bmc.h
+++ b/drivers/char/ipmi/kcs_bmc.h
@@ -8,6 +8,12 @@

#include <linux/miscdevice.h>

+#include "kcs_bmc_client.h"
+
+#define KCS_BMC_STR_OBF BIT(0)
+#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.
@@ -66,19 +72,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 +105,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);
-void 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_bmc_aspeed.c
index b07cbc423dd5..fdfba745302a 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,17 @@ static void aspeed_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable)
}
}

+static const struct kcs_bmc_device_ops aspeed_kcs_ops = {
+ .io_inputb = aspeed_kcs_inb,
+ .io_outputb = aspeed_kcs_outb,
+ .io_updateb = aspeed_kcs_updateb,
+};
+
static irqreturn_t aspeed_kcs_irq(int irq, void *arg)
{
struct kcs_bmc *kcs_bmc = arg;

- if (!kcs_bmc_handle_event(kcs_bmc))
- return IRQ_HANDLED;
-
- return IRQ_NONE;
+ return kcs_bmc_handle_event(kcs_bmc);
}

static int aspeed_kcs_config_irq(struct kcs_bmc *kcs_bmc,
@@ -364,9 +367,7 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
kcs_bmc->dev = &pdev->dev;
kcs_bmc->channel = channel;
kcs_bmc->ioreg = ast_kcs_bmc_ioregs[channel - 1];
- kcs_bmc->io_inputb = aspeed_kcs_inb;
- kcs_bmc->io_outputb = aspeed_kcs_outb;
- kcs_bmc->io_updateb = aspeed_kcs_updateb;
+ kcs_bmc->ops = &aspeed_kcs_ops;

priv->map = 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 5060643bf530..476ad6d541d5 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 *client)
{
+ return container_of(client, struct kcs_bmc, client);
+}
+
+static irqreturn_t kcs_bmc_ipmi_event(struct kcs_bmc_client *client)
+{
+ struct kcs_bmc *kcs_bmc;
unsigned long flags;
- int ret = -ENODATA;
u8 status;
+ int ret;
+
+ kcs_bmc = 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 = 0;
+ ret = IRQ_HANDLED;
+ } else {
+ ret = IRQ_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 = {
+ .event = 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 = to_kcs_bmc(filp);
+ struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
int ret = 0;

spin_lock_irq(&kcs_bmc->lock);
@@ -228,7 +239,7 @@ static int kcs_bmc_ipmi_open(struct inode *inode, struct file *filp)

static __poll_t kcs_bmc_ipmi_poll(struct file *filp, poll_table *wait)
{
- struct kcs_bmc *kcs_bmc = to_kcs_bmc(filp);
+ struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
__poll_t mask = 0;

poll_wait(filp, &kcs_bmc->queue, wait);
@@ -244,7 +255,7 @@ 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 = to_kcs_bmc(filp);
+ struct kcs_bmc *kcs_bmc = 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, 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 = to_kcs_bmc(filp);
+ struct kcs_bmc *kcs_bmc = 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, const char __user *buf,
static long kcs_bmc_ipmi_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
{
- struct kcs_bmc *kcs_bmc = to_kcs_bmc(filp);
+ struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);
long ret = 0;

spin_lock_irq(&kcs_bmc->lock);
@@ -372,7 +383,7 @@ static long kcs_bmc_ipmi_ioctl(struct file *filp, unsigned int cmd,

static int kcs_bmc_ipmi_release(struct inode *inode, struct file *filp)
{
- struct kcs_bmc *kcs_bmc = to_kcs_bmc(filp);
+ struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp);

spin_lock_irq(&kcs_bmc->lock);
kcs_bmc->running = 0;
@@ -401,6 +412,8 @@ int kcs_bmc_ipmi_add_device(struct kcs_bmc *kcs_bmc)
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);
diff --git a/drivers/char/ipmi/kcs_bmc_client.h b/drivers/char/ipmi/kcs_bmc_client.h
new file mode 100644
index 000000000000..dad8774aebce
--- /dev/null
+++ b/drivers/char/ipmi/kcs_bmc_client.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2021, IBM Corp. */
+
+#ifndef __KCS_BMC_CONSUMER_H__
+#define __KCS_BMC_CONSUMER_H__
+
+#include <linux/irqreturn.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 {
+ irqreturn_t (*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_bmc_device.h
new file mode 100644
index 000000000000..dd8bf1307ad2
--- /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);
+};
+
+irqreturn_t kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc);
+int kcs_bmc_add_device(struct kcs_bmc *kcs_bmc);
+void 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_bmc_npcm7xx.c
index e06250285113..ebb691af28c5 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
@@ -128,10 +128,7 @@ static irqreturn_t npcm7xx_kcs_irq(int irq, void *arg)
{
struct kcs_bmc *kcs_bmc = arg;

- if (!kcs_bmc_handle_event(kcs_bmc))
- return IRQ_HANDLED;
-
- return IRQ_NONE;
+ return kcs_bmc_handle_event(kcs_bmc);
}

static int npcm7xx_kcs_config_irq(struct kcs_bmc *kcs_bmc,
@@ -148,6 +145,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 = {
+ .io_inputb = npcm7xx_kcs_inb,
+ .io_outputb = npcm7xx_kcs_outb,
+ .io_updateb = npcm7xx_kcs_updateb,
+};
+
static int npcm7xx_kcs_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -179,9 +182,7 @@ static int npcm7xx_kcs_probe(struct platform_device *pdev)
kcs_bmc->ioreg.idr = priv->reg->dib;
kcs_bmc->ioreg.odr = priv->reg->dob;
kcs_bmc->ioreg.str = priv->reg->sts;
- kcs_bmc->io_inputb = npcm7xx_kcs_inb;
- kcs_bmc->io_outputb = npcm7xx_kcs_outb;
- kcs_bmc->io_updateb = npcm7xx_kcs_updateb;
+ kcs_bmc->ops = &npcm7xx_kcs_ops;

platform_set_drvdata(pdev, priv);

--
2.30.2

2021-06-09 00:47:10

by Andrew Jeffery

[permalink] [raw]
Subject: [PATCH v4 16/16] ipmi: kcs_bmc_aspeed: Optionally apply status address

Some Aspeed KCS devices can derive the status register address from the
address of the data register. As such, the address of the status
register can be implicit in the configuration if desired. On the other
hand, sometimes address schemes might be requested that are incompatible
with the default addressing scheme. Allow these requests where possible
if the devicetree specifies the status register address.

Signed-off-by: Andrew Jeffery <[email protected]>
Reviewed-by: Chia-Wei Wang <[email protected]>
---
drivers/char/ipmi/kcs_bmc_aspeed.c | 116 +++++++++++++++++++++--------
1 file changed, 83 insertions(+), 33 deletions(-)

diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c b/drivers/char/ipmi/kcs_bmc_aspeed.c
index 461cb2c9cc7e..0401089f8895 100644
--- a/drivers/char/ipmi/kcs_bmc_aspeed.c
+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
@@ -83,6 +83,8 @@
#define LPC_STR2 0x040
#define LPC_STR3 0x044
#define LPC_HICRB 0x100
+#define LPC_HICRB_EN16LADR2 BIT(5)
+#define LPC_HICRB_EN16LADR1 BIT(4)
#define LPC_HICRB_IBFIE4 BIT(1)
#define LPC_HICRB_LPC4E BIT(0)
#define LPC_HICRC 0x104
@@ -96,6 +98,11 @@
#define LPC_IDR4 0x114
#define LPC_ODR4 0x118
#define LPC_STR4 0x11C
+#define LPC_LSADR12 0x120
+#define LPC_LSADR12_LSADR2_MASK GENMASK(31, 16)
+#define LPC_LSADR12_LSADR2_SHIFT 16
+#define LPC_LSADR12_LSADR1_MASK GENMASK(15, 0)
+#define LPC_LSADR12_LSADR1_SHIFT 0

#define OBE_POLL_PERIOD (HZ / 2)

@@ -123,7 +130,7 @@ struct aspeed_kcs_bmc {

struct aspeed_kcs_of_ops {
int (*get_channel)(struct platform_device *pdev);
- int (*get_io_address)(struct platform_device *pdev);
+ int (*get_io_address)(struct platform_device *pdev, u32 addrs[2]);
};

static inline struct aspeed_kcs_bmc *to_aspeed_kcs_bmc(struct kcs_bmc_device *kcs_bmc)
@@ -217,38 +224,64 @@ static void aspeed_kcs_updateb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 mask,
* C. KCS4
* D / C : CA4h / CA5h
*/
-static void aspeed_kcs_set_address(struct kcs_bmc_device *kcs_bmc, u16 addr)
+static int aspeed_kcs_set_address(struct kcs_bmc_device *kcs_bmc, u32 addrs[2], int nr_addrs)
{
struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);

- switch (kcs_bmc->channel) {
+ if (WARN_ON(nr_addrs < 1 || nr_addrs > 2))
+ return -EINVAL;
+
+ switch (priv->kcs_bmc.channel) {
case 1:
- regmap_update_bits(priv->map, LPC_HICR4,
- LPC_HICR4_LADR12AS, 0);
- regmap_write(priv->map, LPC_LADR12H, addr >> 8);
- regmap_write(priv->map, LPC_LADR12L, addr & 0xFF);
+ regmap_update_bits(priv->map, LPC_HICR4, LPC_HICR4_LADR12AS, 0);
+ regmap_write(priv->map, LPC_LADR12H, addrs[0] >> 8);
+ regmap_write(priv->map, LPC_LADR12L, addrs[0] & 0xFF);
+ if (nr_addrs == 2) {
+ regmap_update_bits(priv->map, LPC_LSADR12, LPC_LSADR12_LSADR1_MASK,
+ addrs[1] << LPC_LSADR12_LSADR1_SHIFT);
+
+ regmap_update_bits(priv->map, LPC_HICRB, LPC_HICRB_EN16LADR1,
+ LPC_HICRB_EN16LADR1);
+ }
break;

case 2:
- regmap_update_bits(priv->map, LPC_HICR4,
- LPC_HICR4_LADR12AS, LPC_HICR4_LADR12AS);
- regmap_write(priv->map, LPC_LADR12H, addr >> 8);
- regmap_write(priv->map, LPC_LADR12L, addr & 0xFF);
+ regmap_update_bits(priv->map, LPC_HICR4, LPC_HICR4_LADR12AS, LPC_HICR4_LADR12AS);
+ regmap_write(priv->map, LPC_LADR12H, addrs[0] >> 8);
+ regmap_write(priv->map, LPC_LADR12L, addrs[0] & 0xFF);
+ if (nr_addrs == 2) {
+ regmap_update_bits(priv->map, LPC_LSADR12, LPC_LSADR12_LSADR2_MASK,
+ addrs[1] << LPC_LSADR12_LSADR2_SHIFT);
+
+ regmap_update_bits(priv->map, LPC_HICRB, LPC_HICRB_EN16LADR2,
+ LPC_HICRB_EN16LADR2);
+ }
break;

case 3:
- regmap_write(priv->map, LPC_LADR3H, addr >> 8);
- regmap_write(priv->map, LPC_LADR3L, addr & 0xFF);
+ if (nr_addrs == 2) {
+ dev_err(priv->kcs_bmc.dev,
+ "Channel 3 only supports inferred status IO address\n");
+ return -EINVAL;
+ }
+
+ regmap_write(priv->map, LPC_LADR3H, addrs[0] >> 8);
+ regmap_write(priv->map, LPC_LADR3L, addrs[0] & 0xFF);
break;

case 4:
- regmap_write(priv->map, LPC_LADR4, ((addr + 1) << 16) |
- addr);
+ if (nr_addrs == 1)
+ regmap_write(priv->map, LPC_LADR4, ((addrs[0] + 1) << 16) | addrs[0]);
+ else
+ regmap_write(priv->map, LPC_LADR4, (addrs[1] << 16) | addrs[0]);
+
break;

default:
- break;
+ return -EINVAL;
}
+
+ return 0;
}

static inline int aspeed_kcs_map_serirq_type(u32 dt_type)
@@ -457,18 +490,18 @@ static int aspeed_kcs_of_v1_get_channel(struct platform_device *pdev)
return channel;
}

-static int aspeed_kcs_of_v1_get_io_address(struct platform_device *pdev)
+static int
+aspeed_kcs_of_v1_get_io_address(struct platform_device *pdev, u32 addrs[2])
{
- u32 slave;
int rc;

- rc = of_property_read_u32(pdev->dev.of_node, "kcs_addr", &slave);
- if (rc || slave > 0xffff) {
+ rc = of_property_read_u32(pdev->dev.of_node, "kcs_addr", addrs);
+ if (rc || addrs[0] > 0xffff) {
dev_err(&pdev->dev, "no valid 'kcs_addr' configured\n");
return -EINVAL;
}

- return slave;
+ return 1;
}

static int aspeed_kcs_of_v2_get_channel(struct platform_device *pdev)
@@ -504,18 +537,30 @@ static int aspeed_kcs_of_v2_get_channel(struct platform_device *pdev)
return -EINVAL;
}

-static int aspeed_kcs_of_v2_get_io_address(struct platform_device *pdev)
+static int
+aspeed_kcs_of_v2_get_io_address(struct platform_device *pdev, u32 addrs[2])
{
- uint32_t slave;
int rc;

- rc = of_property_read_u32(pdev->dev.of_node, "aspeed,lpc-io-reg", &slave);
- if (rc || slave > 0xffff) {
- dev_err(&pdev->dev, "no valid 'aspeed,lpc-io-reg' configured\n");
+ rc = of_property_read_variable_u32_array(pdev->dev.of_node,
+ "aspeed,lpc-io-reg",
+ addrs, 1, 2);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "No valid 'aspeed,lpc-io-reg' configured\n");
+ return rc;
+ }
+
+ if (addrs[0] > 0xffff) {
+ dev_err(&pdev->dev, "Invalid data address in 'aspeed,lpc-io-reg'\n");
+ return -EINVAL;
+ }
+
+ if (rc == 2 && addrs[1] > 0xffff) {
+ dev_err(&pdev->dev, "Invalid status address in 'aspeed,lpc-io-reg'\n");
return -EINVAL;
}

- return slave;
+ return rc;
}

static int aspeed_kcs_probe(struct platform_device *pdev)
@@ -524,9 +569,11 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
struct kcs_bmc_device *kcs_bmc;
struct aspeed_kcs_bmc *priv;
struct device_node *np;
- int rc, channel, addr;
bool have_upstream_irq;
u32 upstream_irq[2];
+ int rc, channel;
+ int nr_addrs;
+ u32 addrs[2];

np = pdev->dev.of_node->parent;
if (!of_device_is_compatible(np, "aspeed,ast2400-lpc-v2") &&
@@ -544,9 +591,9 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
if (channel < 0)
return channel;

- addr = ops->get_io_address(pdev);
- if (addr < 0)
- return addr;
+ nr_addrs = ops->get_io_address(pdev, addrs);
+ if (nr_addrs < 0)
+ return nr_addrs;

np = pdev->dev.of_node;
rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", upstream_irq, 2);
@@ -575,7 +622,9 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
priv->obe.remove = false;
timer_setup(&priv->obe.timer, aspeed_kcs_check_obe, 0);

- aspeed_kcs_set_address(kcs_bmc, addr);
+ rc = aspeed_kcs_set_address(kcs_bmc, addrs, nr_addrs);
+ if (rc)
+ return rc;

/* Host to BMC IRQ */
rc = aspeed_kcs_config_downstream_irq(kcs_bmc, pdev);
@@ -602,7 +651,8 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
return rc;
}

- dev_info(&pdev->dev, "Initialised channel %d at 0x%x\n", kcs_bmc->channel, addr);
+ dev_info(&pdev->dev, "Initialised channel %d at 0x%x\n",
+ kcs_bmc->channel, addrs[0]);

return 0;
}
--
2.30.2

2021-06-15 18:47:55

by Corey Minyard

[permalink] [raw]
Subject: Re: [PATCH v4 00/16] ipmi: Allow raw access to KCS devices

On Tue, Jun 08, 2021 at 08:17:41PM +0930, Andrew Jeffery wrote:
>
> Hello,
>
> This is the 4th spin of the series refactoring the keyboard-controller-style
> device drivers in the IPMI subsystem.

Ok, no comments and everything looks good, I have this queued for the
next Linux release.

Thanks,

-corey

>
> v3 can be found at:
>
> https://lore.kernel.org/lkml/[email protected]/
>
> v4:
>
> * Makes kcs_bmc_add_device() return an error if no client successfully
> initialised with respect to the binding of the device driver
> * Retains the existing single-open semantics (v3 allowed multiple-open)
> * Fixes the OBE macro for the NPCM7xx KCS driver
> * Cleans up Yoda-style masks (mask constant on the LHS rather than RHS)
> * Cleans up includes in kcs_bmc_client.h
> * Adds some comments to the SerIO adapter to clarify object lifetimes
>
> Previously:
>
> Changes in v3:
>
> * The series was rebased onto v5.13-rc1
> * v5.13-rc1 includes Chiawei's patches reworking the LPC devicetree bindings,
> so they're no-longer required in the series.
> * After some discussion with Arnd[1] and investigating the serio subsystem,
> I've replaced the "raw" KCS driver (patch 16/21 in v2) with a serio adaptor
> (patch 11/16 in this series). The adaptor allows us to take advantage of the
> existing chardevs provided by serio.
>
> [1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
>
> Changes in v2 include:
>
> * A rebase onto v5.12-rc2
> * Incorporation of off-list feedback on SerIRQ configuration from
> Chiawei
> * Further validation on hardware for ASPEED KCS devices 2, 3 and 4
> * Lifting the existing single-open constraint of the IPMI chardev
> * Fixes addressing Rob's feedback on the conversion of the ASPEED KCS
> binding to dt-schema
> * Fixes addressing Rob's feedback on the new aspeed,lpc-interrupts
> property definition for the ASPEED KCS binding
>
> Please test and review!
>
> Andrew
>
> Andrew Jeffery (16):
> ipmi: kcs_bmc_aspeed: Use of match data to extract KCS properties
> ipmi: kcs_bmc: Make status update atomic
> ipmi: kcs_bmc: Rename {read,write}_{status,data}() functions
> ipmi: kcs_bmc: Split out kcs_bmc_cdev_ipmi
> ipmi: kcs_bmc: Turn the driver data-structures inside-out
> ipmi: kcs_bmc: Split headers into device and client
> ipmi: kcs_bmc: Strip private client data from struct kcs_bmc
> ipmi: kcs_bmc: Decouple the IPMI chardev from the core
> ipmi: kcs_bmc: Allow clients to control KCS IRQ state
> ipmi: kcs_bmc: Enable IBF on open
> ipmi: kcs_bmc: Add serio adaptor
> dt-bindings: ipmi: Convert ASPEED KCS binding to schema
> dt-bindings: ipmi: Add optional SerIRQ property to ASPEED KCS devices
> ipmi: kcs_bmc_aspeed: Implement KCS SerIRQ configuration
> ipmi: kcs_bmc_aspeed: Fix IBFIE typo from datasheet
> ipmi: kcs_bmc_aspeed: Optionally apply status address
>
> .../bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml | 106 +++
> .../bindings/ipmi/aspeed-kcs-bmc.txt | 33 -
> drivers/char/ipmi/Kconfig | 27 +
> drivers/char/ipmi/Makefile | 2 +
> drivers/char/ipmi/kcs_bmc.c | 523 ++++-----------
> drivers/char/ipmi/kcs_bmc.h | 92 +--
> drivers/char/ipmi/kcs_bmc_aspeed.c | 633 +++++++++++++-----
> drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 568 ++++++++++++++++
> drivers/char/ipmi/kcs_bmc_client.h | 45 ++
> drivers/char/ipmi/kcs_bmc_device.h | 22 +
> drivers/char/ipmi/kcs_bmc_npcm7xx.c | 92 ++-
> drivers/char/ipmi/kcs_bmc_serio.c | 157 +++++
> 12 files changed, 1594 insertions(+), 706 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml
> delete mode 100644 Documentation/devicetree/bindings/ipmi/aspeed-kcs-bmc.txt
> create mode 100644 drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
> create mode 100644 drivers/char/ipmi/kcs_bmc_client.h
> create mode 100644 drivers/char/ipmi/kcs_bmc_device.h
> create mode 100644 drivers/char/ipmi/kcs_bmc_serio.c
>
> --
> 2.30.2
>

2021-06-15 22:47:17

by Andrew Jeffery

[permalink] [raw]
Subject: Re: [PATCH v4 00/16] ipmi: Allow raw access to KCS devices



On Wed, 16 Jun 2021, at 04:16, Corey Minyard wrote:
> On Tue, Jun 08, 2021 at 08:17:41PM +0930, Andrew Jeffery wrote:
> >
> > Hello,
> >
> > This is the 4th spin of the series refactoring the keyboard-controller-style
> > device drivers in the IPMI subsystem.
>
> Ok, no comments and everything looks good, I have this queued for the
> next Linux release.

Thanks!

Andrew

2021-06-19 08:07:41

by Zev Weiss

[permalink] [raw]
Subject: Re: [PATCH v4 11/16] ipmi: kcs_bmc: Add serio adaptor

On Tue, Jun 08, 2021 at 05:47:52AM CDT, Andrew Jeffery wrote:
>kcs_bmc_serio acts as a bridge between the KCS drivers in the IPMI
>subsystem and the existing userspace interfaces available through the
>serio subsystem. This is useful when userspace would like to make use of
>the BMC KCS devices for purposes that aren't IPMI.
>
>Signed-off-by: Andrew Jeffery <[email protected]>
>---
> drivers/char/ipmi/Kconfig | 14 +++
> drivers/char/ipmi/Makefile | 1 +
> drivers/char/ipmi/kcs_bmc_serio.c | 157 ++++++++++++++++++++++++++++++
> 3 files changed, 172 insertions(+)
> create mode 100644 drivers/char/ipmi/kcs_bmc_serio.c
>

Reviewed-by: Zev Weiss <[email protected]>

2021-06-19 08:50:13

by Zev Weiss

[permalink] [raw]
Subject: Re: [PATCH v4 06/16] ipmi: kcs_bmc: Split headers into device and client

On Tue, Jun 08, 2021 at 05:47:47AM 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 <[email protected]>
>---
> drivers/char/ipmi/kcs_bmc.c | 23 ++++++++++------
> drivers/char/ipmi/kcs_bmc.h | 27 +++++++++----------
> drivers/char/ipmi/kcs_bmc_aspeed.c | 17 ++++++------
> drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 39 ++++++++++++++++++---------
> drivers/char/ipmi/kcs_bmc_client.h | 27 +++++++++++++++++++
> drivers/char/ipmi/kcs_bmc_device.h | 19 +++++++++++++
> drivers/char/ipmi/kcs_bmc_npcm7xx.c | 17 ++++++------
> 7 files changed, 117 insertions(+), 52 deletions(-)
> create mode 100644 drivers/char/ipmi/kcs_bmc_client.h
> create mode 100644 drivers/char/ipmi/kcs_bmc_device.h
>

Reviewed-by: Zev Weiss <[email protected]>