date:20211018

On Mon, Oct 18, 2021 at 04:50:33PM -0400, Tom Rini wrote:
> On Fri, Oct 15, 2021 at 11:42:40AM +0300, Ilias Apalodimas wrote:
> 
> > OF_HOSTFILE is used on sandbox configs only.  Although it's pretty
> > unique and rarely causes confusion,  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 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
> 
> With the previous series applied, this breaks xilinx_zynq_virt:
> https://source.denx.de/u-boot/u-boot/-/jobs/337428 
> 
> I had a little trouble in general getting the board to run locally, even
> without the patches, so I didn't debug further yet.

Here's another thing, that might be related.  The dependency series has
minor size changes, in basically expected platforms and places.  This
patch adds tons of size changes on virtually all platforms.  That might
provide a clue or two as to what's going on.

-- 
Tom


signature.asc
Description: PGP signature

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

2021-10-18 Thread Jaehoon Chung

Hi,

+Cc Peng,

On 10/18/21 8:33 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 
> Change-Id: Icc79e352db6305c232527c4a432f17b7a30fec8e

Remove Change-ID.

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: [PATCH 2/2] dt-bindings: u-boot: Add an initial binding for config

2021-10-18 Thread Rob Herring

On Wed, Oct 13, 2021 at 11:33 AM Simon Glass  wrote:
>
> "
> Hi Rob,
>
> On Tue, 12 Oct 2021 at 09:05, Rob Herring  wrote:
> >
> >  On Tue, Oct 12, 2021 at 8:41 AM Simon Glass  wrote:
> > >
> > > Hi Rob,
> > >
> > > On Mon, 4 Oct 2021 at 13:30, Rob Herring  wrote:
> > > >
> > > > On Sun, Oct 03, 2021 at 12:51:53PM -0600, Simon Glass wrote:
> > > > > U-Boot makes use of the devicetree for its driver model. Devices are 
> > > > > bound
> > > > > based on the hardware description in the devicetree.
> > > > >
> > > > > Since U-Boot is not an operating system, it has no command line or 
> > > > > user
> > > > > space to provide configuration and policy information. This must be 
> > > > > made
> > > > > available in some other way.
> > > > >
> > > > > Therefore U-Boot uses devicetree for configuration and run-time 
> > > > > control
> > > > > and has done for approximately 9 years. This works extremely well in 
> > > > > the
> > > > > project and is very flexible. However the bindings have never been
> > > > > incorporated in the devicetree bindings in the Linux tree. This could 
> > > > > be
> > > > > a good time to start this work as we try to create standard bindings 
> > > > > for
> > > > > communicating between firmware components.
> > > > >
> > > > > Add an initial binding for this node, covering just the config node, 
> > > > > which
> > > > > is the main requirement. It is similar in concept to the chosen node, 
> > > > > but
> > > > > used for passing information between firmware components, instead of 
> > > > > from
> > > > > firmware to operating system.
> > > > >
> > > > > Signed-off-by: Simon Glass 
> > > > > ---
> > > > > Please be kind in your review. Some words about why this is needed are
> > > > > included in the description in config.yaml file.
> > > > >
> > > > > The last attempt to add just one property needed by U-Boot went into 
> > > > > the
> > > > > weeds 6 years ago, with what I see as confusion about the role of the
> > > > > chosen node in devicetree[1].
> > > > >
> > > > > I am trying again in the hope of reaching resolution rather than just
> > > > > going around in circles with the 'devicetree is a hardware 
> > > > > description'
> > > > > argument :-)
> > > > >
> > > > > Quoting from the introduction to latest devicetree spec[2]:
> > > > >
> > > > > >>>
> > > > > To initialize and boot a computer system, various software components
> > > > > interact. Firmware might perform low-level initialization of the 
> > > > > system
> > > > > hardware before passing control to software such as an operating 
> > > > > system,
> > > > > bootloader, or  hypervisor. Bootloaders and hypervisors can, in turn,
> > > > > load and transfer control to operating systems. Standard, consistent
> > > > > interfaces and conventions facilitate the interactions between these
> > > > > software components. In this document the term boot program is used to
> > > > > generically refer to a software component that initializes the system
> > > > > state and executes another software component referred to as a client
> > > > > program.
> > > > > <<<
> > > > >
> > > > > This clearly envisages multiple software components in the firmware
> > > > > domain and in fact that is the case today. They need some way to
> > > > > communicate configuration data such as memory setup, runtime-feature
> > > > > selection and developer conveniences. Devicetree seems ideal, at 
> > > > > least for
> > > > > components where the performance / memory requirements of devicetree 
> > > > > are
> > > > > affordable.
> > > > >
> > > > > I hope that the Linux community (which owns the devicetree bindings) 
> > > > > finds
> > > > > this initiative valuable and acceptable.
> > > >
> > > > Owns? I'm having a sale and can make you a good offer. Buy 1 binding,
> > > > get 2000 free. :)
> > >
> > > Yes, it's the price of that first binding that surely puts everyone off.
> > >
> > > (sorry for sitting on this for a week, my spam filter doesn't like
> > > some mailing lists and I'm working on it)
> > >
> > > >
> > > > >
> > > > > [1] https://lists.denx.de/pipermail/u-boot/2015-July/218585.html
> > > > > [2] 
> > > > > https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.3
> > > > >
> > > > >  .../devicetree/bindings/u-boot/config.yaml| 137 
> > > > > ++
> > > > >  1 file changed, 137 insertions(+)
> > > > >  create mode 100644 
> > > > > Documentation/devicetree/bindings/u-boot/config.yaml
> > > >
> > > > Might as well put this into dt-schema rather than the kernel. But might
> > > > get more review here first.
> > >
> > > OK, so does that mean a PR to https://github.com/robherring/dt-schema
> >
> > Wrong one: https://github.com/devicetree-org/dt-schema
> >
> > I need to update the readme there for the old one.
>
> OK thanks.
>
> >
> > > or is there a mailing list for it? I think I am missing some
> > > understanding here.
> >
> > You can send a PR or to a DT mailing list, but the mail list will

Re: [PATCH v3] sandbox: Remove OF_HOSTFILE

On Fri, Oct 15, 2021 at 11:42:40AM +0300, Ilias Apalodimas wrote:

> OF_HOSTFILE is used on sandbox configs only.  Although it's pretty
> unique and rarely causes confusion,  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 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

With the previous series applied, this breaks xilinx_zynq_virt:
https://source.denx.de/u-boot/u-boot/-/jobs/337428 

I had a little trouble in general getting the board to run locally, even
without the patches, so I didn't debug further yet.

-- 
Tom


signature.asc
Description: PGP signature

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

Make various updates suggested during review of the rST conversion.

Signed-off-by: Simon Glass 
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 v8 8/8] bootm: Tidy up use of autostart env var

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 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 | 4 ++--
 env/common.c  | 7 +++
 include/env.h | 7 +++
 6 files changed, 19 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..b14019179dd 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -165,8 +165,8 @@ 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 anything other than "yes", 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..ef9502a34f7 100644
--- a/env/common.c
+++ b/env/common.c
@@ -47,6 +47,13 @@ int env_get_yesno(const char *var)
1 : 0;
 }
 
