All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH linux dev-4.7 0/8] BT driver sync up
@ 2016-10-26  6:57 Cédric Le Goater
  2016-10-26  6:57 ` [PATCH linux dev-4.7 1/8] Revert "misc: Add Aspeed BT IPMI host driver" Cédric Le Goater
                   ` (9 more replies)
  0 siblings, 10 replies; 31+ messages in thread
From: Cédric Le Goater @ 2016-10-26  6:57 UTC (permalink / raw)
  To: openbmc

Hello,

Here is a serie syncing the OpenBMC kernel with the BT driver merged
in mainline linux. This kernel needs an updated version of btbridged :

     https://github.com/legoater/btbridge/commits/master

The last two patches add a request expiry list and some configuration
through sysfs. These are for review only as they need to be sent to
mainline first.

Thanks,

C.

Cédric Le Goater (8):
  Revert "misc: Add Aspeed BT IPMI host driver"
  ARM: aspeed: remove previous definitions in default config
  ARM: dts: aspeed: remove previous iBT definitions
  ipmi: add an Aspeed BT IPMI BMC driver
  ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_BMC
  ARM: dts: aspeed: Enable BT IPMI BMC device
  ipmi: maintain a request expiry list
  ipmi: add a sysfs file for configure maximum response time

 .../bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt   |  23 +
 arch/arm/boot/dts/aspeed-g4.dtsi                   |   2 +-
 arch/arm/boot/dts/aspeed-g5.dtsi                   |   2 +-
 arch/arm/configs/aspeed_g4_defconfig               |   2 +-
 arch/arm/configs/aspeed_g5_defconfig               |   2 +-
 drivers/Makefile                                   |   2 +-
 drivers/char/ipmi/Kconfig                          |   8 +
 drivers/char/ipmi/Makefile                         |   1 +
 drivers/char/ipmi/bt-bmc.c                         | 668 +++++++++++++++++++++
 drivers/misc/Kconfig                               |   5 -
 drivers/misc/Makefile                              |   1 -
 drivers/misc/bt-host.c                             | 427 -------------
 include/uapi/linux/Kbuild                          |   2 +-
 include/uapi/linux/{bt-host.h => bt-bmc.h}         |  12 +-
 14 files changed, 712 insertions(+), 445 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
 create mode 100644 drivers/char/ipmi/bt-bmc.c
 delete mode 100644 drivers/misc/bt-host.c
 rename include/uapi/linux/{bt-host.h => bt-bmc.h} (55%)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH linux dev-4.7 1/8] Revert "misc: Add Aspeed BT IPMI host driver"
  2016-10-26  6:57 [PATCH linux dev-4.7 0/8] BT driver sync up Cédric Le Goater
@ 2016-10-26  6:57 ` Cédric Le Goater
  2016-10-26  6:57 ` [PATCH linux dev-4.7 2/8] ARM: aspeed: remove previous definitions in default config Cédric Le Goater
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 31+ messages in thread
From: Cédric Le Goater @ 2016-10-26  6:57 UTC (permalink / raw)
  To: openbmc

This reverts commit f7b4775fdf656f00f1b692bf3db97b3a8088c057.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 drivers/misc/Kconfig         |   5 -
 drivers/misc/Makefile        |   1 -
 drivers/misc/bt-host.c       | 427 -------------------------------------------
 include/uapi/linux/Kbuild    |   1 -
 include/uapi/linux/bt-host.h |  18 --
 5 files changed, 452 deletions(-)
 delete mode 100644 drivers/misc/bt-host.c
 delete mode 100644 include/uapi/linux/bt-host.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 4617ddc3c538..a216b4667742 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -804,11 +804,6 @@ config PANEL_BOOT_MESSAGE
 	  An empty message will only clear the display at driver init time. Any other
 	  printf()-formatted message is valid with newline and escape codes.
 
-config ASPEED_BT_IPMI_HOST
-	tristate "BT IPMI host driver"
-	help
-	  Support for the Aspeed BT ipmi host.
-
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 724861b7f291..b2fb6dbffcef 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -57,4 +57,3 @@ obj-$(CONFIG_ECHO)		+= echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
 obj-$(CONFIG_PANEL)             += panel.o
-obj-$(CONFIG_ASPEED_BT_IPMI_HOST)	+= bt-host.o
diff --git a/drivers/misc/bt-host.c b/drivers/misc/bt-host.c
deleted file mode 100644
index a5e765d683fc..000000000000
--- a/drivers/misc/bt-host.c
+++ /dev/null
@@ -1,427 +0,0 @@
-/*
- * Copyright 2016 IBM Corporation
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#include <linux/module.h>
-#include <linux/moduleparam.h>
-#include <linux/errno.h>
-#include <linux/poll.h>
-#include <linux/sched.h>
-#include <linux/spinlock.h>
-#include <linux/slab.h>
-#include <linux/init.h>
-#include <linux/device.h>
-#include <linux/of.h>
-#include <linux/of_irq.h>
-#include <linux/platform_device.h>
-#include <linux/io.h>
-#include <linux/interrupt.h>
-#include <linux/delay.h>
-#include <linux/miscdevice.h>
-#include <linux/timer.h>
-#include <linux/jiffies.h>
-#include <linux/bt-host.h>
-
-#define DEVICE_NAME	"bt-host"
-
-#define BT_IO_BASE	0xe4
-#define BT_IRQ		10
-
-#define BT_CR0		0x0
-#define   BT_CR0_IO_BASE		16
-#define   BT_CR0_IRQ			12
-#define   BT_CR0_EN_CLR_SLV_RDP		0x8
-#define   BT_CR0_EN_CLR_SLV_WRP		0x4
-#define   BT_CR0_ENABLE_IBT		0x1
-#define BT_CR1		0x4
-#define   BT_CR1_IRQ_H2B	0x01
-#define   BT_CR1_IRQ_HBUSY	0x40
-#define BT_CR2		0x8
-#define   BT_CR2_IRQ_H2B	0x01
-#define   BT_CR2_IRQ_HBUSY	0x40
-#define BT_CR3		0xc
-#define BT_CTRL		0x10
-#define   BT_CTRL_B_BUSY		0x80
-#define   BT_CTRL_H_BUSY		0x40
-#define   BT_CTRL_OEM0			0x20
-#define   BT_CTRL_SMS_ATN		0x10
-#define   BT_CTRL_B2H_ATN		0x08
-#define   BT_CTRL_H2B_ATN		0x04
-#define   BT_CTRL_CLR_RD_PTR		0x02
-#define   BT_CTRL_CLR_WR_PTR		0x01
-#define BT_BMC2HOST	0x14
-#define BT_INTMASK	0x18
-#define   BT_INTMASK_B2H_IRQEN		0x01
-#define   BT_INTMASK_B2H_IRQ		0x02
-#define   BT_INTMASK_BMC_HWRST		0x80
-
-struct bt_host {
-	struct device		dev;
-	struct miscdevice	miscdev;
-	void __iomem		*base;
-	int			open_count;
-	int			irq;
-	wait_queue_head_t	queue;
-	struct timer_list	poll_timer;
-};
-
-static u8 bt_inb(struct bt_host *bt_host, int reg)
-{
-	return ioread8(bt_host->base + reg);
-}
-
-static void bt_outb(struct bt_host *bt_host, u8 data, int reg)
-{
-	iowrite8(data, bt_host->base + reg);
-}
-
-static void clr_rd_ptr(struct bt_host *bt_host)
-{
-	bt_outb(bt_host, BT_CTRL_CLR_RD_PTR, BT_CTRL);
-}
-
-static void clr_wr_ptr(struct bt_host *bt_host)
-{
-	bt_outb(bt_host, BT_CTRL_CLR_WR_PTR, BT_CTRL);
-}
-
-static void clr_h2b_atn(struct bt_host *bt_host)
-{
-	bt_outb(bt_host, BT_CTRL_H2B_ATN, BT_CTRL);
-}
-
-static void set_b_busy(struct bt_host *bt_host)
-{
-	if (!(bt_inb(bt_host, BT_CTRL) & BT_CTRL_B_BUSY))
-		bt_outb(bt_host, BT_CTRL_B_BUSY, BT_CTRL);
-}
-
-static void clr_b_busy(struct bt_host *bt_host)
-{
-	if (bt_inb(bt_host, BT_CTRL) & BT_CTRL_B_BUSY)
-		bt_outb(bt_host, BT_CTRL_B_BUSY, BT_CTRL);
-}
-
-static void set_b2h_atn(struct bt_host *bt_host)
-{
-	bt_outb(bt_host, BT_CTRL_B2H_ATN, BT_CTRL);
-}
-
-static u8 bt_read(struct bt_host *bt_host)
-{
-	return bt_inb(bt_host, BT_BMC2HOST);
-}
-
-static void bt_write(struct bt_host *bt_host, u8 c)
-{
-	bt_outb(bt_host, c, BT_BMC2HOST);
-}
-
-static void set_sms_atn(struct bt_host *bt_host)
-{
-	bt_outb(bt_host, BT_CTRL_SMS_ATN, BT_CTRL);
-}
-
-static struct bt_host *file_bt_host(struct file *file)
-{
-	return container_of(file->private_data, struct bt_host, miscdev);
-}
-
-static int bt_host_open(struct inode *inode, struct file *file)
-{
-	struct bt_host *bt_host = file_bt_host(file);
-
-	clr_b_busy(bt_host);
-
-	return 0;
-}
-
-static ssize_t bt_host_read(struct file *file, char __user *buf,
-				size_t count, loff_t *ppos)
-{
-	struct bt_host *bt_host = file_bt_host(file);
-	char __user *p = buf;
-	u8 len;
-
-	if (!access_ok(VERIFY_WRITE, buf, count))
-		return -EFAULT;
-
-	WARN_ON(*ppos);
-
-	if (wait_event_interruptible(bt_host->queue,
-				bt_inb(bt_host, BT_CTRL) & BT_CTRL_H2B_ATN))
-		return -ERESTARTSYS;
-
-	set_b_busy(bt_host);
-	clr_h2b_atn(bt_host);
-	clr_rd_ptr(bt_host);
-
-	len = bt_read(bt_host);
-	__put_user(len, p++);
-
-	/* We pass the length back as well */
-	if (len + 1 > count)
-		len = count - 1;
-
-	while(len) {
-		if (__put_user(bt_read(bt_host), p))
-			return -EFAULT;
-		len--; p++;
-	}
-
-	clr_b_busy(bt_host);
-
-	return p - buf;
-}
-
-static ssize_t bt_host_write(struct file *file, const char __user *buf,
-				size_t count, loff_t *ppos)
-{
-	struct bt_host *bt_host = file_bt_host(file);
-	const char __user *p = buf;
-	u8 c;
-
-	if (!access_ok(VERIFY_READ, buf, count))
-		return -EFAULT;
-
-	WARN_ON(*ppos);
-
-	/* There's no interrupt for clearing host busy so we have to
-	 * poll */
-	if (wait_event_interruptible(bt_host->queue,
-				!(bt_inb(bt_host, BT_CTRL) &
-					(BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN))))
-		return -ERESTARTSYS;
-
-	clr_wr_ptr(bt_host);
-
-	while (count) {
-		if (__get_user(c, p))
-			return -EFAULT;
-
-		bt_write(bt_host, c);
-		count--; p++;
-	}
-
-	set_b2h_atn(bt_host);
-
-	return p - buf;
-}
-
-static long bt_host_ioctl(struct file *file, unsigned int cmd,
-		unsigned long param)
-{
-	struct bt_host *bt_host = file_bt_host(file);
-	switch (cmd) {
-	case BT_HOST_IOCTL_SMS_ATN:
-		set_sms_atn(bt_host);
-		return 0;
-	}
-	return -EINVAL;
-}
-
-static int bt_host_release(struct inode *inode, struct file *file)
-{
-	struct bt_host *bt_host = file_bt_host(file);
-	set_b_busy(bt_host);
-	return 0;
-}
-
-static unsigned int bt_host_poll(struct file *file, poll_table *wait)
-{
-	struct bt_host *bt_host = file_bt_host(file);
-	unsigned int mask = 0;
-	uint8_t ctrl;
-
-	poll_wait(file, &bt_host->queue, wait);
-
-	ctrl = bt_inb(bt_host, BT_CTRL);
-
-	if (ctrl & BT_CTRL_H2B_ATN)
-		mask |= POLLIN;
-
-	if (!(ctrl & (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN)))
-		mask |= POLLOUT;
-
-	return mask;
-}
-
-static const struct file_operations bt_host_fops = {
-	.owner		= THIS_MODULE,
-	.open		= bt_host_open,
-	.read		= bt_host_read,
-	.write		= bt_host_write,
-	.release	= bt_host_release,
-	.poll		= bt_host_poll,
-	.unlocked_ioctl	= bt_host_ioctl,
-};
-
-static void poll_timer(unsigned long data)
-{
-	struct bt_host *bt_host = (void *)data;
-	bt_host->poll_timer.expires += msecs_to_jiffies(500);
-	wake_up(&bt_host->queue);
-	add_timer(&bt_host->poll_timer);
-}
-
-static irqreturn_t bt_host_irq(int irq, void *arg)
-{
-	struct bt_host *bt_host = arg;
-	uint32_t reg;
-
-	reg = ioread32(bt_host->base + BT_CR2);
-	reg &= BT_CR2_IRQ_H2B | BT_CR2_IRQ_HBUSY;
-	if (!reg)
-		return IRQ_NONE;
-
-	/* ack pending IRQs */
-	iowrite32(reg, bt_host->base + BT_CR2);
-
-	wake_up(&bt_host->queue);
-	return IRQ_HANDLED;
-}
-
-static int bt_host_config_irq(struct bt_host *bt_host,
-		struct platform_device *pdev)
-{
-	struct device *dev = &pdev->dev;
-	uint32_t reg;
-	int rc;
-
-	bt_host->irq = irq_of_parse_and_map(dev->of_node, 0);
-	if (!bt_host->irq)
-		return -ENODEV;
-
-	rc = devm_request_irq(dev, bt_host->irq, bt_host_irq, IRQF_SHARED,
-			DEVICE_NAME, bt_host);
-	if (rc < 0) {
-		dev_warn(dev, "Unable to request IRQ %d\n", bt_host->irq);
-		bt_host->irq = 0;
-		return rc;
-	}
-
-	/* Configure IRQs on the host clearing the H2B and HBUSY bits;
-	 * H2B will be asserted when the host has data for us; HBUSY
-	 * will be cleared (along with B2H) when we can write the next
-	 * message to the BT buffer */
-	reg = ioread32(bt_host->base + BT_CR1);
-	reg |= BT_CR1_IRQ_H2B | BT_CR1_IRQ_HBUSY;
-	iowrite32(reg, bt_host->base + BT_CR1);
-
-	return 0;
-}
-
-static int bt_host_probe(struct platform_device *pdev)
-{
-	struct bt_host *bt_host;
-	struct device *dev;
-	struct resource *res;
-	int rc;
-
-	if (!pdev || !pdev->dev.of_node)
-		return -ENODEV;
-
-	dev = &pdev->dev;
-	dev_info(dev, "Found bt host device\n");
-
-	bt_host = devm_kzalloc(dev, sizeof(*bt_host), GFP_KERNEL);
-	if (!bt_host)
-		return -ENOMEM;
-
-	dev_set_drvdata(&pdev->dev, bt_host);
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(dev, "Unable to find resources\n");
-		rc = -ENXIO;
-		goto out_free;
-	}
-
-	bt_host->base = devm_ioremap_resource(&pdev->dev, res);
-	if (!bt_host->base) {
-		rc = -ENOMEM;
-		goto out_free;
-	}
-
-	init_waitqueue_head(&bt_host->queue);
-
-	bt_host->miscdev.minor	= MISC_DYNAMIC_MINOR,
-	bt_host->miscdev.name	= DEVICE_NAME,
-	bt_host->miscdev.fops	= &bt_host_fops,
-	bt_host->miscdev.parent = dev;
-	rc = misc_register(&bt_host->miscdev);
-	if (rc) {
-		dev_err(dev, "Unable to register device\n");
-		goto out_unmap;
-	}
-
-	bt_host_config_irq(bt_host, pdev);
-
-	if (bt_host->irq) {
-		dev_info(dev, "Using IRQ %d\n", bt_host->irq);
-	} else {
-		dev_info(dev, "No IRQ; using timer\n");
-		setup_timer(&bt_host->poll_timer, poll_timer,
-				(unsigned long)bt_host);
-		bt_host->poll_timer.expires = jiffies + msecs_to_jiffies(10);
-		add_timer(&bt_host->poll_timer);
-	}
-
-	iowrite32((BT_IO_BASE << BT_CR0_IO_BASE) |
-		  (BT_IRQ << BT_CR0_IRQ) |
-		  BT_CR0_EN_CLR_SLV_RDP |
-		  BT_CR0_EN_CLR_SLV_WRP |
-		  BT_CR0_ENABLE_IBT,
-		  bt_host->base + BT_CR0);
-
-	clr_b_busy(bt_host);
-
-	return 0;
-
-out_unmap:
-	devm_iounmap(&pdev->dev, bt_host->base);
-
-out_free:
-	devm_kfree(dev, bt_host);
-	return rc;
-
-}
-
-static int bt_host_remove(struct platform_device *pdev)
-{
-	struct bt_host *bt_host = dev_get_drvdata(&pdev->dev);
-	misc_deregister(&bt_host->miscdev);
-	if (!bt_host->irq)
-		del_timer_sync(&bt_host->poll_timer);
-	devm_iounmap(&pdev->dev, bt_host->base);
-	devm_kfree(&pdev->dev, bt_host);
-	bt_host = NULL;
-
-	return 0;
-}
-
-static const struct of_device_id bt_host_match[] = {
-	{ .compatible = "aspeed,bt-host" },
-	{ },
-};
-
-static struct platform_driver bt_host_driver = {
-	.driver = {
-		.name		= DEVICE_NAME,
-		.of_match_table = bt_host_match,
-	},
-	.probe = bt_host_probe,
-	.remove = bt_host_remove,
-};
-
-module_platform_driver(bt_host_driver);
-
-MODULE_DEVICE_TABLE(of, bt_host_match);
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Alistair Popple <alistair@popple.id.au>");
-MODULE_DESCRIPTION("Linux device interface to the BT interface");
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index b55c81eff9d7..ec10cfef166a 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -74,7 +74,6 @@ header-y += bpf_common.h
 header-y += bpf.h
 header-y += bpqether.h
 header-y += bsg.h
