Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH 2/2] ACPI / platform: Add ACPI ID for Intel IOSF-SB

65 views
Skip to first unread message

David E. Box

unread,
Nov 22, 2013, 1:10:02 AM11/22/13
to
From: "David E. Box" <david...@linux.intel.com>

Adds ACPI ID for Intel IOSF-SB MailBox device found in BayTrail platforms.

Signed-off-by: David E. Box <david...@linux.intel.com>
---
drivers/acpi/acpi_platform.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
index 1bde127..12aa55c 100644
--- a/drivers/acpi/acpi_platform.c
+++ b/drivers/acpi/acpi_platform.c
@@ -29,7 +29,8 @@ ACPI_MODULE_NAME("platform");
static const struct acpi_device_id acpi_platform_device_ids[] = {

{ "PNP0D40" },
-
+ /* Intel IOSF-SB Message Bus */
+ { "INT33BD" },
{ }
};

--
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

David E. Box

unread,
Nov 22, 2013, 1:10:03 AM11/22/13
to
From: "David E. Box" <david...@linux.intel.com>

Current Intel SOC cores use a MailBox Interface (MBI) to provide access to unit
devices connected to the system fabric. This driver implements the API required
for other kernel drivers to configure the unit registers accessible through this
interface. Drivers that will require access to the MBI include but are not
limited to, RAPL, DPTF, as well as future drivers. Serialized access is handled
by all exported routines with spinlocks.

The API includes 3 functions for access to unit registers:

u32 iosf_mbi_read(u8 port, u8 opcode, u32 offset)
void iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
void iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)

port: indicating the unit being accessed
opcode: the read or write port specific opcode
offset: the register offset within the port
mdr: the register data to be written, or modified
mask: bit locations in mdr to change

Note: GPU code handles access to the GFX unit. Therefore access to that unit
with this driver is disallowed to avoid conflicts.

Signed-off-by: David E. Box <david...@linux.intel.com>
---
arch/x86/Kconfig | 8 ++
arch/x86/include/asm/iosf_mbi.h | 89 ++++++++++++++++++++++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/iosf_mbi.c | 160 +++++++++++++++++++++++++++++++++++++++
4 files changed, 258 insertions(+)
create mode 100644 arch/x86/include/asm/iosf_mbi.h
create mode 100644 arch/x86/kernel/Module.symvers
create mode 100644 arch/x86/kernel/iosf_mbi.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ee2fb9d..4d1b365 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1113,6 +1113,14 @@ config X86_CPUID
with major 203 and minors 0 to 31 for /dev/cpu/0/cpuid to
/dev/cpu/31/cpuid.

+config X86_IOSF_MBI
+ bool "IOSF-SB MailBox Interface access support for Intel SOC's"
+ ---help---
+ This device provides access to the Intel On-Chip System Fabric
+ Sideband MailBox Interface required for configuration of unit
+ devices connected to the fabric. Drivers that require access to the
+ MBI include but are not limited to RAPL and DPTF.
+
choice
prompt "High Memory Support"
default HIGHMEM64G if X86_NUMAQ
diff --git a/arch/x86/include/asm/iosf_mbi.h b/arch/x86/include/asm/iosf_mbi.h
new file mode 100644
index 0000000..861834f
--- /dev/null
+++ b/arch/x86/include/asm/iosf_mbi.h
@@ -0,0 +1,89 @@
+/*
+ * iosf_mbi.h: IOSF MailBox Interface access for Intel SOC's.
+ */
+
+#ifndef INTEL_IOSF_MBI_SYMS_H
+#define INTEL_IOSF_MBI_SYMS_H
+
+#define IOSF_MBI_MCR_OFFSET 0x0
+#define IOSF_MBI_MDR_OFFSET 0x4
+#define IOSF_MBI_MCRX_OFFSET 0x8
+
+#define IOSF_MBI_RD_MASK 0xFEFFFFFF
+#define IOSF_MBI_WR_MASK 0X01000000
+
+#define IOSF_MBI_MASK_HI 0xFFFFFF00
+#define IOSF_MBI_MASK_LO 0x000000FF
+#define IOSF_MBI_ENABLE 0xF0
+
+#define IOSF_MBI_MAX_PORTS 0xFF
+
+/* IOSF-SB unit access methods */
+#define IOSF_MBI_UNIT_AUNIT 0x00
+#define IOSF_MBI_UNIT_SMC 0x01
+#define IOSF_MBI_UNIT_CPU 0x02
+#define IOSF_MBI_UNIT_BUNIT 0x03
+#define IOSF_MBI_UNIT_PMC 0x04
+#define IOSF_MBI_UNIT_GFX 0x06
+#define IOSF_MBI_UNIT_SMI 0x0C
+#define IOSF_MBI_UNIT_USB 0x43
+#define IOSF_MBI_UNIT_SATA 0xA3
+#define IOSF_MBI_UNIT_PCIE 0xA6
+
+/* Read/write opcodes */
+#define IOSF_MBI_AUNIT_READ 0x10
+#define IOSF_MBI_AUNIT_WRITE 0x11
+#define IOSF_MBI_SMC_READ 0x10
+#define IOSF_MBI_SMC_WRITE 0x11
+#define IOSF_MBI_CPU_READ 0x10
+#define IOSF_MBI_CPU_WRITE 0x11
+#define IOSF_MBI_BUNIT_READ 0x10
+#define IOSF_MBI_BUNIT_WRITE 0x11
+#define IOSF_MBI_PMC_READ 0x06
+#define IOSF_MBI_PMC_WRITE 0x07
+#define IOSF_MBI_GFX_READ 0x00
+#define IOSF_MBI_GFX_WRITE 0x01
+#define IOSF_MBI_SMIO_READ 0x06
+#define IOSF_MBI_SMIO_WRITE 0x07
+#define IOSF_MBI_USB_READ 0x06
+#define IOSF_MBI_USB_WRITE 0x07
+#define IOSF_MBI_SATA_READ 0x00
+#define IOSF_MBI_SATA_WRITE 0x01
+#define IOSF_MBI_PCIE_READ 0x00
+#define IOSF_MBI_PCIE_WRITE 0x01
+
+/**
+ * iosf_mbi_read() - IOSF MailBox Interface read command
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ * Return: 32 bit value of mdr register
+ */
+u32 iosf_mbi_read(u8 port, u8 opcode, u32 offset);
+
+/**
+ * iosf_mbi_write() - IOSF MailBox unmasked write command
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ * @mdr: register data to be written
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ */
+void iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr);
+
+/**
+ * iosf_mbi_modify() - IOSF MailBox maksed write command
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ * @mdr: register data being modified
+ * @mask: mask indicating bits in mdr to be modified
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ */
+void iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask);
+
+#endif /* INTEL_IOSF_MBI_SYMS_H */
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index a5408b9..6d1e9ab 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -50,6 +50,7 @@ obj-y += acpi/
obj-y += reboot.o
obj-$(CONFIG_X86_MSR) += msr.o
obj-$(CONFIG_X86_CPUID) += cpuid.o
+obj-$(CONFIG_X86_IOSF_MBI) += iosf_mbi.o
obj-$(CONFIG_PCI) += early-quirks.o
apm-y := apm_32.o
obj-$(CONFIG_APM) += apm.o
diff --git a/arch/x86/kernel/Module.symvers b/arch/x86/kernel/Module.symvers
new file mode 100644
index 0000000..e69de29
diff --git a/arch/x86/kernel/iosf_mbi.c b/arch/x86/kernel/iosf_mbi.c
new file mode 100644
index 0000000..d2e990e
--- /dev/null
+++ b/arch/x86/kernel/iosf_mbi.c
@@ -0,0 +1,160 @@
+/*
+ * IOSF-SB MailBox Interface Driver
+ * Copyright (c) 2013, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ *
+ * The IOSF-SB is a fabric bus available on Atom based SOC's that uses a
+ * mailbox interface (MBI) to communicate with mutiple devices. This
+ * driver implements access to this interface.
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+#include <linux/acpi.h>
+#include <linux/platform_device.h>
+
+#include <asm/iosf_mbi.h>
+
+static DEFINE_SPINLOCK(iosf_mbi_lock);
+
+static inline u32 iosf_mbi_form_mcr(u8 op, u8 port, u8 offset)
+{
+ return (op << 24) | (port << 16) | (offset << 8) | IOSF_MBI_ENABLE;
+}
+
+static struct acpi_device_id iosf_mbi_priv_ids[] = {
+ { "INT33BD", 0},
+ { "", 0},
+};
+MODULE_DEVICE_TABLE(acpi, iosf_mbi_priv_ids);
+
+static struct {
+ void __iomem *addr;
+} iosf_mbi_data;
+
+/* Hold lock before calling */
+static u32 iosf_mbi_read_mdr(u32 mcrx, u32 mcr, void __iomem *addr)
+{
+ if (mcrx)
+ iowrite32(mcrx, addr + IOSF_MBI_MCRX_OFFSET);
+ iowrite32(mcr, addr + IOSF_MBI_MCR_OFFSET);
+ return ioread32(addr + IOSF_MBI_MDR_OFFSET);
+}
+
+/* Hold lock before calling */
+static void iosf_mbi_write_mdr(u32 mcrx, u32 mcr, u32 mdr, void __iomem *addr)
+{
+
+ iowrite32(mdr, addr + IOSF_MBI_MDR_OFFSET);
+ if (mcrx)
+ iowrite32(mcrx, addr + IOSF_MBI_MCRX_OFFSET);
+ iowrite32(mcr, addr + IOSF_MBI_MCR_OFFSET);
+}
+
+u32 iosf_mbi_read(u8 port, u8 opcode, u32 offset)
+{
+ u32 mcr, mcrx;
+ u32 ret;
+ unsigned long flags;
+
+ /*Access to the GFX unit is handled by GPU code */
+ BUG_ON(port == IOSF_MBI_UNIT_GFX);
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & IOSF_MBI_MASK_LO);
+ mcrx = offset & IOSF_MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+ ret = iosf_mbi_read_mdr(mcrx, mcr, iosf_mbi_data.addr);
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+
+ return ret;
+}
+EXPORT_SYMBOL(iosf_mbi_read);
+
+void iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
+{
+ u32 mcr, mcrx;
+ unsigned long flags;
+
+ /*Access to the GFX unit is handled by GPU code */
+ BUG_ON(port == IOSF_MBI_UNIT_GFX);
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & IOSF_MBI_MASK_LO);
+ mcrx = offset & IOSF_MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+ iosf_mbi_write_mdr(mcrx, mcr, mdr, iosf_mbi_data.addr);
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+}
+EXPORT_SYMBOL(iosf_mbi_write);
+
+void iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)
+{
+ u32 mcr, mcrx;
+ u32 value;
+ unsigned long flags;
+
+ /*Access to the GFX unit is handled by GPU code */
+ BUG_ON(port == IOSF_MBI_UNIT_GFX);
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & IOSF_MBI_MASK_LO);
+ mcrx = offset & IOSF_MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+
+ /* Read current mdr value */
+ value = iosf_mbi_read_mdr(mcrx, mcr & IOSF_MBI_RD_MASK,
+ iosf_mbi_data.addr);
+
+ /* Apply mask */
+ value &= ~mask;
+ mdr &= mask;
+ value |= mdr;
+
+ /* Write back */
+ iosf_mbi_write_mdr(mcrx, mcr | IOSF_MBI_WR_MASK, value,
+ iosf_mbi_data.addr);
+
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+}
+EXPORT_SYMBOL(iosf_mbi_modify);
+
+static int iosf_mbi_probe(struct platform_device *pdev)
+{
+ struct resource *mem;
+
+ /* Get and map MBI address space */
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ iosf_mbi_data.addr = devm_ioremap_resource(&pdev->dev, mem);
+ if (IS_ERR(iosf_mbi_data.addr))
+ return PTR_ERR(iosf_mbi_data.addr);
+
+ return 0;
+}
+
+static struct platform_driver iosf_mbi_driver = {
+ .driver = {
+ .name = "iosf_mbi",
+ .owner = THIS_MODULE,
+ .acpi_match_table = ACPI_PTR(iosf_mbi_priv_ids),
+ },
+ .probe = iosf_mbi_probe,
+};
+
+module_platform_driver(iosf_mbi_driver);
+
+MODULE_AUTHOR("David Box");
+MODULE_DESCRIPTION("IOSF-SB Message Bus Interface accessor");
+MODULE_LICENSE("GPL v2");