+bool env_get_autostart(void)
+{
+   const char *val = env_get("autostart");
+
+   return val && !strcmp(val, "yes");
+}
+
 /*
  * 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 v8 6/8] doc: Mention CONFIG_DEFAULT_ENV_FILE

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

Suggested-by: Rasmus Villemoes 

Signed-off-by: Simon Glass 
---

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 v8 4/8] env: Allow U-Boot scripts to be placed in a .env file

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 
---

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| 63 
 8 files changed, 243 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
 defaultenv_h := include/generated/defaultenv_autogenerated.h
 dt_h := include/generated/dt.h
+env_h := include/generated/environment.h
 
 no-dot-config-targets := clean clobber mrproper distclean \
 help %docs check% coccicheck \
@@ -1785,6 +1786,69 @@ quiet_cmd_sym ?= SYM $@
 u-boot.sym: u-boot FORCE
$(call if_changed,sym)
 
+# Environment processing
+#

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

Use a text file for the environment instead of the #define settings.

Signed-off-by: Simon Glass 
---

(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 v8 3/8] doc: Move environment documentation to rST

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 
---

(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 it must reside in

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

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 
---

(no changes since v1)

 tools/binman/ftest.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index cea3ebf2b9f..8199a4fc7e0 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -4565,8 +4565,7 @@ class TestFunctional(unittest.TestCase):
 with self.assertRaises(ValueError) as e:
 self._DoTestFile('202_section_timeout.dts',
  test_section_timeout=True)
-self.assertIn("Node '/binman/section@0': Timed out obtaining contents",
-  str(e.exception))
+self.assertIn("Timed out obtaining contents", str(e.exception))
 
 def testTiming(self):
 """Test output of timing information"""
-- 
2.33.0.1079.g6e70778dc9-goog

[PATCH v8 2/8] sandbox: Drop distro_boot

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 
---

(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 v8 0/8] env: Allow environment in text files

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 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 rST format instead of README
- Add new patch to move environment documentation to rST

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
- Add new patch to use a text-based environment for sandbox

Changes in v2:
- Move .env file from include/configs to board/
- Use awk script to

Re: [PATCH v2 07/39] efi: Add a script for building and testing U-Boot on UEFI

Hi Heinrich,

On Mon, 18 Oct 2021 at 11:35, Heinrich Schuchardt  wrote:
>
> On 9/25/21 2:30 AM, Simon Glass wrote:
> > It is quite complicating to run U-Boot on qemu since we have four
>
> Nits:
> %s/complicating/complicated/   %/qemu/QEMU/
>
> > different builds and they must use different versions of qemu and the
>
> QEMU
>
> > UEFI binaries.
> >
> > Add a script to help.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> > Changes in v2:
> > - Add MAINTAINERS entry
> > - Add support for creating a partition table with a filesystem inside
> > - Add support for running qemu with just a serial console (no display)
> >
> >   MAINTAINERS|   1 +
> >   doc/develop/uefi/u-boot_on_efi.rst |  62 ++
> >   scripts/build-efi.sh   | 188 +
> >   3 files changed, 251 insertions(+)
> >   create mode 100755 scripts/build-efi.sh

[..]

> > diff --git a/scripts/build-efi.sh b/scripts/build-efi.sh
> > new file mode 100755
> > index 000..e6c035a7495
> > --- /dev/null
> > +++ b/scripts/build-efi.sh
> > @@ -0,0 +1,188 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Script to build an EFI thing suitable for booting with qemu, possibly 
> > running
> > +# it also.
> > +
> > +# This just an example. It assumes that
> > +
> > +# - you build U-Boot in /tmp/b/ where  is the U-Boot board 
> > config
> > +# - /mnt/x is a directory used for mounting
> > +# - you have access to the 'pure UEFI' builds for qemu
> > +#
> > +# UEFI binaries for QEMU used for testing this script:
> > +#
> > +# OVMF-pure-efi.i386.fd at
> > +# 
> > https://drive.google.com/file/d/1jWzOAZfQqMmS2_dAK2G518GhIgj9r2RY/view?usp=sharing
> > +
> > +# OVMF-pure-efi.x64.fd at
> > +# 
> > https://drive.google.com/file/d/1c39YI9QtpByGQ4V0UNNQtGqttEzS-eFV/view?usp=sharing
> > +
> > +set -e
> > +
> > +usage() {
> > + echo "Usage: $0 [-a | -p] [other opts]" 1>&2
> > + echo 1>&2
> > + echo "   -a   - Package up the app" 1>&2
> > + echo "   -o   - Use old EFI app build (before 32/64 split)" 1>&2
>
> Why is this option needed?

I think it is wise for now, in case we hit regressions. This allows
comparing the 'old' build with the new one. I have used it quite a few
times. We can drop this option once everything is settled.

[..]

Regards,
Simon

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

Hi Wolfgang,

On Mon, 18 Oct 2021 at 05:59, Wolfgang Denk  wrote:
>
> Dear Simon,
>
> In message 
>  you 
> wrote:
> >
> > > I really think your fixed filename proposal does not work well in
> > > reality.  The file name should be Kconfig configurable. See [1]
> > > for details.
> > >
> > > [1] https://lists.denx.de/pipermail/u-boot/2021-October/462668.html
> >
> > Yes I saw that but I forgot to look at it. I think it makes sense - we
> > do that with devicetree, for example.
> >
> > Is that the only thing holding you back?
>
> Basically yes - the only other concerns I have is about this +=
> construct which makes the '+' character an illegal character for
> environment variable names, but only when used at the end of the
> variable.  This is anything but nice or consistent. Iwonder what
> happens with notations like these:
>
> foo+=bar-> "bar" gets appended to current value of "foo"
> But what for:
> foo\+=bar
> or
> foo+ = bar
>
> ?

Can we just ban + ?

In the above, foo\+ gives an unknown escape sequence from the C
preprocessor, then the whole line is ignored by the script

foo+ = bar   produes a variable called "foo+ " in the environment with
the value " bar" so you probably don't want that.

>
> And please see also my comments about changing the autostart
> functionality for the user.

Yes I saw that and I think it is OK to change it. What do you think?

>
> > I haven't seen any positive comments to this series yet...
>
> Maybe many long-term users of U-Boot don't see the current situation
> as such a big problem?  I have no idea.

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.

Regards,
Simon

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

Hi Wolfgang,

On Mon, 18 Oct 2021 at 08:10, Wolfgang Denk  wrote:
>
> Dear Tom,
>
> In message <20211018133728.GQ7964@bill-the-cat> you wrote:
> >
> > > And please see also my comments about changing the autostart
> > > functionality for the user.
> >
> > Perhaps we should just make "+" an illegal character in the variable
> > name, for consistency?
>
> And break backward compatibility?  I'd rather see a better
> definition of the syntax of the environment files, plus maybe a more
> powerful parser.
>
> I mean, there is no technical reason to forbid the '+' character -
> and then it's only at the end of the variable name.
>
> Hm... I can't find it right now but did I not also read about other
> restrictions to variable names, like they must noch begin with '_'
> when using this new tool?

Yes but I took that out (I think in v6). I'll update the commit message.

>
>
> I feel it is wrong to place new restrictions on something that was
> constant for 21 years, just because our parser cannot parse it...

We need the + thing and perhaps we should ask people to avoid
punctuation, etc? But for now I'm not requiring it in this series,
apart from +

Regards,
Simon

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

Hi Wolfgang,

On Mon, 18 Oct 2021 at 06:07, Wolfgang Denk  wrote:
>
> Dear Simon,
>
> In message <20211016003339.723169-1-...@chromium.org> you wrote:
> >
> > 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.
>
> That should be board//.env , right?

Yes, will fix in v8.

>
>
> Reviewed-by: Wolfgang Denk 

Thanks...the cover letter or the series?

Regards,
Simon

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

On Sun, 17 Oct 2021 at 09:37, Marek Behún  wrote:
>
> From: Marek Behún 
>
> In the past the env_match() function was used to match envs with
> - name, i.e. string "name"
> - variable assignment, i.e. string "name=other_value"
>
> The latter is not the case anymore, since the env_match() function is
> now used only in env_get_f(), and so we can simplify the function into
> a simple strncmp() with an additional comparison to '='.
>
> Let's do this, and since the resulting function is quite simple, let's
> also inline its code into env_get_f().
>
> Signed-off-by: Marek Behún 
> ---
>  cmd/nvedit.c | 19 +--
>  1 file changed, 5 insertions(+), 14 deletions(-)

Reviewed-by: Simon Glass

Re: [PATCH] pxe_utils: Fix arguments to x86 zboot

Hi Zhaofeng,

On Sat, 16 Oct 2021 at 00:16, Zhaofeng Li  wrote:
>
> bootm and zboot accept different arguments:
>
> > bootm [addr [arg ...]]
> >- boot application image stored in memory
> >passing arguments 'arg ...'; when booting a Linux kernel,
> >'arg' can be the address of an initrd image
>
> > zboot [addr] [size] [initrd addr] [initrd size] [setup] [cmdline]
> >   addr -The optional starting address of the bzimage.
> > If not set it defaults to the environment
> > variable "fileaddr".
> >   size -The optional size of the bzimage. Defaults to
> > zero.
> >   initrd addr - The address of the initrd image to use, if any.
> >   initrd size - The size of the initrd image to use, if any.
>
> In the zboot flow, the current code will reuse the bootm args and attempt
> to pass the initrd arg (argv[2]) as the kernel size (should be argv[3]).
> zboot also expects the initrd address and size to be separate arguments.
>
> Let's untangle them and have separate argv/argc locals.
>
> Signed-off-by: Zhaofeng Li 
> Cc: Simon Glass 
> Cc: Bin Meng 
>
> diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c
> index 067c24e5ff..78ebfdcc79 100644
> --- a/cmd/pxe_utils.c
> +++ b/cmd/pxe_utils.c
> @@ -441,11 +441,14 @@ skip_overlay:
>  static int label_boot(struct cmd_tbl *cmdtp, struct pxe_label *label)
>  {
> char *bootm_argv[] = { "bootm", NULL, NULL, NULL, NULL };
> +   char *zboot_argv[] = { "zboot", NULL, "0", NULL, NULL };
> char initrd_str[28];
> +   char initrd_filesize[10];
> char mac_str[29] = "";
> char ip_str[68] = "";
> char *fit_addr = NULL;
> int bootm_argc = 2;
> +   int zboot_argc = 3;
> int len = 0;
> ulong kernel_addr;
> void *buf;
> @@ -478,6 +481,11 @@ static int label_boot(struct cmd_tbl *cmdtp, struct 
> pxe_label *label)
> strcat(bootm_argv[2], ":");
> strncat(bootm_argv[2], env_get("filesize"), 9);
> bootm_argc = 3;
> +
> +   strncpy(initrd_filesize, env_get("filesize"), 9);
> +   zboot_argv[3] = env_get("ramdisk_addr_r");
> +   zboot_argv[4] = initrd_filesize;
> +   zboot_argc = 5;
> }
>
> if (get_relfile_envaddr(cmdtp, label->kernel, "kernel_addr_r") < 0) {
> @@ -529,6 +537,8 @@ static int label_boot(struct cmd_tbl *cmdtp, struct 
> pxe_label *label)
> }
>
> bootm_argv[1] = env_get("kernel_addr_r");
> +   zboot_argv[1] = env_get("kernel_addr_r");
> +
> /* for FIT, append the configuration identifier */
> if (label->config) {
> int len = strlen(bootm_argv[1]) + strlen(label->config) + 1;
> @@ -665,7 +675,7 @@ static int label_boot(struct cmd_tbl *cmdtp, struct 
> pxe_label *label)
> do_bootz(cmdtp, 0, bootm_argc, bootm_argv);
> /* Try booting an x86_64 Linux kernel image */
> else if (IS_ENABLED(CONFIG_CMD_ZBOOT))
> -   do_zboot_parent(cmdtp, 0, bootm_argc, bootm_argv, NULL);
> +   do_zboot_parent(cmdtp, 0, zboot_argc, zboot_argv, NULL);
>
> unmap_sysmem(buf);
>
> --
> 2.33.0
>

The real fix here will be when we can call the boot/zimage code more
directly, rather than via the CLI.

Would it be worth adding more local variables for the strings being
created? We already have initrd_str and mac_str, for example. We could
add one for kernel_addr and ramdisk_addr:

char *ramdisk_addr

ramdisk_addr = env_get("ramdisk_addr_r")

It means that the bootm_argv[1] and bootm_argv[2] references would be
replaced by local variables.

Then, at the end of the function, create the bootm_argv  or zboot array:

bootm_argv[1] = kernel_addr;
bootm_argv[2] = ramdisk_addr

Maybe this is too painful and we should just wait for a change to
directly call the API?

So I'll add my review tag since I think this patch is reasonable.

Reviewed-by: Simon Glass 

Regards,
SImon

Re: [PATCH v2 07/39] efi: Add a script for building and testing U-Boot on UEFI


On 9/25/21 2:30 AM, Simon Glass wrote:

It is quite complicating to run U-Boot on qemu since we have four


Nits:
%s/complicating/complicated/   %/qemu/QEMU/


different builds and they must use different versions of qemu and the


QEMU


UEFI binaries.

Add a script to help.

Signed-off-by: Simon Glass 
---

Changes in v2:
- Add MAINTAINERS entry
- Add support for creating a partition table with a filesystem inside
- Add support for running qemu with just a serial console (no display)

  MAINTAINERS|   1 +
  doc/develop/uefi/u-boot_on_efi.rst |  62 ++
  scripts/build-efi.sh   | 188 +
  3 files changed, 251 insertions(+)
  create mode 100755 scripts/build-efi.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index 019c87592c5..84750be81a9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -706,6 +706,7 @@ M:  Heinrich Schuchardt 
  S:Maintained
  W:https://u-boot.readthedocs.io/en/latest/develop/uefi/u-boot_on_efi.html
  F:lib/efi/efi_app.c
+F: scripts/build-efi.sh

  EFI PAYLOAD
  M:Heinrich Schuchardt 
diff --git a/doc/develop/uefi/u-boot_on_efi.rst 
b/doc/develop/uefi/u-boot_on_efi.rst
index c9a41bc919f..4b2a733076d 100644
--- a/doc/develop/uefi/u-boot_on_efi.rst
+++ b/doc/develop/uefi/u-boot_on_efi.rst
@@ -96,6 +96,11 @@ that EFI does not support booting a 64-bit application from 
a 32-bit
  EFI (or vice versa). Also it will often fail to print an error message if
  you get this wrong.

+You may find the script `scripts/build-efi.sh` helpful for building and testing
+U-Boot on UEFI on QEMU. It also includes links to UEFI binaries dating from
+2021.
+
+See `Example run`_ for an example run.

  Inner workings
  --
@@ -191,6 +196,63 @@ of code is built this way (see the extra- line in 
lib/efi/Makefile).
  Everything else is built as a normal U-Boot, so is always 32-bit on x86 at
  present.

+Example run
+---
+
+This shows running with serial enabled (see `include/configs/efi-x86_app.h`)::
+
+   $ scripts/build-efi.sh -wsPr
+   Packaging efi-x86_app32
+   Running qemu-system-i386
+
+   BdsDxe: failed to load Boot0001 "UEFI QEMU HARDDISK QM5 " from 
PciRoot(0x0)/Pci(0x3,0x0)/Sata(0x0,0x,0x0): Not Found
+   BdsDxe: loading Boot0002 "EFI Internal Shell" from 
Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(7C04A583-9E3E-4F1C-AD65-E05268D0B4D1)
+   BdsDxe: starting Boot0002 "EFI Internal Shell" from 
Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(7C04A583-9E3E-4F1C-AD65-E05268D0B4D1)
+
+   UEFI Interactive Shell v2.2
+   EDK II
+   UEFI v2.70 (EDK II, 0x0001)
+   Mapping table
+ FS0: Alias(s):HD0a65535a1:;BLK1:
+ 
PciRoot(0x0)/Pci(0x3,0x0)/Sata(0x0,0x,0x0)/HD(1,GPT,0FFD5E61-3B0C-4326-8049-BDCDC910AF72,0x800,0xB000)
+BLK0: Alias(s):
+ PciRoot(0x0)/Pci(0x3,0x0)/Sata(0x0,0x,0x0)
+
+   Press ESC in 5 seconds to skip startup.nsh or any other key to continue.
+   Shell> fs0:u-boot-app.efi
+   U-Boot EFI App (using allocated RAM address 47d4000) key=8d4, image=06a6f610
+   starting
+
+
+   U-Boot 2022.01-rc4 (Sep 19 2021 - 14:03:20 -0600)
+
+   CPU: x86, vendor Intel, device 663h
+   DRAM:  32 MiB
+0: efi_media_0  PciRoot(0x0)/Pci(0x3,0x0)/Sata(0x0,0x,0x0)
+1:   
PciRoot(0x0)/Pci(0x3,0x0)/Sata(0x0,0x,0x0)/HD(1,GPT,0FFD5E61-3B0C-4326-8049-BDCDC910AF72,0x800,0xB000)
+   Loading Environment from nowhere... OK
+   Model: EFI x86 Application
+   Hit any key to stop autoboot:  0
+
+   Partition Map for EFI device 0  --   Partition Type: EFI
+
+   PartStart LBA   End LBAName
+   Attributes
+   Type GUID
+   Partition GUID
+ 1 0x0800  0xb7ff  "boot"
+   attrs:  0x
+   type:   ebd0a0a2-b9e5-4433-87c0-68b6b72699c7
+   guid:   0ffd5e61-3b0c-4326-8049-bdcdc910af72
+  19   startup.nsh
+  528384   u-boot-app.efi
+   10181   NvVars
+
+   3 file(s), 0 dir(s)
+
+   => QEMU: Terminated
+
+
  Future work
  ---
  This work could be extended in a number of ways:
diff --git a/scripts/build-efi.sh b/scripts/build-efi.sh
new file mode 100755
index 000..e6c035a7495
--- /dev/null
+++ b/scripts/build-efi.sh
@@ -0,0 +1,188 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Script to build an EFI thing suitable for booting with qemu, possibly running
+# it also.
+
+# This just an example. It assumes that
+
+# - you build U-Boot in /tmp/b/ where  is the U-Boot board config
+# - /mnt/x is a directory used for mounting
+# - you have access to the 'pure UEFI' builds for qemu
+#
+# UEFI binaries for QEMU used for testing this script:
+#
+# OVMF-pure-efi.i386.fd at
+# 
https://drive.google.com/file/d/1jWzOAZfQqMmS2_dAK2G518GhIgj9r2RY/view?usp=sharing
+
+# OVMF-pure-efi.x64.fd at
+# 
https://drive.google.com/file/d/1c39YI9QtpByGQ4V0UNNQtGqttEzS-eFV/view?usp=sharing
+
+set -e
+
+usage() {
+   echo "Usage: $0 [-a | -p] [other

Re: [PATCH v2 06/39] x86: Tidy up global_data pointer for 64-bit


On 9/25/21 2:30 AM, Simon Glass wrote:

Add an extern declaration so that it is possible to use this macro in
files other than the one that defines it.

Signed-off-by: Simon Glass 
Reviewed-by: Heinrich Schuchardt 
---

(no changes since v1)

  arch/x86/cpu/x86_64/cpu.c  | 3 +++
  arch/x86/include/asm/global_data.h | 2 ++
  2 files changed, 5 insertions(+)

diff --git a/arch/x86/cpu/x86_64/cpu.c b/arch/x86/cpu/x86_64/cpu.c
index 90a766c3c57..e090b1b478a 100644
--- a/arch/x86/cpu/x86_64/cpu.c
+++ b/arch/x86/cpu/x86_64/cpu.c
@@ -8,6 +8,9 @@
  #include 
  #include 
  #include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;

  /*
   * Global declaration of gd.
diff --git a/arch/x86/include/asm/global_data.h 
b/arch/x86/include/asm/global_data.h
index 3e4044593c8..f95fb5a1931 100644
--- a/arch/x86/include/asm/global_data.h
+++ b/arch/x86/include/asm/global_data.h
@@ -133,6 +133,8 @@ struct arch_global_data {
  #ifndef __ASSEMBLY__
  # if defined(CONFIG_EFI_APP) || CONFIG_IS_ENABLED(X86_64)

+extern struct global_data *global_data_ptr;


You already have:
arch/x86/include/asm/global_data.h:139:
#define DECLARE_GLOBAL_DATA_PTR   extern struct global_data *global_data_ptr

Why don't you use DECLARE_GLOBAL_DATA_PTR where needed?

Best regards

Heinrich


+
  /* TODO(s...@chromium.org): Consider using a fixed register for gd on x86_64 
*/
  #define gd global_data_ptr

Re: [PATCH v2 05/39] x86: Show some EFI info with the bdinfo command


On 9/25/21 2:30 AM, Simon Glass wrote:

It is useful to see some basic EFI info with the command as it forms part
of the information about a board.

Add a hook for this and show the table address as a start.

While here, fix an invalid cast in setup_efi_info(). Note that this
function is using a data structure defined by Linux so we cannot change
it. Also note that ulong is used since this is the standard in U-Boot
(>6k uses), despite there being quite a bit of the more verbose uintptr_t
(930 uses).

Signed-off-by: Simon Glass 
---

Changes in v2:
- Update commit message to mention why the data structure is like it is
- Update commit message to mention why ulong is used

  arch/x86/cpu/efi/payload.c | 13 +++--
  arch/x86/include/asm/efi.h |  7 +++
  arch/x86/lib/Makefile  |  1 +
  arch/x86/lib/bdinfo.c  | 22 ++
  4 files changed, 41 insertions(+), 2 deletions(-)
  create mode 100644 arch/x86/lib/bdinfo.c

diff --git a/arch/x86/cpu/efi/payload.c b/arch/x86/cpu/efi/payload.c
index 9a73b768e9b..3a9f7d72868 100644
--- a/arch/x86/cpu/efi/payload.c
+++ b/arch/x86/cpu/efi/payload.c
@@ -280,15 +280,24 @@ void setup_efi_info(struct efi_info *efi_info)
}
efi_info->efi_memdesc_size = map->desc_size;
efi_info->efi_memdesc_version = map->version;
-   efi_info->efi_memmap = (u32)(map->desc);
+   efi_info->efi_memmap = (ulong)(map->desc);
efi_info->efi_memmap_size = size - sizeof(struct efi_entry_memmap);

  #ifdef CONFIG_EFI_STUB_64BIT
efi_info->efi_systab_hi = table->sys_table >> 32;
-   efi_info->efi_memmap_hi = (u64)(u32)(map->desc) >> 32;
+   efi_info->efi_memmap_hi = (u64)(ulong)map->desc >> 32;
signature = EFI64_LOADER_SIGNATURE;
  #else
signature = EFI32_LOADER_SIGNATURE;
  #endif
memcpy(_info->efi_loader_signature, signature, 4);
  }
