diff mbox

[U-Boot,3/9] Only perform DFU board_usb_init() for TRATS

Message ID 1354106642-4587-4-git-send-email-panto@antoniou-consulting.com
State Superseded
Delegated to: Marek Vasut
Headers show

Commit Message

Pantelis Antoniou Nov. 28, 2012, 12:43 p.m. UTC
USB initialization shouldn't happen for all the boards.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 common/cmd_dfu.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Marek Vasut Nov. 28, 2012, 2:45 a.m. UTC | #1
Dear Pantelis Antoniou,

> USB initialization shouldn't happen for all the boards.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  common/cmd_dfu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
> index 01d6b3a..327c738 100644
> --- a/common/cmd_dfu.c
> +++ b/common/cmd_dfu.c
> @@ -55,7 +55,10 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc,
> char * const argv[]) goto done;
>  	}
> 
> +#ifdef CONFIG_TRATS
>  	board_usb_init();
> +#endif
> +

It's common code:

1) Why is it called "board_usb_init()" ? Does this have anything to do with usb 
host?

2) Make it __weak, then if it's undefined for your board, something default will 
be called.

>  	g_dnl_register(s);
>  	while (1) {
>  		if (ctrlc())

Best regards,
Marek Vasut
Pantelis Antoniou Nov. 28, 2012, 8:26 a.m. UTC | #2
Hi Marek,

On Nov 28, 2012, at 4:45 AM, Marek Vasut wrote:

> Dear Pantelis Antoniou,
> 
>> USB initialization shouldn't happen for all the boards.
>> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>> ---
>> common/cmd_dfu.c | 3 +++
>> 1 file changed, 3 insertions(+)
>> 
>> diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
>> index 01d6b3a..327c738 100644
>> --- a/common/cmd_dfu.c
>> +++ b/common/cmd_dfu.c
>> @@ -55,7 +55,10 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc,
>> char * const argv[]) goto done;
>> 	}
>> 
>> +#ifdef CONFIG_TRATS
>> 	board_usb_init();
>> +#endif
>> +
> 
> It's common code:
> 
> 1) Why is it called "board_usb_init()" ? Does this have anything to do with usb 
> host?
> 

No idea. It makes no sense to me, but it was there from the original DFU poster.
I don't have a TRATS board to test it anyway, but I didn't want to affect it.

> 2) Make it __weak, then if it's undefined for your board, something default will 
> be called.
> 

I see no reason why it should even exist. Perhaps we should ask the original poster.

>> 	g_dnl_register(s);
>> 	while (1) {
>> 		if (ctrlc())
> 
> Best regards,
> Marek Vasut

Regards

-- Pantelis
Marek Vasut Nov. 29, 2012, 6:28 a.m. UTC | #3
Dear Pantelis Antoniou,

> Hi Marek,
> 
> On Nov 28, 2012, at 4:45 AM, Marek Vasut wrote:
> > Dear Pantelis Antoniou,
> > 
> >> USB initialization shouldn't happen for all the boards.
> >> 
> >> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> >> ---
> >> common/cmd_dfu.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >> 
> >> diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
> >> index 01d6b3a..327c738 100644
> >> --- a/common/cmd_dfu.c
> >> +++ b/common/cmd_dfu.c
> >> @@ -55,7 +55,10 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int
> >> argc, char * const argv[]) goto done;
> >> 
> >> 	}
> >> 
> >> +#ifdef CONFIG_TRATS
> >> 
> >> 	board_usb_init();
> >> 
> >> +#endif
> >> +
> > 
> > It's common code:
> > 
> > 1) Why is it called "board_usb_init()" ? Does this have anything to do
> > with usb host?
> 
> No idea. It makes no sense to me, but it was there from the original DFU
> poster. I don't have a TRATS board to test it anyway, but I didn't want to
> affect it.

Can you please rename it to a more sensible name then?

> > 2) Make it __weak, then if it's undefined for your board, something
> > default will be called.
> 
> I see no reason why it should even exist. Perhaps we should ask the
> original poster.

Please do it then

> >> 	g_dnl_register(s);
> >> 	while (1) {
> >> 	
> >> 		if (ctrlc())
> > 
> > Best regards,
> > Marek Vasut
> 
> Regards
> 
> -- Pantelis

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
index 01d6b3a..327c738 100644
--- a/common/cmd_dfu.c
+++ b/common/cmd_dfu.c
@@ -55,7 +55,10 @@  static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		goto done;
 	}
 
+#ifdef CONFIG_TRATS
 	board_usb_init();
+#endif
+
 	g_dnl_register(s);
 	while (1) {
 		if (ctrlc())