-header-y += bt-host.h
 header-y += btrfs.h
 header-y += can.h
 header-y += capability.h
diff --git a/include/uapi/linux/bt-host.h b/include/uapi/linux/bt-host.h
deleted file mode 100644
index a4298d9e7e26..000000000000
--- a/include/uapi/linux/bt-host.h
+++ /dev/null
@@ -1,18 +0,0 @@
-/*
- * Copyright 2015 IBM Corp.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#ifndef _UAPI_LINUX_BT_HOST_H
-#define _UAPI_LINUX_BT_HOST_H
-
-#include <linux/ioctl.h>
-
-#define __BT_HOST_IOCTL_MAGIC	0xb1
-#define BT_HOST_IOCTL_SMS_ATN	_IO(__BT_HOST_IOCTL_MAGIC, 0x00)
-
-#endif /* _UAPI_LINUX_BT_HOST_H */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH linux dev-4.7 2/8] ARM: aspeed: remove previous definitions in default config
  2016-10-26  6:57 [PATCH linux dev-4.7 0/8] BT driver sync up Cédric Le Goater
  2016-10-26  6:57 ` [PATCH linux dev-4.7 1/8] Revert "misc: Add Aspeed BT IPMI host driver" Cédric Le Goater
@ 2016-10-26  6:57 ` Cédric Le Goater
  2016-10-26  6:57 ` [PATCH linux dev-4.7 3/8] ARM: dts: aspeed: remove previous iBT definitions Cédric Le Goater
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 31+ messages in thread
From: Cédric Le Goater @ 2016-10-26  6:57 UTC (permalink / raw)
  To: openbmc

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/arm/configs/aspeed_g4_defconfig | 1 -
 arch/arm/configs/aspeed_g5_defconfig | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig
index 5cc4d4121462..fab53889b2cc 100644
--- a/arch/arm/configs/aspeed_g4_defconfig
+++ b/arch/arm/configs/aspeed_g4_defconfig
@@ -60,7 +60,6 @@ CONFIG_MTD_BLOCK=y
 CONFIG_MTD_SPI_NOR=y
 CONFIG_ASPEED_FLASH_SPI=y
 CONFIG_BLK_DEV_RAM=y
-CONFIG_ASPEED_BT_IPMI_HOST=y
 CONFIG_EEPROM_AT24=y
 CONFIG_NETDEVICES=y
 # CONFIG_NET_VENDOR_ARC is not set
diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index d19cf3b498cf..3ccc91db96e3 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -62,7 +62,6 @@ CONFIG_MTD_BLOCK=y
 CONFIG_MTD_SPI_NOR=y
 CONFIG_ASPEED_FLASH_SPI=y
 CONFIG_BLK_DEV_RAM=y
-CONFIG_ASPEED_BT_IPMI_HOST=y
 CONFIG_EEPROM_AT24=y
 CONFIG_NETDEVICES=y
 # CONFIG_NET_VENDOR_ARC is not set
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH linux dev-4.7 3/8] ARM: dts: aspeed: remove previous iBT definitions
  2016-10-26  6:57 [PATCH linux dev-4.7 0/8] BT driver sync up Cédric Le Goater
  2016-10-26  6:57 ` [PATCH linux dev-4.7 1/8] Revert "misc: Add Aspeed BT IPMI host driver" Cédric Le Goater
  2016-10-26  6:57 ` [PATCH linux dev-4.7 2/8] ARM: aspeed: remove previous definitions in default config Cédric Le Goater
@ 2016-10-26  6:57 ` Cédric Le Goater
  2016-10-26  6:57 ` [PATCH linux dev-4.7 4/8] ipmi: add an Aspeed BT IPMI BMC driver Cédric Le Goater
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 31+ messages in thread
From: Cédric Le Goater @ 2016-10-26  6:57 UTC (permalink / raw)
  To: openbmc

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/arm/boot/dts/aspeed-g4.dtsi | 6 ------
 arch/arm/boot/dts/aspeed-g5.dtsi | 6 ------
 2 files changed, 12 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
index 68eadec4ab34..cfe5c95d863b 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -857,12 +857,6 @@
 				clocks = <&clk_apb>;
 			};
 
-			ibt: ibt@1e789140 {
-				compatible = "aspeed,bt-host";
-				reg = <0x1e789140 0x18>;
-				interrupts = <8>;
-			};
-
 			wdt1: wdt@1e785000 {
 				compatible = "aspeed,ast2400-wdt";
 				reg = <0x1e785000 0x1c>;
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index d72aea7ab8f2..163393a748ed 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -991,12 +991,6 @@
 				clocks = <&clk_apb>;
 			};
 
-			ibt: ibt@1e789140 {
-				compatible = "aspeed,bt-host";
-				reg = <0x1e789140 0x18>;
-				interrupts = <8>;
-			};
-
 			wdt1: wdt@1e785000 {
 				compatible = "aspeed,ast2500-wdt";
 				reg = <0x1e785000 0x1c>;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH linux dev-4.7 4/8] ipmi: add an Aspeed BT IPMI BMC driver
  2016-10-26  6:57 [PATCH linux dev-4.7 0/8] BT driver sync up Cédric Le Goater
                   ` (2 preceding siblings ...)
  2016-10-26  6:57 ` [PATCH linux dev-4.7 3/8] ARM: dts: aspeed: remove previous iBT definitions Cédric Le Goater
@ 2016-10-26  6:57 ` Cédric Le Goater
  2016-11-02  0:00   ` Joel Stanley
  2016-11-03  0:56   ` Cyril Bur
  2016-10-26  6:57 ` [PATCH linux dev-4.7 5/8] ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_BMC Cédric Le Goater
                   ` (5 subsequent siblings)
  9 siblings, 2 replies; 31+ messages in thread
From: Cédric Le Goater @ 2016-10-26  6:57 UTC (permalink / raw)
  To: openbmc

Backport from mainline of the main IPMI BMC driver patch plus fixes :

 - commit d94655b405ba ("ipmi/bt-bmc: remove redundant return value
   check of platform_get_resource()")
 - commit a3e6061bad62 ("ipmi/bt-bmc: add a dependency on
   ARCH_ASPEED")
 - commit 1a377a79211a ("ipmi: Fix ioremap error handling in bt-bmc")
 - commit 54f9c4d0778b ("ipmi: add an Aspeed BT IPMI BMC driver")

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 .../bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt   |  23 +
 drivers/Makefile                                   |   2 +-
 drivers/char/ipmi/Kconfig                          |   8 +
 drivers/char/ipmi/Makefile                         |   1 +
 drivers/char/ipmi/bt-bmc.c                         | 505 +++++++++++++++++++++
 include/uapi/linux/Kbuild                          |   1 +
 include/uapi/linux/bt-bmc.h                        |  18 +
 7 files changed, 557 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
 create mode 100644 drivers/char/ipmi/bt-bmc.c
 create mode 100644 include/uapi/linux/bt-bmc.h

diff --git a/Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt b/Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
new file mode 100644
index 000000000000..fbbacd958240
--- /dev/null
+++ b/Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
@@ -0,0 +1,23 @@
+* Aspeed BT (Block Transfer) IPMI interface
+
+The Aspeed SOCs (AST2400 and AST2500) are commonly used as BMCs
+(BaseBoard Management Controllers) and the BT interface can be used to
+perform in-band IPMI communication with their host.
+
+Required properties:
+
+- compatible : should be "aspeed,ast2400-bt-bmc"
+- reg: physical address and size of the registers
+
+Optional properties:
+
+- interrupts: interrupt generated by the BT interface. without an
+  interrupt, the driver will operate in poll mode.
+
+Example:
+
+	ibt@1e789140 {
+		compatible = "aspeed,ast2400-bt-bmc";
+		reg = <0x1e789140 0x18>;
+		interrupts = <8>;
+	};
diff --git a/drivers/Makefile b/drivers/Makefile
index 0b6f3d60193d..0a7c60d3b6cc 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -21,7 +21,7 @@ obj-y				+= video/
 obj-y				+= idle/
 
 # IPMI must come before ACPI in order to provide IPMI opregion support
-obj-$(CONFIG_IPMI_HANDLER)	+= char/ipmi/
+obj-y				+= char/ipmi/
 
 obj-$(CONFIG_ACPI)		+= acpi/
 obj-$(CONFIG_SFI)		+= sfi/
diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index 6ed9e9fe5233..927fe355442a 100644
--- a/drivers/char/ipmi/Kconfig
+++ b/drivers/char/ipmi/Kconfig
@@ -88,3 +88,11 @@ config IPMI_POWEROFF
 	 the IPMI management controller is capable of this.
 
 endif # IPMI_HANDLER
+
+config ASPEED_BT_IPMI_BMC
+	depends on ARCH_ASPEED
+	tristate "BT IPMI bmc driver"
+	help
+	  Provides a driver for the BT (Block Transfer) IPMI interface
+	  found on Aspeed SOCs (AST2400 and AST2500). The driver
+	  implements the BMC side of the BT interface.
diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
index f3ffde1f5f1f..0d98cd91def1 100644
--- a/drivers/char/ipmi/Makefile
+++ b/drivers/char/ipmi/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_IPMI_SSIF) += ipmi_ssif.o
 obj-$(CONFIG_IPMI_POWERNV) += ipmi_powernv.o
 obj-$(CONFIG_IPMI_WATCHDOG) += ipmi_watchdog.o
 obj-$(CONFIG_IPMI_POWEROFF) += ipmi_poweroff.o
+obj-$(CONFIG_ASPEED_BT_IPMI_BMC) += bt-bmc.o
diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
new file mode 100644
index 000000000000..b49e61320952
--- /dev/null
+++ b/drivers/char/ipmi/bt-bmc.c
@@ -0,0 +1,505 @@
+/*
+ * Copyright (c) 2015-2016, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/atomic.h>
+#include <linux/bt-bmc.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/poll.h>
+#include <linux/sched.h>
+#include <linux/timer.h>
+
+/*
+ * This is a BMC device used to communicate to the host
+ */
+#define DEVICE_NAME	"ipmi-bt-host"
+
+#define BT_IO_BASE	0xe4
+#define BT_IRQ		10
+
+#define BT_CR0		0x0
+#define   BT_CR0_IO_BASE		16
+#define   BT_CR0_IRQ			12
+#define   BT_CR0_EN_CLR_SLV_RDP		0x8
+#define   BT_CR0_EN_CLR_SLV_WRP		0x4
+#define   BT_CR0_ENABLE_IBT		0x1
+#define BT_CR1		0x4
+#define   BT_CR1_IRQ_H2B	0x01
+#define   BT_CR1_IRQ_HBUSY	0x40
+#define BT_CR2		0x8
+#define   BT_CR2_IRQ_H2B	0x01
+#define   BT_CR2_IRQ_HBUSY	0x40
+#define BT_CR3		0xc
+#define BT_CTRL		0x10
+#define   BT_CTRL_B_BUSY		0x80
+#define   BT_CTRL_H_BUSY		0x40
+#define   BT_CTRL_OEM0			0x20
+#define   BT_CTRL_SMS_ATN		0x10
+#define   BT_CTRL_B2H_ATN		0x08
+#define   BT_CTRL_H2B_ATN		0x04
+#define   BT_CTRL_CLR_RD_PTR		0x02
+#define   BT_CTRL_CLR_WR_PTR		0x01
+#define BT_BMC2HOST	0x14
+#define BT_INTMASK	0x18
+#define   BT_INTMASK_B2H_IRQEN		0x01
+#define   BT_INTMASK_B2H_IRQ		0x02
+#define   BT_INTMASK_BMC_HWRST		0x80
+
+#define BT_BMC_BUFFER_SIZE 256
+
+struct bt_bmc {
+	struct device		dev;
+	struct miscdevice	miscdev;
+	void __iomem		*base;
+	int			irq;
+	wait_queue_head_t	queue;
+	struct timer_list	poll_timer;
+	struct mutex		mutex;
+};
+
+static atomic_t open_count = ATOMIC_INIT(0);
+
+static u8 bt_inb(struct bt_bmc *bt_bmc, int reg)
+{
+	return ioread8(bt_bmc->base + reg);
+}
+
+static void bt_outb(struct bt_bmc *bt_bmc, u8 data, int reg)
+{
+	iowrite8(data, bt_bmc->base + reg);
+}
+
+static void clr_rd_ptr(struct bt_bmc *bt_bmc)
+{
+	bt_outb(bt_bmc, BT_CTRL_CLR_RD_PTR, BT_CTRL);
+}
+
+static void clr_wr_ptr(struct bt_bmc *bt_bmc)
+{
+	bt_outb(bt_bmc, BT_CTRL_CLR_WR_PTR, BT_CTRL);
+}
+
+static void clr_h2b_atn(struct bt_bmc *bt_bmc)
+{
+	bt_outb(bt_bmc, BT_CTRL_H2B_ATN, BT_CTRL);
+}
+
+static void set_b_busy(struct bt_bmc *bt_bmc)
+{
+	if (!(bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_B_BUSY))
+		bt_outb(bt_bmc, BT_CTRL_B_BUSY, BT_CTRL);
+}
+
+static void clr_b_busy(struct bt_bmc *bt_bmc)
+{
+	if (bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_B_BUSY)
+		bt_outb(bt_bmc, BT_CTRL_B_BUSY, BT_CTRL);
+}
+
+static void set_b2h_atn(struct bt_bmc *bt_bmc)
+{
+	bt_outb(bt_bmc, BT_CTRL_B2H_ATN, BT_CTRL);
+}
+
+static u8 bt_read(struct bt_bmc *bt_bmc)
+{
+	return bt_inb(bt_bmc, BT_BMC2HOST);
+}
+
+static ssize_t bt_readn(struct bt_bmc *bt_bmc, u8 *buf, size_t n)
+{
+	int i;
+
+	for (i = 0; i < n; i++)
+		buf[i] = bt_read(bt_bmc);
+	return n;
+}
+
+static void bt_write(struct bt_bmc *bt_bmc, u8 c)
+{
+	bt_outb(bt_bmc, c, BT_BMC2HOST);
+}
+
+static ssize_t bt_writen(struct bt_bmc *bt_bmc, u8 *buf, size_t n)
+{
+	int i;
+
+	for (i = 0; i < n; i++)
+		bt_write(bt_bmc, buf[i]);
+	return n;
+}
+
+static void set_sms_atn(struct bt_bmc *bt_bmc)
+{
+	bt_outb(bt_bmc, BT_CTRL_SMS_ATN, BT_CTRL);
+}
+
+static struct bt_bmc *file_bt_bmc(struct file *file)
+{
+	return container_of(file->private_data, struct bt_bmc, miscdev);
+}
+
+static int bt_bmc_open(struct inode *inode, struct file *file)
+{
+	struct bt_bmc *bt_bmc = file_bt_bmc(file);
+
+	if (atomic_inc_return(&open_count) == 1) {
+		clr_b_busy(bt_bmc);
+		return 0;
+	}
+
+	atomic_dec(&open_count);
+	return -EBUSY;
+}
+
+/*
+ * The BT (Block Transfer) interface means that entire messages are
+ * buffered by the host before a notification is sent to the BMC that
+ * there is data to be read. The first byte is the length and the
+ * message data follows. The read operation just tries to capture the
+ * whole before returning it to userspace.
+ *
+ * BT Message format :
+ *
+ *    Byte 1  Byte 2     Byte 3  Byte 4  Byte 5:N
+ *    Length  NetFn/LUN  Seq     Cmd     Data
+ *
+ */
+static ssize_t bt_bmc_read(struct file *file, char __user *buf,
+			   size_t count, loff_t *ppos)
+{
+	struct bt_bmc *bt_bmc = file_bt_bmc(file);
+	u8 len;
+	int len_byte = 1;
+	u8 kbuffer[BT_BMC_BUFFER_SIZE];
+	ssize_t ret = 0;
+	ssize_t nread;
+
+	if (!access_ok(VERIFY_WRITE, buf, count))
+		return -EFAULT;
+
+	WARN_ON(*ppos);
+
+	if (wait_event_interruptible(bt_bmc->queue,
+				     bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_H2B_ATN))
+		return -ERESTARTSYS;
+
+	mutex_lock(&bt_bmc->mutex);
+
+	if (unlikely(!(bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_H2B_ATN))) {
+		ret = -EIO;
+		goto out_unlock;
+	}
+
+	set_b_busy(bt_bmc);
+	clr_h2b_atn(bt_bmc);
+	clr_rd_ptr(bt_bmc);
+
+	/*
+	 * The BT frames start with the message length, which does not
+	 * include the length byte.
+	 */
+	kbuffer[0] = bt_read(bt_bmc);
+	len = kbuffer[0];
+
+	/* We pass the length back to userspace as well */
+	if (len + 1 > count)
+		len = count - 1;
+
+	while (len) {
+		nread = min_t(ssize_t, len, sizeof(kbuffer) - len_byte);
+
+		bt_readn(bt_bmc, kbuffer + len_byte, nread);
+
+		if (copy_to_user(buf, kbuffer, nread + len_byte)) {
+			ret = -EFAULT;
+			break;
+		}
+		len -= nread;
+		buf += nread + len_byte;
+		ret += nread + len_byte;
+		len_byte = 0;
+	}
+
+	clr_b_busy(bt_bmc);
+
+out_unlock:
+	mutex_unlock(&bt_bmc->mutex);
+	return ret;
+}
+
+/*
+ * BT Message response format :
+ *
+ *    Byte 1  Byte 2     Byte 3  Byte 4  Byte 5  Byte 6:N
+ *    Length  NetFn/LUN  Seq     Cmd     Code    Data
+ */
+static ssize_t bt_bmc_write(struct file *file, const char __user *buf,
+			    size_t count, loff_t *ppos)
+{
+	struct bt_bmc *bt_bmc = file_bt_bmc(file);
+	u8 kbuffer[BT_BMC_BUFFER_SIZE];
+	ssize_t ret = 0;
+	ssize_t nwritten;
+
+	/*
+	 * send a minimum response size
+	 */
+	if (count < 5)
+		return -EINVAL;
+
+	if (!access_ok(VERIFY_READ, buf, count))
+		return -EFAULT;
+
+	WARN_ON(*ppos);
+
+	/*
+	 * There's no interrupt for clearing bmc busy so we have to
+	 * poll
+	 */
+	if (wait_event_interruptible(bt_bmc->queue,
+				     !(bt_inb(bt_bmc, BT_CTRL) &
+				       (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN))))
+		return -ERESTARTSYS;
+
+	mutex_lock(&bt_bmc->mutex);
+
+	if (unlikely(bt_inb(bt_bmc, BT_CTRL) &
+		     (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN))) {
+		ret = -EIO;
+		goto out_unlock;
+	}
+
+	clr_wr_ptr(bt_bmc);
+
+	while (count) {
+		nwritten = min_t(ssize_t, count, sizeof(kbuffer));
+		if (copy_from_user(&kbuffer, buf, nwritten)) {
+			ret = -EFAULT;
+			break;
+		}
+
+		bt_writen(bt_bmc, kbuffer, nwritten);
+
+		count -= nwritten;
+		buf += nwritten;
+		ret += nwritten;
+	}
+
+	set_b2h_atn(bt_bmc);
+
+out_unlock:
+	mutex_unlock(&bt_bmc->mutex);
+	return ret;
+}
+
+static long bt_bmc_ioctl(struct file *file, unsigned int cmd,
+			 unsigned long param)
+{
+	struct bt_bmc *bt_bmc = file_bt_bmc(file);
+
+	switch (cmd) {
+	case BT_BMC_IOCTL_SMS_ATN:
+		set_sms_atn(bt_bmc);
+		return 0;
+	}
+	return -EINVAL;
+}
+
+static int bt_bmc_release(struct inode *inode, struct file *file)
+{
+	struct bt_bmc *bt_bmc = file_bt_bmc(file);
+
+	atomic_dec(&open_count);
+	set_b_busy(bt_bmc);
+	return 0;
+}
+
+static unsigned int bt_bmc_poll(struct file *file, poll_table *wait)
+{
+	struct bt_bmc *bt_bmc = file_bt_bmc(file);
+	unsigned int mask = 0;
+	u8 ctrl;
+
+	poll_wait(file, &bt_bmc->queue, wait);
+
+	ctrl = bt_inb(bt_bmc, BT_CTRL);
+
+	if (ctrl & BT_CTRL_H2B_ATN)
+		mask |= POLLIN;
+
+	if (!(ctrl & (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN)))
+		mask |= POLLOUT;
+
+	return mask;
+}
+
+static const struct file_operations bt_bmc_fops = {
+	.owner		= THIS_MODULE,
+	.open		= bt_bmc_open,
+	.read		= bt_bmc_read,
+	.write		= bt_bmc_write,
+	.release	= bt_bmc_release,
+	.poll		= bt_bmc_poll,
+	.unlocked_ioctl	= bt_bmc_ioctl,
+};
+
+static void poll_timer(unsigned long data)
+{
+	struct bt_bmc *bt_bmc = (void *)data;
+
+	bt_bmc->poll_timer.expires += msecs_to_jiffies(500);
+	wake_up(&bt_bmc->queue);
+	add_timer(&bt_bmc->poll_timer);
+}
+
+static irqreturn_t bt_bmc_irq(int irq, void *arg)
+{
+	struct bt_bmc *bt_bmc = arg;
+	u32 reg;
+
+	reg = ioread32(bt_bmc->base + BT_CR2);
+	reg &= BT_CR2_IRQ_H2B | BT_CR2_IRQ_HBUSY;
+	if (!reg)
+		return IRQ_NONE;
+
+	/* ack pending IRQs */
+	iowrite32(reg, bt_bmc->base + BT_CR2);
+
+	wake_up(&bt_bmc->queue);
+	return IRQ_HANDLED;
+}
+
+static int bt_bmc_config_irq(struct bt_bmc *bt_bmc,
+			     struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	u32 reg;
+	int rc;
+
+	bt_bmc->irq = platform_get_irq(pdev, 0);
+	if (!bt_bmc->irq)
+		return -ENODEV;
+
+	rc = devm_request_irq(dev, bt_bmc->irq, bt_bmc_irq, IRQF_SHARED,
+			      DEVICE_NAME, bt_bmc);
+	if (rc < 0) {
+		dev_warn(dev, "Unable to request IRQ %d\n", bt_bmc->irq);
+		bt_bmc->irq = 0;
+		return rc;
+	}
+
+	/*
+	 * Configure IRQs on the bmc clearing the H2B and HBUSY bits;
+	 * H2B will be asserted when the bmc has data for us; HBUSY
+	 * will be cleared (along with B2H) when we can write the next
+	 * message to the BT buffer
+	 */
+	reg = ioread32(bt_bmc->base + BT_CR1);
+	reg |= BT_CR1_IRQ_H2B | BT_CR1_IRQ_HBUSY;
+	iowrite32(reg, bt_bmc->base + BT_CR1);
+
+	return 0;
+}
+
+static int bt_bmc_probe(struct platform_device *pdev)
+{
+	struct bt_bmc *bt_bmc;
+	struct device *dev;
+	struct resource *res;
+	int rc;
+
+	if (!pdev || !pdev->dev.of_node)
+		return -ENODEV;
+
+	dev = &pdev->dev;
+	dev_info(dev, "Found bt bmc device\n");
+
+	bt_bmc = devm_kzalloc(dev, sizeof(*bt_bmc), GFP_KERNEL);
+	if (!bt_bmc)
+		return -ENOMEM;
+
+	dev_set_drvdata(&pdev->dev, bt_bmc);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	bt_bmc->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(bt_bmc->base))
+		return PTR_ERR(bt_bmc->base);
+
+	mutex_init(&bt_bmc->mutex);
+	init_waitqueue_head(&bt_bmc->queue);
+
+	bt_bmc->miscdev.minor	= MISC_DYNAMIC_MINOR,
+		bt_bmc->miscdev.name	= DEVICE_NAME,
+		bt_bmc->miscdev.fops	= &bt_bmc_fops,
+		bt_bmc->miscdev.parent = dev;
+	rc = misc_register(&bt_bmc->miscdev);
+	if (rc) {
+		dev_err(dev, "Unable to register misc device\n");
+		return rc;
+	}
+
+	bt_bmc_config_irq(bt_bmc, pdev);
+
+	if (bt_bmc->irq) {
+		dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
+	} else {
+		dev_info(dev, "No IRQ; using timer\n");
+		setup_timer(&bt_bmc->poll_timer, poll_timer,
+			    (unsigned long)bt_bmc);
+		bt_bmc->poll_timer.expires = jiffies + msecs_to_jiffies(10);
+		add_timer(&bt_bmc->poll_timer);
+	}
+
+	iowrite32((BT_IO_BASE << BT_CR0_IO_BASE) |
+		  (BT_IRQ << BT_CR0_IRQ) |
+		  BT_CR0_EN_CLR_SLV_RDP |
+		  BT_CR0_EN_CLR_SLV_WRP |
+		  BT_CR0_ENABLE_IBT,
+		  bt_bmc->base + BT_CR0);
+
+	clr_b_busy(bt_bmc);
+
+	return 0;
+}
+
+static int bt_bmc_remove(struct platform_device *pdev)
+{
+	struct bt_bmc *bt_bmc = dev_get_drvdata(&pdev->dev);
+
+	misc_deregister(&bt_bmc->miscdev);
+	if (!bt_bmc->irq)
+		del_timer_sync(&bt_bmc->poll_timer);
+	return 0;
+}
+
+static const struct of_device_id bt_bmc_match[] = {
+	{ .compatible = "aspeed,ast2400-bt-bmc" },
+	{ },
+};
+
+static struct platform_driver bt_bmc_driver = {
+	.driver = {
+		.name		= DEVICE_NAME,
+		.of_match_table = bt_bmc_match,
+	},
+	.probe = bt_bmc_probe,
+	.remove = bt_bmc_remove,
+};
+
+module_platform_driver(bt_bmc_driver);
+
+MODULE_DEVICE_TABLE(of, bt_bmc_match);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Alistair Popple <alistair@popple.id.au>");
+MODULE_DESCRIPTION("Linux device interface to the BT interface");
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index ec10cfef166a..9493842de93b 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -74,6 +74,7 @@ header-y += bpf_common.h
 header-y += bpf.h
 header-y += bpqether.h
 header-y += bsg.h