+
+void efi_show_bdinfo(void)
+{
+   struct efi_entry_systable *table = NULL;
+   int size, ret;
+
+   ret = efi_info_get(EFIET_SYS_TABLE, (void **), );
+   bdinfo_print_num_l("efi_table", (ulong)table);


Some properties of the UEFI API depend on the UEFI API version
(EFI_SYSTEM_TABLE.FirmwareRevision)

The revision of the EFI_FILE_PROTOCOL may also be relevant for U-Boot.

Maybe you want to add this information in a later patch.

Reviewed-by: Heinrich Schuchardt 


+}
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index c1735e4dc5f..dfd858b78ba 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -29,4 +29,11 @@ void setup_video(struct screen_info *screen_info);
   */
  void setup_efi_info(struct efi_info *efi_info);

+/**
+ * efi_show_bdinfo() - Show information about EFI for the 'bdinfo' command
+ *
+ * This looks up the EFI table pointer and shows related info
+ */
+void efi_show_bdinfo(void);
+
  #endif
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index 65d9b3bd6a3..18757b29aa9 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -3,6 +3,7 @@
  # (C) Copyright 2002-2006
  # Wolfgang Denk, DENX Software Engineering, w...@denx.de.

+obj-y  += bdinfo.o
  ifndef CONFIG_X86_64
  ifndef CONFIG_TPL_BUILD
  obj-y += bios.o
diff --git a/arch/x86/lib/bdinfo.c b/arch/x86/lib/bdinfo.c
new file mode 100644
index 000..0cb79b01bd3
--- /dev/null
+++ b/arch/x86/lib/bdinfo.c
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * x86-specific information for the 'bd' command
+ *
+ * Copyright 2021 Google LLC
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+void arch_print_bdinfo(void)
+{
+   bdinfo_print_num_l("prev table", gd->arch.table);
+
+   if (IS_ENABLED(CONFIG_EFI_STUB))
+   efi_show_bdinfo();
+}

Re: [PATCH v2 04/39] x86: Create a new header for EFI


On 9/25/21 2:30 AM, Simon Glass wrote:

The setup routines are called from zimage but don't really belong in the
zimage header. Add a new EFI header to house these. Add comments so it is
clear what the functions do.

Note that these functions are x86-specific. The zimage business is not
used on other architectures.

Signed-off-by: Simon Glass 


Reviewed-by: Heinrich Schuchardt 


---

Changes in v2:
- Update commit message to mention that these function are x86-specific

  arch/x86/include/asm/efi.h| 32 
  arch/x86/include/asm/zimage.h |  3 ---
  arch/x86/lib/zimage.c |  1 +
  3 files changed, 33 insertions(+), 3 deletions(-)
  create mode 100644 arch/x86/include/asm/efi.h

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
new file mode 100644
index 000..c1735e4dc5f
--- /dev/null
+++ b/arch/x86/include/asm/efi.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright Google LLC
+ */
+
+#ifndef _ASM_EFI_H_
+#define _ASM_EFI_H_
+
+struct efi_info;
+struct screen_info;
+
+/**
+ * setup_video() - Set up the screen info in the x86 setup
+ *
+ * This is needed so Linux can use the display (when U-Boot is an EFI payload)
+ *
+ * @efi_info: Pointer to place to put the screen info in the x86 setup base
+ */
+void setup_video(struct screen_info *screen_info);
+
+/**
+ * setup_efi_info() - Set up the EFI info needed by Linux to boot
+ *
+ * This writes a suitable signature, table pointers, memory-map pointer, etc.
+ * These are needed for Linux to boot from U-Boot (when U-Boot is an EFI
+ * payload).
+ *
+ * @efi_info: Pointer to place to put the EFI info in the x86 setup base
+ */
+void setup_efi_info(struct efi_info *efi_info);
+
+#endif
diff --git a/arch/x86/include/asm/zimage.h b/arch/x86/include/asm/zimage.h
index 6679767d16b..fa6e7f76e05 100644
--- a/arch/x86/include/asm/zimage.h
+++ b/arch/x86/include/asm/zimage.h
@@ -72,7 +72,4 @@ int setup_zimage(struct boot_params *setup_base, char 
*cmd_line, int auto_boot,
   */
  void zimage_dump(struct boot_params *base_ptr);

-void setup_video(struct screen_info *screen_info);
-void setup_efi_info(struct efi_info *efi_info);
-
  #endif
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c
index 9938c80a42b..7ce02226ef9 100644
--- a/arch/x86/lib/zimage.c
+++ b/arch/x86/lib/zimage.c
@@ -29,6 +29,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #ifdef CONFIG_SYS_COREBOOT
  #include

Re: [PATCH v2 24/39] efi: serial: Support arrow keys


On 9/25/21 2:30 AM, Simon Glass wrote:

At present only the backspace key is supported in U-Boot, when running as
an EFI app. Add support for arrows, home and end as well, to make the CLI
more friendly.

Signed-off-by: Simon Glass 
---

(no changes since v1)

  drivers/serial/serial_efi.c | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/serial_efi.c b/drivers/serial/serial_efi.c
index 33ddbd6080c..0067576389d 100644
--- a/drivers/serial/serial_efi.c
+++ b/drivers/serial/serial_efi.c
@@ -24,6 +24,9 @@ struct serial_efi_priv {
bool have_key;
  };

+/* Convert a lower-case character to its ctrl-char equivalent */
+#define CTL_CH(c)  ((c) - 'a' + 1)
+
  int serial_efi_setbrg(struct udevice *dev, int baudrate)
  {
return 0;
@@ -49,6 +52,7 @@ static int serial_efi_get_key(struct serial_efi_priv *priv)
  static int serial_efi_getc(struct udevice *dev)
  {
struct serial_efi_priv *priv = dev_get_priv(dev);
+   char conv_scan[10] = {0, 'p', 'n', 'f', 'b', 'a', 'e', 0, 8};
int ret, ch;

ret = serial_efi_get_key(priv);
@@ -63,8 +67,11 @@ static int serial_efi_getc(struct udevice *dev)
 * key scan code of 8. Handle this so that backspace works correctly
 * in the U-Boot command line.
 */
-   if (!ch && priv->key.scan_code == 8)
-   ch = 8;
+   if (!ch && priv->key.scan_code < sizeof(conv_scan))
+   ch = conv_scan[priv->key.scan_code];
+   if (ch >= 'a')
+   ch -= 'a' - 1;


All 23 EFI Scan Codes should be supported. Can't we use something like
kbd_to_ansi364[] from drivers/input/input.c to create the appropriate
escape sequences?

Best regards

Heinrich


+   }
debug(" [%x %x %x] ", ch, priv->key.unicode_char, priv->key.scan_code);

return ch;

Re: [PATCH v2 02/39] efi: Add a separate maintainer entry for the app


On 9/25/21 2:30 AM, Simon Glass wrote:

Separate this out slightly from the payload, with a new entry.

We might consider renaming EFI PAYLOAD to EFI LOADER, but that would
require quite a lot of file changes.

Signed-off-by: Simon Glass 


Reviewed-by: Heinrich Schuchardt 


---

Changes in v2:
- Add new patch with maintainer entry

  MAINTAINERS | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 687c72dcd83..019c87592c5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -700,6 +700,13 @@ F: drivers/core/
  F:include/dm/
  F:test/dm/

+EFI APP
+M: Simon Glass 
+M: Heinrich Schuchardt 
+S: Maintained
+W: https://u-boot.readthedocs.io/en/latest/develop/uefi/u-boot_on_efi.html
+F: lib/efi/efi_app.c
+
  EFI PAYLOAD
  M:Heinrich Schuchardt 
  R:Alexander Graf

Re: [PATCH] xilinx: zynqmp: Use CONFIG_SPL_FS_LOAD_PAYLOAD_NAME

2021-10-18 Thread Oleksandr Suvorov

On Mon, Oct 18, 2021 at 3:02 PM Michal Simek  wrote:
>
> There is no need to use u-boot.itb as name when this is already recorded in
> CONFIG_SPL_FS_LOAD_PAYLOAD_NAME macro.
>
> Signed-off-by: Michal Simek 

Reviewed-by: Oleksandr Suvorov 


> ---
>
>  board/xilinx/zynqmp/zynqmp.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 5fce74d6085a..381f2f82eb8c 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -877,20 +877,23 @@ void set_dfu_alt_info(char *interface, char *devstr)
> if (!multiboot)
> snprintf(buf, DFU_ALT_BUF_LEN,
>  "mmc %d:1=boot.bin fat %d 1;"
> -"u-boot.itb fat %d 1",
> -bootseq, bootseq, bootseq);
> +"%s fat %d 1",
> +bootseq, bootseq,
> +CONFIG_SPL_FS_LOAD_PAYLOAD_NAME, bootseq);
> else
> snprintf(buf, DFU_ALT_BUF_LEN,
>  "mmc %d:1=boot%04d.bin fat %d 1;"
> -"u-boot.itb fat %d 1",
> -bootseq, multiboot, bootseq, bootseq);
> +"%s fat %d 1",
> +bootseq, multiboot, bootseq,
> +CONFIG_SPL_FS_LOAD_PAYLOAD_NAME, bootseq);
> break;
> case QSPI_MODE_24BIT:
> case QSPI_MODE_32BIT:
> snprintf(buf, DFU_ALT_BUF_LEN,
>  "sf 0:0=boot.bin raw %x 0x150;"
> -"u-boot.itb raw 0x%x 0x50",
> -multiboot * SZ_32K, CONFIG_SYS_SPI_U_BOOT_OFFS);
> +"%s raw 0x%x 0x50",
> +multiboot * SZ_32K, CONFIG_SPL_FS_LOAD_PAYLOAD_NAME,
> +CONFIG_SYS_SPI_U_BOOT_OFFS);
> break;
> default:
> return;
> --
> 2.33.1
>


--
Best regards,

Oleksandr Suvorov
Software Engineer
T: +380 63 8489656
E: oleksandr.suvo...@foundries.io
W: www.foundries.io

Re: [PATCH] xilinx: zynqmp: Handle fallthrough statement properly

2021-10-18 Thread Oleksandr Suvorov

On Mon, Oct 18, 2021 at 2:30 PM Michal Simek  wrote:
>
> Now intentional fallthrough in switch-case should be labelled that's why
> convert comment.
>
> Signed-off-by: Michal Simek 

Reviewed-by: Oleksandr Suvorov 

