[U-Boot] [RFC PATCH 1/3] dm: core: Add pre-OS remove flag to device_remove()

[Stefan Roese]

Hi Simon,

On 08.03.2017 22:01, Simon Glass wrote:
> Hi Stefan,
>
> On 2 March 2017 at 23:24, Stefan Roese <sr at denx.de> wrote:
>> Hi Simon,
>>
>> On 03.03.2017 05:53, Simon Glass wrote:
>>> On 1 March 2017 at 03:23, Stefan Roese <sr at 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 at denx.de>
>>>> Cc: Simon Glass <sjg at 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