+header-y += bt-bmc.h
 header-y += btrfs.h
 header-y += can.h
 header-y += capability.h
diff --git a/include/uapi/linux/bt-bmc.h b/include/uapi/linux/bt-bmc.h
new file mode 100644
index 000000000000..d9ec766a63d0
--- /dev/null
+++ b/include/uapi/linux/bt-bmc.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright (c) 2015-2016, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#ifndef _UAPI_LINUX_BT_BMC_H
+#define _UAPI_LINUX_BT_BMC_H
+
+#include <linux/ioctl.h>
+
+#define __BT_BMC_IOCTL_MAGIC	0xb1
+#define BT_BMC_IOCTL_SMS_ATN	_IO(__BT_BMC_IOCTL_MAGIC, 0x00)
+
+#endif /* _UAPI_LINUX_BT_BMC_H */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH linux dev-4.7 5/8] ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_BMC
  2016-10-26  6:57 [PATCH linux dev-4.7 0/8] BT driver sync up Cédric Le Goater
                   ` (3 preceding siblings ...)
  2016-10-26  6:57 ` [PATCH linux dev-4.7 4/8] ipmi: add an Aspeed BT IPMI BMC driver Cédric Le Goater
@ 2016-10-26  6:57 ` Cédric Le Goater
  2016-10-26  6:57 ` [PATCH linux dev-4.7 6/8] ARM: dts: aspeed: Enable BT IPMI BMC device Cédric Le Goater
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 31+ messages in thread
From: Cédric Le Goater @ 2016-10-26  6:57 UTC (permalink / raw)
  To: openbmc

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/arm/configs/aspeed_g4_defconfig | 1 +
 arch/arm/configs/aspeed_g5_defconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig
index fab53889b2cc..f77acab80c00 100644
--- a/arch/arm/configs/aspeed_g4_defconfig
+++ b/arch/arm/configs/aspeed_g4_defconfig
@@ -101,6 +101,7 @@ CONFIG_SERIAL_8250_EXTENDED=y
 CONFIG_SERIAL_8250_SHARE_IRQ=y
 CONFIG_SERIAL_OF_PLATFORM=y
 CONFIG_SERIAL_ASPEED_VUART=y
+CONFIG_ASPEED_BT_IPMI_BMC=y
 # CONFIG_HW_RANDOM is not set
 CONFIG_I2C=y
 # CONFIG_I2C_COMPAT is not set
diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index 3ccc91db96e3..18f5a555a63d 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -102,6 +102,7 @@ CONFIG_SERIAL_8250_RUNTIME_UARTS=6
 CONFIG_SERIAL_8250_EXTENDED=y
 CONFIG_SERIAL_8250_SHARE_IRQ=y
 CONFIG_SERIAL_OF_PLATFORM=y
+CONFIG_ASPEED_BT_IPMI_BMC=y
 CONFIG_SERIAL_ASPEED_VUART=y
 # CONFIG_HW_RANDOM is not set
 CONFIG_I2C=y
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH linux dev-4.7 6/8] ARM: dts: aspeed: Enable BT IPMI BMC device
  2016-10-26  6:57 [PATCH linux dev-4.7 0/8] BT driver sync up Cédric Le Goater
                   ` (4 preceding siblings ...)
  2016-10-26  6:57 ` [PATCH linux dev-4.7 5/8] ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_BMC Cédric Le Goater
@ 2016-10-26  6:57 ` Cédric Le Goater
  2016-10-26  6:57 ` [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list Cédric Le Goater
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 31+ messages in thread
From: Cédric Le Goater @ 2016-10-26  6:57 UTC (permalink / raw)
  To: openbmc

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/arm/boot/dts/aspeed-g4.dtsi | 6 ++++++
 arch/arm/boot/dts/aspeed-g5.dtsi | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
index cfe5c95d863b..cb1607f70941 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -857,6 +857,12 @@
 				clocks = <&clk_apb>;
 			};
 
+			ibt: ibt@1e789140 {
+				compatible = "aspeed,ast2400-bt-bmc";
+				reg = <0x1e789140 0x18>;
+				interrupts = <8>;
+			};
+
 			wdt1: wdt@1e785000 {
 				compatible = "aspeed,ast2400-wdt";
 				reg = <0x1e785000 0x1c>;
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 163393a748ed..be95fd42dbd7 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -991,6 +991,12 @@
 				clocks = <&clk_apb>;
 			};
 
