[PATCHv2 0/2] New driver for Intel IOSF MBI access

[PATCHv2 0/2] New driver for Intel IOSF MBI access

[David E. Box]

From: "David E. Box" <david.e.box@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 <mjg59@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

-- 
1.7.10.4

[PATCHv2 1/2] New Driver for IOSF-SB MBI access on Intel SOCs

[David E. Box]

From: "David E. Box" <david.e.box@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.e.box@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 @@
+/*
+ * 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 "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);
+	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
@@ -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 */
-- 
1.7.10.4

Re: [PATCHv2 1/2] New Driver for IOSF-SB MBI access on Intel SOCs

[Andi Kleen]

"David E. Box" <david.e.box@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

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

[PATCHv2 2/2] ACPI/platform: Add ACPI ID for Intel MBI device

[David E. Box]

From: "David E. Box" <david.e.box@linux.intel.com>

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

Signed-off-by: David E. Box <david.e.box@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

Re: [PATCHv2 2/2] ACPI/platform: Add ACPI ID for Intel MBI device

[Matthew Garrett]

On Tue, Dec 03, 2013 at 03:59:38PM -0800, David E. Box wrote:
> From: "David E. Box" <david.e.box@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 | mjg59@srcf.ucam.org

Re: [PATCHv2 2/2] ACPI/platform: Add ACPI ID for Intel MBI device

[David E. Box]

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.e.box@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 | mjg59@srcf.ucam.org

Re: [PATCHv2 2/2] ACPI/platform: Add ACPI ID for Intel MBI device

[Matthew Garrett]

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?

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

Re: [PATCHv2 2/2] ACPI/platform: Add ACPI ID for Intel MBI device

[David E. Box]

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 | mjg59@srcf.ucam.org

Re: [PATCHv2 2/2] ACPI/platform: Add ACPI ID for Intel MBI device

[Matthew Garrett]

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.

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

Re: [PATCHv2 2/2] ACPI/platform: Add ACPI ID for Intel MBI device

[Rafael J. Wysocki]

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

[PATCH] X86 platform: New IOSF-SB MBI driver for Intel SOCs

[David E. Box]

From: "David E. Box" <david.e.box@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.e.box@linux.intel.com>
---
v3: Converted to PNP ACPI driver as sugessted by Rafael Wysocki <rjw@rjwysocki.net>
    Removed config visibility to user as suggested by Andi Kleen <andi@firstfloor.org>

v2: Made modular since there was no longer a reason not to
    Moved to x86 platform as suggested by Mathhew Garrett <mjg59@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
--- /dev/null
+++ b/drivers/platform/x86/iosf_mbi.c
@@ -0,0 +1,188 @@
+/*
+ * 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/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;
+} 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_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);
+}
+
+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
@@ -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 */
-- 
1.7.10.4

Re: [PATCH] X86 platform: New IOSF-SB MBI driver for Intel SOCs

[Rafael J. Wysocki]

On Thursday, December 05, 2013 12:01:46 PM David E. Box wrote:
> From: "David E. Box" <david.e.box@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.e.box@linux.intel.com>
> ---
> v3: Converted to PNP ACPI driver as sugessted by Rafael Wysocki <rjw@rjwysocki.net>
>     Removed config visibility to user as suggested by Andi Kleen <andi@firstfloor.org>
> 
> v2: Made modular since there was no longer a reason not to
>     Moved to x86 platform as suggested by Mathhew Garrett <mjg59@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

You may as well write that as

	depends on PNPACPI

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

Thanks!

> +	---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
> --- /dev/null
> +++ b/drivers/platform/x86/iosf_mbi.c
> @@ -0,0 +1,188 @@
> +/*
> + * 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/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;
> +} 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_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);
> +}
> +
> +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
> @@ -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 */
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH] X86 platform: New BayTrail IOSF-SB MBI driver

[David E. Box]

From: "David E. Box" <david.e.box@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.e.box@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.gao@linux.intel.com>
    Renamed register macros and API funcitons to platform specific names.
    Changed dependency to PNPACPI as sugessted by Rafael Wysocki <rjw@rjwysocki.net>

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

v2: Made modular since there was no longer a reason not to
    Moved to x86 platform as suggested by Mathhew Garrett <mjg59@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);
