linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Enable more Intel Mid platforms on IPC driver
@ 2013-11-13 20:14 David Cohen
  2013-11-13 20:14 ` [PATCH 1/3] ipc: Added platform data structure David Cohen
                   ` (3 more replies)
  0 siblings, 4 replies; 22+ messages in thread
From: David Cohen @ 2013-11-13 20:14 UTC (permalink / raw)
  To: matthew.garrett
  Cc: platform-driver-x86, linux-kernel, eric.ernst, David Cohen

Hi,

I am resubmitting these patches to enable other Intel Mid platforms on IPC
driver.
This change is necessary prior to enable more features for Mid on upstream.

Br, David Cohen

---
Kuppuswamy Sathyanarayanan (3):
  ipc: Added platform data structure
  ipc: Enabled ipc support for additional intel platforms
  ipc: Added support for IPC interrupt mode

 drivers/platform/x86/Kconfig         |  23 +++++++
 drivers/platform/x86/intel_scu_ipc.c | 114 +++++++++++++++++++++++++++++++----
 2 files changed, 126 insertions(+), 11 deletions(-)

-- 
1.8.4.2


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

* [PATCH 1/3] ipc: Added platform data structure
  2013-11-13 20:14 [PATCH 0/3] Enable more Intel Mid platforms on IPC driver David Cohen
@ 2013-11-13 20:14 ` David Cohen
  2013-11-14 13:42   ` One Thousand Gnomes
  2013-11-13 20:14 ` [PATCH 2/3] ipc: Enabled ipc support for additional intel platforms David Cohen
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 22+ messages in thread
From: David Cohen @ 2013-11-13 20:14 UTC (permalink / raw)
  To: matthew.garrett
  Cc: platform-driver-x86, linux-kernel, eric.ernst,
	Kuppuswamy Sathyanarayanan, David Cohen

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Since the same ipc driver can be used by many platforms, using
macros for defining ipc_base and i2c_base addresses is not
a scalable approach. So added a platform data structure to pass
this information.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: David Cohen <david.a.cohen@linux.intel.com>
---
 drivers/platform/x86/intel_scu_ipc.c | 37 ++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index d654f831410d..cc8e4c7d3629 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -58,12 +58,29 @@
  *    message handler is called within firmware.
  */
 
-#define IPC_BASE_ADDR     0xFF11C000	/* IPC1 base register address */
-#define IPC_MAX_ADDR      0x100		/* Maximum IPC regisers */
 #define IPC_WWBUF_SIZE    20		/* IPC Write buffer Size */
 #define IPC_RWBUF_SIZE    20		/* IPC Read buffer Size */
-#define IPC_I2C_BASE      0xFF12B000	/* I2C control register base address */
-#define IPC_I2C_MAX_ADDR  0x10		/* Maximum I2C regisers */
+
+enum {
+	SCU_IPC_LINCROFT,
+};
+
+/* intel scu ipc driver data*/
+struct intel_scu_ipc_pdata_t {
+	u32 ipc_base;
+	u32 i2c_base;
+	u32 ipc_len;
+	u32 i2c_len;
+};
+
+static struct intel_scu_ipc_pdata_t intel_scu_ipc_pdata[] = {
+	[SCU_IPC_LINCROFT] = {
+		.ipc_base = 0xff11c000,
+		.i2c_base = 0xff12b000,
+		.ipc_len = 0x100,
+		.i2c_len = 0x10,
+	},
+};
 
 static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id);
 static void ipc_remove(struct pci_dev *pdev);
@@ -504,12 +521,16 @@ static irqreturn_t ioc(int irq, void *dev_id)
  */
 static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
-	int err;
+	int err, pid;
+	struct intel_scu_ipc_pdata_t *pdata;
 	resource_size_t pci_resource;
 
 	if (ipcdev.pdev)		/* We support only one SCU */
 		return -EBUSY;
 
+	pid = id->driver_data;
+	pdata = &intel_scu_ipc_pdata[pid];
+
 	ipcdev.pdev = pci_dev_get(dev);
 
 	err = pci_enable_device(dev);
@@ -527,11 +548,11 @@ static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (request_irq(dev->irq, ioc, 0, "intel_scu_ipc", &ipcdev))
 		return -EBUSY;
 
-	ipcdev.ipc_base = ioremap_nocache(IPC_BASE_ADDR, IPC_MAX_ADDR);
+	ipcdev.ipc_base = ioremap_nocache(pdata->ipc_base, pdata->ipc_len);
 	if (!ipcdev.ipc_base)
 		return -ENOMEM;
 
-	ipcdev.i2c_base = ioremap_nocache(IPC_I2C_BASE, IPC_I2C_MAX_ADDR);
+	ipcdev.i2c_base = ioremap_nocache(pdata->i2c_base, pdata->i2c_len);
 	if (!ipcdev.i2c_base) {
 		iounmap(ipcdev.ipc_base);
 		return -ENOMEM;
@@ -564,7 +585,7 @@ static void ipc_remove(struct pci_dev *pdev)
 }
 
 static DEFINE_PCI_DEVICE_TABLE(pci_ids) = {
-	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x082a)},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x082a), SCU_IPC_LINCROFT},
 	{ 0,}
 };
 MODULE_DEVICE_TABLE(pci, pci_ids);
-- 
1.8.4.2


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

* [PATCH 2/3] ipc: Enabled ipc support for additional intel platforms
  2013-11-13 20:14 [PATCH 0/3] Enable more Intel Mid platforms on IPC driver David Cohen
  2013-11-13 20:14 ` [PATCH 1/3] ipc: Added platform data structure David Cohen
@ 2013-11-13 20:14 ` David Cohen
  2013-11-14 13:43   ` One Thousand Gnomes
  2013-11-13 20:14 ` [PATCH 3/3] ipc: Added support for IPC interrupt mode David Cohen
  2013-11-14 22:15 ` [PATCH v2 0/4] Enable more Intel Mid platforms on IPC driver David Cohen
  3 siblings, 1 reply; 22+ messages in thread
From: David Cohen @ 2013-11-13 20:14 UTC (permalink / raw)
  To: matthew.garrett
  Cc: platform-driver-x86, linux-kernel, eric.ernst,
	Kuppuswamy Sathyanarayanan, David Cohen

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Enabled ipc support for penwell, clovertrail & tangier platforms.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: David Cohen <david.a.cohen@linux.intel.com>
---
 drivers/platform/x86/intel_scu_ipc.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index cc8e4c7d3629..4851560c88fe 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -63,6 +63,9 @@
 
 enum {
 	SCU_IPC_LINCROFT,
+	SCU_IPC_PENWELL,
+	SCU_IPC_CLOVERVIEW,
+	SCU_IPC_TANGIER,
 };
 
 /* intel scu ipc driver data*/
@@ -80,6 +83,24 @@ static struct intel_scu_ipc_pdata_t intel_scu_ipc_pdata[] = {
 		.ipc_len = 0x100,
 		.i2c_len = 0x10,
 	},
+	[SCU_IPC_PENWELL] = {
+		.ipc_base = 0xff11c000,
+		.i2c_base = 0xff12b000,
+		.ipc_len = 0x100,
+		.i2c_len = 0x10,
+	},
+	[SCU_IPC_CLOVERVIEW] = {
+		.ipc_base = 0xff11c000,
+		.i2c_base = 0xff12b000,
+		.ipc_len = 0x100,
+		.i2c_len = 0x10,
+	},
+	[SCU_IPC_TANGIER] = {
+		.ipc_base = 0xff009000,
+		.i2c_base  = 0xff00d000,
+		.ipc_len  = 0x100,
+		.i2c_len = 0x10,
+	},
 };
 
 static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id);
