From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Kocialkowski Date: Tue, 16 Jun 2015 23:25:26 +0200 Subject: [U-Boot] [PATCH v2 3/4] usb: board_usb_init and board_usb_cleanup calls in the fastboot command In-Reply-To: <55808DF2.3050209@broadcom.com> References: <1434131821-7641-1-git-send-email-contact@paulk.fr> <1434131821-7641-3-git-send-email-contact@paulk.fr> <55808DF2.3050209@broadcom.com> Message-ID: <1434489926.2276.20.camel@collins> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Le mardi 16 juin 2015 ? 13:58 -0700, Steve Rae a ?crit : > Hi Paul, > > On 15-06-12 10:57 AM, Paul Kocialkowski wrote: > > Each USB download function command calls board_usb_init before registering the > > USB gadget and board_usb_cleanup after de-registering it. On devices currently > > using fasboot, musb-new is usually initialized earlier, but some other boards > > might need the board_usb_init call to properly initialize musb-new. > > > > This requires adding an argument (the USB controller index) to the fastboot > > command, as it is currently done with other USB download gadget functions. > > > > Signed-off-by: Paul Kocialkowski > > --- > > common/cmd_fastboot.c | 31 +++++++++++++++++++++++++------ > > include/configs/ti_omap5_common.h | 2 +- > > 2 files changed, 26 insertions(+), 7 deletions(-) > > > > diff --git a/common/cmd_fastboot.c b/common/cmd_fastboot.c > > index d52ccfb..86fbddf 100644 > > --- a/common/cmd_fastboot.c > > +++ b/common/cmd_fastboot.c > > @@ -10,11 +10,26 @@ > > #include > > #include > > #include > > +#include > > > > static int do_fastboot(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) > > { > > + int controller_index; > > + char *usb_controller; > > int ret; > > > > + if (argc < 2) > > + return CMD_RET_USAGE; > > + > > + usb_controller = argv[1]; > > Not backwards compatible.... I would prefer to make it optional: > if (argc < 2) > controller_index = 0; > else { > usb_controller = argv[1]; > controller_index = simple_strtoul(usb_controller, NULL, 0); > } This is definitely a "bug fix". There is no reason to assume that the USB controller index is 0 and it was incorrect to assume that from the very beginning. Other download USB gadget commands had the controller index as a non-optional parameter already. Since I fixed the configs that use it in U-Boot, I think it's fair enough. This may indeed break some hand-written scripts but those will be straightforward to fix. I am strongly against keeping deprecated legacy fallbacks that make the code inconsistent and harder to maintain just for the sake of keeping backwards compatibility with users' hand-written scripts. > > + controller_index = simple_strtoul(usb_controller, NULL, 0); > > + > > + ret = board_usb_init(controller_index, USB_INIT_DEVICE); > > + if (ret) { > > + error("USB init failed: %d", ret); > > + return CMD_RET_FAILURE; > > + } > > + > > g_dnl_clear_detach(); > > ret = g_dnl_register("usb_dnl_fastboot"); > > if (ret) > > @@ -23,9 +38,8 @@ static int do_fastboot(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) > > if (!g_dnl_board_usb_cable_connected()) { > > puts("\rUSB cable not detected.\n" \ > > "Command exit.\n"); > > - g_dnl_unregister(); > > - g_dnl_clear_detach(); > > - return CMD_RET_FAILURE; > > + ret = CMD_RET_FAILURE; > > + goto exit; > > } > > > > while (1) { > > @@ -36,14 +50,19 @@ static int do_fastboot(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) > > usb_gadget_handle_interrupts(0); > > } > > > > + ret = CMD_RET_SUCCESS; > > + > > +exit: > > g_dnl_unregister(); > > g_dnl_clear_detach(); > > - return CMD_RET_SUCCESS; > > + board_usb_cleanup(controller_index, USB_INIT_DEVICE); > > + > > + return ret; > > } > > > > U_BOOT_CMD( > > - fastboot, 1, 0, do_fastboot, > > + fastboot, 2, 1, do_fastboot, > > "use USB Fastboot protocol", > > - "\n" > > + "\n" > > make it optional: > "[]\n" > > > " - run as a fastboot usb device" > > ); > > diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h > > index 4faffef..4fd5669 100644 > > --- a/include/configs/ti_omap5_common.h > > +++ b/include/configs/ti_omap5_common.h > > @@ -137,7 +137,7 @@ > > "if test ${dofastboot} -eq 1; then " \ > > "echo Boot fastboot requested, resetting dofastboot ...;" \ > > "setenv dofastboot 0; saveenv;" \ > > - "echo Booting into fastboot ...; fastboot;" \ > > + "echo Booting into fastboot ...; fastboot 0;" \ > > then this isn't needed either.... > > > "fi;" \ > > "run findfdt; " \ > > "run mmcboot;" \ > > > > Thanks, Steve -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: