2007-09-25 12:11:17

by Joachim Fenkes

[permalink] [raw]
Subject: [PATCH 0/5] PowerPC: ibmebus refactoring and fixes

This patchset will merge the ibmebus and of_platform bus drivers by basing a
lot of ibmebus functionality on of_platform code and adding the features
specific to ibmebus on top of that.

I split the actual ibmebus rework into three patches (2/5-4/5) for easier
readability. The kernel will compile during the intermediate states, and
ibmebus will not crash, but not work either.

As a side-effect of patch 3/5, a problem with bus_id collisions in case of
two devices sharing the same location code is resolved -- the bus_id is now
determined differently.

[1/5] moves of_device allocation into of_device.[ch]
[2/5] removes the old bus match/probe/remove functions
[3/5] adds device creation and bus probing based on of_device
[4/5] finally moves to of_device and of_platform_driver by changing
ibmebus.h and matching the eHCA and eHEA drivers
[5/5] just changes a nit in ibmebus_store_probe()

These patches should apply cleanly, in order, against 2.6.23-rc5 and against
Linus' git. Please review and comment them, and queue them up for 2.6.24 if
you think they're okay.

Thanks and regards,
Joachim

arch/powerpc/kernel/ibmebus.c | 263 ++++++++---------------------
arch/powerpc/kernel/of_device.c | 80 +++++++++
arch/powerpc/kernel/of_platform.c | 70 +--------
drivers/infiniband/hw/ehca/ehca_classes.h | 2 +-
drivers/infiniband/hw/ehca/ehca_eq.c | 6 +-
drivers/infiniband/hw/ehca/ehca_main.c | 32 ++--
drivers/net/ehea/ehea.h | 2 +-
drivers/net/ehea/ehea_main.c | 72 ++++----
include/asm-powerpc/ibmebus.h | 38 +----
include/asm-powerpc/of_device.h | 4 +
include/linux/of_device.h | 5 +
11 files changed, 226 insertions(+), 348 deletions(-)

--
Joachim Fenkes ?-- ?eHCA Linux Driver Developer and Hardware Tamer
IBM Deutschland Entwicklung GmbH ?-- ?Dept. 3627 (I/O Firmware Dev. 2)
Schoenaicher Strasse 220 ?-- ?71032 Boeblingen ?-- ?Germany
eMail: [email protected]


2007-09-25 12:11:40

by Joachim Fenkes

[permalink] [raw]
Subject: [PATCH 1/5] PowerPC: Move of_device allocation into of_device.[ch]

Extract generic of_device allocation code from of_platform_device_create()
and move it into of_device.[ch], called of_device_alloc(). Also, there's now
of_device_free() which puts the device node.

Signed-off-by: Joachim Fenkes <[email protected]>
---
include/asm-powerpc/of_device.h | 4 ++
include/linux/of_device.h | 5 ++
arch/powerpc/kernel/of_device.c | 80 +++++++++++++++++++++++++++++++++++++
arch/powerpc/kernel/of_platform.c | 70 +-------------------------------
4 files changed, 91 insertions(+), 68 deletions(-)

diff --git a/include/asm-powerpc/of_device.h b/include/asm-powerpc/of_device.h
index ec2a8a2..9ab469d 100644
--- a/include/asm-powerpc/of_device.h
+++ b/include/asm-powerpc/of_device.h
@@ -17,6 +17,10 @@ struct of_device
struct device dev; /* Generic device interface */
};