@@ -586,6 +607,9 @@ static void ipc_remove(struct pci_dev *pdev)
 
 static DEFINE_PCI_DEVICE_TABLE(pci_ids) = {
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x082a), SCU_IPC_LINCROFT},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x080e), SCU_IPC_PENWELL},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x08ea), SCU_IPC_CLOVERVIEW},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x11a0), SCU_IPC_TANGIER},
 	{ 0,}
 };
 MODULE_DEVICE_TABLE(pci, pci_ids);
-- 
1.8.4.2


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

* [PATCH 3/3] ipc: Added support for IPC interrupt mode
  2013-11-13 20:14 [PATCH 0/3] Enable more Intel Mid platforms on IPC driver David Cohen
  2013-11-13 20:14 ` [PATCH 1/3] ipc: Added platform data structure David Cohen
  2013-11-13 20:14 ` [PATCH 2/3] ipc: Enabled ipc support for additional intel platforms David Cohen
@ 2013-11-13 20:14 ` David Cohen
  2013-11-14 13:48   ` One Thousand Gnomes
  2013-11-14 22:15 ` [PATCH v2 0/4] Enable more Intel Mid platforms on IPC driver David Cohen
  3 siblings, 1 reply; 22+ messages in thread
From: David Cohen @ 2013-11-13 20:14 UTC (permalink / raw)
  To: matthew.garrett
  Cc: platform-driver-x86, linux-kernel, eric.ernst,
	Kuppuswamy Sathyanarayanan, David Cohen

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

This patch adds support for ipc command interrupt mode.

Also added following access mode config options.

CONFIG_INTEL_SCU_IPC_INTR_MODE - Selecting this option
will configure the driver to receive IOC interrupt for
each successful ipc_command.

CONFIG_INTEL_SCU_IPC_POLL_MODE - Makes driver use
polling method to track the command completion status.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: David Cohen <david.a.cohen@linux.intel.com>
---
 drivers/platform/x86/Kconfig         | 23 ++++++++++++++++
 drivers/platform/x86/intel_scu_ipc.c | 53 ++++++++++++++++++++++++++++++++++--
 2 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index b51a7460cc49..ade424c68eee 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -655,6 +655,29 @@ config INTEL_SCU_IPC
 	  some embedded Intel x86 platforms. This is not needed for PC-type
 	  machines.
 
+choice
+	prompt "IPC access mode"
+	depends on INTEL_SCU_IPC
+	default INTEL_SCU_IPC_INTR_MODE
+	---help---
+	Select the desired access mode for IPC call.
+
+config INTEL_SCU_IPC_INTR_MODE
+	bool "Intel SCU IPC interrupt mode"
+	---help---
+	 Selecting this option will configure the driver
+	 to receive IOC interrupt for each successful
+	 ipc_command.
+
+
+config INTEL_SCU_IPC_POLL_MODE
+	bool "Intel SCU IPC polling mode"
+	---help---
+	 Makes driver use polling method to track the
+	 command completion status
+
+endchoice
+
 config INTEL_SCU_IPC_UTIL
 	tristate "Intel SCU IPC utility driver"
 	depends on INTEL_SCU_IPC
diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index 4851560c88fe..52327dd12018 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -60,6 +60,7 @@
 
 #define IPC_WWBUF_SIZE    20		/* IPC Write buffer Size */
 #define IPC_RWBUF_SIZE    20		/* IPC Read buffer Size */
+#define IPC_IOC	          0x100		/* IPC command register IOC bit */
 
 enum {
 	SCU_IPC_LINCROFT,
@@ -110,6 +111,9 @@ struct intel_scu_ipc_dev {
 	struct pci_dev *pdev;
 	void __iomem *ipc_base;
 	void __iomem *i2c_base;
+#ifdef CONFIG_INTEL_SCU_IPC_INTR_MODE
+	struct completion cmd_complete;
+#endif
 };
 
 static struct intel_scu_ipc_dev  ipcdev; /* Only one for now */
@@ -136,7 +140,12 @@ static DEFINE_MUTEX(ipclock); /* lock used to prevent multiple call to SCU */
  */
 static inline void ipc_command(u32 cmd) /* Send ipc command */
 {
+#ifdef CONFIG_INTEL_SCU_IPC_INTR_MODE
+	INIT_COMPLETION(ipcdev.cmd_complete);
+	writel(cmd | IPC_IOC, ipcdev.ipc_base);
+#else
 	writel(cmd, ipcdev.ipc_base);
+#endif
 }
 
 /*
@@ -194,6 +203,37 @@ static inline int busy_loop(void) /* Wait till scu status is busy */
 	return 0;
 }
 
+#ifdef CONFIG_INTEL_SCU_IPC_INTR_MODE
+/* Wait till ipc ioc interrupt is received or timeout in 3 HZ */
+static inline int ipc_wait_for_interrupt(void)
+{
+	int status;
+	int ret = 0;
+
+	if (!wait_for_completion_timeout(&ipcdev.cmd_complete, 3 * HZ)) {
+		ret = -ETIMEDOUT;
+		goto end;
+	}
+
+	status = ipc_read_status();
+
+	if ((status >> 1) & 1)
+		ret = -EIO;
+
+end:
+	return ret;
+}
+#endif
+
+int intel_scu_ipc_check_status(void)
+{
+#ifdef CONFIG_INTEL_SCU_IPC_INTR_MODE
+	return ipc_wait_for_interrupt();
+#else
+	return busy_loop();
+#endif
+}
+
 /* Read/Write power control(PMIC in Langwell, MSIC in PenWell) registers */
 static int pwr_reg_rdwr(u16 *addr, u8 *data, u32 count, u32 op, u32 id)
 {
@@ -234,7 +274,7 @@ static int pwr_reg_rdwr(u16 *addr, u8 *data, u32 count, u32 op, u32 id)
 		ipc_command(4 << 16 |  id << 12 | 0 << 8 | op);
 	}
 
-	err = busy_loop();
+	err = intel_scu_ipc_check_status();
 	if (id == IPC_CMD_PCNTRL_R) { /* Read rbuf */
 		/* Workaround: values are read as 0 without memcpy_fromio */
 		memcpy_fromio(cbuf, ipcdev.ipc_base + 0x90, 16);
@@ -429,7 +469,7 @@ int intel_scu_ipc_simple_command(int cmd, int sub)
 		return -ENODEV;
 	}
 	ipc_command(sub << 12 | cmd);
-	err = busy_loop();
+	err = intel_scu_ipc_check_status();
 	mutex_unlock(&ipclock);
 	return err;
 }
@@ -463,7 +503,7 @@ int intel_scu_ipc_command(int cmd, int sub, u32 *in, int inlen,
 		ipc_data_writel(*in++, 4 * i);
 
 	ipc_command((inlen << 16) | (sub << 12) | cmd);
-	err = busy_loop();
+	err = intel_scu_ipc_check_status();
 
 	for (i = 0; i < outlen; i++)
 		*out++ = ipc_data_readl(4 * i);
@@ -529,6 +569,9 @@ EXPORT_SYMBOL(intel_scu_ipc_i2c_cntrl);
  */
 static irqreturn_t ioc(int irq, void *dev_id)
 {
+#ifdef CONFIG_INTEL_SCU_IPC_INTR_MODE
+	complete(&ipcdev.cmd_complete);
+#endif
 	return IRQ_HANDLED;
 }
 
@@ -566,6 +609,10 @@ static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (!pci_resource)
 		return -ENOMEM;
 
+#ifdef CONFIG_INTEL_SCU_IPC_INTR_MODE
+	init_completion(&ipcdev.cmd_complete);
+#endif
+
 	if (request_irq(dev->irq, ioc, 0, "intel_scu_ipc", &ipcdev))
 		return -EBUSY;
 
-- 
1.8.4.2


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

* Re: [PATCH 1/3] ipc: Added platform data structure
  2013-11-13 20:14 ` [PATCH 1/3] ipc: Added platform data structure David Cohen
@ 2013-11-14 13:42   ` One Thousand Gnomes
  0 siblings, 0 replies; 22+ messages in thread
From: One Thousand Gnomes @ 2013-11-14 13:42 UTC (permalink / raw)
  To: David Cohen
  Cc: matthew.garrett, platform-driver-x86, linux-kernel, eric.ernst,
	Kuppuswamy Sathyanarayanan

On Wed, 13 Nov 2013 12:14:29 -0800
David Cohen <david.a.cohen@linux.intel.com> wrote:

> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> Since the same ipc driver can be used by many platforms, using
> macros for defining ipc_base and i2c_base addresses is not
> a scalable approach. So added a platform data structure to pass
> this information.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Cc: David Cohen <david.a.cohen@linux.intel.com>

Acked-by: Alan Cox <gnomes@lxorguk.ukuu.org.uk>

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

* Re: [PATCH 2/3] ipc: Enabled ipc support for additional intel platforms
  2013-11-13 20:14 ` [PATCH 2/3] ipc: Enabled ipc support for additional intel platforms David Cohen
@ 2013-11-14 13:43   ` One Thousand Gnomes
  2013-11-14 17:31     ` David Cohen
  0 siblings, 1 reply; 22+ messages in thread
From: One Thousand Gnomes @ 2013-11-14 13:43 UTC (permalink / raw)
  To: David Cohen
  Cc: matthew.garrett, platform-driver-x86, linux-kernel, eric.ernst,
	Kuppuswamy Sathyanarayanan

On Wed, 13 Nov 2013 12:14:30 -0800
David Cohen <david.a.cohen@linux.intel.com> wrote:

> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> Enabled ipc support for penwell, clovertrail & tangier platforms.

At some point this really ought to be discoverable in your SFI, ACPI or
similar data tables not continually hard coded.

Alan

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

* Re: [PATCH 3/3] ipc: Added support for IPC interrupt mode
  2013-11-13 20:14 ` [PATCH 3/3] ipc: Added support for IPC interrupt mode David Cohen
@ 2013-11-14 13:48   ` One Thousand Gnomes
  2013-11-14 17:36     ` David Cohen
  0 siblings, 1 reply; 22+ messages in thread
From: One Thousand Gnomes @ 2013-11-14 13:48 UTC (permalink / raw)
  To: David Cohen
  Cc: matthew.garrett, platform-driver-x86, linux-kernel, eric.ernst,
	Kuppuswamy Sathyanarayanan

O> +	prompt "IPC access mode"
> +	depends on INTEL_SCU_IPC
> +	default INTEL_SCU_IPC_INTR_MODE
> +	---help---
> +	Select the desired access mode for IPC call.

This seems to depend at runtime on the platform so ifdefs seem
inappropriate.

>  static inline void ipc_command(u32 cmd) /* Send ipc command */
>  {
> +#ifdef CONFIG_INTEL_SCU_IPC_INTR_MODE
> +	INIT_COMPLETION(ipcdev.cmd_complete);
> +	writel(cmd | IPC_IOC, ipcdev.ipc_base);
> +#else
>  	writel(cmd, ipcdev.ipc_base);
> +#endif
>  }

If this is platform specific then add an IRQ to your platform data and
check for it then set an irq field in your scu objects and check at
runtime. If it depends upon the command and/or user then pass irq as a
parameter.

>  
>  /*
> @@ -194,6 +203,37 @@ static inline int busy_loop(void) /* Wait till scu status is busy */
>  	return 0;
>  }
>  
> +#ifdef CONFIG_INTEL_SCU_IPC_INTR_MODE
> +/* Wait till ipc ioc interrupt is received or timeout in 3 HZ */
> +static inline int ipc_wait_for_interrupt(void)
> +{
> +	int status;
> +	int ret = 0;
> +
> +	if (!wait_for_completion_timeout(&ipcdev.cmd_complete, 3 * HZ)) {
> +		ret = -ETIMEDOUT;
> +		goto end;
> +	}
> +
> +	status = ipc_read_status();
> +
> +	if ((status >> 1) & 1)
> +		ret = -EIO;
> +
> +end:
> +	return ret;

What happens on a timeout if the command then completes. Will it not
potentially produce a bogus completion on the next command just being
issued in some cases. Also it should probably be logged ?


So I think

1. Pass the informatio upon whether IRQ mode should be used in the
platform information and remove all the ifdeffery
2. Log timeouts
3. Explain what happens on a timeout that allows you to issue further
commands without a race - eg do you need to do any kind of reset or will
the next command issue stall sufficiently etc ?

Alan

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

* Re: [PATCH 2/3] ipc: Enabled ipc support for additional intel platforms
  2013-11-14 13:43   ` One Thousand Gnomes
@ 2013-11-14 17:31     ` David Cohen
  2013-11-14 21:01       ` One Thousand Gnomes
  0 siblings, 1 reply; 22+ messages in thread
From: David Cohen @ 2013-11-14 17:31 UTC (permalink / raw)
  To: One Thousand Gnomes
  Cc: matthew.garrett, platform-driver-x86, linux-kernel, eric.ernst,
	Kuppuswamy Sathyanarayanan

On 11/14/2013 05:43 AM, One Thousand Gnomes wrote:
> On Wed, 13 Nov 2013 12:14:30 -0800
> David Cohen <david.a.cohen@linux.intel.com> wrote:
>
>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>
>> Enabled ipc support for penwell, clovertrail & tangier platforms.
>
> At some point this really ought to be discoverable in your SFI, ACPI or
> similar data tables not continually hard coded.

Agreed. But it depends on fw changes, which I believe to happen on
newer platforms only. On driver side we need to fill the missing
resource info with hardcoded values :/

Br, David

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

* Re: [PATCH 3/3] ipc: Added support for IPC interrupt mode
  2013-11-14 13:48   ` One Thousand Gnomes
@ 2013-11-14 17:36     ` David Cohen
  2013-11-14 21:00       ` One Thousand Gnomes
  0 siblings, 1 reply; 22+ messages in thread
From: David Cohen @ 2013-11-14 17:36 UTC (permalink / raw)
  To: One Thousand Gnomes
  Cc: matthew.garrett, platform-driver-x86, linux-kernel, eric.ernst,
	Kuppuswamy Sathyanarayanan

Hi Alan,

[snip]

> So I think
>
> 1. Pass the informatio upon whether IRQ mode should be used in the
> platform information and remove all the ifdeffery

I will double check with Sathya. But in case polling mode is left for
development purpose, make we can create a static variable to set irq
mode or not.

> 2. Log timeouts

Agreed.

> 3. Explain what happens on a timeout that allows you to issue further
> commands without a race - eg do you need to do any kind of reset or will
> the next command issue stall sufficiently etc ?

I'll check with Sathya.

Thanks,

David Cohen

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

* Re: [PATCH 3/3] ipc: Added support for IPC interrupt mode
  2013-11-14 17:36     ` David Cohen
@ 2013-11-14 21:00       ` One Thousand Gnomes
  2013-11-14 21:31         ` David Cohen
  0 siblings, 1 reply; 22+ messages in thread
From: One Thousand Gnomes @ 2013-11-14 21:00 UTC (permalink / raw)
  To: David Cohen
  Cc: matthew.garrett, platform-driver-x86, linux-kernel, eric.ernst,
	Kuppuswamy Sathyanarayanan

On Thu, 14 Nov 2013 09:36:18 -0800
David Cohen <david.a.cohen@linux.intel.com> wrote:

> Hi Alan,
> 
> [snip]
> 
> > So I think
> >
> > 1. Pass the informatio upon whether IRQ mode should be used in the
> > platform information and remove all the ifdeffery
> 
> I will double check with Sathya. But in case polling mode is left for
> development purpose, make we can create a static variable to set irq
> mode or not.

If its only for development it doesn't IMHO need to be upstream. If its
useful for debug and/or needed on real chipsets (as I believe is the case
for the older ones) then a module (and thus kernel command line) option
would be sensible yes.

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

