Re: [PATCH 12/20] gitlab: Use -w flag for all builds

2020-03-14 Thread Simon Glass
Hi Tom,

On Mon, 9 Mar 2020 at 11:58, Tom Rini  wrote:
>
> On Fri, Mar 06, 2020 at 08:07:26PM -0700, Simon Glass wrote:
> > Avoid needing to know about the internal .bm-work directory, by passing
> > the -w flag to buildman.
> >
> > Also drop the repeated call to buildman since the first one should show
> > all the expected output. We only need to use -s if we are building
> > multiple boards and want the errors to be coalesced. In this case we are
> > only building a single board.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> >  .gitlab-ci.yml | 9 -
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index b29d59d942..bbd05aa872 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -29,11 +29,11 @@ stages:
> >script:
> >  # From buildman, exit code 129 means warnings only.  If we've been 
> > asked to
> >  # use clang only do one configuration.
> > +- export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}
> >  - ret=0;
> > -  tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} 
> > ${OVERRIDE}
> > -|| ret=$?;
> > +  tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E
> > +--board ${TEST_PY_BD} ${OVERRIDE} || ret=$?;
> >if [[ $ret -ne 0 && $ret -ne 129 ]]; then
> > -tools/buildman/buildman -o /tmp -seP --board ${TEST_PY_BD};
> >  exit $ret;
> >fi
>
> The repeated call is so that when we have a CI error from buildman the
> error is at the bottom of the output and we don't have to hunt for it,
> so I'm not sure this is a developer-friendly change.

I don't quite get this, since the two buildman calls are one after the
other. What difference do you see in the output?

Regards,
Simon


Re: [PATCH 00/20] gitlab: Simplify the test script

2020-03-14 Thread Simon Glass
Hi Tom,

On Mon, 9 Mar 2020 at 11:55, Tom Rini  wrote:
>
> On Fri, Mar 06, 2020 at 08:07:14PM -0700, Simon Glass wrote:
>
> > At present there are several things in the gitlab script which work around
> > limitations in buildman. With a few small feature additions these can be
> > removed.
> >
> > This series adds some new features to buildman and simplifies the script:
> > - Option to run a single build in a specified output directory
> > - Allow ignoring warnings
> > - Removes a restriction on the build output directory
> >
> > It also
> > - moves test.py over to use buildman for the --build option
> > - makes one change to azure since the same approach should be possible there
> > - fixes a few minor problems noticed in main and sandbox docs
>
> One general comment is that while it's clear from this series that
> you're focusing on test.py invocation most of the time, a lot of the
> commit messages aren't clear that you're changing the
> buildman_and_testpy_template stanza.

I'm not sure what you are looking for there. Do you want the commit
message to mention which part of the gitlab file is being changed?

>
> Also, while you're keeping Azure in sync (as it's largely based on the
> GitLab logic), Travis is being left out.  But the GitLab logic is based
> on the Travis logic and as much as I dislike Travis for being slow and
> having network induced failures, it's still a valid and supported CI
> flow that we can't drop, so please keep it in sync with these
> improvements, thanks!

OK.

Regards,
Simon


Re: [PATCHv2 2/8] Kconfig: Remove redundant variable sets

2020-03-14 Thread Masahiro Yamada
On Thu, Mar 12, 2020 at 7:11 AM Tom Rini  wrote:
>
> In a few places we have Kconfig entries that set SPL_LDSCRIPT to what is
> the default value anyways.  Drop these.
>
> Cc: Michal Simek 
> Cc: Rick Chen 
> Cc: Philippe Reynes 
> Cc: Eric Jarrige 
> Signed-off-by: Tom Rini 
> --

Reviewed-by: Masahiro Yamada 


-- 
Best Regards
Masahiro Yamada


Re: [PATCHv2 1/8] spl: Kconfig: Escape '$(ARCH)' in LDSCRIPT entries

2020-03-14 Thread Masahiro Yamada
On Thu, Mar 12, 2020 at 7:11 AM Tom Rini  wrote:
>
> The default SPL / TPL linker script is in the $(ARCH) directory.  The
> way we use this today works but isn't ideal.  With an update to Kconfig
> to re-sync with the Linux Kernel, we need to escape the '$' here so that
> it will end up being evaluated by make.
>
> Cc: Masahiro Yamada 
> Signed-off-by: Tom Rini 


Reviewed-by: Masahiro Yamada 




-- 
Best Regards
Masahiro Yamada


Re: [PATCHv2 7/8] kconfig / kbuild: re-sync with Linux 4.18

2020-03-14 Thread Masahiro Yamada
Hi Tom,

