dm: core: Read parent ofdata before children

At present a device can read its ofdata before its parent has done the
same. This can cause problems in the case where the parent has a 'ranges'
property, thus affecting the operation of dev_read_addr(), for example.

We already probe parent devices before children so it does not seem to be
a large step to do the same with ofdata.

Make the change and update the documentation in this area.

Signed-off-by: Simon Glass <sjg@chromium.org>
Tested-by: Ley Foon Tan <ley.foon.tan@intel.com>

doc/driver-model/design.rst

@@ -683,11 +683,17 @@ probe/remove which is independent of bind/unbind. This is partly because in
 U-Boot it may be expensive to probe devices and we don't want to do it until
 they are needed, or perhaps until after relocation.
 
-Activation/probe
-^^^^^^^^^^^^^^^^
+Reading ofdata
+^^^^^^^^^^^^^^
 
-When a device needs to be used, U-Boot activates it, by following these
-steps (see device_probe()):
+Most devices have data in the device tree which they can read to find out the
+base address of hardware registers and parameters relating to driver
+operation. This is called 'ofdata' (Open-Firmware data).
+
+The device's_ofdata_to_platdata() implemnents allocation and reading of
+platdata. A parent's ofdata is always read before a child.
+
+The steps are:
 
    1. If priv_auto_alloc_size is non-zero, then the device-private space
    is allocated for the device and zeroed. It will be accessible as
@@ -713,32 +719,72 @@ steps (see device_probe()):
    space. The controller can hold information about the USB state of each
    of its children.
 
-   5. All parent devices are probed. It is not possible to activate a device
+   5. If the driver provides an ofdata_to_platdata() method, then this is
+   called to convert the device tree data into platform data. This should
+   do various calls like dev_read_u32(dev, ...) to access the node and store
+   the resulting information into dev->platdata. After this point, the device
+   works the same way whether it was bound using a device tree node or
+   U_BOOT_DEVICE() structure. In either case, the platform data is now stored
+   in the platdata structure. Typically you will use the
+   platdata_auto_alloc_size feature to specify the size of the platform data
+   structure, and U-Boot will automatically allocate and zero it for you before
+   entry to ofdata_to_platdata(). But if not, you can allocate it yourself in
+   ofdata_to_platdata(). Note that it is preferable to do all the device tree
+   decoding in ofdata_to_platdata() rather than in probe(). (Apart from the
+   ugliness of mixing configuration and run-time data, one day it is possible
+   that U-Boot will cache platform data for devices which are regularly
+   de/activated).
+
+   5. The device is marked 'platdata valid'.
+
+Note that ofdata reading is always done (for a child and all its parents)
+before probing starts. Thus devices go through two distinct states when
+probing: reading platform data and actually touching the hardware to bring
+the device up.
+
+Having probing separate from ofdata-reading helps deal with of-platdata, where
+the probe() method is common to both DT/of-platdata operation, but the
+ofdata_to_platdata() method is implemented differently.
+
+Another case has come up where this separate is useful. Generation of ACPI
+tables uses the of-platdata but does not want to probe the device. Probing
+would cause U-Boot to violate one of its design principles, viz that it
+should only probe devices that are used. For ACPI we want to generate a
+table for each device, even if U-Boot does not use it. In fact it may not
+even be possible to probe the device - e.g. an SD card which is not
+present will cause an error on probe, yet we still must tell Linux about
+the SD card connector in case it is used while Linux is running.
+
+It is important that the ofdata_to_platdata() method does not actually probe
+the device itself. However there are cases where other devices must be probed
+in the ofdata_to_platdata() method. An example is where a device requires a
+GPIO for it to operate. To select a GPIO obviously requires that the GPIO
+device is probed. This is OK when used by common, core devices such as GPIO,
+clock, interrupts, reset and the like.
+
+If your device relies on its parent setting up a suitable address space, so
+that dev_read_addr() works correctly, then make sure that the parent device
+has its setup code in ofdata_to_platdata(). If it has it in the probe method,
+then you cannot call dev_read_addr() from the child device's
+ofdata_to_platdata() method. Move it to probe() instead. Buses like PCI can
+fall afoul of this rule.
+
+Activation/probe
+^^^^^^^^^^^^^^^^
+
+When a device needs to be used, U-Boot activates it, by first reading ofdata
+as above and then following these steps (see device_probe()):
+
+   1. All parent devices are probed. It is not possible to activate a device
    unless its predecessors (all the way up to the root device) are activated.
    This means (for example) that an I2C driver will require that its bus
    be activated.
 
