[U-Boot] [PATCH 1/3] dm: core: Add flags parameter to device_remove()

[Simon Glass]

Hi Stefan,

On 10 March 2017 at 06:50, Stefan Roese <sr at denx.de> wrote:
> This patch adds the flags parameter to device_remove() and changes all
> calls to this function to provide the default value of DM_REMOVE_NORMAL
> for "normal" device removal.
>
> This is in preparation for the driver specific pre-OS (e.g. DMA
> cancelling) 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            | 26 ++++++++++++++++++++++++++
>  test/dm/bus.c                  |  8 ++++----
>  test/dm/core.c                 | 16 ++++++++--------
>  test/dm/eth.c                  |  2 +-
>  test/dm/spi.c                  |  2 +-
>  21 files changed, 65 insertions(+), 37 deletions(-)
>

[...]

> diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
> index 0bf8707493..2cabc87338 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
> + * @flags: Flags for selective device removal
>   * @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, uint flags);
>  #else
> -static inline int device_remove(struct udevice *dev) { return 0; }
> +static inline int device_remove(struct udevice *dev, uint flags) { return 0; }
>  #endif
>
>  /**
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 4e95fb7773..88d11e912e 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -46,6 +46,32 @@ struct driver_info;
>
>  #define DM_FLAG_OF_PLATDATA            (1 << 8)
>
> +/*
> + * Call driver remove function to stop currently active DMA transfers or
> + * give DMA buffers back to the HW / controller. This may be needed for
> + * some drivers to do some final stage cleanup before the OS is called
> + * (U-Boot exit)
> + */
> +#define DM_FLAG_ACTIVE_DMA_REMOVE      (1 << 9)

Do we need _REMOVE on this end of this? It seems to me that we could
just flag that this driver has active DMA.

> +
> +/*
> + * One or multiple of these flags are passed to device_remove() so that
> + * a selective device removal as specified by the remove-stage and the
> + * driver flags can be done.
> + */
> +enum {
> +       /* Normal remove, remove all devices */
> +       DM_REMOVE_NORMAL     = 1 << 0,
> +
> +       /* Remove only devices with active DMA */

Can we remove the word 'only'? I think it gets confusing if one flag
negates another.

> +       DM_REMOVE_ACTIVE_DMA = DM_FLAG_ACTIVE_DMA_REMOVE,
> +
> +       /* Add more use cases here */
> +
> +       /* Remove devices with any active flag */
> +       DM_REMOVE_ACTIVE_ALL = DM_REMOVE_ACTIVE_DMA,
> +};
> +
>  /**
>   * struct udevice - An instance of a driver
>   *

[..]

Regards,
Simon