Hi Svyatoslav,
On mar., nov. 14, 2023 at 12:30, Svyatoslav Ryhel clamor95@gmail.com wrote:
14 листопада 2023 р. 12:24:52 GMT+02:00, Mattijs Korpershoek mkorpershoek@baylibre.com написав(-ла):
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
"oem console" serves to read console record buffer.
Signed-off-by: Ion Agorria ion@agorria.com Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com
doc/android/fastboot.rst | 1 + drivers/fastboot/Kconfig | 7 +++++++ drivers/fastboot/fb_command.c | 39 +++++++++++++++++++++++++++++++++++ include/fastboot.h | 1 + 4 files changed, 48 insertions(+)
diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst index 1ad8a897c8..05d8f77759 100644 --- a/doc/android/fastboot.rst +++ b/doc/android/fastboot.rst @@ -29,6 +29,7 @@ The following OEM commands are supported (if enabled): with <arg> = boot_ack boot_partition
- ``oem bootbus`` - this executes ``mmc bootbus %x %s`` to configure eMMC
- ``oem run`` - this executes an arbitrary U-Boot command
+- ``oem console`` - this dumps U-Boot console record buffer
Support for both eMMC and NAND devices is included.
diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig index 837c6f1180..58b08120a4 100644 --- a/drivers/fastboot/Kconfig +++ b/drivers/fastboot/Kconfig @@ -241,6 +241,13 @@ config FASTBOOT_OEM_RUN this feature if you are using verified boot, as it will allow an attacker to bypass any restrictions you have in place.
+config FASTBOOT_CMD_OEM_CONSOLE
- bool "Enable the 'oem console' command"
- depends on CONSOLE_RECORD
- help
Add support for the "oem console" command to input and read consolerecord buffer.endif # FASTBOOT
endmenu diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c index 6f621df074..acf5971108 100644 --- a/drivers/fastboot/fb_command.c +++ b/drivers/fastboot/fb_command.c @@ -41,6 +41,7 @@ static void reboot_recovery(char *, char *); static void oem_format(char *, char *); static void oem_partconf(char *, char *); static void oem_bootbus(char *, char *); +static void oem_console(char *, char *); static void run_ucmd(char *, char *); static void run_acmd(char *, char *);
@@ -108,6 +109,10 @@ static const struct { .command = "oem run", .dispatch = CONFIG_IS_ENABLED(FASTBOOT_OEM_RUN, (run_ucmd), (NULL)) },
- [FASTBOOT_COMMAND_OEM_CONSOLE] = {
.command = "oem console",.dispatch = CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_CONSOLE, (oem_console), (NULL))- }, [FASTBOOT_COMMAND_UCMD] = { .command = "UCmd", .dispatch = CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (run_ucmd), (NULL))
@@ -159,6 +164,23 @@ void fastboot_multiresponse(int cmd, char *response) case FASTBOOT_COMMAND_GETVAR: fastboot_getvar_all(response); break; +#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_CONSOLE)
Checkpatch also complains about this.
Can we rewrite this using if (IS_ENABLED(CONFIG...)) please ?
Please, do not relay on checkpatch that much. In this case #ifdef is better since in this case all under ifdef will be cut off while using if(...) requires all code under the if to be able to be run even if config is not enabled. Thanks.
I understand that sometimes, checkpatch generates false positives or bad suggestions. I also understand the differences between #ifdef and if (IS_ENABLED()).
I did not measure whether the binary size is bigger when switching from #ifdef to "if IS_ENABLED()" but I suspect that the compiler can optimize this out as it's known at compile-time.
This file (and the fastboot code in general) mostly uses "if (IS_ENABLED())" and to keep the code consistent I recommend using it.
Therefore, can we please use if (IS_ENABLED()) here ?
Thank you
Mattijs
- case FASTBOOT_COMMAND_OEM_CONSOLE:
char buf[FASTBOOT_RESPONSE_LEN] = { 0 };if (console_record_isempty()) {console_record_reset();fastboot_okay(NULL, response);} else {int ret = console_record_readline(buf, sizeof(buf) - 5);if (ret < 0)fastboot_fail("Error reading console", response);elsefastboot_response("INFO", response, "%s", buf);}break;+#endif default: fastboot_fail("Unknown multiresponse command", response); break; @@ -503,3 +525,20 @@ static void __maybe_unused oem_bootbus(char *cmd_parameter, char *response) else fastboot_okay(NULL, response); }
+/**
- oem_console() - Execute the OEM console command
- @cmd_parameter: Pointer to command parameter
- @response: Pointer to fastboot response buffer
- */
+static void __maybe_unused oem_console(char *cmd_parameter, char *response) +{
- if (cmd_parameter)
console_in_puts(cmd_parameter);- if (console_record_isempty())
fastboot_fail("Empty console", response);- else
fastboot_response("MORE", response, NULL);MORE -> TEXT
+} diff --git a/include/fastboot.h b/include/fastboot.h index d1a2b74b2f..23d26fb4be 100644 --- a/include/fastboot.h +++ b/include/fastboot.h @@ -37,6 +37,7 @@ enum { FASTBOOT_COMMAND_OEM_PARTCONF, FASTBOOT_COMMAND_OEM_BOOTBUS, FASTBOOT_COMMAND_OEM_RUN,
- FASTBOOT_COMMAND_OEM_CONSOLE, FASTBOOT_COMMAND_ACMD, FASTBOOT_COMMAND_UCMD, FASTBOOT_COMMAND_COUNT
-- 2.40.1