> ---
>
>  board/xilinx/zynqmp/zynqmp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 064788c89db1..286874ab0597 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -711,7 +711,7 @@ int board_late_init(void)
> break;
> case SD1_LSHFT_MODE:
> puts("LVL_SHFT_");
> -   /* fall through */
> +   fallthrough;
> case SD_MODE1:
> puts("SD_MODE1\n");
> if (uclass_get_device_by_name(UCLASS_MMC,
> --
> 2.33.1
>


--
Best regards,

Oleksandr Suvorov
Software Engineer
T: +380 63 8489656
E: oleksandr.suvo...@foundries.io
W: www.foundries.io

Re: [PATCH v2 00/39] efi: Improvements to U-Boot running on top of UEFI

Hi Heinrich,

On Fri, 24 Sept 2021 at 18:31, Simon Glass  wrote:
>
> At present U-Boot can be built as an EFI app, but it is really just for
> testing, with very few features. Instead, the payload build is used for
> booting on top of UEFI, where U-Boot takes over the machine immediately
> and supplies its own drivers.
>
> But the app could be made more useful.
>
> This series provides access to EFI block devices and the video console
> within U-Boot. It also restructures the app to make it possible to boot
> a kernel, although further work is needed to implement this.
>
> This can be thought of as making U-Boot perform a little like 'grub', in
> that it runs purely based on UEFI-provided services.
>
> Of course a lot more work is required, but this is a start.
>
> Note: It would be very useful to include qemu tests of the app and stub
> in CI.
>
> This is available at u-boot-dm/efi-working
>
> Changes in v2:
> - Add MAINTAINERS entry
> - Add a better boot command too
> - Add a note that EFI_GRAPHICS_OUTPUT_PROTOCOL is used
> - Add a sentence about what the patch does
> - Add a work-around to avoid a toolchain crash
> - Add new patch to drop the OF_EMBED warning for EFI
> - Add new patch to enable DM_ETH for the app
> - Add new patch to support the efi command in the app
> - Add new patch with maintainer entry
> - Add support for creating a partition table with a filesystem inside
> - Add support for running qemu with just a serial console (no display)
> - Drop mention of partitions from the commit message
> - Expand the commit message to make things clearer
> - Fix 'as' typo
> - Mention why this patch is included in this series
> - Put this patch first so that it is separated from the rest of the series
> - Show the correct interface type with 'part list'
> - Update commit message to mention that these function are x86-specific
> - Update commit message to mention why the data structure is like it is
> - Update commit message to mention why ulong is used
> - Update documentation
> - Update the commit message to explain things better
> - Use log_info() instead of printf()
>
> Simon Glass (39):
>   RFC: efi: Drop code that doesn't work with driver model
>   efi: Add a separate maintainer entry for the app
>   x86: Keep symbol information in u-boot ELF file
>   x86: Create a new header for EFI
>   x86: Show some EFI info with the bdinfo command
>   x86: Tidy up global_data pointer for 64-bit
>   efi: Add a script for building and testing U-Boot on UEFI
>   efi: Enable DM_ETH for the app
>   efi: Drop the OF_EMBED warning for EFI
>   x86: Create a 32/64-bit selection for the app
>   efi: Create a 64-bit app
>   x86: Don't duplicate global_ptr in 64-bit EFI app
>   efi: Add a way to obtain boot services in the app
>   efi: Add video support to the app
>   efi: Add EFI uclass for media
>   efi: Add a media/block driver for EFI block devices
>   efi: Locate all block devices in the app
>   patman: Use a ValueError exception if tools.Run() fails
>   binman: Report an error if test files fail to compile
>   binman: Support reading the offset of an ELF-file symbol
>   binman: Allow timeout to occur in the image or its section
>   binman: Tidy up comments on _DoTestFile()
>   binman: Support updating the dtb in an ELF file
>   efi: serial: Support arrow keys
>   bloblist: Support allocating the bloblist
>   x86: Allow booting a kernel from the EFI app
>   x86: Don't process the kernel command line unless enabled
>   x86: efi: Add room for the binman definition in the dtb
>   efi: Add comments to struct efi_priv
>   efi: Fix ll_boot_init() operation with the app
>   efi: Add a few comments to the stub
>   efi: Share struct efi_priv between the app and stub code
>   efi: Move exit_boot_services into a function
>   efi: Check for failure when initing the app
>   efi: Mention that efi_info_get() is only used in the stub
>   efi: Show when allocated pages are used
>   efi: Allow easy selection of serial-only operation
>   efi: Update efi_get_next_mem_desc() to avoid needing a map
>   efi: Support the efi command in the app
>
>  MAINTAINERS   |  14 +
>  Makefile  |  10 +-
>  arch/sandbox/dts/test.dts |   4 +
>  arch/x86/config.mk|   4 +-
>  arch/x86/cpu/efi/payload.c|  21 +-
>  arch/x86/cpu/intel_common/Makefile|   2 +-
>  arch/x86/cpu/u-boot-64.lds|   2 +
>  arch/x86/cpu/x86_64/Makefile  |   4 +
>  arch/x86/cpu/x86_64/cpu.c |  32 +-
>  arch/x86/cpu/x86_64/misc.c|  41 +++
>  arch/x86/dts/Makefile |   2 +-
>  arch/x86/dts/efi-x86_app.dts  |   4 +
>  arch/x86/include/asm/efi.h|  39 +++
>  arch/x86/include/asm/global_data.h|   2 +
>  arch/x86/include/asm/zimage.h |   3 -
>  arch/x86/lib/Makefile

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

On Mon, Oct 18, 2021 at 04:10:34PM +0200, Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <20211018133728.GQ7964@bill-the-cat> you wrote:
> > 
> > > And please see also my comments about changing the autostart
> > > functionality for the user.
> >
> > Perhaps we should just make "+" an illegal character in the variable
> > name, for consistency?
> 
> And break backward compatibility?  I'd rather see a better
> definition of the syntax of the environment files, plus maybe a more
> powerful parser.

Are there examples today of scripts that use "+" in the variable names?
That maybe someone wrote a custom an private thing that uses + in the
name isn't the best argument.  Someone saying that did would be better.

> I mean, there is no technical reason to forbid the '+' character -
> and then it's only at the end of the variable name.

Of course yes, if we can just make the parser handle it, without it also
being a tricky nightmare, that's the better solution.

> Hm... I can't find it right now but did I not also read about other
> restrictions to variable names, like they must noch begin with '_'
> when using this new tool?

Any invalid characters need to be clearly documented, if they aren't,
yes.

> I feel it is wrong to place new restrictions on something that was
> constant for 21 years, just because our parser cannot parse it...

Sure.  But if it's also the case that for 21 years no one has been using
foo+bar, baz+, etc, in their variable names, maybe we just document
that's not valid and move on?

-- 
Tom

signature.asc
Description: PGP signature

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

Dear Tom,

In message <20211018133728.GQ7964@bill-the-cat> you wrote:
> 
> > And please see also my comments about changing the autostart
> > functionality for the user.
>
> Perhaps we should just make "+" an illegal character in the variable
> name, for consistency?

And break backward compatibility?  I'd rather see a better
definition of the syntax of the environment files, plus maybe a more
powerful parser.

I mean, there is no technical reason to forbid the '+' character -
and then it's only at the end of the variable name.

Hm... I can't find it right now but did I not also read about other
restrictions to variable names, like they must noch begin with '_'
when using this new tool?

I feel it is wrong to place new restrictions on something that was
constant for 21 years, just because our parser cannot parse it...

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
"You ain't experienced..." "Well, nor are you." "That's true. But the
point is ... the point is ... the point is we've been not experienced
for a lot longer than you. We've got  a  lot  of  experience  of  not
having any experience."   - Terry Pratchett, _Witches Abroad_

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

2021-10-18 Thread Alex G.


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.


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 d8c041a877..380ad4f670 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -566,6 +566,7 @@ config ARCH_KIRKWOOD
select BOARD_EARLY_INIT_F
select CPU_ARM926EJS
select GPIO_EXTRA_HEADER
+   select TOOLS_LIBCRYPTO
  
  config ARCH_MVEBU

bool "Marvell MVEBU family (Armada XP/375/38x/3700/7K/8K)"
@@ -580,12 +581,14 @@ config ARCH_MVEBU
select OF_CONTROL
select OF_SEPARATE
select SPI
+   select TOOLS_LIBCRYPTO
imply CMD_DM
  
  config ARCH_ORION5X

bool "Marvell Orion"
select CPU_ARM926EJS
select GPIO_EXTRA_HEADER
+   select TOOLS_LIBCRYPTO
  
  config TARGET_STV0991

bool "Support stv0991"
diff --git a/arch/arm/mach-imx/mxs/Kconfig b/arch/arm/mach-imx/mxs/Kconfig
index b2026a3758..6f138d25e9 100644
--- a/arch/arm/mach-imx/mxs/Kconfig
+++ b/arch/arm/mach-imx/mxs/Kconfig
@@ -3,6 +3,7 @@ if ARCH_MX23
  config MX23
bool
default y
+   select TOOLS_LIBCRYPTO
  
  choice

prompt "MX23 board select"
@@ -34,6 +35,7 @@ if ARCH_MX28
  config MX28
bool
default y
+   select TOOLS_LIBCRYPTO
  
  choice

prompt "MX28 board select"
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
index 3a6865dc70..bea6b6f83b 100644
--- a/scripts/config_whitelist.txt
+++ b/scripts/config_whitelist.txt
@@ -838,7 +838,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),)

Re: Broken build with disabling OpenSSL crypto

2021-10-18 Thread Alex G.


On 10/15/21 3:30 PM, Pali Rohár wrote:

On Friday 15 October 2021 09:35:43 Alex G. wrote:

On 10/15/21 6:34 AM, Pali Rohár wrote:

On Wednesday 06 October 2021 17:05:24 Alex G. wrote:

Hi Jernej,

On 10/6/21 4:27 PM, Jernej Škrabec wrote:

Hi everyone!

Commit cb9faa6f98ae ("tools: Use a single target-independent config to enable
OpenSSL") recently introduced option to disable usage of OpenSSL via
CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit b4f3cc2c42d9
("tools: kwbimage: Do not hide usage of secure header under
CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That totally
defeats the purpose of first commit. I suggest that it gets reverted.

I would like disable OpenSSL for my usage, since it gives me troubles when
cross-compiling U-Boot inside LibreELEC build system. It's not needed for our
case anyway.

Best regards,



Can you please give the following diff a try, and if it works for you, submit 
as patch?


This change is incorrect and will break mvebu builds. mvebu requires
kwbimage for building boot images and so you cannot disable it or make
it optional.



If kwbimage is required and missing the CI builds and tests don't catch
that. I ran buildman with the change, and nothing broke. Sounds like that
needs to be addressed.


It is possible that tests do not covert all scenarios.


That being said, I'm not okay with making everyone a slave to OpenSSL
because of any given platform.

I propose to revert commit b4f3cc2c42d9 ("tools: kwbimage: Do not hide usage
of secure header under CONFIG_ARMADA_38X"), and rework it such that it
doesn't force libcrypto on everyone. And we very likely need a CI test
against libcrypto linkage when TOOLS_LIBCRYPTO is not set.


Reverting that commit is not a solution as it can lead to broken
kwbimage (when crypto stuff is not enabled). Plus there is lot of other
changes and fixes in kwboot and kwbimage...


There are lots of CI tests that do not build usable images. I caution 
against conflating testability with usability, as anyone who intends to 
run images on hardware will likely have done their diligence and enabled 
TOOLS_LIBCRYPTO.


Alex




Some information with another approach how to solve build issues are in
this email:
https://lore.kernel.org/u-boot/20211015114735.rig3e4cuc7mn6a7e@pali/


Alex



diff --git a/tools/Makefile b/tools/Makefile
index 4a86321f64..7f72ff9645 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -96,7 +96,8 @@ AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/, \

   # Cryptographic helpers that depend on openssl/libcrypto
   LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
-   fdt-libcrypto.o)
+   fdt-libcrypto.o) \
+   kwbimage.o

   ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o

@@ -117,7 +118,6 @@ dumpimage-mkimage-objs := aisimage.o \
imximage.o \
imx8image.o \
imx8mimage.o \
-   kwbimage.o \
lib/md5.o \
lpc32xximage.o \
mxsimage.o \
@@ -169,8 +169,8 @@ 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),)
+# MXSImage needs LibSSL <- Nope! Read the frogging notice at the top
+ifneq ($(CONFIG_TOOLS_LIBCRYPTO),)
   HOSTCFLAGS_kwbimage.o += \
$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
   HOSTLDLIBS_mkimage += \

Re: [PATCH v7 5/7] doc: Mention CONFIG_DEFAULT_ENV_FILE

Dear Simon,

In message <20211016003339.723169-4-...@chromium.org> you wrote:
> Add mention of this option this it does a similar thing to the text
> environment.
>
> Suggested-by: Rasmus Villemoes 
>
> Signed-off-by: Simon Glass 
...

> +The format is the same as accepted by the mkenvimage tool: lines containing
> +key=value pairs, blank lines and lines beginning with # are ignored.

This is misleading. It can be read as: "lines containing key=value
pairs ... are ignored."

