diff mbox

[U-Boot,v2,26/29] dm: Add child_pre_probe() and child_post_remove() methods

Message ID 1404877099-7314-27-git-send-email-sjg@chromium.org
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

Simon Glass July 9, 2014, 3:38 a.m. UTC
Some devices (particularly bus devices) must track their children, knowing
when a new child is added so that it can be set up for communication on the
bus.

Add a child_pre_probe() method to provide this feature, and a corresponding
child_post_remove() method.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 doc/driver-model/README.txt | 68 ++++++++++++++++++++++++++++++++++++++++++++-
 drivers/core/device.c       | 16 ++++++++++-
 include/dm/device.h         |  6 ++++
 include/dm/test.h           |  4 +++
 test/dm/bus.c               | 68 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 160 insertions(+), 2 deletions(-)

Comments

Pavel Herrmann July 15, 2014, 8:26 a.m. UTC | #1
Hi

On Tuesday 08 of July 2014 21:38:16 Simon Glass wrote:
> ...
> +
> +Note that the information that controls this behaviour is in the bus's
> +driver, not the child's. In fact it is possible that child has no knowledge
> +that it is connected to a bus. The same child device may even be used on
> two +different bus types. As an example. the 'flash' device shown above may
> also +be connected on a SATA bus or standalone with no bus:
> +
> +   xhci_usb (UCLASS_USB)
> +      flash (UCLASS_FLASH_STORAGE)  - parent data/methods defined by USB
> bus +
> +   sata (UCLASS_SATA)
> +      flash (UCLASS_FLASH_STORAGE)  - parent data/methods defined by SATA
> bus +
> +   flash (UCLASS_FLASH_STORAGE)  - no parent data/methods (not on a bus)

this is not the best example, since the driver actually needs to have an idea 
what parent bus it is connected to, as it should use the parents driver.ops to 
communicate with the device.

the better (more realistic) version would show that the same device would 
operate under various xhci_usb, ohci_usb and ehci_usb busses, which might very 
well have different parent_priv structure (for example, ohci_usb would probably 
not store maximum speed supported by the device, since the bus only has the 
basic one)

as a side note, flash is a bit tricky here, since USB and SATA do not provide 
you with "flash-like" interface even for flash-based devices, but instead have a 
disk-like interface, which is simpler (does not give you the ability to 
control bad block management, among other things).


regards
Pavel Herrmann
Simon Glass July 17, 2014, 5:41 a.m. UTC | #2
Hi Pavel,

On 15 July 2014 02:26, Pavel Herrmann <morpheus.ibis@gmail.com> wrote:
> Hi
>
> On Tuesday 08 of July 2014 21:38:16 Simon Glass wrote:
>> ...
>> +
>> +Note that the information that controls this behaviour is in the bus's
>> +driver, not the child's. In fact it is possible that child has no knowledge
>> +that it is connected to a bus. The same child device may even be used on
>> two +different bus types. As an example. the 'flash' device shown above may
>> also +be connected on a SATA bus or standalone with no bus:
>> +
>> +   xhci_usb (UCLASS_USB)
>> +      flash (UCLASS_FLASH_STORAGE)  - parent data/methods defined by USB
>> bus +
>> +   sata (UCLASS_SATA)
>> +      flash (UCLASS_FLASH_STORAGE)  - parent data/methods defined by SATA
>> bus +
>> +   flash (UCLASS_FLASH_STORAGE)  - no parent data/methods (not on a bus)
>
> this is not the best example, since the driver actually needs to have an idea
> what parent bus it is connected to, as it should use the parents driver.ops to
> communicate with the device.

Yes, it's not a good example.

>
> the better (more realistic) version would show that the same device would
> operate under various xhci_usb, ohci_usb and ehci_usb busses, which might very
> well have different parent_priv structure (for example, ohci_usb would probably
> not store maximum speed supported by the device, since the bus only has the
> basic one)

They still use the parent driver ops, but now they are all the same?
Do you mean that the uclass would be the same for each bus?

>
> as a side note, flash is a bit tricky here, since USB and SATA do not provide
> you with "flash-like" interface even for flash-based devices, but instead have a
> disk-like interface, which is simpler (does not give you the ability to
> control bad block management, among other things).

Another reason why the example is poor - I was more thinking of flash
as a disk than a raw NAND device.

Regards,
Simon
Pavel Herrmann July 17, 2014, 7:09 a.m. UTC | #3
On Wednesday 16 of July 2014 23:41:57 Simon Glass wrote:
> Hi Pavel,
> 
> On 15 July 2014 02:26, Pavel Herrmann <morpheus.ibis@gmail.com> wrote:
> > Hi
> > 
> > On Tuesday 08 of July 2014 21:38:16 Simon Glass wrote:
> >> ...
> >> +
> >> +Note that the information that controls this behaviour is in the bus's
> >> +driver, not the child's. In fact it is possible that child has no
> >> knowledge +that it is connected to a bus. The same child device may even
> >> be used on two +different bus types. As an example. the 'flash' device
> >> shown above may also +be connected on a SATA bus or standalone with no
> >> bus:
> >> +
> >> +   xhci_usb (UCLASS_USB)
> >> +      flash (UCLASS_FLASH_STORAGE)  - parent data/methods defined by USB
> >> bus +
> >> +   sata (UCLASS_SATA)
> >> +      flash (UCLASS_FLASH_STORAGE)  - parent data/methods defined by
> >> SATA
> >> bus +
> >> +   flash (UCLASS_FLASH_STORAGE)  - no parent data/methods (not on a bus)
> > 
> > this is not the best example, since the driver actually needs to have an
> > idea what parent bus it is connected to, as it should use the parents
> > driver.ops to communicate with the device.
> 
> Yes, it's not a good example.
> 
> > the better (more realistic) version would show that the same device would
> > operate under various xhci_usb, ohci_usb and ehci_usb busses, which might
> > very well have different parent_priv structure (for example, ohci_usb
> > would probably not store maximum speed supported by the device, since the
> > bus only has the basic one)
> 
> They still use the parent driver ops, but now they are all the same?
> Do you mean that the uclass would be the same for each bus?

Yes, I imagine the uclass for USB host controller would the same, not 
depending on the speed of the controller.

> > as a side note, flash is a bit tricky here, since USB and SATA do not
> > provide you with "flash-like" interface even for flash-based devices, but
> > instead have a disk-like interface, which is simpler (does not give you
> > the ability to control bad block management, among other things).
> 
> Another reason why the example is poor - I was more thinking of flash
> as a disk than a raw NAND device.
> 
> Regards,
> Simon

regards
Pavel Herrmann
Simon Glass July 17, 2014, 3:20 p.m. UTC | #4
Hi Pavel,

On 17 July 2014 01:09, Pavel Herrmann <morpheus.ibis@gmail.com> wrote:
>
> On Wednesday 16 of July 2014 23:41:57 Simon Glass wrote:
> > Hi Pavel,
> >
> > On 15 July 2014 02:26, Pavel Herrmann <morpheus.ibis@gmail.com> wrote:
> > > Hi
> > >
> > > On Tuesday 08 of July 2014 21:38:16 Simon Glass wrote:
> > >> ...
> > >> +
> > >> +Note that the information that controls this behaviour is in the bus's
> > >> +driver, not the child's. In fact it is possible that child has no
> > >> knowledge +that it is connected to a bus. The same child device may even
> > >> be used on two +different bus types. As an example. the 'flash' device
> > >> shown above may also +be connected on a SATA bus or standalone with no
> > >> bus:
> > >> +
> > >> +   xhci_usb (UCLASS_USB)
> > >> +      flash (UCLASS_FLASH_STORAGE)  - parent data/methods defined by USB
> > >> bus +
> > >> +   sata (UCLASS_SATA)
> > >> +      flash (UCLASS_FLASH_STORAGE)  - parent data/methods defined by
> > >> SATA
> > >> bus +
> > >> +   flash (UCLASS_FLASH_STORAGE)  - no parent data/methods (not on a bus)
> > >
> > > this is not the best example, since the driver actually needs to have an
> > > idea what parent bus it is connected to, as it should use the parents
> > > driver.ops to communicate with the device.
> >
> > Yes, it's not a good example.
> >
> > > the better (more realistic) version would show that the same device would
> > > operate under various xhci_usb, ohci_usb and ehci_usb busses, which might
> > > very well have different parent_priv structure (for example, ohci_usb
> > > would probably not store maximum speed supported by the device, since the
> > > bus only has the basic one)
> >
> > They still use the parent driver ops, but now they are all the same?
> > Do you mean that the uclass would be the same for each bus?
>
> Yes, I imagine the uclass for USB host controller would the same, not
> depending on the speed of the controller.

