RE: [PATCH v7 04/22] lib: Makefile: build crc7.c when CONFIG_MMC_SPI

[Pragnesh Patel]

>-Original Message-
>From: U-Boot  On Behalf Of Pragnesh Patel
>Sent: 05 May 2020 21:25
>To: Jagan Teki ; Heinrich Schuchardt
>; Bin Meng 
>Cc: U-Boot Mailing List ; Atish Patra
>; Palmer Dabbelt ; Paul
>Walmsley ; Troy Benjegerdes
>; Anup Patel ; Sagar
>Kadam ; Rick Chen ; Peng
>Fan ; Lukasz Majewski ; Simon
>Goldschmidt ; Simon Glass
>; Markus Klotzbuecher
>; Baruch Siach ; Joel
>Johnson ; Anatolij Gustschin ; AKASHI
>Takahiro ; Marek Behún 
>Subject: RE: [PATCH v7 04/22] lib: Makefile: build crc7.c when
>CONFIG_MMC_SPI
>
>
>
>>-Original Message-
>>From: U-Boot  On Behalf Of Pragnesh Patel
>>Sent: 04 May 2020 11:15
>>To: Jagan Teki ; Heinrich Schuchardt
>>; Bin Meng 
>>Cc: U-Boot Mailing List ; Atish Patra
>>; Palmer Dabbelt ;
>Paul
>>Walmsley ; Troy Benjegerdes
>>; Anup Patel ; Sagar
>>Kadam ; Rick Chen ; Peng
>>Fan ; Lukasz Majewski ; Simon
>>Goldschmidt ; Simon Glass
>>; Markus Klotzbuecher
>>; Baruch Siach ;
>>Joel Johnson ; Anatolij Gustschin ;
>>AKASHI Takahiro ; Marek Behún
>>
>>Subject: RE: [PATCH v7 04/22] lib: Makefile: build crc7.c when
>>CONFIG_MMC_SPI
>>
>>>-Original Message-
>>>From: Jagan Teki 
>>>Sent: 02 May 2020 21:04
>>>To: Heinrich Schuchardt ; Pragnesh Patel
>>>; Bin Meng 
>>>Cc: U-Boot Mailing List ; Atish Patra
>>>; Palmer Dabbelt ;
>>Paul
>>>Walmsley ; Troy Benjegerdes
>>>; Anup Patel ; Sagar
>>>Kadam ; Rick Chen ; Peng
>>>Fan ; Lukasz Majewski ; Simon
>>>Goldschmidt ; Simon Glass
>>>; Markus Klotzbuecher
>>>; Baruch Siach ;
>>>Joel Johnson ; Anatolij Gustschin ;
>>>AKASHI Takahiro ; Marek Behún
>>>
>>>Subject: Re: [PATCH v7 04/22] lib: Makefile: build crc7.c when
>>>CONFIG_MMC_SPI
>>>
>>>[External Email] Do not click links or attachments unless you
>>>recognize the sender and know the content is safe
>>>
>>>On Sat, May 2, 2020 at 6:45 PM Heinrich Schuchardt
>>>
>>>wrote:

 Am May 2, 2020 11:47:10 AM UTC schrieb Bin Meng
>>>:
 >Hi Heinrich,
 >
 >On Sat, May 2, 2020 at 6:30 PM Heinrich Schuchardt
 >
 >wrote:
 >>
 >> On 5/2/20 12:06 PM, Pragnesh Patel wrote:
 >> > When build U-Boot SPL, meet an issue of undefined reference to
 >> > 'crc7' for drivers/mmc/mmc_spi.c, so let's compile crc7.c when
 >> > CONFIG_MMC_SPI selected.
 >> >
 >> > Signed-off-by: Pragnesh Patel 
 >> > Reviewed-by: Jagan Teki 
 >> > Reviewed-by: Bin Meng 
 >> > ---
 >> >  common/spl/Kconfig  | 6 ++  drivers/mmc/Kconfig | 1 +
 >> >  lib/Makefile| 1 +
 >> >  3 files changed, 8 insertions(+)
 >> >
 >> > diff --git a/common/spl/Kconfig b/common/spl/Kconfig index
 >> > ef5bf66696..d1f0e6bc4c 100644
 >> > --- a/common/spl/Kconfig
 >> > +++ b/common/spl/Kconfig
 >> > @@ -401,6 +401,12 @@ config SPL_CRC32_SUPPORT
 >> > for detected accidental image corruption. For secure
 >applications you
 >> > should consider SHA1 or SHA256.
 >> >
 >> > +config SPL_CRC7_SUPPORT
 >> > + bool "Support CRC7"
 >> > + help
 >> > +   Enable CRC7 hashing for drivers which are using in SPL.
 >> > +   This is a 32-bit checksum value that can be used to
 >> > +verify
 >images.
 >> > +
 >> >  config SPL_MD5_SUPPORT
 >> >   bool "Support MD5"
 >> >   depends on SPL_FIT
 >> > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index
 >> > 8f0df568b9..139599072a 100644
 >> > --- a/drivers/mmc/Kconfig
 >> > +++ b/drivers/mmc/Kconfig
 >> > @@ -49,6 +49,7 @@ if MMC
 >> >  config MMC_SPI
 >> >   bool "Support for SPI-based MMC controller"
 >> >   depends on DM_MMC && DM_SPI
 >> > + select SPL_CRC7_SUPPORT if SPL
 >> >   help
 >> > This selects SPI-based MMC controllers.
 >> > If you have an MMC controller on a SPI bus, say Y here.
 >> > diff --git a/lib/Makefile b/lib/Makefile index
 >> > c6f862b0c2..fcd934857f 100644
 >> > --- a/lib/Makefile
 >> > +++ b/lib/Makefile
 >> > @@ -80,6 +80,7 @@ endif
 >> >
 >> >  ifdef CONFIG_SPL_BUILD
 >> >  obj-$(CONFIG_SPL_YMODEM_SUPPORT) += crc16.o
 >> > +obj-$(CONFIG_SPL_CRC7_SUPPORT) += crc7.o
 >>
 >> crc7.o is not needed in main U-Boot if MMC_SPI is not selected.
 >>
 >> So instead of adding a new configuration variable simply correct
 >> the existing line in lib/Makefile
 >>
 >> -obj-y += crc7.o
 >> +obj-$(CONFIG_MMC_SPI) += crc7.o
 >
 >This looks incorrect to me. CRC7 can be useful for other drivers too.
 >It should not depend on CONFIG_MMC_SPI.

 Using this argument you would always compile everything. Compiling
 files
>>>that are not used is simply a waste of CPU time.

 Following your argument. you could also always compile crc7.c for
 SPL and
>>>hope the compiler will eliminate it.

 Either way we do not need a new config symbol.
>>>
>>>This is one of the reasons I just suggested 

[PATCH] efi_loader: variable: check a return value of uuid__str_to_bin()

[AKASHI Takahiro]

The only error case is that a given UUID is in wrong format.
So just return EFI_INVALID_PARAMETER here.

