[PATCH 0/3] ARM: aspeed: add support for the BT IPMI host device

[PATCH 0/3] ARM: aspeed: add support for the BT IPMI host device

[Cédric Le Goater]

Hello,

This serie adds support for the BT IPMI host device on Aspeed SoCs
ast2400 and ast2500. 

Cheers,

C.

Alistair Popple (1):
  misc: Add Aspeed BT IPMI host driver

Cédric Le Goater (2):
  ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_HOST
  ARM: dts: aspeed: Enable BT IPMI host device

 .../devicetree/bindings/misc/aspeed,bt-host.txt    |  19 +
 arch/arm/boot/dts/aspeed-g4.dtsi                   |   6 +
 arch/arm/boot/dts/aspeed-g5.dtsi                   |   6 +
 arch/arm/configs/aspeed_g4_defconfig               |   1 +
 arch/arm/configs/aspeed_g5_defconfig               |   1 +
 drivers/misc/Kconfig                               |   5 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/bt-host.c                             | 433 +++++++++++++++++++++
 include/uapi/linux/Kbuild                          |   1 +
 include/uapi/linux/bt-host.h                       |  18 +
 10 files changed, 491 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
 create mode 100644 drivers/misc/bt-host.c
 create mode 100644 include/uapi/linux/bt-host.h

-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 0/3] ARM: aspeed: add support for the BT IPMI host device

[Cédric Le Goater]

Hello,

This serie adds support for the BT IPMI host device on Aspeed SoCs
ast2400 and ast2500. 

Cheers,

C.

Alistair Popple (1):
  misc: Add Aspeed BT IPMI host driver

C?dric Le Goater (2):
  ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_HOST
  ARM: dts: aspeed: Enable BT IPMI host device

 .../devicetree/bindings/misc/aspeed,bt-host.txt    |  19 +
 arch/arm/boot/dts/aspeed-g4.dtsi                   |   6 +
 arch/arm/boot/dts/aspeed-g5.dtsi                   |   6 +
 arch/arm/configs/aspeed_g4_defconfig               |   1 +
 arch/arm/configs/aspeed_g5_defconfig               |   1 +
 drivers/misc/Kconfig                               |   5 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/bt-host.c                             | 433 +++++++++++++++++++++
 include/uapi/linux/Kbuild                          |   1 +
 include/uapi/linux/bt-host.h                       |  18 +
 10 files changed, 491 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
 create mode 100644 drivers/misc/bt-host.c
 create mode 100644 include/uapi/linux/bt-host.h

-- 
2.7.4

[PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Cédric Le Goater]

From: Alistair Popple <alistair@popple.id.au>

This patch adds a simple device driver to expose the iBT interface on
Aspeed chips as a character device (/dev/bt).

The iBT interface is used to perform in-band IPMI communication from a
BMC to the host.

Signed-off-by: Alistair Popple <alistair@popple.id.au>
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
[clg: checkpatch fixes
      devicetree binding documentation]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 .../devicetree/bindings/misc/aspeed,bt-host.txt    |  19 +
 drivers/misc/Kconfig                               |   5 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/bt-host.c                             | 433 +++++++++++++++++++++
 include/uapi/linux/Kbuild                          |   1 +
 include/uapi/linux/bt-host.h                       |  18 +
 6 files changed, 477 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
 create mode 100644 drivers/misc/bt-host.c
 create mode 100644 include/uapi/linux/bt-host.h

diff --git a/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
new file mode 100644
index 000000000000..938c5998c331
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
@@ -0,0 +1,19 @@
+* Aspeed BT IPMI interface
+
+Required properties:
+
+- compatible : should be "aspeed,bt-host"
+- 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,bt-host";
+		reg = <0x1e789140 0x18>;
+		interrupts = <8>;
+	};
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a216b4667742..47f761861120 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -804,6 +804,11 @@ 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 7410c6d9a34d..71a7b9feb0f0 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -57,6 +57,7 @@ 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
 
 lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
 lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o