OK, but there is no requirement for the child data to be the same for
each USB device. Each device has its own child data structure - this
is not defined by the uclass but by the udevice.

Regards,
Simon
diff mbox

Patch

diff --git a/doc/driver-model/README.txt b/doc/driver-model/README.txt
index cb6b8fd..742b7c8 100644
--- a/doc/driver-model/README.txt
+++ b/doc/driver-model/README.txt
@@ -95,7 +95,7 @@  are provided in test/dm. To run them, try:
 You should see something like this:
 
     <...U-Boot banner...>
-    Running 20 driver model tests
+    Running 21 driver model tests
     Test: dm_test_autobind
     Test: dm_test_autoprobe
     Test: dm_test_bus_children
@@ -104,6 +104,7 @@  You should see something like this:
     Device 'c-test@1': seq 1 is in use by 'd-test'
     Test: dm_test_bus_children_funcs
     Test: dm_test_bus_parent_data
+    Test: dm_test_bus_parent_ops
     Test: dm_test_children
     Test: dm_test_fdt
     Device 'd-test': seq 3 is in use by 'b-test'
@@ -425,6 +426,71 @@  entirely under the control of the board author so a conflict is generally
 an error.
 
 
+Bus Drivers
+-----------
+
+A common use of driver model is to implement a bus, a device which provides
+access to other devices. Example of buses include SPI and I2C. Typically
+the bus provides some sort of transport or translation that makes it
+possible to talk to the devices on the bus.
+
+Driver model provides a few useful features to help with implementing
+buses. Firstly, a bus can request that its children store some 'parent
+data' which can be used to keep track of child state. Secondly, the bus can
+define methods which are called when a child is probed or removed. This is
+similar to the methods the uclass driver provides.
+
+Here an explanation of how a bus fits with a uclass may be useful. Consider
+a USB bus with several devices attached to it, each from a different (made
+up) uclass:
+
+   xhci_usb (UCLASS_USB)
+      eth (UCLASS_ETHERNET)
+      camera (UCLASS_CAMERA)
+      flash (UCLASS_FLASH_STORAGE)
+
+Each of the devices is connected to a different address on the USB bus.
+The bus device wants to store this address and some other information such
+as the bus speed for each device.
+
+To achieve this, the bus device can use dev->parent_priv in each of its
+three children. This can be auto-allocated if the bus driver has a non-zero
+value for per_child_auto_alloc_size. If not, then the bus device can
+allocate the space itself before the child device is probed.
+
+Also the bus driver can define the child_pre_probe() and child_post_remove()
+methods to allow it to do some processing before the child is activated or
+after it is deactivated.
+
+Note that the information that controls this behaviour is in the bus's
+driver, not the child's. In fact it is possible that child has no knowledge
+that it is connected to a bus. The same child device may even be used on two
+different bus types. As an example. the 'flash' device shown above may also
+be connected on a SATA bus or standalone with no bus:
+
+   xhci_usb (UCLASS_USB)
+      flash (UCLASS_FLASH_STORAGE)  - parent data/methods defined by USB bus
+
+   sata (UCLASS_SATA)
+      flash (UCLASS_FLASH_STORAGE)  - parent data/methods defined by SATA bus
+
+   flash (UCLASS_FLASH_STORAGE)  - no parent data/methods (not on a bus)
+
+Above you can see that the driver for xhci_usb/sata controls the child's
+bus methods. In the third example the device is not on a bus, and therefore
+will not have these methods at all. Consider the case where the flash
+device defines child methods. These would be used for *its* children, and
+would be quite separate from the methods defined by the driver for the bus
+that the flash device is connetced to. The act of attaching a device to a
+parent device which is a bus, causes the device to start behaving like a
+bus device, regardless of its own views on the matter.
+
+The uclass for the device can also contain data private to that uclass.
+But note that each device on the bus may be a memeber of a different
+uclass, and this data has nothing to do with the child data for each child
+on the bus.
+
+
 Driver Lifecycle
 ----------------
 
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 42d250f..166b073 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -291,6 +291,12 @@  int device_probe(struct udevice *dev)
 	}
 	dev->seq = seq;
 