Matthew Garrett

unread,
Nov 22, 2013, 1:20:01 PM11/22/13
to
On Thu, Nov 21, 2013 at 10:05:56PM -0800, David E. Box wrote:

> + bool "IOSF-SB MailBox Interface access support for Intel SOC's"

Any reason this can't be modular?

> diff --git a/arch/x86/kernel/iosf_mbi.c b/arch/x86/kernel/iosf_mbi.c

This is effectively a driver, so putting it under platform/driver/x86
might make more sense.

--
Matthew Garrett | mj...@srcf.ucam.org

One Thousand Gnomes

unread,
Nov 23, 2013, 7:50:02 PM11/23/13
to
On Thu, 21 Nov 2013 22:05:56 -0800
"David E. Box" <david...@linux.intel.com> wrote:

> +static int iosf_mbi_probe(struct platform_device *pdev)
> +{
> + struct resource *mem;
> +
> + /* Get and map MBI address space */
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + iosf_mbi_data.addr = devm_ioremap_resource(&pdev->dev, mem);

Purely "defensive driv(er)ing" but platform_get_resource can return NULL

Alan

David E. Box

unread,
Dec 3, 2013, 7:10:01 PM12/3/13
to
From: "David E. Box" <david...@linux.intel.com>

Adds ACPI ID for Intel IOSF-SB MailBox device found in BayTrail platforms.

Signed-off-by: David E. Box <david...@linux.intel.com>
---
drivers/acpi/acpi_platform.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
index 1bde127..12aa55c 100644
--- a/drivers/acpi/acpi_platform.c
+++ b/drivers/acpi/acpi_platform.c
@@ -29,7 +29,8 @@ ACPI_MODULE_NAME("platform");
static const struct acpi_device_id acpi_platform_device_ids[] = {

{ "PNP0D40" },
-
+ /* Intel IOSF-SB Message Bus */
+ { "INT33BD" },
{ }
};

David E. Box

unread,
Dec 3, 2013, 7:10:02 PM12/3/13
to
From: "David E. Box" <david...@linux.intel.com>

David E. Box (2):
New Driver for IOSF-SB MBI access on Intel SOCs
ACPI/platform: Add ACPI ID for Intel MBI device

v2: Made modular as reasons for built-in only no longer a concern.
Moved to x86 platform driver, as suggested by Matthew Garrett <mj...@srcf.ucam.org>
Changed initialization sequence to do probe during init, thereby forcing an
exit if the device does not exist. Module exit does not otherwise occur
if module_platform_driver() is used and the device doesn't exist.

drivers/acpi/acpi_platform.c | 3 +-
drivers/platform/x86/Kconfig | 8 ++
drivers/platform/x86/Makefile | 2 +
drivers/platform/x86/iosf_mbi.c | 188 +++++++++++++++++++++++++++++++++++++++
drivers/platform/x86/iosf_mbi.h | 89 ++++++++++++++++++
5 files changed, 289 insertions(+), 1 deletion(-)
create mode 100644 drivers/platform/x86/iosf_mbi.c
create mode 100644 drivers/platform/x86/iosf_mbi.h

David E. Box

unread,
Dec 3, 2013, 7:10:01 PM12/3/13
to
From: "David E. Box" <david...@linux.intel.com>

Current Intel SOC cores use a MailBox Interface (MBI) to provide access to unit
devices connected to the system fabric. This driver implements the API required
for other kernel drivers to configure the unit registers accessible through this
interface. Drivers requiring access to the MBI include RAPL as well as future
drivers. Serialized access is handled by all exported routines with spinlocks.

The API includes 3 functions for access to unit registers:

u32 iosf_mbi_read(u8 port, u8 opcode, u32 offset)
void iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
void iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)

port: indicating the unit being accessed
opcode: the read or write port specific opcode
offset: the register offset within the port
mdr: the register data to be written, or modified
mask: bit locations in mdr to change

Note: GPU code handles access to the GFX unit. Therefore access to that unit
with this driver is disallowed to avoid conflicts.

Signed-off-by: David E. Box <david...@linux.intel.com>
---
drivers/platform/x86/Kconfig | 8 ++
drivers/platform/x86/Makefile | 2 +
drivers/platform/x86/iosf_mbi.c | 188 +++++++++++++++++++++++++++++++++++++++
drivers/platform/x86/iosf_mbi.h | 89 ++++++++++++++++++
4 files changed, 287 insertions(+)
create mode 100644 drivers/platform/x86/iosf_mbi.c
create mode 100644 drivers/platform/x86/iosf_mbi.h

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index b51a746..3124bc4 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -819,4 +819,12 @@ config PVPANIC
a paravirtualized device provided by QEMU; it lets a virtual machine
(guest) communicate panic events to the host.

+config X86_IOSF_MBI
+ tristate "IOSF-SB MailBox Interface access support for Intel SOCs"
+ depends on ACPI
+ ---help---
+ This device provides access to the Intel On-Chip System Fabric
+ Sideband MailBox Interface which required for the configuration of
+ unit devices that are connected to the fabric.
+
endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 5dbe193..ee93755 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -55,3 +55,5 @@ obj-$(CONFIG_INTEL_RST) += intel-rst.o
obj-$(CONFIG_INTEL_SMARTCONNECT) += intel-smartconnect.o

obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-$(CONFIG_X86_IOSF_MBI) += iosf_mbi.o
+
diff --git a/drivers/platform/x86/iosf_mbi.c b/drivers/platform/x86/iosf_mbi.c
new file mode 100644
index 0000000..7eae6c4
--- /dev/null
+++ b/drivers/platform/x86/iosf_mbi.c
@@ -0,0 +1,188 @@
+#include "iosf_mbi.h"
+ if (!mem)
+ return -ENOMEM;
+
+ iosf_mbi_data.addr = devm_ioremap_resource(&pdev->dev, mem);
+ if (IS_ERR(iosf_mbi_data.addr))
+ return PTR_ERR(iosf_mbi_data.addr);
+
+ return 0;
+}
+
+static int iosf_mbi_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static struct platform_driver iosf_mbi_driver = {
+ .driver = {
+ .name = "iosf_mbi",
+ .owner = THIS_MODULE,
+ .acpi_match_table = ACPI_PTR(iosf_mbi_priv_ids),
+ },
+ .probe = iosf_mbi_probe,
+ .remove = iosf_mbi_remove,
+};
+
+static int __init iosf_mbi_init(void)
+{
+ int ret = 0;
+
+ ret = platform_driver_probe(&iosf_mbi_driver, iosf_mbi_probe);
+ if (ret) {
+ pr_err("%s: platform driver registration fail\n", __func__);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void __exit iosf_mbi_exit(void)
+{
+ platform_driver_unregister(&iosf_mbi_driver);
+}
+
+module_init(iosf_mbi_init);
+module_exit(iosf_mbi_exit);
+
+MODULE_AUTHOR("David Box");
+MODULE_DESCRIPTION("IOSF-SB Message Bus Interface accessor");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/platform/x86/iosf_mbi.h b/drivers/platform/x86/iosf_mbi.h
new file mode 100644
index 0000000..861834f
--- /dev/null
+++ b/drivers/platform/x86/iosf_mbi.h

Matthew Garrett

unread,
Dec 3, 2013, 8:40:03 PM12/3/13
to
On Tue, Dec 03, 2013 at 03:59:38PM -0800, David E. Box wrote:
> From: "David E. Box" <david...@linux.intel.com>
>
> Adds ACPI ID for Intel IOSF-SB MailBox device found in BayTrail platforms.

Little bit confused here. This is a new driver and only declares
modaliases for the ACPI IDs - why does it need to be added here?

--
Matthew Garrett | mj...@srcf.ucam.org

David E. Box

unread,
Dec 3, 2013, 9:20:02 PM12/3/13
to
On Wed, Dec 04, 2013 at 01:30:01AM +0000, Matthew Garrett wrote:
> On Tue, Dec 03, 2013 at 03:59:38PM -0800, David E. Box wrote:
> > From: "David E. Box" <david...@linux.intel.com>
> >
> > Adds ACPI ID for Intel IOSF-SB MailBox device found in BayTrail platforms.
>
> Little bit confused here. This is a new driver and only declares
> modaliases for the ACPI IDs - why does it need to be added here?
>

This is per the requirement in Documentation/acpi/enumeration.txt:

"Currently the kernel is not able to automatically determine from which ACPI
device it should make the corresponding platform device so we need to add
the ACPI device explicitly to acpi_platform_device_ids list defined in
drivers/acpi/acpi_platform.c"

Without adding the device here it would not be discovered and probe would not be
called during init.

David Box

Matthew Garrett

unread,
Dec 3, 2013, 9:30:01 PM12/3/13
to
On Tue, Dec 03, 2013 at 06:17:03PM -0800, David E. Box wrote:
> This is per the requirement in Documentation/acpi/enumeration.txt:
>
> "Currently the kernel is not able to automatically determine from which ACPI
> device it should make the corresponding platform device so we need to add
> the ACPI device explicitly to acpi_platform_device_ids list defined in
> drivers/acpi/acpi_platform.c"

Well sure, but why do you need to be a platform device at all? This
functionality was intended for cases where we already have a driver for
the part that enumerated it via some other mechanism. If the driver's
only intended for ACPI systems then why not just be an ACPI device?

David E. Box

unread,
Dec 3, 2013, 9:50:01 PM12/3/13
to
On Wed, Dec 04, 2013 at 02:21:30AM +0000, Matthew Garrett wrote:
> On Tue, Dec 03, 2013 at 06:17:03PM -0800, David E. Box wrote:
> > This is per the requirement in Documentation/acpi/enumeration.txt:
> >
> > "Currently the kernel is not able to automatically determine from which ACPI
> > device it should make the corresponding platform device so we need to add
> > the ACPI device explicitly to acpi_platform_device_ids list defined in
> > drivers/acpi/acpi_platform.c"
>
> Well sure, but why do you need to be a platform device at all? This
> functionality was intended for cases where we already have a driver for
> the part that enumerated it via some other mechanism. If the driver's
> only intended for ACPI systems then why not just be an ACPI device?
>

It was my understanding that with ACPI 5.0 it was becoming more common to use
ACPI ID's exclusively for device enumeration. I originally wrote this as an
acpi_bus driver but Rafeal advised me that the model is being phased out and
suggeted the platform model instead.

Matthew Garrett

unread,
Dec 3, 2013, 10:00:01 PM12/3/13
to
On Tue, Dec 03, 2013 at 06:44:52PM -0800, David E. Box wrote:
> On Wed, Dec 04, 2013 at 02:21:30AM +0000, Matthew Garrett wrote:
> > Well sure, but why do you need to be a platform device at all? This
> > functionality was intended for cases where we already have a driver for
> > the part that enumerated it via some other mechanism. If the driver's
> > only intended for ACPI systems then why not just be an ACPI device?
> >
>
> It was my understanding that with ACPI 5.0 it was becoming more common to use
> ACPI ID's exclusively for device enumeration. I originally wrote this as an
> acpi_bus driver but Rafeal advised me that the model is being phased out and
> suggeted the platform model instead.

If you're not adding ACPI support to an existing platform driver, you
shouldn't be adding entries to acpi_platform.c. I'm not actually happy
that I merged the ideapad-laptop patch that did the same thing - I'm
inclined to revert it, because this really is an ugly way to do things.

Rafael, why did we convert the AC driver this way? It means we have to
keep track of ACPI IDs in multiple places, which is worth it when it
avoids having to write a pile of new code (such as the sdhci case) but
doesn't seem to provide benefits otherwise.

Andi Kleen

unread,
Dec 4, 2013, 1:50:01 AM12/4/13
to
"David E. Box" <david...@linux.intel.com> writes:
>
> +config X86_IOSF_MBI
> + tristate "IOSF-SB MailBox Interface access support for Intel
> SOCs"

This is only implicitly used by other drivers, right?

Please make it not user visible (drop the string after tristate), as users
will not know when to enable it.

The other drivers using it should always select it instead.

-Andi

--
a...@linux.intel.com -- Speaking for myself only

Rafael J. Wysocki

unread,
Dec 4, 2013, 4:30:03 PM12/4/13
to
On Wednesday, December 04, 2013 02:54:07 AM Matthew Garrett wrote:
> On Tue, Dec 03, 2013 at 06:44:52PM -0800, David E. Box wrote:
> > On Wed, Dec 04, 2013 at 02:21:30AM +0000, Matthew Garrett wrote:
> > > Well sure, but why do you need to be a platform device at all? This
> > > functionality was intended for cases where we already have a driver for
> > > the part that enumerated it via some other mechanism. If the driver's
> > > only intended for ACPI systems then why not just be an ACPI device?

Because struct acpi_device things are firmawre objects that shouldn't be
treated as devices.

> >
> > It was my understanding that with ACPI 5.0 it was becoming more common to use
> > ACPI ID's exclusively for device enumeration. I originally wrote this as an
> > acpi_bus driver but Rafeal advised me that the model is being phased out and
> > suggeted the platform model instead.
>
> If you're not adding ACPI support to an existing platform driver, you
> shouldn't be adding entries to acpi_platform.c. I'm not actually happy
> that I merged the ideapad-laptop patch that did the same thing - I'm
> inclined to revert it, because this really is an ugly way to do things.
>
> Rafael, why did we convert the AC driver this way? It means we have to
> keep track of ACPI IDs in multiple places,

Yes, in two places to be precise.

> which is worth it when it avoids having to write a pile of new code (such as
> the sdhci case) but doesn't seem to provide benefits otherwise.

Well, I thought we discussed that, but perhaps we didn't.

There's some confusion about what a struct acpi_device is, because sometimes it
is treated as a "companion" of a physical device and sometimes it is treated as
a physical device itself. This leads to incorrect code as well sometimes,
because ACPI drivers may attempt to bind to things that already have physical
comapnions, for example.

The approach to this I thought we could use, and that we've been using to some
extent already, is to create "physical" companions for all struct acpi_device
objects. In this particular this "physical companion" is a platform device, so
it needs to be added to the list in acpi_platform.c. [Otherwise the ACPI PNP
core would create a PNP device for it (unless an ACPI driver is bound to that
struct acpi_device before, but that makes the correctness of the thing depend
on the ordering of driver probing, which is not awesome).]

In this approach device drivers bind to the "physical" devices (platform devices
etc.) instead of struct acpi_device themselves and at least everything is
consistent.

Thanks,
Rafael

David E. Box

unread,
Dec 5, 2013, 3:10:02 PM12/5/13
to
From: "David E. Box" <david...@linux.intel.com>

Current Intel SOC cores use a MailBox Interface (MBI) to provide access to unit
devices connected to the system fabric. This driver implements the API required
for other kernel drivers to configure the unit registers accessible through this
interface. Drivers requiring access to the MBI include RAPL as well as future
drivers. Serialized access is handled by all exported routines with spinlocks.

The API includes 3 functions for access to unit registers:

u32 iosf_mbi_read(u8 port, u8 opcode, u32 offset)
void iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
void iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)

port: indicating the unit being accessed
opcode: the read or write port specific opcode
offset: the register offset within the port
mdr: the register data to be written, or modified
mask: bit locations in mdr to change

Note: GPU code handles access to the GFX unit. Therefore access to that unit
with this driver is disallowed to avoid conflicts.

Signed-off-by: David E. Box <david...@linux.intel.com>
---
v3: Converted to PNP ACPI driver as sugessted by Rafael Wysocki <r...@rjwysocki.net>
Removed config visibility to user as suggested by Andi Kleen <an...@firstfloor.org>

v2: Made modular since there was no longer a reason not to
Moved to x86 platform as suggested by Mathhew Garrett <mj...@srcf.ucam.org>
Added probe to init to cause driver load to fail if device not detected.

drivers/platform/x86/Kconfig | 9 ++
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/iosf_mbi.c | 188 +++++++++++++++++++++++++++++++++++++++
drivers/platform/x86/iosf_mbi.h | 89 ++++++++++++++++++
4 files changed, 287 insertions(+)
create mode 100644 drivers/platform/x86/iosf_mbi.c
create mode 100644 drivers/platform/x86/iosf_mbi.h

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index b51a746..ba6c316 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -819,4 +819,13 @@ config PVPANIC
a paravirtualized device provided by QEMU; it lets a virtual machine
(guest) communicate panic events to the host.

+config INTEL_IOSF_MBI
+ tristate
+ depends on PNP
+ depends on ACPI
+ ---help---
+ This device provides access to the Intel On-Chip System Fabric
+ Sideband MailBox Interface which required for the configuration of
+ some unit devices connected on the fabric.
+
endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 5dbe193..9b8e8b8 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -55,3 +55,4 @@ obj-$(CONFIG_INTEL_RST) += intel-rst.o
obj-$(CONFIG_INTEL_SMARTCONNECT) += intel-smartconnect.o

obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-$(CONFIG_INTEL_IOSF_MBI) += iosf_mbi.o
diff --git a/drivers/platform/x86/iosf_mbi.c b/drivers/platform/x86/iosf_mbi.c
new file mode 100644
index 0000000..7936812
+#include <linux/pnp.h>
+
+#include "iosf_mbi.h"
+
+static DEFINE_SPINLOCK(iosf_mbi_lock);
+
+static inline u32 iosf_mbi_form_mcr(u8 op, u8 port, u8 offset)
+{
+ return (op << 24) | (port << 16) | (offset << 8) | IOSF_MBI_ENABLE;
+}
+
+static struct {
+ void __iomem *addr;
+ bool probed;
+static int iosf_mbi_pnp_probe(struct pnp_dev *pnp,
+ const struct pnp_device_id *dev_id)
+{
+ struct resource *mem;
+
+ /* Get and map MBI address space */
+ mem = pnp_get_resource(pnp, IORESOURCE_MEM, 0);
+ if (!mem)
+ return -ENOMEM;
+
+ iosf_mbi_data.addr = devm_ioremap_resource(&pnp->dev, mem);
+ if (IS_ERR(iosf_mbi_data.addr))
+ return PTR_ERR(iosf_mbi_data.addr);
+
+ iosf_mbi_data.probed = true;
+ return 0;
+}
+
+static void iosf_mbi_pnp_remove(struct pnp_dev *pdev)
+{
+ return;
+}
+
+static const struct pnp_device_id iosf_mbi_dev_table[] = {
+ { "INT33BD", 0},
+ { "", 0},
+};
+MODULE_DEVICE_TABLE(pnp, iosf_mbi_dev_table);
+
+static struct pnp_driver iosf_mbi_pnp_driver = {
+ .name = "iosf_mbi",
+ .probe = iosf_mbi_pnp_probe,
+ .remove = iosf_mbi_pnp_remove,
+ .id_table = iosf_mbi_dev_table,
+};
+
+static int __init iosf_mbi_init(void)
+{
+ int ret;
+
+ iosf_mbi_data.probed = false;
+
+ ret = pnp_register_driver(&iosf_mbi_pnp_driver);
+ if (!ret && !iosf_mbi_data.probed) {
+ pnp_unregister_driver(&iosf_mbi_pnp_driver);
+ return -ENODEV;
+ }
+
+ return ret;
+}
+
+static void __exit iosf_mbi_exit(void)
+{
+ pnp_unregister_driver(&iosf_mbi_pnp_driver);

Rafael J. Wysocki

unread,
Dec 5, 2013, 5:20:02 PM12/5/13
to
You may as well write that as

depends on PNPACPI

Apart from this minor nit the patch looks OK to me.

Thanks!
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

David E. Box

unread,
Dec 6, 2013, 4:10:03 PM12/6/13
to
From: "David E. Box" <david...@linux.intel.com>

Current Intel SOC cores use a MailBox Interface (MBI) to provide access to unit
devices connected to the system fabric. This driver implements access to this
interface on BayTrail platforms. This is a requirement for drivers that need
access to unit registers on the platform (e.g. accessing the PUNIT for power
management features such as RAPL). Serialized access is handled by all exported
routines with spinlocks.

The API includes 3 functions for access to unit registers:

u32 bt_mbi_read(u8 port, u8 opcode, u32 offset)
void bt_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
void bt_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)

port: indicating the unit being accessed
opcode: the read or write port specific opcode
offset: the register offset within the port
mdr: the register data to be written, or modified
mask: bit locations in mdr to change

Note: GPU code handles access to the GFX unit. Therefore access to that unit
with this driver is disallowed to avoid conflicts.

Signed-off-by: David E. Box <david...@linux.intel.com>
---
v4: Define driver as platform specific to BayTrail as some platforms cannot
enumerate the MBI using ACPI as noted by Bin Gao <bin...@linux.intel.com>
Renamed register macros and API funcitons to platform specific names.
Changed dependency to PNPACPI as sugessted by Rafael Wysocki <r...@rjwysocki.net>

v3: Converted to PNP ACPI driver as sugessted by Rafael Wysocki <r...@rjwysocki.net>
Removed config visibility to user as suggested by Andi Kleen <an...@firstfloor.org>

v2: Made modular since there was no longer a reason not to
Moved to x86 platform as suggested by Mathhew Garrett <mj...@srcf.ucam.org>
Added probe to init to cause driver load to fail if device not detected.

drivers/platform/x86/Kconfig | 8 ++
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/intel_baytrail.c | 188 +++++++++++++++++++++++++++++++++
drivers/platform/x86/intel_baytrail.h | 89 ++++++++++++++++
4 files changed, 286 insertions(+)
create mode 100644 drivers/platform/x86/intel_baytrail.c
create mode 100644 drivers/platform/x86/intel_baytrail.h

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index b51a746..830f915 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -819,4 +819,12 @@ config PVPANIC
a paravirtualized device provided by QEMU; it lets a virtual machine
(guest) communicate panic events to the host.

+config INTEL_BAYTRAIL_MBI
+ tristate
+ depends on PNPACPI
+ ---help---
+ Needed on Baytrail platforms for access to the IOSF Sideband Mailbox
+ Interface. This is a requirement for systems that need to configure
+ the PUNIT for power management features such as RAPL.
+
endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 5dbe193..b3d4cfd 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -55,3 +55,4 @@ obj-$(CONFIG_INTEL_RST) += intel-rst.o
obj-$(CONFIG_INTEL_SMARTCONNECT) += intel-smartconnect.o

obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-$(CONFIG_INTEL_BAYTRAIL_MBI) += intel_baytrail.o
diff --git a/drivers/platform/x86/intel_baytrail.c b/drivers/platform/x86/intel_baytrail.c
new file mode 100644
index 0000000..265ab39
--- /dev/null
+++ b/drivers/platform/x86/intel_baytrail.c
@@ -0,0 +1,188 @@
+/*
+ * Baytrail IOSF-SB MailBox Interface Driver
+ * Copyright (c) 2013, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ *
+ * The IOSF-SB is a fabric bus available on Atom based SOC's that uses a
+ * mailbox interface (MBI) to communicate with mutiple devices. This
+ * driver implements BayTrail-specific access to this interface.
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/spinlock.h>
+#include <linux/pnp.h>
+
+#include "intel_baytrail.h"
+
+static DEFINE_SPINLOCK(iosf_mbi_lock);
+
+static inline u32 iosf_mbi_form_mcr(u8 op, u8 port, u8 offset)
+{
+ return (op << 24) | (port << 16) | (offset << 8) | BT_MBI_ENABLE;
+}
+
+static struct {
+ void __iomem *addr;
+ bool probed;
+} iosf_mbi_data;
+
+/* Hold lock before calling */
+static u32 iosf_mbi_read_mdr(u32 mcrx, u32 mcr, void __iomem *addr)
+{
+ if (mcrx)
+ iowrite32(mcrx, addr + BT_MBI_MCRX_OFFSET);
+ iowrite32(mcr, addr + BT_MBI_MCR_OFFSET);
+ return ioread32(addr + BT_MBI_MDR_OFFSET);
+}
+
+/* Hold lock before calling */
+static void iosf_mbi_write_mdr(u32 mcrx, u32 mcr, u32 mdr, void __iomem *addr)
+{
+
+ iowrite32(mdr, addr + BT_MBI_MDR_OFFSET);
+ if (mcrx)
+ iowrite32(mcrx, addr + BT_MBI_MCRX_OFFSET);
+ iowrite32(mcr, addr + BT_MBI_MCR_OFFSET);
+}
+
+u32 bt_mbi_read(u8 port, u8 opcode, u32 offset)
+{
+ u32 mcr, mcrx;
+ u32 ret;
+ unsigned long flags;
+
+ /*Access to the GFX unit is handled by GPU code */
+ BUG_ON(port == BT_MBI_UNIT_GFX);
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & BT_MBI_MASK_LO);
+ mcrx = offset & BT_MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+ ret = iosf_mbi_read_mdr(mcrx, mcr, iosf_mbi_data.addr);
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+
+ return ret;
+}
+EXPORT_SYMBOL(bt_mbi_read);
+
+void bt_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
+{
+ u32 mcr, mcrx;
+ unsigned long flags;
+
+ /*Access to the GFX unit is handled by GPU code */
+ BUG_ON(port == BT_MBI_UNIT_GFX);
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & BT_MBI_MASK_LO);
+ mcrx = offset & BT_MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+ iosf_mbi_write_mdr(mcrx, mcr, mdr, iosf_mbi_data.addr);
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+}
+EXPORT_SYMBOL(bt_mbi_write);
+
+void bt_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)
+{
+ u32 mcr, mcrx;
+ u32 value;
+ unsigned long flags;
+
+ /*Access to the GFX unit is handled by GPU code */
+ BUG_ON(port == BT_MBI_UNIT_GFX);
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & BT_MBI_MASK_LO);
+ mcrx = offset & BT_MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+
+ /* Read current mdr value */
+ value = iosf_mbi_read_mdr(mcrx, mcr & BT_MBI_RD_MASK,
+ iosf_mbi_data.addr);
+
+ /* Apply mask */
+ value &= ~mask;
+ mdr &= mask;
+ value |= mdr;
+
+ /* Write back */
+ iosf_mbi_write_mdr(mcrx, mcr | BT_MBI_WR_MASK, value,
+ iosf_mbi_data.addr);
+
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+}
+EXPORT_SYMBOL(bt_mbi_modify);
+ .name = "bt_iosf_mbi",
+MODULE_AUTHOR("David E. Box <david...@linux.intel.com>");
+MODULE_DESCRIPTION("BayTrail Mailbox Interface accessor");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/platform/x86/intel_baytrail.h b/drivers/platform/x86/intel_baytrail.h
new file mode 100644
index 0000000..dac3de6
--- /dev/null
+++ b/drivers/platform/x86/intel_baytrail.h
@@ -0,0 +1,89 @@
+/*
+ * intel_baytrail.h: MailBox access support for Intel BayTrail platforms
+ */
+
+#ifndef INTEL_BAYTRAIL_MBI_SYMS_H
+#define INTEL_BAYTRAIL_MBI_SYMS_H
+
+#define BT_MBI_MCR_OFFSET 0x0
+#define BT_MBI_MDR_OFFSET 0x4
+#define BT_MBI_MCRX_OFFSET 0x8
+
+#define BT_MBI_RD_MASK 0xFEFFFFFF
+#define BT_MBI_WR_MASK 0X01000000
+
+#define BT_MBI_MASK_HI 0xFFFFFF00
+#define BT_MBI_MASK_LO 0x000000FF
+#define BT_MBI_ENABLE 0xF0
+
+#define BT_MBI_MAX_PORTS 0xFF
+
+/* BT-SB unit access methods */
+#define BT_MBI_UNIT_AUNIT 0x00
+#define BT_MBI_UNIT_SMC 0x01
+#define BT_MBI_UNIT_CPU 0x02
+#define BT_MBI_UNIT_BUNIT 0x03
+#define BT_MBI_UNIT_PMC 0x04
+#define BT_MBI_UNIT_GFX 0x06
+#define BT_MBI_UNIT_SMI 0x0C
+#define BT_MBI_UNIT_USB 0x43
+#define BT_MBI_UNIT_SATA 0xA3
+#define BT_MBI_UNIT_PCIE 0xA6
+
+/* Read/write opcodes */
+#define BT_MBI_AUNIT_READ 0x10
+#define BT_MBI_AUNIT_WRITE 0x11
+#define BT_MBI_SMC_READ 0x10
+#define BT_MBI_SMC_WRITE 0x11
+#define BT_MBI_CPU_READ 0x10
+#define BT_MBI_CPU_WRITE 0x11
+#define BT_MBI_BUNIT_READ 0x10
+#define BT_MBI_BUNIT_WRITE 0x11
+#define BT_MBI_PMC_READ 0x06
+#define BT_MBI_PMC_WRITE 0x07
+#define BT_MBI_GFX_READ 0x00
+#define BT_MBI_GFX_WRITE 0x01
+#define BT_MBI_SMIO_READ 0x06
+#define BT_MBI_SMIO_WRITE 0x07
+#define BT_MBI_USB_READ 0x06
+#define BT_MBI_USB_WRITE 0x07
+#define BT_MBI_SATA_READ 0x00
+#define BT_MBI_SATA_WRITE 0x01
+#define BT_MBI_PCIE_READ 0x00
+#define BT_MBI_PCIE_WRITE 0x01
+
+/**
+ * bt_mbi_read() - MailBox Interface read command
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ * Return: 32 bit value of mdr register
+ */
+u32 bt_mbi_read(u8 port, u8 opcode, u32 offset);
+
+/**
+ * bt_mbi_write() - MailBox unmasked write command
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ * @mdr: register data to be written
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ */
+void bt_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr);
+
+/**
+ * bt_mbi_modify() - MailBox masked write command
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ * @mdr: register data being modified
+ * @mask: mask indicating bits in mdr to be modified
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ */
+void bt_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask);
+
+#endif /* INTEL_BAYTRAIL_MBI_SYMS_H */

