subject:"\[U\-Boot\] \[RFC PATCH 1\/3\] dm\: core\: Add pre\-OS remove flag to device

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

2017-03-10 Thread Stefan Roese

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  wrote:
>>> Hi Simon,
>>>
>>> On 03.03.2017 05:53, Simon Glass wrote:
 On 1 March 2017 at 03:23, Stefan Roese  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 
> Cc: Simon Glass 
> ---
>  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

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

2017-03-10 Thread Stefan Roese


Hi Simon,

On 08.03.2017 22:01, Simon Glass wrote:

Hi Stefan,

On 2 March 2017 at 23:24, Stefan Roese  wrote:

Hi Simon,

On 03.03.2017 05:53, Simon Glass wrote:

On 1 March 2017 at 03:23, Stefan Roese  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 
Cc: Simon Glass 
---
 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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2017-03-08 Thread Simon Glass

Hi Stefan,

On 2 March 2017 at 23:24, Stefan Roese  wrote:
> Hi Simon,
>
> On 03.03.2017 05:53, Simon Glass wrote:
>> On 1 March 2017 at 03:23, Stefan Roese  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 
>>> Cc: Simon Glass 
>>> ---
>>>  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 */
};

Then, DM_REMOVE_ACTIVE_ALL means everything will be removed, and if
that flag is not set, the other flags can be used.

I am assuming that there actually will be other cases - your email
suggests that could be true.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2017-03-02 Thread Stefan Roese

Hi Simon,

On 03.03.2017 05:53, Simon Glass wrote:
> On 1 March 2017 at 03:23, Stefan Roese  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 
>> Cc: Simon Glass 
>> ---
>>  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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/listinfo/u-boot

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

2017-03-02 Thread Simon Glass

Hi Stefan,

On 1 March 2017 at 03:23, Stefan Roese  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 
> Cc: Simon Glass 
> ---
>  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.

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?

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/listinfo/u-boot

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

2017-03-01 Thread Stefan Roese

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 
Cc: Simon Glass 
---
 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(-)

diff --git a/arch/x86/cpu/queensbay/tnc.c b/arch/x86/cpu/queensbay/tnc.c
index f307c622c8..32157f4347 100644
--- a/arch/x86/cpu/queensbay/tnc.c
+++ b/arch/x86/cpu/queensbay/tnc.c
@@ -76,13 +76,13 @@ static int __maybe_unused disable_igd(void)
 *
 * So the only option we have is to manually remove these two devices.
 */
-   ret = device_remove(igd);
+   ret = device_remove(igd, false);
if (ret)
return ret;
ret = device_unbind(igd);
if (ret)
return ret;
-   ret = device_remove(sdvo);
+   ret = device_remove(sdvo, false);
if (ret)
return ret;
ret = device_unbind(sdvo);
diff --git a/cmd/cros_ec.c b/cmd/cros_ec.c
index 9d42f870dc..abadfee860 100644
--- a/cmd/cros_ec.c
+++ b/cmd/cros_ec.c
@@ -110,7 +110,7 @@ static int do_cros_ec(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[])
/* Remove any existing device */
ret = uclass_find_device(UCLASS_CROS_EC, 0, );
if (!ret)
-   device_remove(udev);
+   device_remove(udev, false);
ret = uclass_get_device(UCLASS_CROS_EC, 0, );
if (ret) {
printf("Could not init cros_ec device (err %d)\n", ret);
diff --git a/cmd/sf.c b/cmd/sf.c
index 65b117feee..15e5b435fa 100644
--- a/cmd/sf.c
+++ b/cmd/sf.c
@@ -124,7 +124,7 @@ static int do_spi_flash_probe(int argc, char * const argv[])
/* Remove the old device, otherwise probe will just be a nop */
ret = spi_find_bus_and_cs(bus, cs, _dev, );
if (!ret) {
-   device_remove(new);
+   device_remove(new, false);
}
flash = NULL;
ret = spi_flash_probe_bus_cs(bus, cs, speed, mode, );
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 38cb9388da..41fd983976 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -530,7 +530,7 @@ int blk_unbind_all(int if_type)
struct blk_desc *desc = dev_get_uclass_platdata(dev);
 
if (desc->if_type == if_type) {
-   ret = device_remove(dev);
+   ret = device_remove(dev, false);
if (ret)
return ret;
ret = device_unbind(dev);
diff --git a/drivers/block/sandbox.c b/drivers/block/sandbox.c
index 36c2ff3007..1e78d0bbb3 100644
--- a/drivers/block/sandbox.c
+++ b/drivers/block/sandbox.c
@@ -98,7 +98,7 @@ int host_dev_bind(int devnum, char *filename)
/* Remove and unbind the old device, if any */
ret = blk_get_device(IF_TYPE_HOST, devnum, );
if (ret == 0) {
-   ret = device_remove(dev);
+   ret = device_remove(dev, false);
if (ret)
return ret;
ret = device_unbind(dev);
diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index a7f77b4a21..4725d4751c 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -46,9 +46,10 @@ static int device_chld_unbind(struct udevice *dev)
 /**
  * device_chld_remove() - Stop all device's children
  * @dev:   The device whose children are to be removed
+ * @pre_os_remove: Flag, if this functions is called in the pre-OS stage
  * @return 0 on success, -ve on error
  */
-static int device_chld_remove(struct udevice *dev)
+static int device_chld_remove(struct udevice *dev, bool pre_os_remove)
 {
struct udevice *pos, *n;
int ret;
@@ -56,7 +57,7 @@ static int device_chld_remove(struct udevice *dev)
assert(dev);
 
list_for_each_entry_safe(pos, n, >child_head,

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

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

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

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

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

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

6 matches

Site Navigation

Mail list logo

Footer information