+			ibt: ibt@1e789140 {
+				compatible = "aspeed,ast2400-bt-bmc";
+				reg = <0x1e789140 0x18>;
+				interrupts = <8>;
+			};
+
 			wdt1: wdt@1e785000 {
 				compatible = "aspeed,ast2500-wdt";
 				reg = <0x1e785000 0x1c>;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list
  2016-10-26  6:57 [PATCH linux dev-4.7 0/8] BT driver sync up Cédric Le Goater
                   ` (5 preceding siblings ...)
  2016-10-26  6:57 ` [PATCH linux dev-4.7 6/8] ARM: dts: aspeed: Enable BT IPMI BMC device Cédric Le Goater
@ 2016-10-26  6:57 ` Cédric Le Goater
  2016-11-03  0:26   ` Cyril Bur
  2016-10-26  6:57 ` [PATCH linux dev-4.7 8/8] ipmi: add a sysfs file for configure maximum response time Cédric Le Goater
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2016-10-26  6:57 UTC (permalink / raw)
  To: openbmc

Regarding the response expiration handling, the IPMI spec says :

   The BMC must not return a given response once the corresponding
   Request-to-Response interval has passed. The BMC can ensure this
   by maintaining its own internal list of outstanding requests through
   the interface. The BMC could age and expire the entries in the list
   by expiring the entries at an interval that is somewhat shorter than
   the specified Request-to-Response interval....

To handle such case, we maintain list of received requests using the
seq number of the BT message to identify them. The list is updated
each time a request is received and a response is sent. The expiration
of the reponses is handled at each updates but also with a timer.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 drivers/char/ipmi/bt-bmc.c | 132 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)

diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
index b49e61320952..228ecdb689de 100644
--- a/drivers/char/ipmi/bt-bmc.c
+++ b/drivers/char/ipmi/bt-bmc.c
@@ -17,6 +17,7 @@
 #include <linux/platform_device.h>
 #include <linux/poll.h>
 #include <linux/sched.h>
+#include <linux/slab.h>
 #include <linux/timer.h>
 
 /*
@@ -57,6 +58,15 @@
 
 #define BT_BMC_BUFFER_SIZE 256
 
+#define BT_BMC_RESPONSE_TIME	5 /* seconds */
+
+struct ipmi_request {
+	struct list_head list;
+
+	u8 seq;
+	unsigned long expires;
+};
+
 struct bt_bmc {
 	struct device		dev;
 	struct miscdevice	miscdev;
@@ -65,6 +75,11 @@ struct bt_bmc {
 	wait_queue_head_t	queue;
 	struct timer_list	poll_timer;
 	struct mutex		mutex;
+
+	unsigned int		response_time;
+	struct timer_list	requests_timer;
+	spinlock_t              requests_lock;
+	struct list_head        requests;
 };
 
 static atomic_t open_count = ATOMIC_INIT(0);
@@ -163,6 +178,94 @@ static int bt_bmc_open(struct inode *inode, struct file *file)
 }
 
 /*
+ * lock should be held
+ */
+static void drop_expired_requests(struct bt_bmc *bt_bmc)
+{
+	unsigned long flags = 0;
+	struct ipmi_request *req, *next;
+	unsigned long next_expires = 0;
+	int start_timer = 0;
+
+	spin_lock_irqsave(&bt_bmc->requests_lock, flags);
+	list_for_each_entry_safe(req, next, &bt_bmc->requests, list) {
+		if (time_after_eq(jiffies, req->expires)) {
+			pr_warn("BT: request seq:%d has expired. dropping\n",
+				req->seq);
+			list_del(&req->list);
+			kfree(req);
+			continue;
+		}
+		if (!start_timer) {
+			start_timer = 1;
+			next_expires = req->expires;
+		} else {
+			next_expires = min(next_expires, req->expires);
+		}
+	}
+	spin_unlock_irqrestore(&bt_bmc->requests_lock, flags);
+
+	/* and possibly restart */
+	if (start_timer)
+		mod_timer(&bt_bmc->requests_timer, next_expires);
+}
+
+static void requests_timer(unsigned long data)
+{
+	struct bt_bmc *bt_bmc = (void *)data;
+
+	drop_expired_requests(bt_bmc);
+}
+
+static int bt_bmc_add_request(struct bt_bmc *bt_bmc, u8 seq)
+{
+	struct ipmi_request *req;
+
+	req = kmalloc(sizeof(*req), GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	req->seq = seq;
+	req->expires = jiffies +
+		msecs_to_jiffies(bt_bmc->response_time * 1000);
+
+	spin_lock(&bt_bmc->requests_lock);
+	list_add(&req->list, &bt_bmc->requests);
+	spin_unlock(&bt_bmc->requests_lock);
+
+	drop_expired_requests(bt_bmc);
+	return 0;
+}
+
+static int bt_bmc_remove_request(struct bt_bmc *bt_bmc, u8 seq)
+{
+	struct ipmi_request *req, *next;
+	int ret = -EBADRQC; /* Invalid request code */
+
+	spin_lock(&bt_bmc->requests_lock);
+	list_for_each_entry_safe(req, next, &bt_bmc->requests, list) {
+		/*
+		 * The sequence number should be unique, so remove the
+		 * first matching request found. If there are others,
+		 * they should expire
+		 */
+		if (req->seq == seq) {
+			list_del(&req->list);
+			kfree(req);
+			ret = 0;
+			break;
+		}
+	}
+	spin_unlock(&bt_bmc->requests_lock);
+
+	if (ret)
+		pr_warn("BT: request seq:%d is invalid\n", seq);
+
+	drop_expired_requests(bt_bmc);
+	return ret;
+}
+
+/*
  * The BT (Block Transfer) interface means that entire messages are
  * buffered by the host before a notification is sent to the BMC that
  * there is data to be read. The first byte is the length and the
@@ -231,6 +334,13 @@ static ssize_t bt_bmc_read(struct file *file, char __user *buf,
 		len_byte = 0;
 	}
 
+	if (ret > 0) {
+		int ret2 = bt_bmc_add_request(bt_bmc, kbuffer[2]);
+
+		if (ret2)
+			ret = ret2;
+	}
+
 	clr_b_busy(bt_bmc);
 
 out_unlock:
@@ -283,12 +393,20 @@ static ssize_t bt_bmc_write(struct file *file, const char __user *buf,
 	clr_wr_ptr(bt_bmc);
 
 	while (count) {
+		int ret2;
+
 		nwritten = min_t(ssize_t, count, sizeof(kbuffer));
 		if (copy_from_user(&kbuffer, buf, nwritten)) {
 			ret = -EFAULT;
 			break;
 		}
 
+		ret2 = bt_bmc_remove_request(bt_bmc, kbuffer[2]);
+		if (ret2) {
+			ret = ret2;
+			break;
+		}
+
 		bt_writen(bt_bmc, kbuffer, nwritten);
 
 		count -= nwritten;
@@ -438,6 +556,8 @@ static int bt_bmc_probe(struct platform_device *pdev)
 
 	mutex_init(&bt_bmc->mutex);
 	init_waitqueue_head(&bt_bmc->queue);
+	INIT_LIST_HEAD(&bt_bmc->requests);
+	spin_lock_init(&bt_bmc->requests_lock);
 
 	bt_bmc->miscdev.minor	= MISC_DYNAMIC_MINOR,
 		bt_bmc->miscdev.name	= DEVICE_NAME,
@@ -451,6 +571,8 @@ static int bt_bmc_probe(struct platform_device *pdev)
 
 	bt_bmc_config_irq(bt_bmc, pdev);
 
+	bt_bmc->response_time = BT_BMC_RESPONSE_TIME;
+
 	if (bt_bmc->irq) {
 		dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
 	} else {
@@ -468,6 +590,9 @@ static int bt_bmc_probe(struct platform_device *pdev)
 		  BT_CR0_ENABLE_IBT,
 		  bt_bmc->base + BT_CR0);
 
+	setup_timer(&bt_bmc->requests_timer, requests_timer,
+		    (unsigned long)bt_bmc);
+
 	clr_b_busy(bt_bmc);
 
 	return 0;
@@ -476,10 +601,17 @@ static int bt_bmc_probe(struct platform_device *pdev)
 static int bt_bmc_remove(struct platform_device *pdev)
 {
 	struct bt_bmc *bt_bmc = dev_get_drvdata(&pdev->dev);
+	struct ipmi_request *req, *next;
 
 	misc_deregister(&bt_bmc->miscdev);
 	if (!bt_bmc->irq)
 		del_timer_sync(&bt_bmc->poll_timer);
+
+	del_timer_sync(&bt_bmc->requests_timer);
+	list_for_each_entry_safe(req, next, &bt_bmc->requests, list) {
+		list_del(&req->list);
+		kfree(req);
+	}
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH linux dev-4.7 8/8] ipmi: add a sysfs file for configure maximum response time
  2016-10-26  6:57 [PATCH linux dev-4.7 0/8] BT driver sync up Cédric Le Goater
                   ` (6 preceding siblings ...)
  2016-10-26  6:57 ` [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list Cédric Le Goater
@ 2016-10-26  6:57 ` Cédric Le Goater
  2016-11-03  0:30 ` [PATCH linux dev-4.7 0/8] BT driver sync up Cyril Bur
  2016-11-10  9:13 ` Cédric Le Goater
  9 siblings, 0 replies; 31+ messages in thread
From: Cédric Le Goater @ 2016-10-26  6:57 UTC (permalink / raw)
  To: openbmc

We could also use an ioctl for that purpose. sysfs seems a better
approach.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 drivers/char/ipmi/bt-bmc.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
index 228ecdb689de..1ebb0171c724 100644
--- a/drivers/char/ipmi/bt-bmc.c
+++ b/drivers/char/ipmi/bt-bmc.c
@@ -84,6 +84,33 @@ struct bt_bmc {
 
 static atomic_t open_count = ATOMIC_INIT(0);
 
+static ssize_t bt_bmc_show_response_time(struct device *dev,
+					 struct device_attribute *attr,
+					 char *buf)
+{
+	struct bt_bmc *bt_bmc = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE - 1, "%d\n", bt_bmc->response_time);
+}
+
+static ssize_t bt_bmc_set_response_time(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t count)
+{
+	struct bt_bmc *bt_bmc = dev_get_drvdata(dev);
+	unsigned long val;
+	int err;
+
+	err = kstrtoul(buf, 0, &val);
+	if (err)
+		return err;
+	bt_bmc->response_time = val;
+	return count;
+}
+
+static DEVICE_ATTR(response_time, S_IWUSR | S_IRUGO,
+		bt_bmc_show_response_time, bt_bmc_set_response_time);
+
 static u8 bt_inb(struct bt_bmc *bt_bmc, int reg)
 {
 	return ioread8(bt_bmc->base + reg);
@@ -572,6 +599,10 @@ static int bt_bmc_probe(struct platform_device *pdev)
 	bt_bmc_config_irq(bt_bmc, pdev);
 
 	bt_bmc->response_time = BT_BMC_RESPONSE_TIME;
+	rc = device_create_file(&pdev->dev, &dev_attr_response_time);
+	if (rc)
+		dev_warn(&pdev->dev, "can't create response_time file\n");
+
 
 	if (bt_bmc->irq) {
 		dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 4/8] ipmi: add an Aspeed BT IPMI BMC driver
  2016-10-26  6:57 ` [PATCH linux dev-4.7 4/8] ipmi: add an Aspeed BT IPMI BMC driver Cédric Le Goater
@ 2016-11-02  0:00   ` Joel Stanley
  2016-11-02  7:08     ` Cédric Le Goater
  2016-11-03  0:56   ` Cyril Bur
  1 sibling, 1 reply; 31+ messages in thread
From: Joel Stanley @ 2016-11-02  0:00 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: OpenBMC Maillist, Jeremy Kerr

Hey Cedric,

On Wed, Oct 26, 2016 at 5:27 PM, Cédric Le Goater <clg@kaod.org> wrote:
> Backport from mainline of the main IPMI BMC driver patch plus fixes :
>
>  - commit d94655b405ba ("ipmi/bt-bmc: remove redundant return value
>    check of platform_get_resource()")
>  - commit a3e6061bad62 ("ipmi/bt-bmc: add a dependency on
>    ARCH_ASPEED")
>  - commit 1a377a79211a ("ipmi: Fix ioremap error handling in bt-bmc")
>  - commit 54f9c4d0778b ("ipmi: add an Aspeed BT IPMI BMC driver")

It has just occurred to me that this driver is written for the iBT
hardware in the Aspeed, and we also have the BT hardware.

If we were to write a driver for the BT hardware it would clash with
this one. In particular the compatible string and the character device
name.

I propose sending a patch to upstream before the release of 4.9 that
changes the chardev name to ipmi-ibt-host and changing the comaptible
string to ast2400-ibt-bmc.

What do you think?

Cheers,

Joel

>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  .../bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt   |  23 +
>  drivers/Makefile                                   |   2 +-
>  drivers/char/ipmi/Kconfig                          |   8 +
>  drivers/char/ipmi/Makefile                         |   1 +
>  drivers/char/ipmi/bt-bmc.c                         | 505 +++++++++++++++++++++
>  include/uapi/linux/Kbuild                          |   1 +
>  include/uapi/linux/bt-bmc.h                        |  18 +
>  7 files changed, 557 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
>  create mode 100644 drivers/char/ipmi/bt-bmc.c
>  create mode 100644 include/uapi/linux/bt-bmc.h
>
> diff --git a/Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt b/Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
> new file mode 100644
> index 000000000000..fbbacd958240
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
> @@ -0,0 +1,23 @@
> +* Aspeed BT (Block Transfer) IPMI interface
> +
> +The Aspeed SOCs (AST2400 and AST2500) are commonly used as BMCs
> +(BaseBoard Management Controllers) and the BT interface can be used to
> +perform in-band IPMI communication with their host.
> +
> +Required properties:
> +
> +- compatible : should be "aspeed,ast2400-bt-bmc"
> +- reg: physical address and size of the registers
> +
> +Optional properties:
> +
> +- interrupts: interrupt generated by the BT interface. without an
> +  interrupt, the driver will operate in poll mode.
> +
> +Example:
> +
> +       ibt@1e789140 {
> +               compatible = "aspeed,ast2400-bt-bmc";
> +               reg = <0x1e789140 0x18>;
> +               interrupts = <8>;
> +       };
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 0b6f3d60193d..0a7c60d3b6cc 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -21,7 +21,7 @@ obj-y                         += video/
>  obj-y                          += idle/
>
>  # IPMI must come before ACPI in order to provide IPMI opregion support
> -obj-$(CONFIG_IPMI_HANDLER)     += char/ipmi/
> +obj-y                          += char/ipmi/
>
>  obj-$(CONFIG_ACPI)             += acpi/
>  obj-$(CONFIG_SFI)              += sfi/
> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> index 6ed9e9fe5233..927fe355442a 100644
> --- a/drivers/char/ipmi/Kconfig
> +++ b/drivers/char/ipmi/Kconfig
> @@ -88,3 +88,11 @@ config IPMI_POWEROFF
>          the IPMI management controller is capable of this.
>
>  endif # IPMI_HANDLER
> +
> +config ASPEED_BT_IPMI_BMC
> +       depends on ARCH_ASPEED
> +       tristate "BT IPMI bmc driver"
> +       help
> +         Provides a driver for the BT (Block Transfer) IPMI interface
> +         found on Aspeed SOCs (AST2400 and AST2500). The driver
> +         implements the BMC side of the BT interface.
> diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
> index f3ffde1f5f1f..0d98cd91def1 100644
> --- a/drivers/char/ipmi/Makefile
> +++ b/drivers/char/ipmi/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_IPMI_SSIF) += ipmi_ssif.o
>  obj-$(CONFIG_IPMI_POWERNV) += ipmi_powernv.o
>  obj-$(CONFIG_IPMI_WATCHDOG) += ipmi_watchdog.o
>  obj-$(CONFIG_IPMI_POWEROFF) += ipmi_poweroff.o
> +obj-$(CONFIG_ASPEED_BT_IPMI_BMC) += bt-bmc.o
> diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
> new file mode 100644
> index 000000000000..b49e61320952
> --- /dev/null
> +++ b/drivers/char/ipmi/bt-bmc.c
> @@ -0,0 +1,505 @@
> +/*
> + * Copyright (c) 2015-2016, IBM Corporation.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/atomic.h>
> +#include <linux/bt-bmc.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/poll.h>
> +#include <linux/sched.h>
> +#include <linux/timer.h>
> +
> +/*
> + * This is a BMC device used to communicate to the host
> + */
> +#define DEVICE_NAME    "ipmi-bt-host"
> +
> +#define BT_IO_BASE     0xe4
> +#define BT_IRQ         10
> +
> +#define BT_CR0         0x0
> +#define   BT_CR0_IO_BASE               16
> +#define   BT_CR0_IRQ                   12
> +#define   BT_CR0_EN_CLR_SLV_RDP                0x8
> +#define   BT_CR0_EN_CLR_SLV_WRP                0x4
> +#define   BT_CR0_ENABLE_IBT            0x1
> +#define BT_CR1         0x4
> +#define   BT_CR1_IRQ_H2B       0x01
> +#define   BT_CR1_IRQ_HBUSY     0x40
> +#define BT_CR2         0x8
> +#define   BT_CR2_IRQ_H2B       0x01
> +#define   BT_CR2_IRQ_HBUSY     0x40
> +#define BT_CR3         0xc
> +#define BT_CTRL                0x10
> +#define   BT_CTRL_B_BUSY               0x80
> +#define   BT_CTRL_H_BUSY               0x40
> +#define   BT_CTRL_OEM0                 0x20
> +#define   BT_CTRL_SMS_ATN              0x10
> +#define   BT_CTRL_B2H_ATN              0x08
> +#define   BT_CTRL_H2B_ATN              0x04
> +#define   BT_CTRL_CLR_RD_PTR           0x02
> +#define   BT_CTRL_CLR_WR_PTR           0x01
> +#define BT_BMC2HOST    0x14
> +#define BT_INTMASK     0x18
> +#define   BT_INTMASK_B2H_IRQEN         0x01
> +#define   BT_INTMASK_B2H_IRQ           0x02
> +#define   BT_INTMASK_BMC_HWRST         0x80
> +
> +#define BT_BMC_BUFFER_SIZE 256
> +
> +struct bt_bmc {
> +       struct device           dev;
> +       struct miscdevice       miscdev;
> +       void __iomem            *base;
> +       int                     irq;
> +       wait_queue_head_t       queue;
> +       struct timer_list       poll_timer;
> +       struct mutex            mutex;
> +};
> +
> +static atomic_t open_count = ATOMIC_INIT(0);
> +
> +static u8 bt_inb(struct bt_bmc *bt_bmc, int reg)
> +{
> +       return ioread8(bt_bmc->base + reg);
> +}
> +
> +static void bt_outb(struct bt_bmc *bt_bmc, u8 data, int reg)
> +{
> +       iowrite8(data, bt_bmc->base + reg);
> +}
> +
> +static void clr_rd_ptr(struct bt_bmc *bt_bmc)
> +{
> +       bt_outb(bt_bmc, BT_CTRL_CLR_RD_PTR, BT_CTRL);
> +}
> +
> +static void clr_wr_ptr(struct bt_bmc *bt_bmc)
> +{
> +       bt_outb(bt_bmc, BT_CTRL_CLR_WR_PTR, BT_CTRL);
> +}
> +
> +static void clr_h2b_atn(struct bt_bmc *bt_bmc)
> +{
> +       bt_outb(bt_bmc, BT_CTRL_H2B_ATN, BT_CTRL);
> +}
> +
> +static void set_b_busy(struct bt_bmc *bt_bmc)
> +{
> +       if (!(bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_B_BUSY))
> +               bt_outb(bt_bmc, BT_CTRL_B_BUSY, BT_CTRL);
> +}
> +
> +static void clr_b_busy(struct bt_bmc *bt_bmc)
> +{
> +       if (bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_B_BUSY)
> +               bt_outb(bt_bmc, BT_CTRL_B_BUSY, BT_CTRL);
> +}
> +
> +static void set_b2h_atn(struct bt_bmc *bt_bmc)
> +{
> +       bt_outb(bt_bmc, BT_CTRL_B2H_ATN, BT_CTRL);
> +}
> +
> +static u8 bt_read(struct bt_bmc *bt_bmc)
> +{
> +       return bt_inb(bt_bmc, BT_BMC2HOST);
> +}
> +
> +static ssize_t bt_readn(struct bt_bmc *bt_bmc, u8 *buf, size_t n)
> +{
> +       int i;
> +
> +       for (i = 0; i < n; i++)
> +               buf[i] = bt_read(bt_bmc);
> +       return n;
> +}
> +
> +static void bt_write(struct bt_bmc *bt_bmc, u8 c)
> +{
> +       bt_outb(bt_bmc, c, BT_BMC2HOST);
> +}
> +
> +static ssize_t bt_writen(struct bt_bmc *bt_bmc, u8 *buf, size_t n)
> +{
> +       int i;
> +
> +       for (i = 0; i < n; i++)
> +               bt_write(bt_bmc, buf[i]);
> +       return n;
> +}
> +
> +static void set_sms_atn(struct bt_bmc *bt_bmc)
> +{
> +       bt_outb(bt_bmc, BT_CTRL_SMS_ATN, BT_CTRL);
> +}
> +
> +static struct bt_bmc *file_bt_bmc(struct file *file)
> +{
> +       return container_of(file->private_data, struct bt_bmc, miscdev);
> +}
> +
> +static int bt_bmc_open(struct inode *inode, struct file *file)
> +{
> +       struct bt_bmc *bt_bmc = file_bt_bmc(file);
> +
> +       if (atomic_inc_return(&open_count) == 1) {
> +               clr_b_busy(bt_bmc);
> +               return 0;
> +       }
> +
> +       atomic_dec(&open_count);
> +       return -EBUSY;
> +}
> +
> +/*
> + * The BT (Block Transfer) interface means that entire messages are
> + * buffered by the host before a notification is sent to the BMC that
> + * there is data to be read. The first byte is the length and the
> + * message data follows. The read operation just tries to capture the
> + * whole before returning it to userspace.
> + *
> + * BT Message format :
> + *
> + *    Byte 1  Byte 2     Byte 3  Byte 4  Byte 5:N
> + *    Length  NetFn/LUN  Seq     Cmd     Data
> + *
> + */
> +static ssize_t bt_bmc_read(struct file *file, char __user *buf,
> +                          size_t count, loff_t *ppos)
> +{
> +       struct bt_bmc *bt_bmc = file_bt_bmc(file);
> +       u8 len;
> +       int len_byte = 1;
> +       u8 kbuffer[BT_BMC_BUFFER_SIZE];
> +       ssize_t ret = 0;
> +       ssize_t nread;
> +
> +       if (!access_ok(VERIFY_WRITE, buf, count))
> +               return -EFAULT;
> +
> +       WARN_ON(*ppos);
> +
> +       if (wait_event_interruptible(bt_bmc->queue,
> +                                    bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_H2B_ATN))
> +               return -ERESTARTSYS;
> +
> +       mutex_lock(&bt_bmc->mutex);
> +
> +       if (unlikely(!(bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_H2B_ATN))) {
> +               ret = -EIO;
> +               goto out_unlock;
> +       }
> +
> +       set_b_busy(bt_bmc);
> +       clr_h2b_atn(bt_bmc);
> +       clr_rd_ptr(bt_bmc);
> +
> +       /*
> +        * The BT frames start with the message length, which does not
> +        * include the length byte.
> +        */
> +       kbuffer[0] = bt_read(bt_bmc);
> +       len = kbuffer[0];
> +
> +       /* We pass the length back to userspace as well */
> +       if (len + 1 > count)
> +               len = count - 1;
> +
> +       while (len) {
> +               nread = min_t(ssize_t, len, sizeof(kbuffer) - len_byte);
> +
> +               bt_readn(bt_bmc, kbuffer + len_byte, nread);
> +
> +               if (copy_to_user(buf, kbuffer, nread + len_byte)) {
> +                       ret = -EFAULT;
> +                       break;
> +               }
> +               len -= nread;
> +               buf += nread + len_byte;
> +               ret += nread + len_byte;
> +               len_byte = 0;
> +       }
> +
> +       clr_b_busy(bt_bmc);
> +
> +out_unlock:
> +       mutex_unlock(&bt_bmc->mutex);
> +       return ret;
> +}
> +
> +/*
> + * BT Message response format :
> + *
> + *    Byte 1  Byte 2     Byte 3  Byte 4  Byte 5  Byte 6:N
> + *    Length  NetFn/LUN  Seq     Cmd     Code    Data
> + */
> +static ssize_t bt_bmc_write(struct file *file, const char __user *buf,
> +                           size_t count, loff_t *ppos)
> +{
> +       struct bt_bmc *bt_bmc = file_bt_bmc(file);
> +       u8 kbuffer[BT_BMC_BUFFER_SIZE];
> +       ssize_t ret = 0;
> +       ssize_t nwritten;
> +
> +       /*
> +        * send a minimum response size
> +        */
> +       if (count < 5)
> +               return -EINVAL;
> +
> +       if (!access_ok(VERIFY_READ, buf, count))
> +               return -EFAULT;
> +
> +       WARN_ON(*ppos);
> +
> +       /*
> +        * There's no interrupt for clearing bmc busy so we have to
> +        * poll
> +        */
> +       if (wait_event_interruptible(bt_bmc->queue,
> +                                    !(bt_inb(bt_bmc, BT_CTRL) &
> +                                      (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN))))
> +               return -ERESTARTSYS;
> +
> +       mutex_lock(&bt_bmc->mutex);
> +
> +       if (unlikely(bt_inb(bt_bmc, BT_CTRL) &
> +                    (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN))) {
> +               ret = -EIO;
> +               goto out_unlock;
> +       }
> +
> +       clr_wr_ptr(bt_bmc);
> +
> +       while (count) {
> +               nwritten = min_t(ssize_t, count, sizeof(kbuffer));
> +               if (copy_from_user(&kbuffer, buf, nwritten)) {
> +                       ret = -EFAULT;
> +                       break;
> +               }
> +
> +               bt_writen(bt_bmc, kbuffer, nwritten);
> +
> +               count -= nwritten;
> +               buf += nwritten;
> +               ret += nwritten;
> +       }
> +
> +       set_b2h_atn(bt_bmc);
> +
> +out_unlock:
> +       mutex_unlock(&bt_bmc->mutex);
> +       return ret;
> +}
> +
> +static long bt_bmc_ioctl(struct file *file, unsigned int cmd,
> +                        unsigned long param)
> +{
> +       struct bt_bmc *bt_bmc = file_bt_bmc(file);
> +
> +       switch (cmd) {
> +       case BT_BMC_IOCTL_SMS_ATN:
> +               set_sms_atn(bt_bmc);
> +               return 0;
> +       }
> +       return -EINVAL;
> +}
> +
> +static int bt_bmc_release(struct inode *inode, struct file *file)
> +{
> +       struct bt_bmc *bt_bmc = file_bt_bmc(file);
> +
> +       atomic_dec(&open_count);
> +       set_b_busy(bt_bmc);
> +       return 0;
> +}
> +
> +static unsigned int bt_bmc_poll(struct file *file, poll_table *wait)
> +{
> +       struct bt_bmc *bt_bmc = file_bt_bmc(file);
> +       unsigned int mask = 0;
> +       u8 ctrl;
> +
> +       poll_wait(file, &bt_bmc->queue, wait);
> +
> +       ctrl = bt_inb(bt_bmc, BT_CTRL);
> +
> +       if (ctrl & BT_CTRL_H2B_ATN)
> +               mask |= POLLIN;
> +
> +       if (!(ctrl & (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN)))
> +               mask |= POLLOUT;
> +
> +       return mask;
> +}
> +
> +static const struct file_operations bt_bmc_fops = {
> +       .owner          = THIS_MODULE,
> +       .open           = bt_bmc_open,
> +       .read           = bt_bmc_read,
> +       .write          = bt_bmc_write,
> +       .release        = bt_bmc_release,
> +       .poll           = bt_bmc_poll,
> +       .unlocked_ioctl = bt_bmc_ioctl,
> +};
> +
> +static void poll_timer(unsigned long data)
> +{
> +       struct bt_bmc *bt_bmc = (void *)data;
> +
> +       bt_bmc->poll_timer.expires += msecs_to_jiffies(500);
> +       wake_up(&bt_bmc->queue);
> +       add_timer(&bt_bmc->poll_timer);
> +}
> +
> +static irqreturn_t bt_bmc_irq(int irq, void *arg)
> +{
> +       struct bt_bmc *bt_bmc = arg;
> +       u32 reg;
> +
> +       reg = ioread32(bt_bmc->base + BT_CR2);
> +       reg &= BT_CR2_IRQ_H2B | BT_CR2_IRQ_HBUSY;
> +       if (!reg)
> +               return IRQ_NONE;
> +
> +       /* ack pending IRQs */
> +       iowrite32(reg, bt_bmc->base + BT_CR2);
> +
> +       wake_up(&bt_bmc->queue);
> +       return IRQ_HANDLED;
> +}
> +
> +static int bt_bmc_config_irq(struct bt_bmc *bt_bmc,
> +                            struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       u32 reg;
> +       int rc;
> +
> +       bt_bmc->irq = platform_get_irq(pdev, 0);
> +       if (!bt_bmc->irq)
> +               return -ENODEV;
> +
> +       rc = devm_request_irq(dev, bt_bmc->irq, bt_bmc_irq, IRQF_SHARED,
> +                             DEVICE_NAME, bt_bmc);
> +       if (rc < 0) {
> +               dev_warn(dev, "Unable to request IRQ %d\n", bt_bmc->irq);
> +               bt_bmc->irq = 0;
> +               return rc;
> +       }
> +
> +       /*
> +        * Configure IRQs on the bmc clearing the H2B and HBUSY bits;
> +        * H2B will be asserted when the bmc has data for us; HBUSY
> +        * will be cleared (along with B2H) when we can write the next
> +        * message to the BT buffer
> +        */
> +       reg = ioread32(bt_bmc->base + BT_CR1);
> +       reg |= BT_CR1_IRQ_H2B | BT_CR1_IRQ_HBUSY;
> +       iowrite32(reg, bt_bmc->base + BT_CR1);
> +
> +       return 0;
> +}
> +
> +static int bt_bmc_probe(struct platform_device *pdev)
> +{
> +       struct bt_bmc *bt_bmc;
> +       struct device *dev;
> +       struct resource *res;
> +       int rc;
> +
> +       if (!pdev || !pdev->dev.of_node)
> +               return -ENODEV;
> +
> +       dev = &pdev->dev;
> +       dev_info(dev, "Found bt bmc device\n");
> +
> +       bt_bmc = devm_kzalloc(dev, sizeof(*bt_bmc), GFP_KERNEL);
> +       if (!bt_bmc)
> +               return -ENOMEM;
> +
> +       dev_set_drvdata(&pdev->dev, bt_bmc);
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       bt_bmc->base = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(bt_bmc->base))
> +               return PTR_ERR(bt_bmc->base);
> +
> +       mutex_init(&bt_bmc->mutex);
> +       init_waitqueue_head(&bt_bmc->queue);
> +
> +       bt_bmc->miscdev.minor   = MISC_DYNAMIC_MINOR,
> +               bt_bmc->miscdev.name    = DEVICE_NAME,
> +               bt_bmc->miscdev.fops    = &bt_bmc_fops,
> +               bt_bmc->miscdev.parent = dev;
> +       rc = misc_register(&bt_bmc->miscdev);
> +       if (rc) {
> +               dev_err(dev, "Unable to register misc device\n");
> +               return rc;
> +       }
> +
> +       bt_bmc_config_irq(bt_bmc, pdev);
> +
> +       if (bt_bmc->irq) {
> +               dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
> +       } else {
> +               dev_info(dev, "No IRQ; using timer\n");
> +               setup_timer(&bt_bmc->poll_timer, poll_timer,
> +                           (unsigned long)bt_bmc);
> +               bt_bmc->poll_timer.expires = jiffies + msecs_to_jiffies(10);
> +               add_timer(&bt_bmc->poll_timer);
> +       }
> +
> +       iowrite32((BT_IO_BASE << BT_CR0_IO_BASE) |
> +                 (BT_IRQ << BT_CR0_IRQ) |
> +                 BT_CR0_EN_CLR_SLV_RDP |
> +                 BT_CR0_EN_CLR_SLV_WRP |
> +                 BT_CR0_ENABLE_IBT,
> +                 bt_bmc->base + BT_CR0);
> +
> +       clr_b_busy(bt_bmc);
> +
> +       return 0;
> +}
> +
> +static int bt_bmc_remove(struct platform_device *pdev)
> +{
> +       struct bt_bmc *bt_bmc = dev_get_drvdata(&pdev->dev);
> +
> +       misc_deregister(&bt_bmc->miscdev);
> +       if (!bt_bmc->irq)
> +               del_timer_sync(&bt_bmc->poll_timer);
> +       return 0;
> +}
> +
> +static const struct of_device_id bt_bmc_match[] = {
> +       { .compatible = "aspeed,ast2400-bt-bmc" },
> +       { },
> +};
> +
> +static struct platform_driver bt_bmc_driver = {
> +       .driver = {
> +               .name           = DEVICE_NAME,
> +               .of_match_table = bt_bmc_match,
> +       },
> +       .probe = bt_bmc_probe,
> +       .remove = bt_bmc_remove,
> +};
> +
> +module_platform_driver(bt_bmc_driver);
> +
> +MODULE_DEVICE_TABLE(of, bt_bmc_match);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Alistair Popple <alistair@popple.id.au>");
> +MODULE_DESCRIPTION("Linux device interface to the BT interface");
> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> index ec10cfef166a..9493842de93b 100644
> --- a/include/uapi/linux/Kbuild
> +++ b/include/uapi/linux/Kbuild
> @@ -74,6 +74,7 @@ header-y += bpf_common.h
>  header-y += bpf.h
>  header-y += bpqether.h
>  header-y += bsg.h
> +header-y += bt-bmc.h
>  header-y += btrfs.h
>  header-y += can.h
>  header-y += capability.h
> diff --git a/include/uapi/linux/bt-bmc.h b/include/uapi/linux/bt-bmc.h
> new file mode 100644
> index 000000000000..d9ec766a63d0
> --- /dev/null
> +++ b/include/uapi/linux/bt-bmc.h
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright (c) 2015-2016, IBM Corporation.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#ifndef _UAPI_LINUX_BT_BMC_H
> +#define _UAPI_LINUX_BT_BMC_H
> +
> +#include <linux/ioctl.h>
> +
> +#define __BT_BMC_IOCTL_MAGIC   0xb1
> +#define BT_BMC_IOCTL_SMS_ATN   _IO(__BT_BMC_IOCTL_MAGIC, 0x00)
> +
> +#endif /* _UAPI_LINUX_BT_BMC_H */
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 4/8] ipmi: add an Aspeed BT IPMI BMC driver
  2016-11-02  0:00   ` Joel Stanley
@ 2016-11-02  7:08     ` Cédric Le Goater
  0 siblings, 0 replies; 31+ messages in thread
From: Cédric Le Goater @ 2016-11-02  7:08 UTC (permalink / raw)
  To: Joel Stanley; +Cc: OpenBMC Maillist, Jeremy Kerr

On 11/02/2016 01:00 AM, Joel Stanley wrote:
> Hey Cedric,
> 
> On Wed, Oct 26, 2016 at 5:27 PM, Cédric Le Goater <clg@kaod.org> wrote:
>> Backport from mainline of the main IPMI BMC driver patch plus fixes :
>>
>>  - commit d94655b405ba ("ipmi/bt-bmc: remove redundant return value
>>    check of platform_get_resource()")
>>  - commit a3e6061bad62 ("ipmi/bt-bmc: add a dependency on
>>    ARCH_ASPEED")
>>  - commit 1a377a79211a ("ipmi: Fix ioremap error handling in bt-bmc")
>>  - commit 54f9c4d0778b ("ipmi: add an Aspeed BT IPMI BMC driver")
> 
> It has just occurred to me that this driver is written for the iBT
> hardware in the Aspeed, and we also have the BT hardware.

This is true. one is IPMI compliant, the other follows H8S/2168.
 
> If we were to write a driver for the BT hardware it would clash with
> this one. In particular the compatible string and the character device
> name.
> 
> I propose sending a patch to upstream before the release of 4.9 that
> changes the chardev name to ipmi-ibt-host and changing the comaptible
> string to ast2400-ibt-bmc.
> 
> What do you think?

I can include the change in the patchset I will send for the expiry list 
and see how Corey reacts. 

C.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list
  2016-10-26  6:57 ` [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list Cédric Le Goater
@ 2016-11-03  0:26   ` Cyril Bur
  2016-11-03  9:06     ` Cédric Le Goater
  2016-11-03 18:23     ` Patrick Williams
  0 siblings, 2 replies; 31+ messages in thread
From: Cyril Bur @ 2016-11-03  0:26 UTC (permalink / raw)
  To: Cédric Le Goater, openbmc

On Wed, 2016-10-26 at 08:57 +0200, Cédric Le Goater wrote:
> Regarding the response expiration handling, the IPMI spec says :
> 
>    The BMC must not return a given response once the corresponding
>    Request-to-Response interval has passed. The BMC can ensure this
>    by maintaining its own internal list of outstanding requests
> through
>    the interface. The BMC could age and expire the entries in the
> list
>    by expiring the entries at an interval that is somewhat shorter
> than
>    the specified Request-to-Response interval....
> 
> To handle such case, we maintain list of received requests using the
> seq number of the BT message to identify them. The list is updated
> each time a request is received and a response is sent. The
> expiration
> of the reponses is handled at each updates but also with a timer.
> 

I agree that the BMC kernel is most logical place to do this, at the
moment btbridged does attempt something similar no?

Should we patch btbridged to not? Should I least remove duplicated
logic? 

> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  drivers/char/ipmi/bt-bmc.c | 132
> +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 132 insertions(+)
> 
> diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
> index b49e61320952..228ecdb689de 100644
> --- a/drivers/char/ipmi/bt-bmc.c
> +++ b/drivers/char/ipmi/bt-bmc.c
> @@ -17,6 +17,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/poll.h>
>  #include <linux/sched.h>
> +#include <linux/slab.h>
>  #include <linux/timer.h>
>  
>  /*
> @@ -57,6 +58,15 @@
>  
>  #define BT_BMC_BUFFER_SIZE 256
>  
> +#define BT_BMC_RESPONSE_TIME	5 /* seconds */
> +
> +struct ipmi_request {
> +	struct list_head list;
> +
> +	u8 seq;
> +	unsigned long expires;
> +};
> +
>  struct bt_bmc {
>  	struct device		dev;
>  	struct miscdevice	miscdev;
> @@ -65,6 +75,11 @@ struct bt_bmc {
>  	wait_queue_head_t	queue;
>  	struct timer_list	poll_timer;
>  	struct mutex		mutex;
> +
> +	unsigned int		response_time;
> +	struct timer_list	requests_timer;
> +	spinlock_t              requests_lock;
> +	struct list_head        requests;
>  };
>  
>  static atomic_t open_count = ATOMIC_INIT(0);
> @@ -163,6 +178,94 @@ static int bt_bmc_open(struct inode *inode,
> struct file *file)
>  }
>  
>  /*
> + * lock should be held
> + */
> +static void drop_expired_requests(struct bt_bmc *bt_bmc)
> +{
> +	unsigned long flags = 0;
> +	struct ipmi_request *req, *next;
> +	unsigned long next_expires = 0;
> +	int start_timer = 0;
> +
> +	spin_lock_irqsave(&bt_bmc->requests_lock, flags);
> +	list_for_each_entry_safe(req, next, &bt_bmc->requests, list)
> {
> +		if (time_after_eq(jiffies, req->expires)) {
> +			pr_warn("BT: request seq:%d has expired.
> dropping\n",
> +				req->seq);
> +			list_del(&req->list);
> +			kfree(req);
> +			continue;
> +		}
> +		if (!start_timer) {
> +			start_timer = 1;
> +			next_expires = req->expires;
> +		} else {
> +			next_expires = min(next_expires, req-
> >expires);
> +		}
> +	}
> +	spin_unlock_irqrestore(&bt_bmc->requests_lock, flags);
> +
> +	/* and possibly restart */
> +	if (start_timer)
> +		mod_timer(&bt_bmc->requests_timer, next_expires);
> +}
> +
> +static void requests_timer(unsigned long data)
> +{
> +	struct bt_bmc *bt_bmc = (void *)data;
> +
> +	drop_expired_requests(bt_bmc);
> +}
> +
> +static int bt_bmc_add_request(struct bt_bmc *bt_bmc, u8 seq)
> +{
> +	struct ipmi_request *req;
> +
> +	req = kmalloc(sizeof(*req), GFP_KERNEL);
> +	if (!req)
> +		return -ENOMEM;
> +
> +	req->seq = seq;
> +	req->expires = jiffies +
> +		msecs_to_jiffies(bt_bmc->response_time * 1000);
> +
> +	spin_lock(&bt_bmc->requests_lock);
> +	list_add(&req->list, &bt_bmc->requests);
> +	spin_unlock(&bt_bmc->requests_lock);
> +
> +	drop_expired_requests(bt_bmc);
> +	return 0;
> +}
> +
> +static int bt_bmc_remove_request(struct bt_bmc *bt_bmc, u8 seq)
> +{
> +	struct ipmi_request *req, *next;
> +	int ret = -EBADRQC; /* Invalid request code */
> +
> +	spin_lock(&bt_bmc->requests_lock);
> +	list_for_each_entry_safe(req, next, &bt_bmc->requests, list)
> {
> +		/*
> +		 * The sequence number should be unique, so remove
> the
> +		 * first matching request found. If there are
> others,
> +		 * they should expire
> +		 */
> +		if (req->seq == seq) {
> +			list_del(&req->list);
> +			kfree(req);
> +			ret = 0;
> +			break;
> +		}
> +	}
> +	spin_unlock(&bt_bmc->requests_lock);
> +
> +	if (ret)
> +		pr_warn("BT: request seq:%d is invalid\n", seq);
> +
> +	drop_expired_requests(bt_bmc);
> +	return ret;
> +}
> +
> +/*
>   * The BT (Block Transfer) interface means that entire messages are
>   * buffered by the host before a notification is sent to the BMC
> that
>   * there is data to be read. The first byte is the length and the
> @@ -231,6 +334,13 @@ static ssize_t bt_bmc_read(struct file *file,
> char __user *buf,
>  		len_byte = 0;
>  	}
>  
> +	if (ret > 0) {
> +		int ret2 = bt_bmc_add_request(bt_bmc, kbuffer[2]);
> +
> +		if (ret2)
> +			ret = ret2;
> +	}
> +
>  	clr_b_busy(bt_bmc);
>  
>  out_unlock:
> @@ -283,12 +393,20 @@ static ssize_t bt_bmc_write(struct file *file,
> const char __user *buf,
>  	clr_wr_ptr(bt_bmc);
>  
>  	while (count) {
> +		int ret2;
> +
>  		nwritten = min_t(ssize_t, count, sizeof(kbuffer));
>  		if (copy_from_user(&kbuffer, buf, nwritten)) {
>  			ret = -EFAULT;
>  			break;
>  		}
>  
> +		ret2 = bt_bmc_remove_request(bt_bmc, kbuffer[2]);
> +		if (ret2) {
> +			ret = ret2;
> +			break;
> +		}
> +
>  		bt_writen(bt_bmc, kbuffer, nwritten);
>  
>  		count -= nwritten;
> @@ -438,6 +556,8 @@ static int bt_bmc_probe(struct platform_device
> *pdev)
>  
>  	mutex_init(&bt_bmc->mutex);
>  	init_waitqueue_head(&bt_bmc->queue);
> +	INIT_LIST_HEAD(&bt_bmc->requests);
> +	spin_lock_init(&bt_bmc->requests_lock);
>  
>  	bt_bmc->miscdev.minor	= MISC_DYNAMIC_MINOR,
>  		bt_bmc->miscdev.name	= DEVICE_NAME,
> @@ -451,6 +571,8 @@ static int bt_bmc_probe(struct platform_device
> *pdev)
>  
>  	bt_bmc_config_irq(bt_bmc, pdev);
>  
> +	bt_bmc->response_time = BT_BMC_RESPONSE_TIME;
> +
>  	if (bt_bmc->irq) {
>  		dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
>  	} else {
> @@ -468,6 +590,9 @@ static int bt_bmc_probe(struct platform_device
> *pdev)
>  		  BT_CR0_ENABLE_IBT,
>  		  bt_bmc->base + BT_CR0);
>  
> +	setup_timer(&bt_bmc->requests_timer, requests_timer,
> +		    (unsigned long)bt_bmc);
> +
>  	clr_b_busy(bt_bmc);
>  
>  	return 0;
> @@ -476,10 +601,17 @@ static int bt_bmc_probe(struct platform_device
> *pdev)
>  static int bt_bmc_remove(struct platform_device *pdev)
>  {
>  	struct bt_bmc *bt_bmc = dev_get_drvdata(&pdev->dev);
> +	struct ipmi_request *req, *next;
>  
>  	misc_deregister(&bt_bmc->miscdev);
>  	if (!bt_bmc->irq)
>  		del_timer_sync(&bt_bmc->poll_timer);
> +
> +	del_timer_sync(&bt_bmc->requests_timer);
> +	list_for_each_entry_safe(req, next, &bt_bmc->requests, list)
> {
> +		list_del(&req->list);
> +		kfree(req);
> +	}
>  	return 0;
>  }
>  

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 0/8] BT driver sync up
  2016-10-26  6:57 [PATCH linux dev-4.7 0/8] BT driver sync up Cédric Le Goater
                   ` (7 preceding siblings ...)
  2016-10-26  6:57 ` [PATCH linux dev-4.7 8/8] ipmi: add a sysfs file for configure maximum response time Cédric Le Goater
@ 2016-11-03  0:30 ` Cyril Bur
  2016-11-10  9:13 ` Cédric Le Goater
  9 siblings, 0 replies; 31+ messages in thread
From: Cyril Bur @ 2016-11-03  0:30 UTC (permalink / raw)
  To: Cédric Le Goater, openbmc

On Wed, 2016-10-26 at 08:57 +0200, Cédric Le Goater wrote:
> Hello,
> 
> Here is a serie syncing the OpenBMC kernel with the BT driver merged
> in mainline linux. This kernel needs an updated version of btbridged
> :
> 
>      https://github.com/legoater/btbridge/commits/master
> 
> The last two patches add a request expiry list and some configuration
> through sysfs. These are for review only as they need to be sent to
> mainline first.
> 

Tested this series with patched btbridge on palmetto, host comes up
with no problems.

> Thanks,
> 
> C.
> 
> Cédric Le Goater (8):
>   Revert "misc: Add Aspeed BT IPMI host driver"
>   ARM: aspeed: remove previous definitions in default config
>   ARM: dts: aspeed: remove previous iBT definitions
>   ipmi: add an Aspeed BT IPMI BMC driver
>   ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_BMC
>   ARM: dts: aspeed: Enable BT IPMI BMC device
>   ipmi: maintain a request expiry list
>   ipmi: add a sysfs file for configure maximum response time
> 
>  .../bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt   |  23 +
>  arch/arm/boot/dts/aspeed-g4.dtsi                   |   2 +-
>  arch/arm/boot/dts/aspeed-g5.dtsi                   |   2 +-
>  arch/arm/configs/aspeed_g4_defconfig               |   2 +-
>  arch/arm/configs/aspeed_g5_defconfig               |   2 +-
>  drivers/Makefile                                   |   2 +-
>  drivers/char/ipmi/Kconfig                          |   8 +
>  drivers/char/ipmi/Makefile                         |   1 +
>  drivers/char/ipmi/bt-bmc.c                         | 668
> +++++++++++++++++++++
>  drivers/misc/Kconfig                               |   5 -
>  drivers/misc/Makefile                              |   1 -
>  drivers/misc/bt-host.c                             | 427 ---------
> ----
>  include/uapi/linux/Kbuild                          |   2 +-
>  include/uapi/linux/{bt-host.h => bt-bmc.h}         |  12 +-
>  14 files changed, 712 insertions(+), 445 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
>  create mode 100644 drivers/char/ipmi/bt-bmc.c
>  delete mode 100644 drivers/misc/bt-host.c
>  rename include/uapi/linux/{bt-host.h => bt-bmc.h} (55%)
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 4/8] ipmi: add an Aspeed BT IPMI BMC driver
  2016-10-26  6:57 ` [PATCH linux dev-4.7 4/8] ipmi: add an Aspeed BT IPMI BMC driver Cédric Le Goater
  2016-11-02  0:00   ` Joel Stanley
@ 2016-11-03  0:56   ` Cyril Bur
  2016-11-03  2:46     ` Joel Stanley
  1 sibling, 1 reply; 31+ messages in thread
From: Cyril Bur @ 2016-11-03  0:56 UTC (permalink / raw)
  To: Cédric Le Goater, openbmc

On Wed, 2016-10-26 at 08:57 +0200, Cédric Le Goater wrote:
> Backport from mainline of the main IPMI BMC driver patch plus fixes :
> 
>  - commit d94655b405ba ("ipmi/bt-bmc: remove redundant return value
>    check of platform_get_resource()")
>  - commit a3e6061bad62 ("ipmi/bt-bmc: add a dependency on
>    ARCH_ASPEED")
>  - commit 1a377a79211a ("ipmi: Fix ioremap error handling in bt-bmc")
>  - commit 54f9c4d0778b ("ipmi: add an Aspeed BT IPMI BMC driver")
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---

[snip]

> +
> +static int bt_bmc_probe(struct platform_device *pdev)
> +{
> +	struct bt_bmc *bt_bmc;
> +	struct device *dev;
> +	struct resource *res;
> +	int rc;
> +
> +	if (!pdev || !pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	dev = &pdev->dev;
> +	dev_info(dev, "Found bt bmc device\n");
> +
> +	bt_bmc = devm_kzalloc(dev, sizeof(*bt_bmc), GFP_KERNEL);
> +	if (!bt_bmc)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(&pdev->dev, bt_bmc);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	bt_bmc->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(bt_bmc->base))
> +		return PTR_ERR(bt_bmc->base);
> +
> +	mutex_init(&bt_bmc->mutex);
> +	init_waitqueue_head(&bt_bmc->queue);
> +
> +	bt_bmc->miscdev.minor	= MISC_DYNAMIC_MINOR,
> +		bt_bmc->miscdev.name	= DEVICE_NAME,
> +		bt_bmc->miscdev.fops	= &bt_bmc_fops,
> +		bt_bmc->miscdev.parent = dev;
> +	rc = misc_register(&bt_bmc->miscdev);
> +	if (rc) {
> +		dev_err(dev, "Unable to register misc device\n");
> +		return rc;
> +	}
> +
> +	bt_bmc_config_irq(bt_bmc, pdev);
> +
> +	if (bt_bmc->irq) {
> +		dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
> +	} else {
> +		dev_info(dev, "No IRQ; using timer\n");
> +		setup_timer(&bt_bmc->poll_timer, poll_timer,
> +			    (unsigned long)bt_bmc);
> +		bt_bmc->poll_timer.expires = jiffies +
> msecs_to_jiffies(10);
> +		add_timer(&bt_bmc->poll_timer);
> +	}
> +
> +	iowrite32((BT_IO_BASE << BT_CR0_IO_BASE) |
> +		  (BT_IRQ << BT_CR0_IRQ) |
> +		  BT_CR0_EN_CLR_SLV_RDP |
> +		  BT_CR0_EN_CLR_SLV_WRP |
> +		  BT_CR0_ENABLE_IBT,
> +		  bt_bmc->base + BT_CR0);
> +
> +	clr_b_busy(bt_bmc);
> +
> +	return 0;
> +}
> +
> +static int bt_bmc_remove(struct platform_device *pdev)
> +{
> +	struct bt_bmc *bt_bmc = dev_get_drvdata(&pdev->dev);
> +
> +	misc_deregister(&bt_bmc->miscdev);
> +	if (!bt_bmc->irq)
> +		del_timer_sync(&bt_bmc->poll_timer);

The old bt-host driver had:
       devm_iounmap(&pdev->dev, bt_host->base);
       devm_kfree(&pdev->dev, bt_host);

Is there kernel magic that means they aren't needed?

> +	return 0;
> +}
> +
> +static const struct of_device_id bt_bmc_match[] = {
> +	{ .compatible = "aspeed,ast2400-bt-bmc" },
> +	{ },
> +};
> +
> +static struct platform_driver bt_bmc_driver = {
> +	.driver = {
> +		.name		= DEVICE_NAME,
> +		.of_match_table = bt_bmc_match,
> +	},
> +	.probe = bt_bmc_probe,
> +	.remove = bt_bmc_remove,
> +};
> +
> +module_platform_driver(bt_bmc_driver);
> +
> +MODULE_DEVICE_TABLE(of, bt_bmc_match);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Alistair Popple <alistair@popple.id.au>");
> +MODULE_DESCRIPTION("Linux device interface to the BT interface");
> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> index ec10cfef166a..9493842de93b 100644
> --- a/include/uapi/linux/Kbuild
> +++ b/include/uapi/linux/Kbuild
> @@ -74,6 +74,7 @@ header-y += bpf_common.h
>  header-y += bpf.h
>  header-y += bpqether.h
>  header-y += bsg.h
> +header-y += bt-bmc.h
>  header-y += btrfs.h
>  header-y += can.h
>  header-y += capability.h
> diff --git a/include/uapi/linux/bt-bmc.h b/include/uapi/linux/bt-
> bmc.h
> new file mode 100644
> index 000000000000..d9ec766a63d0
> --- /dev/null
> +++ b/include/uapi/linux/bt-bmc.h
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright (c) 2015-2016, IBM Corporation.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#ifndef _UAPI_LINUX_BT_BMC_H
> +#define _UAPI_LINUX_BT_BMC_H
> +
> +#include <linux/ioctl.h>
> +
> +#define __BT_BMC_IOCTL_MAGIC	0xb1
> +#define BT_BMC_IOCTL_SMS_ATN	_IO(__BT_BMC_IOCTL_MAGIC, 0x00)
> +
> +#endif /* _UAPI_LINUX_BT_BMC_H */

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 4/8] ipmi: add an Aspeed BT IPMI BMC driver
  2016-11-03  0:56   ` Cyril Bur
@ 2016-11-03  2:46     ` Joel Stanley
  2016-11-03  2:48       ` Cyril Bur
  0 siblings, 1 reply; 31+ messages in thread
From: Joel Stanley @ 2016-11-03  2:46 UTC (permalink / raw)
  To: Cyril Bur; +Cc: Cédric Le Goater, OpenBMC Maillist

On Thu, Nov 3, 2016 at 11:26 AM, Cyril Bur <cyrilbur@gmail.com> wrote:
>
>> +static int bt_bmc_remove(struct platform_device *pdev)
>> +{
>> +     struct bt_bmc *bt_bmc = dev_get_drvdata(&pdev->dev);
>> +
>> +     misc_deregister(&bt_bmc->miscdev);
>> +     if (!bt_bmc->irq)
>> +             del_timer_sync(&bt_bmc->poll_timer);
>
> The old bt-host driver had:
>        devm_iounmap(&pdev->dev, bt_host->base);
>        devm_kfree(&pdev->dev, bt_host);
>
> Is there kernel magic that means they aren't needed?

Yep. The devm_ api's in the kernel are "managed" (hence the m) by the
driver subsystem. When the driver detaches, all of the resources
allocated with devm_ calls are automatically freed in the order they
were allocated.

 https://www.kernel.org/doc/Documentation/driver-model/devres.txt
 http://haifux.org/lectures/323/haifux-devres.pdf

Cheers,

Joel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 4/8] ipmi: add an Aspeed BT IPMI BMC driver
  2016-11-03  2:46     ` Joel Stanley
@ 2016-11-03  2:48       ` Cyril Bur
  0 siblings, 0 replies; 31+ messages in thread
From: Cyril Bur @ 2016-11-03  2:48 UTC (permalink / raw)
  To: Joel Stanley; +Cc: Cédric Le Goater, OpenBMC Maillist

On Thu, 2016-11-03 at 13:16 +1030, Joel Stanley wrote:
> On Thu, Nov 3, 2016 at 11:26 AM, Cyril Bur <cyrilbur@gmail.com>
> wrote:
> > 
> > > +static int bt_bmc_remove(struct platform_device *pdev)
> > > +{
> > > +     struct bt_bmc *bt_bmc = dev_get_drvdata(&pdev->dev);
> > > +
> > > +     misc_deregister(&bt_bmc->miscdev);
> > > +     if (!bt_bmc->irq)
> > > +             del_timer_sync(&bt_bmc->poll_timer);
> > 
> > The old bt-host driver had:
> >        devm_iounmap(&pdev->dev, bt_host->base);
> >        devm_kfree(&pdev->dev, bt_host);
> > 
> > Is there kernel magic that means they aren't needed?
> 
> Yep. The devm_ api's in the kernel are "managed" (hence the m) by the
> driver subsystem. When the driver detaches, all of the resources
> allocated with devm_ calls are automatically freed in the order they
> were allocated.
> 
>  https://www.kernel.org/doc/Documentation/driver-model/devres.txt
>  http://haifux.org/lectures/323/haifux-devres.pdf
> 

*magic*

Thanks!

> Cheers,
> 
> Joel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list
  2016-11-03  0:26   ` Cyril Bur
@ 2016-11-03  9:06     ` Cédric Le Goater
  2016-11-10  7:53       ` Cédric Le Goater
  2016-11-03 18:23     ` Patrick Williams
  1 sibling, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2016-11-03  9:06 UTC (permalink / raw)
  To: Cyril Bur, openbmc, Joel Stanley

On 11/03/2016 01:26 AM, Cyril Bur wrote:
> On Wed, 2016-10-26 at 08:57 +0200, Cédric Le Goater wrote:
>> Regarding the response expiration handling, the IPMI spec says :
>>
>>    The BMC must not return a given response once the corresponding
>>    Request-to-Response interval has passed. The BMC can ensure this
>>    by maintaining its own internal list of outstanding requests
>> through
>>    the interface. The BMC could age and expire the entries in the
>> list
>>    by expiring the entries at an interval that is somewhat shorter
>> than
>>    the specified Request-to-Response interval....
>>
>> To handle such case, we maintain list of received requests using the
>> seq number of the BT message to identify them. The list is updated
>> each time a request is received and a response is sent. The
>> expiration
>> of the reponses is handled at each updates but also with a timer.
>>
> 
> I agree that the BMC kernel is most logical place to do this, at the
> moment btbridged does attempt something similar no?
>
> 
> Should we patch btbridged to not? Should I least remove duplicated
> logic? 

yes probably. Let's see what mainline says about it first. 
The patch has a minor flaw: struct ipmi_request should include 
the NetFn and Command values. Corey might ask for a resend.


More globally speaking, the patch raises the question on how we 
manage the BT interface capabilities, the in/out buffer message 
sizes, allowed retries, etc. For now, this is a bit foggy, there
are bits in the kernel, in btbridged and in phosphor-host-ipmid.

I think we should configure the driver BT capabilities when 
btbridged is started. I have sent a patch using sysfs but maybe 
we will switch to an ioctl as Joel proposed. if btbridged knows 
about the driver configuration, it should probably handle the 
"Get BT Interface Capabilities" and not phosphor-host-ipmid.


May be I should open an issue on that topic ? 

C.


>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  drivers/char/ipmi/bt-bmc.c | 132
>> +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 132 insertions(+)
>>
>> diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
>> index b49e61320952..228ecdb689de 100644
>> --- a/drivers/char/ipmi/bt-bmc.c
>> +++ b/drivers/char/ipmi/bt-bmc.c
>> @@ -17,6 +17,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/poll.h>
>>  #include <linux/sched.h>
>> +#include <linux/slab.h>
>>  #include <linux/timer.h>
>>  
>>  /*
>> @@ -57,6 +58,15 @@
>>  
>>  #define BT_BMC_BUFFER_SIZE 256
>>  
>> +#define BT_BMC_RESPONSE_TIME	5 /* seconds */
>> +
>> +struct ipmi_request {
>> +	struct list_head list;
>> +
>> +	u8 seq;
>> +	unsigned long expires;
>> +};
>> +
>>  struct bt_bmc {
>>  	struct device		dev;
>>  	struct miscdevice	miscdev;
>> @@ -65,6 +75,11 @@ struct bt_bmc {
>>  	wait_queue_head_t	queue;
>>  	struct timer_list	poll_timer;
>>  	struct mutex		mutex;
>> +
>> +	unsigned int		response_time;
>> +	struct timer_list	requests_timer;
>> +	spinlock_t              requests_lock;
>> +	struct list_head        requests;
>>  };
>>  
>>  static atomic_t open_count = ATOMIC_INIT(0);
>> @@ -163,6 +178,94 @@ static int bt_bmc_open(struct inode *inode,
>> struct file *file)
>>  }
>>  
>>  /*
>> + * lock should be held
>> + */
>> +static void drop_expired_requests(struct bt_bmc *bt_bmc)
>> +{
>> +	unsigned long flags = 0;
>> +	struct ipmi_request *req, *next;
>> +	unsigned long next_expires = 0;
>> +	int start_timer = 0;
>> +
>> +	spin_lock_irqsave(&bt_bmc->requests_lock, flags);
>> +	list_for_each_entry_safe(req, next, &bt_bmc->requests, list)
>> {
>> +		if (time_after_eq(jiffies, req->expires)) {
>> +			pr_warn("BT: request seq:%d has expired.
>> dropping\n",
>> +				req->seq);
>> +			list_del(&req->list);
>> +			kfree(req);
>> +			continue;
>> +		}
>> +		if (!start_timer) {
>> +			start_timer = 1;
>> +			next_expires = req->expires;
>> +		} else {
>> +			next_expires = min(next_expires, req-
>>> expires);
>> +		}
>> +	}
>> +	spin_unlock_irqrestore(&bt_bmc->requests_lock, flags);
>> +
>> +	/* and possibly restart */
>> +	if (start_timer)
>> +		mod_timer(&bt_bmc->requests_timer, next_expires);
>> +}
>> +
>> +static void requests_timer(unsigned long data)
>> +{
>> +	struct bt_bmc *bt_bmc = (void *)data;
>> +
>> +	drop_expired_requests(bt_bmc);
>> +}
>> +
>> +static int bt_bmc_add_request(struct bt_bmc *bt_bmc, u8 seq)
>> +{
>> +	struct ipmi_request *req;
>> +
>> +	req = kmalloc(sizeof(*req), GFP_KERNEL);
>> +	if (!req)
>> +		return -ENOMEM;
>> +
>> +	req->seq = seq;
>> +	req->expires = jiffies +
>> +		msecs_to_jiffies(bt_bmc->response_time * 1000);
>> +
>> +	spin_lock(&bt_bmc->requests_lock);
>> +	list_add(&req->list, &bt_bmc->requests);
>> +	spin_unlock(&bt_bmc->requests_lock);
>> +
>> +	drop_expired_requests(bt_bmc);
>> +	return 0;
>> +}
>> +
>> +static int bt_bmc_remove_request(struct bt_bmc *bt_bmc, u8 seq)
>> +{
>> +	struct ipmi_request *req, *next;
>> +	int ret = -EBADRQC; /* Invalid request code */
>> +
>> +	spin_lock(&bt_bmc->requests_lock);
>> +	list_for_each_entry_safe(req, next, &bt_bmc->requests, list)
>> {
>> +		/*
>> +		 * The sequence number should be unique, so remove
>> the
>> +		 * first matching request found. If there are
>> others,
>> +		 * they should expire
>> +		 */
>> +		if (req->seq == seq) {
>> +			list_del(&req->list);
>> +			kfree(req);
>> +			ret = 0;
>> +			break;
>> +		}
>> +	}
>> +	spin_unlock(&bt_bmc->requests_lock);
>> +
>> +	if (ret)
>> +		pr_warn("BT: request seq:%d is invalid\n", seq);
>> +
>> +	drop_expired_requests(bt_bmc);
>> +	return ret;
>> +}
>> +
>> +/*
>>   * The BT (Block Transfer) interface means that entire messages are
>>   * buffered by the host before a notification is sent to the BMC
>> that
>>   * there is data to be read. The first byte is the length and the
>> @@ -231,6 +334,13 @@ static ssize_t bt_bmc_read(struct file *file,
>> char __user *buf,
>>  		len_byte = 0;
>>  	}
>>  
>> +	if (ret > 0) {
>> +		int ret2 = bt_bmc_add_request(bt_bmc, kbuffer[2]);
>> +
>> +		if (ret2)
>> +			ret = ret2;
>> +	}
>> +
>>  	clr_b_busy(bt_bmc);
>>  
>>  out_unlock:
>> @@ -283,12 +393,20 @@ static ssize_t bt_bmc_write(struct file *file,
>> const char __user *buf,
>>  	clr_wr_ptr(bt_bmc);
>>  
>>  	while (count) {
>> +		int ret2;
>> +
>>  		nwritten = min_t(ssize_t, count, sizeof(kbuffer));
>>  		if (copy_from_user(&kbuffer, buf, nwritten)) {
>>  			ret = -EFAULT;
>>  			break;
>>  		}
>>  
>> +		ret2 = bt_bmc_remove_request(bt_bmc, kbuffer[2]);
>> +		if (ret2) {
>> +			ret = ret2;
>> +			break;
>> +		}
>> +
>>  		bt_writen(bt_bmc, kbuffer, nwritten);
>>  
>>  		count -= nwritten;
>> @@ -438,6 +556,8 @@ static int bt_bmc_probe(struct platform_device
>> *pdev)
>>  
>>  	mutex_init(&bt_bmc->mutex);
>>  	init_waitqueue_head(&bt_bmc->queue);
>> +	INIT_LIST_HEAD(&bt_bmc->requests);
>> +	spin_lock_init(&bt_bmc->requests_lock);
>>  
>>  	bt_bmc->miscdev.minor	= MISC_DYNAMIC_MINOR,
>>  		bt_bmc->miscdev.name	= DEVICE_NAME,
>> @@ -451,6 +571,8 @@ static int bt_bmc_probe(struct platform_device
>> *pdev)
>>  
>>  	bt_bmc_config_irq(bt_bmc, pdev);
>>  
>> +	bt_bmc->response_time = BT_BMC_RESPONSE_TIME;
>> +
>>  	if (bt_bmc->irq) {
>>  		dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
>>  	} else {
>> @@ -468,6 +590,9 @@ static int bt_bmc_probe(struct platform_device
>> *pdev)
>>  		  BT_CR0_ENABLE_IBT,
>>  		  bt_bmc->base + BT_CR0);
>>  
>> +	setup_timer(&bt_bmc->requests_timer, requests_timer,
>> +		    (unsigned long)bt_bmc);
>> +
>>  	clr_b_busy(bt_bmc);
>>  
>>  	return 0;
>> @@ -476,10 +601,17 @@ static int bt_bmc_probe(struct platform_device
>> *pdev)
>>  static int bt_bmc_remove(struct platform_device *pdev)
>>  {
>>  	struct bt_bmc *bt_bmc = dev_get_drvdata(&pdev->dev);
>> +	struct ipmi_request *req, *next;
>>  
>>  	misc_deregister(&bt_bmc->miscdev);
>>  	if (!bt_bmc->irq)
>>  		del_timer_sync(&bt_bmc->poll_timer);
>> +
>> +	del_timer_sync(&bt_bmc->requests_timer);
>> +	list_for_each_entry_safe(req, next, &bt_bmc->requests, list)
>> {
>> +		list_del(&req->list);
>> +		kfree(req);
>> +	}
>>  	return 0;
>>  }
>>  

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list
  2016-11-03  0:26   ` Cyril Bur
  2016-11-03  9:06     ` Cédric Le Goater
@ 2016-11-03 18:23     ` Patrick Williams
  2016-11-03 18:46       ` Cédric Le Goater
  1 sibling, 1 reply; 31+ messages in thread
From: Patrick Williams @ 2016-11-03 18:23 UTC (permalink / raw)
  To: Cyril Bur, Brendan Higgins; +Cc: Cédric Le Goater, openbmc

[-- Attachment #1: Type: text/plain, Size: 1415 bytes --]

On Thu, Nov 03, 2016 at 11:26:09AM +1100, Cyril Bur wrote:
> On Wed, 2016-10-26 at 08:57 +0200, Cédric Le Goater wrote:
> > Regarding the response expiration handling, the IPMI spec says :
> > 
> >    The BMC must not return a given response once the corresponding
> >    Request-to-Response interval has passed. The BMC can ensure this
> >    by maintaining its own internal list of outstanding requests
> > through
> >    the interface. The BMC could age and expire the entries in the
> > list
> >    by expiring the entries at an interval that is somewhat shorter
> > than
> >    the specified Request-to-Response interval....
> > 
> > To handle such case, we maintain list of received requests using the
> > seq number of the BT message to identify them. The list is updated
> > each time a request is received and a response is sent. The
> > expiration
> > of the reponses is handled at each updates but also with a timer.
> > 
> 
> I agree that the BMC kernel is most logical place to do this, at the
> moment btbridged does attempt something similar no?
> 
> Should we patch btbridged to not? Should I least remove duplicated
> logic? 
> 

Brendan,

Are you paying attention to this discussion?  I think you had some
opposition to the kernel doing any additional work in this space because
you wanted to run a non-IPMI protocol over the IPMI bridge.

-- 
Patrick Williams

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list
  2016-11-03 18:23     ` Patrick Williams
@ 2016-11-03 18:46       ` Cédric Le Goater
  2016-11-04  1:55         ` Patrick Williams
  0 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2016-11-03 18:46 UTC (permalink / raw)
  To: Patrick Williams, Cyril Bur, Brendan Higgins; +Cc: openbmc

On 11/03/2016 07:23 PM, Patrick Williams wrote:
> On Thu, Nov 03, 2016 at 11:26:09AM +1100, Cyril Bur wrote:
>> On Wed, 2016-10-26 at 08:57 +0200, Cédric Le Goater wrote:
>>> Regarding the response expiration handling, the IPMI spec says :
>>>
>>>    The BMC must not return a given response once the corresponding
>>>    Request-to-Response interval has passed. The BMC can ensure this
>>>    by maintaining its own internal list of outstanding requests
>>> through
>>>    the interface. The BMC could age and expire the entries in the
>>> list
>>>    by expiring the entries at an interval that is somewhat shorter
>>> than
>>>    the specified Request-to-Response interval....
>>>
>>> To handle such case, we maintain list of received requests using the
>>> seq number of the BT message to identify them. The list is updated
>>> each time a request is received and a response is sent. The
>>> expiration
>>> of the reponses is handled at each updates but also with a timer.
>>>
>>
>> I agree that the BMC kernel is most logical place to do this, at the
>> moment btbridged does attempt something similar no?
>>
>> Should we patch btbridged to not? Should I least remove duplicated
>> logic? 
>>
> 
> Brendan,
> 
> Are you paying attention to this discussion?  I think you had some
> opposition to the kernel doing any additional work in this space because
> you wanted to run a non-IPMI protocol over the IPMI bridge.

We will then need a new kernel driver for the non-IPMI interface. 
This one is for the iBT interface which is IPMI oriented. 

I am sure we can find some common points though. Hopefully, the 
userspace interface would be the same. For the moment we only
have an ioctl for the SMS ATN interrupt but we should be adding
some more.

Thanks,

C.  

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list
  2016-11-03 18:46       ` Cédric Le Goater
@ 2016-11-04  1:55         ` Patrick Williams
  2016-11-04  8:09           ` Cédric Le Goater
  0 siblings, 1 reply; 31+ messages in thread
From: Patrick Williams @ 2016-11-04  1:55 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: Cyril Bur, Brendan Higgins, openbmc

[-- Attachment #1: Type: text/plain, Size: 2217 bytes --]

On Thu, Nov 03, 2016 at 07:46:28PM +0100, Cédric Le Goater wrote:
> On 11/03/2016 07:23 PM, Patrick Williams wrote:
> > On Thu, Nov 03, 2016 at 11:26:09AM +1100, Cyril Bur wrote:
> >> On Wed, 2016-10-26 at 08:57 +0200, Cédric Le Goater wrote:
> >>> Regarding the response expiration handling, the IPMI spec says :
> >>>
> >>>    The BMC must not return a given response once the corresponding
> >>>    Request-to-Response interval has passed. The BMC can ensure this
> >>>    by maintaining its own internal list of outstanding requests
> >>> through
> >>>    the interface. The BMC could age and expire the entries in the
> >>> list
> >>>    by expiring the entries at an interval that is somewhat shorter
> >>> than
> >>>    the specified Request-to-Response interval....
> >>>
> >>> To handle such case, we maintain list of received requests using the
> >>> seq number of the BT message to identify them. The list is updated
> >>> each time a request is received and a response is sent. The
> >>> expiration
> >>> of the reponses is handled at each updates but also with a timer.
> >>>
> >>
> >> I agree that the BMC kernel is most logical place to do this, at the
> >> moment btbridged does attempt something similar no?
> >>
> >> Should we patch btbridged to not? Should I least remove duplicated
> >> logic? 
> >>
> > 
> > Brendan,
> > 
> > Are you paying attention to this discussion?  I think you had some
> > opposition to the kernel doing any additional work in this space because
> > you wanted to run a non-IPMI protocol over the IPMI bridge.
> 
> We will then need a new kernel driver for the non-IPMI interface. 
> This one is for the iBT interface which is IPMI oriented. 
> 
> I am sure we can find some common points though. Hopefully, the 
> userspace interface would be the same. For the moment we only
> have an ioctl for the SMS ATN interrupt but we should be adding
> some more.
> 

I believe Brendan was keeping the IPMI protocol but implementing
different flow control on top of it.  That means, specifically, that
having the kernel track commands and timeouts would not be what he
wanted.

> Thanks,
> 
> C.  
> 

-- 
Patrick Williams

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list
  2016-11-04  1:55         ` Patrick Williams
@ 2016-11-04  8:09           ` Cédric Le Goater
  2016-11-04 18:22             ` Brendan Higgins
  0 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2016-11-04  8:09 UTC (permalink / raw)
  To: Patrick Williams; +Cc: Cyril Bur, Brendan Higgins, openbmc

On 11/04/2016 02:55 AM, Patrick Williams wrote:
> On Thu, Nov 03, 2016 at 07:46:28PM +0100, Cédric Le Goater wrote:
>> On 11/03/2016 07:23 PM, Patrick Williams wrote:
>>> On Thu, Nov 03, 2016 at 11:26:09AM +1100, Cyril Bur wrote:
>>>> On Wed, 2016-10-26 at 08:57 +0200, Cédric Le Goater wrote:
>>>>> Regarding the response expiration handling, the IPMI spec says :
>>>>>
>>>>>    The BMC must not return a given response once the corresponding
>>>>>    Request-to-Response interval has passed. The BMC can ensure this
>>>>>    by maintaining its own internal list of outstanding requests
>>>>> through
>>>>>    the interface. The BMC could age and expire the entries in the
>>>>> list
>>>>>    by expiring the entries at an interval that is somewhat shorter
>>>>> than
>>>>>    the specified Request-to-Response interval....
>>>>>
>>>>> To handle such case, we maintain list of received requests using the
>>>>> seq number of the BT message to identify them. The list is updated
>>>>> each time a request is received and a response is sent. The
>>>>> expiration
>>>>> of the reponses is handled at each updates but also with a timer.
>>>>>
>>>>
>>>> I agree that the BMC kernel is most logical place to do this, at the
>>>> moment btbridged does attempt something similar no?
>>>>
>>>> Should we patch btbridged to not? Should I least remove duplicated
>>>> logic? 
>>>>
>>>
>>> Brendan,
>>>
>>> Are you paying attention to this discussion?  I think you had some
>>> opposition to the kernel doing any additional work in this space because
>>> you wanted to run a non-IPMI protocol over the IPMI bridge.
>>
>> We will then need a new kernel driver for the non-IPMI interface. 
>> This one is for the iBT interface which is IPMI oriented. 
>>
>> I am sure we can find some common points though. Hopefully, the 
>> userspace interface would be the same. For the moment we only
>> have an ioctl for the SMS ATN interrupt but we should be adding
>> some more.
>>
> 
> I believe Brendan was keeping the IPMI protocol but implementing
> different flow control on top of it.  That means, specifically, that
> having the kernel track commands and timeouts would not be what he
> wanted.

ok. I have lost track of this. May be we could start a wiki on 
btbridge to hold all these ideas, development progress, etc. 
Like we have done for qemu and u-boot ?

Thanks,

C. 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list
  2016-11-04  8:09           ` Cédric Le Goater
@ 2016-11-04 18:22             ` Brendan Higgins
  2016-11-07  5:25               ` Cédric Le Goater
  0 siblings, 1 reply; 31+ messages in thread
From: Brendan Higgins @ 2016-11-04 18:22 UTC (permalink / raw)
  To: Cédric Le Goater, Patrick Williams; +Cc: Cyril Bur, OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 281 bytes --]

Thanks for roping me in,

Patrick summed up my intention perfectly. IPMI messages will be structured
the same and use compatible fields, but would allow for different flow
control. In particular, the goal is to allow multiple requests and
responses under a single sequence number.

[-- Attachment #2: Type: text/html, Size: 327 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list
  2016-11-04 18:22             ` Brendan Higgins
@ 2016-11-07  5:25               ` Cédric Le Goater
  2016-11-07 15:02                 ` Patrick Williams
  0 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2016-11-07  5:25 UTC (permalink / raw)
  To: Brendan Higgins, Patrick Williams; +Cc: Cyril Bur, OpenBMC Maillist

Hello,

On 11/04/2016 07:22 PM, Brendan Higgins wrote:
> Thanks for roping me in,
> 
> Patrick summed up my intention perfectly. IPMI messages will be structured 
> the same and use compatible fields, but would allow for different flow control. 
> In particular, the goal is to allow multiple requests and responses under a 
> single sequence number.

So if the combinations of { Seq, Command, NetFn } values are unique, we 
should be fine. The expiry list patch needs to be more precise when doing 
matching though.

Thanks,

C. 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list
  2016-11-07  5:25               ` Cédric Le Goater
@ 2016-11-07 15:02                 ` Patrick Williams
  2016-11-07 19:29                   ` Brendan Higgins
  0 siblings, 1 reply; 31+ messages in thread
From: Patrick Williams @ 2016-11-07 15:02 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: Brendan Higgins, Cyril Bur, OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

On Mon, Nov 07, 2016 at 06:25:41AM +0100, Cédric Le Goater wrote:
> Hello,
> 
> On 11/04/2016 07:22 PM, Brendan Higgins wrote:
> > Thanks for roping me in,
> > 
> > Patrick summed up my intention perfectly. IPMI messages will be structured 
> > the same and use compatible fields, but would allow for different flow control. 
> > In particular, the goal is to allow multiple requests and responses under a 
> > single sequence number.
> 
> So if the combinations of { Seq, Command, NetFn } values are unique, we 
> should be fine. The expiry list patch needs to be more precise when doing 
> matching though.

Brendan was implying that this is incorrect for OEM commands.

Brendan can you clarify?

-- 
Patrick Williams

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list
  2016-11-07 15:02                 ` Patrick Williams
@ 2016-11-07 19:29                   ` Brendan Higgins
  2016-11-08  9:58                     ` Cédric Le Goater
  0 siblings, 1 reply; 31+ messages in thread
From: Brendan Higgins @ 2016-11-07 19:29 UTC (permalink / raw)
  To: Patrick Williams, Cédric Le Goater; +Cc: Cyril Bur, OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]

Patrick, you are correct.

The { Seq, Command, NetFn } set does not necessarily uniquely identify a
message. The the OEM/Group extension messages as defined in the IPMI v2.0
Specification Document Revision 1.1, Section 5.1 Network Function Codes,
Table 5, under the name OEM/Group defines unique message endpoints by the
following set: { NetFn, Payload{0, 1, 2}, Command }; where Payload{0, 1, 2}
are the first three bytes of a message payload.

In addition, I am working on an OEM/Group extension that would allow a
single transaction to take place across multiple messages, and in
particular would allow multiple IPMI requests to be sent, potentially only
sending (a) response(s) at the end of the transaction; in particular, this
would allow the following:

Master     | Slave
Request -> |
Request -> |
Request -> |
           | <- Response
           | <- Response

Where Request and Response have the same sequence number. Note: the change
I am making would also make the above example valid if there were only one
request and two responses.

@Cédric I see two remedies for your patch to allow this:

1) Allow users to opt out of the expiry list.

