mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/5 v2] sandbox fix + bus
@ 2012-09-19 14:43 Jean-Christophe PLAGNIOL-VILLARD
  2012-09-19 14:47 ` [PATCH 1/5] sandbox: check only image option in the second getopt Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 8+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-09-19 14:43 UTC (permalink / raw)
  To: barebox

HI,

	v2:
	 - on the sandbox we register device before barebox started
	   this is working today as nothing need to be initialized before
	   regisering a device. But as I attached device to bus this will no
	   be the case anymore so fix it

	 - net/tap fix allocated data to be set to 0

The following changes since commit 4f2aa8ffb124a7f40da2a5bc1a2c225554b24bdd:

  Merge branch 'for-next/testing-menu' into next (2012-09-17 12:15:53 +0200)

are available in the git repository at:


  git://git.jcrosoft.org/barebox.git delivery/bus

for you to fetch changes up to 8802c029970c7ac43a1544c1fe17c114e69aa573:

  driver: search device and driver based on the bus instead of all (2012-09-19 22:44:16 +0800)

----------------------------------------------------------------
Jean-Christophe PLAGNIOL-VILLARD (5):
      sandbox: check only image option in the second getopt
      sandbox: do not register device before base is started
      net/tap: use xzalloc to allocate data
      driver: register bus
      driver: search device and driver based on the bus instead of all

 arch/sandbox/board/Makefile   |    1 +
 arch/sandbox/board/console.c  |    2 +-
 arch/sandbox/board/devices.c  |   35 +++++++++++++++++++++++++++++++++++
 arch/sandbox/board/hostfile.c |   19 +++++++++++++++++--
 arch/sandbox/os/common.c      |   12 +-----------
 drivers/base/Makefile         |    1 +
 drivers/base/bus.c            |   37 +++++++++++++++++++++++++++++++++++++
 drivers/base/driver.c         |   11 ++++++-----
 drivers/base/platform.c       |   14 ++++----------
 drivers/i2c/i2c.c             |    7 +++++++
 drivers/net/phy/phy.c         |    7 +++++++
 drivers/net/tap.c             |    4 ++--
 drivers/spi/spi.c             |    6 ++++++
 drivers/usb/core/usb.c        |    5 +++++
 fs/fs.c                       |    6 ++++++
 include/driver.h              |   20 ++++++++++++++++++++
 16 files changed, 156 insertions(+), 31 deletions(-)
 create mode 100644 arch/sandbox/board/devices.c
 create mode 100644 drivers/base/bus.c

Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 1/5] sandbox: check only image option in the second getopt
  2012-09-19 14:43 [PATCH 0/5 v2] sandbox fix + bus Jean-Christophe PLAGNIOL-VILLARD
@ 2012-09-19 14:47 ` Jean-Christophe PLAGNIOL-VILLARD
  2012-09-19 14:47   ` [PATCH 2/5] sandbox: do not register device before base is started Jean-Christophe PLAGNIOL-VILLARD
                     ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-09-19 14:47 UTC (permalink / raw)
  To: barebox

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 arch/sandbox/os/common.c |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/arch/sandbox/os/common.c b/arch/sandbox/os/common.c
index ef5ebdf..0e8e60d 100644
--- a/arch/sandbox/os/common.c
+++ b/arch/sandbox/os/common.c
@@ -358,10 +358,6 @@ int main(int argc, char *argv[])
 			break;
 
 		switch (opt) {
-		case 'h':
-			break;
-		case 'm':
-			break;
 		case 'i':
 			sprintf(str, "fd%d", fdno);
 			ret = add_image(optarg, str);
@@ -369,14 +365,8 @@ int main(int argc, char *argv[])
 				exit(1);
 			fdno++;
 			break;
-		case 'e':
-			break;
-		case 'O':
-			break;
-		case 'I':
-			break;
 		default:
-			exit(1);
+			break;
 		}
 	}
 
-- 
1.7.10.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 2/5] sandbox: do not register device before base is started
  2012-09-19 14:47 ` [PATCH 1/5] sandbox: check only image option in the second getopt Jean-Christophe PLAGNIOL-VILLARD
