Re: [PATCH v9 3/4] Boot var automatic management for removable medias

2023-06-17 Thread Heinrich Schuchardt

On 6/7/23 17:05, Raymond Mao wrote:

Changes for complying to EFI spec §3.5.1.1
'Removable Media Boot Behavior'.
Boot variables can be automatically generated during a removable
media is probed. At the same time, unused boot variables will be
detected and removed.

Please note that currently the function 'efi_disk_remove' has no
ability to distinguish below two scenarios
a) Unplugging of a removable media under U-Boot
b) U-Boot existing and booting an OS


%s/existing/exiting/


Thus currently the boot variables management is not added into
'efi_disk_remove' to avoid boot options being added/erased
repeatedly under scenario b) during power cycles
See TODO comments under function 'efi_disk_remove' for more details

Signed-off-by: Raymond Mao 
Reviewed-by: Heinrich Schuchardt 
Reviewed-by: Ilias Apalodimas 
---
Changes in v3
- Split the patch into moving and renaming functions and
   individual patches for each changed functionality
Changes in v5
- Move function call of efi_bootmgr_update_media_device_boot_option()
   from efi_init_variables() to efi_init_obj_list()
Changes in v6
- Revert unrelated changes
Changes in v7
- adapt the return code of function
   efi_bootmgr_update_media_device_boot_option()
Changes in v8
- add a note in the commit message for future reference
Changes in v9
- amend the note text in the commit message
- add a TODO comment in 'efi_disk_remove'

  lib/efi_loader/efi_disk.c  | 18 ++
  lib/efi_loader/efi_setup.c |  5 +
  2 files changed, 23 insertions(+)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index d2256713a8..8ccf3dd158 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -687,6 +687,13 @@ int efi_disk_probe(void *ctx, struct event *event)
return -1;
}

+   /* only do the boot option management when UEFI sub-system is 
initialized */
+   if (efi_obj_list_initialized == EFI_SUCCESS) {
+   ret = efi_bootmgr_update_media_device_boot_option();
+   if (ret != EFI_SUCCESS)
+   return -1;
+   }
+
return 0;
  }

@@ -773,6 +780,17 @@ int efi_disk_remove(void *ctx, struct event *event)
return efi_disk_delete_part(dev);
else
return 0;
+
+   /*
+* TODO A flag to distinguish below 2 different scenarios of this
+* function call is needed:
+* a) Unplugging of a removable media under U-Boot
+* b) U-Boot existing and booting an OS


I guess you meant exiting.

b) ExitBootServices() invoked or booting an operating system

I will fix this when merging.

Best regards

Heinrich


+* In case of scenario a), efi_bootmgr_update_media_device_boot_option()
+* needs to be invoked here to update the boot options and remove the
+* unnecessary ones.
+*/
+
  }

  /**
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 58d4e13402..877f3878d6 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -245,6 +245,11 @@ efi_status_t efi_init_obj_list(void)
if (ret != EFI_SUCCESS)
goto out;

+   /* update boot option after variable service initialized */
+   ret = efi_bootmgr_update_media_device_boot_option();
+   if (ret != EFI_SUCCESS)
+   goto out;
+
/* Define supported languages */
ret = efi_init_platform_lang();
if (ret != EFI_SUCCESS)




Re: [PATCH 0/8] SUNIV SPI NAND support in SPL

2023-06-17 Thread Sam Edwards

Hi again Icenowy,

On 6/6/23 17:09, Sam Edwards wrote:
Still, I believe it's sensible that, when we know for sure we're coming 
from SPI-NAND (because it's a newer sunxi that reports 0x04, or we know 
from the suniv stack-checking hack), we should call that its own SPL 
load method, which does not fall back to SPI-NOR. The SPI(-NOR) load 
method naturally has to implement the try-NAND-first logic for some of 
these SoCs, but perhaps we could call it a "quirk" and only do that for 
chips that aren't known to report SPI-NAND directly?


Since other methods that care about flash type (e.g. env_get_location) 
don't understand BOOT_DEVICE_SPI to mean "SPI, but the actual flash type 
is ambiguous," I'm starting to think we need to resolve the ambiguity 
*before* the load method runs.


What are your thoughts on this:
1. Introduce SUNXI_BOOTED_FROM_SPI_NAND, value 4 (matching the value 
used by Allwinner's newer BROMs).

2. Map SUNIV_BOOTED_FROM_NAND to SUNXI_BOOTED_FROM_SPI_NAND.
3. Map SUNXI_BOOTED_FROM_SPI_NAND to BOOT_DEVICE_NAND (and not _SPI).
4. Implement the SPI-NAND stuff to provide `nand_spl_load_image` (so 
that the common SPL framework's NAND loader can take care of it). 
Possibly go even further and implement the methods needed by the UBI 
loader too.
5. On sunxis (e.g. V3s) which are known to report SUNXI_BOOTED_FROM_SPI 
for SPI-NAND, add a small check to sunxi_get_boot_device() which 
disambiguates this situation by probing for SPI-NAND.


If #4 isn't really feasible, we might instead want to introduce a 
BOOT_DEVICE_SPI_NAND, but I don't know if adding sunxi to the list of 
platforms that have their own BOOT_DEVICE_ names is a great call.


In any case, I'd hope also to see some kind of bad block handling in 
here -- a simple check for a bad block each time we get to page 0 (which 
skips to the next block) should be pretty easy, don't you think?


Any thoughts? Any other way I can be of service? (Feel free to delegate 
some of these ideas to me if you think they're good but don't have the 
time to execute them!)


Warm regards,
Sam


Re: [PATCH] menu: Re-enable the ANSI codes

2023-06-17 Thread Heinrich Schuchardt

On 6/17/23 12:48, Simon Glass wrote:

Hi Pali,

On Thu, 15 Jun 2023 at 17:56, Pali Rohár  wrote:


On Thursday 15 June 2023 13:34:09 Simon Glass wrote:

Hi Pali,

On Mon, 12 Jun 2023 at 22:33, Pali Rohár  wrote:


On Monday 12 June 2023 22:17:48 Simon Glass wrote:

Hi Pali,

On Mon, 12 Jun 2023 at 21:22, Pali Rohár  wrote:


On Monday 12 June 2023 21:14:32 Simon Glass wrote:

The intent here was to allow ANSI codes to be disabled, since it was
proving impoosible to test operation of the menu code when it kept moving
the cursor. Unfortunately this ended up in the patch.

Correct this by enabling ANSI again.

Signed-off-by: Simon Glass 
Reported-by: Pali Rohár 
Reported-by: Mark Kettenis 
Reported-by: Frank Wunderlich 
Fixes: 32bab0eae51b ("menu: Make use of CLI character processing")
---

  common/menu.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/menu.c b/common/menu.c
index 94514177e4e9..b55cf7b99967 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -15,7 +15,7 @@

  #include "menu.h"

-#define ansi 0
+#define ansi 1

  /*
   * Internally, each item in a menu is represented by a struct menu_item.
--
2.41.0.162.gfafddb0af9-goog



Hello, I have tested this change but bootmenu still does not work. There
is still same issue which I reported month ago. When I press DOWN key
then bootmenu immediately quit instead of moving into the next entry.


Thanks for testing this.

Is there a way for me to test this with sandbox? Or does your Nokia
test check it?


I guess that bootmenu command could work in sandbox. But I have not
tried it.

Nokia CI test does not try any terminal, keyboard or VGA interaction, so
broken rendering or broken keyboard input is not caught by CI.

But it is possible to test it manually. See U-Boot documentation how to
run Nokia u-boot image in qemu. Bootmenu is automatically started.
https://u-boot.readthedocs.io/en/latest/board/nokia/rx51.html#run-in-qemu


I tried to follow this but got stuck here:

./configure --enable-system --target-list=arm-softmmu --disable-werror

ERROR: Cannot use 'python', Python 2.4 or later is required.
Note that Python 3 or later is not yet supported.
Use --python=/path/to/python to specify a supported Python.

Python 2 has been deprecated for years and I think it was removed recently.


Our Docker image uses Ubuntu 22.04 (Jammy). Ubuntu 22.10 (Kinetic) was
the last Ubuntu release providing Python 2.7. So when upgrading our
Docker image next year we will have to quit emulating that 14 year old
device.



Ach :-( Is not there some configure option to disable python?


I don't know, but the qemu you are using seems to be 10 years old?


The Nokia N900 support never existed in upstream QEMU. Is is only in a
Linaro repository that has not been update since 2015. (No clue why our
CI uses the 2013 version and not the 2015 one.)

I anybody cares about N900 being used in U-Boot's CI he should rework
the N900 patch from the Linaro QEMU archive and get it integrated into
upstream QEMU.

Best regards

Heinrich






In U-Boot CI is qemu compiling without issues (but variant without GUI).


You mean that it still uses python 2? But for how much longer?

I don't think it is reasonable to base a test on an old version of qemu.





Bootmenu is available on both VGA display and terminal output. Problem
with DOWN key is on the terminal.



I'm sure you have this commit:?

17b45e684af9 ("cli: Correct several bugs in cli_getch()")


Yes, as it is in the master branch already.


I tried the following with sandbox:

setenv bootmenu_0 "my item=echo item 0"
setenv bootmenu_1 "second item=echo item 1"
setenv bootmenu_delay 10
bootmenu

I see that with the ANSI patch the 'press key' prompt now appears in
the right place.

When I press down key repeatedly, it goes down until the last item and
then refuses to go any further.

So I can't repeat this problem.


Hm... so it looks like some ARM or UART specific thing... or something
which is not triggered by sandbox.

Just some suggestions, could you try to add some more entries? Or trying
to wait e.g. 30s before pressing another key (qemu is slow so there can
be some delay issue?)? Or could you try to run bootmenu on some real
hardware via UART?


I did get it going by hacking my machine.

I'll send a series with some debugging, etc. I am not quite sure what
the problem is, but it seems to be timing-related, or perhaps a bug in
the keyboard code (I found one bug already).




I tried looking at the Nokia code. The keyboard driver should be in
drivers/input, but I eventually found it in the board directory.

Are you sure that rx51_kp_tstc() behaves correctly? Can you add
debugging to see what codes it is emitting?

Also you don't need to generate escape sequences in rx51_kp_fill().
You can just use Ctrl-N, Ctrl-P, Ctrl-B and Ctrl-F (see cli_ch_esc()).
That should simplify things a bit.

If you can find a way to repeat this problem and have verified that
the Nokia code is correct, I can take 

Re: [PATCH] doc: uefi: explicitly describe manual dtb update is required

2023-06-17 Thread Heinrich Schuchardt

On 6/15/23 10:03, Masahisa Kojima wrote:

To enforce anti-rollback to any older version, dtb must be
always update manually. This should be described in the
documentation.

Signed-off-by: Masahisa Kojima 
---
  doc/develop/uefi/uefi.rst | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index ffd13cebe9..d5f8c5f236 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -552,6 +552,9 @@ update using a capsule file with --fw-version of 5, the 
update will fail.
  When the --fw-version in the capsule file is updated, lowest-supported-version
  in the dtb might be updated accordingly.

+If user needs to enroce anti-rollback to any older version,
+the lowest-supported-version property in dtb must be always updated manually.


Thank you for updating the documentation.

Allowing to circumvent the rollback protection is a security issue. On a
secure system you would probably want to disable console commands like
mc and fdt. Shouldn't we provide an advice for safe settings?

E.g.

"If a user wanted to enable a rollback to a version forbidden by the
lowest-supported-version property specified in U-Boot's control
device-tree, they could change this property using the fdt command.
Secure systems should not enable this command."

Best regards

Heinrich


+
  To insert the lowest supported version into a dtb

  .. code-block:: console

base-commit: e350d0c60d413d441cbdfa9432ebadb56f625903




Re: Trying to boot custom kernel on Wink Hub (i.MX28)

2023-06-17 Thread Rogan Dawes
Thank you so much for the response! It is much appreciated!

I do hope to get JTAG working on the Hub v1, and then will be able to try
to build a modern U-Boot which I can flash.

Regards,

Rogan


On Thu, 15 Jun 2023 at 22:01, Fabio Estevam  wrote:

> Hi Rogan,
>
> On Thu, Jun 15, 2023 at 9:00 AM Rogan Dawes  wrote:
> >
> > Hi folks,
> >
> > I am trying to boot a custom kernel on a Wink Hub v1, which has an i.MX28
> > CPU, and is running U-Boot (U-Boot 2014.01-14400-gda781c6-dirty (Apr 30
> > 2014 - 22:35:38)).
> >
> > I have tried compiling my own modern kernel and putting it in place of
> the
> > vendor (Linux version 2.6.35.3-flex-dvt) kernel on the flash chip, but
> when
> > loading, I simply get the expected U-Boot loading messages, but then
> > nothing at all from the serial port after that.
> >
> > I'm trying to figure out where my problem is most likely to be. As far
> as I
> > am aware from long-ago poking at U-Boot, the way that U-Boot passes
> > arguments to the kernel has changed recently? Previously it was ATAGS,
> and
> > now it expects a Device Tree Blob? Might this explain the failure to boot
> > that I am seeing? And yes, I understand that given that
>
> Yes, to boot a modern kernel, you need to generate a devicetree for
> the Wink Hubv1 board.
>
> Then you build it and generate a .dtb file.
>
> To boot zImage + dtb using an old bootloader that does not support booting
> dtb,
> you would need to select the following options in the kernel:
>
> CONFIG_ARM_APPENDED_DTB=y
> CONFIG_ARM_ATAG_DTB_COMPAT=y
>
> And then:
>
> cat arch/arm/boot/zImage arch/arm/boot/dts/imx28-wink-hub-v1.dtb >
> zImage_with_dtb
>
> And then boot zImage_with_dtb as you do with 2.6.35.
>
> Ideally, you should also port a modern U-Boot version, which supports
> dtb by default and then you don't need the
> steps above.
>
> Take a look at board/freescale/mx28eevk support for a reference.
>
> Regards,
>
> Fabio Estevam
>


Re: [PATCH] Revert "lib: sparse: Make CHUNK_TYPE_RAW buffer aligned"

2023-06-17 Thread Tom Rini
On Fri, Jun 16, 2023 at 03:50:06PM +0200, Mattijs Korpershoek wrote:
> On ven., juin 16, 2023 at 13:56, Mattijs Korpershoek 
>  wrote:
> 
> > Hi Gary, Sean,
> >
> > On lun., nov. 21, 2022 at 10:09, Sean Anderson  
> > wrote:
> >
> >> On 11/21/22 09:50, Gary Bisson wrote:
> >>> Hi,
> >>> 
> >>> On Fri, Nov 18, 2022 at 10:36:58AM -0500, Sean Anderson wrote:
>  On 11/18/22 07:13, Gary Bisson wrote:
>  > This reverts commit 62649165cb02ab95b57360bb362886935f524f26.
>  > 
>  > The patch decreased the write performance quite a bit.
>  > Here is an example on an i.MX 8M Quad platform.
>  > - Before the revert:
>  > Sending sparse 'vendor' 1/2 (516436 KB)OKAY [  5.113s]
>  > Writing 'vendor'   OKAY [128.335s]
>  > Sending sparse 'vendor' 2/2 (76100 KB) OKAY [  0.802s]
>  > Writing 'vendor'   OKAY [ 27.902s]
>  > - After the revert:
>  > Sending sparse 'vendor' 1/2 (516436 KB)OKAY [  5.310s]
>  > Writing 'vendor'   OKAY [ 18.041s]
>  > Sending sparse 'vendor' 2/2 (76100 KB) OKAY [  1.244s]
>  > Writing 'vendor'   OKAY [  2.663s]
>  > 
>  > Considering that the patch only moves buffer around to avoid a warning
>  > message about misaligned buffers, let's keep the best performances.
>  
>  So what is the point of this warning?
> >>> 
> >>> Well the warning does say something true that the cache operation is not
> >>> aligned. Better ask Simon as he's the one who changed the print from a
> >>> debug to warn_non_spl one:
> >>> bcc53bf0958 arm: Show cache warnings in U-Boot proper only
> >>> 
> >>> BTW, in my case I couldn't see the misaligned messages, yet I saw the
> >>> performance hit described above.
> >
> > I also reproduce this problem on AM62x SK EVM.
> >
> > Before the revert:
> > Sending sparse 'super' 1/2 (768793 KB) OKAY [ 23.954s]
> > Writing 'super'OKAY [ 75.926s]
> > Sending sparse 'super' 2/2 (629819 KB) OKAY [ 19.641s]
> > Writing 'super'OKAY [ 62.849s]
> > Finished. Total time: 182.474s
> >
> > After the revert:
> > Sending sparse 'super' 1/2 (768793 KB) OKAY [ 23.895s]
> > Writing 'super'OKAY [ 12.961s]
> > Sending sparse 'super' 2/2 (629819 KB) OKAY [ 19.562s]
> > Writing 'super'OKAY [ 12.805s]
> > Finished. Total time: 69.327s
> >
> > And like Gary, I did not observe the misaligned messages.
> >
> > Did we come up with a solution for this performance regression?
> >
> > I will continue looking on my end but please let me know if you already
> > solved this.
> 
> Answering to myself here. My attempt of solving this problem has been
> submitted here:
> 
> https://lore.kernel.org/r/20230616-sparse-flash-fix-v1-1-6bafeacc5...@baylibre.com

Thanks for digging in to this!

-- 
Tom


signature.asc
Description: PGP signature


Re: Pull request efi-2023-07-rc5

2023-06-17 Thread Tom Rini
On Fri, Jun 16, 2023 at 09:22:30AM +0200, Heinrich Schuchardt wrote:

> Dear Tom,
> 
> The following changes since commit 2f4664f5c3edc55b18d8906f256a4c8e303243c0:
> 
>   Merge branch '2023-06-14-assorted-fixes' (2023-06-14 15:50:04 -0400)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-efi.git
> tags/efi-2023-07-rc5
> 
> for you to fetch changes up to 5669591dd8d2b21bc79237b161107300eb7f2b12:
> 
>   efi_selftests: fix protocol repeated selftesting (2023-06-16 06:48:46
> +0200)
> 
> Gitlab showed no issues:
> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/16607
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


[PATCH 6/6] menu: Add debugging

2023-06-17 Thread Simon Glass
Signed-off-by: Simon Glass 
---

 common/menu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/menu.c b/common/menu.c
index c81494b3d1f3..fe27262ebdd0 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -15,7 +15,7 @@
 
 #include "menu.h"
 
-#define ansi 1
+#define ansi 0
 
 /*
  * Internally, each item in a menu is represented by a struct menu_item.
@@ -447,8 +447,10 @@ enum bootmenu_key bootmenu_autoboot_loop(struct 
bootmenu_data *menu,
}
 
c = getchar();
+   printf("\n [c=%x] \n", c);
 
ichar = cli_ch_process(cch, c);
+   printf("\n [i=%x %d] \n", ichar, cch->esc_len);
 
if (!ichar)
continue;
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 5/6] getch: debugging

2023-06-17 Thread Simon Glass
Signed-off-by: Simon Glass 
---

 common/cli_getch.c | 23 ---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/common/cli_getch.c b/common/cli_getch.c
index 61d4cb261b81..cd44415a9f67 100644
--- a/common/cli_getch.c
+++ b/common/cli_getch.c
@@ -6,8 +6,11 @@
  * Copyright 2022 Google LLC
  */
 
+#define LOG_CATEGORY LOGC_CORE
+
 #include 
 #include 
+#include 
 
 /**
  * enum cli_esc_state_t - indicates what to do with an escape character
@@ -134,6 +137,8 @@ static int cli_ch_esc(struct cli_ch_state *cch, int ichar,
 
 int cli_ch_process(struct cli_ch_state *cch, int ichar)
 {
+   log_debug(" [ichar=%x, esc_len=%d] ", ichar, cch->esc_len);
+
/*
 * ichar=0x0 when error occurs in U-Boot getchar() or when the caller
 * wants to check if there are more characters saved in the escape
@@ -141,12 +146,18 @@ int cli_ch_process(struct cli_ch_state *cch, int ichar)
 */
if (!ichar) {
if (cch->emitting) {
-   if (cch->emit_upto < cch->esc_len)
-   return cch->esc_save[cch->emit_upto++];
+   if (cch->emit_upto < cch->esc_len) {
+   int ch;
+
+   ch = cch->esc_save[cch->emit_upto++];
+   log_debug(" [->%x] ", ch);
+   return ch;
+   }
cch->emit_upto = 0;
cch->emitting = false;
cch->esc_len = 0;
}
+   log_debug(" [->0] ");
return 0;
} else if (ichar == -ETIMEDOUT) {
/*
@@ -156,15 +167,19 @@ int cli_ch_process(struct cli_ch_state *cch, int ichar)
 */
if (cch->esc_len) {
cch->esc_len = 0;
+   log_debug(" [->esc] ");
return '\e';
}
 
/* Otherwise there is nothing to return */
+   log_debug(" [->0] ");
return 0;
}
 
-   if (ichar == '\n' || ichar == '\r')
+   if (ichar == '\n' || ichar == '\r') {
+   log_debug(" [->nl] ");
return '\n';
+   }
 
/* handle standard linux xterm esc sequences for arrow key, etc. */
if (cch->esc_len != 0) {
@@ -186,6 +201,7 @@ int cli_ch_process(struct cli_ch_state *cch, int ichar)
cch->esc_save[cch->esc_len++] = ichar;
ichar = cch->esc_save[cch->emit_upto++];
cch->emitting = true;
+   log_debug(" [->reject %x] ", ichar);
return ichar;
case ESC_CONVERTED:
/* valid escape sequence, return the resulting char */
@@ -205,5 +221,6 @@ int cli_ch_process(struct cli_ch_state *cch, int ichar)
return 0;
}
 
+   log_debug(" [->%x] ", ichar);
return ichar;
 }
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 3/6] bootmenu: Cancel delay only when a real key is pressed