+
+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		= "bt_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);
+}
+
+module_init(iosf_mbi_init);
+module_exit(iosf_mbi_exit);
+
+MODULE_AUTHOR("David E. Box <david.e.box@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 */
-- 
1.7.10.4

Re: [PATCH] X86 platform: New BayTrail IOSF-SB MBI driver

[Rafael J. Wysocki]

On Friday, December 06, 2013 12:59:33 PM David E. Box wrote:
> From: "David E. Box" <david.e.box@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.e.box@linux.intel.com>

Matthew, if you're going to take this:

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

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

Thanks!

> ---
> v4: Define driver as platform specific to BayTrail as some platforms cannot
>     enumerate the MBI using ACPI as noted by Bin Gao <bin.gao@linux.intel.com>
>     Renamed register macros and API funcitons to platform specific names.
>     Changed dependency to PNPACPI as sugessted by Rafael Wysocki <rjw@rjwysocki.net>
> 
> v3: Converted to PNP ACPI driver as sugessted by Rafael Wysocki <rjw@rjwysocki.net>
>     Removed config visibility to user as suggested by Andi Kleen <andi@firstfloor.org>
> 
> v2: Made modular since there was no longer a reason not to
>     Moved to x86 platform as suggested by Mathhew Garrett <mjg59@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);
> +
> +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		= "bt_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);
> +}
> +
> +module_init(iosf_mbi_init);
> +module_exit(iosf_mbi_exit);
> +
> +MODULE_AUTHOR("David E. Box <david.e.box@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 */
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] X86 platform: New BayTrail IOSF-SB MBI driver

[David E. Box]

On Sat, Dec 07, 2013 at 02:29:44AM +0100, Rafael J. Wysocki wrote:
> On Friday, December 06, 2013 12:59:33 PM David E. Box wrote:
> > From: "David E. Box" <david.e.box@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.e.box@linux.intel.com>
> 
> Matthew, if you're going to take this:
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Or if you prefer me to take it, please let me know (unless you have objections
> against the patch, of course).
> 
> Thanks!
> 

Matthre are you okay with the driver?

Dave

> > ---
> > v4: Define driver as platform specific to BayTrail as some platforms cannot
> >     enumerate the MBI using ACPI as noted by Bin Gao <bin.gao@linux.intel.com>
> >     Renamed register macros and API funcitons to platform specific names.
> >     Changed dependency to PNPACPI as sugessted by Rafael Wysocki <rjw@rjwysocki.net>
> > 
> > v3: Converted to PNP ACPI driver as sugessted by Rafael Wysocki <rjw@rjwysocki.net>
> >     Removed config visibility to user as suggested by Andi Kleen <andi@firstfloor.org>
> > 
> > v2: Made modular since there was no longer a reason not to
> >     Moved to x86 platform as suggested by Mathhew Garrett <mjg59@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);
> > +
> > +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		= "bt_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);
> > +}
> > +
> > +module_init(iosf_mbi_init);
> > +module_exit(iosf_mbi_exit);
> > +
> > +MODULE_AUTHOR("David E. Box <david.e.box@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 */
> > 
> -- 
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5][RESEND] X86 platform: New BayTrail IOSF-SB MBI driver

[David E. Box]

From: "David E. Box" <david.e.box@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.e.box@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.gao@linux.intel.com>
    Renamed register macros and API funcitons to platform specific names.
    Changed dependency to PNPACPI as sugessted by Rafael Wysocki <rjw@rjwysocki.net>

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

v2: Made modular since there was no longer a reason not to
    Moved to x86 platform as suggested by Mathhew Garrett <mjg59@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.e.box@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
+
+/* 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
+ * @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);
+
+#endif /* INTEL_BAYTRAIL_MBI_SYMS_H */
-- 
1.7.10.4

Re: [PATCH v5][RESEND] X86 platform: New BayTrail IOSF-SB MBI driver

[Rafael J. Wysocki]

On Thursday, December 19, 2013 02:37:46 PM David E. Box wrote:
> From: "David E. Box" <david.e.box@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.e.box@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.gao@linux.intel.com>
>     Renamed register macros and API funcitons to platform specific names.
>     Changed dependency to PNPACPI as sugessted by Rafael Wysocki <rjw@rjwysocki.net>
> 
> v3: Converted to PNP ACPI driver as sugessted by Rafael Wysocki <rjw@rjwysocki.net>
>     Removed config visibility to user as suggested by Andi Kleen <andi@firstfloor.org>
> 
> v2: Made modular since there was no longer a reason not to
>     Moved to x86 platform as suggested by Mathhew Garrett <mjg59@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 */

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.

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

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.

> +
> +	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.e.box@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
> +
> +/* 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
> + * @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);
> +
> +#endif /* INTEL_BAYTRAIL_MBI_SYMS_H */

Are the users of the interface supposed to include this file?

Rafael

Re: [PATCH v5][RESEND] X86 platform: New BayTrail IOSF-SB MBI driver

[David E. Box]

On Fri, Dec 20, 2013 at 02:59:29AM +0100, Rafael J. Wysocki wrote:
> On Thursday, December 19, 2013 02:37:46 PM David E. Box wrote:
> > From: "David E. Box" <david.e.box@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.e.box@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.gao@linux.intel.com>
> >     Renamed register macros and API funcitons to platform specific names.
> >     Changed dependency to PNPACPI as sugessted by Rafael Wysocki <rjw@rjwysocki.net>
> > 
> > v3: Converted to PNP ACPI driver as sugessted by Rafael Wysocki <rjw@rjwysocki.net>
> >     Removed config visibility to user as suggested by Andi Kleen <andi@firstfloor.org>
> > 
> > v2: Made modular since there was no longer a reason not to
> >     Moved to x86 platform as suggested by Mathhew Garrett <mjg59@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 */
> 
> 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?
> 

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

