Invoke board specific board_usb_cleanup function

PATCH New Inha Song 5 participants 16 messages Jan 19, 2015 – Feb 26, 2015

lore

Share & tools

lore

Inha Song

author

2015-01-19 07:07 UTC #0 lore raw

cover Invoke board specific board_usb_cleanup function

This patches invoke board-specific usb cleanup interface (board_usb_cleanup) After USB initalization.

Inha Song (2):
usb: common: provide a _weak board_usb_cleanup() function
usb: invoke board specific USB cleanup interface

 common/cmd_dfu.c              | 1 +
 common/cmd_thordown.c         | 1 +
 common/cmd_usb_mass_storage.c | 1 +
 common/usb.c                  | 6 ++++++
 4 files changed, 9 insertions(+)

Signature · 1 line

2.0.0.390.gcb682f8

Inha Song

author

2015-01-19 07:07 UTC #1 lore raw

PATCH 1/2 usb: common: provide a _weak board_usb_cleanup() function

This patch implement _weak function for board_usb_cleanup. This function (usb_board_cleanup) implementation is board-specific feature.

board_usb_cleanup function can be used to clean up after failed board-specific USB initialization.

Signed-off-by Inha Song <[email protected]>

---
 common/usb.c | 6 ++++++
 1 file changed, 6 insertions(+)

common/usb.c +6 -0

index 736cd9f..fbb4495 100644
@@ -1063,4 +1063,10 @@ int board_usb_init(int index, enum usb_init_type init)
 {
 	return 0;
 }
+
+__weak
+int board_usb_cleanup(int index, enum usb_init_type init)
+{
+	return 0;
+}
 /* EOF */

Signature · 1 line

2.0.0.390.gcb682f8

Lukasz Majewski

2015-02-24 11:56 UTC #2 lore raw

PATCH 1/2 usb: common: provide a _weak board_usb_cleanup() function

Hi Marek

Quoted 17 lines

This patch implement _weak function for board_usb_cleanup. This function (usb_board_cleanup) implementation is board-specific feature.
board_usb_cleanup function can be used to clean up after failed board-specific USB initialization.
Signed-off-by Inha Song <[email protected]>
---
 common/usb.c | 6 ++++++
 1 file changed, 6 insertions(+)
common/usb.c +0 -0
index 736cd9f..fbb4495 100644