@ 2012-09-19 14:47   ` Jean-Christophe PLAGNIOL-VILLARD
  2012-09-19 17:41     ` Sascha Hauer
  2012-09-19 14:47   ` [PATCH 3/5] net/tap: use xzalloc to allocate data Jean-Christophe PLAGNIOL-VILLARD
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-09-19 14:47 UTC (permalink / raw)
  To: barebox

This will crash when use registered bus with device registered to it.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 arch/sandbox/board/Makefile   |    1 +
 arch/sandbox/board/console.c  |    2 +-
 arch/sandbox/board/devices.c  |   35 +++++++++++++++++++++++++++++++++++
 arch/sandbox/board/hostfile.c |   19 +++++++++++++++++--
 4 files changed, 54 insertions(+), 3 deletions(-)
 create mode 100644 arch/sandbox/board/devices.c

diff --git a/arch/sandbox/board/Makefile b/arch/sandbox/board/Makefile
index 266c3a3..5104f5c 100644
--- a/arch/sandbox/board/Makefile
+++ b/arch/sandbox/board/Makefile
@@ -2,5 +2,6 @@ obj-y += board.o
 obj-y += clock.o
 obj-y += hostfile.o
 obj-y += console.o
+obj-y += devices.o
 
 extra-y += barebox.lds
diff --git a/arch/sandbox/board/console.c b/arch/sandbox/board/console.c
index 18b63e1..b0afa54 100644
--- a/arch/sandbox/board/console.c
+++ b/arch/sandbox/board/console.c
@@ -44,6 +44,6 @@ int barebox_register_console(char *name, int stdinfd, int stdoutfd)
 	data->stdoutfd = stdoutfd;
 	data->stdinfd  = stdinfd;
 
-	return register_device(dev);
+	return sandbox_add_device(dev);
 }
 
diff --git a/arch/sandbox/board/devices.c b/arch/sandbox/board/devices.c
new file mode 100644
index 0000000..d3fad78
--- /dev/null
+++ b/arch/sandbox/board/devices.c
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2012 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
+ *
+ * Under GPLv2 only
+ */
+
+#include <common.h>
+#include <driver.h>
+#include <init.h>
+
+static LIST_HEAD(sandbox_device_list);
+
+int sandbox_add_device(struct device_d *dev)
+{
+	if (!dev)
+		return -EINVAL;
+
+	list_add(&dev->list, &sandbox_device_list);
+	return 0;
+}
+
+static int sandbox_device_init(void)
+{
+	struct device_d *dev, *tmp;
+
+	list_for_each_entry_safe(dev, tmp, &sandbox_device_list, list) {
+		/* reset the list before register for real */
+		dev->list.prev = NULL;
+		dev->list.next = NULL;
+		register_device(dev);
+	}
+
+	return 0;
+}
+postcore_initcall(sandbox_device_init);
diff --git a/arch/sandbox/board/hostfile.c b/arch/sandbox/board/hostfile.c
index f5acc78..2cc7c1f 100644
--- a/arch/sandbox/board/hostfile.c
+++ b/arch/sandbox/board/hostfile.c
@@ -99,7 +99,22 @@ device_initcall(hf_init);
 
 int barebox_register_filedev(struct hf_platform_data *hf)
 {
-	return !add_generic_device("hostfile", DEVICE_ID_DYNAMIC, NULL, hf->base, hf->size,
-			   IORESOURCE_MEM, hf);
+	struct device_d *dev;
+	struct resource *res;
+
+	dev = xzalloc(sizeof(*dev));
+	strcpy(dev->name, "hostfile");
+	dev->id = DEVICE_ID_DYNAMIC;
+	dev->platform_data = hf;
+
+	res = xzalloc(sizeof(struct resource));
+	res[0].start = hf->base;
+	res[0].end = hf->base + hf->size - 1;
+	res[0].flags = IORESOURCE_MEM;
+
+	dev->resource = res;
+	dev->num_resources = 1;
+
+	return sandbox_add_device(dev);
 }
 
-- 
1.7.10.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 3/5] net/tap: use xzalloc to allocate data
  2012-09-19 14:47 ` [PATCH 1/5] sandbox: check only image option in the second getopt Jean-Christophe PLAGNIOL-VILLARD
  2012-09-19 14:47   ` [PATCH 2/5] sandbox: do not register device before base is started Jean-Christophe PLAGNIOL-VILLARD
@ 2012-09-19 14:47   ` Jean-Christophe PLAGNIOL-VILLARD
  2012-09-19 14:47   ` [PATCH 4/5] driver: register bus Jean-Christophe PLAGNIOL-VILLARD
  2012-09-19 14:47   ` [PATCH 5/5] driver: search device and driver based on the bus instead of all Jean-Christophe PLAGNIOL-VILLARD
  3 siblings, 0 replies; 8+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-09-19 14:47 UTC (permalink / raw)
  To: barebox