2) Update the keep alive list when *either* a new request or a new response
is sent with a given sequence number (change the list to be a hashmap of
sequence numbers to timeouts).

I will try to get a design doc for discussion describing this extension out
hopefully sometime this week; I think that might clear some things up.

Thanks!

[-- Attachment #2: Type: text/html, Size: 1615 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list
  2016-11-07 19:29                   ` Brendan Higgins
@ 2016-11-08  9:58                     ` Cédric Le Goater
  0 siblings, 0 replies; 31+ messages in thread
From: Cédric Le Goater @ 2016-11-08  9:58 UTC (permalink / raw)
  To: Brendan Higgins, Patrick Williams; +Cc: Cyril Bur, OpenBMC Maillist

Hello Brendan,

On 11/07/2016 08:29 PM, Brendan Higgins wrote:
> Patrick, you are correct.
> 
> The { Seq, Command, NetFn } set does not necessarily uniquely identify a message. 
> The the OEM/Group extension messages as defined in the IPMI v2.0 Specification 
> Document Revision 1.1, Section 5.1 Network Function Codes, Table 5, under the 
> name OEM/Group defines unique message endpoints by the following set: 
> 
>    { NetFn, Payload{0, 1, 2}, Command }; 
>
> where Payload{0, 1, 2} are the first three bytes of a message payload.

OK. I missed that part. I was referring to : 

  11.1 BT Interface-BMC Request Message Format

    ...
    The Seq field is used in combination with the NetFn and Command fields to 
    form a unique value. I.e. the same Seq value could be used in multiple 
    outstanding requests, as long as the combinations of Seq value, NetFn, 
    and Command were unique among the requests.
    ...
and

  11.3 Using the Seq Field
  11.4 Response Expiration Handling

    ...
    The BMC can define its own internal Seq value or tracking number for
    this purpose, or it could use the Seq, NetFn, and Command values in the
    same manner as SMS.
    ...

> In addition, I am working on an OEM/Group extension that would allow a single 
> transaction to take place across multiple messages, and in particular would 
> allow multiple IPMI requests to be sent, potentially only sending (a) 
> response(s) at the end of the transaction; in particular, this would allow 
> the following:
> 
> Master     | Slave
> Request -> |
> Request -> |
> Request -> |
>            | <- Response
>            | <- Response
> 
> Where Request and Response have the same sequence number. Note: the change 
> I am making would also make the above example valid if there were only one 
> request and two responses.

I am wondering how that fits with the BT Interface Capabilities which can 
recommend retries.

> @Cédric I see two remedies for your patch to allow this:
> 
> 1) Allow users to opt out of the expiry list.