2023-06-17 Thread Simon Glass
We need to decode the input character before deciding if it is a real
key, or just part of an escape sequence. Check this.

Signed-off-by: Simon Glass 
---

 common/menu.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/common/menu.c b/common/menu.c
index b55cf7b99967..c81494b3d1f3 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -446,11 +446,14 @@ enum bootmenu_key bootmenu_autoboot_loop(struct 
bootmenu_data *menu,
continue;
}
 
-   menu->delay = -1;
c = getchar();
 
ichar = cli_ch_process(cch, c);
 
+   if (!ichar)
+   continue;
+   menu->delay = -1;
+
switch (ichar) {
case '\0':
key = BKEY_NONE;
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 4/6] boobtmenu: Add debugging

2023-06-17 Thread Simon Glass
Signed-off-by: Simon Glass 
---

 cmd/bootmenu.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index 6baeedc69f91..b2fb119e70f9 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -18,6 +18,8 @@
 #include 
 #include 
 
+#define ansi 0
+
 /* maximum bootmenu entries */
 #define MAX_COUNT  99
 
@@ -101,6 +103,7 @@ static char *bootmenu_choice_entry(void *data)
/* Some key was pressed, so autoboot was stopped */
key = bootmenu_loop(menu, cch);
}
+   printf("[got bkey %d]\n", key);
 