On Thu, Mar 12, 2020 at 7:11 AM Tom Rini  wrote:
>
> Align Kconfig and Kbuild logic to Linux 4.18 release with minimal impact
> on files outside of this scope.
>
> Our previous Kconfig sync was done by commit e91610da7c8a ("kconfig:
> re-sync with Linux 4.17-rc4").
>
> A very small number of changes upstream since our sync with v4.17-rc4
> that exist in the v4.18 release have already been applied here and have
> been omitted from the list in this commit (and are readily available in
> our own git history).
>
> The imported Linux commits are:
> [From priot to v4.17-rc4]


Just a nit.

"priot" -> "prior"


Other than that, looks good to me.

Reviewed-by: Masahiro Yamada 


Thanks.


> 39a33ff80a25 kbuild: remove cc-option-align
> db547ef19064 Kbuild: don't add obj tree in additional includes
> b999596b963a Kbuild: don't add ../../ to include path
>
> [From v4.17 to v4.18]
> b3aa58d2e85d fixdep: suppress consecutive / from file paths in dependency 
> list files
> 74656b682902 kbuild: disable new dtc graph and unit-address warnings
> 74d931716151 genksyms: remove symbol prefix support
> e6ecfb45072c kbuild: do not display CHK for filechk
> 0b669a5076fd kconfig: refactor Qt package checks for building qconf
> b464ef583dc7 kconfig: refactor GTK+ package checks for building gconf
> 1c5af5cf9308 kconfig: refactor ncurses package checks for building mconf and 
> nconf
> 694c49a7c01c kconfig: drop localization support
> 96f60dfa5819 trace: Use -mcount-record for dynamic ftrace
> bb222ceeb327 kconfig: remove string expansion in file_lookup()
> 96d8e48da55a kconfig: remove string expansion for mainmenu after yyparse()
> 5b31a9746756 kconfig: remove sym_expand_string_value()
> 137c0118a900 kconfig: make default prompt of mainmenu less specific
> e298f3b49def kconfig: add built-in function support
> 2fd5b09c201e kconfig: add 'shell' built-in function
> 9de071536c87 kconfig: begin PARAM state only when seeing a command keyword
> 9ced3bddec08 kconfig: support user-defined function and recursively expanded 
> variable
> 1175c02506ff kconfig: support simply expanded variable
> ed2a22f277c6 kconfig: support append assignment operator
> 82bc8bd82e5c kconfig: expand lefthand side of assignment statement
> 1d6272e6fe43 kconfig: add 'info', 'warning-if', and 'error-if' built-in 
> functions
> a702a6176e2f kconfig: add 'filename' and 'lineno' built-in variables
> 915f64901eb3 kconfig: error out if a recursive variable references itself
> 2bece88f89fa kconfig: test: add Kconfig macro language tests
> 21c54b774744 kconfig: show compiler version text in the top comment
> 59f7b5847b0c kbuild: $(CHECK) doesnt need NOSTDINC_FLAGS twice
> 145167650b96 kbuild: add endianness flag to CHEKCFLAGS
> 1f2f01b122d7 kbuild: add machine size to CHECKFLAGS
> d6a0c8a1326b kconfig: Add testconfig into make help output
> bb6d83dde191 kbuild: Move last word of nconfig help to the previous line
> 8593080c0fcf kconfig: fix localmodconfig
> ed7d40bc67b8 tracing: Fix SKIP_STACK_VALIDATION=1 build due to bad merge with 
> -mrecord-mcount
> b2d00d7c61c8 kconfig: fix line numbers for if-entries in menu tree
> ecd53ac2f2c6 kconfig: handle P_SYMBOL in print_symbol()
> 73d1c580f92b kconfig: loop boundary condition fix
> 48f6e3cf5bc6 kbuild: do not drop -I without parameter
> bd412d81b7ea kbuild: .PHONY is not a variable, but PHONY is
> 6916162c7308 kbuild: remove duplicated comments about PHONY
>
> Cc: Masahiro Yamada 
> Signed-off-by: Tom Rini 
> ---


-- 
Best Regards
Masahiro Yamada


[PATCH] [RFC] net: smc911x: Drop the standalone EEPROM example

2020-03-14 Thread Marek Vasut
Drop the example, for two reasons. First, it is tapping directly into
the IO accessors of the SMC911x, while it should instead go through
the net device API. Second, this makes conversion of the SMC911x driver
to DM real hard.

Signed-off-by: Marek Vasut 
Cc: Joe Hershberger 
Cc: Tom Rini 
---
 examples/standalone/Makefile |   1 -
 examples/standalone/smc911x_eeprom.c | 379 ---
 2 files changed, 380 deletions(-)
 delete mode 100644 examples/standalone/smc911x_eeprom.c

diff --git a/examples/standalone/Makefile b/examples/standalone/Makefile
index 0b17a91804..5c8d56c92c 100644
--- a/examples/standalone/Makefile
+++ b/examples/standalone/Makefile
@@ -5,7 +5,6 @@
 
 extra-y:= hello_world
 extra-$(CONFIG_SMC9)   += smc9_eeprom
-extra-$(CONFIG_SMC911X)+= smc911x_eeprom
 extra-$(CONFIG_SPI_FLASH_ATMEL)+= atmel_df_pow2
 extra-$(CONFIG_PPC)+= sched
 
diff --git a/examples/standalone/smc911x_eeprom.c 
b/examples/standalone/smc911x_eeprom.c
deleted file mode 100644
index 2c05ed902d..00
--- a/examples/standalone/smc911x_eeprom.c
+++ /dev/null
@@ -1,379 +0,0 @@
-/*
- * smc911x_eeprom.c - EEPROM interface to SMC911x parts.
- * Only tested on SMSC9118 though ...
- *
- * Copyright 2004-2009 Analog Devices Inc.
- *
- * Licensed under the GPL-2 or later.
- *
- * Based on smc9_eeprom.c which:
- * Heavily borrowed from the following peoples GPL'ed software:
- *  - Wolfgang Denk, DENX Software Engineering, w...@denx.de
- *   Das U-Boot
- *  - Ladislav Michl la...@linux-mips.org
- *   A rejected patch on the U-Boot mailing list
- */
-
-#include 
-#include 
-#include 
-#include 
-#include "../drivers/net/smc911x.h"
-
-/**
- * smsc_ctrlc - detect press of CTRL+C (common ctrlc() isnt exported!?)
- */
-static int smsc_ctrlc(void)
-{
-   return (tstc() && getc() == 0x03);
-}
-
-/**
- * usage - dump usage information
- */
-static void usage(void)
-{
-   puts(
-   "MAC/EEPROM Commands:\n"
-   " P : Print the MAC addresses\n"
-   " D : Dump the EEPROM contents\n"
-   " M : Dump the MAC contents\n"
-   " C : Copy the MAC address from the EEPROM to the MAC\n"
-   " W : Write a register in the EEPROM or in the MAC\n"
-   " Q : Quit\n"
-   "\n"
-   "Some commands take arguments:\n"
-   " W   \n"
-   "E: EEPROM   M: MAC\n"
-   );
-}
-
-/**
- * dump_regs - dump the MAC registers
- *
- * Registers 0x00 - 0x50 are FIFOs.  The 0x50+ are the control registers
- * and they're all 32bits long.  0xB8+ are reserved, so don't bother.
- */
-static void dump_regs(struct eth_device *dev)
-{
-   u8 i, j = 0;
-   for (i = 0x50; i < 0xB8; i += sizeof(u32))
-   printf("%02x: 0x%08x %c", i,
-   smc911x_reg_read(dev, i),
-   (j++ % 2 ? '\n' : ' '));
-}
-
-/**
- * do_eeprom_cmd - handle eeprom communication
- */
-static int do_eeprom_cmd(struct eth_device *dev, int cmd, u8 reg)
-{
-   if (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY) {
-   printf("eeprom_cmd: busy at start (E2P_CMD = 0x%08x)\n",
-   smc911x_reg_read(dev, E2P_CMD));
-   return -1;
-   }
-
-   smc911x_reg_write(dev, E2P_CMD, E2P_CMD_EPC_BUSY | cmd | reg);
-
-   while (smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY)
-   if (smsc_ctrlc()) {
-   printf("eeprom_cmd: timeout (E2P_CMD = 0x%08x)\n",
-   smc911x_reg_read(dev, E2P_CMD));
-   return -1;
-   }
-
-   return 0;
-}
-
-/**
- * read_eeprom_reg - read specified register in EEPROM
- */
-static u8 read_eeprom_reg(struct eth_device *dev, u8 reg)
-{
-   int ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_READ, reg);
-   return (ret ? : smc911x_reg_read(dev, E2P_DATA));
-}
-
-/**
- * write_eeprom_reg - write specified value into specified register in 
EEPROM
- */
-static int write_eeprom_reg(struct eth_device *dev, u8 value, u8 reg)
-{
-   int ret;
-
-   /* enable erasing/writing */
-   ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_EWEN, reg);
-   if (ret)
-   goto done;
-
-   /* erase the eeprom reg */
-   ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_ERASE, reg);
-   if (ret)
-   goto done;
-
-   /* write the eeprom reg */
-   smc911x_reg_write(dev, E2P_DATA, value);
-   ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_WRITE, reg);
-   if (ret)
-   goto done;
-
-   /* disable erasing/writing */
-   ret = do_eeprom_cmd(dev, E2P_CMD_EPC_CMD_EWDS, reg);
-
- done:
-   return ret;
-}
-
-/**
- * skip_space - find first non-whitespace in given pointer
- */
-static char *skip_space(char *buf)
-{
-   while (isblank(buf[0]))
-   ++buf;
-   return buf;
-}
-
-/**
- * 

[PATCH] net: smc911x: Remove pkt_data_{push,pull}

2020-03-14 Thread Marek Vasut
These functions are never used and are likely a pre-DM remnant
from times long past, just remove them.

Signed-off-by: Marek Vasut 
Cc: Joe Hershberger 
---
 drivers/net/smc911x.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
index 257b0385c2..24b4eaeb3f 100644
--- a/drivers/net/smc911x.c
+++ b/drivers/net/smc911x.c
@@ -13,11 +13,6 @@
 
 #include "smc911x.h"
 
-u32 pkt_data_pull(struct eth_device *dev, u32 addr) \
-   __attribute__ ((weak, alias ("smc911x_reg_read")));
-void pkt_data_push(struct eth_device *dev, u32 addr, u32 val) \
-   __attribute__ ((weak, alias ("smc911x_reg_write")));
-
 static void smc911x_handle_mac_address(struct eth_device *dev)
 {
unsigned long addrh, addrl;
@@ -157,7 +152,7 @@ static int smc911x_send(struct eth_device *dev, void 
*packet, int length)
tmplen = (length + 3) / 4;
 
while (tmplen--)
-   pkt_data_push(dev, TX_DATA_FIFO, *data++);
+   smc911x_reg_write(dev, TX_DATA_FIFO, *data++);
 
/* wait for transmission */
while (!((smc911x_reg_read(dev, TX_FIFO_INF) &
@@ -203,7 +198,7 @@ static int smc911x_rx(struct eth_device *dev)
 
tmplen = (pktlen + 3) / 4;
while (tmplen--)
-   *data++ = pkt_data_pull(dev, RX_DATA_FIFO);
+   *data++ = smc911x_reg_read(dev, RX_DATA_FIFO);
 
if (status & RX_STS_ES)
printf(DRIVERNAME
-- 
2.25.0



[PATCH] ARM: rmobile: Convert Gen2 Blanche to DM_SPI{,_FLASH}

2020-03-14 Thread Marek Vasut
Enable DM_SPI and DM_SPI_FLASH in U-Boot on V2H Blanche.

Signed-off-by: Marek Vasut 
Cc: Nobuhiro Iwamatsu 
---
 configs/blanche_defconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configs/blanche_defconfig b/configs/blanche_defconfig
index 5c7b517e7c..64b7bccfb4 100644
--- a/configs/blanche_defconfig
+++ b/configs/blanche_defconfig
@@ -24,7 +24,6 @@ CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
 CONFIG_CMD_PCI=y
 CONFIG_CMD_SDRAM=y
-CONFIG_CMD_SF=y
 CONFIG_CMD_SPI=y
 CONFIG_CMD_USB=y
 CONFIG_CMD_DHCP=y
@@ -53,7 +52,7 @@ CONFIG_MTD=y
 CONFIG_MTD_NOR_FLASH=y
 CONFIG_FLASH_CFI_DRIVER=y
 CONFIG_SYS_FLASH_CFI=y
-CONFIG_SPI_FLASH=y
+CONFIG_DM_SPI_FLASH=y
 CONFIG_SPI_FLASH_SPANSION=y
 CONFIG_SMC911X=y
 CONFIG_SMC911X_BASE=0x1800
@@ -68,6 +67,7 @@ CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_DM_REGULATOR_GPIO=y
 CONFIG_SCIF_CONSOLE=y
 CONFIG_SPI=y
+CONFIG_DM_SPI=y
 CONFIG_SH_QSPI=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
-- 
2.25.0



Re: [PATCH 1/2] sandbox: add reserved-memory node in device tree

2020-03-14 Thread Simon Glass
On Sat, 14 Mar 2020 at 05:13, Heinrich Schuchardt  wrote:
>
> For testing the handling of memory reservations create a reserved-memory
> node in sandbox.dts and sandbox64.dts.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  arch/sandbox/dts/sandbox.dts   | 19 +++
>  arch/sandbox/dts/sandbox64.dts | 20 
>  2 files changed, 39 insertions(+)
>

Reviewed-by: Simon Glass 


Re: [PATCH 2/2] sandbox: implement ft_board_setup()

2020-03-14 Thread Simon Glass
On Sat, 14 Mar 2020 at 05:13, Heinrich Schuchardt  wrote:
>
> Currently we are not able to test reservations created by ft_board_setup().
>
> Implement ft_board_setup() to create an arbitrary reservation and enable
> OF_BOARD_SETUP.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  arch/Kconfig| 1 +
>  board/sandbox/sandbox.c | 6 ++
>  2 files changed, 7 insertions(+)

Reviewed-by: Simon Glass 


Re: [PATCH 084/108] x86: acpi: Support generation of the DBG2 table

2020-03-14 Thread Simon Glass
Hi Leif,

On Mon, 27 Jan 2020 at 05:34, Leif Lindholm  wrote:
>
> Hi Simon,
>
> A portion of this set is very much not x86-specific. (more below)
>
> On Sun, Jan 26, 2020 at 22:06:31 -0700, Simon Glass wrote:
> > Add an implementation of the DBG2 (Debug Port Table 2) ACPI table.
> > Adjust one of the header includes to be in the correct order, before
> > adding more.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> >  arch/x86/lib/acpi_table.c | 112 +-
> >  include/acpi_table.h  |  31 +++
> >  2 files changed, 142 insertions(+), 1 deletion(-)
> >

[..]

> > +void acpi_create_dbg2(struct acpi_dbg2_header *dbg2,
> > +   int port_type, int port_subtype,
> > +   struct acpi_gen_regaddr *address, u32 address_size,
> > +   const char *device_path)
>
> But this one would be usable by any of the architectures supported by
> ACPI. Could the acpi_create_* functions be moved somewhere
> non-arch-specific, like a new lib/acpi?
>
> /
> Leif

OK I'll move this one. In general I don't know which ones are generic
and I'd rather keep them x86-specific unless we know.

Regards,
Simon


Re: [PATCH 2/2] test: verbatim character entry with CTRL-V

2020-03-14 Thread Simon Glass
Hi Heinrich,

On Sat, 25 Jan 2020 at 03:45, Heinrich Schuchardt  wrote:
>
> On 1/25/20 12:43 AM, Tom Rini wrote:
> > On Sun, Jan 12, 2020 at 05:33:39PM +0100, Heinrich Schuchardt wrote:
> >
> >> Provide a unit test checking that CTRL-V can be used to add control
> >> characters to the line buffer.
> >>
> >> Signed-off-by: Heinrich Schuchardt 
> >
> > I don't know why but this test, run when built with clang fails.  A log
> > is here: https://gitlab.denx.de/u-boot/u-boot/-/jobs/50540 but I saw it
> > on Travis and Azure as well.
> >
>
> Hello Tom, hello Simon
>
> when I clang U-Boot branch bisect-failure with in the Gitlab Docker
> image and run
>
> ./u-boot -d arch/sandbox/dts/test.dtb
> ut dm usb_keyb
>
> the test runs fine.
>
> When I run
>
> valgrind ./u-boot -d arch/sandbox/dts/test.dtb
> usb start
> ut dm usb_keyb
>
> I see more than 100 use-after-free errors.
>
> dm_test_destroy() calling uclass_destroy() seems to be the culprit.
>
> @Simon:
> Why would we destroy any uclass in `ut dm` that was in use before
> executing the command?

This is because the tests might require them to be in their initial state.

In general running a unit test along with other code is not safe.

I wonder if the situation has improved with the move to SDL2, but I'm not sure.
[..]

Regards,
Simon


Re: [PATCH 1/1] cmd: map addresses to sysmem in efidebug memmap

2020-03-14 Thread Simon Glass
On Sat, 14 Mar 2020 at 02:21, Heinrich Schuchardt  wrote:
>
> Addresses in the sandbox's device tree are in the sandbox's virtual address
> space. If we want to compare memory reservations in the device-tree with
> the output of 'efidebug memmap', we need to convert back to this address
> space.
>
> Adjust the output of the 'efidebug memmap' command.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  cmd/efidebug.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH 1/1] sandbox: enable CMD_BOOTEFI_HELLO and CMD_EFIDEBUG

2020-03-14 Thread Simon Glass
On Sat, 14 Mar 2020 at 05:27, Heinrich Schuchardt  wrote:
>
> 'bootefi hello' is used in one of the Python tests.
>
> efidebug can be used to verify the correct initialization of the UEFI
> sub-system.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  configs/sandbox64_defconfig| 2 ++
>  configs/sandbox_defconfig  | 1 +
>  configs/sandbox_flattree_defconfig | 2 ++
>  configs/sandbox_spl_defconfig  | 2 ++
>  4 files changed, 7 insertions(+)
>

I have to wonder if we should add this as an 'imply' in arch/Kconfig?

Reviewed-by: Simon Glass 


Re: [PATCH v3 1/9] image: android: Add functions for handling dtb field

2020-03-14 Thread Simon Glass
Hi Sam,

On Fri, 10 Jan 2020 at 08:33, Sam Protsenko  wrote:
>
> Hi Simon,
>
> On Wed, Jan 8, 2020 at 7:39 PM Simon Glass  wrote:
> >
> > Hi Sam,
> >
> > On Tue, 24 Dec 2019 at 12:55, Sam Protsenko  
> > wrote:
> > >
> > > Android Boot Image v2 adds "DTB" payload (and corresponding field in the
> > > image header). Provide functions for its handling:
> > >
> > >   - android_image_get_dtb_by_index(): Obtain DTB blob from "DTB" part of
> > > boot image, by blob's index
> > >   - android_image_print_dtb_contents(): Iterate over all DTB blobs in
> > > "DTB" part of boot image and print those blobs info
> > >
> > > "DTB" payload might be in one of the following formats:
> > >   1. concatenated DTB blobs
> > >   2. Android DTBO format
> > >
> > > The latter requires "android-image-dt.c" functionality, so this commit
> > > selects that file for building for CONFIG_ANDROID_BOOT_IMAGE option.
> > >
> > > Right now this new functionality isn't used, but it can be used further.
> > > As it's required to apply some specific dtbo blob(s) from "dtbo"
> > > partition, we can't automate this process inside of "bootm" command. But
> > > we can do next:
> > >   - come up with some new command like "abootimg" to extract dtb blob
> > > from boot image (using functions from this patch)
> > >   - extract desired dtbo blobs from "dtbo" partition using "adtimg"
> > > command
> > >   - merge dtbo blobs into dtb blob using "fdt apply" command
> > >   - pass resulting dtb blob into bootm command in order to boot the
> > > Android kernel with Android ramdisk from boot image
> > >
> > > Signed-off-by: Sam Protsenko 
> > > ---
> > >  common/Makefile|   2 +-
> > >  common/image-android.c | 214 +
> > >  include/image.h|   5 +
> > >  3 files changed, 220 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/common/Makefile b/common/Makefile
> > > index 029cc0f2ce..1ffddc2f94 100644
> > > --- a/common/Makefile
> > > +++ b/common/Makefile
> > > @@ -108,7 +108,7 @@ endif
> > >
> > >  obj-y += image.o
> > >  obj-$(CONFIG_ANDROID_AB) += android_ab.o
> > > -obj-$(CONFIG_ANDROID_BOOT_IMAGE) += image-android.o
> > > +obj-$(CONFIG_ANDROID_BOOT_IMAGE) += image-android.o image-android-dt.o
> > >  obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += image-fdt.o
> > >  obj-$(CONFIG_$(SPL_TPL_)FIT) += image-fit.o
> > >  obj-$(CONFIG_$(SPL_)MULTI_DTB_FIT) += boot_fit.o common_fit.o
> > > diff --git a/common/image-android.c b/common/image-android.c
> > > index 3564a64221..1ccad6c556 100644
> > > --- a/common/image-android.c
> > > +++ b/common/image-android.c
> > > @@ -6,10 +6,12 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> >
> > Prefer underscores if possible.
> >
>
> It's existing header file, related to another feature. So it can be
> renamed in some separate patch outside of this patch series. Btw, is
> there any background on why we should stick to underscore in file
> names?

I added a comment here:

https://www.denx.de/wiki/U-Boot/CodingStyle

>
> > >  #include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >
> > >  #define ANDROID_IMAGE_DEFAULT_KERNEL_ADDR  0x10008000
> > >
> > > @@ -195,6 +197,121 @@ int android_image_get_second(const struct 
> > > andr_img_hdr *hdr,
> > > return 0;
> > >  }
> > >
> > > +/**
> > > + * android_image_get_dtb_img_addr() - Get the address of DTB area in 
> > > boot image.
> > > + * @hdr_addr: Boot image header address
> > > + * @addr: Will contain the address of DTB area in boot image
> > > + *
> > > + * Return: true on success or false on fail.
> > > + */
> > > +static bool android_image_get_dtb_img_addr(ulong hdr_addr, ulong *addr)
> > > +{
> > > +   const struct andr_img_hdr *hdr;
> > > +   ulong dtb_img_addr;
> > > +   bool res = true;
> >
> > Perhaps this isn't universal but at least with DM we use 'ret' for
> > return code instead of 'res' for result.
> >
>
> I think it's just a matter of taste. Not a major one, right?

Not major perhaps, but this sort of consistency makes the code much
easier to read, IMO. Using error codes as return values is pretty
universal in U-Boot so I think you should do that instead of
true/false.

>
> > > +
> > > +   hdr = map_sysmem(hdr_addr, sizeof(*hdr));
> > > +   if (android_image_check_header(hdr)) {
> > > +   printf("Error: Boot Image header is incorrect\n");
> > > +   res = false;
> >
> > Could this function return an error code?
> >
>
> Frankly, don't see much value in error code in this particular case.
> All we can do to handle any error in this function further is just to
> print corresponding error message, which I do in this function already
> (sticking to principle to print error messages where we actually know
> what happened). So I'd stick to just bool, if you don't mind, to not
> over-complicate this without actual reason.

It is not a great idea to print an error message in a leaf function.

Re: [PATCH] qemu: arm: Scan the pci bus in board_init

2020-03-14 Thread Simon Glass
Hi Heinrich,

On Tue, 31 Dec 2019 at 09:59, Heinrich Schuchardt  wrote:
>
> On 12/31/19 4:49 PM, Sughosh Ganu wrote:
> >
> > On Tue, 31 Dec 2019 at 20:38, Heinrich Schuchardt  > > wrote:
> >
> > On 12/31/19 2:31 PM, Sughosh Ganu wrote:
> >  > Scan the pci bus in board_init routine before scanning the virtio
> >  > devices. This enumerates all the virtio devices, including devices
> >  > found on the pci bus.
> >  >
> >  > Signed-off-by: Sughosh Ganu  > >
> >  > ---
> >  >  board/emulation/qemu-arm/qemu-arm.c | 7 +++
> >  >  1 file changed, 7 insertions(+)
> >  >
> >  > diff --git a/board/emulation/qemu-arm/qemu-arm.c
> > b/board/emulation/qemu-arm/qemu-arm.c
> >  > index 4e18733..6c5335c 100644
> >  > --- a/board/emulation/qemu-arm/qemu-arm.c
> >  > +++ b/board/emulation/qemu-arm/qemu-arm.c
> >  > @@ -63,6 +63,13 @@ struct mm_region *mem_map = qemu_arm64_mem_map;
> >  >  int board_init(void)
> >  >  {
> >  >   /*
> >  > +  * Scan the pci bus before calling virtio_init. This
> >  > +  * enumerates all virtio devices, including devices
> >  > +  * on the pci bus.
> >
> > The last sentence is easy to misunderstand. You surely do not mean that
> > pci_init() would enumerate all virtio devices including mmio virtio
> > devices.
> >
> > I would suggest to rephrase it to "This enumerates all pci devices
> > including virtio devices on the pci bus."
> >
> >
> > Ok.
> >
> >
> > With the patch I no longer need to run `virtio scan` on
> > qemu_arm64_defonfig before using the rng command supplied in patch
> >
> > [PATCH 1/1] cmd: add rng command
> > https://lists.denx.de/pipermail/u-boot/2019-December/394539.html
> >
> > Unfortunately you only considered the ARM architecture. We need a
> > solution that works on all QEMU architectures (MIPS, RISC-V, X86, ARM).
> >
> >
> > That is fine with me, but currently, only risc-v and arm architectures
> > are calling virtio_init.
> >
> >
> > How about eliminating board_init() for ARM and RISC-V and moving
> > virtio_init() to common/board_r.c after initr_pci?
> >
> >
> > That can be done, but currently, for some reason, initr_pci function
> > calls pci_init only when CONFIG_DM_PCI is not defined. Not sure why that
> > is the case, although initr_pci gets called after initr_dm.
> >
> > -sughosh
>
> This dates back to Simon's commit ff3e077bd23c ("dm: pci: Add a uclass
> for PCI").
>
> I hope that Simon can tell us why he chose not to call pci_init() if
> DM_PCI is defined.

Driver model uses lazy init. We have discussed this before, and
considered using a uclass flag to tell U-Boot that the uclass needs to
be fully probed (i.e. probe all devices in that uclass, recursively).

But I'm not sure we got agreement. If you think it is a good idea you
could send a patch. The only thing is that I think some platforms
don't need PCI to boot so might not want this?

Regards,
Simon


Re: [RFC PATCH] cmd: mp: change the command name from cpu to mp

2020-03-14 Thread Simon Glass
Hi,

On Sat, 14 Mar 2020 at 06:47, Pragnesh Patel  wrote:
>
> when CONFIG_CMD_CPU and CONFIG_MP both are enabled, u-boot compilation
> gives an error of "multiple definition of `_u_boot_list_2_cmd_2_cpu'"
> so cpu command(cmd/cpu.c) and mp command(cmd/mp.c) should have different
> command name.
>
> Signed-off-by: Pragnesh Patel 
> ---
>  cmd/mp.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/cmd/mp.c b/cmd/mp.c
> index 4c8f5fc3fa..26995c9976 100644
> --- a/cmd/mp.c
> +++ b/cmd/mp.c
> @@ -68,11 +68,11 @@ cpu_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * 
> const argv[])
>
>  #ifdef CONFIG_SYS_LONGHELP
>  static char cpu_help_text[] =
> -   " reset - Reset cpu \n"
> -   "cpu status  - Status of all cpus\n"
> -   "cpu  status- Status of cpu \n"
> -   "cpu  disable   - Disable cpu \n"
> -   "cpu  release  [args] - Release cpu  at  with 
> [args]"
> +   "mp  reset - Reset cpu \n"
> +   "mp status  - Status of all cpus\n"
> +   "mp  status- Status of cpu \n"
> +   "mp  disable   - Disable cpu \n"
> +   "mp  release  [args] - Release cpu  at  with 
> [args]"

This interface should be migrated to driver model (using UCLASS_CPU) instead.

Regards,
Simon


Re: `uclass_find_device_by_name` fails to find the device

2020-03-14 Thread Simon Glass
Hi Nandor,

On Tue, 7 Jan 2020 at 02:16, Nandor Han  wrote:
>
>
> Hi Simon and thanks,
>
> On 2019-12-24 17:58, Simon Glass wrote:
>
> 
>
> >
> > Can you use a phandle to refer to the device from another node,
> > perhaps a UCLASS_BOARD node if needed? It is not a good idea to use
> > this function.
>
> I'm not sure, I'll have to check that.

OK, that is the preferred method.

> Anyway
> `uclass_find_device_by_name` is used by `uclass_get_device_by_name`,
> which already seems to be used in plenty of places.
>
> >
> > Alternatively we could add a new function which finds by partial name.
>
> My fix was to ignore the suffix starting from "@", if that exist, since
> that's more like metadata...and compare entire string when "@" is missing.
>
> Something like:
>
>  uclass_foreach_dev(dev, uc) {
> -   if (!strcmp(dev->name, name)) {
> +   char *at_char = strchr(dev->name, '@');
> +   int name_length;
> +
> +   if (at_char)
> +   name_length = at_char - dev->name;
> +   else
> +   name_length = max(strlen(dev->name), strlen(name));
> +
> +   if (!strncmp(dev->name, name, name_length)) {
>  *devp = dev;
>  return 0;
>  }
>
> I could probably to prepare an RFC with this change.

I think this is OK as a new function, but do add a comment in the
header that it should not be used and phandles should be used instead.
Also please remember to add a test.

Regards,
Simon


Re: [PATCH 034/108] x86: acpi: Move MADT up a bit

2020-03-14 Thread Simon Glass
On Mon, 27 Jan 2020 at 04:02, Leif Lindholm  wrote:
>
> On Sun, Jan 26, 2020 at 22:05:41 -0700, Simon Glass wrote:
> > Put this table before MCFG so that it matches the order that coreboot uses
> > when passing tables to Linux. This is a cosmetic change since the order of
> > the tables does not otherwise matter.
>
> The patch looks like it's doing the opposite of what the commit
> message says. Rebasing issue, or am I being daft? (If this is a stack
> operation, use of stack terminology in the commit message would be
> more daft-friendly.)

Ooops, thanks, fixed!


- Simon


Re: [PATCH v2 30/39] acpi: Add functions to generate ACPI code

2020-03-14 Thread Simon Glass
Hi Wolfgang,

On Fri, 13 Mar 2020 at 03:55, Wolfgang Wallner
 wrote:
>
> Hi Simon,
>
> -"Simon Glass"  schrieb: -
> >
> > Sometimes we need to generate ACPI code on the fly based on things only
> > known at run time. Add a new 'acpigen' library to handle this. This code
> > comes from coreboot and has been modified to support the acpi_ctx struct.
> >
> > Also add acpi_device.c to the build, since these files are co-dependent.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> > Changes in v2: None
> >
> >  include/acpigen.h  |  482 +
> >  include/dm/acpi.h  |7 +
> >  include/irq.h  |2 +
> >  lib/acpi/Makefile  |2 +
> >  lib/acpi/acpigen.c | 1683 
> >  5 files changed, 2176 insertions(+)
> >  create mode 100644 include/acpigen.h
> >  create mode 100644 lib/acpi/acpigen.c
>
> What coreboot version is this based on?
> I have compared it to the current master in coreboot and there are some
> differences (ignoring the changes for acpi_ctx support).

I don't think coreboot does releases very often. The commit is efa3084d63.

Regards,
Simon


Re: [PATCH] usb: ehci: Fix "EHCI timed out on TD - token=XXXX" error on ehci-hcd

2020-03-14 Thread Marek Vasut
On 3/2/20 6:00 PM, Tom Rini wrote:
> On Mon, Mar 02, 2020 at 01:39:49AM +0100, Marek Vasut wrote:
>> On 3/2/20 12:04 AM, Tom Rini wrote:
>> [...]
>>
> 3 USB Device(s) found
>scanning usb for ethernet devices... 0 Ethernet Device(s) found
> Hit any key to stop autoboot:  2  0 
> BeagleBoard # usb tree
> USB device tree:
>   1  Hub (480 Mb/s, 0mA)
>   |  u-boot EHCI Host Controller 
>   |
>   |+-2  Hub (480 Mb/s, 2mA)
> |
> |+-3  See Interface (480 Mb/s, 0mA)
>  ??? ??? ???
>
> BeagleBoard # 
>
> Note that the hub and ethernet are on-SBC and not something I'm plugging
> in.  Thanks!

 The device #3 is a usb mass storage or what is it ?
>>>
>>> It's a usb ethernet device.
>>>
 Can you try and implement usb_get_max_xfer_size for musb and make it
 report 240*512 unconditionally (*size = 240*512; return 0;) ? I think
 that would "fix" it for you on omap too.
>>>
>>> I'll pencil in some time to try that, thanks.
>>
>> That's not gonna help you with USB ethernet. I recall seeing flakiness
>> with asix devices, maybe that's what you're running into?
>>
>> Do you have any further details on that device ? lsusb -vvv would help.
> 
> I can only give:
> 
> BeagleBoard # usb info
> 1: Hub,  USB Revision 2.0
>  - u-boot EHCI Host Controller
>  - Class: Hub
>  - PacketSize: 64  Configurations: 1
>  - Vendor: 0x  Product 0x Version 1.0
>Configuration: 1
>- Interfaces: 1 Self Powered 0mA
>  Interface: 0
>  - Alternate Setting 0, Endpoints: 1
>  - Class Hub
>  - Endpoint 1 In Interrupt MaxPacket 8 Interval 255ms
> 
> 2: Hub,  USB Revision 2.0
>  - Class: Hub
>  - PacketSize: 64  Configurations: 1
>  - Vendor: 0x0424  Product 0x9514 Version 2.0
>Configuration: 1
>- Interfaces: 1 Self Powered Remote Wakeup 2mA
>  Interface: 0
>  - Alternate Setting 0, Endpoints: 1
>  - Class Hub
>  - Endpoint 1 In Interrupt MaxPacket 1 Interval 12ms
>  - Endpoint 1 In Interrupt MaxPacket 1 Interval 12ms
> 
> 3: Vendor specific,  USB Revision 2.0
>  - Class: Vendor specific
>  - PacketSize: 64  Configurations: 1
>  - Vendor: 0x0424  Product 0xec00 Version 2.0
>Configuration: 1
>- Interfaces: 1 Self Powered Remote Wakeup 2mA
>  Interface: 0
>  - Alternate Setting 0, Endpoints: 3
>  - Class Vendor specific
>  - Endpoint 1 In Bulk MaxPacket 512
>  - Endpoint 2 Out Bulk MaxPacket 512
>  - Endpoint 3 In Interrupt MaxPacket 16 Interval 4ms
> 
> As the old Fedora core image on the SD card there isn't happy and I
> don't have time to dig in to that problem right now.  But please note
> that the issue here is a relatively recent regression as Guillaume
> reported.

Link would be helpful.

I tried this on R-Car3 EHCI via USB 2.0 hub and I don't see any issue
with detecting either ASIX 0b95:7720 Zoltan Tech ZoWii , asix_usb ,
ethernet OR SMSC 95xx 0454:9e00 ATMES oabrDongle . So I suspect this
must be some limitation of the EHCI on the AM335x somehow ?

Or does it also happen on other SoCs and/or other USB controllers and/or
USB ethernet adapters? I need a way to replicate the issue if I should
debug it.

-- 
Best regards,
Marek Vasut


[RFC PATCH v2] cmd: mp: change the command name from cpu to mp

2020-03-14 Thread Pragnesh Patel
When CONFIG_CMD_CPU and CONFIG_MP both are enabled, U-Boot compilation
gives an error of "multiple definition of `_u_boot_list_2_cmd_2_cpu'"
so cpu command(cmd/cpu.c) and mp command(cmd/mp.c) should have different
command name.

Signed-off-by: Pragnesh Patel 
---
 cmd/mp.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/cmd/mp.c b/cmd/mp.c
index 4c8f5fc3fa..acb56917a9 100644
--- a/cmd/mp.c
+++ b/cmd/mp.c
@@ -26,7 +26,7 @@ static int cpu_status_all(void)
 }
 
 static int
-cpu_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+mp_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
unsigned long cpuid;
 
@@ -67,12 +67,12 @@ cpu_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const 
argv[])
 }
 
 #ifdef CONFIG_SYS_LONGHELP
-static char cpu_help_text[] =
-   " reset - Reset cpu \n"
-   "cpu status  - Status of all cpus\n"
-   "cpu  status- Status of cpu \n"
-   "cpu  disable   - Disable cpu \n"
-   "cpu  release  [args] - Release cpu  at  with 
[args]"
+static char mp_help_text[] =
+   "mp  reset - Reset cpu \n"
+   "mp status  - Status of all cpus\n"
+   "mp  status- Status of cpu \n"
+   "mp  disable   - Disable cpu \n"
+   "mp  release  [args] - Release cpu  at  with 
[args]"
 #ifdef CONFIG_PPC
"\n"
" [args] :   \n" \
@@ -90,6 +90,6 @@ static char cpu_help_text[] =
 #endif
 
 U_BOOT_CMD(
-   cpu, CONFIG_SYS_MAXARGS, 1, cpu_cmd,
-   "Multiprocessor CPU boot manipulation and release", cpu_help_text
+   mp, CONFIG_SYS_MAXARGS, 1, mp_cmd,
+   "Multiprocessor CPU boot manipulation and release", mp_help_text
 );
-- 
2.17.1



RE: [RFC PATCH v2] cmd: mp: change the command name from cpu to mp

2020-03-14 Thread Pragnesh Patel
Please ignore this patch, same patch sent again.

>-Original Message-
>From: Pragnesh Patel 
>Sent: 14 March 2020 20:16
>To: u-boot@lists.denx.de
>Cc: atish.pa...@wdc.com; palmerdabb...@google.com;
>bmeng...@gmail.com; Paul Walmsley ;
>r...@andestech.com; Pragnesh Patel ; Simon
>Glass 
>Subject: [RFC PATCH v2] cmd: mp: change the command name from cpu to mp
>
>When CONFIG_CMD_CPU and CONFIG_MP both are enabled, U-Boot
>compilation gives an error of "multiple definition of
>`_u_boot_list_2_cmd_2_cpu'"
>so cpu command(cmd/cpu.c) and mp command(cmd/mp.c) should have
>different command name.
>
>Signed-off-by: Pragnesh Patel 
>---
> cmd/mp.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
>diff --git a/cmd/mp.c b/cmd/mp.c
>index 4c8f5fc3fa..26995c9976 100644
>--- a/cmd/mp.c
>+++ b/cmd/mp.c
>@@ -68,11 +68,11 @@ cpu_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char *
>const argv[])
>
> #ifdef CONFIG_SYS_LONGHELP
> static char cpu_help_text[] =
>-  " reset - Reset cpu \n"
>-  "cpu status  - Status of all cpus\n"
>-  "cpu  status- Status of cpu \n"
>-  "cpu  disable   - Disable cpu \n"
>-  "cpu  release  [args] - Release cpu  at  with
>[args]"
>+  "mp  reset - Reset cpu \n"
>+  "mp status  - Status of all cpus\n"
>+  "mp  status- Status of cpu \n"
>+  "mp  disable   - Disable cpu \n"
>+  "mp  release  [args] - Release cpu  at  with
>[args]"
> #ifdef CONFIG_PPC
>   "\n"
>   " [args] :   \n" \
>@@ -90,6 +90,6 @@ static char cpu_help_text[] =  #endif
>
> U_BOOT_CMD(
>-  cpu, CONFIG_SYS_MAXARGS, 1, cpu_cmd,
>+  mp, CONFIG_SYS_MAXARGS, 1, cpu_cmd,
>   "Multiprocessor CPU boot manipulation and release", cpu_help_text
>);
>--
>2.17.1



[RFC PATCH v2] cmd: mp: change the command name from cpu to mp

2020-03-14 Thread Pragnesh Patel
When CONFIG_CMD_CPU and CONFIG_MP both are enabled, U-Boot compilation
gives an error of "multiple definition of `_u_boot_list_2_cmd_2_cpu'"
so cpu command(cmd/cpu.c) and mp command(cmd/mp.c) should have different
command name.

Signed-off-by: Pragnesh Patel 
---
 cmd/mp.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/cmd/mp.c b/cmd/mp.c
index 4c8f5fc3fa..26995c9976 100644
--- a/cmd/mp.c
+++ b/cmd/mp.c
@@ -68,11 +68,11 @@ cpu_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const 
argv[])
 
 #ifdef CONFIG_SYS_LONGHELP
 static char cpu_help_text[] =
-   " reset - Reset cpu \n"
-   "cpu status  - Status of all cpus\n"
-   "cpu  status- Status of cpu \n"
-   "cpu  disable   - Disable cpu \n"
-   "cpu  release  [args] - Release cpu  at  with 
[args]"
+   "mp  reset - Reset cpu \n"
+   "mp status  - Status of all cpus\n"
+   "mp  status- Status of cpu \n"
+   "mp  disable   - Disable cpu \n"
+   "mp  release  [args] - Release cpu  at  with 
[args]"
 #ifdef CONFIG_PPC
"\n"
" [args] :   \n" \
@@ -90,6 +90,6 @@ static char cpu_help_text[] =
 #endif
 
 U_BOOT_CMD(
-   cpu, CONFIG_SYS_MAXARGS, 1, cpu_cmd,
+   mp, CONFIG_SYS_MAXARGS, 1, cpu_cmd,
"Multiprocessor CPU boot manipulation and release", cpu_help_text
 );
-- 
2.17.1



Re: [RFC PATCH] cmd: mp: change the command name from cpu to mp

2020-03-14 Thread Bin Meng
On Sat, Mar 14, 2020 at 8:47 PM Pragnesh Patel
 wrote:
>
> when CONFIG_CMD_CPU and CONFIG_MP both are enabled, u-boot compilation
> gives an error of "multiple definition of `_u_boot_list_2_cmd_2_cpu'"
> so cpu command(cmd/cpu.c) and mp command(cmd/mp.c) should have different
> command name.
>
> Signed-off-by: Pragnesh Patel 
> ---
>  cmd/mp.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/cmd/mp.c b/cmd/mp.c
> index 4c8f5fc3fa..26995c9976 100644
> --- a/cmd/mp.c
> +++ b/cmd/mp.c
> @@ -68,11 +68,11 @@ cpu_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * 
> const argv[])
>
>  #ifdef CONFIG_SYS_LONGHELP
>  static char cpu_help_text[] =

rename this to mp_help_text[]?

> -   " reset - Reset cpu \n"
> -   "cpu status  - Status of all cpus\n"
> -   "cpu  status- Status of cpu \n"
> -   "cpu  disable   - Disable cpu \n"
> -   "cpu  release  [args] - Release cpu  at  with 
> [args]"
> +   "mp  reset - Reset cpu \n"
> +   "mp status  - Status of all cpus\n"
> +   "mp  status- Status of cpu \n"
> +   "mp  disable   - Disable cpu \n"
> +   "mp  release  [args] - Release cpu  at  with 
> [args]"
>  #ifdef CONFIG_PPC
> "\n"
> " [args] :   \n" \
> @@ -90,6 +90,6 @@ static char cpu_help_text[] =
>  #endif
>
>  U_BOOT_CMD(
> -   cpu, CONFIG_SYS_MAXARGS, 1, cpu_cmd,
> +   mp, CONFIG_SYS_MAXARGS, 1, cpu_cmd,

mp_cmd

> "Multiprocessor CPU boot manipulation and release", cpu_help_text
>  );
> --

Please check the same file, there are some additional places using 'cpu'

Regards,
Bin


[PATCH v2] riscv: ax25: cache: Remove SPL_RISCV_MMODE config check

2020-03-14 Thread Pragnesh Patel
CONFIG_IS_ENABLED(FOO) will check FOO config option for U-Boot,
SPL and TPL, so remove unnecessary CONFIG_IS_ENABLED()

Signed-off-by: Pragnesh Patel 
Reviewed-by: Bin Meng 
---
 arch/riscv/cpu/ax25/cache.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/cpu/ax25/cache.c b/arch/riscv/cpu/ax25/cache.c
index 9f424198b4..9df629d23c 100644
--- a/arch/riscv/cpu/ax25/cache.c
+++ b/arch/riscv/cpu/ax25/cache.c
@@ -12,7 +12,7 @@
 #include 
 
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
 /* mcctlcommand */
 #define CCTL_REG_MCCTLCOMMAND_NUM  0x7cc
 
@@ -47,7 +47,7 @@ void flush_dcache_all(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL);
 #endif
 #endif
@@ -68,7 +68,7 @@ void icache_enable(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"csrr t1, mcache_ctl\n\t"
"ori t0, t1, 0x1\n\t"
@@ -83,7 +83,7 @@ void icache_disable(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"fence.i\n\t"
"csrr t1, mcache_ctl\n\t"
@@ -99,7 +99,7 @@ void dcache_enable(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"csrr t1, mcache_ctl\n\t"
"ori t0, t1, 0x2\n\t"
@@ -117,7 +117,7 @@ void dcache_disable(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL);
asm volatile (
"csrr t1, mcache_ctl\n\t"
@@ -137,7 +137,7 @@ int icache_status(void)
int ret = 0;
 
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"csrr t1, mcache_ctl\n\t"
"andi   %0, t1, 0x01\n\t"
@@ -156,7 +156,7 @@ int dcache_status(void)
int ret = 0;
 
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"csrr t1, mcache_ctl\n\t"
"andi   %0, t1, 0x02\n\t"
-- 
2.17.1



RE: [PATCH v2] riscv: ax25: cache: Remove SPL_RISCV_MMODE config check

2020-03-14 Thread Pragnesh Patel
Hi Bin,

>-Original Message-
>From: Bin Meng 
>Sent: 14 March 2020 18:57
>To: Pragnesh Patel 
>Cc: U-Boot Mailing List ; Atish Patra
>; Palmer Dabbelt ; Paul
>Walmsley ; Rick Chen ;
>Trevor Woerner ; Simon Glass 
>Subject: Re: [PATCH v2] riscv: ax25: cache: Remove SPL_RISCV_MMODE config
>check
>
>Hi Pragnesh,
>
>On Sat, Mar 14, 2020 at 6:48 PM Pragnesh Patel 
>wrote:
>>
>> CONFIG_IS_ENABLED(FOO) will check FOO config option for U-boot,
>
>Looks you forgot to update ...

Ooops, Will send v2 again, please ignore this patch.

>
>> SPL and TPL, so remove unnecessary CONFIG_IS_ENABLED()
>>
>> Signed-off-by: Pragnesh Patel 
>> Reviewed-by: Bin Meng 
>> ---
>>  arch/riscv/cpu/ax25/cache.c | 16 
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>
>Regards,
>Bin


Re: [PATCH v2] riscv: ax25: cache: Remove SPL_RISCV_MMODE config check

2020-03-14 Thread Bin Meng
Hi Pragnesh,

On Sat, Mar 14, 2020 at 6:48 PM Pragnesh Patel
 wrote:
>
> CONFIG_IS_ENABLED(FOO) will check FOO config option for U-boot,

Looks you forgot to update ...

> SPL and TPL, so remove unnecessary CONFIG_IS_ENABLED()
>
> Signed-off-by: Pragnesh Patel 
> Reviewed-by: Bin Meng 
> ---
>  arch/riscv/cpu/ax25/cache.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
>

Regards,
Bin


[RESEND Patch v2] configs: lx2160a: Enable FSPI support

2020-03-14 Thread Kuldeep Singh
Enable FSPI controller support. So, flash environment can now be used.

Signed-off-by: Kuldeep Singh 
---
v2:
-Rebased to top.
-Drop other patches from series as already accepted.
-Add ENV_SECT_SIZE value as 0x2

 configs/lx2160aqds_tfa_SECURE_BOOT_defconfig | 1 +
 configs/lx2160aqds_tfa_defconfig | 3 +++
 configs/lx2160ardb_tfa_SECURE_BOOT_defconfig | 1 +
 configs/lx2160ardb_tfa_defconfig | 3 +++
 4 files changed, 8 insertions(+)

diff --git a/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig 
b/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig
index 7c3b827..92fac5d 100644
--- a/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig
+++ b/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig
@@ -65,6 +65,7 @@ CONFIG_DM_SERIAL=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
 CONFIG_FSL_DSPI=y
+CONFIG_NXP_FSPI=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USB_XHCI_HCD=y
diff --git a/configs/lx2160aqds_tfa_defconfig b/configs/lx2160aqds_tfa_defconfig
index 449b3cb..e472c12 100644
--- a/configs/lx2160aqds_tfa_defconfig
+++ b/configs/lx2160aqds_tfa_defconfig
@@ -4,6 +4,7 @@ CONFIG_TFABOOT=y
 CONFIG_SYS_TEXT_BASE=0x8200
 CONFIG_SYS_MALLOC_F_LEN=0x6000
 CONFIG_ENV_SIZE=0x2000
+CONFIG_ENV_SECT_SIZE=0x2
 CONFIG_ENV_OFFSET=0x50
 CONFIG_DM_GPIO=y
 CONFIG_FSPI_AHB_EN_4BYTE=y
@@ -32,6 +33,7 @@ CONFIG_OF_CONTROL=y
 CONFIG_OF_BOARD_FIXUP=y
 CONFIG_DEFAULT_DEVICE_TREE="fsl-lx2160a-qds"
 CONFIG_ENV_IS_IN_MMC=y
+CONFIG_ENV_IS_IN_SPI_FLASH=y
 CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_DM=y
 CONFIG_SATA_CEVA=y
@@ -65,6 +67,7 @@ CONFIG_DM_SCSI=y
 CONFIG_DM_SERIAL=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
+CONFIG_NXP_FSPI=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USB_XHCI_HCD=y
diff --git a/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig 
b/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig
index d1fffb3..e754eb7 100644
--- a/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig
+++ b/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig
@@ -60,6 +60,7 @@ CONFIG_DM_SCSI=y
 CONFIG_DM_SERIAL=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
+CONFIG_NXP_FSPI=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USB_XHCI_HCD=y
diff --git a/configs/lx2160ardb_tfa_defconfig b/configs/lx2160ardb_tfa_defconfig
index 93f3e20..5d4580e 100644
--- a/configs/lx2160ardb_tfa_defconfig
+++ b/configs/lx2160ardb_tfa_defconfig
@@ -4,6 +4,7 @@ CONFIG_TFABOOT=y
 CONFIG_SYS_TEXT_BASE=0x8200
 CONFIG_SYS_MALLOC_F_LEN=0x6000
 CONFIG_ENV_SIZE=0x2000
+CONFIG_ENV_SECT_SIZE=0x2
 CONFIG_ENV_OFFSET=0x50
 CONFIG_DM_GPIO=y
 CONFIG_EMC2305=y
@@ -33,6 +34,7 @@ CONFIG_OF_CONTROL=y
 CONFIG_OF_BOARD_FIXUP=y
 CONFIG_DEFAULT_DEVICE_TREE="fsl-lx2160a-rdb"
 CONFIG_ENV_IS_IN_MMC=y
+CONFIG_ENV_IS_IN_SPI_FLASH=y
 CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_DM=y
 CONFIG_SATA_CEVA=y
@@ -64,6 +66,7 @@ CONFIG_DM_SCSI=y
 CONFIG_DM_SERIAL=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
+CONFIG_NXP_FSPI=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USB_XHCI_HCD=y
-- 
2.7.4



[Patch v3 3/3] arm: dts: lx2160a: Use flexspi in octal I/O mode

2020-03-14 Thread Kuldeep Singh
Configure RX and TX bus-width values to use flexspi in octal I/O mode.
If bus-widths are not specified, then single I/O mode is set by default.

Signed-off-by: Kuldeep Singh 
---
v3: Reword commit message.
v2: No change.

 arch/arm/dts/fsl-lx2160a-qds.dts | 2 ++
 arch/arm/dts/fsl-lx2160a-rdb.dts | 4 
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/dts/fsl-lx2160a-qds.dts b/arch/arm/dts/fsl-lx2160a-qds.dts
index 33bedae..592fd59 100644
--- a/arch/arm/dts/fsl-lx2160a-qds.dts
+++ b/arch/arm/dts/fsl-lx2160a-qds.dts
@@ -58,6 +58,8 @@
compatible = "jedec,spi-nor";
spi-max-frequency = <5000>;
reg = <0>;
+   spi-rx-bus-width = <8>;
+   spi-tx-bus-width = <1>;
};
 };
 
diff --git a/arch/arm/dts/fsl-lx2160a-rdb.dts b/arch/arm/dts/fsl-lx2160a-rdb.dts
index e542c69..87617ca 100644
--- a/arch/arm/dts/fsl-lx2160a-rdb.dts
+++ b/arch/arm/dts/fsl-lx2160a-rdb.dts
@@ -39,6 +39,8 @@
compatible = "jedec,spi-nor";
spi-max-frequency = <5000>;
reg = <0>;
+   spi-rx-bus-width = <8>;
+   spi-tx-bus-width = <1>;
};
 
mt35xu512aba1: flash@1 {
@@ -47,6 +49,8 @@
compatible = "jedec,spi-nor";
spi-max-frequency = <5000>;
reg = <1>;
+   spi-rx-bus-width = <8>;
+   spi-tx-bus-width = <1>;
};
 };
 
-- 
2.7.4



[Patch v3 2/3] arm: dts: ls1028a: Use flexspi in octal I/O mode

2020-03-14 Thread Kuldeep Singh
Configure RX and TX bus-width values to use flexspi in octal I/O mode.
If bus-widths are not specified, then single I/O mode is set by default.

Signed-off-by: Kuldeep Singh 
---
v3: Reword commit message.
v2: No change.

 arch/arm/dts/fsl-ls1028a-qds.dts | 2 ++
 arch/arm/dts/fsl-ls1028a-rdb.dts | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm/dts/fsl-ls1028a-qds.dts b/arch/arm/dts/fsl-ls1028a-qds.dts
index 3fd37be..029a8e3 100644
--- a/arch/arm/dts/fsl-ls1028a-qds.dts
+++ b/arch/arm/dts/fsl-ls1028a-qds.dts
@@ -49,6 +49,8 @@
compatible = "jedec,spi-nor";
spi-max-frequency = <5000>;
reg = <0>;
+   spi-rx-bus-width = <8>;
+   spi-tx-bus-width = <1>;
};
 };
 
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index a8f4085..85b4815 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -48,6 +48,8 @@
compatible = "jedec,spi-nor";
spi-max-frequency = <5000>;
reg = <0>;
+   spi-rx-bus-width = <8>;
+   spi-tx-bus-width = <1>;
};
 };
 
-- 
2.7.4



[Patch v3 0/3] Enable octal read support for mt35xu* flashes

2020-03-14 Thread Kuldeep Singh
Patch series enable octal read(1-1-8) support for LX2160ARDB/QDS and
LS1028ARDB/QDS which have mt35xu512aba and mt35xu02g flashes
respectively.

mt35xu512aba and mt35xu02g flashes support SINGLE and OCTAL I/O.
Previously, 1 bit mode was used in u-boot and now use octal mode for the
flashes.

Patch 1 enables octal read flag for flashes in framework.
Patch 2/3 adds RX,TX buswidth in flexspi dts entries to use octal mode
for LS1028ARDB/QDS, LX2160ARDB/QDS.

v3:
-Patch 3 has dependency on https://patchwork.ozlabs.org/patch/1247579/.
-Reword commit message for patch 2/3.

v2:
-Update dependencies of the patches
-Add lx2160aqds node buswidth in patch3.
-Patch3 has dependency on https://patchwork.ozlabs.org/patch/1236164/.

Kuldeep Singh (3):
  mtd: spi-nor-ids: Enable SPI_NOR_OCTAL_READ flag for mt35xu*
  arm: dts: ls1028a: Use flexspi in octal I/O mode
  arm: dts: lx2160a: Use flexspi in octal I/O mode

 arch/arm/dts/fsl-ls1028a-qds.dts | 2 ++
 arch/arm/dts/fsl-ls1028a-rdb.dts | 2 ++
 arch/arm/dts/fsl-lx2160a-qds.dts | 2 ++
 arch/arm/dts/fsl-lx2160a-rdb.dts | 4 
 drivers/mtd/spi/spi-nor-ids.c| 4 ++--
 5 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.7.4



[Patch v3 1/3] mtd: spi-nor-ids: Enable SPI_NOR_OCTAL_READ flag for mt35xu*

2020-03-14 Thread Kuldeep Singh
Commit 658df8bd9464 ("mtd: spi-nor-core: Add octal mode support")
enables octal mode(1-1-8) support in spi-nor framework.

mt35xu512aba and mt35xu02g supports SINGLE and OCTAL I/O. Hence, enable
SPI_NOR_OCTAL_READ flag for these flashes.

Signed-off-by: Kuldeep Singh 
Reviewed-by: Vignesh Raghavendra 
---
v3: No change
v2: Reword commit message

 drivers/mtd/spi/spi-nor-ids.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index 973b6f8..334c074 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -182,8 +182,8 @@ const struct flash_info spi_nor_ids[] = {
{ INFO("n25q00",  0x20ba21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | 
SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
{ INFO("n25q00a", 0x20bb21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | 
SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
{ INFO("mt25qu02g",   0x20bb22, 0, 64 * 1024, 4096, SECT_4K | USE_FSR | 
SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
-   { INFO("mt35xu512aba", 0x2c5b1a, 0,  128 * 1024,  512, USE_FSR | 
SPI_NOR_4B_OPCODES) },
-   { INFO("mt35xu02g",  0x2c5b1c, 0, 128 * 1024,  2048, USE_FSR | 
SPI_NOR_4B_OPCODES) },
+   { INFO("mt35xu512aba", 0x2c5b1a, 0,  128 * 1024,  512, USE_FSR | 
SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mt35xu02g",  0x2c5b1c, 0, 128 * 1024,  2048, USE_FSR | 
SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) },
 #endif
 #ifdef CONFIG_SPI_FLASH_SPANSION   /* SPANSION */
/* Spansion/Cypress -- single (large) sector size only, at least
-- 
2.7.4



[RFC PATCH] cmd: mp: change the command name from cpu to mp

2020-03-14 Thread Pragnesh Patel
when CONFIG_CMD_CPU and CONFIG_MP both are enabled, u-boot compilation
gives an error of "multiple definition of `_u_boot_list_2_cmd_2_cpu'"
so cpu command(cmd/cpu.c) and mp command(cmd/mp.c) should have different
command name.

Signed-off-by: Pragnesh Patel 
---
 cmd/mp.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/cmd/mp.c b/cmd/mp.c
index 4c8f5fc3fa..26995c9976 100644
--- a/cmd/mp.c
+++ b/cmd/mp.c
@@ -68,11 +68,11 @@ cpu_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const 
argv[])
 
 #ifdef CONFIG_SYS_LONGHELP
 static char cpu_help_text[] =
-   " reset - Reset cpu \n"
-   "cpu status  - Status of all cpus\n"
-   "cpu  status- Status of cpu \n"
-   "cpu  disable   - Disable cpu \n"
-   "cpu  release  [args] - Release cpu  at  with 
[args]"
+   "mp  reset - Reset cpu \n"
+   "mp status  - Status of all cpus\n"
+   "mp  status- Status of cpu \n"
+   "mp  disable   - Disable cpu \n"
+   "mp  release  [args] - Release cpu  at  with 
[args]"
 #ifdef CONFIG_PPC
"\n"
" [args] :   \n" \
@@ -90,6 +90,6 @@ static char cpu_help_text[] =
 #endif
 
 U_BOOT_CMD(
-   cpu, CONFIG_SYS_MAXARGS, 1, cpu_cmd,
+   mp, CONFIG_SYS_MAXARGS, 1, cpu_cmd,
"Multiprocessor CPU boot manipulation and release", cpu_help_text
 );
-- 
2.17.1



Re: [PATCH 0/3] watchdog: honour hw_margin_ms property

2020-03-14 Thread Stefan Roese

On 13.03.20 17:04, Rasmus Villemoes wrote:

Some watchdogs must be reset more often than the once-per-second
ratelimit used by the generic watchdog_reset function in
wdt-uclass.c. There's precedent (from the gpio-wdt driver in linux)
for using a property called hw_margin_ms to let the device tree tell
the driver how often the device needs resetting. So use that
generically. No change in default behaviour.

On top of https://patchwork.ozlabs.org/patch/1242772/ .

Stefan, something like this?


Yes, thanks for looking into this.


That at least solves half my problems and
might be useful to others as well. Then I'll have to figure out the
time-stands-still problem in some other way.


If its too hard to enable interrupts in SPL for you or to provide some
other means of a working get_timer() API, then we needto find another
solution. You started with this weak function, which of course works.
What other options are there? Adding a callback mechanism to register
platform specific callback functions? Even though this might get a
little bit too complicated.

Thanks,
Stefan


Re: [PATCH 3/3] watchdog: honour hw_margin_ms DT property

2020-03-14 Thread Stefan Roese

On 13.03.20 17:04, Rasmus Villemoes wrote:

Some watchdog devices, e.g. external gpio-triggered ones, must be
reset more often than once per second, which means that the current
rate-limiting logic in watchdog_reset() fails to keep the board alive.

gpio-wdt.txt in the linux source tree defines a "hw_margin_ms"
property used to specifiy the maximum time allowed between resetting
the device. Allow any watchdog device to specify such a property, and
then use a reset period of one quarter of that. We keep the current
default of resetting once every 1000ms.

Signed-off-by: Rasmus Villemoes 
---
  drivers/watchdog/wdt-uclass.c | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index fb3e247c5f..436a52fd08 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -15,6 +15,12 @@ DECLARE_GLOBAL_DATA_PTR;
  
  #define WATCHDOG_TIMEOUT_SECS	(CONFIG_WATCHDOG_TIMEOUT_MSECS / 1000)
  
+/*

+ * Reset every 1000ms, or however often is required as indicated by a
+ * hw_margin_ms property.
+ */
+static ulong reset_period = 1000;
+
  int initr_watchdog(void)
  {
u32 timeout = WATCHDOG_TIMEOUT_SECS;
@@ -36,6 +42,8 @@ int initr_watchdog(void)
if (CONFIG_IS_ENABLED(OF_CONTROL)) {
timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec",
   WATCHDOG_TIMEOUT_SECS);
+   reset_period = dev_read_u32_default(gd->watchdog_dev, 
"hw_margin_ms",
+   4*reset_period)/4;


Nitpicking: checkpatch will most likely complain about missing spaces
here.


}
  
  	wdt_start(gd->watchdog_dev, timeout * 1000, 0);

@@ -117,7 +125,7 @@ void watchdog_reset(void)
/* Do not reset the watchdog too often */
now = get_timer(0);
if (time_after(now, next_reset)) {
-   next_reset = now + 1000;/* reset every 1000ms */
+   next_reset = now + reset_period;
wdt_reset(gd->watchdog_dev);
}
  }



Other than that:

Reviewed-by: Stefan Roese 

Thanks,
Stefan



Re: [PATCH 2/3] watchdog: move initr_watchdog() to wdt-uclass.c

2020-03-14 Thread Stefan Roese

On 13.03.20 17:04, Rasmus Villemoes wrote:

This function is a bit large for an inline function, and for U-Boot
proper, it is called via a function pointer anyway (in board_r.c), so
cannot be inlined.

It will shortly set a global variable to be used by the
watchdog_reset() function in wdt-uclass.c, so this also allows making
that variable local to wdt-uclass.c.

The WATCHDOG_TIMEOUT_SECS define is not used elsewhere.

Signed-off-by: Rasmus Villemoes 
---
  drivers/watchdog/wdt-uclass.c | 33 +
  include/wdt.h | 35 +--
  2 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 309a0e9c5b..fb3e247c5f 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -13,6 +13,39 @@
  
  DECLARE_GLOBAL_DATA_PTR;
  
+#define WATCHDOG_TIMEOUT_SECS	(CONFIG_WATCHDOG_TIMEOUT_MSECS / 1000)

+
+int initr_watchdog(void)
+{
+   u32 timeout = WATCHDOG_TIMEOUT_SECS;
+
+   /*
+* Init watchdog: This will call the probe function of the
+* watchdog driver, enabling the use of the device
+*/
+   if (uclass_get_device_by_seq(UCLASS_WDT, 0,
+(struct udevice **)>watchdog_dev)) {
+   debug("WDT:   Not found by seq!\n");
+   if (uclass_get_device(UCLASS_WDT, 0,
+ (struct udevice **)>watchdog_dev)) {
+   printf("WDT:   Not found!\n");
+   return 0;
+   }
+   }
+
+   if (CONFIG_IS_ENABLED(OF_CONTROL)) {
+   timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec",
+  WATCHDOG_TIMEOUT_SECS);
+   }
+
+   wdt_start(gd->watchdog_dev, timeout * 1000, 0);
+   gd->flags |= GD_FLG_WDT_READY;
+   printf("WDT:   Started with%s servicing (%ds timeout)\n",
+  IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
+
+   return 0;
+}
+
  int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
  {
const struct wdt_ops *ops = device_get_ops(dev);
diff --git a/include/wdt.h b/include/wdt.h
index e833d3a772..aea5abc768 100644
--- a/include/wdt.h
+++ b/include/wdt.h
@@ -106,39 +106,6 @@ struct wdt_ops {
int (*expire_now)(struct udevice *dev, ulong flags);
  };
  
-#if CONFIG_IS_ENABLED(WDT)

-#define WATCHDOG_TIMEOUT_SECS  (CONFIG_WATCHDOG_TIMEOUT_MSECS / 1000)
-
-static inline int initr_watchdog(void)
-{
-   u32 timeout = WATCHDOG_TIMEOUT_SECS;
-
-   /*
-* Init watchdog: This will call the probe function of the
-* watchdog driver, enabling the use of the device
-*/
-   if (uclass_get_device_by_seq(UCLASS_WDT, 0,
-(struct udevice **)>watchdog_dev)) {
-   debug("WDT:   Not found by seq!\n");
-   if (uclass_get_device(UCLASS_WDT, 0,
- (struct udevice **)>watchdog_dev)) {
-   printf("WDT:   Not found!\n");
-   return 0;
-   }
-   }
-
-   if (CONFIG_IS_ENABLED(OF_CONTROL)) {
-   timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec",
-  WATCHDOG_TIMEOUT_SECS);
-   }
-
-   wdt_start(gd->watchdog_dev, timeout * 1000, 0);
-   gd->flags |= GD_FLG_WDT_READY;
-   printf("WDT:   Started with%s servicing (%ds timeout)\n",
-  IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
-
-   return 0;
-}
-#endif
+int initr_watchdog(void);
  
  #endif  /* _WDT_H_ */




Reviewed-by: Stefan Roese 

Thanks,
Stefan


Re: [PATCH 1/3] watchdog: remove stale ifndef CONFIG_WATCHDOG_TIMEOUT_MSECS from wdt.h

2020-03-14 Thread Stefan Roese

On 13.03.20 17:04, Rasmus Villemoes wrote:

Since WATCHDOG_TIMEOUT_MSECS was converted to Kconfig (commit
ca51ef7c0c), CONFIG_WATCHDOG_TIMEOUT_MSECS has been guaranteed to be
defined. So remove the dead fallback ifdeffery.

Signed-off-by: Rasmus Villemoes 
---
  include/wdt.h | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/include/wdt.h b/include/wdt.h
index 5bcff24ab3..e833d3a772 100644
--- a/include/wdt.h
+++ b/include/wdt.h
@@ -107,9 +107,6 @@ struct wdt_ops {
  };
  
  #if CONFIG_IS_ENABLED(WDT)

-#ifndef CONFIG_WATCHDOG_TIMEOUT_MSECS
-#define CONFIG_WATCHDOG_TIMEOUT_MSECS  (60 * 1000)
-#endif
  #define WATCHDOG_TIMEOUT_SECS (CONFIG_WATCHDOG_TIMEOUT_MSECS / 1000)
  
  static inline int initr_watchdog(void)




Reviewed-by: Stefan Roese 

Thanks,
Stefan


[PATCH 1/1] sandbox: enable CMD_BOOTEFI_HELLO and CMD_EFIDEBUG

2020-03-14 Thread Heinrich Schuchardt
'bootefi hello' is used in one of the Python tests.

efidebug can be used to verify the correct initialization of the UEFI
sub-system.

Signed-off-by: Heinrich Schuchardt 
---
 configs/sandbox64_defconfig| 2 ++
 configs/sandbox_defconfig  | 1 +
 configs/sandbox_flattree_defconfig | 2 ++
 configs/sandbox_spl_defconfig  | 2 ++
 4 files changed, 7 insertions(+)

diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 71a4d7fccb..c03b425405 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -22,6 +22,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
+CONFIG_CMD_BOOTEFI_HELLO=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
@@ -53,6 +54,7 @@ CONFIG_CMD_DNS=y
 CONFIG_CMD_LINK_LOCAL=y
 CONFIG_CMD_ETHSW=y
 CONFIG_CMD_BMP=y
+CONFIG_CMD_EFIDEBUG=y
 CONFIG_CMD_TIME=y
 CONFIG_CMD_TIMER=y
 CONFIG_CMD_SOUND=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index f96891ecae..78b09ba5f3 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -25,6 +25,7 @@ CONFIG_ANDROID_AB=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
+CONFIG_CMD_BOOTEFI_HELLO=y
 CONFIG_CMD_ABOOTIMG=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
diff --git a/configs/sandbox_flattree_defconfig 
b/configs/sandbox_flattree_defconfig
index 43efefda51..6960da31a1 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -19,6 +19,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
+CONFIG_CMD_BOOTEFI_HELLO=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
@@ -43,6 +44,7 @@ CONFIG_CMD_CDP=y
 CONFIG_CMD_SNTP=y
 CONFIG_CMD_DNS=y
 CONFIG_CMD_LINK_LOCAL=y
+CONFIG_CMD_EFIDEBUG=y
 CONFIG_CMD_TIME=y
 CONFIG_CMD_TIMER=y
 CONFIG_CMD_SOUND=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index cb387e744b..de22c27608 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -28,6 +28,7 @@ CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
+CONFIG_CMD_BOOTEFI_HELLO=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
@@ -56,6 +57,7 @@ CONFIG_CMD_SNTP=y
 CONFIG_CMD_DNS=y
 CONFIG_CMD_LINK_LOCAL=y
 CONFIG_CMD_BMP=y
+CONFIG_CMD_EFIDEBUG=y
 CONFIG_CMD_TIME=y
 CONFIG_CMD_TIMER=y
 CONFIG_CMD_SOUND=y
--
2.25.1



[PATCH 2/2] sandbox: implement ft_board_setup()

2020-03-14 Thread Heinrich Schuchardt
Currently we are not able to test reservations created by ft_board_setup().

Implement ft_board_setup() to create an arbitrary reservation and enable
OF_BOARD_SETUP.

Signed-off-by: Heinrich Schuchardt 
---
 arch/Kconfig| 1 +
 board/sandbox/sandbox.c | 6 ++
 2 files changed, 7 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index ae9c93ed7b..91e049b322 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -96,6 +96,7 @@ config SANDBOX
select DM_SPI_FLASH
select HAVE_BLOCK_DEVICE
select LZO
+   select OF_BOARD_SETUP
select PCI_ENDPOINT
select SPI
select SUPPORT_OF_CONTROL
diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
index 0c3d245dff..51881b025d 100644
--- a/board/sandbox/sandbox.c
+++ b/board/sandbox/sandbox.c
@@ -58,6 +58,12 @@ int board_init(void)
return 0;
 }

+int ft_board_setup(void *fdt, bd_t *bd)
+{
+   /* Create an arbitrary reservation to allow testing OF_BOARD_SETUP.*/
+   return fdt_add_mem_rsv(fdt, 0x00d02000, 0x4000);
+}
+
 #ifdef CONFIG_BOARD_LATE_INIT
 int board_late_init(void)
 {
--
2.25.1



[PATCH 0/2] sandbox: provide memory reservations

2020-03-14 Thread Heinrich Schuchardt
Currently we are not able to test reservations on the sandbox. Memory
reservations may be either be created in ft_board_setup() or via the device
tree. This series implements both types of reservations for the sandbox.

Further patches will be needed to implement an actual test which could be
based on the 'bootefi hello' and 'efidebug memmap' commands.

Heinrich Schuchardt (2):
  sandbox: add reserved-memory node in device tree
  sandbox: implement ft_board_setup()

 arch/Kconfig   |  1 +
 arch/sandbox/dts/sandbox.dts   | 19 +++
 arch/sandbox/dts/sandbox64.dts | 20 
 board/sandbox/sandbox.c|  6 ++
 4 files changed, 46 insertions(+)

--
2.25.1



[PATCH 1/2] sandbox: add reserved-memory node in device tree

2020-03-14 Thread Heinrich Schuchardt
For testing the handling of memory reservations create a reserved-memory
node in sandbox.dts and sandbox64.dts.

Signed-off-by: Heinrich Schuchardt 
---
 arch/sandbox/dts/sandbox.dts   | 19 +++
 arch/sandbox/dts/sandbox64.dts | 20 
 2 files changed, 39 insertions(+)

diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
index 4dd82f6a32..d2e8cd8319 100644
--- a/arch/sandbox/dts/sandbox.dts
+++ b/arch/sandbox/dts/sandbox.dts
@@ -20,6 +20,25 @@
reg = <0 CONFIG_SYS_SDRAM_SIZE>;
};

+   reserved-memory {
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   reservation_test0 {
+   size = <0x4000>;
+   alignment = <0x2000>;
+   };
+
+   reservation_test1: restest@a000 {
+   reg = <0x00d0a000 0x2000>;
+   };
+
+   reservation_test2: restest@7000 {
+   reg = <0x00d07000 0x1000>;
+   };
+   };
+
cros_ec: cros-ec {
reg = <0 0>;
u-boot,dm-pre-reloc;
diff --git a/arch/sandbox/dts/sandbox64.dts b/arch/sandbox/dts/sandbox64.dts
index 5c95cee9d7..56dd703a1e 100644
--- a/arch/sandbox/dts/sandbox64.dts
+++ b/arch/sandbox/dts/sandbox64.dts
@@ -20,6 +20,26 @@
reg = /bits/ 64 <0 CONFIG_SYS_SDRAM_SIZE>;
};

+   reserved-memory {
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+
+   reservation_test_size {
+   size = <0 0x4000>;
+   alignment = <0 0x2000>;
+   };
+
+   reservation_test@a000 {
+   reg = <0 0x00d0a000 0 0x2000>;
+   };
+
+   reservation_test@7000 {
+   reg = <0 0x00d07000 0 0x1000>;
+   };
+   };
+
+   /* ... */
cros_ec: cros-ec {
reg = <0 0 0 0>;
u-boot,dm-pre-reloc;
--
2.25.1



[PATCH v2] riscv: ax25: cache: Remove SPL_RISCV_MMODE config check

2020-03-14 Thread Pragnesh Patel
CONFIG_IS_ENABLED(FOO) will check FOO config option for U-boot,
SPL and TPL, so remove unnecessary CONFIG_IS_ENABLED()

Signed-off-by: Pragnesh Patel 
Reviewed-by: Bin Meng 
---
 arch/riscv/cpu/ax25/cache.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/cpu/ax25/cache.c b/arch/riscv/cpu/ax25/cache.c
index 9f424198b4..9df629d23c 100644
--- a/arch/riscv/cpu/ax25/cache.c
+++ b/arch/riscv/cpu/ax25/cache.c
@@ -12,7 +12,7 @@
 #include 
 
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
 /* mcctlcommand */
 #define CCTL_REG_MCCTLCOMMAND_NUM  0x7cc
 
@@ -47,7 +47,7 @@ void flush_dcache_all(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL);
 #endif
 #endif
@@ -68,7 +68,7 @@ void icache_enable(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"csrr t1, mcache_ctl\n\t"
"ori t0, t1, 0x1\n\t"
@@ -83,7 +83,7 @@ void icache_disable(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"fence.i\n\t"
"csrr t1, mcache_ctl\n\t"
@@ -99,7 +99,7 @@ void dcache_enable(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"csrr t1, mcache_ctl\n\t"
"ori t0, t1, 0x2\n\t"
@@ -117,7 +117,7 @@ void dcache_disable(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL);
asm volatile (
"csrr t1, mcache_ctl\n\t"
@@ -137,7 +137,7 @@ int icache_status(void)
int ret = 0;
 
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"csrr t1, mcache_ctl\n\t"
"andi   %0, t1, 0x01\n\t"
@@ -156,7 +156,7 @@ int dcache_status(void)
int ret = 0;
 
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"csrr t1, mcache_ctl\n\t"
"andi   %0, t1, 0x02\n\t"
-- 
2.17.1



RE: [PATCH] riscv: ax25: cache: Remove SPL_RISCV_MMODE config check

2020-03-14 Thread Pragnesh Patel
Hi,

>-Original Message-
>From: Bin Meng 
>Sent: 14 March 2020 14:41
>To: Pragnesh Patel 
>Cc: U-Boot Mailing List ; Atish Patra
>; Palmer Dabbelt ; Paul
>Walmsley ; Rick Chen ;
>Simon Glass ; Alexey Brodkin
>; Trevor Woerner 
>Subject: Re: [PATCH] riscv: ax25: cache: Remove SPL_RISCV_MMODE config
>check
>
>On Sat, Mar 14, 2020 at 4:48 PM Pragnesh Patel 
>wrote:
>>
>> CONFIG_IS_ENABLED(FOO) will check FOO config option for U-boot proper,
>
>nits: U-Boot

Will update in v2.

>
>> SPL and TPL, so remove unnecessary CONFIG_IS_ENABLED()
>>
>> Signed-off-by: Pragnesh Patel 
>> ---
>>  arch/riscv/cpu/ax25/cache.c | 16 
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>
>Reviewed-by: Bin Meng 


Re: [PATCH v2 4/4] riscv: Setup reserved-memory node for FU540

2020-03-14 Thread Bin Meng
Hi Atish,

On Sat, Mar 14, 2020 at 8:11 AM Atish Patra  wrote:
>
> FU540 uses OF_SEPARATE instead of OF_PRIOR.
>
> Enable OF_BOARD_FIXUP to update the DT with reserved-memory node.
>
> Signed-off-by: Atish Patra 
> ---
>  board/sifive/fu540/fu540.c | 15 +++
>  configs/sifive_fu540_defconfig |  1 +
>  2 files changed, 16 insertions(+)
>
> diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c
> index 47a20902517c..82b3a9c8e729 100644
> --- a/board/sifive/fu540/fu540.c
> +++ b/board/sifive/fu540/fu540.c
> @@ -141,6 +141,21 @@ int misc_init_r(void)
>
>  #endif
>
> +#ifdef CONFIG_OF_BOARD_FIXUP
> +int board_fix_fdt(void *fdt)

This is used to fix-up the DT that U-Boot uses, not the DT that
operating system uses.

As I mentioned in
https://github.com/riscv/riscv-sbi-doc/pull/37#issuecomment-596184723,
we need consider U-Boot SPL usage.

> +{
> +   int err;
> +
> +   err = riscv_board_reserved_mem_fixup(fdt);
> +   if (err < 0) {
> +   printf("failed to fixup DT for reserved memory: %d\n", err);
> +   return err;
> +   }
> +
> +   return 0;
> +}
> +#endif
> +
>  int board_init(void)
>  {
> /* For now nothing to do here. */
> diff --git a/configs/sifive_fu540_defconfig b/configs/sifive_fu540_defconfig
> index 6d61e6c960ee..8fb3794cd578 100644
> --- a/configs/sifive_fu540_defconfig
> +++ b/configs/sifive_fu540_defconfig
> @@ -12,3 +12,4 @@ CONFIG_DISPLAY_BOARDINFO=y
>  CONFIG_DEFAULT_DEVICE_TREE="hifive-unleashed-a00"
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>  CONFIG_DM_MTD=y
> +CONFIG_OF_BOARD_FIXUP=y
> --

Regards,
Bin


[PATCH 1/1] efi_loader: create reservations after ft_board_setup

2020-03-14 Thread Heinrich Schuchardt
Some memory reservations are made in ft_board_setup(). Ensure that we
create reserved memory map entries after ft_board_setup().

The downside of this patch is that if bootefi is called multiple times with
an devicetree argument superfluous reservations for the old copies of the
device tree will exist. But that is still better than missing a reservation.

Deleting the superfluous reservations is not possible because reservations
in the memory map are rounded to page size and may be coallesced.

Signed-off-by: Heinrich Schuchardt 
---
 cmd/bootefi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 485f4b408a..9990959fe7 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -293,9 +293,6 @@ efi_status_t efi_install_fdt(void *fdt)
return EFI_LOAD_ERROR;
}

-   /* Create memory reservations as indicated by the device tree */
-   efi_carve_out_dt_rsv(fdt);
-
/* Prepare device tree for payload */
ret = copy_fdt();
if (ret) {
@@ -303,6 +300,9 @@ efi_status_t efi_install_fdt(void *fdt)
return EFI_OUT_OF_RESOURCES;
}

+   /* Create memory reservations as indicated by the device tree */
+   efi_carve_out_dt_rsv(fdt);
+
/* Install device tree as UEFI table */
ret = efi_install_configuration_table(_guid_fdt, fdt);
if (ret != EFI_SUCCESS) {
--
2.25.1



Re: [PATCH v2 3/4] riscv: Provide a mechanism for riscv boards to parse reserved memory

2020-03-14 Thread Bin Meng
Hi Atish,

On Sat, Mar 14, 2020 at 8:54 AM Atish Patra  wrote:
>
> On Fri, Mar 13, 2020 at 5:12 PM Atish Patra  wrote:
> >
> > In RISC-V, M-mode software can reserve physical memory regions
> > by setting appropriate physical memory protection (PMP) csr. As the
> > PMP csr are accessible only in M-mode, S-mode U-Boot can not read
> > this configuration directly. However, M-mode software can pass this
> > information via reserved-memory node in device tree so that S-mode
> > software can access this information.
> >
> > In U-boot, any board may use the DT in following ways.

nits: U-Boot

> > 1. OF_SEPARTE: It ignores the DT from previous stage and uses the DT

typo: OF_SEPARATE

> > from U-Boot sources.
> > 2. OF_PRIOR_STATE: It reuses the DT from previous stage.

typo: OF_PRIOR_STAGE

> > For case 1: U-Boot needs to parse the reserved-memory node from the
> > DT passed from the previous stage and update the DT in use.
> >
> > This patch provides a framework to do that from any RISC-V boards.
> >
> > Signed-off-by: Atish Patra 
> > ---
> >  arch/riscv/cpu/start.S|  1 +
> >  arch/riscv/include/asm/global_data.h  |  1 +
> >  arch/riscv/include/asm/u-boot-riscv.h |  1 +
> >  arch/riscv/lib/asm-offsets.c  |  1 +
> >  arch/riscv/lib/bootm.c| 37 +++
> >  5 files changed, 41 insertions(+)
> >
> > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> > index 6b3ff99c3882..0282685c2906 100644
> > --- a/arch/riscv/cpu/start.S
> > +++ b/arch/riscv/cpu/start.S
> > @@ -121,6 +121,7 @@ call_board_init_f_0:
> >
> > jal board_init_f_init_reserve
> >
> > +   SREGs1, GD_FIRMWARE_FDT_ADDR(gp)
> > /* save the boot hart id to global_data */
> > SREGtp, GD_BOOT_HART(gp)
> >
> > diff --git a/arch/riscv/include/asm/global_data.h 
> > b/arch/riscv/include/asm/global_data.h
> > index b74bd7e738bb..51ac8d1c98e2 100644
> > --- a/arch/riscv/include/asm/global_data.h
> > +++ b/arch/riscv/include/asm/global_data.h
> > @@ -15,6 +15,7 @@
> >  /* Architecture-specific global data */
> >  struct arch_global_data {
> > long boot_hart; /* boot hart id */
> > +   phys_addr_t firmware_fdt_addr;
> >  #ifdef CONFIG_SIFIVE_CLINT
> > void __iomem *clint;/* clint base address */
> >  #endif
> > diff --git a/arch/riscv/include/asm/u-boot-riscv.h 
> > b/arch/riscv/include/asm/u-boot-riscv.h
> > index 49febd588102..b7bea0ba184d 100644
> > --- a/arch/riscv/include/asm/u-boot-riscv.h
> > +++ b/arch/riscv/include/asm/u-boot-riscv.h
> > @@ -17,5 +17,6 @@ int cleanup_before_linux(void);
> >  /* board/.../... */
> >  int board_init(void);
> >  void board_quiesce_devices(void);
> > +int riscv_board_reserved_mem_fixup(void *fdt);
> >
> >  #endif /* _U_BOOT_RISCV_H_ */
> > diff --git a/arch/riscv/lib/asm-offsets.c b/arch/riscv/lib/asm-offsets.c
> > index 4fa4fd371473..7301c1b98e23 100644
> > --- a/arch/riscv/lib/asm-offsets.c
> > +++ b/arch/riscv/lib/asm-offsets.c
> > @@ -14,6 +14,7 @@
> >  int main(void)
> >  {
> > DEFINE(GD_BOOT_HART, offsetof(gd_t, arch.boot_hart));
> > +   DEFINE(GD_FIRMWARE_FDT_ADDR, offsetof(gd_t, 
> > arch.firmware_fdt_addr));
> >  #ifndef CONFIG_XIP
> > DEFINE(GD_AVAILABLE_HARTS, offsetof(gd_t, arch.available_harts));
> >  #endif
> > diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> > index f927694ae32f..3a4d0bf14c86 100644
> > --- a/arch/riscv/lib/bootm.c
> > +++ b/arch/riscv/lib/bootm.c
> > @@ -19,6 +19,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > @@ -26,6 +27,42 @@ __weak void board_quiesce_devices(void)
> >  {
> >  }
> >
> > +int riscv_board_reserved_mem_fixup(void *fdt)
> > +{
> > +   uint32_t phandle;
> > +   struct fdt_memory pmp_mem;
> > +   int err;
> > +   void *src_fdt_addr;
> > +   int offset, node;
> > +   phys_addr_t addr, size;
> > +
> > +   src_fdt_addr = map_sysmem(gd->arch.firmware_fdt_addr, 0);
> > +   offset = fdt_path_offset(src_fdt_addr, "/reserved-memory");
> > +   if (offset < 0) {
> > +   printf("No reserved memory region found in FDT\n");
> > +   return offset;

We should return 0 to allow cases that do not have /reserved-memory
node. Otherwise we are mandating a newer OpenSBI version to work with
us.

> > +   }
> > +
> > +   fdt_for_each_subnode(node, src_fdt_addr, offset) {
> > +   const char *name = fdt_get_name(src_fdt_addr, node, NULL);
> > +
> > +   addr = fdtdec_get_addr_size(src_fdt_addr, node, "reg", 
> > );
> > +   if (addr == FDT_ADDR_T_NONE) {
> > +   debug("failed to read address/size for %s\n", name);
> > +   continue;
> > +   }
> > +   pmp_mem.start = addr;
> > +   pmp_mem.end = addr + size;
> > +   err = fdtdec_add_reserved_memory(fdt, name, _mem, 
> > );
> > +   

Re: [PATCH] riscv: ax25: cache: Remove SPL_RISCV_MMODE config check

2020-03-14 Thread Bin Meng
On Sat, Mar 14, 2020 at 4:48 PM Pragnesh Patel
 wrote:
>
> CONFIG_IS_ENABLED(FOO) will check FOO config option for U-boot proper,

nits: U-Boot

> SPL and TPL, so remove unnecessary CONFIG_IS_ENABLED()
>
> Signed-off-by: Pragnesh Patel 
> ---
>  arch/riscv/cpu/ax25/cache.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 2/4] cmd: bootefi: Parse reserved-memory node from DT

2020-03-14 Thread Heinrich Schuchardt

On 3/14/20 8:14 AM, Heinrich Schuchardt wrote:

On 3/14/20 1:55 AM, Atish Patra wrote:

On Fri, Mar 13, 2020 at 5:12 PM Atish Patra  wrote:


Currently, bootefi only parses memory reservation block to setup
EFI reserved memory mappings. However, it doesn't parse the
reserved-memory[1] device tree node that also can contain the
reserved memory regions.

Add capability to parse reserved-memory node and update the EFI memory
mappings accordingly.


Hello Atish,

thanks for reporting the problem.

I would appreciate a test to verify that the code really works. We
should add both types of reserved-memory device tree node to the sandbox
device-tree and parse the output of 'efidebug memmap' and compare it to
the output of 'fdt print'. Could you, please, provide a first patch for
sandbox.dts and sandbox64.dts. The next step then would be to build a
Python test.

This patch series could be split into two. One for EFI and one for RISC-V.

(see comment below)



1. source>/doc/device-tree-bindings/reserved-memory/reserved-memory.txt]


Signed-off-by: Atish Patra 
---
  cmd/bootefi.c | 42 +-
  1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 24fc42ae898e..43b36fbacfcd 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -149,6 +149,20 @@ done:
 return ret;
  }

+static void efi_reserve_memory(uint64_t addr, uint64_t size)
+{
+   uint64_t pages;
+
+   /* Convert from sandbox address space. */
+   addr = (uintptr_t)map_sysmem(addr, 0);
+   pages = efi_size_in_pages(size + (addr & EFI_PAGE_MASK));
+   addr &= ~EFI_PAGE_MASK;
+   if (efi_add_memory_map(addr, pages, EFI_RESERVED_MEMORY_TYPE,
+  false) != EFI_SUCCESS)
+   printf("Reserved memory mapping failed addr %llx size 
%llx\n",
+ (unsigned long long)addr, (unsigned long 
long)size);

+}
+
  /**
   * efi_carve_out_dt_rsv() - Carve out DT reserved memory ranges
   *
@@ -161,7 +175,8 @@ done:
  static void efi_carve_out_dt_rsv(void *fdt)
  {
 int nr_rsv, i;
-   uint64_t addr, size, pages;
+   uint64_t addr, size;
+   int nodeoffset, subnode;

 nr_rsv = fdt_num_mem_rsv(fdt);

@@ -169,15 +184,24 @@ static void efi_carve_out_dt_rsv(void *fdt)
 for (i = 0; i < nr_rsv; i++) {
 if (fdt_get_mem_rsv(fdt, i, , ) != 0)
 continue;
+   efi_reserve_memory(addr, size);
+   }

-   /* Convert from sandbox address space. */
-   addr = (uintptr_t)map_sysmem(addr, 0);
-
-   pages = efi_size_in_pages(size + (addr & 
EFI_PAGE_MASK));

-   addr &= ~EFI_PAGE_MASK;
-   if (efi_add_memory_map(addr, pages, 
EFI_RESERVED_MEMORY_TYPE,

-  false) != EFI_SUCCESS)
-   printf("FDT memrsv map %d: Failed to add to 
map\n", i);

+   /* process reserved-memory */
+   nodeoffset = fdt_subnode_offset(fdt, 0, "reserved-memory");
+   if (nodeoffset >= 0) {
+   subnode = fdt_first_subnode(fdt, nodeoffset);
+   while (subnode >= 0) {
+   /* check if this subnode has a reg property */
+   addr = fdtdec_get_addr_size(fdt, subnode, "reg",
+   (fdt_size_t 
*));

+   if (addr == FDT_ADDR_T_NONE) {
+   debug("failed to read address/size\n");


Linux
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt has:

"Each child of the reserved-memory node specifies one or more regions of
reserved memory. Each child node may either use a 'reg' property to
specify a specific range of reserved memory, or a 'size' property with
optional constraints to request a dynamically allocated block of memory.
... If both reg and size are present, then the reg property takes
precedence and size is ignored."

So finding no reg property should not be considered a bug. I see no need
for a debug statement here.

For the sandbox test we should have a node with 'size' and at least two
nodes with 'reg'.


+   continue;


You do not advance subnode here, creating an endless loop. So this 
should be:


/*
 * The /reserved-memory node may have children with
 * a size instead of a reg property.
 */
if (addr != FDT_ADDR_T_NONE)
efi_reserve_memory(addr, size);
    subnode = fdt_next_subnode(fdt, subnode);
}

Best regards

Heinrich


+   }
+   efi_reserve_memory(addr, size);
+   subnode = fdt_next_subnode(fdt, subnode);
+   }
 }
  }

--
2.25.1



Fixed palmer's email address. Sorry for the spam.







[PATCH] riscv: ax25: cache: Remove SPL_RISCV_MMODE config check

2020-03-14 Thread Pragnesh Patel
CONFIG_IS_ENABLED(FOO) will check FOO config option for U-boot proper,
SPL and TPL, so remove unnecessary CONFIG_IS_ENABLED()

Signed-off-by: Pragnesh Patel 
---
 arch/riscv/cpu/ax25/cache.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/cpu/ax25/cache.c b/arch/riscv/cpu/ax25/cache.c
index 9f424198b4..9df629d23c 100644
--- a/arch/riscv/cpu/ax25/cache.c
+++ b/arch/riscv/cpu/ax25/cache.c
@@ -12,7 +12,7 @@
 #include 
 
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
 /* mcctlcommand */
 #define CCTL_REG_MCCTLCOMMAND_NUM  0x7cc
 
@@ -47,7 +47,7 @@ void flush_dcache_all(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL);
 #endif
 #endif
@@ -68,7 +68,7 @@ void icache_enable(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"csrr t1, mcache_ctl\n\t"
"ori t0, t1, 0x1\n\t"
@@ -83,7 +83,7 @@ void icache_disable(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"fence.i\n\t"
"csrr t1, mcache_ctl\n\t"
@@ -99,7 +99,7 @@ void dcache_enable(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"csrr t1, mcache_ctl\n\t"
"ori t0, t1, 0x2\n\t"
@@ -117,7 +117,7 @@ void dcache_disable(void)
 {
 #if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
csr_write(CCTL_REG_MCCTLCOMMAND_NUM, CCTL_L1D_WBINVAL_ALL);
asm volatile (
"csrr t1, mcache_ctl\n\t"
@@ -137,7 +137,7 @@ int icache_status(void)
int ret = 0;
 
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"csrr t1, mcache_ctl\n\t"
"andi   %0, t1, 0x01\n\t"
@@ -156,7 +156,7 @@ int dcache_status(void)
int ret = 0;
 
 #ifdef CONFIG_RISCV_NDS_CACHE
-#if CONFIG_IS_ENABLED(RISCV_MMODE) || CONFIG_IS_ENABLED(SPL_RISCV_MMODE)
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
asm volatile (
"csrr t1, mcache_ctl\n\t"
"andi   %0, t1, 0x02\n\t"
-- 
2.17.1



[PATCH 1/1] cmd: map addresses to sysmem in efidebug memmap

2020-03-14 Thread Heinrich Schuchardt
Addresses in the sandbox's device tree are in the sandbox's virtual address
space. If we want to compare memory reservations in the device-tree with
the output of 'efidebug memmap', we need to convert back to this address
space.

Adjust the output of the 'efidebug memmap' command.

Signed-off-by: Heinrich Schuchardt 
---
 cmd/efidebug.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 8c3681c37d..bb7c13d6a1 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 

@@ -488,9 +489,10 @@ static int do_efi_show_memmap(cmd_tbl_t *cmdtp, int flag,

printf("%-16s %.*llx-%.*llx", type,
   EFI_PHYS_ADDR_WIDTH,
-  map->physical_start,
+  (u64)map_to_sysmem((void *)map->physical_start),
   EFI_PHYS_ADDR_WIDTH,
-  map->physical_start + map->num_pages * EFI_PAGE_SIZE);
+  (u64)map_to_sysmem((void *)map->physical_start +
+ map->num_pages * EFI_PAGE_SIZE));

print_memory_attributes(map->attribute);
putc('\n');
--
2.25.1



Re: [PATCH v2 2/4] cmd: bootefi: Parse reserved-memory node from DT

2020-03-14 Thread Heinrich Schuchardt

On 3/14/20 1:55 AM, Atish Patra wrote:

On Fri, Mar 13, 2020 at 5:12 PM Atish Patra  wrote:


Currently, bootefi only parses memory reservation block to setup
EFI reserved memory mappings. However, it doesn't parse the
reserved-memory[1] device tree node that also can contain the
reserved memory regions.

Add capability to parse reserved-memory node and update the EFI memory
mappings accordingly.


Hello Atish,

thanks for reporting the problem.

I would appreciate a test to verify that the code really works. We
should add both types of reserved-memory device tree node to the sandbox
device-tree and parse the output of 'efidebug memmap' and compare it to
the output of 'fdt print'. Could you, please, provide a first patch for
sandbox.dts and sandbox64.dts. The next step then would be to build a
Python test.

This patch series could be split into two. One for EFI and one for RISC-V.

(see comment below)



1. /doc/device-tree-bindings/reserved-memory/reserved-memory.txt]

Signed-off-by: Atish Patra 
---
  cmd/bootefi.c | 42 +-
  1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 24fc42ae898e..43b36fbacfcd 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -149,6 +149,20 @@ done:
 return ret;
  }

+static void efi_reserve_memory(uint64_t addr, uint64_t size)
+{
+   uint64_t pages;
+
+   /* Convert from sandbox address space. */
+   addr = (uintptr_t)map_sysmem(addr, 0);
+   pages = efi_size_in_pages(size + (addr & EFI_PAGE_MASK));
+   addr &= ~EFI_PAGE_MASK;
+   if (efi_add_memory_map(addr, pages, EFI_RESERVED_MEMORY_TYPE,
+  false) != EFI_SUCCESS)
+   printf("Reserved memory mapping failed addr %llx size %llx\n",
+ (unsigned long long)addr, (unsigned long long)size);
+}
+
  /**
   * efi_carve_out_dt_rsv() - Carve out DT reserved memory ranges
   *
@@ -161,7 +175,8 @@ done:
  static void efi_carve_out_dt_rsv(void *fdt)
  {
 int nr_rsv, i;
-   uint64_t addr, size, pages;
+   uint64_t addr, size;
+   int nodeoffset, subnode;

 nr_rsv = fdt_num_mem_rsv(fdt);

@@ -169,15 +184,24 @@ static void efi_carve_out_dt_rsv(void *fdt)
 for (i = 0; i < nr_rsv; i++) {
 if (fdt_get_mem_rsv(fdt, i, , ) != 0)
 continue;
+   efi_reserve_memory(addr, size);
+   }

-   /* Convert from sandbox address space. */
-   addr = (uintptr_t)map_sysmem(addr, 0);
-
-   pages = efi_size_in_pages(size + (addr & EFI_PAGE_MASK));
-   addr &= ~EFI_PAGE_MASK;
-   if (efi_add_memory_map(addr, pages, EFI_RESERVED_MEMORY_TYPE,
-  false) != EFI_SUCCESS)
-   printf("FDT memrsv map %d: Failed to add to map\n", i);
+   /* process reserved-memory */
+   nodeoffset = fdt_subnode_offset(fdt, 0, "reserved-memory");
+   if (nodeoffset >= 0) {
+   subnode = fdt_first_subnode(fdt, nodeoffset);
+   while (subnode >= 0) {
+   /* check if this subnode has a reg property */
+   addr = fdtdec_get_addr_size(fdt, subnode, "reg",
+   (fdt_size_t *));
+   if (addr == FDT_ADDR_T_NONE) {
+   debug("failed to read address/size\n");


Linux
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt has:

"Each child of the reserved-memory node specifies one or more regions of
reserved memory. Each child node may either use a 'reg' property to
specify a specific range of reserved memory, or a 'size' property with
optional constraints to request a dynamically allocated block of memory.
... If both reg and size are present, then the reg property takes
precedence and size is ignored."

So finding no reg property should not be considered a bug. I see no need
for a debug statement here.

For the sandbox test we should have a node with 'size' and at least two
nodes with 'reg'.

Otherwise:

Reviewed-by: Heinrich Schuchardt 


+   continue;
+   }
+   efi_reserve_memory(addr, size);
+   subnode = fdt_next_subnode(fdt, subnode);
+   }
 }
  }

--
2.25.1



Fixed palmer's email address. Sorry for the spam.