This reminds me of a sign in the door of a shop in Cornwall which
read: "No prams or dogs or food being eaten!"  I mean eating dogs is
bad enough, but eating prams? ? ?

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
"There's only one way to have a happy marriage and as soon as I learn
what it is I'll get married again."  - Clint Eastwood

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

On Mon, Oct 18, 2021 at 01:58:57PM +0200, Wolfgang Denk wrote:
> Dear Simon,
> 
> In message 
>  you 
> wrote:
> >
> > > I really think your fixed filename proposal does not work well in
> > > reality.  The file name should be Kconfig configurable. See [1]
> > > for details.
> > >
> > > [1] https://lists.denx.de/pipermail/u-boot/2021-October/462668.html
> >
> > Yes I saw that but I forgot to look at it. I think it makes sense - we
> > do that with devicetree, for example.
> >
> > Is that the only thing holding you back?
> 
> Basically yes - the only other concerns I have is about this +=
> construct which makes the '+' character an illegal character for
> environment variable names, but only when used at the end of the
> variable.  This is anything but nice or consistent. Iwonder what
> happens with notations like these:
> 
>   foo+=bar-> "bar" gets appended to current value of "foo"
> But what for:
>   foo\+=bar
> or
>   foo+ = bar
> 
> ?
> 
> And please see also my comments about changing the autostart
> functionality for the user.

Perhaps we should just make "+" an illegal character in the variable
name, for consistency?

> > I haven't seen any positive comments to this series yet...
> 
> Maybe many long-term users of U-Boot don't see the current situation
> as such a big problem?  I have no idea.

Considering the number of external to U-Boot projects for dealing with
the environment in U-Boot, I don't think that's the case.

Simon, perhaps it's worth poking some of those people off-list with a
pointer to your series?  Something like this should make integration
much easier, for most of the existing OTA solutions that support U-Boot.
It should also make it easier for OE/buildroot/etc to explain to users
how to modify the environment as well.

-- 
Tom


signature.asc
Description: PGP signature

Re: Pull request: u-boot-rockchip-20211015

On Mon, Oct 18, 2021 at 09:11:03AM +0800, Kever Yang wrote:

> Hi Tom,
> 
> Please pull the rockchip fixes:
> - Fix for Rockchip mmc HS400 mode;
> - Fix for px30 board Odroid Go;
> - rockchip_sfc update;
> - rk3568 clk update;
> - doc fix;
> 
> Gitlab ci:
> https://gitlab.denx.de/u-boot/custodians/u-boot-rockchip/pipelines/9491
> 
> Thanks,
> - Kever
> 
> The following changes since commit 2d34be0a9e12cf050de39ad95375213f48e685bd:
> 
>   Merge branch '2021-10-14-assorted-updates' (2021-10-15 07:50:59 -0400)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-rockchip.git 
> tags/u-boot-rockchip-20211015
> 
> for you to fetch changes up to 022f552704b6467966e4fad39c85a6aca9204c94:
> 
>   mmc: rockchip_sdhci: enable strobe line for HS400 (2021-10-15 20:57:41 
> +0800)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

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

Dear Simon,

In message 
<20211015183321.v7.3.Ie78bfbfca0d01d9cba501e127f446ec48e1f7afe@changeid> you 
wrote:
>
> 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//env
> directory called .env (or common.env if you want the same
> environment for all boards).

The cover letter does not mention a separate "env" directory - and
do we really need it?  I will almost always contain just a single
file - so env/.env is an just unnecessary complication.

> Also support += to allow variables to be appended to. This is needed when
> using the preprocessor.
...
> - Explain why variables starting with _ , and / are not supported
...
> - Move use of += to this patch
> - Explain that environment variables may not end in +

I feel all these restrictions should also be mentioned in the commit
message, not only in the code.

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
Command, n.:
Statement presented by a human and accepted by a computer
in such a manner as to make the human feel as if he is in control.

Re: [PATCH v2] env: mmc: Add support for redundant env in both eMMC boot partitions

2021-10-18 Thread Marek Vasut


On 10/18/21 4:02 AM, Jaehoon Chung wrote:

Hi Marek,


Hi,


On 10/18/21 2:23 AM, Marek Vasut wrote:

Currently the MMC environment driver supports storing redundant environment
only in one eMMC partition at different offsets. This is sub-optimal, since
if this one boot partition is erased, both copies of environment are lost.
Since the eMMC has two boot partitions, add support for storing one copy of
environment in each of the two boot partitions.

To enable this functionality, select CONFIG_SYS_REDUNDAND_ENVIRONMENT to
indicate redundant environment should be used. Set CONFIG_SYS_MMC_ENV_PART
to 1 to indicate environment should be stored in eMMC boot partition. Set
CONFIG_ENV_OFFSET equal to CONFIG_ENV_OFFSET_REDUND, and both to the offset
from start of eMMC boot partition where the environment should be located.


Is there any patch before apply this? When I had been trying to test this, I 
can't apply this from patchwork.


Uh, pick this one fix, I missed it in this v2 resend:
https://patchwork.ozlabs.org/project/uboot/patch/20211006162954.63596-1-ma...@denx.de/

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

Dear Simon,

In message <20211016003339.723169-1-...@chromium.org> you wrote:
>
> 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.

That should be board//.env , right?


Reviewed-by: Wolfgang Denk 


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
READ THIS BEFORE OPENING PACKAGE: According to Certain Suggested Ver-
sions of the Grand Unified Theory, the Primary Particles Constituting
this Product May Decay to Nothingness Within the  Next  Four  Hundred
Million Years.

[PATCH] xilinx: zynqmp: Use CONFIG_SPL_FS_LOAD_PAYLOAD_NAME

2021-10-18 Thread Michal Simek

There is no need to use u-boot.itb as name when this is already recorded in
CONFIG_SPL_FS_LOAD_PAYLOAD_NAME macro.

Signed-off-by: Michal Simek 
---

 board/xilinx/zynqmp/zynqmp.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 5fce74d6085a..381f2f82eb8c 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -877,20 +877,23 @@ void set_dfu_alt_info(char *interface, char *devstr)
if (!multiboot)
snprintf(buf, DFU_ALT_BUF_LEN,
 "mmc %d:1=boot.bin fat %d 1;"
-"u-boot.itb fat %d 1",
-bootseq, bootseq, bootseq);
+"%s fat %d 1",
+bootseq, bootseq,
+CONFIG_SPL_FS_LOAD_PAYLOAD_NAME, bootseq);
else
snprintf(buf, DFU_ALT_BUF_LEN,
 "mmc %d:1=boot%04d.bin fat %d 1;"
-"u-boot.itb fat %d 1",
-bootseq, multiboot, bootseq, bootseq);
+"%s fat %d 1",
+bootseq, multiboot, bootseq,
+CONFIG_SPL_FS_LOAD_PAYLOAD_NAME, bootseq);
break;
case QSPI_MODE_24BIT:
case QSPI_MODE_32BIT:
snprintf(buf, DFU_ALT_BUF_LEN,
 "sf 0:0=boot.bin raw %x 0x150;"
-"u-boot.itb raw 0x%x 0x50",
-multiboot * SZ_32K, CONFIG_SYS_SPI_U_BOOT_OFFS);
+"%s raw 0x%x 0x50",
+multiboot * SZ_32K, CONFIG_SPL_FS_LOAD_PAYLOAD_NAME,
+CONFIG_SYS_SPI_U_BOOT_OFFS);
break;
default:
return;
-- 
2.33.1

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

Dear Simon,

In message  
you wrote:
>
> > I really think your fixed filename proposal does not work well in
> > reality.  The file name should be Kconfig configurable. See [1]
> > for details.
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2021-October/462668.html
>
> Yes I saw that but I forgot to look at it. I think it makes sense - we
> do that with devicetree, for example.
>
> Is that the only thing holding you back?

Basically yes - the only other concerns I have is about this +=
construct which makes the '+' character an illegal character for
environment variable names, but only when used at the end of the
variable.  This is anything but nice or consistent. Iwonder what
happens with notations like these:

foo+=bar-> "bar" gets appended to current value of "foo"
But what for:
foo\+=bar
or
foo+ = bar

?

And please see also my comments about changing the autostart
functionality for the user.

> I haven't seen any positive comments to this series yet...

Maybe many long-term users of U-Boot don't see the current situation
as such a big problem?  I have no idea.

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
Real Programmers always confuse Christmas and Halloween because
OCT 31 == DEC 25 !  - Andrew Rutherford (andr...@ucs.adelaide.edu.au)

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

2021-10-18 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 
Change-Id: Icc79e352db6305c232527c4a432f17b7a30fec8e
---
 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

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

2021-10-18 Thread Kris Chaplin

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 
---
 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

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

2021-10-18 Thread Kris Chaplin

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 
---
 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

[PATCH v4 4/4] mtd: spi-nor-core: Add support for Macronix Octal flash

Adding Macronix Octal flash for Octal DTR support.

The octaflash series can be divided into the following types:

MX25 series : Serial NOR Flash.
MX66 series : Serial NOR Flash with stacked die.(Size larger than 1Gb)
LM/UM series : Up to 250MHz clock frequency with both DTR/STR operation.
LW/UW series : Support simultaneous Read-while-Write operation in multiple
   bank architecture. Read-while-write feature which means read
   data one bank while another bank is programing or erasing.

MX25LM : 3.0V Octal I/O
 
-https://www.mxic.com.tw/Lists/Datasheet/Attachments/7841/MX25LM51245G,%203V,%20512Mb,%20v1.1.pdf

MX25UM : 1.8V Octal I/O
 
-https://www.mxic.com.tw/Lists/Datasheet/Attachments/7525/MX25UM51245G%20Extreme%20Speed,%201.8V,%20512Mb,%20v1.0.pdf

MX66LM : 3.0V Octal I/O with stacked die
 
-https://www.mxic.com.tw/Lists/Datasheet/Attachments/7929/MX66LM1G45G,%203V,%201Gb,%20v1.1.pdf

MX66UM : 1.8V Octal I/O with stacked die
 
-https://www.mxic.com.tw/Lists/Datasheet/Attachments/7721/MX66UM1G45G,%201.8V,%201Gb,%20v1.1.pdf

MX25LW : 3.0V Octal I/O with Read-while-Write
MX25UW : 1.8V Octal I/O with Read-while-Write
MX66LW : 3.0V Octal I/O with Read-while-Write and stack die
MX66UW : 1.8V Octal I/O with Read-while-Write and stack die

About LW/UW series, please contact us freely if you have any
questions. For adding Octal NOR Flash IDs, we have validated
each Flash on plateform zynq-picozed.

