Hi Simon, On 31.10.26 09:47, Simon Glass wrote: > Hi Frieder, > > On 2026-10-27T14:16:39+11:00, Frieder Schrempf <[email protected]> wrote: >> blk: Unbind partition devices when removing a block device >> >> blk_post_probe() binds a UCLASS_PARTITION child for each partition >> found on the device. Removing the block device removes these children, >> but does not unbind them. When the block device is probed again, a >> second set of partition devices is bound next to the first one. >> >> This happens for example when the probe of a block device fails in the >> EVT_DM_POST_PROBE event and is retried later. The EFI disk code then >> iterates over all children and tries to install a second EFI handle >> with the same device path for the stale partition device, which fails >> with EFI_ALREADY_STARTED: >> >> Adding partition for [email protected]:1 failed >> switch to partitions #0, ERROR >> >> As every further probe adds another set of partition devices, the >> block device can't be used anymore until the next reset. This was seen >> on an i.MX93 board where a single failed read during the first probe >> of the SD card made it unusable for the rest of the boot. >> >> Unbind the partition devices in the pre_remove hook of the block >> uclass, so that they are bound and unbound together with the probe and >> remove of the block device. When only some devices are removed, like >> before booting an OS, the partition devices don't match the remove >> flags and are still active at this point, so remove them first. > > I don't think this is true any more. Since commit 3632b5d63c8 ("dm: > Remove children when parent is removed by flags"), when the parent > matches the flags, device_remove() removes the children with > DM_REMOVE_ACTIVE_ALL masked out, before uclass_pre_remove_device() is > called. The partition driver is not vital, so by the time > blk_pre_remove() runs the partition devices are always inactive. > Please can you drop this sentence? > >> [...] >> >> drivers/block/blk-uclass.c | 22 ++++++++++++++++++++++ >> 1 file changed, 22 insertions(+) > >> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c >> @@ -841,9 +841,31 @@ static int blk_post_probe(struct udevice *dev) >> + /* >> + * The partition devices are bound in blk_post_probe(), so unbind them >> + * here. Otherwise the next probe adds a second set of them. They are >> + * still active if the remove flags don't match their driver, so >> + * remove them first. >> + */ >> + ret = device_chld_remove(dev, drv, DM_REMOVE_NORMAL); >> + if (ret) >> + return ret; >> + >> + return device_chld_unbind(dev, drv); > > For the same reason, the device_chld_remove() call does nothing, since > device_remove() has already removed all the children by this point. > You could just call device_chld_unbind() and trim the last sentence of > the comment. If you want to keep the call as a safety net, please > change the comment so it doesn't say the partitions can still be > active here. If they ever were, device_unbind() would fail with > -EINVAL rather than leave things in a bad state.
Thanks for pointing that out. I was forward porting this from an older version and missed the upstream 3632b5d63c8. I addressed this in v2. Best regards Frieder