this will ensure the data are set to 0 (list as example)

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 drivers/net/tap.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index 5c3a1e3..d5c7031 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -76,7 +76,7 @@ int tap_probe(struct device_d *dev)
 	struct tap_priv *priv;
 	int ret = 0;
 
-	priv = xmalloc(sizeof(struct tap_priv));
+	priv = xzalloc(sizeof(struct tap_priv));
 	priv->name = "barebox";
 
 	priv->fd = tap_alloc(priv->name);
@@ -85,7 +85,7 @@ int tap_probe(struct device_d *dev)
 		goto out;
 	}
 
-	edev = xmalloc(sizeof(struct eth_device));
+	edev = xzalloc(sizeof(struct eth_device));
 	edev->priv = priv;
 	edev->parent = dev;
 
-- 
1.7.10.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 4/5] driver: register bus
  2012-09-19 14:47 ` [PATCH 1/5] sandbox: check only image option in the second getopt Jean-Christophe PLAGNIOL-VILLARD
  2012-09-19 14:47   ` [PATCH 2/5] sandbox: do not register device before base is started Jean-Christophe PLAGNIOL-VILLARD
  2012-09-19 14:47   ` [PATCH 3/5] net/tap: use xzalloc to allocate data Jean-Christophe PLAGNIOL-VILLARD
@ 2012-09-19 14:47   ` Jean-Christophe PLAGNIOL-VILLARD
  2012-09-19 14:47   ` [PATCH 5/5] driver: search device and driver based on the bus instead of all Jean-Christophe PLAGNIOL-VILLARD
  3 siblings, 0 replies; 8+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-09-19 14:47 UTC (permalink / raw)
  To: barebox

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 drivers/base/Makefile   |    1 +
 drivers/base/bus.c      |   37 +++++++++++++++++++++++++++++++++++++
 drivers/base/platform.c |   14 ++++----------
 drivers/i2c/i2c.c       |    7 +++++++
 drivers/net/phy/phy.c   |    7 +++++++
 drivers/spi/spi.c       |    6 ++++++
 drivers/usb/core/usb.c  |    5 +++++
 fs/fs.c                 |    6 ++++++
 include/driver.h        |    8 ++++++++
 9 files changed, 81 insertions(+), 10 deletions(-)
 create mode 100644 drivers/base/bus.c

diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 957ca5a..e1f1c7a 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -1,3 +1,4 @@
+obj-y	+= bus.o
 obj-y	+= driver.o
 obj-y	+= platform.o
 obj-y	+= resource.o
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
new file mode 100644
index 0000000..1dd139f
--- /dev/null
+++ b/drivers/base/bus.c
@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2012 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
+ *
+ * Under GPLv2
+ */
+
+#include <common.h>
+#include <driver.h>
+#include <errno.h>
+
+LIST_HEAD(bus_list);
+EXPORT_SYMBOL(bus_list);
+
+struct bus_type *get_bus_by_name(const char *name)
+{
+	struct bus_type *bus;
+
+	for_each_bus(bus) {
+		if(!strcmp(bus->name, name))
+			return bus;
+	}
+
+	return NULL;
+}
+
+int bus_register(struct bus_type *bus)
+{
+	if (get_bus_by_name(bus->name))
+		return -EEXIST;
+
+	INIT_LIST_HEAD(&bus->device_list);
+	INIT_LIST_HEAD(&bus->driver_list);
+
+	list_add_tail(&bus->list, &bus_list);
+
+	return 0;
+}
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index afeee05..d3021ab 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -19,6 +19,7 @@
 #include <common.h>
 #include <driver.h>
 #include <errno.h>
+#include <init.h>
 
 static int platform_match(struct device_d *dev, struct driver_d *drv)
 {
@@ -61,15 +62,8 @@ struct bus_type platform_bus = {
 	.remove = platform_remove,
 };
 
-#if 0
-LIST_HEAD(bus_list);
-EXPORT_SYMBOL(bus_list);
-
-int bus_register(struct bus_type *bus)
+static int plarform_init(void)
 {
-	list_add_tail(&bus->list, &bus_list);
-
-	return 0;
+	return bus_register(&platform_bus);
 }
-#endif
-
+pure_initcall(plarform_init);
diff --git a/drivers/i2c/i2c.c b/drivers/i2c/i2c.c
index 555722b..27fd256 100644
--- a/drivers/i2c/i2c.c
+++ b/drivers/i2c/i2c.c
@@ -21,6 +21,7 @@
 #include <errno.h>
 #include <malloc.h>
 #include <xfuncs.h>
+#include <init.h>
 
 #include <i2c/i2c.h>
 
@@ -395,3 +396,9 @@ struct bus_type i2c_bus = {
 	.probe = i2c_probe,
 	.remove = i2c_remove,
 };
+
+static int i2c_bus_init(void)
+{
+	return bus_register(&i2c_bus);
+}
+pure_initcall(i2c_bus_init);
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index bfebe3b..424de44 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -22,6 +22,7 @@
 #include <net.h>
 #include <malloc.h>
 #include <miidev.h>
+#include <init.h>
 #include <linux/phy.h>
 #include <linux/err.h>
 
@@ -685,3 +686,9 @@ int phy_driver_register(struct phy_driver *phydrv)
 
 	return register_driver(&phydrv->drv);
 }
+
+static int phy_bus_init(void)
+{
+	return bus_register(&phy_bustype);
+}
+pure_initcall(phy_bus_init);
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 99a5069..17aae93 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -291,3 +291,9 @@ struct bus_type spi_bus = {
 	.probe = spi_probe,
 	.remove = spi_remove,
 };
+
+static int spi_bus_init(void)
+{
+	return bus_register(&spi_bus);
+}
+pure_initcall(spi_bus_init);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 662705e..9dc931b 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1418,3 +1418,8 @@ struct bus_type usb_bus_type = {
 	.remove	= usb_remove,
 };
 
+static int usb_bus_init(void)
+{
+	return bus_register(&usb_bus_type);
+}
+pure_initcall(usb_bus_init);
diff --git a/fs/fs.c b/fs/fs.c
index bbee124..b9a1f17 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -1138,6 +1138,12 @@ struct bus_type fs_bus = {
 	.remove = fs_remove,
 };
 
+static int fs_bus_init(void)
+{
+	return bus_register(&fs_bus);
+}
+pure_initcall(fs_bus_init);
+
 int register_fs_driver(struct fs_driver_d *fsdrv)
 {
 	fsdrv->drv.bus = &fs_bus;
diff --git a/include/driver.h b/include/driver.h
index 7c31c24..f89bfff 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -379,6 +379,14 @@ struct bus_type {
 	struct list_head list;
 };
 
+int bus_register(struct bus_type *bus);
+
+extern struct list_head bus_list;
+
+/* Iterate over all buses
+ */
+#define for_each_bus(bus) list_for_each_entry(bus, &bus_list, list)
+
 extern struct bus_type platform_bus;
 
 struct file_operations {
-- 
1.7.10.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 5/5] driver: search device and driver based on the bus instead of all
  2012-09-19 14:47 ` [PATCH 1/5] sandbox: check only image option in the second getopt Jean-Christophe PLAGNIOL-VILLARD
                     ` (2 preceding siblings ...)
  2012-09-19 14:47   ` [PATCH 4/5] driver: register bus Jean-Christophe PLAGNIOL-VILLARD