yes that's should not be too hard.

> 2) Update the keep alive list when *either* a new request or a new response 
> is sent with a given sequence number (change the list to be a hashmap of 
> sequence numbers to timeouts).

The BMC driver could also define its own 'Seq' identifier (using the tuple 
above) to do the request/response matching instead of using the 'Seq' byte.

> I will try to get a design doc for discussion describing this extension out 
> hopefully sometime this week; I think that might clear some things up.

OK but we should be having this discussion on :
 
	openipmi-developer@lists.sourceforge.net

where the patches are being discussed. Corey would have some very valuable
input. Could you join that thread ? 

	http://lists.infradead.org/pipermail/linux-arm-kernel/2016-November/465131.html
	
Or I can copy you, it might be easier and then you can start another thread
with your design.

Thanks!

C. 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list
  2016-11-03  9:06     ` Cédric Le Goater
@ 2016-11-10  7:53       ` Cédric Le Goater
  0 siblings, 0 replies; 31+ messages in thread
From: Cédric Le Goater @ 2016-11-10  7:53 UTC (permalink / raw)
  To: Cyril Bur, openbmc, Joel Stanley, Brendan Higgins

On 11/03/2016 10:06 AM, Cédric Le Goater wrote:
> On 11/03/2016 01:26 AM, Cyril Bur wrote:
>> On Wed, 2016-10-26 at 08:57 +0200, Cédric Le Goater wrote:
>>> Regarding the response expiration handling, the IPMI spec says :
>>>
>>>    The BMC must not return a given response once the corresponding
>>>    Request-to-Response interval has passed. The BMC can ensure this
>>>    by maintaining its own internal list of outstanding requests through
>>>    the interface. The BMC could age and expire the entries in the list
>>>    by expiring the entries at an interval that is somewhat shorter than
>>>    the specified Request-to-Response interval....
>>>
>>> To handle such case, we maintain list of received requests using the
>>> seq number of the BT message to identify them. The list is updated
>>> each time a request is received and a response is sent. The
>>> expirationof the reponses is handled at each updates but also 
>>> with a timer.
>>>
>>
>> I agree that the BMC kernel is most logical place to do this, at the
>> moment btbridged does attempt something similar no?
>>
>>
>> Should we patch btbridged to not? Should I least remove duplicated
>> logic? 
> 
> yes probably. Let's see what mainline says about it first. 
> The patch has a minor flaw: struct ipmi_request should include 
> the NetFn and Command values. Corey might ask for a resend.

