On Tue, May 1, 2018 at 7:50 AM Jocelyn Bohr <[email protected]> wrote:
> On Mon, Apr 30, 2018 at 1:33 AM Alex Kiernan <[email protected]> wrote: >> Extract fb_set_reboot_flag() from USB code and ensure all the overides >> are included, then make the UDP fastboot code go through this same >> path. >> Note this changes the behaviour of the fastboot net code such that >> "reboot-bootloader" is no longer written to CONFIG_FASTBOOT_BUF_ADDR for >> use as a marker on reboot (the AOSP code in common/android-bootloader.c >> uses this marker - this code could be reinstated there if that gets >> merged). >> Signed-off-by: Alex Kiernan <[email protected]> >> --- >> Changes in v2: None >> arch/arm/mach-omap2/boot-common.c | 2 +- >> arch/arm/mach-rockchip/rk3128-board.c | 2 +- >> arch/arm/mach-rockchip/rk322x-board.c | 2 +- >> drivers/fastboot/fb_common.c | 5 +++++ >> drivers/usb/gadget/f_fastboot.c | 5 ----- >> include/fastboot.h | 1 + >> net/fastboot.c | 17 +++++++++-------- >> 7 files changed, 18 insertions(+), 16 deletions(-) >> diff --git a/arch/arm/mach-omap2/boot-common.c b/arch/arm/mach-omap2/boot-common.c >> index f9ab5da..2be5c11 100644 >> --- a/arch/arm/mach-omap2/boot-common.c >> +++ b/arch/arm/mach-omap2/boot-common.c >> @@ -238,7 +238,7 @@ void arch_preboot_os(void) >> } >> #endif >> -#if defined(CONFIG_USB_FUNCTION_FASTBOOT) && !defined(CONFIG_ENV_IS_NOWHERE) >> +#if CONFIG_IS_ENABLED(FASTBOOT) && !CONFIG_IS_ENABLED(ENV_IS_NOWHERE) >> int fb_set_reboot_flag(void) >> { >> printf("Setting reboot to fastboot flag ...\n"); >> diff --git a/arch/arm/mach-rockchip/rk3128-board.c b/arch/arm/mach-rockchip/rk3128-board.c >> index 2e8393d..00ad563 100644 >> --- a/arch/arm/mach-rockchip/rk3128-board.c >> +++ b/arch/arm/mach-rockchip/rk3128-board.c >> @@ -112,7 +112,7 @@ int board_usb_cleanup(int index, enum usb_init_type init) >> } >> #endif >> -#if defined(CONFIG_USB_FUNCTION_FASTBOOT) >> +#if CONFIG_IS_ENABLED(FASTBOOT) >> int fb_set_reboot_flag(void) >> { >> struct rk3128_grf *grf; >> diff --git a/arch/arm/mach-rockchip/rk322x-board.c b/arch/arm/mach-rockchip/rk322x-board.c >> index 8642a90..0ddfac8 100644 >> --- a/arch/arm/mach-rockchip/rk322x-board.c >> +++ b/arch/arm/mach-rockchip/rk322x-board.c >> @@ -140,7 +140,7 @@ int board_usb_cleanup(int index, enum usb_init_type init) >> } >> #endif >> -#if defined(CONFIG_USB_FUNCTION_FASTBOOT) >> +#if CONFIG_IS_ENABLED(FASTBOOT) >> int fb_set_reboot_flag(void) >> { >> struct rk322x_grf *grf; >> diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c >> index 8b3627b..36ef669 100644 >> --- a/drivers/fastboot/fb_common.c >> +++ b/drivers/fastboot/fb_common.c >> @@ -102,3 +102,8 @@ int fastboot_lookup_command(const char *cmd_string) >> return -1; >> } >> + >> +int __weak fb_set_reboot_flag(void) >> +{ >> + return -1; >> +} > Instead could this write the string "reboot-bootloader" to CONFIG_FASTBOOT_BUF_ADDR? I guess the thing you lose is you don't then get a default "I don't know how to do that" response. Am I right in thinking you're only using this behaviour on Raspberry Pi, or is it broader? >> diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c >> index a493c75..84515da 100644 >> --- a/drivers/usb/gadget/f_fastboot.c >> +++ b/drivers/usb/gadget/f_fastboot.c >> @@ -357,11 +357,6 @@ static void compl_do_reset(struct usb_ep *ep, struct usb_request *req) >> do_reset(NULL, 0, 0, NULL); >> } >> -int __weak fb_set_reboot_flag(void) >> -{ >> - return -ENOSYS; >> -} >> - >> static void cb_reboot(struct usb_ep *ep, struct usb_request *req) >> { >> char *cmd = req->buf; >> diff --git a/include/fastboot.h b/include/fastboot.h >> index de07220..9767065 100644 >> --- a/include/fastboot.h >> +++ b/include/fastboot.h >> @@ -75,4 +75,5 @@ void timed_send_info(ulong *start, const char *msg); >> int strcmp_l1(const char *s1, const char *s2); >> int fastboot_lookup_command(const char *cmd_string); >> +int fb_set_reboot_flag(void); >> #endif /* _FASTBOOT_H_ */ >> diff --git a/net/fastboot.c b/net/fastboot.c >> index 155049a..edf78df 100644 >> --- a/net/fastboot.c >> +++ b/net/fastboot.c >> @@ -65,7 +65,7 @@ static void cb_flash(char *, char *, unsigned int, char *); >> static void cb_erase(char *, char *, unsigned int, char *); >> #endif >> static void cb_continue(char *, char *, unsigned int, char *); >> -static void cb_reboot(char *, char *, unsigned int, char *); >> +static void cb_reboot_bootloader(char *, char *, unsigned int, char *); >> static void (*fb_net_dispatch[])(char *cmd_parameter, >> char *fastboot_data, >> @@ -83,8 +83,8 @@ static void (*fb_net_dispatch[])(char *cmd_parameter, >> #endif >> [FB_CMD_BOOT] = cb_okay, >> [FB_CMD_CONTINUE] = cb_continue, >> - [FB_CMD_REBOOT] = cb_reboot, > Why is the normal reboot removed here? The reboot handling is really all in the post-packet handling, so for a reboot all you need is the OKAY which gets added two lines down in that patch: - [FB_CMD_REBOOT] = cb_reboot, - [FB_CMD_REBOOT_BOOTLOADER] = cb_reboot, + [FB_CMD_REBOOT] = cb_okay, + [FB_CMD_REBOOT_BOOTLOADER] = cb_reboot_bootloader, And then leverage the reboot handling further down the flow: } else if (cmd == FB_CMD_REBOOT || cmd == FB_CMD_REBOOT_BOOTLOADER) { do_reset(NULL, 0, 0, NULL); } I guess the biggest change overall here is to introduce named identifiers for the commands rather than doing prefix matching. That said I should split this patch out as it's not doing what the commit says. -- Alex Kiernan _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
