From: "Andrew Jeffery" <andrew@aj.id.au>
To: "Zev Weiss" <zweiss@equinix.com>
Cc: "openipmi-developer@lists.sourceforge.net"
<openipmi-developer@lists.sourceforge.net>,
"openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
"Corey Minyard" <minyard@acm.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"Ryan Chen" <ryan_chen@aspeedtech.com>,
"Tomer Maimon" <tmaimon77@gmail.com>,
"linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>,
"Avi Fishman" <avifishman70@gmail.com>,
"Patrick Venture" <venture@google.com>,
"Linus Walleij" <linus.walleij@linaro.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Tali Perry" <tali.perry1@gmail.com>,
"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
"Rob Herring" <robh+dt@kernel.org>,
"Lee Jones" <lee.jones@linaro.org>,
"Chia-Wei, Wang" <chiawei_wang@aspeedtech.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"Benjamin Fair" <benjaminfair@google.com>
Subject: Re: [PATCH v2 16/21] ipmi: kcs_bmc: Add a "raw" character device interface
Date: Fri, 09 Apr 2021 16:16:52 +0930 [thread overview]
Message-ID: <c24bdd7a-64f6-4f4b-bd40-640efea8b059@www.fastmail.com> (raw)
In-Reply-To: <YG/jZyx3huwqewgX@packtop>
On Fri, 9 Apr 2021, at 14:47, Zev Weiss wrote:
> On Fri, Mar 19, 2021 at 01:27:47AM CDT, Andrew Jeffery wrote:
> >The existing IPMI chardev encodes IPMI behaviours as the name suggests.
> >However, KCS devices are useful beyond IPMI (or keyboards), as they
> >provide a means to generate IRQs and exchange arbitrary data between a
> >BMC and its host system.
> >
> >Implement a "raw" KCS character device that exposes the IDR, ODR and STR
> >registers to userspace via read() and write() implemented on a character
> >device:
> >
> >+--------+--------+---------+
> >| Offset | read() | write() |
> >+--------+--------+---------+
> >| 0 | IDR | ODR |
> >+--------+--------+---------+
> >| 1 | STR | STR |
> >+--------+--------+---------+
> >
> >This interface allows userspace to implement arbitrary (though somewhat
> >inefficient) protocols for exchanging information between a BMC and host
> >firmware. Conceptually the KCS interface can be used as an out-of-band
> >machanism for interrupt-signaled control messages while bulk data
>
> Typo ("mechanism")
Ack.
>
> >transfers occur over more appropriate interfaces between the BMC and the
> >host (which may lack their own interrupt mechanism, e.g. LPC FW cycles).
> >
> >poll() is provided, which will wait for IBF or OBE conditions for data
> >reads and writes respectively. Reads of STR on its own never blocks,
> >though accessing both offsets in the one system call may block if the
> >data registers are not ready.
> >
> >Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> >---
> > Documentation/ABI/testing/dev-raw-kcs | 25 ++
> > drivers/char/ipmi/Kconfig | 17 +
> > drivers/char/ipmi/Makefile | 1 +
> > drivers/char/ipmi/kcs_bmc_cdev_raw.c | 443 ++++++++++++++++++++++++++
> > 4 files changed, 486 insertions(+)
> > create mode 100644 Documentation/ABI/testing/dev-raw-kcs
> > create mode 100644 drivers/char/ipmi/kcs_bmc_cdev_raw.c
> >
> >diff --git a/Documentation/ABI/testing/dev-raw-kcs b/Documentation/ABI/testing/dev-raw-kcs
> >new file mode 100644
> >index 000000000000..06e7e2071562
> >--- /dev/null
> >+++ b/Documentation/ABI/testing/dev-raw-kcs
> >@@ -0,0 +1,25 @@
> >+What: /dev/raw-kcs*
> >+Date: 2021-02-15
> >+KernelVersion: 5.13
> >+Contact: openbmc@lists.ozlabs.org
> >+Contact: openipmi-developer@lists.sourceforge.net
> >+Contact: Andrew Jeffery <andrew@aj.id.au>
> >+Description: ``/dev/raw-kcs*`` exposes to userspace the data and
> >+ status registers of Keyboard-Controller-Style (KCS) IPMI
> >+ interfaces via read() and write() syscalls. Direct
> >+ exposure of the data and status registers enables
> >+ inefficient but arbitrary protocols to be implemented
> >+ over the device. A typical approach is to use KCS
> >+ devices for out-of-band signalling for bulk data
> >+ transfers over other interfaces between a Baseboard
> >+ Management Controller and its host.
> >+
> >+ +--------+--------+---------+
> >+ | Offset | read() | write() |
> >+ +--------+--------+---------+
> >+ | 0 | IDR | ODR |
> >+ +--------+--------+---------+
> >+ | 1 | STR | STR |
> >+ +--------+--------+---------+
> >+
> >+Users: libmctp: https://github.com/openbmc/libmctp
> >diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> >index bc5f81899b62..273ac1a1f870 100644
> >--- a/drivers/char/ipmi/Kconfig
> >+++ b/drivers/char/ipmi/Kconfig
> >@@ -137,6 +137,23 @@ 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_CDEV_RAW
> >+ depends on IPMI_KCS_BMC
> >+ tristate "Raw character device interface for BMC KCS devices"
> >+ help
> >+ Provides a BMC-side character device directly exposing the
> >+ data and status registers of a KCS device to userspace. While
> >+ KCS devices are commonly used to implement IPMI message
> >+ passing, they provide a general interface for exchange of
> >+ interrupts, data and status information between the BMC and
> >+ its host.
> >+
> >+ Say YES if you wish to use the KCS devices to implement
> >+ protocols that are not IPMI.
> >+
> >+ This support is also available as a module. The module will be
> >+ called kcs_bmc_cdev_raw.
> >+
> > 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..c8cc248ddd90 100644
> >--- a/drivers/char/ipmi/Makefile
> >+++ b/drivers/char/ipmi/Makefile
> >@@ -24,6 +24,7 @@ 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_CDEV_IPMI) += kcs_bmc_cdev_ipmi.o
> >+obj-$(CONFIG_IPMI_KCS_BMC_CDEV_RAW) += kcs_bmc_cdev_raw.o
> > obj-$(CONFIG_ASPEED_BT_IPMI_BMC) += bt-bmc.o
> > obj-$(CONFIG_ASPEED_KCS_IPMI_BMC) += kcs_bmc_aspeed.o
> > obj-$(CONFIG_NPCM7XX_KCS_IPMI_BMC) += kcs_bmc_npcm7xx.o
> >diff --git a/drivers/char/ipmi/kcs_bmc_cdev_raw.c b/drivers/char/ipmi/kcs_bmc_cdev_raw.c
> >new file mode 100644
> >index 000000000000..bdd258648c8e
> >--- /dev/null
> >+++ b/drivers/char/ipmi/kcs_bmc_cdev_raw.c
> >@@ -0,0 +1,443 @@
> >+// 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/fs.h>
> >+#include <linux/list.h>
> >+#include <linux/miscdevice.h>
> >+#include <linux/module.h>
> >+#include <linux/poll.h>
> >+
> >+#include "kcs_bmc_client.h"
> >+
> >+#define DEVICE_NAME "raw-kcs"
> >+
> >+struct kcs_bmc_raw {
> >+ struct list_head entry;
> >+
> >+ struct kcs_bmc_client client;
> >+
> >+ wait_queue_head_t queue;
> >+ u8 events;
> >+ bool writable;
> >+ bool readable;
> >+ u8 idr;
> >+
> >+ struct miscdevice miscdev;
> >+};
> >+
> >+static inline struct kcs_bmc_raw *client_to_kcs_bmc_raw(struct kcs_bmc_client *client)
> >+{
> >+ return container_of(client, struct kcs_bmc_raw, client);
> >+}
> >+
> >+/* Call under priv->queue.lock */
> >+static void kcs_bmc_raw_update_event_mask(struct kcs_bmc_raw *priv, u8 mask, u8 state)
> >+{
> >+ kcs_bmc_update_event_mask(priv->client.dev, mask, state);
> >+ priv->events &= ~mask;
> >+ priv->events |= state & mask;
> >+}
> >+
> >+static int kcs_bmc_raw_event(struct kcs_bmc_client *client)
> >+{
> >+ struct kcs_bmc_raw *priv;
> >+ struct device *dev;
> >+ u8 status, handled;
> >+
> >+ priv = client_to_kcs_bmc_raw(client);
> >+ dev = priv->miscdev.this_device;
> >+
> >+ spin_lock(&priv->queue.lock);
> >+
> >+ status = kcs_bmc_read_status(client->dev);
> >+ handled = 0;
> >+
> >+ if ((priv->events & KCS_BMC_EVENT_TYPE_IBF) && (status & KCS_BMC_STR_IBF)) {
> >+ if (priv->readable)
> >+ dev_err(dev, "Storm brewing!");
>
> That seems a *touch* cryptic...
Uh, yeah. That wasn't meant to be there in that form.
>
> >+
> >+ dev_dbg(dev, "Disabling IDR events for back-pressure\n");
> >+ kcs_bmc_raw_update_event_mask(priv, KCS_BMC_EVENT_TYPE_IBF, 0);
> >+ priv->idr = kcs_bmc_read_data(client->dev);
> >+ priv->readable = true;
> >+
> >+ dev_dbg(dev, "IDR read, waking waiters\n");
> >+ wake_up_locked(&priv->queue);
> >+
> >+ handled |= KCS_BMC_EVENT_TYPE_IBF;
> >+ }
> >+
> >+ if ((priv->events & KCS_BMC_EVENT_TYPE_OBE) && !(status & KCS_BMC_STR_OBF)) {
> >+ kcs_bmc_raw_update_event_mask(priv, KCS_BMC_EVENT_TYPE_OBE, 0);
> >+ priv->writable = true;
> >+
> >+ dev_dbg(dev, "ODR writable, waking waiters\n");
> >+ wake_up_locked(&priv->queue);
> >+
> >+ handled |= KCS_BMC_EVENT_TYPE_OBE;
> >+ }
> >+
> >+ spin_unlock(&priv->queue.lock);
> >+
> >+ return handled ? KCS_BMC_EVENT_HANDLED : KCS_BMC_EVENT_NONE;
>
> Hm, if we're just treating it as a boolean here, is there any need to
> muck around with setting specific bits of 'handled' in the if-blocks
> above?
I don't think it matters? If we want to debug we can print the handled bitmask.
>
> >+}
> >+
> >+static const struct kcs_bmc_client_ops kcs_bmc_raw_client_ops = {
> >+ .event = kcs_bmc_raw_event,
> >+};
> >+
> >+static inline struct kcs_bmc_raw *file_to_kcs_bmc_raw(struct file *filp)
> >+{
> >+ return container_of(filp->private_data, struct kcs_bmc_raw, miscdev);
> >+}
> >+
> >+static int kcs_bmc_raw_open(struct inode *inode, struct file *filp)
> >+{
> >+ struct kcs_bmc_raw *priv = file_to_kcs_bmc_raw(filp);
> >+
> >+ return kcs_bmc_enable_device(priv->client.dev, &priv->client);
> >+}
> >+
> >+static bool kcs_bmc_raw_prepare_obe(struct kcs_bmc_raw *priv)
> >+{
> >+ bool writable;
> >+
> >+ /* Enable the OBE event so we can catch the host clearing OBF */
> >+ kcs_bmc_raw_update_event_mask(priv, KCS_BMC_EVENT_TYPE_OBE, KCS_BMC_EVENT_TYPE_OBE);
> >+
> >+ /* Now that we'll catch an OBE event, check if it's already occurred */
> >+ writable = !(kcs_bmc_read_status(priv->client.dev) & KCS_BMC_STR_OBF);
> >+
> >+ /* If OBF is clear we've missed the OBE event, so disable it */
> >+ if (writable)
> >+ kcs_bmc_raw_update_event_mask(priv, KCS_BMC_EVENT_TYPE_OBE, 0);
> >+
> >+ return writable;
> >+}
> >+
> >+static __poll_t kcs_bmc_raw_poll(struct file *filp, poll_table *wait)
> >+{
> >+ struct kcs_bmc_raw *priv;
> >+ __poll_t events = 0;
> >+
> >+ priv = file_to_kcs_bmc_raw(filp);
> >+
> >+ poll_wait(filp, &priv->queue, wait);
> >+
> >+ spin_lock_irq(&priv->queue.lock);
> >+ if (kcs_bmc_raw_prepare_obe(priv))
> >+ events |= (EPOLLOUT | EPOLLWRNORM);
> >+
> >+ if (priv->readable || (kcs_bmc_read_status(priv->client.dev) & KCS_BMC_STR_IBF))
> >+ events |= (EPOLLIN | EPOLLRDNORM);
> >+ spin_unlock_irq(&priv->queue.lock);
> >+
> >+ return events;
> >+}
> >+
> >+static ssize_t kcs_bmc_raw_read(struct file *filp, char __user *buf,
> >+ size_t count, loff_t *ppos)
> >+{
> >+ struct kcs_bmc_device *kcs_bmc;
> >+ struct kcs_bmc_raw *priv;
> >+ bool read_idr, read_str;
> >+ struct device *dev;
> >+ u8 idr, str;
> >+ ssize_t rc;
> >+
> >+ priv = file_to_kcs_bmc_raw(filp);
> >+ kcs_bmc = priv->client.dev;
> >+ dev = priv->miscdev.this_device;
> >+
> >+ if (!count)
> >+ return 0;
> >+
> >+ if (count > 2 || *ppos > 1)
> >+ return -EINVAL;
> >+
> >+ if (*ppos + count > 2)
> >+ return -EINVAL;
> >+
> >+ read_idr = (*ppos == 0);
> >+ read_str = (*ppos == 1) || (count == 2);
> >+
> >+ spin_lock_irq(&priv->queue.lock);
> >+ if (read_idr) {
> >+ dev_dbg(dev, "Waiting for IBF\n");
> >+ str = kcs_bmc_read_status(kcs_bmc);
> >+ if ((filp->f_flags & O_NONBLOCK) && (str & KCS_BMC_STR_IBF)) {
> >+ rc = -EWOULDBLOCK;
> >+ goto out;
> >+ }
> >+
> >+ rc = wait_event_interruptible_locked(priv->queue,
> >+ priv->readable || (str & KCS_BMC_STR_IBF));
> >+ if (rc < 0)
> >+ goto out;
> >+
> >+ if (signal_pending(current)) {
> >+ dev_dbg(dev, "Interrupted waiting for IBF\n");
> >+ rc = -EINTR;
> >+ goto out;
> >+ }
> >+
> >+ /*
> >+ * Re-enable events prior to possible read of IDR (which clears
> >+ * IBF) to ensure we receive interrupts for subsequent writes
> >+ * to IDR. Writes to IDR by the host should not occur while IBF
> >+ * is set.
> >+ */
> >+ dev_dbg(dev, "Woken by IBF, enabling IRQ\n");
> >+ kcs_bmc_raw_update_event_mask(priv, KCS_BMC_EVENT_TYPE_IBF,
> >+ KCS_BMC_EVENT_TYPE_IBF);
> >+
> >+ /* Read data out of IDR into internal storage if necessary */
> >+ if (!priv->readable) {
> >+ WARN(!(str & KCS_BMC_STR_IBF), "Unknown reason for wakeup!");
> >+
> >+ priv->idr = kcs_bmc_read_data(kcs_bmc);
> >+ }
> >+
> >+ /* Copy data from internal storage to userspace */
> >+ idr = priv->idr;
> >+
> >+ /* We're done consuming the internally stored value */
> >+ priv->readable = false;
> >+ }
> >+
> >+ if (read_str) {
> >+ str = kcs_bmc_read_status(kcs_bmc);
> >+ if (*ppos == 0 || priv->readable)
> >+ /*
> >+ * If we got this far with `*ppos == 0` then we've read
> >+ * data out of IDR, so set IBF when reporting back to
> >+ * userspace so userspace knows the IDR value is valid.
> >+ */
> >+ str |= KCS_BMC_STR_IBF;
> >+
> >+ dev_dbg(dev, "Read status 0x%x\n", str);
> >+
> >+ }
> >+
> >+ rc = count;
> >+out:
> >+ spin_unlock_irq(&priv->queue.lock);
> >+
> >+ if (rc < 0)
> >+ return rc;
> >+
> >+ /* Now copy the data in to the userspace buffer */
> >+
> >+ if (read_idr)
> >+ if (copy_to_user(buf++, &idr, sizeof(idr)))
> >+ return -EFAULT;
> >+
> >+ if (read_str)
> >+ if (copy_to_user(buf, &str, sizeof(str)))
> >+ return -EFAULT;
> >+
> >+ return count;
> >+}
> >+
> >+static ssize_t kcs_bmc_raw_write(struct file *filp, const char __user *buf,
> >+ size_t count, loff_t *ppos)
> >+{
> >+ struct kcs_bmc_device *kcs_bmc;
> >+ bool write_odr, write_str;
> >+ struct kcs_bmc_raw *priv;
> >+ struct device *dev;
> >+ uint8_t data[2];
> >+ ssize_t result;
> >+ u8 str;
> >+
> >+ priv = file_to_kcs_bmc_raw(filp);
> >+ kcs_bmc = priv->client.dev;
> >+ dev = priv->miscdev.this_device;
> >+
> >+ if (!count)
> >+ return count;
> >+
> >+ if (count > 2)
> >+ return -EINVAL;
> >+
> >+ if (*ppos >= 2)
> >+ return -EINVAL;
> >+
> >+ if (*ppos + count > 2)
> >+ return -EINVAL;
> >+
> >+ if (copy_from_user(data, buf, count))
> >+ return -EFAULT;
> >+
> >+ write_odr = (*ppos == 0);
> >+ write_str = (*ppos == 1) || (count == 2);
> >+
> >+ spin_lock_irq(&priv->queue.lock);
> >+
> >+ /* Always write status before data, we generate the SerIRQ by writing ODR */
> >+ if (write_str) {
> >+ /* The index of STR in the userspace buffer depends on whether ODR is written */
> >+ str = data[*ppos == 0];
> >+ if (!(str & KCS_BMC_STR_OBF))
> >+ dev_warn(dev, "Clearing OBF with status write: 0x%x\n", str);
> >+ dev_dbg(dev, "Writing status 0x%x\n", str);
> >+ kcs_bmc_write_status(kcs_bmc, str);
> >+ }
> >+
> >+ if (write_odr) {
> >+ /* If we're writing ODR it's always the first byte in the buffer */
> >+ u8 odr = data[0];
> >+
> >+ str = kcs_bmc_read_status(kcs_bmc);
> >+ if (str & KCS_BMC_STR_OBF) {
> >+ if (filp->f_flags & O_NONBLOCK) {
> >+ result = -EWOULDBLOCK;
> >+ goto out;
> >+ }
> >+
> >+ priv->writable = kcs_bmc_raw_prepare_obe(priv);
> >+
> >+ /* Now either OBF is already clear, or we'll get an OBE event to wake us */
> >+ dev_dbg(dev, "Waiting for OBF to clear\n");
> >+ wait_event_interruptible_locked(priv->queue, priv->writable);
> >+
> >+ if (signal_pending(current)) {
> >+ kcs_bmc_raw_update_event_mask(priv, KCS_BMC_EVENT_TYPE_OBE, 0);
> >+ result = -EINTR;
> >+ goto out;
> >+ }
> >+
> >+ WARN_ON(kcs_bmc_read_status(kcs_bmc) & KCS_BMC_STR_OBF);
> >+ }
> >+
> >+ dev_dbg(dev, "Writing 0x%x to ODR\n", odr);
> >+ kcs_bmc_write_data(kcs_bmc, odr);
> >+ }
> >+
> >+ result = count;
> >+out:
> >+ spin_unlock_irq(&priv->queue.lock);
> >+
> >+ return result;
> >+}
> >+
> >+static int kcs_bmc_raw_release(struct inode *inode, struct file *filp)
> >+{
> >+ struct kcs_bmc_raw *priv = file_to_kcs_bmc_raw(filp);
> >+
> >+ kcs_bmc_disable_device(priv->client.dev, &priv->client);
> >+
> >+ return 0;
> >+}
> >+
> >+static const struct file_operations kcs_bmc_raw_fops = {
> >+ .owner = THIS_MODULE,
> >+ .open = kcs_bmc_raw_open,
> >+ .llseek = no_seek_end_llseek,
> >+ .read = kcs_bmc_raw_read,
> >+ .write = kcs_bmc_raw_write,
> >+ .poll = kcs_bmc_raw_poll,
> >+ .release = kcs_bmc_raw_release,
> >+};
> >+
> >+static DEFINE_SPINLOCK(kcs_bmc_raw_instances_lock);
> >+static LIST_HEAD(kcs_bmc_raw_instances);
> >+
> >+static int kcs_bmc_raw_attach_cdev(struct kcs_bmc_device *kcs_bmc)
> >+{
> >+ struct kcs_bmc_raw *priv;
> >+ int rc;
> >+
> >+ priv = devm_kzalloc(kcs_bmc->dev, sizeof(*priv), GFP_KERNEL);
> >+ if (!priv)
> >+ return -ENOMEM;
> >+
> >+ priv->client.dev = kcs_bmc;
> >+ priv->client.ops = &kcs_bmc_raw_client_ops;
> >+
> >+ init_waitqueue_head(&priv->queue);
> >+ priv->writable = false;
> >+ priv->readable = false;
> >+
> >+ priv->miscdev.minor = MISC_DYNAMIC_MINOR;
> >+ priv->miscdev.name = devm_kasprintf(kcs_bmc->dev, GFP_KERNEL, "%s%u", DEVICE_NAME,
> >+ kcs_bmc->channel);
> >+ if (!priv->miscdev.name)
> >+ return -EINVAL;
> >+
> >+ priv->miscdev.fops = &kcs_bmc_raw_fops;
> >+
> >+ /* Initialise our expected events. Listen for IBF but ignore OBE until necessary */
> >+ kcs_bmc_raw_update_event_mask(priv, (KCS_BMC_EVENT_TYPE_IBF | KCS_BMC_EVENT_TYPE_OBE),
> >+ KCS_BMC_EVENT_TYPE_IBF);
> >+
> >+ rc = misc_register(&priv->miscdev);
> >+ if (rc) {
> >+ dev_err(kcs_bmc->dev, "Unable to register device\n");
> >+ return rc;
> >+ }
> >+
> >+ spin_lock_irq(&kcs_bmc_raw_instances_lock);
> >+ list_add(&priv->entry, &kcs_bmc_raw_instances);
> >+ spin_unlock_irq(&kcs_bmc_raw_instances_lock);
> >+
> >+ dev_info(kcs_bmc->dev, "Initialised raw client for channel %d", kcs_bmc->channel);
> >+
> >+ return 0;
> >+}
> >+
> >+static int kcs_bmc_raw_detach_cdev(struct kcs_bmc_device *kcs_bmc)
> >+{
> >+ struct kcs_bmc_raw *priv = NULL, *pos;
> >+
> >+ spin_lock_irq(&kcs_bmc_raw_instances_lock);
> >+ list_for_each_entry(pos, &kcs_bmc_raw_instances, entry) {
> >+ if (pos->client.dev == kcs_bmc) {
> >+ priv = pos;
> >+ list_del(&pos->entry);
> >+ break;
> >+ }
> >+ }
> >+ spin_unlock_irq(&kcs_bmc_raw_instances_lock);
> >+
> >+ if (!priv)
> >+ return 0;
>
> Similarly to patch #12, might we want to indicate some sort of failure
> here, or is this a normal/expected case?
I replied on 12/21, I'll have another think about it.
Cheers,
Andrew
next prev parent reply other threads:[~2021-04-09 6:47 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-19 6:27 [PATCH v2 01/21] dt-bindings: aspeed-lpc: Remove LPC partitioning Andrew Jeffery
2021-03-19 6:27 ` [PATCH v2 02/21] ARM: dts: Remove LPC BMC and Host partitions Andrew Jeffery
2021-03-19 6:27 ` [PATCH v2 03/21] ipmi: kcs: aspeed: Adapt to new LPC DTS layout Andrew Jeffery
2021-04-09 3:35 ` Joel Stanley
2021-03-19 6:27 ` [PATCH v2 04/21] pinctrl: aspeed-g5: Adapt to new LPC device tree layout Andrew Jeffery
2021-04-09 3:36 ` Joel Stanley
2021-03-19 6:27 ` [PATCH v2 05/21] soc: aspeed: " Andrew Jeffery
2021-04-09 3:38 ` Joel Stanley
2021-03-19 6:27 ` [PATCH v2 06/21] ipmi: kcs_bmc_aspeed: Use of match data to extract KCS properties Andrew Jeffery
2021-04-06 6:07 ` ChiaWei Wang
2021-04-09 3:24 ` Zev Weiss
2021-03-19 6:27 ` [PATCH v2 07/21] ipmi: kcs_bmc: Make status update atomic Andrew Jeffery
2021-04-09 5:32 ` Zev Weiss
2021-03-19 6:27 ` [PATCH v2 08/21] ipmi: kcs_bmc: Rename {read,write}_{status,data}() functions Andrew Jeffery
2021-04-09 5:33 ` [PATCH v2 08/21] ipmi: kcs_bmc: Rename {read, write}_{status, data}() functions Zev Weiss
2021-03-19 6:27 ` [PATCH v2 09/21] ipmi: kcs_bmc: Split out kcs_bmc_cdev_ipmi Andrew Jeffery
2021-04-09 3:56 ` Zev Weiss
2021-04-09 5:48 ` Andrew Jeffery
2021-04-09 19:21 ` Zev Weiss
2021-03-19 6:27 ` [PATCH v2 10/21] ipmi: kcs_bmc: Turn the driver data-structures inside-out Andrew Jeffery
2021-04-09 3:57 ` Zev Weiss
2021-04-09 5:59 ` Andrew Jeffery
2021-04-09 6:25 ` Zev Weiss
2021-04-09 19:26 ` Zev Weiss
2021-04-11 23:00 ` Andrew Jeffery
2021-03-19 6:27 ` [PATCH v2 11/21] ipmi: kcs_bmc: Split headers into device and client Andrew Jeffery
2021-04-09 4:01 ` Zev Weiss
2021-04-09 6:06 ` Andrew Jeffery
2021-03-19 6:27 ` [PATCH v2 12/21] ipmi: kcs_bmc: Strip private client data from struct kcs_bmc Andrew Jeffery
2021-04-09 4:07 ` Zev Weiss
2021-04-09 6:15 ` Andrew Jeffery
2021-03-19 6:27 ` [PATCH v2 13/21] ipmi: kcs_bmc: Decouple the IPMI chardev from the core Andrew Jeffery
2021-04-06 6:07 ` ChiaWei Wang
2021-04-09 4:35 ` Zev Weiss
2021-04-09 6:24 ` Andrew Jeffery
2021-03-19 6:27 ` [PATCH v2 14/21] ipmi: kcs_bmc: Allow clients to control KCS IRQ state Andrew Jeffery
2021-04-09 4:37 ` Zev Weiss
2021-04-09 6:39 ` Andrew Jeffery
2021-03-19 6:27 ` [PATCH v2 15/21] ipmi: kcs_bmc: Don't enforce single-open policy in the kernel Andrew Jeffery
2021-04-09 5:07 ` Zev Weiss
2021-03-19 6:27 ` [PATCH v2 16/21] ipmi: kcs_bmc: Add a "raw" character device interface Andrew Jeffery
2021-04-09 5:17 ` Zev Weiss
2021-04-09 6:46 ` Andrew Jeffery [this message]
2021-04-09 7:55 ` Arnd Bergmann
2021-04-12 1:33 ` Andrew Jeffery
2021-04-12 8:48 ` Arnd Bergmann
2021-04-12 23:45 ` Andrew Jeffery
2021-04-13 8:22 ` Arnd Bergmann
2021-04-14 0:30 ` Andrew Jeffery
2021-03-19 6:27 ` [PATCH v2 17/21] dt-bindings: ipmi: Convert ASPEED KCS binding to schema Andrew Jeffery
2021-03-26 1:48 ` Rob Herring
2021-04-09 5:15 ` Zev Weiss
2021-04-09 5:33 ` Andrew Jeffery
2021-04-09 5:44 ` Zev Weiss
2021-04-09 8:46 ` Zev Weiss
2021-03-19 6:27 ` [PATCH v2 18/21] dt-bindings: ipmi: Add optional SerIRQ property to ASPEED KCS devices Andrew Jeffery
2021-03-26 1:49 ` Rob Herring
2021-03-19 6:27 ` [PATCH v2 19/21] ipmi: kcs_bmc_aspeed: Implement KCS SerIRQ configuration Andrew Jeffery
2021-04-01 9:30 ` [EXTERNAL] " Zev Weiss
2021-03-19 6:27 ` [PATCH v2 20/21] ipmi: kcs_bmc_aspeed: Fix IBFIE typo from datasheet Andrew Jeffery
2021-04-09 5:40 ` Zev Weiss
2021-03-19 6:27 ` [PATCH v2 21/21] ipmi: kcs_bmc_aspeed: Optionally apply status address Andrew Jeffery
2021-04-01 18:18 ` Re " Zev Weiss
2021-04-06 6:09 ` ChiaWei Wang
2021-04-09 3:18 ` [PATCH v2 01/21] dt-bindings: aspeed-lpc: Remove LPC partitioning Joel Stanley
2021-04-09 5:24 ` Andrew Jeffery
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c24bdd7a-64f6-4f4b-bd40-640efea8b059@www.fastmail.com \
--to=andrew@aj.id.au \
--cc=avifishman70@gmail.com \
--cc=benjaminfair@google.com \
--cc=chiawei_wang@aspeedtech.com \
--cc=devicetree@vger.kernel.org \
--cc=lee.jones@linaro.org \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-aspeed@lists.ozlabs.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=minyard@acm.org \
--cc=openbmc@lists.ozlabs.org \
--cc=openipmi-developer@lists.sourceforge.net \
--cc=robh+dt@kernel.org \
--cc=ryan_chen@aspeedtech.com \
--cc=tali.perry1@gmail.com \
--cc=tmaimon77@gmail.com \
--cc=venture@google.com \
--cc=zweiss@equinix.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).