| Message ID | 20170301102358.9127-1-sr@denx.de |
|---|---|
| State | RFC |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Stefan, On 1 March 2017 at 03:23, Stefan Roese <sr@denx.de> wrote: > This patch adds the pre_os_remove boolean flag to device_remove() and > changes all calls to this function to provide the default value of > "false". This is in preparation for the driver specific pre-OS remove > support. > > Signed-off-by: Stefan Roese <sr@denx.de> > Cc: Simon Glass <sjg@chromium.org> > --- > arch/x86/cpu/queensbay/tnc.c | 4 ++-- > cmd/cros_ec.c | 2 +- > cmd/sf.c | 2 +- > drivers/block/blk-uclass.c | 2 +- > drivers/block/sandbox.c | 2 +- > drivers/core/device-remove.c | 9 +++++---- > drivers/core/device.c | 2 +- > drivers/core/root.c | 2 +- > drivers/core/uclass.c | 2 +- > drivers/mmc/mmc-uclass.c | 2 +- > drivers/mtd/spi/sandbox.c | 2 +- > drivers/mtd/spi/sf-uclass.c | 2 +- > drivers/spi/spi-uclass.c | 4 ++-- > drivers/usb/emul/sandbox_hub.c | 2 +- > drivers/usb/host/usb-uclass.c | 4 ++-- > include/dm/device-internal.h | 5 +++-- > include/dm/device.h | 3 +++ > test/dm/bus.c | 8 ++++---- > test/dm/core.c | 16 ++++++++-------- > test/dm/eth.c | 2 +- > test/dm/spi.c | 2 +- > 21 files changed, 42 insertions(+), 37 deletions(-) I think adding a parameter to device_remove() makes sense, but how about using flags instead? The true/false meaning is not clear here and your comment in device.h doesn't really help. Also I think it is better to name it after the required function rather than state related to the caller. IOW instead of 'pre-os' use something like 'active_dma_only' or as a flag ONLY_REMOVE_ACTIVE_DMA. Do you think the presence of DMA should be a device flag? Regards, Simon
Hi Simon, On 03.03.2017 05:53, Simon Glass wrote: > On 1 March 2017 at 03:23, Stefan Roese <sr@denx.de> wrote: >> This patch adds the pre_os_remove boolean flag to device_remove() and >> changes all calls to this function to provide the default value of >> "false". This is in preparation for the driver specific pre-OS remove >> support. >> >> Signed-off-by: Stefan Roese <sr@denx.de> >> Cc: Simon Glass <sjg@chromium.org> >> --- >> arch/x86/cpu/queensbay/tnc.c | 4 ++-- >> cmd/cros_ec.c | 2 +- >> cmd/sf.c | 2 +- >> drivers/block/blk-uclass.c | 2 +- >> drivers/block/sandbox.c | 2 +- >> drivers/core/device-remove.c | 9 +++++---- >> drivers/core/device.c | 2 +- >> drivers/core/root.c | 2 +- >> drivers/core/uclass.c | 2 +- >> drivers/mmc/mmc-uclass.c | 2 +- >> drivers/mtd/spi/sandbox.c | 2 +- >> drivers/mtd/spi/sf-uclass.c | 2 +- >> drivers/spi/spi-uclass.c | 4 ++-- >> drivers/usb/emul/sandbox_hub.c | 2 +- >> drivers/usb/host/usb-uclass.c | 4 ++-- >> include/dm/device-internal.h | 5 +++-- >> include/dm/device.h | 3 +++ >> test/dm/bus.c | 8 ++++---- >> test/dm/core.c | 16 ++++++++-------- >> test/dm/eth.c | 2 +- >> test/dm/spi.c | 2 +- >> 21 files changed, 42 insertions(+), 37 deletions(-) > > I think adding a parameter to device_remove() makes sense, but how > about using flags instead? The true/false meaning is not clear here > and your comment in device.h doesn't really help. So you are suggesting something like this: int device_remove(struct udevice *dev, uin32_t remove_flags); ? > Also I think it is better to name it after the required function > rather than state related to the caller. IOW instead of 'pre-os' use > something like 'active_dma_only' or as a flag ONLY_REMOVE_ACTIVE_DMA. > > Do you think the presence of DMA should be a device flag? The usage of flags instead of this pre-os parameter could make sense to me, as its much more flexible. But I'm not so sure about the flag (re-)naming to something specific like DMA. As there could be multiple reasons other than DMA related for this last-stage driver cleanup / configuration before the OS is started. E.g. if a driver needs to stop an internal timer before the OS is started, it would need to "abuse" this DMA flag to get called at the last pre-OS stage. Or is your thinking that in such cases (e.g. stopping of timer) a new flag should get introduced and added to this "remove_flags" parameter in bootm? Thanks, Stefan
Hi Stefan, On 2 March 2017 at 23:24, Stefan Roese <sr@denx.de> wrote: > Hi Simon, > > On 03.03.2017 05:53, Simon Glass wrote: >> On 1 March 2017 at 03:23, Stefan Roese <sr@denx.de> wrote: >>> This patch adds the pre_os_remove boolean flag to device_remove() and >>> changes all calls to this function to provide the default value of >>> "false". This is in preparation for the driver specific pre-OS remove >>> support. >>> >>> Signed-off-by: Stefan Roese <sr@denx.de> >>> Cc: Simon Glass <sjg@chromium.org> >>> --- >>> arch/x86/cpu/queensbay/tnc.c | 4 ++-- >>> cmd/cros_ec.c | 2 +- >>> cmd/sf.c | 2 +- >>> drivers/block/blk-uclass.c | 2 +- >>> drivers/block/sandbox.c | 2 +- >>> drivers/core/device-remove.c | 9 +++++---- >>> drivers/core/device.c | 2 +- >>> drivers/core/root.c | 2 +- >>> drivers/core/uclass.c | 2 +- >>> drivers/mmc/mmc-uclass.c | 2 +- >>> drivers/mtd/spi/sandbox.c | 2 +- >>> drivers/mtd/spi/sf-uclass.c | 2 +- >>> drivers/spi/spi-uclass.c | 4 ++-- >>> drivers/usb/emul/sandbox_hub.c | 2 +- >>> drivers/usb/host/usb-uclass.c | 4 ++-- >>> include/dm/device-internal.h | 5 +++-- >>> include/dm/device.h | 3 +++ >>> test/dm/bus.c | 8 ++++---- >>> test/dm/core.c | 16 ++++++++-------- >>> test/dm/eth.c | 2 +- >>> test/dm/spi.c | 2 +- >>> 21 files changed, 42 insertions(+), 37 deletions(-) >> >> I think adding a parameter to device_remove() makes sense, but how >> about using flags instead? The true/false meaning is not clear here >> and your comment in device.h doesn't really help. > > So you are suggesting something like this: > > int device_remove(struct udevice *dev, uin32_t remove_flags); Yes, or really 'uint remove_flags' > > ? > >> Also I think it is better to name it after the required function >> rather than state related to the caller. IOW instead of 'pre-os' use >> something like 'active_dma_only' or as a flag ONLY_REMOVE_ACTIVE_DMA. >> >> Do you think the presence of DMA should be a device flag? > > The usage of flags instead of this pre-os parameter could make > sense to me, as its much more flexible. But I'm not so sure about > the flag (re-)naming to something specific like DMA. As there > could be multiple reasons other than DMA related for this last-stage > driver cleanup / configuration before the OS is started. E.g. > if a driver needs to stop an internal timer before the OS is started, > it would need to "abuse" this DMA flag to get called at the last > pre-OS stage. Or is your thinking that in such cases (e.g. stopping > of timer) a new flag should get introduced and added to this > "remove_flags" parameter in bootm? Yes, so that it is explicit. Another approach would be: enum { DM_REMOVE_ACTIVE_ALL = 1 << 0, /* Remove all devices */ DM_REMOVE_ACTIVE_DMA = 1 << 1, /* Remove only devices with active DMA */ /* Add more use cases here */ }; Then, DM_REMOVE_ACTIVE_ALL means everything will be removed, and if that flag is not set, the other flags can be used. I am assuming that there actually will be other cases - your email suggests that could be true. Regards, Simon
Hi Simon, On 08.03.2017 22:01, Simon Glass wrote: > Hi Stefan, > > On 2 March 2017 at 23:24, Stefan Roese <sr@denx.de> wrote: >> Hi Simon, >> >> On 03.03.2017 05:53, Simon Glass wrote: >>> On 1 March 2017 at 03:23, Stefan Roese <sr@denx.de> wrote: >>>> This patch adds the pre_os_remove boolean flag to device_remove() and >>>> changes all calls to this function to provide the default value of >>>> "false". This is in preparation for the driver specific pre-OS remove >>>> support. >>>> >>>> Signed-off-by: Stefan Roese <sr@denx.de> >>>> Cc: Simon Glass <sjg@chromium.org> >>>> --- >>>> arch/x86/cpu/queensbay/tnc.c | 4 ++-- >>>> cmd/cros_ec.c | 2 +- >>>> cmd/sf.c | 2 +- >>>> drivers/block/blk-uclass.c | 2 +- >>>> drivers/block/sandbox.c | 2 +- >>>> drivers/core/device-remove.c | 9 +++++---- >>>> drivers/core/device.c | 2 +- >>>> drivers/core/root.c | 2 +- >>>> drivers/core/uclass.c | 2 +- >>>> drivers/mmc/mmc-uclass.c | 2 +- >>>> drivers/mtd/spi/sandbox.c | 2 +- >>>> drivers/mtd/spi/sf-uclass.c | 2 +- >>>> drivers/spi/spi-uclass.c | 4 ++-- >>>> drivers/usb/emul/sandbox_hub.c | 2 +- >>>> drivers/usb/host/usb-uclass.c | 4 ++-- >>>> include/dm/device-internal.h | 5 +++-- >>>> include/dm/device.h | 3 +++ >>>> test/dm/bus.c | 8 ++++---- >>>> test/dm/core.c | 16 ++++++++-------- >>>> test/dm/eth.c | 2 +- >>>> test/dm/spi.c | 2 +- >>>> 21 files changed, 42 insertions(+), 37 deletions(-) >>> >>> I think adding a parameter to device_remove() makes sense, but how >>> about using flags instead? The true/false meaning is not clear here >>> and your comment in device.h doesn't really help. >> >> So you are suggesting something like this: >> >> int device_remove(struct udevice *dev, uin32_t remove_flags); > > Yes, or really 'uint remove_flags' > >> >> ? >> >>> Also I think it is better to name it after the required function >>> rather than state related to the caller. IOW instead of 'pre-os' use >>> something like 'active_dma_only' or as a flag ONLY_REMOVE_ACTIVE_DMA. >>> >>> Do you think the presence of DMA should be a device flag? >> >> The usage of flags instead of this pre-os parameter could make >> sense to me, as its much more flexible. But I'm not so sure about >> the flag (re-)naming to something specific like DMA. As there >> could be multiple reasons other than DMA related for this last-stage >> driver cleanup / configuration before the OS is started. E.g. >> if a driver needs to stop an internal timer before the OS is started, >> it would need to "abuse" this DMA flag to get called at the last >> pre-OS stage. Or is your thinking that in such cases (e.g. stopping >> of timer) a new flag should get introduced and added to this >> "remove_flags" parameter in bootm? > > Yes, so that it is explicit. Another approach would be: > > enum { > DM_REMOVE_ACTIVE_ALL = 1 << 0, /* Remove all devices */ > DM_REMOVE_ACTIVE_DMA = 1 << 1, /* Remove only devices with active DMA */ > /* Add more use cases here */ > }; Okay, I'll go forward with this suggestion and will generate a new patchset version soon. Stay tuned... Thanks, Stefan
Hi Simon, On 10.03.2017 12:31, Stefan Roese wrote: > Hi Simon, > > On 08.03.2017 22:01, Simon Glass wrote: >> Hi Stefan, >> >> On 2 March 2017 at 23:24, Stefan Roese <sr@denx.de> wrote: >>> Hi Simon, >>> >>> On 03.03.2017 05:53, Simon Glass wrote: >>>> On 1 March 2017 at 03:23, Stefan Roese <sr@denx.de> wrote: >>>>> This patch adds the pre_os_remove boolean flag to device_remove() and >>>>> changes all calls to this function to provide the default value of >>>>> "false". This is in preparation for the driver specific pre-OS remove >>>>> support. >>>>> >>>>> Signed-off-by: Stefan Roese <sr@denx.de> >>>>> Cc: Simon Glass <sjg@chromium.org> >>>>> --- >>>>> arch/x86/cpu/queensbay/tnc.c | 4 ++-- >>>>> cmd/cros_ec.c | 2 +- >>>>> cmd/sf.c | 2 +- >>>>> drivers/block/blk-uclass.c | 2 +- >>>>> drivers/block/sandbox.c | 2 +- >>>>> drivers/core/device-remove.c | 9 +++++---- >>>>> drivers/core/device.c | 2 +- >>>>> drivers/core/root.c | 2 +- >>>>> drivers/core/uclass.c | 2 +- >>>>> drivers/mmc/mmc-uclass.c | 2 +- >>>>> drivers/mtd/spi/sandbox.c | 2 +- >>>>> drivers/mtd/spi/sf-uclass.c | 2 +- >>>>> drivers/spi/spi-uclass.c | 4 ++-- >>>>> drivers/usb/emul/sandbox_hub.c | 2 +- >>>>> drivers/usb/host/usb-uclass.c | 4 ++-- >>>>> include/dm/device-internal.h | 5 +++-- >>>>> include/dm/device.h | 3 +++ >>>>> test/dm/bus.c | 8 ++++---- >>>>> test/dm/core.c | 16 ++++++++-------- >>>>> test/dm/eth.c | 2 +- >>>>> test/dm/spi.c | 2 +- >>>>> 21 files changed, 42 insertions(+), 37 deletions(-) >>>> >>>> I think adding a parameter to device_remove() makes sense, but how >>>> about using flags instead? The true/false meaning is not clear here >>>> and your comment in device.h doesn't really help. >>> >>> So you are suggesting something like this: >>> >>> int device_remove(struct udevice *dev, uin32_t remove_flags); >> >> Yes, or really 'uint remove_flags' >> >>> >>> ? >>> >>>> Also I think it is better to name it after the required function >>>> rather than state related to the caller. IOW instead of 'pre-os' use >>>> something like 'active_dma_only' or as a flag ONLY_REMOVE_ACTIVE_DMA. >>>> >>>> Do you think the presence of DMA should be a device flag? >>> >>> The usage of flags instead of this pre-os parameter could make >>> sense to me, as its much more flexible. But I'm not so sure about >>> the flag (re-)naming to something specific like DMA. As there >>> could be multiple reasons other than DMA related for this last-stage >>> driver cleanup / configuration before the OS is started. E.g. >>> if a driver needs to stop an internal timer before the OS is started, >>> it would need to "abuse" this DMA flag to get called at the last >>> pre-OS stage. Or is your thinking that in such cases (e.g. stopping >>> of timer) a new flag should get introduced and added to this >>> "remove_flags" parameter in bootm? >> >> Yes, so that it is explicit. Another approach would be: >> >> enum { >> DM_REMOVE_ACTIVE_ALL = 1 << 0, /* Remove all devices */ >> DM_REMOVE_ACTIVE_DMA = 1 << 1, /* Remove only devices with active >> DMA */ >> /* Add more use cases here */ >> }; > > Okay, I'll go forward with this suggestion and will generate a new > patchset version soon. Stay tuned... Thinking about it, we need a bit for the "normal" remove case as well. How about this naming: enum { DM_REMOVE_NORMAL = 1 << 0, /* Remove all devices */ DM_REMOVE_ACTIVE_ALL = 1 << 1, /* Remove devices with any active flag set */ DM_REMOVE_ACTIVE_DMA = 1 << 2, /* Remove only devices with active DMA */ /* Add more use cases here */ }; Or do you have another suggestion in mind for this "normal" device remove case? Thanks, Stefan
diff --git a/arch/x86/cpu/queensbay/tnc.c b/arch/x86/cpu/queensbay/tnc.c index f307c622c8..32157f4347 100644 --- a/arch/x86/cpu/queensbay/tnc.c +++ b/arch/x86/cpu/queensbay/tnc.c @@ -76,13 +76,13 @@ static int __maybe_unused disable_igd(void) * * So the only option we have is to manually remove these two devices. */ - ret = device_remove(igd); + ret = device_remove(igd, false); if (ret) return ret; ret = device_unbind(igd); if (ret) return ret; - ret = device_remove(sdvo); + ret = device_remove(sdvo, false); if (ret) return ret; ret = device_unbind(sdvo); diff --git a/cmd/cros_ec.c b/cmd/cros_ec.c index 9d42f870dc..abadfee860 100644 --- a/cmd/cros_ec.c +++ b/cmd/cros_ec.c @@ -110,7 +110,7 @@ static int do_cros_ec(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) /* Remove any existing device */ ret = uclass_find_device(UCLASS_CROS_EC, 0, &udev); if (!ret) - device_remove(udev); + device_remove(udev, false); ret = uclass_get_device(UCLASS_CROS_EC, 0, &udev); if (ret) { printf("Could not init cros_ec device (err %d)\n", ret); diff --git a/cmd/sf.c b/cmd/sf.c index 65b117feee..15e5b435fa 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -124,7 +124,7 @@ static int do_spi_flash_probe(int argc, char * const argv[]) /* Remove the old device, otherwise probe will just be a nop */ ret = spi_find_bus_and_cs(bus, cs, &bus_dev, &new); if (!ret) { - device_remove(new); + device_remove(new, false); } flash = NULL; ret = spi_flash_probe_bus_cs(bus, cs, speed, mode, &new); diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index 38cb9388da..41fd983976 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -530,7 +530,7 @@ int blk_unbind_all(int if_type) struct blk_desc *desc = dev_get_uclass_platdata(dev); if (desc->if_type == if_type) { - ret = device_remove(dev); + ret = device_remove(dev, false); if (ret) return ret; ret = device_unbind(dev); diff --git a/drivers/block/sandbox.c b/drivers/block/sandbox.c index 36c2ff3007..1e78d0bbb3 100644 --- a/drivers/block/sandbox.c +++ b/drivers/block/sandbox.c @@ -98,7 +98,7 @@ int host_dev_bind(int devnum, char *filename) /* Remove and unbind the old device, if any */ ret = blk_get_device(IF_TYPE_HOST, devnum, &dev); if (ret == 0) { - ret = device_remove(dev); + ret = device_remove(dev, false); if (ret) return ret; ret = device_unbind(dev); diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index a7f77b4a21..4725d4751c 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -46,9 +46,10 @@ static int device_chld_unbind(struct udevice *dev) /** * device_chld_remove() - Stop all device's children * @dev: The device whose children are to be removed + * @pre_os_remove: Flag, if this functions is called in the pre-OS stage * @return 0 on success, -ve on error */ -static int device_chld_remove(struct udevice *dev) +static int device_chld_remove(struct udevice *dev, bool pre_os_remove) { struct udevice *pos, *n; int ret; @@ -56,7 +57,7 @@ static int device_chld_remove(struct udevice *dev) assert(dev); list_for_each_entry_safe(pos, n, &dev->child_head, sibling_node) { - ret = device_remove(pos); + ret = device_remove(pos, pre_os_remove); if (ret) return ret; } @@ -151,7 +152,7 @@ void device_free(struct udevice *dev) devres_release_probe(dev); } -int device_remove(struct udevice *dev) +int device_remove(struct udevice *dev, bool pre_os_remove) { const struct driver *drv; int ret; @@ -169,7 +170,7 @@ int device_remove(struct udevice *dev) if (ret) return ret; - ret = device_chld_remove(dev); + ret = device_chld_remove(dev, pre_os_remove); if (ret) goto err; diff --git a/drivers/core/device.c b/drivers/core/device.c index 70fcfc23e0..bede787b1b 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -378,7 +378,7 @@ int device_probe(struct udevice *dev) return 0; fail_uclass: - if (device_remove(dev)) { + if (device_remove(dev, false)) { dm_warn("%s: Device '%s' failed to remove on error path\n", __func__, dev->name); } diff --git a/drivers/core/root.c b/drivers/core/root.c index 175fd3fb25..583daa88b0 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -176,7 +176,7 @@ int dm_init(void) int dm_uninit(void) { - device_remove(dm_root()); + device_remove(dm_root(), false); device_unbind(dm_root()); return 0; diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index 7de370644d..61144174ae 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -116,7 +116,7 @@ int uclass_destroy(struct uclass *uc) while (!list_empty(&uc->dev_head)) { dev = list_first_entry(&uc->dev_head, struct udevice, uclass_node); - ret = device_remove(dev); + ret = device_remove(dev, false); if (ret) return ret; ret = device_unbind(dev); diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c index 5bb446bcc2..9f75fff5a5 100644 --- a/drivers/mmc/mmc-uclass.c +++ b/drivers/mmc/mmc-uclass.c @@ -232,7 +232,7 @@ int mmc_unbind(struct udevice *dev) device_find_first_child(dev, &bdev); if (bdev) { - device_remove(bdev); + device_remove(bdev, false); device_unbind(bdev); } diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c index 36a50fe3a1..8cf6bdc4df 100644 --- a/drivers/mtd/spi/sandbox.c +++ b/drivers/mtd/spi/sandbox.c @@ -595,7 +595,7 @@ void sandbox_sf_unbind_emul(struct sandbox_state *state, int busnum, int cs) struct udevice *dev; dev = state->spi[busnum][cs].emul; - device_remove(dev); + device_remove(dev, false); device_unbind(dev); state->spi[busnum][cs].emul = NULL; } diff --git a/drivers/mtd/spi/sf-uclass.c b/drivers/mtd/spi/sf-uclass.c index 19de964e61..55eba5ed79 100644 --- a/drivers/mtd/spi/sf-uclass.c +++ b/drivers/mtd/spi/sf-uclass.c @@ -46,7 +46,7 @@ struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs, void spi_flash_free(struct spi_flash *flash) { - device_remove(flash->spi->dev); + device_remove(flash->spi->dev, false); } int spi_flash_probe_bus_cs(unsigned int busnum, unsigned int cs, diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c index ac17da0777..900590f93d 100644 --- a/drivers/spi/spi-uclass.c +++ b/drivers/spi/spi-uclass.c @@ -343,7 +343,7 @@ err: debug("%s: Error path, created=%d, device '%s'\n", __func__, created, dev->name); if (created) { - device_remove(dev); + device_remove(dev, false); device_unbind(dev); } @@ -384,7 +384,7 @@ struct spi_slave *spi_setup_slave(unsigned int busnum, unsigned int cs, void spi_free_slave(struct spi_slave *slave) { - device_remove(slave->dev); + device_remove(slave->dev, false); slave->dev = NULL; } diff --git a/drivers/usb/emul/sandbox_hub.c b/drivers/usb/emul/sandbox_hub.c index c3a8e73389..def5591904 100644 --- a/drivers/usb/emul/sandbox_hub.c +++ b/drivers/usb/emul/sandbox_hub.c @@ -156,7 +156,7 @@ static int clrset_post_state(struct udevice *hub, int port, int clear, int set) } else if (clear & USB_PORT_STAT_POWER) { debug("%s: %s: power off, removed, ret=%d\n", __func__, dev->name, ret); - ret = device_remove(dev); + ret = device_remove(dev, false); clear |= USB_PORT_STAT_CONNECTION; } } diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 5cf1e9a36c..763cb24e6c 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -154,7 +154,7 @@ int usb_stop(void) uc_priv = uc->priv; uclass_foreach_dev(bus, uc) { - ret = device_remove(bus); + ret = device_remove(bus, false); if (ret && !err) err = ret; } @@ -358,7 +358,7 @@ int usb_setup_ehci_gadget(struct ehci_ctrl **ctlrp) ret = uclass_find_device_by_seq(UCLASS_USB, 0, true, &dev); if (ret) return ret; - ret = device_remove(dev); + ret = device_remove(dev, false); if (ret) return ret; diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index 0bf8707493..0518304b34 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -96,12 +96,13 @@ int device_probe(struct udevice *dev); * children are deactivated first. * * @dev: Pointer to device to remove + * @pre_os_remove: Flag, if this functions is called in the pre-OS stage * @return 0 if OK, -ve on error (an error here is normally a very bad thing) */ #if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE) -int device_remove(struct udevice *dev); +int device_remove(struct udevice *dev, bool pre_os_remove); #else -static inline int device_remove(struct udevice *dev) { return 0; } +static inline int device_remove(struct udevice *dev, bool pre_os_remove) { return 0; } #endif /** diff --git a/include/dm/device.h b/include/dm/device.h index 4e95fb7773..037eaa4423 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -46,6 +46,9 @@ struct driver_info; #define DM_FLAG_OF_PLATDATA (1 << 8) +/* Call driver remove function before the OS is called (U-Boot exit) */ +#define DM_FLAG_PRE_OS_REMOVE (1 << 9) + /** * struct udevice - An instance of a driver * diff --git a/test/dm/bus.c b/test/dm/bus.c index d94dcf7a60..03acc7ce47 100644 --- a/test/dm/bus.c +++ b/test/dm/bus.c @@ -222,7 +222,7 @@ static int test_bus_parent_data(struct unit_test_state *uts) /* Check that it starts at 0 and goes away when device is removed */ parent_data->sum += 5; ut_asserteq(5, parent_data->sum); - device_remove(dev); + device_remove(dev, false); ut_asserteq_ptr(NULL, dev_get_parent_priv(dev)); /* Check that we can do this twice */ @@ -323,7 +323,7 @@ static int dm_test_bus_parent_ops(struct unit_test_state *uts) continue; parent_data = dev_get_parent_priv(dev); ut_asserteq(FLAG_CHILD_PROBED, parent_data->flag); - ut_assertok(device_remove(dev)); + ut_assertok(device_remove(dev, false)); ut_asserteq_ptr(NULL, dev_get_parent_priv(dev)); ut_asserteq_ptr(dms->removed, dev); } @@ -360,7 +360,7 @@ static int test_bus_parent_platdata(struct unit_test_state *uts) plat->count++; ut_asserteq(1, plat->count); device_probe(dev); - device_remove(dev); + device_remove(dev, false); ut_asserteq_ptr(plat, dev_get_parent_platdata(dev)); ut_asserteq(1, plat->count); @@ -370,7 +370,7 @@ static int test_bus_parent_platdata(struct unit_test_state *uts) ut_asserteq(3, child_count); /* Removing the bus should also have no effect (it is still bound) */ - device_remove(bus); + device_remove(bus, false); for (device_find_first_child(bus, &dev), child_count = 0; dev; device_find_next_child(&dev)) { diff --git a/test/dm/core.c b/test/dm/core.c index 70bf4d0605..2ccab09b5b 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -319,7 +319,7 @@ static int dm_test_lifecycle(struct unit_test_state *uts) /* Now remove device 3 */ ut_asserteq(0, dm_testdrv_op_count[DM_TEST_OP_PRE_REMOVE]); - ut_assertok(device_remove(dev)); + ut_assertok(device_remove(dev, false)); ut_asserteq(1, dm_testdrv_op_count[DM_TEST_OP_PRE_REMOVE]); ut_asserteq(0, dm_testdrv_op_count[DM_TEST_OP_UNBIND]); @@ -352,7 +352,7 @@ static int dm_test_ordering(struct unit_test_state *uts) ut_assert(dev_last); /* Now remove device 3 */ - ut_assertok(device_remove(dev)); + ut_assertok(device_remove(dev, false)); ut_assertok(device_unbind(dev)); /* The device numbering should have shifted down one */ @@ -371,9 +371,9 @@ static int dm_test_ordering(struct unit_test_state *uts) ut_assert(pingret == 102); /* Remove 3 and 4 */ - ut_assertok(device_remove(dev_penultimate)); + ut_assertok(device_remove(dev_penultimate, false)); ut_assertok(device_unbind(dev_penultimate)); - ut_assertok(device_remove(dev_last)); + ut_assertok(device_remove(dev_last, false)); ut_assertok(device_unbind(dev_last)); /* Our device should now be in position 3 */ @@ -381,7 +381,7 @@ static int dm_test_ordering(struct unit_test_state *uts) ut_assert(dev == test_dev); /* Now remove device 3 */ - ut_assertok(device_remove(dev)); + ut_assertok(device_remove(dev, false)); ut_assertok(device_unbind(dev)); return 0; @@ -457,7 +457,7 @@ static int dm_test_remove(struct unit_test_state *uts) ut_assert(dev); ut_assertf(dev->flags & DM_FLAG_ACTIVATED, "Driver %d/%s not activated", i, dev->name); - ut_assertok(device_remove(dev)); + ut_assertok(device_remove(dev, false)); ut_assertf(!(dev->flags & DM_FLAG_ACTIVATED), "Driver %d/%s should have deactivated", i, dev->name); @@ -611,14 +611,14 @@ static int dm_test_children(struct unit_test_state *uts) ut_asserteq(total, dm_testdrv_op_count[DM_TEST_OP_PROBE]); /* Remove a top-level child and check that the children are removed */ - ut_assertok(device_remove(top[2])); + ut_assertok(device_remove(top[2], false)); ut_asserteq(NODE_COUNT + 1, dm_testdrv_op_count[DM_TEST_OP_REMOVE]); dm_testdrv_op_count[DM_TEST_OP_REMOVE] = 0; /* Try one with grandchildren */ ut_assertok(uclass_get_device(UCLASS_TEST, 5, &dev)); ut_asserteq_ptr(dev, top[5]); - ut_assertok(device_remove(dev)); + ut_assertok(device_remove(dev, false)); ut_asserteq(1 + NODE_COUNT * (1 + NODE_COUNT), dm_testdrv_op_count[DM_TEST_OP_REMOVE]); diff --git a/test/dm/eth.c b/test/dm/eth.c index 6288ae24ab..674bdb817d 100644 --- a/test/dm/eth.c +++ b/test/dm/eth.c @@ -116,7 +116,7 @@ static int dm_test_eth_act(struct unit_test_state *uts) for (i = 0; i < DM_TEST_ETH_NUM; i++) { ut_assertok(uclass_find_device_by_name(UCLASS_ETH, ethname[i], &dev[i])); - ut_assertok(device_remove(dev[i])); + ut_assertok(device_remove(dev[i], false)); /* Invalidate MAC address */ strcpy(ethaddr[i], getenv(addrname[i])); diff --git a/test/dm/spi.c b/test/dm/spi.c index f52cb733c7..078a29eaad 100644 --- a/test/dm/spi.c +++ b/test/dm/spi.c @@ -36,7 +36,7 @@ static int dm_test_spi_find(struct unit_test_state *uts) ut_asserteq(0, uclass_get_device_by_seq(UCLASS_SPI, busnum, &bus)); ut_assertok(spi_cs_info(bus, cs, &info)); of_offset = dev_of_offset(info.dev); - device_remove(info.dev); + device_remove(info.dev, false); device_unbind(info.dev); /*
This patch adds the pre_os_remove boolean flag to device_remove() and changes all calls to this function to provide the default value of "false". This is in preparation for the driver specific pre-OS remove support. Signed-off-by: Stefan Roese <sr@denx.de> Cc: Simon Glass <sjg@chromium.org> --- arch/x86/cpu/queensbay/tnc.c | 4 ++-- cmd/cros_ec.c | 2 +- cmd/sf.c | 2 +- drivers/block/blk-uclass.c | 2 +- drivers/block/sandbox.c | 2 +- drivers/core/device-remove.c | 9 +++++---- drivers/core/device.c | 2 +- drivers/core/root.c | 2 +- drivers/core/uclass.c | 2 +- drivers/mmc/mmc-uclass.c | 2 +- drivers/mtd/spi/sandbox.c | 2 +- drivers/mtd/spi/sf-uclass.c | 2 +- drivers/spi/spi-uclass.c | 4 ++-- drivers/usb/emul/sandbox_hub.c | 2 +- drivers/usb/host/usb-uclass.c | 4 ++-- include/dm/device-internal.h | 5 +++-- include/dm/device.h | 3 +++ test/dm/bus.c | 8 ++++---- test/dm/core.c | 16 ++++++++-------- test/dm/eth.c | 2 +- test/dm/spi.c | 2 +- 21 files changed, 42 insertions(+), 37 deletions(-)