So it seems this patch is raising more questions than solving 
problems. Let's leave it aside for the moment and keep the list
in btbridged.

Thanks,

C.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 0/8] BT driver sync up
  2016-10-26  6:57 [PATCH linux dev-4.7 0/8] BT driver sync up Cédric Le Goater
                   ` (8 preceding siblings ...)
  2016-11-03  0:30 ` [PATCH linux dev-4.7 0/8] BT driver sync up Cyril Bur
@ 2016-11-10  9:13 ` Cédric Le Goater
  2016-11-10 11:33   ` Joel Stanley
  9 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2016-11-10  9:13 UTC (permalink / raw)
  To: openbmc

Joel,

On 10/26/2016 08:57 AM, Cédric Le Goater wrote:
> Hello,
> 
> Here is a serie syncing the OpenBMC kernel with the BT driver merged
> in mainline linux. This kernel needs an updated version of btbridged :
> 
>      https://github.com/legoater/btbridge/commits/master
> 
> The last two patches add a request expiry list and some configuration
> through sysfs. These are for review only as they need to be sent to
> mainline first.

So please drop these two patches. This is too early.

I will send later an interesting fix from Corey which handles 
concurrent reads and writes a little better than we do today. 
But after this is all merged.

Thanks,
 
C.


> 
> Cédric Le Goater (8):
>   Revert "misc: Add Aspeed BT IPMI host driver"
>   ARM: aspeed: remove previous definitions in default config
>   ARM: dts: aspeed: remove previous iBT definitions
>   ipmi: add an Aspeed BT IPMI BMC driver
>   ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_BMC
>   ARM: dts: aspeed: Enable BT IPMI BMC device
>   ipmi: maintain a request expiry list
>   ipmi: add a sysfs file for configure maximum response time
> 
>  .../bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt   |  23 +
>  arch/arm/boot/dts/aspeed-g4.dtsi                   |   2 +-
>  arch/arm/boot/dts/aspeed-g5.dtsi                   |   2 +-
>  arch/arm/configs/aspeed_g4_defconfig               |   2 +-
>  arch/arm/configs/aspeed_g5_defconfig               |   2 +-
>  drivers/Makefile                                   |   2 +-
>  drivers/char/ipmi/Kconfig                          |   8 +
>  drivers/char/ipmi/Makefile                         |   1 +
>  drivers/char/ipmi/bt-bmc.c                         | 668 +++++++++++++++++++++
>  drivers/misc/Kconfig                               |   5 -
>  drivers/misc/Makefile                              |   1 -
>  drivers/misc/bt-host.c                             | 427 -------------
>  include/uapi/linux/Kbuild                          |   2 +-
>  include/uapi/linux/{bt-host.h => bt-bmc.h}         |  12 +-
>  14 files changed, 712 insertions(+), 445 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
>  create mode 100644 drivers/char/ipmi/bt-bmc.c
>  delete mode 100644 drivers/misc/bt-host.c
>  rename include/uapi/linux/{bt-host.h => bt-bmc.h} (55%)
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 0/8] BT driver sync up
  2016-11-10  9:13 ` Cédric Le Goater
@ 2016-11-10 11:33   ` Joel Stanley
  2016-11-10 12:04     ` Cédric Le Goater
  0 siblings, 1 reply; 31+ messages in thread
From: Joel Stanley @ 2016-11-10 11:33 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: OpenBMC Maillist

On Thu, Nov 10, 2016 at 7:43 PM, Cédric Le Goater <clg@kaod.org> wrote:
> Joel,
>
> On 10/26/2016 08:57 AM, Cédric Le Goater wrote:
>> Hello,
>>
>> Here is a serie syncing the OpenBMC kernel with the BT driver merged
>> in mainline linux. This kernel needs an updated version of btbridged :
>>
>>      https://github.com/legoater/btbridge/commits/master
>>
>> The last two patches add a request expiry list and some configuration
>> through sysfs. These are for review only as they need to be sent to
>> mainline first.
>
> So please drop these two patches. This is too early.
>
> I will send later an interesting fix from Corey which handles
> concurrent reads and writes a little better than we do today.
> But after this is all merged.

Okay, thanks for the clarification. I applied these patches to dev-v4.7:

 ARM: dts: aspeed: Enable BT IPMI BMC device
 ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_BMC
 ipmi: add an Aspeed BT IPMI BMC driver
 ARM: dts: aspeed: remove previous iBT definitions
 ARM: aspeed: remove previous definitions in default config
 Revert "misc: Add Aspeed BT IPMI host driver"

Cheers,

Joel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 0/8] BT driver sync up
  2016-11-10 11:33   ` Joel Stanley
