* [PATCH v3 REPOST 0/4] i2c : i2c-ibm-iic : use interrupts to perform the data transfer
From: jean-jacques hiblot @ 2013-12-20 15:12 UTC (permalink / raw)
To: wsa; +Cc: gregory.clement, jean-jacques hiblot, linuxppc-dev, linux-i2c
Hello Wolfram,
I'm reposting this patch set with the linux-ppc list in cc as the ibm I2c
controller is primarily (only?) available on ppc platforms.
This patch set aims to improve the performance of the driver for the IBM iic
controller by implementing the data transfer in the interrupt handler.
Using interrupts to trigger the data transfer reduces and make more
deterministic the latencies between indivdual bytes, and consequently reduces
the total transfer time,
In our test environement with multiple masters, this significantly reduces the
rate of i2c errors.
Changes since v2:
- cosmectics change (comments style, removed a hard-coded value)
- moved some parts from one patch to another
Changes since v1:
- split the patch in 4 individual patches. The code has been refactored a bit
to make the diff easier to read.
- changed some dev_dbg in dev_err or dev_warn when more appropriate
jean-jacques hiblot (4):
i2c: i2c-ibm-iic: cleanup.
i2c: i2c-ibm-iic: perform the transfer in the interrupt handler
i2c: i2c-ibm-iic: Implements transfer abortion
i2c: i2c-ibm-iic: Implements a polling mode
drivers/i2c/busses/i2c-ibm_iic.c | 490 +++++++++++++++++++++++----------------
drivers/i2c/busses/i2c-ibm_iic.h | 20 +-
2 files changed, 309 insertions(+), 201 deletions(-)
--
1.8.4.2
^ permalink raw reply
* [PATCH v3 REPOST 1/4] i2c: i2c-ibm-iic: cleanup.
From: jean-jacques hiblot @ 2013-12-20 15:12 UTC (permalink / raw)
To: wsa
Cc: gregory.clement, jean-jacques hiblot, linuxppc-dev, linux-i2c,
jean-jacques hiblot
In-Reply-To: <1387552376-12986-1-git-send-email-jjhiblot@traphandler.com>
From: jean-jacques hiblot <jjhiblot@gmail.com>
* removed unneeded 'volatile' qualifiers and casts
* use the dev_dbg, dev_err etc. instead of printk
* removed unneeded members for the driver's private data
* fixed the style of the multi-line comments
Signed-off-by: jean-jacques hiblot <jjhiblot@traphandler.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
drivers/i2c/busses/i2c-ibm_iic.c | 135 +++++++++++++++++++++------------------
drivers/i2c/busses/i2c-ibm_iic.h | 9 +--
2 files changed, 78 insertions(+), 66 deletions(-)
diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index ff3caa0..03ab756 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -68,22 +68,26 @@ MODULE_PARM_DESC(iic_force_fast, "Force fast mode (400 kHz)");
#undef DBG2
#endif
+# define ERR(dev, f, x...) dev_err(dev->adap.dev.parent, f, ##x)
+
#if DBG_LEVEL > 0
-# define DBG(f,x...) printk(KERN_DEBUG "ibm-iic" f, ##x)
+# define DBG(dev, f, x...) dev_dbg(dev->adap.dev.parent, f, ##x)
#else
-# define DBG(f,x...) ((void)0)
+# define DBG(dev, f, x...) ((void)0)
#endif
#if DBG_LEVEL > 1
-# define DBG2(f,x...) DBG(f, ##x)
+# define DBG2(dev, f, x...) DBG(dev, f, ##x)
#else
-# define DBG2(f,x...) ((void)0)
+# define DBG2(dev, f, x...) ((void)0)
#endif
#if DBG_LEVEL > 2
static void dump_iic_regs(const char* header, struct ibm_iic_private* dev)
{
- volatile struct iic_regs __iomem *iic = dev->vaddr;
- printk(KERN_DEBUG "ibm-iic%d: %s\n", dev->idx, header);
- printk(KERN_DEBUG
+ struct iic_regs __iomem *iic = dev->vaddr;
+ struct device *device = dev.adap.dev.parent;
+
+ dev_dbg(device, ": %s\n", header);
+ dev_dbg(device,
" cntl = 0x%02x, mdcntl = 0x%02x\n"
" sts = 0x%02x, extsts = 0x%02x\n"
" clkdiv = 0x%02x, xfrcnt = 0x%02x\n"
@@ -133,9 +137,9 @@ static inline void iic_interrupt_mode(struct ibm_iic_private* dev, int enable)
*/
static void iic_dev_init(struct ibm_iic_private* dev)
{
- volatile struct iic_regs __iomem *iic = dev->vaddr;
+ struct iic_regs __iomem *iic = dev->vaddr;
- DBG("%d: init\n", dev->idx);
+ DBG(dev, "init\n");
/* Clear master address */
out_8(&iic->lmadr, 0);
@@ -178,11 +182,11 @@ static void iic_dev_init(struct ibm_iic_private* dev)
*/
static void iic_dev_reset(struct ibm_iic_private* dev)
{
- volatile struct iic_regs __iomem *iic = dev->vaddr;
+ struct iic_regs __iomem *iic = dev->vaddr;
int i;
u8 dc;
- DBG("%d: soft reset\n", dev->idx);
+ DBG(dev, "soft reset\n");
DUMP_REGS("reset", dev);
/* Place chip in the reset state */
@@ -191,7 +195,7 @@ static void iic_dev_reset(struct ibm_iic_private* dev)
/* Check if bus is free */
dc = in_8(&iic->directcntl);
if (!DIRCTNL_FREE(dc)){
- DBG("%d: trying to regain bus control\n", dev->idx);
+ DBG(dev, "trying to regain bus control\n");
/* Try to set bus free state */
out_8(&iic->directcntl, DIRCNTL_SDAC | DIRCNTL_SCC);
@@ -226,7 +230,7 @@ static void iic_dev_reset(struct ibm_iic_private* dev)
*/
/* Wait for SCL and/or SDA to be high */
-static int iic_dc_wait(volatile struct iic_regs __iomem *iic, u8 mask)
+static int iic_dc_wait(struct iic_regs __iomem *iic, u8 mask)
{
unsigned long x = jiffies + HZ / 28 + 2;
while ((in_8(&iic->directcntl) & mask) != mask){
@@ -239,19 +243,18 @@ static int iic_dc_wait(volatile struct iic_regs __iomem *iic, u8 mask)
static int iic_smbus_quick(struct ibm_iic_private* dev, const struct i2c_msg* p)
{
- volatile struct iic_regs __iomem *iic = dev->vaddr;
+ struct iic_regs __iomem *iic = dev->vaddr;
const struct i2c_timings* t = &timings[dev->fast_mode ? 1 : 0];
u8 mask, v, sda;
int i, res;
/* Only 7-bit addresses are supported */
if (unlikely(p->flags & I2C_M_TEN)){
- DBG("%d: smbus_quick - 10 bit addresses are not supported\n",
- dev->idx);
+ DBG(dev, "smbus_quick - 10 bit addresses are not supported\n");
return -EINVAL;
}
- DBG("%d: smbus_quick(0x%02x)\n", dev->idx, p->addr);
+ DBG(dev, "smbus_quick(0x%02x)\n", p->addr);
/* Reset IIC interface */
out_8(&iic->xtcntlss, XTCNTLSS_SRST);
@@ -304,7 +307,7 @@ static int iic_smbus_quick(struct ibm_iic_private* dev, const struct i2c_msg* p)
ndelay(t->buf);
- DBG("%d: smbus_quick -> %s\n", dev->idx, res ? "NACK" : "ACK");
+ DBG(dev, "smbus_quick -> %s\n", res ? "NACK" : "ACK");
out:
/* Remove reset */
out_8(&iic->xtcntlss, 0);
@@ -314,7 +317,7 @@ out:
return res;
err:
- DBG("%d: smbus_quick - bus is stuck\n", dev->idx);
+ DBG(dev, "smbus_quick - bus is stuck\n");
res = -EREMOTEIO;
goto out;
}
@@ -325,10 +328,10 @@ err:
static irqreturn_t iic_handler(int irq, void *dev_id)
{
struct ibm_iic_private* dev = (struct ibm_iic_private*)dev_id;
- volatile struct iic_regs __iomem *iic = dev->vaddr;
+ struct iic_regs __iomem *iic = dev->vaddr;
- DBG2("%d: irq handler, STS = 0x%02x, EXTSTS = 0x%02x\n",
- dev->idx, in_8(&iic->sts), in_8(&iic->extsts));
+ DBG2(dev, "irq handler, STS = 0x%02x, EXTSTS = 0x%02x\n",
+ in_8(&iic->sts), in_8(&iic->extsts));
/* Acknowledge IRQ and wakeup iic_wait_for_tc */
out_8(&iic->sts, STS_IRQA | STS_SCMP);
@@ -343,10 +346,10 @@ static irqreturn_t iic_handler(int irq, void *dev_id)
*/
static int iic_xfer_result(struct ibm_iic_private* dev)
{
- volatile struct iic_regs __iomem *iic = dev->vaddr;
+ struct iic_regs __iomem *iic = dev->vaddr;
if (unlikely(in_8(&iic->sts) & STS_ERR)){
- DBG("%d: xfer error, EXTSTS = 0x%02x\n", dev->idx,
+ DBG(dev, "xfer error, EXTSTS = 0x%02x\n",
in_8(&iic->extsts));
/* Clear errors and possible pending IRQs */
@@ -356,13 +359,14 @@ static int iic_xfer_result(struct ibm_iic_private* dev)
/* Flush master data buffer */
out_8(&iic->mdcntl, in_8(&iic->mdcntl) | MDCNTL_FMDB);
- /* Is bus free?
+ /*
+ * Is bus free?
* If error happened during combined xfer
* IIC interface is usually stuck in some strange
* state, the only way out - soft reset.
*/
if ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE){
- DBG("%d: bus is stuck, resetting\n", dev->idx);
+ DBG(dev, "bus is stuck, resetting\n");
iic_dev_reset(dev);
}
return -EREMOTEIO;
@@ -376,10 +380,10 @@ static int iic_xfer_result(struct ibm_iic_private* dev)
*/
static void iic_abort_xfer(struct ibm_iic_private* dev)
{
- volatile struct iic_regs __iomem *iic = dev->vaddr;
+ struct iic_regs __iomem *iic = dev->vaddr;
unsigned long x;
- DBG("%d: iic_abort_xfer\n", dev->idx);
+ DBG(dev, "iic_abort_xfer\n");
out_8(&iic->cntl, CNTL_HMT);
@@ -390,7 +394,7 @@ static void iic_abort_xfer(struct ibm_iic_private* dev)
x = jiffies + 2;
while ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE){
if (time_after(jiffies, x)){
- DBG("%d: abort timeout, resetting...\n", dev->idx);
+ DBG(dev, "abort timeout, resetting...\n");
iic_dev_reset(dev);
return;
}
@@ -408,7 +412,7 @@ static void iic_abort_xfer(struct ibm_iic_private* dev)
*/
static int iic_wait_for_tc(struct ibm_iic_private* dev){
- volatile struct iic_regs __iomem *iic = dev->vaddr;
+ struct iic_regs __iomem *iic = dev->vaddr;
int ret = 0;
if (dev->irq >= 0){
@@ -417,9 +421,9 @@ static int iic_wait_for_tc(struct ibm_iic_private* dev){
!(in_8(&iic->sts) & STS_PT), dev->adap.timeout);
if (unlikely(ret < 0))
- DBG("%d: wait interrupted\n", dev->idx);
+ DBG(dev, "wait interrupted\n");
else if (unlikely(in_8(&iic->sts) & STS_PT)){
- DBG("%d: wait timeout\n", dev->idx);
+ DBG(dev, "wait timeout\n");
ret = -ETIMEDOUT;
}
}
@@ -429,13 +433,13 @@ static int iic_wait_for_tc(struct ibm_iic_private* dev){
while (in_8(&iic->sts) & STS_PT){
if (unlikely(time_after(jiffies, x))){
- DBG("%d: poll timeout\n", dev->idx);
+ DBG(dev, "poll timeout\n");
ret = -ETIMEDOUT;
break;
}
if (unlikely(signal_pending(current))){
- DBG("%d: poll interrupted\n", dev->idx);
+ DBG(dev, "poll interrupted\n");
ret = -ERESTARTSYS;
break;
}
@@ -448,7 +452,7 @@ static int iic_wait_for_tc(struct ibm_iic_private* dev){
else
ret = iic_xfer_result(dev);
- DBG2("%d: iic_wait_for_tc -> %d\n", dev->idx, ret);
+ DBG2(dev, "iic_wait_for_tc -> %d\n", ret);
return ret;
}
@@ -459,7 +463,7 @@ static int iic_wait_for_tc(struct ibm_iic_private* dev){
static int iic_xfer_bytes(struct ibm_iic_private* dev, struct i2c_msg* pm,
int combined_xfer)
{
- volatile struct iic_regs __iomem *iic = dev->vaddr;
+ struct iic_regs __iomem *iic = dev->vaddr;
char* buf = pm->buf;
int i, j, loops, ret = 0;
int len = pm->len;
@@ -475,14 +479,14 @@ static int iic_xfer_bytes(struct ibm_iic_private* dev, struct i2c_msg* pm,
if (!(cntl & CNTL_RW))
for (j = 0; j < count; ++j)
- out_8((void __iomem *)&iic->mdbuf, *buf++);
+ out_8(&iic->mdbuf, *buf++);
if (i < loops - 1)
cmd |= CNTL_CHT;
else if (combined_xfer)
cmd |= CNTL_RPST;
- DBG2("%d: xfer_bytes, %d, CNTL = 0x%02x\n", dev->idx, count, cmd);
+ DBG2(dev, "xfer_bytes, %d, CNTL = 0x%02x\n", count, cmd);
/* Start transfer */
out_8(&iic->cntl, cmd);
@@ -493,8 +497,8 @@ static int iic_xfer_bytes(struct ibm_iic_private* dev, struct i2c_msg* pm,
if (unlikely(ret < 0))
break;
else if (unlikely(ret != count)){
- DBG("%d: xfer_bytes, requested %d, transferred %d\n",
- dev->idx, count, ret);
+ DBG(dev, "xfer_bytes, requested %d, transferred %d\n",
+ count, ret);
/* If it's not a last part of xfer, abort it */
if (combined_xfer || (i < loops - 1))
@@ -506,7 +510,7 @@ static int iic_xfer_bytes(struct ibm_iic_private* dev, struct i2c_msg* pm,
if (cntl & CNTL_RW)
for (j = 0; j < count; ++j)
- *buf++ = in_8((void __iomem *)&iic->mdbuf);
+ *buf++ = in_8(&iic->mdbuf);
}
return ret > 0 ? 0 : ret;
@@ -517,10 +521,10 @@ static int iic_xfer_bytes(struct ibm_iic_private* dev, struct i2c_msg* pm,
*/
static inline void iic_address(struct ibm_iic_private* dev, struct i2c_msg* msg)
{
- volatile struct iic_regs __iomem *iic = dev->vaddr;
+ struct iic_regs __iomem *iic = dev->vaddr;
u16 addr = msg->addr;
- DBG2("%d: iic_address, 0x%03x (%d-bit)\n", dev->idx,
+ DBG2(dev, "iic_address, 0x%03x (%d-bit)\n",
addr, msg->flags & I2C_M_TEN ? 10 : 7);
if (msg->flags & I2C_M_TEN){
@@ -553,46 +557,49 @@ static inline int iic_address_neq(const struct i2c_msg* p1,
static int iic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
{
struct ibm_iic_private* dev = (struct ibm_iic_private*)(i2c_get_adapdata(adap));
- volatile struct iic_regs __iomem *iic = dev->vaddr;
+ struct iic_regs __iomem *iic = dev->vaddr;
int i, ret = 0;
- DBG2("%d: iic_xfer, %d msg(s)\n", dev->idx, num);
+ DBG2(dev, "iic_xfer, %d msg(s)\n", num);
if (!num)
return 0;
- /* Check the sanity of the passed messages.
+ /*
+ * Check the sanity of the passed messages.
* Uhh, generic i2c layer is more suitable place for such code...
*/
if (unlikely(iic_invalid_address(&msgs[0]))){
- DBG("%d: invalid address 0x%03x (%d-bit)\n", dev->idx,
+ DBG(dev, "invalid address 0x%03x (%d-bit)\n",
msgs[0].addr, msgs[0].flags & I2C_M_TEN ? 10 : 7);
return -EINVAL;
}
for (i = 0; i < num; ++i){
if (unlikely(msgs[i].len <= 0)){
if (num == 1 && !msgs[0].len){
- /* Special case for I2C_SMBUS_QUICK emulation.
+ /*
+ * Special case for I2C_SMBUS_QUICK emulation.
* IBM IIC doesn't support 0-length transactions
* so we have to emulate them using bit-banging.
*/
return iic_smbus_quick(dev, &msgs[0]);
}
- DBG("%d: invalid len %d in msg[%d]\n", dev->idx,
+ DBG(dev, "invalid len %d in msg[%d]\n",
msgs[i].len, i);
return -EINVAL;
}
if (unlikely(iic_address_neq(&msgs[0], &msgs[i]))){
- DBG("%d: invalid addr in msg[%d]\n", dev->idx, i);
+ DBG(dev, "invalid addr in msg[%d]\n", i);
return -EINVAL;
}
}
/* Check bus state */
if (unlikely((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE)){
- DBG("%d: iic_xfer, bus is not free\n", dev->idx);
+ DBG(dev, "iic_xfer, bus is not free\n");
- /* Usually it means something serious has happened.
+ /*
+ * Usually it means something serious has happened.
* We *cannot* have unfinished previous transfer
* so it doesn't make any sense to try to stop it.
* Probably we were not able to recover from the
@@ -603,7 +610,7 @@ static int iic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
iic_dev_reset(dev);
if ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE){
- DBG("%d: iic_xfer, bus is still not free\n", dev->idx);
+ DBG(dev, "iic_xfer, bus is still not free\n");
return -EREMOTEIO;
}
}
@@ -635,15 +642,18 @@ static const struct i2c_algorithm iic_algo = {
/*
* Calculates IICx_CLCKDIV value for a specific OPB clock frequency
*/
-static inline u8 iic_clckdiv(unsigned int opb)
+static inline u8 iic_clckdiv(struct ibm_iic_private *dev, unsigned int opb)
{
- /* Compatibility kludge, should go away after all cards
+ struct device *device = dev->adap.dev.parent;
+
+ /*
+ * Compatibility kludge, should go away after all cards
* are fixed to fill correct value for opbfreq.
* Previous driver version used hardcoded divider value 4,
* it corresponds to OPB frequency from the range (40, 50] MHz
*/
if (!opb){
- printk(KERN_WARNING "ibm-iic: using compatibility value for OPB freq,"
+ dev_warn(device, "iic_clckdiv: using compatibility value for OPB freq,"
" fix your board specific setup\n");
opb = 50000000;
}
@@ -652,7 +662,7 @@ static inline u8 iic_clckdiv(unsigned int opb)
opb /= 1000000;
if (opb < 20 || opb > 150){
- printk(KERN_WARNING "ibm-iic: invalid OPB clock frequency %u MHz\n",
+ dev_warn(device, "iic_clckdiv: invalid OPB clock frequency %u MHz\n",
opb);
opb = opb < 20 ? 20 : 150;
}
@@ -670,16 +680,17 @@ static int iic_request_irq(struct platform_device *ofdev,
irq = irq_of_parse_and_map(np, 0);
if (!irq) {
- dev_err(&ofdev->dev, "irq_of_parse_and_map failed\n");
+ ERR(dev, "irq_of_parse_and_map failed\n");
return 0;
}
- /* Disable interrupts until we finish initialization, assumes
+ /*
+ * Disable interrupts until we finish initialization, assumes
* level-sensitive IRQ setup...
*/
iic_interrupt_mode(dev, 0);
if (request_irq(irq, iic_handler, 0, "IBM IIC", dev)) {
- dev_err(&ofdev->dev, "request_irq %d failed\n", irq);
+ ERR(dev, "request_irq %d failed\n", irq);
/* Fallback to the polling mode */
return 0;
}
@@ -717,7 +728,7 @@ static int iic_probe(struct platform_device *ofdev)
dev->irq = iic_request_irq(ofdev, dev);
if (!dev->irq)
- dev_warn(&ofdev->dev, "using polling mode\n");
+ dev_info(&ofdev->dev, "using polling mode\n");
/* Board specific settings */
if (iic_force_fast || of_get_property(np, "fast-mode", NULL))
@@ -733,7 +744,7 @@ static int iic_probe(struct platform_device *ofdev)
}
}
- dev->clckdiv = iic_clckdiv(*freq);
+ dev->clckdiv = iic_clckdiv(dev, *freq);
dev_dbg(&ofdev->dev, "clckdiv = %d\n", dev->clckdiv);
/* Initialize IIC interface */
diff --git a/drivers/i2c/busses/i2c-ibm_iic.h b/drivers/i2c/busses/i2c-ibm_iic.h
index fdaa482..1efce5d 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.h
+++ b/drivers/i2c/busses/i2c-ibm_iic.h
@@ -25,8 +25,10 @@
#include <linux/i2c.h>
struct iic_regs {
- u16 mdbuf;
- u16 sbbuf;
+ u8 mdbuf;
+ u8 reserved1;
+ u8 sbbuf;
+ u8 reserved2;
u8 lmadr;
u8 hmadr;
u8 cntl;
@@ -44,9 +46,8 @@ struct iic_regs {
struct ibm_iic_private {
struct i2c_adapter adap;
- volatile struct iic_regs __iomem *vaddr;
+ struct iic_regs __iomem *vaddr;
wait_queue_head_t wq;
- int idx;
int irq;
int fast_mode;
u8 clckdiv;
--
1.8.4.2
^ permalink raw reply related
* [PATCH v3 REPOST 2/4] i2c: i2c-ibm-iic: perform the transfer in the interrupt handler
From: jean-jacques hiblot @ 2013-12-20 15:12 UTC (permalink / raw)
To: wsa
Cc: gregory.clement, jean-jacques hiblot, linuxppc-dev, linux-i2c,
jean-jacques hiblot
In-Reply-To: <1387552376-12986-1-git-send-email-jjhiblot@traphandler.com>
From: jean-jacques hiblot <jjhiblot@gmail.com>
The current implementation uses the interrupt only to wakeup the process doing
the data transfer. While this is working, it introduces indesirable latencies.
This patch implements the data transfer in the interrupt handler. It keeps the
latency between individual bytes low and the jitter on the total transfer time
is reduced.
Note: transfer abortion and polling mode are not working and will be supported
in further patches
This was tested on a custom board built around a PPC460EX with several
different I2C devices (including EEPROMs and smart batteries)
Signed-off-by: jean-jacques hiblot <jjhiblot@traphandler.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
drivers/i2c/busses/i2c-ibm_iic.c | 230 ++++++++++++++++++++++++++++-----------
drivers/i2c/busses/i2c-ibm_iic.h | 8 ++
2 files changed, 177 insertions(+), 61 deletions(-)
diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index 03ab756..a92e8f6 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -58,6 +58,8 @@ static bool iic_force_fast;
module_param(iic_force_fast, bool, 0);
MODULE_PARM_DESC(iic_force_fast, "Force fast mode (400 kHz)");
+#define FIFO_FLUSH_TIMEOUT 100
+
#define DBG_LEVEL 0
#ifdef DBG
@@ -125,6 +127,7 @@ static struct i2c_timings {
.high = 600,
.buf = 1300,
}};
+static int iic_xfer_bytes(struct ibm_iic_private *dev);
/* Enable/disable interrupt generation */
static inline void iic_interrupt_mode(struct ibm_iic_private* dev, int enable)
@@ -167,8 +170,8 @@ static void iic_dev_init(struct ibm_iic_private* dev)
/* Clear control register */
out_8(&iic->cntl, 0);
- /* Enable interrupts if possible */
- iic_interrupt_mode(dev, dev->irq >= 0);
+ /* Start with each individual interrupt masked*/
+ iic_interrupt_mode(dev, 0);
/* Set mode control */
out_8(&iic->mdcntl, MDCNTL_FMDB | MDCNTL_EINT | MDCNTL_EUBS
@@ -327,16 +330,8 @@ err:
*/
static irqreturn_t iic_handler(int irq, void *dev_id)
{
- struct ibm_iic_private* dev = (struct ibm_iic_private*)dev_id;
- struct iic_regs __iomem *iic = dev->vaddr;
-
- DBG2(dev, "irq handler, STS = 0x%02x, EXTSTS = 0x%02x\n",
- in_8(&iic->sts), in_8(&iic->extsts));
-
- /* Acknowledge IRQ and wakeup iic_wait_for_tc */
- out_8(&iic->sts, STS_IRQA | STS_SCMP);
- wake_up_interruptible(&dev->wq);
-
+ struct ibm_iic_private *dev = (struct ibm_iic_private *) dev_id;
+ iic_xfer_bytes(dev);
return IRQ_HANDLED;
}
@@ -460,60 +455,129 @@ static int iic_wait_for_tc(struct ibm_iic_private* dev){
/*
* Low level master transfer routine
*/
-static int iic_xfer_bytes(struct ibm_iic_private* dev, struct i2c_msg* pm,
- int combined_xfer)
+static int iic_xfer_bytes(struct ibm_iic_private *dev)
{
- struct iic_regs __iomem *iic = dev->vaddr;
- char* buf = pm->buf;
- int i, j, loops, ret = 0;
- int len = pm->len;
-
- u8 cntl = (in_8(&iic->cntl) & CNTL_AMD) | CNTL_PT;
- if (pm->flags & I2C_M_RD)
- cntl |= CNTL_RW;
+ struct i2c_msg *pm = &(dev->msgs[dev->current_msg]);
+ struct iic_regs *iic = dev->vaddr;
+ u8 cntl = in_8(&iic->cntl) & CNTL_AMD;
+ int count;
+ u32 status;
+ u32 ext_status;
+
+ /* First check the status register */
+ status = in_8(&iic->sts);
+ ext_status = in_8(&iic->extsts);
+
+ /* and acknowledge the interrupt */
+ out_8(&iic->extsts, EXTSTS_IRQP | EXTSTS_IRQD | EXTSTS_LA |
+ EXTSTS_ICT | EXTSTS_XFRA);
+ out_8(&iic->sts, STS_IRQA | STS_SCMP);
- loops = (len + 3) / 4;
- for (i = 0; i < loops; ++i, len -= 4){
- int count = len > 4 ? 4 : len;
- u8 cmd = cntl | ((count - 1) << CNTL_TCT_SHIFT);
+ if ((status & STS_ERR) ||
+ (ext_status & (EXTSTS_LA | EXTSTS_ICT | EXTSTS_XFRA))) {
+ DBG(dev, "status 0x%x\n", status);
+ DBG(dev, "extended status 0x%x\n", ext_status);
+ if (status & STS_ERR)
+ ERR(dev, "Error detected\n");
+ if (ext_status & EXTSTS_LA)
+ DBG(dev, "Lost arbitration\n");
+ if (ext_status & EXTSTS_ICT)
+ ERR(dev, "Incomplete transfer\n");
+ if (ext_status & EXTSTS_XFRA)
+ ERR(dev, "Transfer aborted\n");
+
+ dev->status = -EIO;
+ dev->transfer_complete = 1;
+ complete(&dev->iic_compl);
+ return dev->status;
+ }
- if (!(cntl & CNTL_RW))
- for (j = 0; j < count; ++j)
- out_8(&iic->mdbuf, *buf++);
+ if (dev->msgs == NULL) {
+ DBG(dev, "spurious !!!!!\n");
+ dev->status = -EINVAL;
+ return dev->status;
+ }
- if (i < loops - 1)
- cmd |= CNTL_CHT;
- else if (combined_xfer)
- cmd |= CNTL_RPST;
+ /* check if there's any data to read from the fifo */
+ if (pm->flags & I2C_M_RD) {
+ while (dev->current_byte_rx < dev->current_byte) {
+ u8 *buf = pm->buf + dev->current_byte_rx;
+ *buf = in_8(&iic->mdbuf);
+ dev->current_byte_rx++;
+ DBG2(dev, "read 0x%x\n", *buf);
+ }
+ }
+ /* check if we must go with the next tranfer */
+ if (pm->len == dev->current_byte) {
+ DBG2(dev, "going to next transfer\n");
+ dev->current_byte = 0;
+ dev->current_byte_rx = 0;
+ dev->current_msg++;
+ if (dev->current_msg == dev->num_msgs) {
+ DBG2(dev, "end of transfer\n");
+ dev->transfer_complete = 1;
+ complete(&dev->iic_compl);
+ return dev->status;
+ }
+ pm++;
+ }
- DBG2(dev, "xfer_bytes, %d, CNTL = 0x%02x\n", count, cmd);
+ /* take care of the direction */
+ if (pm->flags & I2C_M_RD)
+ cntl |= CNTL_RW;
- /* Start transfer */
- out_8(&iic->cntl, cmd);
+ /*
+ * how much data are we going to transfer this time ?
+ * (up to 4 bytes at once)
+ */
+ count = pm->len - dev->current_byte;
+ count = (count > 4) ? 4 : count;
+ cntl |= (count - 1) << CNTL_TCT_SHIFT;
+
+ if ((pm->flags & I2C_M_RD) == 0) {
+ /* This is a write. we must fill the fifo with the data */
+ int i;
+ u8 *buf = pm->buf + dev->current_byte;
+
+ for (i = 0; i < count; i++) {
+ out_8(&iic->mdbuf, buf[i]);
+ DBG2(dev, "write : 0x%x\n", buf[i]);
+ }
+ }
- /* Wait for completion */
- ret = iic_wait_for_tc(dev);
+ /* will the current transfer complete with this chunk of data ? */
+ if (pm->len > dev->current_byte + 4) {
+ /*
+ * we're not done with the current transfer.
+ * Don't send a repeated start
+ */
+ cntl |= CNTL_CHT;
+ }
+ /*
+ * This transfer will be complete with this chunk of data
+ * BUT is this the last transfer of the list ?
+ */
+ else if (dev->current_msg != (dev->num_msgs-1)) {
+ /* not the last tranfer */
+ cntl |= CNTL_RPST; /* Do not send a STOP condition */
+ /* check if the NEXT transfer needs a repeated start */
+ if (pm[1].flags & I2C_M_NOSTART)
+ cntl |= CNTL_CHT;
+ }
- if (unlikely(ret < 0))
- break;
- else if (unlikely(ret != count)){
- DBG(dev, "xfer_bytes, requested %d, transferred %d\n",
- count, ret);
+ if ((cntl & CNTL_RPST) == 0)
+ DBG2(dev, "STOP required\n");
- /* If it's not a last part of xfer, abort it */
- if (combined_xfer || (i < loops - 1))
- iic_abort_xfer(dev);
+ if ((cntl & CNTL_CHT) == 0)
+ DBG2(dev, "next transfer will begin with START\n");
- ret = -EREMOTEIO;
- break;
- }
+ /* keep track of the amount of data transfered (RX and TX) */
+ dev->current_byte += count;
- if (cntl & CNTL_RW)
- for (j = 0; j < count; ++j)
- *buf++ = in_8(&iic->mdbuf);
- }
+ /* actually start the transfer of the current data chunk */
+ out_8(&iic->cntl, cntl | CNTL_PT);
- return ret > 0 ? 0 : ret;
+ return 0;
}
/*
@@ -614,19 +678,63 @@ static int iic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
return -EREMOTEIO;
}
}
- else {
- /* Flush master data buffer (just in case) */
- out_8(&iic->mdcntl, in_8(&iic->mdcntl) | MDCNTL_FMDB);
+
+ dev->current_byte = dev->current_msg = dev->current_byte_rx = 0;
+ dev->transfer_complete = 0;
+ dev->status = 0;
+ dev->msgs = msgs;
+ dev->num_msgs = num;
+
+ /* clear the buffers */
+ out_8(&iic->mdcntl, MDCNTL_FMDB);
+ i = FIFO_FLUSH_TIMEOUT;
+ while (in_8(&iic->mdcntl) & MDCNTL_FMDB) {
+ if (i-- < 0) {
+ DBG(dev, "iic_xfer, unable to flush\n");
+ return -EREMOTEIO;
+ }
}
+ /* clear all interrupts masks */
+ out_8(&iic->intmsk, 0x0);
+ /* clear any status */
+ out_8(&iic->sts, STS_IRQA | STS_SCMP);
+ out_8(&iic->extsts, EXTSTS_IRQP | EXTSTS_IRQD | EXTSTS_LA |
+ EXTSTS_ICT | EXTSTS_XFRA);
+
/* Load slave address */
iic_address(dev, &msgs[0]);
- /* Do real transfer */
- for (i = 0; i < num && !ret; ++i)
- ret = iic_xfer_bytes(dev, &msgs[i], i < num - 1);
+ init_completion(&dev->iic_compl);
+
+ /* start the transfer */
+ ret = iic_xfer_bytes(dev);
+
+ if (ret == 0) {
+ /* enable the interrupts */
+ out_8(&iic->mdcntl, MDCNTL_EINT);
+ /* unmask the interrupts */
+ out_8(&iic->intmsk, INTRMSK_EIMTC | INTRMSK_EITA |
+ INTRMSK_EIIC | INTRMSK_EIHE);
+ /* wait for the transfer to complete */
+ ret = wait_for_completion_interruptible_timeout(
+ &dev->iic_compl, num * HZ);
+ /* mask the interrupts */
+ out_8(&iic->intmsk, 0);
+ }
- return ret < 0 ? ret : num;
+ if (ret == 0) {
+ ERR(dev, "transfer timed out\n");
+ ret = -ETIMEDOUT;
+ } else if (ret < 0) {
+ if (ret == -ERESTARTSYS)
+ ERR(dev, "transfer interrupted\n");
+ } else {
+ /* Transfer is complete */
+ ret = (dev->status) ? dev->status : num;
+ }
+
+ return ret;
}
static u32 iic_func(struct i2c_adapter *adap)
diff --git a/drivers/i2c/busses/i2c-ibm_iic.h b/drivers/i2c/busses/i2c-ibm_iic.h
index 1efce5d..76c476a 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.h
+++ b/drivers/i2c/busses/i2c-ibm_iic.h
@@ -51,6 +51,14 @@ struct ibm_iic_private {
int irq;
int fast_mode;
u8 clckdiv;
+ struct i2c_msg *msgs;
+ int num_msgs;
+ int current_msg;
+ int current_byte;
+ int current_byte_rx;
+ int transfer_complete;
+ int status;
+ struct completion iic_compl;
};
/* IICx_CNTL register */
--
1.8.4.2
^ permalink raw reply related
* [PATCH v3 REPOST 3/4] i2c: i2c-ibm-iic: Implements transfer abortion
From: jean-jacques hiblot @ 2013-12-20 15:12 UTC (permalink / raw)
To: wsa
Cc: gregory.clement, jean-jacques hiblot, linuxppc-dev, linux-i2c,
jean-jacques hiblot
In-Reply-To: <1387552376-12986-1-git-send-email-jjhiblot@traphandler.com>
From: jean-jacques hiblot <jjhiblot@gmail.com>
Clean-up properly when a transfer fails for whatever reason.
Cancel the transfer when the process is signaled.
Signed-off-by: jean-jacques hiblot <jjhiblot@traphandler.com>
---
drivers/i2c/busses/i2c-ibm_iic.c | 146 ++++++++++-----------------------------
drivers/i2c/busses/i2c-ibm_iic.h | 2 +-
2 files changed, 36 insertions(+), 112 deletions(-)
diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index a92e8f6..857259e 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -336,120 +336,25 @@ static irqreturn_t iic_handler(int irq, void *dev_id)
}
/*
- * Get master transfer result and clear errors if any.
- * Returns the number of actually transferred bytes or error (<0)
- */
-static int iic_xfer_result(struct ibm_iic_private* dev)
-{
- struct iic_regs __iomem *iic = dev->vaddr;
-
- if (unlikely(in_8(&iic->sts) & STS_ERR)){
- DBG(dev, "xfer error, EXTSTS = 0x%02x\n",
- in_8(&iic->extsts));
-
- /* Clear errors and possible pending IRQs */
- out_8(&iic->extsts, EXTSTS_IRQP | EXTSTS_IRQD |
- EXTSTS_LA | EXTSTS_ICT | EXTSTS_XFRA);
-
- /* Flush master data buffer */
- out_8(&iic->mdcntl, in_8(&iic->mdcntl) | MDCNTL_FMDB);
-
- /*
- * Is bus free?
- * If error happened during combined xfer
- * IIC interface is usually stuck in some strange
- * state, the only way out - soft reset.
- */
- if ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE){
- DBG(dev, "bus is stuck, resetting\n");
- iic_dev_reset(dev);
- }
- return -EREMOTEIO;
- }
- else
- return in_8(&iic->xfrcnt) & XFRCNT_MTC_MASK;
-}
-
-/*
* Try to abort active transfer.
*/
-static void iic_abort_xfer(struct ibm_iic_private* dev)
+static void iic_abort_xfer(struct ibm_iic_private *dev)
{
- struct iic_regs __iomem *iic = dev->vaddr;
- unsigned long x;
-
- DBG(dev, "iic_abort_xfer\n");
+ struct device *device = dev->adap.dev.parent;
+ unsigned long end;
- out_8(&iic->cntl, CNTL_HMT);
+ DBG(dev, "aborting transfer\n");
+ /* transfer should be aborted within 10ms */
+ end = jiffies + 10;
+ dev->abort = 1;
- /*
- * Wait for the abort command to complete.
- * It's not worth to be optimized, just poll (timeout >= 1 tick)
- */
- x = jiffies + 2;
- while ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE){
- if (time_after(jiffies, x)){
- DBG(dev, "abort timeout, resetting...\n");
- iic_dev_reset(dev);
- return;
- }
+ while (time_after(end, jiffies) && !dev->transfer_complete)
schedule();
- }
- /* Just to clear errors */
- iic_xfer_result(dev);
-}
-
-/*
- * Wait for master transfer to complete.
- * It puts current process to sleep until we get interrupt or timeout expires.
- * Returns the number of transferred bytes or error (<0)
- */
-static int iic_wait_for_tc(struct ibm_iic_private* dev){
-
- struct iic_regs __iomem *iic = dev->vaddr;
- int ret = 0;
-
- if (dev->irq >= 0){
- /* Interrupt mode */
- ret = wait_event_interruptible_timeout(dev->wq,
- !(in_8(&iic->sts) & STS_PT), dev->adap.timeout);
-
- if (unlikely(ret < 0))
- DBG(dev, "wait interrupted\n");
- else if (unlikely(in_8(&iic->sts) & STS_PT)){
- DBG(dev, "wait timeout\n");
- ret = -ETIMEDOUT;
- }
- }
- else {
- /* Polling mode */
- unsigned long x = jiffies + dev->adap.timeout;
-
- while (in_8(&iic->sts) & STS_PT){
- if (unlikely(time_after(jiffies, x))){
- DBG(dev, "poll timeout\n");
- ret = -ETIMEDOUT;
- break;
- }
-
- if (unlikely(signal_pending(current))){
- DBG(dev, "poll interrupted\n");
- ret = -ERESTARTSYS;
- break;
- }
- schedule();
- }
+ if (!dev->transfer_complete) {
+ dev_err(device, "abort operation failed\n");
+ iic_dev_reset(dev);
}
-
- if (unlikely(ret < 0))
- iic_abort_xfer(dev);
- else
- ret = iic_xfer_result(dev);
-
- DBG2(dev, "iic_wait_for_tc -> %d\n", ret);
-
- return ret;
}
/*
@@ -473,6 +378,13 @@ static int iic_xfer_bytes(struct ibm_iic_private *dev)
EXTSTS_ICT | EXTSTS_XFRA);
out_8(&iic->sts, STS_IRQA | STS_SCMP);
+ if (dev->status == -ECANCELED) {
+ DBG(dev, "abort completed\n");
+ dev->transfer_complete = 1;
+ complete(&dev->iic_compl);
+ return dev->status;
+ }
+
if ((status & STS_ERR) ||
(ext_status & (EXTSTS_LA | EXTSTS_ICT | EXTSTS_XFRA))) {
DBG(dev, "status 0x%x\n", status);
@@ -577,7 +489,14 @@ static int iic_xfer_bytes(struct ibm_iic_private *dev)
/* actually start the transfer of the current data chunk */
out_8(&iic->cntl, cntl | CNTL_PT);
- return 0;
+ /* The transfer must be aborted. */
+ if (dev->abort) {
+ DBG(dev, "aborting\n");
+ out_8(&iic->cntl, CNTL_HMT);
+ dev->status = -ECANCELED;
+ }
+
+ return dev->status;
}
/*
@@ -682,6 +601,7 @@ static int iic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
dev->current_byte = dev->current_msg = dev->current_byte_rx = 0;
dev->transfer_complete = 0;
dev->status = 0;
+ dev->abort = 0;
dev->msgs = msgs;
dev->num_msgs = num;
@@ -719,8 +639,10 @@ static int iic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
/* wait for the transfer to complete */
ret = wait_for_completion_interruptible_timeout(
&dev->iic_compl, num * HZ);
- /* mask the interrupts */
- out_8(&iic->intmsk, 0);
+ /*
+ * we don't mask the interrupts here because we may
+ * need them to abort the transfer gracefully
+ */
}
if (ret == 0) {
@@ -729,11 +651,15 @@ static int iic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
} else if (ret < 0) {
if (ret == -ERESTARTSYS)
ERR(dev, "transfer interrupted\n");
+ iic_abort_xfer(dev);
} else {
/* Transfer is complete */
ret = (dev->status) ? dev->status : num;
}
+ /* mask the interrupts */
+ out_8(&iic->intmsk, 0);
+
return ret;
}
@@ -832,8 +758,6 @@ static int iic_probe(struct platform_device *ofdev)
goto error_cleanup;
}
- init_waitqueue_head(&dev->wq);
-
dev->irq = iic_request_irq(ofdev, dev);
if (!dev->irq)
dev_info(&ofdev->dev, "using polling mode\n");
diff --git a/drivers/i2c/busses/i2c-ibm_iic.h b/drivers/i2c/busses/i2c-ibm_iic.h
index 76c476a..0ee28a9 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.h
+++ b/drivers/i2c/busses/i2c-ibm_iic.h
@@ -47,7 +47,6 @@ struct iic_regs {
struct ibm_iic_private {
struct i2c_adapter adap;
struct iic_regs __iomem *vaddr;
- wait_queue_head_t wq;
int irq;
int fast_mode;
u8 clckdiv;
@@ -58,6 +57,7 @@ struct ibm_iic_private {
int current_byte_rx;
int transfer_complete;
int status;
+ int abort;
struct completion iic_compl;
};
--
1.8.4.2
^ permalink raw reply related
* [PATCH v3 REPOST 4/4] i2c: i2c-ibm-iic: Implements a polling mode
From: jean-jacques hiblot @ 2013-12-20 15:12 UTC (permalink / raw)
To: wsa
Cc: gregory.clement, jean-jacques hiblot, linuxppc-dev, linux-i2c,
jean-jacques hiblot
In-Reply-To: <1387552376-12986-1-git-send-email-jjhiblot@traphandler.com>
From: jean-jacques hiblot <jjhiblot@gmail.com>
When no valid interrupt is defined for the controller, use polling to handle
the transfers.
The polling mode can also be forced with the "iic_force_poll" module parameter.
Signed-off-by: jean-jacques hiblot <jjhiblot@traphandler.com>
---
drivers/i2c/busses/i2c-ibm_iic.c | 91 ++++++++++++++++++++++++++++++++--------
drivers/i2c/busses/i2c-ibm_iic.h | 1 +
2 files changed, 74 insertions(+), 18 deletions(-)
diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index 857259e..aefe228 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -336,11 +336,45 @@ static irqreturn_t iic_handler(int irq, void *dev_id)
}
/*
+ * Polling used when interrupt can't be used
+ */
+static int poll_for_end_of_transfer(struct ibm_iic_private *dev, u32 timeout)
+{
+ struct iic_regs __iomem *iic = dev->vaddr;
+ u32 status;
+ unsigned long end = jiffies + timeout;
+
+ while ((dev->transfer_complete == 0) &&
+ time_after(end, jiffies) &&
+ !signal_pending(current)) {
+ status = in_8(&iic->sts);
+ /* check if the transfer is done or an error occured */
+ if ((status & (STS_ERR | STS_SCMP)) || !(status & STS_PT))
+ iic_xfer_bytes(dev);
+ /* The transfer is not complete,
+ * calling schedule relaxes the CPU load and allows to know
+ * if the process is being signaled (for abortion)
+ */
+ if (dev->transfer_complete == 0)
+ schedule();
+ }
+
+ if (signal_pending(current))
+ return -ERESTARTSYS;
+
+ if (dev->transfer_complete == 0)
+ return 0;
+
+ return 1;
+}
+
+/*
* Try to abort active transfer.
*/
static void iic_abort_xfer(struct ibm_iic_private *dev)
{
struct device *device = dev->adap.dev.parent;
+ struct iic_regs __iomem *iic = dev->vaddr;
unsigned long end;
DBG(dev, "aborting transfer\n");
@@ -348,8 +382,17 @@ static void iic_abort_xfer(struct ibm_iic_private *dev)
end = jiffies + 10;
dev->abort = 1;
- while (time_after(end, jiffies) && !dev->transfer_complete)
- schedule();
+ while (time_after(end, jiffies) && !dev->transfer_complete) {
+ u32 sts;
+ if (dev->use_polling) {
+ sts = in_8(&iic->sts);
+ /* check if the transfer is done or an error occured */
+ if ((sts & (STS_ERR | STS_SCMP)) || !(sts & STS_PT))
+ iic_xfer_bytes(dev);
+ }
+ if (dev->transfer_complete == 0)
+ schedule();
+ }
if (!dev->transfer_complete) {
dev_err(device, "abort operation failed\n");
@@ -381,7 +424,8 @@ static int iic_xfer_bytes(struct ibm_iic_private *dev)
if (dev->status == -ECANCELED) {
DBG(dev, "abort completed\n");
dev->transfer_complete = 1;
- complete(&dev->iic_compl);
+ if (!dev->use_polling)
+ complete(&dev->iic_compl);
return dev->status;
}
@@ -400,7 +444,8 @@ static int iic_xfer_bytes(struct ibm_iic_private *dev)
dev->status = -EIO;
dev->transfer_complete = 1;
- complete(&dev->iic_compl);
+ if (!dev->use_polling)
+ complete(&dev->iic_compl);
return dev->status;
}
@@ -428,7 +473,8 @@ static int iic_xfer_bytes(struct ibm_iic_private *dev)
if (dev->current_msg == dev->num_msgs) {
DBG2(dev, "end of transfer\n");
dev->transfer_complete = 1;
- complete(&dev->iic_compl);
+ if (!dev->use_polling)
+ complete(&dev->iic_compl);
return dev->status;
}
pm++;
@@ -625,24 +671,29 @@ static int iic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
/* Load slave address */
iic_address(dev, &msgs[0]);
- init_completion(&dev->iic_compl);
+ if (!dev->use_polling)
+ init_completion(&dev->iic_compl);
/* start the transfer */
ret = iic_xfer_bytes(dev);
if (ret == 0) {
- /* enable the interrupts */
- out_8(&iic->mdcntl, MDCNTL_EINT);
- /* unmask the interrupts */
- out_8(&iic->intmsk, INTRMSK_EIMTC | INTRMSK_EITA |
- INTRMSK_EIIC | INTRMSK_EIHE);
- /* wait for the transfer to complete */
- ret = wait_for_completion_interruptible_timeout(
- &dev->iic_compl, num * HZ);
- /*
- * we don't mask the interrupts here because we may
- * need them to abort the transfer gracefully
- */
+ if (dev->use_polling) {
+ ret = poll_for_end_of_transfer(dev, num * HZ);
+ } else {
+ /* enable the interrupts */
+ out_8(&iic->mdcntl, MDCNTL_EINT);
+ /* unmask the interrupts */
+ out_8(&iic->intmsk, INTRMSK_EIMTC | INTRMSK_EITA |
+ INTRMSK_EIIC | INTRMSK_EIHE);
+ /* wait for the transfer to complete */
+ ret = wait_for_completion_interruptible_timeout(
+ &dev->iic_compl, num * HZ);
+ /*
+ * we don't mask the interrupts here because we may
+ * need them to abort the transfer gracefully
+ */
+ }
}
if (ret == 0) {
@@ -709,6 +760,8 @@ static int iic_request_irq(struct platform_device *ofdev,
struct device_node *np = ofdev->dev.of_node;
int irq;
+ dev->use_polling = 1;
+
if (iic_force_poll)
return 0;
@@ -729,6 +782,8 @@ static int iic_request_irq(struct platform_device *ofdev,
return 0;
}
+ dev->use_polling = 0;
+
return irq;
}
diff --git a/drivers/i2c/busses/i2c-ibm_iic.h b/drivers/i2c/busses/i2c-ibm_iic.h
index 0ee28a9..523cfc1 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.h
+++ b/drivers/i2c/busses/i2c-ibm_iic.h
@@ -58,6 +58,7 @@ struct ibm_iic_private {
int transfer_complete;
int status;
int abort;
+ int use_polling;
struct completion iic_compl;
};
--
1.8.4.2
^ permalink raw reply related
* Re: [RFC] linux/pci: move pci_platform_pm_ops to linux/pci.h
From: Bjorn Helgaas @ 2013-12-20 16:42 UTC (permalink / raw)
To: Dongsheng Wang
Cc: Linux PM list, roy.zang, Rafael J. Wysocki,
linux-pci@vger.kernel.org, Scott Wood, linuxppc-dev
In-Reply-To: <1387533832-34554-1-git-send-email-dongsheng.wang@freescale.com>
On Fri, Dec 20, 2013 at 3:03 AM, Dongsheng Wang
<dongsheng.wang@freescale.com> wrote:
> From: Wang Dongsheng <dongsheng.wang@freescale.com>
>
> make Freescale platform use pci_platform_pm_ops struct.
This changelog doesn't say anything about what the patch does.
I infer that you want to use pci_platform_pm_ops from some Freescale
code. This patch should be posted along with the patches that add
that Freescale code, so we can see how you intend to use it.
The existing use is in drivers/pci/pci-acpi.c, so it's possible that
your new use should be added in the same way, in drivers/pci, so we
don't have to make pci_platform_pm_ops part of the public PCI
interface in include/linux/pci.h.
That said, if Raphael thinks this makes sense, it's OK with me.
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> ---
>
> If device's not set power state, we will use this interface to put the
> device's into the correct state.
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 9c91ecc..48f8b1a 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -33,36 +33,6 @@ int pci_mmap_fits(struct pci_dev *pdev, int resno, struct vm_area_struct *vmai,
> #endif
> int pci_probe_reset_function(struct pci_dev *dev);
>
> -/**
> - * struct pci_platform_pm_ops - Firmware PM callbacks
> - *
> - * @is_manageable: returns 'true' if given device is power manageable by the
> - * platform firmware
> - *
> - * @set_state: invokes the platform firmware to set the device's power state
> - *
> - * @choose_state: returns PCI power state of given device preferred by the
> - * platform; to be used during system-wide transitions from a
> - * sleeping state to the working state and vice versa
> - *
> - * @sleep_wake: enables/disables the system wake up capability of given device
> - *
> - * @run_wake: enables/disables the platform to generate run-time wake-up events
> - * for given device (the device's wake-up capability has to be
> - * enabled by @sleep_wake for this feature to work)
> - *
> - * If given platform is generally capable of power managing PCI devices, all of
> - * these callbacks are mandatory.
> - */
> -struct pci_platform_pm_ops {
> - bool (*is_manageable)(struct pci_dev *dev);
> - int (*set_state)(struct pci_dev *dev, pci_power_t state);
> - pci_power_t (*choose_state)(struct pci_dev *dev);
> - int (*sleep_wake)(struct pci_dev *dev, bool enable);
> - int (*run_wake)(struct pci_dev *dev, bool enable);
> -};
> -
> -int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
> void pci_update_current_state(struct pci_dev *dev, pci_power_t state);
> void pci_power_up(struct pci_dev *dev);
> void pci_disable_enabled_device(struct pci_dev *dev);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 1084a15..20e07b8 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -365,6 +365,37 @@ struct pci_dev {
> size_t romlen; /* Length of ROM if it's not from the BAR */
> };
>
> +/**
> + * struct pci_platform_pm_ops - Firmware PM callbacks
> + *
> + * @is_manageable: returns 'true' if given device is power manageable by the
> + * platform firmware
> + *
> + * @set_state: invokes the platform firmware to set the device's power state
> + *
> + * @choose_state: returns PCI power state of given device preferred by the
> + * platform; to be used during system-wide transitions from a
> + * sleeping state to the working state and vice versa
> + *
> + * @sleep_wake: enables/disables the system wake up capability of given device
> + *
> + * @run_wake: enables/disables the platform to generate run-time wake-up events
> + * for given device (the device's wake-up capability has to be
> + * enabled by @sleep_wake for this feature to work)
> + *
> + * If given platform is generally capable of power managing PCI devices, all of
> + * these callbacks are mandatory.
> + */
> +struct pci_platform_pm_ops {
> + bool (*is_manageable)(struct pci_dev *dev);
> + int (*set_state)(struct pci_dev *dev, pci_power_t state);
> + pci_power_t (*choose_state)(struct pci_dev *dev);
> + int (*sleep_wake)(struct pci_dev *dev, bool enable);
> + int (*run_wake)(struct pci_dev *dev, bool enable);
> +};
> +
> +int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
> +
> static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
> {
> #ifdef CONFIG_PCI_IOV
> --
> 1.8.5
>
>
^ permalink raw reply
* Re: [PATCH] powernv: eeh: fix possible buffer overrun in ioda_eeh_phb_diag()
From: Brian W Hart @ 2013-12-20 18:15 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20131220015937.GA13868@shangw.(null)>
On Fri, Dec 20, 2013 at 09:59:37AM +0800, Gavin Shan wrote:
> On Fri, Dec 20, 2013 at 09:35:39AM +0800, Gavin Shan wrote:
> >On Thu, Dec 19, 2013 at 05:14:07PM -0600, Brian W Hart wrote:
> >>PHB diagnostic buffer may be smaller than PAGE_SIZE, especially when
> >>PAGE_SIZE > 4KB.
> >>
> >
> >I think you're talking about that PAGE_SIZE could be configured
> >to have variable size (e.g. 4KB). So it's not safe to pass PAGE_SIZE
> >to OPAL API opal_pci_get_phb_diag_data2(). Instead, we should pass
> >PNV_PCI_DIAG_BUF_SIZE and it makes sense to me :-)
Yeah, I noticed the problem because our test machine has PAGE_SIZE of
64K with the buffer only being 8K.
[...]
> Sorry, Brian. It has been fixed as part of the following commit, which
> has been put into Ben's powerpc-next branch :-)
Thank you!
^ permalink raw reply
* Re: [PATCH] powernv: eeh: add buffer for P7IOC hub error data
From: Brian W Hart @ 2013-12-20 18:17 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20131220014504.GB10795@shangw.(null)>
On Fri, Dec 20, 2013 at 09:45:04AM +0800, Gavin Shan wrote:
> On Thu, Dec 19, 2013 at 05:18:53PM -0600, Brian W Hart wrote:
> >Prevent ioda_eeh_hub_diag() from clobbering itself when called by supplying
> >a buffer for P7IOC hub diagnostic data. Take care to inform OPAL of the
> >correct size for the buffer.
> >
> >Signed-off-by: Brian W Hart <hartb@linux.vnet.ibm.com>
> >---
> >
> >I hope I've understood this correctly. It looks to me like
> >ioda_eeh_hub_data is effectively asking OPAL to clobber its own
> >text (via 'data') when it makes the call to retrieve the hub data.
> >
>
> Yeah, we should have used following variable as HUB diag-data instead.
>
> static char *hub_diag = NULL;
>
> However, it's not safe to allocate page-sized buffer for "hub_diag".
>
> >Added a hub diagnostic structure per-phb. Perhaps the diagnostic
> >structure better belongs in the phb->diag union, but I wasn't sure whether
> >we'd need to carry the hub and PHB diag data at the same time.
> >
>
> Please put hub diag-data to struct pnv_phb::diag since we don't need
> carry hub and PHB diag-data at same time. With it, please remove
> variable "hub_diag" as well.
Thanks; will send another patch.
^ permalink raw reply
* [PATCH v2 0/9] cpuidle: rework device state count handling
From: Bartlomiej Zolnierkiewicz @ 2013-12-20 18:47 UTC (permalink / raw)
To: rjw
Cc: linux-samsung-soc, linux-pm, b.zolnierkie, daniel.lezcano,
linux-kernel, kyungmin.park, linuxppc-dev, lenb
Hi,
Some cpuidle drivers assume that cpuidle core will handle cases where
device->state_count is smaller than driver->state_count, unfortunately
currently this is untrue (device->state_count is used only for handling
cpuidle state sysfs entries and driver->state_count is used for all
other cases) and will not be fixed in the future as device->state_count
is planned to be removed [1].
This patchset fixes such drivers (ARM EXYNOS cpuidle driver and ACPI
cpuidle driver), removes superflous device->state_count initialization
from drivers for which device->state_count equals driver->state_count
(POWERPC pseries cpuidle driver and intel_idle driver) and finally
removes state_count field from struct cpuidle_device.
Additionaly (while at it) this patchset fixes C1E promotion disable
quirk handling (in intel_idle driver) and converts cpuidle drivers code
to use the common cpuidle_[un]register() routines (in POWERPC pseries
cpuidle driver and intel_idle driver).
[1] http://permalink.gmane.org/gmane.linux.power-management.general/36908
Reference to v1:
http://comments.gmane.org/gmane.linux.power-management.general/37390
Changes since v1:
- synced patch series with next-20131220
- added ACKs from Daniel Lezcano
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Bartlomiej Zolnierkiewicz (9):
ARM: EXYNOS: cpuidle: fix AFTR mode check
POWERPC: pseries: cpuidle: remove superfluous dev->state_count
initialization
POWERPC: pseries: cpuidle: use the common cpuidle_[un]register()
routines
ACPI / cpuidle: fix max idle state handling with hotplug CPU support
ACPI / cpuidle: remove dev->state_count setting
intel_idle: do C1E promotion disable quirk for hotplugged CPUs
intel_idle: remove superfluous dev->state_count initialization
intel_idle: use the common cpuidle_[un]register() routines
cpuidle: remove state_count field from struct cpuidle_device
arch/arm/mach-exynos/cpuidle.c | 8 +-
arch/powerpc/platforms/pseries/processor_idle.c | 59 +---------
drivers/acpi/processor_idle.c | 29 +++--
drivers/cpuidle/cpuidle.c | 3 -
drivers/cpuidle/sysfs.c | 5 +-
drivers/idle/intel_idle.c | 140 +++++-------------------
include/linux/cpuidle.h | 1 -
7 files changed, 51 insertions(+), 194 deletions(-)
--
1.8.2.3
^ permalink raw reply
* [PATCH v2 1/9] ARM: EXYNOS: cpuidle: fix AFTR mode check
From: Bartlomiej Zolnierkiewicz @ 2013-12-20 18:47 UTC (permalink / raw)
To: rjw
Cc: linux-samsung-soc, linux-pm, b.zolnierkie, daniel.lezcano,
linux-kernel, kyungmin.park, Kukjin Kim, linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-1-git-send-email-b.zolnierkie@samsung.com>
The EXYNOS cpuidle driver code assumes that cpuidle core will handle
dev->state_count smaller than drv->state_count but currently this is
untrue (dev->state_count is used only for handling cpuidle state sysfs
entries and drv->state_count is used for all other cases) and will not
be fixed in the future as dev->state_count is planned to be removed.
Fix the issue by checking for the max supported idle state in AFTR
state's ->enter handler (exynos4_enter_lowpower()) and entering AFTR
mode only when cores other than CPU0 are offline.
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>
---
arch/arm/mach-exynos/cpuidle.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index da65b03..f57cb91 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -172,8 +172,8 @@ static int exynos4_enter_lowpower(struct cpuidle_device *dev,
{
int new_index = index;
- /* This mode only can be entered when other core's are offline */
- if (num_online_cpus() > 1)
+ /* AFTR can only be entered when cores other than CPU0 are offline */
+ if (num_online_cpus() > 1 || dev->cpu != 0)
new_index = drv->safe_state_index;
if (new_index == 0)
@@ -235,10 +235,6 @@ static int exynos_cpuidle_probe(struct platform_device *pdev)
device = &per_cpu(exynos4_cpuidle_device, cpu_id);
device->cpu = cpu_id;
- /* Support IDLE only */
- if (cpu_id != 0)
- device->state_count = 1;
-
ret = cpuidle_register_device(device);
if (ret) {
dev_err(&pdev->dev, "failed to register cpuidle device\n");
--
1.8.2.3
^ permalink raw reply related
* [PATCH v2 2/9] POWERPC: pseries: cpuidle: remove superfluous dev->state_count initialization
From: Bartlomiej Zolnierkiewicz @ 2013-12-20 18:47 UTC (permalink / raw)
To: rjw
Cc: Deepthi Dharwar, linux-samsung-soc, linux-pm, b.zolnierkie,
daniel.lezcano, linux-kernel, kyungmin.park, linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-1-git-send-email-b.zolnierkie@samsung.com>
pseries cpuidle driver sets dev->state_count to drv->state_count so
the default dev->state_count initialization in cpuidle_enable_device()
(called from cpuidle_register_device()) can be used instead.
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/processor_idle.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/processor_idle.c b/arch/powerpc/platforms/pseries/processor_idle.c
index a166e38..8aa8c40 100644
--- a/arch/powerpc/platforms/pseries/processor_idle.c
+++ b/arch/powerpc/platforms/pseries/processor_idle.c
@@ -271,7 +271,6 @@ static void pseries_idle_devices_uninit(void)
static int pseries_idle_devices_init(void)
{
int i;
- struct cpuidle_driver *drv = &pseries_idle_driver;
struct cpuidle_device *dev;
pseries_cpuidle_devices = alloc_percpu(struct cpuidle_device);
@@ -280,7 +279,6 @@ static int pseries_idle_devices_init(void)
for_each_possible_cpu(i) {
dev = per_cpu_ptr(pseries_cpuidle_devices, i);
- dev->state_count = drv->state_count;
dev->cpu = i;
if (cpuidle_register_device(dev)) {
printk(KERN_DEBUG \
--
1.8.2.3
^ permalink raw reply related
* [PATCH v2 3/9] POWERPC: pseries: cpuidle: use the common cpuidle_[un]register() routines
From: Bartlomiej Zolnierkiewicz @ 2013-12-20 18:47 UTC (permalink / raw)
To: rjw
Cc: Deepthi Dharwar, linux-samsung-soc, linux-pm, b.zolnierkie,
daniel.lezcano, linux-kernel, kyungmin.park, linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-1-git-send-email-b.zolnierkie@samsung.com>
It is now possible to use the common cpuidle_[un]register() routines
(instead of open-coding them) so do it.
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/processor_idle.c | 57 ++-----------------------
1 file changed, 3 insertions(+), 54 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/processor_idle.c b/arch/powerpc/platforms/pseries/processor_idle.c
index 8aa8c40..94134a5 100644
--- a/arch/powerpc/platforms/pseries/processor_idle.c
+++ b/arch/powerpc/platforms/pseries/processor_idle.c
@@ -28,7 +28,6 @@ struct cpuidle_driver pseries_idle_driver = {
#define MAX_IDLE_STATE_COUNT 2
static int max_idle_state = MAX_IDLE_STATE_COUNT - 1;
-static struct cpuidle_device __percpu *pseries_cpuidle_devices;
static struct cpuidle_state *cpuidle_state_table;
static inline void idle_loop_prolog(unsigned long *in_purr)
@@ -191,7 +190,7 @@ static int pseries_cpuidle_add_cpu_notifier(struct notifier_block *n,
{
int hotcpu = (unsigned long)hcpu;
struct cpuidle_device *dev =
- per_cpu_ptr(pseries_cpuidle_devices, hotcpu);
+ per_cpu_ptr(cpuidle_devices, hotcpu);
if (dev && cpuidle_get_driver()) {
switch (action) {
@@ -248,48 +247,6 @@ static int pseries_cpuidle_driver_init(void)
return 0;
}
-/* pseries_idle_devices_uninit(void)
- * unregister cpuidle devices and de-allocate memory
- */
-static void pseries_idle_devices_uninit(void)
-{
- int i;
- struct cpuidle_device *dev;
-
- for_each_possible_cpu(i) {
- dev = per_cpu_ptr(pseries_cpuidle_devices, i);
- cpuidle_unregister_device(dev);
- }
-
- free_percpu(pseries_cpuidle_devices);
- return;
-}
-
-/* pseries_idle_devices_init()
- * allocate, initialize and register cpuidle device
- */
-static int pseries_idle_devices_init(void)
-{
- int i;
- struct cpuidle_device *dev;
-
- pseries_cpuidle_devices = alloc_percpu(struct cpuidle_device);
- if (pseries_cpuidle_devices == NULL)
- return -ENOMEM;
-
- for_each_possible_cpu(i) {
- dev = per_cpu_ptr(pseries_cpuidle_devices, i);
- dev->cpu = i;
- if (cpuidle_register_device(dev)) {
- printk(KERN_DEBUG \
- "cpuidle_register_device %d failed!\n", i);
- return -EIO;
- }
- }
-
- return 0;
-}
-
/*
* pseries_idle_probe()
* Choose state table for shared versus dedicated partition
@@ -325,19 +282,12 @@ static int __init pseries_processor_idle_init(void)
return retval;
pseries_cpuidle_driver_init();
- retval = cpuidle_register_driver(&pseries_idle_driver);
+ retval = cpuidle_register(&pseries_idle_driver, NULL);
if (retval) {
printk(KERN_DEBUG "Registration of pseries driver failed.\n");
return retval;
}
- retval = pseries_idle_devices_init();
- if (retval) {
- pseries_idle_devices_uninit();
- cpuidle_unregister_driver(&pseries_idle_driver);
- return retval;
- }
-
register_cpu_notifier(&setup_hotplug_notifier);
printk(KERN_DEBUG "pseries_idle_driver registered\n");
@@ -348,8 +298,7 @@ static void __exit pseries_processor_idle_exit(void)
{
unregister_cpu_notifier(&setup_hotplug_notifier);
- pseries_idle_devices_uninit();
- cpuidle_unregister_driver(&pseries_idle_driver);
+ cpuidle_unregister(&pseries_idle_driver);
return;
}
--
1.8.2.3
^ permalink raw reply related
* [PATCH v2 4/9] ACPI / cpuidle: fix max idle state handling with hotplug CPU support
From: Bartlomiej Zolnierkiewicz @ 2013-12-20 18:47 UTC (permalink / raw)
To: rjw
Cc: linux-samsung-soc, linux-pm, b.zolnierkie, daniel.lezcano,
linux-kernel, kyungmin.park, linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-1-git-send-email-b.zolnierkie@samsung.com>
acpi_processor_hotplug() calls acpi_processor_setup_cpuidle_cx()
without calling acpi_processor_setup_cpuidle_states() first so it
is possible that dev->state_count becomes different from
drv->state_count (in case of SMP system with unsupported C2/C3
states + enabled CPU hotplug and num_online_cpus() becoming > 1).
The driver code assumes that cpuidle core will handle such cases
but currently this is untrue (dev->state_count is used only for
handling cpuidle state sysfs entries and drv->state_count is used
for all other cases) and will not be fixed in the future as
dev->state_count is planned to be removed.
Fix the issue by checking for the max supported idle state in
C2/C3 state's ->enter handler (acpi_idle_enter_simple() for C2/C3
and acpi_idle_enter_bm() for C3 + bm_check flag set) and setting
the C1 state (instead of higher states) when needed.
Also remove no longer needed max idle state checks from
acpi_processor_setup_cpuidle_[states,cx]().
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Len Brown <lenb@kernel.org>
---
drivers/acpi/processor_idle.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index a0cc5ef4f..c080c99 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -783,6 +783,13 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
if (unlikely(!pr))
return -EINVAL;
+#ifdef CONFIG_HOTPLUG_CPU
+ if ((cx->type != ACPI_STATE_C1) && (num_online_cpus() > 1) &&
+ !pr->flags.has_cst &&
+ !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
+ return acpi_idle_enter_c1(dev, drv, CPUIDLE_DRIVER_STATE_START);
+#endif
+
if (cx->entry_method == ACPI_CSTATE_FFH) {
if (current_set_polling_and_test())
return -EINVAL;
@@ -829,6 +836,13 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
if (unlikely(!pr))
return -EINVAL;
+#ifdef CONFIG_HOTPLUG_CPU
+ if ((cx->type != ACPI_STATE_C1) && (num_online_cpus() > 1) &&
+ !pr->flags.has_cst &&
+ !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
+ return acpi_idle_enter_c1(dev, drv, CPUIDLE_DRIVER_STATE_START);
+#endif
+
if (!cx->bm_sts_skip && acpi_idle_bm_check()) {
if (drv->safe_state_index >= 0) {
return drv->states[drv->safe_state_index].enter(dev,
@@ -930,12 +944,6 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
if (!cx->valid)
continue;
-#ifdef CONFIG_HOTPLUG_CPU
- if ((cx->type != ACPI_STATE_C1) && (num_online_cpus() > 1) &&
- !pr->flags.has_cst &&
- !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
- continue;
-#endif
per_cpu(acpi_cstate[count], dev->cpu) = cx;
count++;
@@ -985,13 +993,6 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
if (!cx->valid)
continue;
-#ifdef CONFIG_HOTPLUG_CPU
- if ((cx->type != ACPI_STATE_C1) && (num_online_cpus() > 1) &&
- !pr->flags.has_cst &&
- !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
- continue;
-#endif
-
state = &drv->states[count];
snprintf(state->name, CPUIDLE_NAME_LEN, "C%d", i);
strncpy(state->desc, cx->desc, CPUIDLE_DESC_LEN);
--
1.8.2.3
^ permalink raw reply related
* [PATCH v2 5/9] ACPI / cpuidle: remove dev->state_count setting
From: Bartlomiej Zolnierkiewicz @ 2013-12-20 18:47 UTC (permalink / raw)
To: rjw
Cc: linux-samsung-soc, linux-pm, b.zolnierkie, daniel.lezcano,
linux-kernel, kyungmin.park, linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-1-git-send-email-b.zolnierkie@samsung.com>
dev->state_count is now always equal to drv->state_count and
drv->state_count no longer can change during driver's lifetime so
the default dev->state_count initialization in cpuidle_enable_device()
(called from cpuidle_register_device()) can be used instead.
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Len Brown <lenb@kernel.org>
---
drivers/acpi/processor_idle.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index c080c99..da8ce91 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -951,8 +951,6 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
break;
}
- dev->state_count = count;
-
if (!count)
return -EINVAL;
--
1.8.2.3
^ permalink raw reply related
* [PATCH v2 6/9] intel_idle: do C1E promotion disable quirk for hotplugged CPUs
From: Bartlomiej Zolnierkiewicz @ 2013-12-20 18:47 UTC (permalink / raw)
To: rjw
Cc: linux-samsung-soc, linux-pm, b.zolnierkie, daniel.lezcano,
linux-kernel, kyungmin.park, linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-1-git-send-email-b.zolnierkie@samsung.com>
If the system is booted with some CPUs offline C1E promotion disable quirk
won't be applied because on_each_cpu() in intel_idle_cpuidle_driver_init()
operates only on online CPUs. Fix it by adding the C1E promotion disable
handling to intel_idle_cpu_init() (which is also called during CPU_ONLINE
operation).
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Len Brown <lenb@kernel.org>
---
drivers/idle/intel_idle.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 92d1206..38da3fb 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -683,6 +683,9 @@ static int intel_idle_cpu_init(int cpu)
if (icpu->auto_demotion_disable_flags)
smp_call_function_single(cpu, auto_demotion_disable, NULL, 1);
+ if (icpu->disable_promotion_to_c1e)
+ smp_call_function_single(cpu, c1e_promotion_disable, NULL, 1);
+
return 0;
}
--
1.8.2.3
^ permalink raw reply related
* [PATCH v2 7/9] intel_idle: remove superfluous dev->state_count initialization
From: Bartlomiej Zolnierkiewicz @ 2013-12-20 18:47 UTC (permalink / raw)
To: rjw
Cc: linux-samsung-soc, linux-pm, b.zolnierkie, daniel.lezcano,
linux-kernel, kyungmin.park, linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-1-git-send-email-b.zolnierkie@samsung.com>
intel_idle driver sets dev->state_count to drv->state_count so
the default dev->state_count initialization in cpuidle_enable_device()
(called from cpuidle_register_device()) can be used instead.
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Len Brown <lenb@kernel.org>
---
drivers/idle/intel_idle.c | 29 -----------------------------
1 file changed, 29 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 38da3fb..524d07b 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -639,39 +639,10 @@ static int __init intel_idle_cpuidle_driver_init(void)
*/
static int intel_idle_cpu_init(int cpu)
{
- int cstate;
struct cpuidle_device *dev;
dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
- dev->state_count = 1;
-
- for (cstate = 0; cstate < CPUIDLE_STATE_MAX; ++cstate) {
- int num_substates, mwait_hint, mwait_cstate, mwait_substate;
-
- if (cpuidle_state_table[cstate].enter == NULL)
- break;
-
- if (cstate + 1 > max_cstate) {
- printk(PREFIX "max_cstate %d reached\n", max_cstate);
- break;
- }
-
- mwait_hint = flg2MWAIT(cpuidle_state_table[cstate].flags);
- mwait_cstate = MWAIT_HINT2CSTATE(mwait_hint);
- mwait_substate = MWAIT_HINT2SUBSTATE(mwait_hint);
-
- /* does the state exist in CPUID.MWAIT? */
- num_substates = (mwait_substates >> ((mwait_cstate + 1) * 4))
- & MWAIT_SUBSTATE_MASK;
-
- /* if sub-state in table is not enumerated by CPUID */
- if ((mwait_substate + 1) > num_substates)
- continue;
-
- dev->state_count += 1;
- }
-
dev->cpu = cpu;
if (cpuidle_register_device(dev)) {
--
1.8.2.3
^ permalink raw reply related
* [PATCH v2 8/9] intel_idle: use the common cpuidle_[un]register() routines
From: Bartlomiej Zolnierkiewicz @ 2013-12-20 18:47 UTC (permalink / raw)
To: rjw
Cc: linux-samsung-soc, linux-pm, b.zolnierkie, daniel.lezcano,
linux-kernel, kyungmin.park, linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-1-git-send-email-b.zolnierkie@samsung.com>
It is now possible to use the common cpuidle_[un]register() routines
(instead of open-coding them) so do it.
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Len Brown <lenb@kernel.org>
---
drivers/idle/intel_idle.c | 114 ++++++++++++----------------------------------
1 file changed, 29 insertions(+), 85 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 524d07b..a1a4dbd 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -93,10 +93,8 @@ struct idle_cpu {
};
static const struct idle_cpu *icpu;
-static struct cpuidle_device __percpu *intel_idle_cpuidle_devices;
static int intel_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int index);
-static int intel_idle_cpu_init(int cpu);
static struct cpuidle_state *cpuidle_state_table;
@@ -400,11 +398,27 @@ static void __setup_broadcast_timer(void *arg)
clockevents_notify(reason, &cpu);
}
+static void auto_demotion_disable(void *dummy)
+{
+ unsigned long long msr_bits;
+
+ rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
+ msr_bits &= ~(icpu->auto_demotion_disable_flags);
+ wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
+}
+static void c1e_promotion_disable(void *dummy)
+{
+ unsigned long long msr_bits;
+
+ rdmsrl(MSR_IA32_POWER_CTL, msr_bits);
+ msr_bits &= ~0x2;
+ wrmsrl(MSR_IA32_POWER_CTL, msr_bits);
+}
+
static int cpu_hotplug_notify(struct notifier_block *n,
unsigned long action, void *hcpu)
{
int hotcpu = (unsigned long)hcpu;
- struct cpuidle_device *dev;
switch (action & ~CPU_TASKS_FROZEN) {
case CPU_ONLINE:
@@ -416,11 +430,15 @@ static int cpu_hotplug_notify(struct notifier_block *n,
/*
* Some systems can hotplug a cpu at runtime after
* the kernel has booted, we have to initialize the
- * driver in this case
+ * hardware in this case.
*/
- dev = per_cpu_ptr(intel_idle_cpuidle_devices, hotcpu);
- if (!dev->registered)
- intel_idle_cpu_init(hotcpu);
+ if (icpu->auto_demotion_disable_flags)
+ smp_call_function_single(hotcpu, auto_demotion_disable,
+ NULL, 1);
+
+ if (icpu->disable_promotion_to_c1e)
+ smp_call_function_single(hotcpu, c1e_promotion_disable,
+ NULL, 1);
break;
}
@@ -431,23 +449,6 @@ static struct notifier_block cpu_hotplug_notifier = {
.notifier_call = cpu_hotplug_notify,
};
-static void auto_demotion_disable(void *dummy)
-{
- unsigned long long msr_bits;
-
- rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
- msr_bits &= ~(icpu->auto_demotion_disable_flags);
- wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
-}
-static void c1e_promotion_disable(void *dummy)
-{
- unsigned long long msr_bits;
-
- rdmsrl(MSR_IA32_POWER_CTL, msr_bits);
- msr_bits &= ~0x2;
- wrmsrl(MSR_IA32_POWER_CTL, msr_bits);
-}
-
static const struct idle_cpu idle_cpu_nehalem = {
.state_table = nehalem_cstates,
.auto_demotion_disable_flags = NHM_C1_AUTO_DEMOTE | NHM_C3_AUTO_DEMOTE,
@@ -560,23 +561,6 @@ static int __init intel_idle_probe(void)
}
/*
- * intel_idle_cpuidle_devices_uninit()
- * unregister, free cpuidle_devices
- */
-static void intel_idle_cpuidle_devices_uninit(void)
-{
- int i;
- struct cpuidle_device *dev;
-
- for_each_online_cpu(i) {
- dev = per_cpu_ptr(intel_idle_cpuidle_devices, i);
- cpuidle_unregister_device(dev);
- }
-
- free_percpu(intel_idle_cpuidle_devices);
- return;
-}
-/*
* intel_idle_cpuidle_driver_init()
* allocate, initialize cpuidle_states
*/
@@ -632,37 +616,9 @@ static int __init intel_idle_cpuidle_driver_init(void)
}
-/*
- * intel_idle_cpu_init()
- * allocate, initialize, register cpuidle_devices
- * @cpu: cpu/core to initialize
- */
-static int intel_idle_cpu_init(int cpu)
-{
- struct cpuidle_device *dev;
-
- dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
-
- dev->cpu = cpu;
-
- if (cpuidle_register_device(dev)) {
- pr_debug(PREFIX "cpuidle_register_device %d failed!\n", cpu);
- intel_idle_cpuidle_devices_uninit();
- return -EIO;
- }
-
- if (icpu->auto_demotion_disable_flags)
- smp_call_function_single(cpu, auto_demotion_disable, NULL, 1);
-
- if (icpu->disable_promotion_to_c1e)
- smp_call_function_single(cpu, c1e_promotion_disable, NULL, 1);
-
- return 0;
-}
-
static int __init intel_idle_init(void)
{
- int retval, i;
+ int retval;
/* Do not load intel_idle at all for now if idle= is passed */
if (boot_option_idle_override != IDLE_NO_OVERRIDE)
@@ -673,7 +629,8 @@ static int __init intel_idle_init(void)
return retval;
intel_idle_cpuidle_driver_init();
- retval = cpuidle_register_driver(&intel_idle_driver);
+
+ retval = cpuidle_register(&intel_idle_driver, NULL);
if (retval) {
struct cpuidle_driver *drv = cpuidle_get_driver();
printk(KERN_DEBUG PREFIX "intel_idle yielding to %s",
@@ -681,17 +638,6 @@ static int __init intel_idle_init(void)
return retval;
}
- intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device);
- if (intel_idle_cpuidle_devices == NULL)
- return -ENOMEM;
-
- for_each_online_cpu(i) {
- retval = intel_idle_cpu_init(i);
- if (retval) {
- cpuidle_unregister_driver(&intel_idle_driver);
- return retval;
- }
- }
register_cpu_notifier(&cpu_hotplug_notifier);
return 0;
@@ -699,9 +645,7 @@ static int __init intel_idle_init(void)
static void __exit intel_idle_exit(void)
{
- intel_idle_cpuidle_devices_uninit();
- cpuidle_unregister_driver(&intel_idle_driver);
-
+ cpuidle_unregister(&intel_idle_driver);
if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
on_each_cpu(__setup_broadcast_timer, (void *)false, 1);
--
1.8.2.3
^ permalink raw reply related
* [PATCH v2 9/9] cpuidle: remove state_count field from struct cpuidle_device
From: Bartlomiej Zolnierkiewicz @ 2013-12-20 18:47 UTC (permalink / raw)
To: rjw
Cc: linux-samsung-soc, linux-pm, b.zolnierkie, daniel.lezcano,
linux-kernel, kyungmin.park, linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-1-git-send-email-b.zolnierkie@samsung.com>
dev->state_count is now always equal to drv->state_count so
it can be removed.
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/cpuidle/cpuidle.c | 3 ---
drivers/cpuidle/sysfs.c | 5 +++--
include/linux/cpuidle.h | 1 -
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index a55e68f..e3d2052 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -252,9 +252,6 @@ int cpuidle_enable_device(struct cpuidle_device *dev)
if (!dev->registered)
return -EINVAL;
- if (!dev->state_count)
- dev->state_count = drv->state_count;
-
ret = cpuidle_add_device_sysfs(dev);
if (ret)
return ret;
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index e918b6d..dcaae4c 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -398,7 +398,7 @@ static int cpuidle_add_state_sysfs(struct cpuidle_device *device)
struct cpuidle_driver *drv = cpuidle_get_cpu_driver(device);
/* state statistics */
- for (i = 0; i < device->state_count; i++) {
+ for (i = 0; i < drv->state_count; i++) {
kobj = kzalloc(sizeof(struct cpuidle_state_kobj), GFP_KERNEL);
if (!kobj)
goto error_state;
@@ -430,9 +430,10 @@ error_state:
*/
static void cpuidle_remove_state_sysfs(struct cpuidle_device *device)
{
+ struct cpuidle_driver *drv = cpuidle_get_cpu_driver(device);
int i;
- for (i = 0; i < device->state_count; i++)
+ for (i = 0; i < drv->state_count; i++)
cpuidle_free_state_kobj(device, i);
}
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 50fcbb0..d133817 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -69,7 +69,6 @@ struct cpuidle_device {
unsigned int cpu;
int last_residency;
- int state_count;
struct cpuidle_state_usage states_usage[CPUIDLE_STATE_MAX];
struct cpuidle_state_kobj *kobjs[CPUIDLE_STATE_MAX];
struct cpuidle_driver_kobj *kobj_driver;
--
1.8.2.3
^ permalink raw reply related
* [PATCH v2] powernv: eeh: add buffer for P7IOC hub error data
From: Brian W Hart @ 2013-12-20 19:06 UTC (permalink / raw)
To: linuxppc-dev
V2: Replace driver-global 'hub_diag' with a per-PHB hub diag structure.
Prevent ioda_eeh_hub_diag() from clobbering itself when called by supplying
a per-PHB buffer for P7IOC hub diagnostic data. Take care to inform OPAL of
the correct size for the buffer.
Signed-off-by: Brian W Hart <hartb@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/eeh-ioda.c | 15 ++-------------
arch/powerpc/platforms/powernv/pci.h | 4 +++-
2 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 8184ef5..4790275 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -36,7 +36,6 @@
#include "powernv.h"
#include "pci.h"
-static char *hub_diag = NULL;
static int ioda_eeh_nb_init = 0;
static int ioda_eeh_event(struct notifier_block *nb,
@@ -140,15 +139,6 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
ioda_eeh_nb_init = 1;
}
- /* We needn't HUB diag-data on PHB3 */
- if (phb->type == PNV_PHB_IODA1 && !hub_diag) {
- hub_diag = (char *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
- if (!hub_diag) {
- pr_err("%s: Out of memory !\n", __func__);
- return -ENOMEM;
- }
- }
-
#ifdef CONFIG_DEBUG_FS
if (phb->dbgfs) {
debugfs_create_file("err_injct_outbound", 0600,
@@ -633,11 +623,10 @@ static void ioda_eeh_hub_diag_common(struct OpalIoP7IOCErrorData *data)
static void ioda_eeh_hub_diag(struct pci_controller *hose)
{
struct pnv_phb *phb = hose->private_data;
- struct OpalIoP7IOCErrorData *data;
+ struct OpalIoP7IOCErrorData *data = &phb->diag.hub_diag;
long rc;
- data = (struct OpalIoP7IOCErrorData *)ioda_eeh_hub_diag;
- rc = opal_pci_get_hub_diag_data(phb->hub_id, data, PAGE_SIZE);
+ rc = opal_pci_get_hub_diag_data(phb->hub_id, data, sizeof *data);
if (rc != OPAL_SUCCESS) {
pr_warning("%s: Failed to get HUB#%llx diag-data (%ld)\n",
__func__, phb->hub_id, rc);
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 911c24e..1ed8d5f 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -172,11 +172,13 @@ struct pnv_phb {
} ioda;
};
- /* PHB status structure */
+ /* PHB and hub status structure */
union {
unsigned char blob[PNV_PCI_DIAG_BUF_SIZE];
struct OpalIoP7IOCPhbErrorData p7ioc;
+ struct OpalIoP7IOCErrorData hub_diag;
} diag;
+
};
extern struct pci_ops pnv_pci_ops;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 1/9] ARM: EXYNOS: cpuidle: fix AFTR mode check
From: Kukjin Kim @ 2013-12-20 20:47 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: linux-samsung-soc, linux-pm, daniel.lezcano, rjw, linux-kernel,
kyungmin.park, Kukjin Kim, linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-2-git-send-email-b.zolnierkie@samsung.com>
On 12/21/13 03:47, Bartlomiej Zolnierkiewicz wrote:
> The EXYNOS cpuidle driver code assumes that cpuidle core will handle
> dev->state_count smaller than drv->state_count but currently this is
> untrue (dev->state_count is used only for handling cpuidle state sysfs
> entries and drv->state_count is used for all other cases) and will not
> be fixed in the future as dev->state_count is planned to be removed.
>
> Fix the issue by checking for the max supported idle state in AFTR
> state's ->enter handler (exynos4_enter_lowpower()) and entering AFTR
> mode only when cores other than CPU0 are offline.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz<b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
> Acked-by: Daniel Lezcano<daniel.lezcano@linaro.org>
> Cc: Kukjin Kim<kgene.kim@samsung.com>
Acked-by: Kukjin Kim <kgene.kim@samsung.com>
Thanks,
Kukjin
> ---
> arch/arm/mach-exynos/cpuidle.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
> index da65b03..f57cb91 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -172,8 +172,8 @@ static int exynos4_enter_lowpower(struct cpuidle_device *dev,
> {
> int new_index = index;
>
> - /* This mode only can be entered when other core's are offline */
> - if (num_online_cpus()> 1)
> + /* AFTR can only be entered when cores other than CPU0 are offline */
> + if (num_online_cpus()> 1 || dev->cpu != 0)
> new_index = drv->safe_state_index;
>
> if (new_index == 0)
> @@ -235,10 +235,6 @@ static int exynos_cpuidle_probe(struct platform_device *pdev)
> device =&per_cpu(exynos4_cpuidle_device, cpu_id);
> device->cpu = cpu_id;
>
> - /* Support IDLE only */
> - if (cpu_id != 0)
> - device->state_count = 1;
> -
> ret = cpuidle_register_device(device);
> if (ret) {
> dev_err(&pdev->dev, "failed to register cpuidle device\n");
^ permalink raw reply
* Re: [PATCH v2 1/9] ARM: EXYNOS: cpuidle: fix AFTR mode check
From: Daniel Lezcano @ 2013-12-20 21:16 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, rjw
Cc: linux-samsung-soc, linux-pm, linux-kernel, kyungmin.park,
Kukjin Kim, linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-2-git-send-email-b.zolnierkie@samsung.com>
On 12/20/2013 07:47 PM, Bartlomiej Zolnierkiewicz wrote:
> The EXYNOS cpuidle driver code assumes that cpuidle core will handle
> dev->state_count smaller than drv->state_count but currently this is
> untrue (dev->state_count is used only for handling cpuidle state sysfs
> entries and drv->state_count is used for all other cases) and will not
> be fixed in the future as dev->state_count is planned to be removed.
>
> Fix the issue by checking for the max supported idle state in AFTR
> state's ->enter handler (exynos4_enter_lowpower()) and entering AFTR
> mode only when cores other than CPU0 are offline.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> ---
> arch/arm/mach-exynos/cpuidle.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
> index da65b03..f57cb91 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -172,8 +172,8 @@ static int exynos4_enter_lowpower(struct cpuidle_device *dev,
> {
> int new_index = index;
>
> - /* This mode only can be entered when other core's are offline */
> - if (num_online_cpus() > 1)
> + /* AFTR can only be entered when cores other than CPU0 are offline */
> + if (num_online_cpus() > 1 || dev->cpu != 0)
> new_index = drv->safe_state_index;
>
> if (new_index == 0)
> @@ -235,10 +235,6 @@ static int exynos_cpuidle_probe(struct platform_device *pdev)
> device = &per_cpu(exynos4_cpuidle_device, cpu_id);
> device->cpu = cpu_id;
>
> - /* Support IDLE only */
> - if (cpu_id != 0)
> - device->state_count = 1;
> -
> ret = cpuidle_register_device(device);
> if (ret) {
> dev_err(&pdev->dev, "failed to register cpuidle device\n");
>
Hi Bartlomiej,
thanks for this cleanup. May be you can also add another patch to switch
to the generic cpuidle_register function ?
Thanks
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH v2] powerpc/512x: dts: disable MPC5125 usb module
From: Anatolij Gustschin @ 2013-12-20 21:16 UTC (permalink / raw)
To: Matteo Facchinetti; +Cc: gsi, linuxppc-dev
In-Reply-To: <1387530982-6115-1-git-send-email-matteo.facchinetti@sirius-es.it>
On Fri, 20 Dec 2013 10:16:22 +0100
Matteo Facchinetti <matteo.facchinetti@sirius-es.it> wrote:
> At the moment the USB controller's pin muxing is not setup
> correctly and causes a kernel panic upon system startup, so
> disable the USB1 device tree node in the MPC5125 tower board
> dts file.
>
> The USB controller is connected to an USB3320 ULPI transceiver
> and the device tree should receive an update to reflect correct
> dependencies and required initialization data before the USB1
> node can get re-enabled.
>
> Signed-off-by: Matteo Facchinetti <matteo.facchinetti@sirius-es.it>
> ---
> v2:
> * improve the text of the commit as suggested by Gerhard Sittig <gsi@denx.de>
> * put the 'status = "disabled"' to the last line in the list of properties
> * rewiew the comment related to USB1 device tree node
> ---
> arch/powerpc/boot/dts/mpc5125twr.dts | 5 +++++
> 1 file changed, 5 insertions(+)
Applied to merge branch. Thanks!
Anatolij
^ permalink raw reply
* Re: [PATCH v2 9/9] cpuidle: remove state_count field from struct cpuidle_device
From: Daniel Lezcano @ 2013-12-20 21:27 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, rjw
Cc: linux-samsung-soc, linux-pm, linux-kernel, kyungmin.park,
linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-10-git-send-email-b.zolnierkie@samsung.com>
On 12/20/2013 07:47 PM, Bartlomiej Zolnierkiewicz wrote:
> dev->state_count is now always equal to drv->state_count so
> it can be removed.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> drivers/cpuidle/cpuidle.c | 3 ---
> drivers/cpuidle/sysfs.c | 5 +++--
> include/linux/cpuidle.h | 1 -
> 3 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index a55e68f..e3d2052 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -252,9 +252,6 @@ int cpuidle_enable_device(struct cpuidle_device *dev)
> if (!dev->registered)
> return -EINVAL;
>
> - if (!dev->state_count)
> - dev->state_count = drv->state_count;
> -
> ret = cpuidle_add_device_sysfs(dev);
> if (ret)
> return ret;
> diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
> index e918b6d..dcaae4c 100644
> --- a/drivers/cpuidle/sysfs.c
> +++ b/drivers/cpuidle/sysfs.c
> @@ -398,7 +398,7 @@ static int cpuidle_add_state_sysfs(struct cpuidle_device *device)
> struct cpuidle_driver *drv = cpuidle_get_cpu_driver(device);
>
> /* state statistics */
> - for (i = 0; i < device->state_count; i++) {
> + for (i = 0; i < drv->state_count; i++) {
> kobj = kzalloc(sizeof(struct cpuidle_state_kobj), GFP_KERNEL);
> if (!kobj)
> goto error_state;
> @@ -430,9 +430,10 @@ error_state:
> */
> static void cpuidle_remove_state_sysfs(struct cpuidle_device *device)
> {
> + struct cpuidle_driver *drv = cpuidle_get_cpu_driver(device);
> int i;
>
> - for (i = 0; i < device->state_count; i++)
> + for (i = 0; i < drv->state_count; i++)
> cpuidle_free_state_kobj(device, i);
> }
>
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 50fcbb0..d133817 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -69,7 +69,6 @@ struct cpuidle_device {
> unsigned int cpu;
>
> int last_residency;
> - int state_count;
> struct cpuidle_state_usage states_usage[CPUIDLE_STATE_MAX];
> struct cpuidle_state_kobj *kobjs[CPUIDLE_STATE_MAX];
> struct cpuidle_driver_kobj *kobj_driver;
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH v2 8/9] intel_idle: use the common cpuidle_[un]register() routines
From: Daniel Lezcano @ 2013-12-20 21:42 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, rjw
Cc: linux-samsung-soc, linux-pm, linux-kernel, kyungmin.park,
linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-9-git-send-email-b.zolnierkie@samsung.com>
On 12/20/2013 07:47 PM, Bartlomiej Zolnierkiewicz wrote:
> It is now possible to use the common cpuidle_[un]register() routines
> (instead of open-coding them) so do it.
Just an addition:
The cpuidle_register common routine calls cpuidle_register_driver which
initialize the driver's cpumask to cpu_possible_mask if not set (which
is the default on most platform) and right after it uses this mask to
register the cpuidle devices. That's why the cpu hotplug does not need
to register the device unlike before this patch where the cpumask was
cpu_online_mask. So we can't fall in the "Some systems can hotplug a cpu
at runtime after the kernel has booted" case.
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Len Brown <lenb@kernel.org>
> ---
> drivers/idle/intel_idle.c | 114 ++++++++++++----------------------------------
> 1 file changed, 29 insertions(+), 85 deletions(-)
>
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 524d07b..a1a4dbd 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -93,10 +93,8 @@ struct idle_cpu {
> };
>
> static const struct idle_cpu *icpu;
> -static struct cpuidle_device __percpu *intel_idle_cpuidle_devices;
> static int intel_idle(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int index);
> -static int intel_idle_cpu_init(int cpu);
>
> static struct cpuidle_state *cpuidle_state_table;
>
> @@ -400,11 +398,27 @@ static void __setup_broadcast_timer(void *arg)
> clockevents_notify(reason, &cpu);
> }
>
> +static void auto_demotion_disable(void *dummy)
> +{
> + unsigned long long msr_bits;
> +
> + rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
> + msr_bits &= ~(icpu->auto_demotion_disable_flags);
> + wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
> +}
> +static void c1e_promotion_disable(void *dummy)
> +{
> + unsigned long long msr_bits;
> +
> + rdmsrl(MSR_IA32_POWER_CTL, msr_bits);
> + msr_bits &= ~0x2;
> + wrmsrl(MSR_IA32_POWER_CTL, msr_bits);
> +}
> +
> static int cpu_hotplug_notify(struct notifier_block *n,
> unsigned long action, void *hcpu)
> {
> int hotcpu = (unsigned long)hcpu;
> - struct cpuidle_device *dev;
>
> switch (action & ~CPU_TASKS_FROZEN) {
> case CPU_ONLINE:
> @@ -416,11 +430,15 @@ static int cpu_hotplug_notify(struct notifier_block *n,
> /*
> * Some systems can hotplug a cpu at runtime after
> * the kernel has booted, we have to initialize the
> - * driver in this case
> + * hardware in this case.
> */
> - dev = per_cpu_ptr(intel_idle_cpuidle_devices, hotcpu);
> - if (!dev->registered)
> - intel_idle_cpu_init(hotcpu);
> + if (icpu->auto_demotion_disable_flags)
> + smp_call_function_single(hotcpu, auto_demotion_disable,
> + NULL, 1);
> +
> + if (icpu->disable_promotion_to_c1e)
> + smp_call_function_single(hotcpu, c1e_promotion_disable,
> + NULL, 1);
>
> break;
> }
> @@ -431,23 +449,6 @@ static struct notifier_block cpu_hotplug_notifier = {
> .notifier_call = cpu_hotplug_notify,
> };
>
> -static void auto_demotion_disable(void *dummy)
> -{
> - unsigned long long msr_bits;
> -
> - rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
> - msr_bits &= ~(icpu->auto_demotion_disable_flags);
> - wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
> -}
> -static void c1e_promotion_disable(void *dummy)
> -{
> - unsigned long long msr_bits;
> -
> - rdmsrl(MSR_IA32_POWER_CTL, msr_bits);
> - msr_bits &= ~0x2;
> - wrmsrl(MSR_IA32_POWER_CTL, msr_bits);
> -}
> -
> static const struct idle_cpu idle_cpu_nehalem = {
> .state_table = nehalem_cstates,
> .auto_demotion_disable_flags = NHM_C1_AUTO_DEMOTE | NHM_C3_AUTO_DEMOTE,
> @@ -560,23 +561,6 @@ static int __init intel_idle_probe(void)
> }
>
> /*
> - * intel_idle_cpuidle_devices_uninit()
> - * unregister, free cpuidle_devices
> - */
> -static void intel_idle_cpuidle_devices_uninit(void)
> -{
> - int i;
> - struct cpuidle_device *dev;
> -
> - for_each_online_cpu(i) {
> - dev = per_cpu_ptr(intel_idle_cpuidle_devices, i);
> - cpuidle_unregister_device(dev);
> - }
> -
> - free_percpu(intel_idle_cpuidle_devices);
> - return;
> -}
> -/*
> * intel_idle_cpuidle_driver_init()
> * allocate, initialize cpuidle_states
> */
> @@ -632,37 +616,9 @@ static int __init intel_idle_cpuidle_driver_init(void)
> }
>
>
> -/*
> - * intel_idle_cpu_init()
> - * allocate, initialize, register cpuidle_devices
> - * @cpu: cpu/core to initialize
> - */
> -static int intel_idle_cpu_init(int cpu)
> -{
> - struct cpuidle_device *dev;
> -
> - dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
> -
> - dev->cpu = cpu;
> -
> - if (cpuidle_register_device(dev)) {
> - pr_debug(PREFIX "cpuidle_register_device %d failed!\n", cpu);
> - intel_idle_cpuidle_devices_uninit();
> - return -EIO;
> - }
> -
> - if (icpu->auto_demotion_disable_flags)
> - smp_call_function_single(cpu, auto_demotion_disable, NULL, 1);
> -
> - if (icpu->disable_promotion_to_c1e)
> - smp_call_function_single(cpu, c1e_promotion_disable, NULL, 1);
> -
> - return 0;
> -}
> -
> static int __init intel_idle_init(void)
> {
> - int retval, i;
> + int retval;
>
> /* Do not load intel_idle at all for now if idle= is passed */
> if (boot_option_idle_override != IDLE_NO_OVERRIDE)
> @@ -673,7 +629,8 @@ static int __init intel_idle_init(void)
> return retval;
>
> intel_idle_cpuidle_driver_init();
> - retval = cpuidle_register_driver(&intel_idle_driver);
> +
> + retval = cpuidle_register(&intel_idle_driver, NULL);
> if (retval) {
> struct cpuidle_driver *drv = cpuidle_get_driver();
> printk(KERN_DEBUG PREFIX "intel_idle yielding to %s",
> @@ -681,17 +638,6 @@ static int __init intel_idle_init(void)
> return retval;
> }
>
> - intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device);
> - if (intel_idle_cpuidle_devices == NULL)
> - return -ENOMEM;
> -
> - for_each_online_cpu(i) {
> - retval = intel_idle_cpu_init(i);
> - if (retval) {
> - cpuidle_unregister_driver(&intel_idle_driver);
> - return retval;
> - }
> - }
> register_cpu_notifier(&cpu_hotplug_notifier);
>
> return 0;
> @@ -699,9 +645,7 @@ static int __init intel_idle_init(void)
>
> static void __exit intel_idle_exit(void)
> {
> - intel_idle_cpuidle_devices_uninit();
> - cpuidle_unregister_driver(&intel_idle_driver);
> -
> + cpuidle_unregister(&intel_idle_driver);
>
> if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
> on_each_cpu(__setup_broadcast_timer, (void *)false, 1);
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH v2 7/9] intel_idle: remove superfluous dev->state_count initialization
From: Daniel Lezcano @ 2013-12-20 21:48 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, rjw
Cc: linux-samsung-soc, linux-pm, linux-kernel, kyungmin.park,
linuxppc-dev, lenb
In-Reply-To: <1387565251-7051-8-git-send-email-b.zolnierkie@samsung.com>
On 12/20/2013 07:47 PM, Bartlomiej Zolnierkiewicz wrote:
> intel_idle driver sets dev->state_count to drv->state_count so
> the default dev->state_count initialization in cpuidle_enable_device()
> (called from cpuidle_register_device()) can be used instead.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Len Brown <lenb@kernel.org>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> drivers/idle/intel_idle.c | 29 -----------------------------
> 1 file changed, 29 deletions(-)
>
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 38da3fb..524d07b 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -639,39 +639,10 @@ static int __init intel_idle_cpuidle_driver_init(void)
> */
> static int intel_idle_cpu_init(int cpu)
> {
> - int cstate;
> struct cpuidle_device *dev;
>
> dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
>
> - dev->state_count = 1;
> -
> - for (cstate = 0; cstate < CPUIDLE_STATE_MAX; ++cstate) {
> - int num_substates, mwait_hint, mwait_cstate, mwait_substate;
> -
> - if (cpuidle_state_table[cstate].enter == NULL)
> - break;
> -
> - if (cstate + 1 > max_cstate) {
> - printk(PREFIX "max_cstate %d reached\n", max_cstate);
> - break;
> - }
> -
> - mwait_hint = flg2MWAIT(cpuidle_state_table[cstate].flags);
> - mwait_cstate = MWAIT_HINT2CSTATE(mwait_hint);
> - mwait_substate = MWAIT_HINT2SUBSTATE(mwait_hint);
> -
> - /* does the state exist in CPUID.MWAIT? */
> - num_substates = (mwait_substates >> ((mwait_cstate + 1) * 4))
> - & MWAIT_SUBSTATE_MASK;
> -
> - /* if sub-state in table is not enumerated by CPUID */
> - if ((mwait_substate + 1) > num_substates)
> - continue;
> -
> - dev->state_count += 1;
> - }
> -
> dev->cpu = cpu;
>
> if (cpuidle_register_device(dev)) {
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox