dm: allow setting driver_data before/during bind

From: Stephen Warren <[email protected]>

This will allow a driver's bind function to use the driver data. One example is the Tegra186 GPIO driver, which instantiates child devices for each of its GPIO ports, yet supports two different HW instances each with a different set of ports, and identified by the udevice_id .data field.

Signed-off-by Stephen Warren <[email protected]>

---
 drivers/core/device.c            | 7 ++++---
 drivers/core/lists.c             | 6 +++---
 drivers/gpio/dwapb_gpio.c        | 2 +-
 drivers/gpio/s5p_gpio.c          | 2 +-
 drivers/gpio/sunxi_gpio.c        | 2 +-
 drivers/gpio/tegra_gpio.c        | 2 +-
 drivers/mtd/spi/sandbox.c        | 2 +-
 drivers/net/mvpp2.c              | 3 ++-
 drivers/pci/pci-uclass.c         | 5 ++---
 drivers/power/pmic/pmic-uclass.c | 2 +-
 drivers/usb/host/usb-uclass.c    | 5 ++---
 include/dm/device-internal.h     | 5 +++--
 12 files changed, 22 insertions(+), 21 deletions(-)

Hi Stephen,

Quoted 9 lines · Stephen Warren

From: Stephen Warren <[email protected]>

This will allow a driver's bind function to use the driver data. One example is the Tegra186 GPIO driver, which instantiates child devices for each of its GPIO ports, yet supports two different HW instances each with a different set of ports, and identified by the udevice_id .data field.

Signed-off-by Stephen Warren <[email protected]>

Quoted 14 lines

---
  drivers/core/device.c            | 7 ++++---
  drivers/core/lists.c             | 6 +++---
  drivers/gpio/dwapb_gpio.c        | 2 +-
  drivers/gpio/s5p_gpio.c          | 2 +-
  drivers/gpio/sunxi_gpio.c        | 2 +-
  drivers/gpio/tegra_gpio.c        | 2 +-
  drivers/mtd/spi/sandbox.c        | 2 +-
  drivers/net/mvpp2.c              | 3 ++-
  drivers/pci/pci-uclass.c         | 5 ++---
  drivers/power/pmic/pmic-uclass.c | 2 +-
  drivers/usb/host/usb-uclass.c    | 5 ++---
  include/dm/device-internal.h     | 5 +++--
  12 files changed, 22 insertions(+), 21 deletions(-)

I'm not sure this extra parameter carries its weight:

- most callers just pass 0

- the field is supposed to be set up by device tree and probing tables, not code

- bind() methods should not care about the driver data (they are not allowed to touch hardware), so setting it later is fine

- you can already pass platform data to the driver which is the preferred communication method from a parent to its children

Also it's not clear from your Tegra 186 GPIO patch where you are using this.

(quoted diff) +5 -0

