Re: [PATCH v1 1/5] fastboot: multiresponse support

Hi Svyatoslav,

Thank you for your patch.

On mar., nov. 07, 2023 at 14:42, Svyatoslav Ryhel clamor95@gmail.com wrote:

From: Ion Agorria ion@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 ion@agorria.com Signed-off-by: Svyatoslav Ryhel clamor95@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...

• 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