[U-Boot] [RFC PATCH 3/3] arm: bootm: Add dm_pre_os_remove() call to announce_and_cleanup()

[Simon Glass]

Hi Stefan,

On 2 March 2017 at 23:35, 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 a call to dm_pre_os_remove() to announce_and_cleanup()
>>> so that drivers that have the flag DM_FLAG_PRE_OS_REMOVE set may do
>>> some last-stage cleanup before the OS is started.
>>>
>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>> Cc: Simon Glass <sjg at chromium.org>
>>> ---
>>>  arch/arm/lib/bootm.c | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
>>> index 8125cf023f..84f3415c1e 100644
>>> --- a/arch/arm/lib/bootm.c
>>> +++ b/arch/arm/lib/bootm.c
>>> @@ -91,6 +91,14 @@ static void announce_and_cleanup(int fake)
>>>
>>>         board_quiesce_devices();
>>>
>>> +       /*
>>> +        * Call remove function of all devices with the pre-OS remove
>>> flag
>>> +        * set (DM_FLAG_PRE_OS_REMOVE). This may be useful for last-stage
>>> +        * operations, like cancelling of DMA operation or releasing
>>> device
>>> +        * internal buffers.
>>> +        */
>>> +       dm_pre_os_remove();
>>
>>
>> In a full DM world we could perhaps have devices which use the DMA
>> uclass, so we can tell which ones need to be removed.
>>
>> How about dm_remove_dma_devices()?
>
>
> I'm not so sure. As we would perhaps need to add other calls for
> further pre-OS removal reasons (e.g. the stop timer example). How
> about calling a function with the removal flags as parameter:
>
>         dm_remove_devices_conditional(ONLY_REMOVE_ACTIVE_DMA | ...);
>
> or
>
>         dm_remove_devices_flags(ONLY_REMOVE_ACTIVE_DMA | ...);
>
> What do you think?

Seems reasonable - see my previous email for a flag naming idea (i.e.
having an explicit ALL flag to avoid all the flags having to use
negative logic).

>
>>> +
>>>         cleanup_before_linux();
>>>  }
>>>
>>> --
>>> 2.12.0
>>>
>>
>> Also (once we have things figured out) this needs some sort of test in
>> test/.
>
>
> I already feared that. ;)
>
> Sure, I'll try to add some test, once the path is clear.
>
> Thanks,
> Stefan

Regards,
Simon