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