Hi Stefan,
On 2 March 2017 at 23:24, Stefan Roese <s...@denx.de> wrote:
> Hi Simon,
>
> On 03.03.2017 05:53, Simon Glass wrote:
>> On 1 March 2017 at 03:23, Stefan Roese <s...@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 <s...@denx.de>
>>> Cc: Simon Glass <s...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
