* Re: [PATCH 2/2] i2c-ibm_iic driver
From: Sean MacLennan @ 2008-02-19 1:42 UTC (permalink / raw)
To: Jean Delvare; +Cc: LinuxPPC-dev, i2c
In-Reply-To: <20080216103123.50a0128b@hyperion.delvare>
An updated version of the patch. This one should answer all of Jean's
concerns.
Cheers,
Sean
Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
---
--- for-2.6.25/drivers/i2c/busses/orig-i2c-ibm_iic.c 2008-02-18 16:36:30.000000000 -0500
+++ for-2.6.25/drivers/i2c/busses/i2c-ibm_iic.c 2008-02-18 17:39:53.000000000 -0500
@@ -6,6 +6,9 @@
* Copyright (c) 2003, 2004 Zultys Technologies.
* Eugene Surovegin <eugene.surovegin@zultys.com> or <ebs@ebshome.net>
*
+ * Copyright (c) 2008 PIKA Technologies
+ * Sean MacLennan <smaclennan@pikatech.com>
+ *
* Based on original work by
* Ian DaSilva <idasilva@mvista.com>
* Armin Kuster <akuster@mvista.com>
@@ -39,12 +42,17 @@
#include <asm/io.h>
#include <linux/i2c.h>
#include <linux/i2c-id.h>
+
+#ifdef CONFIG_IBM_OCP
#include <asm/ocp.h>
#include <asm/ibm4xx.h>
+#else
+#include <linux/of_platform.h>
+#endif
#include "i2c-ibm_iic.h"
-#define DRIVER_VERSION "2.1"
+#define DRIVER_VERSION "2.2"
MODULE_DESCRIPTION("IBM IIC driver v" DRIVER_VERSION);
MODULE_LICENSE("GPL");
@@ -657,6 +665,7 @@
return (u8)((opb + 9) / 10 - 1);
}
+#ifdef CONFIG_IBM_OCP
/*
* Register single IIC interface
*/
@@ -828,5 +837,182 @@
ocp_unregister_driver(&ibm_iic_driver);
}
+#else /* !CONFIG_IBM_OCP */
+
+static int __devinit iic_request_irq(struct of_device *ofdev,
+ struct ibm_iic_private *dev)
+{
+ struct device_node *np = ofdev->node;
+ int irq;
+
+ if (iic_force_poll)
+ return NO_IRQ;
+
+ irq = irq_of_parse_and_map(np, 0);
+ if (irq == NO_IRQ) {
+ dev_err(&ofdev->dev, "irq_of_parse_and_map failed\n");
+ return NO_IRQ;
+ }
+
+ /* 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);
+ /* Fallback to the polling mode */
+ return NO_IRQ;
+ }
+
+ return irq;
+}
+
+/*
+ * Register single IIC interface
+ */
+static int __devinit iic_probe(struct of_device *ofdev,
+ const struct of_device_id *match)
+{
+ struct device_node *np = ofdev->node;
+ struct ibm_iic_private *dev;
+ struct i2c_adapter *adap;
+ const u32 *indexp, *freq;
+ int ret;
+
+
+ dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (!dev) {
+ dev_err(&ofdev->dev, "failed to allocate device data\n");
+ return -ENOMEM;
+ }
+
+ dev_set_drvdata(&ofdev->dev, dev);
+
+ indexp = of_get_property(np, "index", NULL);
+ if (!indexp) {
+ dev_err(&ofdev->dev, "no index specified.\n");
+ ret = -EINVAL;
+ goto error_cleanup;
+ }
+ dev->idx = *indexp;
+
+ dev->vaddr = of_iomap(np, 0);
+ if (dev->vaddr == NULL) {
+ dev_err(&ofdev->dev, "failed to iomap device\n");
+ ret = -ENXIO;
+ goto error_cleanup;
+ }
+
+ init_waitqueue_head(&dev->wq);
+
+ dev->irq = iic_request_irq(ofdev, dev);
+ if (dev->irq == NO_IRQ)
+ dev_warn(&ofdev->dev, "using polling mode\n");
+
+ /* Board specific settings */
+ if (iic_force_fast || of_get_property(np, "fast-mode", NULL))
+ dev->fast_mode = 1;
+
+ freq = of_get_property(np, "clock-frequency", NULL);
+ if (freq == NULL) {
+ freq = of_get_property(np->parent, "clock-frequency", NULL);
+ if (freq == NULL) {
+ dev_err(&ofdev->dev, "Unable to get bus frequency\n");
+ ret = -EBUSY;
+ goto error_cleanup;
+ }
+ }
+
+ dev->clckdiv = iic_clckdiv(*freq);
+ dev_dbg(&ofdev->dev, "clckdiv = %d\n", dev->clckdiv);
+
+ /* Initialize IIC interface */
+ iic_dev_init(dev);
+
+ /* Register it with i2c layer */
+ adap = &dev->adap;
+ adap->dev.parent = &ofdev->dev;
+ strlcpy(adap->name, "IBM IIC", sizeof(adap->name));
+ i2c_set_adapdata(adap, dev);
+ adap->id = I2C_HW_OCP;
+ adap->class = I2C_CLASS_HWMON;
+ adap->algo = &iic_algo;
+ adap->timeout = 1;
+ adap->nr = dev->idx;
+
+ ret = i2c_add_numbered_adapter(adap);
+ if (ret < 0) {
+ dev_err(&ofdev->dev, "failed to register i2c adapter\n");
+ goto error_cleanup;
+ }
+
+ dev_dbg(&ofdev->dev, "using %s mode\n",
+ dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)");
+
+ return 0;
+
+error_cleanup:
+ if (dev->irq != NO_IRQ) {
+ iic_interrupt_mode(dev, 0);
+ free_irq(dev->irq, dev);
+ }
+
+ if (dev->vaddr)
+ iounmap(dev->vaddr);
+
+ dev_set_drvdata(&ofdev->dev, NULL);
+ kfree(dev);
+ return ret;
+}
+
+/*
+ * Cleanup initialized IIC interface
+ */
+static int __devexit iic_remove(struct of_device *ofdev)
+{
+ struct ibm_iic_private *dev = dev_get_drvdata(&ofdev->dev);
+
+ dev_set_drvdata(&ofdev->dev, NULL);
+
+ i2c_del_adapter(&dev->adap);
+
+ if (dev->irq != NO_IRQ) {
+ iic_interrupt_mode(dev, 0);
+ free_irq(dev->irq, dev);
+ }
+
+ iounmap(dev->vaddr);
+ kfree(dev);
+
+ return 0;
+}
+
+static const struct of_device_id ibm_iic_match[] = {
+ { .compatible = "ibm,iic-405ex", },
+ { .compatible = "ibm,iic-405gp", },
+ { .compatible = "ibm,iic-440gp", },
+ { .compatible = "ibm,iic-440gpx", },
+ { .compatible = "ibm,iic-440grx", },
+ {}
+};
+
+static struct of_platform_driver ibm_iic_driver = {
+ .name = "ibm-iic",
+ .match_table = ibm_iic_match,
+ .probe = iic_probe,
+ .remove = __devexit_p(iic_remove),
+};
+
+static int __init iic_init(void)
+{
+ return of_register_platform_driver(&ibm_iic_driver);
+}
+
+static void __exit iic_exit(void)
+{
+ of_unregister_platform_driver(&ibm_iic_driver);
+}
+#endif /* CONFIG_IBM_OCP */
+
module_init(iic_init);
module_exit(iic_exit);
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index b61f56b..44c0984 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -244,7 +244,6 @@ config I2C_PIIX4
config I2C_IBM_IIC
tristate "IBM PPC 4xx on-chip I2C interface"
- depends on IBM_OCP
help
Say Y here if you want to use IIC peripheral found on
embedded IBM PPC 4xx based systems.
^ permalink raw reply related
* Re: [PATCH] i2c-ibm_iic driver bonus patch
From: Sean MacLennan @ 2008-02-19 2:02 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <477EF225.4070505@pikatech.com>
Here is an optional bonus patch that cleans up most of the checkpatch
warnings in the common code. I left in the volatiles, since I don't
understand why they where needed. The memory always seems to be access
with in_8 and out_8, which are declared volatile. But they could be
there to fix a very specific bug.
Cheers,
Sean
Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
---
--- for-2.6.25/drivers/i2c/busses/submitted-i2c-ibm_iic.c 2008-02-18 20:44:06.000000000 -0500
+++ for-2.6.25/drivers/i2c/busses/i2c-ibm_iic.c 2008-02-18 20:53:53.000000000 -0500
@@ -76,17 +76,17 @@
#endif
#if DBG_LEVEL > 0
-# define DBG(f,x...) printk(KERN_DEBUG "ibm-iic" f, ##x)
+# define DBG(f, x...) printk(KERN_DEBUG "ibm-iic" f, ##x)
#else
-# define DBG(f,x...) ((void)0)
+# define DBG(f, x...) ((void)0)
#endif
#if DBG_LEVEL > 1
-# define DBG2(f,x...) DBG(f, ##x)
+# define DBG2(f, x...) DBG(f, ##x)
#else
-# define DBG2(f,x...) ((void)0)
+# define DBG2(f, x...) ((void)0)
#endif
#if DBG_LEVEL > 2
-static void dump_iic_regs(const char* header, struct ibm_iic_private* dev)
+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);
@@ -98,9 +98,9 @@
in_8(&iic->extsts), in_8(&iic->clkdiv), in_8(&iic->xfrcnt),
in_8(&iic->xtcntlss), in_8(&iic->directcntl));
}
-# define DUMP_REGS(h,dev) dump_iic_regs((h),(dev))
+# define DUMP_REGS(h, dev) dump_iic_regs((h), (dev))
#else
-# define DUMP_REGS(h,dev) ((void)0)
+# define DUMP_REGS(h, dev) ((void)0)
#endif
/* Bus timings (in ns) for bit-banging */
@@ -111,25 +111,26 @@
unsigned int high;
unsigned int buf;
} timings [] = {
-/* Standard mode (100 KHz) */
-{
- .hd_sta = 4000,
- .su_sto = 4000,
- .low = 4700,
- .high = 4000,
- .buf = 4700,
-},
-/* Fast mode (400 KHz) */
-{
- .hd_sta = 600,
- .su_sto = 600,
- .low = 1300,
- .high = 600,
- .buf = 1300,
-}};
+ /* Standard mode (100 KHz) */
+ {
+ .hd_sta = 4000,
+ .su_sto = 4000,
+ .low = 4700,
+ .high = 4000,
+ .buf = 4700,
+ },
+ /* Fast mode (400 KHz) */
+ {
+ .hd_sta = 600,
+ .su_sto = 600,
+ .low = 1300,
+ .high = 600,
+ .buf = 1300,
+ }
+};
/* Enable/disable interrupt generation */
-static inline void iic_interrupt_mode(struct ibm_iic_private* dev, int enable)
+static inline void iic_interrupt_mode(struct ibm_iic_private *dev, int enable)
{
out_8(&dev->vaddr->intmsk, enable ? INTRMSK_EIMTC : 0);
}
@@ -137,7 +138,7 @@
/*
* Initialize IIC interface.
*/
-static void iic_dev_init(struct ibm_iic_private* dev)
+static void iic_dev_init(struct ibm_iic_private *dev)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
@@ -182,7 +183,7 @@
/*
* Reset IIC interface
*/
-static void iic_dev_reset(struct ibm_iic_private* dev)
+static void iic_dev_reset(struct ibm_iic_private *dev)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
int i;
@@ -191,19 +192,19 @@
DBG("%d: soft reset\n", dev->idx);
DUMP_REGS("reset", dev);
- /* Place chip in the reset state */
+ /* Place chip in the reset state */
out_8(&iic->xtcntlss, XTCNTLSS_SRST);
/* Check if bus is free */
dc = in_8(&iic->directcntl);
- if (!DIRCTNL_FREE(dc)){
+ if (!DIRCTNL_FREE(dc)) {
DBG("%d: trying to regain bus control\n", dev->idx);
/* Try to set bus free state */
out_8(&iic->directcntl, DIRCNTL_SDAC | DIRCNTL_SCC);
/* Wait until we regain bus control */
- for (i = 0; i < 100; ++i){
+ for (i = 0; i < 100; ++i) {
dc = in_8(&iic->directcntl);
if (DIRCTNL_FREE(dc))
break;
@@ -235,7 +236,7 @@
static int iic_dc_wait(volatile struct iic_regs __iomem *iic, u8 mask)
{
unsigned long x = jiffies + HZ / 28 + 2;
- while ((in_8(&iic->directcntl) & mask) != mask){
+ while ((in_8(&iic->directcntl) & mask) != mask) {
if (unlikely(time_after(jiffies, x)))
return -1;
cond_resched();
@@ -243,15 +244,15 @@
return 0;
}
-static int iic_smbus_quick(struct ibm_iic_private* dev, const struct i2c_msg* p)
+static int iic_smbus_quick(struct ibm_iic_private *dev, const struct i2c_msg *p)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
- const struct i2c_timings* t = &timings[dev->fast_mode ? 1 : 0];
+ 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)){
+ if (unlikely(p->flags & I2C_M_TEN)) {
DBG("%d: smbus_quick - 10 bit addresses are not supported\n",
dev->idx);
return -EINVAL;
@@ -275,7 +276,7 @@
/* Send address */
v = (u8)((p->addr << 1) | ((p->flags & I2C_M_RD) ? 1 : 0));
- for (i = 0, mask = 0x80; i < 8; ++i, mask >>= 1){
+ for (i = 0, mask = 0x80; i < 8; ++i, mask >>= 1) {
out_8(&iic->directcntl, sda);
ndelay(t->low / 2);
sda = (v & mask) ? DIRCNTL_SDAC : 0;
@@ -330,7 +331,7 @@
*/
static irqreturn_t iic_handler(int irq, void *dev_id)
{
- struct ibm_iic_private* dev = (struct ibm_iic_private*)dev_id;
+ struct ibm_iic_private *dev = (struct ibm_iic_private *)dev_id;
volatile struct iic_regs __iomem *iic = dev->vaddr;
DBG2("%d: irq handler, STS = 0x%02x, EXTSTS = 0x%02x\n",
@@ -347,11 +348,11 @@
* 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)
+static int iic_xfer_result(struct ibm_iic_private *dev)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
- if (unlikely(in_8(&iic->sts) & STS_ERR)){
+ if (unlikely(in_8(&iic->sts) & STS_ERR)) {
DBG("%d: xfer error, EXTSTS = 0x%02x\n", dev->idx,
in_8(&iic->extsts));
@@ -367,20 +368,19 @@
* 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){
+ if ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE) {
DBG("%d: bus is stuck, resetting\n", dev->idx);
iic_dev_reset(dev);
}
return -EREMOTEIO;
- }
- else
+ } 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)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
unsigned long x;
@@ -394,8 +394,8 @@
* 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)){
+ while ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE) {
+ if (time_after(jiffies, x)) {
DBG("%d: abort timeout, resetting...\n", dev->idx);
iic_dev_reset(dev);
return;
@@ -412,19 +412,19 @@
* 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){
-
+static int iic_wait_for_tc(struct ibm_iic_private *dev)
+{
volatile struct iic_regs __iomem *iic = dev->vaddr;
int ret = 0;
- if (dev->irq >= 0){
+ if (dev->irq >= 0) {
/* Interrupt mode */
ret = wait_event_interruptible_timeout(dev->wq,
!(in_8(&iic->sts) & STS_PT), dev->adap.timeout * HZ);
if (unlikely(ret < 0))
DBG("%d: wait interrupted\n", dev->idx);
- else if (unlikely(in_8(&iic->sts) & STS_PT)){
+ else if (unlikely(in_8(&iic->sts) & STS_PT)) {
DBG("%d: wait timeout\n", dev->idx);
ret = -ETIMEDOUT;
}
@@ -433,14 +433,14 @@
/* Polling mode */
unsigned long x = jiffies + dev->adap.timeout * HZ;
- while (in_8(&iic->sts) & STS_PT){
- if (unlikely(time_after(jiffies, x))){
+ while (in_8(&iic->sts) & STS_PT) {
+ if (unlikely(time_after(jiffies, x))) {
DBG("%d: poll timeout\n", dev->idx);
ret = -ETIMEDOUT;
break;
}
- if (unlikely(signal_pending(current))){
+ if (unlikely(signal_pending(current))) {
DBG("%d: poll interrupted\n", dev->idx);
ret = -ERESTARTSYS;
break;
@@ -462,11 +462,11 @@
/*
* Low level master transfer routine
*/
-static int iic_xfer_bytes(struct ibm_iic_private* dev, struct i2c_msg* pm,
+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;
- char* buf = pm->buf;
+ char *buf = pm->buf;
int i, j, loops, ret = 0;
int len = pm->len;
@@ -475,7 +475,7 @@
cntl |= CNTL_RW;
loops = (len + 3) / 4;
- for (i = 0; i < loops; ++i, len -= 4){
+ for (i = 0; i < loops; ++i, len -= 4) {
int count = len > 4 ? 4 : len;
u8 cmd = cntl | ((count - 1) << CNTL_TCT_SHIFT);
@@ -498,7 +498,7 @@
if (unlikely(ret < 0))
break;
- else if (unlikely(ret != count)){
+ else if (unlikely(ret != count)) {
DBG("%d: xfer_bytes, requested %d, transfered %d\n",
dev->idx, count, ret);
@@ -521,7 +521,7 @@
/*
* Set target slave address for master transfer
*/
-static inline void iic_address(struct ibm_iic_private* dev, struct i2c_msg* msg)
+static inline void iic_address(struct ibm_iic_private *dev, struct i2c_msg *msg)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
u16 addr = msg->addr;
@@ -529,24 +529,24 @@
DBG2("%d: iic_address, 0x%03x (%d-bit)\n", dev->idx,
addr, msg->flags & I2C_M_TEN ? 10 : 7);
- if (msg->flags & I2C_M_TEN){
+ if (msg->flags & I2C_M_TEN) {
out_8(&iic->cntl, CNTL_AMD);
out_8(&iic->lmadr, addr);
out_8(&iic->hmadr, 0xf0 | ((addr >> 7) & 0x06));
- }
- else {
+ } else {
out_8(&iic->cntl, 0);
out_8(&iic->lmadr, addr << 1);
}
}
-static inline int iic_invalid_address(const struct i2c_msg* p)
+static inline int iic_invalid_address(const struct i2c_msg *p)
{
- return (p->addr > 0x3ff) || (!(p->flags & I2C_M_TEN) && (p->addr > 0x7f));
+ return (p->addr > 0x3ff) ||
+ (!(p->flags & I2C_M_TEN) && (p->addr > 0x7f));
}
-static inline int iic_address_neq(const struct i2c_msg* p1,
- const struct i2c_msg* p2)
+static inline int iic_address_neq(const struct i2c_msg *p1,
+ const struct i2c_msg *p2)
{
return (p1->addr != p2->addr)
|| ((p1->flags & I2C_M_TEN) != (p2->flags & I2C_M_TEN));
@@ -558,9 +558,13 @@
*/
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 ibm_iic_private *dev;
+ volatile struct iic_regs __iomem *iic;
int i, ret = 0;
+ u8 extsts;
+
+ dev = (struct ibm_iic_private *)(i2c_get_adapdata(adap));
+ iic = dev->vaddr;
DBG2("%d: iic_xfer, %d msg(s)\n", dev->idx, num);
@@ -570,14 +574,14 @@
/* 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]))){
+ if (unlikely(iic_invalid_address(&msgs[0]))) {
DBG("%d: invalid address 0x%03x (%d-bit)\n", dev->idx,
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){
+ 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.
* IBM IIC doesn't support 0-length transactions
* so we have to emulate them using bit-banging.
@@ -588,14 +592,15 @@
msgs[i].len, i);
return -EINVAL;
}
- if (unlikely(iic_address_neq(&msgs[0], &msgs[i]))){
+ if (unlikely(iic_address_neq(&msgs[0], &msgs[i]))) {
DBG("%d: invalid addr in msg[%d]\n", dev->idx, i);
return -EINVAL;
}
}
/* Check bus state */
- if (unlikely((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE)){
+ extsts = in_8(&iic->extsts);
+ if (unlikely((extsts & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE)) {
DBG("%d: iic_xfer, bus is not free\n", dev->idx);
/* Usually it means something serious has happend.
@@ -608,12 +613,11 @@
*/
iic_dev_reset(dev);
- if ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE){
+ if ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE) {
DBG("%d: iic_xfer, bus is still not free\n", dev->idx);
return -EREMOTEIO;
}
- }
- else {
+ } else {
/* Flush master data buffer (just in case) */
out_8(&iic->mdcntl, in_8(&iic->mdcntl) | MDCNTL_FMDB);
}
@@ -622,7 +626,7 @@
iic_address(dev, &msgs[0]);
/* Do real transfer */
- for (i = 0; i < num && !ret; ++i)
+ for (i = 0; i < num && !ret; ++i)
ret = iic_xfer_bytes(dev, &msgs[i], i < num - 1);
return ret < 0 ? ret : num;
@@ -648,7 +652,7 @@
* Previous driver version used hardcoded divider value 4,
* it corresponds to OPB frequency from the range (40, 50] MHz
*/
- if (!opb){
+ if (!opb) {
printk(KERN_WARNING "ibm-iic: using compatibility value for OPB freq,"
" fix your board specific setup\n");
opb = 50000000;
@@ -657,9 +661,9 @@
/* Convert to MHz */
opb /= 1000000;
- if (opb < 20 || opb > 150){
- printk(KERN_WARNING "ibm-iic: invalid OPB clock frequency %u MHz\n",
- opb);
+ if (opb < 20 || opb > 150) {
+ printk(KERN_WARNING "ibm-iic: "
+ "invalid OPB clock frequency %u MHz\n", opb);
opb = opb < 20 ? 20 : 150;
}
return (u8)((opb + 9) / 10 - 1);
^ permalink raw reply
* Re: [PATCH 1/3] Fix Unlikely(x) == y
From: Arjan van de Ven @ 2008-02-19 2:40 UTC (permalink / raw)
To: Nick Piggin
Cc: Roel Kluin, lkml, cbe-oss-dev, linuxppc-dev, Andi Kleen,
Willy Tarreau
In-Reply-To: <200802191333.53607.nickpiggin@yahoo.com.au>
On Tue, 19 Feb 2008 13:33:53 +1100
Nick Piggin <nickpiggin@yahoo.com.au> wrote:
>
> Actually one thing I don't like about gcc is that I think it still
> emits cmovs for likely/unlikely branches, which is silly (the gcc
> developers seem to be in love with that instruction). If that goes
> away, then branch hints may be even better.
only for -Os and only if the result is smaller afaik.
(cmov tends to be a performance loss most of the time so for -O2 and such it
isn't used as far as I know.. it does make for nice small code however ;-)
--
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
^ permalink raw reply
* Re: compile quirk linux-2.6.24 (with workaround)
From: Josh Boyer @ 2008-02-19 2:52 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev, debian-powerpc, paulus, Bernhard Reiter
In-Reply-To: <20080205093820.5918a216@zod.rchland.ibm.com>
On Tue, 5 Feb 2008 09:38:20 -0600
Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> On Tue, 5 Feb 2008 08:24:38 -0700
> "Grant Likely" <grant.likely@secretlab.ca> wrote:
>
> > On 2/5/08, Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> > > > I mean, if you have not included 4xx support in the kernel, as is the
> > > > case here, it does not make sense to add the 4xx bootwrapper code, no ?
> > >
> > > It does, in a manner. There are both generic and platform specific
> > > pieces to the bootwrapper. Having everything always built helps keep
> > > the generic bits from breaking, which is important as they're often
> > > tightly coupled. That's at least the reason I can think of.
> > >
> > > The powerpc maintainers have been over this quite a bit and I don't see
> > > it changing anytime soon.
> >
> > That would mean we're dropping support for compilers which can't build
> > 405/440 specific wrapper bits (or other core specific quirks that need
> > to go in the wrapper) That doesn't sound appropriate to me.
>
> No it doesn't. At least not yet. I said I'd try to come up with a
> patch soon-ish. We haven't failed!(yet) Also, this isn't a
> core-specific quirk. It's an architected instruction of Book III-E in
> the PowerPC ISA. I can't help it if other chips don't implement this
> wonderful control mechanism ;)
>
> Taking a step back though, there will always be odd cases like this as
> we move forward. Toolchain XXX will eventually not support instruction
> YYYY which will eventually be used, etc. I'll try to make this
> specific case work because it's scope is quite limited. But this
> problem as a whole will still remain.
My apologies for taking so long on this. Digging through gcc history
isn't exactly fun :)
Ok, so it seems -mcpu=440 was added in gcc 3.4. The -mcpu=405 option
has been around since 2001. Seeing as how there really isn't anything
440 specific in the files effected, we should be able to pass -mcpu=405
for everything and have it still work.
Bernhard, can you try the patch below? I've compile test it, and
booted it on an Ebony board (PowerPC 440GP). If anyone else cares to
test that would also be welcome.
josh
[POWERPC] Fix bootwrapper builds with older gcc versions
GCC versions before 3.4 did not support the -mcpu=440 option. Use
-mcpu=405 for the 4xx specific bootwrapper files, as that has been
around for much longer.
Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 63d07cc..e3993a6 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -35,10 +35,10 @@ endif
BOOTCFLAGS += -I$(obj) -I$(srctree)/$(obj) -I$(srctree)/$(src)/libfdt
-$(obj)/4xx.o: BOOTCFLAGS += -mcpu=440
-$(obj)/ebony.o: BOOTCFLAGS += -mcpu=440
-$(obj)/cuboot-taishan.o: BOOTCFLAGS += -mcpu=440
-$(obj)/cuboot-katmai.o: BOOTCFLAGS += -mcpu=440
+$(obj)/4xx.o: BOOTCFLAGS += -mcpu=405
+$(obj)/ebony.o: BOOTCFLAGS += -mcpu=405
+$(obj)/cuboot-taishan.o: BOOTCFLAGS += -mcpu=405
+$(obj)/cuboot-katmai.o: BOOTCFLAGS += -mcpu=405
$(obj)/treeboot-walnut.o: BOOTCFLAGS += -mcpu=405
^ permalink raw reply related
* Re: [PATCH] i2c-ibm_iic driver bonus patch
From: Arnd Bergmann @ 2008-02-19 3:27 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Sean MacLennan
In-Reply-To: <47BA38C1.6040609@pikatech.com>
On Tuesday 19 February 2008, Sean MacLennan wrote:
> I left in the volatiles, since I don't
> understand why they where needed. The memory always seems to be access
> with in_8 and out_8, which are declared volatile. But they could be
> there to fix a very specific bug.
It's very unlikely that they were really needed, and you certainly
shouldn't mark data as volatile in new code. It's very common to
mark I/O data structures as volatile when they should be __iomem,
because that's what people learn at university, but that is never
the right solution, even if it can hide other bugs in your code.
Of course, unlike the other changes in your patch, it does impact
code generation, so if you want to change it, that should be a
separate patch.
Arnd <><
^ permalink raw reply
* Re: [PATCH 1/3] Fix Unlikely(x) == y
From: Nick Piggin @ 2008-02-19 2:33 UTC (permalink / raw)
To: Andi Kleen
Cc: Roel Kluin, lkml, Willy Tarreau, linuxppc-dev, cbe-oss-dev,
Arjan van de Ven
In-Reply-To: <p731w7axrat.fsf@bingen.suse.de>
On Tuesday 19 February 2008 01:39, Andi Kleen wrote:
> Arjan van de Ven <arjan@infradead.org> writes:
> > you have more faith in the authors knowledge of how his code actually
> > behaves than I think is warranted :)
>
> iirc there was a mm patch some time ago to keep track of the actual
> unlikely values at runtime and it showed indeed some wrong ones. But the
> far majority of them are probably correct.
>
> > Or faith in that he knows what "unlikely" means.
> > I should write docs about this; but unlikely() means:
> > 1) It happens less than 0.01% of the cases.
> > 2) The compiler couldn't have figured this out by itself
> > (NULL pointer checks are compiler done already, same for some other
> > conditions) 3) It's a hot codepath where shaving 0.5 cycles (less even on
> > x86) matters (and the author is ok with taking a 500 cycles hit if he's
> > wrong)
>
> One more thing unlikely() does is to move the unlikely code out of line.
> So it should conserve some icache in critical functions, which might
> well be worth some more cycles (don't have numbers though).
I actually once measured context switching performance in the scheduler,
and removing the unlikely hint for testing RT tasks IIRC gave about 5%
performance drop.
This was on a P4 which is very different from more modern CPUs both in
terms of branch performance characteristics, and icache characteristics.
However, the P4's branch predictor is pretty good, and it should easily
be able to correctly predict the rt_task check if it has enough entries.
So I think much of the savings came from code transformation and movement.
Anyway, it is definitely worthwhile if used correctly.
Actually one thing I don't like about gcc is that I think it still emits
cmovs for likely/unlikely branches, which is silly (the gcc developers
seem to be in love with that instruction). If that goes away, then
branch hints may be even better.
>
> But overall I agree with you that unlikely is in most cases a bad
> idea (and I submitted the original patch introducing it originally @). That
> is because it is often used in situations where gcc's default branch
> prediction heuristics do would make exactly the same decision
>
> if (unlikely(x == NULL))
>
> is simply totally useless because gcc already assumes all x == NULL
> tests are unlikely. I appended some of the builtin heuristics from
> a recent gcc source so people can see them.
>
> Note in particular the last predictors; assuming branch ending
> with goto, including call, causing early function return or
> returning negative constant are not taken. Just these alone
> are likely 95+% of the unlikelies in the kernel.
Yes, gcc should be able to do pretty good heuristics, considering
the quite good numbers that cold CPU predictors can attain. However
for really performance critical code (or really "never" executed
code), then I think it is OK to have the hints and not have to rely
on gcc heuristics.
>
> -Andi
[snip]
Interesting, thanks!
^ permalink raw reply
* Re: [PATCH 1/3] Fix Unlikely(x) == y
From: Nick Piggin @ 2008-02-19 4:41 UTC (permalink / raw)
To: Arjan van de Ven
Cc: Roel Kluin, lkml, cbe-oss-dev, linuxppc-dev, Andi Kleen,
Willy Tarreau
In-Reply-To: <20080218184027.429cf47b@laptopd505.fenrus.org>
On Tuesday 19 February 2008 13:40, Arjan van de Ven wrote:
> On Tue, 19 Feb 2008 13:33:53 +1100
>
> Nick Piggin <nickpiggin@yahoo.com.au> wrote:
> > Actually one thing I don't like about gcc is that I think it still
> > emits cmovs for likely/unlikely branches, which is silly (the gcc
> > developers seem to be in love with that instruction). If that goes
> > away, then branch hints may be even better.
>
> only for -Os and only if the result is smaller afaik.
What is your evidence for saying this? Because here, with the latest
kernel and recent gcc-4.3 snapshot, it spits out cmov like crazy even
when compiled with -O2.
npiggin@am:~/usr/src/linux-2.6$ grep cmov kernel/sched.s | wc -l
45
And yes it even does for hinted branches and even at -O2/3
npiggin@am:~/tests$ cat cmov.c
int test(int a, int b)
{
if (__builtin_expect(a < b, 0))
return a;
else
return b;
}
npiggin@am:~/tests$ gcc-4.3 -S -O2 cmov.c
npiggin@am:~/tests$ head -13 cmov.s
.file "cmov.c"
.text
.p2align 4,,15
..globl test
.type test, @function
test:
..LFB2:
cmpl %edi, %esi
cmovle %esi, %edi
movl %edi, %eax
ret
..LFE2:
.size test, .-test
This definitely should be a branch, IMO.
> (cmov tends to be a performance loss most of the time so for -O2 and such
> it isn't used as far as I know.. it does make for nice small code however
> ;-)
It shouldn't be hard to work out the cutover point based on how
expensive cmov is, how expensive branch and branch mispredicts are,
and how often the branch is likely to be mispredicted. For an
unpredictable branch, cmov is normally quite a good win even on
modern CPUs. But gcc overuses it I think.
^ permalink raw reply
* [patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
From: Bjorn Helgaas @ 2008-02-19 4:39 UTC (permalink / raw)
To: linux-pci, linux-arch
Cc: Chris Zankel, Grant Grundler, linux-parisc, Matthew Wilcox,
Kyle McMartin, linuxppc-dev, Paul Mackerras, linux-arm-kernel,
Russell King
There are many implementations of pcibios_enable_resources() that differ
in minor ways that look more like bugs than architectural differences.
This patch series consolidates most of them to use the x86 version.
This series is for discussion only at this point. I'm interested in
feedback about whether any of the differences are "real" and need to
be preserved.
ARM and PA-RISC, in particular, have interesting differences:
- ARM always enables bridge devices, which no other arch does
- PA-RISC always turns on SERR and PARITY, which no other arch does
Should other arches do the same thing, or are these somehow related to
ARM and PA-RISC architecture?
Bjorn
--
^ permalink raw reply
* [patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
From: Bjorn Helgaas @ 2008-02-19 4:39 UTC (permalink / raw)
To: linux-pci, linux-arch
Cc: Chris Zankel, Grant Grundler, linux-parisc, Matthew Wilcox,
Kyle McMartin, linuxppc-dev, Paul Mackerras, linux-arm-kernel,
Russell King
In-Reply-To: <20080219043952.845136014@ldl.fc.hp.com>
There are many implementations of pcibios_enable_resources() that differ
in minor ways that look more like bugs than architectural differences.
This patch consolidates most of them to use the x86 version.
This patch is for discussion only at this point. I'm interested in
feedback about whether any of the differences are "real" and need to
be preserved.
ARM and PA-RISC, in particular, have interesting differences:
- ARM always enables bridge devices, which no other arch does
- PA-RISC always turns on SERR and PARITY, which no other arch does
Should other arches do the same thing, or are these somehow related to
ARM and PA-RISC architecture?
Here's the x86 version, which seems most complete and up-to-date:
int pcibios_enable_resources(struct pci_dev *dev, int mask)
{
u16 cmd, old_cmd;
int i;
struct resource *r;
(0)
pci_read_config_word(dev, PCI_COMMAND, &cmd);
old_cmd = cmd;
(1) for (i = 0; i < PCI_NUM_RESOURCES; i++) {
(2) if (!(mask & (1 << i)))
continue;
r = &dev->resource[i];
(3) if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
continue;
(4) if ((i == PCI_ROM_RESOURCE) &&
(!(r->flags & IORESOURCE_ROM_ENABLE)))
continue;
(5) if (!r->start && r->end) {
dev_err(&dev->dev, "device not available because of "
"resource %d collisions\n", i);
return -EINVAL;
}
if (r->flags & IORESOURCE_IO)
cmd |= PCI_COMMAND_IO;
if (r->flags & IORESOURCE_MEM)
cmd |= PCI_COMMAND_MEMORY;
}
(6)
(7) if (cmd != old_cmd) {
dev_info(&dev->dev, "enabling device (%04x -> %04x)\n",
old_cmd, cmd);
pci_write_config_word(dev, PCI_COMMAND, cmd);
}
return 0;
}
Compared with the x86 version, other architectures have the following
functional differences:
alpha: ignores mask at (2), has no PCI_ROM_RESOURCE check at (4),
has no collision check at (5)
arm: checks only 6 resources at (1), has no PCI_ROM_RESOURCE check at (4),
always fully enables bridges at (6)
cris: checks only 6 resources at (1), has a different ROM
resource check at (4) and (6) that ignores IORESOURCE_ROM_ENABLE
frv: checks only 6 resources at (1), has a different ROM
resource check at (4) and (6) that ignores IORESOURCE_ROM_ENABLE
ia64: checks for NULL dev at (0)
mips: has no IORESOURCE_{IO,MEM} check at (3), has a different
ROM resource check at (4) and (6) that ignores IORESOURCE_ROM_ENABLE
mn10300: checks only 6 resources at (1), has no IORESOURCE_{IO,MEM}
check at (3), has a different ROM resource check at (4) and (6)
that ignores IORESOURCE_ROM_ENABLE
parisc: checks DEVICE_COUNT_RESOURCE (12) instead of PCI_NUM_RESOURCES (11)
resources at (1), has no IORESOURCE_{IO,MEM} check at (3),
has no PCI_ROM_RESOURCE check at (4), has no collision check at (5)
always turns on PCI_COMMAND_SERR | PCI_COMMAND_PARITY at (6),
writes cmd even if unchanged at (7)
powerpc: has a different collision check at (5)
ppc: checks only 6 resources at (1), has no IORESOURCE_{IO,MEM} check
at (3), has a different ROM resource check at (4) and (6) that
ignores IORESOURCE_ROM_ENABLE, has a different collision check using
IORESOURCE_UNSET at (5)
sh: checks only 6 resources at (1), has no IORESOURCE_{IO,MEM} check
at (3), has a different ROM resource check at (4) and (6) that
ignores IORESOURCE_ROM_ENABLE
sparc64: has no IORESOURCE_{IO,MEM} check at (3), has no PCI_ROM_RESOURCE
check at (4)
v850: checks only 6 resources at (1), has no IORESOURCE_{IO,MEM} check
at (3), has no PCI_ROM_RESOURCE check at (4)
xtensa: checks only 6 resources at (1), has no IORESOURCE_{IO,MEM} check
at (3), has a different ROM resource check at (4) and (6) that
ignores IORESOURCE_ROM_ENABLE
The mips/pmc-sierra implementation of pcibios_enable_resources() is
cluttered with a bunch of titan stuff, so I can't immediately consolidate
it with the others. So I made the generic version "weak" so pmc-sierra
can override it.
Not-Yet-Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
arch/alpha/kernel/pci.c | 27 -------------
arch/arm/kernel/bios32.c | 43 ---------------------
arch/cris/arch-v32/drivers/pci/bios.c | 32 ----------------
arch/frv/mb93090-mb00/pci-frv.c | 32 ----------------
arch/ia64/pci/pci.c | 42 ---------------------
arch/mips/pci/pci.c | 32 ----------------
arch/mn10300/unit-asb2305/pci-asb2305.c | 39 -------------------
arch/parisc/kernel/pci.c | 41 --------------------
arch/powerpc/kernel/pci-common.c | 36 ------------------
arch/ppc/kernel/pci.c | 33 ----------------
arch/sh/drivers/pci/pci.c | 32 ----------------
arch/sparc64/kernel/pci.c | 30 ---------------
arch/v850/kernel/rte_mb_a_pci.c | 28 --------------
arch/x86/pci/i386.c | 38 -------------------
arch/x86/pci/pci.h | 1
arch/xtensa/kernel/pci.c | 31 ---------------
drivers/pci/Makefile | 2 -
drivers/pci/bios.c | 64 ++++++++++++++++++++++++++++++++
include/linux/pci.h | 1
19 files changed, 66 insertions(+), 518 deletions(-)
Index: work6/arch/alpha/kernel/pci.c
===================================================================
--- work6.orig/arch/alpha/kernel/pci.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/alpha/kernel/pci.c 2008-02-18 21:16:38.000000000 -0700
@@ -370,33 +370,6 @@
#endif
int
-pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd, oldcmd;
- int i;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- oldcmd = cmd;
-
- for (i = 0; i < PCI_NUM_RESOURCES; i++) {
- struct resource *res = &dev->resource[i];
-
- if (res->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- else if (res->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
-
- if (cmd != oldcmd) {
- printk(KERN_DEBUG "PCI: Enabling device: (%s), cmd %x\n",
- pci_name(dev), cmd);
- /* Enable the appropriate bits in the PCI command register. */
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
-int
pcibios_enable_device(struct pci_dev *dev, int mask)
{
return pcibios_enable_resources(dev, mask);
Index: work6/arch/arm/kernel/bios32.c
===================================================================
--- work6.orig/arch/arm/kernel/bios32.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/arm/kernel/bios32.c 2008-02-18 21:16:38.000000000 -0700
@@ -654,49 +654,6 @@
res->start = (start + align - 1) & ~(align - 1);
}
-/**
- * pcibios_enable_resources - Enable I/O and memory.
- * @dev: PCI device to be enabled
- */
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for (idx = 0; idx < 6; idx++) {
- /* Only set up the requested stuff */
- if (!(mask & (1 << idx)))
- continue;
-
- r = dev->resource + idx;
- if (!r->start && r->end) {
- printk(KERN_ERR "PCI: Device %s not available because"
- " of resource collisions\n", pci_name(dev));
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
-
- /*
- * Bridges (eg, cardbus bridges) need to be fully enabled
- */
- if ((dev->class >> 16) == PCI_BASE_CLASS_BRIDGE)
- cmd |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY;
-
- if (cmd != old_cmd) {
- printk("PCI: enabling device %s (%04x -> %04x)\n",
- pci_name(dev), old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
int pcibios_enable_device(struct pci_dev *dev, int mask)
{
return pcibios_enable_resources(dev, mask);
Index: work6/arch/cris/arch-v32/drivers/pci/bios.c
===================================================================
--- work6.orig/arch/cris/arch-v32/drivers/pci/bios.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/cris/arch-v32/drivers/pci/bios.c 2008-02-18 21:16:38.000000000 -0700
@@ -55,38 +55,6 @@
}
}
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for(idx=0; idx<6; idx++) {
- /* Only set up the requested stuff */
- if (!(mask & (1<<idx)))
- continue;
-
- r = &dev->resource[idx];
- if (!r->start && r->end) {
- printk(KERN_ERR "PCI: Device %s not available because of resource collisions\n", pci_name(dev));
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
- if (dev->resource[PCI_ROM_RESOURCE].start)
- cmd |= PCI_COMMAND_MEMORY;
- if (cmd != old_cmd) {
- printk("PCI: Enabling device %s (%04x -> %04x)\n", pci_name(dev), old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
int pcibios_enable_irq(struct pci_dev *dev)
{
dev->irq = EXT_INTR_VECT;
Index: work6/arch/frv/mb93090-mb00/pci-frv.c
===================================================================
--- work6.orig/arch/frv/mb93090-mb00/pci-frv.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/frv/mb93090-mb00/pci-frv.c 2008-02-18 21:16:38.000000000 -0700
@@ -231,38 +231,6 @@
pcibios_assign_resources();
}
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for(idx=0; idx<6; idx++) {
- /* Only set up the requested stuff */
- if (!(mask & (1<<idx)))
- continue;
-
- r = &dev->resource[idx];
- if (!r->start && r->end) {
- printk(KERN_ERR "PCI: Device %s not available because of resource collisions\n", pci_name(dev));
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
- if (dev->resource[PCI_ROM_RESOURCE].start)
- cmd |= PCI_COMMAND_MEMORY;
- if (cmd != old_cmd) {
- printk("PCI: Enabling device %s (%04x -> %04x)\n", pci_name(dev), old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
/*
* If we set up a device for bus mastering, we need to check the latency
* timer as certain crappy BIOSes forget to set it properly.
Index: work6/arch/ia64/pci/pci.c
===================================================================
--- work6.orig/arch/ia64/pci/pci.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/ia64/pci/pci.c 2008-02-18 21:16:38.000000000 -0700
@@ -499,48 +499,6 @@
/* ??? FIXME -- record old value for shutdown. */
}
-static inline int
-pcibios_enable_resources (struct pci_dev *dev, int mask)
-{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
- unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM;
-
- if (!dev)
- return -EINVAL;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for (idx=0; idx<PCI_NUM_RESOURCES; idx++) {
- /* Only set up the desired resources. */
- if (!(mask & (1 << idx)))
- continue;
-
- r = &dev->resource[idx];
- if (!(r->flags & type_mask))
- continue;
- if ((idx == PCI_ROM_RESOURCE) &&
- (!(r->flags & IORESOURCE_ROM_ENABLE)))
- continue;
- if (!r->start && r->end) {
- printk(KERN_ERR
- "PCI: Device %s not available because of resource collisions\n",
- pci_name(dev));
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
- if (cmd != old_cmd) {
- printk("PCI: Enabling device %s (%04x -> %04x)\n", pci_name(dev), old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
int
pcibios_enable_device (struct pci_dev *dev, int mask)
{
Index: work6/arch/mips/pci/pci.c
===================================================================
--- work6.orig/arch/mips/pci/pci.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/mips/pci/pci.c 2008-02-18 21:16:38.000000000 -0700
@@ -163,38 +163,6 @@
subsys_initcall(pcibios_init);
-static int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for (idx=0; idx < PCI_NUM_RESOURCES; idx++) {
- /* Only set up the requested stuff */
- if (!(mask & (1<<idx)))
- continue;
-
- r = &dev->resource[idx];
- if (!r->start && r->end) {
- printk(KERN_ERR "PCI: Device %s not available because of resource collisions\n", pci_name(dev));
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
- if (dev->resource[PCI_ROM_RESOURCE].start)
- cmd |= PCI_COMMAND_MEMORY;
- if (cmd != old_cmd) {
- printk("PCI: Enabling device %s (%04x -> %04x)\n", pci_name(dev), old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
/*
* If we set up a device for bus mastering, we need to check the latency
* timer as certain crappy BIOSes forget to set it properly.
Index: work6/arch/mn10300/unit-asb2305/pci-asb2305.c
===================================================================
--- work6.orig/arch/mn10300/unit-asb2305/pci-asb2305.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/mn10300/unit-asb2305/pci-asb2305.c 2008-02-18 21:16:38.000000000 -0700
@@ -218,45 +218,6 @@
pcibios_allocate_resources(1);
}
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
-
- for (idx = 0; idx < 6; idx++) {
- /* Only set up the requested stuff */
- if (!(mask & (1 << idx)))
- continue;
-
- r = &dev->resource[idx];
-
- if (!r->start && r->end) {
- printk(KERN_ERR
- "PCI: Device %s not available because of"
- " resource collisions\n",
- pci_name(dev));
- return -EINVAL;
- }
-
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
-
- if (dev->resource[PCI_ROM_RESOURCE].start)
- cmd |= PCI_COMMAND_MEMORY;
-
- if (cmd != old_cmd)
- pci_write_config_word(dev, PCI_COMMAND, cmd);
-
- return 0;
-}
-
/*
* If we set up a device for bus mastering, we need to check the latency
* timer as certain crappy BIOSes forget to set it properly.
Index: work6/arch/parisc/kernel/pci.c
===================================================================
--- work6.orig/arch/parisc/kernel/pci.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/parisc/kernel/pci.c 2008-02-18 21:16:38.000000000 -0700
@@ -276,47 +276,6 @@
/* The caller updates the end field, we don't. */
}
-
-/*
- * A driver is enabling the device. We make sure that all the appropriate
- * bits are set to allow the device to operate as the driver is expecting.
- * We enable the port IO and memory IO bits if the device has any BARs of
- * that type, and we enable the PERR and SERR bits unconditionally.
- * Drivers that do not need parity (eg graphics and possibly networking)
- * can clear these bits if they want.
- */
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd;
- int idx;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
-
- for (idx = 0; idx < DEVICE_COUNT_RESOURCE; idx++) {
- struct resource *r = &dev->resource[idx];
-
- /* only setup requested resources */
- if (!(mask & (1<<idx)))
- continue;
-
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
-
- cmd |= (PCI_COMMAND_SERR | PCI_COMMAND_PARITY);
-
-#if 0
- /* If bridge/bus controller has FBB enabled, child must too. */
- if (dev->bus->bridge_ctl & PCI_BRIDGE_CTL_FAST_BACK)
- cmd |= PCI_COMMAND_FAST_BACK;
-#endif
- DBGC("PCIBIOS: Enabling device %s cmd 0x%04x\n", pci_name(dev), cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- return 0;
-}
-
int pcibios_enable_device(struct pci_dev *dev, int mask)
{
return pcibios_enable_resources(dev, mask);
Index: work6/arch/powerpc/kernel/pci-common.c
===================================================================
--- work6.orig/arch/powerpc/kernel/pci-common.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/powerpc/kernel/pci-common.c 2008-02-18 21:16:38.000000000 -0700
@@ -1153,42 +1153,6 @@
EXPORT_SYMBOL_GPL(pcibios_claim_one_bus);
#endif /* CONFIG_HOTPLUG */
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for (idx = 0; idx < PCI_NUM_RESOURCES; idx++) {
- /* Only set up the requested stuff */
- if (!(mask & (1 << idx)))
- continue;
- r = &dev->resource[idx];
- if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
- continue;
- if ((idx == PCI_ROM_RESOURCE) &&
- (!(r->flags & IORESOURCE_ROM_ENABLE)))
- continue;
- if (r->parent == NULL) {
- printk(KERN_ERR "PCI: Device %s not available because"
- " of resource collisions\n", pci_name(dev));
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
- if (cmd != old_cmd) {
- printk("PCI: Enabling device %s (%04x -> %04x)\n",
- pci_name(dev), old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
int pcibios_enable_device(struct pci_dev *dev, int mask)
{
if (ppc_md.pcibios_enable_device_hook)
Index: work6/arch/ppc/kernel/pci.c
===================================================================
--- work6.orig/arch/ppc/kernel/pci.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/ppc/kernel/pci.c 2008-02-18 21:16:38.000000000 -0700
@@ -578,39 +578,6 @@
}
-int
-pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for (idx=0; idx<6; idx++) {
- /* Only set up the requested stuff */
- if (!(mask & (1<<idx)))
- continue;
-
- r = &dev->resource[idx];
- if (r->flags & IORESOURCE_UNSET) {
- printk(KERN_ERR "PCI: Device %s not available because of resource collisions\n", pci_name(dev));
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
- if (dev->resource[PCI_ROM_RESOURCE].start)
- cmd |= PCI_COMMAND_MEMORY;
- if (cmd != old_cmd) {
- printk("PCI: Enabling device %s (%04x -> %04x)\n", pci_name(dev), old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
static int next_controller_index;
struct pci_controller * __init
Index: work6/arch/sh/drivers/pci/pci.c
===================================================================
--- work6.orig/arch/sh/drivers/pci/pci.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/sh/drivers/pci/pci.c 2008-02-18 21:16:38.000000000 -0700
@@ -131,38 +131,6 @@
}
}
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for(idx=0; idx<6; idx++) {
- if (!(mask & (1 << idx)))
- continue;
- r = &dev->resource[idx];
- if (!r->start && r->end) {
- printk(KERN_ERR "PCI: Device %s not available because "
- "of resource collisions\n", pci_name(dev));
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
- if (dev->resource[PCI_ROM_RESOURCE].start)
- cmd |= PCI_COMMAND_MEMORY;
- if (cmd != old_cmd) {
- printk(KERN_INFO "PCI: Enabling device %s (%04x -> %04x)\n",
- pci_name(dev), old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
int pcibios_enable_device(struct pci_dev *dev, int mask)
{
return pcibios_enable_resources(dev, mask);
Index: work6/arch/sparc64/kernel/pci.c
===================================================================
--- work6.orig/arch/sparc64/kernel/pci.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/sparc64/kernel/pci.c 2008-02-18 21:16:38.000000000 -0700
@@ -946,36 +946,6 @@
{
}
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd, oldcmd;
- int i;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- oldcmd = cmd;
-
- for (i = 0; i < PCI_NUM_RESOURCES; i++) {
- struct resource *res = &dev->resource[i];
-
- /* Only set up the requested stuff */
- if (!(mask & (1<<i)))
- continue;
-
- if (res->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (res->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
-
- if (cmd != oldcmd) {
- printk(KERN_DEBUG "PCI: Enabling device: (%s), cmd %x\n",
- pci_name(dev), cmd);
- /* Enable the appropriate bits in the PCI command register. */
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
int pcibios_enable_device(struct pci_dev *dev, int mask)
{
return pcibios_enable_resources(dev, mask);
Index: work6/arch/v850/kernel/rte_mb_a_pci.c
===================================================================
--- work6.orig/arch/v850/kernel/rte_mb_a_pci.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/v850/kernel/rte_mb_a_pci.c 2008-02-18 21:16:38.000000000 -0700
@@ -217,34 +217,6 @@
}
\f
-int __nomods_init pcibios_enable_resources (struct pci_dev *dev, int mask)
-{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for (idx = 0; idx < 6; idx++) {
- r = &dev->resource[idx];
- if (!r->start && r->end) {
- printk(KERN_ERR "PCI: Device %s not available because "
- "of resource collisions\n", pci_name(dev));
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
- if (cmd != old_cmd) {
- printk("PCI: Enabling device %s (%04x -> %04x)\n",
- pci_name(dev), old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
int __nomods_init pcibios_enable_device (struct pci_dev *dev, int mask)
{
return pcibios_enable_resources(dev, mask);
Index: work6/arch/x86/pci/i386.c
===================================================================
--- work6.orig/arch/x86/pci/i386.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/x86/pci/i386.c 2008-02-18 21:16:38.000000000 -0700
@@ -238,44 +238,6 @@
*/
fs_initcall(pcibios_assign_resources);
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for (idx = 0; idx < PCI_NUM_RESOURCES; idx++) {
- /* Only set up the requested stuff */
- if (!(mask & (1 << idx)))
- continue;
-
- r = &dev->resource[idx];
- if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
- continue;
- if ((idx == PCI_ROM_RESOURCE) &&
- (!(r->flags & IORESOURCE_ROM_ENABLE)))
- continue;
- if (!r->start && r->end) {
- printk(KERN_ERR "PCI: Device %s not available "
- "because of resource %d collisions\n",
- pci_name(dev), idx);
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
- if (cmd != old_cmd) {
- printk("PCI: Enabling device %s (%04x -> %04x)\n",
- pci_name(dev), old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
/*
* If we set up a device for bus mastering, we need to check the latency
* timer as certain crappy BIOSes forget to set it properly.
Index: work6/arch/x86/pci/pci.h
===================================================================
--- work6.orig/arch/x86/pci/pci.h 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/x86/pci/pci.h 2008-02-18 21:16:38.000000000 -0700
@@ -44,7 +44,6 @@
extern unsigned int pcibios_max_latency;
void pcibios_resource_survey(void);
-int pcibios_enable_resources(struct pci_dev *, int);
/* pci-pc.c */
Index: work6/arch/xtensa/kernel/pci.c
===================================================================
--- work6.orig/arch/xtensa/kernel/pci.c 2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/xtensa/kernel/pci.c 2008-02-18 21:16:38.000000000 -0700
@@ -91,37 +91,6 @@
}
}
-int
-pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for(idx=0; idx<6; idx++) {
- r = &dev->resource[idx];
- if (!r->start && r->end) {
- printk (KERN_ERR "PCI: Device %s not available because "
- "of resource collisions\n", pci_name(dev));
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
- if (dev->resource[PCI_ROM_RESOURCE].start)
- cmd |= PCI_COMMAND_MEMORY;
- if (cmd != old_cmd) {
- printk("PCI: Enabling device %s (%04x -> %04x)\n",
- pci_name(dev), old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
-}
-
struct pci_controller * __init pcibios_alloc_controller(void)
{
struct pci_controller *pci_ctrl;
Index: work6/drivers/pci/Makefile
===================================================================
--- work6.orig/drivers/pci/Makefile 2008-02-18 21:16:36.000000000 -0700
+++ work6/drivers/pci/Makefile 2008-02-18 21:16:38.000000000 -0700
@@ -2,7 +2,7 @@
# Makefile for the PCI bus specific drivers.
#
-obj-y += access.o bus.o probe.o remove.o pci.o quirks.o \
+obj-y += access.o bios.o bus.o probe.o remove.o pci.o quirks.o \
pci-driver.o search.o pci-sysfs.o rom.o setup-res.o
obj-$(CONFIG_PROC_FS) += proc.o
Index: work6/drivers/pci/bios.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ work6/drivers/pci/bios.c 2008-02-18 21:16:38.000000000 -0700
@@ -0,0 +1,64 @@
+/*
+ * Generic pcibios routines, derived from x86 version
+ *
+ * Copyright 1993, 1994 Drew Eckhardt
+ * Visionary Computing
+ * (Unix and Linux consulting and custom programming)
+ * Drew@Colorado.EDU
+ * +1 (303) 786-7975
+ *
+ * Drew's work was sponsored by:
+ * iX Multiuser Multitasking Magazine
+ * Hannover, Germany
+ * hm@ix.de
+ *
+ * Copyright 1997--2000 Martin Mares <mj@ucw.cz>
+ */
+
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/ioport.h>
+#include <linux/errno.h>
+
+int __attribute__ ((weak)) pcibios_enable_resources(struct pci_dev *dev,
+ int mask)
+{
+ u16 cmd, old_cmd;
+ int i;
+ struct resource *r;
+
+ pci_read_config_word(dev, PCI_COMMAND, &cmd);
+ old_cmd = cmd;
+
+ for (i = 0; i < PCI_NUM_RESOURCES; i++) {
+ if (!(mask & (1 << i)))
+ continue;
+
+ r = &dev->resource[i];
+
+ if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
+ continue;
+ if ((i == PCI_ROM_RESOURCE) &&
+ (!(r->flags & IORESOURCE_ROM_ENABLE)))
+ continue;
+
+ if (!r->start && r->end) {
+ dev_err(&dev->dev, "device not available because of "
+ "resource %d collisions\n", i);
+ return -EINVAL;
+ }
+
+ if (r->flags & IORESOURCE_IO)
+ cmd |= PCI_COMMAND_IO;
+ if (r->flags & IORESOURCE_MEM)
+ cmd |= PCI_COMMAND_MEMORY;
+ }
+
+ if (cmd != old_cmd) {
+ dev_info(&dev->dev, "enabling device (%04x -> %04x)\n",
+ old_cmd, cmd);
+ pci_write_config_word(dev, PCI_COMMAND, cmd);
+ }
+ return 0;
+}
Index: work6/include/linux/pci.h
===================================================================
--- work6.orig/include/linux/pci.h 2008-02-18 21:16:36.000000000 -0700
+++ work6/include/linux/pci.h 2008-02-18 21:16:38.000000000 -0700
@@ -443,6 +443,7 @@
extern int no_pci_devices(void);
void pcibios_fixup_bus(struct pci_bus *);
+int pcibios_enable_resources(struct pci_dev *, int mask);
int __must_check pcibios_enable_device(struct pci_dev *, int mask);
char *pcibios_setup(char *str);
--
^ permalink raw reply
* [patch 1/4] PCI: split pcibios_enable_resources() out of pcibios_enable_device()
From: Bjorn Helgaas @ 2008-02-19 4:39 UTC (permalink / raw)
To: linux-pci, linux-arch
Cc: Chris Zankel, Grant Grundler, linux-parisc, Matthew Wilcox,
Kyle McMartin, linuxppc-dev, Paul Mackerras, linux-arm-kernel,
Russell King
In-Reply-To: <20080219043952.845136014@ldl.fc.hp.com>
On x86, pcibios_enable_device() is factored into
pcibios_enable_resources() and pcibios_enable_irq(). On several other
architectures, the functional equivalent of pcibios_enable_resources()
is expanded directly inside pcibios_enable_device().
This splits these pcibios_enable_device() implementations to make them
more similar to the x86 implementation.
There should be no functional change from this patch.
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
arch/alpha/kernel/pci.c | 8 +++++++-
arch/arm/kernel/bios32.c | 9 +++++++--
arch/parisc/kernel/pci.c | 6 +++++-
arch/powerpc/kernel/pci-common.c | 14 +++++++++-----
arch/sh/drivers/pci/pci.c | 7 ++++++-
arch/sparc64/kernel/pci.c | 7 ++++++-
arch/v850/kernel/rte_mb_a_pci.c | 7 ++++++-
7 files changed, 46 insertions(+), 12 deletions(-)
Index: work6/arch/alpha/kernel/pci.c
===================================================================
--- work6.orig/arch/alpha/kernel/pci.c 2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/alpha/kernel/pci.c 2008-02-18 10:45:14.000000000 -0700
@@ -370,7 +370,7 @@
#endif
int
-pcibios_enable_device(struct pci_dev *dev, int mask)
+pcibios_enable_resources(struct pci_dev *dev, int mask)
{
u16 cmd, oldcmd;
int i;
@@ -396,6 +396,12 @@
return 0;
}
+int
+pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+ return pcibios_enable_resources(dev, mask);
+}
+
/*
* If we set up a device for bus mastering, we need to check the latency
* timer as certain firmware forgets to set it properly, as seen
Index: work6/arch/arm/kernel/bios32.c
===================================================================
--- work6.orig/arch/arm/kernel/bios32.c 2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/arm/kernel/bios32.c 2008-02-18 10:45:14.000000000 -0700
@@ -655,10 +655,10 @@
}
/**
- * pcibios_enable_device - Enable I/O and memory.
+ * pcibios_enable_resources - Enable I/O and memory.
* @dev: PCI device to be enabled
*/
-int pcibios_enable_device(struct pci_dev *dev, int mask)
+int pcibios_enable_resources(struct pci_dev *dev, int mask)
{
u16 cmd, old_cmd;
int idx;
@@ -697,6 +697,11 @@
return 0;
}
+int pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+ return pcibios_enable_resources(dev, mask);
+}
+
int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
enum pci_mmap_state mmap_state, int write_combine)
{
Index: work6/arch/parisc/kernel/pci.c
===================================================================
--- work6.orig/arch/parisc/kernel/pci.c 2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/parisc/kernel/pci.c 2008-02-18 10:45:14.000000000 -0700
@@ -285,7 +285,7 @@
* Drivers that do not need parity (eg graphics and possibly networking)
* can clear these bits if they want.
*/
-int pcibios_enable_device(struct pci_dev *dev, int mask)
+int pcibios_enable_resources(struct pci_dev *dev, int mask)
{
u16 cmd;
int idx;
@@ -317,6 +317,10 @@
return 0;
}
+int pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+ return pcibios_enable_resources(dev, mask);
+}
/* PA-RISC specific */
void pcibios_register_hba(struct pci_hba_data *hba)
Index: work6/arch/powerpc/kernel/pci-common.c
===================================================================
--- work6.orig/arch/powerpc/kernel/pci-common.c 2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/powerpc/kernel/pci-common.c 2008-02-18 10:45:14.000000000 -0700
@@ -1153,16 +1153,12 @@
EXPORT_SYMBOL_GPL(pcibios_claim_one_bus);
#endif /* CONFIG_HOTPLUG */
-int pcibios_enable_device(struct pci_dev *dev, int mask)
+int pcibios_enable_resources(struct pci_dev *dev, int mask)
{
u16 cmd, old_cmd;
int idx;
struct resource *r;
- if (ppc_md.pcibios_enable_device_hook)
- if (ppc_md.pcibios_enable_device_hook(dev))
- return -EINVAL;
-
pci_read_config_word(dev, PCI_COMMAND, &cmd);
old_cmd = cmd;
for (idx = 0; idx < PCI_NUM_RESOURCES; idx++) {
@@ -1193,3 +1189,11 @@
return 0;
}
+int pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+ if (ppc_md.pcibios_enable_device_hook)
+ if (ppc_md.pcibios_enable_device_hook(dev))
+ return -EINVAL;
+
+ return pcibios_enable_resources(dev, mask);
+}
Index: work6/arch/sh/drivers/pci/pci.c
===================================================================
--- work6.orig/arch/sh/drivers/pci/pci.c 2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/sh/drivers/pci/pci.c 2008-02-18 10:45:14.000000000 -0700
@@ -131,7 +131,7 @@
}
}
-int pcibios_enable_device(struct pci_dev *dev, int mask)
+int pcibios_enable_resources(struct pci_dev *dev, int mask)
{
u16 cmd, old_cmd;
int idx;
@@ -163,6 +163,11 @@
return 0;
}
+int pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+ return pcibios_enable_resources(dev, mask);
+}
+
/*
* If we set up a device for bus mastering, we need to check and set
* the latency timer as it may not be properly set.
Index: work6/arch/sparc64/kernel/pci.c
===================================================================
--- work6.orig/arch/sparc64/kernel/pci.c 2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/sparc64/kernel/pci.c 2008-02-18 10:45:14.000000000 -0700
@@ -946,7 +946,7 @@
{
}
-int pcibios_enable_device(struct pci_dev *dev, int mask)
+int pcibios_enable_resources(struct pci_dev *dev, int mask)
{
u16 cmd, oldcmd;
int i;
@@ -976,6 +976,11 @@
return 0;
}
+int pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+ return pcibios_enable_resources(dev, mask);
+}
+
void pcibios_resource_to_bus(struct pci_dev *pdev, struct pci_bus_region *region,
struct resource *res)
{
Index: work6/arch/v850/kernel/rte_mb_a_pci.c
===================================================================
--- work6.orig/arch/v850/kernel/rte_mb_a_pci.c 2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/v850/kernel/rte_mb_a_pci.c 2008-02-18 10:45:14.000000000 -0700
@@ -217,7 +217,7 @@
}
\f
-int __nomods_init pcibios_enable_device (struct pci_dev *dev, int mask)
+int __nomods_init pcibios_enable_resources (struct pci_dev *dev, int mask)
{
u16 cmd, old_cmd;
int idx;
@@ -245,6 +245,11 @@
return 0;
}
+int __nomods_init pcibios_enable_device (struct pci_dev *dev, int mask)
+{
+ return pcibios_enable_resources(dev, mask);
+}
+
\f
/* Resource allocation. */
static void __devinit pcibios_assign_resources (void)
--
^ permalink raw reply
* [patch 2/4] ppc: make pcibios_enable_device() use pcibios_enable_resources()
From: Bjorn Helgaas @ 2008-02-19 4:39 UTC (permalink / raw)
To: linux-pci, linux-arch
Cc: Chris Zankel, Grant Grundler, linux-parisc, Matthew Wilcox,
Kyle McMartin, linuxppc-dev, Paul Mackerras, linux-arm-kernel,
Russell King
In-Reply-To: <20080219043952.845136014@ldl.fc.hp.com>
pcibios_enable_device() has an almost verbatim copy of
pcibios_enable_resources(), (the only difference is that
pcibios_enable_resources() turns on PCI_COMMAND_MEMORY if
there's a ROM resource).
The duplication might be intentional, but I don't see any callers
of pcibios_enable_resources() on ppc, so I think it's more
likely a historical accident.
This patch removes the duplication, making pcibios_enable_device()
simply call pcibios_enable_resources() as x86 does.
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Index: work6/arch/ppc/kernel/pci.c
===================================================================
--- work6.orig/arch/ppc/kernel/pci.c 2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/ppc/kernel/pci.c 2008-02-18 11:31:23.000000000 -0700
@@ -785,33 +785,11 @@
int pcibios_enable_device(struct pci_dev *dev, int mask)
{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
if (ppc_md.pcibios_enable_device_hook)
if (ppc_md.pcibios_enable_device_hook(dev, 0))
return -EINVAL;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for (idx=0; idx<6; idx++) {
- r = &dev->resource[idx];
- if (r->flags & IORESOURCE_UNSET) {
- printk(KERN_ERR "PCI: Device %s not available because of resource collisions\n", pci_name(dev));
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
- if (cmd != old_cmd) {
- printk("PCI: Enabling device %s (%04x -> %04x)\n",
- pci_name(dev), old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
- return 0;
+
+ return pcibios_enable_resources(dev, mask);
}
struct pci_controller*
--
^ permalink raw reply
* [patch 3/4] xtensa: make pcibios_enable_device() use pcibios_enable_resources()
From: Bjorn Helgaas @ 2008-02-19 4:39 UTC (permalink / raw)
To: linux-pci, linux-arch
Cc: Chris Zankel, Grant Grundler, linux-parisc, Matthew Wilcox,
Kyle McMartin, linuxppc-dev, Paul Mackerras, linux-arm-kernel,
Russell King
In-Reply-To: <20080219043952.845136014@ldl.fc.hp.com>
pcibios_enable_device() has an almost verbatim copy of
pcibios_enable_resources(), (the only difference is that
pcibios_enable_resources() turns on PCI_COMMAND_MEMORY if
there's a ROM resource).
The duplication might be intentional, but I don't see any callers
of pcibios_enable_resources() on xtensa, so I think it's more
likely a historical accident copied from ppc.
This patch removes the duplication, making pcibios_enable_device()
simply call pcibios_enable_resources() as x86 does.
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Index: work6/arch/xtensa/kernel/pci.c
===================================================================
--- work6.orig/arch/xtensa/kernel/pci.c 2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/xtensa/kernel/pci.c 2008-02-18 11:32:12.000000000 -0700
@@ -238,31 +238,7 @@
int pcibios_enable_device(struct pci_dev *dev, int mask)
{
- u16 cmd, old_cmd;
- int idx;
- struct resource *r;
-
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- old_cmd = cmd;
- for (idx=0; idx<6; idx++) {
- r = &dev->resource[idx];
- if (!r->start && r->end) {
- printk(KERN_ERR "PCI: Device %s not available because "
- "of resource collisions\n", pci_name(dev));
- return -EINVAL;
- }
- if (r->flags & IORESOURCE_IO)
- cmd |= PCI_COMMAND_IO;
- if (r->flags & IORESOURCE_MEM)
- cmd |= PCI_COMMAND_MEMORY;
- }
- if (cmd != old_cmd) {
- printk("PCI: Enabling device %s (%04x -> %04x)\n",
- pci_name(dev), old_cmd, cmd);
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
-
- return 0;
+ return pcibios_enable_resources(dev, mask);
}
#ifdef CONFIG_PROC_FS
--
^ permalink raw reply
* Re: [PATCH 1/3] Fix Unlikely(x) == y
From: Willy Tarreau @ 2008-02-19 5:58 UTC (permalink / raw)
To: Nick Piggin
Cc: Roel Kluin, lkml, linuxppc-dev, Andi Kleen, cbe-oss-dev,
Arjan van de Ven
In-Reply-To: <200802191333.53607.nickpiggin@yahoo.com.au>
On Tue, Feb 19, 2008 at 01:33:53PM +1100, Nick Piggin wrote:
> > Note in particular the last predictors; assuming branch ending
> > with goto, including call, causing early function return or
> > returning negative constant are not taken. Just these alone
> > are likely 95+% of the unlikelies in the kernel.
>
> Yes, gcc should be able to do pretty good heuristics, considering
> the quite good numbers that cold CPU predictors can attain. However
> for really performance critical code (or really "never" executed
> code), then I think it is OK to have the hints and not have to rely
> on gcc heuristics.
in my experience, the real problem is that gcc does what *it* wants and not
what *you* want. I've been annoyed a lot by the way it coded some loops that
could really be blazingly fast, but which resulted in a ton of branches due
to its predictors. And using unlikely() there was a real mess, because instead
of just hinting the compiler with probabilities to write some linear code for
the *most* common case, it ended up with awful branches everywhere with code
sent far away and even duplicated for some branches.
Sometimes, for performance critical paths, I would like gcc to be dumb and
follow *my* code and not its hard-coded probabilities. For instance, in a
tree traversal, you really know how you want to build your loop. And these
days, it seems like the single method of getting it your way is doing asm,
which obviously is not portable :-(
Maybe one thing we would need would be the ability to assign probabilities
to each branch based on what we expect, so that gcc could build a better
tree keeping most frequently used code tight.
Hmm I've just noticed -fno-guess-branch-probability in the man, I never tried
it.
regards,
Willy
^ permalink raw reply
* Re: [patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
From: Kyle McMartin @ 2008-02-19 6:11 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-arch, Chris Zankel, Grant Grundler, linux-parisc,
Matthew Wilcox, Kyle McMartin, linuxppc-dev, Paul Mackerras,
linux-pci, linux-arm-kernel, Russell King
In-Reply-To: <20080219043952.845136014@ldl.fc.hp.com>
On Mon, Feb 18, 2008 at 09:39:52PM -0700, Bjorn Helgaas wrote:
> - PA-RISC always turns on SERR and PARITY, which no other arch does
>
I suspect this is because we set the host bus adapters to hard fail
(HPMC) on detecting an error, since we don't want to
1) return possibly bogus (-1) data
2) write code to use the (undocumented) error detection
More to the point, I suspect it's extra paranoia because firmware has
set it up this way. I put in a quick hack to test whether those bits
were set, and they came enabled that way by firmware on all the boxes I
tested. (Disclaimer, I didn't have any easily accessible boxes with
add-on cards installed, so firmware might just set it up for core
devices, and we're making sure its set everywhere.)
cheers, Kyle
^ permalink raw reply
* Re: [patch 1/4] PCI: split pcibios_enable_resources() out of pcibios_enable_device()
From: Benjamin Herrenschmidt @ 2008-02-19 6:27 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-arch, Chris Zankel, Grant Grundler, linux-parisc,
Matthew Wilcox, Kyle McMartin, linuxppc-dev, Paul Mackerras,
linux-pci, linux-arm-kernel, Russell King
In-Reply-To: <20080219044307.157131373@ldl.fc.hp.com>
On Mon, 2008-02-18 at 21:39 -0700, Bjorn Helgaas wrote:
> plain text document attachment (make-pcibios_enable_resources)
> On x86, pcibios_enable_device() is factored into
> pcibios_enable_resources() and pcibios_enable_irq(). On several other
> architectures, the functional equivalent of pcibios_enable_resources()
> is expanded directly inside pcibios_enable_device().
>
> This splits these pcibios_enable_device() implementations to make them
> more similar to the x86 implementation.
>
> There should be no functional change from this patch.
>
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
> ---
> arch/alpha/kernel/pci.c | 8 +++++++-
> arch/arm/kernel/bios32.c | 9 +++++++--
> arch/parisc/kernel/pci.c | 6 +++++-
> arch/powerpc/kernel/pci-common.c | 14 +++++++++-----
> arch/sh/drivers/pci/pci.c | 7 ++++++-
> arch/sparc64/kernel/pci.c | 7 ++++++-
> arch/v850/kernel/rte_mb_a_pci.c | 7 ++++++-
> 7 files changed, 46 insertions(+), 12 deletions(-)
>
> Index: work6/arch/alpha/kernel/pci.c
> ===================================================================
> --- work6.orig/arch/alpha/kernel/pci.c 2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/alpha/kernel/pci.c 2008-02-18 10:45:14.000000000 -0700
> @@ -370,7 +370,7 @@
> #endif
>
> int
> -pcibios_enable_device(struct pci_dev *dev, int mask)
> +pcibios_enable_resources(struct pci_dev *dev, int mask)
> {
> u16 cmd, oldcmd;
> int i;
> @@ -396,6 +396,12 @@
> return 0;
> }
>
> +int
> +pcibios_enable_device(struct pci_dev *dev, int mask)
> +{
> + return pcibios_enable_resources(dev, mask);
> +}
> +
> /*
> * If we set up a device for bus mastering, we need to check the latency
> * timer as certain firmware forgets to set it properly, as seen
> Index: work6/arch/arm/kernel/bios32.c
> ===================================================================
> --- work6.orig/arch/arm/kernel/bios32.c 2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/arm/kernel/bios32.c 2008-02-18 10:45:14.000000000 -0700
> @@ -655,10 +655,10 @@
> }
>
> /**
> - * pcibios_enable_device - Enable I/O and memory.
> + * pcibios_enable_resources - Enable I/O and memory.
> * @dev: PCI device to be enabled
> */
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> +int pcibios_enable_resources(struct pci_dev *dev, int mask)
> {
> u16 cmd, old_cmd;
> int idx;
> @@ -697,6 +697,11 @@
> return 0;
> }
>
> +int pcibios_enable_device(struct pci_dev *dev, int mask)
> +{
> + return pcibios_enable_resources(dev, mask);
> +}
> +
> int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
> enum pci_mmap_state mmap_state, int write_combine)
> {
> Index: work6/arch/parisc/kernel/pci.c
> ===================================================================
> --- work6.orig/arch/parisc/kernel/pci.c 2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/parisc/kernel/pci.c 2008-02-18 10:45:14.000000000 -0700
> @@ -285,7 +285,7 @@
> * Drivers that do not need parity (eg graphics and possibly networking)
> * can clear these bits if they want.
> */
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> +int pcibios_enable_resources(struct pci_dev *dev, int mask)
> {
> u16 cmd;
> int idx;
> @@ -317,6 +317,10 @@
> return 0;
> }
>
> +int pcibios_enable_device(struct pci_dev *dev, int mask)
> +{
> + return pcibios_enable_resources(dev, mask);
> +}
>
> /* PA-RISC specific */
> void pcibios_register_hba(struct pci_hba_data *hba)
> Index: work6/arch/powerpc/kernel/pci-common.c
> ===================================================================
> --- work6.orig/arch/powerpc/kernel/pci-common.c 2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/powerpc/kernel/pci-common.c 2008-02-18 10:45:14.000000000 -0700
> @@ -1153,16 +1153,12 @@
> EXPORT_SYMBOL_GPL(pcibios_claim_one_bus);
> #endif /* CONFIG_HOTPLUG */
>
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> +int pcibios_enable_resources(struct pci_dev *dev, int mask)
> {
> u16 cmd, old_cmd;
> int idx;
> struct resource *r;
>
> - if (ppc_md.pcibios_enable_device_hook)
> - if (ppc_md.pcibios_enable_device_hook(dev))
> - return -EINVAL;
> -
> pci_read_config_word(dev, PCI_COMMAND, &cmd);
> old_cmd = cmd;
> for (idx = 0; idx < PCI_NUM_RESOURCES; idx++) {
> @@ -1193,3 +1189,11 @@
> return 0;
> }
>
> +int pcibios_enable_device(struct pci_dev *dev, int mask)
> +{
> + if (ppc_md.pcibios_enable_device_hook)
> + if (ppc_md.pcibios_enable_device_hook(dev))
> + return -EINVAL;
> +
> + return pcibios_enable_resources(dev, mask);
> +}
> Index: work6/arch/sh/drivers/pci/pci.c
> ===================================================================
> --- work6.orig/arch/sh/drivers/pci/pci.c 2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/sh/drivers/pci/pci.c 2008-02-18 10:45:14.000000000 -0700
> @@ -131,7 +131,7 @@
> }
> }
>
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> +int pcibios_enable_resources(struct pci_dev *dev, int mask)
> {
> u16 cmd, old_cmd;
> int idx;
> @@ -163,6 +163,11 @@
> return 0;
> }
>
> +int pcibios_enable_device(struct pci_dev *dev, int mask)
> +{
> + return pcibios_enable_resources(dev, mask);
> +}
> +
> /*
> * If we set up a device for bus mastering, we need to check and set
> * the latency timer as it may not be properly set.
> Index: work6/arch/sparc64/kernel/pci.c
> ===================================================================
> --- work6.orig/arch/sparc64/kernel/pci.c 2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/sparc64/kernel/pci.c 2008-02-18 10:45:14.000000000 -0700
> @@ -946,7 +946,7 @@
> {
> }
>
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> +int pcibios_enable_resources(struct pci_dev *dev, int mask)
> {
> u16 cmd, oldcmd;
> int i;
> @@ -976,6 +976,11 @@
> return 0;
> }
>
> +int pcibios_enable_device(struct pci_dev *dev, int mask)
> +{
> + return pcibios_enable_resources(dev, mask);
> +}
> +
> void pcibios_resource_to_bus(struct pci_dev *pdev, struct pci_bus_region *region,
> struct resource *res)
> {
> Index: work6/arch/v850/kernel/rte_mb_a_pci.c
> ===================================================================
> --- work6.orig/arch/v850/kernel/rte_mb_a_pci.c 2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/v850/kernel/rte_mb_a_pci.c 2008-02-18 10:45:14.000000000 -0700
> @@ -217,7 +217,7 @@
> }
>
> \f
> -int __nomods_init pcibios_enable_device (struct pci_dev *dev, int mask)
> +int __nomods_init pcibios_enable_resources (struct pci_dev *dev, int mask)
> {
> u16 cmd, old_cmd;
> int idx;
> @@ -245,6 +245,11 @@
> return 0;
> }
>
> +int __nomods_init pcibios_enable_device (struct pci_dev *dev, int mask)
> +{
> + return pcibios_enable_resources(dev, mask);
> +}
> +
> \f
> /* Resource allocation. */
> static void __devinit pcibios_assign_resources (void)
>
^ permalink raw reply
* Re: [patch 2/4] ppc: make pcibios_enable_device() use pcibios_enable_resources()
From: Benjamin Herrenschmidt @ 2008-02-19 6:26 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-arch, Chris Zankel, Grant Grundler, linux-parisc,
Matthew Wilcox, Kyle McMartin, linuxppc-dev, Paul Mackerras,
linux-pci, linux-arm-kernel, Russell King
In-Reply-To: <20080219044307.479128872@ldl.fc.hp.com>
On Mon, 2008-02-18 at 21:39 -0700, Bjorn Helgaas wrote:
> plain text document attachment (ppc-pcibios_enable_resources)
> pcibios_enable_device() has an almost verbatim copy of
> pcibios_enable_resources(), (the only difference is that
> pcibios_enable_resources() turns on PCI_COMMAND_MEMORY if
> there's a ROM resource).
>
> The duplication might be intentional, but I don't see any callers
> of pcibios_enable_resources() on ppc, so I think it's more
> likely a historical accident.
>
> This patch removes the duplication, making pcibios_enable_device()
> simply call pcibios_enable_resources() as x86 does.
>
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Ack. arch/ppc is being phased out soon anyway.
Ben.
>
> Index: work6/arch/ppc/kernel/pci.c
> ===================================================================
> --- work6.orig/arch/ppc/kernel/pci.c 2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/ppc/kernel/pci.c 2008-02-18 11:31:23.000000000 -0700
> @@ -785,33 +785,11 @@
>
> int pcibios_enable_device(struct pci_dev *dev, int mask)
> {
> - u16 cmd, old_cmd;
> - int idx;
> - struct resource *r;
> -
> if (ppc_md.pcibios_enable_device_hook)
> if (ppc_md.pcibios_enable_device_hook(dev, 0))
> return -EINVAL;
> -
> - pci_read_config_word(dev, PCI_COMMAND, &cmd);
> - old_cmd = cmd;
> - for (idx=0; idx<6; idx++) {
> - r = &dev->resource[idx];
> - if (r->flags & IORESOURCE_UNSET) {
> - printk(KERN_ERR "PCI: Device %s not available because of resource collisions\n", pci_name(dev));
> - return -EINVAL;
> - }
> - if (r->flags & IORESOURCE_IO)
> - cmd |= PCI_COMMAND_IO;
> - if (r->flags & IORESOURCE_MEM)
> - cmd |= PCI_COMMAND_MEMORY;
> - }
> - if (cmd != old_cmd) {
> - printk("PCI: Enabling device %s (%04x -> %04x)\n",
> - pci_name(dev), old_cmd, cmd);
> - pci_write_config_word(dev, PCI_COMMAND, cmd);
> - }
> - return 0;
> +
> + return pcibios_enable_resources(dev, mask);
> }
>
> struct pci_controller*
>
^ permalink raw reply
* Re: [patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
From: Benjamin Herrenschmidt @ 2008-02-19 6:31 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-arch, Chris Zankel, Grant Grundler, linux-parisc,
Matthew Wilcox, Kyle McMartin, linuxppc-dev, Paul Mackerras,
linux-pci, linux-arm-kernel, Russell King
In-Reply-To: <20080219044307.878416912@ldl.fc.hp.com>
On Mon, 2008-02-18 at 21:39 -0700, Bjorn Helgaas wrote:
> powerpc: has a different collision check at (5)
I've always found the collision check dodgy. I tend to want to keep
the way powerpc does it here.
pci_enable_device() should only enable resources that have successfully
been added to the resource tree (that have passed all the collision
check etc...). There is a simple & clear indication of that: res->parent
is non-NULL. I think that is a better check than the test x86 does on
start and end.
That is, whatever the arch code decides to use to decide whether
resources are assigned by firmware or by the first pass assignment code
or not and collide or not, once that phase is finished (which is the
case when calling pcibios_enable_device(), having the resource in the
resource-tree or not is, I believe, the proper way to test whether it's
a useable resource.
Ben.
^ permalink raw reply
* Re: [patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
From: Benjamin Herrenschmidt @ 2008-02-19 6:33 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-arch, Chris Zankel, Grant Grundler, linux-parisc,
Matthew Wilcox, Kyle McMartin, linuxppc-dev, Paul Mackerras,
linux-pci, linux-arm-kernel, Russell King
In-Reply-To: <20080219044307.878416912@ldl.fc.hp.com>
> Index: work6/drivers/pci/Makefile
> ===================================================================
> --- work6.orig/drivers/pci/Makefile 2008-02-18 21:16:36.000000000 -0700
> +++ work6/drivers/pci/Makefile 2008-02-18 21:16:38.000000000 -0700
> @@ -2,7 +2,7 @@
> # Makefile for the PCI bus specific drivers.
> #
>
> -obj-y += access.o bus.o probe.o remove.o pci.o quirks.o \
> +obj-y += access.o bios.o bus.o probe.o remove.o pci.o quirks.o \
> pci-driver.o search.o pci-sysfs.o rom.o setup-res.o
> obj-$(CONFIG_PROC_FS) += proc.o
>
> Index: work6/drivers/pci/bios.c
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ work6/drivers/pci/bios.c 2008-02-18 21:16:38.000000000 -070
^^^^^^
Yuck :-)
Please, don't call this bios ... whatever is in this file really has
nothing to do with a "BIOS" in any shape or form :-) I know we used to
call those things pcibios_* but that's really historical...
If you want to make clear it's for "utilities" that can be overriden
by the arch, maybe call it utils.c, or just stick the function in
pci.c, or setup-res.c
Ben.
^ permalink raw reply
* Re: [PATCH 1/3] Fix Unlikely(x) == y
From: Nick Piggin @ 2008-02-19 6:20 UTC (permalink / raw)
To: Willy Tarreau
Cc: Roel Kluin, lkml, linuxppc-dev, Andi Kleen, cbe-oss-dev,
Arjan van de Ven
In-Reply-To: <20080219055806.GA8404@1wt.eu>
On Tuesday 19 February 2008 16:58, Willy Tarreau wrote:
> On Tue, Feb 19, 2008 at 01:33:53PM +1100, Nick Piggin wrote:
> > > Note in particular the last predictors; assuming branch ending
> > > with goto, including call, causing early function return or
> > > returning negative constant are not taken. Just these alone
> > > are likely 95+% of the unlikelies in the kernel.
> >
> > Yes, gcc should be able to do pretty good heuristics, considering
> > the quite good numbers that cold CPU predictors can attain. However
> > for really performance critical code (or really "never" executed
> > code), then I think it is OK to have the hints and not have to rely
> > on gcc heuristics.
>
> in my experience, the real problem is that gcc does what *it* wants and not
> what *you* want. I've been annoyed a lot by the way it coded some loops
> that could really be blazingly fast, but which resulted in a ton of
> branches due to its predictors. And using unlikely() there was a real mess,
> because instead of just hinting the compiler with probabilities to write
> some linear code for the *most* common case, it ended up with awful
> branches everywhere with code sent far away and even duplicated for some
> branches.
>
> Sometimes, for performance critical paths, I would like gcc to be dumb and
> follow *my* code and not its hard-coded probabilities. For instance, in a
> tree traversal, you really know how you want to build your loop. And these
> days, it seems like the single method of getting it your way is doing asm,
> which obviously is not portable :-(
Probably all true.
> Maybe one thing we would need would be the ability to assign probabilities
> to each branch based on what we expect, so that gcc could build a better
> tree keeping most frequently used code tight.
I don't know if that would *directly* lead to gcc being smarter. I
think perhaps they probably don't benchmark on code bases that have
much explicit annotation (I'm sure they wouldn't seriously benchmark
any parts of Linux as part of daily development). I think the key is
to continue to use annotations _properly_, and eventually gcc should
go in the right direction if enough code uses it.
And if you have really good examples like it sounds like above, then
I guess that should be reported to gcc?
^ permalink raw reply
* Re: [patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
From: Kyle McMartin @ 2008-02-19 7:03 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-arch, Chris Zankel, Grant Grundler, linux-parisc,
Matthew Wilcox, Kyle McMartin, linuxppc-dev, Paul Mackerras,
linux-pci, linux-arm-kernel, Russell King
In-Reply-To: <20080219044307.878416912@ldl.fc.hp.com>
On Mon, Feb 18, 2008 at 09:39:56PM -0700, Bjorn Helgaas wrote:
> parisc: checks DEVICE_COUNT_RESOURCE (12) instead of PCI_NUM_RESOURCES
> (11) resources at (1),
>
Good catch.
> has no IORESOURCE_{IO,MEM} check at (3),
What else can it be?
> has no PCI_ROM_RESOURCE check at (4), has no collision check at (5)
> always turns on PCI_COMMAND_SERR | PCI_COMMAND_PARITY at (6),
> writes cmd even if unchanged at (7)
>
I'll have to check into 4 & 5, there might be a good reason for it. For
instance on a four port HP ethernet card (pci-pci bridge + 4 tulips) all
4 of the rom resources are mapped to the same address, which afaict, is
allowed but breaks things in mysterious and subtle ways.
That said, the parisc pci code is a rats nest...
cheers, Kyle
^ permalink raw reply
* Re: [PATCH 1/3] Fix Unlikely(x) == y
From: Adrian Bunk @ 2008-02-19 7:43 UTC (permalink / raw)
To: Michael Ellerman
Cc: Roel Kluin, lkml, cbe-oss-dev, linuxppc-dev, Geert Uytterhoeven,
Willy Tarreau, Arjan van de Ven
In-Reply-To: <1203371163.6844.2.camel@concordia>
On Tue, Feb 19, 2008 at 08:46:03AM +1100, Michael Ellerman wrote:
> On Mon, 2008-02-18 at 16:13 +0200, Adrian Bunk wrote:
> > On Mon, Feb 18, 2008 at 03:01:35PM +0100, Geert Uytterhoeven wrote:
> > > On Mon, 18 Feb 2008, Adrian Bunk wrote:
> > > >
> > > > This means it generates faster code with a current gcc for your platform.
> > > >
> > > > But a future gcc might e.g. replace the whole loop with a division
> > > > (gcc SVN head (that will soon become gcc 4.3) already does
> > > > transformations like replacing loops with divisions [1]).
> > >
> > > Hence shouldn't we ask the gcc people what's the purpose of __builtin_expect(),
> > > if it doesn't live up to its promise?
> >
> > That's a different issue.
> >
> > My point here is that we do not know how the latest gcc available in the
> > year 2010 might transform this code, and how a likely/unlikely placed
> > there might influence gcc's optimizations then.
>
> You're right, we don't know. But if giving the compiler _more_
> information causes it to produce vastly inferior code then we should be
> filing gcc bugs. After all the unlikely/likely is just a hint, if gcc
> knows better it can always ignore it.
It's the other way round, gcc assumes that you know better than gcc when
you give it a __builtin_expect().
The example you gave had only a 1:3 ratio, which is far outside of the
ratios where __builtin_expect() should be used.
What if you gave this annotation for the 1:3 case and gcc generates code
that performs better for ratios > 1:1000 but much worse for a 1:3 ratio
since your hint did override a better estimate of gcc?
And I'm sure that > 90% of all kernel developers (including me) are
worse in such respects than the gcc heuristics.
I'm a firm believer in the following:
- it's the programmer's job to write clean and efficient C code
- it's the compiler's job to convert C code into efficient assembler
code
The stable interface between the programmer and the compiler is C, and
when the programmer starts manually messing with internals of the
compiler that's a layering violation that requires a _good_
justification.
With a "good justification" not consisting of some microbenchmark but of
measurements of the actual annotations in the kernel code.
> cheers
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply
* Re: [PATCH 2/2] i2c-ibm_iic driver
From: Jean Delvare @ 2008-02-19 8:23 UTC (permalink / raw)
To: Sean MacLennan; +Cc: LinuxPPC-dev, i2c
In-Reply-To: <47BA3416.5080405@pikatech.com>
Hi Sean,
On Mon, 18 Feb 2008 20:42:46 -0500, Sean MacLennan wrote:
> An updated version of the patch. This one should answer all of Jean's
> concerns.
>
> Cheers,
> Sean
>
> Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
> ---
> --- for-2.6.25/drivers/i2c/busses/orig-i2c-ibm_iic.c 2008-02-18 16:36:30.000000000 -0500
> +++ for-2.6.25/drivers/i2c/busses/i2c-ibm_iic.c 2008-02-18 17:39:53.000000000 -0500
> @@ -6,6 +6,9 @@
> * Copyright (c) 2003, 2004 Zultys Technologies.
> * Eugene Surovegin <eugene.surovegin@zultys.com> or <ebs@ebshome.net>
> *
> + * Copyright (c) 2008 PIKA Technologies
> + * Sean MacLennan <smaclennan@pikatech.com>
> + *
> * Based on original work by
> * Ian DaSilva <idasilva@mvista.com>
> * Armin Kuster <akuster@mvista.com>
> @@ -39,12 +42,17 @@
> #include <asm/io.h>
> #include <linux/i2c.h>
> #include <linux/i2c-id.h>
> +
> +#ifdef CONFIG_IBM_OCP
> #include <asm/ocp.h>
> #include <asm/ibm4xx.h>
> +#else
> +#include <linux/of_platform.h>
> +#endif
>
> #include "i2c-ibm_iic.h"
>
> -#define DRIVER_VERSION "2.1"
> +#define DRIVER_VERSION "2.2"
>
> MODULE_DESCRIPTION("IBM IIC driver v" DRIVER_VERSION);
> MODULE_LICENSE("GPL");
> @@ -657,6 +665,7 @@
> return (u8)((opb + 9) / 10 - 1);
> }
>
> +#ifdef CONFIG_IBM_OCP
> /*
> * Register single IIC interface
> */
> @@ -828,5 +837,182 @@
> ocp_unregister_driver(&ibm_iic_driver);
> }
>
> +#else /* !CONFIG_IBM_OCP */
> +
> +static int __devinit iic_request_irq(struct of_device *ofdev,
> + struct ibm_iic_private *dev)
> +{
> + struct device_node *np = ofdev->node;
> + int irq;
> +
> + if (iic_force_poll)
> + return NO_IRQ;
> +
> + irq = irq_of_parse_and_map(np, 0);
> + if (irq == NO_IRQ) {
> + dev_err(&ofdev->dev, "irq_of_parse_and_map failed\n");
> + return NO_IRQ;
> + }
> +
> + /* 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);
> + /* Fallback to the polling mode */
> + return NO_IRQ;
> + }
> +
> + return irq;
> +}
> +
> +/*
> + * Register single IIC interface
> + */
> +static int __devinit iic_probe(struct of_device *ofdev,
> + const struct of_device_id *match)
> +{
> + struct device_node *np = ofdev->node;
> + struct ibm_iic_private *dev;
> + struct i2c_adapter *adap;
> + const u32 *indexp, *freq;
> + int ret;
> +
> +
Double blank line is prohibited inside a function.
> + dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> + if (!dev) {
> + dev_err(&ofdev->dev, "failed to allocate device data\n");
> + return -ENOMEM;
> + }
> +
> + dev_set_drvdata(&ofdev->dev, dev);
> +
> + indexp = of_get_property(np, "index", NULL);
> + if (!indexp) {
> + dev_err(&ofdev->dev, "no index specified.\n");
This is the only error message that has a trailing dot. Remove it?
> + ret = -EINVAL;
Isn't it a bit inconsistent to return -EINVAL here...
> + goto error_cleanup;
> + }
> + dev->idx = *indexp;
> +
> + dev->vaddr = of_iomap(np, 0);
> + if (dev->vaddr == NULL) {
> + dev_err(&ofdev->dev, "failed to iomap device\n");
> + ret = -ENXIO;
> + goto error_cleanup;
> + }
> +
> + init_waitqueue_head(&dev->wq);
> +
> + dev->irq = iic_request_irq(ofdev, dev);
> + if (dev->irq == NO_IRQ)
> + dev_warn(&ofdev->dev, "using polling mode\n");
> +
> + /* Board specific settings */
> + if (iic_force_fast || of_get_property(np, "fast-mode", NULL))
> + dev->fast_mode = 1;
> +
> + freq = of_get_property(np, "clock-frequency", NULL);
> + if (freq == NULL) {
> + freq = of_get_property(np->parent, "clock-frequency", NULL);
> + if (freq == NULL) {
> + dev_err(&ofdev->dev, "Unable to get bus frequency\n");
> + ret = -EBUSY;
... but -EBUSY there?
> + goto error_cleanup;
> + }
> + }
> +
> + dev->clckdiv = iic_clckdiv(*freq);
> + dev_dbg(&ofdev->dev, "clckdiv = %d\n", dev->clckdiv);
> +
> + /* Initialize IIC interface */
> + iic_dev_init(dev);
> +
> + /* Register it with i2c layer */
> + adap = &dev->adap;
> + adap->dev.parent = &ofdev->dev;
> + strlcpy(adap->name, "IBM IIC", sizeof(adap->name));
> + i2c_set_adapdata(adap, dev);
> + adap->id = I2C_HW_OCP;
> + adap->class = I2C_CLASS_HWMON;
> + adap->algo = &iic_algo;
> + adap->timeout = 1;
> + adap->nr = dev->idx;
> +
> + ret = i2c_add_numbered_adapter(adap);
> + if (ret < 0) {
> + dev_err(&ofdev->dev, "failed to register i2c adapter\n");
> + goto error_cleanup;
> + }
> +
> + dev_dbg(&ofdev->dev, "using %s mode\n",
dev_info?
> + dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)");
> +
> + return 0;
> +
> +error_cleanup:
> + if (dev->irq != NO_IRQ) {
> + iic_interrupt_mode(dev, 0);
> + free_irq(dev->irq, dev);
> + }
> +
> + if (dev->vaddr)
> + iounmap(dev->vaddr);
> +
> + dev_set_drvdata(&ofdev->dev, NULL);
> + kfree(dev);
> + return ret;
> +}
> +
> +/*
> + * Cleanup initialized IIC interface
> + */
> +static int __devexit iic_remove(struct of_device *ofdev)
> +{
> + struct ibm_iic_private *dev = dev_get_drvdata(&ofdev->dev);
> +
> + dev_set_drvdata(&ofdev->dev, NULL);
> +
> + i2c_del_adapter(&dev->adap);
> +
> + if (dev->irq != NO_IRQ) {
> + iic_interrupt_mode(dev, 0);
> + free_irq(dev->irq, dev);
> + }
> +
> + iounmap(dev->vaddr);
> + kfree(dev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ibm_iic_match[] = {
> + { .compatible = "ibm,iic-405ex", },
> + { .compatible = "ibm,iic-405gp", },
> + { .compatible = "ibm,iic-440gp", },
> + { .compatible = "ibm,iic-440gpx", },
> + { .compatible = "ibm,iic-440grx", },
> + {}
> +};
> +
> +static struct of_platform_driver ibm_iic_driver = {
> + .name = "ibm-iic",
> + .match_table = ibm_iic_match,
> + .probe = iic_probe,
> + .remove = __devexit_p(iic_remove),
> +};
> +
> +static int __init iic_init(void)
> +{
> + return of_register_platform_driver(&ibm_iic_driver);
> +}
> +
> +static void __exit iic_exit(void)
> +{
> + of_unregister_platform_driver(&ibm_iic_driver);
> +}
> +#endif /* CONFIG_IBM_OCP */
> +
> module_init(iic_init);
> module_exit(iic_exit);
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index b61f56b..44c0984 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -244,7 +244,6 @@ config I2C_PIIX4
>
> config I2C_IBM_IIC
> tristate "IBM PPC 4xx on-chip I2C interface"
> - depends on IBM_OCP
> help
> Say Y here if you want to use IIC peripheral found on
> embedded IBM PPC 4xx based systems.
With this Kconfig change, "make menuconfig" lets me select the
i2c-ibm_iic driver on x86_64, but it fails to build horribly. I think
that you want to restrict the build to PPC machines somehow, or at
least make sure that either IBM_OCP or OF support is present.
The rest looks fine to me. If you can address my last few comments, I
can push your 2 i2c-ibm_iic patches to -mm.
Thanks,
--
Jean Delvare
^ permalink raw reply
* Re: [BUG] Linux 2.6.25-rc2 - Regression from 2.6.24-rc1-git1 softlockup while bootup on powerpc
From: KAMEZAWA Hiroyuki @ 2008-02-19 8:04 UTC (permalink / raw)
To: Jens Axboe
Cc: Dhaval Giani, Srivatsa Vaddagiri, Linux Kernel Mailing List,
linuxppc-dev, Ingo Molnar, Kamalesh Babulal, Balbir Singh
In-Reply-To: <20080217192913.GO23197@kernel.dk>
On Sun, 17 Feb 2008 20:29:13 +0100
Jens Axboe <jens.axboe@oracle.com> wrote:
> It's odd stuff. Could you perhaps try and add some printks to
> block/cfq-iosched.c:call_for_each_cic(), like dumping the 'nr' return
> from radix_tree_gang_lookup() and the pointer value of cics[i] in the
> for() loop after the lookup?
>
I met the same issue on ia64/NUMA box.
seems cisc[]->key is NULL and index for radix_tree_gang_lookup() was always '1'.
Attached patch works well for me,
but I don't know much about cfq. please confirm.
Regards,
-Kame
==
cics[]->key can be NULL.
In that case, cics[]->dead_key has key value.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Index: linux-2.6.25-rc2/block/cfq-iosched.c
===================================================================
--- linux-2.6.25-rc2.orig/block/cfq-iosched.c
+++ linux-2.6.25-rc2/block/cfq-iosched.c
@@ -1171,7 +1171,11 @@ call_for_each_cic(struct io_context *ioc
break;
called += nr;
- index = 1 + (unsigned long) cics[nr - 1]->key;
+
+ if (!cics[nr - 1]->key)
+ index = 1 + (unsigned long) cics[nr - 1]->dead_key;
+ else
+ index = 1 + (unsigned long) cics[nr - 1]->key;
for (i = 0; i < nr; i++)
func(ioc, cics[i]);
^ permalink raw reply
* Re: [BUG] Linux 2.6.25-rc2 - Regression from 2.6.24-rc1-git1 softlockup while bootup on powerpc
From: Jens Axboe @ 2008-02-19 8:36 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki
Cc: Dhaval Giani, Srivatsa Vaddagiri, Linux Kernel Mailing List,
linuxppc-dev, Ingo Molnar, Kamalesh Babulal, Balbir Singh
In-Reply-To: <20080219170432.9c04376f.kamezawa.hiroyu@jp.fujitsu.com>
On Tue, Feb 19 2008, KAMEZAWA Hiroyuki wrote:
> On Sun, 17 Feb 2008 20:29:13 +0100
> Jens Axboe <jens.axboe@oracle.com> wrote:
>
> > It's odd stuff. Could you perhaps try and add some printks to
> > block/cfq-iosched.c:call_for_each_cic(), like dumping the 'nr' return
> > from radix_tree_gang_lookup() and the pointer value of cics[i] in the
> > for() loop after the lookup?
> >
> I met the same issue on ia64/NUMA box.
> seems cisc[]->key is NULL and index for radix_tree_gang_lookup() was
> always '1'.
Why does it keep repeating then? If ->key is NULL, the next lookup index
should be 1UL.
But I think the radix 'scan over entire tree' is a bit fragile. This
patch adds a parallel hlist for ease of properly browsing the members,
does that work for you? It compiles, but I haven't booted it here yet...
> Attached patch works well for me, but I don't know much about cfq.
> please confirm.
It doesn't make a lot of sense, I'm afraid.
block/blk-ioc.c | 35 +++++++++++++++--------------------
block/cfq-iosched.c | 37 +++++++++++--------------------------
include/linux/iocontext.h | 2 ++
3 files changed, 28 insertions(+), 46 deletions(-)
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 80245dc..73c7002 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -17,17 +17,13 @@ static struct kmem_cache *iocontext_cachep;
static void cfq_dtor(struct io_context *ioc)
{
- struct cfq_io_context *cic[1];
- int r;
+ if (!hlist_empty(&ioc->cic_list)) {
+ struct cfq_io_context *cic;
- /*
- * We don't have a specific key to lookup with, so use the gang
- * lookup to just retrieve the first item stored. The cfq exit
- * function will iterate the full tree, so any member will do.
- */
- r = radix_tree_gang_lookup(&ioc->radix_root, (void **) cic, 0, 1);
- if (r > 0)
- cic[0]->dtor(ioc);
+ cic = list_entry(ioc->cic_list.first, struct cfq_io_context,
+ cic_list);
+ cic->dtor(ioc);
+ }
}
/*
@@ -57,18 +53,16 @@ EXPORT_SYMBOL(put_io_context);
static void cfq_exit(struct io_context *ioc)
{
- struct cfq_io_context *cic[1];
- int r;
-
rcu_read_lock();
- /*
- * See comment for cfq_dtor()
- */
- r = radix_tree_gang_lookup(&ioc->radix_root, (void **) cic, 0, 1);
- rcu_read_unlock();
- if (r > 0)
- cic[0]->exit(ioc);
+ if (!hlist_empty(&ioc->cic_list)) {
+ struct cfq_io_context *cic;
+
+ cic = list_entry(ioc->cic_list.first, struct cfq_io_context,
+ cic_list);
+ cic->exit(ioc);
+ }
+ rcu_read_unlock();
}
/* Called by the exitting task */
@@ -105,6 +99,7 @@ struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
ret->nr_batch_requests = 0; /* because this is 0 */
ret->aic = NULL;
INIT_RADIX_TREE(&ret->radix_root, GFP_ATOMIC | __GFP_HIGH);
+ INIT_HLIST_HEAD(&ret->cic_list);
ret->ioc_data = NULL;
}
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index ca198e6..62eda3f 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -1145,38 +1145,19 @@ static void cfq_put_queue(struct cfq_queue *cfqq)
/*
* Call func for each cic attached to this ioc. Returns number of cic's seen.
*/
-#define CIC_GANG_NR 16
static unsigned int
call_for_each_cic(struct io_context *ioc,
void (*func)(struct io_context *, struct cfq_io_context *))
{
- struct cfq_io_context *cics[CIC_GANG_NR];
- unsigned long index = 0;
- unsigned int called = 0;
- int nr;
+ struct cfq_io_context *cic;
+ struct hlist_node *n;
+ int called = 0;
rcu_read_lock();
-
- do {
- int i;
-
- /*
- * Perhaps there's a better way - this just gang lookups from
- * 0 to the end, restarting after each CIC_GANG_NR from the
- * last key + 1.
- */
- nr = radix_tree_gang_lookup(&ioc->radix_root, (void **) cics,
- index, CIC_GANG_NR);
- if (!nr)
- break;
-
- called += nr;
- index = 1 + (unsigned long) cics[nr - 1]->key;
-
- for (i = 0; i < nr; i++)
- func(ioc, cics[i]);
- } while (nr == CIC_GANG_NR);
-
+ hlist_for_each_entry_rcu(cic, n, &ioc->cic_list, cic_list) {
+ func(ioc, cic);
+ called++;
+ }
rcu_read_unlock();
return called;
@@ -1190,6 +1171,7 @@ static void cic_free_func(struct io_context *ioc, struct cfq_io_context *cic)
spin_lock_irqsave(&ioc->lock, flags);
radix_tree_delete(&ioc->radix_root, cic->dead_key);
+ hlist_del_rcu(&cic->cic_list);
spin_unlock_irqrestore(&ioc->lock, flags);
kmem_cache_free(cfq_ioc_pool, cic);
@@ -1280,6 +1262,7 @@ cfq_alloc_io_context(struct cfq_data *cfqd, gfp_t gfp_mask)
if (cic) {
cic->last_end_request = jiffies;
INIT_LIST_HEAD(&cic->queue_list);
+ INIT_HLIST_NODE(&cic->cic_list);
cic->dtor = cfq_free_io_context;
cic->exit = cfq_exit_io_context;
elv_ioc_count_inc(ioc_count);
@@ -1501,6 +1484,7 @@ cfq_drop_dead_cic(struct cfq_data *cfqd, struct io_context *ioc,
rcu_assign_pointer(ioc->ioc_data, NULL);
radix_tree_delete(&ioc->radix_root, (unsigned long) cfqd);
+ hlist_del_rcu(&cic->cic_list);
spin_unlock_irqrestore(&ioc->lock, flags);
cfq_cic_free(cic);
@@ -1561,6 +1545,7 @@ static int cfq_cic_link(struct cfq_data *cfqd, struct io_context *ioc,
spin_lock_irqsave(&ioc->lock, flags);
ret = radix_tree_insert(&ioc->radix_root,
(unsigned long) cfqd, cic);
+ hlist_add_head_rcu(&cic->cic_list, &ioc->cic_list);
spin_unlock_irqrestore(&ioc->lock, flags);
radix_tree_preload_end();
diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
index 593b222..1b4ccf2 100644
--- a/include/linux/iocontext.h
+++ b/include/linux/iocontext.h
@@ -50,6 +50,7 @@ struct cfq_io_context {
sector_t seek_mean;
struct list_head queue_list;
+ struct hlist_node cic_list;
void (*dtor)(struct io_context *); /* destructor */
void (*exit)(struct io_context *); /* called on task exit */
@@ -77,6 +78,7 @@ struct io_context {
struct as_io_context *aic;
struct radix_tree_root radix_root;
+ struct hlist_head cic_list;
void *ioc_data;
};
--
Jens Axboe
^ permalink raw reply related
* Re: [patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
From: Russell King @ 2008-02-19 8:09 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-arch, Chris Zankel, Grant Grundler, linux-parisc,
Matthew Wilcox, Kyle McMartin, linuxppc-dev, Paul Mackerras,
linux-pci, linux-arm-kernel
In-Reply-To: <20080219043952.845136014@ldl.fc.hp.com>
On Mon, Feb 18, 2008 at 09:39:52PM -0700, Bjorn Helgaas wrote:
> There are many implementations of pcibios_enable_resources() that differ
> in minor ways that look more like bugs than architectural differences.
> This patch series consolidates most of them to use the x86 version.
>
> This series is for discussion only at this point. I'm interested in
> feedback about whether any of the differences are "real" and need to
> be preserved.
>
> ARM and PA-RISC, in particular, have interesting differences:
> - ARM always enables bridge devices, which no other arch does
ARM does this because there is nothing else which would do that - which
means devices behind bridges would be completely inaccessible.
> - PA-RISC always turns on SERR and PARITY, which no other arch does
ARM also does this, unless pdev_bad_for_parity(dev) is true. See
ARMs pcibios_fixup_bus().
> Should other arches do the same thing, or are these somehow related to
> ARM and PA-RISC architecture?
I suspect they're architecture specific; I wouldn't like to do either
on x86, but they're either required or preferred on ARM.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
^ 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