@ 2012-09-19 14:47   ` Jean-Christophe PLAGNIOL-VILLARD
  3 siblings, 0 replies; 8+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-09-19 14:47 UTC (permalink / raw)
  To: barebox

This will allow reduce the number of driver and device to search on.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 drivers/base/driver.c |   11 ++++++-----
 include/driver.h      |   12 ++++++++++++
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 18ac70c..5d25a3c 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -82,8 +82,6 @@ static int match(struct driver_d *drv, struct device_d *dev)
 
 	dev->driver = drv;
 
-	if (dev->bus != drv->bus)
-		goto err_out;
 	if (dev->bus->match(dev, drv))
 		goto err_out;
 	if (dev->bus->probe(dev))
@@ -119,7 +117,7 @@ int register_device(struct device_d *new_device)
 	if (new_device->bus == &platform_bus && new_device->resource) {
 		struct device_d *dev;
 
-		for_each_device(dev) {
+		for_each_bus_device(new_device->bus, dev) {
 			if (!dev->resource)
 				continue;
 			if (dev->resource->start == new_device->resource->start) {
@@ -129,12 +127,13 @@ int register_device(struct device_d *new_device)
 	}
 
 	list_add_tail(&new_device->list, &device_list);
+	list_add_tail(&new_device->bus_list, &new_device->bus->device_list);
 	INIT_LIST_HEAD(&new_device->children);
 	INIT_LIST_HEAD(&new_device->cdevs);
 	INIT_LIST_HEAD(&new_device->parameters);
 	INIT_LIST_HEAD(&new_device->active);
 
-	for_each_driver(drv) {
+	for_each_bus_driver(new_device->bus, drv) {
 		if (!match(drv, new_device))
 			break;
 	}
@@ -166,6 +165,7 @@ int unregister_device(struct device_d *old_dev)
 	}
 
 	list_del(&old_dev->list);
+	list_del(&old_dev->bus_list);
 	list_del(&old_dev->active);
 
 	/* remove device from parents child list */
@@ -219,13 +219,14 @@ int register_driver(struct driver_d *drv)
 	}
 
 	list_add_tail(&drv->list, &driver_list);
+	list_add_tail(&drv->bus_list, &drv->bus->driver_list);
 
 	if (!drv->info)
 		drv->info = noinfo;
 	if (!drv->shortinfo)
 		drv->shortinfo = noshortinfo;
 
-	for_each_device(dev)
+	for_each_bus_device(drv->bus, dev)
 		match(drv, dev);
 
 	return 0;
diff --git a/include/driver.h b/include/driver.h
index f89bfff..6a182d8 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -88,6 +88,7 @@ struct device_d {
 	struct driver_d *driver; /*! The driver for this device */
 
 	struct list_head list;     /* The list of all devices */
+	struct list_head bus_list; /* our bus            */
 	struct list_head children; /* our children            */
 	struct list_head sibling;
 	struct list_head active;   /* The list of all devices which have a driver */
@@ -115,6 +116,7 @@ struct driver_d {
 	const char *name;
 
 	struct list_head list;
+	struct list_head bus_list; /* our bus            */
 
 	/*! Called if an instance of a device is found */
 	int     (*probe) (struct device_d *);
@@ -377,6 +379,8 @@ struct bus_type {
 	void (*remove)(struct device_d *dev);
 
 	struct list_head list;
+	struct list_head device_list;
+	struct list_head driver_list;
 };
 
 int bus_register(struct bus_type *bus);
@@ -387,6 +391,14 @@ extern struct list_head bus_list;
  */
 #define for_each_bus(bus) list_for_each_entry(bus, &bus_list, list)
 
+/* Iterate over all devices of a bus
+ */
+#define for_each_bus_device(bus, dev) list_for_each_entry(dev, &bus->device_list, bus_list)
+
+/* Iterate over all drivers of a bus
+ */
+#define for_each_bus_driver(bus, drv) list_for_each_entry(drv, &bus->driver_list, bus_list)
+
 extern struct bus_type platform_bus;
 
 struct file_operations {
-- 
1.7.10.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 2/5] sandbox: do not register device before base is started
  2012-09-19 14:47   ` [PATCH 2/5] sandbox: do not register device before base is started Jean-Christophe PLAGNIOL-VILLARD