> > +	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);
> 
> Is that a good enough reason to crash the kernel?  Maybe WARN_ON() + return error
> would be sufficient?
> 

Ok.

> > +
> > +	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.
> 
> > +
> > +	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.e.box@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
> > +
> > +/* 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
> > + * @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);
> > +
> > +#endif /* INTEL_BAYTRAIL_MBI_SYMS_H */
> 
> Are the users of the interface supposed to include this file?
>

Only for the macro definitions.

> Rafael

- Dave

[PATCH v6] X86 platform: New BayTrail IOSF-SB MBI driver

[David E. Box]

From: "David E. Box" <david.e.box@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.e.box@linux.intel.com>
---
v6: Changed BUG_ON to WARN_ON on attempted access to GFX port as suggested by
    Rafael Wysocki <rjw@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.gao@linux.intel.com>
    Renamed register macros and API funcitons to platform specific names.
    Changed dependency to PNPACPI as suggested by Rafael Wysocki <rjw@rjwysocki.net>

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

v2: Made modular since there was no longer a reason not to
    Moved to x86 platform as suggested by Mathhew Garrett <mjg59@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 @@
+/*
+ * 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 */
+
+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;
+	}
+
+	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.e.box@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
+
+/* 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
+ * @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);
+
+#endif /* INTEL_BAYTRAIL_MBI_SYMS_H */
-- 
1.7.10.4

[PATCH v6][RESEND] platform: x86: New BayTrail IOSF-SB MBI driver

[David E. Box]

From: "David E. Box" <david.e.box@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.e.box@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.gao@linux.intel.com>
    Renamed register macros and API funcitons to platform specific names.
    Changed dependency to PNPACPI as sugessted by Rafael Wysocki <rjw@rjwysocki.net>

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

v2: Made modular since there was no longer a reason not to
    Moved to x86 platform as suggested by Mathhew Garrett <mjg59@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 @@
+/*
+ * 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 */
+
+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;
+	}
+
+	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.e.box@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
+
+/* 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
+ * @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);
+
+#endif /* INTEL_BAYTRAIL_MBI_SYMS_H */
-- 
1.7.10.4

Re: [PATCH v6][RESEND] platform: x86: New BayTrail IOSF-SB MBI driver

[Randy Dunlap]

On 01/07/14 10:03, David E. Box wrote:
> From: "David E. Box" <david.e.box@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

Re: [PATCH v6][RESEND] platform: x86: New BayTrail IOSF-SB MBI driver

[David E. Box]

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.e.box@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

Re: [PATCH v6][RESEND] platform: x86: New BayTrail IOSF-SB MBI driver

[Randy Dunlap]

On 01/07/14 10:48, David E. Box wrote:
> 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.e.box@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.

I see.  Thanks.


-- 
~Randy

Re: [PATCH v6][RESEND] platform: x86: New BayTrail IOSF-SB MBI driver

[Rafael J. Wysocki]

On Tuesday, January 07, 2014 10:48:05 AM David E. Box wrote:
> 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.e.box@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.

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.

Re: [PATCH v6][RESEND] platform: x86: New BayTrail IOSF-SB MBI driver

[David E. Box]

On Tue, Jan 07, 2014 at 09:46:57PM +0100, Rafael J. Wysocki wrote:
> On Tuesday, January 07, 2014 10:48:05 AM David E. Box wrote:
> > 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.e.box@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.
> 
> What about generic x86 distro kernels?  They won't know in advance whether or
> not they will need this feature.

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

Re: [PATCH v6][RESEND] platform: x86: New BayTrail IOSF-SB MBI driver

[Rafael J. Wysocki]

On Tuesday, January 07, 2014 01:43:00 PM David E. Box wrote:
> On Tue, Jan 07, 2014 at 09:46:57PM +0100, Rafael J. Wysocki wrote:
> > On Tuesday, January 07, 2014 10:48:05 AM David E. Box wrote:
> > > 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.e.box@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.
> > 
> > What about generic x86 distro kernels?  They won't know in advance whether or
> > not they will need this feature.
> 
> 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.

Thanks!

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

Re: [PATCH v6][RESEND] platform: x86: New BayTrail IOSF-SB MBI driver

[H. Peter Anvin]

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

Re: [PATCH v6][RESEND] platform: x86: New BayTrail IOSF-SB MBI driver

[David E. Box]

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

Re: [PATCH v6][RESEND] platform: x86: New BayTrail IOSF-SB MBI driver

[Rafael J. Wysocki]

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.