Signed-off-by: JaimeLiao 
---
 drivers/mtd/spi/spi-nor-ids.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index cb3a08872d..5c13ea3a78 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -169,7 +169,27 @@ const struct flash_info spi_nor_ids[] = {
{ INFO("mx66l1g45g",  0xc2201b, 0, 64 * 1024, 2048, SECT_4K | 
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ INFO("mx25l1633e", 0xc22415, 0, 64 * 1024,   32, SPI_NOR_QUAD_READ | 
SPI_NOR_4B_OPCODES | SECT_4K) },
{ INFO("mx25r6435f", 0xc22817, 0, 64 * 1024,   128,  SECT_4K) },
-   { INFO("mx66uw2g345g", 0xc2943c, 0, 64 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx66uw2g345gx0", 0xc2943c, 0, 64 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx66lm1g45g",0xc2853b, 0, 32 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25lm51245g",   0xc2853a, 0, 16 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25lw51245g",   0xc2863a, 0, 16 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25lm25645g",   0xc28539, 0, 8 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx66um2g45g",0xc2803c, 0, 64 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx66uw2g345g",   0xc2843c, 0, 64 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx66um1g45g",0xc2803b, 0, 32 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx66uw1g45g",0xc2813b, 0, 32 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25um51245g",   0xc2803a, 0, 16 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw51245g",   0xc2813a, 0, 16 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw51345g",   0xc2843a, 0, 16 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25um25645g",   0xc28039, 0, 8 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw25645g",   0xc28139, 0, 8 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25um25345g",   0xc28339, 0, 8 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw25345g",   0xc28439, 0, 8 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw12845g",   0xc28138, 0, 4 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw12a45g",   0xc28938, 0, 4 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw12345g",   0xc28438, 0, 4 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw6445g",0xc28137, 0, 2 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw6345g",0xc28437, 0, 2 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
 #endif
 
 #ifdef CONFIG_SPI_FLASH_STMICRO/* STMICRO */
-- 
2.17.1

[PATCH v4 3/4] mtd: spi-nor-core: set 4byte opcode when possible

Following linux kernel to check address width and 4byte flag to enable
4byte opcode setting.

Signed-off-by: JaimeLiao 
---
 drivers/mtd/spi/spi-nor-core.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 87a7de7d60..7d6671ade2 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -3902,6 +3902,10 @@ int spi_nor_scan(struct spi_nor *nor)
return -EINVAL;
}
 
+   /* Set 4byte opcodes when possible. */
+   if (nor->addr_width == 4 && info->flags & SPI_NOR_4B_OPCODES)
+   spi_nor_set_4byte_opcodes(nor, info);
+
/* Send all the required SPI flash commands to initialize device */
ret = spi_nor_init(nor);
if (ret)
-- 
2.17.1

[PATCH v4 2/4] mtd: spi-nor-core: Adding different type of command extension in Soft Reset

Power-on-Reset is a method to restore flash back to 1S-1S-1S mode from 8D-8D-8D
in the begging of probe.

Command extension type is not standardized across flash vendors in DTR mode.

For suiting different vendor flash devices, adding a flag to seperate types for
soft reset on boot.

Signed-off-by: JaimeLiao 
---
 drivers/mtd/spi/Kconfig| 7 +++
 drivers/mtd/spi/spi-nor-core.c | 7 ++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig
index 67599b32c9..8304d6c973 100644
--- a/drivers/mtd/spi/Kconfig
+++ b/drivers/mtd/spi/Kconfig
@@ -97,6 +97,13 @@ config SPI_FLASH_SMART_HWCAPS
 can support a type of operation in a much more refined way compared
 to using flags like SPI_RX_DUAL, SPI_TX_QUAD, etc.
 
+config SPI_NOR_BOOT_SOFT_RESET_EXT_INVERT
+   bool "Command extension type is INVERT for Software Reset on boot"
+   default n
+   help
+Because of SFDP information can not be get before boot.
+So define command extension type is INVERT when Software Reset on boot 
only.
+
 config SPI_FLASH_SOFT_RESET
bool "Software Reset support for SPI NOR flashes"
default n
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index fdaca0a0d3..87a7de7d60 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -3661,7 +3661,12 @@ static int spi_nor_soft_reset(struct spi_nor *nor)
enum spi_nor_cmd_ext ext;
 
ext = nor->cmd_ext_type;
-   nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
+   if (nor->cmd_ext_type == SPI_NOR_EXT_NONE) {
+   nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
+#if CONFIG_IS_ENABLED(SPI_NOR_BOOT_SOFT_RESET_EXT_INVERT)
+   nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
+#endif /* SPI_NOR_BOOT_SOFT_RESET_EXT_INVERT */
+   }
 
op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 0),
SPI_MEM_OP_NO_DUMMY,
-- 
2.17.1

[PATCH v4 1/4] mtd: spi-nor: macronix: add support for Macronix Octal

Follow patch "f6adec1af4b2f5d3012480c6cdce7743b74a6156" for adding
Macronix flash in Octal DTR mode.

Enable Octal DTR mode with 20 dummy cycles to allow running at the
maximum supported frequency.
 
-https://www.mxic.com.tw/Lists/Datasheet/Attachments/7841/MX25LM51245G,%203V,%20512Mb,%20v1.1.pdf

Signed-off-by: JaimeLiao 
---
 drivers/mtd/spi/Kconfig|  7 +++
 drivers/mtd/spi/spi-nor-core.c | 83 ++
 include/linux/mtd/spi-nor.h| 13 +-
 3 files changed, 101 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig
index 1b2ef37e92..67599b32c9 100644
--- a/drivers/mtd/spi/Kconfig
+++ b/drivers/mtd/spi/Kconfig
@@ -162,6 +162,13 @@ config SPI_FLASH_MACRONIX
help
  Add support for various Macronix SPI flash chips (MX25Lxxx)
 
+config SPI_FLASH_MACRONIX_OCTAL
+   bool "Macronix octal flash support"
+   depends on SPI_FLASH_MACRONIX
+   help
+Add support for the Macronix octal flash. This is a separate config
+because the fixup hooks octal enable for this flash.
+
 config SPI_FLASH_SPANSION
bool "Spansion SPI flash support"
help
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index d5d905fa5a..fdaca0a0d3 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -3489,6 +3489,85 @@ static struct spi_nor_fixups mt35xu512aba_fixups = {
 };
 #endif /* CONFIG_SPI_FLASH_MT35XU */
 
+#if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX_OCTAL)
+/**
+ * spi_nor_macronix_octal_dtr_enable() - set DTR OPI Enable bit in 
Configuration Register 2.
+ * @nor:   pointer to a 'struct spi_nor'
+ *
+ * Set the DTR OPI Enable (DOPI) bit in Configuration Register 2.
+ * Bit 2 of  Configuration Register 2 is the DOPI bit for Macronix like OPI 
memories.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor)
+{
+   struct spi_mem_op op;
+   int ret;
+   u8 buf;
+
+   ret = write_enable(nor);
+   if (ret)
+   return ret;
+
+   buf = SPINOR_REG_MXIC_DC_20;
+   op = (struct spi_mem_op)
+   SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1),
+  SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_DC, 1),
+  SPI_MEM_OP_NO_DUMMY,
+  SPI_MEM_OP_DATA_OUT(1, , 1));
+
+   ret = spi_mem_exec_op(nor->spi, );
+   if (ret)
+   return ret;
+
+   ret = spi_nor_wait_till_ready(nor);
+   if (ret)
+   return ret;
+
+   nor->read_dummy = MXIC_MAX_DC;
+   ret = write_enable(nor);
+   if (ret)
+   return ret;
+
+   buf = SPINOR_REG_MXIC_OPI_DTR_EN;
+   op = (struct spi_mem_op)
+   SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1),
+  SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_MODE, 1),
+  SPI_MEM_OP_NO_DUMMY,
+  SPI_MEM_OP_DATA_OUT(1, , 1));
+
+   ret = spi_mem_exec_op(nor->spi, );
+   if (ret) {
+   dev_err(nor->dev, "Failed to enable octal DTR mode\n");
+   return ret;
+   }
+   nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
+
+   return 0;
+}
+
+static void macronix_octal_default_init(struct spi_nor *nor)
+{
+   nor->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable;
+}
+
+static void macronix_octal_post_sfdp_fixup(struct spi_nor *nor,
+struct spi_nor_flash_parameter *params)
+{
+   /*
+* Adding SNOR_HWCAPS_PP_8_8_8_DTR in hwcaps.mask when
+* SPI_NOR_OCTAL_DTR_READ flag exists.
+*/
+   if (params->hwcaps.mask & SNOR_HWCAPS_READ_8_8_8_DTR)
+   params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR;
+}
+
+static struct spi_nor_fixups macronix_octal_fixups = {
+   .default_init = macronix_octal_default_init,
+   .post_sfdp = macronix_octal_post_sfdp_fixup,
+};
+#endif /* CONFIG_SPI_FLASH_MACRONIX_OCTAL */
+
 /** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed
  * @nor: pointer to a 'struct spi_nor'
  *
@@ -3655,6 +3734,10 @@ void spi_nor_set_fixups(struct spi_nor *nor)
if (!strcmp(nor->info->name, "mt35xu512aba"))
nor->fixups = _fixups;
 #endif
+
+#if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX_OCTAL)
+   nor->fixups = _octal_fixups;
+#endif /* SPI_FLASH_MACRONIX_OCTAL */
 }
 
 int spi_nor_scan(struct spi_nor *nor)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 7ddc4ba2bf..2ad579f66d 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -116,8 +116,17 @@
 #define XSR_RDYBIT(7)  /* Ready */
 
 /* Used for Macronix and Winbond flashes. */
-#define SPINOR_OP_EN4B 0xb7/* Enter 4-byte mode */
-#define SPINOR_OP_EX4B 0xe9/* Exit 4-byte mode */
+#define SPINOR_OP_EN4B 0xb7

[PATCH v4 0/4] Add octal DTR support for Macronix flash

This series add support for Macronix octal DTR flash, add flag for
Softreset with "INVERT" command extension type on boot and follow
linux kernel to enable 4byte opcode when possible.

v4:
  Add flag SPI_NOR_BOOT_SOFT_RESET_EXT_INVERT to seperate command extension
  types.
  Replace ifdef with CONFIG_IS_ENABLED and modify uncorrect descriptions.

v3:
  Add flag  SPI_NOR_CMD_EXT_INVERT to seperate command extension types.
  replace CONFIG_SPI_FLASH_MACRONIX with SPI_FLASH_MACRONIX_OCTAL for
  spi_nor_macronix_octal_dtr_enable function.
v2:
  add ret checking for write enable in spi_nor_macronix_octal_dtr_enable
  function.

JaimeLiao (4):
  mtd: spi-nor: macronix: add support for Macronix Octal
  mtd: spi-nor-core: Adding different type of command extension in Soft