* Re: [PATCH 2/3] ipc: Enabled ipc support for additional intel platforms
  2013-11-14 17:31     ` David Cohen
@ 2013-11-14 21:01       ` One Thousand Gnomes
  0 siblings, 0 replies; 22+ messages in thread
From: One Thousand Gnomes @ 2013-11-14 21:01 UTC (permalink / raw)
  To: David Cohen
  Cc: matthew.garrett, platform-driver-x86, linux-kernel, eric.ernst,
	Kuppuswamy Sathyanarayanan

On Thu, 14 Nov 2013 09:31:38 -0800
David Cohen <david.a.cohen@linux.intel.com> wrote:

> On 11/14/2013 05:43 AM, One Thousand Gnomes wrote:
> > On Wed, 13 Nov 2013 12:14:30 -0800
> > David Cohen <david.a.cohen@linux.intel.com> wrote:
> >
> >> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> >>
> >> Enabled ipc support for penwell, clovertrail & tangier platforms.
> >
> > At some point this really ought to be discoverable in your SFI, ACPI or
> > similar data tables not continually hard coded.
> 
> Agreed. But it depends on fw changes, which I believe to happen on
> newer platforms only. On driver side we need to fill the missing
> resource info with hardcoded values :/

Oh agreed entirely, but please twist the arms of those responsible and
make sure they are very keen to get it into the firmware ACPI tables
ASAP.. 8)

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

* Re: [PATCH 3/3] ipc: Added support for IPC interrupt mode
  2013-11-14 21:00       ` One Thousand Gnomes
@ 2013-11-14 21:31         ` David Cohen
  0 siblings, 0 replies; 22+ messages in thread
From: David Cohen @ 2013-11-14 21:31 UTC (permalink / raw)
  To: One Thousand Gnomes
  Cc: matthew.garrett, platform-driver-x86, linux-kernel, eric.ernst,
	Kuppuswamy Sathyanarayanan

On 11/14/2013 01:00 PM, One Thousand Gnomes wrote:
> On Thu, 14 Nov 2013 09:36:18 -0800
> David Cohen <david.a.cohen@linux.intel.com> wrote:
>
>> Hi Alan,
>>
>> [snip]
>>
>>> So I think
>>>
>>> 1. Pass the informatio upon whether IRQ mode should be used in the
>>> platform information and remove all the ifdeffery
>>
>> I will double check with Sathya. But in case polling mode is left for
>> development purpose, make we can create a static variable to set irq
>> mode or not.
>
> If its only for development it doesn't IMHO need to be upstream. If its
> useful for debug and/or needed on real chipsets (as I believe is the case
> for the older ones) then a module (and thus kernel command line) option
> would be sensible yes.

Looks like this patch is keeping poll mode for Lincroft support, since
interrupt mode was never tested there. We're sending new version of
this patch set soon.

Br, David

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

* [PATCH v2 0/4] Enable more Intel Mid platforms on IPC driver
  2013-11-13 20:14 [PATCH 0/3] Enable more Intel Mid platforms on IPC driver David Cohen
                   ` (2 preceding siblings ...)
  2013-11-13 20:14 ` [PATCH 3/3] ipc: Added support for IPC interrupt mode David Cohen
@ 2013-11-14 22:15 ` David Cohen
  2013-11-14 22:15   ` [PATCH v2 1/4] ipc: Added platform data structure David Cohen
                     ` (4 more replies)
  3 siblings, 5 replies; 22+ messages in thread
From: David Cohen @ 2013-11-14 22:15 UTC (permalink / raw)
  To: matthew.garrett
  Cc: platform-driver-x86, linux-kernel, gnomes, david.a.cohen,
	sathyanarayanan.kuppuswamy, eric.ernst

Hi,

I am resubmitting these patches to enable other Intel Mid platforms on IPC
driver.
This change is necessary prior to enable more features for Mid on upstream.

Changes from v1 to v2:
 - Fixed bug on patch 1/4: changed PCI_DEVICE to PCI_VDEVICE
 - Addressed Alan Cox'c comments:
   * Added log message for timeouts
   * Remove all #ifdefs from code

Br, David Cohen

---
Kuppuswamy Sathyanarayanan (4):
  ipc: Added platform data structure
  ipc: Enabled ipc support for additional intel platforms
  ipc: Handle error conditions in ipc command
  ipc: Added support for IPC interrupt mode

 drivers/platform/x86/intel_scu_ipc.c | 117 ++++++++++++++++++++++++++++++-----
 1 file changed, 103 insertions(+), 14 deletions(-)

-- 
1.8.4.2


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

* [PATCH v2 1/4] ipc: Added platform data structure
  2013-11-14 22:15 ` [PATCH v2 0/4] Enable more Intel Mid platforms on IPC driver David Cohen
@ 2013-11-14 22:15   ` David Cohen
  2013-11-14 22:15   ` [PATCH v2 2/4] ipc: Enabled ipc support for additional intel platforms David Cohen
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: David Cohen @ 2013-11-14 22:15 UTC (permalink / raw)
  To: matthew.garrett
  Cc: platform-driver-x86, linux-kernel, gnomes, david.a.cohen,
	sathyanarayanan.kuppuswamy, eric.ernst

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Since the same ipc driver can be used by many platforms, using
macros for defining ipc_base and i2c_base addresses is not
a scalable approach. So added a platform data structure to pass
this information.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Acked-by: Alan Cox <gnomes@lxorguk.ukuu.org.uk>
Cc: David Cohen <david.a.cohen@linux.intel.com>
---
 drivers/platform/x86/intel_scu_ipc.c | 37 ++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index d654f831410d..39ff57bdf18f 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -58,12 +58,29 @@
  *    message handler is called within firmware.
  */
 
-#define IPC_BASE_ADDR     0xFF11C000	/* IPC1 base register address */
-#define IPC_MAX_ADDR      0x100		/* Maximum IPC regisers */
 #define IPC_WWBUF_SIZE    20		/* IPC Write buffer Size */
 #define IPC_RWBUF_SIZE    20		/* IPC Read buffer Size */
-#define IPC_I2C_BASE      0xFF12B000	/* I2C control register base address */
-#define IPC_I2C_MAX_ADDR  0x10		/* Maximum I2C regisers */
+
+enum {
+	SCU_IPC_LINCROFT,
+};
+
+/* intel scu ipc driver data*/
+struct intel_scu_ipc_pdata_t {
+	u32 ipc_base;
+	u32 i2c_base;
+	u32 ipc_len;
+	u32 i2c_len;
+};
+
+static struct intel_scu_ipc_pdata_t intel_scu_ipc_pdata[] = {
+	[SCU_IPC_LINCROFT] = {
+		.ipc_base = 0xff11c000,
+		.i2c_base = 0xff12b000,
+		.ipc_len = 0x100,
+		.i2c_len = 0x10,
+	},
+};
 
 static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id);
 static void ipc_remove(struct pci_dev *pdev);
@@ -504,12 +521,16 @@ static irqreturn_t ioc(int irq, void *dev_id)
  */
 static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
-	int err;
+	int err, pid;
+	struct intel_scu_ipc_pdata_t *pdata;
 	resource_size_t pci_resource;
 
 	if (ipcdev.pdev)		/* We support only one SCU */
 		return -EBUSY;
 
+	pid = id->driver_data;
+	pdata = &intel_scu_ipc_pdata[pid];
+
 	ipcdev.pdev = pci_dev_get(dev);
 
 	err = pci_enable_device(dev);
@@ -527,11 +548,11 @@ static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (request_irq(dev->irq, ioc, 0, "intel_scu_ipc", &ipcdev))
 		return -EBUSY;
 
-	ipcdev.ipc_base = ioremap_nocache(IPC_BASE_ADDR, IPC_MAX_ADDR);
+	ipcdev.ipc_base = ioremap_nocache(pdata->ipc_base, pdata->ipc_len);
 	if (!ipcdev.ipc_base)
 		return -ENOMEM;
 