diff --git a/drivers/misc/bt-host.c b/drivers/misc/bt-host.c
new file mode 100644
index 000000000000..3d9bfbaa8940
--- /dev/null
+++ b/drivers/misc/bt-host.c
@@ -0,0 +1,433 @@
+/*
+ * 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/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 185f8ea2702f..c763a35c2147 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-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
new file mode 100644
index 000000000000..e2aedddf0bf4
--- /dev/null
+++ b/include/uapi/linux/bt-host.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_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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Cédric Le Goater]

From: Alistair Popple <alistair@popple.id.au>

This patch adds a simple device driver to expose the iBT interface on
Aspeed chips as a character device (/dev/bt).

The iBT interface is used to perform in-band IPMI communication from a
BMC to the host.

Signed-off-by: Alistair Popple <alistair@popple.id.au>
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
[clg: checkpatch fixes
      devicetree binding documentation]
Signed-off-by: C?dric Le Goater <clg@kaod.org>
---
 .../devicetree/bindings/misc/aspeed,bt-host.txt    |  19 +
 drivers/misc/Kconfig                               |   5 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/bt-host.c                             | 433 +++++++++++++++++++++
 include/uapi/linux/Kbuild                          |   1 +
 include/uapi/linux/bt-host.h                       |  18 +
 6 files changed, 477 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
 create mode 100644 drivers/misc/bt-host.c
 create mode 100644 include/uapi/linux/bt-host.h

diff --git a/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
new file mode 100644
index 000000000000..938c5998c331
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
@@ -0,0 +1,19 @@
+* Aspeed BT IPMI interface
+
+Required properties:
+
+- compatible : should be "aspeed,bt-host"
+- 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 at 1e789140 {
+		compatible = "aspeed,bt-host";
+		reg = <0x1e789140 0x18>;
+		interrupts = <8>;
+	};
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a216b4667742..47f761861120 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -804,6 +804,11 @@ 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 7410c6d9a34d..71a7b9feb0f0 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -57,6 +57,7 @@ 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
 
 lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
 lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o
diff --git a/drivers/misc/bt-host.c b/drivers/misc/bt-host.c
new file mode 100644
index 000000000000..3d9bfbaa8940
--- /dev/null
+++ b/drivers/misc/bt-host.c
@@ -0,0 +1,433 @@
+/*
+ * 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/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 185f8ea2702f..c763a35c2147 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-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
new file mode 100644
index 000000000000..e2aedddf0bf4
--- /dev/null
+++ b/include/uapi/linux/bt-host.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_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

[PATCH 2/3] ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_HOST

[Cédric Le Goater]

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 ca39c04fec6b..7f39d64357fa 100644
--- a/arch/arm/configs/aspeed_g4_defconfig
+++ b/arch/arm/configs/aspeed_g4_defconfig
@@ -40,6 +40,7 @@ CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 # CONFIG_PREVENT_FIRMWARE_BUILD is not set
+CONFIG_ASPEED_BT_IPMI_HOST=y
 # CONFIG_INPUT is not set
 # CONFIG_SERIO is not set
 # CONFIG_VT is not set
diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index 4f366b0370e9..53d74afd05ea 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -41,6 +41,7 @@ CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 # CONFIG_PREVENT_FIRMWARE_BUILD is not set
+CONFIG_ASPEED_BT_IPMI_HOST=y
 # CONFIG_INPUT is not set
 # CONFIG_SERIO is not set
 # CONFIG_VT is not set
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] ARM: aspeed: Add defconfigs for CONFIG_ASPEED_BT_IPMI_HOST

[Cédric Le Goater]

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 ca39c04fec6b..7f39d64357fa 100644
--- a/arch/arm/configs/aspeed_g4_defconfig
+++ b/arch/arm/configs/aspeed_g4_defconfig
@@ -40,6 +40,7 @@ CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 # CONFIG_PREVENT_FIRMWARE_BUILD is not set
+CONFIG_ASPEED_BT_IPMI_HOST=y
 # CONFIG_INPUT is not set
 # CONFIG_SERIO is not set
 # CONFIG_VT is not set
diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index 4f366b0370e9..53d74afd05ea 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -41,6 +41,7 @@ CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 # CONFIG_PREVENT_FIRMWARE_BUILD is not set
+CONFIG_ASPEED_BT_IPMI_HOST=y
 # CONFIG_INPUT is not set
 # CONFIG_SERIO is not set
 # CONFIG_VT is not set
-- 
2.7.4

[PATCH 3/3] ARM: dts: aspeed: Enable BT IPMI host device

[Cédric Le Goater]

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 22dee5937d5c..1cf57a2b08d3 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -82,6 +82,12 @@
 				clocks = <&clk_apb>;
 			};
 
+			ibt: ibt@1e789140 {
+				compatible = "aspeed,bt-host";
+				reg = <0x1e789140 0x18>;
+				interrupts = <8>;
+			};
+
 			wdt1: wdt@1e785000 {
 				compatible = "aspeed,wdt";
 				reg = <0x1e785000 0x1c>;
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index dd94d9361fda..9f29c409d606 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -86,6 +86,12 @@
 				clocks = <&clk_apb>;
 			};
 
+			ibt: ibt@1e789140 {
+				compatible = "aspeed,bt-host";
+				reg = <0x1e789140 0x18>;
+				interrupts = <8>;
+			};
+
 			wdt1: wdt@1e785000 {
 				compatible = "aspeed,wdt";
 				reg = <0x1e785000 0x1c>;
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] ARM: dts: aspeed: Enable BT IPMI host device

[Cédric Le Goater]

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 22dee5937d5c..1cf57a2b08d3 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -82,6 +82,12 @@
 				clocks = <&clk_apb>;
 			};
 
+			ibt: ibt at 1e789140 {
+				compatible = "aspeed,bt-host";
+				reg = <0x1e789140 0x18>;
+				interrupts = <8>;
+			};
+
 			wdt1: wdt at 1e785000 {
 				compatible = "aspeed,wdt";
 				reg = <0x1e785000 0x1c>;
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index dd94d9361fda..9f29c409d606 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -86,6 +86,12 @@
 				clocks = <&clk_apb>;
 			};
 
+			ibt: ibt at 1e789140 {
+				compatible = "aspeed,bt-host";
+				reg = <0x1e789140 0x18>;
+				interrupts = <8>;
+			};
+
 			wdt1: wdt at 1e785000 {
 				compatible = "aspeed,wdt";
 				reg = <0x1e785000 0x1c>;
-- 
2.7.4

Re: [PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Arnd Bergmann]

On Wednesday, August 31, 2016 7:24:17 PM CEST Cédric Le Goater wrote:
> From: Alistair Popple <alistair-Y4h6yKqj69EXC2x5gXVKYQ@public.gmane.org>
> 
> This patch adds a simple device driver to expose the iBT interface on
> Aspeed chips as a character device (/dev/bt).
> 
> The iBT interface is used to perform in-band IPMI communication from a
> BMC to the host.
> 
> Signed-off-by: Alistair Popple <alistair-Y4h6yKqj69EXC2x5gXVKYQ@public.gmane.org>
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
> [clg: checkpatch fixes
>       devicetree binding documentation]
> Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>
> ---
>  .../devicetree/bindings/misc/aspeed,bt-host.txt    |  19 +
>  drivers/misc/Kconfig                               |   5 +
>  drivers/misc/Makefile                              |   1 +
>  drivers/misc/bt-host.c                             | 433 +++++++++++++++++++++
>  include/uapi/linux/Kbuild                          |   1 +
>  include/uapi/linux/bt-host.h                       |  18 +
>  6 files changed, 477 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
>  create mode 100644 drivers/misc/bt-host.c
>  create mode 100644 include/uapi/linux/bt-host.h
> 
> diff --git a/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
> new file mode 100644
> index 000000000000..938c5998c331
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt

"misc" seems like a bad category here. Does this fit nowhere else?

> @@ -0,0 +1,19 @@
> +* Aspeed BT IPMI interface

What does "BT" stand for? IPMI is a more commonly known acronym,
but maybe list both with their full name as well.

> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 7410c6d9a34d..71a7b9feb0f0 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -57,6 +57,7 @@ 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
>  
>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o

Maybe put this in a subdirectory of drivers/char/ipmi?
I understand that this is the other end of the protocol,
but they are closely related after all.

> +#define DEVICE_NAME	"bt-host"

here maybe "ipmi/bt-host" or "ipmi-bt-host"?

> +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++;
> +	}

If there are larger chunks of data to be transferred,
using a temporary buffer with copy_from_user/copy_to_user
would be more efficient. Since the size appears to
be limited to 256 bytes anyway, that easily fits on the stack.

> +
> +	clr_b_busy(bt_host);
> +
> +	return p - buf;
> +}

What is the motivation for only allowing complete messages
to be transferred or truncated for short buffers?

Have you considered reading the message into a device specific
buffer and allowing continued reads?

I don't see an obvious reason one way or another, and I suppose
you had an idea of what you were doing, so maybe explain it
in a comment.

> +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;
> +}

Is this ioctl interface defined in a way that makes sense on
any IPMI host hardware, or did you just do it like this because
it is the easiest way on the hardware. I think it's important
for the user interface to be extensible to other implementations
if we ever add any.

> +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;

I think platform_get_irq() is the preferred interface here.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Arnd Bergmann]

On Wednesday, August 31, 2016 7:24:17 PM CEST C?dric Le Goater wrote:
> From: Alistair Popple <alistair@popple.id.au>
> 
> This patch adds a simple device driver to expose the iBT interface on
> Aspeed chips as a character device (/dev/bt).
> 
> The iBT interface is used to perform in-band IPMI communication from a
> BMC to the host.
> 
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> [clg: checkpatch fixes
>       devicetree binding documentation]
> Signed-off-by: C?dric Le Goater <clg@kaod.org>
> ---
>  .../devicetree/bindings/misc/aspeed,bt-host.txt    |  19 +
>  drivers/misc/Kconfig                               |   5 +
>  drivers/misc/Makefile                              |   1 +
>  drivers/misc/bt-host.c                             | 433 +++++++++++++++++++++
>  include/uapi/linux/Kbuild                          |   1 +
>  include/uapi/linux/bt-host.h                       |  18 +
>  6 files changed, 477 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
>  create mode 100644 drivers/misc/bt-host.c
>  create mode 100644 include/uapi/linux/bt-host.h
> 
> diff --git a/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
> new file mode 100644
> index 000000000000..938c5998c331
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt

"misc" seems like a bad category here. Does this fit nowhere else?

> @@ -0,0 +1,19 @@
> +* Aspeed BT IPMI interface

What does "BT" stand for? IPMI is a more commonly known acronym,
but maybe list both with their full name as well.

> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 7410c6d9a34d..71a7b9feb0f0 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -57,6 +57,7 @@ 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
>  
>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o

Maybe put this in a subdirectory of drivers/char/ipmi?
I understand that this is the other end of the protocol,
but they are closely related after all.

> +#define DEVICE_NAME	"bt-host"

here maybe "ipmi/bt-host" or "ipmi-bt-host"?

> +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++;
> +	}

If there are larger chunks of data to be transferred,
using a temporary buffer with copy_from_user/copy_to_user
would be more efficient. Since the size appears to
be limited to 256 bytes anyway, that easily fits on the stack.

> +
> +	clr_b_busy(bt_host);
> +
> +	return p - buf;
> +}

What is the motivation for only allowing complete messages
to be transferred or truncated for short buffers?

Have you considered reading the message into a device specific
buffer and allowing continued reads?

I don't see an obvious reason one way or another, and I suppose
you had an idea of what you were doing, so maybe explain it
in a comment.

> +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;
> +}

Is this ioctl interface defined in a way that makes sense on
any IPMI host hardware, or did you just do it like this because
it is the easiest way on the hardware. I think it's important
for the user interface to be extensible to other implementations
if we ever add any.

> +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;

I think platform_get_irq() is the preferred interface here.

	Arnd

Re: [PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Cédric Le Goater]

Hello,

Adding Corey in cc: . I guess I should have done that in the first place.

Arnd is suggesting to put this IPMI BT driver under drivers/char/ipmi 
which is indeed a better place than drivers/misc. 

Below some comments,

On 08/31/2016 09:57 PM, Arnd Bergmann wrote:
> On Wednesday, August 31, 2016 7:24:17 PM CEST Cédric Le Goater wrote:
>> From: Alistair Popple <alistair@popple.id.au>
>>
>> This patch adds a simple device driver to expose the iBT interface on
>> Aspeed chips as a character device (/dev/bt).
>>
>> The iBT interface is used to perform in-band IPMI communication from a
>> BMC to the host.
>>
>> Signed-off-by: Alistair Popple <alistair@popple.id.au>
>> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> [clg: checkpatch fixes
>>       devicetree binding documentation]
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  .../devicetree/bindings/misc/aspeed,bt-host.txt    |  19 +
>>  drivers/misc/Kconfig                               |   5 +
>>  drivers/misc/Makefile                              |   1 +
>>  drivers/misc/bt-host.c                             | 433 +++++++++++++++++++++
>>  include/uapi/linux/Kbuild                          |   1 +
>>  include/uapi/linux/bt-host.h                       |  18 +
>>  6 files changed, 477 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
>>  create mode 100644 drivers/misc/bt-host.c
>>  create mode 100644 include/uapi/linux/bt-host.h
>>
>> diff --git a/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
>> new file mode 100644
>> index 000000000000..938c5998c331
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
> 
> "misc" seems like a bad category here. Does this fit nowhere else?
> 
>> @@ -0,0 +1,19 @@
>> +* Aspeed BT IPMI interface
> 
> What does "BT" stand for? IPMI is a more commonly known acronym,
> but maybe list both with their full name as well.

"Block Transfer" which is described in the IPMI specs. 

yes, I need to rephrase the commit log a bit and put some references 
to the specs.

> 
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 7410c6d9a34d..71a7b9feb0f0 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -57,6 +57,7 @@ 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
>>  
>>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
>>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o
> 
> Maybe put this in a subdirectory of drivers/char/ipmi?
> I understand that this is the other end of the protocol,
> but they are closely related after all.

I agree. There are also some definitions we could make common.

Let's see what Corey thinks about it.

>> +#define DEVICE_NAME	"bt-host"
> 
> here maybe "ipmi/bt-host" or "ipmi-bt-host"?

yes. a name containing 'ipmi' is certainly wanted.
  
>> +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++;
>> +	}
> 
> If there are larger chunks of data to be transferred,
> using a temporary buffer with copy_from_user/copy_to_user
> would be more efficient. Since the size appears to
> be limited to 256 bytes anyway, that easily fits on the stack.

ok. I will change that.

>> +
>> +	clr_b_busy(bt_host);
>> +
>> +	return p - buf;
>> +}
> 
> What is the motivation for only allowing complete messages
> to be transferred or truncated for short buffers?
> 
> Have you considered reading the message into a device specific
> buffer and allowing continued reads?
> 
> I don't see an obvious reason one way or another, and I suppose
> you had an idea of what you were doing, so maybe explain it
> in a comment.

The interface is not byte oriented. It is called 'Block Transfer'
because an entire message is buffered and then the host or the bmc
is notified that there is data to be read.  

I will add a comment on that.

>> +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;
>> +}
> 
> Is this ioctl interface defined in a way that makes sense on
> any IPMI host hardware, or did you just do it like this because 
> it is the easiest way  on the hardware. 

This platform runs OpenBMC on which a dbus daemon acts as a proxy 
between the IPMI BT char device and the rest of the system. So yes, 
the ioctl is relatively easy to use. 

> I think it's important for the user interface to be extensible 
> to other implementations if we ever add any.

I agree but I don't know of any other BMC side implementations. 
May be others ? 

>> +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;
> 
> I think platform_get_irq() is the preferred interface here.

OK.

Thanks,

C.

[PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Cédric Le Goater]

Hello,

Adding Corey in cc: . I guess I should have done that in the first place.

Arnd is suggesting to put this IPMI BT driver under drivers/char/ipmi 
which is indeed a better place than drivers/misc. 

Below some comments,

On 08/31/2016 09:57 PM, Arnd Bergmann wrote:
> On Wednesday, August 31, 2016 7:24:17 PM CEST C?dric Le Goater wrote:
>> From: Alistair Popple <alistair@popple.id.au>
>>
>> This patch adds a simple device driver to expose the iBT interface on
>> Aspeed chips as a character device (/dev/bt).
>>
>> The iBT interface is used to perform in-band IPMI communication from a
>> BMC to the host.
>>
>> Signed-off-by: Alistair Popple <alistair@popple.id.au>
>> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> [clg: checkpatch fixes
>>       devicetree binding documentation]
>> Signed-off-by: C?dric Le Goater <clg@kaod.org>
>> ---
>>  .../devicetree/bindings/misc/aspeed,bt-host.txt    |  19 +
>>  drivers/misc/Kconfig                               |   5 +
>>  drivers/misc/Makefile                              |   1 +
>>  drivers/misc/bt-host.c                             | 433 +++++++++++++++++++++
>>  include/uapi/linux/Kbuild                          |   1 +
>>  include/uapi/linux/bt-host.h                       |  18 +
>>  6 files changed, 477 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
>>  create mode 100644 drivers/misc/bt-host.c
>>  create mode 100644 include/uapi/linux/bt-host.h
>>
>> diff --git a/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
>> new file mode 100644
>> index 000000000000..938c5998c331
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
> 
> "misc" seems like a bad category here. Does this fit nowhere else?
> 
>> @@ -0,0 +1,19 @@
>> +* Aspeed BT IPMI interface
> 
> What does "BT" stand for? IPMI is a more commonly known acronym,
> but maybe list both with their full name as well.

"Block Transfer" which is described in the IPMI specs. 

yes, I need to rephrase the commit log a bit and put some references 
to the specs.

> 
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 7410c6d9a34d..71a7b9feb0f0 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -57,6 +57,7 @@ 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
>>  
>>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
>>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o
> 
> Maybe put this in a subdirectory of drivers/char/ipmi?
> I understand that this is the other end of the protocol,
> but they are closely related after all.

I agree. There are also some definitions we could make common.

Let's see what Corey thinks about it.

>> +#define DEVICE_NAME	"bt-host"
> 
> here maybe "ipmi/bt-host" or "ipmi-bt-host"?

yes. a name containing 'ipmi' is certainly wanted.
  
>> +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++;
>> +	}
> 
> If there are larger chunks of data to be transferred,
> using a temporary buffer with copy_from_user/copy_to_user
> would be more efficient. Since the size appears to
> be limited to 256 bytes anyway, that easily fits on the stack.

ok. I will change that.

>> +
>> +	clr_b_busy(bt_host);
>> +
>> +	return p - buf;
>> +}
> 
> What is the motivation for only allowing complete messages
> to be transferred or truncated for short buffers?
> 
> Have you considered reading the message into a device specific
> buffer and allowing continued reads?
> 
> I don't see an obvious reason one way or another, and I suppose
> you had an idea of what you were doing, so maybe explain it
> in a comment.

The interface is not byte oriented. It is called 'Block Transfer'
because an entire message is buffered and then the host or the bmc
is notified that there is data to be read.  

I will add a comment on that.

>> +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;
>> +}
> 
> Is this ioctl interface defined in a way that makes sense on
> any IPMI host hardware, or did you just do it like this because 
> it is the easiest way  on the hardware. 

This platform runs OpenBMC on which a dbus daemon acts as a proxy 
between the IPMI BT char device and the rest of the system. So yes, 
the ioctl is relatively easy to use. 

> I think it's important for the user interface to be extensible 
> to other implementations if we ever add any.

I agree but I don't know of any other BMC side implementations. 
May be others ? 

>> +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;
> 
> I think platform_get_irq() is the preferred interface here.

OK.

Thanks,

C.

Re: [PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Corey Minyard]

On 09/02/2016 08:22 AM, Cédric Le Goater wrote:
> Hello,
>
> Adding Corey in cc: . I guess I should have done that in the first place.

Yes, probably so.  I've been travelling and didn't see it on the mailing
lists until now.

There is already a BT driver in the kernel, in drivers/char/ipmi, why
won't that work?

-corey

>
> Arnd is suggesting to put this IPMI BT driver under drivers/char/ipmi
> which is indeed a better place than drivers/misc.
>
> Below some comments,
>
> On 08/31/2016 09:57 PM, Arnd Bergmann wrote:
>> On Wednesday, August 31, 2016 7:24:17 PM CEST Cédric Le Goater wrote:
>>> From: Alistair Popple <alistair-Y4h6yKqj69EXC2x5gXVKYQ@public.gmane.org>
>>>
>>> This patch adds a simple device driver to expose the iBT interface on
>>> Aspeed chips as a character device (/dev/bt).
>>>
>>> The iBT interface is used to perform in-band IPMI communication from a
>>> BMC to the host.
>>>
>>> Signed-off-by: Alistair Popple <alistair-Y4h6yKqj69EXC2x5gXVKYQ@public.gmane.org>
>>> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>>> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
>>> [clg: checkpatch fixes
>>>        devicetree binding documentation]
>>> Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>
>>> ---
>>>   .../devicetree/bindings/misc/aspeed,bt-host.txt    |  19 +
>>>   drivers/misc/Kconfig                               |   5 +
>>>   drivers/misc/Makefile                              |   1 +
>>>   drivers/misc/bt-host.c                             | 433 +++++++++++++++++++++
>>>   include/uapi/linux/Kbuild                          |   1 +
>>>   include/uapi/linux/bt-host.h                       |  18 +
>>>   6 files changed, 477 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
>>>   create mode 100644 drivers/misc/bt-host.c
>>>   create mode 100644 include/uapi/linux/bt-host.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
>>> new file mode 100644
>>> index 000000000000..938c5998c331
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
>> "misc" seems like a bad category here. Does this fit nowhere else?
>>
>>> @@ -0,0 +1,19 @@
>>> +* Aspeed BT IPMI interface
>> What does "BT" stand for? IPMI is a more commonly known acronym,
>> but maybe list both with their full name as well.
> "Block Transfer" which is described in the IPMI specs.
>
> yes, I need to rephrase the commit log a bit and put some references
> to the specs.
>
>>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>>> index 7410c6d9a34d..71a7b9feb0f0 100644
>>> --- a/drivers/misc/Makefile
>>> +++ b/drivers/misc/Makefile
>>> @@ -57,6 +57,7 @@ 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
>>>   
>>>   lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
>>>   lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o
>> Maybe put this in a subdirectory of drivers/char/ipmi?
>> I understand that this is the other end of the protocol,
>> but they are closely related after all.
> I agree. There are also some definitions we could make common.
>
> Let's see what Corey thinks about it.
>
>>> +#define DEVICE_NAME	"bt-host"
>> here maybe "ipmi/bt-host" or "ipmi-bt-host"?
> yes. a name containing 'ipmi' is certainly wanted.
>    
>>> +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++;
>>> +	}
>> If there are larger chunks of data to be transferred,
>> using a temporary buffer with copy_from_user/copy_to_user
>> would be more efficient. Since the size appears to
>> be limited to 256 bytes anyway, that easily fits on the stack.
> ok. I will change that.
>
>>> +
>>> +	clr_b_busy(bt_host);
>>> +
>>> +	return p - buf;
>>> +}
>> What is the motivation for only allowing complete messages
>> to be transferred or truncated for short buffers?
>>
>> Have you considered reading the message into a device specific
>> buffer and allowing continued reads?
>>
>> I don't see an obvious reason one way or another, and I suppose
>> you had an idea of what you were doing, so maybe explain it
>> in a comment.
> The interface is not byte oriented. It is called 'Block Transfer'
> because an entire message is buffered and then the host or the bmc
> is notified that there is data to be read.
>
> I will add a comment on that.
>
>>> +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;
>>> +}
>> Is this ioctl interface defined in a way that makes sense on
>> any IPMI host hardware, or did you just do it like this because
>> it is the easiest way  on the hardware.
> This platform runs OpenBMC on which a dbus daemon acts as a proxy
> between the IPMI BT char device and the rest of the system. So yes,
> the ioctl is relatively easy to use.
>
>> I think it's important for the user interface to be extensible
>> to other implementations if we ever add any.
> I agree but I don't know of any other BMC side implementations.
> May be others ?
>
>>> +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;
>> I think platform_get_irq() is the preferred interface here.
> OK.
>
> Thanks,
>
> C.
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Corey Minyard]

On 09/02/2016 08:22 AM, C?dric Le Goater wrote:
> Hello,
>
> Adding Corey in cc: . I guess I should have done that in the first place.

Yes, probably so.  I've been travelling and didn't see it on the mailing
lists until now.

There is already a BT driver in the kernel, in drivers/char/ipmi, why
won't that work?

-corey

>
> Arnd is suggesting to put this IPMI BT driver under drivers/char/ipmi
> which is indeed a better place than drivers/misc.
>
> Below some comments,
>
> On 08/31/2016 09:57 PM, Arnd Bergmann wrote:
>> On Wednesday, August 31, 2016 7:24:17 PM CEST C?dric Le Goater wrote:
>>> From: Alistair Popple <alistair@popple.id.au>
>>>
>>> This patch adds a simple device driver to expose the iBT interface on
>>> Aspeed chips as a character device (/dev/bt).
>>>
>>> The iBT interface is used to perform in-band IPMI communication from a
>>> BMC to the host.
>>>
>>> Signed-off-by: Alistair Popple <alistair@popple.id.au>
>>> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>> [clg: checkpatch fixes
>>>        devicetree binding documentation]
>>> Signed-off-by: C?dric Le Goater <clg@kaod.org>
>>> ---
>>>   .../devicetree/bindings/misc/aspeed,bt-host.txt    |  19 +
>>>   drivers/misc/Kconfig                               |   5 +
>>>   drivers/misc/Makefile                              |   1 +
>>>   drivers/misc/bt-host.c                             | 433 +++++++++++++++++++++
>>>   include/uapi/linux/Kbuild                          |   1 +
>>>   include/uapi/linux/bt-host.h                       |  18 +
>>>   6 files changed, 477 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
>>>   create mode 100644 drivers/misc/bt-host.c
>>>   create mode 100644 include/uapi/linux/bt-host.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
>>> new file mode 100644
>>> index 000000000000..938c5998c331
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/misc/aspeed,bt-host.txt
>> "misc" seems like a bad category here. Does this fit nowhere else?
>>
>>> @@ -0,0 +1,19 @@
>>> +* Aspeed BT IPMI interface
>> What does "BT" stand for? IPMI is a more commonly known acronym,
>> but maybe list both with their full name as well.
> "Block Transfer" which is described in the IPMI specs.
>
> yes, I need to rephrase the commit log a bit and put some references
> to the specs.
>
>>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>>> index 7410c6d9a34d..71a7b9feb0f0 100644
>>> --- a/drivers/misc/Makefile
>>> +++ b/drivers/misc/Makefile
>>> @@ -57,6 +57,7 @@ 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
>>>   
>>>   lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
>>>   lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o
>> Maybe put this in a subdirectory of drivers/char/ipmi?
>> I understand that this is the other end of the protocol,
>> but they are closely related after all.
> I agree. There are also some definitions we could make common.
>
> Let's see what Corey thinks about it.
>
>>> +#define DEVICE_NAME	"bt-host"
>> here maybe "ipmi/bt-host" or "ipmi-bt-host"?
> yes. a name containing 'ipmi' is certainly wanted.
>    
>>> +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++;
>>> +	}
>> If there are larger chunks of data to be transferred,
>> using a temporary buffer with copy_from_user/copy_to_user
>> would be more efficient. Since the size appears to
>> be limited to 256 bytes anyway, that easily fits on the stack.
> ok. I will change that.
>
>>> +
>>> +	clr_b_busy(bt_host);
>>> +
>>> +	return p - buf;
>>> +}
>> What is the motivation for only allowing complete messages
>> to be transferred or truncated for short buffers?
>>
>> Have you considered reading the message into a device specific
>> buffer and allowing continued reads?
>>
>> I don't see an obvious reason one way or another, and I suppose
>> you had an idea of what you were doing, so maybe explain it
>> in a comment.
> The interface is not byte oriented. It is called 'Block Transfer'
> because an entire message is buffered and then the host or the bmc
> is notified that there is data to be read.
>
> I will add a comment on that.
>
>>> +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;
>>> +}
>> Is this ioctl interface defined in a way that makes sense on
>> any IPMI host hardware, or did you just do it like this because
>> it is the easiest way  on the hardware.
> This platform runs OpenBMC on which a dbus daemon acts as a proxy
> between the IPMI BT char device and the rest of the system. So yes,
> the ioctl is relatively easy to use.
>
>> I think it's important for the user interface to be extensible
>> to other implementations if we ever add any.
> I agree but I don't know of any other BMC side implementations.
> May be others ?
>
>>> +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;
>> I think platform_get_irq() is the preferred interface here.
> OK.
>
> Thanks,
>
> C.
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Arnd Bergmann]

On Monday, September 12, 2016 1:55:40 PM CEST Corey Minyard wrote:
> On 09/02/2016 08:22 AM, Cédric Le Goater wrote:
> > Hello,
> >
> > Adding Corey in cc: . I guess I should have done that in the first place.
> 
> Yes, probably so.  I've been travelling and didn't see it on the mailing
> lists until now.
> 
> There is already a BT driver in the kernel, in drivers/char/ipmi, why
> won't that work?

The new driver is the host side (running on the BMC), the existing one
is the client (running on the PC).

	Arnd

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Arnd Bergmann]

On Monday, September 12, 2016 1:55:40 PM CEST Corey Minyard wrote:
> On 09/02/2016 08:22 AM, C?dric Le Goater wrote:
> > Hello,
> >
> > Adding Corey in cc: . I guess I should have done that in the first place.
> 
> Yes, probably so.  I've been travelling and didn't see it on the mailing
> lists until now.
> 
> There is already a BT driver in the kernel, in drivers/char/ipmi, why
> won't that work?

The new driver is the host side (running on the BMC), the existing one
is the client (running on the PC).

	Arnd

Re: [PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Corey Minyard]

On 09/12/2016 02:15 PM, Arnd Bergmann wrote:
> On Monday, September 12, 2016 1:55:40 PM CEST Corey Minyard wrote:
>> On 09/02/2016 08:22 AM, Cédric Le Goater wrote:
>>> Hello,
>>>
>>> Adding Corey in cc: . I guess I should have done that in the first place.
>> Yes, probably so.  I've been travelling and didn't see it on the mailing
>> lists until now.
>>
>> There is already a BT driver in the kernel, in drivers/char/ipmi, why
>> won't that work?
> The new driver is the host side (running on the BMC), the existing one
> is the client (running on the PC).
>
> 	Arnd

Ok, that's not really clear from the documentation or the Kconfig.
In the IPMI spec the "host" side is the computer side, not the BMC
side.  Like:

    11.6.1 BT Host Interface Registers
    The Host BT interface provides an independent set of registers and
    interrupts to allow the Host driver to
    communicate with the baseboard management controller without
    conflicting with the O/S ACPI driver.

In light of that, this should probably be named the bt-bmc driver.

I haven't reviewed this in detail, but I'm ok with putting it in
drivers/char/ipmi.  The state machine part looks reasonably
generic.  The configuration part isn't, but that could be split
out later if necessary.

The biggest thing I don't like is the byte at a time interfaces
from userspace.  That seems fairly inefficient if the system
does extra work for each userspace access.  IIRC some
systems do and some don't.

-corey

>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Corey Minyard]

On 09/12/2016 02:15 PM, Arnd Bergmann wrote:
> On Monday, September 12, 2016 1:55:40 PM CEST Corey Minyard wrote:
>> On 09/02/2016 08:22 AM, C?dric Le Goater wrote:
>>> Hello,
>>>
>>> Adding Corey in cc: . I guess I should have done that in the first place.
>> Yes, probably so.  I've been travelling and didn't see it on the mailing
>> lists until now.
>>
>> There is already a BT driver in the kernel, in drivers/char/ipmi, why
>> won't that work?
> The new driver is the host side (running on the BMC), the existing one
> is the client (running on the PC).
>
> 	Arnd

Ok, that's not really clear from the documentation or the Kconfig.
In the IPMI spec the "host" side is the computer side, not the BMC
side.  Like:

    11.6.1 BT Host Interface Registers
    The Host BT interface provides an independent set of registers and
    interrupts to allow the Host driver to
    communicate with the baseboard management controller without
    conflicting with the O/S ACPI driver.

In light of that, this should probably be named the bt-bmc driver.

I haven't reviewed this in detail, but I'm ok with putting it in
drivers/char/ipmi.  The state machine part looks reasonably
generic.  The configuration part isn't, but that could be split
out later if necessary.

The biggest thing I don't like is the byte at a time interfaces
from userspace.  That seems fairly inefficient if the system
does extra work for each userspace access.  IIRC some
systems do and some don't.

-corey

>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Cédric Le Goater]

Hello, 

On 09/12/2016 10:33 PM, Corey Minyard wrote:
> On 09/12/2016 02:15 PM, Arnd Bergmann wrote:
>> On Monday, September 12, 2016 1:55:40 PM CEST Corey Minyard wrote:
>>> On 09/02/2016 08:22 AM, Cédric Le Goater wrote:
>>>> Hello,
>>>>
>>>> Adding Corey in cc: . I guess I should have done that in the first place.
>>> Yes, probably so.  I've been travelling and didn't see it on the mailing
>>> lists until now.
>>>
>>> There is already a BT driver in the kernel, in drivers/char/ipmi, why
>>> won't that work?
>> The new driver is the host side (running on the BMC), the existing one
>> is the client (running on the PC).
>>
>>     Arnd
> 
> Ok, that's not really clear from the documentation or the Kconfig.
> In the IPMI spec the "host" side is the computer side, not the BMC
> side.  Like:
> 
>    11.6.1 BT Host Interface Registers
>    The Host BT interface provides an independent set of registers and
>    interrupts to allow the Host driver to
>    communicate with the baseboard management controller without
>    conflicting with the O/S ACPI driver.
> 
> In light of that, this should probably be named the bt-bmc driver.
> 
> I haven't reviewed this in detail, but I'm ok with putting it in
> drivers/char/ipmi.  The state machine part looks reasonably
> generic.  The configuration part isn't, but that could be split
> out later if necessary.
> 
> The biggest thing I don't like is the byte at a time interfaces
> from userspace.  That seems fairly inefficient if the system
> does extra work for each userspace access.  IIRC some
> systems do and some don't.

What about the ioctl to send an SMS ATN event to the host ? Is 
that ok for you ?

Thanks,

C.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Cédric Le Goater]

Hello, 

On 09/12/2016 10:33 PM, Corey Minyard wrote:
> On 09/12/2016 02:15 PM, Arnd Bergmann wrote:
>> On Monday, September 12, 2016 1:55:40 PM CEST Corey Minyard wrote:
>>> On 09/02/2016 08:22 AM, C?dric Le Goater wrote:
>>>> Hello,
>>>>
>>>> Adding Corey in cc: . I guess I should have done that in the first place.
>>> Yes, probably so.  I've been travelling and didn't see it on the mailing
>>> lists until now.
>>>
>>> There is already a BT driver in the kernel, in drivers/char/ipmi, why
>>> won't that work?
>> The new driver is the host side (running on the BMC), the existing one
>> is the client (running on the PC).
>>
>>     Arnd
> 
> Ok, that's not really clear from the documentation or the Kconfig.
> In the IPMI spec the "host" side is the computer side, not the BMC
> side.  Like:
> 
>    11.6.1 BT Host Interface Registers
>    The Host BT interface provides an independent set of registers and
>    interrupts to allow the Host driver to
>    communicate with the baseboard management controller without
>    conflicting with the O/S ACPI driver.
> 
> In light of that, this should probably be named the bt-bmc driver.
> 
> I haven't reviewed this in detail, but I'm ok with putting it in
> drivers/char/ipmi.  The state machine part looks reasonably
> generic.  The configuration part isn't, but that could be split
> out later if necessary.
> 
> The biggest thing I don't like is the byte at a time interfaces
> from userspace.  That seems fairly inefficient if the system
> does extra work for each userspace access.  IIRC some
> systems do and some don't.

What about the ioctl to send an SMS ATN event to the host ? Is 
that ok for you ?

Thanks,

C.

Re: [PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Corey Minyard]

On 09/12/2016 04:23 PM, Cédric Le Goater wrote:
> Hello,
>
> On 09/12/2016 10:33 PM, Corey Minyard wrote:
>> On 09/12/2016 02:15 PM, Arnd Bergmann wrote:
>>> On Monday, September 12, 2016 1:55:40 PM CEST Corey Minyard wrote:
>>>> On 09/02/2016 08:22 AM, Cédric Le Goater wrote:
>>>>> Hello,
>>>>>
>>>>> Adding Corey in cc: . I guess I should have done that in the first place.
>>>> Yes, probably so.  I've been travelling and didn't see it on the mailing
>>>> lists until now.
>>>>
>>>> There is already a BT driver in the kernel, in drivers/char/ipmi, why
>>>> won't that work?
>>> The new driver is the host side (running on the BMC), the existing one
>>> is the client (running on the PC).
>>>
>>>      Arnd
>> Ok, that's not really clear from the documentation or the Kconfig.
>> In the IPMI spec the "host" side is the computer side, not the BMC
>> side.  Like:
>>
>>     11.6.1 BT Host Interface Registers
>>     The Host BT interface provides an independent set of registers and
>>     interrupts to allow the Host driver to
>>     communicate with the baseboard management controller without
>>     conflicting with the O/S ACPI driver.
>>
>> In light of that, this should probably be named the bt-bmc driver.
>>
>> I haven't reviewed this in detail, but I'm ok with putting it in
>> drivers/char/ipmi.  The state machine part looks reasonably
>> generic.  The configuration part isn't, but that could be split
>> out later if necessary.
>>
>> The biggest thing I don't like is the byte at a time interfaces
>> from userspace.  That seems fairly inefficient if the system
>> does extra work for each userspace access.  IIRC some
>> systems do and some don't.
> What about the ioctl to send an SMS ATN event to the host ? Is
> that ok for you ?

Yeah, that's necessary, there's no way the low-level driver can
know when to do that.

I have a similar piece of code in the qemu IPMI emulator, even
the external IPMI emulator has a message it sends to set ATTN.

I did look at similarities between the two pieces of code, but
the qemu one has to be register-read/write driven, so it's
completely different in design.

-corey

> Thanks,
>
> C.


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Corey Minyard]

On 09/12/2016 04:23 PM, C?dric Le Goater wrote:
> Hello,
>
> On 09/12/2016 10:33 PM, Corey Minyard wrote:
>> On 09/12/2016 02:15 PM, Arnd Bergmann wrote:
>>> On Monday, September 12, 2016 1:55:40 PM CEST Corey Minyard wrote:
>>>> On 09/02/2016 08:22 AM, C?dric Le Goater wrote:
>>>>> Hello,
>>>>>
>>>>> Adding Corey in cc: . I guess I should have done that in the first place.
>>>> Yes, probably so.  I've been travelling and didn't see it on the mailing
>>>> lists until now.
>>>>
>>>> There is already a BT driver in the kernel, in drivers/char/ipmi, why
>>>> won't that work?
>>> The new driver is the host side (running on the BMC), the existing one
>>> is the client (running on the PC).
>>>
>>>      Arnd
>> Ok, that's not really clear from the documentation or the Kconfig.
>> In the IPMI spec the "host" side is the computer side, not the BMC
>> side.  Like:
>>
>>     11.6.1 BT Host Interface Registers
>>     The Host BT interface provides an independent set of registers and
>>     interrupts to allow the Host driver to
>>     communicate with the baseboard management controller without
>>     conflicting with the O/S ACPI driver.
>>
>> In light of that, this should probably be named the bt-bmc driver.
>>
>> I haven't reviewed this in detail, but I'm ok with putting it in
>> drivers/char/ipmi.  The state machine part looks reasonably
>> generic.  The configuration part isn't, but that could be split
>> out later if necessary.
>>
>> The biggest thing I don't like is the byte at a time interfaces
>> from userspace.  That seems fairly inefficient if the system
>> does extra work for each userspace access.  IIRC some
>> systems do and some don't.
> What about the ioctl to send an SMS ATN event to the host ? Is
> that ok for you ?

Yeah, that's necessary, there's no way the low-level driver can
know when to do that.

I have a similar piece of code in the qemu IPMI emulator, even
the external IPMI emulator has a message it sends to set ATTN.

I did look at similarities between the two pieces of code, but
the qemu one has to be register-read/write driven, so it's
completely different in design.

-corey

> Thanks,
>
> C.

Re: [PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Cédric Le Goater]

On 09/12/2016 10:33 PM, Corey Minyard wrote:
> On 09/12/2016 02:15 PM, Arnd Bergmann wrote:
>> On Monday, September 12, 2016 1:55:40 PM CEST Corey Minyard wrote:
>>> On 09/02/2016 08:22 AM, Cédric Le Goater wrote:
>>>> Hello,
>>>>
>>>> Adding Corey in cc: . I guess I should have done that in the first place.
>>> Yes, probably so.  I've been travelling and didn't see it on the mailing
>>> lists until now.
>>>
>>> There is already a BT driver in the kernel, in drivers/char/ipmi, why
>>> won't that work?
>> The new driver is the host side (running on the BMC), the existing one
>> is the client (running on the PC).
>>
>>     Arnd
> 
> Ok, that's not really clear from the documentation or the Kconfig.
> In the IPMI spec the "host" side is the computer side, not the BMC
> side.  Like:
> 
>    11.6.1 BT Host Interface Registers
>    The Host BT interface provides an independent set of registers and
>    interrupts to allow the Host driver to
>    communicate with the baseboard management controller without
>    conflicting with the O/S ACPI driver.
> 
> In light of that, this should probably be named the bt-bmc driver.
> 
> I haven't reviewed this in detail, but I'm ok with putting it in
> drivers/char/ipmi.  The state machine part looks reasonably
> generic.  The configuration part isn't, but that could be split
> out later if necessary.

what do you mean by configuration ? I am ready to send a v2. May be
I can add a few other things.

Thanks,

C. 

[PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Cédric Le Goater]

On 09/12/2016 10:33 PM, Corey Minyard wrote:
> On 09/12/2016 02:15 PM, Arnd Bergmann wrote:
>> On Monday, September 12, 2016 1:55:40 PM CEST Corey Minyard wrote:
>>> On 09/02/2016 08:22 AM, C?dric Le Goater wrote:
>>>> Hello,
>>>>
>>>> Adding Corey in cc: . I guess I should have done that in the first place.
>>> Yes, probably so.  I've been travelling and didn't see it on the mailing
>>> lists until now.
>>>
>>> There is already a BT driver in the kernel, in drivers/char/ipmi, why
>>> won't that work?
>> The new driver is the host side (running on the BMC), the existing one
>> is the client (running on the PC).
>>
>>     Arnd
> 
> Ok, that's not really clear from the documentation or the Kconfig.
> In the IPMI spec the "host" side is the computer side, not the BMC
> side.  Like:
> 
>    11.6.1 BT Host Interface Registers
>    The Host BT interface provides an independent set of registers and
>    interrupts to allow the Host driver to
>    communicate with the baseboard management controller without
>    conflicting with the O/S ACPI driver.
> 
> In light of that, this should probably be named the bt-bmc driver.
> 
> I haven't reviewed this in detail, but I'm ok with putting it in
> drivers/char/ipmi.  The state machine part looks reasonably
> generic.  The configuration part isn't, but that could be split
> out later if necessary.

what do you mean by configuration ? I am ready to send a v2. May be
I can add a few other things.

Thanks,

C. 

Re: [PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Corey Minyard]

On 09/15/2016 01:51 AM, Cédric Le Goater wrote:
> On 09/12/2016 10:33 PM, Corey Minyard wrote:
>> On 09/12/2016 02:15 PM, Arnd Bergmann wrote:
>>> On Monday, September 12, 2016 1:55:40 PM CEST Corey Minyard wrote:
>>>> On 09/02/2016 08:22 AM, Cédric Le Goater wrote:
>>>>> Hello,
>>>>>
>>>>> Adding Corey in cc: . I guess I should have done that in the first place.
>>>> Yes, probably so.  I've been travelling and didn't see it on the mailing
>>>> lists until now.
>>>>
>>>> There is already a BT driver in the kernel, in drivers/char/ipmi, why
>>>> won't that work?
>>> The new driver is the host side (running on the BMC), the existing one
>>> is the client (running on the PC).
>>>
>>>      Arnd
>> Ok, that's not really clear from the documentation or the Kconfig.
>> In the IPMI spec the "host" side is the computer side, not the BMC
>> side.  Like:
>>
>>     11.6.1 BT Host Interface Registers
>>     The Host BT interface provides an independent set of registers and
>>     interrupts to allow the Host driver to
>>     communicate with the baseboard management controller without
>>     conflicting with the O/S ACPI driver.
>>
>> In light of that, this should probably be named the bt-bmc driver.
>>
>> I haven't reviewed this in detail, but I'm ok with putting it in
>> drivers/char/ipmi.  The state machine part looks reasonably
>> generic.  The configuration part isn't, but that could be split
>> out later if necessary.
> what do you mean by configuration ? I am ready to send a v2. May be
> I can add a few other things.

The part that sets everything up, basically everything from
bt_host_fops down.

If you were to use this on another system, that code would have to be
made more generic and the machine-specific part split into different
files, but that's not necessary now, I don't think.

-corey

> Thanks,
>
> C.


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] misc: Add Aspeed BT IPMI host driver

[Corey Minyard]

On 09/15/2016 01:51 AM, C?dric Le Goater wrote:
> On 09/12/2016 10:33 PM, Corey Minyard wrote:
>> On 09/12/2016 02:15 PM, Arnd Bergmann wrote:
>>> On Monday, September 12, 2016 1:55:40 PM CEST Corey Minyard wrote:
>>>> On 09/02/2016 08:22 AM, C?dric Le Goater wrote:
>>>>> Hello,
>>>>>
>>>>> Adding Corey in cc: . I guess I should have done that in the first place.
>>>> Yes, probably so.  I've been travelling and didn't see it on the mailing
>>>> lists until now.
>>>>
>>>> There is already a BT driver in the kernel, in drivers/char/ipmi, why
>>>> won't that work?
>>> The new driver is the host side (running on the BMC), the existing one
>>> is the client (running on the PC).
>>>
>>>      Arnd
>> Ok, that's not really clear from the documentation or the Kconfig.
>> In the IPMI spec the "host" side is the computer side, not the BMC
>> side.  Like:
>>
>>     11.6.1 BT Host Interface Registers
>>     The Host BT interface provides an independent set of registers and
>>     interrupts to allow the Host driver to
>>     communicate with the baseboard management controller without
>>     conflicting with the O/S ACPI driver.
>>
>> In light of that, this should probably be named the bt-bmc driver.
>>
>> I haven't reviewed this in detail, but I'm ok with putting it in
>> drivers/char/ipmi.  The state machine part looks reasonably
>> generic.  The configuration part isn't, but that could be split
>> out later if necessary.
> what do you mean by configuration ? I am ready to send a v2. May be
> I can add a few other things.

The part that sets everything up, basically everything from
bt_host_fops down.

If you were to use this on another system, that code would have to be
made more generic and the machine-specific part split into different
files, but that's not necessary now, I don't think.

-corey

> Thanks,
>
> C.