Hi Simon,
On 10.03.2017 12:31, Stefan Roese wrote:
> Hi Simon,
>
> On 08.03.2017 22:01, Simon Glass wrote:
>> 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 */
>> };
>
> Okay, I'll go forward with this suggestion and will generate a new
> patchset version soon. Stay tuned...
Thinking about it, we need a bit for the "normal" remove case as well.
How about this naming:
enum {
DM_REMOVE_NORMAL = 1 << 0, /* Remove all devices */
DM_REMOVE_ACTIVE_ALL = 1 << 1, /* Remove devices with any active flag
set */
DM_REMOVE_ACTIVE_DMA = 1 << 2, /* Remove only devices with active DMA
*/
/* Add more use cases here */
};
Or do you have another suggestion in mind for this "normal" device
remove case?
Thanks,
Stefan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
