Hi Svyatoslav,
Thank you for your patch.
On mar., nov. 07, 2023 at 14:42, Svyatoslav Ryhel <clamo...@gmail.com> wrote:
> From: Ion Agorria <i...@agorria.com>
>
> Currently u-boot fastboot can only send one message back to host,
> so if there is a need to print more than one line messages must be
> kept sending until all the required data is obtained. This behavior
> can be adjusted using multiresponce ability (getting multiple lines
> of response) proposed in this patch.
>
> Signed-off-by: Ion Agorria <i...@agorria.com>
> Signed-off-by: Svyatoslav Ryhel <clamo...@gmail.com>
> ---
> drivers/fastboot/fb_command.c | 10 ++++++++++
> drivers/usb/gadget/f_fastboot.c | 29 +++++++++++++++++++++++++++++
> include/fastboot.h | 8 ++++++++
> net/fastboot_udp.c | 25 +++++++++++++++++++------
> 4 files changed, 66 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c
> index 5fcadcdf50..ab72d8c781 100644
> --- a/drivers/fastboot/fb_command.c
> +++ b/drivers/fastboot/fb_command.c
> @@ -5,6 +5,7 @@
>
> #include <common.h>
> #include <command.h>
> +#include <console.h>
> #include <env.h>
> #include <fastboot.h>
> #include <fastboot-internal.h>
> @@ -152,6 +153,15 @@ int fastboot_handle_command(char *cmd_string, char
> *response)
> return -1;
> }
>
> +void fastboot_multiresponse(int cmd, char *response)
> +{
> + switch (cmd) {
> + default:
> + fastboot_fail("Unknown multiresponse command", response);
> + break;
> + }
> +}
> +
> /**
> * okay() - Send bare OKAY response
> *
> diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
> index 741775a7bc..7d8c22b948 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -497,6 +497,25 @@ static void do_bootm_on_complete(struct usb_ep *ep,
> struct usb_request *req)
> do_exit_on_complete(ep, req);
> }
>
> +int multiresponse_cmd = -1;
This is non static, because it is also used in net/fastboot_udp.c
However, if we:
1. disable CONFIG_USB_FUNCTION_FASTBOOT
2. enable CONFIG_UDP_FUNCTION_FASTBOOT
The build will break with:
net/fastboot_udp.c: In function 'fastboot_send':
net/fastboot_udp.c:187:64: error: 'multiresponse_cmd' undeclared (first use in
this function)
187 |
fastboot_multiresponse(multiresponse_cmd, response);
|
^~~~~~~~~~~~~~~~~
net/fastboot_udp.c:187:64: note: each undeclared identifier is reported only
once for each function it appears in
Since having only CONFIG_UDP_FUNCTION_FASTBOOT is valid (there is no
dependency on CONFIG_USB_FASTBOOT), can we think of something else to
handle multi-response commands without breaking UDP-only fastboot
support ?
> +static void multiresponse_on_complete(struct usb_ep *ep, struct usb_request
> *req)
> +{
> + char response[FASTBOOT_RESPONSE_LEN] = {0};
> +
> + if (multiresponse_cmd == -1)
> + return;
> +
> + /* Call handler to obtain next response */
> + fastboot_multiresponse(multiresponse_cmd, response);
> + fastboot_tx_write_str(response);
> +
> + /* If response is not an INFO disconnect this handler and unset cmd */
> + if (strncmp("INFO", response, 4) != 0) {
> + multiresponse_cmd = -1;
> + fastboot_func->in_req->complete = fastboot_complete;
> + }
> +}
> +
> static void do_acmd_complete(struct usb_ep *ep, struct usb_request *req)
> {
> /* When usb dequeue complete will be called
> @@ -524,6 +543,16 @@ static void rx_handler_command(struct usb_ep *ep, struct
> usb_request *req)
> fastboot_fail("buffer overflow", response);
> }
>
> + if (!strncmp("MORE", response, 4)) {
As discussed in [1], please use TEXT instead of MORE
[1]
https://lore.kernel.org/all/capvz0n2foxehqooz-obpwucq7ldfskzfgxzw5bp-qf8saxl...@mail.gmail.com/
> + multiresponse_cmd = cmd;
> + fastboot_multiresponse(multiresponse_cmd, response);
> +
> + /* Only add if first is a INFO */
> + if (!strncmp("INFO", response, 4)) {
> + fastboot_func->in_req->complete =
> multiresponse_on_complete;
> + }
> + }
> +
> if (!strncmp("DATA", response, 4)) {
> req->complete = rx_handler_dl_image;
> req->length = rx_bytes_expected(ep);
> diff --git a/include/fastboot.h b/include/fastboot.h
> index 296451f89d..d1a2b74b2f 100644
> --- a/include/fastboot.h
> +++ b/include/fastboot.h
> @@ -172,5 +172,13 @@ void fastboot_data_download(const void *fastboot_data,
> */
> void fastboot_data_complete(char *response);
>
> +/**
> + * fastboot_handle_multiresponse() - Called for each response to send
> + *
> + * @cmd: Command id that requested multiresponse
> + * @response: Pointer to fastboot response buffer
> + */
> +void fastboot_multiresponse(int cmd, char *response);
> +
> void fastboot_acmd_complete(void);
> #endif /* _FASTBOOT_H_ */
> diff --git a/net/fastboot_udp.c b/net/fastboot_udp.c
> index d690787478..ca3278f7b4 100644
> --- a/net/fastboot_udp.c
> +++ b/net/fastboot_udp.c
> @@ -42,16 +42,15 @@ static int fastboot_remote_port;
> static int fastboot_our_port;
>
> /**
> - * fastboot_udp_send_info() - Send an INFO packet during long commands.
> + * fastboot_udp_send_response() - Send an response into UDP
> *
> - * @msg: String describing the reason for waiting
> + * @response: Response to send
> */
> -static void fastboot_udp_send_info(const char *msg)
> +static void fastboot_udp_send_response(const char *response)
> {
> uchar *packet;
> uchar *packet_base;
> int len = 0;
> - char response[FASTBOOT_RESPONSE_LEN] = {0};
>
> struct fastboot_header response_header = {
> .id = FASTBOOT_FASTBOOT,
> @@ -66,7 +65,6 @@ static void fastboot_udp_send_info(const char *msg)
> memcpy(packet, &response_header, sizeof(response_header));
> packet += sizeof(response_header);
> /* Write response */
> - fastboot_response("INFO", response, "%s", msg);
> memcpy(packet, response, strlen(response));
> packet += strlen(response);
>
> @@ -91,6 +89,7 @@ static void fastboot_udp_send_info(const char *msg)
> static void fastboot_timed_send_info(const char *msg)
> {
> static ulong start;
> + char response[FASTBOOT_RESPONSE_LEN] = {0};
>
> /* Initialize timer */
> if (start == 0)
> @@ -99,7 +98,8 @@ static void fastboot_timed_send_info(const char *msg)
> /* Send INFO packet to host every 30 seconds */
> if (time >= 30000) {
> start = get_timer(0);
> - fastboot_udp_send_info(msg);
> + fastboot_response("INFO", response, "%s", msg);
> + fastboot_udp_send_response(response);
> }
> }
>
> @@ -180,6 +180,19 @@ static void fastboot_send(struct fastboot_header header,
> char *fastboot_data,
> } else {
> cmd = fastboot_handle_command(command, response);
> pending_command = false;
> +
> + if (!strncmp("MORE", response, 4)) {
MORE -> TEXT
> + while (1) {
> + /* Call handler to obtain next response
> */
> +
> fastboot_multiresponse(multiresponse_cmd, response);
As written previously, multiresponse_cmd is undefined when:
CONFIG_USB_FUNCTION_FASTBOOT=n && CONFIG_UDP_FUNCTION_FASTBOOT=y
> +
> + /* Send INFO or break to send final
> response */
> + if (!strncmp("INFO", response, 4))
> +
> fastboot_udp_send_response(response);
> + else
> + break;
> + }
> + }
> }
> /*
> * Sent some INFO packets, need to update sequence number in
> --
> 2.40.1
