Re: [PATCH v1 0/5] Implement fastboot multiresponce

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Svyatoslav,

On jeu., nov. 09, 2023 at 11:01, Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> 9 листопада 2023 р. 10:41:30 GMT+02:00, Mattijs Korpershoek <mkorpershoek@baylibre.com> написав(-ла):
>>Hi Svyatoslav, Ion,
>>
>>Thank you for these series.
>>
>>On mar., nov. 07, 2023 at 14:42, Svyatoslav Ryhel <clamor95@gmail.com> wrote:
>>
>>> 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 set.
>>>
>>> Implementation of multiresponce leads to ability to dump content of
>>> console buffer which, with use of "oem run", allows to entirely avoid
>>> need in UART.
>>
>
> Hello Mattijs!
> Thank you for your suggestions and so rapid responce!
>
>>I'm happy to fastboot getvar all implemented since it's standardly
>>supported on Android phones.
>>
>>fastboot oem console seems like a nice debug tool as well when no
>>console is available.
>>
>>I do have some concerns with the series, mainly because the introduction
>>of the "MORE" byte response.
>>"MORE" is not part of the fastboot specification [1].
>>
>>However, that specification documents "TEXT", which, per my
>>understanding, should allow to send multiple responses as well.
>>
>>Could we implement this using "TEXT" instead ?
>
> I see no issues with replacing MORE with TEXT whatsoever.
>
> These patches were mainly developed and used for our internal development needs but since they served extremely well, I have decided to push them upstream so other developers could benefit as well.

Thank you for taking the time to upstream.

>
> If you have no other suggestions then swap more > text, please send your reviewed-by to patch and I will reload v2 with swap included.

I will do a more thorough review in the coming days, I wanted to get
feedback on MORE->TEXT first.

>
> Best regards,
> Svyatoslav R.
>
>>[1] https://android.googlesource.com/platform/system/core/+/master/fastboot/#transport-and-framing
>>
>>>
>>> Ion Agorria (5):
>>>   fastboot: multiresponse support
>>>   fastboot: implement "getvar all"
>>>   commonn: console: introduce overflow and isempty calls
>>>   lib: membuff: fix readline not returning line in case of overflow
>>>   fastboot: add oem console command support
>>>
>>>  boot/bootmeth_extlinux.c          |  2 +-
>>>  common/console.c                  | 17 +++++--
>>>  doc/android/fastboot-protocol.rst |  3 ++
>>>  doc/android/fastboot.rst          |  1 +
>>>  drivers/fastboot/Kconfig          |  7 +++
>>>  drivers/fastboot/fb_command.c     | 52 +++++++++++++++++++++
>>>  drivers/fastboot/fb_getvar.c      | 75 +++++++++++++++++++++++++------
>>>  drivers/usb/gadget/f_fastboot.c   | 29 ++++++++++++
>>>  include/console.h                 | 14 ++++++
>>>  include/fastboot-internal.h       |  7 +++
>>>  include/fastboot.h                |  9 ++++
>>>  include/membuff.h                 |  5 ++-
>>>  lib/membuff.c                     |  4 +-
>>>  net/fastboot_udp.c                | 25 ++++++++---
>>>  test/ut.c                         |  9 ++--
>>>  15 files changed, 226 insertions(+), 33 deletions(-)
>>>
>>> -- 
>>> 2.40.1