+extern struct of_device *of_device_alloc(struct device_node *np,
+ const char *bus_id,
+ struct device *parent);
+
extern ssize_t of_device_get_modalias(struct of_device *ofdev,
char *str, ssize_t len);
extern int of_device_uevent(struct device *dev,
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index 91bf84b..212bffb 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -22,5 +22,10 @@ extern int of_device_register(struct of_device *ofdev);
extern void of_device_unregister(struct of_device *ofdev);
extern void of_release_dev(struct device *dev);

+static inline void of_device_free(struct of_device *dev)
+{
+ of_release_dev(&dev->dev);
+}
+
#endif /* __KERNEL__ */
#endif /* _LINUX_OF_DEVICE_H */
diff --git a/arch/powerpc/kernel/of_device.c b/arch/powerpc/kernel/of_device.c
index 89b911e..ecb8b0e 100644
--- a/arch/powerpc/kernel/of_device.c
+++ b/arch/powerpc/kernel/of_device.c
@@ -7,8 +7,88 @@
#include <linux/slab.h>

#include <asm/errno.h>
+#include <asm/dcr.h>
#include <asm/of_device.h>

+static void of_device_make_bus_id(struct of_device *dev)
+{
+ static atomic_t bus_no_reg_magic;
+ struct device_node *node = dev->node;
+ char *name = dev->dev.bus_id;
+ const u32 *reg;
+ u64 addr;
+ int magic;
+
+ /*
+ * If it's a DCR based device, use 'd' for native DCRs
+ * and 'D' for MMIO DCRs.
+ */
+#ifdef CONFIG_PPC_DCR
+ reg = of_get_property(node, "dcr-reg", NULL);
+ if (reg) {
+#ifdef CONFIG_PPC_DCR_NATIVE
+ snprintf(name, BUS_ID_SIZE, "d%x.%s",
+ *reg, node->name);
+#else /* CONFIG_PPC_DCR_NATIVE */
+ addr = of_translate_dcr_address(node, *reg, NULL);
+ if (addr != OF_BAD_ADDR) {
+ snprintf(name, BUS_ID_SIZE,
+ "D%llx.%s", (unsigned long long)addr,
+ node->name);
+ return;
+ }
+#endif /* !CONFIG_PPC_DCR_NATIVE */
+ }
+#endif /* CONFIG_PPC_DCR */
+
+ /*
+ * For MMIO, get the physical address
+ */
+ reg = of_get_property(node, "reg", NULL);
+ if (reg) {
+ addr = of_translate_address(node, reg);
+ if (addr != OF_BAD_ADDR) {
+ snprintf(name, BUS_ID_SIZE,
+ "%llx.%s", (unsigned long long)addr,
+ node->name);
+ return;
+ }
+ }
+
+ /*
+ * No BusID, use the node name and add a globally incremented
+ * counter (and pray...)
+ */
+ magic = atomic_add_return(1, &bus_no_reg_magic);
+ snprintf(name, BUS_ID_SIZE, "%s.%d", node->name, magic - 1);
+}
+
+struct of_device *of_device_alloc(struct device_node *np,
+ const char *bus_id,
+ struct device *parent)
+{
+ struct of_device *dev;
+
+ dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ return NULL;
+
+ dev->node = of_node_get(np);
+ dev->dev.dma_mask = &dev->dma_mask;
+ dev->dev.parent = parent;
+ dev->dev.release = of_release_dev;
+ dev->dev.archdata.of_node = np;
+ dev->dev.archdata.numa_node = of_node_to_nid(np);
+
+ if (bus_id)
+ strlcpy(dev->dev.bus_id, bus_id, BUS_ID_SIZE);
+ else
+ of_device_make_bus_id(dev);
+
+ return dev;
+}
+EXPORT_SYMBOL(of_device_alloc);
+
ssize_t of_device_get_modalias(struct of_device *ofdev,
char *str, ssize_t len)
{
diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
index f70e787..1d96b82 100644
--- a/arch/powerpc/kernel/of_platform.c
+++ b/arch/powerpc/kernel/of_platform.c
@@ -21,7 +21,6 @@
#include <linux/pci.h>

#include <asm/errno.h>
-#include <asm/dcr.h>
#include <asm/of_device.h>
#include <asm/of_platform.h>
#include <asm/topology.h>
@@ -53,8 +52,6 @@ static struct of_device_id of_default_bus_ids[] = {
{},
};

-static atomic_t bus_no_reg_magic;
-
struct bus_type of_platform_bus_type = {
.uevent = of_device_uevent,
};
@@ -84,89 +81,26 @@ void of_unregister_platform_driver(struct of_platform_driver *drv)
}
EXPORT_SYMBOL(of_unregister_platform_driver);

-static void of_platform_make_bus_id(struct of_device *dev)
-{
- struct device_node *node = dev->node;
- char *name = dev->dev.bus_id;
- const u32 *reg;
- u64 addr;
- int magic;
-
- /*
- * If it's a DCR based device, use 'd' for native DCRs
- * and 'D' for MMIO DCRs.
- */
-#ifdef CONFIG_PPC_DCR
- reg = of_get_property(node, "dcr-reg", NULL);
- if (reg) {
-#ifdef CONFIG_PPC_DCR_NATIVE
- snprintf(name, BUS_ID_SIZE, "d%x.%s",
- *reg, node->name);
-#else /* CONFIG_PPC_DCR_NATIVE */
- addr = of_translate_dcr_address(node, *reg, NULL);
- if (addr != OF_BAD_ADDR) {
- snprintf(name, BUS_ID_SIZE,
- "D%llx.%s", (unsigned long long)addr,
- node->name);
- return;
- }
-#endif /* !CONFIG_PPC_DCR_NATIVE */
- }
-#endif /* CONFIG_PPC_DCR */
-
- /*
- * For MMIO, get the physical address
- */
- reg = of_get_property(node, "reg", NULL);
- if (reg) {
- addr = of_translate_address(node, reg);
- if (addr != OF_BAD_ADDR) {
- snprintf(name, BUS_ID_SIZE,
- "%llx.%s", (unsigned long long)addr,
- node->name);
- return;
- }
- }
-
- /*
- * No BusID, use the node name and add a globally incremented
- * counter (and pray...)
- */
- magic = atomic_add_return(1, &bus_no_reg_magic);
- snprintf(name, BUS_ID_SIZE, "%s.%d", node->name, magic - 1);
-}
-
struct of_device* of_platform_device_create(struct device_node *np,
const char *bus_id,
struct device *parent)
{
struct of_device *dev;

- dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ dev = of_device_alloc(np, bus_id, parent);
if (!dev)
return NULL;

- dev->node = of_node_get(np);
dev->dma_mask = 0xffffffffUL;
- dev->dev.dma_mask = &dev->dma_mask;
- dev->dev.parent = parent;
dev->dev.bus = &of_platform_bus_type;
- dev->dev.release = of_release_dev;
- dev->dev.archdata.of_node = np;
- dev->dev.archdata.numa_node = of_node_to_nid(np);

/* We do not fill the DMA ops for platform devices by default.
* This is currently the responsibility of the platform code
* to do such, possibly using a device notifier
*/

- if (bus_id)
- strlcpy(dev->dev.bus_id, bus_id, BUS_ID_SIZE);
- else
- of_platform_make_bus_id(dev);
-
if (of_device_register(dev) != 0) {
- kfree(dev);
+ of_device_free(dev);
return NULL;
}

--
1.5.2


2007-09-25 12:12:17

by Joachim Fenkes

[permalink] [raw]
Subject: [PATCH 2/5] ibmebus: Remove bus match/probe/remove functions

ibmebus_{,un}register_driver() are replaced by dummy functions because
ibmebus is temporarily unusable in this transitional state.

Signed-off-by: Joachim Fenkes <[email protected]>
---
arch/powerpc/kernel/ibmebus.c | 199 ++---------------------------------------
1 files changed, 6 insertions(+), 193 deletions(-)

diff --git a/arch/powerpc/kernel/ibmebus.c b/arch/powerpc/kernel/ibmebus.c
index d6a38cd..cc80f84 100644
--- a/arch/powerpc/kernel/ibmebus.c
+++ b/arch/powerpc/kernel/ibmebus.c
@@ -41,6 +41,7 @@
#include <linux/kobject.h>
#include <linux/dma-mapping.h>
#include <linux/interrupt.h>
+#include <linux/of_platform.h>
#include <asm/ibmebus.h>
#include <asm/abs_addr.h>

@@ -123,183 +124,14 @@ static struct dma_mapping_ops ibmebus_dma_ops = {
.dma_supported = ibmebus_dma_supported,
};

-static int ibmebus_bus_probe(struct device *dev)
-{
- struct ibmebus_dev *ibmebusdev = to_ibmebus_dev(dev);
- struct ibmebus_driver *ibmebusdrv = to_ibmebus_driver(dev->driver);
- const struct of_device_id *id;
- int error = -ENODEV;
-
- if (!ibmebusdrv->probe)
- return error;
-
- id = of_match_device(ibmebusdrv->id_table, &ibmebusdev->ofdev);
- if (id) {
- error = ibmebusdrv->probe(ibmebusdev, id);
- }
-
- return error;
-}
-
-static int ibmebus_bus_remove(struct device *dev)
-{
- struct ibmebus_dev *ibmebusdev = to_ibmebus_dev(dev);
- struct ibmebus_driver *ibmebusdrv = to_ibmebus_driver(dev->driver);
-
- if (ibmebusdrv->remove) {
- return ibmebusdrv->remove(ibmebusdev);
- }
-
- return 0;
-}
-
-static void __devinit ibmebus_dev_release(struct device *dev)
-{
- of_node_put(to_ibmebus_dev(dev)->ofdev.node);
- kfree(to_ibmebus_dev(dev));
-}
-
-static int __devinit ibmebus_register_device_common(
- struct ibmebus_dev *dev, const char *name)
-{
- int err = 0;
-
- dev->ofdev.dev.parent = &ibmebus_bus_device;
- dev->ofdev.dev.bus = &ibmebus_bus_type;
- dev->ofdev.dev.release = ibmebus_dev_release;
-
- dev->ofdev.dev.archdata.of_node = dev->ofdev.node;
- dev->ofdev.dev.archdata.dma_ops = &ibmebus_dma_ops;
- dev->ofdev.dev.archdata.numa_node = of_node_to_nid(dev->ofdev.node);
-
- /* An ibmebusdev is based on a of_device. We have to change the
- * bus type to use our own DMA mapping operations.
- */
- if ((err = of_device_register(&dev->ofdev)) != 0) {
- printk(KERN_ERR "%s: failed to register device (%d).\n",
- __FUNCTION__, err);
- return -ENODEV;
- }
-
- return 0;
-}
-
-static struct ibmebus_dev* __devinit ibmebus_register_device_node(
- struct device_node *dn)
-{
- struct ibmebus_dev *dev;
- int i, len, bus_len;
-
- dev = kzalloc(sizeof(struct ibmebus_dev), GFP_KERNEL);
- if (!dev)
- return ERR_PTR(-ENOMEM);
-
- dev->ofdev.node = of_node_get(dn);
-
- len = strlen(dn->full_name + 1);
- bus_len = min(len, BUS_ID_SIZE - 1);
- memcpy(dev->ofdev.dev.bus_id, dn->full_name + 1
- + (len - bus_len), bus_len);
- for (i = 0; i < bus_len; i++)
- if (dev->ofdev.dev.bus_id[i] == '/')
- dev->ofdev.dev.bus_id[i] = '_';
-
- /* Register with generic device framework. */
- if (ibmebus_register_device_common(dev, dn->name) != 0) {
- kfree(dev);
- return ERR_PTR(-ENODEV);
- }
-
- return dev;
-}
-
-static void ibmebus_probe_of_nodes(char* name)
-{
- struct device_node *dn = NULL;
-
- while ((dn = of_find_node_by_name(dn, name))) {
- if (IS_ERR(ibmebus_register_device_node(dn))) {
- of_node_put(dn);
- return;
- }
- }
-
- of_node_put(dn);
-
- return;
-}
-
-static void ibmebus_add_devices_by_id(struct of_device_id *idt)
-{
- while (strlen(idt->name) > 0) {
- ibmebus_probe_of_nodes(idt->name);
- idt++;
- }
-
- return;
-}
-
-static int ibmebus_match_name(struct device *dev, void *data)
-{
- const struct ibmebus_dev *ebus_dev = to_ibmebus_dev(dev);
- const char *name;
-
- name = of_get_property(ebus_dev->ofdev.node, "name", NULL);
-
- if (name && (strcmp(data, name) == 0))
- return 1;
-
- return 0;
-}
-
-static int ibmebus_unregister_device(struct device *dev)
-{
- of_device_unregister(to_of_device(dev));
-
- return 0;
-}
-
-static void ibmebus_remove_devices_by_id(struct of_device_id *idt)
-{
- struct device *dev;
-
- while (strlen(idt->name) > 0) {
- while ((dev = bus_find_device(&ibmebus_bus_type, NULL,
- (void*)idt->name,
- ibmebus_match_name))) {
- ibmebus_unregister_device(dev);
- }
- idt++;
- }
-
- return;
-}
-
int ibmebus_register_driver(struct ibmebus_driver *drv)
{
- int err = 0;
-
- drv->driver.name = drv->name;
- drv->driver.bus = &ibmebus_bus_type;
- drv->driver.probe = ibmebus_bus_probe;
- drv->driver.remove = ibmebus_bus_remove;
-
- if ((err = driver_register(&drv->driver) != 0))
- return err;
-
- /* remove all supported devices first, in case someone
- * probed them manually before registering the driver */
- ibmebus_remove_devices_by_id(drv->id_table);
- ibmebus_add_devices_by_id(drv->id_table);
-
return 0;
}
EXPORT_SYMBOL(ibmebus_register_driver);

void ibmebus_unregister_driver(struct ibmebus_driver *drv)
{
- driver_unregister(&drv->driver);
- ibmebus_remove_devices_by_id(drv->id_table);
}
EXPORT_SYMBOL(ibmebus_unregister_driver);

@@ -327,23 +159,6 @@ void ibmebus_free_irq(struct ibmebus_dev *dev, u32 ist, void *dev_id)
}
EXPORT_SYMBOL(ibmebus_free_irq);

-static int ibmebus_bus_match(struct device *dev, struct device_driver *drv)
-{
- const struct ibmebus_dev *ebus_dev = to_ibmebus_dev(dev);
- struct ibmebus_driver *ebus_drv = to_ibmebus_driver(drv);
- const struct of_device_id *ids = ebus_drv->id_table;
- const struct of_device_id *found_id;
-
- if (!ids)
- return 0;
-
- found_id = of_match_device(ids, &ebus_dev->ofdev);
- if (found_id)
- return 1;
-
- return 0;
-}
-
static ssize_t name_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -404,7 +219,7 @@ static ssize_t ibmebus_store_probe(struct bus_type *bus,
}

if ((dn = of_find_node_by_path(path))) {
- dev = ibmebus_register_device_node(dn);
+/* dev = ibmebus_register_device_node(dn); */
of_node_put(dn);
rc = IS_ERR(dev) ? PTR_ERR(dev) : count;
} else {
@@ -430,7 +245,7 @@ static ssize_t ibmebus_store_remove(struct bus_type *bus,

if ((dev = bus_find_device(&ibmebus_bus_type, NULL, path,
ibmebus_match_path))) {
- ibmebus_unregister_device(dev);
+/* ibmebus_unregister_device(dev); */

kfree(path);
return count;
@@ -450,8 +265,6 @@ static struct bus_attribute ibmebus_bus_attrs[] = {
};

struct bus_type ibmebus_bus_type = {
- .name = "ibmebus",
- .match = ibmebus_bus_match,
.dev_attrs = ibmebus_dev_attrs,
.bus_attrs = ibmebus_bus_attrs
};
@@ -463,9 +276,9 @@ static int __init ibmebus_bus_init(void)

printk(KERN_INFO "IBM eBus Device Driver\n");

- err = bus_register(&ibmebus_bus_type);
+ err = of_bus_type_init(&ibmebus_bus_type, "ibmebus");
if (err) {
- printk(KERN_ERR ":%s: failed to register IBM eBus.\n",
+ printk(KERN_ERR "%s: failed to register IBM eBus.\n",
__FUNCTION__);
return err;
}
@@ -481,4 +294,4 @@ static int __init ibmebus_bus_init(void)

return 0;
}
-__initcall(ibmebus_bus_init);
+postcore_initcall(ibmebus_bus_init);
--
1.5.2


2007-09-25 12:12:41

by Joachim Fenkes

[permalink] [raw]
Subject: [PATCH 3/5] ibmebus: Add device creation and bus probing based on of_device

The devtree root is now searched for devices matching a built-in whitelist
during boot, so these devices appear on the bus from the beginning. It is
still possible to manually add/remove devices to/from the bus by using the
probe/remove sysfs interface. Also, when a device driver registers itself,
the devtree is matched against its matchlist.

Signed-off-by: Joachim Fenkes <[email protected]>
---
arch/powerpc/kernel/ibmebus.c | 97 ++++++++++++++++++++++++++++++++++-------
1 files changed, 81 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/kernel/ibmebus.c b/arch/powerpc/kernel/ibmebus.c
index cc80f84..c506e0d 100644
--- a/arch/powerpc/kernel/ibmebus.c
+++ b/arch/powerpc/kernel/ibmebus.c
@@ -51,6 +51,15 @@ static struct device ibmebus_bus_device = { /* fake "parent" device */

struct bus_type ibmebus_bus_type;

+/* These devices will automatically be added to the bus during init */
+static struct of_device_id builtin_matches[] = {
+ { .name = "lhca" },
+ { .compatible = "IBM,lhca" },
+ { .name = "lhea" },
+ { .compatible = "IBM,lhea" },
+ {},
+};
+
static void *ibmebus_alloc_coherent(struct device *dev,
size_t size,
dma_addr_t *dma_handle,
@@ -124,6 +133,67 @@ static struct dma_mapping_ops ibmebus_dma_ops = {
.dma_supported = ibmebus_dma_supported,
};

+static int ibmebus_match_path(struct device *dev, void *data)
+{
+ struct device_node *dn = to_of_device(dev)->node;
+ return (dn->full_name &&
+ (strcasecmp((char *)data, dn->full_name) == 0));
+}
+
+static int ibmebus_match_node(struct device *dev, void *data)
+{
+ return to_of_device(dev)->node == data;
+}
+
+static int ibmebus_create_device(struct device_node *dn)
+{
+ struct of_device *dev;
+ int ret;
+
+ dev = of_device_alloc(dn, NULL, &ibmebus_bus_device);
+ if (!dev)
+ return -ENOMEM;
+
+ dev->dev.bus = &ibmebus_bus_type;
+ dev->dev.archdata.dma_ops = &ibmebus_dma_ops;
+
+ ret = of_device_register(dev);
+ if (ret) {
+ of_device_free(dev);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ibmebus_create_devices(const struct of_device_id *matches)
+{
+ struct device_node *root, *child;
+ int ret = 0;
+
+ root = of_find_node_by_path("/");
+
+ for (child = NULL; (child = of_get_next_child(root, child)); ) {
+ if (!of_match_node(matches, child))
+ continue;
+
+ if (bus_find_device(&ibmebus_bus_type, NULL, child,
+ ibmebus_match_node))
+ continue;
+
+ ret = ibmebus_create_device(child);
+ if (ret) {
+ printk(KERN_ERR "%s: failed to create device (%i)",
+ __FUNCTION__, ret);
+ of_node_put(child);
+ break;
+ }
+ }
+
+ of_node_put(root);
+ return ret;
+}
+
int ibmebus_register_driver(struct ibmebus_driver *drv)
{
return 0;
@@ -172,18 +242,6 @@ static struct device_attribute ibmebus_dev_attrs[] = {
__ATTR_NULL
};

-static int ibmebus_match_path(struct device *dev, void *data)
-{
- int rc;
- struct device_node *dn =
- of_node_get(to_ibmebus_dev(dev)->ofdev.node);
-
- rc = (dn->full_name && (strcasecmp((char*)data, dn->full_name) == 0));
-
- of_node_put(dn);
- return rc;
-}
-
static char *ibmebus_chomp(const char *in, size_t count)
{
char *out = (char*)kmalloc(count + 1, GFP_KERNEL);
@@ -202,7 +260,6 @@ static ssize_t ibmebus_store_probe(struct bus_type *bus,
const char *buf, size_t count)
{
struct device_node *dn = NULL;
- struct ibmebus_dev *dev;
char *path;
ssize_t rc;

@@ -219,9 +276,9 @@ static ssize_t ibmebus_store_probe(struct bus_type *bus,
}

if ((dn = of_find_node_by_path(path))) {
-/* dev = ibmebus_register_device_node(dn); */
+ rc = ibmebus_create_device(dn);
of_node_put(dn);
- rc = IS_ERR(dev) ? PTR_ERR(dev) : count;
+ rc = rc ? rc : count;
} else {
printk(KERN_WARNING "%s: no such device node: %s\n",
__FUNCTION__, path);
@@ -245,7 +302,7 @@ static ssize_t ibmebus_store_remove(struct bus_type *bus,

if ((dev = bus_find_device(&ibmebus_bus_type, NULL, path,
ibmebus_match_path))) {
-/* ibmebus_unregister_device(dev); */
+ of_device_unregister(to_of_device(dev));

kfree(path);
return count;
@@ -265,6 +322,7 @@ static struct bus_attribute ibmebus_bus_attrs[] = {
};

struct bus_type ibmebus_bus_type = {
+ .uevent = of_device_uevent,
.dev_attrs = ibmebus_dev_attrs,
.bus_attrs = ibmebus_bus_attrs
};
@@ -292,6 +350,13 @@ static int __init ibmebus_bus_init(void)
return err;
}

+ err = ibmebus_create_devices(builtin_matches);
+ if (err) {
+ device_unregister(&ibmebus_bus_device);
+ bus_unregister(&ibmebus_bus_type);
+ return err;
+ }
+
return 0;
}
postcore_initcall(ibmebus_bus_init);
--
1.5.2


2007-09-25 12:13:20

by Joachim Fenkes

[permalink] [raw]
Subject: [PATCH 4/5] ibmebus: Move to of_device and of_platform_driver, match eHCA and eHEA drivers

Signed-off-by: Joachim Fenkes <[email protected]>
---
drivers/infiniband/hw/ehca/ehca_classes.h | 2 +-
drivers/net/ehea/ehea.h | 2 +-
include/asm-powerpc/ibmebus.h | 38 +++------------
arch/powerpc/kernel/ibmebus.c | 28 ++++++-----
drivers/infiniband/hw/ehca/ehca_eq.c | 6 +-
drivers/infiniband/hw/ehca/ehca_main.c | 32 ++++++------
drivers/net/ehea/ehea_main.c | 72 ++++++++++++++--------------
7 files changed, 79 insertions(+), 101 deletions(-)

diff --git a/drivers/infiniband/hw/ehca/ehca_classes.h b/drivers/infiniband/hw/ehca/ehca_classes.h
index c2edd4c..8ca4dd4 100644
--- a/drivers/infiniband/hw/ehca/ehca_classes.h
+++ b/drivers/infiniband/hw/ehca/ehca_classes.h
@@ -106,7 +106,7 @@ struct ehca_sport {

struct ehca_shca {
struct ib_device ib_device;
- struct ibmebus_dev *ibmebus_dev;
+ struct of_device *ofdev;
u8 num_ports;
int hw_level;
struct list_head shca_list;
diff --git a/drivers/net/ehea/ehea.h b/drivers/net/ehea/ehea.h
index 8d58be5..830a66a 100644
--- a/drivers/net/ehea/ehea.h
+++ b/drivers/net/ehea/ehea.h
@@ -382,7 +382,7 @@ struct ehea_port_res {
#define EHEA_MAX_PORTS 16
struct ehea_adapter {
u64 handle;
- struct ibmebus_dev *ebus_dev;
+ struct of_device *ofdev;
struct ehea_port *port[EHEA_MAX_PORTS];
struct ehea_eq *neq; /* notification event queue */
struct workqueue_struct *ehea_wq;
diff --git a/include/asm-powerpc/ibmebus.h b/include/asm-powerpc/ibmebus.h
index 87d396e..1a9d9ae 100644
--- a/include/asm-powerpc/ibmebus.h
+++ b/include/asm-powerpc/ibmebus.h
@@ -43,42 +43,18 @@
#include <linux/device.h>
#include <linux/interrupt.h>
#include <linux/mod_devicetable.h>
-#include <asm/of_device.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>

extern struct bus_type ibmebus_bus_type;

-struct ibmebus_dev {
- struct of_device ofdev;
-};
+int ibmebus_register_driver(struct of_platform_driver *drv);
+void ibmebus_unregister_driver(struct of_platform_driver *drv);

-struct ibmebus_driver {
- char *name;
- struct of_device_id *id_table;
- int (*probe) (struct ibmebus_dev *dev, const struct of_device_id *id);
- int (*remove) (struct ibmebus_dev *dev);
- struct device_driver driver;
-};
-
-int ibmebus_register_driver(struct ibmebus_driver *drv);
-void ibmebus_unregister_driver(struct ibmebus_driver *drv);
-
-int ibmebus_request_irq(struct ibmebus_dev *dev,
- u32 ist,
- irq_handler_t handler,
- unsigned long irq_flags, const char * devname,
+int ibmebus_request_irq(u32 ist, irq_handler_t handler,
+ unsigned long irq_flags, const char *devname,
void *dev_id);
-void ibmebus_free_irq(struct ibmebus_dev *dev, u32 ist, void *dev_id);
-
-static inline struct ibmebus_driver *to_ibmebus_driver(struct device_driver *drv)
-{
- return container_of(drv, struct ibmebus_driver, driver);
-}
-
-static inline struct ibmebus_dev *to_ibmebus_dev(struct device *dev)
-{
- return container_of(dev, struct ibmebus_dev, ofdev.dev);
-}
-
+void ibmebus_free_irq(u32 ist, void *dev_id);

#endif /* __KERNEL__ */
#endif /* _ASM_IBMEBUS_H */
diff --git a/arch/powerpc/kernel/ibmebus.c b/arch/powerpc/kernel/ibmebus.c
index c506e0d..379472f 100644
--- a/arch/powerpc/kernel/ibmebus.c
+++ b/arch/powerpc/kernel/ibmebus.c
@@ -194,21 +194,26 @@ static int ibmebus_create_devices(const struct of_device_id *matches)
return ret;
}

-int ibmebus_register_driver(struct ibmebus_driver *drv)
+int ibmebus_register_driver(struct of_platform_driver *drv)
{
- return 0;
+ /* If the driver uses devices that ibmebus doesn't know, add them */
+ ibmebus_create_devices(drv->match_table);
+
+ drv->driver.name = drv->name;
+ drv->driver.bus = &ibmebus_bus_type;
+
+ return driver_register(&drv->driver);
}
EXPORT_SYMBOL(ibmebus_register_driver);

-void ibmebus_unregister_driver(struct ibmebus_driver *drv)
+void ibmebus_unregister_driver(struct of_platform_driver *drv)
{
+ driver_unregister(&drv->driver);
}
EXPORT_SYMBOL(ibmebus_unregister_driver);

-int ibmebus_request_irq(struct ibmebus_dev *dev,
- u32 ist,
- irq_handler_t handler,
- unsigned long irq_flags, const char * devname,
+int ibmebus_request_irq(u32 ist, irq_handler_t handler,
+ unsigned long irq_flags, const char *devname,
void *dev_id)
{
unsigned int irq = irq_create_mapping(NULL, ist);
@@ -216,12 +221,11 @@ int ibmebus_request_irq(struct ibmebus_dev *dev,
if (irq == NO_IRQ)
return -EINVAL;

- return request_irq(irq, handler,
- irq_flags, devname, dev_id);
+ return request_irq(irq, handler, irq_flags, devname, dev_id);
}
EXPORT_SYMBOL(ibmebus_request_irq);

-void ibmebus_free_irq(struct ibmebus_dev *dev, u32 ist, void *dev_id)
+void ibmebus_free_irq(u32 ist, void *dev_id)
{
unsigned int irq = irq_find_mapping(NULL, ist);

@@ -232,9 +236,7 @@ EXPORT_SYMBOL(ibmebus_free_irq);
static ssize_t name_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- struct ibmebus_dev *ebus_dev = to_ibmebus_dev(dev);
- const char *name = of_get_property(ebus_dev->ofdev.node, "name", NULL);
- return sprintf(buf, "%s\n", name);
+ return sprintf(buf, "%s\n", to_of_device(dev)->node->name);
}

static struct device_attribute ibmebus_dev_attrs[] = {
diff --git a/drivers/infiniband/hw/ehca/ehca_eq.c b/drivers/infiniband/hw/ehca/ehca_eq.c
index 1d41faa..b4ac617 100644
--- a/drivers/infiniband/hw/ehca/ehca_eq.c
+++ b/drivers/infiniband/hw/ehca/ehca_eq.c
@@ -123,7 +123,7 @@ int ehca_create_eq(struct ehca_shca *shca,

/* register interrupt handlers and initialize work queues */
if (type == EHCA_EQ) {
- ret = ibmebus_request_irq(NULL, eq->ist, ehca_interrupt_eq,
+ ret = ibmebus_request_irq(eq->ist, ehca_interrupt_eq,
IRQF_DISABLED, "ehca_eq",
(void *)shca);
if (ret < 0)
@@ -131,7 +131,7 @@ int ehca_create_eq(struct ehca_shca *shca,

tasklet_init(&eq->interrupt_task, ehca_tasklet_eq, (long)shca);
} else if (type == EHCA_NEQ) {
- ret = ibmebus_request_irq(NULL, eq->ist, ehca_interrupt_neq,
+ ret = ibmebus_request_irq(eq->ist, ehca_interrupt_neq,
IRQF_DISABLED, "ehca_neq",
(void *)shca);
if (ret < 0)
@@ -171,7 +171,7 @@ int ehca_destroy_eq(struct ehca_shca *shca, struct ehca_eq *eq)
u64 h_ret;

spin_lock_irqsave(&eq->spinlock, flags);
- ibmebus_free_irq(NULL, eq->ist, (void *)shca);
+ ibmebus_free_irq(eq->ist, (void *)shca);

h_ret = hipz_h_destroy_eq(shca->ipz_hca_handle, eq);

diff --git a/drivers/infiniband/hw/ehca/ehca_main.c b/drivers/infiniband/hw/ehca/ehca_main.c
index c84e310..8a038b1 100644
--- a/drivers/infiniband/hw/ehca/ehca_main.c
+++ b/drivers/infiniband/hw/ehca/ehca_main.c
@@ -404,7 +404,7 @@ int ehca_init_device(struct ehca_shca *shca)
shca->ib_device.node_type = RDMA_NODE_IB_CA;
shca->ib_device.phys_port_cnt = shca->num_ports;
shca->ib_device.num_comp_vectors = 1;
- shca->ib_device.dma_device = &shca->ibmebus_dev->ofdev.dev;
+ shca->ib_device.dma_device = &shca->ofdev->dev;
shca->ib_device.query_device = ehca_query_device;
shca->ib_device.query_port = ehca_query_port;
shca->ib_device.query_gid = ehca_query_gid;
@@ -658,7 +658,7 @@ static struct attribute_group ehca_dev_attr_grp = {
.attrs = ehca_dev_attrs
};

-static int __devinit ehca_probe(struct ibmebus_dev *dev,
+static int __devinit ehca_probe(struct of_device *dev,
const struct of_device_id *id)
{
struct ehca_shca *shca;
@@ -666,16 +666,16 @@ static int __devinit ehca_probe(struct ibmebus_dev *dev,
struct ib_pd *ibpd;
int ret;

- handle = of_get_property(dev->ofdev.node, "ibm,hca-handle", NULL);
+ handle = of_get_property(dev->node, "ibm,hca-handle", NULL);
if (!handle) {
ehca_gen_err("Cannot get eHCA handle for adapter: %s.",
- dev->ofdev.node->full_name);
+ dev->node->full_name);
return -ENODEV;
}

if (!(*handle)) {
ehca_gen_err("Wrong eHCA handle for adapter: %s.",
- dev->ofdev.node->full_name);
+ dev->node->full_name);
return -ENODEV;
}

@@ -686,9 +686,9 @@ static int __devinit ehca_probe(struct ibmebus_dev *dev,
}
mutex_init(&shca->modify_mutex);

- shca->ibmebus_dev = dev;
+ shca->ofdev = dev;
shca->ipz_hca_handle.handle = *handle;
- dev->ofdev.dev.driver_data = shca;
+ dev->dev.driver_data = shca;

ret = ehca_sense_attributes(shca);
if (ret < 0) {
@@ -764,7 +764,7 @@ static int __devinit ehca_probe(struct ibmebus_dev *dev,
}
}

- ret = sysfs_create_group(&dev->ofdev.dev.kobj, &ehca_dev_attr_grp);
+ ret = sysfs_create_group(&dev->dev.kobj, &ehca_dev_attr_grp);
if (ret) /* only complain; we can live without attributes */
ehca_err(&shca->ib_device,
"Cannot create device attributes ret=%d", ret);
@@ -814,12 +814,12 @@ probe1:
return -EINVAL;
}

-static int __devexit ehca_remove(struct ibmebus_dev *dev)
+static int __devexit ehca_remove(struct of_device *dev)
{
- struct ehca_shca *shca = dev->ofdev.dev.driver_data;
+ struct ehca_shca *shca = dev->dev.driver_data;
int ret;

- sysfs_remove_group(&dev->ofdev.dev.kobj, &ehca_dev_attr_grp);
+ sysfs_remove_group(&dev->dev.kobj, &ehca_dev_attr_grp);

if (ehca_open_aqp1 == 1) {
int i;
@@ -870,11 +870,11 @@ static struct of_device_id ehca_device_table[] =
{},
};

-static struct ibmebus_driver ehca_driver = {
- .name = "ehca",
- .id_table = ehca_device_table,
- .probe = ehca_probe,
- .remove = ehca_remove,
+static struct of_platform_driver ehca_driver = {
+ .name = "ehca",
+ .match_table = ehca_device_table,
+ .probe = ehca_probe,
+ .remove = ehca_remove,
};

void ehca_poll_eqs(unsigned long data)
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index 717b129..a4d4150 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -89,10 +89,10 @@ struct workqueue_struct *ehea_driver_wq;
struct work_struct ehea_rereg_mr_task;


-static int __devinit ehea_probe_adapter(struct ibmebus_dev *dev,
+static int __devinit ehea_probe_adapter(struct of_device *dev,
const struct of_device_id *id);

-static int __devexit ehea_remove(struct ibmebus_dev *dev);
+static int __devexit ehea_remove(struct of_device *dev);

static struct of_device_id ehea_device_table[] = {
{
@@ -102,9 +102,9 @@ static struct of_device_id ehea_device_table[] = {
{},
};

-static struct ibmebus_driver ehea_driver = {
+static struct of_platform_driver ehea_driver = {
.name = "ehea",
- .id_table = ehea_device_table,
+ .match_table = ehea_device_table,
.probe = ehea_probe_adapter,
.remove = ehea_remove,
};
@@ -968,7 +968,7 @@ static int ehea_reg_interrupts(struct net_device *dev)
snprintf(port->int_aff_name, EHEA_IRQ_NAME_SIZE - 1, "%s-aff",
dev->name);

- ret = ibmebus_request_irq(NULL, port->qp_eq->attr.ist1,
+ ret = ibmebus_request_irq(port->qp_eq->attr.ist1,
ehea_qp_aff_irq_handler,
IRQF_DISABLED, port->int_aff_name, port);
if (ret) {
@@ -986,7 +986,7 @@ static int ehea_reg_interrupts(struct net_device *dev)
pr = &port->port_res[i];
snprintf(pr->int_send_name, EHEA_IRQ_NAME_SIZE - 1,
"%s-queue%d", dev->name, i);
- ret = ibmebus_request_irq(NULL, pr->eq->attr.ist1,
+ ret = ibmebus_request_irq(pr->eq->attr.ist1,
ehea_recv_irq_handler,
IRQF_DISABLED, pr->int_send_name,
pr);
@@ -1007,11 +1007,11 @@ out:
out_free_req:
while (--i >= 0) {
u32 ist = port->port_res[i].eq->attr.ist1;
- ibmebus_free_irq(NULL, ist, &port->port_res[i]);
+ ibmebus_free_irq(ist, &port->port_res[i]);
}

out_free_qpeq:
- ibmebus_free_irq(NULL, port->qp_eq->attr.ist1, port);
+ ibmebus_free_irq(port->qp_eq->attr.ist1, port);
i = port->num_def_qps;

goto out;
@@ -1028,14 +1028,14 @@ static void ehea_free_interrupts(struct net_device *dev)

for (i = 0; i < port->num_def_qps + port->num_add_tx_qps; i++) {
pr = &port->port_res[i];
- ibmebus_free_irq(NULL, pr->eq->attr.ist1, pr);
+ ibmebus_free_irq(pr->eq->attr.ist1, pr);
if (netif_msg_intr(port))
ehea_info("free send irq for res %d with handle 0x%X",
i, pr->eq->attr.ist1);
}

/* associated events */
- ibmebus_free_irq(NULL, port->qp_eq->attr.ist1, port);
+ ibmebus_free_irq(port->qp_eq->attr.ist1, port);
if (netif_msg_intr(port))
ehea_info("associated event interrupt for handle 0x%X freed",
port->qp_eq->attr.ist1);
@@ -2548,7 +2548,7 @@ static struct device *ehea_register_port(struct ehea_port *port,
int ret;

port->ofdev.node = of_node_get(dn);
- port->ofdev.dev.parent = &port->adapter->ebus_dev->ofdev.dev;
+ port->ofdev.dev.parent = &port->adapter->ofdev->dev;
port->ofdev.dev.bus = &ibmebus_bus_type;

sprintf(port->ofdev.dev.bus_id, "port%d", port_name_cnt++);
@@ -2729,7 +2729,7 @@ static int ehea_setup_ports(struct ehea_adapter *adapter)
const u32 *dn_log_port_id;
int i = 0;

- lhea_dn = adapter->ebus_dev->ofdev.node;
+ lhea_dn = adapter->ofdev->node;
while ((eth_dn = of_get_next_child(lhea_dn, eth_dn))) {

dn_log_port_id = of_get_property(eth_dn, "ibm,hea-port-no",
@@ -2769,7 +2769,7 @@ static struct device_node *ehea_get_eth_dn(struct ehea_adapter *adapter,
struct device_node *eth_dn = NULL;
const u32 *dn_log_port_id;

- lhea_dn = adapter->ebus_dev->ofdev.node;
+ lhea_dn = adapter->ofdev->node;
while ((eth_dn = of_get_next_child(lhea_dn, eth_dn))) {

dn_log_port_id = of_get_property(eth_dn, "ibm,hea-port-no",
@@ -2875,31 +2875,31 @@ static ssize_t ehea_remove_port(struct device *dev,
static DEVICE_ATTR(probe_port, S_IWUSR, NULL, ehea_probe_port);
static DEVICE_ATTR(remove_port, S_IWUSR, NULL, ehea_remove_port);

-int ehea_create_device_sysfs(struct ibmebus_dev *dev)
+int ehea_create_device_sysfs(struct of_device *dev)
{
- int ret = device_create_file(&dev->ofdev.dev, &dev_attr_probe_port);
+ int ret = device_create_file(&dev->dev, &dev_attr_probe_port);
if (ret)
goto out;

- ret = device_create_file(&dev->ofdev.dev, &dev_attr_remove_port);
+ ret = device_create_file(&dev->dev, &dev_attr_remove_port);
out:
return ret;
}

-void ehea_remove_device_sysfs(struct ibmebus_dev *dev)
+void ehea_remove_device_sysfs(struct of_device *dev)
{
- device_remove_file(&dev->ofdev.dev, &dev_attr_probe_port);
- device_remove_file(&dev->ofdev.dev, &dev_attr_remove_port);
+ device_remove_file(&dev->dev, &dev_attr_probe_port);
+ device_remove_file(&dev->dev, &dev_attr_remove_port);
}

-static int __devinit ehea_probe_adapter(struct ibmebus_dev *dev,
+static int __devinit ehea_probe_adapter(struct of_device *dev,
const struct of_device_id *id)
{
struct ehea_adapter *adapter;
const u64 *adapter_handle;
int ret;

- if (!dev || !dev->ofdev.node) {
+ if (!dev || !dev->node) {
ehea_error("Invalid ibmebus device probed");
return -EINVAL;
}
@@ -2907,36 +2907,36 @@ static int __devinit ehea_probe_adapter(struct ibmebus_dev *dev,
adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
if (!adapter) {
ret = -ENOMEM;
- dev_err(&dev->ofdev.dev, "no mem for ehea_adapter\n");
+ dev_err(&dev->dev, "no mem for ehea_adapter\n");
goto out;
}

list_add(&adapter->list, &adapter_list);

- adapter->ebus_dev = dev;
+ adapter->ofdev = dev;

- adapter_handle = of_get_property(dev->ofdev.node, "ibm,hea-handle",
+ adapter_handle = of_get_property(dev->node, "ibm,hea-handle",
NULL);
if (adapter_handle)
adapter->handle = *adapter_handle;

if (!adapter->handle) {
- dev_err(&dev->ofdev.dev, "failed getting handle for adapter"
- " '%s'\n", dev->ofdev.node->full_name);
+ dev_err(&dev->dev, "failed getting handle for adapter"
+ " '%s'\n", dev->node->full_name);
ret = -ENODEV;
goto out_free_ad;
}

adapter->pd = EHEA_PD_ID;

- dev->ofdev.dev.driver_data = adapter;
+ dev->dev.driver_data = adapter;


/* initialize adapter and ports */
/* get adapter properties */
ret = ehea_sense_adapter_attr(adapter);
if (ret) {
- dev_err(&dev->ofdev.dev, "sense_adapter_attr failed: %d", ret);
+ dev_err(&dev->dev, "sense_adapter_attr failed: %d", ret);
goto out_free_ad;
}

@@ -2944,18 +2944,18 @@ static int __devinit ehea_probe_adapter(struct ibmebus_dev *dev,
EHEA_NEQ, EHEA_MAX_ENTRIES_EQ, 1);
if (!adapter->neq) {
ret = -EIO;
- dev_err(&dev->ofdev.dev, "NEQ creation failed");
+ dev_err(&dev->dev, "NEQ creation failed");
goto out_free_ad;
}

tasklet_init(&adapter->neq_tasklet, ehea_neq_tasklet,
(unsigned long)adapter);

- ret = ibmebus_request_irq(NULL, adapter->neq->attr.ist1,
+ ret = ibmebus_request_irq(adapter->neq->attr.ist1,
ehea_interrupt_neq, IRQF_DISABLED,
"ehea_neq", adapter);
if (ret) {
- dev_err(&dev->ofdev.dev, "requesting NEQ IRQ failed");
+ dev_err(&dev->dev, "requesting NEQ IRQ failed");
goto out_kill_eq;
}

@@ -2971,7 +2971,7 @@ static int __devinit ehea_probe_adapter(struct ibmebus_dev *dev,

ret = ehea_setup_ports(adapter);
if (ret) {
- dev_err(&dev->ofdev.dev, "setup_ports failed");
+ dev_err(&dev->dev, "setup_ports failed");
goto out_rem_dev_sysfs;
}

@@ -2985,7 +2985,7 @@ out_kill_wq:
destroy_workqueue(adapter->ehea_wq);

out_free_irq:
- ibmebus_free_irq(NULL, adapter->neq->attr.ist1, adapter);
+ ibmebus_free_irq(adapter->neq->attr.ist1, adapter);

out_kill_eq:
ehea_destroy_eq(adapter->neq);
@@ -2996,9 +2996,9 @@ out:
return ret;
}

-static int __devexit ehea_remove(struct ibmebus_dev *dev)
+static int __devexit ehea_remove(struct of_device *dev)
{
- struct ehea_adapter *adapter = dev->ofdev.dev.driver_data;
+ struct ehea_adapter *adapter = dev->dev.driver_data;
int i;

for (i = 0; i < EHEA_MAX_PORTS; i++)
@@ -3011,7 +3011,7 @@ static int __devexit ehea_remove(struct ibmebus_dev *dev)

destroy_workqueue(adapter->ehea_wq);

- ibmebus_free_irq(NULL, adapter->neq->attr.ist1, adapter);
+ ibmebus_free_irq(adapter->neq->attr.ist1, adapter);
tasklet_kill(&adapter->neq_tasklet);

ehea_destroy_eq(adapter->neq);
--
1.5.2


2007-09-25 12:13:54

by Joachim Fenkes

[permalink] [raw]
Subject: [PATCH 5/5] ibmebus: More speaking error return code in ibmebus_store_probe()

Signed-off-by: Joachim Fenkes <[email protected]>
---
arch/powerpc/kernel/ibmebus.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/ibmebus.c b/arch/powerpc/kernel/ibmebus.c
index 379472f..8c08a98 100644
--- a/arch/powerpc/kernel/ibmebus.c
+++ b/arch/powerpc/kernel/ibmebus.c
@@ -270,10 +270,10 @@ static ssize_t ibmebus_store_probe(struct bus_type *bus,
return -ENOMEM;

if (bus_find_device(&ibmebus_bus_type, NULL, path,
- ibmebus_match_path)) {
+ ibmebus_match_path)) {
printk(KERN_WARNING "%s: %s has already been probed\n",
__FUNCTION__, path);
- rc = -EINVAL;
+ rc = -EEXIST;
goto out;
}

--
1.5.2


2007-09-25 14:36:59

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 1/5] PowerPC: Move of_device allocation into of_device.[ch]

On Tuesday 25 September 2007, Joachim Fenkes wrote:
> Extract generic of_device allocation code from of_platform_device_create()
> and move it into of_device.[ch], called of_device_alloc(). Also, there's now
> of_device_free() which puts the device node.
>
> Signed-off-by: Joachim Fenkes <[email protected]>
> ---
> ?include/asm-powerpc/of_device.h ? | ? ?4 ++
> ?include/linux/of_device.h ? ? ? ? | ? ?5 ++
> ?arch/powerpc/kernel/of_device.c ? | ? 80 +++++++++++++++++++++++++++++++++++++
> ?arch/powerpc/kernel/of_platform.c | ? 70 +-------------------------------
> ?4 files changed, 91 insertions(+), 68 deletions(-)
>

Sorry I didn't review the patches earlier when you sent them in private.
The patch looks good to me, especially since you did exactly what I
suggested ;-)

Maybe the description should have another sentence in it about what
the change is good for. You have that in the 0/5 mail, but that does
not go into the changelog, so the information gets lost in the process.

Arnd <><

2007-09-25 14:38:30

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/5] ibmebus: Remove bus match/probe/remove functions

On Tuesday 25 September 2007, Joachim Fenkes wrote:
> ibmebus_{,un}register_driver() are replaced by dummy functions because
> ibmebus is temporarily unusable in this transitional state.
>
> Signed-off-by: Joachim Fenkes <[email protected]>
> ---
> ?arch/powerpc/kernel/ibmebus.c | ?199 ++---------------------------------------
> ?1 files changed, 6 insertions(+), 193 deletions(-)
>

Great diffstat!

The description makes it sound like a git-bisect would get broken
by this patch, which should never happen. If the patch indeed
ends up with a broken kernel, it would be better to merge it with
the later patch that fixes the code again.

Arnd <><

2007-09-25 14:47:30

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 3/5] ibmebus: Add device creation and bus probing based on of_device

On Tuesday 25 September 2007, Joachim Fenkes wrote:
> The devtree root is now searched for devices matching a built-in whitelist
> during boot, so these devices appear on the bus from the beginning. It is
> still possible to manually add/remove devices to/from the bus by using the
> probe/remove sysfs interface. Also, when a device driver registers itself,
> the devtree is matched against its matchlist.
>
> Signed-off-by: Joachim Fenkes <[email protected]>
> ---
> arch/powerpc/kernel/ibmebus.c | 97 ++++++++++++++++++++++++++++++++++-------
> 1 files changed, 81 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/kernel/ibmebus.c b/arch/powerpc/kernel/ibmebus.c
> index cc80f84..c506e0d 100644
> --- a/arch/powerpc/kernel/ibmebus.c
> +++ b/arch/powerpc/kernel/ibmebus.c
> @@ -51,6 +51,15 @@ static struct device ibmebus_bus_device = { /* fake "parent" device */
>
> struct bus_type ibmebus_bus_type;
>
> +/* These devices will automatically be added to the bus during init */
> +static struct of_device_id builtin_matches[] = {
> + { .name = "lhca" },
> + { .compatible = "IBM,lhca" },
> + { .name = "lhea" },
> + { .compatible = "IBM,lhea" },
> + {},
> +};
> +

Hmm, do you have devices that only have the matching name property
but not the compatible property? If not, I'd suggest only looking
for compatible, so you have less chance of false positives.

> +static int ibmebus_create_device(struct device_node *dn)
> +{
> + struct of_device *dev;
> + int ret;
> +
> + dev = of_device_alloc(dn, NULL, &ibmebus_bus_device);
> + if (!dev)
> + return -ENOMEM;
> +
> + dev->dev.bus = &ibmebus_bus_type;
> + dev->dev.archdata.dma_ops = &ibmebus_dma_ops;
> +
> + ret = of_device_register(dev);
> + if (ret) {
> + of_device_free(dev);
> + return ret;
> + }
> +
> + return 0;
> +}

nice!

> @@ -219,9 +276,9 @@ static ssize_t ibmebus_store_probe(struct bus_type *bus,
> }
>
> if ((dn = of_find_node_by_path(path))) {
> -/* dev = ibmebus_register_device_node(dn); */
> + rc = ibmebus_create_device(dn);
> of_node_put(dn);
> - rc = IS_ERR(dev) ? PTR_ERR(dev) : count;
> + rc = rc ? rc : count;

the last line looks a bit silly. Maybe instead do

rc = ibmebus_create_device(dn);
of_node_put(dn);
}

kfree(path);
if (rc)
return rc;
return count;
}

2007-09-25 14:51:17

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 4/5] ibmebus: Move to of_device and of_platform_driver, match eHCA and eHEA drivers

On Tuesday 25 September 2007, Joachim Fenkes wrote:
> Signed-off-by: Joachim Fenkes <[email protected]>

This is missing a description, but the patch looks good.

Arnd <><

2007-09-26 08:44:10

by Joachim Fenkes

[permalink] [raw]
Subject: Re: [PATCH 4/5] ibmebus: Move to of_device and of_platform_driver, match eHCA and eHEA drivers

Arnd Bergmann <[email protected]> wrote on 25.09.2007 16:42:40:

> This is missing a description,

The description is "ibmebus: Move to of_device and of_platform_driver,
match eHCA and eHEA drivers" -- I thought that should be enough since the
patch is rather straightforward. I can add a more detailed description,
though. Revised patch will follow.

> but the patch looks good.

Thanks =)

Joachim

2007-09-26 08:58:52

by Joachim Fenkes

[permalink] [raw]
Subject: Re: [PATCH 3/5] ibmebus: Add device creation and bus probing based on of_device

> > +/* These devices will automatically be added to the bus during init
*/
> > +static struct of_device_id builtin_matches[] = {
> > + { .name = "lhca" },
> > + { .compatible = "IBM,lhca" },
> > + { .name = "lhea" },
> > + { .compatible = "IBM,lhea" },
> > + {},
> > +};
> > +
>
> Hmm, do you have devices that only have the matching name property
> but not the compatible property? If not, I'd suggest only looking
> for compatible, so you have less chance of false positives.

If a device that's not an lhca is called "lhca", that's its own fault, i
guess ;) But i concur that looking for the compatible property will
probably suffice.

> > +static int ibmebus_create_device(struct device_node *dn)
> > [...]
>
> nice!

Thanks.

> > - rc = IS_ERR(dev) ? PTR_ERR(dev) : count;
> > + rc = rc ? rc : count;
>
> the last line looks a bit silly. Maybe instead do
>
> rc = ibmebus_create_device(dn);
> of_node_put(dn);
> }
>
> kfree(path);
> if (rc)
> return rc;
> return count;
> }

More code lines? ;) But yes, that looks more like "standard kernel
pattern" - I'll change that.

Joachim

2007-09-26 09:04:32

by Joachim Fenkes

[permalink] [raw]
Subject: Re: [PATCH 2/5] ibmebus: Remove bus match/probe/remove functions

Arnd Bergmann <[email protected]> wrote on 25.09.2007 16:29:51:

> The description makes it sound like a git-bisect would get broken
> by this patch, which should never happen. If the patch indeed
> ends up with a broken kernel, it would be better to merge it with
> the later patch that fixes the code again.

I took extra care to prevent just that from happening. ibmebus will simply
be disabled during the transition (because of {un,}register_driver being
empty dummies), but the kernel builds and boots without problems. So
unless you're trying to find an ibmebus-based problem, git bisect will be
fine. I'll repost 2/5 with an updated description.

I split the ibmebus rework into three patches because the merged patch was
impossible to read. Makes reviewing easier.

Joachim

2007-09-26 09:08:28

by Joachim Fenkes

[permalink] [raw]
Subject: Re: [PATCH 1/5] PowerPC: Move of_device allocation into of_device.[ch]

Arnd Bergmann <[email protected]> wrote on 25.09.2007 16:27:57:

> The patch looks good to me, especially since you did exactly what I
> suggested ;-)

Yes, our discussions were very productive. Thanks and sorry I forgot to
mention your input.

> Maybe the description should have another sentence in it about what
> the change is good for. You have that in the 0/5 mail, but that does
> not go into the changelog, so the information gets lost in the process.

Can do. New patch coming right up!

Joachim