+	if (dev->parent && dev->parent->driver->child_pre_probe) {
+		ret = dev->parent->driver->child_pre_probe(dev);
+		if (ret)
+			goto fail;
+	}
+
 	if (drv->ofdata_to_platdata && dev->of_offset >= 0) {
 		ret = drv->ofdata_to_platdata(dev);
 		if (ret)
@@ -352,12 +358,20 @@  int device_remove(struct udevice *dev)
 			goto err_remove;
 	}
 
+	if (dev->parent && dev->parent->driver->child_post_remove) {
+		ret = dev->parent->driver->child_post_remove(dev);
+		if (ret) {
+			dm_warn("%s: Device '%s' failed child_post_remove()",
+				__func__, dev->name);
+		}
+	}
+
 	device_free(dev);
 
 	dev->seq = -1;
 	dev->flags &= ~DM_FLAG_ACTIVATED;
 
-	return 0;
+	return ret;
 
 err_remove:
 	/* We can't put the children back */
diff --git a/include/dm/device.h b/include/dm/device.h
index 20207ce..c8a4072 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -118,6 +118,10 @@  struct udevice_id {
  * @remove: Called to remove a device, i.e. de-activate it
  * @unbind: Called to unbind a device from its driver
  * @ofdata_to_platdata: Called before probe to decode device tree data
+ * @child_pre_probe: Called before a child device is probed. The device has
+ * memory allocated but it has not yet been probed.
+ * @child_post_remove: Called after a child device is removed. The device
+ * has memory allocated but its device_remove() method has been called.
  * @priv_auto_alloc_size: If non-zero this is the size of the private data
  * to be allocated in the device's ->priv pointer. If zero, then the driver
  * is responsible for allocating any data required.
@@ -143,6 +147,8 @@  struct driver {
 	int (*remove)(struct udevice *dev);
 	int (*unbind)(struct udevice *dev);
 	int (*ofdata_to_platdata)(struct udevice *dev);
+	int (*child_pre_probe)(struct udevice *dev);
+	int (*child_post_remove)(struct udevice *dev);
 	int priv_auto_alloc_size;
 	int platdata_auto_alloc_size;
 	int per_child_auto_alloc_size;
diff --git a/include/dm/test.h b/include/dm/test.h
index 7b04850..235d728 100644
--- a/include/dm/test.h
+++ b/include/dm/test.h
@@ -86,9 +86,11 @@  struct dm_test_uclass_priv {
  * struct dm_test_parent_data - parent's information on each child
  *
  * @sum: Test value used to check parent data works correctly
+ * @flag: Used to track calling of parent operations
  */
 struct dm_test_parent_data {
 	int sum;
+	int flag;
 };
 
 /*
@@ -109,6 +111,7 @@  extern struct dm_test_state global_test_state;
  * @fail_count: Number of tests that failed
  * @force_fail_alloc: Force all memory allocs to fail
  * @skip_post_probe: Skip uclass post-probe processing
+ * @removed: Used to keep track of a device that was removed
  */
 struct dm_test_state {
 	struct udevice *root;
@@ -116,6 +119,7 @@  struct dm_test_state {
 	int fail_count;
 	int force_fail_alloc;
 	int skip_post_probe;
+	struct udevice *removed;
 };
 
 /* Test flags for each test */
diff --git a/test/dm/bus.c b/test/dm/bus.c
index df8edcb3..873d64e 100644
--- a/test/dm/bus.c
+++ b/test/dm/bus.c
@@ -14,11 +14,39 @@ 
 
 DECLARE_GLOBAL_DATA_PTR;
 
+enum {
+	FLAG_CHILD_PROBED	= 10,
+	FLAG_CHILD_REMOVED	= -7,
+};
+
+static struct dm_test_state *test_state;
+
 static int testbus_drv_probe(struct udevice *dev)
 {
 	return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false);
 }
 
+static int testbus_child_pre_probe(struct udevice *dev)
+{
+	struct dm_test_parent_data *parent_data = dev_get_parentdata(dev);
+
+	parent_data->flag += FLAG_CHILD_PROBED;
+
+	return 0;
+}
+
+static int testbus_child_post_remove(struct udevice *dev)
+{
+	struct dm_test_parent_data *parent_data = dev_get_parentdata(dev);
+	struct dm_test_state *dms = test_state;
+
+	parent_data->flag += FLAG_CHILD_REMOVED;
+	if (dms)
+		dms->removed = dev;
+
+	return 0;
+}
+
 static const struct udevice_id testbus_ids[] = {
 	{
 		.compatible = "denx,u-boot-test-bus",
@@ -34,6 +62,8 @@  U_BOOT_DRIVER(testbus_drv) = {
 	.priv_auto_alloc_size = sizeof(struct dm_test_priv),
 	.platdata_auto_alloc_size = sizeof(struct dm_test_pdata),
 	.per_child_auto_alloc_size = sizeof(struct dm_test_parent_data),
+	.child_pre_probe = testbus_child_pre_probe,
+	.child_post_remove = testbus_child_post_remove,
 };
 
 UCLASS_DRIVER(testbus) = {
@@ -172,3 +202,41 @@  static int dm_test_bus_parent_data(struct dm_test_state *dms)
 }
 
 DM_TEST(dm_test_bus_parent_data, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+/* Test that the bus ops are called when a child is probed/removed */
+static int dm_test_bus_parent_ops(struct dm_test_state *dms)
+{
+	struct dm_test_parent_data *parent_data;
+	struct udevice *bus, *dev;
+	struct uclass *uc;
+
+	test_state = dms;
+	ut_assertok(uclass_get_device(UCLASS_TEST_BUS, 0, &bus));
+	ut_assertok(uclass_get(UCLASS_TEST_FDT, &uc));
+
+	uclass_foreach_dev(dev, uc) {
+		/* Ignore these if they are not on this bus */
+		if (dev->parent != bus)
+			continue;
+		ut_asserteq_ptr(NULL, dev_get_parentdata(dev));
+
+		ut_assertok(device_probe(dev));
+		parent_data = dev_get_parentdata(dev);
+		ut_asserteq(FLAG_CHILD_PROBED, parent_data->flag);
+	}
+
+	uclass_foreach_dev(dev, uc) {
+		/* Ignore these if they are not on this bus */
+		if (dev->parent != bus)
+			continue;
+		parent_data = dev_get_parentdata(dev);
+		ut_asserteq(FLAG_CHILD_PROBED, parent_data->flag);
+		ut_assertok(device_remove(dev));
+		ut_asserteq_ptr(NULL, dev_get_parentdata(dev));
+		ut_asserteq_ptr(dms->removed, dev);
+	}
+	test_state = NULL;
+
+	return 0;
+}
+DM_TEST(dm_test_bus_parent_ops, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);