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

[Stefan Roese]

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);

?
 
> 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