@ 2016-11-10 12:04     ` Cédric Le Goater
  2016-11-10 12:11       ` Joel Stanley
  0 siblings, 1 reply; 31+ messages in thread
From: Cédric Le Goater @ 2016-11-10 12:04 UTC (permalink / raw)
  To: Joel Stanley; +Cc: OpenBMC Maillist

On 11/10/2016 12:33 PM, Joel Stanley wrote:
> On Thu, Nov 10, 2016 at 7:43 PM, Cédric Le Goater <clg@kaod.org> wrote:
>> Joel,
>>
>> On 10/26/2016 08:57 AM, Cédric Le Goater wrote:
>>> Hello,
>>>
>>> Here is a serie syncing the OpenBMC kernel with the BT driver merged
>>> in mainline linux. This kernel needs an updated version of btbridged :
>>>
>>>      https://github.com/legoater/btbridge/commits/master
>>>
>>> The last two patches add a request expiry list and some configuration
>>> through sysfs. These are for review only as they need to be sent to
>>> mainline first.
>>
>> So please drop these two patches. This is too early.
>>
>> I will send later an interesting fix from Corey which handles
>> concurrent reads and writes a little better than we do today.
>> But after this is all merged.
> 
> Okay, thanks for the clarification. I applied these patches to dev-v4.7:
> 
>  ARM: dts: aspeed: Enable BT IPMI BMC device
>  ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_BMC
>  ipmi: add an Aspeed BT IPMI BMC driver
>  ARM: dts: aspeed: remove previous iBT definitions
>  ARM: aspeed: remove previous definitions in default config
>  Revert "misc: Add Aspeed BT IPMI host driver"

It looks safe to take that one also :

	http://patchwork.ozlabs.org/patch/690292/

May be, it is time now for a followup with :

+	{ .compatible = "aspeed,ast2500-ibt-bmc" },

? 

C.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH linux dev-4.7 0/8] BT driver sync up
  2016-11-10 12:04     ` Cédric Le Goater
@ 2016-11-10 12:11       ` Joel Stanley
  0 siblings, 0 replies; 31+ messages in thread
From: Joel Stanley @ 2016-11-10 12:11 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: OpenBMC Maillist

On Thu, Nov 10, 2016 at 10:34 PM, Cédric Le Goater <clg@kaod.org> wrote:
>>> So please drop these two patches. This is too early.
>>>
>>> I will send later an interesting fix from Corey which handles
>>> concurrent reads and writes a little better than we do today.
>>> But after this is all merged.
>>
>> Okay, thanks for the clarification. I applied these patches to dev-v4.7:
>>
>>  ARM: dts: aspeed: Enable BT IPMI BMC device
>>  ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_BMC
>>  ipmi: add an Aspeed BT IPMI BMC driver
>>  ARM: dts: aspeed: remove previous iBT definitions
>>  ARM: aspeed: remove previous definitions in default config
>>  Revert "misc: Add Aspeed BT IPMI host driver"
>
> It looks safe to take that one also :
>
>         http://patchwork.ozlabs.org/patch/690292/

Oops, will need to update the device trees again. Yep, I will include
this one along with some updates to the device trees.

>
> May be, it is time now for a followup with :
>
> +       { .compatible = "aspeed,ast2500-ibt-bmc" },
>
> ?

For upstream? Yeah. I would consider collecting all of the acks from
the patch you posted and adding the second string in too, then posting
as a v2.

Cheers,

Joel

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2016-11-10 12:13 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-26  6:57 [PATCH linux dev-4.7 0/8] BT driver sync up Cédric Le Goater
2016-10-26  6:57 ` [PATCH linux dev-4.7 1/8] Revert "misc: Add Aspeed BT IPMI host driver" Cédric Le Goater
2016-10-26  6:57 ` [PATCH linux dev-4.7 2/8] ARM: aspeed: remove previous definitions in default config Cédric Le Goater
2016-10-26  6:57 ` [PATCH linux dev-4.7 3/8] ARM: dts: aspeed: remove previous iBT definitions Cédric Le Goater
2016-10-26  6:57 ` [PATCH linux dev-4.7 4/8] ipmi: add an Aspeed BT IPMI BMC driver Cédric Le Goater
2016-11-02  0:00   ` Joel Stanley
2016-11-02  7:08     ` Cédric Le Goater
2016-11-03  0:56   ` Cyril Bur
2016-11-03  2:46     ` Joel Stanley
2016-11-03  2:48       ` Cyril Bur
2016-10-26  6:57 ` [PATCH linux dev-4.7 5/8] ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_BMC Cédric Le Goater
2016-10-26  6:57 ` [PATCH linux dev-4.7 6/8] ARM: dts: aspeed: Enable BT IPMI BMC device Cédric Le Goater
2016-10-26  6:57 ` [PATCH linux dev-4.7 7/8] ipmi: maintain a request expiry list Cédric Le Goater
2016-11-03  0:26   ` Cyril Bur
2016-11-03  9:06     ` Cédric Le Goater
2016-11-10  7:53       ` Cédric Le Goater
2016-11-03 18:23     ` Patrick Williams
2016-11-03 18:46       ` Cédric Le Goater
2016-11-04  1:55         ` Patrick Williams
2016-11-04  8:09           ` Cédric Le Goater
2016-11-04 18:22             ` Brendan Higgins
2016-11-07  5:25               ` Cédric Le Goater
2016-11-07 15:02                 ` Patrick Williams
2016-11-07 19:29                   ` Brendan Higgins
2016-11-08  9:58                     ` Cédric Le Goater
2016-10-26  6:57 ` [PATCH linux dev-4.7 8/8] ipmi: add a sysfs file for configure maximum response time Cédric Le Goater
2016-11-03  0:30 ` [PATCH linux dev-4.7 0/8] BT driver sync up Cyril Bur
2016-11-10  9:13 ` Cédric Le Goater
2016-11-10 11:33   ` Joel Stanley
2016-11-10 12:04     ` Cédric Le Goater
2016-11-10 12:11       ` Joel Stanley

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.