[PATCH 0/3] Adjust how autoprobe is implemented
This little series makes a minor change to how autoprobe is implemeneted, as discussed on the list.
SPL is left out for now, but we can discuss how best to deal with that.
Simon Glass (3): common: Drop check for DM in initf_dm() dm: core: Simplify dm_probe_devices() common: Move autoprobe out to board init
common/board_f.c | 9 +++++++-- common/board_r.c | 4 ++++ drivers/core/root.c | 27 ++++++++++++++++----------- include/dm/root.h | 10 ++++++++++ 4 files changed, 37 insertions(+), 13 deletions(-)
This is enabled by all boards, so drop the condition.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/board_f.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/common/board_f.c b/common/board_f.c index f1bd70fdd6c..8684ed71284 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -811,9 +811,11 @@ static int initf_bootstage(void)
static int initf_dm(void) { -#if defined(CONFIG_DM) && CONFIG_IS_ENABLED(SYS_MALLOC_F) int ret;
+ if (!CONFIG_IS_ENABLED(SYS_MALLOC_F)) + return 0; + bootstage_start(BOOTSTAGE_ID_ACCUM_DM_F, "dm_f"); ret = dm_init_and_scan(true); bootstage_accum(BOOTSTAGE_ID_ACCUM_DM_F); @@ -825,7 +827,6 @@ static int initf_dm(void) if (ret) return ret; } -#endif
return 0; }
On Mon, Oct 21, 2024 at 05:51:29PM +0200, Simon Glass wrote:
This is enabled by all boards, so drop the condition.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com
There is no point in checking the pre_reloc flag, since devices not marked as pre-reloc will not have been bound, so won't exist yet.
There doesn't seem to be any point in checking if the device has a valid devicetree node either, so drop that too.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/root.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/core/root.c b/drivers/core/root.c index 7a714f5478a..2d4f078f97f 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -281,26 +281,20 @@ void *dm_priv_to_rw(void *priv) } #endif
-static int dm_probe_devices(struct udevice *dev, bool pre_reloc_only) +static int dm_probe_devices(struct udevice *dev) { - ofnode node = dev_ofnode(dev); struct udevice *child; - int ret; - - if (pre_reloc_only && - (!ofnode_valid(node) || !ofnode_pre_reloc(node)) && - !(dev->driver->flags & DM_FLAG_PRE_RELOC)) - goto probe_children;
if (dev_get_flags(dev) & DM_FLAG_PROBE_AFTER_BIND) { + int ret; + ret = device_probe(dev); if (ret) return ret; }
-probe_children: list_for_each_entry(child, &dev->child_head, sibling_node) - dm_probe_devices(child, pre_reloc_only); + dm_probe_devices(child);
return 0; } @@ -337,7 +331,7 @@ static int dm_scan(bool pre_reloc_only) if (ret) return ret;
- return dm_probe_devices(gd->dm_root, pre_reloc_only); + return dm_probe_devices(gd->dm_root); }
int dm_init_and_scan(bool pre_reloc_only)
Rather than doing autoprobe within the driver model code, move it out to the board-init code. This makes it clear that it is a separate step from binding devices.
For now this is always done twice, before and after relocation, but we should discuss whether it might be possible to drop the post-relocation probe.
This patch also drops autoprobe from SPL. If that is needed, we could add a call in SPL.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/board_f.c | 4 ++++ common/board_r.c | 4 ++++ drivers/core/root.c | 17 ++++++++++++++--- include/dm/root.h | 10 ++++++++++ 4 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/common/board_f.c b/common/board_f.c index 8684ed71284..6564b2ac2f7 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -822,6 +822,10 @@ static int initf_dm(void) if (ret) return ret;
+ ret = dm_autoprobe(); + if (ret) + return ret; + if (IS_ENABLED(CONFIG_TIMER_EARLY)) { ret = dm_timer_init(); if (ret) diff --git a/common/board_r.c b/common/board_r.c index e5f33f40643..a2948b7b55c 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -243,6 +243,10 @@ static int initr_dm(void) if (ret) return ret;
+ ret = dm_autoprobe(); + if (ret) + return ret; + return 0; } #endif diff --git a/drivers/core/root.c b/drivers/core/root.c index 2d4f078f97f..f6bda84b2b9 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -281,7 +281,7 @@ void *dm_priv_to_rw(void *priv) } #endif
-static int dm_probe_devices(struct udevice *dev) +static int dm_probe_devices_(struct udevice *dev) { struct udevice *child;
@@ -294,7 +294,18 @@ static int dm_probe_devices(struct udevice *dev) }
list_for_each_entry(child, &dev->child_head, sibling_node) - dm_probe_devices(child); + dm_probe_devices_(child); + + return 0; +} + +int dm_autoprobe(void) +{ + int ret; + + ret = dm_probe_devices_(gd->dm_root); + if (ret) + return log_msg_ret("pro", ret);
return 0; } @@ -331,7 +342,7 @@ static int dm_scan(bool pre_reloc_only) if (ret) return ret;
- return dm_probe_devices(gd->dm_root); + return 0; }
int dm_init_and_scan(bool pre_reloc_only) diff --git a/include/dm/root.h b/include/dm/root.h index b2f30a842f5..2ba5104ce1f 100644 --- a/include/dm/root.h +++ b/include/dm/root.h @@ -136,6 +136,16 @@ int dm_scan_other(bool pre_reloc_only); */ int dm_init_and_scan(bool pre_reloc_only);
+/** + * dm_autoprobe() - Prove devices which are marked for probe-after-bind + * + * This probes all devices with a DM_FLAG_PROBE_AFTER_BIND flag. It checks the + * entire tree, so parent nodes need not have the flag set. + * + * Return: 0 if OK, -ve on error + */ +int dm_autoprobe(void); + /** * dm_init() - Initialise Driver Model structures *
Hi Simon,
On 2024-10-21 17:51, Simon Glass wrote:
Rather than doing autoprobe within the driver model code, move it out to the board-init code. This makes it clear that it is a separate step from binding devices.
It could probably be good to mention that after this patch the events EVT_DM_POST_INIT_R/F will happen before autoprobe instead of after.
For now this is always done twice, before and after relocation, but we should discuss whether it might be possible to drop the post-relocation probe.
If you have plans to drop post-relocation probe, I suggest you first look into having OF_LIVE working at pre-reloaction to speed up boot, the use of fdtdec is super slow in getting parent node/offset.
This patch also drops autoprobe from SPL. If that is needed, we could add a call in SPL.
This patch will break a feature used by the Pine64 PineTab2 because of this change, please restore autoprobe in SPL.
The PineTab2 have a feature to power itself off when power-on happen because of power cable was plugged in, for this to work it currently depends on DM probe after bind works in SPL.
Signed-off-by: Simon Glass sjg@chromium.org
common/board_f.c | 4 ++++ common/board_r.c | 4 ++++ drivers/core/root.c | 17 ++++++++++++++--- include/dm/root.h | 10 ++++++++++ 4 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/common/board_f.c b/common/board_f.c index 8684ed71284..6564b2ac2f7 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -822,6 +822,10 @@ static int initf_dm(void) if (ret) return ret;
- ret = dm_autoprobe();
- if (ret)
return ret;- if (IS_ENABLED(CONFIG_TIMER_EARLY)) { ret = dm_timer_init(); if (ret)
diff --git a/common/board_r.c b/common/board_r.c index e5f33f40643..a2948b7b55c 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -243,6 +243,10 @@ static int initr_dm(void) if (ret) return ret;
- ret = dm_autoprobe();
- if (ret)
return ret;- return 0;
} #endif diff --git a/drivers/core/root.c b/drivers/core/root.c index 2d4f078f97f..f6bda84b2b9 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -281,7 +281,7 @@ void *dm_priv_to_rw(void *priv) } #endif
-static int dm_probe_devices(struct udevice *dev) +static int dm_probe_devices_(struct udevice *dev)
Why is this function renamed?
Regards, Jonas
{ struct udevice *child;
@@ -294,7 +294,18 @@ static int dm_probe_devices(struct udevice *dev) }
list_for_each_entry(child, &dev->child_head, sibling_node)
dm_probe_devices(child);
dm_probe_devices_(child);- return 0;
+}
+int dm_autoprobe(void) +{
int ret;
ret = dm_probe_devices_(gd->dm_root);
if (ret)
return log_msg_ret("pro", ret);return 0;
} @@ -331,7 +342,7 @@ static int dm_scan(bool pre_reloc_only) if (ret) return ret;
- return dm_probe_devices(gd->dm_root);
- return 0;
}
int dm_init_and_scan(bool pre_reloc_only) diff --git a/include/dm/root.h b/include/dm/root.h index b2f30a842f5..2ba5104ce1f 100644 --- a/include/dm/root.h +++ b/include/dm/root.h @@ -136,6 +136,16 @@ int dm_scan_other(bool pre_reloc_only); */ int dm_init_and_scan(bool pre_reloc_only);
+/**
- dm_autoprobe() - Prove devices which are marked for probe-after-bind
- This probes all devices with a DM_FLAG_PROBE_AFTER_BIND flag. It checks the
- entire tree, so parent nodes need not have the flag set.
- Return: 0 if OK, -ve on error
- */
+int dm_autoprobe(void);
/**
- dm_init() - Initialise Driver Model structures
On Mon, Oct 21, 2024 at 06:10:09PM +0200, Jonas Karlman wrote:
Hi Simon,
On 2024-10-21 17:51, Simon Glass wrote:
Rather than doing autoprobe within the driver model code, move it out to the board-init code. This makes it clear that it is a separate step from binding devices.
It could probably be good to mention that after this patch the events EVT_DM_POST_INIT_R/F will happen before autoprobe instead of after.
And in updating some part of doc/develop/driver-model/ and referencing the discussion on the list, in the commit message with some Link: lines.
And I mention the last one because I fear this is related to the regulator changes from Marek and it would be good to hear from him that this series doesn't break his devices, for example.
Hi Tom,
On Mon, 21 Oct 2024 at 20:21, Tom Rini trini@konsulko.com wrote:
On Mon, Oct 21, 2024 at 06:10:09PM +0200, Jonas Karlman wrote:
Hi Simon,
On 2024-10-21 17:51, Simon Glass wrote:
Rather than doing autoprobe within the driver model code, move it out to the board-init code. This makes it clear that it is a separate step from binding devices.
It could probably be good to mention that after this patch the events EVT_DM_POST_INIT_R/F will happen before autoprobe instead of after.
And in updating some part of doc/develop/driver-model/ and referencing the discussion on the list, in the commit message with some Link: lines.
Oh, nice, I didn't know about that feature.
And I mention the last one because I fear this is related to the regulator changes from Marek and it would be good to hear from him that this series doesn't break his devices, for example.
Yes, given him a week or so to reply, but I'd like to get this in before rc2 since the ordering thing that Jonas pointed out could conceivably cause issues.. With the updated patch, we have SPL covered though.
Regards, Simon
Hi Jonas,
On Mon, 21 Oct 2024 at 18:10, Jonas Karlman jonas@kwiboo.se wrote:
Hi Simon,
On 2024-10-21 17:51, Simon Glass wrote:
Rather than doing autoprobe within the driver model code, move it out to the board-init code. This makes it clear that it is a separate step from binding devices.
It could probably be good to mention that after this patch the events EVT_DM_POST_INIT_R/F will happen before autoprobe instead of after.
Yes, good point.
For now this is always done twice, before and after relocation, but we should discuss whether it might be possible to drop the post-relocation probe.
If you have plans to drop post-relocation probe, I suggest you first look into having OF_LIVE working at pre-reloaction to speed up boot, the use of fdtdec is super slow in getting parent node/offset.
Indeed. I keep hoping someone else will look at this.
This patch also drops autoprobe from SPL. If that is needed, we could add a call in SPL.
This patch will break a feature used by the Pine64 PineTab2 because of this change, please restore autoprobe in SPL.
The PineTab2 have a feature to power itself off when power-on happen because of power cable was plugged in, for this to work it currently depends on DM probe after bind works in SPL.
OK. We can always make it optional later. At least with this series it is explicit.
Regards, Simon
Signed-off-by: Simon Glass sjg@chromium.org
common/board_f.c | 4 ++++ common/board_r.c | 4 ++++ drivers/core/root.c | 17 ++++++++++++++--- include/dm/root.h | 10 ++++++++++ 4 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/common/board_f.c b/common/board_f.c index 8684ed71284..6564b2ac2f7 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -822,6 +822,10 @@ static int initf_dm(void) if (ret) return ret;
ret = dm_autoprobe();if (ret)return ret;if (IS_ENABLED(CONFIG_TIMER_EARLY)) { ret = dm_timer_init(); if (ret)diff --git a/common/board_r.c b/common/board_r.c index e5f33f40643..a2948b7b55c 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -243,6 +243,10 @@ static int initr_dm(void) if (ret) return ret;
ret = dm_autoprobe();if (ret)return ret;return 0;} #endif diff --git a/drivers/core/root.c b/drivers/core/root.c index 2d4f078f97f..f6bda84b2b9 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -281,7 +281,7 @@ void *dm_priv_to_rw(void *priv) } #endif
-static int dm_probe_devices(struct udevice *dev) +static int dm_probe_devices_(struct udevice *dev)
Why is this function renamed?
It's not needed, since I decided on a different name for the function. I'll change it back.
Thank you for the review on this.
I am hoping Marek will chime in too!
[..]
Regards, Simon
Hi Simon,
Just a typo/nit
On 21/10/2024 17:51, Simon Glass wrote:
Rather than doing autoprobe within the driver model code, move it out to the board-init code. This makes it clear that it is a separate step from binding devices.
For now this is always done twice, before and after relocation, but we should discuss whether it might be possible to drop the post-relocation probe.
This patch also drops autoprobe from SPL. If that is needed, we could add a call in SPL.
Signed-off-by: Simon Glass sjg@chromium.org
common/board_f.c | 4 ++++ common/board_r.c | 4 ++++ drivers/core/root.c | 17 ++++++++++++++--- include/dm/root.h | 10 ++++++++++ 4 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/common/board_f.c b/common/board_f.c index 8684ed71284..6564b2ac2f7 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -822,6 +822,10 @@ static int initf_dm(void) if (ret) return ret;
- ret = dm_autoprobe();
- if (ret)
return ret;- if (IS_ENABLED(CONFIG_TIMER_EARLY)) { ret = dm_timer_init(); if (ret)
diff --git a/common/board_r.c b/common/board_r.c index e5f33f40643..a2948b7b55c 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -243,6 +243,10 @@ static int initr_dm(void) if (ret) return ret;
- ret = dm_autoprobe();
- if (ret)
return ret;- return 0; } #endif
diff --git a/drivers/core/root.c b/drivers/core/root.c index 2d4f078f97f..f6bda84b2b9 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -281,7 +281,7 @@ void *dm_priv_to_rw(void *priv) } #endif
-static int dm_probe_devices(struct udevice *dev) +static int dm_probe_devices_(struct udevice *dev) { struct udevice *child;
@@ -294,7 +294,18 @@ static int dm_probe_devices(struct udevice *dev) }
list_for_each_entry(child, &dev->child_head, sibling_node)
dm_probe_devices(child);
dm_probe_devices_(child);- return 0;
+}
+int dm_autoprobe(void) +{
int ret;
ret = dm_probe_devices_(gd->dm_root);
if (ret)
return log_msg_ret("pro", ret);return 0; }
@@ -331,7 +342,7 @@ static int dm_scan(bool pre_reloc_only) if (ret) return ret;
- return dm_probe_devices(gd->dm_root);
return 0; }
int dm_init_and_scan(bool pre_reloc_only)
diff --git a/include/dm/root.h b/include/dm/root.h index b2f30a842f5..2ba5104ce1f 100644 --- a/include/dm/root.h +++ b/include/dm/root.h @@ -136,6 +136,16 @@ int dm_scan_other(bool pre_reloc_only); */ int dm_init_and_scan(bool pre_reloc_only);
+/**
- dm_autoprobe() - Prove devices which are marked for probe-after-bind
s/prove/probe
Kind regards,
- This probes all devices with a DM_FLAG_PROBE_AFTER_BIND flag. It checks the
- entire tree, so parent nodes need not have the flag set.
- Return: 0 if OK, -ve on error
- */
+int dm_autoprobe(void);
- /**
- dm_init() - Initialise Driver Model structures
On Mon, 21 Oct 2024 at 17:02, Simon Glass sjg@chromium.org wrote:
This little series makes a minor change to how autoprobe is implemeneted, as discussed on the list
Please add a link to the discussion when you mention a discussion.
SPL is left out for now, but we can discuss how best to deal with that.
Simon Glass (3): common: Drop check for DM in initf_dm() dm: core: Simplify dm_probe_devices() common: Move autoprobe out to board init
common/board_f.c | 9 +++++++-- common/board_r.c | 4 ++++ drivers/core/root.c | 27 ++++++++++++++++----------- include/dm/root.h | 10 ++++++++++ 4 files changed, 37 insertions(+), 13 deletions(-)
-- 2.43.0
participants (5)
-
Caleb Connolly -
Jonas Karlman -
Peter Robinson -
Simon Glass -
Tom Rini