switch (key) {
case BKEY_UP:
@@ -521,9 +524,11 @@ static enum bootmenu_ret bootmenu_show(int delay)
/* Default menu entry is always first */
menu_default_set(menu, "0");
 
-   puts(ANSI_CURSOR_HIDE);
-   puts(ANSI_CLEAR_CONSOLE);
-   printf(ANSI_CURSOR_POSITION, 1, 1);
+   if (ansi) {
+// puts(ANSI_CURSOR_HIDE);
+// puts(ANSI_CLEAR_CONSOLE);
+// printf(ANSI_CURSOR_POSITION, 1, 1);
+   }
 
init = 1;
 
@@ -566,10 +571,10 @@ cleanup:
menu_destroy(menu);
bootmenu_destroy(bootmenu);
 
-   if (init) {
-   puts(ANSI_CURSOR_SHOW);
-   puts(ANSI_CLEAR_CONSOLE);
-   printf(ANSI_CURSOR_POSITION, 1, 1);
+   if (init && ansi) {
+// puts(ANSI_CURSOR_SHOW);
+// puts(ANSI_CLEAR_CONSOLE);
+// printf(ANSI_CURSOR_POSITION, 1, 1);
}
 
if (title && command) {
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 2/6] nokia_rx51: Correct tstc() implementation

2023-06-17 Thread Simon Glass
This returns false even when there are keys in the buffer. Fix it.

Signed-off-by: Simon Glass 
---

 board/nokia/rx51/rx51.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/board/nokia/rx51/rx51.c b/board/nokia/rx51/rx51.c
index 238b9637badd..138f5a811eb6 100644
--- a/board/nokia/rx51/rx51.c
+++ b/board/nokia/rx51/rx51.c
@@ -668,6 +668,9 @@ static int rx51_kp_tstc(struct udevice *dev)
u8 intr;
u8 mods;
 
+   if ((KEYBUF_SIZE + keybuf_tail - keybuf_head) % KEYBUF_SIZE)
+   return 1;
+
/* localy lock twl4030 i2c bus */
if (test_and_set_bit(0, _i2c_lock))
return 0;
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 1/6] menu: Re-enable the ANSI codes

2023-06-17 Thread Simon Glass
The intent here was to allow ANSI codes to be disabled, since it was
proving impoosible to test operation of the menu code when it kept moving
the cursor. Unfortunately this ended up in the patch.

Correct this by enabling ANSI again.

Signed-off-by: Simon Glass 
Reported-by: Pali Rohár 
Reported-by: Mark Kettenis 
Reported-by: Frank Wunderlich 
Fixes: 32bab0eae51b ("menu: Make use of CLI character processing")
Tested-by: Mark Kettenis 
Reviewed-by: Mark Kettenis 
---

 common/menu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/menu.c b/common/menu.c
index 94514177e4e9..b55cf7b99967 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -15,7 +15,7 @@
 
 #include "menu.h"
 
-#define ansi 0
+#define ansi 1
 
 /*
  * Internally, each item in a menu is represented by a struct menu_item.
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 0/6] nokia_nx51: Attempts to debug keyboard

2023-06-17 Thread Simon Glass
This series is an attempt to get the keyboard working properly with
bootmenu.

It fixes the board's tstc() function, which should be in drivers/input

It also adjusts stopping of the menu autodelay - it should only stop when
a whole escape sequence is received, not when the first escape character
is received. That is pretty minor, so we could drop that patch.

This series also adds some debugging output. This seems to make things
work correctly, suggesting that there is some other problem.

I also see this message fairly often:

cyclic function rx51_watchdog took too long: 1us vs 1000us max


Simon Glass (6):
  menu: Re-enable the ANSI codes
  nokia_rx51: Correct tstc() implementation
  bootmenu: Cancel delay only when a real key is pressed
  boobtmenu: Add debugging
  getch: debugging
  menu: Add debugging

 board/nokia/rx51/rx51.c |  3 +++
 cmd/bootmenu.c  | 19 ---
 common/cli_getch.c  | 23 ---
 common/menu.c   |  7 ++-
 4 files changed, 41 insertions(+), 11 deletions(-)

-- 
2.41.0.162.gfafddb0af9-goog



Re: [PATCH] menu: Re-enable the ANSI codes

2023-06-17 Thread Simon Glass
Hi Pali,

On Thu, 15 Jun 2023 at 17:56, Pali Rohár  wrote:
>
> On Thursday 15 June 2023 13:34:09 Simon Glass wrote:
> > Hi Pali,
> >
> > On Mon, 12 Jun 2023 at 22:33, Pali Rohár  wrote:
> > >
> > > On Monday 12 June 2023 22:17:48 Simon Glass wrote:
> > > > Hi Pali,
> > > >
> > > > On Mon, 12 Jun 2023 at 21:22, Pali Rohár  wrote:
> > > > >
> > > > > On Monday 12 June 2023 21:14:32 Simon Glass wrote:
> > > > > > The intent here was to allow ANSI codes to be disabled, since it was
> > > > > > proving impoosible to test operation of the menu code when it kept 
> > > > > > moving
> > > > > > the cursor. Unfortunately this ended up in the patch.
> > > > > >
> > > > > > Correct this by enabling ANSI again.
> > > > > >
> > > > > > Signed-off-by: Simon Glass 
> > > > > > Reported-by: Pali Rohár 
> > > > > > Reported-by: Mark Kettenis 
> > > > > > Reported-by: Frank Wunderlich 
> > > > > > Fixes: 32bab0eae51b ("menu: Make use of CLI character processing")
> > > > > > ---
> > > > > >
> > > > > >  common/menu.c | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/common/menu.c b/common/menu.c
> > > > > > index 94514177e4e9..b55cf7b99967 100644
> > > > > > --- a/common/menu.c
> > > > > > +++ b/common/menu.c
> > > > > > @@ -15,7 +15,7 @@
> > > > > >
> > > > > >  #include "menu.h"
> > > > > >
> > > > > > -#define ansi 0
> > > > > > +#define ansi 1
> > > > > >
> > > > > >  /*
> > > > > >   * Internally, each item in a menu is represented by a struct 
> > > > > > menu_item.
> > > > > > --
> > > > > > 2.41.0.162.gfafddb0af9-goog
> > > > > >
> > > > >
> > > > > Hello, I have tested this change but bootmenu still does not work. 
> > > > > There
> > > > > is still same issue which I reported month ago. When I press DOWN key
> > > > > then bootmenu immediately quit instead of moving into the next entry.
> > > >
> > > > Thanks for testing this.
> > > >
> > > > Is there a way for me to test this with sandbox? Or does your Nokia
> > > > test check it?
> > >
> > > I guess that bootmenu command could work in sandbox. But I have not
> > > tried it.
> > >
> > > Nokia CI test does not try any terminal, keyboard or VGA interaction, so
> > > broken rendering or broken keyboard input is not caught by CI.
> > >
> > > But it is possible to test it manually. See U-Boot documentation how to
> > > run Nokia u-boot image in qemu. Bootmenu is automatically started.
> > > https://u-boot.readthedocs.io/en/latest/board/nokia/rx51.html#run-in-qemu
> >
> > I tried to follow this but got stuck here:
> >
> > ./configure --enable-system --target-list=arm-softmmu --disable-werror
> >
> > ERROR: Cannot use 'python', Python 2.4 or later is required.
> >Note that Python 3 or later is not yet supported.
> >Use --python=/path/to/python to specify a supported Python.
> >
> > Python 2 has been deprecated for years and I think it was removed recently.
>
> Ach :-( Is not there some configure option to disable python?

I don't know, but the qemu you are using seems to be 10 years old?

>
> In U-Boot CI is qemu compiling without issues (but variant without GUI).

You mean that it still uses python 2? But for how much longer?

I don't think it is reasonable to base a test on an old version of qemu.

>
> > >
> > > Bootmenu is available on both VGA display and terminal output. Problem
> > > with DOWN key is on the terminal.
> > >
> > > >
> > > > I'm sure you have this commit:?
> > > >
> > > > 17b45e684af9 ("cli: Correct several bugs in cli_getch()")
> > >
> > > Yes, as it is in the master branch already.
> >
> > I tried the following with sandbox:
> >
> > setenv bootmenu_0 "my item=echo item 0"
> > setenv bootmenu_1 "second item=echo item 1"
> > setenv bootmenu_delay 10
> > bootmenu
> >
> > I see that with the ANSI patch the 'press key' prompt now appears in
> > the right place.
> >
> > When I press down key repeatedly, it goes down until the last item and
> > then refuses to go any further.
> >
> > So I can't repeat this problem.
>
> Hm... so it looks like some ARM or UART specific thing... or something
> which is not triggered by sandbox.
>
> Just some suggestions, could you try to add some more entries? Or trying
> to wait e.g. 30s before pressing another key (qemu is slow so there can
> be some delay issue?)? Or could you try to run bootmenu on some real
> hardware via UART?

I did get it going by hacking my machine.

I'll send a series with some debugging, etc. I am not quite sure what
the problem is, but it seems to be timing-related, or perhaps a bug in
the keyboard code (I found one bug already).

>
> > I tried looking at the Nokia code. The keyboard driver should be in
> > drivers/input, but I eventually found it in the board directory.
> >
> > Are you sure that rx51_kp_tstc() behaves correctly? Can you add
> > debugging to see what codes it is emitting?
> >
> > Also you don't need to generate escape sequences in rx51_kp_fill().
> > You can just use Ctrl-N, Ctrl-P, Ctrl-B and Ctrl-F (see 

Re: [RFC PATCH 00/17] sunxi: rework pinctrl and add T113s support

2023-06-17 Thread Maxim Kiselev
Hi Andre,

пт, 16 июн. 2023 г. в 19:36, Andre Przywara :
[..]
>
> thanks for the reply! If you have anything that is missing or broken in
> the new version of the patchset I put on github, please let me know.

I tried the new version and everything looks pretty good for me. Great job!
Just one note. Could you please add SUNXI_R_CPUCFG_BASE 0x07000400
to cpu_sunxi_ncat2.h file. This register is required for Sam's PCSI patches.
Otherwise it leads to undeclared error:
  In file included from ./arch/arm/include/asm/armv7.h:60,
  from arch/arm/cpu/armv7/sunxi/psci.c:16:
  arch/arm/cpu/armv7/sunxi/psci.c: In function ‘sunxi_cpu_set_entry’:
  arch/arm/cpu/armv7/sunxi/psci.c:138:24: error:
‘SUNXI_R_CPUCFG_BASE’ undeclared (first use in this function); did you
mean ‘SUNXI_CPUCFG_BASE’?
  138 |SUNXI_R_CPUCFG_BASE + SUN8I_R528_SOFT_ENTRY);
|^~~


Re: [PATCH] rockchip: allow env defines for SPL build

2023-06-17 Thread Kever Yang

Hi Eugen,

On 2023/6/16 16:55, Eugen Hristev wrote:

For environment in SPL, all these defines are required, otherwise
build fails:

[...]
include/env_default.h:120:9: note: in expansion of macro 
‘CFG_EXTRA_ENV_SETTINGS’
   120 | CFG_EXTRA_ENV_SETTINGS
   | ^~
In file included from env/common.c:32:
[...]

Environment in SPL is needed e.g. for DFU, as dfu_alt is kept as
env variable.

Signed-off-by: Eugen Hristev 
---
  include/configs/rockchip-common.h | 4 
  1 file changed, 4 deletions(-)

diff --git a/include/configs/rockchip-common.h 
b/include/configs/rockchip-common.h
index 9121bba37384..be20b135066e 100644
--- a/include/configs/rockchip-common.h
+++ b/include/configs/rockchip-common.h
@@ -11,8 +11,6 @@
  #define CFG_CPUID_OFFSET  0x7
  #endif
  
-#ifndef CONFIG_SPL_BUILD

-
  #define BOOT_TARGETS  "mmc1 mmc0 nvme scsi usb pxe dhcp spi"


These boot targets are for U-Boot proper only, not available for SPL, so 
I don't think remove


the SPL_BUILD limit is correct.

The DFU you mentioned is also used in U-Boot instead of SPL for most of 
the project.



Thanks,
- Kever
  
  #ifdef CONFIG_ARM64

@@ -28,6 +26,4 @@
"name=boot,size=112M,bootable,uuid=${uuid_gpt_boot};" \
"name=rootfs,size=-,uuid="ROOT_UUID
  
-#endif

-
  #endif /* _ROCKCHIP_COMMON_H_ */


Re: [PATCH 2/5] mkeficapsule: add support for multiple payloads inside capsule

2023-06-17 Thread Sughosh Ganu
On Sat, 17 Jun 2023 at 06:26, AKASHI Takahiro
 wrote:
>
> On Fri, Jun 16, 2023 at 06:02:52PM +0530, Sughosh Ganu wrote:
> > On Fri, 16 Jun 2023 at 17:56, Sughosh Ganu  wrote:
> > >
> > > hi Stefan,
> > >
> > > On Fri, 16 Jun 2023 at 17:04, Stefan Herbrechtsmeier
> > >  wrote:
> > > >
> > > > From: Malte Schmidt 
> > > >
> > > > The UEFI [1] specification allows multiple payloads inside the capsule
> > > > body. Add support for this. The command line arguments are kept
> > > > backwards-compatible.
> > > >
> > > > [1] https://uefi.org/specs/UEFI/2.10/index.html
> > >
> > > I am trying to upstream support for specifying the capsule parameters
> > > for multiple payloads through a config file [1]. This is on similar
> > > lines to the support in the Edk2 GenerateCapule tool where multiple
> > > payloads can be specified through a json file. I think you can base
> > > your changes on my series.
> >
> > Btw, with the support being added for getting the capsule parameters
> > through a config file, I believe your changes would be pretty much
> > simplified. Instead of passing all those parameters through the
> > command line, they can instead be read from the config file and used
> > to generate a single capsule file consisting of multiple payloads.
> > That would be a much simpler implementation.
>
> As I said in my reply to the patch[0/5], I don't think we have a strong
> reason to support multiple images because there is already a FIT-based
> capsule support.
> That said, if there is a good reason to do so, Sughosh's suggestion
> makes much sense to me.
>
> BTW, sughosh's patch implements yet another key:value format for
> config files. I wondered if we could use a generic (standardized) format,
> like a device tree or yaml, or others.

I chose the key:value pairs primarily because I wanted to keep the
syntax of the config file as similar to the one in EDK2 as possible. I
believe keeping the format simple is better especially when we are not
dealing with multiple values, or an array of u32 cells like in device
tree.

-sughosh

>
> -Takahiro Akashi
>
>
>
> > -sughosh
> >
> > >
> > > -sughosh
> > >
> > > [1] - 
> > > https://lore.kernel.org/u-boot/20230613103806.812065-1-sughosh.g...@linaro.org/T/#mc8c0500863bd3a1580c572679370a565f8d7f2c8
> > >
> > > >
> > > > Signed-off-by: Malte Schmidt 
> > > > Signed-off-by: Stefan Herbrechtsmeier 
> > > > 
> > > > ---
> > > >
> > > >  tools/eficapsule.h   |   5 -
> > > >  tools/mkeficapsule.c | 636 ---
> > > >  2 files changed, 475 insertions(+), 166 deletions(-)
> > > >
> > > > diff --git a/tools/eficapsule.h b/tools/eficapsule.h
> > > > index 753fb73313..001af3217c 100644
> > > > --- a/tools/eficapsule.h
> > > > +++ b/tools/eficapsule.h
> > > > @@ -138,9 +138,4 @@ struct fmp_payload_header {
> > > > uint32_t lowest_supported_version;
> > > >  };
> > > >
> > > > -struct fmp_payload_header_params {
> > > > -   bool have_header;
> > > > -   uint32_t fw_version;
> > > > -};
> > > > -
> > > >  #endif /* _EFI_CAPSULE_H */
> > > > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> > > > index b8db00b16b..1a4de0f092 100644
> > > > --- a/tools/mkeficapsule.c
> > > > +++ b/tools/mkeficapsule.c
> > > > @@ -29,7 +29,7 @@ static const char *tool_name = "mkeficapsule";
> > > >  efi_guid_t efi_guid_fm_capsule = 
> > > > EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> > > >  efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
> > > >
> > > > -static const char *opts_short = "g:i:I:v:p:c:m:o:dhAR";
> > > > +static const char *opts_short = "g:i:b:I:v:p:c:m:o:dhAR";
> > > >
> > > >  enum {
> > > > CAPSULE_NORMAL_BLOB = 0,
> > > > @@ -40,6 +40,7 @@ enum {
> > > >  static struct option options[] = {
> > > > {"guid", required_argument, NULL, 'g'},
> > > > {"index", required_argument, NULL, 'i'},
> > > > +   {"image_blob", required_argument, NULL, 'b'},
> > > > {"instance", required_argument, NULL, 'I'},
> > > > {"fw-version", required_argument, NULL, 'v'},
> > > > {"private-key", required_argument, NULL, 'p'},
> > > > @@ -55,21 +56,22 @@ static struct option options[] = {
> > > >
> > > >  static void print_usage(void)
> > > >  {
> > > > -   fprintf(stderr, "Usage: %s [options]   > > > file>\n"
> > > > +   fprintf(stderr, "Usage: %s [options] []  > > > file>\n"
> > > > "Options:\n"
> > > >
> > > > -   "\t-g, --guid guid for image blob 
> > > > type\n"
> > > > -   "\t-i, --index  update image index\n"
> > > > -   "\t-I, --instanceupdate hardware 
> > > > instance\n"
> > > > -   "\t-v, --fw-version   firmware version\n"
> > > > -   "\t-p, --private-key   private key file\n"
> > > > -   "\t-c, --certificate  signer's 
> > > > certificate file\n"
> > > > -   "\t-m, --monotonic-count  monotonic count\n"
> > > > -   "\t-d, --dump_sig

Re: [PATCH] usb: musb-new: sunxi: read SRAM controller address from DT

2023-06-17 Thread Jernej Škrabec
Dne torek, 13. junij 2023 ob 13:21:58 CEST je Andre Przywara napisal(a):
> Some older SoCs (<=A20, F1C100s) do not have a dedicated SRAM buffer for
> the USB-OTG controller, but require the CPU to relinquish one of its SRAM
> blocks to the USB controller. This is done by flipping a bit in the SRAM
> controller (aka "syscon").
> So far we were doing this using the hardcoded SRAM controller address,
> but that breaks the abstraction, and we can find that address in the DT.
> 
> Follow the "allwinner,sram" phandle property in the devicetree to get to
> the SRAM node. The reg property in there gives us the SRAM MMIO address,
> as seen by the CPU, but we need the SRAM *controller* address, which is
> two levels up from that node.
> 
> Utilise U-Boot's DT code to do the traversal and DT address translation
> for us, then store the controller address in the glue struct, to later
> hand it to the routine that programs the SRAM controller.
> This allows us to drop the usage of the hardcoded SUNXI_SRAMC_BASE from
> the MUSB code.
> 
> Signed-off-by: Andre Przywara 
> ---
> Hi,
> 
> this patch goes on top of Sam's musb sunxi fixes. Can someone look at
> the DT calls? There are quite some interfaces to U-Boot's DT framework,
> so I want to be sure to got the right one.
> If that patch looks fine, I would send a new series including Sam's two
> patches, this patch here, and a (simple) patch adding F1C100s support.
> 
> Cheers,
> Andre
> 
>  drivers/usb/musb-new/sunxi.c | 26 +++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
> index 4bf0ee346a4..1da05e29a56 100644
> --- a/drivers/usb/musb-new/sunxi.c
> +++ b/drivers/usb/musb-new/sunxi.c
> @@ -23,13 +23,12 @@
>  #include 
>  #include 
>  #include 
> -#include 
> -#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include "linux-compat.h"
>  #include "musb_core.h"
> @@ -93,6 +92,7 @@ struct sunxi_glue {
>   struct sunxi_musb_config *cfg;
>   struct device dev;
>   struct phy phy;
> + uintptr_t syscon_base;
>  };
>  #define to_sunxi_glue(d) container_of(d, struct sunxi_glue, dev)
>  
> @@ -181,6 +181,7 @@ static void USBC_ForceVbusValidToHigh(__iomem void *base)
>   */
>  static void sunxi_musb_claim_sram(uintptr_t syscon_base)
>  {
> + debug("%s(0x%lx);\n", __func__, (unsigned long)syscon_base);
>   /*
>* BIT(0) of SRAM_CTRL_REG1 (syscon+0x04) controls SRAM-D ownership:
>* '0' -> exclusive access by CPU
> @@ -326,7 +327,7 @@ static int sunxi_musb_init(struct musb *musb)
>* block 'D', ownership of which needs to be handed over by
>* the CPU
>*/
> - sunxi_musb_claim_sram(SUNXI_SRAMC_BASE);
> + sunxi_musb_claim_sram(glue->syscon_base);
>   }
>  
>   USBC_EnableDpDmPullUp(musb->mregs);
> @@ -450,6 +451,7 @@ static int musb_usb_probe(struct udevice *dev)
>   struct sunxi_glue *glue = dev_get_priv(dev);
>   struct musb_host_data *host = >mdata;
>   struct musb_hdrc_platform_data pdata;
> + struct ofnode_phandle_args args;
>   void *base = dev_read_addr_ptr(dev);
>   int ret;
>  
> @@ -476,6 +478,24 @@ static int musb_usb_probe(struct udevice *dev)
>   return ret;
>   }
>  
> + ret = dev_read_phandle_with_args(dev, "allwinner,sram", NULL, 1, 0,
> +  );
> + if (ret && ret != -ENOENT) {
> + dev_err(dev, "failed to get SRAM node\n");
> + return ret;
> + } else if (!ret) {
> + /* The SRAM *controller* address is in the grandparent node. */
> + ofnode node = ofnode_get_parent(ofnode_get_parent(args.node));

Above is not the safest thing to do in case of corrupted DT. It will trigger 
assert.

> + struct resource res;
> +
> + ret = ofnode_read_resource(node, 0, );

ofnode_get_addr() seems better choice here. What do you think?

Best regards,
Jernej

> + if (ret) {
> + dev_err(dev, "failed to read SRAM controller base\n");
> + return ret;
> + }
> + glue->syscon_base = res.start;
> + }
> +
>   ret = generic_phy_get_by_name(dev, "usb", >phy);
>   if (ret) {
>   pr_err("failed to get usb PHY\n");
>