Reset
  mtd: spi-nor-core: set 4byte opcode when possible
  mtd: spi-nor-core: Add support for Macronix Octal flash

 drivers/mtd/spi/Kconfig| 14 +
 drivers/mtd/spi/spi-nor-core.c | 94 +-
 drivers/mtd/spi/spi-nor-ids.c  | 22 +++-
 include/linux/mtd/spi-nor.h| 13 -
 4 files changed, 139 insertions(+), 4 deletions(-)

-- 
2.17.1

[PATCH] image.h: make image_sign_info.fit point to const

2021-10-18 Thread Hannu Lounento

The data blob apparently does not need to be modified through the fit
field of the image_sign_info struct so make it point to const to avoid
the need to cast away constness in functions that assign a pointer to
const data to the field.

fit_image_setup_verify already had to cast away constness as it assigned
a const void * argument to the field. The cast can now be removed.

Signed-off-by: Hannu Lounento 
---
 common/image-fit-sig.c | 2 +-
 include/image.h| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
index 4edebbf2d32..63e5423c925 100644
--- a/common/image-fit-sig.c
+++ b/common/image-fit-sig.c
@@ -85,7 +85,7 @@ static int fit_image_setup_verify(struct image_sign_info 
*info,
 
memset(info, '\0', sizeof(*info));
info->keyname = fdt_getprop(fit, noffset, FIT_KEY_HINT, NULL);
-   info->fit = (void *)fit;
+   info->fit = fit;
info->node_offset = noffset;
info->name = algo_name;
info->checksum = image_get_checksum_algo(algo_name);
diff --git a/include/image.h b/include/image.h
index 34d13ada84b..fd662e74b41 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1159,7 +1159,7 @@ struct image_sign_info {
const char *keydir; /* Directory conaining keys */
const char *keyname;/* Name of key to use */
const char *keyfile;/* Filename of private or public key */
-   void *fit;  /* Pointer to FIT blob */
+   const void *fit;/* Pointer to FIT blob */
int node_offset;/* Offset of signature node */
const char *name;   /* Algorithm name */
struct checksum_algo *checksum; /* Checksum algorithm information */
-- 
2.31.1

[PATCH] riscv: Avoid io read/write cause wrong result

2021-10-18 Thread Nick Hu

io read/write may cause wrong result because they may read/write data
from/to register instead of memory. Add 'volatile' to avoid it.

Signed-off-by: Nick Hu 
---
 arch/riscv/include/asm/io.h | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
index acf5a96449..3540773c4f 100644
--- a/arch/riscv/include/asm/io.h
+++ b/arch/riscv/include/asm/io.h
@@ -44,15 +44,15 @@ static inline phys_addr_t map_to_sysmem(const void *ptr)
  * read/writes.  We define __arch_*[bl] here, and leave __arch_*w
  * to the architecture specific code.
  */
-#define __arch_getb(a) (*(unsigned char *)(a))
-#define __arch_getw(a) (*(unsigned short *)(a))
-#define __arch_getl(a) (*(unsigned int *)(a))
-#define __arch_getq(a) (*(unsigned long long *)(a))
-
-#define __arch_putb(v, a)  (*(unsigned char *)(a) = (v))
-#define __arch_putw(v, a)  (*(unsigned short *)(a) = (v))
-#define __arch_putl(v, a)  (*(unsigned int *)(a) = (v))
-#define __arch_putq(v, a)  (*(unsigned long long *)(a) = (v))
+#define __arch_getb(a) (*(volatile unsigned char *)(a))
+#define __arch_getw(a) (*(volatile unsigned short *)(a))
+#define __arch_getl(a) (*(volatile unsigned int *)(a))
+#define __arch_getq(a) (*(volatile unsigned long long *)(a))
+
+#define __arch_putb(v, a)  (*(volatile unsigned char *)(a) = (v))
+#define __arch_putw(v, a)  (*(volatile unsigned short *)(a) = (v))
+#define __arch_putl(v, a)  (*(volatile unsigned int *)(a) = (v))
+#define __arch_putq(v, a)  (*(volatile unsigned long long *)(a) = 
(v))
 
 #define __raw_writeb(v, a) __arch_putb(v, a)
 #define __raw_writew(v, a) __arch_putw(v, a)
-- 
2.31.1

[PATCH] xilinx: zynqmp: Handle fallthrough statement properly

2021-10-18 Thread Michal Simek

Now intentional fallthrough in switch-case should be labelled that's why
convert comment.

Signed-off-by: Michal Simek 
---

 board/xilinx/zynqmp/zynqmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 064788c89db1..286874ab0597 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -711,7 +711,7 @@ int board_late_init(void)
break;
case SD1_LSHFT_MODE:
puts("LVL_SHFT_");
-   /* fall through */
+   fallthrough;
case SD_MODE1:
puts("SD_MODE1\n");
if (uclass_get_device_by_name(UCLASS_MMC,
-- 
2.33.1

Re: [PATCH 2/2] firmware: zynqmp: fix write to an uninitialised pointer in ipi_req()

2021-10-18 Thread Adrian Fiergolski

On 15.10.2021 16:57, Michal Simek wrote:
> When a caller is not interested in the returned message, the ret_payload
> pointer is set to NULL in the u-boot-sources. In this case, under EL3, the
> memory from address 0x0 would be overwritten by ipi_req() with the returned
> IPI message, damaging the original data under this address. The patch, in
> case ret_payload is NULL, assigns the pointer to the array holding the IPI
> message being sent.
>
> Signed-off-by: Adrian Fiergolski 
> Signed-off-by: Michal Simek 
Reviewed-by: Adrian Fiergolski 

Thanks,

Adrian

> ---
>
> Based on origin series from Adrian. That's why also adding his SoB line.
> https://lore.kernel.org/r/20211014124349.1429696-1-adrian.fiergol...@fastree3d.com
>
> Adrian: The patch is just suggestion how we could avoid that NULL pointer
> writes but done in ipi_req()
>
> ---
>  drivers/firmware/firmware-zynqmp.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/firmware/firmware-zynqmp.c 
> b/drivers/firmware/firmware-zynqmp.c
> index 1391aab0a160..7e498d8169e8 100644
> --- a/drivers/firmware/firmware-zynqmp.c
> +++ b/drivers/firmware/firmware-zynqmp.c
> @@ -30,6 +30,10 @@ static int ipi_req(const u32 *req, size_t req_len, u32 
> *res, size_t res_maxlen)
>  {
>   struct zynqmp_ipi_msg msg;
>   int ret;
> + u32 buffer[PAYLOAD_ARG_CNT];
> +
> + if (!res)
> + res = buffer;
>  
>   if (req_len > PMUFW_PAYLOAD_ARG_CNT ||
>   res_maxlen > PMUFW_PAYLOAD_ARG_CNT)

Re: [PATCH 1/2] firmware: zynqmp: Handle errors from ipi_req properly

2021-10-18 Thread Adrian Fiergolski



On 15.10.2021 16:57, Michal Simek wrote:
> There are multiple errors what can happen in ipi_req but they are not
> propagated properly. That's why propage all error properly.
>
> Signed-off-by: Michal Simek 

Reviewed-by: Adrian Fiergolski 

Thanks,

Adrian

> ---
>
>  drivers/firmware/firmware-zynqmp.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/firmware-zynqmp.c 
> b/drivers/firmware/firmware-zynqmp.c
> index c22bdca282fc..1391aab0a160 100644
> --- a/drivers/firmware/firmware-zynqmp.c
> +++ b/drivers/firmware/firmware-zynqmp.c
> @@ -165,6 +165,7 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 
> arg0, u32 arg1, u32 arg2,
>* firmware API is limited by the SMC call size
>*/
>   u32 regs[] = {api_id, arg0, arg1, arg2, arg3};
> + int ret;
>  
>   if (api_id == PM_FPGA_LOAD) {
>   /* Swap addr_hi/low because of incompatibility */
> @@ -174,7 +175,10 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 
> arg0, u32 arg1, u32 arg2,
>   regs[2] = temp;
>   }
>  
> - ipi_req(regs, PAYLOAD_ARG_CNT, ret_payload, PAYLOAD_ARG_CNT);
> + ret = ipi_req(regs, PAYLOAD_ARG_CNT, ret_payload,
> +   PAYLOAD_ARG_CNT);
> + if (ret)
> + return ret;
>  #else
>   return -EPERM;
>  #endif

i.MX LPDDR4 Firmware

2021-10-18 Thread Heiko Thiery

Hi all,

I see that in the NXP firmware-imx package different versions of the
LPDDR4 train firmware is included but I cannot find a Release Note or
something else that describes the differences. Does anyone know what
is the correct version to be used?

Here is the content of the latest NXP release:
lpddr4_pmu_train_1d_dmem_201904.bin
lpddr4_pmu_train_1d_dmem_202006.bin
lpddr4_pmu_train_1d_dmem.bin
lpddr4_pmu_train_1d_imem_201904.bin
lpddr4_pmu_train_1d_imem_202006.bin
lpddr4_pmu_train_1d_imem.bin
lpddr4_pmu_train_2d_dmem_201904.bin
lpddr4_pmu_train_2d_dmem_202006.bin
lpddr4_pmu_train_2d_dmem.bin
lpddr4_pmu_train_2d_imem_201904.bin
lpddr4_pmu_train_2d_imem_202006.bin
lpddr4_pmu_train_2d_imem.bin

Thanks
-- 
Heiko

Re: [PATCH v3 18/18] pxe: Allow calling the pxe_get logic directly


OK nice its will be usefull

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 17/18] doc: Move distro boot doc to rST


OK

Reviewed on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin

Re: [PATCH v3 16/18] pxe: Refactor sysboot to have one helper


OK is clear

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 15/18] pxe: Return the file size from the getfile() function


OK will be usefull

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 14/18] lib: Add a function to convert a string to a hex value


OK will be usefull

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 12/18] pxe: Drop get_bootfile_path()


OK

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 11/18] pxe: Clean up the use of bootfile


OK its really needed!

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 10/18] pxe: Move common parsing coding into pxe_util


OK

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 09/18] pxe: Tidy up code style a little in pxe_utils


OK

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 08/18] pxe: Tidy up some comments in pxe_utils


OK

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 07/18] pxe: Move pxe_utils files


OK

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 06/18] pxe: Tidy up the is_pxe global


OK nice

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 05/18] pxe: Add a userdata field to the context


OK nice

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 04/18] pxe: Move do_getfile() into the context


OK nice

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 03/18] pxe: Use a context pointer


OK nice

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 02/18] pxe: Move API comments to the header files


OK

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin

Re: [PATCH v3 01/18] Create a new boot/ directory




OK

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST 
Reviewed-by: Artem Lapkin 

Tested-by:  Artem Lapkin

Re: [PATCH v3 13/18] lib: Add tests for simple_itoa()