-   6. The device's sequence number is assigned, either the requested one
+   2. The device's sequence number is assigned, either the requested one
    (assuming no conflicts) or the next available one if there is a conflict
    or nothing particular is requested.
 
-   7. If the driver provides an ofdata_to_platdata() method, then this is
-   called to convert the device tree data into platform data. This should
-   do various calls like fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), ...)
-   to access the node and store the resulting information into dev->platdata.
-   After this point, the device works the same way whether it was bound
-   using a device tree node or U_BOOT_DEVICE() structure. In either case,
-   the platform data is now stored in the platdata structure. Typically you
-   will use the platdata_auto_alloc_size feature to specify the size of the
-   platform data structure, and U-Boot will automatically allocate and zero
-   it for you before entry to ofdata_to_platdata(). But if not, you can
-   allocate it yourself in ofdata_to_platdata(). Note that it is preferable
-   to do all the device tree decoding in ofdata_to_platdata() rather than
-   in probe(). (Apart from the ugliness of mixing configuration and run-time
-   data, one day it is possible that U-Boot will cache platform data for
-   devices which are regularly de/activated).
-
-   8. The device's probe() method is called. This should do anything that
+   4. The device's probe() method is called. This should do anything that
    is required by the device to get it going. This could include checking
    that the hardware is actually present, setting up clocks for the
    hardware and setting up hardware registers to initial values. The code
@@ -753,7 +799,7 @@ steps (see device_probe()):
    allocate the priv space here yourself. The same applies also to
    platdata_auto_alloc_size. Remember to free them in the remove() method.
 
-   9. The device is marked 'activated'
+   5. The device is marked 'activated'
 
    10. The uclass's post_probe() method is called, if one exists. This may
    cause the uclass to do some housekeeping to record the device as

drivers/core/device.c

@@ -321,6 +321,22 @@ int device_ofdata_to_platdata(struct udevice *dev)
 	if (dev->flags & DM_FLAG_PLATDATA_VALID)
 		return 0;
 
+	/* Ensure all parents have ofdata */
+	if (dev->parent) {
+		ret = device_ofdata_to_platdata(dev->parent);
+		if (ret)
+			goto fail;
+
+		/*
+		 * The device might have already been probed during
+		 * the call to device_probe() on its parent device
+		 * (e.g. PCI bridge devices). Test the flags again
+		 * so that we don't mess up the device.
+		 */
+		if (dev->flags & DM_FLAG_PLATDATA_VALID)
+			return 0;
+	}
+
 	drv = dev->driver;
 	assert(drv);
 

test/dm/test-fdt.c

@@ -992,3 +992,28 @@ static int dm_test_first_child_probe(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_first_child_probe, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+/* Test that ofdata is read for parents before children */
+static int dm_test_ofdata_order(struct unit_test_state *uts)
+{
+	struct udevice *bus, *dev;
+
+	ut_assertok(uclass_find_first_device(UCLASS_I2C, &bus));
+	ut_assertnonnull(bus);
+	ut_assert(!(bus->flags & DM_FLAG_PLATDATA_VALID));
+
+	ut_assertok(device_find_first_child(bus, &dev));
+	ut_assertnonnull(dev);
+	ut_assert(!(dev->flags & DM_FLAG_PLATDATA_VALID));
+
+	/* read the child's ofdata which should cause the parent's to be read */
+	ut_assertok(device_ofdata_to_platdata(dev));
+	ut_assert(dev->flags & DM_FLAG_PLATDATA_VALID);
+	ut_assert(bus->flags & DM_FLAG_PLATDATA_VALID);
+
+	ut_assert(!(dev->flags & DM_FLAG_ACTIVATED));
+	ut_assert(!(bus->flags & DM_FLAG_ACTIVATED));
+
+	return 0;
+}
+DM_TEST(dm_test_ofdata_order, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);