Signed-off-by: AKASHI Takahiro 
Reported-by: Coverity (CID 300333)
---
 lib/efi_loader/efi_variable.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index f40b194b13bf..38d21b9314f2 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -866,7 +866,10 @@ static efi_status_t parse_uboot_variable(char *variable,
/* guid */
c = *(name - 1);
*(name - 1) = '\0'; /* guid need be null-terminated here */
-   uuid_str_to_bin(guid, (unsigned char *)vendor, UUID_STR_FORMAT_GUID);
+   if (uuid_str_to_bin(guid, (unsigned char *)vendor,
+   UUID_STR_FORMAT_GUID))
+   /* The only error would be EINVAL. */
+   return EFI_INVALID_PARAMETER;
*(name - 1) = c;
 
/* attributes */
-- 
2.25.2

[PATCH] efi_loader: image_loader: fix a Coverity check against array access

[AKASHI Takahiro]

Coverity detected:
  Using ">CheckSum" as an array.  This might corrupt or misinterpret
  adjacent memory locations.

The code should work as far as a structure, IMAGE_OPTIONAL_HEADER(64) is
packed, but modify it in more logical form. Subsystem is a member next to
CheckSum.

Signed-off-by: AKASHI Takahiro 
Reported-by: Coverity (CID 300339)
---
 lib/efi_loader/efi_image_loader.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c 
b/lib/efi_loader/efi_image_loader.c
index 2f270e5497aa..894103c6e4dd 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -293,12 +293,12 @@ bool efi_image_parse(void *efi, size_t len, struct 
efi_image_regions **regp,
efi_image_region_add(regs, efi, >CheckSum, 0);
if (nt64->OptionalHeader.NumberOfRvaAndSizes <= ctidx) {
efi_image_region_add(regs,
->CheckSum + 1,
+>Subsystem,
 efi + opt->SizeOfHeaders, 0);
} else {
/* Skip Certificates Table */
efi_image_region_add(regs,
->CheckSum + 1,
+>Subsystem,
 >DataDirectory[ctidx], 0);
efi_image_region_add(regs,
 >DataDirectory[ctidx] + 1,
@@ -313,7 +313,7 @@ bool efi_image_parse(void *efi, size_t len, struct 
efi_image_regions **regp,
IMAGE_OPTIONAL_HEADER32 *opt = >OptionalHeader;
 
efi_image_region_add(regs, efi, >CheckSum, 0);
-   efi_image_region_add(regs, >CheckSum + 1,
+   efi_image_region_add(regs, >Subsystem,
 >DataDirectory[ctidx], 0);
efi_image_region_add(regs, >DataDirectory[ctidx] + 1,
 efi + opt->SizeOfHeaders, 0);
-- 
2.25.2

[PATCH] cmd: env: fix unreachable statements

[AKASHI Takahiro]

C's switch statement takes an integer value for switching.
As efi_status_t is defined as unsigned long and each error code has
the top bit set, all "cases" cannot be reachable.

Signed-off-by: AKASHI Takahiro 
Reported-by: Coverity (CID 300335)
---
 cmd/nvedit_efi.c | 20 ++--
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/cmd/nvedit_efi.c b/cmd/nvedit_efi.c
index 837e39e02179..84cba0c7324b 100644
--- a/cmd/nvedit_efi.c
+++ b/cmd/nvedit_efi.c
@@ -597,26 +597,18 @@ int do_env_set_efi(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[])
} else {
const char *msg;
 
-   switch (ret) {
-   case EFI_NOT_FOUND:
+   if (ret == EFI_NOT_FOUND)
msg = " (not found)";
-   break;
-   case EFI_WRITE_PROTECTED:
+   else if (ret == EFI_WRITE_PROTECTED)
msg = " (read only)";
-   break;
-   case EFI_INVALID_PARAMETER:
+   else if (ret == EFI_INVALID_PARAMETER)
msg = " (invalid parameter)";
-   break;
-   case EFI_SECURITY_VIOLATION:
+   else if (ret == EFI_SECURITY_VIOLATION)
msg = " (validation failed)";
-   break;
-   case EFI_OUT_OF_RESOURCES:
+   else if (ret == EFI_OUT_OF_RESOURCES)
msg = " (out of memory)";
-   break;
-   default:
+   else
msg = "";
-   break;
-   }
printf("## Failed to set EFI variable%s\n", msg);
ret = CMD_RET_FAILURE;
}
-- 
2.25.2

[PATCH] efi_loader: variable: fix an reachable statement

[AKASHI Takahiro]

The code should jump into error recovery instead of just returning
an error.

Signed-off-by: AKASHI Takahiro 
Reported-by: Coverity (CID 300332)
---
 lib/efi_loader/efi_variable.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 5b86b77c6660..f40b194b13bf 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -1094,7 +1094,7 @@ efi_status_t EFIAPI efi_set_variable_common(u16 
*variable_name,
if (append) {
old_data = malloc(old_size);
if (!old_data) {
-   return EFI_OUT_OF_RESOURCES;
+   ret = EFI_OUT_OF_RESOURCES;
goto err;
}
ret = EFI_CALL(efi_get_variable(variable_name, vendor,
-- 
2.25.2

[PATCH] cmd: efidebug: add a comment against Coverity check (300329)

[AKASHI Takahiro]

The check here, "Null pointer dereferences," is a false positive.
So leave a comment.

Signed-off-by: AKASHI Takahiro 
Reported-by: Coverity (CID 300329)
---
 cmd/efidebug.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 9769aeeac58b..70aba446a937 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -481,6 +481,11 @@ static int do_efi_show_memmap(cmd_tbl_t *cmdtp, int flag,
   EFI_PHYS_ADDR_WIDTH - 5, spc, EFI_PHYS_ADDR_WIDTH - 3, spc);
printf(" %.*s %.*s ==\n",
   EFI_PHYS_ADDR_WIDTH, sep, EFI_PHYS_ADDR_WIDTH, sep);
+   /*
+* Coverity check: dereferencing null pointer "map."
+* This is a false positive as memmap will always be
+* populated by allocate_pool() above.
+*/
for (i = 0, map = memmap; i < map_size / sizeof(*map); map++, i++) {
if (map->type < EFI_MAX_MEMORY_TYPE)
type = efi_mem_type_string[map->type];
-- 
2.25.2

[PATCH] cmd: efidebug: fix a wrong handling of arguments

[AKASHI Takahiro]

Coverity detected a dead code, but actually there is a bug in a check
against a number of arguments. So simply fix it.

Signed-off-by: AKASHI Takahiro 
Reported-by: Coverity (CID 300330)
---
 cmd/efidebug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 70aba446a937..56bf9af77248 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -606,7 +606,7 @@ static int do_efi_boot_add(cmd_tbl_t *cmdtp, int flag,
+ sizeof(struct efi_device_path); /* for END */
 
/* optional data */
-   if (argc < 6)
+   if (argc == 6)
lo.optional_data = NULL;
else
lo.optional_data = (const u8 *)argv[6];
-- 
2.25.2

Pull request for UEFI sub-system for efi-2020-07-rc2 (3)

[Heinrich Schuchardt]

The following changes since commit 69bf66ad8c0d53cc5e64d0f4f2e3bc9ad18e61aa:

  Merge branch '2020-05-06-master-imports' (2020-05-07 09:02:28 -0400)

are available in the Git repository at:

  https://gitlab.denx.de/u-boot/custodians/u-boot-efi.git
tags/efi-2020-07-rc2-3

for you to fetch changes up to d7ca3ce3d3b990503cb6e0bafad91aa2a7c96b9d:

  efi_loader: crypto/pkcs7_parser.h is not a local include (2020-05-07
18:23:18 +0200)


Pull request for UEFI sub-system for efi-2020-07-rc2-3

This series contains bug fixes and code simplifications.

Following clarification in the discussion of the EBBR specification
device trees will be passed as EfiACPIReclaimMemory to UEFI applications.

No problems where reported by Gitlab and Travis:

https://gitlab.denx.de/u-boot/custodians/u-boot-efi/pipelines/3131
https://travis-ci.org/github/xypron2/u-boot/builds/684346617


Heinrich Schuchardt (6):
  efi_loader: remove redundant assignment in dp_fill()
  efi_loader: error handling in efi_set_variable_common().
  efi_loader: do not unnecessarily use EFI_CALL()
  efi_loader: use logical and in do_env_print_efi()
  efi_loader: put device tree into EfiACPIReclaimMemory
  efi_loader: crypto/pkcs7_parser.h is not a local include

Jan Kiszka (1):
  kbuild: efi: Avoid rebuilding efi targets

Patrick Wildt (2):
  efi_loader: efi_variable_parse_signature() returns NULL on error
  efi_loader: pkcs7_parse_message() returns error pointer

 cmd/bootefi.c  |  4 ++--
 cmd/nvedit_efi.c   |  2 +-
 lib/efi_loader/Makefile|  1 +
 lib/efi_loader/efi_device_path.c   |  2 +-
 lib/efi_loader/efi_image_loader.c  |  6 --
 lib/efi_loader/efi_variable.c  | 39
++
 lib/efi_selftest/efi_selftest_memory.c |  4 ++--
 scripts/Makefile.lib   |  2 ++
 8 files changed, 34 insertions(+), 26 deletions(-)

Re: [PATCH] imx8mp_evk: Make SPL binary size smaller

[Fabio Estevam]

Hi Marek,

On Thu, May 7, 2020 at 11:56 PM Fabio Estevam  wrote:

> I will activate them on SPL tomorrow.

After activating the SPL clocks I see:

 clk  69  [   ]   imx_clk_gate2 |   |
`-- dram1_root_clk

The dram1_root_clk is not getting turned on.

This clock is marked as CLK_IS_CRITICAL, but it seems that U-Boot core
clock driver does not handle it yet.

I have also tried to enable it manually by doing:

clk_get_by_id(IMX8MP_CLK_DRAM_CORE, );
clk_enable(clk);
clk_get_by_id(IMX8MP_CLK_DRAM1_ROOT, );
clk_enable(clk);

but it did not work.

The complete clock dump is here:
https://pastebin.com/raw/40U6hQny

Thanks

Re: wandboard does not boot with current mainline

[Heiko Schocher]

Hello Fabio,

Am 07.05.2020 um 23:16 schrieb Fabio Estevam:

Hi Heiko,

On Thu, May 7, 2020 at 9:12 AM Heiko Schocher  wrote:


Hello Fabio,

I have my wandboard DL in my automated daily build setup example:

http://xeidos.ddns.net/ubtestresults/

and wondered, why my last result is from may 4th ...

(Ok, there is a bug, that I do not see not booting boards in this page,
   but this is just a proof of concept page ...)


So, what I see is:

│   │   ├─UBOOT (wandboard-uboot)
│   │   │<> Connecting to
/dev/serial/by-id/usb-Prolific_Technology_Inc._USB-Serial_Controller_D-if00-port0,
 speed 115200
│   │   │<>  Escape character: Ctrl-\ (ASCII 28, FS): enabled
│   │   │<> Type the escape character followed by C to get back,
│   │   │<> or followed by ? to see other options.
│   │   │<> 
│   │   │<>
│   │   │<> U-Boot SPL 2020.07-rc1-tbot-00298-g425fefa9a3 (May 05 2020 - 
04:18:47 +0200)
│   │   │<> Trying to boot from MMC1


and no more output ...

Do you have any idea, what is wrong with current mainline?


Thanks for the report.

This boot failure is caused by:

commit 20a154f95bfe0a3b5bfba90bea7f001c58217536
Author: Marek Vasut 
Date:   Fri May 1 17:40:25 2020 +0200

 mkimage: fit: Do not tail-pad fitImage with external data

 There is no reason to tail-pad fitImage with external data to 4-bytes,
 while fitImage without external data does not have any such padding and
 is often unaligned. DT spec also does not mandate any such padding.

 Moreover, the tail-pad fills the last few bytes with uninitialized data,
 which could lead to a potential information leak.

 $ echo -n xy > /tmp/data ; \
 ./tools/mkimage -E -f auto -d /tmp/data /tmp/fitImage ; \
 hexdump -vC /tmp/fitImage | tail -n 3

 before:
 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69
|a-offset.data-si|
 0270  7a 65 00 00 78 79 64 64   |ze..xydd|
^^   ^^ ^^
 after:
 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69
|a-offset.data-si|
 0270  7a 65 00 78 79|ze.xy|

 Signed-off-by: Marek Vasut 
 Reviewed-by: Simon Glass 
 Cc: Heinrich Schuchardt 
 Cc: Tom Rini 

which has been reverted by Tom in commit:

commit 7946a814a31989998120b4b4aa417222ba21b2fa
Author: Tom Rini 
Date:   Wed May 6 11:05:17 2020 -0400

 Revert "mkimage: fit: Do not tail-pad fitImage with external data"

 This has been reported to break booting of U-Boot from SPL on a number
 of platforms due to a lack of alignment of the external data.  The
 issues this commit is addressing will need to be resolved another way.

 Re-introduce a data leak in the padding for now.

 This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.

 Reported-by: Alex Kiernan 
 Reported-by: Michael Walle 
 Tested-by: Jan Kiszka 
 Signed-off-by: Tom Rini 



Yep, thanks! Board works again, see:

http://xeidos.ddns.net/ubtestresults/result/60

BTW: since commit c693f212c5b0 size of u-boot.img has increased
from 561004 bytes to 562068 bytes for the wandboard

http://xeidos.ddns.net/ubtestresults/stats/wandboard_defconfig/8

bye,
Heiko
--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: h...@denx.de

[PATCH] ddr: k3-am654: EMIF Tool update to 2.02 for IO optimizations and fixes

[praneeth]

From: Praneeth Bajjuri 

EMIF tool for AM65x [1] is now updated from rev 1.98 to 2.02

This update includes
* Optimizations in IO configuration.
* Fix for byte enablement in GCR registers.
* Fixes for PG2.0 including ZQ control.

[1]: http://www.ti.com/lit/zip/sprcah7

Acked-by: James Doublesin 
Signed-off-by: Praneeth Bajjuri 
---
 .../dts/k3-am654-base-board-ddr4-1600MTs.dtsi | 28 +--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm/dts/k3-am654-base-board-ddr4-1600MTs.dtsi 
b/arch/arm/dts/k3-am654-base-board-ddr4-1600MTs.dtsi
index d07aaea93f..5638321903 100644
--- a/arch/arm/dts/k3-am654-base-board-ddr4-1600MTs.dtsi
+++ b/arch/arm/dts/k3-am654-base-board-ddr4-1600MTs.dtsi
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
- * This file was generated by AM65x_DRA80xM_EMIF_Tool_1.98.xlsm
+ * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/
+ * This file was generated by AM65x_DRA80xM_EMIF_Tool_2.02.xlsm
  * http://www.ti.com/lit/pdf/spracj0
  * Configuration Parameters
  * Memory Type: DDR4
@@ -24,7 +24,7 @@
 #define DDRCTL_INIT4 0x0020
 #define DDRCTL_INIT5 0x0010
 #define DDRCTL_INIT6 0x0480
-#define DDRCTL_INIT7 0x04E8
+#define DDRCTL_INIT7 0x0497
 #define DDRCTL_DRAMTMG0 0x0C0A1B0D
 #define DDRCTL_DRAMTMG1 0x00030313
 #define DDRCTL_DRAMTMG2 0x0506050A
@@ -33,10 +33,10 @@
 #define DDRCTL_DRAMTMG5 0x04040302
 #define DDRCTL_DRAMTMG6 0x0004
 #define DDRCTL_DRAMTMG7 0x0404
-#define DDRCTL_DRAMTMG8 0x03030A05
+#define DDRCTL_DRAMTMG8 0x03030C05
 #define DDRCTL_DRAMTMG9 0x00020208
 #define DDRCTL_DRAMTMG10 0x001C180A
-#define DDRCTL_DRAMTMG11 0x0E06010E
+#define DDRCTL_DRAMTMG11 0x1106010E
 #define DDRCTL_DRAMTMG12 0x00020008
 #define DDRCTL_DRAMTMG13 0x0B12
 #define DDRCTL_DRAMTMG14 0x
@@ -84,33 +84,33 @@
 #define DDRPHY_DCR 0x040C
 #define DDRPHY_DTPR0 0x041A0B06
 #define DDRPHY_DTPR1 0x2814
-#define DDRPHY_DTPR2 0x0034E255
-#define DDRPHY_DTPR3 0x01D50800
+#define DDRPHY_DTPR2 0x0034E300
+#define DDRPHY_DTPR3 0x02800800
 #define DDRPHY_DTPR4 0x31180805
 #define DDRPHY_DTPR5 0x00250B06
 #define DDRPHY_DTPR6 0x0505
 #define DDRPHY_ZQCR 0x008A2A58
 #define DDRPHY_ZQ0PR0 0x77DD
-#define DDRPHY_ZQ1PR0 0x77DD
+#define DDRPHY_ZQ1PR0 0x7799
 #define DDRPHY_MR0 0x0214
 #define DDRPHY_MR1 0x0501
 #define DDRPHY_MR2 0x
 #define DDRPHY_MR3 0x0020
 #define DDRPHY_MR4 0x
 #define DDRPHY_MR5 0x0480
-#define DDRPHY_MR6 0x04E8
+#define DDRPHY_MR6 0x0497
 #define DDRPHY_MR11 0x
 #define DDRPHY_MR12 0x
 #define DDRPHY_MR13 0x
 #define DDRPHY_MR14 0x
 #define DDRPHY_MR22 0x
-#define DDRPHY_VTCR0 0xF3C32028
+#define DDRPHY_VTCR0 0xF3C32017
 #define DDRPHY_DX8SL0PLLCR0 0x021c4000
 #define DDRPHY_DX8SL1PLLCR0 0x021c4000
 #define DDRPHY_DX8SL2PLLCR0 0x021c4000
 #define DDRPHY_DTCR0 0x8000B1C7
 #define DDRPHY_DTCR1 0x00010236
-#define DDRPHY_ACIOCR0 0x3007
+#define DDRPHY_ACIOCR0 0xF007
 #define DDRPHY_ACIOCR3 0x0001
 #define DDRPHY_ACIOCR5 0x0480
 #define DDRPHY_IOVCR0 0x0F0C0C0C
@@ -157,6 +157,6 @@
 #define DDRPHY_DX8SL0DXCTL2 0x00141830
 #define DDRPHY_DX8SL1DXCTL2 0x00141830
 #define DDRPHY_DX8SL2DXCTL2 0x00141830
-#define DDRPHY_DX8SL0DQSCTL 0x01264000
-#define DDRPHY_DX8SL1DQSCTL 0x01264000
-#define DDRPHY_DX8SL2DQSCTL 0x01264000
+#define DDRPHY_DX8SL0DQSCTL 0x01264300
+#define DDRPHY_DX8SL1DQSCTL 0x01264300
+#define DDRPHY_DX8SL2DQSCTL 0x01264300
-- 
2.17.1

Re: wandboard does not boot with current mainline

[Heiko Schocher]

Hello Tom,

Am 07.05.2020 um 16:38 schrieb Tom Rini:

On Thu, May 07, 2020 at 02:53:49PM +0200, Heiko Schocher wrote:

Hello Fabio,

Am 07.05.2020 um 14:12 schrieb Heiko Schocher:

Hello Fabio,

I have my wandboard DL in my automated daily build setup example:

http://xeidos.ddns.net/ubtestresults/

and wondered, why my last result is from may 4th ...

(Ok, there is a bug, that I do not see not booting boards in this page,
   but this is just a proof of concept page ...)


Ok, found this error, now I have one broken build in above webpage.
Also, the newest commit adds > 300 bytes imge size...

http://xeidos.ddns.net/ubtestresults/stats/wandboard_defconfig/6


That's not what I see:
$ (export SOURCE_DATE_EPOCH=`date +%s`; ./tools/buildman/buildman -o /tmp/wandboard3 
--step 0 -SBC -b 125956..69bf66 -devl wandboard && ./tools/buildman/buildman -o 
/tmp/wandboard3 --step 0 -SsB -b 125956..69bf66 -devl wandboard)
Building 2 commits for 1 boards (1 thread, 16 jobs per thread)
01: scripts/get_default_envs.sh: preserve order of multiple entries for same 
variable
15: Merge branch '2020-05-06-master-imports'
 200 /2  wandboard
Completed: 2 total built, duration 0:00:23
Summary of 2 commits for 1 boards (1 thread, 16 jobs per thread)
01: scripts/get_default_envs.sh: preserve order of multiple entries for same 
variable
15: Merge branch '2020-05-06-master-imports'
arm: (for 1/1 boards) bss -16.0 text +16.0
 wandboard  : bss -16 text +16
u-boot: add: 0/0, grow: 3/0 bytes: 12/0 (12)
  function   old new   delta
  menu_create 84  88  +4
  handle_pxe_menu284 288  +4
  do_dcache   92  96  +4
(no errors to report)
$

What details are you recording in your setup?


I simply track the size of u-boot.img for the wandboard:

https://github.com/EmbLux-Kft/tbot-tbot2go/blob/wandboard-devel-messe/tc/wandboard/tc_wandboard.py#L95




Unfortunately I cannot automate git bisect with tbot:

http://tbot.tools/modules/tc.html#tbot.tc.git.GitRepository.bisect

as I do not know how to switch for example with an gpio and a relay
bootmode on wandboard ... do you know, if this is possible?


In general, or on the wandboard?  For the general side, I have a network
controllable relay controller that I can use to toggle a GPIO on
hardware that has a force reset exposed.  I don't have any imx platforms
exposed however as I wasn't able to rig up some way to boot via USB on
the platforms I have here.  That was a while ago tho.


Only for the wandboard. I have a collection of usbrelais and simple
relais connected to gpios, my preffered setup currently:

https://www.amazon.de/gp/product/B07RNWLXJM/ref=ppx_yo_dt_b_asin_title_o06_s00?ie=UTF8=1

attached to a raspberry PI, so I have with the PI a complete "lab host"
which runs tbot incl. wifi to connect to the rest of the world ;-)

But it seems wandboard has only sd card boot, so I need for this
a sd card simulator... what I do not have yet.

Thanks!

bye,
Heiko
--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: h...@denx.de

Re: [PATCH] imx8mp_evk: Make SPL binary size smaller

[Fabio Estevam]

Hi Marek,

On Thu, May 7, 2020 at 11:59 PM Marek Vasut  wrote:

> Enable CONFIG_WATCHDOG in your U-Boot config, that will enable WDT
> servicing.

Thanks. I changed the defconfig like this:

--- a/configs/imx8mm_evk_defconfig
+++ b/configs/imx8mm_evk_defconfig
@@ -85,5 +85,4 @@ CONFIG_SPL_SYSRESET=y
 CONFIG_SYSRESET_PSCI=y
 CONFIG_SYSRESET_WATCHDOG=y
 CONFIG_DM_THERMAL=y
-# CONFIG_WATCHDOG is not set
 CONFIG_IMX_WATCHDOG=y

I will submit this patch and for other boards tomorrow.

Thanks

Re: [PATCH] kbuild: add -Werror=implicit-function-declaration

[Simon Glass]

Hi Masahiro,

On Thu, 7 May 2020 at 19:54, Masahiro Yamada  wrote:
>
> On Fri, May 8, 2020 at 10:39 AM Simon Glass  wrote:
> >
> > Hi Masahiro,
> >
> > On Thu, 7 May 2020 at 06:21, Masahiro Yamada
> >  wrote:
> > >
> > > Add -Werror=implicit-function-declaration as Linux does.
> > >
> > > If you do not check the prototype, it may go wrong run-time.
> > > It is better to break the build, and require to include correct
> > > headers.
> > >
> > > Signed-off-by: Masahiro Yamada 
> > > ---
> > >
> > >  Makefile | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > NAK
> >
> > We already get a warning in this situation. This makes it painful for
> > development since things that should be warnings end up being errors.
> > So your build fails when really it should work well enough to do a
> > test run with your new code. I don't think it has any benefit on code
> > quality since we already detect warnings in gitlab, etc.
> >
> > U-Boot is set up so that warnings are reported and are easy to spot if
> > you use 'make -s' (i.e. not buried in the output).
> >
> > Regards,
> > Simon
>
>
>
> Linux added this flag in 2007.
>
> The intention seems to break the build earlier
> when a non-existing function is used.
>
> I have not seen compliant about this flag in Linux.
> What does it make different for U-Boot ?

Well that commit message is quite misleading. The author is presumably
ignoring the warnings that come out in the compile phase. For me they
come up loud and clear. I don't know why it takes half an hour to get
to the link stage. My average incremental build time is under 4
seconds including the link.

Finally, the warning does not tell you anything about whether the
function doesn't exist. It just tells you you have left out a header
file.

I know how much of a pain this is, because coreboot does this. It does
it partly because there is so much build output that the warnings are
invisible unless they actually halt the build. Even then you have to
search for what went wrong.

Regards,
SImon

>
>
>
>
> commit 94bed2a9c4ae980838003f5d32681eef794ecc28
> Author: Dave Jones 
> Date:   Sun Jul 15 23:41:38 2007 -0700
>
> Add -Werror-implicit-function-declaration
>
> Add -Werror-implicit-function-declaration
> This makes builds fail sooner if something is implicitly defined instead
> of having to wait half an hour for it to fail at the linking stage.
>
> Signed-off-by: Dave Jones 
> Cc: Sam Ravnborg 
> Signed-off-by: Andrew Morton 
> Signed-off-by: Linus Torvalds 
>
>
>
> --
> Best Regards
> Masahiro Yamada

Re: USB keyboard with XHCI on x86?

[Bin Meng]

Hi Simon,

On Fri, May 8, 2020 at 10:38 AM Simon Glass  wrote:
>
> Hi Bin,
>
> Does this work for you? It doesn't work for me in interrupt mode, only
> in control-endpoint mode. Is that expected?
>
> Also I notice that some broken devices don't work or cause a crash.
>

I don't have a board to test xHCI right now, but from the commit
message I wrote the interrupt mode should be working.

commit 1897d60130976ece389d5875187b78ba0d41428f
Author: Bin Meng 
Date:   Mon Sep 18 06:40:41 2017 -0700

usb: xhci: Add interrupt transfer support

xHCI uses normal TRBs for both bulk and interrupt. This adds the
missing interrupt transfer support to xHCI so that devices like
USB keyboard that uses interrupt transfer can work.

Signed-off-by: Bin Meng 

Regards,
Bin

Re: [PATCH] imx8mp_evk: Make SPL binary size smaller

[Marek Vasut]

On 5/8/20 4:56 AM, Fabio Estevam wrote:
> Hi Marek,
> 
> On Thu, May 7, 2020 at 10:40 PM Marek Vasut  wrote:
> 
>> Disable tiny printf in SPL, because that's really broken with
>> dm_dump_all(), run dm_dump_all() just after power_init_board() in
>> board/freescale/imx8m{p,q}_evk/spl.c  and compare the results. There is
>> likely gonna be a missing driver, my bet would still be on the clock.
> 
> That's a good suggestion. Yes, the clock drivers are missing for i.MX8MP.
> 
> Here is the comparison against a i.MX8MM EVK:
> https://pastebin.com/raw/XVc6AAvr
> 
> I will activate them on SPL tomorrow.

Great, thanks.

> Also, I noticed that i.MX8MM EVK has the following watchdog issues on master:
> 
> U-Boot SPL 2020.07-rc1-00387-g67887903af (May 07 2020 - 23:49:27 -0300)
> Normal Boot
> WDT:   Started without servicing (60s timeout)
> Trying to boot from MMC1
> 
> 
> U-Boot 2020.07-rc1-00387-g67887903af (May 07 2020 - 23:49:27 -0300)
> 
> CPU:   Freescale i.MX8MMQ rev1.0 at 1200 MHz
> Reset cause: POR
> Model: FSL i.MX8MM EVK board
> DRAM:  2 GiB
> WDT:   Started without servicing (60s timeout)
> MMC:   FSL_SDHC: 1, FSL_SDHC: 2
> Loading Environment from MMC... OK
> In:serial
> Out:   serial
> Err:   serial
> Net:
> Warning: ethernet@30be using MAC address from ROM
> eth0: ethernet@30be
> Hit any key to stop autoboot:  0
> u-boot=>
> 
> It reboots after staying 60s in the U-Boot prompt.

Enable CONFIG_WATCHDOG in your U-Boot config, that will enable WDT
servicing.

Re: USB keyboard with XHCI on x86?

[Marek Vasut]

On 5/8/20 4:41 AM, Simon Glass wrote:
> Hi Marek,
> 
> On Thu, 7 May 2020 at 20:39, Marek Vasut  wrote:
>>
>> On 5/8/20 4:38 AM, Simon Glass wrote:
>>> Hi Bin,
>>>
>>> Does this work for you? It doesn't work for me in interrupt mode, only
>>> in control-endpoint mode. Is that expected?
>>>
>>> Also I notice that some broken devices don't work or cause a crash.
>>
>> I recall some keyboards didn't work in interrupt mode, which is why I
>> added the control endpoint polling. Does it work on EHCI for you ?
> 
> I'm not sure how to try it with an XHCI controller.

Do you have an EHCI controller on that system too ?

Re: [PATCH] imx8mp_evk: Make SPL binary size smaller

[Fabio Estevam]

Hi Marek,

On Thu, May 7, 2020 at 10:40 PM Marek Vasut  wrote:

> Disable tiny printf in SPL, because that's really broken with
> dm_dump_all(), run dm_dump_all() just after power_init_board() in
> board/freescale/imx8m{p,q}_evk/spl.c  and compare the results. There is
> likely gonna be a missing driver, my bet would still be on the clock.

That's a good suggestion. Yes, the clock drivers are missing for i.MX8MP.

Here is the comparison against a i.MX8MM EVK:
https://pastebin.com/raw/XVc6AAvr

I will activate them on SPL tomorrow.

Also, I noticed that i.MX8MM EVK has the following watchdog issues on master:

U-Boot SPL 2020.07-rc1-00387-g67887903af (May 07 2020 - 23:49:27 -0300)
Normal Boot
WDT:   Started without servicing (60s timeout)
Trying to boot from MMC1


U-Boot 2020.07-rc1-00387-g67887903af (May 07 2020 - 23:49:27 -0300)

CPU:   Freescale i.MX8MMQ rev1.0 at 1200 MHz
Reset cause: POR
Model: FSL i.MX8MM EVK board
DRAM:  2 GiB
WDT:   Started without servicing (60s timeout)
MMC:   FSL_SDHC: 1, FSL_SDHC: 2
Loading Environment from MMC... OK
In:serial
Out:   serial
Err:   serial
Net:
Warning: ethernet@30be using MAC address from ROM
eth0: ethernet@30be
Hit any key to stop autoboot:  0
u-boot=>

It reboots after staying 60s in the U-Boot prompt.

Thanks

Re: USB keyboard with XHCI on x86?

[Simon Glass]

Hi Marek,

On Thu, 7 May 2020 at 20:39, Marek Vasut  wrote:
>
> On 5/8/20 4:38 AM, Simon Glass wrote:
> > Hi Bin,
> >
> > Does this work for you? It doesn't work for me in interrupt mode, only
> > in control-endpoint mode. Is that expected?
> >
> > Also I notice that some broken devices don't work or cause a crash.
>
> I recall some keyboards didn't work in interrupt mode, which is why I
> added the control endpoint polling. Does it work on EHCI for you ?

I'm not sure how to try it with an XHCI controller.

Regards,
Simon

Re: [PATCH] ARM: uniphier: delete or replace includes

[Simon Glass]

Hi Masahiro,

On Thu, 7 May 2020 at 20:31, Masahiro Yamada  wrote:
>
> Hi Simon,
>
>
> On Fri, May 8, 2020 at 10:37 AM Simon Glass  wrote:
> >
> > Hi Masahiro,
> >
> > On Thu, 7 May 2020 at 07:10, Masahiro Yamada
> >  wrote:
> > >
> > >  pulls in a lot of bloat.  is unneeded in most of
> > > places.
> > >
> > > Signed-off-by: Masahiro Yamada 
> > > ---
> > >
> > >
> >
> > I'm wary of this. I think that every file should include common.h
>
>
> I disagree.
>
> "Please include  at the beginning of every file"
> is a fragile rule.
> You have no way to check it.

We can add it to checkpatch.

>
> Our goal is to get rid of the
> special treatment of 

As we get closer though I've been thinking about the goal. Do we want
people to include config.h specifically if common.h has nothing in it?
I feel it is safer to keep common.h, perhaps just with config.h
included, until we fully understand what we need.

>
>
>
> > and
> > the solution is to remove the bloat. I have been plugging away at
> > that. There is a pending series that reduces it down further, to 14
> > includes. Please help review!
> >
> > http://patchwork.ozlabs.org/project/uboot/list/?series=169491
>
> I saw it.
>
> Humans cannot check it.
> If buildman does not raise a flag, it is fine.

OK.

>
>
>
> >
> > The problem is that when someone uses #ifdef CONFIG options the
> > config.h has to be included. So your patch is a bit brittle. As soon
> > as someone uses CONFIG it may break.
>
>
> For the legacy CONFIG options, yes.
> The options in Kconfig are all safe.

How come? If config.h is included, the options are not defined.

>
> Common options were mostly moved to Kconfig.
>
> We still have lots in scripts/config_whitelist.txt
> but most of them are platform-specific craps.

Yes...perhaps we should try to have two whitelists, so we know which
ones matter more.

Regards,
Simon

Re: USB keyboard with XHCI on x86?

[Marek Vasut]

On 5/8/20 4:38 AM, Simon Glass wrote:
> Hi Bin,
> 
> Does this work for you? It doesn't work for me in interrupt mode, only
> in control-endpoint mode. Is that expected?
> 
> Also I notice that some broken devices don't work or cause a crash.

I recall some keyboards didn't work in interrupt mode, which is why I
added the control endpoint polling. Does it work on EHCI for you ?

USB keyboard with XHCI on x86?

[Simon Glass]

Hi Bin,

Does this work for you? It doesn't work for me in interrupt mode, only
in control-endpoint mode. Is that expected?

Also I notice that some broken devices don't work or cause a crash.

Regards,
SImon

Re: [PATCH] ARM: uniphier: delete or replace includes

[Masahiro Yamada]

Hi Simon,


On Fri, May 8, 2020 at 10:37 AM Simon Glass  wrote:
>
> Hi Masahiro,
>
> On Thu, 7 May 2020 at 07:10, Masahiro Yamada
>  wrote:
> >
> >  pulls in a lot of bloat.  is unneeded in most of
> > places.
> >
> > Signed-off-by: Masahiro Yamada 
> > ---
> >
> >
>
> I'm wary of this. I think that every file should include common.h


I disagree.

"Please include  at the beginning of every file"
is a fragile rule.
You have no way to check it.

Our goal is to get rid of the
special treatment of 



> and
> the solution is to remove the bloat. I have been plugging away at
> that. There is a pending series that reduces it down further, to 14
> includes. Please help review!
>
> http://patchwork.ozlabs.org/project/uboot/list/?series=169491

I saw it.

Humans cannot check it.
If buildman does not raise a flag, it is fine.



>
> The problem is that when someone uses #ifdef CONFIG options the
> config.h has to be included. So your patch is a bit brittle. As soon
> as someone uses CONFIG it may break.


For the legacy CONFIG options, yes.
The options in Kconfig are all safe.

Common options were mostly moved to Kconfig.

We still have lots in scripts/config_whitelist.txt
but most of them are platform-specific craps.




> Anyway this is uniphier code so it's up to you.
>
> Regards,
> Simon



--
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: add -Werror=implicit-function-declaration

[Masahiro Yamada]

On Fri, May 8, 2020 at 10:39 AM Simon Glass  wrote:
>
> Hi Masahiro,
>
> On Thu, 7 May 2020 at 06:21, Masahiro Yamada
>  wrote:
> >
> > Add -Werror=implicit-function-declaration as Linux does.
> >
> > If you do not check the prototype, it may go wrong run-time.
> > It is better to break the build, and require to include correct
> > headers.
> >
> > Signed-off-by: Masahiro Yamada 
> > ---
> >
> >  Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> NAK
>
> We already get a warning in this situation. This makes it painful for
> development since things that should be warnings end up being errors.
> So your build fails when really it should work well enough to do a
> test run with your new code. I don't think it has any benefit on code
> quality since we already detect warnings in gitlab, etc.
>
> U-Boot is set up so that warnings are reported and are easy to spot if
> you use 'make -s' (i.e. not buried in the output).
>
> Regards,
> Simon



Linux added this flag in 2007.

The intention seems to break the build earlier
when a non-existing function is used.

I have not seen compliant about this flag in Linux.
What does it make different for U-Boot ?




commit 94bed2a9c4ae980838003f5d32681eef794ecc28
Author: Dave Jones 
Date:   Sun Jul 15 23:41:38 2007 -0700

Add -Werror-implicit-function-declaration

Add -Werror-implicit-function-declaration
This makes builds fail sooner if something is implicitly defined instead
of having to wait half an hour for it to fail at the linking stage.

Signed-off-by: Dave Jones 
Cc: Sam Ravnborg 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] imx8mp_evk: Make SPL binary size smaller

[Marek Vasut]

On 5/8/20 3:07 AM, Fabio Estevam wrote:
> On Thu, May 7, 2020 at 8:59 AM Fabio Estevam  wrote:
>>
>> Commit f24dea4e1b52 ("ARM: imx8m: Fix reset in SPL on NXP iMX8MP EVK") caused
>> the u-boot-spl.bin binary size to grow by around 2000 bytes, which makes the
>> board to fail to boot.
> 
> The new SPL size is 97kB, which is below the limit define in imx8mp_evk.h:
> #define CONFIG_SPL_MAX_SIZE (152 * 1024)
> 
> It seems the problem is not related to SPL size.

Disable tiny printf in SPL, because that's really broken with
dm_dump_all(), run dm_dump_all() just after power_init_board() in
board/freescale/imx8m{p,q}_evk/spl.c  and compare the results. There is
likely gonna be a missing driver, my bet would still be on the clock.

Re: [PATCH 1/2] test: describe naming conventions for macro UNIT_TEST

[Simon Glass]

On Wed, 6 May 2020 at 17:28, Stephen Warren  wrote:
>
> On 5/6/20 10:26 AM, Heinrich Schuchardt wrote:
> > Strict naming conventions have to be followed for Python function
> > generate_ut_subtest() to collect C unit tests to be executed via
> > command 'ut'.
> >
> > Describe the requirements both on the C as well on the Python side.
>
> > +/**
> > + * UNIT_TEST() - create linker generated list entry for unit a unit test
> > + *
> > + * The macro UNIT_TEST() is used to create a linker generated list entry. 
> > These
> > + * list entries are enumerate tests that can be execute using the ut 
> > command.
> > + * The list entries are used both by the implementation of the ut command 
> > as
> > + * well as in a related Python test.
> > + *
> > + * For Python testing the subtests are collected in Python function
> > + * generate_ut_subtest() by applying a regular expression to the lines of 
> > file
> > + * u-boot.sym. The list entries have to follow strict naming conventions 
> > to be
> > + * matched by the expression.
> > + *
> > + * Use UNIT_TEST(foo_test_bar, _flags, foo_test) for a test bar in test 
> > suite
> > + * foo that can be executed via command 'ut foo bar' and is implemented in
> > + * function foo_test_bar().
> > + *
> > + * @_name:   concatenation of name of the test suite, "_test_", and the 
> > name
> > + *   of the test
> > + * @_flags:  an integer field that can be evaluated by the test suite
> > + *   implementation
> > + * @_suite:  name of the test suite concatenated with "_test"
> > + */
>
> Perhaps the macro could simply take "foo" and "bar" as parameters, and
> generate the function name foo_test_bar internally rather than having
> the user pass it in. That way, compilation will actively fail if the
> function isn't named correctly, since it won't match the reference
> created by this macro.
>
> To help make this easier, we could add another macro e.g.
> UNIT_TEST_FUNC() that evaluates to just the expected function name, so
> that people wouldn't have to know the naming convention when they
> implement the function; they'd just write e.g.:
>
> static int UNIT_TEST_FUNC(log, nolog_err)(struct unit_test_state *uts)

I am not a huge fan of that. It looks weird to have the function name
auto-generated, and it defeats ctags, code search, etc.. Another
option might be to check for exported test/ functions that don't match
and print a warning?

>
> But certainly this series is a good first step, and fine even if we
> don't implement this suggestion.

Yes definitely.

>
> The series,
> Reviewed-by: Stephen Warren 

Re: [RFC] dm: uclass: add functions to get device by platdata

[Simon Glass]

Hi Walter,

On Wed, 6 May 2020 at 06:57, Walter Lozano  wrote:
>
> Hi Simon,
>
> On 23/4/20 00:38, Walter Lozano wrote:
> > Hi Simon,
> >
> > On 21/4/20 14:36, Simon Glass wrote:
> >> Hi Walter,
> >>
> >> On Fri, 17 Apr 2020 at 15:18, Walter Lozano
> >>  wrote:
> >>> Hi Simon,
> >>>
> >>> On 9/4/20 16:36, Simon Glass wrote:
>  HI Walter,
> 
>  On Thu, 9 Apr 2020 at 12:57, Walter Lozano
>   wrote:
> > Hi Simon,
> >
> > On 8/4/20 00:14, Simon Glass wrote:
> >> Hi Walter,
> >>
> >> On Tue, 7 Apr 2020 at 14:05, Walter
> >> Lozano  wrote:
> >>> Hi Simon,
> >>>
> >>> On 6/4/20 00:43, Simon Glass wrote:
>  Hi Walter,
> 
>  On Mon, 9 Mar 2020 at 12:27, Walter
>  Lozano   wrote:
> > Hi Simon
> >
> > On 6/3/20 17:32, Simon Glass wrote:
> >> Hi Walter,
> >>
> >> On Fri, 6 Mar 2020 at 09:10, Walter
> >> Lozano wrote:
> >>> Hi Simon,
> >>>
> >>> Thanks again for taking the time to check my comments.
> >>>
> >>> On 6/3/20 10:17, Simon Glass wrote:
>  Hi Walter,
> 
>  On Thu, 5 Mar 2020 at 06:54, Walter
>  Lozano wrote:
> > Hi Simon,
> >
> > Thanks for taking the time to check for my comments
> >
> > On 4/3/20 20:11, Simon Glass wrote:
> >
> >> Hi Walter,
> >>
> >> On Wed, 4 Mar 2020 at 12:40, Walter
> >> Lozano wrote:
> >>> When OF_PLATDATA is enabled DT information is parsed and
> >>> platdata
> >>> structures are populated. In this context the links
> >>> between DT nodes are
> >>> represented as pointers to platdata structures, and
> >>> there is no clear way
> >>> to access to the device which owns the structure.
> >>>
> >>> This patch implements a set of functions:
> >>>
> >>> - device_find_by_platdata
> >>> - uclass_find_device_by_platdata
> >>>
> >>> to access to the device.
> >>>
> >>> Signed-off-by: Walter Lozano
> >>> ---
> >>> drivers/core/device.c| 19
> >>> +++
> >>> drivers/core/uclass.c| 34
> >>> ++
> >>> include/dm/device.h |  2 ++
> >>> include/dm/uclass-internal.h |  3 +++
> >>> include/dm/uclass.h |  2 ++
> >>> 5 files changed, 60 insertions(+)
> >> This is interesting. Could you also add the motivation
> >> for this? It's
> >> not clear to me who would call this function.
> > I have been reviewing the OF_PLATDATA support as an R
> > project, in this context, in order to have
> > a better understanding on the possibilities and
> > limitations I decided to add its support to iMX6,
> > more particularly to the MMC drivers. The link issue
> > arises when I tried to setup the GPIO for
> > Card Detection, which is trivial when DT is available.
> > However, when OF_PLATDATA is enabled
> > this seems, at least for me, not straightforward.
> >
> > In order to overcome this limitation I think that having a
> > set of functions to find/get devices
> > based on platdata could be useful. Of course, there might
> > be a better approach/idea, so that is
> > the motivation for this RFC.
> >
> > An example of the usage could be
> >
> > #if CONFIG_IS_ENABLED(DM_GPIO)
> >
> >struct udevice *gpiodev;
> >
> >ret =
> > uclass_get_device_by_platdata(UCLASS_GPIO, (void
> > *)dtplat->cd_gpios->node, );
> >
> >if (ret)
> >return ret;
> >
> >ret = gpio_dev_request_index(gpiodev,
> > gpiodev->name, "cd-gpios",
> > dtplat->cd_gpios->arg[0], GPIOD_IS_IN,
> > dtplat->cd_gpios->arg[1], >cd_gpio);
> >
> >if (ret)
> >return ret;
> >
> > #endif
> >
> > This is part of my current work, a series of patches to
> > add OF_PLATDATA support as explained.
> >
> > Does this make sense to you?
>  Not yet :-)
> 
>  What is the context of this call? Typically dtplat is only
>  

Re: [PATCH] kbuild: add -Werror=implicit-function-declaration

[Simon Glass]

Hi Masahiro,

On Thu, 7 May 2020 at 06:21, Masahiro Yamada
 wrote:
>
> Add -Werror=implicit-function-declaration as Linux does.
>
> If you do not check the prototype, it may go wrong run-time.
> It is better to break the build, and require to include correct
> headers.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

NAK

We already get a warning in this situation. This makes it painful for
development since things that should be warnings end up being errors.
So your build fails when really it should work well enough to do a
test run with your new code. I don't think it has any benefit on code
quality since we already detect warnings in gitlab, etc.

U-Boot is set up so that warnings are reported and are easy to spot if
you use 'make -s' (i.e. not buried in the output).

Regards,
Simon

Re: U-CLASS SPI Bus and Devices

[Simon Glass]

Hi Rudolf,

On Wed, 18 Mar 2020 at 05:25, Rudolf J Streif  wrote:
>
> I ran into an issue today with a U-CLASS SPI NOR flash device on a NXP
> FlexSPI controller. U-Boot started correctly from the flash device but
> using 'sf probe 0:0' would always return 'Invalid bus 0 (err=-19)'. This
> error message is emitted by spi_get_bus_and_cs() in
> drivers/spi/spi-uclass.c. I traced the issue to
> uclass_get_device_by_seq() in drivers/core/uclass.c.
>
> The function first searches the device list for a device that already
> claimed the sequence number (dev->seq). If not found it would look if a
> device requested that sequence number (dev->seq_req). That would always
> fail for my device. The bus had not been probed yet and hence dev->seq
> was -1 and the device also had dev->req_seq set to -1.
>
> The board is using a device tree hence it would only make sense to set
> the requested sequence number via the device tree. However, there is no
> such thing and even if there was it might not be specified.
>
> Consequently, the device was never probed although the driver was
> correctly set up via device tree.
>
> I worked around it by simply setting dev->req_seq of the first device
> that had it set to -1 to the sequence number the search function was
> looking for (see patch below). It solved my problem but I don't know if
> that is the right way of addressing it. I could not find any other
> solution for this particular problem anywhere.
>
> Rudi

You can put the SPI flash in the device tree with an alias pointing to
it.. That is the intended way with driver model.

Regards,
SImon



>
>
>
> From 0f05ab964fcc7d29d8d467e663d7daa72328cf66 Mon Sep 17 00:00:00 2001
> From: Rudolf J Streif 
> Date: Tue, 17 Mar 2020 17:13:07 -0700
> Subject: [PATCH] Fix issue with SPI device sequence number
>
> If the requested sequence number for a SPI device was -1 (any)
> then the device would never be probed. This fix simply assigns
> the sequence number asked for at probing to the device if it has
> not been probed yet and the requested sequence number is -1.
>
> Signed-off-by: Rudolf J Streif 
> ---
>  drivers/core/uclass.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
> index fc3157de39..e791103153 100644
> --- a/drivers/core/uclass.c
> +++ b/drivers/core/uclass.c
> @@ -310,6 +310,8 @@ int uclass_find_device_by_seq(enum uclass_id id, int
> seq_or_req_seq,
>
> uclass_foreach_dev(dev, uc) {
> debug("   - %d %d '%s'\n", dev->req_seq, dev->seq,
> dev->name);
> +   if (find_req_seq && dev->req_seq == -1)
> +   dev->req_seq = seq_or_req_seq;
> if ((find_req_seq ? dev->req_seq : dev->seq) ==
> seq_or_req_seq) {
> *devp = dev;
> --
> 2.23.0
>
>
>

Re: [PATCH v4 5/5] doc: add bind/unbind command documentation

[Simon Glass]

On Thu, 30 Apr 2020 at 04:06, Patrice Chotard  wrote:
>
> Add documentation in doc/drivel-model for the bind/unbind command.
> Part of this documentation is extracted from original patch commit
> message:
> commit 49c752c93a78 ("cmd: Add bind/unbind commands to bind a device to a 
> driver from the command line")
>
> Signed-off-by: Patrice Chotard 
>
> ---
>
> Changes in v4:
>- fix make htmldocs error "Title underline too short"
>
> Changes in v3:
>- fix typo
>- add bind/unbind parameters description and how to find them
>
> Changes in v2: None
>
>  doc/driver-model/bind.rst  | 49 ++
>  doc/driver-model/index.rst |  1 +
>  2 files changed, 50 insertions(+)
>  create mode 100644 doc/driver-model/bind.rst

I'm still a bit unsure why we must specify the driver name.

Reviewed-by: Simon Glass 

Re: [PATCH v1 3/3] drivers: pinctrl-single: add request api

[Simon Glass]

Hi Rayagonda,

On Thu, 30 Apr 2020 at 05:03, Rayagonda Kokatanur
 wrote:
>
> Hi Simon,
>
> On Wed, Apr 29, 2020 at 11:34 PM Simon Glass  wrote:
> >
> > Hi Rayagonda,
> >
> > +Stephen Warren
> >
> > On Wed, 29 Apr 2020 at 10:36, Rayagonda Kokatanur
> >  wrote:
> > >
> > > Add pinctrl_ops->request api to configure pctrl
> > > pad register in gpio mode.
> > >
> > > Signed-off-by: Rayagonda Kokatanur 
> > > ---
> > >  drivers/pinctrl/pinctrl-single.c | 29 +
> > >  1 file changed, 29 insertions(+)
> >
> > This should use the Kconfig also (and needs a test)
>
> Please elaborate "need a test".
> Do you mean a testing procedure as part of the commit message ?

No I mean U-Boot has automated tests. So when we add code we need to
add tests for that code. See test/dm/rtc.c for an example. You can run
them with:

   make qcheck

or a single one with:



>
> I feel we don't need the Kconfig option, looking at Linux pinctrl-single 
> driver.
> Please let me know.

OK we can go without one. It is easy to add later if needed.

Regards,
Simon

Re: [PATCH v1 2/3] drivers: pinctrl-single: add support to parse gpio properties

[Simon Glass]

Hi Rayagonda,

On Thu, 30 Apr 2020 at 05:10, Rayagonda Kokatanur
 wrote:
>
> Hi Simon,
>
> On Wed, Apr 29, 2020 at 11:34 PM Simon Glass  wrote:
> >
> > Hi Rayagonda,
> >
> > +Stephen Warren
> >
> > On Wed, 29 Apr 2020 at 10:35, Rayagonda Kokatanur
> >  wrote:
> > >
> > > Parse different gpio properties from dt as part of probe
> > > function. This detail will be used to enable pinctrl pad
> > > later when gpio lines are requested.
> > >
> > > Signed-off-by: Rayagonda Kokatanur 
> > > ---
> > >  drivers/pinctrl/pinctrl-single.c | 62 +++-
> > >  1 file changed, 61 insertions(+), 1 deletion(-)
> >
> > Can you please add the binding and a test? Also I think this feature
> > should be behind a Kconfig flag to avoid code-size increase.
>
> Sorry I didn't get it, please elaborate "binding and a test".
> You mean dt-binding document and test procedure.

That's right. For the tests, it seems we do not have a
test/dm/pinctrl.c but we need one, something simple to start.

The binding will help explain what is going on.

>
> This feature is added by referring to linux pinctrl-single driver and
> code is in align with linux driver.
> This feature is going to be used in most of the gpio controllers where
> they have pin controllers to select
> different modes of gpio lines. I feel this feature should be part of
> the driver by default.

OK makes sense.

Re the Kconfig flag, U-Boot SPL runs in a tight environment. So when
we add new features we sometimes put them behind a flag so that it
does not bloat SPL.

Regards,
Simon

Re: [PATCH] ARM: uniphier: delete or replace includes

[Simon Glass]

Hi Masahiro,

On Thu, 7 May 2020 at 07:10, Masahiro Yamada
 wrote:
>
>  pulls in a lot of bloat.  is unneeded in most of
> places.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  arch/arm/mach-uniphier/arm32/cache-uniphier.c | 1 -
>  arch/arm/mach-uniphier/arm32/psci.c   | 1 -
>  arch/arm/mach-uniphier/arm32/timer.c  | 2 +-
>  arch/arm/mach-uniphier/arm64/mem_map.c| 1 -
>  arch/arm/mach-uniphier/base-address.c | 2 +-
>  arch/arm/mach-uniphier/board_late_init.c  | 1 -
>  arch/arm/mach-uniphier/boards.c   | 2 +-
>  arch/arm/mach-uniphier/boot-device/boot-device-ld11.c | 1 -
>  arch/arm/mach-uniphier/boot-device/boot-device-ld4.c  | 1 -
>  arch/arm/mach-uniphier/boot-device/boot-device-pro5.c | 1 -
>  arch/arm/mach-uniphier/boot-device/boot-device-pxs2.c | 1 -
>  arch/arm/mach-uniphier/boot-device/boot-device-pxs3.c | 2 +-
>  arch/arm/mach-uniphier/boot-device/boot-device.c  | 4 +++-
>  arch/arm/mach-uniphier/clk/clk-dram-ld4.c | 1 -
>  arch/arm/mach-uniphier/clk/clk-dram-pxs2.c| 1 -
>  arch/arm/mach-uniphier/clk/clk-early-ld4.c| 1 -
>  arch/arm/mach-uniphier/clk/clk-ld11.c | 2 +-
>  arch/arm/mach-uniphier/clk/dpll-ld4.c | 2 +-
>  arch/arm/mach-uniphier/clk/dpll-pro4.c| 2 +-
>  arch/arm/mach-uniphier/debug-uart/debug-uart.c| 1 -
>  arch/arm/mach-uniphier/dram/cmd_ddrmphy.c | 3 +--
>  arch/arm/mach-uniphier/dram/cmd_ddrphy.c  | 2 +-
>  arch/arm/mach-uniphier/dram/umc-ld4.c | 1 -
>  arch/arm/mach-uniphier/dram/umc-pro4.c| 1 -
>  arch/arm/mach-uniphier/dram/umc-sld8.c| 1 -
>  arch/arm/mach-uniphier/dram_init.c| 2 +-
>  arch/arm/mach-uniphier/fdt-fixup.c| 2 +-
>  arch/arm/mach-uniphier/init.h | 1 +
>  arch/arm/mach-uniphier/memconf.c  | 1 -
>  arch/arm/mach-uniphier/micro-support-card.c   | 3 ++-
>  arch/arm/mach-uniphier/mmc-boot-mode.c| 1 -
>  arch/arm/mach-uniphier/mmc-first-dev.c| 2 +-
>  arch/arm/mach-uniphier/pinctrl-glue.c | 1 -
>  arch/arm/mach-uniphier/reset.c| 1 -
>  arch/arm/mach-uniphier/sbc/sbc-ld11.c | 1 -
>  arch/arm/mach-uniphier/sbc/sbc.c  | 1 -
>  arch/arm/mach-uniphier/spl_board_init.c   | 1 -
>  37 files changed, 18 insertions(+), 37 deletions(-)
>

I'm wary of this. I think that every file should include common.h and
the solution is to remove the bloat. I have been plugging away at
that. There is a pending series that reduces it down further, to 14
includes. Please help review!

http://patchwork.ozlabs.org/project/uboot/list/?series=169491

The problem is that when someone uses #ifdef CONFIG options the
config.h has to be included. So your patch is a bit brittle. As soon
as someone uses CONFIG it may break.

Anyway this is uniphier code so it's up to you.

Regards,
Simon

Re: [PATCH V2] mkimage: fit: Do not tail-pad fitImage with external data

[Marek Vasut]

On 5/7/20 10:46 PM, Samuel Holland wrote:
> On 5/6/20 12:02 PM, trini at konsulko.com (Tom Rini) wrote:
>> I'm not sure that it is.  Can we easily/safely memmove the data to be
>> aligned?  Is that really a better option in this case than ensuring
>> alignment within the file?
>
> Can't we use the new mkimage -B option to enforce the alignment IFF and
> only IFF it is required ? 

 Perhaps.  But..

> Then we can enforce it separately for 32bit
> and 64bit platforms to 4 and 8 bytes respectively even.

 It's 8 bytes for both.  It's possible that Linux doesn't hard fail if
 you only do 4 byte alignment but the documented requirement is 8, for
 arm32.
>>>
>>> With Linux you usually need to move the kernel anyway, no ? It's 2 MiB
>>> for arm64 for example.
>>
>> For arm64 you have to move it to where text_offset says it needs to be.
>> For arm32 the common (always, practically?) case is you're firing off
>> the zImage which does what's needed.  But..
>>
>>> And what you usually parse in-place would be the DT then.
>>
>> Yes, the practical case is that it's a DT and that needs 8 byte
>> alignment.  And we should just get back to aligning that correctly.
>> Going back to the v1 thread, it turns out the answer to "why do we even
>> have this padding?" is "we need the DT to be aligned".
> 
> This change broke SPL booting for me on MACH_SUN50I as well. One thing that I
> haven't seen brought up yet is that SPL FIT code assumes exactly a 4-byte
> alignment of external data after the FIT. In spl_load_simple_fit():
> 
>  /*
>   * For FIT with external data, figure out where the external images
>   * start. This is the base for the data-offset properties in each
>   * image.
>   */
>  size = fdt_totalsize(fit);
>  size = (size + 3) & ~3;
>  size = board_spl_fit_size_align(size);
>  base_offset = (size + 3) & ~3;

Somehow this doesn't match the 8-byte alignment Tom was suggesting.
And that only leads me to believe that we can either make assumptions
about alignment, which would very likely fail one way or the other OR we
can say that for SPL as a special case, we enforce some alignment.

But that in turn fails for fitImage with embedded data, where the
embedded data are always aligned to 4 bytes, because that's how DTC
aligns properties.

Re: [PATCH v1 1/3] drivers: pinctrl-single: handle different register width

[Simon Glass]

Hi Rayagonda,

On Thu, 30 Apr 2020 at 04:06, Rayagonda Kokatanur
 wrote:
>
> On Wed, Apr 29, 2020 at 11:34 PM Simon Glass  wrote:
> >
> > Hi Rayagonda,
> >
> > +Stephen Warren
> >
> > On Wed, 29 Apr 2020 at 10:35, Rayagonda Kokatanur
> >  wrote:
> > >
> > > Add support to use different register read/write api's
> > > based on register width.
> > >
> > > Signed-off-by: Rayagonda Kokatanur 
> > > ---
> > >  drivers/pinctrl/pinctrl-single.c | 98 
> > >  1 file changed, 74 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/pinctrl-single.c 
> > > b/drivers/pinctrl/pinctrl-single.c
> > > index 738f5bd636..aed113b083 100644
> > > --- a/drivers/pinctrl/pinctrl-single.c
> > > +++ b/drivers/pinctrl/pinctrl-single.c
> > > @@ -10,12 +10,24 @@
> > >  #include 
> > >  #include 
> > >
> > > +/**
> > > + * struct single_pdata - pinctrl device instance
> > > + * @base   first configuration register
> > > + * @offset index of last configuration register
> > > + * @mask   configuration-value mask bits
> > > + * @width  configuration register bit width
> > > + * @bits_per_mux
> > > + * @read   register read function to use
> > > + * @write  register write function to use
> > > + */
> > >  struct single_pdata {
> > > fdt_addr_t base;/* first configuration register */
> > > int offset; /* index of last configuration register */
> > > u32 mask;   /* configuration-value mask bits */
> > > int width;  /* configuration register bit width */
> > > bool bits_per_mux;
> > > +   u32 (*read)(phys_addr_t reg);
> > > +   void (*write)(u32 val, phys_addr_t reg);
> >
> > Can't we just have a read & write function with a switch statement?
> > Why do we need function pointers?
>
> I referred to the linux pinctrl-single.c and kept code similar to linux.
> Please let me know.

See the regmap discussion here which is related:

http://patchwork.ozlabs.org/project/uboot/patch/20191105114700.24989-3-jjhib...@ti.com/

Should this driver use regmap, then?

Regards,
Simon

Re: [PATCH v2 2/4] regmap: Allow providing read/write callbacks through struct regmap_config

[Simon Glass]

Hi Vignesh,

On Thu, 7 May 2020 at 03:08, Vignesh Raghavendra  wrote:
>
>
>
> On 06/05/20 8:17 pm, Simon Glass wrote:
> > Hi Pratyush,
> >
> > On Tue, 5 May 2020 at 13:47, Pratyush Yadav  wrote:
> >>
> >> Hi Simon,
> >>
> >> I would be taking up this series and address the comments.
> >>
> >> On 10/12/19 03:18PM, Simon Glass wrote:
> >>> Hi Jean-Jacques,
> >>>
> >>> On Tue, 5 Nov 2019 at 04:47, Jean-Jacques Hiblot  wrote:
> 
>  Some linux drivers provide their own read/write functions to access data
>  from/of the regmap. Adding support for it.
> 
>  Signed-off-by: Jean-Jacques Hiblot 
> 
>  ---
> 
>  Changes in v2:
>  - Only use custom accessors if {,SPL,TPL}_REGMAP_ACCESSORS is enabled
> 
>   drivers/core/Kconfig  | 25 +
>   drivers/core/regmap.c | 22 --
>   include/regmap.h  | 28 +---
>   3 files changed, 70 insertions(+), 5 deletions(-)
> >>>
> >>> Coming back to the discussion on driver model
> >>
> >> IIUC, you are suggesting that we create a uclass for regmap, and fill in
> >> the ops with the driver's custom read/write functions, correct? This
> >> would mean we will have to create udevice for each regmap. A driver can
> >> have multiple regmaps (up to 18 for example in the Linux Cadence Sierra
> >> PHY driver in kernel). Creating a device for each regmap is very
> >> inefficient. Is the overhead really worth it? Does it solve any
> >> significant problem? Also, a regmap is not a device, it is an
> >> abstraction layer to simplify register access. Does it then make sense
> >> to model it as a device?
> >
> > I was actually referring to the access method of the regmap but I
> > suppose the effect is the same. This question has been running for a
> > while.
> >
> > The issue is that this is getting more and more complicated. We use
> > devices to model complicated things. It is an abstraction. It doesn't
> > have to be a physical device.
> >
> > With regmap we are creating an ad-hoc structure and associated
> > infrastructure to deal with this one case. It is not possible to see
> > the regmaps with 'dm dump', for example.
> >
> >>
> >>> How do you specify the fields? I would expect that this would be done
> >>> in the driver tree? Perhaps in a subnode of the device?
> >>>
> >>> Just to state what I see as the advantages of using a separate device
> >>> for access:
> >>>
> >>> - Remove the #ifdef in the regmap struct
> >>
> >> If you really want to remove the #ifdefs, we can set reg_read to a
> >> default, and let drivers override if when creating a regmap. Then in
> >> regmap_read, just call reg_read. This gets rid of the #ifdefs with a
> >> small change. I suspect this will have a much smaller impact on memory
> >> usage than using a udevice.
> >
> > Yes that sounds good, but you are getting closer and closer to it
> > being a device.
> >
> >>
> >>> - Easy to specify the behaviour in a device-tree node
> >>
> >> Quoting from the kernel's documentation of regmap_config:
> >>
> >>   reg_read: Optional callback that if filled will be used to perform all
> >>   the reads from the registers. Should only be provided for devices
> >>   whose read operation cannot be represented as a simple read operation
> >>   on a bus such as SPI, I2C, etc. Most of the devices do not need this.
> >>   reg_write: Same as above for writing.
> >>
> >> These custom accessors are meant to be used when the read or write needs
> >> more work to be done than the standard regmap reads/writes. And so they
> >> are supposed to be driver-specific functions. I don't think it is at all
> >> possible to specify something like this in devicetree. Am I missing
> >> something?
> >
> > Can you point to an example of this extra read/write code?
>
>
> Here is the cadence sierra PHY driver for which this series is a pre
> requiste
>
> https://elixir.bootlin.com/linux/latest/source/drivers/phy/cadence/phy-cadence-sierra.c
>
> PHY Registers are 16 bit wide. Some implementations (SoCs) have arranged
> these registers continuously (so they are aligned to 16 bit address
> boundary) but some implementation place each register at 32 bit boundary
> (wasting the upper 16 bits)
>
> There are few other nits but above is the major one.
>

OK I see. So it's fairly simple, not accessing through a mailbox, etc.

> Custom regmap read()/write() hooks abstracts all these from actual
> driver logic.
>
> It is possible to re implement driver w/o regmap abstraction but this
> would deviate the driver significantly from that of kernel. PHY drivers
> often get updated with new register settings as and when HW
> characterization progresses which need to be ported to kernel and
> U-Boot. Having U-Boot driver same as kernel will ease porting effort and
> also reduces the chances of errors.

I actually don't think that argument holds much water. I am not
suggesting that the regmap interface be different, so the code should
remain the 

Re: Re: [RFC PATCH 2/2] arch: x86: apl: Use devicetree for FSP configuration

[Simon Glass]

Hi Bernhard,

On Thu, 7 May 2020 at 01:53, Bernhard Messerklinger
 wrote:
>
> Hi Simon,
>
> >Hi Bernhard,
> >
> >On Thu, 30 Apr 2020 at 03:16, Bernhard Messerklinger
> > wrote:
> >>
> >> A the moment the FSP configuration is a mix of hard coded values
> >and
> >> devicetree properties.
> >> This patch makes FSP-M and FSP-S full configurable from devicetree
> >by
> >> adding binding properties for all FSP parameters.
> >>
> >> Co-developed-by: Wolfgang Wallner
> >
> >> Signed-off-by: Wolfgang Wallner
> >
> >> Signed-off-by: Bernhard Messerklinger
> >
> >>
> >> ---
> >>
> >>  arch/x86/cpu/apollolake/Makefile  |1 +
> >>  arch/x86/cpu/apollolake/fsp_bindings.c| 2096
> >+
> >>  arch/x86/cpu/apollolake/fsp_m.c   |  164 +-
> >>  arch/x86/cpu/apollolake/fsp_s.c   |  382 +--
> >>  arch/x86/dts/chromebook_coral.dts |   72 +-
> >>  .../asm/arch-apollolake/fsp/fsp_m_upd.h   |  168 ++
> >>  .../asm/arch-apollolake/fsp/fsp_s_upd.h   |  202 ++
> >>  .../asm/arch-apollolake/fsp_bindings.h|   74 +
> >>  .../fsp/fsp2/apollolake/fsp-m.txt |  320 +++
> >>  .../fsp/fsp2/apollolake/fsp-s.txt |  483 
> >>  10 files changed, 3422 insertions(+), 540 deletions(-)
> >>  create mode 100644 arch/x86/cpu/apollolake/fsp_bindings.c
> >>  create mode 100644
> >arch/x86/include/asm/arch-apollolake/fsp_bindings.h
> >>  create mode 100644
> >doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt
> >>  create mode 100644
> >doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt
> >>
> >
> >Tested on coral:
> >Tested-by: Simon Glass 
> >
> >This looks good to me. I wonder if one day the binding table could be
> >created from the binding .txt file, or compared with it
> >programmatically?
> Yes that's true. But at the moment its just copy paste.
> I generated the binding table from the fsp_s and fsp_m config struct
> with a python script. But this script is also far from being finished.

OK thanks.

> > ...
> >> +#if defined(CONFIG_SPL_BUILD)
> >
> >Do you need these #ifs? I would hope the compiler would only include
> >them if needed.
> Without the #ifs the SPL size stays the same but the u-boot proper size
> increases by about 2 kb.

OK, well then we need the #ifs.

BTW next time you send this you could take off the RFC as I think we
should apply this.

Regards,
Simon

Re: [PATCH] imx8mp_evk: Make SPL binary size smaller

[Fabio Estevam]

On Thu, May 7, 2020 at 8:59 AM Fabio Estevam  wrote:
>
> Commit f24dea4e1b52 ("ARM: imx8m: Fix reset in SPL on NXP iMX8MP EVK") caused
> the u-boot-spl.bin binary size to grow by around 2000 bytes, which makes the
> board to fail to boot.

The new SPL size is 97kB, which is below the limit define in imx8mp_evk.h:
#define CONFIG_SPL_MAX_SIZE (152 * 1024)

It seems the problem is not related to SPL size.

Peng, do you have any ideas?

Thanks

Re: [PATCH 5/8] efi_loader: Make the pkcs7 header parsing function an extern

[Akashi Takahiro]

On Thu, May 07, 2020 at 04:48:30PM +0530, Sughosh Ganu wrote:
> On Thu, 7 May 2020 at 13:04, Akashi Takahiro 
> wrote:
> 
> > On Thu, Apr 30, 2020 at 11:06:27PM +0530, Sughosh Ganu wrote:
> > > The pkcs7 header parsing functionality is pretty generic, and can be
> > > used by other features like capsule authentication. Make the function
> > > as an extern, also changing it's name to efi_parse_pkcs7_header.
> >
> > The patch itself is fine to me, but is "pkcs7 header" a common term?
> >
> 
> I haven't come across it in any other code base. I used it since in the
> concept of a capsule, the signature is prepended to the capsule payload. If
> you can think of a better name, please suggest so. I will change it in the
> next version.

Simply, efi_parse_pkcs7_signature()?

In addition, please update the function description, which still
mentions "variable."

-Takahiro Akashi

> -sughosh
> 
> 
> >
> > -Takahiro Akashi
> >
> > >
> > > Signed-off-by: Sughosh Ganu 
> > > ---
> > >  include/efi_loader.h   |  2 +
> > >  lib/efi_loader/efi_signature.c | 78 
> > >  lib/efi_loader/efi_variable.c  | 82 +-
> > >  3 files changed, 82 insertions(+), 80 deletions(-)
> > >
> > > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > > index b7638d5825..8d923451ce 100644
> > > --- a/include/efi_loader.h
> > > +++ b/include/efi_loader.h
> > > @@ -781,6 +781,8 @@ bool efi_secure_boot_enabled(void);
> > >
> > >  bool efi_image_parse(void *efi, size_t len, struct efi_image_regions
> > **regp,
> > >WIN_CERTIFICATE **auth, size_t *auth_len);
> > > +struct pkcs7_message *efi_parse_pkcs7_header(const void *buf, size_t
> > buflen);
> > > +
> > >  #endif /* CONFIG_EFI_SECURE_BOOT */
> > >
> > >  #ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > diff --git a/lib/efi_loader/efi_signature.c
> > b/lib/efi_loader/efi_signature.c
> > > index bf6f39aab2..9897f5418e 100644
> > > --- a/lib/efi_loader/efi_signature.c
> > > +++ b/lib/efi_loader/efi_signature.c
> > > @@ -25,6 +25,84 @@ const efi_guid_t efi_guid_cert_x509_sha256 =
> > EFI_CERT_X509_SHA256_GUID;
> > >
> > >  #ifdef CONFIG_EFI_SECURE_BOOT
> > >
> > > +static u8 pkcs7_hdr[] = {
> > > + /* SEQUENCE */
> > > + 0x30, 0x82, 0x05, 0xc7,
> > > + /* OID: pkcs7-signedData */
> > > + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x02,
> > > + /* Context Structured? */
> > > + 0xa0, 0x82, 0x05, 0xb8,
> > > +};
> > > +
> > > +/**
> > > + * efi_parse_pkcs7_header - parse a signature in variable
> > > + * @buf: Pointer to the payload's value
> > > + * @buflen:  Length of @buf
> > > + *
> > > + * Parse a signature embedded in variable's value and instantiate
> > > + * a pkcs7_message structure. Since pkcs7_parse_message() accepts only
> > > + * pkcs7's signedData, some header needed be prepended for correctly
> > > + * parsing authentication data, particularly for variable's.
> > > + *
> > > + * Return:   Pointer to pkcs7_message structure on success, NULL on
> > error
> > > + */
> > > +struct pkcs7_message *efi_parse_pkcs7_header(const void *buf, size_t
> > buflen)
> > > +{
> > > + u8 *ebuf;
> > > + size_t ebuflen, len;
> > > + struct pkcs7_message *msg;
> > > +
> > > + /*
> > > +  * This is the best assumption to check if the binary is
> > > +  * already in a form of pkcs7's signedData.
> > > +  */
> > > + if (buflen > sizeof(pkcs7_hdr) &&
> > > + !memcmp(&((u8 *)buf)[4], _hdr[4], 11)) {
> > > + msg = pkcs7_parse_message(buf, buflen);
> > > + goto out;
> > > + }
> > > +
> > > + /*
> > > +  * Otherwise, we should add a dummy prefix sequence for pkcs7
> > > +  * message parser to be able to process.
> > > +  * NOTE: EDK2 also uses similar hack in WrapPkcs7Data()
> > > +  * in CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c
> > > +  * TODO:
> > > +  * The header should be composed in a more refined manner.
> > > +  */
> > > + debug("Makeshift prefix added to authentication data\n");
> > > + ebuflen = sizeof(pkcs7_hdr) + buflen;
> > > + if (ebuflen <= 0x7f) {
> > > + debug("Data is too short\n");
> > > + return NULL;
> > > + }
> > > +
> > > + ebuf = malloc(ebuflen);
> > > + if (!ebuf) {
> > > + debug("Out of memory\n");
> > > + return NULL;
> > > + }
> > > +
> > > + memcpy(ebuf, pkcs7_hdr, sizeof(pkcs7_hdr));
> > > + memcpy(ebuf + sizeof(pkcs7_hdr), buf, buflen);
> > > + len = ebuflen - 4;
> > > + ebuf[2] = (len >> 8) & 0xff;
> > > + ebuf[3] = len & 0xff;
> > > + len = ebuflen - 0x13;
> > > + ebuf[0x11] = (len >> 8) & 0xff;
> > > + ebuf[0x12] = len & 0xff;
> > > +
> > > + msg = pkcs7_parse_message(ebuf, ebuflen);
> > > +
> > > + free(ebuf);
> > > +
> > > +out:
> > > + if (IS_ERR(msg))
> > > + return NULL;
> > > +
> > > + 

Re: [PATCH 6/8] efi: capsule: Add support for uefi capsule authentication

[Akashi Takahiro]

On Thu, May 07, 2020 at 05:20:35PM +0530, Sughosh Ganu wrote:
> On Thu, 7 May 2020 at 13:49, Akashi Takahiro 
> wrote:
> 
> > Sughosh,
> >
> > On Thu, Apr 30, 2020 at 11:06:28PM +0530, Sughosh Ganu wrote:
> > > Add support for authenticating uefi capsules. Most of the signature
> > > verification functionality is shared with the uefi secure boot
> > > feature.
> > >
> > > The root certificate containing the public key used for the signature
> > > verification is stored as an efi variable, similar to the variables
> > > used for uefi secure boot. The root certificate is stored as an efi
> > > signature list(esl) file -- this file contains the x509 certificate
> > > which is the root certificate.
> > >
> > > Signed-off-by: Sughosh Ganu 
> > > ---
> > >  include/efi_api.h  |  17 +
> > >  include/efi_loader.h   |   8 ++-
> > >  lib/efi_loader/Kconfig |  16 +
> > >  lib/efi_loader/efi_capsule.c   | 126 +
> > >  lib/efi_loader/efi_signature.c |   4 +-
> > >  5 files changed, 167 insertions(+), 4 deletions(-)
> > >
> >
> 
> 
> 
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > index ec2976ceba..245deabbed 100644
> > > --- a/lib/efi_loader/Kconfig
> > > +++ b/lib/efi_loader/Kconfig
> > > @@ -110,6 +110,22 @@ config EFI_CAPSULE_FIT_DEVICE
> > >   help
> > > Define storage device, say 1:1, for storing FIT image
> > >
> > > +config EFI_CAPSULE_AUTHENTICATE
> > > + bool "Update Capsule authentication"
> > > + depends on EFI_HAVE_CAPSULE_SUPPORT
> > > + depends on EFI_CAPSULE_ON_DISK
> > > + depends on EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> >
> > Do we need those two configurations?
> >
> 
> Right, I think we can just do with the EFI_CAPSULE_ON_DISK. Will change.

Actually I don't think we need EFI_CAPSULE_ON_DISK neither.

> 
> >
> > > + select SHA256
> > > + select RSA
> > > + select RSA_VERIFY
> > > + select RSA_VERIFY_WITH_PKEY
> > > + select X509_CERTIFICATE_PARSER
> > > + select PKCS7_MESSAGE_PARSER
> > > + default n
> > > + help
> > > +   Select this option if you want to enable capsule
> > > +   authentication
> > > +
> > >  config EFI_DEVICE_PATH_TO_TEXT
> > >   bool "Device path to text protocol"
> > >   default y
> > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > index 931d363edc..a265d36ff0 100644
> > > --- a/lib/efi_loader/efi_capsule.c
> > > +++ b/lib/efi_loader/efi_capsule.c
> > > @@ -12,6 +12,10 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include "../lib/crypto/pkcs7_parser.h"
> > > +
> >
> > As you might notice, the location was changed by
> > my recent patch.
> >
> 
> Will check and update.
> 
> 
> >
> > > +#include 
> > > +#include 
> > >
> > >  const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID;
> > >  static const efi_guid_t efi_guid_firmware_management_capsule_id =
> > > @@ -245,6 +249,128 @@ out:
> > >
> > >   return ret;
> > >  }
> > > +
> > > +#if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE)
> > > +
> > > +const efi_guid_t efi_guid_capsule_root_cert_guid =
> > > + EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> > > +
> > > +__weak u16 *efi_get_truststore_name(void)
> > > +{
> > > + return L"CRT";
> >
> > This variable is not defined by UEFI specification, isn't it?
> > How can this variable be protected?
> >
> 
> This is not part of the uefi specification. The uefi specification does not
> mandate any particular method for storing the root certificate to be used
> for capsule authentication. The edk2 code gets the root certificate through
> a pcd. I had tried using KEK variable for storing the root certificate, and
> had also come up with an implementation. However, the addition/deletion and
> update of the secure variables is very closely tied with the secure boot
> feature and the secure boot state of the system, which is the reason why i
> dropped that solution and used a separate variable, which can be defined by
> the vendor to store the root certificate.

My concern is that, without any protection, the value of this variable
can be modified by a mal-user.
(One simple solution would be to set this variable read-only.)

> 
> >
> > > +}
> > > +
> > > +__weak const efi_guid_t *efi_get_truststore_vendor(void)
> > > +{
> > > +
> > > + return _guid_capsule_root_cert_guid;
> > > +}
> > > +
> > > +/**
> > > + * efi_capsule_authenticate() - Authenticate a uefi capsule
> > > + *
> > > + * @capsule: Capsule file with the authentication
> > > + *   header
> > > + * @capsule_size:Size of the entire capsule
> > > + * @image:   pointer to the image payload minus the
> > > + *   authentication header
> > > + * @image_size:  size of the image payload
> > > + *
> > > + * Authenticate the capsule signature with the public key contained
> > > + * in the root certificate stored as an efi environment variable
> > > 

Re: [PATCH v3 6/7] ARM: dts: rk3399-evb: usb3.0 host support

[Frank Wang]

Hi Peter,

On 2020/5/7 16:17, Peter Robinson wrote:

On Thu, May 7, 2020 at 9:13 AM Frank Wang  wrote:

Configure 'tcphy1' and 'usbdrd_dwc3_1' nodes to support USB3.0 host
for Rockchip RK3399 Evaluation Board.

Signed-off-by: Frank Wang 
---
Changes for v3:
- drop dtsi changes to keep the same with Linux Kernel.
- amend rk3399-evb.dts to support usb3.0 host.

If this is specific to U-Boot it should go in
arch/arm/dts/rk3399-evb-u-boot.dtsi as the rk3399-evb.dts is intended
to be the same as what's in Linux.


Will fix.


  arch/arm/dts/rk3399-evb.dts | 13 +
  1 file changed, 13 insertions(+)

diff --git a/arch/arm/dts/rk3399-evb.dts b/arch/arm/dts/rk3399-evb.dts
index 694b0d08d6..5aa46f4e37 100644
--- a/arch/arm/dts/rk3399-evb.dts
+++ b/arch/arm/dts/rk3399-evb.dts
@@ -417,6 +417,10 @@
 status = "disabled";
  };

+ {
+   status = "okay";
+};
+
   {
 status = "okay";
  };
@@ -439,6 +443,15 @@
 status = "okay";
  };

+_1 {
+   status = "okay";
+};
+
+_dwc3_1 {
+   dr_mode = "host";
+   status = "okay";
+};
+
  _host0_ehci {
 status = "okay";
  };
--
2.17.1

Re: efi_loader: pkcs7_parse_message() returns error pointer

[AKASHI Takahiro]

Heinrich,

On Thu, May 07, 2020 at 04:47:22PM +0200, Heinrich Schuchardt wrote:
> On 07.05.20 02:17, Patrick Wildt wrote:
> > Since pkcs7_parse_message() returns an error pointer, we must not
> > check for NULL.  We have to explicitly set msg to NULL in the error
> > case, otherwise the call to pkcs7_free_message() on the goto err
> > path will assume it's a valid object.
> 
> @Takahiro
> I think we should start documenting the library functions properly. The

Generally I agree, but

> description in lib/crypto/pkcs7_parser.c does not conform to
> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation
> Especially we should describe how errors are returned.

Remember that this file, as well as others under lib/crypto, was
imported from linux kernel source.
I made a minimum set of changes to align it with U-Boot code.
So I'm rather reluctant to modify the file.

Thanks,
-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> >
> > Signed-off-by: Patrick Wildt 
> >
> > diff --git a/lib/efi_loader/efi_image_loader.c 
> > b/lib/efi_loader/efi_image_loader.c
> > index 5a9a6424cc..43a53d3dd1 100644
> > --- a/lib/efi_loader/efi_image_loader.c
> > +++ b/lib/efi_loader/efi_image_loader.c
> > @@ -538,8 +538,9 @@ static bool efi_image_authenticate(void *efi, size_t 
> > efi_size)
> > }
> > msg = pkcs7_parse_message((void *)wincert + sizeof(*wincert),
> >   wincert->dwLength - sizeof(*wincert));
> > -   if (!msg) {
> > +   if (IS_ERR(msg)) {
> > debug("Parsing image's signature failed\n");
> > +   msg = NULL;
> > goto err;
> > }
> >
> >
> 

Re: [PATCH 3/8] qemu: arm64: Add support for efi firmware management protocol routines

[Akashi Takahiro]

On Thu, May 07, 2020 at 10:47:47PM +0200, Heinrich Schuchardt wrote:
> On 5/7/20 4:33 AM, Akashi Takahiro wrote:
> > On Fri, May 01, 2020 at 11:33:42AM +0200, Heinrich Schuchardt wrote:
> >> On 4/30/20 9:13 PM, Sughosh Ganu wrote:
> >>>
> >>> On Fri, 1 May 2020 at 00:09, Heinrich Schuchardt  >>> > wrote:
> >>>
> >>> On 4/30/20 7:36 PM, Sughosh Ganu wrote:
> >>> > Add support for the get_image_info and set_image routines, which are
> >>> > part of the efi firmware management protocol.
> >>> >
> >>> > The current implementation uses the set_image routine for updating 
> >>> the
> >>> > u-boot binary image for the qemu arm64 platform. This is supported
> >>> > using the capsule-on-disk feature of the uefi specification, wherein
> >>> > the firmware image to be updated is placed on the efi system 
> >>> partition
> >>> > as a efi capsule under EFI/UpdateCapsule/ directory. Support has 
> >>> been
> >>> > added for updating the u-boot image on platforms booting with arm
> >>> > trusted firmware(tf-a), where the u-boot image gets booted as the 
> >>> BL33
> >>> > payload(bl33.bin).
> >>> >
> >>> > The feature can be enabled by the following config options
> >>> >
> >>> > CONFIG_EFI_CAPSULE_ON_DISK=y
> >>> > CONFIG_EFI_FIRMWARE_MANAGEMENT_PROTOCOL=y
> >>> >
> >>> > Signed-off-by: Sughosh Ganu  >>> >
> >>>
> >>> U-Boot's UEFI subsystem should work in the same way for x86, ARM, and
> >>> RISC-V. Please, come up with an architecture independent solution.
> >>>
> >>>
> >>> Please check the explanation that I gave in the other mail. If you check
> >>> the patch series, the actual capsule authentication logic has been kept
> >>> architecture agnostic, in efi_capsule.c. The fmp protocol is very much
> >>> intended for allowing platforms to define their firmware update
> >>> routines. Edk2 also has platform specific implementation of the fmp
> >>> protocol under the edk2-platforms directory.
> >>>
> >>> -sughosh
> >>>  
> >>>
> >>
> >> My idea is that for most platforms it will be enough to have a common
> >> FMP implementation that consumes a capsule
> >>
> >> * with one or more binaries
> >
> > Does this assumption apply to most platforms?
> > If so ("one"),
> 
> Raspberry uses a file in the first partition which must be FAT to store
> U-Boot. The file name of U-Boot is indicated in file config.txt to the
> primary boot loader.
> 
> On all other devices I own U-Boot is installed by command 'dd' writing
> to the SD-Card somewhere after the DOS partition table. (When using a
> GUID partition table often you have to shorten it or relocated it to
> after U-Boot.) Some of the devices could alternativley use eMMC for
> U-Boot (e.g. Odroid C2).

"Firmware" doesn't always mean U-Boot binary.
What I had in my mind is that it can be
  - storage for U-Boot environment variables 
  - firmware for other peripherals, or even
  - kernel(/initfs/dtb)
(Remember that FIT format potentially allows for holding them.)
So I believe that it's totally up to systems.

-Takahiro Akashi

> For reference have a look at
> doc/README.rockchip
> https://a-delacruz.github.io/ubuntu/rpi3-setup-64bit-uboot.html
> 
> Best regards
> 
> Heinrich
> 
> >
> >> * a media device path, a start address, and a truncation flag
> >>   for each of the binaries
> >
> > my FIT-based patch[1] meets this assumption and there already
> > are backend drivers for many media (but not for semihosting :)
> > as dfu.
> > (I see little reason to re-invent another set of backend drivers.)
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2020-April/408767.html
> >
> >
> >> The protocol implementation then will write the binaries to the device
> >> paths:
> >>
> >> * to an SD-Card or eMMC exposing the Block IO protocol
> >>   for most devices
> >> * to a file in case of the Raspberry Pi or the Sandbox or QEMU
> >>   (and truncate it if the truncation flag is set)
> >>
> >> If for some devices like a SPI flash we do not have a media device path
> >> yet, then the only platform specific bit would be the block device
> >> driver exposing the media device path.
> >>
> >> Same with a semi-hosted file: just add a driver exposing it as a media
> >> path with an EFI_BLOCK_IO_PROTOCOL.
> >>
> >> For security reasons it may be advisable to make the device read-only
> >> when reaching ExitBootServices() or even better before the first
> >> execution of StartImage(). For this purpose we could use the Reset()
> >> service of the EFI_BLOCK_IO_PROTOCOL or provide a U-Boot specific
> >> service in the EFI_BLOCK_IO_PROTOCOL.
> >>
> >> Best regards
> >>
> >> Heinrich
> 

[PATCH 2/3] mksunxi_fit_atf.sh: Update FIT component descriptions

[Samuel Holland]

Since commit d879616e9e64 ("spl: fit: simplify logic for FDT loading for
non-OS boots"), the SPL looks at the "os" properties of FIT images to
determine where to append the FDT.

The "os" property of the "firmware" image also determines how to execute
the next stage of the boot process, as in 1d3790905d9c ("spl: atf:
introduce spl_invoke_atf and make bl31_entry private").

To support this additional functionality, and to properly model the boot
process, where ATF runs before U-Boot, add the "os" properties and swap
the firmware/loadable images in the FIT image.

Signed-off-by: Samuel Holland 
---
 board/sunxi/mksunxi_fit_atf.sh | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/board/sunxi/mksunxi_fit_atf.sh b/board/sunxi/mksunxi_fit_atf.sh
index 88ad719747..4dfd22db78 100755
--- a/board/sunxi/mksunxi_fit_atf.sh
+++ b/board/sunxi/mksunxi_fit_atf.sh
@@ -31,6 +31,7 @@ cat << __HEADER_EOF
description = "U-Boot (64-bit)";
data = /incbin/("u-boot-nodtb.bin");
type = "standalone";
+   os = "u-boot";
arch = "arm64";
compression = "none";
load = <0x4a00>;
@@ -39,6 +40,7 @@ cat << __HEADER_EOF
description = "ARM Trusted Firmware";
data = /incbin/("$BL31");
type = "firmware";
+   os = "arm-trusted-firmware";
arch = "arm64";
compression = "none";
load = <$BL31_ADDR>;
@@ -73,8 +75,8 @@ do
cat << __CONF_SECTION_EOF
config_$cnt {
description = "$(basename $dtname .dtb)";
-   firmware = "uboot";
-   loadables = "atf";
+   firmware = "atf";
+   loadables = "uboot";
fdt = "fdt_$cnt";
};
 __CONF_SECTION_EOF
-- 
2.24.1

[PATCH] ARC: DTS: cleanup USB node names

[Eugeniy Paltsev]

Remove redundant '0x' from node names.

Signed-off-by: Eugeniy Paltsev 
---
 arch/arc/dts/axs10x_mb.dtsi   | 4 ++--
 arch/arc/dts/hsdk-common.dtsi | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arc/dts/axs10x_mb.dtsi b/arch/arc/dts/axs10x_mb.dtsi
index 5b77642b8d7..33b05934387 100644
--- a/arch/arc/dts/axs10x_mb.dtsi
+++ b/arch/arc/dts/axs10x_mb.dtsi
@@ -62,12 +62,12 @@
max-speed = <100>;
};
 
-   ehci@0x4 {
+   ehci@4 {
compatible = "generic-ehci";
reg = < 0x4 0x100 >;
};
 
-   ohci@0x6 {
+   ohci@6 {
compatible = "generic-ohci";
reg = < 0x6 0x100 >;
};
diff --git a/arch/arc/dts/hsdk-common.dtsi b/arch/arc/dts/hsdk-common.dtsi
index ea6cc468f83..9aa10e4b25d 100644
--- a/arch/arc/dts/hsdk-common.dtsi
+++ b/arch/arc/dts/hsdk-common.dtsi
@@ -84,7 +84,7 @@
phy-mode = "gmii";
};
 
-   ehci@0xf004 {
+   ehci@f004 {
compatible = "generic-ehci";
reg = <0xf004 0x100>;
 
@@ -96,7 +96,7 @@
resets = <_rst HSDK_USB_RESET>;
};
 
-   ohci@0xf006 {
+   ohci@f006 {
compatible = "generic-ohci";
reg = <0xf006 0x100>;
};
-- 
2.21.3

[PATCH 1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY

[Samuel Holland]

Some boards, specifically 64-bit Allwinner boards (sun50i), are
extremely limited on SPL size. One strategy that was used to make space
was to remove the FIT "os" property parsing code, because it uses a
rather large lookup table.

However, this forces the legacy FIT parsing code path, which requires
the "firmware" entry in the FIT to reference the U-Boot binary, even if
U-Boot is not the next binary in the boot sequence (for example, on
sun50i boards, ATF is run first).

This prevents the same FIT image from being used with a SPL with
CONFIG_SPL_FIT_IMAGE_TINY=n and CONFIG_SPL_ATF=y, because the boot
method selection code looks at `spl_image.os`, which is only set from
the "firmware" entry's "os" property.

To be able to use CONFIG_SPL_ATF=y, the "firmware" entry in the FIT
must be ATF, and U-Boot must be a loadable. For this to work, we need to
parse the "os" property just enough to tell U-Boot from other images, so
we can find it in the loadables list to append the FDT, and so we don't
try to append the FDT to ATF (which could clobber adjacent firmware).

So add the minimal code necessary to distinguish U-Boot/non-U-Boot
loadables with CONFIG_SPL_FIT_IMAGE_TINY=y. This adds about 300 bytes,
much less than the 7400 bytes added by CONFIG_SPL_FIT_IMAGE_TINY=n.

Signed-off-by: Samuel Holland 
---
 common/spl/Kconfig   |  4 +---
 common/spl/spl_fit.c | 17 -
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 9feadb5e43..f2fa12354d 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -448,9 +448,7 @@ config SPL_FIT_IMAGE_TINY
  Enable this to reduce the size of the FIT image loading code
  in SPL, if space for the SPL binary is very tight.
 
- This removes the detection of image types (which forces the
- first image to be treated as having a U-Boot style calling
- convention) and skips the recording of each loaded payload
+ This skips the recording of each loaded payload
  (i.e. loadable) into the FDT (modifying the loaded FDT to
  ensure this information is available to the next image
  invoked).
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index c51e4beb1c..b9dd4211aa 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -464,7 +464,22 @@ static int spl_fit_record_loadable(const void *fit, int 
images, int index,
 static int spl_fit_image_get_os(const void *fit, int noffset, uint8_t *os)
 {
 #if CONFIG_IS_ENABLED(FIT_IMAGE_TINY) && !defined(CONFIG_SPL_OS_BOOT)
-   return -ENOTSUPP;
+   const char *name = fdt_getprop(fit, noffset, FIT_OS_PROP, NULL);
+
+   if (!name)
+   return -ENOENT;
+
+   /*
+* We don't care what the type of the image actually is,
+* only whether or not it is U-Boot. This saves some
+* space by omitting the large table of OS types.
+*/
+   if (!strcmp(name, "u-boot"))
+   *os = IH_OS_U_BOOT;
+   else
+   *os = IH_OS_INVALID;
+
+   return 0;
 #else
return fit_image_get_os(fit, noffset, os);
 #endif
-- 
2.24.1

[PATCH 3/3] sunxi: Add support for including SCP firmware

[Samuel Holland]

Allwinner sun50i SoCs contain an OpenRISC 1000 CPU that functions as a
System Control Processor, or SCP. ARM Trusted Firmware (ATF)
communicates with the SCP over SCPI to implement the PSCI system suspend
and shutdown functionality. Currently, SCP firmware is optional; the
system will boot and run without it, but system suspend will be
unavailable.

Since all communication with the SCP is mediated by ATF, the only thing
U-Boot needs to do is load the firmware into SRAM. The SCP firmware
occupies the last 16KiB of SRAM A2, immediately following ATF.

Signed-off-by: Samuel Holland 
---
 board/sunxi/README.sunxi64 | 43 --
 board/sunxi/mksunxi_fit_atf.sh | 23 +++---
 2 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/board/sunxi/README.sunxi64 b/board/sunxi/README.sunxi64
index 258921af22..9a67e5301e 100644
--- a/board/sunxi/README.sunxi64
+++ b/board/sunxi/README.sunxi64
@@ -14,8 +14,12 @@ Quick Start / Overview
 - Build the ARM Trusted Firmware binary (see "ARM Trusted Firmware (ATF)" 
below)
   $ cd /src/arm-trusted-firmware
   $ make PLAT=sun50i_a64 DEBUG=1 bl31
+- Build the SCP firmware binary (see "SCP firmware (Crust)" below)
+  $ cd /src/crust
+  $ make pine64_plus_defconfig && make -j5 scp
 - Build U-Boot (see "SPL/U-Boot" below)
   $ export BL31=/path/to/bl31.bin
+  $ export SCP=/src/crust/build/scp/scp.bin
   $ make pine64_plus_defconfig && make -j5
 - Transfer to an uSD card (see "microSD card" below)
   $ dd if=u-boot-sunxi-with-spl.bin of=/dev/sdx bs=8k seek=1
@@ -24,13 +28,17 @@ Quick Start / Overview
 Building the firmware
 =
 
-The Allwinner A64/H5 firmware consists of three parts: U-Boot's SPL, an
-ARM Trusted Firmware (ATF) build and the U-Boot proper.
-The SPL will load both ATF and U-Boot proper along with the right device
-tree blob (.dtb) and will pass execution to ATF (in EL3), which in turn will
-drop into the U-Boot proper (in EL2).
-As the ATF binary will become part of the U-Boot image file, you will need
-to build it first.
+The Allwinner A64/H5/H6 firmware consists of several parts: U-Boot's SPL,
+ARM Trusted Firmware (ATF), optional System Control Processor (SCP) firmware
+(e.g. Crust), and the U-Boot proper.
+
+The SPL will load all of the other firmware binaries into RAM, along with the
+right device tree blob (.dtb), and will pass execution to ATF (in EL3). If SCP
+firmware was loaded, ATF will power on the SCP and wait for it to boot.
+ATF will then drop into U-Boot proper (in EL2).
+
+As the ATF binary and SCP firmware will become part of the U-Boot image file,
+you will need to build them first.
 
  ARM Trusted Firmware (ATF)
 
@@ -53,6 +61,27 @@ As sometimes the ATF build process is a bit picky about the 
toolchain used,
 or if you can't be bothered with building ATF, there are known working
 binaries in the firmware repository[3], purely for convenience reasons.
 
+ SCP firmware (Crust)
+--
+SCP firmware is responsible for implementing system suspend/resume, and (on
+boards without a PMIC) soft poweroff/on. ATF contains fallback code for CPU
+power control, so SCP firmware is optional if you don't need either of these
+features. It runs on the AR100, with is an or1k CPU, not ARM, so it needs a
+different cross toolchain.
+
+There is one SCP firmware implementation currently available, Crust:
+$ git clone https://github.com/crust-firmware/crust
+$ cd crust
+$ export CROSS_COMPILE=or1k-linux-musl-
+$ make pine64_plus_defconfig
+$ make scp
+
+The same configuration generally works on any board with the same SoC (A64, H5,
+or H6), so if there is no config for your board, use one for a similar board.
+
+Like for ATF, U-Boot finds the SCP firmware binary via an environment variable:
+$ export SCP=/src/crust/build/scp/scp.bin
+
  SPL/U-Boot
 
 Both U-Boot proper and the SPL are using the 64-bit mode. As the boot ROM
diff --git a/board/sunxi/mksunxi_fit_atf.sh b/board/sunxi/mksunxi_fit_atf.sh
index 4dfd22db78..07a2e619ee 100755
--- a/board/sunxi/mksunxi_fit_atf.sh
+++ b/board/sunxi/mksunxi_fit_atf.sh
@@ -1,11 +1,12 @@
 #!/bin/sh
 #
-# script to generate FIT image source for 64-bit sunxi boards with
-# ARM Trusted Firmware and multiple device trees (given on the command line)
+# script to generate FIT image source for 64-bit sunxi boards with ARM Trusted
+# Firmware, SCP firmware, and multiple device trees (given on the command line)
 #
 # usage: $0  [ [&2
@@ -13,10 +14,18 @@ if [ ! -f $BL31 ]; then
BL31=/dev/null
 fi
 
+if [ ! -f $SCP ]; then
+   echo "WARNING: SCP firmware file $SCP NOT found, system suspend will be 
unavailable" >&2
+   echo "Please read the section on SCP firmware in 
board/sunxi/README.sunxi64" >&2
+   SCP=/dev/null
+fi
+
 if grep -q "^CONFIG_MACH_SUN50I_H6=y" .config; then
BL31_ADDR=0x104000
+   SCP_ADDR=0x114000
 else
BL31_ADDR=0x44000
+   SCP_ADDR=0x5
 fi
 
 cat << 

Re: efi_loader: efi_variable_parse_signature() returns NULL on error

[Patrick Wildt]

On Thu, May 07, 2020 at 03:53:29PM +0200, Heinrich Schuchardt wrote:
> On 07.05.20 09:15, AKASHI Takahiro wrote:
> > On Thu, May 07, 2020 at 02:13:18AM +0200, Patrick Wildt wrote:
> >> efi_variable_parse_signature() returns NULL on error, so IS_NULL()
> 
> IS_NULL() seems to be a typo. I will replace it by IS_ERR() when merging.

Hi,

Yes, sorry.

> >> is an incorrect check.  The goto err leads to pkcs7_free_message(),
> >> which works fine on a NULL ptr.
> >>
> >> Signed-off-by: Patrick Wildt 
> >
> > Reviewed-by: AKASHI Takahiro 
> 
> Hello Takahiro,
> 
> there is a second instance of the same error:
> 
> lib/efi_loader/efi_variable.c:412:
> 
>     msg = pkcs7_parse_message(ebuf, ebuflen);
>     free(ebuf);
> out:
> if (IS_ERR(msg))
> return NULL;
> 
> Please, send a patch for that one too.
> 
> Best regards
> 
> Heinrich

No, that is ok.  pkcs7_parse_message() returns an ERR_PTR() on failure,
as you saw in my other diff.  That's why IS_ERR(msg) is fine here, and
we can return NULL.

Best regards,
Patrick

> >
> >>
> >> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> >> index 58f8fae358..c5fe896de2 100644
> >> --- a/lib/efi_loader/efi_variable.c
> >> +++ b/lib/efi_loader/efi_variable.c
> >> @@ -519,9 +519,8 @@ static efi_status_t efi_variable_authenticate(u16 
> >> *variable,
> >>var_sig = efi_variable_parse_signature(auth->auth_info.cert_data,
> >>   auth->auth_info.hdr.dwLength
> >>   - sizeof(auth->auth_info));
> >> -  if (IS_ERR(var_sig)) {
> >> +  if (!var_sig) {
> >>debug("Parsing variable's signature failed\n");
> >> -  var_sig = NULL;
> >>goto err;
> >>}
> >>
> 

Re: Setting up test.py for a platform with 2 U-Boots?

[Stephen Warren]

On 5/7/20 12:48 PM, Tom Rini wrote:
> Hey,
> 
> So I'm trying to enable our test.py framework on am65x_evm_r5 +
> am65x_evm_a53.  The short version is this platform has an R5 core that
> sets things up and fires off the A53 cores.  So there's two U-Boots and
> the console log looks like this (I used SOURCE_DATE_EPOCH to give both
> binaries the same timestamp):
...
> And that's even with:
> env__spl_skipped = True
> in u_boot_boardenv_am65x_evm_a53_na.py for the platform.  Any ideas on what to
> do here?  I even tried turning off serial support in the R5 side of things, 
> but
> it still failed.  Thanks!

It's odd that disabling serial support on the R5 didn't fix this; are
you sure that patch actually prevented the serial output from appearing,
and the board still booted without issue?

Anyway, u_boot_console_base.py:ConsoleBase:ensure_spawned() does roughly
this:

if SPL will print signon message:
Wait for SPL signon message
Wait for main U-Boot signon message

Maybe we need to expand that to a loop that iterates over a list of
signon messages, with the default list value being either [spl, main] or
[main] as configured by the current logic, but if the env file provides
an alternate list, that's used instead, e.g. [spl, main, spl, main] (or
whatever)?

e.g. very roughly:

msgs = self.config.env.get('env__boot_signon_msgs')
if msgs:
signons = parse(msgs)
else if spl:
signons = [spl, main]
else:
signons = [main]
for signon in signons:
self.p.expect(signon)

Re: [PATCH 1/1] test: stabilize test_efi_secboot

[AKASHI Takahiro]

On Thu, May 07, 2020 at 11:14:17PM +0200, Heinrich Schuchardt wrote:
> On 5/7/20 2:36 AM, AKASHI Takahiro wrote:
> > Heinrich,
> >
> > On Mon, May 04, 2020 at 12:33:26PM +0200, Heinrich Schuchardt wrote:
> >> When setting up the console via function efi_console_register() we call
> >> query_console_serial(). This functions sends an escape sequence to the
> >> terminal to query the display size. The response is another escape
> >> sequence.
> >>
> >> console.run_command_list() is looking for a regular expression '^==>'.
> >> If the escape sequence for the screen size precedes the prompt without a
> >> line break, no match is found.
> >>
> >> When efi_disk_register() is called before efi_console_register() this leads
> >> to a test failuere of the UEFI secure boot tests.
> >
> > Why does the order of those calls affect test results?
> 
> Please, have a look at function query_console_serial() and at
> run_command_list().
> 
> As described above:
> '\e7\e[r\e[999;999H\e[6n\e8==>' cannot be matched by regular expression
> '^==>'.

(Probably) right, but what I don't get here is why efi_disk_register()
have an impact on efi_console_register(). The former function has
nothing to do with any console behaviors.

Moreover, I wonder
- why you want to move efi_console_register() after efi_disk_register().
- why python script can see such escape sequences.

> 
> >
> >> We can avoid the problem if the first UEFI command passed to
> >> u_boot_console.run_command_list() produces output. This patch achieves this
> >> by appending '; echo' to the first UEFI related command of the problematic
> >> tests.
> >
> > It looks to be a workaround.
> > We'd better have another approach to fix the problem.
> > Otherwise, similar issues will occur again.
> 
> I would not like to change the logic in Python to detect the U-Boot
> prompt for something UEFI specific. And we need a method to determine
> the size of a serial console.
> 
> The current approach allowed me to merge your patch series which
> otherwise might not have reached the v2020.07 release. Did the problem
> not show up in your Travis CI testing?

No. If your saying is correct, this can happen only if efi_console_register()
is moved after efi_disk_register(). Right?

> If you have a better solution, we can easily merge your patch.

First, I want to understand what's happening.

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> >
> > Thanks,
> > -Takahiro Akashi
> >
> >> Signed-off-by: Heinrich Schuchardt 
> >> ---
> >>  test/py/tests/test_efi_secboot/test_authvar.py  | 8 
> >>  test/py/tests/test_efi_secboot/test_signed.py   | 4 ++--
> >>  test/py/tests/test_efi_secboot/test_unsigned.py | 6 +++---
> >>  3 files changed, 9 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/test/py/tests/test_efi_secboot/test_authvar.py 
> >> b/test/py/tests/test_efi_secboot/test_authvar.py
> >> index 55dcaa95f1..9912694a3e 100644
> >> --- a/test/py/tests/test_efi_secboot/test_authvar.py
> >> +++ b/test/py/tests/test_efi_secboot/test_authvar.py
> >> @@ -133,7 +133,7 @@ class TestEfiAuthVar(object):
> >>  output = u_boot_console.run_command_list([
> >>  'host bind 0 %s' % disk_img,
> >>  'fatload host 0:1 400 PK.auth',
> >> -'setenv -e -nv -bs -rt -at -i 400,$filesize PK',
> >> +'setenv -e -nv -bs -rt -at -i 400,$filesize PK; echo',
> >>  'fatload host 0:1 400 KEK.auth',
> >>  'setenv -e -nv -bs -rt -at -i 400,$filesize KEK',
> >>  'fatload host 0:1 400 db.auth',
> >> @@ -174,7 +174,7 @@ class TestEfiAuthVar(object):
> >>  output = u_boot_console.run_command_list([
> >>  'host bind 0 %s' % disk_img,
> >>  'fatload host 0:1 400 PK.auth',
> >> -'setenv -e -nv -bs -rt -at -i 400,$filesize PK',
> >> +'setenv -e -nv -bs -rt -at -i 400,$filesize PK; echo',
> >>  'fatload host 0:1 400 KEK.auth',
> >>  'setenv -e -nv -bs -rt -at -i 400,$filesize KEK',
> >>  'fatload host 0:1 400 db.auth',
> >> @@ -215,7 +215,7 @@ class TestEfiAuthVar(object):
> >>  output = u_boot_console.run_command_list([
> >>  'host bind 0 %s' % disk_img,
> >>  'fatload host 0:1 400 PK.auth',
> >> -'setenv -e -nv -bs -rt -at -i 400,$filesize PK',
> >> +'setenv -e -nv -bs -rt -at -i 400,$filesize PK; echo',
> >>  'fatload host 0:1 400 KEK.auth',
> >>  'setenv -e -nv -bs -rt -at -i 400,$filesize KEK',
> >>  'fatload host 0:1 400 db.auth',
> >> @@ -249,7 +249,7 @@ class TestEfiAuthVar(object):
> >>  output = u_boot_console.run_command_list([
> >>  'host bind 0 %s' % disk_img,
> >>  'fatload host 0:1 400 PK.auth',
> >> -

[PATCH 3/3] sunxi: H6: Enable Ethernet on the Pine H64

[Samuel Holland]

Now that the EMAC driver supports the H6 SoC, we can enable the Ethernet
hardware on the Pine H64 board.

Signed-off-by: Samuel Holland 
---
 configs/pine_h64_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/pine_h64_defconfig b/configs/pine_h64_defconfig
index 8937c51bd0..87871fd19f 100644
--- a/configs/pine_h64_defconfig
+++ b/configs/pine_h64_defconfig
@@ -10,5 +10,6 @@ CONFIG_SPL_SPI_SUNXI=y
 # CONFIG_PSCI_RESET is not set
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
 CONFIG_DEFAULT_DEVICE_TREE="sun50i-h6-pine-h64"
+CONFIG_SUN8I_EMAC=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_OHCI_HCD=y
-- 
2.24.1

[PATCH 1/3] net: sun8i_emac: Use consistent clock bitfield definitions

[Samuel Holland]

While the R40 uses a different register for EMAC clock configuration
than other chips, the register has a very similar layout. Reuse the
existing bitfield definitions in this file, since they match.

This allows the driver to compile on the H6 platform, where the
CCM_GMAC_CTRL definitions are not present.

Signed-off-by: Samuel Holland 
---
 drivers/net/sun8i_emac.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index 1ae776b446..dcd18833a2 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -296,9 +296,9 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata 
*pdata,
if (priv->variant == R40_GMAC) {
/* Select RGMII for R40 */
reg = readl(priv->sysctl_reg + 0x164);
-   reg |= CCM_GMAC_CTRL_TX_CLK_SRC_INT_RGMII |
-  CCM_GMAC_CTRL_GPIT_RGMII |
-  CCM_GMAC_CTRL_TX_CLK_DELAY(CONFIG_GMAC_TX_DELAY);
+   reg |= SC_ETCS_INT_GMII |
+  SC_EPIT |
+  (CONFIG_GMAC_TX_DELAY << SC_ETXDC_OFFSET);
 
writel(reg, priv->sysctl_reg + 0x164);
return 0;
-- 
2.24.1

[PATCH 2/3] net: sun8i_emac: Add support for the H6 variant

[Samuel Holland]

The H6 EMAC is very similar to the H3 variant, except that it uses the
same pinmux as R40. Add support for it.

Signed-off-by: Samuel Holland 
---
 drivers/net/sun8i_emac.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index dcd18833a2..7ad5070b44 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -107,6 +107,7 @@ enum emac_variant {
H3_EMAC,
A64_EMAC,
R40_GMAC,
+   H6_EMAC,
 };
 
 struct emac_dma_desc {
@@ -306,14 +307,16 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata 
*pdata,
 
reg = readl(priv->sysctl_reg + 0x30);
 
-   if (priv->variant == H3_EMAC) {
+   if (priv->variant == H3_EMAC || priv->variant == H6_EMAC) {
ret = sun8i_emac_set_syscon_ephy(priv, );
if (ret)
return ret;
}
 
reg &= ~(SC_ETCS_MASK | SC_EPIT);
-   if (priv->variant == H3_EMAC || priv->variant == A64_EMAC)
+   if (priv->variant == H3_EMAC ||
+   priv->variant == A64_EMAC ||
+   priv->variant == H6_EMAC)
reg &= ~SC_RMII_EN;
 
switch (priv->interface) {
@@ -325,7 +328,8 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata 
*pdata,
break;
case PHY_INTERFACE_MODE_RMII:
if (priv->variant == H3_EMAC ||
-   priv->variant == A64_EMAC) {
+   priv->variant == A64_EMAC ||
+   priv->variant == H6_EMAC) {
reg |= SC_RMII_EN | SC_ETCS_EXT_GMII;
break;
}
@@ -531,7 +535,7 @@ static int parse_phy_pins(struct udevice *dev)
 
if (priv->variant == H3_EMAC)
sunxi_gpio_set_cfgpin(pin, SUN8I_IOMUX_H3);
-   else if (priv->variant == R40_GMAC)
+   else if (priv->variant == R40_GMAC || priv->variant == H6_EMAC)
sunxi_gpio_set_cfgpin(pin, SUN8I_IOMUX_R40);
else
sunxi_gpio_set_cfgpin(pin, SUN8I_IOMUX);
@@ -1028,6 +1032,8 @@ static const struct udevice_id sun8i_emac_eth_ids[] = {
.data = (uintptr_t)A83T_EMAC },
{.compatible = "allwinner,sun8i-r40-gmac",
.data = (uintptr_t)R40_GMAC },
+   {.compatible = "allwinner,sun50i-h6-emac",
+   .data = (uintptr_t)H6_EMAC },
{ }
 };
 
-- 
2.24.1

[PATCH] spl: Always define preloader_console_init

[Samuel Holland]

A large number of boards call preloader_console_init unconditionally.
Currently, they fail to build with CONFIG_SPL_SERIAL=n, because the
function is undefined in that case. To fix the build, always define
preloader_console_init, but make it no-op when CONFIG_SPL_SERIAL=n.

For the few boards that did check for CONFIG_SPL_SERIAL before calling
preloader_console_init, remove the checks, since the function can now
be called unconditionally.

Signed-off-by: Samuel Holland 
---
 arch/arm/mach-omap2/boot-common.c   | 3 +--
 arch/arm/mach-uniphier/spl_board_init.c | 2 --
 common/spl/spl.c| 7 +++
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/boot-common.c 
b/arch/arm/mach-omap2/boot-common.c
index 7538523724..d57382aabe 100644
--- a/arch/arm/mach-omap2/boot-common.c
+++ b/arch/arm/mach-omap2/boot-common.c
@@ -194,10 +194,9 @@ u32 spl_mmc_boot_mode(const u32 boot_device)
 
 void spl_board_init(void)
 {
-#ifdef CONFIG_SPL_SERIAL_SUPPORT
/* Prepare console output */
preloader_console_init();
-#endif
+
 #if defined(CONFIG_SPL_NAND_SUPPORT) || defined(CONFIG_SPL_ONENAND_SUPPORT)
gpmc_init();
 #endif
diff --git a/arch/arm/mach-uniphier/spl_board_init.c 
b/arch/arm/mach-uniphier/spl_board_init.c
index c7262d70a5..48764a1870 100644
--- a/arch/arm/mach-uniphier/spl_board_init.c
+++ b/arch/arm/mach-uniphier/spl_board_init.c
@@ -112,9 +112,7 @@ void spl_board_init(void)
 
initdata->early_clk_init();
 
-#ifdef CONFIG_SPL_SERIAL_SUPPORT
preloader_console_init();
-#endif
 
ret = initdata->dpll_init(bd);
if (ret) {
diff --git a/common/spl/spl.c b/common/spl/spl.c
index b0f0e1557b..fc5cbbbeba 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -574,8 +574,7 @@ void board_init_f(ulong dummy)
}
}
 
-   if (CONFIG_IS_ENABLED(SERIAL_SUPPORT))
-   preloader_console_init();
+   preloader_console_init();
 }
 #endif
 
@@ -724,13 +723,13 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
jump_to_image_no_args(_image);
 }
 
-#ifdef CONFIG_SPL_SERIAL_SUPPORT
 /*
  * This requires UART clocks to be enabled.  In order for this to work the
  * caller must ensure that the gd pointer is valid.
  */
 void preloader_console_init(void)
 {
+#ifdef CONFIG_SPL_SERIAL_SUPPORT
gd->baudrate = CONFIG_BAUDRATE;
 
serial_init();  /* serial communications setup */
@@ -744,8 +743,8 @@ void preloader_console_init(void)
 #ifdef CONFIG_SPL_DISPLAY_PRINT
spl_display_print();
 #endif
-}
 #endif
+}
 
 /**
  * This function is called before the stack is changed from initial stack to
-- 
2.24.1

[PATCH] sunxi: Silence warning about non-static inline function

[Samuel Holland]

When compiling with CONFIG_SPL_SERIAL=n, gcc warns about
mbus_configure_port not being marked as static:

In file included from include/common.h:34,
 from arch/arm/mach-sunxi/dram_sunxi_dw.c:11:
include/log.h:185:4: warning: 'printf' is static but used in inline function 
'mbus_configure_port' which is not static
  185 |printf(pr_fmt(fmt), ##args); \
  |^~
include/log.h:192:2: note: in expansion of macro 'debug_cond'
  192 |  debug_cond(_DEBUG, fmt, ##args)
  |  ^~
arch/arm/mach-sunxi/dram_sunxi_dw.c:100:2: note: in expansion of macro 'debug'
  100 |  debug("MBUS port %d cfg0 %08x cfg1 %08x\n", port, cfg0, cfg1);
  |  ^

Fix this by updating the function accordingly.

Signed-off-by: Samuel Holland 
---
 arch/arm/mach-sunxi/dram_sunxi_dw.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-sunxi/dram_sunxi_dw.c 
b/arch/arm/mach-sunxi/dram_sunxi_dw.c
index 85e7a1874e..f8d4b32e37 100644
--- a/arch/arm/mach-sunxi/dram_sunxi_dw.c
+++ b/arch/arm/mach-sunxi/dram_sunxi_dw.c
@@ -76,15 +76,15 @@ enum {
MBUS_QOS_HIGHEST
 };
 
-inline void mbus_configure_port(u8 port,
-   bool bwlimit,
-   bool priority,
-   u8 qos, /* MBUS_QOS_LOWEST .. 
MBUS_QOS_HIGEST */
-   u8 waittime,/* 0 .. 0xf */
-   u8 acs, /* 0 .. 0xff */
-   u16 bwl0,   /* 0 .. 0x, bandwidth limit 
in MB/s */
-   u16 bwl1,
-   u16 bwl2)
+static inline void mbus_configure_port(u8 port,
+  bool bwlimit,
+  bool priority,
+  u8 qos, /* MBUS_QOS_LOWEST .. 
MBUS_QOS_HIGEST */
+  u8 waittime,/* 0 .. 0xf */
+  u8 acs, /* 0 .. 0xff */
+  u16 bwl0,   /* 0 .. 0x, 
bandwidth limit in MB/s */
+  u16 bwl1,
+  u16 bwl2)
 {
struct sunxi_mctl_com_reg * const mctl_com =
(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
-- 
2.24.1

Re: wandboard does not boot with current mainline

[Fabio Estevam]

Hi Heiko,

On Thu, May 7, 2020 at 9:12 AM Heiko Schocher  wrote:
>
> Hello Fabio,
>
> I have my wandboard DL in my automated daily build setup example:
>
> http://xeidos.ddns.net/ubtestresults/
>
> and wondered, why my last result is from may 4th ...
>
> (Ok, there is a bug, that I do not see not booting boards in this page,
>   but this is just a proof of concept page ...)
>
>
> So, what I see is:
>
> │   │   ├─UBOOT (wandboard-uboot)
> │   │   │<> Connecting to
> /dev/serial/by-id/usb-Prolific_Technology_Inc._USB-Serial_Controller_D-if00-port0,
>  speed 115200
> │   │   │<>  Escape character: Ctrl-\ (ASCII 28, FS): enabled
> │   │   │<> Type the escape character followed by C to get back,
> │   │   │<> or followed by ? to see other options.
> │   │   │<> 
> │   │   │<>
> │   │   │<> U-Boot SPL 2020.07-rc1-tbot-00298-g425fefa9a3 (May 05 2020 - 
> 04:18:47 +0200)
> │   │   │<> Trying to boot from MMC1
>
>
> and no more output ...
>
> Do you have any idea, what is wrong with current mainline?

Thanks for the report.

This boot failure is caused by:

commit 20a154f95bfe0a3b5bfba90bea7f001c58217536
Author: Marek Vasut 
Date:   Fri May 1 17:40:25 2020 +0200

mkimage: fit: Do not tail-pad fitImage with external data

There is no reason to tail-pad fitImage with external data to 4-bytes,
while fitImage without external data does not have any such padding and
is often unaligned. DT spec also does not mandate any such padding.

Moreover, the tail-pad fills the last few bytes with uninitialized data,
which could lead to a potential information leak.

$ echo -n xy > /tmp/data ; \
./tools/mkimage -E -f auto -d /tmp/data /tmp/fitImage ; \
hexdump -vC /tmp/fitImage | tail -n 3

before:
0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69
|a-offset.data-si|
0270  7a 65 00 00 78 79 64 64   |ze..xydd|
   ^^   ^^ ^^
after:
0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69
|a-offset.data-si|
0270  7a 65 00 78 79|ze.xy|

Signed-off-by: Marek Vasut 
Reviewed-by: Simon Glass 
Cc: Heinrich Schuchardt 
Cc: Tom Rini 

which has been reverted by Tom in commit:

commit 7946a814a31989998120b4b4aa417222ba21b2fa
Author: Tom Rini 
Date:   Wed May 6 11:05:17 2020 -0400

Revert "mkimage: fit: Do not tail-pad fitImage with external data"

This has been reported to break booting of U-Boot from SPL on a number
of platforms due to a lack of alignment of the external data.  The
issues this commit is addressing will need to be resolved another way.

Re-introduce a data leak in the padding for now.

This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.

Reported-by: Alex Kiernan 
Reported-by: Michael Walle 
Tested-by: Jan Kiszka 
Signed-off-by: Tom Rini 

Re: [PATCH 1/1] test: stabilize test_efi_secboot

[Heinrich Schuchardt]

On 5/7/20 2:36 AM, AKASHI Takahiro wrote:
> Heinrich,
>
> On Mon, May 04, 2020 at 12:33:26PM +0200, Heinrich Schuchardt wrote:
>> When setting up the console via function efi_console_register() we call
>> query_console_serial(). This functions sends an escape sequence to the
>> terminal to query the display size. The response is another escape
>> sequence.
>>
>> console.run_command_list() is looking for a regular expression '^==>'.
>> If the escape sequence for the screen size precedes the prompt without a
>> line break, no match is found.
>>
>> When efi_disk_register() is called before efi_console_register() this leads
>> to a test failuere of the UEFI secure boot tests.
>
> Why does the order of those calls affect test results?

Please, have a look at function query_console_serial() and at
run_command_list().

As described above:
'\e7\e[r\e[999;999H\e[6n\e8==>' cannot be matched by regular expression
'^==>'.

>
>> We can avoid the problem if the first UEFI command passed to
>> u_boot_console.run_command_list() produces output. This patch achieves this
>> by appending '; echo' to the first UEFI related command of the problematic
>> tests.
>
> It looks to be a workaround.
> We'd better have another approach to fix the problem.
> Otherwise, similar issues will occur again.

I would not like to change the logic in Python to detect the U-Boot
prompt for something UEFI specific. And we need a method to determine
the size of a serial console.

The current approach allowed me to merge your patch series which
otherwise might not have reached the v2020.07 release. Did the problem
not show up in your Travis CI testing?

If you have a better solution, we can easily merge your patch.

Best regards

Heinrich

>
> Thanks,
> -Takahiro Akashi
>
>> Signed-off-by: Heinrich Schuchardt 
>> ---
>>  test/py/tests/test_efi_secboot/test_authvar.py  | 8 
>>  test/py/tests/test_efi_secboot/test_signed.py   | 4 ++--
>>  test/py/tests/test_efi_secboot/test_unsigned.py | 6 +++---
>>  3 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/test/py/tests/test_efi_secboot/test_authvar.py 
>> b/test/py/tests/test_efi_secboot/test_authvar.py
>> index 55dcaa95f1..9912694a3e 100644
>> --- a/test/py/tests/test_efi_secboot/test_authvar.py
>> +++ b/test/py/tests/test_efi_secboot/test_authvar.py
>> @@ -133,7 +133,7 @@ class TestEfiAuthVar(object):
>>  output = u_boot_console.run_command_list([
>>  'host bind 0 %s' % disk_img,
>>  'fatload host 0:1 400 PK.auth',
>> -'setenv -e -nv -bs -rt -at -i 400,$filesize PK',
>> +'setenv -e -nv -bs -rt -at -i 400,$filesize PK; echo',
>>  'fatload host 0:1 400 KEK.auth',
>>  'setenv -e -nv -bs -rt -at -i 400,$filesize KEK',
>>  'fatload host 0:1 400 db.auth',
>> @@ -174,7 +174,7 @@ class TestEfiAuthVar(object):
>>  output = u_boot_console.run_command_list([
>>  'host bind 0 %s' % disk_img,
>>  'fatload host 0:1 400 PK.auth',
>> -'setenv -e -nv -bs -rt -at -i 400,$filesize PK',
>> +'setenv -e -nv -bs -rt -at -i 400,$filesize PK; echo',
>>  'fatload host 0:1 400 KEK.auth',
>>  'setenv -e -nv -bs -rt -at -i 400,$filesize KEK',
>>  'fatload host 0:1 400 db.auth',
>> @@ -215,7 +215,7 @@ class TestEfiAuthVar(object):
>>  output = u_boot_console.run_command_list([
>>  'host bind 0 %s' % disk_img,
>>  'fatload host 0:1 400 PK.auth',
>> -'setenv -e -nv -bs -rt -at -i 400,$filesize PK',
>> +'setenv -e -nv -bs -rt -at -i 400,$filesize PK; echo',
>>  'fatload host 0:1 400 KEK.auth',
>>  'setenv -e -nv -bs -rt -at -i 400,$filesize KEK',
>>  'fatload host 0:1 400 db.auth',
>> @@ -249,7 +249,7 @@ class TestEfiAuthVar(object):
>>  output = u_boot_console.run_command_list([
>>  'host bind 0 %s' % disk_img,
>>  'fatload host 0:1 400 PK.auth',
>> -'setenv -e -nv -bs -rt -at -i 400,$filesize PK',
>> +'setenv -e -nv -bs -rt -at -i 400,$filesize PK; echo',
>>  'fatload host 0:1 400 KEK.auth',
>>  'setenv -e -nv -bs -rt -at -i 400,$filesize KEK',
>>  'fatload host 0:1 400 db.auth',
>> diff --git a/test/py/tests/test_efi_secboot/test_signed.py 
>> b/test/py/tests/test_efi_secboot/test_signed.py
>> index 584282b338..fc722ab506 100644
>> --- a/test/py/tests/test_efi_secboot/test_signed.py
>> +++ b/test/py/tests/test_efi_secboot/test_signed.py
>> @@ -29,7 +29,7 @@ class TestEfiSignedImage(object):
>>  # Test Case 1a, run signed image if no db/dbx
>>  output = u_boot_console.run_command_list([
>>

Re: [PATCH 8/8] qemu: arm64: Add documentation for capsule update

[Heinrich Schuchardt]

On 5/7/20 4:10 AM, Akashi Takahiro wrote:
> On Fri, May 01, 2020 at 11:17:27AM +0530, Sughosh Ganu wrote:
>> On Fri, 1 May 2020 at 00:57, Tom Rini  wrote:
>>
>>> On Fri, May 01, 2020 at 12:38:45AM +0530, Sughosh Ganu wrote:
 On Fri, 1 May 2020 at 00:07, Heinrich Schuchardt 
>>> wrote:

> On 4/30/20 7:36 PM, Sughosh Ganu wrote:
>> Add documentation highlighting the steps for using the uefi capsule
>> update feature for updating the u-boot firmware image.
>>
>> Signed-off-by: Sughosh Ganu 
>
> UEFI capsule updates should be architecture independent. I would expect
> that the submitted code should work for x86, ARM, and RISC-V. Why does
> this documentation live under the ARM emulation tree?
>

 While the implementation of the core capsule update functionality is
>>> indeed
 architecture agnostic, this series is for implementing the routines of
>>> the
 firmware management protocol, which is very much platform specific -- the
 routines to perform the actual firmware update would be very much tied to
 the platform for which the firmware is being updated. So Takahiro's patch
 series, which adds the core capsule update changes is architecture
 independent, while this series is adding the routines for the firmware
 management protocol, which would be very much platform specific.
>>>
>>> Since we're talking QEMU here, how much of this can be easily dropped in
>>> to QEMU x86_64 and QEMU RISC-V?  If not almost all of it, why?  Can it
>>> be reworked as such?
>>>
>>
>> I don't think it would be too difficult to extend it on other
>> architectures, provided there is some mechanism to access and overwrite the
>> u-boot binary file from the qemu target. It is currently being done using
>> the semihosting interface for the arm architecture. I am not aware if there
>> is an interface like semihosting for accessing the u-boot binary on the
>> other architectures that you mentioned. Will check on this.
>
> Obviously, another choice would be my FIT-based FMP[1]
> as it uses update_tftp(), more specifically dfu_tftp_write(),
> internally.
>
> [1] https://lists.denx.de/pipermail/u-boot/2020-April/408767.html
>
> Thanks,
> -Takahiro Akashi

Please, try to align. We should avoid alternative implementations were
not needed. We could also have a chat on Zoom together.

Best regards

Heinrich

>
>
>> -sughosh

Re: [PATCH 3/8] qemu: arm64: Add support for efi firmware management protocol routines

[Heinrich Schuchardt]

On 5/7/20 4:33 AM, Akashi Takahiro wrote:
> On Fri, May 01, 2020 at 11:33:42AM +0200, Heinrich Schuchardt wrote:
>> On 4/30/20 9:13 PM, Sughosh Ganu wrote:
>>>
>>> On Fri, 1 May 2020 at 00:09, Heinrich Schuchardt >> > wrote:
>>>
>>> On 4/30/20 7:36 PM, Sughosh Ganu wrote:
>>> > Add support for the get_image_info and set_image routines, which are
>>> > part of the efi firmware management protocol.
>>> >
>>> > The current implementation uses the set_image routine for updating the
>>> > u-boot binary image for the qemu arm64 platform. This is supported
>>> > using the capsule-on-disk feature of the uefi specification, wherein
>>> > the firmware image to be updated is placed on the efi system partition
>>> > as a efi capsule under EFI/UpdateCapsule/ directory. Support has been
>>> > added for updating the u-boot image on platforms booting with arm
>>> > trusted firmware(tf-a), where the u-boot image gets booted as the BL33
>>> > payload(bl33.bin).
>>> >
>>> > The feature can be enabled by the following config options
>>> >
>>> > CONFIG_EFI_CAPSULE_ON_DISK=y
>>> > CONFIG_EFI_FIRMWARE_MANAGEMENT_PROTOCOL=y
>>> >
>>> > Signed-off-by: Sughosh Ganu >> >
>>>
>>> U-Boot's UEFI subsystem should work in the same way for x86, ARM, and
>>> RISC-V. Please, come up with an architecture independent solution.
>>>
>>>
>>> Please check the explanation that I gave in the other mail. If you check
>>> the patch series, the actual capsule authentication logic has been kept
>>> architecture agnostic, in efi_capsule.c. The fmp protocol is very much
>>> intended for allowing platforms to define their firmware update
>>> routines. Edk2 also has platform specific implementation of the fmp
>>> protocol under the edk2-platforms directory.
>>>
>>> -sughosh
>>>  
>>>
>>
>> My idea is that for most platforms it will be enough to have a common
>> FMP implementation that consumes a capsule
>>
>> * with one or more binaries
>
> Does this assumption apply to most platforms?
> If so ("one"),

Raspberry uses a file in the first partition which must be FAT to store
U-Boot. The file name of U-Boot is indicated in file config.txt to the
primary boot loader.

On all other devices I own U-Boot is installed by command 'dd' writing
to the SD-Card somewhere after the DOS partition table. (When using a
GUID partition table often you have to shorten it or relocated it to
after U-Boot.) Some of the devices could alternativley use eMMC for
U-Boot (e.g. Odroid C2).

For reference have a look at
doc/README.rockchip
https://a-delacruz.github.io/ubuntu/rpi3-setup-64bit-uboot.html

Best regards

Heinrich

>
>> * a media device path, a start address, and a truncation flag
>>   for each of the binaries
>
> my FIT-based patch[1] meets this assumption and there already
> are backend drivers for many media (but not for semihosting :)
> as dfu.
> (I see little reason to re-invent another set of backend drivers.)
>
> [1] https://lists.denx.de/pipermail/u-boot/2020-April/408767.html
>
>
>> The protocol implementation then will write the binaries to the device
>> paths:
>>
>> * to an SD-Card or eMMC exposing the Block IO protocol
>>   for most devices
>> * to a file in case of the Raspberry Pi or the Sandbox or QEMU
>>   (and truncate it if the truncation flag is set)
>>
>> If for some devices like a SPI flash we do not have a media device path
>> yet, then the only platform specific bit would be the block device
>> driver exposing the media device path.
>>
>> Same with a semi-hosted file: just add a driver exposing it as a media
>> path with an EFI_BLOCK_IO_PROTOCOL.
>>
>> For security reasons it may be advisable to make the device read-only
>> when reaching ExitBootServices() or even better before the first
>> execution of StartImage(). For this purpose we could use the Reset()
>> service of the EFI_BLOCK_IO_PROTOCOL or provide a U-Boot specific
>> service in the EFI_BLOCK_IO_PROTOCOL.
>>
>> Best regards
>>
>> Heinrich

Re: [PATCH V2] mkimage: fit: Do not tail-pad fitImage with external data

[Samuel Holland]

On 5/6/20 12:02 PM, trini at konsulko.com (Tom Rini) wrote:
> I'm not sure that it is.  Can we easily/safely memmove the data to be
> aligned?  Is that really a better option in this case than ensuring
> alignment within the file?

 Can't we use the new mkimage -B option to enforce the alignment IFF and
 only IFF it is required ? 
>>>
>>> Perhaps.  But..
>>>
 Then we can enforce it separately for 32bit
 and 64bit platforms to 4 and 8 bytes respectively even.
>>>
>>> It's 8 bytes for both.  It's possible that Linux doesn't hard fail if
>>> you only do 4 byte alignment but the documented requirement is 8, for
>>> arm32.
>>
>> With Linux you usually need to move the kernel anyway, no ? It's 2 MiB
>> for arm64 for example.
> 
> For arm64 you have to move it to where text_offset says it needs to be.
> For arm32 the common (always, practically?) case is you're firing off
> the zImage which does what's needed.  But..
> 
>> And what you usually parse in-place would be the DT then.
> 
> Yes, the practical case is that it's a DT and that needs 8 byte
> alignment.  And we should just get back to aligning that correctly.
> Going back to the v1 thread, it turns out the answer to "why do we even
> have this padding?" is "we need the DT to be aligned".

This change broke SPL booting for me on MACH_SUN50I as well. One thing that I
haven't seen brought up yet is that SPL FIT code assumes exactly a 4-byte
alignment of external data after the FIT. In spl_load_simple_fit():

 /*
  * For FIT with external data, figure out where the external images
  * start. This is the base for the data-offset properties in each
  * image.
  */
 size = fdt_totalsize(fit);
 size = (size + 3) & ~3;
 size = board_spl_fit_size_align(size);
 base_offset = (size + 3) & ~3;

And then later on in spl_load_fit_image():

 if (!fit_image_get_data_position(fit, node, )) {
 external_data = true;
 } else if (!fit_image_get_data_offset(fit, node, )) {
 offset += base_offset;
 external_data = true;
 }

In my case, after this change, the FIT binary was 0x453 bytes long. But SPL
rounded it up to 0x454, so the external data offsets were off by one, and the
first byte of every loadable was cut off in RAM:

 Trying to boot from MMC1
 fit read sector 50, sectors=3, dst=49fff980, count=3, size=0x454
 firmware: 'uboot'
 External data: dst=4a00, offset=454, size=81208
 src = 4a54, dest = 4a00
 4a00: 00 00 ea 35 00 00 14 00 00 00 00 00 00 00 00 00

If I remove both "(size + 3) & ~3" lines, I can boot successfully:

 Trying to boot from MMC1

 fit read sector 50, sectors=3, dst=49fff980, count=3, size=0x453
 firmware: 'uboot'

 External data: dst=4a00, offset=453, size=81208
 src = 4a53, dest = 4a00

 4a00: 1f 00 00 ea 35 00 00 14 00 00 00 00 00 00 00 00

In other words, it's not just FDTs that are affected by this change, but _all_
external data loaded from SPL.

So if you change the alignment to anything but 4 (be it 1 or 8 or something
else), you must also update the assumptions made by SPL.

Regards,
Samuel

Re: [PATCH 1/1] efi_loader: fix 'efidebug bootorder'

[Heinrich Schuchardt]

On 5/7/20 8:46 AM, AKASHI Takahiro wrote:
> On Wed, Apr 29, 2020 at 09:23:01PM +0200, Heinrich Schuchardt wrote:
>> * don't copy GUIDs for no reason
>> * shorten print format strings by using variable names
>> * don't use the run-time table to access exported functions
>
> I don't much care about those changes, but my initial intent
> was that any efi-related command be implemented as if it were
> an efi application so that it can be converted with less efforts.

Tom and other maintainers want to minimize the binary size of U-Boot.
And these changes save a few bytes.

Best regards

Heinrich

>
> -Takahiro Akashi
>
>> * check the result of malloc() (fixes Coverity CID 300331)
>>
>> Signed-off-by: Heinrich Schuchardt 
>> ---
>>  cmd/efidebug.c | 47 ++-
>>  1 file changed, 26 insertions(+), 21 deletions(-)
>>
>> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
>> index 53cb3cbfaf..d4030fee64 100644
>> --- a/cmd/efidebug.c
>> +++ b/cmd/efidebug.c
>> @@ -852,8 +852,7 @@ static int do_efi_boot_dump(cmd_tbl_t *cmdtp, int flag,
>>   */
>>  static int show_efi_boot_order(void)
>>  {
>> -efi_guid_t guid;
>> -u16 *bootorder = NULL;
>> +u16 *bootorder;
>>  efi_uintn_t size;
>>  int num, i;
>>  char var_name[9];
>> @@ -864,20 +863,25 @@ static int show_efi_boot_order(void)
>>  size_t label_len16, label_len;
>>  efi_status_t ret;
>>
>> -guid = efi_global_variable_guid;
>>  size = 0;
>> -ret = EFI_CALL(RT->get_variable(L"BootOrder", , NULL, ,
>> -NULL));
>> -if (ret == EFI_BUFFER_TOO_SMALL) {
>> -bootorder = malloc(size);
>> -ret = EFI_CALL(RT->get_variable(L"BootOrder", , NULL,
>> -, bootorder));
>> +ret = EFI_CALL(RT->get_variable(L"BootOrder", _global_variable_guid,
>> +NULL, , NULL));
>> +if (ret != EFI_BUFFER_TOO_SMALL) {
>> +if (ret == EFI_NOT_FOUND) {
>> +printf("BootOrder not defined\n");
>> +return CMD_RET_SUCCESS;
>> +} else {
>> +return CMD_RET_FAILURE;
>> +}
>>  }
>> -if (ret == EFI_NOT_FOUND) {
>> -printf("BootOrder not defined\n");
>> -ret = CMD_RET_SUCCESS;
>> -goto out;
>> -} else if (ret != EFI_SUCCESS) {
>> +bootorder = malloc(size);
>> +if (!bootorder) {
>> +printf("ERROR: Out of memory\n");
>> +return CMD_RET_FAILURE;
>> +}
>> +ret = EFI_CALL(efi_get_variable(L"BootOrder", _global_variable_guid,
>> +NULL, , bootorder));
>> +if (ret != EFI_SUCCESS) {
>>  ret = CMD_RET_FAILURE;
>>  goto out;
>>  }
>> @@ -889,11 +893,11 @@ static int show_efi_boot_order(void)
>>  utf8_utf16_strncpy(, var_name, 9);
>>
>>  size = 0;
>> -ret = EFI_CALL(RT->get_variable(var_name16, , NULL, ,
>> -NULL));
>> +ret = EFI_CALL(efi_get_variable(var_name16,
>> +_global_variable_guid, NULL,
>> +, NULL));
>>  if (ret != EFI_BUFFER_TOO_SMALL) {
>> -printf("%2d: Boot%04X: (not defined)\n",
>> -   i + 1, bootorder[i]);
>> +printf("%2d: %s: (not defined)\n", i + 1, var_name);
>>  continue;
>>  }
>>
>> @@ -902,8 +906,9 @@ static int show_efi_boot_order(void)
>>  ret = CMD_RET_FAILURE;
>>  goto out;
>>  }
>> -ret = EFI_CALL(RT->get_variable(var_name16, , NULL, ,
>> -data));
>> +ret = EFI_CALL(efi_get_variable(var_name16,
>> +_global_variable_guid, NULL,
>> +, data));
>>  if (ret != EFI_SUCCESS) {
>>  free(data);
>>  ret = CMD_RET_FAILURE;
>> @@ -922,7 +927,7 @@ static int show_efi_boot_order(void)
>>  }
>>  p = label;
>>  utf16_utf8_strncpy(, lo.label, label_len16);
>> -printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);
>> +printf("%2d: %s: %s\n", i + 1, var_name, label);
>>  free(label);
>>
>>  free(data);
>> --
>> 2.26.2
>>

[PATCH v2] arm: mvebu: Convert CRS305-1G-4S board to CRS3xx-98DX3236

[Luka Kovacic]

Convert the CRS305-1G-4S board to CRS3xx-98DX3236 to enable easier
implementation of new CRS3xx series boards, based on Marvell Prestera
98DX3236.

Signed-off-by: Luka Kovacic 
Cc: Luka Perkov 
Cc: Jakov Petrina 
---
Changes for v2:
   - Fix the patch failing to merge into the current U-Boot tree

 arch/arm/dts/armada-xp-crs305-1g-4s.dts   | 103 +---
 arch/arm/dts/armada-xp-crs305-1g-4s.dtsi  | 111 ++
 arch/arm/mach-mvebu/Kconfig   |  10 +-
 board/mikrotik/crs305-1g-4s/MAINTAINERS   |   7 --
 .../.gitignore|   0
 board/mikrotik/crs3xx-98dx3236/MAINTAINERS|  11 ++
 .../Makefile  |   2 +-
 .../{crs305-1g-4s => crs3xx-98dx3236}/README  |  12 +-
 .../binary.0  |   0
 .../crs3xx-98dx3236.c}|   0
 .../kwbimage.cfg.in   |   2 +-
 configs/crs305-1g-4s_defconfig|  44 +++
 .../{crs305-1g-4s.h => crs3xx-98dx3236.h} |   6 +-
 13 files changed, 165 insertions(+), 143 deletions(-)
 create mode 100644 arch/arm/dts/armada-xp-crs305-1g-4s.dtsi
 delete mode 100644 board/mikrotik/crs305-1g-4s/MAINTAINERS
 rename board/mikrotik/{crs305-1g-4s => crs3xx-98dx3236}/.gitignore (100%)
 create mode 100644 board/mikrotik/crs3xx-98dx3236/MAINTAINERS
 rename board/mikrotik/{crs305-1g-4s => crs3xx-98dx3236}/Makefile (93%)
 rename board/mikrotik/{crs305-1g-4s => crs3xx-98dx3236}/README (64%)
 rename board/mikrotik/{crs305-1g-4s => crs3xx-98dx3236}/binary.0 (100%)
 rename board/mikrotik/{crs305-1g-4s/crs305-1g-4s.c => 
crs3xx-98dx3236/crs3xx-98dx3236.c} (100%)
 rename board/mikrotik/{crs305-1g-4s => crs3xx-98dx3236}/kwbimage.cfg.in (75%)
 rename include/configs/{crs305-1g-4s.h => crs3xx-98dx3236.h} (87%)

diff --git a/arch/arm/dts/armada-xp-crs305-1g-4s.dts 
b/arch/arm/dts/armada-xp-crs305-1g-4s.dts
index 1116f5c96c..010b83b542 100644
--- a/arch/arm/dts/armada-xp-crs305-1g-4s.dts
+++ b/arch/arm/dts/armada-xp-crs305-1g-4s.dts
@@ -1,110 +1,17 @@
 // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
 /*
- * Device Tree file for CRS305-1G-4S board
+ * Device Tree file for MikroTik CRS305-1G-4S+ board
  *
- * Copyright (C) 2016 Allied Telesis Labs
- *
- * Based on armada-xp-db.dts
- *
- * Note: this Device Tree assumes that the bootloader has remapped the
- * internal registers to 0xf100 (instead of the default
- * 0xd000). The 0xf100 is the default used by the recent,
- * DT-capable, U-Boot bootloaders provided by Marvell. Some earlier
- * boards were delivered with an older version of the bootloader that
- * left internal registers mapped at 0xd000. If you are in this
- * situation, you should either update your bootloader (preferred
- * solution) or the below Device Tree should be adjusted.
+ * Copyright (C) 2020 Sartura Ltd.
+ * Author: Luka Kovacic 
  */
 
-/dts-v1/;
-#include "armada-xp-98dx3236.dtsi"
-#include "armada-xp-crs305-1g-4s-u-boot.dtsi"
+#include "armada-xp-crs305-1g-4s.dtsi"
 
 / {
-   model = "CRS305-1G-4S";
-   compatible = "marvell,armadaxp-98dx3236", "marvell,armadaxp-mv78260", 
"marvell,armadaxp", "marvell,armada-370-xp";
-
-   chosen {
-   stdout-path = "serial0:115200n8";
-   bootargs = "console=ttyS0,115200 earlyprintk";
-   };
-
-   aliases {
-   spi0 = 
-   };
-
-   memory {
-   device_type = "memory";
-   reg = <0 0x 0 0x2000>; /* 512 MB */
-   };
-};
-
- {
-   arm,parity-enable;
-   marvell,ecc-enable;
-};
-
-_bootcs {
-   status = "okay";
-
-   /* Device Bus parameters are required */
-
-   /* Read parameters */
-   devbus,bus-width= <16>;
-   devbus,turn-off-ps  = <6>;
-   devbus,badr-skew-ps = <0>;
-   devbus,acc-first-ps = <124000>;
-   devbus,acc-next-ps  = <248000>;
-   devbus,rd-setup-ps  = <0>;
-   devbus,rd-hold-ps   = <0>;
-
-   /* Write parameters */
-   devbus,sync-enable = <0>;
-   devbus,wr-high-ps  = <6>;
-   devbus,wr-low-ps   = <6>;
-   devbus,ale-wr-ps   = <6>;
-};
-
- {
-   status = "okay";
-};
-
- {
-   status = "okay";
-};
-
- {
-   clock-frequency = <10>;
-   status = "okay";
-};
-
- {
-   status = "okay";
+   model = "MikroTik CRS305-1G-4S+";
 };
 
  {
status = "okay";
-
-   spi-flash@0 {
-   #address-cells = <1>;
-   #size-cells = <1>;
-   compatible = "spi-flash", "jedec,spi-nor";
-   reg = <0>; /* Chip select 0 */
-   spi-max-frequency = <10800>;
-   m25p,fast-read;
-
-   partition@u-boot {
-   reg = <0x 0x0010>;
-   label = "u-boot";
-   };
-   partition@u-boot-env {
-   reg = <0x0010 0x0004>;
-   label = "u-boot-env";

Re: [PATCH 0/2] u-boot support for ODROID-C4

[Beniamino Galvani]

On Wed, May 06, 2020 at 09:59:17AM +0200, Neil Armstrong wrote:
> Hi Beniamino,
> 
> On 05/05/2020 22:22, Beniamino Galvani wrote:
> > Hi,
> > 
> > these two patches add initial u-boot support for Hardkernel ODROID-C4.
> 
> Thanks for the patchset, I already have one in my test tree, by you did beat 
> me
> by sending it to the list !

Oh, I didn't notice, sorry.

> > https://wiki.odroid.com/odroid-c4/odroid-c4
> > 
> > Beniamino Galvani (2):
> >   arm: dts: import ODROID-C4 device tree
> 
> The DT is not yet applied by kevin, thus I'll prefer waiting it to be accepted
> and merged in a stable linux tree to keep DT synced.
> 
> >   boards: amlogic: add ODROID-C4 support
> 
> Please re-use the w400 board instead of adding a new board, there is no need 
> for a new
> one until you'll need to add more functionalities.
> 
> I know meson_generate_serial_ethaddr() is absent from w400, I'll add it 
> shortly.

Okay, I'll wait that the DT gets merged and then will resubmit using w400.

Thanks,
Beniamino

[PATCH] arm: mvebu: Convert CRS305-1G-4S board to CRS3xx-98DX3236

[Luka Kovacic]

Convert the CRS305-1G-4S board to CRS3xx-98DX3236 to enable easier
implementation of new CRS3xx series boards, based on Marvell Prestera
98DX3236.

Signed-off-by: Luka Kovacic 
Cc: Luka Perkov 
Cc: Jakov Petrina 
---
 arch/arm/dts/armada-xp-crs305-1g-4s.dts   | 103 +---
 arch/arm/dts/armada-xp-crs305-1g-4s.dtsi  | 111 ++
 arch/arm/mach-mvebu/Kconfig   |  10 +-
 board/mikrotik/crs305-1g-4s/MAINTAINERS   |   7 --
 .../.gitignore|   0
 board/mikrotik/crs3xx-98dx3236/MAINTAINERS|  11 ++
 .../Makefile  |   2 +-
 .../{crs305-1g-4s => crs3xx-98dx3236}/README  |  12 +-
 .../binary.0  |   0
 .../crs3xx-98dx3236.c}|   0
 .../kwbimage.cfg.in   |   2 +-
 configs/crs305-1g-4s_defconfig|  44 +++
 .../{crs305-1g-4s.h => crs3xx-98dx3236.h} |   6 +-
 13 files changed, 165 insertions(+), 143 deletions(-)
 create mode 100644 arch/arm/dts/armada-xp-crs305-1g-4s.dtsi
 delete mode 100644 board/mikrotik/crs305-1g-4s/MAINTAINERS
 rename board/mikrotik/{crs305-1g-4s => crs3xx-98dx3236}/.gitignore (100%)
 create mode 100644 board/mikrotik/crs3xx-98dx3236/MAINTAINERS
 rename board/mikrotik/{crs305-1g-4s => crs3xx-98dx3236}/Makefile (93%)
 rename board/mikrotik/{crs305-1g-4s => crs3xx-98dx3236}/README (64%)
 rename board/mikrotik/{crs305-1g-4s => crs3xx-98dx3236}/binary.0 (100%)
 rename board/mikrotik/{crs305-1g-4s/crs305-1g-4s.c => 
crs3xx-98dx3236/crs3xx-98dx3236.c} (100%)
 rename board/mikrotik/{crs305-1g-4s => crs3xx-98dx3236}/kwbimage.cfg.in (75%)
 rename include/configs/{crs305-1g-4s.h => crs3xx-98dx3236.h} (87%)

diff --git a/arch/arm/dts/armada-xp-crs305-1g-4s.dts 
b/arch/arm/dts/armada-xp-crs305-1g-4s.dts
index 1116f5c96c..010b83b542 100644
--- a/arch/arm/dts/armada-xp-crs305-1g-4s.dts
+++ b/arch/arm/dts/armada-xp-crs305-1g-4s.dts
@@ -1,110 +1,17 @@
 // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
 /*
- * Device Tree file for CRS305-1G-4S board
+ * Device Tree file for MikroTik CRS305-1G-4S+ board
  *
- * Copyright (C) 2016 Allied Telesis Labs
- *
- * Based on armada-xp-db.dts
- *
- * Note: this Device Tree assumes that the bootloader has remapped the
- * internal registers to 0xf100 (instead of the default
- * 0xd000). The 0xf100 is the default used by the recent,
- * DT-capable, U-Boot bootloaders provided by Marvell. Some earlier
- * boards were delivered with an older version of the bootloader that
- * left internal registers mapped at 0xd000. If you are in this
- * situation, you should either update your bootloader (preferred
- * solution) or the below Device Tree should be adjusted.
+ * Copyright (C) 2020 Sartura Ltd.
+ * Author: Luka Kovacic 
  */
 
-/dts-v1/;
-#include "armada-xp-98dx3236.dtsi"
-#include "armada-xp-crs305-1g-4s-u-boot.dtsi"
+#include "armada-xp-crs305-1g-4s.dtsi"
 
 / {
-   model = "CRS305-1G-4S";
-   compatible = "marvell,armadaxp-98dx3236", "marvell,armadaxp-mv78260", 
"marvell,armadaxp", "marvell,armada-370-xp";
-
-   chosen {
-   stdout-path = "serial0:115200n8";
-   bootargs = "console=ttyS0,115200 earlyprintk";
-   };
-
-   aliases {
-   spi0 = 
-   };
-
-   memory {
-   device_type = "memory";
-   reg = <0 0x 0 0x2000>; /* 512 MB */
-   };
-};
-
- {
-   arm,parity-enable;
-   marvell,ecc-enable;
-};
-
-_bootcs {
-   status = "okay";
-
-   /* Device Bus parameters are required */
-
-   /* Read parameters */
-   devbus,bus-width= <16>;
-   devbus,turn-off-ps  = <6>;
-   devbus,badr-skew-ps = <0>;
-   devbus,acc-first-ps = <124000>;
-   devbus,acc-next-ps  = <248000>;
-   devbus,rd-setup-ps  = <0>;
-   devbus,rd-hold-ps   = <0>;
-
-   /* Write parameters */
-   devbus,sync-enable = <0>;
-   devbus,wr-high-ps  = <6>;
-   devbus,wr-low-ps   = <6>;
-   devbus,ale-wr-ps   = <6>;
-};
-
- {
-   status = "okay";
-};
-
- {
-   status = "okay";
-};
-
- {
-   clock-frequency = <10>;
-   status = "okay";
-};
-
- {
-   status = "okay";
+   model = "MikroTik CRS305-1G-4S+";
 };
 
  {
status = "okay";
-
-   spi-flash@0 {
-   #address-cells = <1>;
-   #size-cells = <1>;
-   compatible = "spi-flash", "jedec,spi-nor";
-   reg = <0>; /* Chip select 0 */
-   spi-max-frequency = <10800>;
-   m25p,fast-read;
-
-   partition@u-boot {
-   reg = <0x 0x0010>;
-   label = "u-boot";
-   };
-   partition@u-boot-env {
-   reg = <0x0010 0x0004>;
-   label = "u-boot-env";
-   };
-   partition@unused {
-   reg 

RE: [PATCH] arm: dtsi: sama5d2: Add uart4 definition to sama5d2.dtsi

[Tiaki Rice]

Eugen,

Ah, I suspect the issue might be might be because I changed the subject inside 
the file but the did not change the filename. I shouldn't have done it that way 
anyway. Regarding windows vs linux, doing a diff between them showed no 
differences aside from the tag at the end describing the git version.

I have amended the commit and attached both the windows git patch I originally 
submitted for reference (0001-dtsi...) and the new patch created under CentOS8 
(0001-arm-dtsi...). If you can't see any problems, should I try emailing the 
patch to the list again?


Regards, Tiaki

-Original Message-
From: eugen.hris...@microchip.com  
Sent: Thursday, 7 May 2020 9:55 PM
To: tiakir...@hotmail.com
Cc: u-boot@lists.denx.de
Subject: Re: [PATCH] arm: dtsi: sama5d2: Add uart4 definition to sama5d2.dtsi

On 07.05.2020 14:32, Tiaki Rice wrote:
> Eugen,
> 
> Thanks for the speedy reply, I did not know about this script! I must have 
> glossed over it in the uboot patch documentation page.
> 
> The commit line is 82 characters which is longer than the preferred max of 
> 75. Oops. Don't think this warning would be stopping the patch going through 
> though...
> 

Hi,

For sure that won't be a problem with patchwork.
however , it looks like your patch is windows-generated ? That might be an 
issue, I do not know. Can you send me your patch as attachment file ?

Thanks

> 
> Here is the output for the patch:
> -
>  $ ./scripts/checkpatch.pl 
> 0001-dtsi-sama5d2-Add-uart4-definition-to-sama5d2.dtsi.patch
>  WARNING: Possible unwrapped commit description (prefer a maximum 75 
> chars per line)
>  #7:
>  This patch adds support for uart4 to the processor level device tree 
> include file.
> 
>  total: 0 errors, 1 warnings, 0 checks, 14 lines checked
> 
>  NOTE: For some of the reported defects, checkpatch may be able to
>mechanically convert to the typical style using --fix or 
> --fix-inplace.
> 
>  0001-dtsi-sama5d2-Add-uart4-definition-to-sama5d2.dtsi.patch has style 
> problems, please review.
> 
>  NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX 
> MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE 
> PREFER_ETHER_ADDR_COPY USLEEP_RANGE
> 
>  NOTE: If any of the errors are false positives, please report
>them to the maintainer, see CHECKPATCH in MAINTAINERS.
> -
> 
> Regards,
> 
> Tiaki Rice
> 
> -Original Message-
> From: eugen.hris...@microchip.com 
> Sent: Thursday, 7 May 2020 8:19 PM
> To: tiakir...@hotmail.com
> Cc: u-boot@lists.denx.de
> Subject: Re: [PATCH] arm: dtsi: sama5d2: Add uart4 definition to 
> sama5d2.dtsi
> 
> On 07.05.2020 12:21, Tiaki Rice wrote:
> 
>> Hello Eugen,
>>
>> I'm new to this whole mailing list patchwork thingy so I suspect I didn't 
>> quite submit the patch correctly, maybe you can point me in the right 
>> direction.
>>
>> What I did:
>> - Made the changes on master and locally commited them
>> - Created a patch using  'git format-patch' and modified the patch file 
>> email body.
>> - Used 'git send-email' to send the patch to u-boot@lists.denx.de and 
>> Cc'd you
>>
>> This opened my Outlook of which I checked to make sure the email was plain 
>> text and then sent it. Any ideas?
>>
> 
> Hi Tiaki,
> 
>   From my perspective you did everything very well. The patch looks sent 
> fine. I would only have one question, did you check your patch with the 
> built-in perl script that automatically checks your patch for formatting ?
> ./scripts/checkpatch.pl --strict 
> 
> About patchwork, I think it may be an issue with patchwork.
> 
> Eugen
> 
> 
>>
>> Thanks in advance,
>>
>> Tiaki Rice
>>
>> -Original Message-
>> From: eugen.hris...@microchip.com 
>> Sent: Thursday, 7 May 2020 7:12 PM
>> To: tiakir...@hotmail.com; u-boot@lists.denx.de
>> Subject: Re: [PATCH] arm: dtsi: sama5d2: Add uart4 definition to 
>> sama5d2.dtsi
>>
>> On 07.05.2020 07:43, Tiaki Rice wrote:
>>> This patch adds support for uart4 to the processor level device tree 
>>> include file.
>>>
>>>
>>> Signed-off-by: Tiaki Rice 
>>> Cc: Eugen Hristev 
>>> ---
>>
>> Hi,
>>
>> Thanks for contributing !
>>
>> I have an issue with this patch missing from patchwork.
>> Anyone has any idea why that happened ?
>>
>> Eugen
>>
>>> arch/arm/dts/sama5d2.dtsi | 8 
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/arm/dts/sama5d2.dtsi b/arch/arm/dts/sama5d2.dtsi 
>>> index 5adc47b906..6fb2cb25f9 100644
>>> --- a/arch/arm/dts/sama5d2.dtsi
>>> +++ b/arch/arm/dts/sama5d2.dtsi
>>> @@ -746,6 +746,14 @@
>>> status = "disabled";
>>> };
>>>
>>> +uart4: serial@fc00c000 {
>>> +compatible = "atmel,at91sam9260-usart"; reg = <0xfc00c000 0x100>; 
>>> +clocks = <_clk>; clock-names = "usart"; status = "disabled"; 
>>> +};
>>> +
>>> i2c1: i2c@fc028000 {
>>> compatible = "atmel,sama5d2-i2c";
>>> reg = <0xfc028000 0x100>;
>>> --
>>> 2.22.0.windows.1
>>>
>>
> 

RE: [PATCH] arm: dtsi: sama5d2: Add uart4 definition to sama5d2.dtsi

[Tiaki Rice]

Eugen,

Thanks for the speedy reply, I did not know about this script! I must have 
glossed over it in the uboot patch documentation page.

The commit line is 82 characters which is longer than the preferred max of 75. 
Oops. Don't think this warning would be stopping the patch going through 
though...


Here is the output for the patch:
-
$ ./scripts/checkpatch.pl 
0001-dtsi-sama5d2-Add-uart4-definition-to-sama5d2.dtsi.patch
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars 
per line)
#7:
This patch adds support for uart4 to the processor level device tree 
include file.

total: 0 errors, 1 warnings, 0 checks, 14 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
  mechanically convert to the typical style using --fix or 
--fix-inplace.

0001-dtsi-sama5d2-Add-uart4-definition-to-sama5d2.dtsi.patch has style 
problems, please review.

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX 
MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE 
PREFER_ETHER_ADDR_COPY USLEEP_RANGE

NOTE: If any of the errors are false positives, please report
  them to the maintainer, see CHECKPATCH in MAINTAINERS.
-

Regards,

Tiaki Rice

-Original Message-
From: eugen.hris...@microchip.com  
Sent: Thursday, 7 May 2020 8:19 PM
To: tiakir...@hotmail.com
Cc: u-boot@lists.denx.de
Subject: Re: [PATCH] arm: dtsi: sama5d2: Add uart4 definition to sama5d2.dtsi

On 07.05.2020 12:21, Tiaki Rice wrote:

> Hello Eugen,
> 
> I'm new to this whole mailing list patchwork thingy so I suspect I didn't 
> quite submit the patch correctly, maybe you can point me in the right 
> direction.
> 
> What I did:
> - Made the changes on master and locally commited them
> - Created a patch using  'git format-patch' and modified the patch file email 
> body.
> - Used 'git send-email' to send the patch to u-boot@lists.denx.de and 
> Cc'd you
> 
> This opened my Outlook of which I checked to make sure the email was plain 
> text and then sent it. Any ideas?
> 

Hi Tiaki,

 From my perspective you did everything very well. The patch looks sent fine. I 
would only have one question, did you check your patch with the built-in perl 
script that automatically checks your patch for formatting ?
./scripts/checkpatch.pl --strict 

About patchwork, I think it may be an issue with patchwork.

Eugen


> 
> Thanks in advance,
> 
> Tiaki Rice
> 
> -Original Message-
> From: eugen.hris...@microchip.com 
> Sent: Thursday, 7 May 2020 7:12 PM
> To: tiakir...@hotmail.com; u-boot@lists.denx.de
> Subject: Re: [PATCH] arm: dtsi: sama5d2: Add uart4 definition to 
> sama5d2.dtsi
> 
> On 07.05.2020 07:43, Tiaki Rice wrote:
>> This patch adds support for uart4 to the processor level device tree include 
>> file.
>>
>>
>> Signed-off-by: Tiaki Rice 
>> Cc: Eugen Hristev 
>> ---
> 
> Hi,
> 
> Thanks for contributing !
> 
> I have an issue with this patch missing from patchwork.
> Anyone has any idea why that happened ?
> 
> Eugen
> 
>>arch/arm/dts/sama5d2.dtsi | 8 
>>1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/arm/dts/sama5d2.dtsi b/arch/arm/dts/sama5d2.dtsi 
>> index 5adc47b906..6fb2cb25f9 100644
>> --- a/arch/arm/dts/sama5d2.dtsi
>> +++ b/arch/arm/dts/sama5d2.dtsi
>> @@ -746,6 +746,14 @@
>>status = "disabled";
>>};
>>
>> +uart4: serial@fc00c000 {
>> +compatible = "atmel,at91sam9260-usart"; reg = <0xfc00c000 0x100>; 
>> +clocks = <_clk>; clock-names = "usart"; status = "disabled"; 
>> +};
>> +
>>i2c1: i2c@fc028000 {
>>compatible = "atmel,sama5d2-i2c";
>>reg = <0xfc028000 0x100>;
>> --
>> 2.22.0.windows.1
>>
> 

Re: [PATCH v4 12/12] phy: atheros: consolidate {ar8031|ar8035}_config()

[Tom Rini]

On Thu, May 07, 2020 at 12:11:59AM +0200, Michael Walle wrote:

> The two functions are now exactly the same, remove one of them.
> 
> Signed-off-by: Michael Walle 
> Acked-by: Joe Hershberger 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v4 11/12] phy: atheros: ar8035: remove static clock config

[Tom Rini]

On Thu, May 07, 2020 at 12:11:58AM +0200, Michael Walle wrote:

> We can configure the clock output in the device tree. Disable the
> hardcoded one in here. This is highly board-specific and should have
> never been enabled in the PHY driver.
> 
> If bisecting shows that this commit breaks your board it probably
> depends on the clock output of your Atheros AR8035 PHY. Please have a
> look at doc/device-tree-bindings/net/phy/atheros.txt. You need to set
> "clk-out-frequency = <12500>" because that value was the hardcoded
> value until this commit.
> 
> Signed-off-by: Michael Walle 
> Acked-by: Joe Hershberger 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v4 10/12] phy: atheros: add device tree bindings and config

[Tom Rini]

On Thu, May 07, 2020 at 12:11:57AM +0200, Michael Walle wrote:

> Add support for configuring the CLK_25M pin as well as the RGMII I/O
> voltage by the device tree.
> 
> By default the AT803x PHYs outputs the 25MHz clock of the XTAL input.
> But this output can also be changed by software to other frequencies.
> This commit introduces a generic way to configure this output.
> 
> Also the PHY supports different RGMII I/O voltages: 1.5V, 1.8V and 2.5V.
> An internal LDO is able to provide 1.5V (default) and 1.8V. The 2.5V
> option needs an external supply voltage. This commit adds support to
> switch the internal LDO to 1.8V.
> 
> Signed-off-by: Michael Walle 
> Acked-by: Joe Hershberger 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v4 08/12] phy: atheros: introduce debug read and write functions

[Tom Rini]

On Thu, May 07, 2020 at 12:11:55AM +0200, Michael Walle wrote:

> Provide functions to read and write the Atheros debug registers.
> 
> Signed-off-by: Michael Walle 
> Acked-by: Joe Hershberger 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v4 06/12] phy: atheros: fix AR8021 PHY ID mask

[Tom Rini]

On Thu, May 07, 2020 at 12:11:53AM +0200, Michael Walle wrote:

> The upper bits are all the OUI.
> 
> Signed-off-by: Michael Walle 
> Acked-by: Joe Hershberger 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v4 05/12] phy: atheros: Clarify the intention of ar8021_config

[Tom Rini]

On Thu, May 07, 2020 at 12:11:52AM +0200, Michael Walle wrote:

> From: Vladimir Oltean 
> 
> Debug register 5 contains TX_CLK DELAY at bit 8 and reserved values at
> the other bit positions, just like the other PHYs in the family do.
> Therefore, it is not necessary to hardcode the reserved values, but
> instead simply follow the read-modify-write procedure from the common
> function.
> 
> Signed-off-by: Vladimir Oltean 
> Acked-by: Joe Hershberger 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v4 09/12] phy: atheros: move delay config to common function

[Tom Rini]

On Thu, May 07, 2020 at 12:11:56AM +0200, Michael Walle wrote:

> Signed-off-by: Michael Walle 
> Acked-by: Joe Hershberger 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v4 07/12] phy: atheros: use defines for PHY IDs

[Tom Rini]

On Thu, May 07, 2020 at 12:11:54AM +0200, Michael Walle wrote:

> Signed-off-by: Michael Walle 
> Acked-by: Joe Hershberger 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v4 02/12] phy: atheros: Use common functions for RGMII internal delays

[Tom Rini]

On Thu, May 07, 2020 at 12:11:49AM +0200, Michael Walle wrote:

> From: Vladimir Oltean 
> 
> Signed-off-by: Vladimir Oltean 
> Acked-by: Joe Hershberger 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v4 03/12] phy: atheros: Clarify the configuration of the CLK_25M output pin

[Tom Rini]

On Thu, May 07, 2020 at 12:11:50AM +0200, Michael Walle wrote:

> From: Vladimir Oltean 
> 
> Also take the opportunity to use the phy_read_mmd and phy_write_mmd
> convenience functions.
> 
> Signed-off-by: Vladimir Oltean 
> Acked-by: Joe Hershberger 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v4 04/12] phy: atheros: Explicitly disable RGMII delays

[Tom Rini]

On Thu, May 07, 2020 at 12:11:51AM +0200, Michael Walle wrote:

> From: Vladimir Oltean 
> 
> To eliminate any doubts about the out-of-reset value of the PHY, that
> the driver previously relied on.
> 
> If bisecting shows that this commit breaks your board you probably have
> a wrong PHY interface mode. You probably want the
> PHY_INTERFACE_MODE_RGMII_RXID or PHY_INTERFACE_MODE_RGMII_ID mode.
> 
> Signed-off-by: Vladimir Oltean 
> Acked-by: Joe Hershberger 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v4 01/12] phy: atheros: Make RGMII Tx delays actually configurable for AR8035

[Tom Rini]

On Thu, May 07, 2020 at 12:11:48AM +0200, Michael Walle wrote:

> From: Vladimir Oltean 
> 
> Delete the extraneous write to debug reg 5 that enables Tx delay
> 
> When the driver was originally introduced in commit "6027384a phylib:
> Add Atheros AR8035 GETH PHY support", the Tx delay was being
> unconditionally enabled.
> 
> Then during "2ec4d10b phy: atheros: add support for RGMII_ID, RGMII_TXID
> and RGMII_RXID", the author did not notice that code for enabling Tx
> delay code was already. Therefore, the if condition for Tx delay has
> always been useless for this PHY since this commit introduced it.
> 
> Prior to this patch, every AR8035 PHY in U-boot had Tx delay enabled.
> After this patch, only those who define the interface as RGMII_TXID or
> RGMII_ID will. This is to be expected, but will nonetheless break the
> setups of those who didn't know they rely on Tx delay implicitly.
> 
> Signed-off-by: Vladimir Oltean 
> Acked-by: Joe Hershberger 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Setting up test.py for a platform with 2 U-Boots?

[Tom Rini]

Hey,

So I'm trying to enable our test.py framework on am65x_evm_r5 +
am65x_evm_a53.  The short version is this platform has an R5 core that
sets things up and fires off the A53 cores.  So there's two U-Boots and
the console log looks like this (I used SOURCE_DATE_EPOCH to give both
binaries the same timestamp):

U-Boot SPL 2020.07-rc1-00386-g8737c65fe4e3 (May 07 2020 - 18:29:35 +)
SYSFW ABI: 2.9 (firmware rev 0x0013 '19.12.2-v2019.12b (Terrific Lla')
Trying to boot from MMC2
Starting ATF on ARM64 core...

NOTICE:  BL31: v2.3():v2.3
NOTICE:  BL31: Built : 11:22:40, Apr 21 2020
INFO:GICv3 without legacy support detected.
INFO:ARM GICv3 driver initialized in EL3
INFO:SYSFW ABI: 2.9 (firmware rev 0x0013 '19.12.2-v2019.12b (Terrific Lla')
INFO:BL31: Initializing runtime services
INFO:BL31: cortex_a53: CPU workaround for 855873 was applied
INFO:BL31: Initializing BL32
I/TC: 
I/TC: OP-TEE version: 3.8.0-267-g8287cbcf (gcc version 9.2.1 20191025 (GNU 
Toolchain for the A-profile Architecture 9.2-2019.12 (arm-9.10))) #1 Tue Apr 21 
05:57:19 UTC 2020 aarch64
I/TC: Initialized
INFO:BL31: Preparing for EL3 exit to normal world
INFO:Entry point address = 0x8008
INFO:SPSR = 0x3c9

U-Boot SPL 2020.07-rc1-00386-g8737c65fe4e3 (May 07 2020 - 18:29:35 +)
SYSFW ABI: 2.9 (firmware rev 0x0013 '19.12.2-v2019.12b (Terrific Lla')
Trying to boot from MMC2


U-Boot 2020.07-rc1-00386-g8737c65fe4e3 (May 07 2020 - 18:29:35 +)

SoC:   AM65x SR 1.0
Model: Texas Instruments AM654 Base Board
Board: AM6-COMPROCEVM rev E3
DRAM:  4 GiB
MMC:   sdhci@4f8: 0, sdhci@04FA: 1
Loading Environment from MMC... *** Warning - bad CRC, using default environment

In:serial
Out:   serial
Err:   serial
Detected: SER-PCIEUSBEVM rev E3
Net:   
Warning: cpsw_nuss@04600 using MAC address from ROM
eth0: cpsw_nuss@04600
Hit any key to stop autoboot:  2  0 
=> 

But when I run test.py:
...
if m != 0:
raise Exception('Bad pattern found on console: ' +
>   self.bad_pattern_ids[m - 1])
E   Exception: Bad pattern found on console: 
spl_signon

test/py/u_boot_console_base.py:365: Exception
-- Captured stdout setup 
--
+u-boot-test-reset am65x_evm_a53 na
picocom v2.2

port is: /dev/serial/by-path/pci-:08:00.0-usb-0:1.4.2.5:1.0-port0
flowcontrol: none
baudrate is: 115200
parity is  : none
databits are   : 8
stopbits are   : 1
escape is  : C-a
local echo is  : no
noinit is  : no
noreset is : no
nolock is  : no
send_cmd is: sz -vv
receive_cmd is : rz -vv -E
imap is:
omap is:
emap is: crcrlf,delbs,

Type [C-a] [C-h] to see available commands

Terminal ready

U-Boot SPL 2020.07-rc1-00386-g8737c65fe4e3 (May 07 2020 - 18:29:35 +)
SYSFW ABI: 2.9 (firmware rev 0x0013 '19.12.2-v2019.12b (Terrific Lla')
 308 deselected, 1 error in 
5.30s =

And that's even with:
env__spl_skipped = True
in u_boot_boardenv_am65x_evm_a53_na.py for the platform.  Any ideas on what to
do here?  I even tried turning off serial support in the R5 side of things, but
it still failed.  Thanks!

-- 
Tom


signature.asc
Description: PGP signature

[PATCH v2] drivers: crypto: mod_exp_sw: Re-add DM_FLAG_PRE_RELOC

[Jan Kiszka]

From: Jan Kiszka 

This driver is safe to use in SPL without relocation. Denying
DM_FLAG_PRE_RELOC prevents its usability for verifying the main U-Boot
or other artifacts from the SPL unless needless enabling the full driver
set (SPL_OF_PLATDATA).

Fixes: 17e117408571 ("drivers: crypto: rsa_mod_exp: avoid DM_FLAG_PRE_RELOC")
CC: Heinrich Schuchardt 
CC: Marek Vasut 
Signed-off-by: Jan Kiszka 
---

Changes in v2:
 - patch the file I actually meant to patch
   (was papered over by a local queue)

 drivers/crypto/rsa_mod_exp/mod_exp_sw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/rsa_mod_exp/mod_exp_sw.c 
b/drivers/crypto/rsa_mod_exp/mod_exp_sw.c
index c9b571a461..46b9f1825c 100644
--- a/drivers/crypto/rsa_mod_exp/mod_exp_sw.c
+++ b/drivers/crypto/rsa_mod_exp/mod_exp_sw.c
@@ -31,6 +31,7 @@ U_BOOT_DRIVER(mod_exp_sw) = {
.name   = "mod_exp_sw",
.id = UCLASS_MOD_EXP,
.ops= _exp_ops_sw,
+   .flags  = DM_FLAG_PRE_RELOC,
 };
 
 U_BOOT_DEVICE(mod_exp_sw) = {
-- 
2.26.1

[PATCH] lib: rsa: Also check for presence of r-squared property

[Jan Kiszka]

From: Jan Kiszka 

Better than crashing later if it is missing.

Signed-off-by: Jan Kiszka 
---
 lib/rsa/rsa-verify.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
index 80e817314b..f7ae174cb0 100644
--- a/lib/rsa/rsa-verify.c
+++ b/lib/rsa/rsa-verify.c
@@ -445,7 +445,7 @@ static int rsa_verify_with_keynode(struct image_sign_info 
*info,
 
prop.rr = fdt_getprop(blob, node, "rsa,r-squared", NULL);
 
-   if (!prop.num_bits || !prop.modulus) {
+   if (!prop.num_bits || !prop.modulus || !prop.rr) {
debug("%s: Missing RSA key info", __func__);
return -EFAULT;
}
-- 
2.26.1

[PATCH v2 1/1] efi_loader: put device tree into EfiACPIReclaimMemory

[Heinrich Schuchardt]

According to the UEFI spec ACPI tables should be placed in
EfiACPIReclaimMemory. Let's do the same with the device tree.

Suggested-by: Ard Biesheuvel 
Cc: Grant Likely 
Signed-off-by: Heinrich Schuchardt 
---
v2:
adjust the unit test
---
 cmd/bootefi.c  | 4 ++--
 lib/efi_selftest/efi_selftest_memory.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 54b4b8f984..06573b14e9 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -127,13 +127,13 @@ static efi_status_t copy_fdt(void **fdtp)
new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f0 +
 fdt_size, 0);
ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
-EFI_BOOT_SERVICES_DATA, fdt_pages,
+EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
 _fdt_addr);
if (ret != EFI_SUCCESS) {
/* If we can't put it there, put it somewhere */
new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
-EFI_BOOT_SERVICES_DATA, fdt_pages,
+EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
 _fdt_addr);
if (ret != EFI_SUCCESS) {
printf("ERROR: Failed to reserve space for FDT\n");
diff --git a/lib/efi_selftest/efi_selftest_memory.c 
b/lib/efi_selftest/efi_selftest_memory.c
index e71732dc6d..4d32a28006 100644
--- a/lib/efi_selftest/efi_selftest_memory.c
+++ b/lib/efi_selftest/efi_selftest_memory.c
@@ -176,9 +176,9 @@ static int execute(void)
/* Check memory reservation for the device tree */
if (fdt_addr &&
find_in_memory_map(map_size, memory_map, desc_size, fdt_addr,
-  EFI_BOOT_SERVICES_DATA) != EFI_ST_SUCCESS) {
+  EFI_ACPI_RECLAIM_MEMORY) != EFI_ST_SUCCESS) {
efi_st_error
-   ("Device tree not marked as boot services data\n");
+   ("Device tree not marked as ACPI reclaim memory\n");
return EFI_ST_FAILURE;
}
return EFI_ST_SUCCESS;
--
2.26.2

Re: [PATCH][v4 6/6] board: tbs2910: add documentation

[Soeren Moch]

Denis,

do you plan to re-send a fixed version of this series?

Thanks,
Soeren


On 01.03.20 14:59, Soeren Moch wrote:
> On 27.02.20 01:37, Denis 'GNUtoo' Carikli wrote:
>> This documents the u-boot installation procedure and the
>> hardware in order to get started.
> Thanks for adding this documentation!
>
> Since you added SYSBOOT support in this series, it also might be good to
> mention this (and maybe how to use it) in the documentation.
> Sorry for again bringing up new wishes here, but due to the corrections
> below unfortunately we need a new version of this patch anyway.
>> Signed-off-by: Denis 'GNUtoo' Carikli 
> To ease the review process it is good style to add a patch revision
> history below the sign-off after a "---" separator. This would look like
> ---
> changes in v4:
>  - bla
>
> This is not a problem for this particular patch, just as hint for future
> patch submissions.
>> ---
>>  doc/board/index.rst   |   1 +
>>  doc/board/tbs/index.rst   |   9 ++
>>  doc/board/tbs/tbs2910.rst | 179 ++
>>  3 files changed, 189 insertions(+)
>>  create mode 100644 doc/board/tbs/index.rst
>>  create mode 100644 doc/board/tbs/tbs2910.rst
> This new board specific documentation should be added to the
> board/tbs/tbs2910/MAINTAINERS file.
>> diff --git a/doc/board/index.rst b/doc/board/index.rst
>> index b8b956d730..4564c8245f 100644
>> --- a/doc/board/index.rst
>> +++ b/doc/board/index.rst
>> @@ -16,4 +16,5 @@ Board-specific doc
>> renesas/index
>> rockchip/index
>> sifive/index
>> +   tbs/index
>> xilinx/index
>> diff --git a/doc/board/tbs/index.rst b/doc/board/tbs/index.rst
>> new file mode 100644
>> index 00..b677bc624f
>> --- /dev/null
>> +++ b/doc/board/tbs/index.rst
>> @@ -0,0 +1,9 @@
>> +.. SPDX-License-Identifier: GPL-2.0+
>> +
>> +TBS
>> +===
>> +
>> +.. toctree::
>> +   :maxdepth: 2
>> +
>> +   tbs2910
>> diff --git a/doc/board/tbs/tbs2910.rst b/doc/board/tbs/tbs2910.rst
>> new file mode 100644
>> index 00..2fc84c66ec
>> --- /dev/null
>> +++ b/doc/board/tbs/tbs2910.rst
>> @@ -0,0 +1,179 @@
>> +TBS2910 Matrix ARM miniPC
>> +=
>> +
>> +Building
>> +
>> +To build u-boot for the TBS2910 Matrix ARM miniPC, you can use the following
>> +procedure:
>> +
>> +First add the ARM toolchain to your PATH
>> +
>> +Then setup the ARCH and cross compilation environment variables.
>> +
>> +When this is done you can then build u-boot for the TBS2910 Matrix ARM 
>> miniPC
>> +with the following commands:
>> +
>> +.. code-block:: none
>> +
>> +   make mrproper
>> +   make tbs2910_defconfig
>> +   make
>> +
>> +Once the build is complete, you can find the resulting image as u-boot.imx 
>> in
>> +the current directory.
>> +
>> +UART
>> +
>> +The UART voltage is at 3.3V and its settings are 115200bps 8N1
>> +
>> +BOOT/UPDATE boot switch:
>> +
>> +The BOOT/UPDATE switch (SW11) is connected to the BOOT_MODE0 and
>> +BOOT_MODE1 SoC pins. It has "BOOT" and "UPDATE" markings both on
>> +the PCB and on the plastic case.
>> +
>> +When set to the "UPDATE" position, the SoC will use the "Boot From Fuses"
>> +configuration, and since BT_FUSE_SEL is 0, this makes the SOC jump to serial
>> +downloader.
>> +
>> +When set in the "BOOT" position, the SoC will use the "Internal boot"
>> +configuration, and since BT_FUSE_SEL is 0, it will then use the GPIO pins
>> +for the boot configuration.
>> +
>> +SW6 binary DIP switch array on the PCB revision 2.1:
>> +
>> +On that PCB revision, SW6 has 8 positions.
>> +
>> +Switching a position to ON sets the corresponding
>> +register to 1.
>> +
>> +See the following table for a correspondence between the switch positions 
>> and
>> +registers:
>> +
>> +===
>> +Switch positionRegister
>> +===
>> +1  BOOT_CFG2[3]
>> +2  BOOT_CFG2[4]
>> +3  BOOT_CFG2[5]
>> +4  BOOT_CFG2[6]
>> +5  BOOT_CFG1[4]
>> +6  BOOT_CFG1[5]
>> +7  BOOT_CFG1[6]
>> +8  BOOT_CFG1[7]
>> +===
>> +
>> +For example:
>> +
>> +  - To boot from the eMMC: 1:ON , 2:ON, 3:ON, 4:OFF, 5:OFF, 6:ON, 7:ON, 8:ON
> Due to the schematic we need 8:OFF.
>> +  - To boot from the microSD slot: 1: ON, 2: OFF, 3: OFF, 4: OFF, 5:OFF, 
>> 6:OFF,
>> +7:ON, 8:OFF
>> +  - To boot from the SD slot: 1: OFF, 2: ON, 3: OFF, 4: OFF, 5:OFF, 6:OFF, 
>> 7:ON,
>> +8:OFF
>> +  - To boot from SATA: 5:OFF, 6:ON, 7:OFF, 8:OFF
> Again due to the schematic we need 1:OFF in addition. Probably also a
> good idea to set 2,3,4 OFF.
>
> Thanks for improving tbs2910 support and documentation,
> Soeren
>> +
>> +You can refer to the BOOT_CFG registers in the I.MX6Q reference manual for
>> +additional details.
>> +
>> +SW6 binary DIP switch array on the PCB revision 2.3:
>> 

[PATCH] Makefile: add file 'defconfig' to clean target

[Heinrich Schuchardt]

File 'defconfig' is a build artifact of 'make savedefconfig'.
Remove it when cleaning.

Signed-off-by: Heinrich Schuchardt 
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index cc99873062..88b356c40b 100644
--- a/Makefile
+++ b/Makefile
@@ -2037,7 +2037,7 @@ CLEAN_FILES += include/bmp_logo.h include/bmp_logo_data.h 
tools/version.h \
   boot* u-boot* MLO* SPL System.map fit-dtb.blob* \
   u-boot-ivt.img.log u-boot-dtb.imx.log SPL.log u-boot.imx.log \
   lpc32xx-* bl31.c bl31.elf bl31_*.bin image.map tispl.bin* \
-  idbloader.img flash.bin flash.log
+  idbloader.img flash.bin flash.log defconfig

 # Directories & files removed with 'make mrproper'
 MRPROPER_DIRS  += include/config include/generated spl tpl \
--
2.26.2

Re: efi_loader: pkcs7_parse_message() returns error pointer

[Heinrich Schuchardt]

On 07.05.20 02:17, Patrick Wildt wrote:
> Since pkcs7_parse_message() returns an error pointer, we must not
> check for NULL.  We have to explicitly set msg to NULL in the error
> case, otherwise the call to pkcs7_free_message() on the goto err
> path will assume it's a valid object.
>
> Signed-off-by: Patrick Wildt 
>
> diff --git a/lib/efi_loader/efi_image_loader.c 
> b/lib/efi_loader/efi_image_loader.c
> index 5a9a6424cc..43a53d3dd1 100644
> --- a/lib/efi_loader/efi_image_loader.c
> +++ b/lib/efi_loader/efi_image_loader.c
> @@ -538,8 +538,9 @@ static bool efi_image_authenticate(void *efi, size_t 
> efi_size)
>   }
>   msg = pkcs7_parse_message((void *)wincert + sizeof(*wincert),
> wincert->dwLength - sizeof(*wincert));
> - if (!msg) {
> + if (IS_ERR(msg)) {

Compiling with sandbox_defconfig results in:

lib/efi_loader/efi_image_loader.c: In function ‘efi_image_authenticate’:
lib/efi_loader/efi_image_loader.c:541:7: warning: implicit declaration
of function ‘IS_ERR’ [-Wimplicit-function-declaration]
  541 |   if (IS_ERR(msg)) {
  |   ^~


I will add the missing #include  when merging.

Reviewed-by: Heinrich Schuchardt 

>   debug("Parsing image's signature failed\n");
> + msg = NULL;
>   goto err;
>   }
>
>

Re: Bisected: omap_hsmmc 3.3V IO voltage incompatible with N900 (Was: Re: Bisected: mmc cause reboot loops on N900)

[Pali Rohár]

On Thursday 07 May 2020 19:10:14 Faiz Abbas wrote:
> On 26/04/20 3:59 am, Pali Rohár wrote:
> > On Sunday 26 April 2020 00:20:07 Pali Rohár wrote:
> >> On Saturday 25 April 2020 23:26:15 Pali Rohár wrote:
> >>> Now I tried git bisect and here is problematic commit which caused whole
> >>> reboot loop:
> >>>
> >>> 04a2ea248f58b3b6216d0cd0a6b8698df8b14355 is the first bad commit
> >>> commit 04a2ea248f58b3b6216d0cd0a6b8698df8b14355
> >>> Author: Jean-Jacques Hiblot 
> >>> Date:   Thu Sep 21 16:30:08 2017 +0200
> >>>
> >>> mmc: disable UHS modes if Vcc cannot be switched on and off
> >>> 
> >>> If a power cycle cannot be done on Vcc, it is safer not to try the UHS
> >>> modes because we wouldn't be able to recover from an error occurring
> >>> during the UHS initialization.
> >>> 
> >>> Signed-off-by: Jean-Jacques Hiblot 
> >>>
> >>> :04 04 04de51428c8311a4b2fb3ad876ac3f6071ab57ee 
> >>> ea7a7959a4bd591c92a2c3d413d5643a8457d2ff M  drivers
> >>> :04 04 03f639baf2a2f55003cb750981fd8accc5b4a993 
> >>> fbcb9607d37959f0b5240f5d727133f58cc35379 M  include
> >>>
> >>> It changes only core mmc code, nothing platform / board specific.
> >>> U-Boot compiled from commit before above has fully working eMMC access
> >>> on real Nokia N900. I can read files on FAT eMMC partition without any
> >>> problem.
> >>>
> >>> I'm not sure what is happening here, but it looks like that omap hs mmc
> >>> driver used on Nokia N900 is maybe not correctly hooked for UHS support.
> >>>
> >>> The most suspicious it that this problem cannot be reproduced in qemu
> >>> n900 emulator. It happens only on real N900 hw.
> >>
> >> I took main change from above commit, reverted it on master and U-Boot
> >> stopped crashing! No reboot loop anymore. Here is change which fixes
> >> reboot loop on Nokia N900:
> >>
> >> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> >> index 523c055967..d07c7745da 100644
> >> --- a/drivers/mmc/mmc.c
> >> +++ b/drivers/mmc/mmc.c
> >> @@ -2786,18 +2786,7 @@ int mmc_get_op_cond(struct mmc *mmc)
> >>  MMC_QUIRK_RETRY_APP_CMD;
> >>  #endif
> >>  
> >> -  err = mmc_power_cycle(mmc);
> >> -  if (err) {
> >> -  /*
> >> -   * if power cycling is not supported, we should not try
> >> -   * to use the UHS modes, because we wouldn't be able to
> >> -   * recover from an error during the UHS initialization.
> >> -   */
> >> -  pr_debug("Unable to do a full power cycle. Disabling the UHS 
> >> modes for safety\n");
> >> -  uhs_en = false;
> >> -  mmc->host_caps &= ~UHS_CAPS;
> >> -  err = mmc_power_on(mmc);
> >> -  }
> >> +  err = mmc_power_on(mmc);
> >>if (err)
> >>return err;
> >>  
> >>
> >> Jean, do you have any idea what is happening in above code? It looks
> >> like something is broken in power cycle / power on sequence if it cause
> >> "data abort" and reboot of board.
> >>
> >>
> >> But even with above my change eMMC on N900 cannot be initialized...
> >> I'm running second git bisect with applying above change on every
> >> commit. It is in progress now.
> > 
> > And second bisect finished. Result is:
> > 
> > d2c05f50e12f87128597a28146de7092aaa847c3 is the first bad commit
> > commit d2c05f50e12f87128597a28146de7092aaa847c3
> > Author: Faiz Abbas 
> > Date:   Fri Apr 5 14:18:46 2019 +0530
> > 
> > mmc: omap_hsmmc: Set 3.3V for IO voltage
> > 
> > Pbias voltage should match the IO voltage set for the SD card. With the
> > latest pbias change to 3.3V, update the capabilities and IO voltages
> > settings to 3.3V.
> > 
> > Signed-off-by: Faiz Abbas 
> > 
> > :04 04 819148217b64a6beaf8247464de6b4384d4581a4 
> > e4fd9288ddb794f33596339813d5386f3bed8fd7 M  drivers
> > 
> > I revered above commit on top of master and eMMC on Nokia N900 finally
> > started working again!
> > 
> > Faiz, what is the reason for above commit? It basically changes in omap
> > hs mmc driver IO voltage from 3.0V to 3.3V. And seems this change is
> > incompatible with Nokia N900. Are there any problems with 3.0V on some
> > omap3 boards? If not I would vote for revering that commit. Or at least
> > making IO voltage configurable.
> > 
> 
> For the power_cycle patch, it looks like there is a null pointer dereference
> that might be causing the reboot loops (probably because your platform doesn't
> have DM_MMC enabled) Can you add a few more prints to see where the data abort
> comes from exactly?

Hello Faiz!

Sorry, but this is problematic. Because of reboot loops I cannot read
what exactly is put on the display and reboot cause clearing display.

Month ago I was able to detect that problem is somewhere in function
mmc_set_ios() from mmc.c file. I put long debug line prior and also
after mmc_set_ios() call. And it was printed only prior. Not after.
So I think NULL pointer dereference is in mmc_set_ios() function.

> For the second patch IO voltage patch, 

Re: [PATCH v2] misc: i2c_eeprom: implement different probe test eeprom offset

[Eugen.Hristev]

On 07.05.2020 18:02, Baruch Siach wrote:
> Hi Heiko,
> 
> On Thu, May 07 2020, Heiko Schocher wrote:
>> Am 07.05.2020 um 10:53 schrieb Eugen Hristev:
>>> Because of this commit :
>>> 5ae84860b0 ("misc: i2c_eeprom: verify that the chip is functional at 
>>> probe()")
>>> at probe time, each eeprom is tested for read at offset 0.
>>>
>>> The Atmel AT24MAC402 eeprom has different mapping. One i2c slave address is
>>> used for the lower 0x80 bytes and another i2c slave address is used for the
>>> upper 0x80 bytes. Because of this basically the i2c master sees 2 different
>>> slaves. We need the upper bytes because we read the unique MAC address from
>>> this EEPROM area.
>>>
>>> However this implies that our slave address will return error on reads
>>> from address 0x0 to 0x80.
>>>
>>> To solve this, implemented an offset field inside platform data that is by
>>> default 0 (as it is used now), but can be changed in the compatible table.
>>>
>>> The probe function will now read at this offset and use it, instead of 
>>> blindly
>>> checking offset 0.
>>>
>>> This will fix the regression noticed on these EEPROMs since the commit
>>> abovementioned that introduces the probe failed issue.
>>>
>>> Signed-off-by: Eugen Hristev 
>>> ---
>>>
>>> Please disregard patch
>>> [PATCH] misc: i2c_eeprom: implement different probe test eeprom offset
>>>
>>> as that patch was mistakenly done on an older u-boot version.
>>> This v2 patch applies correctly on u-boot/master
>>>
>>> Changes in v2:
>>> - adapt to latest changes in driver. v1 was done on u-boot 2020.01 
>>> accidentaly.
>>>
>>>drivers/misc/i2c_eeprom.c | 8 +++-
>>>1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> Thanks for rebasing!
>>
>> I prefer to fix the issue instead reverting commit:
>>
>> 5ae84860b0 ("misc: i2c_eeprom: verify that the chip is functional at 
>> probe()")
>>
>> Reviewed-by: Heiko Schocher 
>>
>> @Baruch, is this Ok for you?
> 
> According to the Linux driver there are more devices that need read
> offset. 24cs32 and 24cs64 are affected. This patch does not fix the
> regression for those devices.
> 
> Eugen, would it be possible for you to extend the fix to 24cs32/64 and
> test on real hardware?

Hi Baruch,

This patch fixes my issue at hand and the eeprom which I have at my 
disposal. I will look to see if I have those, but, most likely not.
I can extend the fix, but I cannot test, so, I rather not go blind with it.

Eugen
> 
> baruch
> 
>>> diff --git a/drivers/misc/i2c_eeprom.c b/drivers/misc/i2c_eeprom.c
>>> index ef5f103c98..32a1b20856 100644
>>> --- a/drivers/misc/i2c_eeprom.c
>>> +++ b/drivers/misc/i2c_eeprom.c
>>> @@ -17,6 +17,7 @@ struct i2c_eeprom_drv_data {
>>>   u32 pagesize; /* page size in bytes */
>>>   u32 addr_offset_mask; /* bits in addr used for offset overflow */
>>>   u32 offset_len; /* size in bytes of offset */
>>> +u32 start_offset; /* valid start offset inside memory, by default 0 */
>>>};
>>>  int i2c_eeprom_read(struct udevice *dev, int offset, uint8_t *buf, int
>>> size)
>>> @@ -147,7 +148,11 @@ static int i2c_eeprom_std_probe(struct udevice *dev)
>>>   i2c_set_chip_addr_offset_mask(dev, data->addr_offset_mask);
>>>   /* Verify that the chip is functional */
>>> -ret = i2c_eeprom_read(dev, 0, _byte, 1);
>>> +/*
>>> + * Not all eeproms start from offset 0. Valid offset is available
>>> + * in the platform data struct.
>>> + */
>>> +ret = i2c_eeprom_read(dev, data->start_offset, _byte, 1);
>>>   if (ret)
>>>   return -ENODEV;
>>>@@ -215,6 +220,7 @@ static const struct i2c_eeprom_drv_data
>>> atmel24mac402_data = {
>>>   .pagesize = 16,
>>>   .addr_offset_mask = 0,
>>>   .offset_len = 1,
>>> +.start_offset = 0x80,
>>>};
>>>  static const struct i2c_eeprom_drv_data atmel24c32_data = {
>>>
> 
> 
> --
>   ~. .~   Tk Open Systems
> =}ooO--U--Ooo{=
> - bar...@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
> 

Re: [PATCH v2] misc: i2c_eeprom: implement different probe test eeprom offset

[Baruch Siach]

Hi Heiko,

On Thu, May 07 2020, Heiko Schocher wrote:
> Am 07.05.2020 um 10:53 schrieb Eugen Hristev:
>> Because of this commit :
>> 5ae84860b0 ("misc: i2c_eeprom: verify that the chip is functional at 
>> probe()")
>> at probe time, each eeprom is tested for read at offset 0.
>>
>> The Atmel AT24MAC402 eeprom has different mapping. One i2c slave address is
>> used for the lower 0x80 bytes and another i2c slave address is used for the
>> upper 0x80 bytes. Because of this basically the i2c master sees 2 different
>> slaves. We need the upper bytes because we read the unique MAC address from
>> this EEPROM area.
>>
>> However this implies that our slave address will return error on reads
>> from address 0x0 to 0x80.
>>
>> To solve this, implemented an offset field inside platform data that is by
>> default 0 (as it is used now), but can be changed in the compatible table.
>>
>> The probe function will now read at this offset and use it, instead of 
>> blindly
>> checking offset 0.
>>
>> This will fix the regression noticed on these EEPROMs since the commit
>> abovementioned that introduces the probe failed issue.
>>
>> Signed-off-by: Eugen Hristev 
>> ---
>>
>> Please disregard patch
>> [PATCH] misc: i2c_eeprom: implement different probe test eeprom offset
>>
>> as that patch was mistakenly done on an older u-boot version.
>> This v2 patch applies correctly on u-boot/master
>>
>> Changes in v2:
>> - adapt to latest changes in driver. v1 was done on u-boot 2020.01 
>> accidentaly.
>>
>>   drivers/misc/i2c_eeprom.c | 8 +++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> Thanks for rebasing!
>
> I prefer to fix the issue instead reverting commit:
>
> 5ae84860b0 ("misc: i2c_eeprom: verify that the chip is functional at probe()")
>
> Reviewed-by: Heiko Schocher 
>
> @Baruch, is this Ok for you?

According to the Linux driver there are more devices that need read
offset. 24cs32 and 24cs64 are affected. This patch does not fix the
regression for those devices.

Eugen, would it be possible for you to extend the fix to 24cs32/64 and
test on real hardware?

baruch

>> diff --git a/drivers/misc/i2c_eeprom.c b/drivers/misc/i2c_eeprom.c
>> index ef5f103c98..32a1b20856 100644
>> --- a/drivers/misc/i2c_eeprom.c
>> +++ b/drivers/misc/i2c_eeprom.c
>> @@ -17,6 +17,7 @@ struct i2c_eeprom_drv_data {
>>  u32 pagesize; /* page size in bytes */
>>  u32 addr_offset_mask; /* bits in addr used for offset overflow */
>>  u32 offset_len; /* size in bytes of offset */
>> +u32 start_offset; /* valid start offset inside memory, by default 0 */
>>   };
>> int i2c_eeprom_read(struct udevice *dev, int offset, uint8_t *buf, int
>> size)
>> @@ -147,7 +148,11 @@ static int i2c_eeprom_std_probe(struct udevice *dev)
>>  i2c_set_chip_addr_offset_mask(dev, data->addr_offset_mask);
>>  /* Verify that the chip is functional */
>> -ret = i2c_eeprom_read(dev, 0, _byte, 1);
>> +/*
>> + * Not all eeproms start from offset 0. Valid offset is available
>> + * in the platform data struct.
>> + */
>> +ret = i2c_eeprom_read(dev, data->start_offset, _byte, 1);
>>  if (ret)
>>  return -ENODEV;
>>   @@ -215,6 +220,7 @@ static const struct i2c_eeprom_drv_data
>> atmel24mac402_data = {
>>  .pagesize = 16,
>>  .addr_offset_mask = 0,
>>  .offset_len = 1,
>> +.start_offset = 0x80,
>>   };
>> static const struct i2c_eeprom_drv_data atmel24c32_data = {
>>


-- 
 ~. .~   Tk Open Systems
=}ooO--U--Ooo{=
   - bar...@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

Re: efi_loader: pkcs7_parse_message() returns error pointer

[Heinrich Schuchardt]

On 07.05.20 02:17, Patrick Wildt wrote:
> Since pkcs7_parse_message() returns an error pointer, we must not
> check for NULL.  We have to explicitly set msg to NULL in the error
> case, otherwise the call to pkcs7_free_message() on the goto err
> path will assume it's a valid object.

@Takahiro
I think we should start documenting the library functions properly. The
description in lib/crypto/pkcs7_parser.c does not conform to
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation
Especially we should describe how errors are returned.

Best regards

Heinrich

>
> Signed-off-by: Patrick Wildt 
>
> diff --git a/lib/efi_loader/efi_image_loader.c 
> b/lib/efi_loader/efi_image_loader.c
> index 5a9a6424cc..43a53d3dd1 100644
> --- a/lib/efi_loader/efi_image_loader.c
> +++ b/lib/efi_loader/efi_image_loader.c
> @@ -538,8 +538,9 @@ static bool efi_image_authenticate(void *efi, size_t 
> efi_size)
>   }
>   msg = pkcs7_parse_message((void *)wincert + sizeof(*wincert),
> wincert->dwLength - sizeof(*wincert));
> - if (!msg) {
> + if (IS_ERR(msg)) {
>   debug("Parsing image's signature failed\n");
> + msg = NULL;
>   goto err;
>   }
>
>

Re: [PATCH] sf: Add Macronix MX25R6435F SPI NOR flash to flash parameters array

[Jagan Teki]

On Sun, May 3, 2020 at 6:10 PM Peng Fan  wrote:
>
> From: Ye Li 
>
> On i.mx7ulp EVK board, we use MX25R6435F NOR flash, add its parameters
> and IDs to flash parameter array. Otherwise, the flash probe will fails.
>
> Signed-off-by: Ye Li 
> Signed-off-by: Peng Fan 
> ---

Applied to u-boot-spi/master

Re: [PATCH] clk: Fix clk func names in comments

[Jagan Teki]

On Fri, May 1, 2020 at 11:45 PM Jagan Teki  wrote:
>
> clk function names in comments should be prefix with
> clk instead of clock.
>
> Fix it.
>
> Cc: Simon Glass 
> Cc: Tom Rini 
> Signed-off-by: Jagan Teki 
> ---

Applied to u-boot-spi/master

Re: [PATCH 2/2] spi: Zap lpc32xx_ssp driver-related code

[Jagan Teki]

On Fri, May 1, 2020 at 10:05 PM Jagan Teki  wrote:
>
> lpc32xx_ssp driver is deprecated, no active updates
> and no board user, hence dropped the same.
>
> Cc: Vladimir Zapolskiy 
> Cc: Albert ARIBAUD 
> Cc: Tom Rini 
> Signed-off-by: Jagan Teki 
> ---

Applied to u-boot-spi/master

Re: wandboard does not boot with current mainline

[Tom Rini]

On Thu, May 07, 2020 at 02:53:49PM +0200, Heiko Schocher wrote:
> Hello Fabio,
> 
> Am 07.05.2020 um 14:12 schrieb Heiko Schocher:
> > Hello Fabio,
> > 
> > I have my wandboard DL in my automated daily build setup example:
> > 
> > http://xeidos.ddns.net/ubtestresults/
> > 
> > and wondered, why my last result is from may 4th ...
> > 
> > (Ok, there is a bug, that I do not see not booting boards in this page,
> >   but this is just a proof of concept page ...)
> 
> Ok, found this error, now I have one broken build in above webpage.
> Also, the newest commit adds > 300 bytes imge size...
> 
> http://xeidos.ddns.net/ubtestresults/stats/wandboard_defconfig/6

That's not what I see:
$ (export SOURCE_DATE_EPOCH=`date +%s`; ./tools/buildman/buildman -o 
/tmp/wandboard3 --step 0 -SBC -b 125956..69bf66 -devl wandboard && 
./tools/buildman/buildman -o /tmp/wandboard3 --step 0 -SsB -b 125956..69bf66 
-devl wandboard)
Building 2 commits for 1 boards (1 thread, 16 jobs per thread)
01: scripts/get_default_envs.sh: preserve order of multiple entries for same 
variable
15: Merge branch '2020-05-06-master-imports'
200 /2  wandboard
Completed: 2 total built, duration 0:00:23
Summary of 2 commits for 1 boards (1 thread, 16 jobs per thread)
01: scripts/get_default_envs.sh: preserve order of multiple entries for same 
variable
15: Merge branch '2020-05-06-master-imports'
   arm: (for 1/1 boards) bss -16.0 text +16.0
wandboard  : bss -16 text +16
   u-boot: add: 0/0, grow: 3/0 bytes: 12/0 (12)
 function   old new   delta
 menu_create 84  88  +4
 handle_pxe_menu284 288  +4
 do_dcache   92  96  +4
(no errors to report)
$

What details are you recording in your setup?

> Unfortunately I cannot automate git bisect with tbot:
> 
> http://tbot.tools/modules/tc.html#tbot.tc.git.GitRepository.bisect
> 
> as I do not know how to switch for example with an gpio and a relay
> bootmode on wandboard ... do you know, if this is possible?

In general, or on the wandboard?  For the general side, I have a network
controllable relay controller that I can use to toggle a GPIO on
hardware that has a force reset exposed.  I don't have any imx platforms
exposed however as I wasn't able to rig up some way to boot via USB on
the platforms I have here.  That was a while ago tho.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 2/2] phy: Use _nodev naming convention if non-device clients

[Jagan Teki]

On Fri, May 1, 2020 at 11:44 PM Jagan Teki  wrote:
>
> Clients that are requesting some of uclass API's
> without a device (with ofnode) usually have _nodev
> naming convention.
>
> - clk_get_by_index_nodev
> - clk_get_by_name_nodev
> - reset_get_by_index_nodev
> - gpio_request_by_name_nodev
>
> So, update the same naming convention PHY framework.
>
> This doesn't change the existing functionality.
>
> Cc: Neil Armstrong 
> Cc: Tom Rini 
> Signed-off-by: Jagan Teki 
> ---

Applied both to u-boot-spi/master

Re: [PATCH v3 1/3] iopoll: Add read_poll_timeout common API

[Jagan Teki]

On Sat, May 2, 2020 at 12:45 PM Jagan Teki  wrote:
>
> Add read_poll_timeout common API similar to Linux iopoll.
>
> readx_poll_timeout will trigger read_poll_timeout with
> proper op. This will help to extend the functionalities
> like sleep_us to poll timeout in future.
>
> This change is referenced from Linux from below commit:
> commit <5f5323a14cad19323060a8cbf9d96f2280a462dd> ("iopoll:
> introduce read_poll_timeout macro")
>
> Signed-off-by: Jagan Teki 
> ---

Applied all to u-boot-spi/master

Re: [PATCH] arm: dtsi: sama5d2: Add uart4 definition to sama5d2.dtsi

[Eugen.Hristev]

On 07.05.2020 17:23, Tiaki Rice wrote:
> Eugen,
> 
> Ah, I suspect the issue might be might be because I changed the subject 
> inside the file but the did not change the filename. I shouldn't have done it 
> that way anyway. Regarding windows vs linux, doing a diff between them showed 
> no differences aside from the tag at the end describing the git version.
> 
> I have amended the commit and attached both the windows git patch I 
> originally submitted for reference (0001-dtsi...) and the new patch created 
> under CentOS8 (0001-arm-dtsi...). If you can't see any problems, should I try 
> emailing the patch to the list again?
> 
> 

Ok, your patch looks good. I can see it now in patchwork. Maybe it was a 
patchwork glitch.


I will modify the patch a little bit to comply with the shortlog rules:
ARM must be in caps
then dts, no need to put dtsi, and no need to mention sama5d2.dtsi again 
in shortlog (ARM: dts: sama5d2 means it's sama5d2.dtsi )

like:

ARM: dts: sama5d2: add uart4 definition

if you feel like doing that yourself, modify and send a v2 patch (git 
format-patch -v 2) and send it again with git send-email.

Thanks again !
Eugen



> Regards, Tiaki
> 
> -Original Message-
> From: eugen.hris...@microchip.com 
> Sent: Thursday, 7 May 2020 9:55 PM
> To: tiakir...@hotmail.com
> Cc: u-boot@lists.denx.de
> Subject: Re: [PATCH] arm: dtsi: sama5d2: Add uart4 definition to sama5d2.dtsi
> 
> On 07.05.2020 14:32, Tiaki Rice wrote:
>> Eugen,
>>
>> Thanks for the speedy reply, I did not know about this script! I must have 
>> glossed over it in the uboot patch documentation page.
>>
>> The commit line is 82 characters which is longer than the preferred max of 
>> 75. Oops. Don't think this warning would be stopping the patch going through 
>> though...
>>
> 
> Hi,
> 
> For sure that won't be a problem with patchwork.
> however , it looks like your patch is windows-generated ? That might be an 
> issue, I do not know. Can you send me your patch as attachment file ?
> 
> Thanks
> 
>>
>> Here is the output for the patch:
>> -
>>   $ ./scripts/checkpatch.pl 
>> 0001-dtsi-sama5d2-Add-uart4-definition-to-sama5d2.dtsi.patch
>>   WARNING: Possible unwrapped commit description (prefer a maximum 75 
>> chars per line)
>>   #7:
>>   This patch adds support for uart4 to the processor level device tree 
>> include file.
>>
>>   total: 0 errors, 1 warnings, 0 checks, 14 lines checked
>>
>>   NOTE: For some of the reported defects, checkpatch may be able to
>> mechanically convert to the typical style using --fix or 
>> --fix-inplace.
>>
>>   0001-dtsi-sama5d2-Add-uart4-definition-to-sama5d2.dtsi.patch has style 
>> problems, please review.
>>
>>   NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX
>> MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE
>> PREFER_ETHER_ADDR_COPY USLEEP_RANGE
>>
>>   NOTE: If any of the errors are false positives, please report
>> them to the maintainer, see CHECKPATCH in MAINTAINERS.
>> -
>>
>> Regards,
>>
>> Tiaki Rice
>>
>> -Original Message-
>> From: eugen.hris...@microchip.com 
>> Sent: Thursday, 7 May 2020 8:19 PM
>> To: tiakir...@hotmail.com
>> Cc: u-boot@lists.denx.de
>> Subject: Re: [PATCH] arm: dtsi: sama5d2: Add uart4 definition to
>> sama5d2.dtsi
>>
>> On 07.05.2020 12:21, Tiaki Rice wrote:
>>
>>> Hello Eugen,
>>>
>>> I'm new to this whole mailing list patchwork thingy so I suspect I didn't 
>>> quite submit the patch correctly, maybe you can point me in the right 
>>> direction.
>>>
>>> What I did:
>>> - Made the changes on master and locally commited them
>>> - Created a patch using  'git format-patch' and modified the patch file 
>>> email body.
>>> - Used 'git send-email' to send the patch to u-boot@lists.denx.de and
>>> Cc'd you
>>>
>>> This opened my Outlook of which I checked to make sure the email was plain 
>>> text and then sent it. Any ideas?
>>>
>>
>> Hi Tiaki,
>>
>>From my perspective you did everything very well. The patch looks sent 
>> fine. I would only have one question, did you check your patch with the 
>> built-in perl script that automatically checks your patch for formatting ?
>> ./scripts/checkpatch.pl --strict 
>>
>> About patchwork, I think it may be an issue with patchwork.
>>
>> Eugen
>>
>>
>>>
>>> Thanks in advance,
>>>
>>> Tiaki Rice
>>>
>>> -Original Message-
>>> From: eugen.hris...@microchip.com 
>>> Sent: Thursday, 7 May 2020 7:12 PM
>>> To: tiakir...@hotmail.com; u-boot@lists.denx.de
>>> Subject: Re: [PATCH] arm: dtsi: sama5d2: Add uart4 definition to
>>> sama5d2.dtsi
>>>
>>> On 07.05.2020 07:43, Tiaki Rice wrote:
 This patch adds support for uart4 to the processor level device tree 
 include file.


 Signed-off-by: Tiaki Rice 
 Cc: Eugen Hristev 
 ---
>>>
>>> Hi,
>>>
>>> Thanks for contributing !
>>>
>>> I have an issue with this patch missing from patchwork.
>>> Anyone has any idea why that 

Re: [PATCH] x86: mtrr: Drop the mask display when changing an mtrr

[Bin Meng]

On Thu, May 7, 2020 at 10:13 PM Simon Glass  wrote:
>
> We don't need to print this information since it is shown when the MTRRs
> are displayed. Drop it.
>
> Signed-off-by: Simon Glass 
> ---
>
>  cmd/x86/mtrr.c | 1 -
>  1 file changed, 1 deletion(-)
>

Reviewed-by: Bin Meng 

[PATCH] x86: mtrr: Drop the mask display when changing an mtrr

[Simon Glass]

We don't need to print this information since it is shown when the MTRRs
are displayed. Drop it.

Signed-off-by: Simon Glass 
---

 cmd/x86/mtrr.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c
index d3fd959235..ac36a40854 100644
--- a/cmd/x86/mtrr.c
+++ b/cmd/x86/mtrr.c
@@ -72,7 +72,6 @@ static int do_mtrr_set(uint reg, int argc, char * const 
argv[])
if (valid)
mask |= MTRR_PHYS_MASK_VALID;
 
-   printf("base=%llx, mask=%llx\n", base, mask);
mtrr_open(, true);
wrmsrl(MTRR_PHYS_BASE_MSR(reg), base);
wrmsrl(MTRR_PHYS_MASK_MSR(reg), mask);
-- 
2.26.2.645.ge9eca65c58-goog

Re: efi_loader: efi_variable_parse_signature() returns NULL on error

[Heinrich Schuchardt]

On 07.05.20 09:15, AKASHI Takahiro wrote:
> On Thu, May 07, 2020 at 02:13:18AM +0200, Patrick Wildt wrote:
>> efi_variable_parse_signature() returns NULL on error, so IS_NULL()

IS_NULL() seems to be a typo. I will replace it by IS_ERR() when merging.

>> is an incorrect check.  The goto err leads to pkcs7_free_message(),
>> which works fine on a NULL ptr.
>>
>> Signed-off-by: Patrick Wildt 
>
> Reviewed-by: AKASHI Takahiro 

Hello Takahiro,

there is a second instance of the same error:

lib/efi_loader/efi_variable.c:412:

    msg = pkcs7_parse_message(ebuf, ebuflen);
    free(ebuf);
out:
if (IS_ERR(msg))
return NULL;

Please, send a patch for that one too.

Best regards

Heinrich


>
>>
>> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
>> index 58f8fae358..c5fe896de2 100644
>> --- a/lib/efi_loader/efi_variable.c
>> +++ b/lib/efi_loader/efi_variable.c
>> @@ -519,9 +519,8 @@ static efi_status_t efi_variable_authenticate(u16 
>> *variable,
>>  var_sig = efi_variable_parse_signature(auth->auth_info.cert_data,
>> auth->auth_info.hdr.dwLength
>> - sizeof(auth->auth_info));
>> -if (IS_ERR(var_sig)) {
>> +if (!var_sig) {
>>  debug("Parsing variable's signature failed\n");
>> -var_sig = NULL;
>>  goto err;
>>  }
>>

Re: Bisected: omap_hsmmc 3.3V IO voltage incompatible with N900 (Was: Re: Bisected: mmc cause reboot loops on N900)

[Faiz Abbas]

Pali,


On 26/04/20 3:59 am, Pali Rohár wrote:
> Adding also Faiz to loop...
> 
> On Sunday 26 April 2020 00:20:07 Pali Rohár wrote:
>> On Saturday 25 April 2020 23:26:15 Pali Rohár wrote:
>>> Adding Jean to the loop. Could you please look at this problem? Your
>>> commit (described below) is causing reboot loop on Nokia N900 hardware.
>>>
>>> On Saturday 25 April 2020 13:50:45 Pali Rohár wrote:
 On Saturday 25 April 2020 06:36:58 Adam Ford wrote:
> On Sat, Apr 25, 2020 at 5:42 AM Pali Rohár  wrote:
>>
>> On Thursday 02 April 2020 20:42:31 Pali Rohár wrote:
>>> On Wednesday 01 April 2020 12:32:29 Merlijn Wajer wrote:
>>> ...
 U-Boot 2020.04-rc4-00033-g7dbafe0634-dirty (Apr 01 2020 - 12:15:47 
 +0200)

 OMAP3530-HS ES3.1, CPU-OPP2, L3-165MHz, Max CPU Clock 600 MHz
 Nokia RX-51 + LPDDR/OneNAND
 I2C:   ready
 DRAM:  256 MiB
 NAND:  0 Bytes
>>> ...
 MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1
>>> ...
 OMAP die ID: 0314002404036ac10b01100f
 OMAP3530-HS ES3.1, CPU-OPP2, L3-165MHz, Max CPU Clock 600 MHz

>>>
>>> And then U-Boot freeze, right?
>>>
>>> Any idea how to debug this issue?
>>>
>>> On my N900 I'm getting "data abort" error on display and then instant
>>> reboot.
>>
>> It looks like that omap hs mmc code cause that freeze/reboot on real HW.
>> Was there some significat change to OMAP3 or omap hs mmc?
>
> I booted by board from MMC as shown above.  I also use the pinctrl
> features from the device tree to mux the pins in U-Boot (not SPL), so
> my SPL only does manual muxing the essential components it needs
> during the SPL phase, and lets U-Boot do the rest.   I only  mention
> the pinmux because of message regarding pads/pull-ups.
>
> adam

 I debugged this problem more. I disabled all preboot commands to get
 clean U-Boot setup. And it worked.

 After I called any "mmc" command (e.g. "mmc info") I got that instant
 board reboot. Preboot commands for n900 try to read some files from mmc,
 so this is reason why it get into reboot loop.

 Is there any reason why "mmc info" command can cause "data abort" and
 instant reboot of board?

 And do you know what is needed for proper initialization of omap mmc
 controller for omap3 board? Because it looks like something fundamental
 is missing.

 Currently there are just calls for "omap_mmc_init()" functions and
 "twl4030_power_mmc_init()" functions. See:
 https://gitlab.denx.de/u-boot/u-boot/-/blob/master/board/nokia/rx51/rx51.c
>>>
>>> Now I tried git bisect and here is problematic commit which caused whole
>>> reboot loop:
>>>
>>> 04a2ea248f58b3b6216d0cd0a6b8698df8b14355 is the first bad commit
>>> commit 04a2ea248f58b3b6216d0cd0a6b8698df8b14355
>>> Author: Jean-Jacques Hiblot 
>>> Date:   Thu Sep 21 16:30:08 2017 +0200
>>>
>>> mmc: disable UHS modes if Vcc cannot be switched on and off
>>> 
>>> If a power cycle cannot be done on Vcc, it is safer not to try the UHS
>>> modes because we wouldn't be able to recover from an error occurring
>>> during the UHS initialization.
>>> 
>>> Signed-off-by: Jean-Jacques Hiblot 
>>>
>>> :04 04 04de51428c8311a4b2fb3ad876ac3f6071ab57ee 
>>> ea7a7959a4bd591c92a2c3d413d5643a8457d2ff M  drivers
>>> :04 04 03f639baf2a2f55003cb750981fd8accc5b4a993 
>>> fbcb9607d37959f0b5240f5d727133f58cc35379 M  include
>>>
>>> It changes only core mmc code, nothing platform / board specific.
>>> U-Boot compiled from commit before above has fully working eMMC access
>>> on real Nokia N900. I can read files on FAT eMMC partition without any
>>> problem.
>>>
>>> I'm not sure what is happening here, but it looks like that omap hs mmc
>>> driver used on Nokia N900 is maybe not correctly hooked for UHS support.
>>>
>>> The most suspicious it that this problem cannot be reproduced in qemu
>>> n900 emulator. It happens only on real N900 hw.
>>
>> I took main change from above commit, reverted it on master and U-Boot
>> stopped crashing! No reboot loop anymore. Here is change which fixes
>> reboot loop on Nokia N900:
>>
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index 523c055967..d07c7745da 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -2786,18 +2786,7 @@ int mmc_get_op_cond(struct mmc *mmc)
>>MMC_QUIRK_RETRY_APP_CMD;
>>  #endif
>>  
>> -err = mmc_power_cycle(mmc);
>> -if (err) {
>> -/*
>> - * if power cycling is not supported, we should not try
>> - * to use the UHS modes, because we wouldn't be able to
>> - * recover from an error during the UHS initialization.
>> - */
>> -pr_debug("Unable to do a full power cycle. Disabling the UHS 
>> modes for safety\n");
>> -uhs_en = false;
>> -