<html><body><p><tt><font size="2">"openbmc" <openbmc-bounces+austenc=us.ibm.com@lists.ozlabs.org> wrote on 08/07/2017 10:53:01 PM:<br><br>> From: Brendan Higgins <brendanhiggins@google.com></font></tt><br><tt><font size="2">> To: minyard@acm.org, benjaminfair@google.com, clg@kaod.org, <br>> joel@jms.id.au, andrew@aj.id.au</font></tt><br><tt><font size="2">> Cc: openipmi-developer@lists.sourceforge.net, Brendan Higgins <br>> <brendanhiggins@google.com>, openbmc@lists.ozlabs.org, linux-<br>> kernel@vger.kernel.org</font></tt><br><tt><font size="2">> Date: 08/07/2017 10:55 PM</font></tt><br><tt><font size="2">> Subject: [RFC v1 4/4] ipmi_bmc: bt-aspeed: port driver to IPMI BMC framework</font></tt><br><tt><font size="2">> Sent by: "openbmc" <openbmc-bounces+austenc=us.ibm.com@lists.ozlabs.org></font></tt><br><tt><font size="2">> <br>> From: Benjamin Fair <benjaminfair@google.com><br>> <br>> The driver was handling interaction with userspace on its own. This<br>> patch changes it to use the functionality of the ipmi_bmc framework<br>> instead.<br>> <br>> Note that this removes the ability for the BMC to set SMS_ATN by making<br>> an ioctl. If this functionality is required, it can be added back in<br>> with a later patch.</font></tt><br><br><br><tt><font size="2">Isn't SMS_ATN the way a BMC initiates an IPMI message to the OS? Say for</font></tt><br><tt><font size="2">instance a request to shutdown?</font></tt><br><br><tt><font size="2"><br>> <br>> Signed-off-by: Benjamin Fair <benjaminfair@google.com><br>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com><br>> ---<br>> drivers/char/ipmi_bmc/ipmi_bmc_bt_aspeed.c | 258 ++++++++<br>> +--------------------<br>> include/uapi/linux/bt-bmc.h | 18 --<br>> 2 files changed, 74 insertions(+), 202 deletions(-)<br>> delete mode 100644 include/uapi/linux/bt-bmc.h<br>> <br>> diff --git a/drivers/char/ipmi_bmc/ipmi_bmc_bt_aspeed.c b/drivers/<br>> char/ipmi_bmc/ipmi_bmc_bt_aspeed.c<br>> index 70d434bc1cbf..7c8082c511ee 100644<br>> --- a/drivers/char/ipmi_bmc/ipmi_bmc_bt_aspeed.c<br>> +++ b/drivers/char/ipmi_bmc/ipmi_bmc_bt_aspeed.c<br>> @@ -7,25 +7,19 @@<br>> * 2 of the License, or (at your option) any later version.<br>> */<br>> <br>> -#include <linux/atomic.h><br>> -#include <linux/bt-bmc.h><br>> #include <linux/errno.h><br>> #include <linux/interrupt.h><br>> #include <linux/io.h><br>> +#include <linux/ipmi_bmc.h><br>> #include <linux/mfd/syscon.h><br>> -#include <linux/miscdevice.h><br>> #include <linux/module.h><br>> #include <linux/of.h><br>> #include <linux/platform_device.h><br>> -#include <linux/poll.h><br>> #include <linux/regmap.h><br>> #include <linux/sched.h><br>> #include <linux/timer.h><br>> <br>> -/*<br>> - * This is a BMC device used to communicate to the host<br>> - */<br>> -#define DEVICE_NAME "ipmi-bt-host"<br>> +#define DEVICE_NAME "ipmi-bmc-bt-aspeed"<br>> <br>> #define BT_IO_BASE 0xe4<br>> #define BT_IRQ 10<br>> @@ -61,18 +55,17 @@<br>> #define BT_BMC_BUFFER_SIZE 256<br>> <br>> struct bt_bmc {<br>> + struct ipmi_bmc_bus bus;<br>> struct device dev;<br>> - struct miscdevice miscdev;<br>> + struct ipmi_bmc_ctx *bmc_ctx;<br>> + struct bt_msg request;<br>> struct regmap *map;<br>> int offset;<br>> int irq;<br>> - wait_queue_head_t queue;<br>> struct timer_list poll_timer;<br>> - struct mutex mutex;<br>> + spinlock_t lock;<br>> };<br>> <br>> -static atomic_t open_count = ATOMIC_INIT(0);<br>> -<br>> static const struct regmap_config bt_regmap_cfg = {<br>> .reg_bits = 32,<br>> .val_bits = 32,<br>> @@ -158,27 +151,28 @@ static ssize_t bt_writen(struct bt_bmc <br>> *bt_bmc, u8 *buf, size_t n)<br>> return n;<br>> }<br>> <br>> +/* TODO(benjaminfair): support ioctl BT_BMC_IOCTL_SMS_ATN */<br>> static void set_sms_atn(struct bt_bmc *bt_bmc)<br>> {<br>> bt_outb(bt_bmc, BT_CTRL_SMS_ATN, BT_CTRL);<br>> }<br>> <br>> -static struct bt_bmc *file_bt_bmc(struct file *file)<br>> +/* Called with bt_bmc->lock held */<br>> +static bool __is_request_avail(struct bt_bmc *bt_bmc)<br>> {<br>> - return container_of(file->private_data, struct bt_bmc, miscdev);<br>> + return bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_H2B_ATN;<br>> }<br>> <br>> -static int bt_bmc_open(struct inode *inode, struct file *file)<br>> +static bool is_request_avail(struct bt_bmc *bt_bmc)<br>> {<br>> - struct bt_bmc *bt_bmc = file_bt_bmc(file);<br>> + unsigned long flags;<br>> + bool result;<br>> <br>> - if (atomic_inc_return(&open_count) == 1) {<br>> - clr_b_busy(bt_bmc);<br>> - return 0;<br>> - }<br>> + spin_lock_irqsave(&bt_bmc->lock, flags);<br>> + result = __is_request_avail(bt_bmc);<br>> + spin_unlock_irqrestore(&bt_bmc->lock, flags);<br>> <br>> - atomic_dec(&open_count);<br>> - return -EBUSY;<br>> + return result;<br>> }<br>> <br>> /*<br>> @@ -194,67 +188,43 @@ static int bt_bmc_open(struct inode *inode, <br>> struct file *file)<br>> * Length NetFn/LUN Seq Cmd Data<br>> *<br>> */<br>> -static ssize_t bt_bmc_read(struct file *file, char __user *buf,<br>> - size_t count, loff_t *ppos)<br>> +static void get_request(struct bt_bmc *bt_bmc)<br>> {<br>> - struct bt_bmc *bt_bmc = file_bt_bmc(file);<br>> - u8 len;<br>> - int len_byte = 1;<br>> - u8 kbuffer[BT_BMC_BUFFER_SIZE];<br>> - ssize_t ret = 0;<br>> - ssize_t nread;<br>> + u8 *request_buf = (u8 *) &bt_bmc->request;<br>> + unsigned long flags;<br>> <br>> - if (!access_ok(VERIFY_WRITE, buf, count))<br>> - return -EFAULT;<br>> + spin_lock_irqsave(&bt_bmc->lock, flags);<br>> <br>> - WARN_ON(*ppos);<br>> -<br>> - if (wait_event_interruptible(bt_bmc->queue,<br>> - bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_H2B_ATN))<br>> - return -ERESTARTSYS;<br>> -<br>> - mutex_lock(&bt_bmc->mutex);<br>> -<br>> - if (unlikely(!(bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_H2B_ATN))) {<br>> - ret = -EIO;<br>> - goto out_unlock;<br>> + if (!__is_request_avail(bt_bmc)) {<br>> + spin_unlock_irqrestore(&bt_bmc->lock, flags);<br>> + return;<br>> }<br>> <br>> set_b_busy(bt_bmc);<br>> clr_h2b_atn(bt_bmc);<br>> clr_rd_ptr(bt_bmc);<br>> <br>> - /*<br>> - * The BT frames start with the message length, which does not<br>> - * include the length byte.<br>> - */<br>> - kbuffer[0] = bt_read(bt_bmc);<br>> - len = kbuffer[0];<br>> -<br>> - /* We pass the length back to userspace as well */<br>> - if (len + 1 > count)<br>> - len = count - 1;<br>> -<br>> - while (len) {<br>> - nread = min_t(ssize_t, len, sizeof(kbuffer) - len_byte);<br>> -<br>> - bt_readn(bt_bmc, kbuffer + len_byte, nread);<br>> -<br>> - if (copy_to_user(buf, kbuffer, nread + len_byte)) {<br>> - ret = -EFAULT;<br>> - break;<br>> - }<br>> - len -= nread;<br>> - buf += nread + len_byte;<br>> - ret += nread + len_byte;<br>> - len_byte = 0;<br>> - }<br>> + bt_bmc->request.len = bt_read(bt_bmc);<br>> + bt_readn(bt_bmc, request_buf + 1, bt_bmc->request.len);<br>> <br>> clr_b_busy(bt_bmc);<br>> + ipmi_bmc_handle_request(bt_bmc->bmc_ctx, &bt_bmc->request);<br>> +<br>> + spin_unlock_irqrestore(&bt_bmc->lock, flags);<br>> +}<br>> +<br>> +static bool bt_bmc_is_response_open(struct ipmi_bmc_bus *bus)<br>> +{<br>> + struct bt_bmc *bt_bmc = container_of(bus, struct bt_bmc, bus);<br>> + bool response_in_progress;<br>> + unsigned long flags;<br>> +<br>> + spin_lock_irqsave(&bt_bmc->lock, flags);<br>> + response_in_progress = bt_inb(bt_bmc, BT_CTRL) & (BT_CTRL_H_BUSY |<br>> + BT_CTRL_B2H_ATN);<br>> + spin_unlock_irqrestore(&bt_bmc->lock, flags);<br>> <br>> -out_unlock:<br>> - mutex_unlock(&bt_bmc->mutex);<br>> - return ret;<br>> + return !response_in_progress;<br>> }<br>> <br>> /*<br>> @@ -263,122 +233,38 @@ static ssize_t bt_bmc_read(struct file *file,<br>> char __user *buf,<br>> * Byte 1 Byte 2 Byte 3 Byte 4 Byte 5 Byte 6:N<br>> * Length NetFn/LUN Seq Cmd Code Data<br>> */<br>> -static ssize_t bt_bmc_write(struct file *file, const char __user *buf,<br>> - size_t count, loff_t *ppos)<br>> +static int bt_bmc_send_response(struct ipmi_bmc_bus *bus,<br>> + struct bt_msg *bt_response)<br>> {<br>> - struct bt_bmc *bt_bmc = file_bt_bmc(file);<br>> - u8 kbuffer[BT_BMC_BUFFER_SIZE];<br>> - ssize_t ret = 0;<br>> - ssize_t nwritten;<br>> + struct bt_bmc *bt_bmc = container_of(bus, struct bt_bmc, bus);<br>> + unsigned long flags;<br>> <br>> - /*<br>> - * send a minimum response size<br>> - */<br>> - if (count < 5)<br>> - return -EINVAL;<br>> -<br>> - if (!access_ok(VERIFY_READ, buf, count))<br>> - return -EFAULT;<br>> -<br>> - WARN_ON(*ppos);<br>> -<br>> - /*<br>> - * There's no interrupt for clearing bmc busy so we have to<br>> - * poll<br>> - */<br>> - if (wait_event_interruptible(bt_bmc->queue,<br>> - !(bt_inb(bt_bmc, BT_CTRL) &<br>> - (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN))))<br>> - return -ERESTARTSYS;<br>> -<br>> - mutex_lock(&bt_bmc->mutex);<br>> -<br>> - if (unlikely(bt_inb(bt_bmc, BT_CTRL) &<br>> - (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN))) {<br>> - ret = -EIO;<br>> - goto out_unlock;<br>> + spin_lock_irqsave(&bt_bmc->lock, flags);<br>> + if (!bt_bmc_is_response_open(bus)) {<br>> + spin_unlock_irqrestore(&bt_bmc->lock, flags);<br>> + return -EAGAIN;<br>> }<br>> <br>> clr_wr_ptr(bt_bmc);<br>> -<br>> - while (count) {<br>> - nwritten = min_t(ssize_t, count, sizeof(kbuffer));<br>> - if (copy_from_user(&kbuffer, buf, nwritten)) {<br>> - ret = -EFAULT;<br>> - break;<br>> - }<br>> -<br>> - bt_writen(bt_bmc, kbuffer, nwritten);<br>> -<br>> - count -= nwritten;<br>> - buf += nwritten;<br>> - ret += nwritten;<br>> - }<br>> -<br>> + bt_writen(bt_bmc, (u8 *) bt_response, bt_msg_len(bt_response));<br>> set_b2h_atn(bt_bmc);<br>> <br>> -out_unlock:<br>> - mutex_unlock(&bt_bmc->mutex);<br>> - return ret;<br>> -}<br>> -<br>> -static long bt_bmc_ioctl(struct file *file, unsigned int cmd,<br>> - unsigned long param)<br>> -{<br>> - struct bt_bmc *bt_bmc = file_bt_bmc(file);<br>> -<br>> - switch (cmd) {<br>> - case BT_BMC_IOCTL_SMS_ATN:<br>> - set_sms_atn(bt_bmc);<br>> - return 0;<br>> - }<br>> - return -EINVAL;<br>> -}<br>> -<br>> -static int bt_bmc_release(struct inode *inode, struct file *file)<br>> -{<br>> - struct bt_bmc *bt_bmc = file_bt_bmc(file);<br>> -<br>> - atomic_dec(&open_count);<br>> - set_b_busy(bt_bmc);<br>> + spin_unlock_irqrestore(&bt_bmc->lock, flags);<br>> return 0;<br>> }<br>> <br>> -static unsigned int bt_bmc_poll(struct file *file, poll_table *wait)<br>> -{<br>> - struct bt_bmc *bt_bmc = file_bt_bmc(file);<br>> - unsigned int mask = 0;<br>> - u8 ctrl;<br>> -<br>> - poll_wait(file, &bt_bmc->queue, wait);<br>> -<br>> - ctrl = bt_inb(bt_bmc, BT_CTRL);<br>> -<br>> - if (ctrl & BT_CTRL_H2B_ATN)<br>> - mask |= POLLIN;<br>> -<br>> - if (!(ctrl & (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN)))<br>> - mask |= POLLOUT;<br>> -<br>> - return mask;<br>> -}<br>> -<br>> -static const struct file_operations bt_bmc_fops = {<br>> - .owner = THIS_MODULE,<br>> - .open = bt_bmc_open,<br>> - .read = bt_bmc_read,<br>> - .write = bt_bmc_write,<br>> - .release = bt_bmc_release,<br>> - .poll = bt_bmc_poll,<br>> - .unlocked_ioctl = bt_bmc_ioctl,<br>> -};<br>> -<br>> static void poll_timer(unsigned long data)<br>> {<br>> struct bt_bmc *bt_bmc = (void *)data;<br>> <br>> bt_bmc->poll_timer.expires += msecs_to_jiffies(500);<br>> - wake_up(&bt_bmc->queue);<br>> +<br>> + if (bt_bmc_is_response_open(&bt_bmc->bus))<br>> + ipmi_bmc_signal_response_open(bt_bmc->bmc_ctx);<br>> +<br>> + if (is_request_avail(bt_bmc))<br>> + get_request(bt_bmc);<br>> +<br>> add_timer(&bt_bmc->poll_timer);<br>> }<br>> <br>> @@ -399,7 +285,12 @@ static irqreturn_t bt_bmc_irq(int irq, void *arg)<br>> /* ack pending IRQs */<br>> regmap_write(bt_bmc->map, bt_bmc->offset + BT_CR2, reg);<br>> <br>> - wake_up(&bt_bmc->queue);<br>> + if (bt_bmc_is_response_open(&bt_bmc->bus))<br>> + ipmi_bmc_signal_response_open(bt_bmc->bmc_ctx);<br>> +<br>> + if (is_request_avail(bt_bmc))<br>> + get_request(bt_bmc);<br>> +<br>> return IRQ_HANDLED;<br>> }<br>> <br>> @@ -474,16 +365,14 @@ static int bt_bmc_probe(struct platform_device *pdev)<br>> return rc;<br>> }<br>> <br>> - mutex_init(&bt_bmc->mutex);<br>> - init_waitqueue_head(&bt_bmc->queue);<br>> + spin_lock_init(&bt_bmc->lock);<br>> + bt_bmc->bus.send_response = bt_bmc_send_response;<br>> + bt_bmc->bus.is_response_open = bt_bmc_is_response_open;<br>> + bt_bmc->bmc_ctx = ipmi_bmc_get_global_ctx();<br>> <br>> - bt_bmc->miscdev.minor = MISC_DYNAMIC_MINOR,<br>> - bt_bmc->miscdev.name = DEVICE_NAME,<br>> - bt_bmc->miscdev.fops = &bt_bmc_fops,<br>> - bt_bmc->miscdev.parent = dev;<br>> - rc = misc_register(&bt_bmc->miscdev);<br>> + rc = ipmi_bmc_register_bus(bt_bmc->bmc_ctx, &bt_bmc->bus);<br>> if (rc) {<br>> - dev_err(dev, "Unable to register misc device\n");<br>> + dev_err(dev, "Unable to register IPMI BMC bus\n");<br>> return rc;<br>> }<br>> <br>> @@ -514,11 +403,12 @@ static int bt_bmc_probe(struct platform_device *pdev)<br>> static int bt_bmc_remove(struct platform_device *pdev)<br>> {<br>> struct bt_bmc *bt_bmc = dev_get_drvdata(&pdev->dev);<br>> + int rc;<br>> <br>> - misc_deregister(&bt_bmc->miscdev);<br>> + rc = ipmi_bmc_unregister_bus(bt_bmc->bmc_ctx, &bt_bmc->bus);<br>> if (!bt_bmc->irq)<br>> del_timer_sync(&bt_bmc->poll_timer);<br>> - return 0;<br>> + return rc;<br>> }<br>> <br>> static const struct of_device_id bt_bmc_match[] = {<br>> diff --git a/include/uapi/linux/bt-bmc.h b/include/uapi/linux/bt-bmc.h<br>> deleted file mode 100644<br>> index d9ec766a63d0..000000000000<br>> --- a/include/uapi/linux/bt-bmc.h<br>> +++ /dev/null<br>> @@ -1,18 +0,0 @@<br>> -/*<br>> - * Copyright (c) 2015-2016, IBM Corporation.<br>> - *<br>> - * This program is free software; you can redistribute it and/or<br>> - * modify it under the terms of the GNU General Public License<br>> - * as published by the Free Software Foundation; either version<br>> - * 2 of the License, or (at your option) any later version.<br>> - */<br>> -<br>> -#ifndef _UAPI_LINUX_BT_BMC_H<br>> -#define _UAPI_LINUX_BT_BMC_H<br>> -<br>> -#include <linux/ioctl.h><br>> -<br>> -#define __BT_BMC_IOCTL_MAGIC 0xb1<br>> -#define BT_BMC_IOCTL_SMS_ATN _IO(__BT_BMC_IOCTL_MAGIC, 0x00)<br>> -<br>> -#endif /* _UAPI_LINUX_BT_BMC_H */<br>> -- <br>> 2.14.0.rc1.383.gd1ce394fe2-goog<br>> <br></font></tt><BR>
</body></html>