Rafael J. Wysocki

unread,
Dec 6, 2013, 8:20:01 PM12/6/13
to
On Friday, December 06, 2013 12:59:33 PM David E. Box wrote:
> From: "David E. Box" <david...@linux.intel.com>
>
> Current Intel SOC cores use a MailBox Interface (MBI) to provide access to unit
> devices connected to the system fabric. This driver implements access to this
> interface on BayTrail platforms. This is a requirement for drivers that need
> access to unit registers on the platform (e.g. accessing the PUNIT for power
> management features such as RAPL). Serialized access is handled by all exported
> routines with spinlocks.
>
> The API includes 3 functions for access to unit registers:
>
> u32 bt_mbi_read(u8 port, u8 opcode, u32 offset)
> void bt_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
> void bt_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)
>
> port: indicating the unit being accessed
> opcode: the read or write port specific opcode
> offset: the register offset within the port
> mdr: the register data to be written, or modified
> mask: bit locations in mdr to change
>
> Note: GPU code handles access to the GFX unit. Therefore access to that unit
> with this driver is disallowed to avoid conflicts.
>
> Signed-off-by: David E. Box <david...@linux.intel.com>

Matthew, if you're going to take this:

Acked-by: Rafael J. Wysocki <rafael.j...@intel.com>

Or if you prefer me to take it, please let me know (unless you have objections
against the patch, of course).

Thanks!
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

David E. Box

unread,
Dec 9, 2013, 8:20:01 PM12/9/13
to
Matthre are you okay with the driver?

Dave
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in

David E. Box

unread,
Dec 19, 2013, 5:40:01 PM12/19/13
to
From: "David E. Box" <david...@linux.intel.com>

Current Intel SOC cores use a MailBox Interface (MBI) to provide access to unit
devices connected to the system fabric. This driver implements access to this
interface on BayTrail platforms. This is a requirement for drivers that need
access to unit registers on the platform (e.g. accessing the PUNIT for power
management features such as RAPL). Serialized access is handled by all exported
routines with spinlocks.

The API includes 3 functions for access to unit registers:

int bt_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr)
int bt_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
int bt_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)

port: indicating the unit being accessed
opcode: the read or write port specific opcode
offset: the register offset within the port
mdr: the register data to be read, written, or modified
mask: bit locations in mdr to change

Returns nonzero on error

Note: GPU code handles access to the GFX unit. Therefore access to that unit
with this driver is disallowed to avoid conflicts.

Signed-off-by: David E. Box <david...@linux.intel.com>
---
v5: Converted from pnpacpi/mmio driver to pci/config_space driver as
necessitated by firmware change.

v4: Define driver as platform specific to BayTrail as some platforms cannot
enumerate the MBI using ACPI as noted by Bin Gao <bin...@linux.intel.com>
Renamed register macros and API funcitons to platform specific names.
Changed dependency to PNPACPI as sugessted by Rafael Wysocki <r...@rjwysocki.net>