-	ipcdev.i2c_base = ioremap_nocache(IPC_I2C_BASE, IPC_I2C_MAX_ADDR);
+	ipcdev.i2c_base = ioremap_nocache(pdata->i2c_base, pdata->i2c_len);
 	if (!ipcdev.i2c_base) {
 		iounmap(ipcdev.ipc_base);
 		return -ENOMEM;
@@ -564,7 +585,7 @@ static void ipc_remove(struct pci_dev *pdev)
 }
 
 static DEFINE_PCI_DEVICE_TABLE(pci_ids) = {
-	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x082a)},
+	{PCI_VDEVICE(INTEL, 0x082a), SCU_IPC_LINCROFT},
 	{ 0,}
 };
 MODULE_DEVICE_TABLE(pci, pci_ids);
-- 
1.8.4.2


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

* [PATCH v2 2/4] ipc: Enabled ipc support for additional intel platforms
  2013-11-14 22:15 ` [PATCH v2 0/4] Enable more Intel Mid platforms on IPC driver David Cohen
  2013-11-14 22:15   ` [PATCH v2 1/4] ipc: Added platform data structure David Cohen
@ 2013-11-14 22:15   ` David Cohen
  2013-11-14 22:15   ` [PATCH v2 3/4] ipc: Handle error conditions in ipc command David Cohen
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: David Cohen @ 2013-11-14 22:15 UTC (permalink / raw)
  To: matthew.garrett
  Cc: platform-driver-x86, linux-kernel, gnomes, david.a.cohen,
	sathyanarayanan.kuppuswamy, eric.ernst

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Enabled ipc support for penwell, clovertrail & tangier platforms.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: David Cohen <david.a.cohen@linux.intel.com>
---
 drivers/platform/x86/intel_scu_ipc.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index 39ff57bdf18f..86b6ce2a7a47 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -63,6 +63,9 @@
 
 enum {
 	SCU_IPC_LINCROFT,
+	SCU_IPC_PENWELL,
+	SCU_IPC_CLOVERVIEW,
+	SCU_IPC_TANGIER,
 };
 
 /* intel scu ipc driver data*/
@@ -80,6 +83,24 @@ static struct intel_scu_ipc_pdata_t intel_scu_ipc_pdata[] = {
 		.ipc_len = 0x100,
 		.i2c_len = 0x10,
 	},
+	[SCU_IPC_PENWELL] = {
+		.ipc_base = 0xff11c000,
+		.i2c_base = 0xff12b000,
+		.ipc_len = 0x100,
+		.i2c_len = 0x10,
+	},
+	[SCU_IPC_CLOVERVIEW] = {
+		.ipc_base = 0xff11c000,
+		.i2c_base = 0xff12b000,
+		.ipc_len = 0x100,
+		.i2c_len = 0x10,
+	},
+	[SCU_IPC_TANGIER] = {
+		.ipc_base = 0xff009000,
+		.i2c_base  = 0xff00d000,
+		.ipc_len  = 0x100,
+		.i2c_len = 0x10,
+	},
 };
 
 static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id);
@@ -586,6 +607,9 @@ static void ipc_remove(struct pci_dev *pdev)
 
 static DEFINE_PCI_DEVICE_TABLE(pci_ids) = {
 	{PCI_VDEVICE(INTEL, 0x082a), SCU_IPC_LINCROFT},
+	{PCI_VDEVICE(INTEL, 0x080e), SCU_IPC_PENWELL},
+	{PCI_VDEVICE(INTEL, 0x08ea), SCU_IPC_CLOVERVIEW},
+	{PCI_VDEVICE(INTEL, 0x11a0), SCU_IPC_TANGIER},
 	{ 0,}
 };
 MODULE_DEVICE_TABLE(pci, pci_ids);
-- 
1.8.4.2


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

* [PATCH v2 3/4] ipc: Handle error conditions in ipc command
  2013-11-14 22:15 ` [PATCH v2 0/4] Enable more Intel Mid platforms on IPC driver David Cohen
  2013-11-14 22:15   ` [PATCH v2 1/4] ipc: Added platform data structure David Cohen
  2013-11-14 22:15   ` [PATCH v2 2/4] ipc: Enabled ipc support for additional intel platforms David Cohen
@ 2013-11-14 22:15   ` David Cohen
  2013-11-14 22:15   ` [PATCH v2 4/4] ipc: Added support for IPC interrupt mode David Cohen
  2013-11-20 23:51   ` [PATCH v2 0/4] Enable more Intel Mid platforms on IPC driver Matthew Garrett
  4 siblings, 0 replies; 22+ messages in thread
From: David Cohen @ 2013-11-14 22:15 UTC (permalink / raw)
  To: matthew.garrett
  Cc: platform-driver-x86, linux-kernel, gnomes, david.a.cohen,
	sathyanarayanan.kuppuswamy, eric.ernst

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Handle error conditions in intel_scu_ipc_command() and
pwr_reg_rdwr().

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 drivers/platform/x86/intel_scu_ipc.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index 86b6ce2a7a47..e26830f6c8dd 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -235,7 +235,7 @@ static int pwr_reg_rdwr(u16 *addr, u8 *data, u32 count, u32 op, u32 id)
 	}
 
 	err = busy_loop();