common/usb.c int board_usb_init(int index, enum · line 1072

Quoted 10 lines

common/usb.c +6 -0

@@ -1063,4 +1063,10 @@ int board_usb_init(int index, enum
 usb_init_type init) {
 	return 0;
 }
+
+__weak
+int board_usb_cleanup(int index, enum usb_init_type init)
+{
+	return 0;
+}
 /* EOF */

Acked-by Lukasz Majewski <[email protected]>

Marek could you take this patch (and the following one) to -usb tree?

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

Lukasz Majewski

2015-02-24 16:05 UTC #3 lore raw

PATCH 1/2 usb: common: provide a _weak board_usb_cleanup() function

Hi Lukasz,

Quoted 32 lines

Hi Marek
Quoted 17 lines
This patch implement _weak function for board_usb_cleanup. This function (usb_board_cleanup) implementation is board-specific feature.
board_usb_cleanup function can be used to clean up after failed board-specific USB initialization.
Signed-off-by Inha Song <[email protected]>
---
 common/usb.c | 6 ++++++
 1 file changed, 6 insertions(+)
common/usb.c +0 -0
index 736cd9f..fbb4495 100644
common/usb.c int board_usb_init(int index, enum · line 1072
Quoted 10 lines
common/usb.c +6 -0
@@ -1063,4 +1063,10 @@ int board_usb_init(int index, enum
 usb_init_type init) {
 	return 0;
 }
+
+__weak
+int board_usb_cleanup(int index, enum usb_init_type init)
+{
+	return 0;
+}
 /* EOF */
Acked-by Lukasz Majewski <[email protected]>

Quoted 2 lines

Marek could you take this patch (and the following one) to -usb tree?

I've looked a bit more deep into this and it seems like this code is also defined in patches provided by Kishon:

"common: cmd_dfu: invoke board_usb_cleanup() for cleaning up" b607ea5d62bb0c295f10af86a6de6b33dd1fea75

Marek, it is your call to decide how we should proceed.

My recommendation - we should use Kishon's patches, since it is the part of a larger patchset.

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

Marek Vasut

2015-02-24 17:34 UTC #4 lore raw

PATCH 1/2 usb: common: provide a _weak board_usb_cleanup() function

Quoted 1 line · Lukasz Majewski

Hi Lukasz,

Hi Lukasz,

is your other self doing good too ? ;-)

Quoted 42 lines

Quoted 33 lines
Hi Marek
Quoted 31 lines
This patch implement _weak function for board_usb_cleanup. This function (usb_board_cleanup) implementation is board-specific feature.
board_usb_cleanup function can be used to clean up after failed board-specific USB initialization.
Signed-off-by Inha Song <[email protected]>
---
 common/usb.c | 6 ++++++
 1 file changed, 6 insertions(+)
common/usb.c +6 -0
index 736cd9f..fbb4495 100644
@@ -1063,4 +1063,10 @@ int board_usb_init(int index, enum
 usb_init_type init) {
 
 	return 0;
 
 }
 
+
+__weak
+int board_usb_cleanup(int index, enum usb_init_type init)
+{
+	return 0;
+}
Quoted 5 lines
Quoted 1 line
/* EOF */
Acked-by Lukasz Majewski <[email protected]>
Marek could you take this patch (and the following one) to -usb tree?
I've looked a bit more deep into this and it seems like this code is also defined in patches provided by Kishon:

Quoted 7 lines

"common: cmd_dfu: invoke board_usb_cleanup() for cleaning up" b607ea5d62bb0c295f10af86a6de6b33dd1fea75
Marek, it is your call to decide how we should proceed.
My recommendation - we should use Kishon's patches, since it is the part of a larger patchset.

I'll stick with your recommendation, though I'd like to -- somehow -- work in Inha's credit into those patches too. What do you think please ?

Best regards,
Marek Vasut

Lukasz Majewski

2015-02-25 09:15 UTC #5 lore raw

PATCH 1/2 usb: common: provide a _weak board_usb_cleanup() function

Hi Marek,

Quoted 53 lines · Lukasz Majewski

Quoted 1 line
Hi Lukasz,
Hi Lukasz,
is your other self doing good too ? ;-)
Quoted 46 lines
Quoted 33 lines
Hi Marek
Quoted 31 lines
This patch implement _weak function for board_usb_cleanup. This function (usb_board_cleanup) implementation is board-specific feature.
board_usb_cleanup function can be used to clean up after failed board-specific USB initialization.
Signed-off-by Inha Song <[email protected]>
---
 common/usb.c | 6 ++++++
 1 file changed, 6 insertions(+)
common/usb.c +6 -0
index 736cd9f..fbb4495 100644
@@ -1063,4 +1063,10 @@ int board_usb_init(int index, enum
 usb_init_type init) {
 
 	return 0;
 
 }
 
+
+__weak
+int board_usb_cleanup(int index, enum usb_init_type init)
+{
+	return 0;
+}
Quoted 6 lines
Quoted 1 line
/* EOF */
Acked-by Lukasz Majewski <[email protected]>
Marek could you take this patch (and the following one) to -usb tree?
I've looked a bit more deep into this and it seems like this code is also defined in patches provided by Kishon:
"common: cmd_dfu: invoke board_usb_cleanup() for cleaning up" b607ea5d62bb0c295f10af86a6de6b33dd1fea75

Quoted 8 lines

Quoted 4 lines
Marek, it is your call to decide how we should proceed.
My recommendation - we should use Kishon's patches, since it is the part of a larger patchset.
I'll stick with your recommendation, though I'd like to -- somehow -- work in Inha's credit into those patches too. What do you think please ?

Rob Herring had some comments to Inha original patches, so now I'm confused.

Lets see if Inha fixes issues with original patch. It is also possible to drop only one patch from Kishon, namely "common: cmd_dfu: invoke board_usb_cleanup() for cleaning up"

Quoted 3 lines

Best regards,
Marek Vasut

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

Marek Vasut

2015-02-25 17:02 UTC #6 lore raw

PATCH 1/2 usb: common: provide a _weak board_usb_cleanup() function

Quoted 1 line · Lukasz Majewski

Hi Marek,

Hi!

Quoted 3 lines

Quoted 3 lines · Lukasz Majewski
Quoted 1 line
Hi Lukasz,

[...]

Quoted 10 lines

Quoted 3 lines
I'll stick with your recommendation, though I'd like to -- somehow -- work in Inha's credit into those patches too. What do you think please ?
Rob Herring had some comments to Inha original patches, so now I'm confused.
Lets see if Inha fixes issues with original patch. It is also possible to drop only one patch from Kishon, namely "common: cmd_dfu: invoke board_usb_cleanup() for cleaning up"

Gah, we're loosing traction here. Maybe we need to sync up and we surely need to apply something, otherwise this will become a chaotic mess.

I placed all of the patches into u-boot-usb branch topic/dwc3 . Can you please check if it's working and what is possibly missing ? Let's work on top of that please, OK ?

Best regards,
Marek Vasut

Lukasz Majewski

2015-02-26 08:20 UTC #7 lore raw

PATCH 1/2 usb: common: provide a _weak board_usb_cleanup() function

Hi Marek,

Quoted 23 lines · Lukasz Majewski

Quoted 1 line
Hi Marek,
Hi!
Quoted 4 lines
Quoted 4 lines
Quoted 1 line · Lukasz Majewski
Hi Lukasz,
[...]
Quoted 10 lines
Quoted 3 lines
I'll stick with your recommendation, though I'd like to -- somehow -- work in Inha's credit into those patches too. What do you think please ?
Rob Herring had some comments to Inha original patches, so now I'm confused.
Lets see if Inha fixes issues with original patch. It is also possible to drop only one patch from Kishon, namely "common: cmd_dfu: invoke board_usb_cleanup() for cleaning up"

Quoted 7 lines

Gah, we're loosing traction here. Maybe we need to sync up and we surely need to apply something, otherwise this will become a chaotic mess.
I placed all of the patches into u-boot-usb branch topic/dwc3 . Can you please check if it's working and what is possibly missing ? Let's work on top of that please, OK ?

Thanks for preparing the branch.

However, some fixes - those collected by me need some extra rework, since there were some comments regarding them (mostly from Kishon).

Moreover, we could also add to this branch (on top of Kishon's dwc3 patch set) the linux compatibility rework. On top of that I would like to add next version of dwc3 fixes.

Quoted 3 lines

Best regards,
Marek Vasut

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

Inha Song

author

2015-01-19 07:07 UTC #8 lore raw

PATCH 2/2 usb: invoke board specific USB cleanup interface

In reply to Inha Song

This patch invoke board-specific USB cleanup (board_usb_cleanup) interface.

Signed-off-by Inha Song <[email protected]>

---
 common/cmd_dfu.c              | 1 +
 common/cmd_thordown.c         | 1 +
 common/cmd_usb_mass_storage.c | 1 +
 3 files changed, 3 insertions(+)

common/cmd_dfu.c +1 -0

index e975abe..161d38b 100644
@@ -68,6 +68,7 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	}
 exit:
 	g_dnl_unregister();
+	board_usb_cleanup(controller_index, USB_INIT_DEVICE);
 done:
 	dfu_free_entities();

common/cmd_thordown.c +1 -0

index 8ed1dc6..436b7f5 100644
@@ -56,6 +56,7 @@ int do_thor_down(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 exit:
 	g_dnl_unregister();
+	board_usb_cleanup(controller_index, USB_INIT_DEVICE);
 done:
 	dfu_free_entities();

common/cmd_usb_mass_storage.c +1 -0

index 2c879ea..53a765e 100644
@@ -154,6 +154,7 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
 	}
 exit:
 	g_dnl_unregister();
+	board_usb_cleanup(controller_index, USB_INIT_DEVICE);
 	return CMD_RET_SUCCESS;
 }

Signature · 1 line

2.0.0.390.gcb682f8

Lukasz Majewski

2015-02-24 11:56 UTC #9 lore raw

PATCH 2/2 usb: invoke board specific USB cleanup interface

Hi Inha,

Quoted 39 lines

This patch invoke board-specific USB cleanup (board_usb_cleanup) interface.

Signed-off-by Inha Song <[email protected]>

---
 common/cmd_dfu.c              | 1 +
 common/cmd_thordown.c         | 1 +
 common/cmd_usb_mass_storage.c | 1 +
 3 files changed, 3 insertions(+)

common/cmd_dfu.c +1 -0

index e975abe..161d38b 100644
@@ -68,6 +68,7 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int
 argc, char * const argv[]) }
 exit:
 	g_dnl_unregister();
+	board_usb_cleanup(controller_index, USB_INIT_DEVICE);
 done:
 	dfu_free_entities();

common/cmd_thordown.c +1 -0

index 8ed1dc6..436b7f5 100644
@@ -56,6 +56,7 @@ int do_thor_down(cmd_tbl_t *cmdtp, int flag, int
 argc, char * const argv[]) 
 exit:
 	g_dnl_unregister();
+	board_usb_cleanup(controller_index, USB_INIT_DEVICE);
 done:
 	dfu_free_entities();

(quoted diff) +0 -0

b/common/cmd_usb_mass_storage.c index 2c879ea..53a765e 100644

common/cmd_usb_mass_storage.c int do_usb_mass_storage(cmd_tbl_t *cmdtp, int · line 160

Quoted 7 lines

common/cmd_usb_mass_storage.c +1 -0

@@ -154,6 +154,7 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int
 flag, }
 exit:
 	g_dnl_unregister();
+	board_usb_cleanup(controller_index, USB_INIT_DEVICE);
 	return CMD_RET_SUCCESS;
 }Lukasz Majewski <[email protected]>

Acked-by Lukasz Majewski <[email protected]>

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

Tom Rini

2015-02-25 13:10 UTC #12 lore raw

PATCH 2/2 usb: invoke board specific USB cleanup interface

Quoted 18 lines · Inha Song

Hi, Rob,
Quoted 11 lines · On Tue, 24 Feb 2015 10:20:13 -0600 Rob Herring <[email protected]> wrote:
Quoted 8 lines · Inha Song
This patch invoke board-specific USB cleanup (board_usb_cleanup) interface.
Signed-off-by Inha Song <[email protected]>
---
 common/cmd_dfu.c              | 1 +
 common/cmd_thordown.c         | 1 +
 common/cmd_usb_mass_storage.c | 1 +
You missed fastboot at a minimum.

Quoted 7 lines

Ok, I will add board_usb_cleanup fuction also in fastboot.
Quoted 2 lines
What about other gadget functions such as rndis/cdc_etherent?
I doesn't have any knowledge about rndis/cdc_etherent So, Afraid that to fix these code.

So that's the problem with this approach. Kishon reminded me that on TI parts we see some PM problems because we don't always do a cleanup on the USB side for gadgets. I think the right answer here is to mirror the usb_stop() function in bootm_disable_interrupts() in common/bootm.c for the gadget code as well.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150225/ea95d866/attachment.sig>

Marek Vasut

2015-01-19 08:50 UTC #13 lore raw

cover Invoke board specific board_usb_cleanup function

In reply to Inha Song

Quoted 6 lines · Inha Song

This patches invoke board-specific usb cleanup interface (board_usb_cleanup) After USB initalization.
Inha Song (2):
usb: common: provide a _weak board_usb_cleanup() function
usb: invoke board specific USB cleanup interface

Hi,

where do you plan to use this _weak function please ?

Best regards,
Marek Vasut

Inha Song

author

2015-01-19 09:18 UTC #14 lore raw

cover Invoke board specific board_usb_cleanup function

Hi Marek,

I want to use this function for support Thor/DFU download in Odroid-XU3.
For Odroid-XU3 usb support, we need to DWC3 code. (As I know, DWC3 patchsets (Kishon Vijay Abraham I) are RFC state) So, I used DWC3 patchset temporarily and I knew that I should be call dwc3_uboot_exit() function.

As a result, I have implemented board_usb_cleanup() function in smdk5420.c board file for called dwc3_uboot_exit() function.

If "board_usb_cleanup" is dead code, I will fix it.

Best Regards,
Inha Song.

Quoted 14 lines · On Mon, 19 Jan 2015 09:50:34 +0100 Marek Vasut <[email protected]> wrote:

Quoted 6 lines · Inha Song
This patches invoke board-specific usb cleanup interface (board_usb_cleanup) After USB initalization.
Inha Song (2):
usb: common: provide a _weak board_usb_cleanup() function
usb: invoke board specific USB cleanup interface
Hi,
where do you plan to use this _weak function please ?
Best regards,
Marek Vasut

Marek Vasut

2015-01-19 15:02 UTC #15 lore raw

cover Invoke board specific board_usb_cleanup function

Quoted 11 lines · Inha Song

Hi Marek,
I want to use this function for support Thor/DFU download in Odroid-XU3. For Odroid-XU3 usb support, we need to DWC3 code. (As I know, DWC3 patchsets (Kishon Vijay Abraham I) are RFC state) So, I used DWC3 patchset temporarily and I knew that I should be call dwc3_uboot_exit() function.
As a result, I have implemented board_usb_cleanup() function in smdk5420.c board file for called dwc3_uboot_exit() function.
If "board_usb_cleanup" is dead code, I will fix it.

Hi!

(please do not top-post)

Thanks for the clarification. Do you plan to send the dwc3 stuff as well please ?

Best regards,
Marek Vasut