@ 2012-09-19 17:41     ` Sascha Hauer
  2012-09-20  5:35       ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 8+ messages in thread
From: Sascha Hauer @ 2012-09-19 17:41 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

On Wed, Sep 19, 2012 at 04:47:55PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> This will crash when use registered bus with device registered to it.
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  arch/sandbox/board/Makefile   |    1 +
>  arch/sandbox/board/console.c  |    2 +-
>  arch/sandbox/board/devices.c  |   35 +++++++++++++++++++++++++++++++++++
>  arch/sandbox/board/hostfile.c |   19 +++++++++++++++++--
>  4 files changed, 54 insertions(+), 3 deletions(-)
>  create mode 100644 arch/sandbox/board/devices.c
> 
> diff --git a/arch/sandbox/board/Makefile b/arch/sandbox/board/Makefile
> index 266c3a3..5104f5c 100644
> --- a/arch/sandbox/board/Makefile
> +++ b/arch/sandbox/board/Makefile
> @@ -2,5 +2,6 @@ obj-y += board.o
>  obj-y += clock.o
>  obj-y += hostfile.o
>  obj-y += console.o
> +obj-y += devices.o
>  
>  extra-y += barebox.lds
> diff --git a/arch/sandbox/board/console.c b/arch/sandbox/board/console.c
> index 18b63e1..b0afa54 100644
> --- a/arch/sandbox/board/console.c
> +++ b/arch/sandbox/board/console.c
> @@ -44,6 +44,6 @@ int barebox_register_console(char *name, int stdinfd, int stdoutfd)
>  	data->stdoutfd = stdoutfd;
>  	data->stdinfd  = stdinfd;
>  
> -	return register_device(dev);
> +	return sandbox_add_device(dev);
>  }
>  
> diff --git a/arch/sandbox/board/devices.c b/arch/sandbox/board/devices.c
> new file mode 100644
> index 0000000..d3fad78
> --- /dev/null
> +++ b/arch/sandbox/board/devices.c
> @@ -0,0 +1,35 @@
> +/*
> + * Copyright (c) 2012 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> + *
> + * Under GPLv2 only
> + */
> +
> +#include <common.h>
> +#include <driver.h>
> +#include <init.h>
> +
> +static LIST_HEAD(sandbox_device_list);
> +
> +int sandbox_add_device(struct device_d *dev)
> +{
> +	if (!dev)
> +		return -EINVAL;

Can we please agree that everyone registering NULL pointers deserves a
nice crash? I do not see a reason to check this. All these kinds of
checks do is cover bugs and make tracking them down harder.

Sascha



-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 2/5] sandbox: do not register device before base is started
  2012-09-19 17:41     ` Sascha Hauer