v3: Converted to PNP ACPI driver as sugessted by Rafael Wysocki <r...@rjwysocki.net>
Removed config visibility to user as suggested by Andi Kleen <an...@firstfloor.org>

v2: Made modular since there was no longer a reason not to
Moved to x86 platform as suggested by Mathhew Garrett <mj...@srcf.ucam.org>
Added probe to init to cause driver load to fail if device not detected.

drivers/platform/x86/Kconfig | 8 ++
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/intel_baytrail.c | 224 +++++++++++++++++++++++++++++++++
drivers/platform/x86/intel_baytrail.h | 90 +++++++++++++
4 files changed, 323 insertions(+)
create mode 100644 drivers/platform/x86/intel_baytrail.c
create mode 100644 drivers/platform/x86/intel_baytrail.h

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index b51a746..b482e22 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -819,4 +819,12 @@ config PVPANIC
a paravirtualized device provided by QEMU; it lets a virtual machine
(guest) communicate panic events to the host.

+config INTEL_BAYTRAIL_MBI
+ tristate
+ depends on PCI
+ ---help---
+ Needed on Baytrail platforms for access to the IOSF Sideband Mailbox
+ Interface. This is a requirement for systems that need to configure
+ the PUNIT for power management features such as RAPL.
+
endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 5dbe193..b3d4cfd 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -55,3 +55,4 @@ obj-$(CONFIG_INTEL_RST) += intel-rst.o
obj-$(CONFIG_INTEL_SMARTCONNECT) += intel-smartconnect.o

obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-$(CONFIG_INTEL_BAYTRAIL_MBI) += intel_baytrail.o
diff --git a/drivers/platform/x86/intel_baytrail.c b/drivers/platform/x86/intel_baytrail.c
new file mode 100644
index 0000000..f96626b
--- /dev/null
+++ b/drivers/platform/x86/intel_baytrail.c
@@ -0,0 +1,224 @@
+/*
+ * Baytrail IOSF-SB MailBox Interface Driver
+ * Copyright (c) 2013, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ *
+ * The IOSF-SB is a fabric bus available on Atom based SOC's that uses a
+ * mailbox interface (MBI) to communicate with mutiple devices. This
+ * driver implements BayTrail-specific access to this interface.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/spinlock.h>
+#include <linux/pci.h>
+
+#include "intel_baytrail.h"
+
+static DEFINE_SPINLOCK(iosf_mbi_lock);
+
+static inline u32 iosf_mbi_form_mcr(u8 op, u8 port, u8 offset)
+{
+ return (op << 24) | (port << 16) | (offset << 8) | BT_MBI_ENABLE;
+}
+
+static struct pci_dev *mbi_pdev; /* one mbi device */
+
+/* Hold lock before calling */
+static int iosf_mbi_pci_read_mdr(u32 mcrx, u32 mcr, u32 *mdr)
+{
+ int result;
+
+ if (!mbi_pdev)
+ return -ENODEV;
+
+ if (mcrx) {
+ result = pci_write_config_dword(mbi_pdev,
+ BT_MBI_MCRX_OFFSET, mcrx);
+ if (result < 0)
+ goto iosf_mbi_read_err;
+ }
+
+ result = pci_write_config_dword(mbi_pdev,
+ BT_MBI_MCR_OFFSET, mcr);
+ if (result < 0)
+ goto iosf_mbi_read_err;
+
+ result = pci_read_config_dword(mbi_pdev,
+ BT_MBI_MDR_OFFSET, mdr);
+ if (result < 0)
+ goto iosf_mbi_read_err;
+
+ return 0;
+
+iosf_mbi_read_err:
+ dev_err(&mbi_pdev->dev, "error: PCI config operation returned %d\n",
+ result);
+ return result;
+}
+
+/* Hold lock before calling */
+static int iosf_mbi_pci_write_mdr(u32 mcrx, u32 mcr, u32 mdr)
+{
+ int result;
+
+ if (!mbi_pdev)
+ return -ENODEV;
+
+ result = pci_write_config_dword(mbi_pdev,
+ BT_MBI_MDR_OFFSET, mdr);
+ if (result < 0)
+ goto iosf_mbi_write_err;
+
+ if (mcrx) {
+ result = pci_write_config_dword(mbi_pdev,
+ BT_MBI_MCRX_OFFSET, mcrx);
+ if (result < 0)
+ goto iosf_mbi_write_err;
+ }
+
+ result = pci_write_config_dword(mbi_pdev,
+ BT_MBI_MCR_OFFSET, mcr);
+ if (result < 0)
+ goto iosf_mbi_write_err;
+
+ return 0;
+
+iosf_mbi_write_err:
+ dev_err(&mbi_pdev->dev, "error: PCI config operation returned %d\n",
+ result);
+ return result;
+}
+
+int bt_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr)
+{
+ u32 mcr, mcrx;
+ unsigned long flags;
+ int ret;
+
+ /*Access to the GFX unit is handled by GPU code */
+ BUG_ON(port == BT_MBI_UNIT_GFX);
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & BT_MBI_MASK_LO);
+ mcrx = offset & BT_MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+ ret = iosf_mbi_pci_read_mdr(mcrx, mcr, mdr);
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+
+ return ret;
+}
+EXPORT_SYMBOL(bt_mbi_read);
+
+int bt_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
+{
+ u32 mcr, mcrx;
+ unsigned long flags;
+ int ret;
+
+ /*Access to the GFX unit is handled by GPU code */
+ BUG_ON(port == BT_MBI_UNIT_GFX);
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & BT_MBI_MASK_LO);
+ mcrx = offset & BT_MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+ ret = iosf_mbi_pci_write_mdr(mcrx, mcr, mdr);
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+
+ return ret;
+}
+EXPORT_SYMBOL(bt_mbi_write);
+
+int bt_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)
+{
+ u32 mcr, mcrx;
+ u32 value;
+ unsigned long flags;
+ int ret;
+
+ /*Access to the GFX unit is handled by GPU code */
+ BUG_ON(port == BT_MBI_UNIT_GFX);
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & BT_MBI_MASK_LO);
+ mcrx = offset & BT_MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+
+ /* Read current mdr value */
+ ret = iosf_mbi_pci_read_mdr(mcrx, mcr & BT_MBI_RD_MASK, &value);
+ if (ret < 0) {
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+ return ret;
+ }
+
+ /* Apply mask */
+ value &= ~mask;
+ mdr &= mask;
+ value |= mdr;
+
+ /* Write back */
+ ret = iosf_mbi_pci_write_mdr(mcrx, mcr | BT_MBI_WR_MASK, value);
+
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+
+ return ret;
+}
+EXPORT_SYMBOL(bt_mbi_modify);
+
+static int iosf_mbi_probe(struct pci_dev *pdev,
+ const struct pci_device_id *unused)
+{
+ int ret;
+
+ ret = pci_enable_device(pdev);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "error: could not enable device\n");
+ return ret;
+ }
+
+ mbi_pdev = pci_dev_get(pdev);
+ return 0;
+}
+
+static DEFINE_PCI_DEVICE_TABLE(iosf_mbi_pci_ids) = {
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x0F00) },
+ { 0, },
+};
+MODULE_DEVICE_TABLE(pci, iosf_mbi_pci_ids);
+
+static struct pci_driver iosf_mbi_pci_driver = {
+ .name = "iosf_mbi_pci",
+ .probe = iosf_mbi_probe,
+ .id_table = iosf_mbi_pci_ids,
+};
+
+static int __init bt_mbi_init(void)
+{
+ return pci_register_driver(&iosf_mbi_pci_driver);
+}
+
+static void __exit bt_mbi_exit(void)
+{
+ pci_unregister_driver(&iosf_mbi_pci_driver);
+ if (mbi_pdev) {
+ pci_dev_put(mbi_pdev);
+ mbi_pdev = NULL;
+ }
+}
+
+module_init(bt_mbi_init);
+module_exit(bt_mbi_exit);
+
+MODULE_AUTHOR("David E. Box <david...@linux.intel.com>");
+MODULE_DESCRIPTION("BayTrail Mailbox Interface accessor");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/platform/x86/intel_baytrail.h b/drivers/platform/x86/intel_baytrail.h
new file mode 100644
index 0000000..8bcc311
--- /dev/null
+++ b/drivers/platform/x86/intel_baytrail.h
@@ -0,0 +1,90 @@
+/*
+ * intel_baytrail.h: MailBox access support for Intel BayTrail platforms
+ */
+
+#ifndef INTEL_BAYTRAIL_MBI_SYMS_H
+#define INTEL_BAYTRAIL_MBI_SYMS_H
+
+#define BT_MBI_MCR_OFFSET 0xD0
+#define BT_MBI_MDR_OFFSET 0xD4
+#define BT_MBI_MCRX_OFFSET 0xD8
+
+#define BT_MBI_RD_MASK 0xFEFFFFFF
+#define BT_MBI_WR_MASK 0X01000000
+
+#define BT_MBI_MASK_HI 0xFFFFFF00
+#define BT_MBI_MASK_LO 0x000000FF
+#define BT_MBI_ENABLE 0xF0
+
+ * @mdr: register data to be read
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ * Return: Nonzero on error
+ */
+int bt_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr);
+
+/**
+ * bt_mbi_write() - MailBox unmasked write command
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ * @mdr: register data to be written
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ * Return: Nonzero on error
+ */
+int bt_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr);
+
+/**
+ * bt_mbi_modify() - MailBox masked write command
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ * @mdr: register data being modified
+ * @mask: mask indicating bits in mdr to be modified
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ * Return: Nonzero on error
+ */
+int bt_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask);

Rafael J. Wysocki

unread,
Dec 19, 2013, 8:50:01 PM12/19/13
to
Which lock?

> +static int iosf_mbi_pci_read_mdr(u32 mcrx, u32 mcr, u32 *mdr)
> +{
> + int result;
> +
> + if (!mbi_pdev)
> + return -ENODEV;
> +
> + if (mcrx) {
> + result = pci_write_config_dword(mbi_pdev,
> + BT_MBI_MCRX_OFFSET, mcrx);
> + if (result < 0)
> + goto iosf_mbi_read_err;

Why not to call the label just err?

> + }
> +
> + result = pci_write_config_dword(mbi_pdev,
> + BT_MBI_MCR_OFFSET, mcr);

Doesn't that fit into 80 chars?

> + if (result < 0)
> + goto iosf_mbi_read_err;
> +
> + result = pci_read_config_dword(mbi_pdev,
> + BT_MBI_MDR_OFFSET, mdr);
> + if (result < 0)
> + goto iosf_mbi_read_err;
> +
> + return 0;
> +
> +iosf_mbi_read_err:
> + dev_err(&mbi_pdev->dev, "error: PCI config operation returned %d\n",
> + result);

If you said "PCI config access failed with %d\n", you wouldn't need the "error:" thing.
Is that a good enough reason to crash the kernel? Maybe WARN_ON() + return error
would be sufficient?

> +
> + mcr = iosf_mbi_form_mcr(opcode, port, offset & BT_MBI_MASK_LO);
> + mcrx = offset & BT_MBI_MASK_HI;
> +
> + spin_lock_irqsave(&iosf_mbi_lock, flags);
> + ret = iosf_mbi_pci_read_mdr(mcrx, mcr, mdr);
> + spin_unlock_irqrestore(&iosf_mbi_lock, flags);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(bt_mbi_read);
> +
> +int bt_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
> +{
> + u32 mcr, mcrx;
> + unsigned long flags;
> + int ret;
> +
> + /*Access to the GFX unit is handled by GPU code */
> + BUG_ON(port == BT_MBI_UNIT_GFX);

Same comment here.

> +
> + mcr = iosf_mbi_form_mcr(opcode, port, offset & BT_MBI_MASK_LO);
> + mcrx = offset & BT_MBI_MASK_HI;
> +
> + spin_lock_irqsave(&iosf_mbi_lock, flags);
> + ret = iosf_mbi_pci_write_mdr(mcrx, mcr, mdr);
> + spin_unlock_irqrestore(&iosf_mbi_lock, flags);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(bt_mbi_write);
> +
> +int bt_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)
> +{
> + u32 mcr, mcrx;
> + u32 value;
> + unsigned long flags;
> + int ret;
> +
> + /*Access to the GFX unit is handled by GPU code */
> + BUG_ON(port == BT_MBI_UNIT_GFX);

And here.
Are the users of the interface supposed to include this file?

Rafael

David E. Box

unread,
Dec 20, 2013, 2:10:01 AM12/20/13
to
I like the clarity since there are multiple goto labels

> > + }
> > +
> > + result = pci_write_config_dword(mbi_pdev,
> > + BT_MBI_MCR_OFFSET, mcr);
>
> Doesn't that fit into 80 chars?
>

ok

> > + if (result < 0)
> > + goto iosf_mbi_read_err;
> > +
> > + result = pci_read_config_dword(mbi_pdev,
> > + BT_MBI_MDR_OFFSET, mdr);
> > + if (result < 0)
> > + goto iosf_mbi_read_err;
> > +
> > + return 0;
> > +
> > +iosf_mbi_read_err:
> > + dev_err(&mbi_pdev->dev, "error: PCI config operation returned %d\n",
> > + result);
>
> If you said "PCI config access failed with %d\n", you wouldn't need the "error:" thing.
>

ok
Ok.
Only for the macro definitions.

> Rafael

- Dave

David E. Box

unread,
Dec 30, 2013, 1:20:02 PM12/30/13
to
From: "David E. Box" <david...@linux.intel.com>

Current Intel SOC cores use a MailBox Interface (MBI) to provide access to unit
devices connected to the system fabric. This driver implements access to this
interface on BayTrail platforms. This is a requirement for drivers that need
access to unit registers on the platform (e.g. accessing the PUNIT for power
management features such as RAPL). Serialized access is handled by all exported
routines with spinlocks.

The API includes 3 functions for access to unit registers:

int bt_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr)
int bt_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
int bt_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)

port: indicating the unit being accessed
opcode: the read or write port specific opcode
offset: the register offset within the port
mdr: the register data to be read, written, or modified
mask: bit locations in mdr to change

Returns nonzero on error

Note: GPU code handles access to the GFX unit. Therefore access to that unit
with this driver is disallowed to avoid conflicts.

Signed-off-by: David E. Box <david...@linux.intel.com>
---
v6: Changed BUG_ON to WARN_ON on attempted access to GFX port as suggested by
Rafael Wysocki <r...@rjwysocki.net>
Added line to Kconfig noting available definitions in header.
Other cosmetic changes suggested by Rafael.

v5: Converted from pnpacpi/mmio driver to pci/config_space driver as
necessitated by firmware change.

v4: Define driver as platform specific to BayTrail as some platforms cannot
enumerate the MBI using ACPI as noted by Bin Gao <bin...@linux.intel.com>
Renamed register macros and API funcitons to platform specific names.
Changed dependency to PNPACPI as suggested by Rafael Wysocki <r...@rjwysocki.net>

v3: Converted to PNP ACPI driver as suggested by Rafael Wysocki <r...@rjwysocki.net>
Removed config visibility to user as suggested by Andi Kleen <an...@firstfloor.org>

v2: Made modular since there was no longer a reason not to
Moved to x86 platform as suggested by Mathhew Garrett <mj...@srcf.ucam.org>
Added probe to init to cause driver load to fail if device not detected.

drivers/platform/x86/Kconfig | 10 ++
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/intel_baytrail.c | 225 +++++++++++++++++++++++++++++++++
drivers/platform/x86/intel_baytrail.h | 90 +++++++++++++
4 files changed, 326 insertions(+)
create mode 100644 drivers/platform/x86/intel_baytrail.c
create mode 100644 drivers/platform/x86/intel_baytrail.h

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index b51a746..6e199a5 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -819,4 +819,14 @@ config PVPANIC
a paravirtualized device provided by QEMU; it lets a virtual machine
(guest) communicate panic events to the host.

+config INTEL_BAYTRAIL_MBI
+ tristate
+ depends on PCI
+ ---help---
+ Needed on Baytrail platforms for access to the IOSF Sideband Mailbox
+ Interface. This is a requirement for systems that need to configure
+ the PUNIT for power management features such as RAPL. Register
+ addresses and r/w opcodes are defined in
+ drivers/platform/x86/intel_baytrail.c.
+
endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 5dbe193..b3d4cfd 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -55,3 +55,4 @@ obj-$(CONFIG_INTEL_RST) += intel-rst.o
obj-$(CONFIG_INTEL_SMARTCONNECT) += intel-smartconnect.o

obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-$(CONFIG_INTEL_BAYTRAIL_MBI) += intel_baytrail.o
diff --git a/drivers/platform/x86/intel_baytrail.c b/drivers/platform/x86/intel_baytrail.c
new file mode 100644
index 0000000..fcb49cc
--- /dev/null
+++ b/drivers/platform/x86/intel_baytrail.c
@@ -0,0 +1,225 @@
+static int iosf_mbi_pci_read_mdr(u32 mcrx, u32 mcr, u32 *mdr)
+{
+ int result;
+
+ if (!mbi_pdev)
+ return -ENODEV;
+
+ if (mcrx) {
+ result = pci_write_config_dword(mbi_pdev, BT_MBI_MCRX_OFFSET,
+ mcrx);
+ if (result < 0)
+ goto fail_read;
+ }
+
+ result = pci_write_config_dword(mbi_pdev, BT_MBI_MCR_OFFSET, mcr);
+ if (result < 0)
+ goto fail_read;
+
+ result = pci_read_config_dword(mbi_pdev, BT_MBI_MDR_OFFSET, mdr);
+ if (result < 0)
+ goto fail_read;
+
+ return 0;
+
+fail_read:
+ dev_err(&mbi_pdev->dev, "PCI config access failed with %d\n", result);
+ return result;
+}
+
+static int iosf_mbi_pci_write_mdr(u32 mcrx, u32 mcr, u32 mdr)
+{
+ int result;
+
+ if (!mbi_pdev)
+ return -ENODEV;
+
+ result = pci_write_config_dword(mbi_pdev, BT_MBI_MDR_OFFSET, mdr);
+ if (result < 0)
+ goto fail_write;
+
+ if (mcrx) {
+ result = pci_write_config_dword(mbi_pdev, BT_MBI_MCRX_OFFSET,
+ mcrx);
+ if (result < 0)
+ goto fail_write;
+ }
+
+ result = pci_write_config_dword(mbi_pdev, BT_MBI_MCR_OFFSET, mcr);
+ if (result < 0)
+ goto fail_write;
+
+ return 0;
+
+fail_write:
+ dev_err(&mbi_pdev->dev, "PCI config access failed with %d\n", result);
+ return result;
+}
+
+int bt_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr)
+{
+ u32 mcr, mcrx;
+ unsigned long flags;
+ int ret;
+
+ /*Access to the GFX unit is handled by GPU code */
+ if (port == BT_MBI_UNIT_GFX) {
+ WARN_ON(1);
+ return -EPERM;
+ }
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & BT_MBI_MASK_LO);
+ mcrx = offset & BT_MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+ ret = iosf_mbi_pci_read_mdr(mcrx, mcr, mdr);
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+
+ return ret;
+}
+EXPORT_SYMBOL(bt_mbi_read);
+
+int bt_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
+{
+ u32 mcr, mcrx;
+ unsigned long flags;
+ int ret;
+
+ /*Access to the GFX unit is handled by GPU code */
+ if (port == BT_MBI_UNIT_GFX) {
+ WARN_ON(1);
+ return -EPERM;
+ }
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & BT_MBI_MASK_LO);
+ mcrx = offset & BT_MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+ ret = iosf_mbi_pci_write_mdr(mcrx, mcr, mdr);
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+
+ return ret;
+}
+EXPORT_SYMBOL(bt_mbi_write);
+
+int bt_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)
+{
+ u32 mcr, mcrx;
+ u32 value;
+ unsigned long flags;
+ int ret;
+
+ /*Access to the GFX unit is handled by GPU code */
+ if (port == BT_MBI_UNIT_GFX) {
+ WARN_ON(1);
+ return -EPERM;
+ }

David E. Box

unread,
Jan 7, 2014, 1:10:04 PM1/7/14
to
From: "David E. Box" <david...@linux.intel.com>

Current Intel SOC cores use a MailBox Interface (MBI) to provide access to unit
devices connected to the system fabric. This driver implements access to this
interface on BayTrail platforms. This is a requirement for drivers that need
access to unit registers on the platform (e.g. accessing the PUNIT for power
management features such as RAPL). Serialized access is handled by all exported
routines with spinlocks.

The API includes 3 functions for access to unit registers:

int bt_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr)
int bt_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
int bt_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)

port: indicating the unit being accessed
opcode: the read or write port specific opcode
offset: the register offset within the port
mdr: the register data to be read, written, or modified
mask: bit locations in mdr to change

Returns nonzero on error

Note: GPU code handles access to the GFX unit. Therefore access to that unit
with this driver is disallowed to avoid conflicts.

Signed-off-by: David E. Box <david...@linux.intel.com>
---
Applies to Linux 3.12

v5: Converted from pnpacpi/mmio driver to pci/config_space driver as
necessitated by firmware change.

v4: Define driver as platform specific to BayTrail as some platforms cannot
enumerate the MBI using ACPI as noted by Bin Gao <bin...@linux.intel.com>
Renamed register macros and API funcitons to platform specific names.
Changed dependency to PNPACPI as sugessted by Rafael Wysocki <r...@rjwysocki.net>

v3: Converted to PNP ACPI driver as sugessted by Rafael Wysocki <r...@rjwysocki.net>

Randy Dunlap

unread,
Jan 7, 2014, 1:20:02 PM1/7/14
to
On 01/07/14 10:03, David E. Box wrote:
> From: "David E. Box" <david...@linux.intel.com>
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index b51a746..6e199a5 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -819,4 +819,14 @@ config PVPANIC
> a paravirtualized device provided by QEMU; it lets a virtual machine
> (guest) communicate panic events to the host.
>
> +config INTEL_BAYTRAIL_MBI
> + tristate

Is this kconfig option displayed when you run menuconfig/nconfig/xconfig/gconfig etc.?
Doesn't it need a prompt string?
How did you enable it and test it?

> + depends on PCI
> + ---help---
> + Needed on Baytrail platforms for access to the IOSF Sideband Mailbox
> + Interface. This is a requirement for systems that need to configure
> + the PUNIT for power management features such as RAPL. Register
> + addresses and r/w opcodes are defined in

Think of users reading this. At least change "r/w" to read/write.
What is IOSF? does it matter here?
PUNIT? RAPL?

> + drivers/platform/x86/intel_baytrail.c.
> +
> endif # X86_PLATFORM_DEVICES


--
~Randy

David E. Box

unread,
Jan 7, 2014, 1:50:01 PM1/7/14
to
On Tue, Jan 07, 2014 at 10:15:03AM -0800, Randy Dunlap wrote:
> On 01/07/14 10:03, David E. Box wrote:
> > From: "David E. Box" <david...@linux.intel.com>
> >
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index b51a746..6e199a5 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -819,4 +819,14 @@ config PVPANIC
> > a paravirtualized device provided by QEMU; it lets a virtual machine
> > (guest) communicate panic events to the host.
> >
> > +config INTEL_BAYTRAIL_MBI
> > + tristate
>
> Is this kconfig option displayed when you run menuconfig/nconfig/xconfig/gconfig etc.?
> Doesn't it need a prompt string?
> How did you enable it and test it?
>

It is not displayed on purpose. The driver isn't exposed to user space.
It was tested by adding the option to .config, both as module and built-in.

> > + depends on PCI
> > + ---help---
> > + Needed on Baytrail platforms for access to the IOSF Sideband Mailbox
> > + Interface. This is a requirement for systems that need to configure
> > + the PUNIT for power management features such as RAPL. Register
> > + addresses and r/w opcodes are defined in
>
> Think of users reading this. At least change "r/w" to read/write.
> What is IOSF? does it matter here?
> PUNIT? RAPL?
>

If you don't know what the IOSF Sideband is you probably shouldn't be enabling
this feature. Users of this driver (of which for now there are few) will be
kernel space drivers that should have ample knowledge of the registers this
interface provides access to.

Dave

Randy Dunlap

unread,
Jan 7, 2014, 2:40:01 PM1/7/14
to
I see. Thanks.


--
~Randy

Rafael J. Wysocki

unread,
Jan 7, 2014, 3:40:02 PM1/7/14
to
What about generic x86 distro kernels? They won't know in advance whether or
not they will need this feature.

Thanks!

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

David E. Box

unread,
Jan 7, 2014, 4:50:01 PM1/7/14
to
Ok, I spoke with other developers and I apparently misunderstood the context
here. Distro's enable these features and this is too detailed for them to know
what to do with it. How about simply "Required to enable platform specific power
managemnet features on Baytrail"?

KISS is easier said than done.

Dave

Rafael J. Wysocki

unread,
Jan 7, 2014, 7:00:02 PM1/7/14
to
Well, I personally think that this code should go into arch/x86/ as library code
needed to access IOSF Sideband on some platforms. I probably would make modules
depending on it select it, so for example if the RAPL driver is going to be
built, your driver should be build either and rather unconditionally in that
case.

So the rule should be "if something *may* need it, build it" in my opinion.

Thanks!

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

H. Peter Anvin

unread,
Jan 7, 2014, 7:10:01 PM1/7/14
to
On 01/07/2014 04:11 PM, Rafael J. Wysocki wrote:
>>
>> Ok, I spoke with other developers and I apparently misunderstood the context
>> here. Distro's enable these features and this is too detailed for them to know
>> what to do with it. How about simply "Required to enable platform specific power
>> managemnet features on Baytrail"?
>>
>> KISS is easier said than done.
>
> Well, I personally think that this code should go into arch/x86/ as library code
> needed to access IOSF Sideband on some platforms. I probably would make modules
> depending on it select it, so for example if the RAPL driver is going to be
> built, your driver should be build either and rather unconditionally in that
> case.
>
> So the rule should be "if something *may* need it, build it" in my opinion.
>

I thought we were targeting this for drivers/x86? However, perhaps with
power management tied in that doesn't make too much sense.

-hpa

David E. Box

unread,
Jan 8, 2014, 12:30:01 AM1/8/14
to
On Wed, Jan 08, 2014 at 01:11:22AM +0100, Rafael J. Wysocki wrote:
> Well, I personally think that this code should go into arch/x86/ as library code
> needed to access IOSF Sideband on some platforms.

I don't disagree. However for the record this is not the first time it has been
discussed and I already moved it from arch/x86 to platform on the suggestion of
several maintainers. I will keep the interface generic except that it has to be
stated that it will only support those platforms that can enumerate this device
using PCI. Plus I learned there are others who plan to patch when it gets
accepted to support other platforms using different methods of
enumeration/communication. I would thik this probably cements its placement in
arch/x86.

> I probably would make modules
> depending on it select it, so for example if the RAPL driver is going to be
> built, your driver should be build either and rather unconditionally in that
> case.
>

Wouldn't such a dependency be handled by the RAPL driver et al. How can I force
modules to build this driver? Reverse dependency? Also the biggest consumer of
the driver doesn't have code upstream yet.

> So the rule should be "if something *may* need it, build it" in my opinion.

You suggest that even though non-IOSF systems don't need this driver to enable
RAPL, it should build anyway since it's a dependency for systems that do have
IOSF? Even if this is what you suggest I'd prefer the owner of the driver that
has the dependency be the one to patch this driver to make in build in that
case. I do not know all who would use it.

Dave Box

Rafael J. Wysocki

unread,
Jan 8, 2014, 8:40:02 AM1/8/14
to
On Tuesday, January 07, 2014 09:27:17 PM David E. Box wrote:
> On Wed, Jan 08, 2014 at 01:11:22AM +0100, Rafael J. Wysocki wrote:
> > Well, I personally think that this code should go into arch/x86/ as library code
> > needed to access IOSF Sideband on some platforms.
>
> I don't disagree. However for the record this is not the first time it has been
> discussed and I already moved it from arch/x86 to platform on the suggestion of
> several maintainers. I will keep the interface generic except that it has to be
> stated that it will only support those platforms that can enumerate this device
> using PCI. Plus I learned there are others who plan to patch when it gets
> accepted to support other platforms using different methods of
> enumeration/communication. I would thik this probably cements its placement in
> arch/x86.

I think so.

> > I probably would make modules
> > depending on it select it, so for example if the RAPL driver is going to be
> > built, your driver should be build either and rather unconditionally in that
> > case.
> >
>
> Wouldn't such a dependency be handled by the RAPL driver et al. How can I force
> modules to build this driver? Reverse dependency? Also the biggest consumer of
> the driver doesn't have code upstream yet.

The modules depending on your driver can do

select INTEL_BAYTRAIL_MBI

in their Kconfig entries. That will cause it to be built.

> > So the rule should be "if something *may* need it, build it" in my opinion.
>
> You suggest that even though non-IOSF systems don't need this driver to enable
> RAPL, it should build anyway since it's a dependency for systems that do have
> IOSF? Even if this is what you suggest I'd prefer the owner of the driver that
> has the dependency be the one to patch this driver to make in build in that
> case. I do not know all who would use it.

Simply, don't enable it by default and let its users do the above.

Then, they won't have to do any #ifdef CONFIG_INTEL_BAYTRAIL_MBI stuff in their
code (I'd change the name of the CONFIG_ thing if it goes beyond BayTrail).

Of course, it will be compiled for generic x86 kernels this way, but since
they have to assume they may run on anything, they'll have to build it anyway.

The only "problematic" case will be kernels compiled by users of systems that
don't use IOSF Sideband and do use things like RAPL, but I really wouldn't
try to optimize for that case.

Thanks!

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

David E. Box

unread,
Jan 8, 2014, 4:30:01 PM1/8/14
to
From: "David E. Box" <david...@linux.intel.com>

Current Intel SOC cores use a MailBox Interface (MBI) to provide access to
configuration registers on devices (called units) connected to the system
fabric. This is a support driver that implements access to this interface on
those platforms that can enumerate the device using PCI. Initial support is for
BayTrail, for which port definitons are provided. This is a requirement for
implementing platform specific features (e.g. RAPL driver requires this to
perform platform specific power management using the registers in PUNIT).
Dependant modules should select IOSF_MBI in their respective Kconfig
configuraiton. Serialized access is handled by all exported routines with
spinlocks.

The API includes 3 functions for access to unit registers:

int iosf_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr)
int iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)

port: indicating the unit being accessed
opcode: the read or write port specific opcode
offset: the register offset within the port
mdr: the register data to be read, written, or modified
mask: bit locations in mdr to change

Returns nonzero on error

Note: GPU code handles access to the GFX unit. Therefore access to that unit
with this driver is disallowed to avoid conflicts.

Signed-off-by: David E. Box <david...@linux.intel.com>
---
v7: Moved to arch/x86 as suggested by Rafael Wysocki <r...@rjwysocki.net>
State that the driver supports all platforms that enumerate MBI using PCI
though pci ids initially just BayTrail.
Remove platform-specific naming where not required.

v6: Changed BUG_ON to WARN_ON on attempted access to GFX port as suggested by
Rafael Wysocki <r...@rjwysocki.net>
Added line to Kconfig noting available definitions in header.
Other cosmetic changes suggested by Rafael.

v5: Converted from pnpacpi/mmio driver to pci/config_space driver as
necessitated by firmware change.

v4: Define driver as platform specific to BayTrail as some platforms cannot
enumerate the MBI using ACPI as noted by Bin Gao <bin...@linux.intel.com>
Renamed register macros and API funcitons to platform specific names.
Changed dependency to PNPACPI as suggested by Rafael Wysocki <r...@rjwysocki.net>

v3: Converted to PNP ACPI driver as suggested by Rafael Wysocki <r...@rjwysocki.net>
Removed config visibility to user as suggested by Andi Kleen <an...@firstfloor.org>

v2: Made modular since there was no longer a reason not to
Moved to x86 platform as suggested by Mathhew Garrett <mj...@srcf.ucam.org>
Added probe to init to cause driver load to fail if device not detected.

arch/x86/Kconfig | 8 ++
arch/x86/include/asm/iosf_mbi.h | 90 ++++++++++++++++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/iosf_mbi.c | 226 +++++++++++++++++++++++++++++++++++++++
4 files changed, 325 insertions(+)
create mode 100644 arch/x86/include/asm/iosf_mbi.h
create mode 100644 arch/x86/kernel/iosf_mbi.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f67e839..d3b1f8b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2384,6 +2384,14 @@ config X86_DMA_REMAP
bool
depends on STA2X11

+config IOSF_MBI
+ bool
+ depends on PCI
+ ---help---
+ To be selected by modules requiring access to the Intel OnChip System
+ Fabric (IOSF) Sideband MailBox Interface (MBI). For MBI platforms
+ enumerable by PCI.
+
source "net/Kconfig"

