[PATCH v1 1/3] intel_scu_ipc: fix indentation in few places

[PATCH v1 1/3] intel_scu_ipc: fix indentation in few places

[Andy Shevchenko]

While here, do couple of amendments:
 - move platform variable to the function where it's used
 - define intel_scu_ipc_check_status() static

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/platform/x86/intel_scu_ipc.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index 66a4d32..8938c31 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -1,7 +1,7 @@
 /*
  * intel_scu_ipc.c: Driver for the Intel SCU IPC mechanism
  *
- * (C) Copyright 2008-2010 Intel Corporation
+ * (C) Copyright 2008-2010,2015 Intel Corporation
  * Author: Sreedhara DS (sreedhara.ds@intel.com)
  *
  * This program is free software; you can redistribute it and/or
@@ -67,7 +67,7 @@
 #define PCI_DEVICE_ID_CLOVERVIEW	0x08ea
 #define PCI_DEVICE_ID_TANGIER		0x11a0
 
-/* intel scu ipc driver data*/
+/* intel scu ipc driver data */
 struct intel_scu_ipc_pdata_t {
 	u32 ipc_base;
 	u32 i2c_base;
@@ -114,8 +114,6 @@ struct intel_scu_ipc_dev {
 
 static struct intel_scu_ipc_dev  ipcdev; /* Only one for now */
 
-static int platform;		/* Platform type */
-
 /*
  * IPC Read Buffer (Read Only):
  * 16 byte buffer for receiving data from SCU, if IPC command
@@ -160,7 +158,6 @@ static inline void ipc_data_writel(u32 data, u32 offset) /* Write ipc data */
  * Format:
  * |rfu3(8)|error code(8)|initiator id(8)|cmd id(4)|rfu1(2)|error(1)|busy(1)|
  */
-
 static inline u8 ipc_read_status(void)
 {
 	return __raw_readl(ipcdev.ipc_base + 0x04);
@@ -176,7 +173,8 @@ static inline u32 ipc_data_readl(u32 offset) /* Read ipc u32 data */
 	return readl(ipcdev.ipc_base + IPC_READ_BUFFER + offset);
 }
 
-static inline int busy_loop(void) /* Wait till scu status is busy */
+/* Wait till scu status is busy */
+static inline int busy_loop(void)
 {
 	u32 status = 0;
 	u32 loop_count = 0;
@@ -217,7 +215,7 @@ static inline int ipc_wait_for_interrupt(void)
 	return 0;
 }
 
-int intel_scu_ipc_check_status(void)
+static int intel_scu_ipc_check_status(void)
 {
 	return ipcdev.irq_mode ? ipc_wait_for_interrupt() : busy_loop();
 }
@@ -248,18 +246,18 @@ static int pwr_reg_rdwr(u16 *addr, u8 *data, u32 count, u32 op, u32 id)
 	if (id == IPC_CMD_PCNTRL_R) {
 		for (nc = 0, offset = 0; nc < count; nc++, offset += 4)
 			ipc_data_writel(wbuf[nc], offset);
-		ipc_command((count*2) << 16 |  id << 12 | 0 << 8 | op);
+		ipc_command((count * 2) << 16 | id << 12 | 0 << 8 | op);
 	} else if (id == IPC_CMD_PCNTRL_W) {
 		for (nc = 0; nc < count; nc++, offset += 1)
 			cbuf[offset] = data[nc];
 		for (nc = 0, offset = 0; nc < count; nc++, offset += 4)
 			ipc_data_writel(wbuf[nc], offset);
-		ipc_command((count*3) << 16 |  id << 12 | 0 << 8 | op);
+		ipc_command((count * 3) << 16 | id << 12 | 0 << 8 | op);
 	} else if (id == IPC_CMD_PCNTRL_M) {
 		cbuf[offset] = data[0];
 		cbuf[offset + 1] = data[1];
 		ipc_data_writel(wbuf[0], 0); /* Write wbuff */
-		ipc_command(4 << 16 |  id << 12 | 0 << 8 | op);
+		ipc_command(4 << 16 | id << 12 | 0 << 8 | op);
 	}
 
 	err = intel_scu_ipc_check_status();
@@ -301,7 +299,7 @@ EXPORT_SYMBOL(intel_scu_ipc_ioread8);
  */
 int intel_scu_ipc_ioread16(u16 addr, u16 *data)
 {
-	u16 x[2] = {addr, addr + 1 };
+	u16 x[2] = {addr, addr + 1};
 	return pwr_reg_rdwr(x, (u8 *)data, 2, IPCMSG_PCNTRL, IPC_CMD_PCNTRL_R);
 }
 EXPORT_SYMBOL(intel_scu_ipc_ioread16);
@@ -351,7 +349,7 @@ EXPORT_SYMBOL(intel_scu_ipc_iowrite8);
  */
 int intel_scu_ipc_iowrite16(u16 addr, u16 data)
 {
-	u16 x[2] = {addr, addr + 1 };
+	u16 x[2] = {addr, addr + 1};
 	return pwr_reg_rdwr(x, (u8 *)&data, 2, IPCMSG_PCNTRL, IPC_CMD_PCNTRL_W);
 }
 EXPORT_SYMBOL(intel_scu_ipc_iowrite16);
@@ -412,7 +410,6 @@ int intel_scu_ipc_writev(u16 *addr, u8 *data, int len)
 }
 EXPORT_SYMBOL(intel_scu_ipc_writev);
 
-
 /**
  *	intel_scu_ipc_update_register	-	r/m/w a register
  *	@addr: register address
@@ -475,9 +472,8 @@ EXPORT_SYMBOL(intel_scu_ipc_simple_command);
  *	Issue a command to the SCU which involves data transfers. Do the
  *	data copies under the lock but leave it for the caller to interpret
  */
-
 int intel_scu_ipc_command(int cmd, int sub, u32 *in, int inlen,
-							u32 *out, int outlen)
+			  u32 *out, int outlen)
 {
 	int i, err;
 
@@ -503,7 +499,7 @@ int intel_scu_ipc_command(int cmd, int sub, u32 *in, int inlen,
 }
 EXPORT_SYMBOL(intel_scu_ipc_command);
 
-/*I2C commands */
+/* I2C commands */
 #define IPC_I2C_WRITE 1 /* I2C Write command */
 #define IPC_I2C_READ  2 /* I2C Read command */
 
@@ -666,9 +662,10 @@ static struct pci_driver ipc_driver = {
 	.remove = ipc_remove,
 };
 
-
 static int __init intel_scu_ipc_init(void)
 {
+	int platform;		/* Platform type */
+
 	platform = intel_mid_identify_cpu();
 	if (platform == 0)
 		return -ENODEV;
-- 
2.1.4

[PATCH v1 2/3] intel_scu_ipc: move error check out of the loop

[Andy Shevchenko]

This is small performance optimization of the busy_loop().

While here, use BIT() macro instead of plain integers when check the status.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/platform/x86/intel_scu_ipc.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index 8938c31..e6430a3 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -176,21 +176,21 @@ static inline u32 ipc_data_readl(u32 offset) /* Read ipc u32 data */
 /* Wait till scu status is busy */
 static inline int busy_loop(void)
 {
-	u32 status = 0;
-	u32 loop_count = 0;
+	u32 status = ipc_read_status();
+	u32 loop_count = 100000;
 
-	status = ipc_read_status();
-	while (status & 1) {
+	/* break if scu doesn't reset busy bit after huge retry */
+	while ((status & BIT(0)) && --loop_count) {
 		udelay(1); /* scu processing time is in few u secods */
 		status = ipc_read_status();
-		loop_count++;
-		/* break if scu doesn't reset busy bit after huge retry */
-		if (loop_count > 100000) {
-			dev_err(&ipcdev.pdev->dev, "IPC timed out");
-			return -ETIMEDOUT;
-		}
 	}
-	if ((status >> 1) & 1)
+
+	if (status & BIT(0)) {
+		dev_err(&ipcdev.pdev->dev, "IPC timed out");
+		return -ETIMEDOUT;
+	}
+
+	if (status & BIT(1))
 		return -EIO;
 
 	return 0;
@@ -208,8 +208,7 @@ static inline int ipc_wait_for_interrupt(void)
 	}
 
 	status = ipc_read_status();
-
-	if ((status >> 1) & 1)
+	if (status & BIT(1))
 		return -EIO;
 
 	return 0;
-- 
2.1.4

[PATCH v1 3/3] intel_scu_ipc: take resources from PCI configuration

[Andy Shevchenko]

The patch makes driver to take the resources from PCI BAR0 instead of using
hardcoded values.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/platform/x86/intel_scu_ipc.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index e6430a3..001b199 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -43,10 +43,9 @@
 /*
  * IPC register summary
  *
- * IPC register blocks are memory mapped at fixed address of 0xFF11C000
+ * IPC register blocks are memory mapped at fixed address of PCI BAR 0.
  * To read or write information to the SCU, driver writes to IPC-1 memory
- * mapped registers (base address 0xFF11C000). The following is the IPC
- * mechanism
+ * mapped registers. The following is the IPC mechanism
  *
  * 1. IA core cDMI interface claims this transaction and converts it to a
  *    Transaction Layer Packet (TLP) message which is sent across the cDMI.
@@ -69,34 +68,26 @@
 
 /* intel scu ipc driver data */
 struct intel_scu_ipc_pdata_t {
-	u32 ipc_base;
 	u32 i2c_base;
-	u32 ipc_len;
 	u32 i2c_len;
 	u8 irq_mode;
 };
 
 static struct intel_scu_ipc_pdata_t intel_scu_ipc_lincroft_pdata = {
-	.ipc_base = 0xff11c000,
 	.i2c_base = 0xff12b000,
-	.ipc_len = 0x100,
 	.i2c_len = 0x10,
 	.irq_mode = 0,
 };
 
 /* Penwell and Cloverview */
 static struct intel_scu_ipc_pdata_t intel_scu_ipc_penwell_pdata = {
-	.ipc_base = 0xff11c000,
 	.i2c_base = 0xff12b000,
-	.ipc_len = 0x100,
 	.i2c_len = 0x10,
 	.irq_mode = 1,
 };
 
 static struct intel_scu_ipc_pdata_t intel_scu_ipc_tangier_pdata = {
-	.ipc_base = 0xff009000,
 	.i2c_base  = 0xff00d000,
-	.ipc_len  = 0x100,
 	.i2c_len = 0x10,
 	.irq_mode = 0,
 };
@@ -572,7 +563,7 @@ static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	int err;
 	struct intel_scu_ipc_pdata_t *pdata;
-	resource_size_t pci_resource;
+	resource_size_t base;
 
 	if (ipcdev.pdev)		/* We support only one SCU */
 		return -EBUSY;
@@ -590,8 +581,8 @@ static int ipc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (err)
 		return err;
 
-	pci_resource = pci_resource_start(dev, 0);
-	if (!pci_resource)
+	base = pci_resource_start(dev, 0);
+	if (!base)
 		return -ENOMEM;
 
 	init_completion(&ipcdev.cmd_complete);
@@ -599,7 +590,7 @@ 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(pdata->ipc_base, pdata->ipc_len);
+	ipcdev.ipc_base = ioremap_nocache(base, pci_resource_len(dev, 0));
 	if (!ipcdev.ipc_base)
 		return -ENOMEM;
 
-- 
2.1.4

Re: [PATCH v1 1/3] intel_scu_ipc: fix indentation in few places

[Darren Hart]

On Wed, Jan 21, 2015 at 09:38:09PM +0200, Andy Shevchenko wrote:
> While here, do couple of amendments:
>  - move platform variable to the function where it's used
>  - define intel_scu_ipc_check_status() static
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Series applied and queued for 3.20.

Minor wording change to 3/3 commit subject and log.

Running through testing, will push to for-next tomorrow.

Thank you Andy.

-- 
Darren Hart
Intel Open Source Technology Center