@ 2012-09-20  5:35       ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 8+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-09-20  5:35 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On 19:41 Wed 19 Sep     , Sascha Hauer wrote:
> On Wed, Sep 19, 2012 at 04:47:55PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > This will crash when use registered bus with device registered to it.
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > ---
> >  arch/sandbox/board/Makefile   |    1 +
> >  arch/sandbox/board/console.c  |    2 +-
> >  arch/sandbox/board/devices.c  |   35 +++++++++++++++++++++++++++++++++++
> >  arch/sandbox/board/hostfile.c |   19 +++++++++++++++++--
> >  4 files changed, 54 insertions(+), 3 deletions(-)
> >  create mode 100644 arch/sandbox/board/devices.c
> > 
> > diff --git a/arch/sandbox/board/Makefile b/arch/sandbox/board/Makefile
> > index 266c3a3..5104f5c 100644
> > --- a/arch/sandbox/board/Makefile
> > +++ b/arch/sandbox/board/Makefile
> > @@ -2,5 +2,6 @@ obj-y += board.o
> >  obj-y += clock.o
> >  obj-y += hostfile.o
> >  obj-y += console.o
> > +obj-y += devices.o
> >  
> >  extra-y += barebox.lds
> > diff --git a/arch/sandbox/board/console.c b/arch/sandbox/board/console.c
> > index 18b63e1..b0afa54 100644
> > --- a/arch/sandbox/board/console.c
> > +++ b/arch/sandbox/board/console.c
> > @@ -44,6 +44,6 @@ int barebox_register_console(char *name, int stdinfd, int stdoutfd)
> >  	data->stdoutfd = stdoutfd;
> >  	data->stdinfd  = stdinfd;
> >  
> > -	return register_device(dev);
> > +	return sandbox_add_device(dev);
> >  }
> >  
> > diff --git a/arch/sandbox/board/devices.c b/arch/sandbox/board/devices.c
> > new file mode 100644
> > index 0000000..d3fad78
> > --- /dev/null
> > +++ b/arch/sandbox/board/devices.c
> > @@ -0,0 +1,35 @@
> > +/*
> > + * Copyright (c) 2012 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > + *
> > + * Under GPLv2 only
> > + */
> > +
> > +#include <common.h>
> > +#include <driver.h>
> > +#include <init.h>
> > +
> > +static LIST_HEAD(sandbox_device_list);
> > +
> > +int sandbox_add_device(struct device_d *dev)
> > +{
> > +	if (!dev)
> > +		return -EINVAL;
> 
> Can we please agree that everyone registering NULL pointers deserves a
> nice crash? I do not see a reason to check this. All these kinds of
> checks do is cover bugs and make tracking them down harder.
I agree but not on the crash we need to use BUG_ON

Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

end of thread, other threads:[~2012-09-20  5:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-19 14:43 [PATCH 0/5 v2] sandbox fix + bus Jean-Christophe PLAGNIOL-VILLARD
2012-09-19 14:47 ` [PATCH 1/5] sandbox: check only image option in the second getopt Jean-Christophe PLAGNIOL-VILLARD
2012-09-19 14:47   ` [PATCH 2/5] sandbox: do not register device before base is started Jean-Christophe PLAGNIOL-VILLARD
2012-09-19 17:41     ` Sascha Hauer
2012-09-20  5:35       ` Jean-Christophe PLAGNIOL-VILLARD
2012-09-19 14:47   ` [PATCH 3/5] net/tap: use xzalloc to allocate data Jean-Christophe PLAGNIOL-VILLARD
2012-09-19 14:47   ` [PATCH 4/5] driver: register bus Jean-Christophe PLAGNIOL-VILLARD
2012-09-19 14:47   ` [PATCH 5/5] driver: search device and driver based on the bus instead of all Jean-Christophe PLAGNIOL-VILLARD

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox