[PATCH v2 0/8] dm: Various fixes in dm core/cpu/timer

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 0/8] dm: Various fixes in dm core/cpu/timer

Bin Meng
This fixed several issues identified in dm core/cpu/timer codes.

The issues were found during RISC-V cpu/timer driver development
for QEMU RISC-V port.

This series is available at u-boot-x86/dm-fixes for testing.

Changes in v2:
- rebase on u-boot/master, and fix one build error

Bin Meng (8):
  dm: cpu: Fix print_cpuinfo() output
  cpu: mpc83xx: Remove unnecessary characters in the description string
  dm: util: Add a livetree equivalent API of dm_fdt_pre_reloc()
  dm: core: Respect drivers with the DM_FLAG_PRE_RELOC flag in
    lists_bind_fdt()
  dm: Correct pre_reloc_only parameter description in several APIs'
    comments
  dm: core: Mirror the chosen node parse logic in the livetree scanning
  test: dm: core: Add a test case for driver marked with
    DM_FLAG_PRE_RELOC flag
  timer: Sort Kconfig driver entries

 arch/sandbox/dts/test.dts      |   4 ++
 common/board_f.c               |   2 +-
 drivers/core/device.c          |   2 +-
 drivers/core/lists.c           |   9 +++-
 drivers/core/root.c            |  20 +++++---
 drivers/core/util.c            |  25 ++++++++++
 drivers/cpu/mpc83xx_cpu.c      |   2 +-
 drivers/serial/serial-uclass.c |   2 +-
 drivers/timer/Kconfig          | 110 ++++++++++++++++++++---------------------
 drivers/timer/timer-uclass.c   |   2 +-
 include/dm/device-internal.h   |   4 +-
 include/dm/lists.h             |   9 ++--
 include/dm/root.h              |  17 ++++---
 include/dm/util.h              |  27 +++++++++-
 test/dm/bus.c                  |   2 +-
 test/dm/test-fdt.c             |  29 +++++++++--
 16 files changed, 180 insertions(+), 86 deletions(-)

--
2.7.4

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 1/8] dm: cpu: Fix print_cpuinfo() output

Bin Meng
It was observed that current output of print_cpuinfo() on QEMU
x86 targets does not have an ending '\n', neither have a leading
'CPU:' any more. However it used to have these before.

It turns out commit c0434407b595 introduced a unified DM version
of print_cpuinfo() that exposed such issue on QEMU x86.

Fixes: c0434407b595 ("board_f: Use static print_cpuinfo if CONFIG_CPU is active")
Signed-off-by: Bin Meng <[hidden email]>
---

Changes in v2: None

 common/board_f.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/board_f.c b/common/board_f.c
index 213d044..96503ff 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -187,7 +187,7 @@ static int print_cpuinfo(void)
  return ret;
  }
 
- printf("%s", desc);
+ printf("CPU:   %s\n", desc);
 
  return 0;
 }
--
2.7.4

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 4/8] dm: core: Respect drivers with the DM_FLAG_PRE_RELOC flag in lists_bind_fdt()

Bin Meng
In reply to this post by Bin Meng
Currently the comments of several APIs (eg: dm_init_and_scan()) say:

@pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
flag. If false bind all drivers.

The 'Pre-Relocation Support' chapter in doc/driver-model/README.txt
documents the same that both device tree properties and driver flag
are supported.

However the implementation only checks these special device tree
properties without checking the driver flag at all. This updates
lists_bind_fdt() to consider both scenarios.

Signed-off-by: Bin Meng <[hidden email]>

---

Changes in v2:
- rebase on u-boot/master, and fix one build error

 drivers/core/device.c          |  2 +-
 drivers/core/lists.c           |  9 ++++++++-
 drivers/core/root.c            | 12 ++++--------
 drivers/serial/serial-uclass.c |  2 +-
 drivers/timer/timer-uclass.c   |  2 +-
 include/dm/lists.h             |  5 ++++-
 6 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/core/device.c b/drivers/core/device.c
index feed43c..1451689 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -815,5 +815,5 @@ int dev_enable_by_path(const char *path)
  if (ret)
  return ret;
 
- return lists_bind_fdt(parent, node, NULL);
+ return lists_bind_fdt(parent, node, NULL, false);
 }
diff --git a/drivers/core/lists.c b/drivers/core/lists.c
index a167726..a1f8284 100644
--- a/drivers/core/lists.c
+++ b/drivers/core/lists.c
@@ -122,7 +122,8 @@ static int driver_check_compatible(const struct udevice_id *of_match,
  return -ENOENT;
 }
 
-int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp)
+int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp,
+   bool pre_reloc_only)
 {
  struct driver *driver = ll_entry_start(struct driver, driver);
  const int n_ents = ll_entry_count(struct driver, driver);
@@ -171,6 +172,12 @@ int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp)
  if (entry == driver + n_ents)
  continue;
 
+ if (pre_reloc_only) {
+ if (!dm_ofnode_pre_reloc(node) &&
+    !(entry->flags & DM_FLAG_PRE_RELOC))
+ return 0;
+ }
+
  pr_debug("   - found match at '%s'\n", entry->name);
  ret = device_bind_with_driver_data(parent, entry, name,
    id->data, node, &dev);
diff --git a/drivers/core/root.c b/drivers/core/root.c
index 47d10b8..f918ee1 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -222,14 +222,12 @@ static int dm_scan_fdt_live(struct udevice *parent,
  int ret = 0, err;
 
  for (np = node_parent->child; np; np = np->sibling) {
- if (pre_reloc_only &&
-    !of_find_property(np, "u-boot,dm-pre-reloc", NULL))
- continue;
  if (!of_device_is_available(np)) {
  pr_debug("   - ignoring disabled device\n");
  continue;
  }
- err = lists_bind_fdt(parent, np_to_ofnode(np), NULL);
+ err = lists_bind_fdt(parent, np_to_ofnode(np), NULL,
+     pre_reloc_only);
  if (err && !ret) {
  ret = err;
  debug("%s: ret=%d\n", np->name, ret);
@@ -276,14 +274,12 @@ static int dm_scan_fdt_node(struct udevice *parent, const void *blob,
  continue;
  }
 
- if (pre_reloc_only &&
-    !dm_fdt_pre_reloc(blob, offset))
- continue;
  if (!fdtdec_get_is_enabled(blob, offset)) {
  pr_debug("   - ignoring disabled device\n");
  continue;
  }
- err = lists_bind_fdt(parent, offset_to_ofnode(offset), NULL);
+ err = lists_bind_fdt(parent, offset_to_ofnode(offset), NULL,
+     pre_reloc_only);
  if (err && !ret) {
  ret = err;
  debug("%s: ret=%d\n", fdt_get_name(blob, offset, NULL),
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index ffdcae0..cc11474 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -61,7 +61,7 @@ static int serial_check_stdout(const void *blob, struct udevice **devp)
  * anyway.
  */
  if (node > 0 && !lists_bind_fdt(gd->dm_root, offset_to_ofnode(node),
- devp)) {
+ devp, false)) {
  if (!device_probe(*devp))
  return 0;
  }
diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
index fe73f71..12ee6eb 100644
--- a/drivers/timer/timer-uclass.c
+++ b/drivers/timer/timer-uclass.c
@@ -108,7 +108,7 @@ int notrace dm_timer_init(void)
  * If the timer is not marked to be bound before
  * relocation, bind it anyway.
  */
- if (!lists_bind_fdt(dm_root(), node, &dev)) {
+ if (!lists_bind_fdt(dm_root(), node, &dev, false)) {
  ret = device_probe(dev);
  if (ret)
  return ret;
diff --git a/include/dm/lists.h b/include/dm/lists.h
index 13d1516..59094d7 100644
--- a/include/dm/lists.h
+++ b/include/dm/lists.h
@@ -53,10 +53,13 @@ int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only);
  * @parent: parent device (root)
  * @node: device tree node to bind
  * @devp: if non-NULL, returns a pointer to the bound device
+ * @pre_reloc_only: If true, bind only nodes with special devicetree properties,
+ * or drivers with the DM_FLAG_PRE_RELOC flag. If false bind all drivers.
  * @return 0 if device was bound, -EINVAL if the device tree is invalid,
  * other -ve value on error
  */
-int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp);
+int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp,
+   bool pre_reloc_only);
 
 /**
  * device_bind_driver() - bind a device to a driver
--
2.7.4

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 3/8] dm: util: Add a livetree equivalent API of dm_fdt_pre_reloc()

Bin Meng
In reply to this post by Bin Meng
This adds a new API dm_ofnode_pre_reloc(), a livetree equivalent
API of dm_fdt_pre_reloc().

Signed-off-by: Bin Meng <[hidden email]>
---

Changes in v2: None

 drivers/core/util.c | 25 +++++++++++++++++++++++++
 include/dm/util.h   | 27 ++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/core/util.c b/drivers/core/util.c
index 451d476..27a6848 100644
--- a/drivers/core/util.c
+++ b/drivers/core/util.c
@@ -4,6 +4,7 @@
  */
 
 #include <common.h>
+#include <dm/ofnode.h>
 #include <dm/util.h>
 #include <linux/libfdt.h>
 #include <vsprintf.h>
@@ -53,3 +54,27 @@ bool dm_fdt_pre_reloc(const void *blob, int offset)
 
  return false;
 }
+
+bool dm_ofnode_pre_reloc(ofnode node)
+{
+ if (ofnode_read_bool(node, "u-boot,dm-pre-reloc"))
+ return true;
+
+#ifdef CONFIG_TPL_BUILD
+ if (ofnode_read_bool(node, "u-boot,dm-tpl"))
+ return true;
+#elif defined(CONFIG_SPL_BUILD)
+ if (ofnode_read_bool(node, "u-boot,dm-spl"))
+ return true;
+#else
+ /*
+ * In regular builds individual spl and tpl handling both
+ * count as handled pre-relocation for later second init.
+ */
+ if (ofnode_read_bool(node, "u-boot,dm-spl") ||
+    ofnode_read_bool(node, "u-boot,dm-tpl"))
+ return true;
+#endif
+
+ return false;
+}
diff --git a/include/dm/util.h b/include/dm/util.h
index 898822e..9ff6531 100644
--- a/include/dm/util.h
+++ b/include/dm/util.h
@@ -55,7 +55,7 @@ static inline void dm_dump_devres(void)
  * There are 3 settings currently in use
  * -
  * - u-boot,dm-pre-reloc: legacy and indicates any of TPL or SPL
- *   Existing platforms only use it to indicate nodes needee in
+ *   Existing platforms only use it to indicate nodes needed in
  *   SPL. Should probably be replaced by u-boot,dm-spl for
  *   existing platforms.
  * @blob: devicetree
@@ -65,4 +65,29 @@ static inline void dm_dump_devres(void)
  */
 bool dm_fdt_pre_reloc(const void *blob, int offset);
 
+/**
+ * Check if an of node should be or was bound before relocation.
+ *
+ * Devicetree nodes can be marked as needed to be bound
+ * in the loader stages via special devicetree properties.
+ *
+ * Before relocation this function can be used to check if nodes
+ * are required in either SPL or TPL stages.
+ *
+ * After relocation and jumping into the real U-Boot binary
+ * it is possible to determine if a node was bound in one of
+ * SPL/TPL stages.
+ *
+ * There are 3 settings currently in use
+ * -
+ * - u-boot,dm-pre-reloc: legacy and indicates any of TPL or SPL
+ *   Existing platforms only use it to indicate nodes needed in
+ *   SPL. Should probably be replaced by u-boot,dm-spl for
+ *   existing platforms.
+ * @node: of node
+ *
+ * Returns true if node is needed in SPL/TL, false otherwise.
+ */
+bool dm_ofnode_pre_reloc(ofnode node);
+
 #endif
--
2.7.4

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 8/8] timer: Sort Kconfig driver entries

Bin Meng
In reply to this post by Bin Meng
This is currently out of order. Sort it.

Signed-off-by: Bin Meng <[hidden email]>

---

Changes in v2: None

 drivers/timer/Kconfig | 110 +++++++++++++++++++++++++-------------------------
 1 file changed, 55 insertions(+), 55 deletions(-)

diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
index a7d600b..8c0f356 100644
--- a/drivers/timer/Kconfig
+++ b/drivers/timer/Kconfig
@@ -37,6 +37,12 @@ config TIMER_EARLY
   use an early timer. These functions must be supported by your timer
   driver: timer_early_get_count() and timer_early_get_rate().
 
+config AG101P_TIMER
+ bool "AG101P timer support"
+ depends on TIMER && NDS32
+ help
+  Select this to enable a timer for AG01P devices.
+
 config ALTERA_TIMER
  bool "Altera timer support"
  depends on TIMER
@@ -44,6 +50,34 @@ config ALTERA_TIMER
   Select this to enable a timer for Altera devices. Please find
   details on the "Embedded Peripherals IP User Guide" of Altera.
 
+config ARC_TIMER
+ bool "ARC timer support"
+ depends on TIMER && ARC && CLK
+ help
+  Select this to enable built-in ARC timers.
+  ARC cores may have up to 2 built-in timers: timer0 and timer1,
+  usually at least one of them exists. Either of them is supported
+  in U-Boot.
+
+config AST_TIMER
+ bool "Aspeed ast2400/ast2500 timer support"
+ depends on TIMER
+ default y if ARCH_ASPEED
+ help
+  Select this to enable timer for Aspeed ast2400/ast2500 devices.
+  This is a simple sys timer driver, it is compatible with lib/time.c,
+  but does not support any interrupts. Even though SoC has 8 hardware
+  counters, they are all treated as a single device by this driver.
+  This is mostly because they all share several registers which
+  makes it difficult to completely separate them.
+
+config ATCPIT100_TIMER
+ bool "ATCPIT100 timer support"
+ depends on TIMER
+ help
+  Select this to enable a ATCPIT100 timer which will be embedded
+  in AE3XX, AE250 boards.
+
 config ATMEL_PIT_TIMER
  bool "Atmel periodic interval timer support"
  depends on TIMER
@@ -66,18 +100,12 @@ config DESIGNWARE_APB_TIMER
   Enables support for the Designware APB Timer driver. This timer is
   present on Altera SoCFPGA SoCs.
 
-config SANDBOX_TIMER
- bool "Sandbox timer support"
- depends on SANDBOX && TIMER
- help
-  Select this to enable an emulated timer for sandbox. It gets
-  time from host os.
-
-config X86_TSC_TIMER
- bool "x86 Time-Stamp Counter (TSC) timer support"
- depends on TIMER && X86
+config MPC83XX_TIMER
+ bool "MPC83xx timer support"
+ depends on TIMER
  help
-  Select this to enable Time-Stamp Counter (TSC) timer for x86.
+  Select this to enable support for the timer found on
+  devices based on the MPC83xx family of SoCs.
 
 config OMAP_TIMER
  bool "Omap timer support"
@@ -85,17 +113,19 @@ config OMAP_TIMER
  help
   Select this to enable an timer for Omap devices.
 
-config AST_TIMER
- bool "Aspeed ast2400/ast2500 timer support"
+config ROCKCHIP_TIMER
+ bool "Rockchip timer support"
  depends on TIMER
- default y if ARCH_ASPEED
  help
-  Select this to enable timer for Aspeed ast2400/ast2500 devices.
-  This is a simple sys timer driver, it is compatible with lib/time.c,
-  but does not support any interrupts. Even though SoC has 8 hardware
-  counters, they are all treated as a single device by this driver.
-  This is mostly because they all share several registers which
-  makes it difficult to completely separate them.
+  Select this to enable support for the timer found on
+  Rockchip devices.
+
+config SANDBOX_TIMER
+ bool "Sandbox timer support"
+ depends on SANDBOX && TIMER
+ help
+  Select this to enable an emulated timer for sandbox. It gets
+  time from host os.
 
 config STI_TIMER
  bool "STi timer support"
@@ -104,47 +134,17 @@ config STI_TIMER
  help
   Select this to enable a timer for STi devices.
 
-config ARC_TIMER
- bool "ARC timer support"
- depends on TIMER && ARC && CLK
- help
-  Select this to enable built-in ARC timers.
-  ARC cores may have up to 2 built-in timers: timer0 and timer1,
-  usually at least one of them exists. Either of them is supported
-  in U-Boot.
-
-config AG101P_TIMER
- bool "AG101P timer support"
- depends on TIMER && NDS32
- help
-  Select this to enable a timer for AG01P devices.
-
-config ATCPIT100_TIMER
- bool "ATCPIT100 timer support"
- depends on TIMER
- help
-  Select this to enable a ATCPIT100 timer which will be embeded
- in AE3XX, AE250 boards.
-
-config ROCKCHIP_TIMER
-        bool "Rockchip timer support"
- depends on TIMER
- help
-  Select this to enable support for the timer found on
-  Rockchip devices.
-
 config STM32_TIMER
-        bool "STM32 timer support"
+ bool "STM32 timer support"
  depends on TIMER
  help
   Select this to enable support for the timer found on
   STM32 devices.
 
-config MPC83XX_TIMER
-        bool "MPC83xx timer support"
- depends on TIMER
+config X86_TSC_TIMER
+ bool "x86 Time-Stamp Counter (TSC) timer support"
+ depends on TIMER && X86
  help
-  Select this to enable support for the timer found on
-  devices based on the MPC83xx family of SoCs.
+  Select this to enable Time-Stamp Counter (TSC) timer for x86.
 
 endmenu
--
2.7.4

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 6/8] dm: core: Mirror the chosen node parse logic in the livetree scanning

Bin Meng
In reply to this post by Bin Meng
Commit f2006808f099: ("dm: core: parse chosen node") added a logic
to parse the chosen node during dm_scan_fdt_node(), but unfortunately
it missed adding the same logic in dm_scan_fdt_live(). This mirrors
the logic in the livetree version.

The weird thing is that commit f2006808f099 did update the test case
to test such logic, but I have no idea how the test case could pass.

With this fix, the following 2 test cases now pass:

Test: dm_test_bus_children: bus.c
test/dm/bus.c:112, dm_test_bus_children(): num_devices ==
list_count_items(&uc->dev_head): Expected 7, got 6

Test: dm_test_fdt: test-fdt.c
test/dm/test-fdt.c:184, dm_test_fdt(): num_devices ==
list_count_items(&uc->dev_head): Expected 7, got 6

Fixes: f2006808f099 ("dm: core: parse chosen node")
Signed-off-by: Bin Meng <[hidden email]>
---

Changes in v2: None

 drivers/core/root.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/core/root.c b/drivers/core/root.c
index f918ee1..535084e 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -222,6 +222,16 @@ static int dm_scan_fdt_live(struct udevice *parent,
  int ret = 0, err;
 
  for (np = node_parent->child; np; np = np->sibling) {
+ /* "chosen" node isn't a device itself but may contain some: */
+ if (!strcmp(np->name, "chosen")) {
+ pr_debug("parsing subnodes of \"chosen\"\n");
+
+ err = dm_scan_fdt_live(parent, np, pre_reloc_only);
+ if (err && !ret)
+ ret = err;
+ continue;
+ }
+
  if (!of_device_is_available(np)) {
  pr_debug("   - ignoring disabled device\n");
  continue;
--
2.7.4

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 2/8] cpu: mpc83xx: Remove unnecessary characters in the description string

Bin Meng
In reply to this post by Bin Meng
The description string should not contain unnecessary characters,
like the ending '\n' or the leading 'CPU:'.

Signed-off-by: Bin Meng <[hidden email]>
---

Changes in v2: None

 drivers/cpu/mpc83xx_cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpu/mpc83xx_cpu.c b/drivers/cpu/mpc83xx_cpu.c
index 31717af..7bc86bf 100644
--- a/drivers/cpu/mpc83xx_cpu.c
+++ b/drivers/cpu/mpc83xx_cpu.c
@@ -262,7 +262,7 @@ static int mpc83xx_cpu_get_desc(struct udevice *dev, char *buf, int size)
  determine_cpu_data(dev);
 
  snprintf(buf, size,
- "CPU:   %s, MPC%s%s%s, Rev: %d.%d at %s MHz, CSB: %s MHz\n",
+ "%s, MPC%s%s%s, Rev: %d.%d at %s MHz, CSB: %s MHz",
  e300_names[priv->e300_type],
  cpu_type_names[priv->type],
  priv->is_e_processor ? "E" : "",
--
2.7.4

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 7/8] test: dm: core: Add a test case for driver marked with DM_FLAG_PRE_RELOC flag

Bin Meng
In reply to this post by Bin Meng
Now that we fixed the pre-relocation driver binding for driver marked
with DM_FLAG_PRE_RELOC flag, add a test case to cover that scenario.

Signed-off-by: Bin Meng <[hidden email]>
---

Changes in v2: None

 arch/sandbox/dts/test.dts |  4 ++++
 test/dm/bus.c             |  2 +-
 test/dm/test-fdt.c        | 29 ++++++++++++++++++++++++++---
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 491f889..f12410f 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -146,6 +146,10 @@
  compatible = "denx,u-boot-fdt-test";
  };
 
+ h-test {
+ compatible = "denx,u-boot-fdt-test1";
+ };
+
  clocks {
  clk_fixed: clk-fixed {
  compatible = "fixed-clock";
diff --git a/test/dm/bus.c b/test/dm/bus.c
index e9a4028..c41bcf7 100644
--- a/test/dm/bus.c
+++ b/test/dm/bus.c
@@ -104,7 +104,7 @@ UCLASS_DRIVER(testbus) = {
 /* Test that we can probe for children */
 static int dm_test_bus_children(struct unit_test_state *uts)
 {
- int num_devices = 7;
+ int num_devices = 8;
  struct udevice *bus;
  struct uclass *uc;
 
diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c
index 79b1f1d..3da384f 100644
--- a/test/dm/test-fdt.c
+++ b/test/dm/test-fdt.c
@@ -84,6 +84,25 @@ U_BOOT_DRIVER(testfdt_drv) = {
  .platdata_auto_alloc_size = sizeof(struct dm_test_pdata),
 };
 
+static const struct udevice_id testfdt1_ids[] = {
+ {
+ .compatible = "denx,u-boot-fdt-test1",
+ .data = DM_TEST_TYPE_FIRST },
+ { }
+};
+
+U_BOOT_DRIVER(testfdt1_drv) = {
+ .name = "testfdt1_drv",
+ .of_match = testfdt1_ids,
+ .id = UCLASS_TEST_FDT,
+ .ofdata_to_platdata = testfdt_ofdata_to_platdata,
+ .probe = testfdt_drv_probe,
+ .ops = &test_ops,
+ .priv_auto_alloc_size = sizeof(struct dm_test_priv),
+ .platdata_auto_alloc_size = sizeof(struct dm_test_pdata),
+ .flags = DM_FLAG_PRE_RELOC,
+};
+
 /* From here is the testfdt uclass code */
 int testfdt_ping(struct udevice *dev, int pingval, int *pingret)
 {
@@ -168,7 +187,7 @@ int dm_check_devices(struct unit_test_state *uts, int num_devices)
 /* Test that FDT-based binding works correctly */
 static int dm_test_fdt(struct unit_test_state *uts)
 {
- const int num_devices = 7;
+ const int num_devices = 8;
  struct udevice *dev;
  struct uclass *uc;
  int ret;
@@ -208,8 +227,12 @@ static int dm_test_fdt_pre_reloc(struct unit_test_state *uts)
  ret = uclass_get(UCLASS_TEST_FDT, &uc);
  ut_assert(!ret);
 
- /* These is only one pre-reloc device */
- ut_asserteq(1, list_count_items(&uc->dev_head));
+ /*
+ * These are 2 pre-reloc devices:
+ * one with "u-boot,dm-pre-reloc" property (a-test node), and the other
+ * one whose driver marked with DM_FLAG_PRE_RELOC flag (h-test node).
+ */
+ ut_asserteq(2, list_count_items(&uc->dev_head));
 
  return 0;
 }
--
2.7.4

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 5/8] dm: Correct pre_reloc_only parameter description in several APIs' comments

Bin Meng
In reply to this post by Bin Meng
The pre_reloc_only parameter description currently only mentions
drivers with the DM_FLAG_PRE_RELOC flag, but does not mention the
special device tree properties. Correct them.

Signed-off-by: Bin Meng <[hidden email]>
---

Changes in v2: None

 include/dm/device-internal.h |  4 ++--
 include/dm/lists.h           |  4 ++--
 include/dm/root.h            | 17 +++++++++--------
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
index 02ac4c7..ee2b24a 100644
--- a/include/dm/device-internal.h
+++ b/include/dm/device-internal.h
@@ -74,8 +74,8 @@ int device_bind_with_driver_data(struct udevice *parent,
  * tree.
  *
  * @parent: Pointer to device's parent
- * @pre_reloc_only: If true, bind the driver only if its DM_INIT_F flag is set.
- * If false bind the driver always.
+ * @pre_reloc_only: If true, bind the driver only if its DM_FLAG_PRE_RELOC flag
+ * is set. If false bind the driver always.
  * @info: Name and platdata for this device
  * @devp: if non-NULL, returns a pointer to the bound device
  * @return 0 if OK, -ve on error
diff --git a/include/dm/lists.h b/include/dm/lists.h
index 59094d7..810e244 100644
--- a/include/dm/lists.h
+++ b/include/dm/lists.h
@@ -39,8 +39,8 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id);
  * each one. The devices will have @parent as their parent.
  *
  * @parent: parent device (root)
- * @early_only: If true, bind only drivers with the DM_INIT_F flag. If false
- * bind all drivers.
+ * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC flag.
+ * If false bind all drivers.
  */
 int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only);
 
diff --git a/include/dm/root.h b/include/dm/root.h
index 2b9c6da..c8d629b 100644
--- a/include/dm/root.h
+++ b/include/dm/root.h
@@ -48,8 +48,8 @@ int dm_scan_platdata(bool pre_reloc_only);
  * the top-level subnodes are examined.
  *
  * @blob: Pointer to device tree blob
- * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
- * flag. If false bind all drivers.
+ * @pre_reloc_only: If true, bind only nodes with special devicetree properties,
+ * or drivers with the DM_FLAG_PRE_RELOC flag. If false bind all drivers.
  * @return 0 if OK, -ve on error
  */
 int dm_scan_fdt(const void *blob, bool pre_reloc_only);
@@ -62,8 +62,8 @@ int dm_scan_fdt(const void *blob, bool pre_reloc_only);
  * of "clocks" node.
  *
  * @blob: Pointer to device tree blob
- * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
- * flag. If false bind all drivers.
+ * @pre_reloc_only: If true, bind only nodes with special devicetree properties,
+ * or drivers with the DM_FLAG_PRE_RELOC flag. If false bind all drivers.
  * @return 0 if OK, -ve on error
  */
 int dm_extended_scan_fdt(const void *blob, bool pre_reloc_only);
@@ -76,8 +76,9 @@ int dm_extended_scan_fdt(const void *blob, bool pre_reloc_only);
  * programmaticaly. They should do this by calling device_bind() on each
  * device.
  *
- * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
- * flag. If false bind all drivers.
+ * @pre_reloc_only: If true, bind only nodes with special devicetree properties,
+ * or drivers with the DM_FLAG_PRE_RELOC flag. If false bind all drivers.
+ * @return 0 if OK, -ve on error
  */
 int dm_scan_other(bool pre_reloc_only);
 
@@ -88,8 +89,8 @@ int dm_scan_other(bool pre_reloc_only);
  * then scans and binds available devices from platform data and the FDT.
  * This calls dm_init() to set up Driver Model structures.
  *
- * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
- * flag. If false bind all drivers.
+ * @pre_reloc_only: If true, bind only nodes with special devicetree properties,
+ * or drivers with the DM_FLAG_PRE_RELOC flag. If false bind all drivers.
  * @return 0 if OK, -ve on error
  */
 int dm_init_and_scan(bool pre_reloc_only);
--
2.7.4

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/8] dm: cpu: Fix print_cpuinfo() output

Simon Glass-3
In reply to this post by Bin Meng
On 7 October 2018 at 04:01, Bin Meng <[hidden email]> wrote:

> It was observed that current output of print_cpuinfo() on QEMU
> x86 targets does not have an ending '\n', neither have a leading
> 'CPU:' any more. However it used to have these before.
>
> It turns out commit c0434407b595 introduced a unified DM version
> of print_cpuinfo() that exposed such issue on QEMU x86.
>
> Fixes: c0434407b595 ("board_f: Use static print_cpuinfo if CONFIG_CPU is active")
> Signed-off-by: Bin Meng <[hidden email]>
> ---
>
> Changes in v2: None
>
>  common/board_f.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <[hidden email]>
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 2/8] cpu: mpc83xx: Remove unnecessary characters in the description string

Simon Glass-3
In reply to this post by Bin Meng
On 7 October 2018 at 04:01, Bin Meng <[hidden email]> wrote:

> The description string should not contain unnecessary characters,
> like the ending '\n' or the leading 'CPU:'.
>
> Signed-off-by: Bin Meng <[hidden email]>
> ---
>
> Changes in v2: None
>
>  drivers/cpu/mpc83xx_cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <[hidden email]>
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 3/8] dm: util: Add a livetree equivalent API of dm_fdt_pre_reloc()

Simon Glass-3
In reply to this post by Bin Meng
On 7 October 2018 at 04:01, Bin Meng <[hidden email]> wrote:

> This adds a new API dm_ofnode_pre_reloc(), a livetree equivalent
> API of dm_fdt_pre_reloc().
>
> Signed-off-by: Bin Meng <[hidden email]>
> ---
>
> Changes in v2: None
>
>  drivers/core/util.c | 25 +++++++++++++++++++++++++
>  include/dm/util.h   | 27 ++++++++++++++++++++++++++-
>  2 files changed, 51 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <[hidden email]>

I wonder if it is possible to delete the old code that doesn't support
livetree? This implementation should support both.

- Simon
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 4/8] dm: core: Respect drivers with the DM_FLAG_PRE_RELOC flag in lists_bind_fdt()

Simon Glass-3
In reply to this post by Bin Meng
On 7 October 2018 at 04:01, Bin Meng <[hidden email]> wrote:

> Currently the comments of several APIs (eg: dm_init_and_scan()) say:
>
> @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
> flag. If false bind all drivers.
>
> The 'Pre-Relocation Support' chapter in doc/driver-model/README.txt
> documents the same that both device tree properties and driver flag
> are supported.
>
> However the implementation only checks these special device tree
> properties without checking the driver flag at all. This updates
> lists_bind_fdt() to consider both scenarios.
>
> Signed-off-by: Bin Meng <[hidden email]>
>
> ---
>
> Changes in v2:
> - rebase on u-boot/master, and fix one build error
>
>  drivers/core/device.c          |  2 +-
>  drivers/core/lists.c           |  9 ++++++++-
>  drivers/core/root.c            | 12 ++++--------
>  drivers/serial/serial-uclass.c |  2 +-
>  drivers/timer/timer-uclass.c   |  2 +-
>  include/dm/lists.h             |  5 ++++-
>  6 files changed, 19 insertions(+), 13 deletions(-)

Reviewed-by: Simon Glass <[hidden email]>
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 5/8] dm: Correct pre_reloc_only parameter description in several APIs' comments

Simon Glass-3
In reply to this post by Bin Meng
On 7 October 2018 at 04:01, Bin Meng <[hidden email]> wrote:

> The pre_reloc_only parameter description currently only mentions
> drivers with the DM_FLAG_PRE_RELOC flag, but does not mention the
> special device tree properties. Correct them.
>
> Signed-off-by: Bin Meng <[hidden email]>
> ---
>
> Changes in v2: None
>
>  include/dm/device-internal.h |  4 ++--
>  include/dm/lists.h           |  4 ++--
>  include/dm/root.h            | 17 +++++++++--------
>  3 files changed, 13 insertions(+), 12 deletions(-)

Reviewed-by: Simon Glass <[hidden email]>
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 6/8] dm: core: Mirror the chosen node parse logic in the livetree scanning

Simon Glass-3
In reply to this post by Bin Meng
On 7 October 2018 at 04:01, Bin Meng <[hidden email]> wrote:

> Commit f2006808f099: ("dm: core: parse chosen node") added a logic
> to parse the chosen node during dm_scan_fdt_node(), but unfortunately
> it missed adding the same logic in dm_scan_fdt_live(). This mirrors
> the logic in the livetree version.
>
> The weird thing is that commit f2006808f099 did update the test case
> to test such logic, but I have no idea how the test case could pass.
>
> With this fix, the following 2 test cases now pass:
>
> Test: dm_test_bus_children: bus.c
> test/dm/bus.c:112, dm_test_bus_children(): num_devices ==
> list_count_items(&uc->dev_head): Expected 7, got 6
>
> Test: dm_test_fdt: test-fdt.c
> test/dm/test-fdt.c:184, dm_test_fdt(): num_devices ==
> list_count_items(&uc->dev_head): Expected 7, got 6
>
> Fixes: f2006808f099 ("dm: core: parse chosen node")
> Signed-off-by: Bin Meng <[hidden email]>
> ---
>
> Changes in v2: None
>
>  drivers/core/root.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

I actually don't see that failure. Something odd is going on. Anyway
this looks right to me.

Reviewed-by: Simon Glass <[hidden email]>
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 7/8] test: dm: core: Add a test case for driver marked with DM_FLAG_PRE_RELOC flag

Simon Glass-3
In reply to this post by Bin Meng
On 7 October 2018 at 04:01, Bin Meng <[hidden email]> wrote:

> Now that we fixed the pre-relocation driver binding for driver marked
> with DM_FLAG_PRE_RELOC flag, add a test case to cover that scenario.
>
> Signed-off-by: Bin Meng <[hidden email]>
> ---
>
> Changes in v2: None
>
>  arch/sandbox/dts/test.dts |  4 ++++
>  test/dm/bus.c             |  2 +-
>  test/dm/test-fdt.c        | 29 ++++++++++++++++++++++++++---
>  3 files changed, 31 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <[hidden email]>
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 8/8] timer: Sort Kconfig driver entries

Simon Glass-3
In reply to this post by Bin Meng
On 7 October 2018 at 04:01, Bin Meng <[hidden email]> wrote:

> This is currently out of order. Sort it.
>
> Signed-off-by: Bin Meng <[hidden email]>
>
> ---
>
> Changes in v2: None
>
>  drivers/timer/Kconfig | 110 +++++++++++++++++++++++++-------------------------
>  1 file changed, 55 insertions(+), 55 deletions(-)

Reviewed-by: Simon Glass <[hidden email]>
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 3/8] dm: util: Add a livetree equivalent API of dm_fdt_pre_reloc()

Bin Meng
In reply to this post by Simon Glass-3
Hi Simon,

On Thu, Oct 11, 2018 at 10:01 AM Simon Glass <[hidden email]> wrote:

>
> On 7 October 2018 at 04:01, Bin Meng <[hidden email]> wrote:
> > This adds a new API dm_ofnode_pre_reloc(), a livetree equivalent
> > API of dm_fdt_pre_reloc().
> >
> > Signed-off-by: Bin Meng <[hidden email]>
> > ---
> >
> > Changes in v2: None
> >
> >  drivers/core/util.c | 25 +++++++++++++++++++++++++
> >  include/dm/util.h   | 27 ++++++++++++++++++++++++++-
> >  2 files changed, 51 insertions(+), 1 deletion(-)
>
> Reviewed-by: Simon Glass <[hidden email]>
>
> I wonder if it is possible to delete the old code that doesn't support
> livetree? This implementation should support both.

Agreed. But there are two drivers (drivers/clk/at91/pmc.c and
drivers/clk/altera/clk-arria10.c) that calls dm_fdt_pre_reloc() at
this point. We can clean this up in the future.

Regards,
Bin
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 2/8] cpu: mpc83xx: Remove unnecessary characters in the description string

Mario Six
In reply to this post by Simon Glass-3
On Thu, Oct 11, 2018 at 4:02 AM Simon Glass <[hidden email]> wrote:

>
> On 7 October 2018 at 04:01, Bin Meng <[hidden email]> wrote:
> > The description string should not contain unnecessary characters,
> > like the ending '\n' or the leading 'CPU:'.
> >
> > Signed-off-by: Bin Meng <[hidden email]>
> > ---
> >
> > Changes in v2: None
> >
> >  drivers/cpu/mpc83xx_cpu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Reviewed-by: Simon Glass <[hidden email]>
>
Reviewed-by: Mario Six <[hidden email]>
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot