Skip to contentSkip to conversation

IPC driver for Intel Mobile Internet Device (MID) platforms

PATCH Applied Alan Cox 2 participants 10 messages Apr 23, 2010 – May 7, 2010

Subsystems (4)
lore
Share & tools
lore

Alan Cox

author
#0 lore raw

PATCH 1/1 IPC driver for Intel Mobile Internet Device (MID) platforms

From: Sreedhara DS <[email protected]>

The IPC (inter processor communications) is used to provide the communications between kernel and system control units on some embedded Intel x86 platforms.

(Various bits of cleanup - Alan Cox)

---

 arch/x86/include/asm/intel_scu_ipc.h |  101 ++++
 drivers/platform/x86/Kconfig         |    8 
 drivers/platform/x86/Makefile        |    1 
 drivers/platform/x86/intel_scu_ipc.c |  945 ++++++++++++++++++++++++++++++++++
 4 files changed, 1055 insertions(+), 0 deletions(-)
 create mode 100644 arch/x86/include/asm/intel_scu_ipc.h
 create mode 100644 drivers/platform/x86/intel_scu_ipc.c
arch/x86/include/asm/intel_scu_ipc.h new file +101 -0
new file mode 100644
index 0000000..55303d6
@@ -0,0 +1,101 @@
+#ifndef _ASM_X86_INTEL_SCU_IPC_H_
+#define  _ASM_X86_INTEL_SCU_IPC_H_
+
+struct scu_fw_version {
+	u8 rtMinorVer;		/* SCU RT Firmware Minor Revision */
+	u8 rtMajorVer;		/* SCU RT Firmware Major Revision */
+	u8 romMinorVer;		/* SCU ROM Firmware Minor Revision */
+	u8 romMajorVer;		/* SCU ROM Firmware Major Revision */
+	u8 punitMinorVer;	/* P-unit Microcode Minor Revision */
+	u8 punitMajorVer;	/* P-unit Microcode Major Revision */
+	u8 iaMinorVer;		/* IA-32 Firmware Minor Revision */
+	u8 iaMajorVer;		/* IA-32 Firmware Major Revision */
+	u8 ftlMinorVer;		/* FTL Driver Minor Revision */
+	u8 ftlMajorVer;		/* FTL Driver Major Revision */
+	u8 valMinorVer;		/* Validation Hooks Minor Revision */
+	u8 valMajorVer;		/* Validation Hooks Major Revision */
+	u8 ifwiMinorVer;	/* IFWI Minor Revision */
+	u8 ifwiMajorVer;	/* IFWI Major Revision */
+} ;
+
+struct battery_property {
+	u32 capacity;	/* Charger capacity */
+	u8  crnt;	/* Quick charge current value*/
+	u8  volt;	/* Fine adjustment of constant charge voltage */
+	u8  prot;	/* CHRGPROT register value */
+	u8  prot2;	/* CHRGPROT1 register value */
+	u8 timer;	/* Charging timer */
+} __packed;
+
+/* Read single register */
+int intel_scu_ipc_ioread8(u16 addr, u8 *data);
+
+/* Read two registers */
+int intel_scu_ipc_ioread16(u16 addr, u16 *data);
+
+/* Read four registers */
+int intel_scu_ipc_ioread32(u16 addr, u32 *data);
+
+/* Read a vector */
+int intel_scu_ipc_readv(u16 *addr, u8 *data, int len);
+
+/* Write single register */
+int intel_scu_ipc_iowrite8(u16 addr, u8 data);
+
+/* Write two registers */
+int intel_scu_ipc_iowrite16(u16 addr, u16 data);
+
+/* Write four registers */
+int intel_scu_ipc_iowrite32(u16 addr, u32 data);
+
+/* Write a vector */
+int intel_scu_ipc_writev(u16 *addr, u8 *data, int len);
+
+/*
+ * Update single register based on the mask
+ * First element of data is register value and second element is mask value
+ */
+int intel_scu_ipc_update_register(u16 addr, u8 data[2]);
+
+/*
+ * Indirect register read
+ * Can be used when SCCB(System Controller Configuration Block) register
+ * HRIM(Honor Restricted IPC Messages) is set (bit 23)
+ */
+int intel_scu_ipc_register_read(u32 addr, u32 *data);
+
+/*
+ * Indirect register write
+ * Can be used when SCCB(System Controller Configuration Block) register
+ * HRIM(Honor Restricted IPC Messages) is set (bit 23)
+ */
+int intel_scu_ipc_register_write(u32 addr, u32 data);
+
+int intel_scu_ipc_simple_command(int cmd, int sub);
+
+/* Read the coulomb counter accumulator to monitor the battery charge level */
+int intel_scu_ipc_battery_cc_read(u32 *value);
+
+/* Write the coulomb counter accumulator */
+int intel_scu_ipc_battery_cc_write(u32 value);
+
+/* Read battery properties */
+int intel_scu_ipc_battery_property(struct battery_property *prop);
+
+/*
+ * Used to enable Timer Services for Kernel Watchdog Operation permitting
+ * automatic reset or reboot of the system if tripped.
+ * Set watchdog timer value, returns 0 on success, 1 on error
+ */
+int intel_scu_ipc_set_watchdog(u32 warn_threshold, u32 reset_threshold);
+
+/* I2C control api */
+int intel_scu_ipc_i2c_cntrl(u32 addr, u32 *data);
+
+/* Read FW version */
+int intel_scu_ipc_fw_version(struct scu_fw_version *version);
+
+/* Update FW version */
+int intel_scu_ipc_fw_update(u8 *buffer, u32 length);
+
+#endif
drivers/platform/x86/Kconfig +8 -0
index 7bec458..09060a0 100644
@@ -526,4 +526,12 @@ config ACPI_CMPC
 	  keys as input device, backlight device, tablet and accelerometer
 	  devices.
 
+config INTEL_SCU_IPC
+	bool "Intel SCU IPC Support"
+	depends on X86_MRST
+	default y
+	---help---
+	  IPC is used to bridge the communications between kernel and SCU on
+	  some embedded Intel x86 platforms.
+
 endif # X86_PLATFORM_DEVICES
drivers/platform/x86/Makefile +1 -0
index a906490..8770bfe 100644
@@ -25,3 +25,4 @@ obj-$(CONFIG_ACPI_ASUS)		+= asus_acpi.o
 obj-$(CONFIG_TOPSTAR_LAPTOP)	+= topstar-laptop.o
 obj-$(CONFIG_ACPI_TOSHIBA)	+= toshiba_acpi.o
 obj-$(CONFIG_TOSHIBA_BT_RFKILL)	+= toshiba_bluetooth.o
+obj-$(CONFIG_INTEL_SCU_IPC)	+= intel_scu_ipc.o
drivers/platform/x86/intel_scu_ipc.c new file +945 -0

945 lines not on this page · raw message

#1 lore raw

On Fri, Apr 23, 2010 at 03:30:59PM +0100, Alan Cox wrote:

Seems basically fine, with a few comments inline. One not especially related query in passing - the graphics driver for Moorestown hasn't been submitted yet, and the version in Meego doesn't look like it's especially upstreamable. I seem to recall that Moorestown was going to lack port IO, so does it have VGA compatibility? If not, is there any use in mainline Moorestown support until support for the graphics is submitted?

arch/x86/include/asm/intel_scu_ipc.h line 37

Quoted 8 lines
arch/x86/include/asm/intel_scu_ipc.h new file +8 -0
+/* Read single register */
+int intel_scu_ipc_ioread8(u16 addr, u8 *data);
+
+/* Read two registers */
+int intel_scu_ipc_ioread16(u16 addr, u16 *data);
+
+/* Read four registers */
+int intel_scu_ipc_ioread32(u16 addr, u32 *data);

Are these comments really needed?

Quoted 4 lines

+ * Update single register based on the mask
+ * First element of data is register value and second element is mask value
+ */
+int intel_scu_ipc_update_register(u16 addr, u8 data[2]);

Would seem neater as two separate arguments rather than an array...

Quoted 6 lines

+ * SCU runing in ARC processor communicates with other entity running in IA
+ * core through IPC mechanism which in turn messaging between IA core ad SCU.
+ * SCU has two IPC mechanism IPC-1 and IPC-2. IPC-1 is used between IA32 and
+ * SCU where IPC-2 is used between P-Unit and SCU. This driver delas with
+ * IPC-1 Driver provides an API for power control unit registers (e.g. MSIC)
+ * along with other APIs.

This isn't especially clear. I think I understand what it's talking about (There's a service processor and an x86, and this driver is responsible for handing the communication between the two - but there's also another IPC mechanism for talking to P-Unit, which isn't explained here but at a guess is some kind of hip-hop group and thus outside the scope of this driver?

Quoted 1 line

+#define IPC_BASE_ADDR 0xFF11C000 /* IPC1 base register address */

The address is guaranteed to be stable? This doesn't need to come from SFI or anything?

Quoted 1 line

+static struct intel_scu_ipc_dev ipcdev; /* Only one for now */

If this is going to change in future then it might be sane to get it right now.

drivers/platform/x86/intel_scu_ipc.c line 83

Quoted 3 lines
drivers/platform/x86/intel_scu_ipc.c new file +3 -0
+static int platform = 1;
+module_param(platform, int, 0);
+MODULE_PARM_DESC(platform, "1 for moorestown platform");

What do non-1 values mean? It looks like everything other than 1 provides some default behaviour. Could you check for invalid values and abort in order to avoid breakage if someone depends on this?

Quoted 1 line

+static inline u8 ipc_readl(u32 offset) /* Read ipc u32 data */

read16 in places and readl in others?

Quoted 1 line

+int intel_scu_ipc_battery_cc_read(u32 *value)

I'm a bit confused by the cc functions. The actual battery code isn't in this driver, so why are these functions not in the battery driver?

Quoted 1 line

+int intel_scu_ipc_i2c_cntrl(u32 addr, u32 *data)

Same here. If the i2c functionality is in this device, why not keep the full i2c driver code here?

drivers/platform/x86/intel_scu_ipc.c line 678

Quoted 5 lines
drivers/platform/x86/intel_scu_ipc.c new file +5 -0
+	mutex_lock(&ipclock);
+	cmd = (addr >> 24) & 0xFF;
+	if (cmd == IPC_I2C_READ) {
+		writel(addr, ipcdev.i2c_base + IPC_I2C_CNTRL_ADDR);
+		/* Write not getting updated without delay */

It needs a delay rather than a posting read?

Quoted 4 lines

+/*
+ * Interrupt handler gets called when ioc bit of IPC_COMMAND_REG set to 1
+ * When ioc bit is set to 1, caller api must wait for interrupt handler called
+ * which in turn unlocks the caller api. Currently this is not used

Is this a problem? It looks like ioc can only be set if someone passes it in via intel_scu_ipc_simple_command() and that's a pretty obvious "Don't do that", but it'd be nice to get rid of the busy looping.

All of these symbols are EXPORT_SYMBOL. Is there a reason they're not EXPORT_SYMBOL_GPL?

Signature · 1 line
Matthew Garrett | [email protected]
author
#2 lore raw
Quoted 6 lines

related query in passing - the graphics driver for Moorestown hasn't been submitted yet, and the version in Meego doesn't look like it's especially upstreamable. I seem to recall that Moorestown was going to lack port IO, so does it have VGA compatibility? If not, is there any use in mainline Moorestown support until support for the graphics is submitted?

Graphics is very very late on the merge list for obvious reasons. It's not even practical to start looking at upstreaming any of the graphics stuff until the core platform support is present and you've got a base platform to work from. A fair bit of the core MRST stuff is already merged (eg SFI).

Quoted 10 lines

arch/x86/include/asm/intel_scu_ipc.h line 37

Quoted 8 lines
arch/x86/include/asm/intel_scu_ipc.h new file +8 -0
+/* Read single register */
+int intel_scu_ipc_ioread8(u16 addr, u8 *data);
+
+/* Read two registers */
+int intel_scu_ipc_ioread16(u16 addr, u16 *data);
+
+/* Read four registers */
+int intel_scu_ipc_ioread32(u16 addr, u32 *data);

Are these comments really needed?

I left them in because the IPC is seeing four 8bit registers on its side not one 32bit 'register'. It's not usually an important distinction but it can be.

Quoted 7 lines
Quoted 5 lines

+ * Update single register based on the mask
+ * First element of data is register value and second element is mask value
+ */
+int intel_scu_ipc_update_register(u16 addr, u8 data[2]);

Would seem neater as two separate arguments rather than an array...

There are patches further down the pile that correct that one funnily enough.

Quoted 3 lines

there's also another IPC mechanism for talking to P-Unit, which isn't explained here but at a guess is some kind of hip-hop group and thus outside the scope of this driver?

Correct except for the hip-hop group. Although hopefully someone will run out and form P-Unit at this point.

Quoted 5 lines
Quoted 2 lines

+#define IPC_BASE_ADDR 0xFF11C000 /* IPC1 base register
address */

The address is guaranteed to be stable? This doesn't need to come from SFI or anything?

It's stable. Getting it to come out of the "PCI" BARs would make many people happy.

Quoted 4 lines
Quoted 1 line

+static struct intel_scu_ipc_dev ipcdev; /* Only one for now */

If this is going to change in future then it might be sane to get it right now.

I can't possibly comment on unreleased product and plans but I'm quite happy with it as is.

Quoted 7 lines

drivers/platform/x86/intel_scu_ipc.c line 83

Quoted 3 lines
drivers/platform/x86/intel_scu_ipc.c new file +3 -0
+static int platform = 1;
+module_param(platform, int, 0);
+MODULE_PARM_DESC(platform, "1 for moorestown platform");

What do non-1 values mean? It looks like everything other than 1 provides some default behaviour. Could you check for invalid values and abort in order to avoid breakage if someone depends on this?

There are two message formats (don't blame me I wasn't involved !) depending upon the exact platform. This will eventually get autodetected but Ingo hasn't even responded to that patch despite three sends of it. We already sort of expose the needed info but at the moment it requires drivers go rudely grovelling into x86 boot struct internals because there is no wrapper function.

(cc Ingo to prod him again)

Quoted 3 lines
Quoted 1 line

+static inline u8 ipc_readl(u32 offset) /* Read ipc u32 data */

read16 in places and readl in others?

One is the exposed external API, the other is a little one line internal helper that does unrelated things.

Quoted 4 lines
Quoted 1 line

+int intel_scu_ipc_battery_cc_read(u32 *value)

I'm a bit confused by the cc functions. The actual battery code isn't in this driver, so why are these functions not in the battery driver?

Because the locking is in this driver. You can either smear the locking all over the kernel or put the message format aware functions in one place. I have some ideas how to tackle this better longer term.

Quoted 4 lines
Quoted 1 line

+int intel_scu_ipc_i2c_cntrl(u32 addr, u32 *data)

Same here. If the i2c functionality is in this device, why not keep the full i2c driver code here?

Good question - we don't need the I2C bit yet so we could just drop it for the moment and add it back layer wherever it belongs (however see 'smear the locking all over')

Quoted 8 lines

drivers/platform/x86/intel_scu_ipc.c line 678

Quoted 6 lines
drivers/platform/x86/intel_scu_ipc.c new file +5 -0
+	mutex_lock(&ipclock);
+	cmd = (addr >> 24) & 0xFF;
+	if (cmd == IPC_I2C_READ) {
+		writel(addr, ipcdev.i2c_base + IPC_I2C_CNTRL_ADDR);
+		/* Write not getting updated without delay */

It needs a delay rather than a posting read?

It's not PCI bus, it's magic ;)

Quoted 11 lines
Quoted 6 lines

+/*
+ * Interrupt handler gets called when ioc bit of IPC_COMMAND_REG set to 1
+ * When ioc bit is set to 1, caller api must wait for interrupt handler called
+ * which in turn unlocks the caller api. Currently this is not used

Is this a problem? It looks like ioc can only be set if someone passes it in via intel_scu_ipc_simple_command() and that's a pretty obvious "Don't do that", but it'd be nice to get rid of the busy looping.

If anything the direction will continue to be not using the interrupt. We catch the IRQ just in case an IRQ pops up from somewhere.

Quoted 2 lines

All of these symbols are EXPORT_SYMBOL. Is there a reason they're not EXPORT_SYMBOL_GPL?

That is how I inherited the driver code and it's not a driver internal interface so I believe EXPORT_SYMBOL is correct based upon Linus description of _GPL.

#3 lore raw
Quoted 7 lines · Alan Cox
Quoted 6 lines

related query in passing - the graphics driver for Moorestown hasn't been submitted yet, and the version in Meego doesn't look like it's especially upstreamable. I seem to recall that Moorestown was going to lack port IO, so does it have VGA compatibility? If not, is there any use in mainline Moorestown support until support for the graphics is submitted?

Quoted 5 lines

Graphics is very very late on the merge list for obvious reasons. It's not even practical to start looking at upstreaming any of the graphics stuff until the core platform support is present and you've got a base platform to work from. A fair bit of the core MRST stuff is already merged (eg SFI).

The case I'm worried about is where the graphics driver never gets into a mergable state and we end up with a pile of mainline code that can't sanely run on the hardware. As long as the plan is to get it to avoid duplicating the entire Intel modesetting code and including its own TTM then that seems fair enough, but Intel's track record on this side of things hasn't been great.

Quoted 4 lines
Quoted 1 line

Would seem neater as two separate arguments rather than an array...

There are patches further down the pile that correct that one funnily enough.

Good to know. Worth getting them in before merging, given that it changes the interface?

Quoted 6 lines
Quoted 3 lines

there's also another IPC mechanism for talking to P-Unit, which isn't explained here but at a guess is some kind of hip-hop group and thus outside the scope of this driver?

Correct except for the hip-hop group. Although hopefully someone will run out and form P-Unit at this point.

Ok. It'd be nice if the comment were slightly clearer.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

+#define IPC_BASE_ADDR 0xFF11C000 /* IPC1 base register
address */

The address is guaranteed to be stable? This doesn't need to come from SFI or anything?

It's stable. Getting it to come out of the "PCI" BARs would make many people happy.

But not possible at the moment? Fair enough.

Quoted 8 lines
Quoted 7 lines

drivers/platform/x86/intel_scu_ipc.c line 83

Quoted 3 lines
drivers/platform/x86/intel_scu_ipc.c new file +3 -0
+static int platform = 1;
+module_param(platform, int, 0);
+MODULE_PARM_DESC(platform, "1 for moorestown platform");

What do non-1 values mean? It looks like everything other than 1 provides some default behaviour. Could you check for invalid values and abort in order to avoid breakage if someone depends on this?

Quoted 6 lines

There are two message formats (don't blame me I wasn't involved !) depending upon the exact platform. This will eventually get autodetected but Ingo hasn't even responded to that patch despite three sends of it. We already sort of expose the needed info but at the moment it requires drivers go rudely grovelling into x86 boot struct internals because there is no wrapper function.

Ok. I'd prefer it if passing random numbers resulted in failure rather than default behaviour, but if there's never going to be another platform that uses the Moorestown format then that's not a real problem.

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

+static inline u8 ipc_readl(u32 offset) /* Read ipc u32 data */

read16 in places and readl in others?

One is the exposed external API, the other is a little one line internal helper that does unrelated things.

It just made me double-take.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

+int intel_scu_ipc_battery_cc_read(u32 *value)

I'm a bit confused by the cc functions. The actual battery code isn't in this driver, so why are these functions not in the battery driver?

Because the locking is in this driver. You can either smear the locking all over the kernel or put the message format aware functions in one place. I have some ideas how to tackle this better longer term.

Ok.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

+int intel_scu_ipc_i2c_cntrl(u32 addr, u32 *data)

Same here. If the i2c functionality is in this device, why not keep the full i2c driver code here?

Good question - we don't need the I2C bit yet so we could just drop it for the moment and add it back layer wherever it belongs (however see 'smear the locking all over')

Fair enough.

Quoted 6 lines
Quoted 2 lines

All of these symbols are EXPORT_SYMBOL. Is there a reason they're not EXPORT_SYMBOL_GPL?

That is how I inherited the driver code and it's not a driver internal interface so I believe EXPORT_SYMBOL is correct based upon Linus description of _GPL.

Ok. We expect the platform to be usable without non-GPL drivers that require the IPC mechanism?

Signature · 1 line
Matthew Garrett | [email protected]
author
#4 lore raw
Quoted 6 lines

The case I'm worried about is where the graphics driver never gets into a mergable state and we end up with a pile of mainline code that can't sanely run on the hardware. As long as the plan is to get it to avoid duplicating the entire Intel modesetting code and including its own TTM then that seems fair enough, but Intel's track record on this side of things hasn't been great.

I don't track the public DRM lists but the last I saw on that on the archives was Keith Packard blocking patches to enable such a merge. You'd have to ask him about it. On the acceleration side mrst/psb graphics are not i9xx style.

Quoted 5 lines
Quoted 2 lines

There are patches further down the pile that correct that one funnily enough.

Good to know. Worth getting them in before merging, given that it changes the interface?

I'll pull that one in and push the I²C out.

Quoted 4 lines

Ok. I'd prefer it if passing random numbers resulted in failure rather than default behaviour, but if there's never going to be another platform that uses the Moorestown format then that's not a real problem.

I'd like to think the format would remain the same.

Quoted 4 lines
Quoted 2 lines

One is the exposed external API, the other is a little one line internal helper that does unrelated things.

It just made me double-take.

Good reason to rename it.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
Quoted 1 line

+int intel_scu_ipc_battery_cc_read(u32 *value)

I'm a bit confused by the cc functions. The actual battery code isn't in this driver, so why are these functions not in the battery driver?

Quoted 6 lines
Quoted 4 lines

Because the locking is in this driver. You can either smear the locking all over the kernel or put the message format aware functions in one place. I have some ideas how to tackle this better longer term.

Ok.

Having had a wander down the beach and watched the waves a bit I've got a cunning plan. Watch this space...

Quoted 6 lines
Quoted 3 lines

That is how I inherited the driver code and it's not a driver internal interface so I believe EXPORT_SYMBOL is correct based upon Linus description of _GPL.

Ok. We expect the platform to be usable without non-GPL drivers that require the IPC mechanism?

Certainly. The Meego tree as is should give you a usable platform (barring X graphics accel which isn't something I am in a position to fix unfortunately). There is lots in that tree which isn't pretty and I'm trying to get stuff out of that tree into the kernel while doing the laundry on the way.

Alan
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw
Quoted 11 lines · Alan Cox
Quoted 6 lines

The case I'm worried about is where the graphics driver never gets into a mergable state and we end up with a pile of mainline code that can't sanely run on the hardware. As long as the plan is to get it to avoid duplicating the entire Intel modesetting code and including its own TTM then that seems fair enough, but Intel's track record on this side of things hasn't been great.

I don't track the public DRM lists but the last I saw on that on the archives was Keith Packard blocking patches to enable such a merge. You'd have to ask him about it. On the acceleration side mrst/psb graphics are not i9xx style.

I'm really not terribly comfortable merging enabling code for an architecture if there's no clear plan for merging critical parts of that architecture.

Signature · 1 line
Matthew Garrett | [email protected]
author
#6 lore raw
Quoted 3 lines

I'm really not terribly comfortable merging enabling code for an architecture if there's no clear plan for merging critical parts of that architecture.

I'm not interested in playing graphics driver politics. If I could do something short term about the graphics situation I would. Furthermore there isn't anything I can do about it (even if it does involve its own *clean* mode setting to keep KeithP from exploding) until the core merges are done.

As has already been pointed out large parts of the enabling code are already merged into other bits of the kernel.

Alan

#7 lore raw
Quoted 9 lines · Alan Cox
Quoted 3 lines

I'm really not terribly comfortable merging enabling code for an architecture if there's no clear plan for merging critical parts of that architecture.

I'm not interested in playing graphics driver politics. If I could do something short term about the graphics situation I would. Furthermore there isn't anything I can do about it (even if it does involve its own *clean* mode setting to keep KeithP from exploding) until the core merges are done.

It's been pointed out to me that there are non-graphical targets for this platform as well, so in that case there's no real argument on my side. I'll queue this for .35.

Signature · 1 line
Matthew Garrett | [email protected]
author
#8 lore raw
Quoted 11 lines · On Mon, 26 Apr 2010 18:06:16 +0100 Matthew Garrett <[email protected]> wrote:
Quoted 9 lines · Alan Cox
Quoted 3 lines

I'm really not terribly comfortable merging enabling code for an architecture if there's no clear plan for merging critical parts of that architecture.

I'm not interested in playing graphics driver politics. If I could do something short term about the graphics situation I would. Furthermore there isn't anything I can do about it (even if it does involve its own *clean* mode setting to keep KeithP from exploding) until the core merges are done.

Quoted 3 lines

It's been pointed out to me that there are non-graphical targets for this platform as well, so in that case there's no real argument on my side. I'll queue this for .35.

Here's a better one. This has the modify API fix pulled in and the various command locks fixed by the simple expedient of passing copies of the data around. As its at most about 5 dwords it's not going to kill anyone and it cleans up a fair bit

Alan
--

commit e0d3f5da70f26f39853a000a7cfc9575047c63d7

Author: Sreedhara DS <[email protected]>
Date:   Mon Apr 26 17:05:45 2010 +0100

IPC driver for Intel Mobile Internet Device (MID) platforms

The IPC (inter processor communications) is used to provide the communications between kernel and system control units on some embedded Intel x86 platforms.

(Various bits of clean up and restructuring by Alan Cox)

arch/x86/include/asm/intel_scu_ipc.h new file +55 -0
new file mode 100644
index 0000000..6c6170b
@@ -0,0 +1,55 @@
+#ifndef _ASM_X86_INTEL_SCU_IPC_H_
+#define  _ASM_X86_INTEL_SCU_IPC_H_
+
+/* Read single register */
+int intel_scu_ipc_ioread8(u16 addr, u8 *data);
+
+/* Read two sequential registers */
+int intel_scu_ipc_ioread16(u16 addr, u16 *data);
+
+/* Read four sequential registers */
+int intel_scu_ipc_ioread32(u16 addr, u32 *data);
+
+/* Read a vector */
+int intel_scu_ipc_readv(u16 *addr, u8 *data, int len);
+
+/* Write single register */
+int intel_scu_ipc_iowrite8(u16 addr, u8 data);
+
+/* Write two sequential registers */
+int intel_scu_ipc_iowrite16(u16 addr, u16 data);
+
+/* Write four sequential registers */
+int intel_scu_ipc_iowrite32(u16 addr, u32 data);
+
+/* Write a vector */
+int intel_scu_ipc_writev(u16 *addr, u8 *data, int len);
+
+/* Update single register based on the mask */
+int intel_scu_ipc_update_register(u16 addr, u8 data, u8 mask);
+
+/*
+ * Indirect register read
+ * Can be used when SCCB(System Controller Configuration Block) register
+ * HRIM(Honor Restricted IPC Messages) is set (bit 23)
+ */
+int intel_scu_ipc_register_read(u32 addr, u32 *data);
+
+/*
+ * Indirect register write
+ * Can be used when SCCB(System Controller Configuration Block) register
+ * HRIM(Honor Restricted IPC Messages) is set (bit 23)
+ */
+int intel_scu_ipc_register_write(u32 addr, u32 data);
+
+/* Issue commands to the SCU with or without data */
+int intel_scu_ipc_simple_command(int cmd, int sub);
+int intel_scu_ipc_command(int cmd, int sub, u32 *in, int inlen,
+							u32 * out, int outlen);
+/* I2C control api */
+int intel_scu_ipc_i2c_cntrl(u32 addr, u32 *data);
+
+/* Update FW version */
+int intel_scu_ipc_fw_update(u8 *buffer, u32 length);
+
+#endif
drivers/platform/x86/Kconfig +8 -0
index 7bec458..09060a0 100644
@@ -526,4 +526,12 @@ config ACPI_CMPC
 	  keys as input device, backlight device, tablet and accelerometer
 	  devices.
 
+config INTEL_SCU_IPC
+	bool "Intel SCU IPC Support"
+	depends on X86_MRST
+	default y
+	---help---
+	  IPC is used to bridge the communications between kernel and SCU on
+	  some embedded Intel x86 platforms.
+
 endif # X86_PLATFORM_DEVICES
drivers/platform/x86/Makefile +1 -0
index a906490..8770bfe 100644
@@ -25,3 +25,4 @@ obj-$(CONFIG_ACPI_ASUS)		+= asus_acpi.o
 obj-$(CONFIG_TOPSTAR_LAPTOP)	+= topstar-laptop.o
 obj-$(CONFIG_ACPI_TOSHIBA)	+= toshiba_acpi.o
 obj-$(CONFIG_TOSHIBA_BT_RFKILL)	+= toshiba_bluetooth.o
+obj-$(CONFIG_INTEL_SCU_IPC)	+= intel_scu_ipc.o
drivers/platform/x86/intel_scu_ipc.c new file +829 -0

829 lines not on this page · raw message