+static int tegra186_gpio_bind(struct udevice *parent)
+{
+	struct tegra186_gpio_platdata *parent_plat = parent->platdata;
+	struct tegra186_gpio_ctlr_data *ctlr_data =
+		(struct tegra186_gpio_ctlr_data *)parent->driver_data;

(quoted diff) +3 -0

+	/* If this is a child device, there is nothing to do here */
+	if (parent_plat)
+		return 0;

(quoted diff) +1 -0

+	for (port = 0; port < ctlr_data->port_count; port++) {

Quoted 27 lines · Simon Glass

Hi Stephen,

Quoted 9 lines · Stephen Warren

From: Stephen Warren <[email protected]>

This will allow a driver's bind function to use the driver data. One example is the Tegra186 GPIO driver, which instantiates child devices for each of its GPIO ports, yet supports two different HW instances each with a different set of ports, and identified by the udevice_id .data field.

Signed-off-by Stephen Warren <[email protected]>

Quoted 14 lines

---
  drivers/core/device.c            | 7 ++++---
  drivers/core/lists.c             | 6 +++---
  drivers/gpio/dwapb_gpio.c        | 2 +-
  drivers/gpio/s5p_gpio.c          | 2 +-
  drivers/gpio/sunxi_gpio.c        | 2 +-
  drivers/gpio/tegra_gpio.c        | 2 +-
  drivers/mtd/spi/sandbox.c        | 2 +-
  drivers/net/mvpp2.c              | 3 ++-
  drivers/pci/pci-uclass.c         | 5 ++---
  drivers/power/pmic/pmic-uclass.c | 2 +-
  drivers/usb/host/usb-uclass.c    | 5 ++---
  include/dm/device-internal.h     | 5 +++--
  12 files changed, 22 insertions(+), 21 deletions(-)

Quoted 3 lines

I'm not sure this extra parameter carries its weight:

- most callers just pass 0

The same is true of the existing platdata field in many cases.

Quoted 2 lines

- the field is supposed to be set up by device tree and probing tables, not code

While the existence of this new parameter does allow arbitrary code to set the parameter, this patch only actually sets the parameter in the case where DT and probing tables have determined that value.

Quoted 2 lines

- bind() methods should not care about the driver data (they are not allowed to touch hardware), so setting it later is fine

Not touching HW is fine, but the driver data can still feed into purely SW decisions that bind makes. More details below.

Quoted 2 lines

- you can already pass platform data to the driver which is the preferred communication method from a parent to its children

I don't believe this is possible for devices instantiated from DT is it? In that case, platform data is always NULL:

int lists_bind_fdt(struct udevice *parent, const void *blob, int offset,
struct udevice **devp)
...

         ret = device_bind(parent, entry, name, NULL, id->data,
                   offset, &dev);

(That quoted code is with this patch applied, and the NULL value is the platform data parameter.)

Quoted 2 lines

Also it's not clear from your Tegra 186 GPIO patch where you are using this.

Here's the relevant part from the Tegra186 GPIO driver patch I posted:

Quoted 5 lines

(quoted diff) +5 -0

+static int tegra186_gpio_bind(struct udevice *parent)
+{
+    struct tegra186_gpio_platdata *parent_plat = parent->platdata;
+    struct tegra186_gpio_ctlr_data *ctlr_data =
+        (struct tegra186_gpio_ctlr_data *)parent->driver_data;

...

Quoted 3 lines

(quoted diff) +3 -0

+    /* If this is a child device, there is nothing to do here */
+    if (parent_plat)
+        return 0;

...

Quoted 1 line

+ for (port = 0; port < ctlr_data->port_count; port++) {

...

Review

Quoted 2 lines

(quoted diff) +2 -0

+        plat->name = ctlr_data->ports[port].name;
+        plat->regs = &(regs[ctlr_data->ports[port].offset / 4]);

The data is used to determine how many child devices (one per port) to create, and the name and register offset of each one. This is modelled after the logic in the previous Tegra GPIO driver that you wrote, with the unfortunate modification that the register layout is more "interesting" on Tegra186, and so we can't determine the number of and parameters for the child devices purely algorithmically, since the register layout is decidedly non-linear.

I suppose an alternative would be to create separate U_BOOT_DRIVER()s for each compatible value with different register layout, and then have the bind() for each of those call into some common implementation with a hard-coded parameter. Still, it seems like the usage in the current code is exactly what udevice_id.data is for; to avoid having to implement separate functions that do that.

Perhaps the creation of the child devices could happen in probe() rather than bind()? I imagine there's some reason this wouldn't work (such as this causing the devices to be created too late to be referenced by other drivers?) or you would have done this in the existing Tegra GPIO driver.

Quoted 30 lines · Simon Glass

Hi Stephen,

Quoted 24 lines · Stephen Warren

From: Stephen Warren <[email protected]>

This will allow a driver's bind function to use the driver data. One example is the Tegra186 GPIO driver, which instantiates child devices for each of its GPIO ports, yet supports two different HW instances each with a different set of ports, and identified by the udevice_id .data field.

Signed-off-by Stephen Warren <[email protected]>

---
  drivers/core/device.c            | 7 ++++---
  drivers/core/lists.c             | 6 +++---
  drivers/gpio/dwapb_gpio.c        | 2 +-
  drivers/gpio/s5p_gpio.c          | 2 +-
  drivers/gpio/sunxi_gpio.c        | 2 +-
  drivers/gpio/tegra_gpio.c        | 2 +-
  drivers/mtd/spi/sandbox.c        | 2 +-
  drivers/net/mvpp2.c              | 3 ++-
  drivers/pci/pci-uclass.c         | 5 ++---
  drivers/power/pmic/pmic-uclass.c | 2 +-
  drivers/usb/host/usb-uclass.c    | 5 ++---
  include/dm/device-internal.h     | 5 +++--
  12 files changed, 22 insertions(+), 21 deletions(-)

Quoted 3 lines

I'm not sure this extra parameter carries its weight:

- most callers just pass 0

The same is true of the existing platdata field in many cases.

Quoted 3 lines

- the field is supposed to be set up by device tree and probing tables, not code

While the existence of this new parameter does allow arbitrary code to set the parameter, this patch only actually sets the parameter in the case where DT and probing tables have determined that value.

Quoted 3 lines

- bind() methods should not care about the driver data (they are not allowed to touch hardware), so setting it later is fine

Not touching HW is fine, but the driver data can still feed into purely SW decisions that bind makes. More details below.

Quoted 2 lines

- you can already pass platform data to the driver which is the preferred communication method from a parent to its children

I don't believe this is possible for devices instantiated from DT is it? In that case, platform data is always NULL:

int lists_bind_fdt(struct udevice *parent, const void *blob, int offset,
struct udevice **devp)
...

                ret = device_bind(parent, entry, name, NULL, id->data,
                                  offset, &dev);

(That quoted code is with this patch applied, and the NULL value is the platform data parameter.)

Quoted 2 lines

Also it's not clear from your Tegra 186 GPIO patch where you are using this.

Here's the relevant part from the Tegra186 GPIO driver patch I posted:

Quoted 5 lines

(quoted diff) +5 -0

+static int tegra186_gpio_bind(struct udevice *parent)
+{
+       struct tegra186_gpio_platdata *parent_plat = parent->platdata;
+       struct tegra186_gpio_ctlr_data *ctlr_data =
+               (struct tegra186_gpio_ctlr_data *)parent->driver_data;

...

Quoted 4 lines

(quoted diff) +3 -0

+       /* If this is a child device, there is nothing to do here */
+       if (parent_plat)
+               return 0;

...

Quoted 2 lines

+ for (port = 0; port < ctlr_data->port_count; port++) {

...

Review

Quoted 3 lines

(quoted diff) +2 -0

+               plat->name = ctlr_data->ports[port].name;
+               plat->regs = &(regs[ctlr_data->ports[port].offset / 4]);

The data is used to determine how many child devices (one per port) to create, and the name and register offset of each one. This is modelled after the logic in the previous Tegra GPIO driver that you wrote, with the unfortunate modification that the register layout is more "interesting" on Tegra186, and so we can't determine the number of and parameters for the child devices purely algorithmically, since the register layout is decidedly non-linear.

I suppose an alternative would be to create separate U_BOOT_DRIVER()s for each compatible value with different register layout, and then have the bind() for each of those call into some common implementation with a hard-coded parameter. Still, it seems like the usage in the current code is exactly what udevice_id.data is for; to avoid having to implement separate functions that do that.

Perhaps the creation of the child devices could happen in probe() rather than bind()? I imagine there's some reason this wouldn't work (such as this causing the devices to be created too late to be referenced by other drivers?) or you would have done this in the existing Tegra GPIO driver.

Hi Stephen,

Quoted 33 lines · Stephen Warren

Quoted 32 lines · Simon Glass

Hi Stephen,

Quoted 10 lines · Stephen Warren

From: Stephen Warren <[email protected]>

This will allow a driver's bind function to use the driver data. One example is the Tegra186 GPIO driver, which instantiates child devices for each of its GPIO ports, yet supports two different HW instances each with a different set of ports, and identified by the udevice_id .data field.

Signed-off-by Stephen Warren <[email protected]>

Quoted 14 lines

---
   drivers/core/device.c            | 7 ++++---
   drivers/core/lists.c             | 6 +++---
   drivers/gpio/dwapb_gpio.c        | 2 +-
   drivers/gpio/s5p_gpio.c          | 2 +-
   drivers/gpio/sunxi_gpio.c        | 2 +-
   drivers/gpio/tegra_gpio.c        | 2 +-
   drivers/mtd/spi/sandbox.c        | 2 +-
   drivers/net/mvpp2.c              | 3 ++-
   drivers/pci/pci-uclass.c         | 5 ++---
   drivers/power/pmic/pmic-uclass.c | 2 +-
   drivers/usb/host/usb-uclass.c    | 5 ++---
   include/dm/device-internal.h     | 5 +++--
   12 files changed, 22 insertions(+), 21 deletions(-)

I'm not sure this extra parameter carries its weight:

Quoted 3 lines

Quoted 1 line

- most callers just pass 0

The same is true of the existing platdata field in many cases.

Yes, but platdata is defined to be needed by bind(), whereas driver_data is supposed to be used in probe() to find out which device tree compatible string matched.

Remember that the device tree
properties are not looked at during bind(), only later. So it makes sense to include platdata in the device_bind() call, but not driver_data.

The device's bind() method is permitted to perform simple actions, but should not scan the device tree node, not initialise hardware, nor set up structures or allocate memory. All of these tasks should be left for the probe() method.

Quoted 6 lines

Quoted 2 lines

- the field is supposed to be set up by device tree and probing tables, not code

While the existence of this new parameter does allow arbitrary code to set the parameter, this patch only actually sets the parameter in the case where DT and probing tables have determined that value.

I don't think so. That value is set in lists_bind_fdt().

I wonder if you could set it yourself after calling device_bind()?

Quoted 9 lines

Quoted 2 lines

- bind() methods should not care about the driver data (they are not allowed to touch hardware), so setting it later is fine

Not touching HW is fine, but the driver data can still feed into purely SW decisions that bind makes. More details below.

Quoted 2 lines

- you can already pass platform data to the driver which is the preferred communication method from a parent to its children

Quoted 2 lines

I don't believe this is possible for devices instantiated from DT is it? In that case, platform data is always NULL:

That's right. For DT the paltform data is set up in the ofdata_to_platdata() method. Since you are using DT, you should follow that convention.

Quoted 36 lines

int lists_bind_fdt(struct udevice *parent, const void *blob, int offset,
struct udevice **devp)
...

                 ret = device_bind(parent, entry, name, NULL, id->data,
                                   offset, &dev);

(That quoted code is with this patch applied, and the NULL value is the platform data parameter.)

Quoted 2 lines

Also it's not clear from your Tegra 186 GPIO patch where you are using this.

Here's the relevant part from the Tegra186 GPIO driver patch I posted:

Quoted 5 lines

(quoted diff) +5 -0

+static int tegra186_gpio_bind(struct udevice *parent)
+{
+       struct tegra186_gpio_platdata *parent_plat = parent->platdata;
+       struct tegra186_gpio_ctlr_data *ctlr_data =
+               (struct tegra186_gpio_ctlr_data *)parent->driver_data;

...

Quoted 4 lines

(quoted diff) +3 -0

+       /* If this is a child device, there is nothing to do here */
+       if (parent_plat)
+               return 0;

...

Quoted 2 lines

+ for (port = 0; port < ctlr_data->port_count; port++) {

...

Review

Quoted 3 lines

(quoted diff) +2 -0

+               plat->name = ctlr_data->ports[port].name;
+               plat->regs = &(regs[ctlr_data->ports[port].offset / 4]);

Quoted 7 lines

The data is used to determine how many child devices (one per port) to create, and the name and register offset of each one. This is modelled after the logic in the previous Tegra GPIO driver that you wrote, with the unfortunate modification that the register layout is more "interesting" on Tegra186, and so we can't determine the number of and parameters for the child devices purely algorithmically, since the register layout is decidedly non-linear.

OK I see. This feels like something that your device tree should describe.

Failing that, how about a hard-coded table of information in the source code? You can look through the table and create the appropriate child devices.

Quoted 6 lines

I suppose an alternative would be to create separate U_BOOT_DRIVER()s for each compatible value with different register layout, and then have the bind() for each of those call into some common implementation with a hard-coded parameter. Still, it seems like the usage in the current code is exactly what udevice_id.data is for; to avoid having to implement separate functions that do that.

Yes, but you should use different compatible strings for the nodes.

As
I understand it, you only have a single node, so re-purposing this does not seem right to me.

Quoted 4 lines

Perhaps the creation of the child devices could happen in probe() rather than bind()? I imagine there's some reason this wouldn't work (such as this causing the devices to be created too late to be referenced by other drivers?) or you would have done this in the existing Tegra GPIO driver.

Best not - it is good to have the devices known on start-up. Let me know if the above solution doesn't work.

Quoted 45 lines · Simon Glass

Hi Stephen,

Quoted 41 lines · Stephen Warren

Quoted 37 lines · Simon Glass

Hi Stephen,

Quoted 12 lines · Stephen Warren

From: Stephen Warren <[email protected]>

This will allow a driver's bind function to use the driver data. One example is the Tegra186 GPIO driver, which instantiates child devices for each of its GPIO ports, yet supports two different HW instances each
with a different set of ports, and identified by the udevice_id .data field.

Signed-off-by Stephen Warren <[email protected]>

Quoted 14 lines

---
   drivers/core/device.c            | 7 ++++---
   drivers/core/lists.c             | 6 +++---
   drivers/gpio/dwapb_gpio.c        | 2 +-
   drivers/gpio/s5p_gpio.c          | 2 +-
   drivers/gpio/sunxi_gpio.c        | 2 +-
   drivers/gpio/tegra_gpio.c        | 2 +-
   drivers/mtd/spi/sandbox.c        | 2 +-
   drivers/net/mvpp2.c              | 3 ++-
   drivers/pci/pci-uclass.c         | 5 ++---
   drivers/power/pmic/pmic-uclass.c | 2 +-
   drivers/usb/host/usb-uclass.c    | 5 ++---
   include/dm/device-internal.h     | 5 +++--
   12 files changed, 22 insertions(+), 21 deletions(-)

I'm not sure this extra parameter carries its weight:

- most callers just pass 0

Quoted 6 lines

Quoted 1 line

The same is true of the existing platdata field in many cases.

Yes, but platdata is defined to be needed by bind(), whereas driver_data is supposed to be used in probe() to find out which device tree compatible string matched.

This seems to conflict with the documentation in include/dm/device.h; it claims that platdata is created by calling ofdata_to_platdata() just before calling probe(), at least for the DT case (for the case where U_BOOT_DEVICE is used, the platdata is available right from the start).

I couldn't find anywhere in the documentation that states when driver_data is supposed to be used; could you point me at it so I can read it?

Quoted 6 lines

Remember that the device tree

properties are not looked at during bind(), only later. So it makes sense to include platdata in the device_bind() call, but not driver_data.

Hmm, drivers/gpio/tegra_gpio.c:gpio_tegra_bind() uses DT (which you wrote or at least converted it to DM and chose where to put the DT accesses), and you very recently reviewed and applied "video: tegra: refuse to bind to disabled dcs" which modified drivers/video/tegra.c:tegra_lcd_bind() to use DT.

Admittedly I do now see the following in doc/driver-model/README.txt:

Quoted 4 lines

The device's bind() method is permitted to perform simple actions, but should not scan the device tree node, not initialise hardware, nor set up structures or allocate memory. All of these tasks should be left for the probe() method.

... but then I wonder why that rule was enforced for the patch in this thread, but not in the other cases?

This inconsistency in review is extremely frustrating to me.

Quoted 5 lines

Quoted 5 lines

Quoted 3 lines

- the field is supposed to be set up by device tree and probing tables, not code

Quoted 8 lines

Quoted 5 lines

While the existence of this new parameter does allow arbitrary code to set
the parameter, this patch only actually sets the parameter in the case where
DT and probing tables have determined that value.

I don't think so. That value is set in lists_bind_fdt().

Sure, but that function is only used from 3 places, and explicitly accepts a parameter to indicate which DT node to instantiate a device for. It won't work unless a valid DT node is passed to it, and therefore can only work for DT-based probing.

Quoted 1 line

I wonder if you could set it yourself after calling device_bind()?

The Tegra186 GPIO driver explicitly needs to use the data inside the driver's bind() function in order to know how many child devices to instantiate. Setting the value after calling device_bind() (which the core DM code already does) is too late.

For reference, you can see the exact code at:
http://lists.denx.de/pipermail/u-boot/2016-April/252238.html
"gpio: add Tegra186 GPIO driver"

Search for "tegra186_gpio_bind" and look at the assignment to, and use of, ctlr_data.

I had also quoted that part of the code in my previous email.

Quoted 17 lines

Quoted 15 lines

Quoted 2 lines

- bind() methods should not care about the driver data (they are not allowed to touch hardware), so setting it later is fine

Not touching HW is fine, but the driver data can still feed into purely SW
decisions that bind makes. More details below.

Quoted 2 lines

- you can already pass platform data to the driver which is the preferred communication method from a parent to its children

I don't believe this is possible for devices instantiated from DT is it? In
that case, platform data is always NULL:

Quoted 3 lines

That's right. For DT the paltform data is set up in the ofdata_to_platdata() method. Since you are using DT, you should follow that convention.

This is the opposite of what you said above, which was that platdata is for bind().

Quoted 3 lines

Quoted 3 lines

int lists_bind_fdt(struct udevice *parent, const void *blob, int offset,

Quoted 54 lines

Quoted 42 lines

...

                 ret = device_bind(parent, entry, name, NULL, id->data,
                                   offset, &dev);

(That quoted code is with this patch applied, and the NULL value is the platform data parameter.)

Quoted 2 lines

Also it's not clear from your Tegra 186 GPIO patch where you are using this.

Here's the relevant part from the Tegra186 GPIO driver patch I posted:

Quoted 5 lines

(quoted diff) +5 -0

+static int tegra186_gpio_bind(struct udevice *parent)
+{
+       struct tegra186_gpio_platdata *parent_plat = parent->platdata;
+       struct tegra186_gpio_ctlr_data *ctlr_data =
+               (struct tegra186_gpio_ctlr_data *)parent->driver_data;

...

Quoted 5 lines

(quoted diff) +3 -0

+       /* If this is a child device, there is nothing to do here */
+       if (parent_plat)
+               return 0;

...

Quoted 3 lines

+ for (port = 0; port < ctlr_data->port_count; port++) {

...

Review

Quoted 4 lines

(quoted diff) +2 -0

+               plat->name = ctlr_data->ports[port].name;
+               plat->regs = &(regs[ctlr_data->ports[port].offset / 4]);

Quoted 10 lines

The data is used to determine how many child devices (one per port) to create, and the name and register offset of each one. This is modelled after
the logic in the previous Tegra GPIO driver that you wrote, with the unfortunate modification that the register layout is more "interesting" on
Tegra186, and so we can't determine the number of and parameters for the child devices purely algorithmically, since the register layout is decidedly
non-linear.

Quoted 2 lines

OK I see. This feels like something that your device tree should describe.

DT generally describes the presence of HW blocks, not the internal details of those HW blocks, since that is a fixed facet of the hardware design and can be statically derived from the value of the compatible property.

Equally, I'm not sure how describing the details of the HW differences in DT would help. The details need to be known by the driver's bind() function, which you and the U-Boot documentation both state isn't allowed to access DT.

Quoted 4 lines

Failing that, how about a hard-coded table of information in the source code? You can look through the table and create the appropriate child devices.

That is EXACTLY what the code is doing. The only issue is that the driver supports two different compatible properties and needs to know which one was found in DT in order to use the right table. That's what driver_data is; the .data value from the udevice_id/of_match table.

Quoted 10 lines

Quoted 8 lines

I suppose an alternative would be to create separate U_BOOT_DRIVER()s for each compatible value with different register layout, and then have the bind() for each of those call into some common implementation with a hard-coded parameter. Still, it seems like the usage in the current code is
exactly what udevice_id.data is for; to avoid having to implement separate
functions that do that.

Quoted 1 line

Yes, but you should use different compatible strings for the nodes.

That's EXACTLY what the code does. However, there is currently no way for bind() to find out which entry in the udevice_id table matched, since the DM core currently sets driver_data after calling bind() rather than before.

Quoted 4 lines

As
I understand it, you only have a single node, so re-purposing this does not seem right to me.

I must not understand what you're conveying by "single node" here. To quote the example from the Tegra186 GPIO DT binding documentation, here is what the DT looks like:

        gpio at 2200000 {
                compatible = "nvidia,tegra186-gpio";
                reg-names = "security", "gpio";
                reg =
                        <0x0 0x2200000 0x0 0x10000>,
                        <0x0 0x2210000 0x0 0x10000>;
                interrupts =
                        <0 47 IRQ_TYPE_LEVEL_HIGH>,
                        <0 50 IRQ_TYPE_LEVEL_HIGH>,
                        <0 53 IRQ_TYPE_LEVEL_HIGH>,
                        <0 56 IRQ_TYPE_LEVEL_HIGH>,
                        <0 59 IRQ_TYPE_LEVEL_HIGH>,
                        <0 180 IRQ_TYPE_LEVEL_HIGH>;
                gpio-controller;
                #gpio-cells = <2>;
                interrupt-controller;
                #interrupt-cells = <2>;
        };

        gpio at c2f0000 {
                compatible = "nvidia,tegra186-gpio-aon";
                reg-names = "security", "gpio";
                reg =
                        <0x0 0xc2f0000 0x0 0x1000>,
                        <0x0 0xc2f1000 0x0 0x1000>;
                interrupts =
                        <0 60 IRQ_TYPE_LEVEL_HIGH>;
                gpio-controller;
                #gpio-cells = <2>;
                interrupt-controller;
                #interrupt-cells = <2>;
        };

Those two HW blocks are roughly but not exactly the same HW. Hence, there are two compatible values. The two HW blocks are similar enough to be handled by a single driver though, and hence the udevice_id table has two entries:

static const struct udevice_id tegra186_gpio_ids[] = {
        {
                .compatible = "nvidia,tegra186-gpio",
                .data = (ulong)&tegra186_gpio_main_data,
        },
        {
                .compatible = "nvidia,tegra186-gpio-aon",
                .data = (ulong)&tegra186_gpio_aon_data,
        },
        { }
};

The .data value there provides all necessary information for the driver to handle both HW block designs.

All I need is to be able to access the ".data" from that table in bind(), whereas the DM core currently doesn't allow that.

Quoted 10 lines

Quoted 6 lines

Perhaps the creation of the child devices could happen in probe() rather than bind()? I imagine there's some reason this wouldn't work (such as this
causing the devices to be created too late to be referenced by other drivers?) or you would have done this in the existing Tegra GPIO driver.

Best not - it is good to have the devices known on start-up. Let me know if the above solution doesn't work.

I must admit, I didn't see any solution offered in your email.

If the child devices must be created in bind(), which I do agree makes sense, then bind() must be able to determine which udevice_id table entry is related to the device being bound. Is there another way to do that besides using the driver_data from the udevice_id table? I suppose I could make bind() go read the compatible property from the DT node for the device, and manually search through the udevice_id table and find the matching entry. However, that would be duplicating work the DM core has already done, and exposes by setting the device's driver_data right after calling the driver's bind(), so doesn't make sense to me.