-	if (id == IPC_CMD_PCNTRL_R) { /* Read rbuf */
+	if (!err && id == IPC_CMD_PCNTRL_R) { /* Read rbuf */
 		/* Workaround: values are read as 0 without memcpy_fromio */
 		memcpy_fromio(cbuf, ipcdev.ipc_base + 0x90, 16);
 		for (nc = 0; nc < count; nc++)
@@ -465,8 +465,10 @@ int intel_scu_ipc_command(int cmd, int sub, u32 *in, int inlen,
 	ipc_command((inlen << 16) | (sub << 12) | cmd);
 	err = busy_loop();
 
-	for (i = 0; i < outlen; i++)
-		*out++ = ipc_data_readl(4 * i);
+	if (!err) {
+		for (i = 0; i < outlen; i++)
+			*out++ = ipc_data_readl(4 * i);
+	}
 
 	mutex_unlock(&ipclock);
 	return err;
-- 
1.8.4.2


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

* [PATCH v2 4/4] ipc: Added support for IPC interrupt mode
  2013-11-14 22:15 ` [PATCH v2 0/4] Enable more Intel Mid platforms on IPC driver David Cohen
                     ` (2 preceding siblings ...)
  2013-11-14 22:15   ` [PATCH v2 3/4] ipc: Handle error conditions in ipc command David Cohen
@ 2013-11-14 22:15   ` David Cohen
  2013-11-16  0:21     ` [PATCH v2.1] " David Cohen
  2013-11-20 23:51   ` [PATCH v2 0/4] Enable more Intel Mid platforms on IPC driver Matthew Garrett
  4 siblings, 1 reply; 22+ messages in thread
From: David Cohen @ 2013-11-14 22:15 UTC (permalink / raw)
  To: matthew.garrett
  Cc: platform-driver-x86, linux-kernel, gnomes, david.a.cohen,
	sathyanarayanan.kuppuswamy, eric.ernst

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

This patch adds support for ipc command interrupt mode.
Also added platform data option to select 'irq_mode'

irq_mode = 1: configure the driver to receive IOC interrupt
for each successful ipc_command.

irq_mode = 0: makes driver use polling method to
track the command completion status.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 drivers/platform/x86/intel_scu_ipc.c | 48 +++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index e26830f6c8dd..90521e9c6065 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -60,6 +60,7 @@
 
 #define IPC_WWBUF_SIZE    20		/* IPC Write buffer Size */
 #define IPC_RWBUF_SIZE    20		/* IPC Read buffer Size */
+#define IPC_IOC	          0x100		/* IPC command register IOC bit */
 
 enum {
 	SCU_IPC_LINCROFT,
@@ -74,6 +75,7 @@ struct intel_scu_ipc_pdata_t {
 	u32 i2c_base;
 	u32 ipc_len;
 	u32 i2c_len;
+	u8 irq_mode;
 };
 
 static struct intel_scu_ipc_pdata_t intel_scu_ipc_pdata[] = {
@@ -82,24 +84,28 @@ static struct intel_scu_ipc_pdata_t intel_scu_ipc_pdata[] = {
 		.i2c_base = 0xff12b000,
 		.ipc_len = 0x100,
 		.i2c_len = 0x10,
+		.irq_mode = 0,
 	},
 	[SCU_IPC_PENWELL] = {
 		.ipc_base = 0xff11c000,
 		.i2c_base = 0xff12b000,
 		.ipc_len = 0x100,
 		.i2c_len = 0x10,
+		.irq_mode = 1,
 	},
 	[SCU_IPC_CLOVERVIEW] = {
 		.ipc_base = 0xff11c000,
 		.i2c_base = 0xff12b000,
 		.ipc_len = 0x100,
 		.i2c_len = 0x10,
+		.irq_mode = 1,
 	},
 	[SCU_IPC_TANGIER] = {
 		.ipc_base = 0xff009000,
 		.i2c_base  = 0xff00d000,
 		.ipc_len  = 0x100,
 		.i2c_len = 0x10,
+		.irq_mode = 1,
 	},
 };
 
@@ -110,6 +116,8 @@ struct intel_scu_ipc_dev {
 	struct pci_dev *pdev;
 	void __iomem *ipc_base;
 	void __iomem *i2c_base;
+	struct completion cmd_complete;
+	u8 irq_mode;
 };
 
 static struct intel_scu_ipc_dev  ipcdev; /* Only one for now */
@@ -136,6 +144,10 @@ static DEFINE_MUTEX(ipclock); /* lock used to prevent multiple call to SCU */
  */
 static inline void ipc_command(u32 cmd) /* Send ipc command */
 {
+	if (ipcdev.irq_mode) {
+		INIT_COMPLETION(ipcdev.cmd_complete);
+		writel(cmd | IPC_IOC, ipcdev.ipc_base);
+	}
 	writel(cmd, ipcdev.ipc_base);
 }
 
@@ -194,6 +206,30 @@ static inline int busy_loop(void) /* Wait till scu status is busy */
 	return 0;
 }
 
+/* Wait till ipc ioc interrupt is received or timeout in 3 HZ */
+static inline int ipc_wait_for_interrupt(void)
+{
+	int status;
+
+	if (!wait_for_completion_timeout(&ipcdev.cmd_complete, 3 * HZ)) {
+		struct device *dev = &ipcdev.pdev->dev;
+		dev_err(dev, "IPC timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	status = ipc_read_status();
+
+	if ((status >> 1) & 1)
+		return -EIO;
+
+	return 0;
+}
+
+int intel_scu_ipc_check_status(void)
+{
+	return ipcdev.irq_mode ? ipc_wait_for_interrupt() : busy_loop();
+}
+
 /* Read/Write power control(PMIC in Langwell, MSIC in PenWell) registers */
 static int pwr_reg_rdwr(u16 *addr, u8 *data, u32 count, u32 op, u32 id)
 {
@@ -234,7 +270,7 @@ static int pwr_reg_rdwr(u16 *addr, u8 *data, u32 count, u32 op, u32 id)
 		ipc_command(4 << 16 |  id << 12 | 0 << 8 | op);
 	}
 
-	err = busy_loop();
+	err = intel_scu_ipc_check_status();
 	if (!err && id == IPC_CMD_PCNTRL_R) { /* Read rbuf */
 		/* Workaround: values are read as 0 without memcpy_fromio */
 		memcpy_fromio(cbuf, ipcdev.ipc_base + 0x90, 16);
@@ -429,7 +465,7 @@ int intel_scu_ipc_simple_command(int cmd, int sub)
 		return -ENODEV;
 	}
 	ipc_command(sub << 12 | cmd);
-	err = busy_loop();
+	err = intel_scu_ipc_check_status();
 	mutex_unlock(&ipclock);
 	return err;
 }
@@ -463,7 +499,7 @@ int intel_scu_ipc_command(int cmd, int sub, u32 *in, int inlen,
 		ipc_data_writel(*in++, 4 * i);
 
 	ipc_command((inlen << 16) | (sub << 12) | cmd);
-	err = busy_loop();
+	err = intel_scu_ipc_check_status();
 
 	if (!err) {
 		for (i = 0; i < outlen; i++)
@@ -531,6 +567,9 @@ EXPORT_SYMBOL(intel_scu_ipc_i2c_cntrl);
  */
 static irqreturn_t ioc(int irq, void *dev_id)
 {
+	if (ipcdev.irq_mode)
+		complete(&ipcdev.cmd_complete);
+
 	return IRQ_HANDLED;
 }
 
@@ -555,6 +594,7 @@ static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	pdata = &intel_scu_ipc_pdata[pid];
 
 	ipcdev.pdev = pci_dev_get(dev);
+	ipcdev.irq_mode = pdata->irq_mode;
 
 	err = pci_enable_device(dev);
 	if (err)
@@ -568,6 +608,8 @@ static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (!pci_resource)
 		return -ENOMEM;
 
+	init_completion(&ipcdev.cmd_complete);
+
 	if (request_irq(dev->irq, ioc, 0, "intel_scu_ipc", &ipcdev))
 		return -EBUSY;
 
-- 
1.8.4.2


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

* [PATCH v2.1] ipc: Added support for IPC interrupt mode
  2013-11-14 22:15   ` [PATCH v2 4/4] ipc: Added support for IPC interrupt mode David Cohen
@ 2013-11-16  0:21     ` David Cohen
  2013-11-21  1:45       ` [PATCH v2.2] " Kuppuswamy Sathyanarayanan
  0 siblings, 1 reply; 22+ messages in thread
From: David Cohen @ 2013-11-16  0:21 UTC (permalink / raw)
  To: matthew.garrett
  Cc: platform-driver-x86, linux-kernel, gnomes,
	sathyanarayanan.kuppuswamy, eric.ernst, David Cohen

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

This patch adds support for ipc command interrupt mode.
Also added platform data option to select 'irq_mode'

irq_mode = 1: configure the driver to receive IOC interrupt
for each successful ipc_command.

irq_mode = 0: makes driver use polling method to
track the command completion status.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---

v2 to v2.1:
 - IRQ mode is not stable on Tangier. This new version disables it for such
   platform.

---
 drivers/platform/x86/intel_scu_ipc.c | 48 +++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index e26830f6c8dd..287a55062dc6 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -60,6 +60,7 @@
 
 #define IPC_WWBUF_SIZE    20		/* IPC Write buffer Size */
 #define IPC_RWBUF_SIZE    20		/* IPC Read buffer Size */
+#define IPC_IOC	          0x100		/* IPC command register IOC bit */
 
 enum {
 	SCU_IPC_LINCROFT,
@@ -74,6 +75,7 @@ struct intel_scu_ipc_pdata_t {
 	u32 i2c_base;
 	u32 ipc_len;
 	u32 i2c_len;
+	u8 irq_mode;
 };
 
 static struct intel_scu_ipc_pdata_t intel_scu_ipc_pdata[] = {
@@ -82,24 +84,28 @@ static struct intel_scu_ipc_pdata_t intel_scu_ipc_pdata[] = {
 		.i2c_base = 0xff12b000,
 		.ipc_len = 0x100,
 		.i2c_len = 0x10,
+		.irq_mode = 0,
 	},
 	[SCU_IPC_PENWELL] = {
 		.ipc_base = 0xff11c000,
 		.i2c_base = 0xff12b000,
 		.ipc_len = 0x100,
 		.i2c_len = 0x10,
+		.irq_mode = 1,
 	},
 	[SCU_IPC_CLOVERVIEW] = {
 		.ipc_base = 0xff11c000,
 		.i2c_base = 0xff12b000,
 		.ipc_len = 0x100,
 		.i2c_len = 0x10,
+		.irq_mode = 1,
 	},
 	[SCU_IPC_TANGIER] = {
 		.ipc_base = 0xff009000,
 		.i2c_base  = 0xff00d000,
 		.ipc_len  = 0x100,
 		.i2c_len = 0x10,
+		.irq_mode = 0,
 	},
 };
 
@@ -110,6 +116,8 @@ struct intel_scu_ipc_dev {
 	struct pci_dev *pdev;
 	void __iomem *ipc_base;
 	void __iomem *i2c_base;
+	struct completion cmd_complete;
+	u8 irq_mode;
 };
 
 static struct intel_scu_ipc_dev  ipcdev; /* Only one for now */
@@ -136,6 +144,10 @@ static DEFINE_MUTEX(ipclock); /* lock used to prevent multiple call to SCU */
  */
 static inline void ipc_command(u32 cmd) /* Send ipc command */
 {
+	if (ipcdev.irq_mode) {
+		INIT_COMPLETION(ipcdev.cmd_complete);
+		writel(cmd | IPC_IOC, ipcdev.ipc_base);
+	}
 	writel(cmd, ipcdev.ipc_base);
 }
 
@@ -194,6 +206,30 @@ static inline int busy_loop(void) /* Wait till scu status is busy */
 	return 0;
 }
 
+/* Wait till ipc ioc interrupt is received or timeout in 3 HZ */
+static inline int ipc_wait_for_interrupt(void)
+{
+	int status;
+
+	if (!wait_for_completion_timeout(&ipcdev.cmd_complete, 3 * HZ)) {
+		struct device *dev = &ipcdev.pdev->dev;
+		dev_err(dev, "IPC timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	status = ipc_read_status();
+
+	if ((status >> 1) & 1)
+		return -EIO;
+
+	return 0;
+}
+
+int intel_scu_ipc_check_status(void)
+{
+	return ipcdev.irq_mode ? ipc_wait_for_interrupt() : busy_loop();
+}
+
 /* Read/Write power control(PMIC in Langwell, MSIC in PenWell) registers */
 static int pwr_reg_rdwr(u16 *addr, u8 *data, u32 count, u32 op, u32 id)
 {
@@ -234,7 +270,7 @@ static int pwr_reg_rdwr(u16 *addr, u8 *data, u32 count, u32 op, u32 id)
 		ipc_command(4 << 16 |  id << 12 | 0 << 8 | op);
 	}
 
-	err = busy_loop();
+	err = intel_scu_ipc_check_status();
 	if (!err && id == IPC_CMD_PCNTRL_R) { /* Read rbuf */
 		/* Workaround: values are read as 0 without memcpy_fromio */
 		memcpy_fromio(cbuf, ipcdev.ipc_base + 0x90, 16);
@@ -429,7 +465,7 @@ int intel_scu_ipc_simple_command(int cmd, int sub)
 		return -ENODEV;
 	}
 	ipc_command(sub << 12 | cmd);
-	err = busy_loop();
+	err = intel_scu_ipc_check_status();
 	mutex_unlock(&ipclock);
 	return err;
 }
@@ -463,7 +499,7 @@ int intel_scu_ipc_command(int cmd, int sub, u32 *in, int inlen,
 		ipc_data_writel(*in++, 4 * i);
 
 	ipc_command((inlen << 16) | (sub << 12) | cmd);
-	err = busy_loop();
+	err = intel_scu_ipc_check_status();
 
 	if (!err) {
 		for (i = 0; i < outlen; i++)
@@ -531,6 +567,9 @@ EXPORT_SYMBOL(intel_scu_ipc_i2c_cntrl);
  */
 static irqreturn_t ioc(int irq, void *dev_id)
 {
+	if (ipcdev.irq_mode)
+		complete(&ipcdev.cmd_complete);
+
 	return IRQ_HANDLED;
 }
 
@@ -555,6 +594,7 @@ static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	pdata = &intel_scu_ipc_pdata[pid];
 
 	ipcdev.pdev = pci_dev_get(dev);
+	ipcdev.irq_mode = pdata->irq_mode;
 
 	err = pci_enable_device(dev);
 	if (err)
@@ -568,6 +608,8 @@ static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (!pci_resource)
 		return -ENOMEM;
 
+	init_completion(&ipcdev.cmd_complete);
+
 	if (request_irq(dev->irq, ioc, 0, "intel_scu_ipc", &ipcdev))
 		return -EBUSY;
 
-- 
1.8.4.2


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

* Re: [PATCH v2 0/4] Enable more Intel Mid platforms on IPC driver
  2013-11-14 22:15 ` [PATCH v2 0/4] Enable more Intel Mid platforms on IPC driver David Cohen
                     ` (3 preceding siblings ...)
  2013-11-14 22:15   ` [PATCH v2 4/4] ipc: Added support for IPC interrupt mode David Cohen
@ 2013-11-20 23:51   ` Matthew Garrett
  4 siblings, 0 replies; 22+ messages in thread
From: Matthew Garrett @ 2013-11-20 23:51 UTC (permalink / raw)
  To: david.a.cohen
  Cc: sathyanarayanan.kuppuswamy, platform-driver-x86, linux-kernel,
	gnomes, eric.ernst

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 234 bytes --]

Applied the set, thanks.
-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* [PATCH v2.2] ipc: Added support for IPC interrupt mode
  2013-11-16  0:21     ` [PATCH v2.1] " David Cohen
@ 2013-11-21  1:45       ` Kuppuswamy Sathyanarayanan
  2013-11-21  1:47         ` Matthew Garrett
  0 siblings, 1 reply; 22+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2013-11-21  1:45 UTC (permalink / raw)
  To: matthew.garrett
  Cc: platform-driver-x86, linux-kernel, gnomes,
	sathyanarayanan.kuppuswamy, david.a.cohen, eric.ernst

This patch adds support for ipc command interrupt mode.
Also added platform data option to select 'irq_mode'

irq_mode = 1: configure the driver to receive IOC interrupt
for each successful ipc_command.

irq_mode = 0: makes driver use polling method to
track the command completion status.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---

v2.1 to v2.2:

 - Changed INIT_COMPLETION to reinit_completion.

---
 drivers/platform/x86/intel_scu_ipc.c |   48 +++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index e26830f..60ea476 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -60,6 +60,7 @@
 
 #define IPC_WWBUF_SIZE    20		/* IPC Write buffer Size */
 #define IPC_RWBUF_SIZE    20		/* IPC Read buffer Size */
+#define IPC_IOC	          0x100		/* IPC command register IOC bit */
 
 enum {
 	SCU_IPC_LINCROFT,
@@ -74,6 +75,7 @@ struct intel_scu_ipc_pdata_t {
 	u32 i2c_base;
 	u32 ipc_len;
 	u32 i2c_len;
+	u8 irq_mode;
 };
 
 static struct intel_scu_ipc_pdata_t intel_scu_ipc_pdata[] = {
@@ -82,24 +84,28 @@ static struct intel_scu_ipc_pdata_t intel_scu_ipc_pdata[] = {
 		.i2c_base = 0xff12b000,
 		.ipc_len = 0x100,
 		.i2c_len = 0x10,
+		.irq_mode = 0,
 	},
 	[SCU_IPC_PENWELL] = {
 		.ipc_base = 0xff11c000,
 		.i2c_base = 0xff12b000,
 		.ipc_len = 0x100,
 		.i2c_len = 0x10,
+		.irq_mode = 1,
 	},
 	[SCU_IPC_CLOVERVIEW] = {
 		.ipc_base = 0xff11c000,
 		.i2c_base = 0xff12b000,
 		.ipc_len = 0x100,
 		.i2c_len = 0x10,
+		.irq_mode = 1,
 	},
 	[SCU_IPC_TANGIER] = {
 		.ipc_base = 0xff009000,
 		.i2c_base  = 0xff00d000,
 		.ipc_len  = 0x100,
 		.i2c_len = 0x10,
+		.irq_mode = 0,
 	},
 };
 
@@ -110,6 +116,8 @@ struct intel_scu_ipc_dev {
 	struct pci_dev *pdev;
 	void __iomem *ipc_base;
 	void __iomem *i2c_base;
+	struct completion cmd_complete;
+	u8 irq_mode;
 };
 
 static struct intel_scu_ipc_dev  ipcdev; /* Only one for now */
@@ -136,6 +144,10 @@ static DEFINE_MUTEX(ipclock); /* lock used to prevent multiple call to SCU */
  */
 static inline void ipc_command(u32 cmd) /* Send ipc command */
 {
+	if (ipcdev.irq_mode) {
+		reinit_completion(&ipcdev.cmd_complete);
+		writel(cmd | IPC_IOC, ipcdev.ipc_base);
+	}
 	writel(cmd, ipcdev.ipc_base);
 }
 
@@ -194,6 +206,30 @@ static inline int busy_loop(void) /* Wait till scu status is busy */
 	return 0;
 }
 
+/* Wait till ipc ioc interrupt is received or timeout in 3 HZ */
+static inline int ipc_wait_for_interrupt(void)
+{
+	int status;
+
+	if (!wait_for_completion_timeout(&ipcdev.cmd_complete, 3 * HZ)) {
+		struct device *dev = &ipcdev.pdev->dev;
+		dev_err(dev, "IPC timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	status = ipc_read_status();
+
+	if ((status >> 1) & 1)
+		return -EIO;
+
+	return 0;
+}
+
+int intel_scu_ipc_check_status(void)
+{
+	return ipcdev.irq_mode ? ipc_wait_for_interrupt() : busy_loop();
+}
+
 /* Read/Write power control(PMIC in Langwell, MSIC in PenWell) registers */
 static int pwr_reg_rdwr(u16 *addr, u8 *data, u32 count, u32 op, u32 id)
 {
@@ -234,7 +270,7 @@ static int pwr_reg_rdwr(u16 *addr, u8 *data, u32 count, u32 op, u32 id)
 		ipc_command(4 << 16 |  id << 12 | 0 << 8 | op);
 	}
 
-	err = busy_loop();
+	err = intel_scu_ipc_check_status();
 	if (!err && id == IPC_CMD_PCNTRL_R) { /* Read rbuf */
 		/* Workaround: values are read as 0 without memcpy_fromio */
 		memcpy_fromio(cbuf, ipcdev.ipc_base + 0x90, 16);
@@ -429,7 +465,7 @@ int intel_scu_ipc_simple_command(int cmd, int sub)
 		return -ENODEV;
 	}
 	ipc_command(sub << 12 | cmd);
-	err = busy_loop();
+	err = intel_scu_ipc_check_status();
 	mutex_unlock(&ipclock);
 	return err;
 }
@@ -463,7 +499,7 @@ int intel_scu_ipc_command(int cmd, int sub, u32 *in, int inlen,
 		ipc_data_writel(*in++, 4 * i);
 
 	ipc_command((inlen << 16) | (sub << 12) | cmd);
-	err = busy_loop();
+	err = intel_scu_ipc_check_status();
 
 	if (!err) {
 		for (i = 0; i < outlen; i++)
@@ -531,6 +567,9 @@ EXPORT_SYMBOL(intel_scu_ipc_i2c_cntrl);
  */
 static irqreturn_t ioc(int irq, void *dev_id)
 {
+	if (ipcdev.irq_mode)
+		complete(&ipcdev.cmd_complete);
+
 	return IRQ_HANDLED;
 }
 
@@ -555,6 +594,7 @@ static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	pdata = &intel_scu_ipc_pdata[pid];
 
 	ipcdev.pdev = pci_dev_get(dev);
+	ipcdev.irq_mode = pdata->irq_mode;
 
 	err = pci_enable_device(dev);
 	if (err)
@@ -568,6 +608,8 @@ static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (!pci_resource)
 		return -ENOMEM;
 
+	init_completion(&ipcdev.cmd_complete);
+
 	if (request_irq(dev->irq, ioc, 0, "intel_scu_ipc", &ipcdev))
 		return -EBUSY;
 
-- 
1.7.9.5


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

* Re: [PATCH v2.2] ipc: Added support for IPC interrupt mode
  2013-11-21  1:45       ` [PATCH v2.2] " Kuppuswamy Sathyanarayanan
@ 2013-11-21  1:47         ` Matthew Garrett
  2013-11-21  1:48           ` sathyanarayanan kuppuswamy
  0 siblings, 1 reply; 22+ messages in thread
From: Matthew Garrett @ 2013-11-21  1:47 UTC (permalink / raw)
  To: sathyanarayanan.kuppuswamy
  Cc: platform-driver-x86, linux-kernel, gnomes, david.a.cohen, eric.ernst

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 399 bytes --]

On Wed, 2013-11-20 at 17:45 -0800, Kuppuswamy Sathyanarayanan wrote:

>  - Changed INIT_COMPLETION to reinit_completion.

Thanks, I've already folded that fix into the patch and re-pushed.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* Re: [PATCH v2.2] ipc: Added support for IPC interrupt mode
  2013-11-21  1:47         ` Matthew Garrett
@ 2013-11-21  1:48           ` sathyanarayanan kuppuswamy
  0 siblings, 0 replies; 22+ messages in thread
From: sathyanarayanan kuppuswamy @ 2013-11-21  1:48 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: platform-driver-x86, linux-kernel, gnomes, david.a.cohen, eric.ernst


On 11/20/2013 05:47 PM, Matthew Garrett wrote:
> On Wed, 2013-11-20 at 17:45 -0800, Kuppuswamy Sathyanarayanan wrote:
>
>>   - Changed INIT_COMPLETION to reinit_completion.
> Thanks, I've already folded that fix into the patch and re-pushed.
>
Thanks.

--
Sathyanarayanan Kuppuswamy
Android kernel developer


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

end of thread, other threads:[~2013-11-21  1:48 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-13 20:14 [PATCH 0/3] Enable more Intel Mid platforms on IPC driver David Cohen
2013-11-13 20:14 ` [PATCH 1/3] ipc: Added platform data structure David Cohen
2013-11-14 13:42   ` One Thousand Gnomes
2013-11-13 20:14 ` [PATCH 2/3] ipc: Enabled ipc support for additional intel platforms David Cohen
2013-11-14 13:43   ` One Thousand Gnomes
2013-11-14 17:31     ` David Cohen
2013-11-14 21:01       ` One Thousand Gnomes
2013-11-13 20:14 ` [PATCH 3/3] ipc: Added support for IPC interrupt mode David Cohen
2013-11-14 13:48   ` One Thousand Gnomes
2013-11-14 17:36     ` David Cohen
2013-11-14 21:00       ` One Thousand Gnomes
2013-11-14 21:31         ` David Cohen
2013-11-14 22:15 ` [PATCH v2 0/4] Enable more Intel Mid platforms on IPC driver David Cohen
2013-11-14 22:15   ` [PATCH v2 1/4] ipc: Added platform data structure David Cohen
2013-11-14 22:15   ` [PATCH v2 2/4] ipc: Enabled ipc support for additional intel platforms David Cohen
2013-11-14 22:15   ` [PATCH v2 3/4] ipc: Handle error conditions in ipc command David Cohen
2013-11-14 22:15   ` [PATCH v2 4/4] ipc: Added support for IPC interrupt mode David Cohen
2013-11-16  0:21     ` [PATCH v2.1] " David Cohen
2013-11-21  1:45       ` [PATCH v2.2] " Kuppuswamy Sathyanarayanan
2013-11-21  1:47         ` Matthew Garrett
2013-11-21  1:48           ` sathyanarayanan kuppuswamy
2013-11-20 23:51   ` [PATCH v2 0/4] Enable more Intel Mid platforms on IPC driver Matthew Garrett

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).