source "drivers/Kconfig"
diff --git a/arch/x86/include/asm/iosf_mbi.h b/arch/x86/include/asm/iosf_mbi.h
new file mode 100644
index 0000000..8e71c79
--- /dev/null
+++ b/arch/x86/include/asm/iosf_mbi.h
@@ -0,0 +1,90 @@
+/*
+ * iosf_mbi.h: Intel OnChip System Fabric MailBox access support
+ */
+
+#ifndef IOSF_MBI_SYMS_H
+#define IOSF_MBI_SYMS_H
+
+#define MBI_MCR_OFFSET 0xD0
+#define MBI_MDR_OFFSET 0xD4
+#define MBI_MCRX_OFFSET 0xD8
+
+#define MBI_RD_MASK 0xFEFFFFFF
+#define MBI_WR_MASK 0X01000000
+
+#define MBI_MASK_HI 0xFFFFFF00
+#define MBI_MASK_LO 0x000000FF
+#define MBI_ENABLE 0xF0
+
+/* Baytrail available units */
+#define BT_MBI_UNIT_AUNIT 0x00
+#define BT_MBI_UNIT_SMC 0x01
+#define BT_MBI_UNIT_CPU 0x02
+#define BT_MBI_UNIT_BUNIT 0x03
+#define BT_MBI_UNIT_PMC 0x04
+#define BT_MBI_UNIT_GFX 0x06
+#define BT_MBI_UNIT_SMI 0x0C
+#define BT_MBI_UNIT_USB 0x43
+#define BT_MBI_UNIT_SATA 0xA3
+#define BT_MBI_UNIT_PCIE 0xA6
+
+/* Baytrail read/write opcodes */
+#define BT_MBI_AUNIT_READ 0x10
+#define BT_MBI_AUNIT_WRITE 0x11
+#define BT_MBI_SMC_READ 0x10
+#define BT_MBI_SMC_WRITE 0x11
+#define BT_MBI_CPU_READ 0x10
+#define BT_MBI_CPU_WRITE 0x11
+#define BT_MBI_BUNIT_READ 0x10
+#define BT_MBI_BUNIT_WRITE 0x11
+#define BT_MBI_PMC_READ 0x06
+#define BT_MBI_PMC_WRITE 0x07
+#define BT_MBI_GFX_READ 0x00
+#define BT_MBI_GFX_WRITE 0x01
+#define BT_MBI_SMIO_READ 0x06
+#define BT_MBI_SMIO_WRITE 0x07
+#define BT_MBI_USB_READ 0x06
+#define BT_MBI_USB_WRITE 0x07
+#define BT_MBI_SATA_READ 0x00
+#define BT_MBI_SATA_WRITE 0x01
+#define BT_MBI_PCIE_READ 0x00
+#define BT_MBI_PCIE_WRITE 0x01
+
+/**
+ * iosf_mbi_read() - MailBox Interface read command
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ * @mdr: register data to be read
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ * Return: Nonzero on error
+ */
+int iosf_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr);
+
+/**
+ * iosf_mbi_write() - MailBox unmasked write command
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ * @mdr: register data to be written
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ * Return: Nonzero on error
+ */
+int iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr);
+
+/**
+ * iosf_mbi_modify() - MailBox masked write command
+ * @port: port indicating subunit being accessed
+ * @opcode: port specific read or write opcode
+ * @offset: register address offset
+ * @mdr: register data being modified
+ * @mask: mask indicating bits in mdr to be modified
+ *
+ * Locking is handled by spinlock - cannot sleep.
+ * Return: Nonzero on error
+ */
+int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask);
+
+#endif /* IOSF_MBI_SYMS_H */
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index a5408b9..510551e 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -109,6 +109,7 @@ obj-$(CONFIG_EFI) += sysfb_efi.o

obj-$(CONFIG_PERF_EVENTS) += perf_regs.o
obj-$(CONFIG_TRACING) += tracepoint.o
+obj-$(CONFIG_IOSF_MBI) += iosf_mbi.o

###
# 64 bit specific files
diff --git a/arch/x86/kernel/iosf_mbi.c b/arch/x86/kernel/iosf_mbi.c
new file mode 100644
index 0000000..c3aae66
--- /dev/null
+++ b/arch/x86/kernel/iosf_mbi.c
@@ -0,0 +1,226 @@
+/*
+ * IOSF-SB MailBox Interface Driver
+ * Copyright (c) 2013, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ *
+ * The IOSF-SB is a fabric bus available on Atom based SOC's that uses a
+ * mailbox interface (MBI) to communicate with mutiple devices. This
+ * driver implements access to this interface for those platforms that can
+ * enumerate the device using PCI.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/spinlock.h>
+#include <linux/pci.h>
+
+#include <asm/iosf_mbi.h>
+
+static DEFINE_SPINLOCK(iosf_mbi_lock);
+
+static inline u32 iosf_mbi_form_mcr(u8 op, u8 port, u8 offset)
+{
+ return (op << 24) | (port << 16) | (offset << 8) | MBI_ENABLE;
+}
+
+static struct pci_dev *mbi_pdev; /* one mbi device */
+
+static int iosf_mbi_pci_read_mdr(u32 mcrx, u32 mcr, u32 *mdr)
+{
+ int result;
+
+ if (!mbi_pdev)
+ return -ENODEV;
+
+ if (mcrx) {
+ result = pci_write_config_dword(mbi_pdev, MBI_MCRX_OFFSET,
+ mcrx);
+ if (result < 0)
+ goto fail_read;
+ }
+
+ result = pci_write_config_dword(mbi_pdev, MBI_MCR_OFFSET, mcr);
+ if (result < 0)
+ goto fail_read;
+
+ result = pci_read_config_dword(mbi_pdev, MBI_MDR_OFFSET, mdr);
+ if (result < 0)
+ goto fail_read;
+
+ return 0;
+
+fail_read:
+ dev_err(&mbi_pdev->dev, "PCI config access failed with %d\n", result);
+ return result;
+}
+
+static int iosf_mbi_pci_write_mdr(u32 mcrx, u32 mcr, u32 mdr)
+{
+ int result;
+
+ if (!mbi_pdev)
+ return -ENODEV;
+
+ result = pci_write_config_dword(mbi_pdev, MBI_MDR_OFFSET, mdr);
+ if (result < 0)
+ goto fail_write;
+
+ if (mcrx) {
+ result = pci_write_config_dword(mbi_pdev, MBI_MCRX_OFFSET,
+ mcrx);
+ if (result < 0)
+ goto fail_write;
+ }
+
+ result = pci_write_config_dword(mbi_pdev, MBI_MCR_OFFSET, mcr);
+ if (result < 0)
+ goto fail_write;
+
+ return 0;
+
+fail_write:
+ dev_err(&mbi_pdev->dev, "PCI config access failed with %d\n", result);
+ return result;
+}
+
+int iosf_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr)
+{
+ u32 mcr, mcrx;
+ unsigned long flags;
+ int ret;
+
+ /*Access to the GFX unit is handled by GPU code */
+ if (port == BT_MBI_UNIT_GFX) {
+ WARN_ON(1);
+ return -EPERM;
+ }
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & MBI_MASK_LO);
+ mcrx = offset & MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+ ret = iosf_mbi_pci_read_mdr(mcrx, mcr, mdr);
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+
+ return ret;
+}
+EXPORT_SYMBOL(iosf_mbi_read);
+
+int iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
+{
+ u32 mcr, mcrx;
+ unsigned long flags;
+ int ret;
+
+ /*Access to the GFX unit is handled by GPU code */
+ if (port == BT_MBI_UNIT_GFX) {
+ WARN_ON(1);
+ return -EPERM;
+ }
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & MBI_MASK_LO);
+ mcrx = offset & MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+ ret = iosf_mbi_pci_write_mdr(mcrx, mcr, mdr);
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+
+ return ret;
+}
+EXPORT_SYMBOL(iosf_mbi_write);
+
+int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)
+{
+ u32 mcr, mcrx;
+ u32 value;
+ unsigned long flags;
+ int ret;
+
+ /*Access to the GFX unit is handled by GPU code */
+ if (port == BT_MBI_UNIT_GFX) {
+ WARN_ON(1);
+ return -EPERM;
+ }
+
+ mcr = iosf_mbi_form_mcr(opcode, port, offset & MBI_MASK_LO);
+ mcrx = offset & MBI_MASK_HI;
+
+ spin_lock_irqsave(&iosf_mbi_lock, flags);
+
+ /* Read current mdr value */
+ ret = iosf_mbi_pci_read_mdr(mcrx, mcr & MBI_RD_MASK, &value);
+ if (ret < 0) {
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+ return ret;
+ }
+
+ /* Apply mask */
+ value &= ~mask;
+ mdr &= mask;
+ value |= mdr;
+
+ /* Write back */
+ ret = iosf_mbi_pci_write_mdr(mcrx, mcr | MBI_WR_MASK, value);
+
+ spin_unlock_irqrestore(&iosf_mbi_lock, flags);
+
+ return ret;
+}
+EXPORT_SYMBOL(iosf_mbi_modify);
+static int __init iosf_mbi_init(void)
+{
+ return pci_register_driver(&iosf_mbi_pci_driver);
+}
+
+static void __exit iosf_mbi_exit(void)
+{
+ pci_unregister_driver(&iosf_mbi_pci_driver);
+ if (mbi_pdev) {
+ pci_dev_put(mbi_pdev);
+ mbi_pdev = NULL;
+ }
+}
+
+module_init(iosf_mbi_init);
+module_exit(iosf_mbi_exit);
+
+MODULE_AUTHOR("David E. Box <david...@linux.intel.com>");
+MODULE_DESCRIPTION("IOSF Mailbox Interface accessor");
+MODULE_LICENSE("GPL v2");

tip-bot for David E. Box

unread,
Jan 10, 2014, 5:20:02 PM1/10/14
to
Commit-ID: 46184415368a6095d5da33991c5e011f1084353d
Gitweb: http://git.kernel.org/tip/46184415368a6095d5da33991c5e011f1084353d
Author: David E. Box <david...@linux.intel.com>
AuthorDate: Wed, 8 Jan 2014 13:27:51 -0800
Committer: H. Peter Anvin <h...@linux.intel.com>
CommitDate: Wed, 8 Jan 2014 14:36:29 -0800

arch: x86: New MailBox support driver for Intel SOC's

Current Intel SOC cores use a MailBox Interface (MBI) to provide access to
configuration registers on devices (called units) connected to the system
fabric. This is a support driver that implements access to this interface on
those platforms that can enumerate the device using PCI. Initial support is for
BayTrail, for which port definitons are provided. This is a requirement for
implementing platform specific features (e.g. RAPL driver requires this to
perform platform specific power management using the registers in PUNIT).
Dependant modules should select IOSF_MBI in their respective Kconfig
configuraiton. Serialized access is handled by all exported routines with
spinlocks.

The API includes 3 functions for access to unit registers:

int iosf_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr)
int iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr)
int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)

port: indicating the unit being accessed
opcode: the read or write port specific opcode
offset: the register offset within the port
mdr: the register data to be read, written, or modified
mask: bit locations in mdr to change

Returns nonzero on error

Note: GPU code handles access to the GFX unit. Therefore access to that unit
with this driver is disallowed to avoid conflicts.

Signed-off-by: David E. Box <david...@linux.intel.com>
Link: http://lkml.kernel.org/r/1389216471-734-1-git-...@linux.intel.com
Signed-off-by: H. Peter Anvin <h...@linux.intel.com>
Cc: Rafael J. Wysocki <r...@rjwysocki.net>
Cc: Matthew Garrett <mj...@srcf.ucam.org>
---
arch/x86/Kconfig | 8 ++
arch/x86/include/asm/iosf_mbi.h | 90 ++++++++++++++++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/iosf_mbi.c | 226 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 325 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0952ecd..ca5959a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2393,6 +2393,14 @@ config X86_DMA_REMAP
index 9b0a34e..dbe9bd6 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -111,6 +111,7 @@ obj-$(CONFIG_EFI) += sysfb_efi.o
0 new messages