Re: [PATCH] spl: fit: Skip attempting to load 0 length image

2021-10-19 Thread Aswath Govindraju
On 19/10/21 11:02 pm, Nishanth Menon wrote:
> When, for various reasons, a bad FIT image is used where a loadable
> image is marked as 0 length, attempt is made for a 0 length allocation and
> read of 0 byte read operation.
> 
> Instead provide warning in log and skip attempting to do such a load.
> 
> Signed-off-by: Nishanth Menon 
> ---

Reviewed-by: Aswath Govindraju 

Thanks,
Aswath

>  common/spl/spl_fit.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index f41abca0ccb5..e9540dc2167a 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -286,6 +286,13 @@ static int spl_load_fit_image(struct spl_load_info 
> *info, ulong sector,
>   if (fit_image_get_data_size(fit, node, ))
>   return -ENOENT;
>  
> + /* Dont bother to copy 0 byte data, but warn, though */
> + if (!len) {
> + log_warning("%s: Skip load '%s': image size is 0!\n",
> + __func__, fit_get_name(fit, node, NULL));
> + return 0;
> + }
> +
>   src_ptr = map_sysmem(ALIGN(load_addr, ARCH_DMA_MINALIGN), len);
>   length = len;
>  
> 



Re: [PATCH v2] driver: spi: add bcm iproc qspi support.

2021-10-19 Thread Rayagonda Kokatanur
On Wed, Oct 20, 2021 at 4:31 AM Roman Bacik  wrote:
>
> From: Rayagonda Kokatanur 
>
> IPROC qspi driver supports both BSPI and MSPI modes.
>
> Signed-off-by: Rayagonda Kokatanur 
> Signed-off-by: Bharat Gooty 
> Acked-by: Rayagonda Kokatanur 
>
> Signed-off-by: Roman Bacik 
> ---
>
> Changes in v2:
> - remove include spi-nor.h
> - define and use named BITs for writing register values
> - removed bspi_set_4byte_mode() method
>
>  drivers/spi/Kconfig  |   6 +
>  drivers/spi/Makefile |   1 +
>  drivers/spi/iproc_qspi.c | 712 +++
>  drivers/spi/iproc_qspi.h |  20 ++
>  4 files changed, 739 insertions(+)
>  create mode 100644 drivers/spi/iproc_qspi.c
>  create mode 100644 drivers/spi/iproc_qspi.h
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index d07e9a28af82..e76fadef32dd 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -178,6 +178,12 @@ config ICH_SPI
>   access the SPI NOR flash on platforms embedding this Intel
>   ICH IP core.
>
> +config IPROC_QSPI
> +   bool "QSPI driver for BCM iProc QSPI Controller"
> +   help
> + This selects the BCM iProc QSPI controller.
> + This driver support spi flash single, quad and memory reads.
> +
>  config KIRKWOOD_SPI
> bool "Marvell Kirkwood SPI Driver"
> help
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index d2f24bccefd3..869763187062 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -33,6 +33,7 @@ obj-$(CONFIG_FSL_DSPI) += fsl_dspi.o
>  obj-$(CONFIG_FSL_ESPI) += fsl_espi.o
>  obj-$(CONFIG_SYNQUACER_SPI) += spi-synquacer.o
>  obj-$(CONFIG_ICH_SPI) +=  ich.o
> +obj-$(CONFIG_IPROC_QSPI) += iproc_qspi.o
>  obj-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o
>  obj-$(CONFIG_MESON_SPIFC) += meson_spifc.o
>  obj-$(CONFIG_MPC8XX_SPI) += mpc8xx_spi.o
> diff --git a/drivers/spi/iproc_qspi.c b/drivers/spi/iproc_qspi.c
> new file mode 100644
> index ..91afeb54fc57
> --- /dev/null
> +++ b/drivers/spi/iproc_qspi.c
> @@ -0,0 +1,712 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020-2021 Broadcom
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "iproc_qspi.h"
> +
> +/* 175MHz */
> +#define QSPI_AXI_CLK   17500
> +#define QSPI_DEF_SCK_FREQ  5000
> +#define QSPI_WAIT_TIMEOUT_MS   200U
> +#define DWORD_ALIGNED(a)   (!(((ulong)(a)) & 3))
> +
> +/* Chip attributes */
> +#define SPBR_MIN   8U
> +#define SPBR_MAX   255U
> +#define NUM_CDRAM  16U
> +
> +#define CDRAM_PCS0 2
> +#define CDRAM_CONT BIT(7)
> +#define CDRAM_BITS_EN  BIT(6)
> +#define CDRAM_QUAD_MODEBIT(8)
> +#define CDRAM_RBIT_INPUT   BIT(10)
> +#define MSPI_SPE   BIT(6)
> +#define MSPI_CONT_AFTER_CMDBIT(7)
> +
> +/* Register fields */
> +#define MSPI_SPCR0_MSB_BITS_8  0x0020
> +#define BSPI_RAF_CONTROL_START_MASK0x0001
> +#define BSPI_RAF_STATUS_SESSION_BUSY_MASK  0x0001
> +#define BSPI_RAF_STATUS_FIFO_EMPTY_MASK0x0002
> +#define BSPI_BITS_PER_PHASE_ADDR_MARK  0x0001
> +#define BSPI_BITS_PER_CYCLE_DATA_SHIFT 0
> +#define BSPI_BITS_PER_CYCLE_ADDR_SHIFT 16
> +#define BSPI_STRAP_OVERRIDE_DATA_QUAD_SHIFT3
> +#define BSPI_STRAP_OVERRIDE_DATA_DUAL_SHIFT1
> +#define BSPI_STRAP_OVERRIDE_SHIFT  0
> +
> +/* MSPI registers */
> +#define MSPI_SPCR0_LSB_REG 0x000
> +#define MSPI_SPCR0_MSB_REG 0x004
> +#define MSPI_SPCR1_LSB_REG 0x008
> +#define MSPI_SPCR1_MSB_REG 0x00c
> +#define MSPI_NEWQP_REG 0x010
> +#define MSPI_ENDQP_REG 0x014
> +#define MSPI_SPCR2_REG 0x018
> +#define MSPI_STATUS_REG0x020
> +#define MSPI_CPTQP_REG 0x024
> +#define MSPI_TXRAM_REG 0x040
> +#define MSPI_RXRAM_REG 0x0c0
> +#define MSPI_CDRAM_REG 0x140
> +#define MSPI_WRITE_LOCK_REG0x180
> +#define MSPI_DISABLE_FLUSH_GEN_REG 0x184
> +
> +/* BSPI registers */
> +#define BSPI_REVISION_ID_REG   0x000
> +#define BSPI_SCRATCH_REG   0x004
> +#define BSPI_MAST_N_BOOT_CTRL_REG  0x008
> +#define BSPI_BUSY_STATUS_REG   0x00c
> +#define BSPI_INTR_STATUS_REG   0x010
> +#define BSPI_B0_STATUS_REG 0x014
> +#define BSPI_B0_CTRL_REG   0x018
> +#define 

Re: [PATCH 2/2] sandbox: provide /chosen/boot-hartid property

2021-10-19 Thread Heinrich Schuchardt

On 10/20/21 00:54, Simon Glass wrote:

Hi Heinrich,

On Sat, 28 Aug 2021 at 03:42, Heinrich Schuchardt  wrote:


On RISC-V the sandbox must provide the /chosen/boot-hartid in the
devicetree.

Signed-off-by: Heinrich Schuchardt 
---
  arch/sandbox/lib/Makefile|  2 +-
  arch/sandbox/lib/fdt_fixup.c | 25 +
  2 files changed, 26 insertions(+), 1 deletion(-)
  create mode 100644 arch/sandbox/lib/fdt_fixup.c


Can you add a bit more detail here? This is for the sandbox build, so
we don't actually get to boot the kernel. So who is actually using
this data?


The U-Boot sandbox can be built on RISC-V. It is very convenient to
debug the Linux EFI stub and its interaction with other UEFI software
like GRUB.

The Linux EFI stub reads the /chosen/boot-hartid property in function
get_boot_hartid_from_fdt(), drivers/firmware/efi/libstub/riscv-stub.c.
It will stop booting if /chosen/boot-hartid is not found.

Of course the kernel will fail when trying to set up virtual memory. But
that is after most of the interesting stuff in the EFI stub have
occurred. Linux can run in nommu mode. So maybe you could even run Linux
on the sandbox.

Best regards

Heinrich



Regards,
Simon




diff --git a/arch/sandbox/lib/Makefile b/arch/sandbox/lib/Makefile
index b4ff717e78..a2bc5a7ee6 100644
--- a/arch/sandbox/lib/Makefile
+++ b/arch/sandbox/lib/Makefile
@@ -5,7 +5,7 @@
  # (C) Copyright 2002-2006
  # Wolfgang Denk, DENX Software Engineering, w...@denx.de.

-obj-y  += interrupts.o sections.o
+obj-y  += fdt_fixup.o interrupts.o sections.o
  obj-$(CONFIG_PCI)  += pci_io.o
  obj-$(CONFIG_CMD_BOOTM) += bootm.o
  obj-$(CONFIG_CMD_BOOTZ) += bootm.o
diff --git a/arch/sandbox/lib/fdt_fixup.c b/arch/sandbox/lib/fdt_fixup.c
new file mode 100644
index 00..a646f2059c
--- /dev/null
+++ b/arch/sandbox/lib/fdt_fixup.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#define LOG_CATEGORY LOGC_ARCH
+
+#include 
+#include 
+#include 
+
+#if defined(__riscv)
+int arch_fixup_fdt(void *blob)
+{
+   int ret;
+
+   ret = fdt_find_or_add_subnode(blob, 0, "chosen");;
+   if (ret < 0)
+   goto err;
+   ret = fdt_setprop_u32(blob, ret, "boot-hartid", 1);
+   if (ret < 0)
+   goto err;
+   return 0;
+err:
+   log_err("Setting /chosen/boot-hartid failed: %s\n", fdt_strerror(ret));
+   return ret;
+}
+#endif
--
2.30.2



Re: [PATCH] part: return -ENOSYS when get_info not valid.

2021-10-19 Thread Heinrich Schuchardt




On 10/20/21 04:37, schspa wrote:

在 2021-10-19星期二的 17:23 +0200,Heinrich Schuchardt写道:

On 10/19/21 15:35, zhaohui.shi wrote:

From: schspa 

In some case, get_info() interface can be NULL, add this check to
stop
from crash.


Thank you for reviewing the partition driver code.

There seems to be no driver missing a get_info implementation. Where
did
you run into a problem?


Yes, I do run into a problem, In my spl build, CONFIG_SPL_FS_EXT4,
CONFIG_SPL_FS_FAT, CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION are all
not enabled. In this case, get_info implementation is NULL. see
'part_get_info_ptr' and 'part_print_ptr' and part_efi.c for detail.


Why should we only check .get_info and not .test and not .print?



All part type driver have .test implementation, it can't be NULL,
and .print have NULL pointer judgement already.



Signed-off-by: schspa 


Please, provide a name.


---
   disk/part.c | 7 +++
   1 file changed, 7 insertions(+)

diff --git a/disk/part.c b/disk/part.c
index a6a8f7052b..7af3240ec7 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -668,6 +668,13 @@ int part_get_info_by_name_type(struct blk_desc
*dev_desc, const char *name,
 part_drv = part_driver_lookup_type(dev_desc);
 if (!part_drv)
 return -1;
+
+   if (!part_drv->get_info) {
+   PRINTF("## Driver %s does not have the get_info()
method\n",
+  part_drv->name);


Please, use log_debug() to avoid noise on the console on every boot.



I think it's OK to use PRINTF, because of this BUG occurs only when
user give a bad part configuration, and this error message can prompt
the user that a configuration error has occurred.
Besides, 'part_get_info' use PRINTF for this null pointer protection
too.


Above you write that part_drv->get_info = NULL is part of your
configuration. So it will always be printed. The size of the SPL is very
critical on many boards. We should avoid anything that increases it.

Why is part_get_info_by_name_type() being called in your configuration
which does not provide a partition driver in SPL? Are there some
dependencies missing in the Kconfig files?

Best regards

Heinrich




Best regards

Heinrich


+   return -ENOSYS;
+   }
+
 for (i = 1; i < part_drv->max_entries; i++) {
 ret = part_drv->get_info(dev_desc, i, info);
 if (ret != 0) {





[PATCH 3/3] RFC: Example breakage

2021-10-19 Thread Simon Glass
This is just for illustration. Do not apply.

Signed-off-by: Simon Glass 
---

 Kconfig | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Kconfig b/Kconfig
index 931a22806e4..142f4e7fcc7 100644
--- a/Kconfig
+++ b/Kconfig
@@ -20,6 +20,11 @@ config BROKEN
  This option cannot be enabled. It is used as dependency
  for broken and incomplete features.
 
+config BREAK_ME
+   int "Break things"
+   help
+ This should break things.
+
 config DEPRECATED
bool
help
-- 
2.33.0.1079.g6e70778dc9-goog



[PATCH 2/3] buildman: Detect Kconfig loops

2021-10-19 Thread Simon Glass
Hex and int Kconfig options are supposed to have defaults. This is so we
can configure U-Boot without having to enter particular values for the
items that don't have specific values in the board's defconfig file.

If this rule is not followed, then introducing a new Kconfig can produce
a loop like this:

   Break things (BREAK_ME) [] (NEW)
   Error in reading or end of file.

   Break things (BREAK_ME) [] (NEW)
   Error in reading or end of file.

The continues forever since buildman passes /dev/null to 'conf', and
the build system just tries again. Eventually there is so much output that
buildman runs out of memory.

We can detect this situation by looking for a symbol (like 'BREAK_ME')
which has no default (the '[]' above) and is marked as new. If this
appears multiple times in the output, we know something is wrong.

Add a filter function for the output which detects this situation. Allow
it to return True to terminate the process. Implement this termination in
cros_subprocess.

With this we get a nice message:

   buildman --board sandbox -T0
   Building current source for 1 boards (0 threads, 32 jobs per thread)
  sandbox:  w+   sandbox
   +.config:66:warning: symbol value '' invalid for BREAK_ME
   +
   +Error in reading or end of file.
   +make[3]: *** [scripts/kconfig/Makefile:75: syncconfig] Terminated
   +make[2]: *** [Makefile:569: syncconfig] Terminated
   +make: *** [Makefile:177: sub-make] Terminated
   +(** did you define an int/hex Kconfig with no default? **)

Signed-off-by: Simon Glass 
---

 tools/buildman/builder.py   | 43 -
 tools/patman/command.py |  7 --
 tools/patman/cros_subprocess.py | 10 ++--
 3 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index ce852eb03a3..122f0d14065 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -24,6 +24,17 @@ from patman import gitutil
 from patman import terminal
 from patman.terminal import Print
 
+# This indicates an new int or hex Kconfig property with no default
+# It hangs the build since the 'conf' tool cannot proceed without valid input.
+#
+# We get a repeat sequence of something like this:
+# >>
+# Break things (BREAK_ME) [] (NEW)
+# Error in reading or end of file.
+# <<
+# which indicates that BREAK_ME has an empty default
+RE_NO_DEFAULT = re.compile(b'\((\w+)\) \[] \(NEW\)')
+
 """
 Theory of Operation
 
@@ -200,6 +211,8 @@ class Builder:
 _working_dir: Base working directory containing all threads
 _single_builder: BuilderThread object for the singer builder, if
 threading is not being used
+_terminated: Thread was terminated due to an error
+_restarting_config: True if 'Restart config' is detected in output
 """
 class Outcome:
 """Records a build outcome for a single make invocation
@@ -304,6 +317,8 @@ class Builder:
 self.work_in_output = work_in_output
 if not self.squash_config_y:
 self.config_filenames += EXTRA_CONFIG_FILENAMES
+self._terminated = False
+self._restarting_config = False
 
 self.warnings_as_errors = warnings_as_errors
 self.col = terminal.Color()
@@ -429,9 +444,35 @@ class Builder:
 args: Arguments to pass to make
 kwargs: Arguments to pass to command.RunPipe()
 """
+
+def check_output(stream, data):
+if b'Restart config' in data:
+self._restarting_config = True
+
+# If we see 'Restart config' following by multiple errors
+if self._restarting_config:
+m = RE_NO_DEFAULT.findall(data)
+
+# Number of occurences of each Kconfig item
+multiple = [m.count(val) for val in set(m)]
+
+# If any of them occur more than once, we have a loop
+if [val for val in multiple if val > 1]:
+self._terminated = True
+return True
+return False
+
+self._restarting_config = False
+self._terminated  = False
 cmd = [self.gnu_make] + list(args)
 result = command.RunPipe([cmd], capture=True, capture_stderr=True,
-cwd=cwd, raise_on_error=False, infile='/dev/null', **kwargs)
+cwd=cwd, raise_on_error=False, infile='/dev/null',
+output_func=check_output, **kwargs)
+
+if self._terminated:
+# Try to be helpful
+result.stderr += '(** did you define an int/hex Kconfig with no 
default? **)'
+
 if self.verbose_build:
 result.stdout = '%s\n' % (' '.join(cmd)) + result.stdout
 result.combined = '%s\n' % (' '.join(cmd)) + result.combined
diff --git a/tools/patman/command.py b/tools/patman/command.py
index bf8ea6c8c3c..d54b1e0efce 100644
--- a/tools/patman/command.py
+++ b/tools/patman/command.py
@@ -49,7 +49,8 @@ test_result = None

[PATCH 1/3] buildman: Write output even on fatal error

2021-10-19 Thread Simon Glass
At present buildman does not write any output (to the 'out' and 'err)
files if the build terminates with a fatal error. This is to avoid adding
lots of spam to the logs.

However there are times when this is actually useful, such as when the
build fails for an obscure reason such as a Kconfig loop.

Update the logic to always write the output, so that the user gets a clue
as to what is happening.

Signed-off-by: Simon Glass 
---

 tools/buildman/builderthread.py | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 48128cf6732..3e450e40670 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -300,16 +300,12 @@ class BuilderThread(threading.Thread):
 work_in_output: Use the output directory as the work directory and
 don't write to a separate output directory.
 """
-# Fatal error
-if result.return_code < 0:
-return
-
 # If we think this might have been aborted with Ctrl-C, record the
 # failure but not that we are 'done' with this board. A retry may fix
 # it.
-maybe_aborted =  result.stderr and 'No child processes' in 
result.stderr
+maybe_aborted = result.stderr and 'No child processes' in result.stderr
 
-if result.already_done:
+if result.return_code >= 0 and result.already_done:
 return
 
 # Write the output and stderr
@@ -332,6 +328,10 @@ class BuilderThread(threading.Thread):
 elif os.path.exists(errfile):
 os.remove(errfile)
 
+# Fatal error
+if result.return_code < 0:
+return
+
 if result.toolchain:
 # Write the build result and toolchain information.
 done_file = self.builder.GetDoneFile(result.commit_upto,
-- 
2.33.0.1079.g6e70778dc9-goog



Re: buildman stops (crashed) on current master

2021-10-19 Thread Simon Glass
Hi,

On Tue, 19 Oct 2021 at 17:01, Tom Rini  wrote:
>
> On Tue, Oct 19, 2021 at 04:59:15PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Tue, 19 Oct 2021 at 16:53, Tom Rini  wrote:
> > >
> > > On Tue, Oct 19, 2021 at 05:39:12PM +0200, Stefano Babic wrote:
> > > > Hi Simon,
> > > >
> > > > On 07.10.21 15:43, Simon Glass wrote:
> > > > > Hi Stefano,
> > > > >
> > > > > On Thu, 7 Oct 2021 at 04:37, Stefano Babic  wrote:
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > CI stops by building aarch64 without notice, for reference:
> > > > > >
> > > > > > https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/332319
> > > > > >
> > > > > > There is no error, just process is killed. It looks like it stops at
> > > > > > xilinx_zynqmp_virt,
> > > > > >
> > > > > > ./tools/buildman/buildman -o /tmp -P -E -W aarch64but board can be 
> > > > > > built
> > > > > > without issues.
> > > > > >
> > > > > > If I build on my host (not in docker, anyway), it generally builds 
> > > > > > fine
> > > > > > - but it crashes sometimes, too. On gitlab instance , it crashes.
> > > > > > Issue does not seem that depends on merged patches, and introduces
> > > > > > boards were already built successfully. Any hint ? I have also no 
> > > > > > idea
> > > > > > what I should look as what I see is just
> > > > > >
> > > > > > "usr/bin/bash: line 104:24 Killed
> > > > > > ./tools/buildman/buildman -o /tmp -P -E -W aarch64"
> > > > >
> > > > > I cannot see that link. I am not sure what is going on. Does it say
> > > > > what signal killed it?
> > > >
> > > > Pipelines on our server were not public - I have enbaled now for 
> > > > u-boot-imx.
> > > >
> > > > >
> > > > > Does it sit there for an hour and timeout? If so, then I  did see that
> > > > > myself once recently, when the Kconfig needed stdin, but I could not
> > > > > quitetie it down. I think buildman would provide it, but sometimes
> > > > > not, apparently. So it can happen when there is an existing build
> > > > > there and your new one which adds Kconfig options that don't have
> > > > > defaults, or something like that?
> > > > >
> > > >
> > > > I have investigated further, and I can reproduce it on my host outside 
> > > > the
> > > > gitlab server. buildman causes a OOM, but I cannot find the cause.
> > > >
> > > > Strange enough, this happens with the "aarch64" target, and I cannot
> > > > reproduce it with Tom's master. So it seems that -master is ok, and 
> > > > somethin
> > > > on u-boot-imx generates the OOM.
> > > >
> > > > However
> > > >
> > > > The OOM happens always when -2 (two boards remain) appears. I can see 
> > > > with
> > > > htop that buildman starts to allocate memory until it is exhausted 
> > > > (64GB RAM
> > > > + 8 GB swap). Then the kernel decides that it is enough and kills 
> > > > buildman -
> > > > this is what I see on Ci.
> > > >
> > > > You can see now the pipelines:
> > > >
> > > > https://source.denx.de/u-boot/custodians/u-boot-imx/-/pipelines/9520
> > > >
> > > > I have then split aarch64 and I built imx8 separately - same result. The
> > > > pipeline stops with xilinx board, but they have nothing to do. In fact, 
> > > > I
> > > > can build all xilinx board separately. If I run buildman -W aarch64 -x
> > > > xilinx, OOM is shown by another board.
> > > >
> > > > Strange enough, I can build each single board with buildman without 
> > > > issues,
> > > > neither errors nor warnongs. Just when buildman runs all together 
> > > > (aarch64,
> > > > 308 boards), the OOM is generated.
> > > >
> > > > Bisect does not help: I started bisect, and at the end this commit was
> > > > presented:
> > > >
> > > > commit 53a24dee86fb72ae41e7579607bafe13442616f2
> > > > Author: Fabio Estevam 
> > > > Date:   Mon Aug 23 21:11:09 2021 -0300
> > > >
> > > > imx8mm-cl-iot-gate: Split the defconfigs
> > >
> > > I strongly suspect what's going on here is that these new defconfigs are
> > > out of sync with changes now in Kconfig.  The build itself will just sit
> > > there, waiting for the "oldconfig" prompt to be answered.
> > >
> > > I want to say the problem here is that stdin is open, rather than
> > > pointing to something closed and would lead to the build failing
> > > immediately, rather than once a timeout is hit, or OOM kicks in due to
> > > kconfig chewing up all the memory.
> >
> > Yes that's exactly what I saw...
> >
> > In fact, see this commit:
> >
> > e62a24ce27a buildman: Avoid hanging when the config changes
> >
> > But that was 3 years ago.
>
> Looks like something else needs to be changed then, I've bisected down
> similar failures here before very recently.

I dug into this a bit and I think buildman can detect this situation.
I'll send a little series.

Regards,
Simon


[PATCH next v7 08/12] ARM: dts: ast2600: Add ACRY to device tree

2021-10-19 Thread Chia-Wei Wang
Add ACRY DTS node and enable it for AST2600 EVB.

Signed-off-by: Chia-Wei Wang 
---
 arch/arm/dts/ast2600-evb.dts | 5 +
 arch/arm/dts/ast2600.dtsi| 9 +
 2 files changed, 14 insertions(+)

diff --git a/arch/arm/dts/ast2600-evb.dts b/arch/arm/dts/ast2600-evb.dts
index adb80a30ef..05362d19bd 100644
--- a/arch/arm/dts/ast2600-evb.dts
+++ b/arch/arm/dts/ast2600-evb.dts
@@ -182,3 +182,8 @@
u-boot,dm-pre-reloc;
status = "okay";
 };
+
+ {
+   u-boot,dm-pre-reloc;
+   status = "okay";
+};
diff --git a/arch/arm/dts/ast2600.dtsi b/arch/arm/dts/ast2600.dtsi
index b8fe966c7d..31905fd208 100644
--- a/arch/arm/dts/ast2600.dtsi
+++ b/arch/arm/dts/ast2600.dtsi
@@ -195,6 +195,15 @@
status = "disabled";
};
 
+   acry: acry@1e6fa000 {
+   compatible = "aspeed,ast2600-acry";
+   reg = <0x1e6fa000 0x1000>,
+ <0x1e71 0x1>;
+   interrupts = ;
+   clocks = < ASPEED_CLK_GATE_RSACLK>;
+   status = "disabled";
+   };
+
edac: sdram@1e6e {
compatible = "aspeed,ast2600-sdram-edac";
reg = <0x1e6e 0x174>;
-- 
2.17.1



[PATCH next v7 09/12] ast2600: spl: Locate load buffer in DRAM space

2021-10-19 Thread Chia-Wei Wang
Return CONFIG_SYS_LOAD_ADDR pointing to DRAM space for
spl_get_load_buffer() to allow generic SPL image loading
code (e.g. FIT and Ymodem) to store data in DRAM.

Signed-off-by: Chia-Wei Wang 
---
 arch/arm/mach-aspeed/ast2600/spl.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/arm/mach-aspeed/ast2600/spl.c 
b/arch/arm/mach-aspeed/ast2600/spl.c
index 0d8cb29678..6c49d6aede 100644
--- a/arch/arm/mach-aspeed/ast2600/spl.c
+++ b/arch/arm/mach-aspeed/ast2600/spl.c
@@ -28,14 +28,7 @@ u32 spl_boot_device(void)
 
 struct image_header *spl_get_load_buffer(ssize_t offset, size_t size)
 {
-   /*
-* When boot from SPI, AST2600 already remap 0x ~ 0x0fff
-* to BMC SPI memory space 0x2000 ~ 0x2fff. The next stage BL
-* has been located in SPI for XIP. In this case, the load buffer for
-* SPL image loading will be set to the remapped address of the next
-* BL instead of the DRAM space CONFIG_SYS_LOAD_ADDR
-*/
-   return (struct image_header *)(CONFIG_SYS_TEXT_BASE);
+   return (struct image_header *)(CONFIG_SYS_LOAD_ADDR);
 }
 
 #ifdef CONFIG_SPL_OS_BOOT
-- 
2.17.1



[PATCH next v7 04/12] crypto: aspeed: Add AST2600 HACE support

2021-10-19 Thread Chia-Wei Wang
From: Johnny Huang 

Hash and Crypto Engine (HACE) is designed to accelerate the
throughput of hash data digest, and symmetric-key encryption.

Signed-off-by: Johnny Huang 
Signed-off-by: Chia-Wei Wang 
---
 drivers/crypto/Kconfig  |   2 +
 drivers/crypto/Makefile |   1 +
 drivers/crypto/aspeed/Kconfig   |  10 +
 drivers/crypto/aspeed/Makefile  |   1 +
 drivers/crypto/aspeed/aspeed_hace.c | 381 
 drivers/crypto/hash/Kconfig |   8 +
 6 files changed, 403 insertions(+)
 create mode 100644 drivers/crypto/aspeed/Kconfig
 create mode 100644 drivers/crypto/aspeed/Makefile
 create mode 100644 drivers/crypto/aspeed/aspeed_hace.c

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 0082177c21..675081ecd3 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -4,4 +4,6 @@ source drivers/crypto/hash/Kconfig
 
 source drivers/crypto/fsl/Kconfig
 
+source drivers/crypto/aspeed/Kconfig
+
 endmenu
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index e8bae43e3f..6b762565a1 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_EXYNOS_ACE_SHA)+= ace_sha.o
 obj-y += rsa_mod_exp/
 obj-y += fsl/
 obj-y += hash/
+obj-y += aspeed/
diff --git a/drivers/crypto/aspeed/Kconfig b/drivers/crypto/aspeed/Kconfig
new file mode 100644
index 00..471c06f986
--- /dev/null
+++ b/drivers/crypto/aspeed/Kconfig
@@ -0,0 +1,10 @@
+config ASPEED_HACE
+   bool "ASPEED Hash and Crypto Engine"
+   depends on DM_HASH
+   help
+ Select this option to enable a driver for using the SHA engine in
+ the ASPEED BMC SoCs.
+
+ Enabling this allows the use of SHA operations in hardware without
+ requiring the SHA software implementations. It also improves 
performance
+ and saves code size.
diff --git a/drivers/crypto/aspeed/Makefile b/drivers/crypto/aspeed/Makefile
new file mode 100644
index 00..84e6bfe82a
--- /dev/null
+++ b/drivers/crypto/aspeed/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_ASPEED_HACE) += aspeed_hace.o
diff --git a/drivers/crypto/aspeed/aspeed_hace.c 
b/drivers/crypto/aspeed/aspeed_hace.c
new file mode 100644
index 00..1178cc6a76
--- /dev/null
+++ b/drivers/crypto/aspeed/aspeed_hace.c
@@ -0,0 +1,381 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2021 ASPEED Technology Inc.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* register offsets*/
+#define HACE_STS   0x1C
+#define   HACE_HASH_DATA_OVF   BIT(23)
+#define   HACE_HASH_INTBIT(9)
+#define   HACE_HASH_BUSY   BIT(0)
+#define HACE_HASH_DATA 0x20
+#define HACE_HASH_DIGEST   0x24
+#define HACE_HASH_HMAC_KEY 0x28
+#define HACE_HASH_DATA_LEN 0x2C
+#define HACE_HASH_CMD  0x30
+#define   HACE_HASH_MODE_ACCUM BIT(8)
+#define   HACE_HASH_ALGO_SHA1  BIT(5)
+#define   HACE_HASH_ALGO_SHA256(BIT(6) | BIT(4))
+#define   HACE_HASH_ALGO_SHA384(BIT(10) | BIT(6) | BIT(5))
+#define   HACE_HASH_ALGO_SHA512(BIT(6) | BIT(5))
+#define   HACE_HASH_SHA_BE_EN  BIT(3)
+
+/* buffer size based on SHA-512 need*/
+#define HASH_BLOCK_BUFSZ   128
+#define HASH_DIGEST_BUFSZ  64
+
+struct aspeed_hace_ctx {
+   uint8_t digest[HASH_DIGEST_BUFSZ];
+
+   uint32_t cmd;
+   enum HASH_ALGO algo;
+
+   uint32_t blk_size;
+   uint32_t pad_size;
+   uint64_t total[2];
+
+   uint8_t buf[HASH_BLOCK_BUFSZ];
+   uint32_t buf_cnt;
+} __aligned((8));
+
+struct aspeed_hace {
+   phys_addr_t base;
+   struct clk clk;
+};
+
+static const uint32_t iv_sha1[8] = {
+   0x01234567, 0x89abcdef, 0xfedcba98, 0x76543210,
+   0xf0e1d2c3, 0, 0, 0
+};
+
+static const uint32_t iv_sha256[8] = {
+   0x67e6096a, 0x85ae67bb, 0x72f36e3c, 0x3af54fa5,
+   0x7f520e51, 0x8c68059b, 0xabd9831f, 0x19cde05bUL
+};
+
+static const uint32_t iv_sha384[16] = {
+   0x5d9dbbcb, 0xd89e05c1, 0x2a299a62, 0x07d57c36,
+   0x5a015991, 0x17dd7030, 0xd8ec2f15, 0x39590ef7,
+   0x67263367, 0x310bc0ff, 0x874ab48e, 0x11155868,
+   0x0d2e0cdb, 0xa78ff964, 0x1d48b547, 0xa44ffabeUL
+};
+
+static const uint32_t iv_sha512[16] = {
+   0x67e6096a, 0x08c9bcf3, 0x85ae67bb, 0x3ba7ca84,
+   0x72f36e3c, 0x2bf894fe, 0x3af54fa5, 0xf1361d5f,
+   0x7f520e51, 0xd182e6ad, 0x8c68059b, 0x1f6c3e2b,
+   0xabd9831f, 0x6bbd41fb, 0x19cde05b, 0x79217e13UL
+};
+
+static int aspeed_hace_wait_completion(uint32_t reg, uint32_t flag, int 
timeout_us)
+{
+   uint32_t val;
+
+   return readl_poll_timeout(reg, val, (val & flag) == flag, timeout_us);
+}
+
+static int aspeed_hace_process(struct udevice *dev, void *ctx, const void 
*ibuf, uint32_t ilen)
+{
+   struct aspeed_hace *hace = dev_get_priv(dev);
+   

[PATCH next v7 12/12] configs: ast2600: Boot kernel FIT in DRAM

2021-10-19 Thread Chia-Wei Wang
AST2600 leverages the FIT hash/signature verification to fulfill
secure boot trust chain. To improve the performance and save SW
code size for those crypto operations, the two HW crypto engine,
HACE and ACRY, are enabled.

However, both of the engines can only access to data stored in
DRAM space. Therefore, we need to move the FIT image into DRAM
before the booting.

This patch update the CONFIG_BOOTCOMMAND to execute the pre-defined
ENV variable which consists of FIT image copy to memory and booting.

Signed-off-by: Chia-Wei Wang 
---
 configs/evb-ast2600_defconfig | 2 +-
 include/configs/evb_ast2600.h | 7 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
index eba6940ec1..abb156f13e 100644
--- a/configs/evb-ast2600_defconfig
+++ b/configs/evb-ast2600_defconfig
@@ -27,7 +27,7 @@ CONFIG_SPL_LOAD_FIT_ADDRESS=0x1
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyS4,115200n8 root=/dev/ram rw"
 CONFIG_USE_BOOTCOMMAND=y
-CONFIG_BOOTCOMMAND="bootm 2010"
+CONFIG_BOOTCOMMAND="run bootspi"
 # CONFIG_DISPLAY_CPUINFO is not set
 CONFIG_SPL_SIZE_LIMIT_SUBTRACT_GD=y
 CONFIG_SPL_SIZE_LIMIT_SUBTRACT_MALLOC=y
diff --git a/include/configs/evb_ast2600.h b/include/configs/evb_ast2600.h
index d2aceb6663..83002db317 100644
--- a/include/configs/evb_ast2600.h
+++ b/include/configs/evb_ast2600.h
@@ -14,7 +14,14 @@
 #define CONFIG_SYS_LOAD_ADDR   0x8300
 
 /* Misc */
+#define STR_HELPER(s)  #s
+#define STR(s) STR_HELPER(s)
+
 #define CONFIG_EXTRA_ENV_SETTINGS \
+   "loadaddr=" STR(CONFIG_SYS_LOAD_ADDR) "\0" \
+   "bootspi=fdt addr 2010 && fdt header get fitsize totalsize && " \
+   "cp.b 2010 ${loadaddr} ${fitsize} && bootm; " \
+   "echo Error loading kernel FIT image\0" \
"verify=yes\0"  \
"spi_dma=yes\0" \
""
-- 
2.17.1



[PATCH next v7 10/12] configs: ast2600-evb: Enable SPL FIT support

2021-10-19 Thread Chia-Wei Wang
Enable SPL FIT image load and verification support.
The HW accelerated SHA is also available with the
newly added support of the HACE HW hash engine.

The SPL thumb build is also enabled to keep the binary
less than 64KB to fit into the Aspeed secure boot design.

Signed-off-by: Chia-Wei Wang 
---
 configs/evb-ast2600_defconfig | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
index 56ab885d9b..eba6940ec1 100644
--- a/configs/evb-ast2600_defconfig
+++ b/configs/evb-ast2600_defconfig
@@ -1,7 +1,8 @@
 CONFIG_ARM=y
 CONFIG_SYS_DCACHE_OFF=y
+CONFIG_SPL_SYS_THUMB_BUILD=y
 CONFIG_ARCH_ASPEED=y
-CONFIG_SYS_TEXT_BASE=0x1
+CONFIG_SYS_TEXT_BASE=0x8000
 CONFIG_ASPEED_AST2600=y
 CONFIG_TARGET_EVB_AST2600=y
 CONFIG_SPL_LIBCOMMON_SUPPORT=y
@@ -12,13 +13,17 @@ CONFIG_ENV_SIZE=0x1
 CONFIG_SYS_MALLOC_LEN=0x200
 CONFIG_DEFAULT_DEVICE_TREE="ast2600-evb"
 CONFIG_SPL_SERIAL=y
+CONFIG_SPL_STACK_R_ADDR=0x8300
 CONFIG_SPL_SIZE_LIMIT=0x1
 CONFIG_SPL=y
 # CONFIG_ARMV7_NONSEC is not set
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
 CONFIG_SYS_LOAD_ADDR=0x8300
 CONFIG_FIT=y
-# CONFIG_LEGACY_IMAGE_FORMAT is not set
+CONFIG_SPL_FIT_SIGNATURE=y
+CONFIG_SPL_LOAD_FIT=y
+CONFIG_SPL_LOAD_FIT_ADDRESS=0x1
+# CONFIG_USE_SPL_FIT_GENERATOR is not set
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyS4,115200n8 root=/dev/ram rw"
 CONFIG_USE_BOOTCOMMAND=y
@@ -26,8 +31,10 @@ CONFIG_BOOTCOMMAND="bootm 2010"
 # CONFIG_DISPLAY_CPUINFO is not set
 CONFIG_SPL_SIZE_LIMIT_SUBTRACT_GD=y
 CONFIG_SPL_SIZE_LIMIT_SUBTRACT_MALLOC=y
-# CONFIG_SPL_LEGACY_IMAGE_SUPPORT is not set
 CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x200
+CONFIG_SPL_FIT_IMAGE_TINY=y
 CONFIG_SPL_DM_RESET=y
 CONFIG_SPL_RAM_SUPPORT=y
 CONFIG_SPL_RAM_DEVICE=y
@@ -47,6 +54,9 @@ CONFIG_REGMAP=y
 CONFIG_SPL_OF_TRANSLATE=y
 CONFIG_CLK=y
 CONFIG_SPL_CLK=y
+CONFIG_DM_HASH=y
+CONFIG_HASH_ASPEED=y
+CONFIG_ASPEED_ACRY=y
 CONFIG_DM_I2C=y
 CONFIG_MISC=y
 CONFIG_SPL_MISC=y
@@ -65,5 +75,9 @@ CONFIG_SYS_NS16550=y
 CONFIG_SYSRESET=y
 CONFIG_SPL_SYSRESET=y
 CONFIG_WDT=y
+CONFIG_SHA512_ALGO=y
+CONFIG_SHA512=y
+CONFIG_SHA384=y
 CONFIG_HEXDUMP=y
 # CONFIG_EFI_LOADER is not set
+CONFIG_PHANDLE_CHECK_SEQ=y
-- 
2.17.1



[PATCH next v7 11/12] configs: aspeed: Make EXTRA_ENV_SETTINGS board specific

2021-10-19 Thread Chia-Wei Wang
Move CONFIG_EXTRA_ENV_SETTINGS to board-specific
configuration headers.

Signed-off-by: Chia-Wei Wang 
---
 include/configs/aspeed-common.h | 9 -
 include/configs/evb_ast2500.h   | 9 +
 include/configs/evb_ast2600.h   | 9 +
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/include/configs/aspeed-common.h b/include/configs/aspeed-common.h
index 5177bf20fa..96526e1a75 100644
--- a/include/configs/aspeed-common.h
+++ b/include/configs/aspeed-common.h
@@ -38,13 +38,4 @@
  */
 #define CONFIG_BOOTP_BOOTFILESIZE
 
-/*
- * Miscellaneous configurable options
- */
-
-#define CONFIG_EXTRA_ENV_SETTINGS \
-   "verify=yes\0"  \
-   "spi_dma=yes\0" \
-   ""
-
 #endif /* __AST_COMMON_CONFIG_H */
diff --git a/include/configs/evb_ast2500.h b/include/configs/evb_ast2500.h
index dc032c1a41..a886fd941e 100644
--- a/include/configs/evb_ast2500.h
+++ b/include/configs/evb_ast2500.h
@@ -13,4 +13,13 @@
 
 #define CONFIG_SYS_UBOOT_BASE  CONFIG_SYS_TEXT_BASE
 
+/* Memory Info */
+#define CONFIG_SYS_LOAD_ADDR   0x8300
+
+/* Misc */
+#define CONFIG_EXTRA_ENV_SETTINGS \
+   "verify=yes\0"  \
+   "spi_dma=yes\0" \
+   ""
+
 #endif /* __CONFIG_H */
diff --git a/include/configs/evb_ast2600.h b/include/configs/evb_ast2600.h
index 177a52eb91..d2aceb6663 100644
--- a/include/configs/evb_ast2600.h
+++ b/include/configs/evb_ast2600.h
@@ -10,4 +10,13 @@
 
 #define CONFIG_SYS_UBOOT_BASE  CONFIG_SYS_TEXT_BASE
 
+/* Memory Info */
+#define CONFIG_SYS_LOAD_ADDR   0x8300
+
+/* Misc */
+#define CONFIG_EXTRA_ENV_SETTINGS \
+   "verify=yes\0"  \
+   "spi_dma=yes\0" \
+   ""
+
 #endif /* __CONFIG_H */
-- 
2.17.1



[PATCH next v7 07/12] crypto: aspeed: Add AST2600 ACRY support

2021-10-19 Thread Chia-Wei Wang
ACRY is deisnged to accerlerate ECC/RSA digital signature
generation and verification.

Signed-off-by: Chia-Wei Wang 
---
 drivers/crypto/aspeed/Kconfig   |  10 ++
 drivers/crypto/aspeed/Makefile  |   1 +
 drivers/crypto/aspeed/aspeed_acry.c | 190 
 lib/rsa/Kconfig |  10 +-
 4 files changed, 210 insertions(+), 1 deletion(-)
 create mode 100644 drivers/crypto/aspeed/aspeed_acry.c

diff --git a/drivers/crypto/aspeed/Kconfig b/drivers/crypto/aspeed/Kconfig
index 471c06f986..9bf317177a 100644
--- a/drivers/crypto/aspeed/Kconfig
+++ b/drivers/crypto/aspeed/Kconfig
@@ -8,3 +8,13 @@ config ASPEED_HACE
  Enabling this allows the use of SHA operations in hardware without
  requiring the SHA software implementations. It also improves 
performance
  and saves code size.
+
+config ASPEED_ACRY
+   bool "ASPEED RSA and ECC Engine"
+   depends on ASPEED_AST2600
+   help
+Select this option to enable a driver for using the RSA/ECC engine in
+the ASPEED BMC SoCs.
+
+Enabling this allows the use of RSA/ECC operations in hardware without 
requiring the
+software implementations. It also improves performance and saves code 
size.
diff --git a/drivers/crypto/aspeed/Makefile b/drivers/crypto/aspeed/Makefile
index 84e6bfe82a..58b55fc46e 100644
--- a/drivers/crypto/aspeed/Makefile
+++ b/drivers/crypto/aspeed/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_ASPEED_HACE) += aspeed_hace.o
+obj-$(CONFIG_ASPEED_ACRY) += aspeed_acry.o
diff --git a/drivers/crypto/aspeed/aspeed_acry.c 
b/drivers/crypto/aspeed/aspeed_acry.c
new file mode 100644
index 00..c28cdf374b
--- /dev/null
+++ b/drivers/crypto/aspeed/aspeed_acry.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2021 ASPEED Technology Inc.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* ACRY register offsets */
+#define ACRY_CTRL1 0x00
+#define   ACRY_CTRL1_RSA_DMA   BIT(1)
+#define   ACRY_CTRL1_RSA_START BIT(0)
+#define ACRY_CTRL2 0x44
+#define ACRY_CTRL3 0x48
+#define   ACRY_CTRL3_SRAM_AHB_ACCESS   BIT(8)
+#define   ACRY_CTRL3_ECC_RSA_MODE_MASK GENMASK(5, 4)
+#define   ACRY_CTRL3_ECC_RSA_MODE_SHIFT4
+#define ACRY_DMA_DRAM_SADDR0x4c
+#define ACRY_DMA_DMEM_TADDR0x50
+#define   ACRY_DMA_DMEM_TADDR_LEN_MASK GENMASK(15, 0)
+#define   ACRY_DMA_DMEM_TADDR_LEN_SHIFT0
+#define ACRY_RSA_PARAM 0x58
+#define   ACRY_RSA_PARAM_EXP_MASK  GENMASK(31, 16)
+#define   ACRY_RSA_PARAM_EXP_SHIFT 16
+#define   ACRY_RSA_PARAM_MOD_MASK  GENMASK(15, 0)
+#define   ACRY_RSA_PARAM_MOD_SHIFT 0
+#define ACRY_RSA_INT_EN0x3f8
+#define   ACRY_RSA_INT_EN_RSA_READYBIT(2)
+#define   ACRY_RSA_INT_EN_RSA_CMPLTBIT(1)
+#define ACRY_RSA_INT_STS   0x3fc
+#define   ACRY_RSA_INT_STS_RSA_READY   BIT(2)
+#define   ACRY_RSA_INT_STS_RSA_CMPLT   BIT(1)
+
+/* misc. constant */
+#define ACRY_ECC_MODE  2
+#define ACRY_RSA_MODE  3
+#define ACRY_CTX_BUFSZ 0x600
+
+struct aspeed_acry {
+   phys_addr_t base;
+   phys_addr_t sram_base; /* internal sram */
+   struct clk clk;
+};
+
+static int aspeed_acry_mod_exp(struct udevice *dev, const uint8_t *sig, 
uint32_t sig_len,
+  struct key_prop *prop, uint8_t *out)
+{
+   int i, j;
+   u8 *ctx;
+   u8 *ptr;
+   u32 reg;
+   struct aspeed_acry *acry = dev_get_priv(dev);
+
+   ctx = memalign(16, ACRY_CTX_BUFSZ);
+   if (!ctx)
+   return -ENOMEM;
+
+   memset(ctx, 0, ACRY_CTX_BUFSZ);
+
+   ptr = (u8 *)prop->public_exponent;
+   for (i = prop->exp_len - 1, j = 0; i >= 0; --i) {
+   ctx[j] = ptr[i];
+   j++;
+   j = (j % 16) ? j : j + 32;
+   }
+
+   ptr = (u8 *)prop->modulus;
+   for (i = (prop->num_bits >> 3) - 1, j = 0; i >= 0; --i) {
+   ctx[j + 16] = ptr[i];
+   j++;
+   j = (j % 16) ? j : j + 32;
+   }
+
+   ptr = (u8 *)sig;
+   for (i = sig_len - 1, j = 0; i >= 0; --i) {
+   ctx[j + 32] = ptr[i];
+   j++;
+   j = (j % 16) ? j : j + 32;
+   }
+
+   writel((u32)ctx, acry->base + ACRY_DMA_DRAM_SADDR);
+
+   reg = (((prop->exp_len << 3) << ACRY_RSA_PARAM_EXP_SHIFT) & 
ACRY_RSA_PARAM_EXP_MASK) |
+ ((prop->num_bits << ACRY_RSA_PARAM_MOD_SHIFT) & 
ACRY_RSA_PARAM_MOD_MASK);
+   writel(reg, acry->base + ACRY_RSA_PARAM);
+
+   reg = (ACRY_CTX_BUFSZ << ACRY_DMA_DMEM_TADDR_LEN_SHIFT) & 
ACRY_DMA_DMEM_TADDR_LEN_MASK;
+   writel(reg, acry->base + ACRY_DMA_DMEM_TADDR);
+
+   reg = (ACRY_RSA_MODE << ACRY_CTRL3_ECC_RSA_MODE_SHIFT) & 
ACRY_CTRL3_ECC_RSA_MODE_MASK;
+   writel(reg, acry->base + ACRY_CTRL3);
+
+   writel(ACRY_CTRL1_RSA_DMA | ACRY_CTRL1_RSA_START, acry->base + 
ACRY_CTRL1);
+

[PATCH next v7 05/12] ARM: dts: ast2600: Add HACE to device tree

2021-10-19 Thread Chia-Wei Wang
From: Joel Stanley 

Add HACE DTS node and enable it for AST2600 EVB.

Signed-off-by: Joel Stanley 
Signed-off-by: Chia-Wei Wang 
---
 arch/arm/dts/ast2600-evb.dts | 5 +
 arch/arm/dts/ast2600.dtsi| 8 
 2 files changed, 13 insertions(+)

diff --git a/arch/arm/dts/ast2600-evb.dts b/arch/arm/dts/ast2600-evb.dts
index 2abd31341c..adb80a30ef 100644
--- a/arch/arm/dts/ast2600-evb.dts
+++ b/arch/arm/dts/ast2600-evb.dts
@@ -177,3 +177,8 @@
  0x08 0x04
  0x08 0x04>;
 };
+
+ {
+   u-boot,dm-pre-reloc;
+   status = "okay";
+};
diff --git a/arch/arm/dts/ast2600.dtsi b/arch/arm/dts/ast2600.dtsi
index f121f547e6..b8fe966c7d 100644
--- a/arch/arm/dts/ast2600.dtsi
+++ b/arch/arm/dts/ast2600.dtsi
@@ -187,6 +187,14 @@
};
};
 
+   hace: hace@1e6d {
+   compatible = "aspeed,ast2600-hace";
+   reg = <0x1e6d 0x200>;
+   interrupts = ;
+   clocks = < ASPEED_CLK_GATE_YCLK>;
+   status = "disabled";
+   };
+
edac: sdram@1e6e {
compatible = "aspeed,ast2600-sdram-edac";
reg = <0x1e6e 0x174>;
-- 
2.17.1



[PATCH next v7 06/12] clk: ast2600: Add RSACLK control for ACRY

2021-10-19 Thread Chia-Wei Wang
Add RSACLK enable for ACRY, the HW RSA/ECC crypto engine
of ASPEED AST2600 SoCs.

As ACRY and HACE share the same reset control bit, we do not
perform the reset-hold-n-release operation during their clock
ungating process. Instead, only reset release is conducted to
prevent mutual interference.

Signed-off-by: Chia-Wei Wang 
---
 .../arm/include/asm/arch-aspeed/scu_ast2600.h |  1 +
 drivers/clk/aspeed/clk_ast2600.c  | 22 +--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/arch-aspeed/scu_ast2600.h 
b/arch/arm/include/asm/arch-aspeed/scu_ast2600.h
index d7b500f656..7c5aab98b6 100644
--- a/arch/arm/include/asm/arch-aspeed/scu_ast2600.h
+++ b/arch/arm/include/asm/arch-aspeed/scu_ast2600.h
@@ -8,6 +8,7 @@
 #define SCU_UNLOCK_KEY 0x1688a8a8
 
 #define SCU_CLKGATE1_EMMC  BIT(27)
+#define SCU_CLKGATE1_ACRY  BIT(24)
 #define SCU_CLKGATE1_MAC2  BIT(21)
 #define SCU_CLKGATE1_MAC1  BIT(20)
 #define SCU_CLKGATE1_USB_HUB   BIT(14)
diff --git a/drivers/clk/aspeed/clk_ast2600.c b/drivers/clk/aspeed/clk_ast2600.c
index 69128fd3c4..f6ebf824aa 100644
--- a/drivers/clk/aspeed/clk_ast2600.c
+++ b/drivers/clk/aspeed/clk_ast2600.c
@@ -1018,11 +1018,26 @@ static ulong ast2600_enable_haceclk(struct ast2600_scu 
*scu)
uint32_t reset_bit;
uint32_t clkgate_bit;
 
+   /* share the same reset control bit with ACRY */
reset_bit = BIT(ASPEED_RESET_HACE);
clkgate_bit = SCU_CLKGATE1_HACE;
 
-   writel(reset_bit, >modrst_ctrl1);
-   udelay(100);
+   writel(clkgate_bit, >clkgate_clr1);
+   mdelay(20);
+   writel(reset_bit, >modrst_clr1);
+
+   return 0;
+}
+
+static ulong ast2600_enable_rsaclk(struct ast2600_scu *scu)
+{
+   uint32_t reset_bit;
+   uint32_t clkgate_bit;
+
+   /* share the same reset control bit with HACE */
+   reset_bit = BIT(ASPEED_RESET_HACE);
+   clkgate_bit = SCU_CLKGATE1_ACRY;
+
writel(clkgate_bit, >clkgate_clr1);
mdelay(20);
writel(reset_bit, >modrst_clr1);
@@ -1071,6 +1086,9 @@ static int ast2600_clk_enable(struct clk *clk)
case ASPEED_CLK_GATE_YCLK:
ast2600_enable_haceclk(priv->scu);
break;
+   case ASPEED_CLK_GATE_RSACLK:
+   ast2600_enable_rsaclk(priv->scu);
+   break;
default:
pr_err("can't enable clk\n");
return -ENOENT;
-- 
2.17.1



[PATCH next v7 03/12] clk: ast2600: Add YCLK control for HACE

2021-10-19 Thread Chia-Wei Wang
From: Joel Stanley 

Add YCLK enable for HACE, the HW hash engine of
ASPEED AST2600 SoCs.

Signed-off-by: Joel Stanley 
Signed-off-by: Chia-Wei Wang 
---
 .../arm/include/asm/arch-aspeed/scu_ast2600.h |  5 +++--
 drivers/clk/aspeed/clk_ast2600.c  | 20 +++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/arch-aspeed/scu_ast2600.h 
b/arch/arm/include/asm/arch-aspeed/scu_ast2600.h
index a205fb1f76..d7b500f656 100644
--- a/arch/arm/include/asm/arch-aspeed/scu_ast2600.h
+++ b/arch/arm/include/asm/arch-aspeed/scu_ast2600.h
@@ -10,8 +10,9 @@
 #define SCU_CLKGATE1_EMMC  BIT(27)
 #define SCU_CLKGATE1_MAC2  BIT(21)
 #define SCU_CLKGATE1_MAC1  BIT(20)
-#define SCU_CLKGATE1_USB_HUB   BIT(14)
-#define SCU_CLKGATE1_USB_HOST2 BIT(7)
+#define SCU_CLKGATE1_USB_HUB   BIT(14)
+#define SCU_CLKGATE1_HACE  BIT(13)
+#define SCU_CLKGATE1_USB_HOST2 BIT(7)
 
 #define SCU_CLKGATE2_FSI   BIT(30)
 #define SCU_CLKGATE2_MAC4  BIT(21)
diff --git a/drivers/clk/aspeed/clk_ast2600.c b/drivers/clk/aspeed/clk_ast2600.c
index 3a92739f5c..69128fd3c4 100644
--- a/drivers/clk/aspeed/clk_ast2600.c
+++ b/drivers/clk/aspeed/clk_ast2600.c
@@ -1013,6 +1013,23 @@ static ulong ast2600_enable_usbbhclk(struct ast2600_scu 
*scu)
return 0;
 }
 
+static ulong ast2600_enable_haceclk(struct ast2600_scu *scu)
+{
+   uint32_t reset_bit;
+   uint32_t clkgate_bit;
+
+   reset_bit = BIT(ASPEED_RESET_HACE);
+   clkgate_bit = SCU_CLKGATE1_HACE;
+
+   writel(reset_bit, >modrst_ctrl1);
+   udelay(100);
+   writel(clkgate_bit, >clkgate_clr1);
+   mdelay(20);
+   writel(reset_bit, >modrst_clr1);
+
+   return 0;
+}
+
 static int ast2600_clk_enable(struct clk *clk)
 {
struct ast2600_clk_priv *priv = dev_get_priv(clk->dev);
@@ -1051,6 +1068,9 @@ static int ast2600_clk_enable(struct clk *clk)
case ASPEED_CLK_GATE_USBPORT2CLK:
ast2600_enable_usbbhclk(priv->scu);
break;
+   case ASPEED_CLK_GATE_YCLK:
+   ast2600_enable_haceclk(priv->scu);
+   break;
default:
pr_err("can't enable clk\n");
return -ENOENT;
-- 
2.17.1



[PATCH next v7 00/12] aspeed: Support secure boot chain with FIT image verification

2021-10-19 Thread Chia-Wei Wang
This patch series intends to provide a secure boot chain from SPL to Linux 
kernel
based on the hash and signature verification of FIT image paradigm.

To improve the performance and save code size (SPL is limited to 64KB due to 
HW-RoT),
the drviers of two HW crypto engine HACE and ACRY are also added for AST26xx 
SoCs.

As HACE and ACRY can only access to DRAM space, additional configuration and
boot command are also updated according to move each FIT image before its 
booting.

In addition, the common code of FIT image hash algorithm lookup is also revised
to leverage the HW accelerated calculation.

v7:
 - fix missing interrupt status clear for ACRY RSA operation

v6:
 - fix parameter comment for v5 update

v5:
 - fix inconsistent parameter name due to parallel patch work

v4:
 - add new DM_HASH based driver for Aspeed HACE
 - remove SPL board init, which was originally used to probe non-DM HACE driver
 - fix typo of ARCY to ACRY
 - refactor defconfig based on the new Kconfig of U-Boot next branch

v3:
 - add SW work around for HACE HW DMA issue by resetting HACE
 - add reset control for HACE device tree node
 - sync all of the HACE error message to use debug()

v2:
 - update commit authors

Chia-Wei Wang (9):
  image: fit: Fix parameter name for hash algorithm
  aspeed: ast2600: Enlarge SRAM size
  clk: ast2600: Add RSACLK control for ACRY
  crypto: aspeed: Add AST2600 ACRY support
  ARM: dts: ast2600: Add ACRY to device tree
  ast2600: spl: Locate load buffer in DRAM space
  configs: ast2600-evb: Enable SPL FIT support
  configs: aspeed: Make EXTRA_ENV_SETTINGS board specific
  configs: ast2600: Boot kernel FIT in DRAM

Joel Stanley (2):
  clk: ast2600: Add YCLK control for HACE
  ARM: dts: ast2600: Add HACE to device tree

Johnny Huang (1):
  crypto: aspeed: Add AST2600 HACE support

 arch/arm/dts/ast2600-evb.dts  |  10 +
 arch/arm/dts/ast2600.dtsi |  17 +
 arch/arm/include/asm/arch-aspeed/platform.h   |   2 +-
 .../arm/include/asm/arch-aspeed/scu_ast2600.h |   6 +-
 arch/arm/mach-aspeed/ast2600/spl.c|   9 +-
 common/image-fit.c|   4 +-
 configs/evb-ast2600_defconfig |  22 +-
 drivers/clk/aspeed/clk_ast2600.c  |  38 ++
 drivers/crypto/Kconfig|   2 +
 drivers/crypto/Makefile   |   1 +
 drivers/crypto/aspeed/Kconfig |  20 +
 drivers/crypto/aspeed/Makefile|   2 +
 drivers/crypto/aspeed/aspeed_acry.c   | 190 +
 drivers/crypto/aspeed/aspeed_hace.c   | 381 ++
 drivers/crypto/hash/Kconfig   |   8 +
 include/configs/aspeed-common.h   |   9 -
 include/configs/evb_ast2500.h |   9 +
 include/configs/evb_ast2600.h |  16 +
 lib/rsa/Kconfig   |  10 +-
 19 files changed, 729 insertions(+), 27 deletions(-)
 create mode 100644 drivers/crypto/aspeed/Kconfig
 create mode 100644 drivers/crypto/aspeed/Makefile
 create mode 100644 drivers/crypto/aspeed/aspeed_acry.c
 create mode 100644 drivers/crypto/aspeed/aspeed_hace.c

-- 
2.17.1



[PATCH next v7 01/12] image: fit: Fix parameter name for hash algorithm

2021-10-19 Thread Chia-Wei Wang
Fix inconsistent function parameter name of the hash algorithm.

Signed-off-by: Chia-Wei Wang 
Fixes: 92055e138f2 ("image: Drop if/elseif hash selection in calculate_hash()")
---
 common/image-fit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index 5a0a0cc200..a53a2b5d6f 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1201,7 +1201,7 @@ int fit_set_timestamp(void *fit, int noffset, time_t 
timestamp)
  * calculate_hash - calculate and return hash for provided input data
  * @data: pointer to the input data
  * @data_len: data length
- * @algo: requested hash algorithm
+ * @name: requested hash algorithm name
  * @value: pointer to the char, will hold hash value data (caller must
  * allocate enough free space)
  * value_len: length of the calculated hash
@@ -1229,7 +1229,7 @@ int calculate_hash(const void *data, int data_len, const 
char *name,
return -1;
}
 
-   hash_algo = hash_algo_lookup_by_name(algo);
+   hash_algo = hash_algo_lookup_by_name(name);
if (hash_algo == HASH_ALGO_INVALID) {
debug("Unsupported hash algorithm\n");
return -1;
-- 
2.17.1



[PATCH next v7 02/12] aspeed: ast2600: Enlarge SRAM size

2021-10-19 Thread Chia-Wei Wang
The AST2600 SRAM has been extended to 88KB since A1
chip revision. This patch updates the SRAM size to
offer more space for early stack/heap use.

Signed-off-by: Chia-Wei Wang 
---
 arch/arm/include/asm/arch-aspeed/platform.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/arch-aspeed/platform.h 
b/arch/arm/include/asm/arch-aspeed/platform.h
index d50ec5f8a9..589abd4a3f 100644
--- a/arch/arm/include/asm/arch-aspeed/platform.h
+++ b/arch/arm/include/asm/arch-aspeed/platform.h
@@ -17,7 +17,7 @@
 #define ASPEED_MAC_COUNT   4
 #define ASPEED_DRAM_BASE   0x8000
 #define ASPEED_SRAM_BASE   0x1000
-#define ASPEED_SRAM_SIZE   0x1
+#define ASPEED_SRAM_SIZE   0x16000
 #else
 #err "Unrecognized Aspeed platform."
 #endif
-- 
2.17.1



[PATCH v4 2/4] tools: mkimage: Add Allwinner TOC0 support

2021-10-19 Thread Samuel Holland
Most Allwinner sunxi SoCs have separate boot ROMs in non-secure and
secure mode. The "non-secure" or "normal" boot ROM (NBROM) uses the
existing sunxi_egon image type. The secure boot ROM (SBROM) uses a
completely different image type, known as TOC0.

A TOC0 image is composed of a header and two or more items. One item
is the firmware binary. The others form a chain linking the firmware
signature to the root-of-trust public key (ROTPK), which has its hash
burned in the SoC's eFuses. Signatures are made using RSA-2048 + SHA256.

The pseudo-ASN.1 structure is manually assembled; this is done to work
around bugs/quirks in the boot ROM, which vary between SoCs. This TOC0
implementation has been verified to work with the A50, A64, H5, H6,
and H616 SBROMs, and it may work with other SoCs.

Signed-off-by: Samuel Holland 
---

(no changes since v3)

Changes in v3:
 - Removed TOOLS_LIBCRYPTO selection for sunxi, since most boards
   do not need it
 - Added __packed to all new "ABI" structs
 - Added entry to MAINTAINERS for sunxi tools

Changes in v2:
 - Moved certificate and key item structures out of sunxi_image.h
 - Renamed "main" and "item" variables for clarity
 - Improved error messages, and added a hint about key generation
 - Added a comment explaining the purpose of the various key files
 - Mentioned testing this code on A50 in the commit message

 MAINTAINERS   |   1 +
 common/image.c|   1 +
 include/image.h   |   1 +
 include/sunxi_image.h |  37 ++
 tools/Makefile|   3 +-
 tools/sunxi_toc0.c| 907 ++
 6 files changed, 949 insertions(+), 1 deletion(-)
 create mode 100644 tools/sunxi_toc0.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 71f468c00a..0d62829f51 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -475,6 +475,7 @@ F:  board/sunxi/
 F: drivers/clk/sunxi/
 F: drivers/phy/allwinner/
 F: drivers/video/sunxi/
+F: tools/sunxi*
 
 ARM TEGRA
 M: Tom Warren 
diff --git a/common/image.c b/common/image.c
index 3fa60b5827..d15b47ebbe 100644
--- a/common/image.c
+++ b/common/image.c
@@ -185,6 +185,7 @@ static const table_entry_t uimage_type[] = {
{   IH_TYPE_MTKIMAGE,   "mtk_image",   "MediaTek BootROM loadable 
Image" },
{   IH_TYPE_COPRO, "copro", "Coprocessor Image"},
{   IH_TYPE_SUNXI_EGON, "sunxi_egon",  "Allwinner eGON Boot Image" 
},
+   {   IH_TYPE_SUNXI_TOC0, "sunxi_toc0",  "Allwinner TOC0 Boot Image" 
},
{   -1, "",   "",   },
 };
 
diff --git a/include/image.h b/include/image.h
index 34d13ada84..1547246ec8 100644
--- a/include/image.h
+++ b/include/image.h
@@ -227,6 +227,7 @@ enum {
IH_TYPE_IMX8IMAGE,  /* Freescale IMX8Boot Image */
IH_TYPE_COPRO,  /* Coprocessor Image for remoteproc*/
IH_TYPE_SUNXI_EGON, /* Allwinner eGON Boot Image */
+   IH_TYPE_SUNXI_TOC0, /* Allwinner TOC0 Boot Image */
 
IH_TYPE_COUNT,  /* Number of image types */
 };
diff --git a/include/sunxi_image.h b/include/sunxi_image.h
index 5b2055c0af..379ca9196e 100644
--- a/include/sunxi_image.h
+++ b/include/sunxi_image.h
@@ -9,9 +9,13 @@
  *
  * Shared between mkimage and the SPL.
  */
+
 #ifndefSUNXI_IMAGE_H
 #defineSUNXI_IMAGE_H
 
+#include 
+#include 
+
 #define BOOT0_MAGIC"eGON.BT0"
 #define BROM_STAMP_VALUE   0x5f0a6c39
 #define SPL_SIGNATURE  "SPL" /* marks "sunxi" SPL header */
@@ -79,4 +83,37 @@ struct boot_file_head {
 /* Compile time check to assure proper alignment of structure */
 typedef char boot_file_head_not_multiple_of_32[1 - 2*(sizeof(struct 
boot_file_head) % 32)];
 
+struct __packed toc0_main_info {
+   uint8_t name[8];
+   __le32  magic;
+   __le32  checksum;
+   __le32  serial;
+   __le32  status;
+   __le32  num_items;
+   __le32  length;
+   uint8_t platform[4];
+   uint8_t reserved[8];
+   uint8_t end[4];
+};
+
+#define TOC0_MAIN_INFO_NAME"TOC0.GLH"
+#define TOC0_MAIN_INFO_MAGIC   0x89119800
+#define TOC0_MAIN_INFO_END "MIE;"
+
+struct __packed toc0_item_info {
+   __le32  name;
+   __le32  offset;
+   __le32  length;
+   __le32  status;
+   __le32  type;
+   __le32  load_addr;
+   uint8_t reserved[4];
+   uint8_t end[4];
+};
+
+#define TOC0_ITEM_INFO_NAME_CERT   0x00010101
+#define TOC0_ITEM_INFO_NAME_FIRMWARE   0x00010202
+#define TOC0_ITEM_INFO_NAME_KEY0x00010303
+#define TOC0_ITEM_INFO_END "IIE;"
+
 #endif
diff --git a/tools/Makefile b/tools/Makefile
index a9b3d982d8..e2aeb097aa 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -98,7 +98,8 @@ AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/, \
 LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := \
lib/fdt-libcrypto.o \
kwbimage.o \
-   

[PATCH v4 4/4] sunxi: Support building a SPL as a TOC0 image

2021-10-19 Thread Samuel Holland
Now that mkimage can generate TOC0 images, and the SPL can interpret
them, hook up the build infrastructure so the user can choose which
image type to build. Since the absolute load address is stored in the
TOC0 header, that information must be passed to mkimage.

Signed-off-by: Samuel Holland 
---

(no changes since v2)

Changes in v2:
 - Rebase on top of Icenowy's RISC-V support series
 - Rename Kconfig symbols to include the full image type name

 arch/arm/mach-sunxi/Kconfig |  2 ++
 board/sunxi/Kconfig | 24 
 scripts/Makefile.spl|  5 -
 3 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 board/sunxi/Kconfig

diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index 2c18cf02d1..879efb9f61 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -1050,6 +1050,8 @@ config BLUETOOTH_DT_DEVICE_FIXUP
  The used address is "bdaddr" if set, and "ethaddr" with the LSB
  flipped elsewise.
 
+source "board/sunxi/Kconfig"
+
 endif
 
 config CHIP_DIP_SCAN
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
new file mode 100644
index 00..084a8b0c6c
--- /dev/null
+++ b/board/sunxi/Kconfig
@@ -0,0 +1,24 @@
+choice
+   prompt "SPL Image Type"
+   default SPL_IMAGE_TYPE_SUNXI_EGON
+
+config SPL_IMAGE_TYPE_SUNXI_EGON
+   bool "eGON (normal)"
+   help
+ Select this option to embed the SPL binary in an eGON.BT0 image,
+ which is compatible with the normal boot ROM (NBROM).
+
+ This is usually the correct option to choose.
+
+config SPL_IMAGE_TYPE_SUNXI_TOC0
+   bool "TOC0 (secure)"
+   help
+ Select this option to embed the SPL binary in a TOC0 image,
+ which is compatible with the secure boot ROM (SBROM).
+
+endchoice
+
+config SPL_IMAGE_TYPE
+   string
+   default "sunxi_egon" if SPL_IMAGE_TYPE_SUNXI_EGON
+   default "sunxi_toc0" if SPL_IMAGE_TYPE_SUNXI_TOC0
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 4cc23799db..635fa14cb9 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -411,7 +411,10 @@ endif
 $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin FORCE
$(call if_changed,mkimage)
 
-MKIMAGEFLAGS_sunxi-spl.bin = -A $(ARCH) -T sunxi_egon \
+MKIMAGEFLAGS_sunxi-spl.bin = \
+   -A $(ARCH) \
+   -T $(CONFIG_SPL_IMAGE_TYPE) \
+   -a $(CONFIG_SPL_TEXT_BASE) \
-n $(CONFIG_DEFAULT_DEVICE_TREE)
 
 OBJCOPYFLAGS_u-boot-spl-dtb.hex := -I binary -O ihex 
--change-address=$(CONFIG_SPL_TEXT_BASE)
-- 
2.32.0



[PATCH v4 3/4] sunxi: Support SPL in both eGON and TOC0 images

2021-10-19 Thread Samuel Holland
SPL uses the image header to detect the boot device and to find the
offset of the next U-Boot stage. Since this information is stored
differently in the eGON and TOC0 image headers, add code to find the
correct value based on the image type currently in use.

Signed-off-by: Samuel Holland 
---

(no changes since v3)

Changes in v3:
 - Fixed offset of magic passed to memcmp
 - Refactored functions to not return pointers (fixes ambiguous NULL)

Changes in v2:
 - Moved SPL header signature checks out of sunxi_image.h
 - Refactored SPL header signature checks to use fewer casts

 arch/arm/include/asm/arch-sunxi/spl.h |  2 --
 arch/arm/mach-sunxi/board.c   | 34 ++-
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/spl.h 
b/arch/arm/include/asm/arch-sunxi/spl.h
index 58cdf806d9..157b11e489 100644
--- a/arch/arm/include/asm/arch-sunxi/spl.h
+++ b/arch/arm/include/asm/arch-sunxi/spl.h
@@ -19,8 +19,6 @@
 #define SUNXI_BOOTED_FROM_MMC0_HIGH0x10
 #define SUNXI_BOOTED_FROM_MMC2_HIGH0x12
 
-#define is_boot0_magic(addr)   (memcmp((void *)(addr), BOOT0_MAGIC, 8) == 0)
-
 uint32_t sunxi_get_boot_device(void);
 
 #endif
diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
index b4ba2a72c4..b2cd64bb3f 100644
--- a/arch/arm/mach-sunxi/board.c
+++ b/arch/arm/mach-sunxi/board.c
@@ -243,12 +243,28 @@ void s_init(void)
 
 #define SUNXI_INVALID_BOOT_SOURCE  -1
 
+static int sunxi_egon_valid(struct boot_file_head *egon_head)
+{
+   return !memcmp(egon_head->magic, BOOT0_MAGIC, 8); /* eGON.BT0 */
+}
+
+static int sunxi_toc0_valid(struct toc0_main_info *toc0_info)
+{
+   return !memcmp(toc0_info->name, TOC0_MAIN_INFO_NAME, 8); /* TOC0.GLH */
+}
+
 static int sunxi_get_boot_source(void)
 {
-   if (!is_boot0_magic(SPL_ADDR + 4)) /* eGON.BT0 */
-   return SUNXI_INVALID_BOOT_SOURCE;
+   struct boot_file_head *egon_head = (void *)SPL_ADDR;
+   struct toc0_main_info *toc0_info = (void *)SPL_ADDR;
+
+   if (sunxi_egon_valid(egon_head))
+   return readb(_head->boot_media);
+   if (sunxi_toc0_valid(toc0_info))
+   return readb(_info->platform[0]);
 
-   return readb(SPL_ADDR + 0x28);
+   /* Not a valid image, so we must have been booted via FEL. */
+   return SUNXI_INVALID_BOOT_SOURCE;
 }
 
 /* The sunxi internal brom will try to loader external bootloader
@@ -296,10 +312,16 @@ uint32_t sunxi_get_boot_device(void)
 #ifdef CONFIG_SPL_BUILD
 static u32 sunxi_get_spl_size(void)
 {
-   if (!is_boot0_magic(SPL_ADDR + 4)) /* eGON.BT0 */
-   return 0;
+   struct boot_file_head *egon_head = (void *)SPL_ADDR;
+   struct toc0_main_info *toc0_info = (void *)SPL_ADDR;
 
-   return readl(SPL_ADDR + 0x10);
+   if (sunxi_egon_valid(egon_head))
+   return readl(_head->length);
+   if (sunxi_toc0_valid(toc0_info))
+   return readl(_info->length);
+
+   /* Not a valid image, so use the default U-Boot offset. */
+   return 0;
 }
 
 /*
-- 
2.32.0



[PATCH v4 0/4] sunxi: TOC0 image type support

2021-10-19 Thread Samuel Holland
This series adds support for the TOC0 image format used by the Allwinner
secure boot ROM (SBROM). This series has been tested on the following
SoCs/boards, with the eFuse burnt to enable secure mode:
 - A50: Ainol Q88 Tablet
 - A64: Pine A64 Plus
 - H5: Orange Pi Zero Plus
 - H6: Pine H64 Model B
 - H616: Orange Pi Zero 2

This time I also tested it on boards that are not switched to secure
mode (with A64, H3, and H5).

Due to both series changing Makefile.spl, the last patch depends on:
https://patchwork.ozlabs.org/project/uboot/list/?series=267136

Since this series no longer selects TOOLS_LIBCRYPTO anywhere, building
certain platforms/options may fail with an error like the following if
TOOLS_LIBCRYPTO is disabled:

MKIMAGE spl/sunxi-spl.bin
  ./tools/mkimage: unsupported type Allwinner TOC0 Boot Image
  make[1]: *** [scripts/Makefile.spl:426: spl/sunxi-spl.bin] Error 1
  make: *** [Makefile:1982: spl/u-boot-spl] Error 2

Hopefully this is clear enough.

Changes in v4:
 - Do not select TOOLS_LIBCRYPTO anywhere

Changes in v3:
 - Selected TOOLS_LIBCRYPTO on all platforms that use kwbimage (as best
   as I can tell, using the suggestions from Pali Rohár)
 - Removed TOOLS_LIBCRYPTO selection for sunxi, since most boards
   do not need it
 - Added __packed to all new "ABI" structs
 - Added entry to MAINTAINERS for sunxi tools
 - Fixed offset of magic passed to memcmp
 - Refactored functions to not return pointers (fixes ambiguous NULL)

Changes in v2:
 - Refactored the first patch on top of TOOLS_LIBCRYPTO
 - Moved certificate and key item structures out of sunxi_image.h
 - Renamed "main" and "item" variables for clarity
 - Improved error messages, and added a hint about key generation
 - Added a comment explaining the purpose of the various key files
 - Mentioned testing this code on A50 in the commit message
 - Moved SPL header signature checks out of sunxi_image.h
 - Refactored SPL header signature checks to use fewer casts
 - Rebase on top of Icenowy's RISC-V support series
 - Rename Kconfig symbols to include the full image type name

Samuel Holland (4):
  tools: Separate image types which depend on OpenSSL
  tools: mkimage: Add Allwinner TOC0 support
  sunxi: Support SPL in both eGON and TOC0 images
  sunxi: Support building a SPL as a TOC0 image

 MAINTAINERS   |   1 +
 arch/arm/include/asm/arch-sunxi/spl.h |   2 -
 arch/arm/mach-sunxi/Kconfig   |   2 +
 arch/arm/mach-sunxi/board.c   |  34 +-
 board/sunxi/Kconfig   |  24 +
 common/image.c|   1 +
 include/image.h   |   1 +
 include/sunxi_image.h |  37 ++
 scripts/Makefile.spl  |   5 +-
 scripts/config_whitelist.txt  |   1 -
 tools/Makefile|  20 +-
 tools/mxsimage.c  |   3 -
 tools/sunxi_toc0.c| 907 ++
 13 files changed, 1011 insertions(+), 27 deletions(-)
 create mode 100644 board/sunxi/Kconfig
 create mode 100644 tools/sunxi_toc0.c

-- 
2.32.0



[PATCH v4 1/4] tools: Separate image types which depend on OpenSSL

2021-10-19 Thread Samuel Holland
Some image types (kwbimage and mxsimage) always depend on OpenSSL, so
they can only be included in mkimage when TOOLS_LIBCRYPTO is selected.
Use Makefile logic to conditionally link the files.

Signed-off-by: Samuel Holland 
---

Changes in v4:
 - Do not select TOOLS_LIBCRYPTO anywhere

Changes in v3:
 - Selected TOOLS_LIBCRYPTO on all platforms that use kwbimage (as best
   as I can tell, using the suggestions from Pali Rohár)

Changes in v2:
 - Refactored the first patch on top of TOOLS_LIBCRYPTO

 scripts/config_whitelist.txt |  1 -
 tools/Makefile   | 19 +--
 tools/mxsimage.c |  3 ---
 3 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
index cd94b5777a..affae6875d 100644
--- a/scripts/config_whitelist.txt
+++ b/scripts/config_whitelist.txt
@@ -828,7 +828,6 @@ CONFIG_MXC_UART_BASE
 CONFIG_MXC_USB_FLAGS
 CONFIG_MXC_USB_PORT
 CONFIG_MXC_USB_PORTSC
-CONFIG_MXS
 CONFIG_MXS_AUART
 CONFIG_MXS_AUART_BASE
 CONFIG_MXS_OCOTP
diff --git a/tools/Makefile b/tools/Makefile
index 999fd46531..a9b3d982d8 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -94,9 +94,11 @@ ECDSA_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix 
lib/ecdsa/, ecdsa-libcrypto.
 AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/, \
aes-encrypt.o aes-decrypt.o)
 
-# Cryptographic helpers that depend on openssl/libcrypto
-LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
-   fdt-libcrypto.o)
+# Cryptographic helpers and image types that depend on openssl/libcrypto
+LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := \
+   lib/fdt-libcrypto.o \
+   kwbimage.o \
+   mxsimage.o
 
 ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
 
@@ -118,10 +120,8 @@ dumpimage-mkimage-objs := aisimage.o \
imximage.o \
imx8image.o \
imx8mimage.o \
-   kwbimage.o \
lib/md5.o \
lpc32xximage.o \
-   mxsimage.o \
omapimage.o \
os_support.o \
pblimage.o \
@@ -156,22 +156,13 @@ fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
 fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
 file2include-objs := file2include.o
 
-ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_LIBCRYPTO),)
-# Add CONFIG_MXS into host CFLAGS, so we can check whether or not register
-# the mxsimage support within tools/mxsimage.c .
-HOSTCFLAGS_mxsimage.o += -DCONFIG_MXS
-endif
-
 ifdef CONFIG_TOOLS_LIBCRYPTO
 # This affects include/image.h, but including the board config file
 # is tricky, so manually define this options here.
 HOST_EXTRACFLAGS   += -DCONFIG_FIT_SIGNATURE
 HOST_EXTRACFLAGS   += -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0x
 HOST_EXTRACFLAGS   += -DCONFIG_FIT_CIPHER
-endif
 
-# MXSImage needs LibSSL
-ifneq 
($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_LIBCRYPTO),)
 HOSTCFLAGS_kwbimage.o += \
$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
 HOSTLDLIBS_mkimage += \
diff --git a/tools/mxsimage.c b/tools/mxsimage.c
index 002f4b525a..2bfbb421eb 100644
--- a/tools/mxsimage.c
+++ b/tools/mxsimage.c
@@ -5,8 +5,6 @@
  * Copyright (C) 2012-2013 Marek Vasut 
  */
 
-#ifdef CONFIG_MXS
-
 #include 
 #include 
 #include 
@@ -2363,4 +2361,3 @@ U_BOOT_IMAGE_TYPE(
NULL,
mxsimage_generate
 );
-#endif
-- 
2.32.0



[PATCH] sandbox: Migrate ARCH_MAP_SYSMEM to Kconfig

2021-10-19 Thread Tom Rini
Move this from a hard-coded define in config.mk to Kconfig.

Signed-off-by: Tom Rini 
---
 arch/sandbox/Kconfig   | 3 +++
 arch/sandbox/config.mk | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/sandbox/Kconfig b/arch/sandbox/Kconfig
index f83282d9d56a..7606469c94ec 100644
--- a/arch/sandbox/Kconfig
+++ b/arch/sandbox/Kconfig
@@ -1,6 +1,9 @@
 menu "Sandbox architecture"
depends on SANDBOX
 
+config ARCH_MAP_SYSMEM
+   def_bool y
+
 config SYS_ARCH
default "sandbox"
 
diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
index 1f8cb61c8bfb..2b1b657831c5 100644
--- a/arch/sandbox/config.mk
+++ b/arch/sandbox/config.mk
@@ -2,7 +2,6 @@
 # Copyright (c) 2011 The Chromium OS Authors.
 
 PLATFORM_CPPFLAGS += -D__SANDBOX__ -U_FORTIFY_SOURCE
-PLATFORM_CPPFLAGS += -DCONFIG_ARCH_MAP_SYSMEM
 PLATFORM_CPPFLAGS += -fPIC
 PLATFORM_LIBS += -lrt
 SDL_CONFIG ?= sdl2-config
-- 
2.25.1



[PATCH] fs: yaffs2: Finish Kconfig migration

2021-10-19 Thread Tom Rini
For the symbols which are both hard-coded as enabled and used, move to
Kconfig.  The rest of the CONFIG_YAFFS namespace is unselected anywhere,
so we leave it as is.

Signed-off-by: Tom Rini 
---
 fs/yaffs2/Kconfig  | 12 
 fs/yaffs2/Makefile |  4 
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/fs/yaffs2/Kconfig b/fs/yaffs2/Kconfig
index 45ffdf6c26ff..c81f4555d60b 100644
--- a/fs/yaffs2/Kconfig
+++ b/fs/yaffs2/Kconfig
@@ -1,5 +1,17 @@
+config YAFFS_DIRECT
+   bool
+
+config YAFFS_PROVIDE_DEFS
+   bool
+
+config YAFFSFS_PROVIDE_VALUES
+   bool
+
 config YAFFS2
bool "YAFFS2 filesystem support"
+   select YAFFS_DIRECT
+   select YAFFS_PROVIDE_DEFS
+   select YAFFSFS_PROVIDE_VALUES
help
  This provides access to YAFFS2 filesystems. Yet Another Flash
  Filesystem 2 is a filesystem designed specifically for NAND flash.
diff --git a/fs/yaffs2/Makefile b/fs/yaffs2/Makefile
index 3c1bb4492b6a..02cae2655500 100644
--- a/fs/yaffs2/Makefile
+++ b/fs/yaffs2/Makefile
@@ -16,7 +16,3 @@ obj-y := \
yaffs_packedtags1.o yaffs_packedtags2.o yaffs_qsort.o \
yaffs_summary.o yaffs_tagscompat.o yaffs_verify.o yaffs_yaffs1.o \
yaffs_yaffs2.o yaffs_mtdif.o yaffs_mtdif2.o
-
-ccflags-y = -DCONFIG_YAFFS_DIRECT -DCONFIG_YAFFS_SHORT_NAMES_IN_RAM \
-   -DCONFIG_YAFFS_YAFFS2 -DNO_Y_INLINE \
-   -DCONFIG_YAFFS_PROVIDE_DEFS -DCONFIG_YAFFSFS_PROVIDE_VALUES
-- 
2.25.1



Re: [PATCH] clk: fixed_rate: add dummy disable() function

2021-10-19 Thread Tom Rini
On Tue, Oct 12, 2021 at 07:40:29PM -0500, Samuel Holland wrote:

> commit 6bf6d81c1112 ("clk: fixed_rate: add dummy enable() function")
> implemented .enable, so fixed rate clocks can be used where drivers
> might call clk_enable(). Implement the .disable op for the same reason;
> some drivers, e.g. USB PHYs, may attempt to disable clocks at runtime.
> 
> Signed-off-by: Samuel Holland 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCHv2] tools: Stop re-defining -std= when building tools

2021-10-19 Thread Tom Rini
On Mon, Oct 11, 2021 at 11:11:41AM -0400, Tom Rini wrote:

> While we intentionally set -std=gnu11 for building host tools, and have
> for quite some time, we never dropped -std=gnu99 from tools/Makefile.
> This resulted in passing -std=gnu11 ... -std=gnu99 when building, and
> gnu99 would win.  This in turn would result now in warnings such as:
> tools/mkeficapsule.c:25:15: warning: redefinition of typedef 'u32' is a C11 
> feature [-Wtypedef-redefinition]
> typedef __u32 u32;
>   ^
> 
> Signed-off-by: Tom Rini 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] arm: total_compute: increase DRAM to 8GB

2021-10-19 Thread Tom Rini
On Tue, Oct 12, 2021 at 01:43:16PM +0100, Usama Arif wrote:

> The extra 6GB start at 0x808000.
> 
> Signed-off-by: Usama Arif 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] configs: am335x_evm: enable CONFIG_CLK_TI_CTRL

2021-10-19 Thread Tom Rini
On Mon, Oct 11, 2021 at 02:27:38PM +0200, Amjad Ouled-Ameur wrote:

> This enables the clock controller driver support on TI's SoCs. This will
> fix this GPIO issue at boot time:
> request_and_set_gpio: Unable to request GPIO_PR1_MII_CTRL
> request_and_set_gpio: Unable to request GPIO_MUX_MII_CTRL
> request_and_set_gpio: Unable to request GPIO_FET_SWITCH_CTRL
> request_and_set_gpio: Unable to request GPIO_PHY_RESET
> 
> This issue comes from the fact that the clock controller is not probed.
> 
> Enable the TI's clock controller driver support to solve this.
> 
> Signed-off-by: Amjad Ouled-Ameur 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v1] drivers/gpio: add support for MAX7320 i2c i/o expander

2021-10-19 Thread Tom Rini
On Fri, Oct 01, 2021 at 01:37:57PM +0200, Hannes Schmelzer wrote:

> This commit adds support for the MAX7320 (and clones) gpio expander.
> 
> Signed-off-by: Hannes Schmelzer 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v3 2/2] Makefile: Only build dtc if needed

2021-10-19 Thread Tom Rini
On Wed, Sep 22, 2021 at 11:34:44AM -0600, Simon Glass wrote:

> At present U-Boot always builds dtc if CONFIG_OF_CONTROL is defined, even
> when DTC is provided. The built dtc is not actually used, so this is a
> waste of time.
> 
> Update the Makefile logic to build dtc only if one is not provided to the
> build with the DTC variable. Add documentation to explain this.
> 
> This saves about 3.5 seconds of elapsed time on a clean build of
> sandbox_spl for me.
> 
> Signed-off-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v3 1/2] Revert "kbuild: remove unused dtc-version.sh script"

2021-10-19 Thread Tom Rini
On Wed, Sep 22, 2021 at 11:34:43AM -0600, Simon Glass wrote:

> We need this to make building dtc optional. It makes no sense to build our
> own dtc if the system one works correctly.
> 
> This reverts commit ddb87a0b40262ff99d675e946f57427642303938.
> 
> Signed-off-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


[PATCH v2] driver: spi: add bcm iproc qspi support.

2021-10-19 Thread Roman Bacik
From: Rayagonda Kokatanur 

IPROC qspi driver supports both BSPI and MSPI modes.

Signed-off-by: Rayagonda Kokatanur 
Signed-off-by: Bharat Gooty 
Acked-by: Rayagonda Kokatanur 

Signed-off-by: Roman Bacik 
---

Changes in v2:
- remove include spi-nor.h
- define and use named BITs for writing register values
- removed bspi_set_4byte_mode() method

 drivers/spi/Kconfig  |   6 +
 drivers/spi/Makefile |   1 +
 drivers/spi/iproc_qspi.c | 712 +++
 drivers/spi/iproc_qspi.h |  20 ++
 4 files changed, 739 insertions(+)
 create mode 100644 drivers/spi/iproc_qspi.c
 create mode 100644 drivers/spi/iproc_qspi.h

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index d07e9a28af82..e76fadef32dd 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -178,6 +178,12 @@ config ICH_SPI
  access the SPI NOR flash on platforms embedding this Intel
  ICH IP core.
 
+config IPROC_QSPI
+   bool "QSPI driver for BCM iProc QSPI Controller"
+   help
+ This selects the BCM iProc QSPI controller.
+ This driver support spi flash single, quad and memory reads.
+
 config KIRKWOOD_SPI
bool "Marvell Kirkwood SPI Driver"
help
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index d2f24bccefd3..869763187062 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_FSL_DSPI) += fsl_dspi.o
 obj-$(CONFIG_FSL_ESPI) += fsl_espi.o
 obj-$(CONFIG_SYNQUACER_SPI) += spi-synquacer.o
 obj-$(CONFIG_ICH_SPI) +=  ich.o
+obj-$(CONFIG_IPROC_QSPI) += iproc_qspi.o
 obj-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o
 obj-$(CONFIG_MESON_SPIFC) += meson_spifc.o
 obj-$(CONFIG_MPC8XX_SPI) += mpc8xx_spi.o
diff --git a/drivers/spi/iproc_qspi.c b/drivers/spi/iproc_qspi.c
new file mode 100644
index ..91afeb54fc57
--- /dev/null
+++ b/drivers/spi/iproc_qspi.c
@@ -0,0 +1,712 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2020-2021 Broadcom
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "iproc_qspi.h"
+
+/* 175MHz */
+#define QSPI_AXI_CLK   17500
+#define QSPI_DEF_SCK_FREQ  5000
+#define QSPI_WAIT_TIMEOUT_MS   200U
+#define DWORD_ALIGNED(a)   (!(((ulong)(a)) & 3))
+
+/* Chip attributes */
+#define SPBR_MIN   8U
+#define SPBR_MAX   255U
+#define NUM_CDRAM  16U
+
+#define CDRAM_PCS0 2
+#define CDRAM_CONT BIT(7)
+#define CDRAM_BITS_EN  BIT(6)
+#define CDRAM_QUAD_MODEBIT(8)
+#define CDRAM_RBIT_INPUT   BIT(10)
+#define MSPI_SPE   BIT(6)
+#define MSPI_CONT_AFTER_CMDBIT(7)
+
+/* Register fields */
+#define MSPI_SPCR0_MSB_BITS_8  0x0020
+#define BSPI_RAF_CONTROL_START_MASK0x0001
+#define BSPI_RAF_STATUS_SESSION_BUSY_MASK  0x0001
+#define BSPI_RAF_STATUS_FIFO_EMPTY_MASK0x0002
+#define BSPI_BITS_PER_PHASE_ADDR_MARK  0x0001
+#define BSPI_BITS_PER_CYCLE_DATA_SHIFT 0
+#define BSPI_BITS_PER_CYCLE_ADDR_SHIFT 16
+#define BSPI_STRAP_OVERRIDE_DATA_QUAD_SHIFT3
+#define BSPI_STRAP_OVERRIDE_DATA_DUAL_SHIFT1
+#define BSPI_STRAP_OVERRIDE_SHIFT  0
+
+/* MSPI registers */
+#define MSPI_SPCR0_LSB_REG 0x000
+#define MSPI_SPCR0_MSB_REG 0x004
+#define MSPI_SPCR1_LSB_REG 0x008
+#define MSPI_SPCR1_MSB_REG 0x00c
+#define MSPI_NEWQP_REG 0x010
+#define MSPI_ENDQP_REG 0x014
+#define MSPI_SPCR2_REG 0x018
+#define MSPI_STATUS_REG0x020
+#define MSPI_CPTQP_REG 0x024
+#define MSPI_TXRAM_REG 0x040
+#define MSPI_RXRAM_REG 0x0c0
+#define MSPI_CDRAM_REG 0x140
+#define MSPI_WRITE_LOCK_REG0x180
+#define MSPI_DISABLE_FLUSH_GEN_REG 0x184
+
+/* BSPI registers */
+#define BSPI_REVISION_ID_REG   0x000
+#define BSPI_SCRATCH_REG   0x004
+#define BSPI_MAST_N_BOOT_CTRL_REG  0x008
+#define BSPI_BUSY_STATUS_REG   0x00c
+#define BSPI_INTR_STATUS_REG   0x010
+#define BSPI_B0_STATUS_REG 0x014
+#define BSPI_B0_CTRL_REG   0x018
+#define BSPI_B1_STATUS_REG 0x01c
+#define BSPI_B1_CTRL_REG   0x020
+#define BSPI_STRAP_OVERRIDE_CTRL_REG   0x024
+#define BSPI_FLEX_MODE_ENABLE_REG  0x028
+#define BSPI_BITS_PER_CYCLE_REG0x02C
+#define BSPI_BITS_PER_PHASE_REG   

Re: buildman stops (crashed) on current master

2021-10-19 Thread Tom Rini
On Tue, Oct 19, 2021 at 04:59:15PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 19 Oct 2021 at 16:53, Tom Rini  wrote:
> >
> > On Tue, Oct 19, 2021 at 05:39:12PM +0200, Stefano Babic wrote:
> > > Hi Simon,
> > >
> > > On 07.10.21 15:43, Simon Glass wrote:
> > > > Hi Stefano,
> > > >
> > > > On Thu, 7 Oct 2021 at 04:37, Stefano Babic  wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > CI stops by building aarch64 without notice, for reference:
> > > > >
> > > > > https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/332319
> > > > >
> > > > > There is no error, just process is killed. It looks like it stops at
> > > > > xilinx_zynqmp_virt,
> > > > >
> > > > > ./tools/buildman/buildman -o /tmp -P -E -W aarch64but board can be 
> > > > > built
> > > > > without issues.
> > > > >
> > > > > If I build on my host (not in docker, anyway), it generally builds 
> > > > > fine
> > > > > - but it crashes sometimes, too. On gitlab instance , it crashes.
> > > > > Issue does not seem that depends on merged patches, and introduces
> > > > > boards were already built successfully. Any hint ? I have also no idea
> > > > > what I should look as what I see is just
> > > > >
> > > > > "usr/bin/bash: line 104:24 Killed
> > > > > ./tools/buildman/buildman -o /tmp -P -E -W aarch64"
> > > >
> > > > I cannot see that link. I am not sure what is going on. Does it say
> > > > what signal killed it?
> > >
> > > Pipelines on our server were not public - I have enbaled now for 
> > > u-boot-imx.
> > >
> > > >
> > > > Does it sit there for an hour and timeout? If so, then I  did see that
> > > > myself once recently, when the Kconfig needed stdin, but I could not
> > > > quitetie it down. I think buildman would provide it, but sometimes
> > > > not, apparently. So it can happen when there is an existing build
> > > > there and your new one which adds Kconfig options that don't have
> > > > defaults, or something like that?
> > > >
> > >
> > > I have investigated further, and I can reproduce it on my host outside the
> > > gitlab server. buildman causes a OOM, but I cannot find the cause.
> > >
> > > Strange enough, this happens with the "aarch64" target, and I cannot
> > > reproduce it with Tom's master. So it seems that -master is ok, and 
> > > somethin
> > > on u-boot-imx generates the OOM.
> > >
> > > However
> > >
> > > The OOM happens always when -2 (two boards remain) appears. I can see with
> > > htop that buildman starts to allocate memory until it is exhausted (64GB 
> > > RAM
> > > + 8 GB swap). Then the kernel decides that it is enough and kills 
> > > buildman -
> > > this is what I see on Ci.
> > >
> > > You can see now the pipelines:
> > >
> > > https://source.denx.de/u-boot/custodians/u-boot-imx/-/pipelines/9520
> > >
> > > I have then split aarch64 and I built imx8 separately - same result. The
> > > pipeline stops with xilinx board, but they have nothing to do. In fact, I
> > > can build all xilinx board separately. If I run buildman -W aarch64 -x
> > > xilinx, OOM is shown by another board.
> > >
> > > Strange enough, I can build each single board with buildman without 
> > > issues,
> > > neither errors nor warnongs. Just when buildman runs all together 
> > > (aarch64,
> > > 308 boards), the OOM is generated.
> > >
> > > Bisect does not help: I started bisect, and at the end this commit was
> > > presented:
> > >
> > > commit 53a24dee86fb72ae41e7579607bafe13442616f2
> > > Author: Fabio Estevam 
> > > Date:   Mon Aug 23 21:11:09 2021 -0300
> > >
> > > imx8mm-cl-iot-gate: Split the defconfigs
> >
> > I strongly suspect what's going on here is that these new defconfigs are
> > out of sync with changes now in Kconfig.  The build itself will just sit
> > there, waiting for the "oldconfig" prompt to be answered.
> >
> > I want to say the problem here is that stdin is open, rather than
> > pointing to something closed and would lead to the build failing
> > immediately, rather than once a timeout is hit, or OOM kicks in due to
> > kconfig chewing up all the memory.
> 
> Yes that's exactly what I saw...
> 
> In fact, see this commit:
> 
> e62a24ce27a buildman: Avoid hanging when the config changes
> 
> But that was 3 years ago.

Looks like something else needs to be changed then, I've bisected down
similar failures here before very recently.

-- 
Tom


signature.asc
Description: PGP signature


Re: buildman stops (crashed) on current master

2021-10-19 Thread Simon Glass
Hi Tom,

On Tue, 19 Oct 2021 at 16:53, Tom Rini  wrote:
>
> On Tue, Oct 19, 2021 at 05:39:12PM +0200, Stefano Babic wrote:
> > Hi Simon,
> >
> > On 07.10.21 15:43, Simon Glass wrote:
> > > Hi Stefano,
> > >
> > > On Thu, 7 Oct 2021 at 04:37, Stefano Babic  wrote:
> > > >
> > > > Hi all,
> > > >
> > > > CI stops by building aarch64 without notice, for reference:
> > > >
> > > > https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/332319
> > > >
> > > > There is no error, just process is killed. It looks like it stops at
> > > > xilinx_zynqmp_virt,
> > > >
> > > > ./tools/buildman/buildman -o /tmp -P -E -W aarch64but board can be built
> > > > without issues.
> > > >
> > > > If I build on my host (not in docker, anyway), it generally builds fine
> > > > - but it crashes sometimes, too. On gitlab instance , it crashes.
> > > > Issue does not seem that depends on merged patches, and introduces
> > > > boards were already built successfully. Any hint ? I have also no idea
> > > > what I should look as what I see is just
> > > >
> > > > "usr/bin/bash: line 104:24 Killed
> > > > ./tools/buildman/buildman -o /tmp -P -E -W aarch64"
> > >
> > > I cannot see that link. I am not sure what is going on. Does it say
> > > what signal killed it?
> >
> > Pipelines on our server were not public - I have enbaled now for u-boot-imx.
> >
> > >
> > > Does it sit there for an hour and timeout? If so, then I  did see that
> > > myself once recently, when the Kconfig needed stdin, but I could not
> > > quitetie it down. I think buildman would provide it, but sometimes
> > > not, apparently. So it can happen when there is an existing build
> > > there and your new one which adds Kconfig options that don't have
> > > defaults, or something like that?
> > >
> >
> > I have investigated further, and I can reproduce it on my host outside the
> > gitlab server. buildman causes a OOM, but I cannot find the cause.
> >
> > Strange enough, this happens with the "aarch64" target, and I cannot
> > reproduce it with Tom's master. So it seems that -master is ok, and somethin
> > on u-boot-imx generates the OOM.
> >
> > However
> >
> > The OOM happens always when -2 (two boards remain) appears. I can see with
> > htop that buildman starts to allocate memory until it is exhausted (64GB RAM
> > + 8 GB swap). Then the kernel decides that it is enough and kills buildman -
> > this is what I see on Ci.
> >
> > You can see now the pipelines:
> >
> > https://source.denx.de/u-boot/custodians/u-boot-imx/-/pipelines/9520
> >
> > I have then split aarch64 and I built imx8 separately - same result. The
> > pipeline stops with xilinx board, but they have nothing to do. In fact, I
> > can build all xilinx board separately. If I run buildman -W aarch64 -x
> > xilinx, OOM is shown by another board.
> >
> > Strange enough, I can build each single board with buildman without issues,
> > neither errors nor warnongs. Just when buildman runs all together (aarch64,
> > 308 boards), the OOM is generated.
> >
> > Bisect does not help: I started bisect, and at the end this commit was
> > presented:
> >
> > commit 53a24dee86fb72ae41e7579607bafe13442616f2
> > Author: Fabio Estevam 
> > Date:   Mon Aug 23 21:11:09 2021 -0300
> >
> > imx8mm-cl-iot-gate: Split the defconfigs
>
> I strongly suspect what's going on here is that these new defconfigs are
> out of sync with changes now in Kconfig.  The build itself will just sit
> there, waiting for the "oldconfig" prompt to be answered.
>
> I want to say the problem here is that stdin is open, rather than
> pointing to something closed and would lead to the build failing
> immediately, rather than once a timeout is hit, or OOM kicks in due to
> kconfig chewing up all the memory.

Yes that's exactly what I saw...

In fact, see this commit:

e62a24ce27a buildman: Avoid hanging when the config changes

But that was 3 years ago.

Regards,
Simon


Re: [PATCH 2/2] sandbox: provide /chosen/boot-hartid property

2021-10-19 Thread Simon Glass
Hi Heinrich,

On Sat, 28 Aug 2021 at 03:42, Heinrich Schuchardt  wrote:
>
> On RISC-V the sandbox must provide the /chosen/boot-hartid in the
> devicetree.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  arch/sandbox/lib/Makefile|  2 +-
>  arch/sandbox/lib/fdt_fixup.c | 25 +
>  2 files changed, 26 insertions(+), 1 deletion(-)
>  create mode 100644 arch/sandbox/lib/fdt_fixup.c

Can you add a bit more detail here? This is for the sandbox build, so
we don't actually get to boot the kernel. So who is actually using
this data?

Regards,
Simon


>
> diff --git a/arch/sandbox/lib/Makefile b/arch/sandbox/lib/Makefile
> index b4ff717e78..a2bc5a7ee6 100644
> --- a/arch/sandbox/lib/Makefile
> +++ b/arch/sandbox/lib/Makefile
> @@ -5,7 +5,7 @@
>  # (C) Copyright 2002-2006
>  # Wolfgang Denk, DENX Software Engineering, w...@denx.de.
>
> -obj-y  += interrupts.o sections.o
> +obj-y  += fdt_fixup.o interrupts.o sections.o
>  obj-$(CONFIG_PCI)  += pci_io.o
>  obj-$(CONFIG_CMD_BOOTM) += bootm.o
>  obj-$(CONFIG_CMD_BOOTZ) += bootm.o
> diff --git a/arch/sandbox/lib/fdt_fixup.c b/arch/sandbox/lib/fdt_fixup.c
> new file mode 100644
> index 00..a646f2059c
> --- /dev/null
> +++ b/arch/sandbox/lib/fdt_fixup.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#define LOG_CATEGORY LOGC_ARCH
> +
> +#include 
> +#include 
> +#include 
> +
> +#if defined(__riscv)
> +int arch_fixup_fdt(void *blob)
> +{
> +   int ret;
> +
> +   ret = fdt_find_or_add_subnode(blob, 0, "chosen");;
> +   if (ret < 0)
> +   goto err;
> +   ret = fdt_setprop_u32(blob, ret, "boot-hartid", 1);
> +   if (ret < 0)
> +   goto err;
> +   return 0;
> +err:
> +   log_err("Setting /chosen/boot-hartid failed: %s\n", 
> fdt_strerror(ret));
> +   return ret;
> +}
> +#endif
> --
> 2.30.2
>


Re: buildman stops (crashed) on current master

2021-10-19 Thread Tom Rini
On Tue, Oct 19, 2021 at 05:39:12PM +0200, Stefano Babic wrote:
> Hi Simon,
> 
> On 07.10.21 15:43, Simon Glass wrote:
> > Hi Stefano,
> > 
> > On Thu, 7 Oct 2021 at 04:37, Stefano Babic  wrote:
> > > 
> > > Hi all,
> > > 
> > > CI stops by building aarch64 without notice, for reference:
> > > 
> > > https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/332319
> > > 
> > > There is no error, just process is killed. It looks like it stops at
> > > xilinx_zynqmp_virt,
> > > 
> > > ./tools/buildman/buildman -o /tmp -P -E -W aarch64but board can be built
> > > without issues.
> > > 
> > > If I build on my host (not in docker, anyway), it generally builds fine
> > > - but it crashes sometimes, too. On gitlab instance , it crashes.
> > > Issue does not seem that depends on merged patches, and introduces
> > > boards were already built successfully. Any hint ? I have also no idea
> > > what I should look as what I see is just
> > > 
> > > "usr/bin/bash: line 104:24 Killed
> > > ./tools/buildman/buildman -o /tmp -P -E -W aarch64"
> > 
> > I cannot see that link. I am not sure what is going on. Does it say
> > what signal killed it?
> 
> Pipelines on our server were not public - I have enbaled now for u-boot-imx.
> 
> > 
> > Does it sit there for an hour and timeout? If so, then I  did see that
> > myself once recently, when the Kconfig needed stdin, but I could not
> > quitetie it down. I think buildman would provide it, but sometimes
> > not, apparently. So it can happen when there is an existing build
> > there and your new one which adds Kconfig options that don't have
> > defaults, or something like that?
> > 
> 
> I have investigated further, and I can reproduce it on my host outside the
> gitlab server. buildman causes a OOM, but I cannot find the cause.
> 
> Strange enough, this happens with the "aarch64" target, and I cannot
> reproduce it with Tom's master. So it seems that -master is ok, and somethin
> on u-boot-imx generates the OOM.
> 
> However
> 
> The OOM happens always when -2 (two boards remain) appears. I can see with
> htop that buildman starts to allocate memory until it is exhausted (64GB RAM
> + 8 GB swap). Then the kernel decides that it is enough and kills buildman -
> this is what I see on Ci.
> 
> You can see now the pipelines:
> 
> https://source.denx.de/u-boot/custodians/u-boot-imx/-/pipelines/9520
> 
> I have then split aarch64 and I built imx8 separately - same result. The
> pipeline stops with xilinx board, but they have nothing to do. In fact, I
> can build all xilinx board separately. If I run buildman -W aarch64 -x
> xilinx, OOM is shown by another board.
> 
> Strange enough, I can build each single board with buildman without issues,
> neither errors nor warnongs. Just when buildman runs all together (aarch64,
> 308 boards), the OOM is generated.
> 
> Bisect does not help: I started bisect, and at the end this commit was
> presented:
> 
> commit 53a24dee86fb72ae41e7579607bafe13442616f2
> Author: Fabio Estevam 
> Date:   Mon Aug 23 21:11:09 2021 -0300
> 
> imx8mm-cl-iot-gate: Split the defconfigs

I strongly suspect what's going on here is that these new defconfigs are
out of sync with changes now in Kconfig.  The build itself will just sit
there, waiting for the "oldconfig" prompt to be answered.

I want to say the problem here is that stdin is open, rather than
pointing to something closed and would lead to the build failing
immediately, rather than once a timeout is hit, or OOM kicks in due to
kconfig chewing up all the memory.

-- 
Tom


signature.asc
Description: PGP signature


[PATCH v9 7/7] bootm: Tidy up use of autostart env var

2021-10-19 Thread Simon Glass
This has different semantics in different places. Go with the bootm method
and put it in a common function so that the behaviour is consistent in
U-Boot. Update the docs.

To be clear, this changes the way that 'bootelf' and standalone boot
work. Before, if autostart was set to "fred" or "YES", for example, they
would consider that a "yes". This may change behaviour for some boards,
but the only in-tree boards which mention autostart use "no" to disable
it, which will still work.

Signed-off-by: Simon Glass 
Suggested-by: Wolfgang Denk 
---

Changes in v9:
- More bikeshedding on env_get_autostart()
- Fix '' in cover letter
- Use env_get_yesno() in env_get_autostart() and update docs

Changes in v8:
- Go into more detail about the change of behaviour with autostart

Changes in v7:
- Update the cover letter

Changes in v6:
- Add new patch to tidy up use of autostart env var

 cmd/bootm.c   | 4 +---
 cmd/elf.c | 3 +--
 common/bootm_os.c | 5 +
 doc/usage/environment.rst | 5 +++--
 env/common.c  | 5 +
 include/env.h | 7 +++
 6 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/cmd/bootm.c b/cmd/bootm.c
index 92468d09a1f..b82a872a86c 100644
--- a/cmd/bootm.c
+++ b/cmd/bootm.c
@@ -140,9 +140,7 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, 
char *const argv[])
 
 int bootm_maybe_autostart(struct cmd_tbl *cmdtp, const char *cmd)
 {
-   const char *ep = env_get("autostart");
-
-   if (ep && !strcmp(ep, "yes")) {
+   if (env_get_autostart()) {
char *local_args[2];
local_args[0] = (char *)cmd;
local_args[1] = NULL;
diff --git a/cmd/elf.c b/cmd/elf.c
index d75b21461c2..2b33c50bd02 100644
--- a/cmd/elf.c
+++ b/cmd/elf.c
@@ -41,7 +41,6 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, 
char *const argv[])
unsigned long addr; /* Address of the ELF image */
unsigned long rc; /* Return value from user code */
char *sload = NULL;
-   const char *ep = env_get("autostart");
int rcode = 0;
 
/* Consume 'bootelf' */
@@ -69,7 +68,7 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, 
char *const argv[])
else
addr = load_elf_image_shdr(addr);
 
-   if (ep && !strcmp(ep, "no"))
+   if (!env_get_autostart())
return rcode;
 
printf("## Starting application at 0x%08lx ...\n", addr);
diff --git a/common/bootm_os.c b/common/bootm_os.c
index 39623f9126b..f30dcebbf7d 100644
--- a/common/bootm_os.c
+++ b/common/bootm_os.c
@@ -26,12 +26,9 @@ DECLARE_GLOBAL_DATA_PTR;
 static int do_bootm_standalone(int flag, int argc, char *const argv[],
   bootm_headers_t *images)
 {
-   char *s;
int (*appl)(int, char *const[]);
 
-   /* Don't start if "autostart" is set to "no" */
-   s = env_get("autostart");
-   if ((s != NULL) && !strcmp(s, "no")) {
+   if (!env_get_autostart()) {
env_set_hex("filesize", images->os.image_len);
return 0;
}
diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
index a3eddbaaf2e..282550a3f44 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -165,8 +165,9 @@ autostart
 be automatically started (by internally calling
 "bootm")
 
-If set to "no", a standalone image passed to the
-"bootm" command will be copied to the load address
+If unset, or set to "1"/"yes"/"true" (case insensitive, just the first
+character is enough), a standalone image
+passed to the "bootm" command will be copied to the load address
 (and eventually uncompressed), but NOT be started.
 This can be used to load and uncompress arbitrary
 data.
diff --git a/env/common.c b/env/common.c
index 81e9e0b2aaf..e46a9ee644e 100644
--- a/env/common.c
+++ b/env/common.c
@@ -47,6 +47,11 @@ int env_get_yesno(const char *var)
1 : 0;
 }
 
+bool env_get_autostart(void)
+{
+   return env_get_yesno("autostart") == 1;
+}
+
 /*
  * Look up the variable from the default environment
  */
diff --git a/include/env.h b/include/env.h
index d5e2bcb530f..fdad495691f 100644
--- a/include/env.h
+++ b/include/env.h
@@ -143,6 +143,13 @@ int env_get_f(const char *name, char *buf, unsigned int 
len);
  */
 int env_get_yesno(const char *var);
 
+/**
+ * env_get_autostart() - Check if autostart is enabled
+ *
+ * @return true if the "autostart" env var exists and is set to "yes"
+ */
+bool env_get_autostart(void);
+
 /**
  * env_set() - set an environment variable
  *
-- 
2.33.0.1079.g6e70778dc9-goog



[PATCH v9 6/7] doc: Improve environment documentation

2021-10-19 Thread Simon Glass
Make various updates suggested during review of the rST conversion.

Signed-off-by: Simon Glass 
Reviewed-by: Marek Behún 
Suggested-by: Wolfgang Denk 
---

(no changes since v7)

Changes in v7:
- A few more tweaks

Changes in v6:
- Move all updates to a separate patch
- More updates and improvements

Changes in v5:
- Minor updates as suggested by Wolfgang

Changes in v4:
- Add new patch to move environment documentation to rST

 doc/usage/environment.rst | 36 +++-
 doc/usage/index.rst   |  1 +
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
index adf2e067b58..a3eddbaaf2e 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -4,16 +4,20 @@ Environment Variables
 =
 
 U-Boot supports user configuration using Environment Variables which
-can be made persistent by saving to Flash memory.
+can be made persistent by saving to persistent storage, for example flash
+memory.
 
-Environment Variables are set using "setenv", printed using
-"printenv", and saved to Flash using "saveenv". Using "setenv"
+Environment Variables are set using "env set" (alias "setenv"), printed using
+"env print" (alias "printenv"), and saved to persistent storage using
+"env save" (alias "saveenv"). Using "env set"
 without a value can be used to delete a variable from the
-environment. As long as you don't save the environment you are
+environment. As long as you don't save the environment, you are
 working with an in-memory copy. In case the Flash area containing the
 environment is erased by accident, a default environment is provided.
 
-Some configuration options can be set using Environment Variables.
+Some configuration is controlled by Environment Variables, so that setting the
+variable can adjust the behaviour of U-Boot (e.g. autoboot delay, autoloading
+from tftp).
 
 Text-based Environment
 --
@@ -90,16 +94,24 @@ environment but work is underway to address this.
 List of environment variables
 -
 
+Some configuration options can be set using Environment Variables. In many 
cases
+the value in the default environment comes from a CONFIG option - see
+`include/env_default.h`) for this.
+
 This is most-likely not complete:
 
 baudrate
-see CONFIG_BAUDRATE
+Current baud rate used by the serial console. The built-in value is set by
+CONFIG_BAUDRATE (see `drivers/serial/Kconfig`)
 
 bootdelay
-see CONFIG_BOOTDELAY
+Current autoboot delay. The built-in value is set by CONFIG_BOOTDELAY (see
+`common/Kconfig`)
 
 bootcmd
-see CONFIG_BOOTCOMMAND
+Defines a command string that is automatically executed when no character
+is read on the console interface within a cetain boot delay after reset.
+The built-in value is set by CONFIG_BOOTCOMMAND (see `common/Kconfig`)
 
 bootargs
 Boot arguments when booting an RTOS image
@@ -145,7 +157,7 @@ autoload
 if set to "no" (any string beginning with 'n'),
 "bootp" will just load perform a lookup of the
 configuration from the BOOTP server, but not try to
-load any image using TFTP
+load any image using TFTP or DHCP.
 
 autostart
 if set to "yes", an image loaded using the "bootp",
@@ -311,6 +323,8 @@ vlan
 Ethernet is encapsulated/received over 802.1q
 VLAN tagged frames.
 
+Note: This appears not to be used in U-Boot. See `README.VLAN`.
+
 bootpretryperiod
 Period during which BOOTP/DHCP sends retries.
 Unsigned value, in milliseconds. If not set, the period will
@@ -352,6 +366,10 @@ flash or offset in NAND flash.
 boards currently use other variables for these purposes, and some
 boards use these variables for other purposes.
 
+Also note that most of these variables are just a commonly used set of variable
+names, used in some other variable definitions, but are not hard-coded anywhere
+in U-Boot code.
+
 = ==  ==
 Image File Name  RAM Address  Flash Location
 = ==  ==
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index 356f2a56181..1a79d1c03eb 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -5,6 +5,7 @@ Use U-Boot
:maxdepth: 1
 
dfu
+   environment
fdt_overlays
fit
netconsole
-- 
2.33.0.1079.g6e70778dc9-goog



[PATCH v9 5/7] doc: Mention CONFIG_DEFAULT_ENV_FILE

2021-10-19 Thread Simon Glass
Add mention of this option since it does a similar thing to the text
environment.

Signed-off-by: Simon Glass 
Suggested-by: Rasmus Villemoes 
Reviewed-by: Marek Behún 
---

Changes in v9:
- Fix blank line between tags
- Fix typo in commit message

Changes in v8:
- Fix ambiguity about what is ignored

Changes in v7:
- Add new patch to explain the relationship with DEFAULT_ENV_FILE

 doc/usage/environment.rst | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
index 667fd193ea1..adf2e067b58 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -454,3 +454,18 @@ The signature of the callback functions is::
   include/search.h
 
 The return value is 0 if the variable change is accepted and 1 otherwise.
+
+
+External environment file
+-
+
+The `CONFIG_USE_DEFAULT_ENV_FILE` option provides a way to bypass the
+environment generation in U-Boot. If enabled, then `CONFIG_DEFAULT_ENV_FILE`
+provides the name of a file which is converted into the environment,
+completely bypassing the standard environment variables in `env_default.h`.
+
+The format is the same as accepted by the mkenvimage tool, with lines 
containing
+key=value pairs. Blank lines and lines beginning with # are ignored.
+
+Future work may unify this feature with the text-based environment, perhaps
+moving the contents of `env_default.h` to a text file.
-- 
2.33.0.1079.g6e70778dc9-goog



[PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Simon Glass
At present U-Boot environment variables, and thus scripts, are defined
by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text
to this file and dealing with quoting and newlines is harder than it
should be. It would be better if we could just type the script into a
text file and have it included by U-Boot.

Add a feature that brings in a .env file associated with the board
config, if present. To use it, create a file in a board/
directory, typically called .env and controlled by the
CONFIG_ENV_SOURCE_FILE option.

The environment variables should be of the form "var=value". Values can
extend to multiple lines. See the README under 'Environment Variables:'
for more information and an example. Note that variables names may
not end in + due to the += syntax below.

In many cases environment variables need access to the U-Boot CONFIG
variables to select different options. Enable this so that the environment
scripts can be as useful as the ones currently in the board config files.
This uses the C preprocessor, means that comments can be included in the
environment using /* ... */

Also support += to allow variables to be appended to. This is needed when
using the preprocessor.

Signed-off-by: Simon Glass 
Reviewed-by: Marek Behún 
Tested-by: Marek Behún 
---

Changes in v9:
- Drop mention of other strange characters
- Clarify that the + restriction is on the variable name not its value
- Add some tests for the script
- Deal with leading tabs
- Squash indentation down to one space
- Convert newlines within strings to spaces, which seems more consistent
- Handle appending an empty string to an empty var

Changes in v8:
- Update commit message to avoid mentioning the 'env' subdirectory
- Update commit message to mention the + restriction, etc.
- Overwrite the env file each time, to avoid incremental-build problems

Changes in v7:
- Use 'env' basename instead of 'environment' for intermediate output files
- Show a message indicating the source text file being used
- Give an error if CONFIG_EXTRA_ENV_SETTINGS is also defined
- Use CONFIG_ENV_SOURCE_FILE instead of rules to specify the text-file name
- Make board.env the default name if CONFIG_ENV_SOURCE_FILE is empty
- Rewrite the documentation
- Drop the use of common.env
- Update awk script to output the whole CONFIG string, or just a comment

Changes in v6:
- Combine the two env2string.awk patches into one

Changes in v5:
- Explain how to include the common.env file
- Explain why variables starting with _ , and / are not supported
- Expand the definition of how to declare an environment variable
- Explain what happens to empty variables
- Update maintainer
- Move use of += to this patch
- Explain that environment variables may not end in +

Changes in v4:
- Move this from being part of configuring U-Boot to part of building it
- Don't put the environment in autoconf.mk as it is not needed
- Add documentation in rST format instead of README
- Drop mention of import/export
- Update awk script to ignore blank lines, as generated by clang
- Add documentation in rST format instead of README

Changes in v3:
- Adjust Makefile to generate the .inc and .h files in separate fules
- Add more detail in the README about the format of .env files
- Improve the comment about " in the awk script
- Correctly terminate environment files with \n
- Define __UBOOT_CONFIG__ when collecting environment files

Changes in v2:
- Move .env file from include/configs to board/
- Use awk script to process environment since it is much easier on the brain
- Add information and updated example script to README
- Add dependency rule so that the environment is rebuilt when it changes
- Add separate patch to enable C preprocessor for environment files
- Enable var+=value form to simplify composing variables in multiple steps

 MAINTAINERS   |  7 +++
 Makefile  | 66 ++-
 config.mk |  2 +
 doc/usage/environment.rst | 77 +++-
 env/Kconfig   | 18 
 env/embedded.c|  1 +
 include/env_default.h | 11 +
 scripts/env2string.awk| 71 ++
 test/py/tests/test_env.py | 93 +++
 9 files changed, 344 insertions(+), 2 deletions(-)
 create mode 100644 scripts/env2string.awk

diff --git a/MAINTAINERS b/MAINTAINERS
index 71f468c00a8..36846528368 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -738,6 +738,13 @@ F: test/env/
 F: tools/env*
 F: tools/mkenvimage.c
 
+ENVIRONMENT AS TEXT
+M: Simon Glass 
+R: Wolfgang Denk 
+S: Maintained
+F: doc/usage/environment.rst
+F: scripts/env2string.awk
+
 FPGA
 M: Michal Simek 
 S: Maintained
diff --git a/Makefile b/Makefile
index f911f703443..370c8710eb0 100644
--- a/Makefile
+++ b/Makefile
@@ -513,6 +513,7 @@ version_h := include/generated/version_autogenerated.h
 timestamp_h := include/generated/timestamp_autogenerated.h
 

[PATCH v9 4/7] sandbox: Use a text-based environment

2021-10-19 Thread Simon Glass
Use a text file for the environment instead of the #define settings.

Signed-off-by: Simon Glass 
Reviewed-by: Marek Behún 
---

(no changes since v3)

Changes in v3:
- Add new patch to use a text-based environment for sandbox

 board/sandbox/sandbox.env | 25 +
 include/configs/sandbox.h | 29 -
 2 files changed, 25 insertions(+), 29 deletions(-)
 create mode 100644 board/sandbox/sandbox.env

diff --git a/board/sandbox/sandbox.env b/board/sandbox/sandbox.env
new file mode 100644
index 000..0f8d95b8db0
--- /dev/null
+++ b/board/sandbox/sandbox.env
@@ -0,0 +1,25 @@
+stdin=serial
+#ifdef CONFIG_SANDBOX_SDL
+stdin+=,cros-ec-keyb,usbkbd
+#endif
+stdout=serial,vidconsole
+stderr=serial,vidconsole
+
+ethaddr=00:00:11:22:33:44
+eth2addr=00:00:11:22:33:48
+eth3addr=00:00:11:22:33:45
+eth4addr=00:00:11:22:33:48
+eth5addr=00:00:11:22:33:46
+eth6addr=00:00:11:22:33:47
+ipaddr=1.2.3.4
+
+/*
+ * These are used for distro boot which is not supported. But once bootmethod
+ * is provided these will be used again.
+ */
+bootm_size=0x1000
+kernel_addr_r=0x100
+fdt_addr_r=0xc0
+ramdisk_addr_r=0x200
+scriptaddr=0x1000
+pxefile_addr_r=0x2000
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index c19232f202f..c703a1330c0 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -64,37 +64,8 @@
 #define CONFIG_LCD_BMP_RLE8
 
 #define CONFIG_KEYBOARD
-
-#define SANDBOX_SERIAL_SETTINGS
"stdin=serial,cros-ec-keyb,usbkbd\0" \
-   "stdout=serial,vidconsole\0" \
-   "stderr=serial,vidconsole\0"
-#else
-#define SANDBOX_SERIAL_SETTINGS"stdin=serial\0" \
-   "stdout=serial,vidconsole\0" \
-   "stderr=serial,vidconsole\0"
 #endif
 
-#define SANDBOX_ETH_SETTINGS   "ethaddr=00:00:11:22:33:44\0" \
-   "eth2addr=00:00:11:22:33:48\0" \
-   "eth3addr=00:00:11:22:33:45\0" \
-   "eth4addr=00:00:11:22:33:48\0" \
-   "eth5addr=00:00:11:22:33:46\0" \
-   "eth6addr=00:00:11:22:33:47\0" \
-   "ipaddr=1.2.3.4\0"
-
-#define MEM_LAYOUT_ENV_SETTINGS \
-   "bootm_size=0x1000\0" \
-   "kernel_addr_r=0x100\0" \
-   "fdt_addr_r=0xc0\0" \
-   "ramdisk_addr_r=0x200\0" \
-   "scriptaddr=0x1000\0" \
-   "pxefile_addr_r=0x2000\0"
-
-#define CONFIG_EXTRA_ENV_SETTINGS \
-   SANDBOX_SERIAL_SETTINGS \
-   SANDBOX_ETH_SETTINGS \
-   MEM_LAYOUT_ENV_SETTINGS
-
 #ifndef CONFIG_SPL_BUILD
 #define CONFIG_SYS_IDE_MAXBUS  1
 #define CONFIG_SYS_ATA_IDE0_OFFSET 0
-- 
2.33.0.1079.g6e70778dc9-goog



[PATCH v9 2/7] doc: Move environment documentation to rST

2021-10-19 Thread Simon Glass
Move this from the README to rST format.

Drop i2cfast since it is obviously obsolete and breaks the formatting.
Other changes and improvements are in a following patch.

Signed-off-by: Simon Glass 
Acked-by: Marek Behún 
---

(no changes since v6)

Changes in v6:
- Move all updates to a separate patch

Changes in v5:
- Minor updates as suggested by Wolfgang

Changes in v4:
- Add new patch to move environment documentation to rST

 README| 328 
 doc/usage/environment.rst | 381 ++
 2 files changed, 381 insertions(+), 328 deletions(-)
 create mode 100644 doc/usage/environment.rst

diff --git a/README b/README
index 840b192aae5..f20bc38a41c 100644
--- a/README
+++ b/README
@@ -2999,334 +2999,6 @@ TODO.
 For now: just type "help ".
 
 
-Environment Variables:
-==
-
-U-Boot supports user configuration using Environment Variables which
-can be made persistent by saving to Flash memory.
-
-Environment Variables are set using "setenv", printed using
-"printenv", and saved to Flash using "saveenv". Using "setenv"
-without a value can be used to delete a variable from the
-environment. As long as you don't save the environment you are
-working with an in-memory copy. In case the Flash area containing the
-environment is erased by accident, a default environment is provided.
-
-Some configuration options can be set using Environment Variables.
-
-List of environment variables (most likely not complete):
-
-  baudrate - see CONFIG_BAUDRATE
-
-  bootdelay- see CONFIG_BOOTDELAY
-
-  bootcmd  - see CONFIG_BOOTCOMMAND
-
-  bootargs - Boot arguments when booting an RTOS image
-
-  bootfile - Name of the image to load with TFTP
-
-  bootm_low- Memory range available for image processing in the bootm
- command can be restricted. This variable is given as
- a hexadecimal number and defines lowest address allowed
- for use by the bootm command. See also "bootm_size"
- environment variable. Address defined by "bootm_low" is
- also the base of the initial memory mapping for the Linux
- kernel -- see the description of CONFIG_SYS_BOOTMAPSZ and
- bootm_mapsize.
-
-  bootm_mapsize - Size of the initial memory mapping for the Linux kernel.
- This variable is given as a hexadecimal number and it
- defines the size of the memory region starting at base
- address bootm_low that is accessible by the Linux kernel
- during early boot.  If unset, CONFIG_SYS_BOOTMAPSZ is used
- as the default value if it is defined, and bootm_size is
- used otherwise.
-
-  bootm_size   - Memory range available for image processing in the bootm
- command can be restricted. This variable is given as
- a hexadecimal number and defines the size of the region
- allowed for use by the bootm command. See also "bootm_low"
- environment variable.
-
-  bootstopkeysha256, bootdelaykey, bootstopkey - See README.autoboot
-
-  updatefile   - Location of the software update file on a TFTP server, used
- by the automatic software update feature. Please refer to
- documentation in doc/README.update for more details.
-
-  autoload - if set to "no" (any string beginning with 'n'),
- "bootp" will just load perform a lookup of the
- configuration from the BOOTP server, but not try to
- load any image using TFTP
-
-  autostart- if set to "yes", an image loaded using the "bootp",
- "rarpboot", "tftpboot" or "diskboot" commands will
- be automatically started (by internally calling
- "bootm")
-
- If set to "no", a standalone image passed to the
- "bootm" command will be copied to the load address
- (and eventually uncompressed), but NOT be started.
- This can be used to load and uncompress arbitrary
- data.
-
-  fdt_high - if set this restricts the maximum address that the
- flattened device tree will be copied into upon boot.
- For example, if you have a system with 1 GB memory
- at physical address 0x1000, while Linux kernel
- only recognizes the first 704 MB as low memory, you
- may need to set fdt_high as 0x3C00 to have the
- device tree blob be copied to the maximum address
- of the 704 MB low memory, so that Linux kernel can
- access it during the boot procedure.
-
- If this is set to the special value 0x then
- the fdt will not be copied at all on boot.  For this
- to work 

[PATCH v9 1/7] sandbox: Drop distro_boot

2021-10-19 Thread Simon Glass
This is a complicated set of #defines and it is painful to convert to a
text file. We can (once pending patches are applied) provide the same
functionality with bootmethod. Drop this for sandbox to allow conversion
to a text-file environment.

Signed-off-by: Simon Glass 
Reviewed-by: Marek Behún 
---

(no changes since v1)

 include/configs/sandbox.h | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 24c9a84fa35..c19232f202f 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -49,16 +49,6 @@
 #define CONFIG_SYS_BAUDRATE_TABLE  {4800, 9600, 19200, 38400, 57600,\
115200}
 
-#define BOOT_TARGET_DEVICES(func) \
-   func(HOST, host, 1) \
-   func(HOST, host, 0)
-
-#ifdef __ASSEMBLY__
-#define BOOTENV
-#else
-#include 
-#endif
-
 #define CONFIG_KEEP_SERVERADDR
 #define CONFIG_UDP_CHECKSUM
 #define CONFIG_TIMESTAMP
@@ -103,7 +93,6 @@
 #define CONFIG_EXTRA_ENV_SETTINGS \
SANDBOX_SERIAL_SETTINGS \
SANDBOX_ETH_SETTINGS \
-   BOOTENV \
MEM_LAYOUT_ENV_SETTINGS
 
 #ifndef CONFIG_SPL_BUILD
-- 
2.33.0.1079.g6e70778dc9-goog



[PATCH v9 0/7] env: Allow environment in text files

2021-10-19 Thread Simon Glass
One barrier to completing the 7-year-long Kconfig migration is that
the default environment is implemented using ad-hoc CONFIG options.
At present U-Boot environment variables, and thus scripts, are defined
by CONFIG_EXTRA_ENV_SETTINGS.

It is not really feasible to move the environment to Kconfig as it is
hundreds of lines of text in some cases.

Even considering the current situation, it is painful to add large
amounts of text to the config-header file and dealing with quoting and
newlines is harder than it should be. It would be better if we could just
type the script into a text file and have it included by U-Boot.

This is already supported by the CONFIG_USE_DEFAULT_ENV_FILE feature. but
that does not support use of CONFIG options or comments, so is best suited
for use by other build systems wanting to define the U-Boot environment.

Add a feature that brings in a .env file associated with the board
config, if present. To use it, create a file board//.env or
use CONFIG_ENV_SOURCE_FILE to set a filename.

The environment variables should be of the form "var=value". Values can
extend to multiple lines. This series converts the existing environment
documentation to rST and updates it to explain how to use this.

Note: this series was originally sent eight years ago:

https://patchwork.ozlabs.org/project/uboot/patch/1382763695-2849-4-git-send-email-...@chromium.org/

It has been updated to work with Kconfig, etc. Some review comments in
that patch were infeasible so I have not addressed them. I would like
this series to be considered independently, on its merits.

Rather than deal with the complexity of rewriting the distro-boot
script, this is disabled for sandbox. The forthcoming bootmethod approach
should provide the same functionality without needing the complex
scripting in the environment.

Migration needs more thought, although it can be done later. It may be
possible to do migrate automatically, using buildman to extract the
built-in environmnent from the ELF file.

This would produce a pretty ugly conversion though, since it would drop
all the intermediate variables used to create the environment.

Better would be to parse the config.h file, figure out the components of
CONFIG_EXTRA_ENV_SETTINGS then output these as separate pieces in the
file. It is not clear how easy that would be, nor whether the result would
be very pretty. Also the __stringify() macro needs to be handled somehow.

This series is available at u-boot-dm/env-working

Comments welcome.

Changes in v9:
- Drop mention of other strange characters
- Clarify that the + restriction is on the variable name not its value
- Add some tests for the script
- Deal with leading tabs
- Squash indentation down to one space
- Convert newlines within strings to spaces, which seems more consistent
- Handle appending an empty string to an empty var
- Fix blank line between tags
- Fix typo in commit message
- More bikeshedding on env_get_autostart()
- Fix '' in cover letter
- Use env_get_yesno() in env_get_autostart() and update docs

Changes in v8:
- Update commit message to avoid mentioning the 'env' subdirectory
- Update commit message to mention the + restriction, etc.
- Overwrite the env file each time, to avoid incremental-build problems
- Fix ambiguity about what is ignored
- Go into more detail about the change of behaviour with autostart

Changes in v7:
- Use 'env' basename instead of 'environment' for intermediate output files
- Show a message indicating the source text file being used
- Give an error if CONFIG_EXTRA_ENV_SETTINGS is also defined
- Use CONFIG_ENV_SOURCE_FILE instead of rules to specify the text-file name
- Make board.env the default name if CONFIG_ENV_SOURCE_FILE is empty
- Rewrite the documentation
- Drop the use of common.env
- Update awk script to output the whole CONFIG string, or just a comment
- Add new patch to explain the relationship with DEFAULT_ENV_FILE
- A few more tweaks
- Update the cover letter

Changes in v6:
- Move all updates to a separate patch
- Combine the two env2string.awk patches into one
- Move all updates to a separate patch
- More updates and improvements
- Add new patch to tidy up use of autostart env var

Changes in v5:
- Minor updates as suggested by Wolfgang
- Explain how to include the common.env file
- Explain why variables starting with _ , and / are not supported
- Expand the definition of how to declare an environment variable
- Explain what happens to empty variables
- Update maintainer
- Move use of += to this patch
- Explain that environment variables may not end in +
- Minor updates as suggested by Wolfgang

Changes in v4:
- Add new patch to move environment documentation to rST
- Move this from being part of configuring U-Boot to part of building it
- Don't put the environment in autoconf.mk as it is not needed
- Add documentation in rST format instead of README
- Drop mention of import/export
- Update awk script to ignore blank lines, as generated by clang
- Add documentation in 

[PATCH v9 0/7] env: Allow environment in text files

2021-10-19 Thread Simon Glass
One barrier to completing the 7-year-long Kconfig migration is that
the default environment is implemented using ad-hoc CONFIG options.
At present U-Boot environment variables, and thus scripts, are defined
by CONFIG_EXTRA_ENV_SETTINGS.

It is not really feasible to move the environment to Kconfig as it is
hundreds of lines of text in some cases.

Even considering the current situation, it is painful to add large
amounts of text to the config-header file and dealing with quoting and
newlines is harder than it should be. It would be better if we could just
type the script into a text file and have it included by U-Boot.

This is already supported by the CONFIG_USE_DEFAULT_ENV_FILE feature. but
that does not support use of CONFIG options or comments, so is best suited
for use by other build systems wanting to define the U-Boot environment.

Add a feature that brings in a .env file associated with the board
config, if present. To use it, create a file board//.env or
use CONFIG_ENV_SOURCE_FILE to set a filename.

The environment variables should be of the form "var=value". Values can
extend to multiple lines. This series converts the existing environment
documentation to rST and updates it to explain how to use this.

Note: this series was originally sent eight years ago:

https://patchwork.ozlabs.org/project/uboot/patch/1382763695-2849-4-git-send-email-...@chromium.org/

It has been updated to work with Kconfig, etc. Some review comments in
that patch were infeasible so I have not addressed them. I would like
this series to be considered independently, on its merits.

Rather than deal with the complexity of rewriting the distro-boot
script, this is disabled for sandbox. The forthcoming bootmethod approach
should provide the same functionality without needing the complex
scripting in the environment.

Migration needs more thought, although it can be done later. It may be
possible to do migrate automatically, using buildman to extract the
built-in environmnent from the ELF file.

This would produce a pretty ugly conversion though, since it would drop
all the intermediate variables used to create the environment.

Better would be to parse the config.h file, figure out the components of
CONFIG_EXTRA_ENV_SETTINGS then output these as separate pieces in the
file. It is not clear how easy that would be, nor whether the result would
be very pretty. Also the __stringify() macro needs to be handled somehow.

This series is available at u-boot-dm/env-working

Comments welcome.

Changes in v9:
- Drop mention of other strange characters
- Clarify that the + restriction is on the variable name not its value
- Add some tests for the script
- Deal with leading tabs
- Squash indentation down to one space
- Convert newlines within strings to spaces, which seems more consistent
- Handle appending an empty string to an empty var
- Fix blank line between tags
- Fix typo in commit message
- More bikeshedding on env_get_autostart()
- Fix '' in cover letter
- Use env_get_yesno() in env_get_autostart() and update docs

Changes in v8:
- Update commit message to avoid mentioning the 'env' subdirectory
- Update commit message to mention the + restriction, etc.
- Overwrite the env file each time, to avoid incremental-build problems
- Fix ambiguity about what is ignored
- Go into more detail about the change of behaviour with autostart

Changes in v7:
- Use 'env' basename instead of 'environment' for intermediate output files
- Show a message indicating the source text file being used
- Give an error if CONFIG_EXTRA_ENV_SETTINGS is also defined
- Use CONFIG_ENV_SOURCE_FILE instead of rules to specify the text-file name
- Make board.env the default name if CONFIG_ENV_SOURCE_FILE is empty
- Rewrite the documentation
- Drop the use of common.env
- Update awk script to output the whole CONFIG string, or just a comment
- Add new patch to explain the relationship with DEFAULT_ENV_FILE
- A few more tweaks
- Update the cover letter

Changes in v6:
- Move all updates to a separate patch
- Combine the two env2string.awk patches into one
- Move all updates to a separate patch
- More updates and improvements
- Add new patch to tidy up use of autostart env var

Changes in v5:
- Minor updates as suggested by Wolfgang
- Explain how to include the common.env file
- Explain why variables starting with _ , and / are not supported
- Expand the definition of how to declare an environment variable
- Explain what happens to empty variables
- Update maintainer
- Move use of += to this patch
- Explain that environment variables may not end in +
- Minor updates as suggested by Wolfgang

Changes in v4:
- Add new patch to move environment documentation to rST
- Move this from being part of configuring U-Boot to part of building it
- Don't put the environment in autoconf.mk as it is not needed
- Add documentation in rST format instead of README
- Drop mention of import/export
- Update awk script to ignore blank lines, as generated by clang
- Add documentation in 

Re: please help, "Ram disk image is corrupt or invalid" (qemu virt machine, arm64)

2021-10-19 Thread Jaehoon Chung
Hi,

On 10/19/21 9:31 PM, Chan Kim wrote:
> Hi, Jaehoon,
> 
> Thanks for the help.
> 
>> You can check the debug message in common/image-board.c.
>> Did you create ramdisk image the correct format?
> 
> To make ramdisk image (I made initramfs which I know is not a real disk image 
> format 
> (https://stackoverflow.com/questions/10603104/the-difference-between-initrd-and-initramfs,
>  
> but with -kernel=Image -initrd=initramfs.cpio.gz, qemu ran ok to the shell 
> prompt), 
> I followed this procedure under busybox-1.32.1 directory.
> 
> o build busybox
> 
> make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- defconfig
> make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- menuconfig
>==> set CONFIG_CROSS_COMPILE_PREFIX=aarch64-linux-gnu- and set 
> CONFIG_STATIC=y (position independent executable automatically not set)
> make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-
> make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- install
> 
> o after make install, under _install,
> mkdir -p dev
> sudo mknod dev/console c 5 1
> sudo mknod dev/ram b 1 0
> 
> o make init script which contains (under _install)
> 
> #!/bin/sh
> echo "### INIT SCRIPT ###"
> mkdir /proc /sys /tmp
> mount -t proc none /proc
> mount -t sysfs none /sys
> mount -t tmpfs none /tmp
> echo -e "\nThis boot took $(cut -d' ' -f1 /proc/uptime) seconds\n"
> ifconfig eth0 10.0.2.15 netmask 255.255.255.0 up
> route add default gw 10.0.2.2
> exec /bin/sh
> 
> o make ramdisk image (under _install directory)
> find -print0 | cpio -0oH newc | gzip -9 > ../initramfs.cpio.gz

AFAIK, you need to create the ramdisk image for U-boot with mkimage tool.
or you can include initramfs.cpio.gz in kernel at build time.

Best Regards,
Jaehoon Chung

> 
> 
> I tried to set breakpoint in boot_get_ramdisk function in common/image.c 
> (where "Wrong Ramdisk Image Format" is printed) but somehow the break point 
> doesn't work. (But I could stop at board_init_f function).
> I'll try to figure out why the break point isn't working there.
> 
> Can you find anything suspicious or give me a suggestion?
> 
> Thank you!
> Best regards,
> 
> Chan Kim
> 
>> -Original Message-
>> From: Jaehoon Chung 
>> Sent: Tuesday, October 19, 2021 7:43 PM
>> To: Chan Kim ; 'François Ozog' 
>> Cc: u-boot@lists.denx.de
>> Subject: Re: please help, "Ram disk image is corrupt or invalid" (qemu
>> virt machine, arm64)
>>
>> Hi,
>>
>> On 10/19/21 6:59 PM, Chan Kim wrote:
>>> Hi, François
>>>
>>>
>>>
>>> Thank you for the response.
>>>
>>> I’m now studying u-boot so it’ll take some time to learn about
>> SystemReady boot.
>>>
>>> BTW, inspired by your email, I tried this one. (after tftp of all
>>> three files)
>>>
>>>
>>>
>>> => fdt addr 0x4000
>>>
>>> => fdt chosen 0x4200 0x4211e2a1   // the length of
>> initramfs.cpio.gz is 1172129 = 0x11e2a1)
>>>
>>> => fdt print /chosen
>>>
>>> chosen {
>>>
>>>linux,initrd-end = <0x 0x4211e2a1>;
>>>
>>>linux,initrd-start = <0x 0x4200>;
>>>
>>>stdout-path = "/pl011@900";
>>>
>>> };
>>>
>>> =>  booti 0x4020 0x4200 0x4000
>>>
>>> Moving Image from 0x4020 to 0x4028, end=41c44000
>>>
>>> Wrong Ramdisk Image Format
>>>
>>> Ramdisk image is corrupt or invalid
>>
>> You can check the debug message in common/image-board.c.
>> Did you create ramdisk image the correct format?
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>>
>>>
>>> I hoped now I set the device tree to have the initrd location, the
>> kernel could find it, but not.
>>>
>>> Isn’t there a correct method using this approach?
>>>
>>> Thank you! Best regards,
>>>
>>>
>>>
>>> Chan Kim
>>>
>>>
>>>
>>> From: François Ozog 
>>> Sent: Tuesday, October 19, 2021 4:32 PM
>>> To: Chan Kim 
>>> Cc: u-boot@lists.denx.de
>>> Subject: Re: please help, "Ram disk image is corrupt or invalid" (qemu
>>> virt machine, arm64)
>>>
>>>
>>>
>>> Hi
>>>
>>>
>>>
>>> If you use the second argument to indicate the location of the initrd
>> then Linux will use device tree information to get the size of the initrd.
>> That information is probably absent or incorrect.
>>>
>>>
>>>
>>> The initrd= in your command line is thus useless as you task U-boot to
>> load it.
>>>
>>>
>>>
>>> You may want to consider the Arm defined SystemReady boot flow to avoid
>> issues and  benefit from secure boot flow and future extensions. In the
>> SystemReady boot flow , the Linux efi stub asks uboot to load the initrd
>> and do not need devicectree metadata to get any location or size (this is
>> to reinforce the use of device tree for hardware description as opposed to
>> hacking all information sharing between boot stages).
>>>
>>>
>>>
>>> Cheers
>>>
>>>
>>>
>>> FF
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> Le mer. 13 oct. 2021 à 07:44, Chan Kim >  > a écrit :
>>>
>>>
>>>
>>> Hello all,
>>>
>>>
>>>
>>> I can boot linux kernel using this command line.
>>>
>>> 

Re: [PATCH v2] mmc: arm_pl180_mmci: Enable HWFC for specific versions of MCI

2021-10-19 Thread Jaehoon Chung
On 10/19/21 11:49 PM, Usama Arif wrote:
> There are 4 registers (PERIPHID{0-3}) that contain the ID of MCI.
> For MMCs' with peripheral id 0x02041180 and 0x03041180, H/W flow control
> needs to be enabled for multi block writes (MMC CMD 18).
> 
> Signed-off-by: Usama Arif 

Reviewed-by: Jaehoon Chung 

Best Regards,
Jaehoon Chung

> ---
>  drivers/mmc/arm_pl180_mmci.c | 14 ++
>  drivers/mmc/arm_pl180_mmci.h |  1 +
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
> index f99b5f997e..9c5d48e90c 100644
> --- a/drivers/mmc/arm_pl180_mmci.c
> +++ b/drivers/mmc/arm_pl180_mmci.c
> @@ -282,6 +282,14 @@ static int host_request(struct mmc *dev,
>   return result;
>  }
>  
> +static int check_peripheral_id(struct pl180_mmc_host *host, u32 periph_id)
> +{
> + return readl(>base->periph_id0) == (periph_id & 0xFF) &&
> + readl(>base->periph_id1) == ((periph_id >> 8) & 0xFF)  &&
> + readl(>base->periph_id2) == ((periph_id >> 16) & 0xFF) &&
> + readl(>base->periph_id3) == ((periph_id >> 24) & 0xFF);
> +}
> +
>  static int  host_set_ios(struct mmc *dev)
>  {
>   struct pl180_mmc_host *host = dev->priv;
> @@ -337,6 +345,12 @@ static int  host_set_ios(struct mmc *dev)
>   sdi_clkcr &= ~(SDI_CLKCR_WIDBUS_MASK);
>   sdi_clkcr |= buswidth;
>   }
> + /* For MMCs' with peripheral id 0x02041180 and 0x03041180, H/W flow 
> control
> +  * needs to be enabled for multi block writes (MMC CMD 18).
> +  */
> + if (check_peripheral_id(host, 0x02041180) ||
> + check_peripheral_id(host, 0x03041180))
> + sdi_clkcr |= SDI_CLKCR_HWFCEN;
>  
>   writel(sdi_clkcr, >base->clock);
>   udelay(CLK_CHANGE_DELAY);
> diff --git a/drivers/mmc/arm_pl180_mmci.h b/drivers/mmc/arm_pl180_mmci.h
> index 15c29beadb..fca15910a8 100644
> --- a/drivers/mmc/arm_pl180_mmci.h
> +++ b/drivers/mmc/arm_pl180_mmci.h
> @@ -43,6 +43,7 @@
>  #define SDI_CLKCR_CLKEN  0x0100
>  #define SDI_CLKCR_PWRSAV 0x0200
>  #define SDI_CLKCR_BYPASS 0x0400
> +#define SDI_CLKCR_HWFCEN 0x1000
>  #define SDI_CLKCR_WIDBUS_MASK0x1800
>  #define SDI_CLKCR_WIDBUS_1   0x
>  #define SDI_CLKCR_WIDBUS_4   0x0800
> 



Re: buildman stops (crashed) on current master

2021-10-19 Thread Stefano Babic

Hi Simon,

On 19.10.21 17:52, Simon Glass wrote:

Hi Stefano,

On Tue, 19 Oct 2021 at 09:39, Stefano Babic  wrote:


Hi Simon,

On 07.10.21 15:43, Simon Glass wrote:

Hi Stefano,

On Thu, 7 Oct 2021 at 04:37, Stefano Babic  wrote:


Hi all,

CI stops by building aarch64 without notice, for reference:

https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/332319

There is no error, just process is killed. It looks like it stops at
xilinx_zynqmp_virt,

./tools/buildman/buildman -o /tmp -P -E -W aarch64but board can be built
without issues.

If I build on my host (not in docker, anyway), it generally builds fine
- but it crashes sometimes, too. On gitlab instance , it crashes.
Issue does not seem that depends on merged patches, and introduces
boards were already built successfully. Any hint ? I have also no idea
what I should look as what I see is just

"usr/bin/bash: line 104:24 Killed
./tools/buildman/buildman -o /tmp -P -E -W aarch64"


I cannot see that link. I am not sure what is going on. Does it say
what signal killed it?


Pipelines on our server were not public - I have enbaled now for u-boot-imx.



Does it sit there for an hour and timeout? If so, then I  did see that
myself once recently, when the Kconfig needed stdin, but I could not
quitetie it down. I think buildman would provide it, but sometimes
not, apparently. So it can happen when there is an existing build
there and your new one which adds Kconfig options that don't have
defaults, or something like that?



I have investigated further, and I can reproduce it on my host outside
the gitlab server. buildman causes a OOM, but I cannot find the cause.

Strange enough, this happens with the "aarch64" target, and I cannot
reproduce it with Tom's master. So it seems that -master is ok, and
somethin on u-boot-imx generates the OOM.

However

The OOM happens always when -2 (two boards remain) appears. I can see
with htop that buildman starts to allocate memory until it is exhausted
(64GB RAM + 8 GB swap). Then the kernel decides that it is enough and
kills buildman - this is what I see on Ci.

You can see now the pipelines:

https://source.denx.de/u-boot/custodians/u-boot-imx/-/pipelines/9520

I have then split aarch64 and I built imx8 separately - same result. The
pipeline stops with xilinx board, but they have nothing to do. In fact,
I can build all xilinx board separately. If I run buildman -W aarch64 -x
xilinx, OOM is shown by another board.

Strange enough, I can build each single board with buildman without
issues, neither errors nor warnongs. Just when buildman runs all
together (aarch64, 308 boards), the OOM is generated.

Bisect does not help: I started bisect, and at the end this commit was
presented:

commit 53a24dee86fb72ae41e7579607bafe13442616f2
Author: Fabio Estevam 
Date:   Mon Aug 23 21:11:09 2021 -0300

  imx8mm-cl-iot-gate: Split the defconfigs


But it is a fake: I can revert it, I get the issue again. And the patch
has nothing to do.

It looks to me it is something in binman, maybe triggered by some
changes in tree, but all boards can be built separately without issues.
I supposed to find the cause in code due to applied patches, but because
each board can be built and no help from bisect, I am quite puzzled. I
avoid to send a PR to Tom, else I guess the problem goes into -master,
but I do not know how to proceed, and I have a lot of patches to be applied.

What can be done ?


If that is it, you can repeat it by clearing out your .bm-work


On gitlab, the build starts from scratch.


Can you check that there is definitely nothing around from the previous build?


On my host, there is definitely something - because I cannot access the 
docker image on the server, I installed a local runner on my PC. So I 
can take a deeper look and jump on the container.


It runs, and I get the same issue. All boards are built, then it stucks 
until OOM happens and kernel kills it.


The job for aarch64 is:

./tools/buildman/buildman -o /tmp -P -E -W aarch64

and it runs in Tom's image. If I jump in the container (without running 
buildman), there is no .bm-work at all (this should be in /tmp, right ?)


uboot@e4a810aa6d8a:/$ ls -la tmp/
total 8
drwxrwxrwt 1 root root 4096 Sep 30 15:54 .
drwxr-xr-x 1 root root 4096 Oct 19 20:01 ..

So it is sure, before buildman runs there is no .bm-work. Then of 
course, after each job (defined in .gitlab-ci.yml), /tmp/.bm-work is 
present. But I guess you do not mean to drop .bm-work after each job, 
right ? Else this should be also put in .gitla-ci.yml.


Where build stucks...I do not know, but it looks like that all boards 
were already built successfully (just as example 
https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/337915).







directory then building just that board for one commit, then the next
(with the Kconfig change).


I have run buildman for each single board, all of them were successuful.
With aarch64, I get OOM from buildman.



Buildman is supposed to handle 

Re: [PATCH] mtd: spi-nor-ids: Add is25lp512 and is25wp512 devices

2021-10-19 Thread Pratyush Yadav
On 18/10/21 03:26AM, Kris Chaplin wrote:
> Add is25lp512 and is25wp512 devices to spi-nor id table
> 
> Tested on Intel n5x hardware with QSPI carrier card
> 
> Signed-off-by: Kris Chaplin 
> Tested-by: Kris Chaplin 

Same comment as before about the Tested-by trailer. Also adding SPI NOR 
maintainers in Cc, as mentioned in previous email.

I dunno much about these flashes so I am not commenting on the patch 
contents.

> ---
>  drivers/mtd/spi/spi-nor-ids.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
> index 6ca669c591..5359d09489 100644
> --- a/drivers/mtd/spi/spi-nor-ids.c
> +++ b/drivers/mtd/spi/spi-nor-ids.c
> @@ -133,6 +133,8 @@ const struct flash_info spi_nor_ids[] = {
>   SECT_4K | SPI_NOR_DUAL_READ) },
>   { INFO("is25lp256",  0x9d6019, 0, 64 * 1024, 512,
>   SECT_4K | SPI_NOR_DUAL_READ) },
> + { INFO("is25lp512",  0x9d601a, 0, 64 * 1024, 1024,
> + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>   { INFO("is25wp032",  0x9d7016, 0, 64 * 1024,  64,
>   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>   { INFO("is25wp064",  0x9d7017, 0, 64 * 1024, 128,
> @@ -142,6 +144,8 @@ const struct flash_info spi_nor_ids[] = {
>   { INFO("is25wp256",  0x9d7019, 0, 64 * 1024, 512,
>   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>   SPI_NOR_4B_OPCODES) },
> + { INFO("is25wp512",  0x9d701a, 0, 64 * 1024, 1024,
> + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>  #endif
>  #ifdef CONFIG_SPI_FLASH_MACRONIX /* MACRONIX */
>   /* Macronix */
> -- 
> 2.25.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.


Re: [PATCH] mtd: spi-nor-ids: Add SECT_4K to mt25qu512a

2021-10-19 Thread Pratyush Yadav
On 18/10/21 03:30AM, Kris Chaplin wrote:
> The mt25qu512a supports 4K or 64K sectors, so adding SECT_4K to enable 4K 
> sector usage.
> 
> Datasheet: 
> https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/n25q/n25q_512mb_1ce_3v_65nm.pdf
> 
> Tested on Intel n5x hardware with QSPI carrier card
> 
> Signed-off-by: Kris Chaplin 
> Tested by: Kris Chaplin 

This is unusual. The patch should *always* be tested by the author so 
you don't really need a Tested-by from the author. It is implied.

You should also Cc the subsystem maintainers so that they can see the 
patch and review and apply it. You can use `./scripts/get_maintainer.pl 
` to get the list of people who need to be in Cc. 
Adding Jagan and Vignesh for this patch.

Anyway, with the Tested-by trailer dropped,

Acked-by: Pratyush Yadav 

> ---
>  drivers/mtd/spi/spi-nor-ids.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
> index f0c8041997..5359d09489 100644
> --- a/drivers/mtd/spi/spi-nor-ids.c
> +++ b/drivers/mtd/spi/spi-nor-ids.c
> @@ -190,7 +190,7 @@ const struct flash_info spi_nor_ids[] = {
>   { INFO6("mt25qu256a",  0x20bb19, 0x104400, 64 * 1024,  512, SECT_4K | 
> SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | USE_FSR) },
>   { INFO("n25q256ax1",  0x20bb19, 0, 64 * 1024,  512, SECT_4K | 
> SPI_NOR_QUAD_READ | USE_FSR) },
>   { INFO6("mt25qu512a",  0x20bb20, 0x104400, 64 * 1024, 1024,
> -  SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
> +  SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | 
> SPI_NOR_4B_OPCODES |
>USE_FSR) },
>   { INFO("n25q512a",0x20bb20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | 
> SPI_NOR_QUAD_READ) },
>   { INFO6("mt25ql512a",  0x20ba20, 0x104400, 64 * 1024, 1024, SECT_4K | 
> USE_FSR | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> -- 
> 2.25.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.


Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Simon Glass
Hi Tom,

On Tue, 19 Oct 2021 at 10:44, Tom Rini  wrote:
>
> On Tue, Oct 19, 2021 at 10:39:55AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Tue, 19 Oct 2021 at 10:30, Tom Rini  wrote:
> > >
> > > On Tue, Oct 19, 2021 at 10:24:25AM -0600, Simon Glass wrote:
> > > > Hi Wolfgang,
> > > >
> > > > On Tue, 19 Oct 2021 at 10:20, Wolfgang Denk  wrote:
> > > > >
> > > > > Dear Simon,
> > > > >
> > > > > In message 
> > > > >  
> > > > > you wrote:
> > > > > >
> > > > > > But how do we handle this?
> > > > > >
> > > > > > var+=fred
> > > > > >
> > > > > > Is this appending to var or assigning to var+  ?
> > > > >
> > > > > It is assigning to "var+".
> > > >
> > > > er...
> > > >
> > > > > >
> > > > > > var++=fred
> > > > > >
> > > > > > is unambiguous but very confusing. I think it would be better to 
> > > > > > disallow +
> > > > >
> > > > > It's neither unambiguous nor confusing.  It is assigning to "var++".
> > > >
> > > > What? Can you read that again?
> > >
> > > With the assumption that the append operator is "=+" and NOT "+=" then
> > > yes, your examples are unambiguous because = is not allowed in variable
> > > names, before and after.
> > >
> > > > > I think we should not change what is old and might be in use.
> > > > >
> > > > > It is much easier to change what is new and can be defined at will.
> > > > >
> > > > > If we define for example that "=+" appends, then we can
> > > > > also define our own escape rules, for example:
> > > > >
> > > > > var=fredassigns
> > > > > var=+fred   appends "fred"
> > > > > var=\+fred  assignes the value "+fred"
> > > > > var=++fred  appends "+fred"
> > > >
> > > > I don't like that at all. It requires an escape for a common case and
> > > > is very confusing.
> > >
> > > Wait saying we'll add "+SOMETHING" is a common case?
> >
> > Yes we have places where we add to env vars depending on CONFIG settings.
>
> Yes, but what requires escape is we want "var" to evaluate to or append
> the string "+fred".  Not append "fred" which is common.
>
> > > > Since people will be converting their out-of-tree scripts anyway, they
> > > > can check for this sort of madness at the time. There should be no
> > > > problem.
> > >
> > > I'm not sure I like saying the operator is "=+" rather than "+=" because
> > > "=+" is a less commonly seen operator and tends to be an alternative
> > > appends for special cases / side-effects / position in parsing.
> >
> > Me neither. I started hearing Voltaire's admonition ringing in my head
> > a few emails back.
> >
> > The way I have this, is it fairly trivial to convert an existing
> > script to a text file. I suspect it can be done automatically but I
> > have not actually tried it. I'd really like to keep it simple. I also
> > want to invoke the 'if you are not in mainline you don't exist' maxim
> > at this point.
>
> I really want to see the non-trivially-constructed case where "+" is at
> the end of a variable today.  If we can support it in the middle, yes, I
> can see how there might be example of that in use today.

OK it is supported in the middle at present.

This produces no output for me:

git grep +=include/configs/

I'll add some tests, deal with the comments from Marek and send v9.

Regards,
Simon


Re: [PATCH v8 6/8] doc: Mention CONFIG_DEFAULT_ENV_FILE

2021-10-19 Thread Simon Glass
Hi Marek,

On Tue, 19 Oct 2021 at 10:07, Marek Behún  wrote:
>
> On Tue, 19 Oct 2021 09:52:17 -0600
> Simon Glass  wrote:
>
> > Hi Marek,
> >
> > On Tue, 19 Oct 2021 at 08:34, Marek Behún  wrote:
> > >
> > > On Mon, 18 Oct 2021 12:13:20 -0600
> > > Simon Glass  wrote:
> > >
> > > > Add mention of this option this it does a similar thing to the text
> > > > environment.
> > >
> > > Strange wording.
> >
> > I say that a lot :-)
> >
> > https://ludwig.guru/s/add+mention
>
> I am talking about
>   "of this option this it does"
> shouldn't it be
>   "of this option that it does" ?

OK, got it.

Thanks,
Simon


Re: [PATCH v1] driver: spi: add bcm iproc qspi support.

2021-10-19 Thread Roman Bacik
On Mon, Oct 11, 2021 at 12:03 AM Bharat Gooty  wrote:
>
> + Roman
> Thanks for the review Jagan. Will look and get back to you.
>
> Thanks,
> -Bharat
>
> On Fri, Oct 8, 2021 at 5:57 PM Jagan Teki  wrote:
>>
>> On Wed, Aug 25, 2021 at 6:55 PM Bharat Kumar Reddy Gooty
>>  wrote:
>> >
>> > From: Rayagonda Kokatanur 
>> >
>> > IPROC qspi driver supports both BSPI and MSPI modes.
>> >
>> > Signed-off-by: Rayagonda Kokatanur 
>> > Signed-off-by: Bharat Gooty 
>> > ---
>> >  drivers/spi/Kconfig  |   6 +
>> >  drivers/spi/Makefile |   1 +
>> >  drivers/spi/iproc_qspi.c | 736 +++
>> >  drivers/spi/iproc_qspi.h |  18 +
>> >  4 files changed, 761 insertions(+)
>> >  create mode 100644 drivers/spi/iproc_qspi.c
>> >  create mode 100644 drivers/spi/iproc_qspi.h
>> >
>> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
>> > index e12699bec7..3253d6badf 100644
>> > --- a/drivers/spi/Kconfig
>> > +++ b/drivers/spi/Kconfig
>> > @@ -178,6 +178,12 @@ config ICH_SPI
>> >   access the SPI NOR flash on platforms embedding this Intel
>> >   ICH IP core.
>> >
>> > +config IPROC_QSPI
>> > +   bool "QSPI driver for BCM iProc QSPI Controller"
>> > +   help
>> > + This selects the BCM iProc QSPI controller.
>> > + This driver support spi flash single, quad and memory reads.
>> > +
>> >  config KIRKWOOD_SPI
>> > bool "Marvell Kirkwood SPI Driver"
>> > help
>> > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
>> > index d2f24bccef..8697631870 100644
>> > --- a/drivers/spi/Makefile
>> > +++ b/drivers/spi/Makefile
>> > @@ -33,6 +33,7 @@ obj-$(CONFIG_FSL_DSPI) += fsl_dspi.o
>> >  obj-$(CONFIG_FSL_ESPI) += fsl_espi.o
>> >  obj-$(CONFIG_SYNQUACER_SPI) += spi-synquacer.o
>> >  obj-$(CONFIG_ICH_SPI) +=  ich.o
>> > +obj-$(CONFIG_IPROC_QSPI) += iproc_qspi.o
>> >  obj-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o
>> >  obj-$(CONFIG_MESON_SPIFC) += meson_spifc.o
>> >  obj-$(CONFIG_MPC8XX_SPI) += mpc8xx_spi.o
>> > diff --git a/drivers/spi/iproc_qspi.c b/drivers/spi/iproc_qspi.c
>> > new file mode 100644
>> > index 00..89c6a56858
>> > --- /dev/null
>> > +++ b/drivers/spi/iproc_qspi.c
>> > @@ -0,0 +1,736 @@
>> > +// SPDX-License-Identifier: GPL-2.0+
>> > +/*
>> > + * Copyright 2020-2021 Broadcom
>> > + */
>> > +
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>>
>> Why spi-nor header in spi driver?

Should we use spi-mem instead?

>>
>> > +#include "iproc_qspi.h"
>> > +
>> > +/* 175MHz */
>> > +#define QSPI_AXI_CLK   17500
>> > +#define QSPI_DEF_SCK_FREQ  5000
>> > +#define QSPI_WAIT_TIMEOUT_MS   200U
>> > +#define DWORD_ALIGNED(a)   (!(((ulong)(a)) & 3))
>> > +
>> > +/* Chip attributes */
>> > +#define SPBR_MIN   8U
>> > +#define SPBR_MAX   255U
>> > +#define NUM_CDRAM  16U
>> > +
>> > +#define CDRAM_PCS0 2
>> > +#define CDRAM_CONT BIT(7)
>> > +#define CDRAM_BITS_EN  BIT(6)
>> > +#define CDRAM_QUAD_MODEBIT(8)
>> > +#define CDRAM_RBIT_INPUT   BIT(10)
>> > +#define MSPI_SPE   BIT(6)
>> > +#define MSPI_CONT_AFTER_CMDBIT(7)
>> > +
>> > +/* Register fields */
>> > +#define MSPI_SPCR0_MSB_BITS_8  0x0020
>> > +#define BSPI_RAF_CONTROL_START_MASK0x0001
>> > +#define BSPI_RAF_STATUS_SESSION_BUSY_MASK  0x0001
>> > +#define BSPI_RAF_STATUS_FIFO_EMPTY_MASK0x0002
>> > +#define BSPI_BITS_PER_PHASE_ADDR_MARK  0x0001
>> > +#define BSPI_BITS_PER_CYCLE_DATA_SHIFT 0
>> > +#define BSPI_BITS_PER_CYCLE_ADDR_SHIFT 16
>> > +#define BSPI_STRAP_OVERRIDE_DATA_QUAD_SHIFT3
>> > +#define BSPI_STRAP_OVERRIDE_DATA_DUAL_SHIFT1
>> > +#define BSPI_STRAP_OVERRIDE_SHIFT  0
>> > +
>> > +/* MSPI registers */
>> > +#define MSPI_SPCR0_LSB_REG 0x000
>> > +#define MSPI_SPCR0_MSB_REG 0x004
>> > +#define MSPI_SPCR1_LSB_REG 0x008
>> > +#define MSPI_SPCR1_MSB_REG 0x00c
>> > +#define MSPI_NEWQP_REG 0x010
>> > +#define MSPI_ENDQP_REG 0x014
>> > +#define MSPI_SPCR2_REG 0x018
>> > +#define MSPI_STATUS_REG0x020
>> > +#define MSPI_CPTQP_REG 0x024
>> > +#define MSPI_TXRAM_REG 0x040
>> > +#define MSPI_RXRAM_REG 0x0c0
>> > +#define MSPI_CDRAM_REG 0x140
>> > +#define MSPI_WRITE_LOCK_REG0x180
>> > +#define MSPI_DISABLE_FLUSH_GEN_REG 0x184
>> > +
>> 

[PATCH] spl: fit: Skip attempting to load 0 length image

2021-10-19 Thread Nishanth Menon
When, for various reasons, a bad FIT image is used where a loadable
image is marked as 0 length, attempt is made for a 0 length allocation and
read of 0 byte read operation.

Instead provide warning in log and skip attempting to do such a load.

Signed-off-by: Nishanth Menon 
---
 common/spl/spl_fit.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index f41abca0ccb5..e9540dc2167a 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -286,6 +286,13 @@ static int spl_load_fit_image(struct spl_load_info *info, 
ulong sector,
if (fit_image_get_data_size(fit, node, ))
return -ENOENT;
 
+   /* Dont bother to copy 0 byte data, but warn, though */
+   if (!len) {
+   log_warning("%s: Skip load '%s': image size is 0!\n",
+   __func__, fit_get_name(fit, node, NULL));
+   return 0;
+   }
+
src_ptr = map_sysmem(ALIGN(load_addr, ARCH_DMA_MINALIGN), len);
length = len;
 
-- 
2.32.0



Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Tom Rini
On Tue, Oct 19, 2021 at 10:39:55AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 19 Oct 2021 at 10:30, Tom Rini  wrote:
> >
> > On Tue, Oct 19, 2021 at 10:24:25AM -0600, Simon Glass wrote:
> > > Hi Wolfgang,
> > >
> > > On Tue, 19 Oct 2021 at 10:20, Wolfgang Denk  wrote:
> > > >
> > > > Dear Simon,
> > > >
> > > > In message 
> > > >  
> > > > you wrote:
> > > > >
> > > > > But how do we handle this?
> > > > >
> > > > > var+=fred
> > > > >
> > > > > Is this appending to var or assigning to var+  ?
> > > >
> > > > It is assigning to "var+".
> > >
> > > er...
> > >
> > > > >
> > > > > var++=fred
> > > > >
> > > > > is unambiguous but very confusing. I think it would be better to 
> > > > > disallow +
> > > >
> > > > It's neither unambiguous nor confusing.  It is assigning to "var++".
> > >
> > > What? Can you read that again?
> >
> > With the assumption that the append operator is "=+" and NOT "+=" then
> > yes, your examples are unambiguous because = is not allowed in variable
> > names, before and after.
> >
> > > > I think we should not change what is old and might be in use.
> > > >
> > > > It is much easier to change what is new and can be defined at will.
> > > >
> > > > If we define for example that "=+" appends, then we can
> > > > also define our own escape rules, for example:
> > > >
> > > > var=fredassigns
> > > > var=+fred   appends "fred"
> > > > var=\+fred  assignes the value "+fred"
> > > > var=++fred  appends "+fred"
> > >
> > > I don't like that at all. It requires an escape for a common case and
> > > is very confusing.
> >
> > Wait saying we'll add "+SOMETHING" is a common case?
> 
> Yes we have places where we add to env vars depending on CONFIG settings.

Yes, but what requires escape is we want "var" to evaluate to or append
the string "+fred".  Not append "fred" which is common.

> > > Since people will be converting their out-of-tree scripts anyway, they
> > > can check for this sort of madness at the time. There should be no
> > > problem.
> >
> > I'm not sure I like saying the operator is "=+" rather than "+=" because
> > "=+" is a less commonly seen operator and tends to be an alternative
> > appends for special cases / side-effects / position in parsing.
> 
> Me neither. I started hearing Voltaire's admonition ringing in my head
> a few emails back.
> 
> The way I have this, is it fairly trivial to convert an existing
> script to a text file. I suspect it can be done automatically but I
> have not actually tried it. I'd really like to keep it simple. I also
> want to invoke the 'if you are not in mainline you don't exist' maxim
> at this point.

I really want to see the non-trivially-constructed case where "+" is at
the end of a variable today.  If we can support it in the middle, yes, I
can see how there might be example of that in use today.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Simon Glass
Hi Tom,

On Tue, 19 Oct 2021 at 10:30, Tom Rini  wrote:
>
> On Tue, Oct 19, 2021 at 10:24:25AM -0600, Simon Glass wrote:
> > Hi Wolfgang,
> >
> > On Tue, 19 Oct 2021 at 10:20, Wolfgang Denk  wrote:
> > >
> > > Dear Simon,
> > >
> > > In message 
> > >  you 
> > > wrote:
> > > >
> > > > But how do we handle this?
> > > >
> > > > var+=fred
> > > >
> > > > Is this appending to var or assigning to var+  ?
> > >
> > > It is assigning to "var+".
> >
> > er...
> >
> > > >
> > > > var++=fred
> > > >
> > > > is unambiguous but very confusing. I think it would be better to 
> > > > disallow +
> > >
> > > It's neither unambiguous nor confusing.  It is assigning to "var++".
> >
> > What? Can you read that again?
>
> With the assumption that the append operator is "=+" and NOT "+=" then
> yes, your examples are unambiguous because = is not allowed in variable
> names, before and after.
>
> > > I think we should not change what is old and might be in use.
> > >
> > > It is much easier to change what is new and can be defined at will.
> > >
> > > If we define for example that "=+" appends, then we can
> > > also define our own escape rules, for example:
> > >
> > > var=fredassigns
> > > var=+fred   appends "fred"
> > > var=\+fred  assignes the value "+fred"
> > > var=++fred  appends "+fred"
> >
> > I don't like that at all. It requires an escape for a common case and
> > is very confusing.
>
> Wait saying we'll add "+SOMETHING" is a common case?

Yes we have places where we add to env vars depending on CONFIG settings.

>
> > Since people will be converting their out-of-tree scripts anyway, they
> > can check for this sort of madness at the time. There should be no
> > problem.
>
> I'm not sure I like saying the operator is "=+" rather than "+=" because
> "=+" is a less commonly seen operator and tends to be an alternative
> appends for special cases / side-effects / position in parsing.

Me neither. I started hearing Voltaire's admonition ringing in my head
a few emails back.

The way I have this, is it fairly trivial to convert an existing
script to a text file. I suspect it can be done automatically but I
have not actually tried it. I'd really like to keep it simple. I also
want to invoke the 'if you are not in mainline you don't exist' maxim
at this point.

Regards,
Simon


Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Tom Rini
On Tue, Oct 19, 2021 at 10:24:25AM -0600, Simon Glass wrote:
> Hi Wolfgang,
> 
> On Tue, 19 Oct 2021 at 10:20, Wolfgang Denk  wrote:
> >
> > Dear Simon,
> >
> > In message 
> >  you 
> > wrote:
> > >
> > > But how do we handle this?
> > >
> > > var+=fred
> > >
> > > Is this appending to var or assigning to var+  ?
> >
> > It is assigning to "var+".
> 
> er...
> 
> > >
> > > var++=fred
> > >
> > > is unambiguous but very confusing. I think it would be better to disallow 
> > > +
> >
> > It's neither unambiguous nor confusing.  It is assigning to "var++".
> 
> What? Can you read that again?

With the assumption that the append operator is "=+" and NOT "+=" then
yes, your examples are unambiguous because = is not allowed in variable
names, before and after.

> > I think we should not change what is old and might be in use.
> >
> > It is much easier to change what is new and can be defined at will.
> >
> > If we define for example that "=+" appends, then we can
> > also define our own escape rules, for example:
> >
> > var=fredassigns
> > var=+fred   appends "fred"
> > var=\+fred  assignes the value "+fred"
> > var=++fred  appends "+fred"
> 
> I don't like that at all. It requires an escape for a common case and
> is very confusing.

Wait saying we'll add "+SOMETHING" is a common case?

> Since people will be converting their out-of-tree scripts anyway, they
> can check for this sort of madness at the time. There should be no
> problem.

I'm not sure I like saying the operator is "=+" rather than "+=" because 
"=+" is a less commonly seen operator and tends to be an alternative
appends for special cases / side-effects / position in parsing.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Simon Glass
Hi Wolfgang,

On Tue, 19 Oct 2021 at 10:20, Wolfgang Denk  wrote:
>
> Dear Simon,
>
> In message 
>  you 
> wrote:
> >
> > But how do we handle this?
> >
> > var+=fred
> >
> > Is this appending to var or assigning to var+  ?
>
> It is assigning to "var+".

er...

> >
> > var++=fred
> >
> > is unambiguous but very confusing. I think it would be better to disallow +
>
> It's neither unambiguous nor confusing.  It is assigning to "var++".

What? Can you read that again?

>
>
> I think we should not change what is old and might be in use.
>
> It is much easier to change what is new and can be defined at will.
>
> If we define for example that "=+" appends, then we can
> also define our own escape rules, for example:
>
> var=fredassigns
> var=+fred   appends "fred"
> var=\+fred  assignes the value "+fred"
> var=++fred  appends "+fred"

I don't like that at all. It requires an escape for a common case and
is very confusing.

Since people will be converting their out-of-tree scripts anyway, they
can check for this sort of madness at the time. There should be no
problem.

Regards,
Simon


Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Wolfgang Denk
Dear Simon,

In message  
you wrote:
>
> But how do we handle this?
>
> var+=fred
>
> Is this appending to var or assigning to var+  ?

It is assigning to "var+".
>
> var++=fred
>
> is unambiguous but very confusing. I think it would be better to disallow +

It's neither unambiguous nor confusing.  It is assigning to "var++".


I think we should not change what is old and might be in use.

It is much easier to change what is new and can be defined at will.

If we define for example that "=+" appends, then we can
also define our own escape rules, for example:

var=fredassigns
var=+fred   appends "fred"
var=\+fred  assignes the value "+fred"
var=++fred  appends "+fred"

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Monday is an awful way to spend one seventh of your life.


Re: [PATCH v6 4/7] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Simon Glass
Hi Wolfgang,

On Tue, 19 Oct 2021 at 10:09, Wolfgang Denk  wrote:
>
> Dear Simon,
>
> In message 
>  you 
> wrote:
> >
> > I thought you were trying to use + at the end of a variable.
> >
> > I used:
> >
> > fred\+=aaa
> >
> > and got
> >
> > cc1: warning: unknown escape sequence: '\=''
> >
> > You can try it yourself by editing sandbox.env in the
> > u-boot-dm/env-working tree.
>
> Hmmm...
>
> -> cat foo.c
> #define A ampersand
> #define B berta
>
> foo\+=B;
> -> gcc -E foo.c
> # 0 "foo.c"
> # 0 ""
> # 0 ""
> # 1 "/usr/include/stdc-predef.h" 1 3 4
> # 0 "" 2
> # 1 "foo.c"
>
>
>
> foo\+=berta;
>
>
>
> -> cat bar.c
> fred\+=aaa
> -> gcc -E bar.c
> # 0 "bar.c"
> # 0 ""
> # 0 ""
> # 1 "/usr/include/stdc-predef.h" 1 3 4
> # 0 "" 2
> # 1 "bar.c"
> fred\+=aaa
>
>
> I do not see this problem...

Would you mind actually trying it in the env file, as I suggested?

Regards,
Simon


Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Simon Glass
Hi Wolfgang,

On Tue, 19 Oct 2021 at 10:05, Wolfgang Denk  wrote:
>
> Dear Simon,
>
> In message 
>  you 
> wrote:
> >
> > Well ideally I'd like to avoid Python in this case as it is in the
> > compilation path. I am not sure yet what Wolfgang actually wants,
> > apart from variable names ending with + which I would like to
> > disallow.
> >
> > So if we can clearly understand the goal, then we might be able to do
> > it in awk, but, again, can we just disallow '+' in var names ?

Re lex..

I cannot imagine why we would want to use that, given that we want to
include the environment literally, so far as possible.

Anyway, if we did we would presumably have symbols for + and += so how
would that help with the ambiguity I mentioned?

>
> My goal is to have a parser that does not place new restrictions on
> an ancient interface.  The first step would probably be to define a
> clear syntax description.
>
> Eventually this would be also more allowing for variations in white
> space, so that "=" could also be written as " =
> ".  which in turn would allow for
>
> foo+ = bar
> vs.
> foo += bar
> .
>
> Just my 2¢...

I think we're up to about $12 at this point :-)

But then how do you add a space to an env variable? At the moment you can do:

bootargs=fred
#ifdef CONFIG_SOMETHING
bootargs+= more
#endif

Regards,
Simon


Re: [PATCH v6 4/7] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Wolfgang Denk
Dear Simon,

In message  
you wrote:
>
> I thought you were trying to use + at the end of a variable.
>
> I used:
>
> fred\+=aaa
>
> and got
>
> cc1: warning: unknown escape sequence: '\=''
>
> You can try it yourself by editing sandbox.env in the
> u-boot-dm/env-working tree.

Hmmm...

-> cat foo.c
#define A ampersand
#define B berta

foo\+=B;
-> gcc -E foo.c
# 0 "foo.c"
# 0 ""
# 0 ""
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 0 "" 2
# 1 "foo.c"



foo\+=berta;



-> cat bar.c
fred\+=aaa
-> gcc -E bar.c
# 0 "bar.c"
# 0 ""
# 0 ""
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 0 "" 2
# 1 "bar.c"
fred\+=aaa


I do not see this problem...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
I don't know if it's what you want, but it's what you get.  :-)
  - Larry Wall in <10...@jpl-devvax.jpl.nasa.gov>


Re: [PATCH v8 5/8] sandbox: Use a text-based environment

2021-10-19 Thread Marek Behún
On Tue, 19 Oct 2021 09:52:14 -0600
Simon Glass  wrote:

> Hi Marek,
> 
> On Tue, 19 Oct 2021 at 08:32, Marek Behún  wrote:
> >
> > On Mon, 18 Oct 2021 12:13:19 -0600
> > Simon Glass  wrote:
> >  
> > > --- /dev/null
> > > +++ b/board/sandbox/sandbox.env
> > > @@ -0,0 +1,25 @@
> > > +stdin=serial
> > > +#ifdef CONFIG_SANDBOX_SDL
> > > +stdin+=,cros-ec-keyb,usbkbd  
> >
> > this was CONFIG_KEYBOARD and now is CONFIG_SANDBOX_SDL, is this okay?  
> 
> The old code is:
> 
> #ifdef CONFIG_SANDBOX_SDL
> #define LCD_BPP LCD_COLOR16
> #define CONFIG_LCD_BMP_RLE8
> 
> #define CONFIG_KEYBOARD
> 
> #define SANDBOX_SERIAL_SETTINGS "stdin=serial,cros-ec-keyb,usbkbd\0" \
> "stdout=serial,vidconsole\0" \
> "stderr=serial,vidconsole\0"
> #else
> #define SANDBOX_SERIAL_SETTINGS "stdin=serial\0" \
> "stdout=serial,vidconsole\0" \
> "stderr=serial,vidconsole\0"
> #endif
> 
> 
> so really the check is on CONFIG_SANDBOX_SDL. The setting of
> CONFIG_KEYBOARD is not related to the environment.

Ah, sorry. In that case

Reviewed-by: Marek Behún 


Re: [PATCH v8 6/8] doc: Mention CONFIG_DEFAULT_ENV_FILE

2021-10-19 Thread Marek Behún
On Tue, 19 Oct 2021 09:52:17 -0600
Simon Glass  wrote:

> Hi Marek,
> 
> On Tue, 19 Oct 2021 at 08:34, Marek Behún  wrote:
> >
> > On Mon, 18 Oct 2021 12:13:20 -0600
> > Simon Glass  wrote:
> >  
> > > Add mention of this option this it does a similar thing to the text
> > > environment.  
> >
> > Strange wording.  
> 
> I say that a lot :-)
> 
> https://ludwig.guru/s/add+mention

I am talking about
  "of this option this it does"
shouldn't it be
  "of this option that it does" ?


Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Wolfgang Denk
Dear Simon,

In message  
you wrote:
>
> Well ideally I'd like to avoid Python in this case as it is in the
> compilation path. I am not sure yet what Wolfgang actually wants,
> apart from variable names ending with + which I would like to
> disallow.
>
> So if we can clearly understand the goal, then we might be able to do
> it in awk, but, again, can we just disallow '+' in var names ?

My goal is to have a parser that does not place new restrictions on
an ancient interface.  The first step would probably be to define a
clear syntax description.

Eventually this would be also more allowing for variations in white
space, so that "=" could also be written as " =
".  which in turn would allow for

foo+ = bar
vs.
foo += bar
.

Just my 2¢...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Our universe is a fragile house of atoms, held together by the mortar
of cause-and-effect. One magician would be two too many.
- Terry Pratchett, _The Dark Side of the Sun_


Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Wolfgang Denk
Dear Tom,

In message <20211019140711.GC7964@bill-the-cat> you wrote:
> 
> As much as I and others appreciate that you've written the parser here
> in a classic UNIX tool, awk, since a lot of the problems also seem to
> stem from having the parser be able to handle previously valid
> environment variables, if this was written in Python say, would we have
> this problem?

A classic tool would be lex ...


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Doing the good deeds is like the grass in the garden. You  don't  see
its  growth. But, it does by days. Doing the wicked deeds is like the
hone. You don't see its damage. But, it does by days.- Buddha


Re: [PATCH v4] sandbox: Remove OF_HOSTFILE

2021-10-19 Thread Simon Glass
Hi Ilias, Tom,

On Tue, 19 Oct 2021 at 09:38, Ilias Apalodimas
 wrote:
>
> On Tue, 19 Oct 2021 at 18:36, Tom Rini  wrote:
> >
> > On Tue, Oct 19, 2021 at 06:30:59PM +0300, Ilias Apalodimas wrote:
> > > On Tue, 19 Oct 2021 at 17:17, Tom Rini  wrote:
> > > >
> > > > On Tue, Oct 19, 2021 at 08:03:07AM -0600, Simon Glass wrote:
> > >
> > > [...]
> > >
> > > > >
> > > > > For some reason this still does not apply for me on -master. Can you
> > > > > please confirm the hash you are using?
> > > >
> > > > The hunk for scripts/Makefile.spl still fails (it failed on v3 as well),
> > > > but is easily fixed-up, fwiw.
> > >
> > > The reason is that I rebased on top of the prerequisite series, but
> > > failed to pull in -master.  As Tom says it's a single line of code
> > > that's in conflict.  Shall I send a v5 or will you pull that in Tom?
> >
> > Well, what should that look like exactly?  It fails to apply cleanly on
> > current master.  I had swapped it to CONFIG_SANDBOX, but I gather from
> > Simon's comment it should be OF_BOARD?
> >
>
> It should be OF_SANDBOX.  I tried to fold SANDBOX to OF_BOARD befire
> sending it, but I had some failures I didn't have time to investigate.
> So I'll leave that to Simon's further cleanup

Yes that's fine, my OF_BOARD series can clean it up. We can leave it
as a special case for now.

Regards,
Simon


Re: [EXT] Re: [PATCH v3 01/16] crypto/fsl: Add support for CAAM Job ring driver model

2021-10-19 Thread Simon Glass
Hi Gaurav,

On Tue, 19 Oct 2021 at 00:40, Gaurav Jain  wrote:
>
> Hi Simon
>
> > -Original Message-
> > From: Simon Glass 
> > Sent: Thursday, October 14, 2021 8:40 PM
> > To: Gaurav Jain 
> > Cc: U-Boot Mailing List ; Stefano Babic
> > ; Fabio Estevam ; Peng Fan
> > ; Priyanka Jain ; Ye Li
> > ; Horia Geanta ; Ji Luo
> > ; Franck Lenormand ; Silvano Di
> > Ninno ; Sahil Malhotra ;
> > Pankaj Gupta ; Varun Sethi ; dl-
> > uboot-imx ; Shengzhou Liu ;
> > Mingkai Hu ; Rajesh Bhagat ;
> > Meenakshi Aggarwal ; Wasim Khan
> > ; Alison Wang ; Pramod
> > Kumar ; Andy Tang ;
> > Adrian Alonso ; Vladimir Oltean 
> > Subject: [EXT] Re: [PATCH v3 01/16] crypto/fsl: Add support for CAAM Job 
> > ring
> > driver model
> >
> > Caution: EXT Email
> >
> > Hi,
> >
> > On Mon, 4 Oct 2021 at 23:40, Gaurav Jain  wrote:
> > >
> > > added device tree support for job ring driver.
> > > sec is initialized based on job ring information processed from device
> > > tree.
> > >
> > > Signed-off-by: Gaurav Jain 
> > > Reviewed-by: Ye Li 
> > > ---
> > >  cmd/Kconfig |   1 +
> > >  drivers/crypto/fsl/Kconfig  |   7 +
> > >  drivers/crypto/fsl/Makefile |   3 +-
> > >  drivers/crypto/fsl/jr.c | 318 
> > >  drivers/crypto/fsl/jr.h |  14 ++
> > >  5 files changed, 233 insertions(+), 110 deletions(-)
> >
> > This does not look like a proper driver to me, as it has an exported 
> > function
> > sec_init_idx().
> >
> > What operations does it perform? Should it have a uclass?
> >
> > Regards,
> > Simon
>
> For majority of platforms driver model is followed.
> But due to memory constraints with few platforms, driver model cannot be 
> implemented on SPL.
> so to enable caam on such platforms we are using non DM approach( via 
> sec_init_idx() ).
>
> sec_init_idx() perform tasks that are required for CAAM to work.
> caam reset, caam initialization, job ring initialization, RNG instantiation.

OK.

BTW have you tried using of-platdata?

Regards,
Simon


Re: buildman stops (crashed) on current master

2021-10-19 Thread Simon Glass
Hi Stefano,

On Tue, 19 Oct 2021 at 09:39, Stefano Babic  wrote:
>
> Hi Simon,
>
> On 07.10.21 15:43, Simon Glass wrote:
> > Hi Stefano,
> >
> > On Thu, 7 Oct 2021 at 04:37, Stefano Babic  wrote:
> >>
> >> Hi all,
> >>
> >> CI stops by building aarch64 without notice, for reference:
> >>
> >> https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/332319
> >>
> >> There is no error, just process is killed. It looks like it stops at
> >> xilinx_zynqmp_virt,
> >>
> >> ./tools/buildman/buildman -o /tmp -P -E -W aarch64but board can be built
> >> without issues.
> >>
> >> If I build on my host (not in docker, anyway), it generally builds fine
> >> - but it crashes sometimes, too. On gitlab instance , it crashes.
> >> Issue does not seem that depends on merged patches, and introduces
> >> boards were already built successfully. Any hint ? I have also no idea
> >> what I should look as what I see is just
> >>
> >> "usr/bin/bash: line 104:24 Killed
> >> ./tools/buildman/buildman -o /tmp -P -E -W aarch64"
> >
> > I cannot see that link. I am not sure what is going on. Does it say
> > what signal killed it?
>
> Pipelines on our server were not public - I have enbaled now for u-boot-imx.
>
> >
> > Does it sit there for an hour and timeout? If so, then I  did see that
> > myself once recently, when the Kconfig needed stdin, but I could not
> > quitetie it down. I think buildman would provide it, but sometimes
> > not, apparently. So it can happen when there is an existing build
> > there and your new one which adds Kconfig options that don't have
> > defaults, or something like that?
> >
>
> I have investigated further, and I can reproduce it on my host outside
> the gitlab server. buildman causes a OOM, but I cannot find the cause.
>
> Strange enough, this happens with the "aarch64" target, and I cannot
> reproduce it with Tom's master. So it seems that -master is ok, and
> somethin on u-boot-imx generates the OOM.
>
> However
>
> The OOM happens always when -2 (two boards remain) appears. I can see
> with htop that buildman starts to allocate memory until it is exhausted
> (64GB RAM + 8 GB swap). Then the kernel decides that it is enough and
> kills buildman - this is what I see on Ci.
>
> You can see now the pipelines:
>
> https://source.denx.de/u-boot/custodians/u-boot-imx/-/pipelines/9520
>
> I have then split aarch64 and I built imx8 separately - same result. The
> pipeline stops with xilinx board, but they have nothing to do. In fact,
> I can build all xilinx board separately. If I run buildman -W aarch64 -x
> xilinx, OOM is shown by another board.
>
> Strange enough, I can build each single board with buildman without
> issues, neither errors nor warnongs. Just when buildman runs all
> together (aarch64, 308 boards), the OOM is generated.
>
> Bisect does not help: I started bisect, and at the end this commit was
> presented:
>
> commit 53a24dee86fb72ae41e7579607bafe13442616f2
> Author: Fabio Estevam 
> Date:   Mon Aug 23 21:11:09 2021 -0300
>
>  imx8mm-cl-iot-gate: Split the defconfigs
>
>
> But it is a fake: I can revert it, I get the issue again. And the patch
> has nothing to do.
>
> It looks to me it is something in binman, maybe triggered by some
> changes in tree, but all boards can be built separately without issues.
> I supposed to find the cause in code due to applied patches, but because
> each board can be built and no help from bisect, I am quite puzzled. I
> avoid to send a PR to Tom, else I guess the problem goes into -master,
> but I do not know how to proceed, and I have a lot of patches to be applied.
>
> What can be done ?
>
> > If that is it, you can repeat it by clearing out your .bm-work
>
> On gitlab, the build starts from scratch.

Can you check that there is definitely nothing around from the previous build?

>
> > directory then building just that board for one commit, then the next
> > (with the Kconfig change).
>
> I have run buildman for each single board, all of them were successuful.
> With aarch64, I get OOM from buildman.
>
> >
> > Buildman is supposed to handle this, of course. I'm not sure what has 
> > changed.
> >

I still believe this is due to the reason I said, but I'm happy to be
proved wrong.

Regards,
Simon


Re: [PATCH v8 5/8] sandbox: Use a text-based environment

2021-10-19 Thread Simon Glass
Hi Marek,

On Tue, 19 Oct 2021 at 08:32, Marek Behún  wrote:
>
> On Mon, 18 Oct 2021 12:13:19 -0600
> Simon Glass  wrote:
>
> > --- /dev/null
> > +++ b/board/sandbox/sandbox.env
> > @@ -0,0 +1,25 @@
> > +stdin=serial
> > +#ifdef CONFIG_SANDBOX_SDL
> > +stdin+=,cros-ec-keyb,usbkbd
>
> this was CONFIG_KEYBOARD and now is CONFIG_SANDBOX_SDL, is this okay?

The old code is:

#ifdef CONFIG_SANDBOX_SDL
#define LCD_BPP LCD_COLOR16
#define CONFIG_LCD_BMP_RLE8

#define CONFIG_KEYBOARD

#define SANDBOX_SERIAL_SETTINGS "stdin=serial,cros-ec-keyb,usbkbd\0" \
"stdout=serial,vidconsole\0" \
"stderr=serial,vidconsole\0"
#else
#define SANDBOX_SERIAL_SETTINGS "stdin=serial\0" \
"stdout=serial,vidconsole\0" \
"stderr=serial,vidconsole\0"
#endif


so really the check is on CONFIG_SANDBOX_SDL. The setting of
CONFIG_KEYBOARD is not related to the environment.

Regards,
Simon


Re: [PATCH v8 6/8] doc: Mention CONFIG_DEFAULT_ENV_FILE

2021-10-19 Thread Simon Glass
Hi Marek,

On Tue, 19 Oct 2021 at 08:34, Marek Behún  wrote:
>
> On Mon, 18 Oct 2021 12:13:20 -0600
> Simon Glass  wrote:
>
> > Add mention of this option this it does a similar thing to the text
> > environment.
>
> Strange wording.

I say that a lot :-)

https://ludwig.guru/s/add+mention

Regards,
Simon


Re: [PATCH v4] sandbox: Remove OF_HOSTFILE

2021-10-19 Thread Ilias Apalodimas
On Tue, 19 Oct 2021 at 18:36, Tom Rini  wrote:
>
> On Tue, Oct 19, 2021 at 06:30:59PM +0300, Ilias Apalodimas wrote:
> > On Tue, 19 Oct 2021 at 17:17, Tom Rini  wrote:
> > >
> > > On Tue, Oct 19, 2021 at 08:03:07AM -0600, Simon Glass wrote:
> >
> > [...]
> >
> > > >
> > > > For some reason this still does not apply for me on -master. Can you
> > > > please confirm the hash you are using?
> > >
> > > The hunk for scripts/Makefile.spl still fails (it failed on v3 as well),
> > > but is easily fixed-up, fwiw.
> >
> > The reason is that I rebased on top of the prerequisite series, but
> > failed to pull in -master.  As Tom says it's a single line of code
> > that's in conflict.  Shall I send a v5 or will you pull that in Tom?
>
> Well, what should that look like exactly?  It fails to apply cleanly on
> current master.  I had swapped it to CONFIG_SANDBOX, but I gather from
> Simon's comment it should be OF_BOARD?
>

It should be OF_SANDBOX.  I tried to fold SANDBOX to OF_BOARD befire
sending it, but I had some failures I didn't have time to investigate.
So I'll leave that to Simon's further cleanup

Regards
/Ilias
> --
> Tom


Re: [PATCH v4] sandbox: Remove OF_HOSTFILE

2021-10-19 Thread Tom Rini
On Tue, Oct 19, 2021 at 06:30:59PM +0300, Ilias Apalodimas wrote:
> On Tue, 19 Oct 2021 at 17:17, Tom Rini  wrote:
> >
> > On Tue, Oct 19, 2021 at 08:03:07AM -0600, Simon Glass wrote:
> 
> [...]
> 
> > >
> > > For some reason this still does not apply for me on -master. Can you
> > > please confirm the hash you are using?
> >
> > The hunk for scripts/Makefile.spl still fails (it failed on v3 as well),
> > but is easily fixed-up, fwiw.
> 
> The reason is that I rebased on top of the prerequisite series, but
> failed to pull in -master.  As Tom says it's a single line of code
> that's in conflict.  Shall I send a v5 or will you pull that in Tom?

Well, what should that look like exactly?  It fails to apply cleanly on
current master.  I had swapped it to CONFIG_SANDBOX, but I gather from
Simon's comment it should be OF_BOARD?

-- 
Tom


signature.asc
Description: PGP signature


Re: buildman stops (crashed) on current master

2021-10-19 Thread Stefano Babic

Hi Simon,

On 07.10.21 15:43, Simon Glass wrote:

Hi Stefano,

On Thu, 7 Oct 2021 at 04:37, Stefano Babic  wrote:


Hi all,

CI stops by building aarch64 without notice, for reference:

https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/332319

There is no error, just process is killed. It looks like it stops at
xilinx_zynqmp_virt,

./tools/buildman/buildman -o /tmp -P -E -W aarch64but board can be built
without issues.

If I build on my host (not in docker, anyway), it generally builds fine
- but it crashes sometimes, too. On gitlab instance , it crashes.
Issue does not seem that depends on merged patches, and introduces
boards were already built successfully. Any hint ? I have also no idea
what I should look as what I see is just

"usr/bin/bash: line 104:24 Killed
./tools/buildman/buildman -o /tmp -P -E -W aarch64"


I cannot see that link. I am not sure what is going on. Does it say
what signal killed it?


Pipelines on our server were not public - I have enbaled now for u-boot-imx.



Does it sit there for an hour and timeout? If so, then I  did see that
myself once recently, when the Kconfig needed stdin, but I could not
quitetie it down. I think buildman would provide it, but sometimes
not, apparently. So it can happen when there is an existing build
there and your new one which adds Kconfig options that don't have
defaults, or something like that?



I have investigated further, and I can reproduce it on my host outside 
the gitlab server. buildman causes a OOM, but I cannot find the cause.


Strange enough, this happens with the "aarch64" target, and I cannot 
reproduce it with Tom's master. So it seems that -master is ok, and 
somethin on u-boot-imx generates the OOM.


However

The OOM happens always when -2 (two boards remain) appears. I can see 
with htop that buildman starts to allocate memory until it is exhausted 
(64GB RAM + 8 GB swap). Then the kernel decides that it is enough and 
kills buildman - this is what I see on Ci.


You can see now the pipelines:

https://source.denx.de/u-boot/custodians/u-boot-imx/-/pipelines/9520

I have then split aarch64 and I built imx8 separately - same result. The 
pipeline stops with xilinx board, but they have nothing to do. In fact, 
I can build all xilinx board separately. If I run buildman -W aarch64 -x 
xilinx, OOM is shown by another board.


Strange enough, I can build each single board with buildman without 
issues, neither errors nor warnongs. Just when buildman runs all 
together (aarch64, 308 boards), the OOM is generated.


Bisect does not help: I started bisect, and at the end this commit was 
presented:


commit 53a24dee86fb72ae41e7579607bafe13442616f2
Author: Fabio Estevam 
Date:   Mon Aug 23 21:11:09 2021 -0300

imx8mm-cl-iot-gate: Split the defconfigs


But it is a fake: I can revert it, I get the issue again. And the patch 
has nothing to do.


It looks to me it is something in binman, maybe triggered by some 
changes in tree, but all boards can be built separately without issues. 
I supposed to find the cause in code due to applied patches, but because 
each board can be built and no help from bisect, I am quite puzzled. I 
avoid to send a PR to Tom, else I guess the problem goes into -master, 
but I do not know how to proceed, and I have a lot of patches to be applied.


What can be done ?


If that is it, you can repeat it by clearing out your .bm-work


On gitlab, the build starts from scratch.


directory then building just that board for one commit, then the next
(with the Kconfig change).


I have run buildman for each single board, all of them were successuful. 
With aarch64, I get OOM from buildman.




Buildman is supposed to handle this, of course. I'm not sure what has changed.



Regards,
Stefano

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


Re: [PATCH v4] sandbox: Remove OF_HOSTFILE

2021-10-19 Thread Ilias Apalodimas
On Tue, 19 Oct 2021 at 17:17, Tom Rini  wrote:
>
> On Tue, Oct 19, 2021 at 08:03:07AM -0600, Simon Glass wrote:

[...]

> >
> > For some reason this still does not apply for me on -master. Can you
> > please confirm the hash you are using?
>
> The hunk for scripts/Makefile.spl still fails (it failed on v3 as well),
> but is easily fixed-up, fwiw.

The reason is that I rebased on top of the prerequisite series, but
failed to pull in -master.  As Tom says it's a single line of code
that's in conflict.  Shall I send a v5 or will you pull that in Tom?

Thanks
/Ilias
>
> --
> Tom


Re: [PATCH v4] sandbox: Remove OF_HOSTFILE

2021-10-19 Thread Simon Glass
On Tue, 19 Oct 2021 at 08:17, Tom Rini  wrote:
>
> On Tue, Oct 19, 2021 at 08:03:07AM -0600, Simon Glass wrote:
> > Hi Ilias,
> >
> > On Tue, 19 Oct 2021 at 07:07, Ilias Apalodimas
> >  wrote:
> > >
> > > OF_HOSTFILE is used on sandbox configs only.  Although it's pretty
> > > unique and not causing any confusions,  we are better of having simpler
> > > config options for the DTB.
> > >
> > > So let's replace that with the existing OF_BOARD.  U-Boot would then
> > > have only three config options for the DTB origin.
> > > - OF_SEPARATE, build separately from U-Boot
> > > - OF_BOARD, board specific way of providing the DTB
> > > - OF_EMBED embedded in the u-boot binary(should not be used in production
> > >
> > > Signed-off-by: Ilias Apalodimas 
> > > ---
> > > Note that this must be applied on top of
> > > https://lore.kernel.org/u-boot/20211011210016.135929-1-ilias.apalodi...@linaro.org/
> > > changes since v3:
> > > - fix xilinx platforms based on xilinx_zynq_virt_defconfig
> > > changes since v2:
> > > - Rebased on top of the updated OF_BOARD patchset
> > > Changes since v1:
> > > - Added internal error value on board_fdt_blob_setup().  Arguably
> > >   we can just check against NULL and simplify this even more if we
> > >   don't care about the errno
> > > - OF_BOARD is now default for sandbox builds
> > >  Makefile|  6 ++---
> > >  arch/arm/mach-stm32mp/boot_params.c |  3 ++-
> > >  arch/sandbox/cpu/cpu.c  | 27 +
> > >  arch/sandbox/include/asm/u-boot-sandbox.h   |  8 --
> > >  board/AndesTech/ax25-ae350/ax25-ae350.c |  2 ++
> > >  board/Marvell/octeontx/board-fdt.c  |  3 ++-
> > >  board/Marvell/octeontx2/board-fdt.c |  3 ++-
> > >  board/Marvell/octeontx2/board.c |  3 ++-
> > >  board/armltd/vexpress64/vexpress64.c|  7 --
> > >  board/broadcom/bcmstb/bcmstb.c  |  3 ++-
> > >  board/emulation/qemu-arm/qemu-arm.c |  3 ++-
> > >  board/emulation/qemu-ppce500/qemu-ppce500.c |  3 ++-
> > >  board/emulation/qemu-riscv/qemu-riscv.c |  3 ++-
> > >  board/highbank/highbank.c   |  3 ++-
> > >  board/raspberrypi/rpi/rpi.c |  8 --
> > >  board/sifive/unleashed/unleashed.c  |  3 ++-
> > >  board/sifive/unmatched/unmatched.c  |  3 ++-
> > >  board/socrates/socrates.c   |  4 ++-
> > >  board/xen/xenguest_arm64/xenguest_arm64.c   |  7 --
> > >  board/xilinx/common/board.c |  3 ++-
> > >  configs/sandbox64_defconfig |  1 -
> > >  configs/sandbox_defconfig   |  1 -
> > >  configs/sandbox_flattree_defconfig  |  1 -
> > >  configs/sandbox_noinst_defconfig|  1 -
> > >  configs/sandbox_spl_defconfig   |  1 -
> > >  configs/tools-only_defconfig|  1 -
> > >  doc/develop/devicetree/control.rst  |  7 +++---
> > >  dts/Kconfig | 10 +---
> > >  include/fdtdec.h|  4 ++-
> > >  lib/fdtdec.c| 14 +--
> > >  scripts/Makefile.spl|  4 +--
> > >  31 files changed, 81 insertions(+), 69 deletions(-)
> > >
> >
> > For some reason this still does not apply for me on -master. Can you
> > please confirm the hash you are using?
>
> The hunk for scripts/Makefile.spl still fails (it failed on v3 as well),
> but is easily fixed-up, fwiw.

OK I see.

Reviewed-by: Simon Glass 

This still has the CONFIG_SANDBOX in Makefile.spl but my OF_BOARD
series can tidy that up.

Regards,
Simon


Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Simon Glass
Hi Tom,

On Tue, 19 Oct 2021 at 08:25, Tom Rini  wrote:
>
> On Tue, Oct 19, 2021 at 08:11:08AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Tue, 19 Oct 2021 at 08:07, Tom Rini  wrote:
> > >
> > > On Mon, Oct 18, 2021 at 12:13:18PM -0600, Simon Glass wrote:
> > >
> > > > At present U-Boot environment variables, and thus scripts, are defined
> > > > by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text
> > > > to this file and dealing with quoting and newlines is harder than it
> > > > should be. It would be better if we could just type the script into a
> > > > text file and have it included by U-Boot.
> > > >
> > > > Add a feature that brings in a .env file associated with the board
> > > > config, if present. To use it, create a file in a board/
> > > > directory, typically called .env and controlled by the
> > > > CONFIG_ENV_SOURCE_FILE option.
> > > >
> > > > The environment variables should be of the form "var=value". Values can
> > > > extend to multiple lines. See the README under 'Environment Variables:'
> > > > for more information and an example. Note that environment variables may
> > > > not end in + but can start with other strange characters, including
> > > > underscore, comma and slash.
> > > >
> > > > In many cases environment variables need access to the U-Boot CONFIG
> > > > variables to select different options. Enable this so that the 
> > > > environment
> > > > scripts can be as useful as the ones currently in the board config 
> > > > files.
> > > > This uses the C preprocessor, means that comments can be included in the
> > > > environment using /* ... */
> > > >
> > > > Also support += to allow variables to be appended to. This is needed 
> > > > when
> > > > using the preprocessor.
> > > >
> > > > Signed-off-by: Simon Glass 
> > >
> > > As much as I and others appreciate that you've written the parser here
> > > in a classic UNIX tool, awk, since a lot of the problems also seem to
> > > stem from having the parser be able to handle previously valid
> > > environment variables, if this was written in Python say, would we have
> > > this problem?
> >
> > Well ideally I'd like to avoid Python in this case as it is in the
> > compilation path. I am not sure yet what Wolfgang actually wants,
> > apart from variable names ending with + which I would like to
> > disallow.
> >
> > So if we can clearly understand the goal, then we might be able to do
> > it in awk, but, again, can we just disallow '+' in var names ?
>
> If we say that everything that's valid in the environment today needs to
> continue to be valid, so that includes '+' and only disallowing '=' and
> NUL as Wolfgang has said, can you update the awk parser to handle it?

But how do we handle this?

var+=fred

Is this appending to var or assigning to var+  ?

var++=fred

is unambiguous but very confusing. I think it would be better to disallow +

We can allow it in the middle of the var name if you like.

Regards,
Simon


Re: [PATCH] part: return -ENOSYS when get_info not valid.

2021-10-19 Thread Heinrich Schuchardt

On 10/19/21 15:35, zhaohui.shi wrote:

From: schspa 

In some case, get_info() interface can be NULL, add this check to stop
from crash.


Thank you for reviewing the partition driver code.

There seems to be no driver missing a get_info implementation. Where did
you run into a problem?

Why should we only check .get_info and not .test and not .print?



Signed-off-by: schspa 


Please, provide a name.


---
  disk/part.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/disk/part.c b/disk/part.c
index a6a8f7052b..7af3240ec7 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -668,6 +668,13 @@ int part_get_info_by_name_type(struct blk_desc *dev_desc, 
const char *name,
part_drv = part_driver_lookup_type(dev_desc);
if (!part_drv)
return -1;
+
+   if (!part_drv->get_info) {
+   PRINTF("## Driver %s does not have the get_info() method\n",
+  part_drv->name);


Please, use log_debug() to avoid noise on the console on every boot.

Best regards

Heinrich


+   return -ENOSYS;
+   }
+
for (i = 1; i < part_drv->max_entries; i++) {
ret = part_drv->get_info(dev_desc, i, info);
if (ret != 0) {



Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Marek Behún
On Mon, 18 Oct 2021 12:13:18 -0600
Simon Glass  wrote:

> At present U-Boot environment variables, and thus scripts, are defined
> by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text
> to this file and dealing with quoting and newlines is harder than it
> should be. It would be better if we could just type the script into a
> text file and have it included by U-Boot.
> 
> Add a feature that brings in a .env file associated with the board
> config, if present. To use it, create a file in a board/
> directory, typically called .env and controlled by the
> CONFIG_ENV_SOURCE_FILE option.
> 
> The environment variables should be of the form "var=value". Values can
> extend to multiple lines. See the README under 'Environment Variables:'
> for more information and an example. Note that environment variables may
> not end in + but can start with other strange characters, including
> underscore, comma and slash.
> 
> In many cases environment variables need access to the U-Boot CONFIG
> variables to select different options. Enable this so that the environment
> scripts can be as useful as the ones currently in the board config files.
> This uses the C preprocessor, means that comments can be included in the
> environment using /* ... */
> 
> Also support += to allow variables to be appended to. This is needed when
> using the preprocessor.
> 
> Signed-off-by: Simon Glass 

Reviewed-by: Marek Behún 
Tested-by: Marek Behún 

Tested on Turris Omnia, but we use distroboot, so will convert only
after distroboot is supported.


[PATCH v2] mmc: arm_pl180_mmci: Enable HWFC for specific versions of MCI

2021-10-19 Thread Usama Arif
There are 4 registers (PERIPHID{0-3}) that contain the ID of MCI.
For MMCs' with peripheral id 0x02041180 and 0x03041180, H/W flow control
needs to be enabled for multi block writes (MMC CMD 18).

Signed-off-by: Usama Arif 
---
 drivers/mmc/arm_pl180_mmci.c | 14 ++
 drivers/mmc/arm_pl180_mmci.h |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
index f99b5f997e..9c5d48e90c 100644
--- a/drivers/mmc/arm_pl180_mmci.c
+++ b/drivers/mmc/arm_pl180_mmci.c
@@ -282,6 +282,14 @@ static int host_request(struct mmc *dev,
return result;
 }
 
+static int check_peripheral_id(struct pl180_mmc_host *host, u32 periph_id)
+{
+   return readl(>base->periph_id0) == (periph_id & 0xFF) &&
+   readl(>base->periph_id1) == ((periph_id >> 8) & 0xFF)  &&
+   readl(>base->periph_id2) == ((periph_id >> 16) & 0xFF) &&
+   readl(>base->periph_id3) == ((periph_id >> 24) & 0xFF);
+}
+
 static int  host_set_ios(struct mmc *dev)
 {
struct pl180_mmc_host *host = dev->priv;
@@ -337,6 +345,12 @@ static int  host_set_ios(struct mmc *dev)
sdi_clkcr &= ~(SDI_CLKCR_WIDBUS_MASK);
sdi_clkcr |= buswidth;
}
+   /* For MMCs' with peripheral id 0x02041180 and 0x03041180, H/W flow 
control
+* needs to be enabled for multi block writes (MMC CMD 18).
+*/
+   if (check_peripheral_id(host, 0x02041180) ||
+   check_peripheral_id(host, 0x03041180))
+   sdi_clkcr |= SDI_CLKCR_HWFCEN;
 
writel(sdi_clkcr, >base->clock);
udelay(CLK_CHANGE_DELAY);
diff --git a/drivers/mmc/arm_pl180_mmci.h b/drivers/mmc/arm_pl180_mmci.h
index 15c29beadb..fca15910a8 100644
--- a/drivers/mmc/arm_pl180_mmci.h
+++ b/drivers/mmc/arm_pl180_mmci.h
@@ -43,6 +43,7 @@
 #define SDI_CLKCR_CLKEN0x0100
 #define SDI_CLKCR_PWRSAV   0x0200
 #define SDI_CLKCR_BYPASS   0x0400
+#define SDI_CLKCR_HWFCEN   0x1000
 #define SDI_CLKCR_WIDBUS_MASK  0x1800
 #define SDI_CLKCR_WIDBUS_1 0x
 #define SDI_CLKCR_WIDBUS_4 0x0800
-- 
2.17.1



Re: [PATCH v3 1/4] tools: Separate image types which depend on OpenSSL

2021-10-19 Thread Andre Przywara
On Tue, 19 Oct 2021 08:28:44 -0500
Samuel Holland  wrote:

Hi Samuel,

> On 10/19/21 5:41 AM, Andre Przywara wrote:
> > On Mon, 18 Oct 2021 09:09:04 -0500
> > "Alex G."  wrote:
> > 
> > Hi,
> >   
> >> On 10/14/21 10:19 PM, Samuel Holland wrote:  
> >>> Some image types (kwbimage and mxsimage) always depend on OpenSSL, so
> >>> they can only be included in mkimage when TOOLS_LIBCRYPTO is selected.
> >>> Use Makefile logic to conditionally link the files.
> >>>
> >>> When building for platforms which use those image types, automatically
> >>> select TOOLS_LIBCRYPTO, since it is required for the build to complete.
> >>>
> >>> Signed-off-by: Samuel Holland 
> >>
> >> NAK.
> >>
> >> The intent, as detailed in tools/Makefile, is to _NOT_ to conflate 
> >> target options with tools options.  
> > 
> > I am a bit undecided, because I think the intent was more for *just*
> > building mkimage (tools-only_defconfig, for the u-boot-tools distro
> > package, for instance). (Which doesn't seem to work, btw, with or without
> > this patch.)  
> 
> TOOLS_LIBCRYPTO=n works for me with this patch, and I just
> double-checked and verified that mkimage is compiled/linked without OpenSSL.

For tools-only_defconfig and "make tools"?
I see that the SSL linking errors vanish, but I still get this instead:
$ make -s tools
/usr/bin/ld: tools/image-host.o: in function `fit_image_setup_sig':
image-host.c:(.text+0xec): undefined reference to `image_get_checksum_algo'
/usr/bin/ld: image-host.c:(.text+0xfc): undefined reference to 
`image_get_crypto_algo'
/usr/bin/ld: image-host.c:(.text+0x108): undefined reference to 
`image_get_padding_algo'
/usr/bin/ld: tools/image-host.o: in function `fit_config_add_verification_data':
image-host.c:(.text+0x7b0): undefined reference to `fit_region_make_list'
/usr/bin/ld: tools/image-host.o: in function `fit_check_sign':
image-host.c:(.text+0x1548): undefined reference to `fit_config_verify'
/usr/bin/ld: tools/common/image-fit.o: in function `fit_image_verify_with_data':
image-fit.c:(.text+0x17c0): undefined reference to 
`fit_image_verify_required_sigs'
/usr/bin/ld: image-fit.c:(.text+0x19e0): undefined reference to 
`fit_image_check_sig'
/usr/bin/ld: tools/common/image-fit.o: in function `fit_image_load':
image-fit.c:(.text+0x27ec): undefined reference to `fit_config_verify'
collect2: error: ld returned 1 exit status
make[1]: *** [scripts/Makefile.host:104: tools/dumpimage] Error 1
make: *** [Makefile:1800: tools] Error 2
(On Ubuntu 20.04 arm64)

> 
> > However just building mkimage because it's needed to create a certain
> > board firmware is a different story, I think, and including OpenSSL (if
> > the platform requires that) is hardly a user's choice at this point.
> > 
> > But anyway: Samuel, what is the actual problem this patch is solving?  
> 
> The actual problem is that TOOLS_LIBCRYPTO=n is broken, and would be
> further broken by adding sunxi_toc0.o to dumpimage-mkimage-objs. Fixing
> that requires moving objects that depend on OpenSSL to LIBCRYPTO_OBJS,
> and adding sunxi_toc0.o there in patch 2.

Right, I was just confused because it is still broken for me (see above).

> > TOOLS_LIBCRYPTO is default y, so normally (make foo_defconfig; make)
> > everything should be fine? And it only breaks if a user deliberately and
> > manually deselects it, between "make foo_defconfig" and "make"?
> > 
> > So this patch is somewhat optional, at least for the purpose of TOC0
> > support?  
> 
> The Makefile changes are needed for TOC0 support. The Kconfig changes
> are not.

Right, I see.

> And I think the only controversial part of this patch is the
> "select TOOLS_LIBCRYPTO" lines. So I suggest omitting all of the Kconfig
> changes from this patch (and removing those lines from the commit
> message). I can send v4 or you can fix it up.

You should probably send a v4, so that we get Alex' OK for that.
I need to test on actual non-secure hardware tonight, to verify that it
still works (which I strongly assume now, from looking at the code). Then
I would be eager to merge it, since it booted my Remix Mini PC beautifully.

Cheers,
Andre

> >   
> >> Disabling openssl libs is purely at the user's discretion. If platforms 
> >> can't build a usable image, I suggest just printing a loud warning 
> >> instead of overriding the user.
> >>
> >> Alex
> >>  
> >>> ---
> >>>
> >>> Changes in v3:
> >>>   - Selected TOOLS_LIBCRYPTO on all platforms that use kwbimage (as best
> >>> as I can tell, using the suggestions from Pali Rohár)
> >>>
> >>> Changes in v2:
> >>>   - Refactored the first patch on top of TOOLS_LIBCRYPTO
> >>>
> >>>   arch/arm/Kconfig  |  3 +++
> >>>   arch/arm/mach-imx/mxs/Kconfig |  2 ++
> >>>   scripts/config_whitelist.txt  |  1 -
> >>>   tools/Makefile| 19 +--
> >>>   tools/mxsimage.c  |  3 ---
> >>>   5 files changed, 10 insertions(+), 18 deletions(-)
> >>>
> >>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >>> index 

Re: [PATCH v8 8/8] bootm: Tidy up use of autostart env var

2021-10-19 Thread Marek Behún
On Mon, 18 Oct 2021 12:13:22 -0600
Simon Glass  wrote:

> +bool env_get_autostart(void)
> +{
> + const char *val = env_get("autostart");
> +
> + return val && !strcmp(val, "yes");
> +}

Why not use env_get_yesno() ?


Re: [PATCH v8 7/8] doc: Improve environment documentation

2021-10-19 Thread Marek Behún
On Mon, 18 Oct 2021 12:13:21 -0600
Simon Glass  wrote:

> Make various updates suggested during review of the rST conversion.
> 
> Signed-off-by: Simon Glass 
> Suggested-by: Wolfgang Denk 

Reviewed-by: Marek Behún 


Re: [PATCH v8 6/8] doc: Mention CONFIG_DEFAULT_ENV_FILE

2021-10-19 Thread Marek Behún
On Mon, 18 Oct 2021 12:13:20 -0600
Simon Glass  wrote:

> Add mention of this option this it does a similar thing to the text
> environment.

Strange wording.

> Suggested-by: Rasmus Villemoes 
> 
> Signed-off-by: Simon Glass 

Extra newline.

Other than that

Reviewed-by: Marek Behún 


Re: [PATCH v8 5/8] sandbox: Use a text-based environment

2021-10-19 Thread Marek Behún
On Mon, 18 Oct 2021 12:13:19 -0600
Simon Glass  wrote:

> --- /dev/null
> +++ b/board/sandbox/sandbox.env
> @@ -0,0 +1,25 @@
> +stdin=serial
> +#ifdef CONFIG_SANDBOX_SDL
> +stdin+=,cros-ec-keyb,usbkbd

this was CONFIG_KEYBOARD and now is CONFIG_SANDBOX_SDL, is this okay?


Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Tom Rini
On Tue, Oct 19, 2021 at 08:11:08AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 19 Oct 2021 at 08:07, Tom Rini  wrote:
> >
> > On Mon, Oct 18, 2021 at 12:13:18PM -0600, Simon Glass wrote:
> >
> > > At present U-Boot environment variables, and thus scripts, are defined
> > > by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text
> > > to this file and dealing with quoting and newlines is harder than it
> > > should be. It would be better if we could just type the script into a
> > > text file and have it included by U-Boot.
> > >
> > > Add a feature that brings in a .env file associated with the board
> > > config, if present. To use it, create a file in a board/
> > > directory, typically called .env and controlled by the
> > > CONFIG_ENV_SOURCE_FILE option.
> > >
> > > The environment variables should be of the form "var=value". Values can
> > > extend to multiple lines. See the README under 'Environment Variables:'
> > > for more information and an example. Note that environment variables may
> > > not end in + but can start with other strange characters, including
> > > underscore, comma and slash.
> > >
> > > In many cases environment variables need access to the U-Boot CONFIG
> > > variables to select different options. Enable this so that the environment
> > > scripts can be as useful as the ones currently in the board config files.
> > > This uses the C preprocessor, means that comments can be included in the
> > > environment using /* ... */
> > >
> > > Also support += to allow variables to be appended to. This is needed when
> > > using the preprocessor.
> > >
> > > Signed-off-by: Simon Glass 
> >
> > As much as I and others appreciate that you've written the parser here
> > in a classic UNIX tool, awk, since a lot of the problems also seem to
> > stem from having the parser be able to handle previously valid
> > environment variables, if this was written in Python say, would we have
> > this problem?
> 
> Well ideally I'd like to avoid Python in this case as it is in the
> compilation path. I am not sure yet what Wolfgang actually wants,
> apart from variable names ending with + which I would like to
> disallow.
> 
> So if we can clearly understand the goal, then we might be able to do
> it in awk, but, again, can we just disallow '+' in var names ?

If we say that everything that's valid in the environment today needs to
continue to be valid, so that includes '+' and only disallowing '=' and
NUL as Wolfgang has said, can you update the awk parser to handle it?

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v8 3/8] doc: Move environment documentation to rST

2021-10-19 Thread Marek Behún
On Mon, 18 Oct 2021 12:13:17 -0600
Simon Glass  wrote:

> Move this from the README to rST format.
> 
> Drop i2cfast since it is obviously obsolete and breaks the formatting.
> Other changes and improvements are in a following patch.
> 
> Signed-off-by: Simon Glass 

Acked-by: Marek Behún 


Re: [PATCH v8 2/8] sandbox: Drop distro_boot

2021-10-19 Thread Marek Behún
On Mon, 18 Oct 2021 12:13:16 -0600
Simon Glass  wrote:

> This is a complicated set of #defines and it is painful to convert to a
> text file. We can (once pending patches are applied) provide the same
> functionality with bootmethod. Drop this for sandbox to allow conversion
> to a text-file environment.
> 
> Signed-off-by: Simon Glass 

Reviewed-by: Marek Behún 


Re: [PATCH v8 1/8] binman: Allow timeout to occur in the image or its section

2021-10-19 Thread Marek Behún
On Mon, 18 Oct 2021 12:13:15 -0600
Simon Glass  wrote:

> At present testThreadTimeout() assumes that the expected timeout happens
> first when building the section, but it can just as easily happen at the
> top-level image. Update the test to cope with both.
> 
> Signed-off-by: Simon Glass 

Reviewed-by: Marek Behún 


Re: [PATCH v4] sandbox: Remove OF_HOSTFILE

2021-10-19 Thread Tom Rini
On Tue, Oct 19, 2021 at 08:03:07AM -0600, Simon Glass wrote:
> Hi Ilias,
> 
> On Tue, 19 Oct 2021 at 07:07, Ilias Apalodimas
>  wrote:
> >
> > OF_HOSTFILE is used on sandbox configs only.  Although it's pretty
> > unique and not causing any confusions,  we are better of having simpler
> > config options for the DTB.
> >
> > So let's replace that with the existing OF_BOARD.  U-Boot would then
> > have only three config options for the DTB origin.
> > - OF_SEPARATE, build separately from U-Boot
> > - OF_BOARD, board specific way of providing the DTB
> > - OF_EMBED embedded in the u-boot binary(should not be used in production
> >
> > Signed-off-by: Ilias Apalodimas 
> > ---
> > Note that this must be applied on top of
> > https://lore.kernel.org/u-boot/20211011210016.135929-1-ilias.apalodi...@linaro.org/
> > changes since v3:
> > - fix xilinx platforms based on xilinx_zynq_virt_defconfig
> > changes since v2:
> > - Rebased on top of the updated OF_BOARD patchset
> > Changes since v1:
> > - Added internal error value on board_fdt_blob_setup().  Arguably
> >   we can just check against NULL and simplify this even more if we
> >   don't care about the errno
> > - OF_BOARD is now default for sandbox builds
> >  Makefile|  6 ++---
> >  arch/arm/mach-stm32mp/boot_params.c |  3 ++-
> >  arch/sandbox/cpu/cpu.c  | 27 +
> >  arch/sandbox/include/asm/u-boot-sandbox.h   |  8 --
> >  board/AndesTech/ax25-ae350/ax25-ae350.c |  2 ++
> >  board/Marvell/octeontx/board-fdt.c  |  3 ++-
> >  board/Marvell/octeontx2/board-fdt.c |  3 ++-
> >  board/Marvell/octeontx2/board.c |  3 ++-
> >  board/armltd/vexpress64/vexpress64.c|  7 --
> >  board/broadcom/bcmstb/bcmstb.c  |  3 ++-
> >  board/emulation/qemu-arm/qemu-arm.c |  3 ++-
> >  board/emulation/qemu-ppce500/qemu-ppce500.c |  3 ++-
> >  board/emulation/qemu-riscv/qemu-riscv.c |  3 ++-
> >  board/highbank/highbank.c   |  3 ++-
> >  board/raspberrypi/rpi/rpi.c |  8 --
> >  board/sifive/unleashed/unleashed.c  |  3 ++-
> >  board/sifive/unmatched/unmatched.c  |  3 ++-
> >  board/socrates/socrates.c   |  4 ++-
> >  board/xen/xenguest_arm64/xenguest_arm64.c   |  7 --
> >  board/xilinx/common/board.c |  3 ++-
> >  configs/sandbox64_defconfig |  1 -
> >  configs/sandbox_defconfig   |  1 -
> >  configs/sandbox_flattree_defconfig  |  1 -
> >  configs/sandbox_noinst_defconfig|  1 -
> >  configs/sandbox_spl_defconfig   |  1 -
> >  configs/tools-only_defconfig|  1 -
> >  doc/develop/devicetree/control.rst  |  7 +++---
> >  dts/Kconfig | 10 +---
> >  include/fdtdec.h|  4 ++-
> >  lib/fdtdec.c| 14 +--
> >  scripts/Makefile.spl|  4 +--
> >  31 files changed, 81 insertions(+), 69 deletions(-)
> >
> 
> For some reason this still does not apply for me on -master. Can you
> please confirm the hash you are using?

The hunk for scripts/Makefile.spl still fails (it failed on v3 as well),
but is easily fixed-up, fwiw.

-- 
Tom


signature.asc
Description: PGP signature


[PATCH] spi: zynqmp_gqspi: Fix write issue at low frequencies

2021-10-19 Thread Ashok Reddy Soma
With current implementation we are seeing write issues at low frequencies
below 15Mhz. Make below changes to fix the issue.

1. Remove dummy genfifo entry in zynqmp_qspi_chipselect() which was
   incorrectly added in the past

2. Enable and poll for TX_FIFO_Empty after Tx data is filled in FIFO in
   zynqmp_qspi_fill_tx_fifo().

Signed-off-by: Ashok Reddy Soma 
---

 drivers/spi/zynqmp_gqspi.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/zynqmp_gqspi.c b/drivers/spi/zynqmp_gqspi.c
index 2db4ae20f1..c772bae3cc 100644
--- a/drivers/spi/zynqmp_gqspi.c
+++ b/drivers/spi/zynqmp_gqspi.c
@@ -37,6 +37,7 @@
  */
 #define GQSPI_IXR_TXNFULL_MASK 0x0004 /* QSPI TX FIFO Overflow */
 #define GQSPI_IXR_TXFULL_MASK  0x0008 /* QSPI TX FIFO is full */
+#define GQSPI_IXR_TXFIFOEMPTY_MASK 0x0100 /* QSPI TX FIFO is Empty */
 #define GQSPI_IXR_RXNEMTY_MASK 0x0010 /* QSPI RX FIFO Not Empty */
 #define GQSPI_IXR_GFEMTY_MASK  0x0080 /* QSPI Generic FIFO Empty */
 #define GQSPI_IXR_GFNFULL_MASK 0x0200 /* QSPI GENFIFO not full */
@@ -279,9 +280,6 @@ static void zynqmp_qspi_chipselect(struct zynqmp_qspi_priv 
*priv, int is_on)
 
debug("GFIFO_CMD_CS: 0x%x\n", gqspi_fifo_reg);
 
-   /* Dummy generic FIFO entry */
-   zynqmp_qspi_fill_gen_fifo(priv, 0);
-
zynqmp_qspi_fill_gen_fifo(priv, gqspi_fifo_reg);
 }
 
@@ -470,6 +468,13 @@ static int zynqmp_qspi_fill_tx_fifo(struct 
zynqmp_qspi_priv *priv, u32 size)
}
}
 
+   ret = wait_for_bit_le32(>isr, GQSPI_IXR_TXFIFOEMPTY_MASK, 1,
+   GQSPI_TIMEOUT, 1);
+   if (ret) {
+   printf("%s: Timeout\n", __func__);
+   return ret;
+   }
+
priv->tx_buf += len;
return 0;
 }
-- 
2.17.1



Re: [PATCH v3 12/13] env: Simplify env_match() and inline into env_get_f()

2021-10-19 Thread Simon Glass
Hi Marek,

On Mon, 18 Oct 2021 at 18:50, Marek Behún  wrote:
>
> > Reviewed-by: Simon Glass 
>
> Simon, thanks. Will you be taking this through your tree or should I
> change delegate to Tom?

I see it in my queue so I'll pick it up.

Regards,
Simon


Re: [PATCH v3 1/4] mkimage: add a flag to describe whether -A is specified

2021-10-19 Thread Simon Glass
Hi,

On Tue, 19 Oct 2021 at 07:17, Samuel Holland  wrote:
>
> On 10/19/21 5:49 AM, Andre Przywara wrote:
> > On Thu, 14 Oct 2021 20:53:04 -0500
> > Samuel Holland  wrote:
> >
> > Hi,
> >
> >> From: Icenowy Zheng 
> >>
> >> The sunxi_egon type used to take no -A argument (because we assume sunxi
> >> targets are all ARM). However, as Allwinner D1 appears as the first
> >> RISC-V sunxi target, we need to support -A; in addition, as external
> >> projects rely on U-Boot mkimage to generate sunxi eGON.BT0 header, we
> >> need to keep compatibility with command line without -A.
> >>
> >> As the default value of arch in mkimage is not proper (IH_ARCH_PPC
> >> instead of IH_ARCH_INVALID), to keep more compatibility, add an Aflag
> >> field to image parameters to describe whether an architecture is
> >> explicitly specified.
> >
> > So there was some nitpic^Wdiscussion about the uppercase in the variable
> > name, where Icenowy proposed arch_flag instead. Shall this be changed
> > still? If that's the only thing, I am happy to do it while merging.
>
> My understanding of the discussion was that Aflag made the most sense
> for consistency, and it was acceptable for that reason, which is why I
> left it unchanged in v3.

That might have been my suggestion of naming the variable after its
meaning rather than the single-character flag.

But I think Tom is happy with it as is, so let's go with it.


- Simon

>
> Regards,
> Samuel
>
> > Btw: Shall this (and the TOC0) series go through the sunxi tree?
> >
> > Cheers,
> > Andre
> >
> >
> >>
> >> Reviewed-by: Tom Rini 
> >> Signed-off-by: Icenowy Zheng 
> >> Signed-off-by: Samuel Holland 
> >> ---
> >>
> >> (no changes since v1)
> >>
> >>  tools/imagetool.h | 1 +
> >>  tools/mkimage.c   | 1 +
> >>  2 files changed, 2 insertions(+)
> >>
> >> diff --git a/tools/imagetool.h b/tools/imagetool.h
> >> index e229a34ffc..5dc28312c2 100644
> >> --- a/tools/imagetool.h
> >> +++ b/tools/imagetool.h
> >> @@ -51,6 +51,7 @@ struct image_tool_params {
> >>  int pflag;
> >>  int vflag;
> >>  int xflag;
> >> +int Aflag;
> >>  int skipcpy;
> >>  int os;
> >>  int arch;
> >> diff --git a/tools/mkimage.c b/tools/mkimage.c
> >> index fbe883ce36..cfd97b046c 100644
> >> --- a/tools/mkimage.c
> >> +++ b/tools/mkimage.c
> >> @@ -171,6 +171,7 @@ static void process_args(int argc, char **argv)
> >>  show_valid_options(IH_ARCH);
> >>  usage("Invalid architecture");
> >>  }
> >> +params.Aflag = 1;
> >>  break;
> >>  case 'b':
> >>  if (add_content(IH_TYPE_FLATDT, optarg)) {
> >
>


Re: [PATCH v6 4/7] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Simon Glass
Hi Wolfgang,

On Tue, 19 Oct 2021 at 04:46, Wolfgang Denk  wrote:
>
> Dear Simon,
>
> In message 
>  you 
> wrote:
> >
> > Can we just ban + ?
>
> Why?  Just because your awk script is a lousy parser?
>
> > In the above, foo\+ gives an unknown escape sequence from the C
> > preprocessor, then the whole line is ignored by the script
>
> Really?
>
> -> cat foo.c
> #define A ampersand
> #define B berta
>
> int foo = A \+ B ;
> -> gcc -E foo.c
> # 0 "foo.c"
> # 0 ""
> # 0 ""
> # 1 "/usr/include/stdc-predef.h" 1 3 4
> # 0 "" 2
> # 1 "foo.c"
>
>
>
> int foo = ampersand \+ berta ;
>
> I do not see any such error message?

I thought you were trying to use + at the end of a variable.

I used:

fred\+=aaa

and got

cc1: warning: unknown escape sequence: '\=''

You can try it yourself by editing sandbox.env in the
u-boot-dm/env-working tree.

>
>
> > foo+ = bar   produes a variable called "foo+ " in the environment with
> > the value " bar" so you probably don't want that.
>
> This is only because your "parser" is very primitive and sensitive
> even to minimal white space changes.

Agreed.

>
> > My original motivation was the complexity of getting the env you want
> > using #define
> >
> > My current motivation is to complete the CONFIG migration, now in its 8th 
> > year.
>
> I fully understand your motivation and appreciate your efforts.

OK, so what exactly is needed here? I can do the above tweaks and the
cover letter, but what else?

Regards,
Simon


Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Simon Glass
Hi Tom,

On Tue, 19 Oct 2021 at 08:07, Tom Rini  wrote:
>
> On Mon, Oct 18, 2021 at 12:13:18PM -0600, Simon Glass wrote:
>
> > At present U-Boot environment variables, and thus scripts, are defined
> > by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text
> > to this file and dealing with quoting and newlines is harder than it
> > should be. It would be better if we could just type the script into a
> > text file and have it included by U-Boot.
> >
> > Add a feature that brings in a .env file associated with the board
> > config, if present. To use it, create a file in a board/
> > directory, typically called .env and controlled by the
> > CONFIG_ENV_SOURCE_FILE option.
> >
> > The environment variables should be of the form "var=value". Values can
> > extend to multiple lines. See the README under 'Environment Variables:'
> > for more information and an example. Note that environment variables may
> > not end in + but can start with other strange characters, including
> > underscore, comma and slash.
> >
> > In many cases environment variables need access to the U-Boot CONFIG
> > variables to select different options. Enable this so that the environment
> > scripts can be as useful as the ones currently in the board config files.
> > This uses the C preprocessor, means that comments can be included in the
> > environment using /* ... */
> >
> > Also support += to allow variables to be appended to. This is needed when
> > using the preprocessor.
> >
> > Signed-off-by: Simon Glass 
>
> As much as I and others appreciate that you've written the parser here
> in a classic UNIX tool, awk, since a lot of the problems also seem to
> stem from having the parser be able to handle previously valid
> environment variables, if this was written in Python say, would we have
> this problem?

Well ideally I'd like to avoid Python in this case as it is in the
compilation path. I am not sure yet what Wolfgang actually wants,
apart from variable names ending with + which I would like to
disallow.

So if we can clearly understand the goal, then we might be able to do
it in awk, but, again, can we just disallow '+' in var names ?

Regards,
Simon


Re: [PATCH v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

2021-10-19 Thread Tom Rini
On Mon, Oct 18, 2021 at 12:13:18PM -0600, Simon Glass wrote:

> At present U-Boot environment variables, and thus scripts, are defined
> by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text
> to this file and dealing with quoting and newlines is harder than it
> should be. It would be better if we could just type the script into a
> text file and have it included by U-Boot.
> 
> Add a feature that brings in a .env file associated with the board
> config, if present. To use it, create a file in a board/
> directory, typically called .env and controlled by the
> CONFIG_ENV_SOURCE_FILE option.
> 
> The environment variables should be of the form "var=value". Values can
> extend to multiple lines. See the README under 'Environment Variables:'
> for more information and an example. Note that environment variables may
> not end in + but can start with other strange characters, including
> underscore, comma and slash.
> 
> In many cases environment variables need access to the U-Boot CONFIG
> variables to select different options. Enable this so that the environment
> scripts can be as useful as the ones currently in the board config files.
> This uses the C preprocessor, means that comments can be included in the
> environment using /* ... */
> 
> Also support += to allow variables to be appended to. This is needed when
> using the preprocessor.
> 
> Signed-off-by: Simon Glass 

As much as I and others appreciate that you've written the parser here
in a classic UNIX tool, awk, since a lot of the problems also seem to
stem from having the parser be able to handle previously valid
environment variables, if this was written in Python say, would we have
this problem?

-- 
Tom


signature.asc
Description: PGP signature


  1   2   >