| Message ID | 1525077174-6211-14-git-send-email-alex.kiernan@gmail.com |
|---|---|
| State | RFC |
| Delegated to: | Lukasz Majewski |
| Headers | show |
| Series | Add fastboot UDP support | expand |
On Mon, Apr 30, 2018 at 1:33 AM Alex Kiernan <alex.kiernan@gmail.com> 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 <alex.kiernan@gmail.com> > --- > > 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? > 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? > - [FB_CMD_REBOOT_BOOTLOADER] = cb_reboot, > + [FB_CMD_REBOOT] = cb_okay, + [FB_CMD_REBOOT_BOOTLOADER] = cb_reboot_bootloader, > [FB_CMD_POWERDOWN] = NULL, > [FB_CMD_SET_ACTIVE] = cb_okay, > [FB_CMD_UPLOAD] = NULL, > @@ -370,12 +370,13 @@ static void cb_continue(char *cmd_parameter, char > *fastboot_data, > * > * @param repsonse Pointer to fastboot response buffer > */ > -static void cb_reboot(char *cmd_parameter, char *fastboot_data, > - unsigned int fastboot_data_len, char *response) > +static void cb_reboot_bootloader(char *cmd_parameter, char *fastboot_data, > + unsigned int fastboot_data_len, char > *response) > { > - fastboot_okay(NULL, response); > - if (!strcmp("reboot-bootloader", cmd_string)) > - strcpy((char *)CONFIG_FASTBOOT_BUF_ADDR, > "reboot-bootloader"); > + if (fb_set_reboot_flag()) > + fastboot_fail("Cannot set reboot flag", response); > + else > + fastboot_okay(NULL, response); > } > > /** > -- > 2.7.4 > >
On Tue, May 1, 2018 at 7:50 AM Jocelyn Bohr <bohr@google.com> wrote: > On Mon, Apr 30, 2018 at 1:33 AM Alex Kiernan <alex.kiernan@gmail.com> 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 <alex.kiernan@gmail.com> >> --- >> 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.
On Tue, May 1, 2018 at 8:22 AM Alex Kiernan <alex.kiernan@gmail.com> wrote: > On Tue, May 1, 2018 at 7:50 AM Jocelyn Bohr <bohr@google.com> wrote: > > On Mon, Apr 30, 2018 at 1:33 AM Alex Kiernan <alex.kiernan@gmail.com> > 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 <alex.kiernan@gmail.com> > >> --- > >> 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? Thinking about this some more... if we moved that string write into the default fb_set_reboot_flag implementation and then guarded the reboot-bootloader command with a new Kconfig symbol, you could get the "I don't know how to do that" behaviour by disabling the command?
On Tue, May 1, 2018 at 1:21 AM Alex Kiernan <alex.kiernan@gmail.com> wrote: > On Tue, May 1, 2018 at 8:22 AM Alex Kiernan <alex.kiernan@gmail.com> > wrote: > > > > On Tue, May 1, 2018 at 7:50 AM Jocelyn Bohr <bohr@google.com> wrote: > > > > > > On Mon, Apr 30, 2018 at 1:33 AM Alex Kiernan <alex.kiernan@gmail.com> > > 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 <alex.kiernan@gmail.com> > > >> --- > > > >> 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? > > > Thinking about this some more... if we moved that string write into the > default fb_set_reboot_flag implementation and then guarded the > reboot-bootloader command with a new Kconfig symbol, you could get the "I > don't know how to do that" behaviour by disabling the command? > The "reboot-bootloader" behavior isn't RPi specific, it's part of the fastboot protocol and is required by all Android bootloaders that support fastboot. https://android.googlesource.com/platform/system/core/+/master/fastboot/ So my thinking was that since it's part of the protocol, the weak function could have some default implementation. I can't think of a reason for a device to support fastboot but not the "reboot-bootloader" command.
On Tue, May 1, 2018 at 10:46 PM Jocelyn Bohr <bohr@google.com> wrote: > > > On Tue, May 1, 2018 at 1:21 AM Alex Kiernan <alex.kiernan@gmail.com> > wrote: > >> On Tue, May 1, 2018 at 8:22 AM Alex Kiernan <alex.kiernan@gmail.com> >> wrote: >> >> >> > On Tue, May 1, 2018 at 7:50 AM Jocelyn Bohr <bohr@google.com> wrote: >> >> >> >> > > On Mon, Apr 30, 2018 at 1:33 AM Alex Kiernan <alex.kiernan@gmail.com> >> > 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 <alex.kiernan@gmail.com> >> > >> --- >> >> > >> 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? >> >> >> Thinking about this some more... if we moved that string write into the >> default fb_set_reboot_flag implementation and then guarded the >> reboot-bootloader command with a new Kconfig symbol, you could get the "I >> don't know how to do that" behaviour by disabling the command? >> > > The "reboot-bootloader" behavior isn't RPi specific, it's part of the > fastboot > protocol and is required by all Android bootloaders that support fastboot. > > https://android.googlesource.com/platform/system/core/+/master/fastboot/ > > So my thinking was that since it's part of the protocol, the weak function > could > have some default implementation. I can't think of a reason for a device to > support fastboot but not the "reboot-bootloader" command. > I think I'm also fine with adding a default implementation later, when common/android-bootloader.c from AOSP gets merged. That would probably make more sense, because currently nothing in the upstream code reads the "reboot-bootloader" flag. But I prefer to not add a new Kconfig to guard the reboot-bootloader command. > -- >> Alex Kiernan >> >
On Wed, May 2, 2018 at 6:51 AM Jocelyn Bohr <bohr@verily.com> wrote: > On Tue, May 1, 2018 at 10:46 PM Jocelyn Bohr <bohr@google.com> wrote: >> On Tue, May 1, 2018 at 1:21 AM Alex Kiernan <alex.kiernan@gmail.com> wrote: >>> On Tue, May 1, 2018 at 8:22 AM Alex Kiernan <alex.kiernan@gmail.com> wrote: >>> > On Tue, May 1, 2018 at 7:50 AM Jocelyn Bohr <bohr@google.com> wrote: >>> > > On Mon, Apr 30, 2018 at 1:33 AM Alex Kiernan <alex.kiernan@gmail.com >>> > wrote: ... >>> > >> 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? >>> Thinking about this some more... if we moved that string write into the >>> default fb_set_reboot_flag implementation and then guarded the >>> reboot-bootloader command with a new Kconfig symbol, you could get the "I >>> don't know how to do that" behaviour by disabling the command? >> The "reboot-bootloader" behavior isn't RPi specific, it's part of the fastboot >> protocol and is required by all Android bootloaders that support fastboot. >> https://android.googlesource.com/platform/system/core/+/master/fastboot/ >> So my thinking was that since it's part of the protocol, the weak function could >> have some default implementation. I can't think of a reason for a device to >> support fastboot but not the "reboot-bootloader" command. > I think I'm also fine with adding a default implementation later, when > common/android-bootloader.c from AOSP gets merged. That would probably make > more sense, because currently nothing in the upstream code reads the > "reboot-bootloader" flag. But I prefer to not add a new Kconfig to guard the > reboot-bootloader command. Thanks, I'll leave it like it is... Whenever code to honour it gets added, the various other mechanisms that already exist for fb_set_reboot_flag() want looking at too as there's a mix of approaches today (my least favourite of which is arch/arm/mach-omap2/boot-common.c... which the board I'm interested in ends up using).
On Mon, Apr 30, 2018 at 3:32 AM, Alex Kiernan <alex.kiernan@gmail.com> 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 <alex.kiernan@gmail.com> One nit below, but, Acked-by: Joe Hershberger <joe.hershberger@ni.com> > --- > > 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; Why did you stop returning a proper errno? > +} > 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, > - [FB_CMD_REBOOT_BOOTLOADER] = cb_reboot, > + [FB_CMD_REBOOT] = cb_okay, > + [FB_CMD_REBOOT_BOOTLOADER] = cb_reboot_bootloader, > [FB_CMD_POWERDOWN] = NULL, > [FB_CMD_SET_ACTIVE] = cb_okay, > [FB_CMD_UPLOAD] = NULL, > @@ -370,12 +370,13 @@ static void cb_continue(char *cmd_parameter, char *fastboot_data, > * > * @param repsonse Pointer to fastboot response buffer > */ > -static void cb_reboot(char *cmd_parameter, char *fastboot_data, > - unsigned int fastboot_data_len, char *response) > +static void cb_reboot_bootloader(char *cmd_parameter, char *fastboot_data, > + unsigned int fastboot_data_len, char *response) > { > - fastboot_okay(NULL, response); > - if (!strcmp("reboot-bootloader", cmd_string)) > - strcpy((char *)CONFIG_FASTBOOT_BUF_ADDR, "reboot-bootloader"); > + if (fb_set_reboot_flag()) > + fastboot_fail("Cannot set reboot flag", response); > + else > + fastboot_okay(NULL, response); > } > > /** > -- > 2.7.4 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
On Thu, May 3, 2018 at 10:16 PM Joe Hershberger <joe.hershberger@ni.com> wrote: > On Mon, Apr 30, 2018 at 3:32 AM, Alex Kiernan <alex.kiernan@gmail.com> 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 <alex.kiernan@gmail.com> > One nit below, but, > Acked-by: Joe Hershberger <joe.hershberger@ni.com> > > --- > > > > 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; > Why did you stop returning a proper errno? checkpatch doesn't like it: WARNING: ENOSYS means 'invalid syscall nr' and nothing else #10: FILE: drivers/fastboot/fb_common.c:92: + return -ENOSYS; If that's a warning we're happy to ignore, I'll swap it back.
On Fri, May 4, 2018 at 2:34 AM, Alex Kiernan <alex.kiernan@gmail.com> wrote: > On Thu, May 3, 2018 at 10:16 PM Joe Hershberger <joe.hershberger@ni.com> > wrote: > >> On Mon, Apr 30, 2018 at 3:32 AM, Alex Kiernan <alex.kiernan@gmail.com> > 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 <alex.kiernan@gmail.com> > >> One nit below, but, > >> Acked-by: Joe Hershberger <joe.hershberger@ni.com> > >> > --- >> > >> > 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; > >> Why did you stop returning a proper errno? > > > checkpatch doesn't like it: > > WARNING: ENOSYS means 'invalid syscall nr' and nothing else > #10: FILE: drivers/fastboot/fb_common.c:92: > + return -ENOSYS; > > If that's a warning we're happy to ignore, I'll swap it back. Yep, I'll ignore it. That's a Linux-only complaint. It should be added to the checkpatch config to be skipped. > -- > Alex Kiernan > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
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; +} 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, - [FB_CMD_REBOOT_BOOTLOADER] = cb_reboot, + [FB_CMD_REBOOT] = cb_okay, + [FB_CMD_REBOOT_BOOTLOADER] = cb_reboot_bootloader, [FB_CMD_POWERDOWN] = NULL, [FB_CMD_SET_ACTIVE] = cb_okay, [FB_CMD_UPLOAD] = NULL, @@ -370,12 +370,13 @@ static void cb_continue(char *cmd_parameter, char *fastboot_data, * * @param repsonse Pointer to fastboot response buffer */ -static void cb_reboot(char *cmd_parameter, char *fastboot_data, - unsigned int fastboot_data_len, char *response) +static void cb_reboot_bootloader(char *cmd_parameter, char *fastboot_data, + unsigned int fastboot_data_len, char *response) { - fastboot_okay(NULL, response); - if (!strcmp("reboot-bootloader", cmd_string)) - strcpy((char *)CONFIG_FASTBOOT_BUF_ADDR, "reboot-bootloader"); + if (fb_set_reboot_flag()) + fastboot_fail("Cannot set reboot flag", response); + else + fastboot_okay(NULL, response); } /**
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 <alex.kiernan@gmail.com> --- 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(-)