Re: [PATCH v2 0/8] An effort to bring DT bindings compliance within U-Boot
On 22/12/2023 07:12, Sumit Garg wrote: > Changes in v2: > -- > - Patch #1: excluded gitab CI config check and added commit description. > - Patch #3: s/UBOOT_DTSI_LOC/u_boot_dtsi_loc/ > - Patch #4: s/DEVICE_TREE_LOC/dt_dir/ and s/U-boot/U-Boot/ > - Patch #5: s/U-boot/U-Boot/ > - Patch #6 and #7: Picked up review tags > > Prerequisite > > > This patch series requires devicetree-rebasing git repo to be added as a > subtree to the main U-Boot repo via: > > $ git subtree add --prefix devicetree-rebasing \ > > git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git > \ > v6.6-dts --squash > > Background > -- > > This effort started while I was reviewing patch series corresponding to > Qcom platforms [1] which was about to import modified devicetree source > files from Linux kernel. I suppose keeping devicetree files sync with > Linux kernel without any DT bindings schema validation has been a pain > for U-Boot SoC/platform maintainers. There has been past discussions > about a single DT repo but that hasn't come up and Linux kernel remained > the place where DT source files as well as bindings are placed and > maintained. Thanks for doing this. I really suggest to store information that kernel DTS is directly re-used, thus DTS backward and forward compatibility matters, also in Linux kernel sources. The point is that sub-arch maintainers should be aware of it. I don't think that as DT maintainers we can efficiently keep an eye on it. Maybe create a subsystem profile and include it to maintainer entries of such affected platforms? Best regards, Krzysztof Best regards, Krzysztof
Re: [PATCH v3 5/7] mmc: bcmstb: Add support for bcm2712 SD controller
On 12-21 16:39, Stefan Wahren wrote: > To: Florian Fainelli , "Ivan T. Ivanov" > > > > On 12/18/2023 10:03 PM, Ivan T. Ivanov wrote: > > > Borrow SD quirks from vendor Linux driver. > > > > > > "BCM2712 unfortunately carries with it a perennial bug with the SD > > > controller register interface present on previous chips > > > (2711/2709/2708). > > > Accesses must be dword-sized and a read-modify-write cycle to the 32-bit > > > registers containing the COMMAND, TRANSFER_MODE, BLOCK_SIZE and > > > BLOCK_COUNT registers tramples the upper/lower 16 bits of data written. > > > BCM2712 does not seem to need the extreme delay between each write as on > > > previous chips, just the serialisation of writes to these registers in a > > > single 32-bit operation." > > > > > > Signed-off-by: Ivan T. Ivanov > > > > This is diverging from the Linux sdhci-brcmstb.c driver where no such > > quirk needs to be carried out, rather the logic for such quirks has > > been present in sdhci-iproc.c... > > it seems this patch based the downstream kernel changes [1]. Yep. > I would > suggest to use an existing driver which already handle this bug > (iproc_sdhci or bcm2835_sdhci). > > Does the Rpi 5 still works, if the compatible "brcm,bcm2712-sdhci" is > added to mmc/iproc_sdhci.c? No, it is not working :-) Even after I added shadow variables usage in iproc_readw, which do not have it. Even after I used hard coded values for "clock-freq-min-max", which are missing in RPi5 device tree. Even after I added 2712 specific "cfg" space initialization procedure from the brcnstb driver. On the other hand they are some tuning procedures in iproc driver which I am not sure that are relevant for 2712 controller. And the diff stat is getting bigger that equivalent for brcmstb driver. To me it is a bit easier to add 2712 support to brcmstb driver, because it will be easier to follow any vendor Linux driver changes. It looks like hardware engineers are just making the same mistake when integrating 2712 SDHCI controller as for IPROC controller. Regards, Ivan > > [1] - > https://github.com/raspberrypi/linux/commit/b627647c4500d39cb026924b608841fdf4d4d7e9
Re: [PATCH v2 8/8] dts: meson-gxbb: Drop redundant devicetree files
On 22/12/2023 07:12, Sumit Garg wrote:
Since meson-gxbb based boards switched to using upstream DT, so drop
redundant files from arch/arm/dts directory. Only *-u-boot.dtsi files
kept in arch/arm/dts directory for these boards.
Signed-off-by: Sumit Garg
---
arch/arm/dts/Makefile | 8 -
arch/arm/dts/meson-gxbb-kii-pro.dts | 140
arch/arm/dts/meson-gxbb-nanopi-k2.dts | 415
arch/arm/dts/meson-gxbb-odroidc2.dts| 418
arch/arm/dts/meson-gxbb-p200.dts| 100 ---
arch/arm/dts/meson-gxbb-p201.dts| 26 -
arch/arm/dts/meson-gxbb-p20x.dtsi | 250 ---
arch/arm/dts/meson-gxbb-wetek-hub.dts | 58 --
arch/arm/dts/meson-gxbb-wetek-play2.dts | 119
arch/arm/dts/meson-gxbb-wetek.dtsi | 292
arch/arm/dts/meson-gxbb.dtsi| 856
11 files changed, 2682 deletions(-)
delete mode 100644 arch/arm/dts/meson-gxbb-kii-pro.dts
delete mode 100644 arch/arm/dts/meson-gxbb-nanopi-k2.dts
delete mode 100644 arch/arm/dts/meson-gxbb-odroidc2.dts
delete mode 100644 arch/arm/dts/meson-gxbb-p200.dts
delete mode 100644 arch/arm/dts/meson-gxbb-p201.dts
delete mode 100644 arch/arm/dts/meson-gxbb-p20x.dtsi
delete mode 100644 arch/arm/dts/meson-gxbb-wetek-hub.dts
delete mode 100644 arch/arm/dts/meson-gxbb-wetek-play2.dts
delete mode 100644 arch/arm/dts/meson-gxbb-wetek.dtsi
delete mode 100644 arch/arm/dts/meson-gxbb.dtsi
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 5fc888680b3..45bd1166029 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -212,14 +212,6 @@ dtb-$(CONFIG_ARCH_MESON) += \
meson-a1-ad401.dtb \
meson-axg-s400.dtb \
meson-axg-jethome-jethub-j100.dtb \
- meson-gxbb-kii-pro.dtb \
- meson-gxbb-nanopi-k2.dtb \
- meson-gxbb-odroidc2.dtb \
- meson-gxbb-nanopi-k2.dtb \
- meson-gxbb-p200.dtb \
- meson-gxbb-p201.dtb \
- meson-gxbb-wetek-hub.dtb \
- meson-gxbb-wetek-play2.dtb \
meson-gxl-s805x-libretech-ac.dtb \
meson-gxl-s905d-libretech-pc.dtb \
meson-gxl-s905w-jethome-jethub-j80.dtb \
diff --git a/arch/arm/dts/meson-gxbb-kii-pro.dts
b/arch/arm/dts/meson-gxbb-kii-pro.dts
deleted file mode 100644
index e238f1f1012..000
--- a/arch/arm/dts/meson-gxbb-kii-pro.dts
+++ /dev/null
@@ -1,140 +0,0 @@
-// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
-/*
- * Copyright (c) 2019 Mohammad Rasim
- */
-
-/dts-v1/;
-
-#include "meson-gxbb-p20x.dtsi"
-#include
-#include
-#include
-#include
-
-/ {
- compatible = "videostrong,kii-pro", "amlogic,meson-gxbb";
- model = "Videostrong KII Pro";
-
- spdif_dit: audio-codec-0 {
- #sound-dai-cells = <0>;
- compatible = "linux,spdif-dit";
- status = "okay";
- sound-name-prefix = "DIT";
- };
-
- leds {
- compatible = "gpio-leds";
- led {
- gpios = <&gpio_ao GPIOAO_13 GPIO_ACTIVE_LOW>;
- color = ;
- function = LED_FUNCTION_STATUS;
- default-state = "off";
- };
- };
-
- gpio-keys-polled {
- compatible = "gpio-keys-polled";
- poll-interval = <20>;
-
- button-reset {
- label = "reset";
- linux,code = ;
- gpios = <&gpio_ao GPIOAO_3 GPIO_ACTIVE_HIGH>;
- };
- };
-
- sound {
- compatible = "amlogic,gx-sound-card";
- model = "KII-PRO";
- assigned-clocks = <&clkc CLKID_MPLL0>,
- <&clkc CLKID_MPLL1>,
- <&clkc CLKID_MPLL2>;
- assigned-clock-parents = <0>, <0>, <0>;
- assigned-clock-rates = <294912000>,
- <270950400>,
- <393216000>;
-
- dai-link-0 {
- sound-dai = <&aiu AIU_CPU CPU_I2S_FIFO>;
- };
-
- dai-link-1 {
- sound-dai = <&aiu AIU_CPU CPU_SPDIF_FIFO>;
- };
-
- dai-link-2 {
- sound-dai = <&aiu AIU_CPU CPU_I2S_ENCODER>;
- dai-format = "i2s";
- mclk-fs = <256>;
-
- codec-0 {
- sound-dai = <&aiu AIU_HDMI CTRL_I2S>;
- };
- };
-
- dai-link-3 {
- sound-dai = <&aiu AIU_CPU CPU_SPDIF_ENCODER>;
-
- codec-0 {
- sound-dai = <&spdif_dit>;
- };
- };
-
- dai-link-4 {
- sound-dai = <&aiu AIU_HDMI CTRL_OUT>;
-
- codec-0 {
-
Re: [PATCH 0/7] Add SE HMBSC board support
On Thu, 21 Dec 2023 at 19:09, Caleb Connolly wrote:
>
>
>
> On 20/12/2023 13:36, Sumit Garg wrote:
> > On Tue, 19 Dec 2023 at 21:56, Caleb Connolly
> > wrote:
> >>
> >>
> >>
> >> On 19/12/2023 06:25, Sumit Garg wrote:
> >>> Hi Simon,
> >>>
> >>> On Mon, 18 Dec 2023 at 20:32, Simon Glass wrote:
>
> Hi Sumit,
>
> On Mon, 18 Dec 2023 at 00:24, Sumit Garg wrote:
> >
> > SE HMIBSC board is based on Qcom APQ8016 SoC. One of the major
>
> Could you please add a doc/ file for this board and explain how to
> build it and how to run U-Boot on it?
> >>>
> >>> Ah I forgot to add that since the build/boot instructions are quite
> >>> similar to db410c. BTW, I will add that in the next spin.
> >>
> >> Probably just referencing that document or making the language generic
> >> would be good.
> >>
> >
> > I will rename that doc to be rather SoC specific with board specific
> > sub-sections.
> >
> >> Sumit, could you rebase this series on my generic board support? [1] in
> >> it's current form this series conflicts, and includes some of the major
> >> anti-patterns I'm trying to move away from in mach-snapdragon.
> >
> > Although, I haven't gone through your series but I was expecting those
> > conflicts. Let's work together to make this series compatible on top
> > of yours.
> >
> >>
> >> You should not have to introduce a new CONFIG_TARGET_XYZ variable, and
> >> from what I can tell you shouldn't even need to add the board/schneider
> >> directory at all, you can just set the following in your defconfig:
> >>
> >> CONFIG_SYS_BOARD="dragonboard410c"
> >
> > This is simply a misnomer, its HMIBSC board. I suppose we should
> > rather separate the SoC specific bits into mach-snapdragon and let
> > different boards use them.
>
> Is there any reason why you can't just use the existing db410c board
> code as-is? I don't see why you want to duplicate it.
I don't want to duplicate it but rather we should make the common
parts as part of arch/arm/mach-snapdragon/ and let derivative boards
use them. The HMIBSC board is an industrial board which doesn't need
any generic distro boot but rather FIT booting with OTA updates via
RAUC [1] along with U-boot environment protection. Doesn't that make
it different from db410c?
[1] https://rauc.io/
>
> The only reason the db410c and db820c have their board code is because
> they're old platforms and already supported. For adding new support
> there needs to be some very strong justification to have board-specific
> C code.
>
> I think it would be nice to make the db410c code go away, or be toggled
> at runtime, probably most of it will just work and not break any other
> boards anyway. The db820c code is just part of what should be in the
> pinctrl driver...
>
> Let's move away from this old model and towards having more generic
> U-Boot images. This will snowball towards making future bringup even easier.
As I have mentioned in the other thread, we should give alternatives
to the board code as well. How would you handle the following?
- Fastboot mode entry on a button press.
- Configure MAC address for network support.
- Setup board serial number.
I suppose we don't need #ifdefry in the
arch/arm/mach-snapdragon/board.c file, right?
> >
> >> CONFIG_SYS_CONFIG_NAME="hmibsc"
> >>
> >> This will use the db410c board code (which yours is just a copy/paste of
> >> from what I can tell) and your custom include/configs/hmibsc.h header.
> >>
> >> The addresses set in your environment file should be allocated
> >> automatically at runtime too (see the ("mach-snapdragon: dynamic load
> >> addresses") patch).
> >>
> >> You should also switch to an upstream board DTS based on my series, and
> >> drop the "-uboot.dtsi" file.
> >
> > Unfortunately, currently there isn't any upstream DTS for this board
> > but I will check with the SE team regarding their plans. Until then we
> > have to use a U-Boot specific DTS file.
>
> In that case, perhaps we can take whatever DTS they're using in
> production, or a version of it, and at least split the U-Boot changes
> (if any) out into a separate file as I've done with the other platforms.
> This way we stay consistent and can keep track of what U-Boot specific
> DTS changes we need.
The first step for that is to land that DTS file upstream in the Linux
kernel and then we should make a switch. The middle ground here won't
be maintainable.
>
> I guess this is all new territory for us, but imho if we're adding
> support for a board that doesn't have an upstream DTS, we should still
> follow the same model, open to input here.
With the DT rebasing tree, we actually want to make these bits clear.
Either the board support is fully upstream and then we make a switch
to upstream or you can have a u-boot specific DTS subset compliant
with upstream bindings while upstreaming is in progress. There is
always ABI risk involved for using half baked board DTS files.
-Sumit
> >
> > -Sumit
> >
> >>
> >> [1]:
> >> https://lo
Re: [RFC PATCH 00/16] Introduce ICSSG Ethernet driver
Hi Roger,
On 20/12/23 3:29 pm, Roger Quadros wrote:
> Hi,
>
> On 19/12/2023 12:11, MD Danish Anwar wrote:
>> Introduce ICSSG PRUETH support in uboot. The ICSSG driver is used in TI
>> AM654 SR2.0.
>>
>> The ICSSG PRU Sub-system runs on EMAC firmware. This series Introduces
>> support for ICSSG driver in uboot. This series also adds the driver's
>> dependencies.
>>
>> The ICSSG2 node is added in device tree overlay so that it remains in
>> sync with linux kernel.
>>
>> The series introduces device tree and config changes and AM65x
>> to enable ICSSG driver. The series also enables SPL_LOAD_FIT_APPLY_OVERLAY
>> for AM65x in order to load overlay over spl.
>>
>> This series has been tested on AM65x SR2.0, and the ICSSG interface is
>> able to ping / dhcp and boot kernel using tftp in uboot.
>>
>> To use ICSSG2 ethernet, the ICSSG firmware needs to be loaded to PRU RPROC
>> cores and RPROC cores need to be booted with the firmware. This step is
>> done inside driver in kernel as kernel supports APIs like
>> rproc_set_firmware() and rproc_fw_boot(). But as u-boot doesn't have these
>> APIs, the same needs to be done via u-boot cmds.
>>
>> To make sure icssg-eth works we need to do below steps.
>>
>> 1. Initialize rproc cores i.e. rproc_init()
>> 2. Load $firmware_file from partition '1:2' (root) on device (mmc in this
>>example)
>> 3. Load the firmware file to rproc cores passing. i.e. rproc_load()
>>taking rproc_id, loadaddr and file size as arguments.
>> 4. Start rproc cores. i.e. rproc_start() taking rproc_id as arguments
>>
>> The above steps are done by running the below commands at u-boot prompt.
>>
>> => setenv start_icssg2 'rproc start 14; rproc start 15; rproc start 16;
>> rproc start 17; rproc start 18; rproc start 19'
>> => setenv stop_icssg2 'rproc stop 14; rproc stop 15; rproc stop 16; rproc
>> stop 17; rproc stop 18; rproc stop 19'
>> => setenv firmware_dir '/lib/firmware/ti-pruss'
>> => setenv get_firmware_mmc 'load mmc ${bootpart} ${loadaddr}
>> ${firmware_dir}/${firmware_file}'
>>
>> => setenv init_icssg2 'setenv ethact icssg2-eth; setenv autoload no; rproc
>> init; setenv loadaddr 0x8000; \
>> setenv firmware_file am65x-sr2-pru0-prueth-fw.elf; run get_firmware_mmc;
>> rproc load 14 0x8000 ${filesize}; \
>> setenv loadaddr 0x8900; setenv firmware_file
>> am65x-sr2-rtu0-prueth-fw.elf; run get_firmware_mmc; rproc load 15 0x8900
>> ${filesize}; \
>> setenv loadaddr 0x9000; setenv firmware_file
>> am65x-sr2-txpru0-prueth-fw.elf; run get_firmware_mmc; rproc load 16
>> 0x9000 ${filesize}; \
>> setenv loadaddr 0x8000; setenv firmware_file
>> am65x-sr2-pru1-prueth-fw.elf; run get_firmware_mmc; rproc load 17 0x8000
>> ${filesize}; \
>> setenv loadaddr 0x8900; setenv firmware_file
>> am65x-sr2-rtu1-prueth-fw.elf; run get_firmware_mmc; rproc load 18 0x8900
>> ${filesize}; \
>> setenv loadaddr 0x9000; setenv firmware_file
>> am65x-sr2-txpru1-prueth-fw.elf; run get_firmware_mmc; rproc load 19
>> 0x9000 ${filesize}; \
>> run start_icssg2;'
>
> A whole bunch of commands are required to get ethernet functional.
> This is not at all user friendly and will be a maintenance nightmare.
> What worries me is tracking the 6 different rproc cores and the 6 different
> firmware files to start 1 ethernet device.
> This will get even more interesting when we have to deal with different ICSSG
> instances on different boards.
>
> What is preventing the driver from starting the rproc cores it needs so user
> doesn't need to care about it?
> All the necessary information is in the Device tree. At least this is how it
> is done on Linux.
>
I tried removing the need for these commands and implementing them
inside the driver only. I am able to load the firmware from driver using
the fs_loader API request_firmware_into_buf(). It requires changes to
dt. A DT node called fs-loader needs to be added also CONFIG_FS_LOADER
needs to enabled. In the DT node we need to specify the storage media
that we are using i.e. mmc, ospi, usb. It's upto user to modify the
storage media, the driver will take the media from DT and try to laod
firmware from their.
For loading firmwares to rproc cores, rproc_load() API is needed. Now
this API takes rproc_id, loadaddr and firmware_size as arguments.
loadaddr is fixed for all three pru cores. firmware_size is obtained
from request_firmware_into_buf() but I couldn't find a way to get the
rproc_id. For now based on the ICSSG instance and slice number I am
figuring out the rproc_id and passing it to rproc_load() and
rproc_start() APIs. Please let me know if you have any other / better
way of finding rproc_id.
Below is the entire diff to remove these commands and move their
functionality to driver. Please have a look and let me know if this is ok.
Save New Duplicate & Edit Just Text Twitter
diff --git a/arch/arm/dts/k3-am654-base-board.dts
b/arch/arm/dts/k3-am654-base-board.dts
index cfbcebfa37.
Re: [PATCH v2 6/8] MAINTAINERS: Add myself as devicetree-rebasing maintainer
On Fri, 22 Dec 2023 at 08:13, Sumit Garg wrote: > > Reviewed-by: Simon Glass > Signed-off-by: Sumit Garg > --- > MAINTAINERS | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 969514468cb..253092c345c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -951,6 +951,12 @@ F: cmd/cyclic.c > F: common/cyclic.c > F: include/cyclic.h > > +DEVICETREE REBASING SUBTREE > +M: Sumit Garg > +S: Maintained > +F: devicetree-rebasing/ > +F: dts/arch/ > + > DFU > M: Lukasz Majewski > M: Mattijs Korpershoek > -- > 2.34.1 > Reviewed-by: Ilias Apalodimas
Re: New TPM commands.
Hi Niek,
On Thu, 21 Dec 2023 at 04:12, niek.nooij...@omron.com
wrote:
>
> Hi Ilias
>
> I implemented the feedback and checked the htmldocs build target. no error
> there.
> I also applied the patch to the latest github version (was using an older
> one, but no compatibility issues) and used git format-patch instead of
> git-diff. The checkpatch script now shows no errors.
> There's no real reson to "want" this to be merged, but as our company
> benefits greatly from projects like u-boot, I figured that the least we could
> do was contribute back to the project. We use the TPM's non-volatile memory
> to lock away keys and other secrets and I don't think we're the only one
> needing this.
Thanks! With vacation ahead I'll need sometime to review this, but
I'll respond here if I need any more changes.
/Ilias
>
> anyhow here's the new revision. if I can help with more things I would gladly
> hear from you.
>
> ===BEGIN PATCH=
> From afd377e2aba6df46bc991c0f250bf67d4ad036e0 Mon Sep 17 00:00:00 2001
> From: Niek Nooijens
> Date: Thu, 21 Dec 2023 10:56:14 +0900
> Subject: [PATCH] add tpm nv_commands
>
> Needed to read and write to the TPM's NV memory, in which you can store
> keys and other secret information, which can be locked behind PCR
> policies.
>
> Signed-off-by: Niek Nooijens
> ---
> cmd/tpm-v2.c | 152 +++
> include/tpm-v2.h | 15 +
> lib/tpm-v2.c | 67 -
> 3 files changed, 219 insertions(+), 15 deletions(-)
>
> diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
> index 7e479b9dfe..0d1e9e8e5c 100644
> --- a/cmd/tpm-v2.c
> +++ b/cmd/tpm-v2.c
> @@ -356,6 +356,136 @@ static int do_tpm_pcr_setauthvalue(struct cmd_tbl
> *cmdtp, int flag,
> key, key_sz));
> }
>
> +static int do_tpm_nv_define(struct cmd_tbl *cmdtp, int flag,
> + int argc, char *const argv[])
> +{
> + struct udevice *dev;
> + struct tpm_chip_priv *priv;
> + u32 nv_addr, nv_size, nv_attributes, rc;
> + void *policy_addr = NULL;
> + size_t policy_size = 0;
> + int ret;
> +
> + nv_attributes = 0;
> +
> + if ((argc < 3 && argc > 6) || argc == 4)
> + return CMD_RET_USAGE;
> +
> + ret = get_tpm(&dev);
> + if (ret)
> + return ret;
> +
> + priv = dev_get_uclass_priv(dev);
> + if (!priv)
> + return -EINVAL;
> +
> + nv_addr = simple_strtoul(argv[1], NULL, 0);
> +
> + nv_size = simple_strtoul(argv[2], NULL, 0);
> +
> + if (argc > 3)
> + nv_attributes = simple_strtoul(argv[3], NULL, 0);
> + else
> + nv_attributes = TPMA_NV_PLATFORMCREATE | TPMA_NV_OWNERWRITE |
> TPMA_NV_OWNERREAD | TPMA_NV_PPWRITE | TPMA_NV_PPREAD;
> +
> + if (argc > 4) {
> + policy_addr = map_sysmem(simple_strtoul(argv[4], NULL, 0), 0);
> + if ((nv_attributes & (TPMA_NV_POLICYREAD | TPMA_NV_POLICYWRITE))
> == 0) {
> + printf("ERROR: policy provided, but TPMA_NV_POLICYREAD or
> TPMA_NV_POLICYWRITE are NOT set!\n");
> + return CMD_RET_FAILURE;
> + }
> + policy_size = simple_strtoul(argv[5], NULL, 0);
> + }
> +
> + rc = tpm2_nv_define_space(dev, nv_addr, nv_size, nv_attributes,
> policy_addr, policy_size);
> +
> + if (rc)
> + printf("ERROR: nv_define #%u returns: 0x%x\n", nv_addr, rc);
> +
> + if (policy_addr)
> + unmap_sysmem(policy_addr);
> +
> + return report_return_code(rc);
> +}
> +
> +static int do_tpm_nv_undefine(struct cmd_tbl *cmdtp, int flag,
> + int argc, char *const argv[])
> +{
> + struct udevice *dev;
> + u32 nv_addr, ret, rc;
> +
> + ret = get_tpm(&dev);
> + if (ret)
> + return ret;
> +
> + if (argc != 2)
> + return CMD_RET_USAGE;
> +
> + nv_addr = simple_strtoul(argv[1], NULL, 0);
> + rc = tpm2_nv_undefine_space(dev, nv_addr);
> +
> + return report_return_code(rc);
> +}
> +
> +static int do_tpm_nv_read_value(struct cmd_tbl *cmdtp, int flag,
> + int argc, char *const argv[])
> +{
> + struct udevice *dev;
> + u32 nv_addr, nv_size, rc;
> + int ret;
> + void *out_data;
> +
> + ret = get_tpm(&dev);
> + if (ret)
> + return ret;
> +
> + if (argc != 4)
> + return CMD_RET_USAGE;
> +
> + nv_addr = simple_strtoul(argv[1], NULL, 0);
> +
> + nv_size = simple_strtoul(argv[2], NULL, 0);
> +
> + out_data = map_sysmem(simple_strtoul(argv[3], NULL, 0), 0);
> +
> + rc = tpm2_nv_read_value(dev, nv_addr, out_data, nv_size);
> +
> + if (rc)
> + printf("ERROR: nv_read #%u returns: #%u\n", nv_addr, rc);
> +
> + unmap_sysmem(out_data);
> + return report_return_code(rc);
> +}
> +
> +static int do_tpm_nv_write_value(struct cmd_tbl *cmdtp, int flag,
> + int argc, char *const argv[])
> +{
>
Re: [PATCH v2 1/9] bloblist: add API to check the register conventions
Hi Raymond,
On Thu, 21 Dec 2023 at 02:40, Raymond Mao wrote:
>
> Add bloblist_check_reg_conv() to check whether the bloblist is compliant
> to the register conventions defined in Firmware Handoff specification.
> This API can be used for all Arm platforms.
>
> Signed-off-by: Raymond Mao
> ---
> Changes in v2
> - Refactor of bloblist_check_reg_conv().
>
> common/bloblist.c | 13 +
> include/bloblist.h | 12
> 2 files changed, 25 insertions(+)
>
> diff --git a/common/bloblist.c b/common/bloblist.c
> index 625e480f6b..193122a8af 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -542,3 +542,16 @@ int bloblist_maybe_init(void)
>
> return 0;
> }
> +
> +int bloblist_check_reg_conv(ulong rfdt, ulong rzero)
> +{
> + if (!IS_ENABLED(CONFIG_OF_BOARD))
> + return 0;
> +
> + if (rzero || rfdt != (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT, 0)) {
> + gd->bloblist = NULL; /* Reset the gd bloblist pointer */
> + return -EIO;
> + }
> +
> + return 0;
> +}
The function looks correct, but the name is a bit off. There are no
registers conventions check AFAICT. We are just comparing 2 addresses.
Am I missing anything?
Thanks
/Ilias
> diff --git a/include/bloblist.h b/include/bloblist.h
> index 84fc943819..b5d0f147f6 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -461,4 +461,16 @@ static inline int bloblist_maybe_init(void)
> }
> #endif /* BLOBLIST */
>
> +/**
> + * bloblist_check_reg_conv() - Check whether the bloblist is compliant to
> + *the register conventions according to the
> + *Firmware Handoff spec.
> + *
> + * @rfdt: Register that holds the FDT base address.
> + * @rzero: Register that must be zero.
> + * Return: 0 if OK, -EIO if the bloblist is not compliant to the register
> + *conventions.
> + */
> +int bloblist_check_reg_conv(ulong rfdt, ulong rzero);
> +
> #endif /* __BLOBLIST_H */
> --
> 2.25.1
>
Re: [PATCH v2 2/9] bloblist: check bloblist with specified buffer size
On Thu, 21 Dec 2023 at 02:40, Raymond Mao wrote:
>
> Instead of expecting the bloblist total size to be the same as the
> pre-allocated buffer size, practically we are more interested in
> whether the pre-allocated buffer size is bigger than the bloblist
> total size.
>
> Signed-off-by: Raymond Mao
> ---
> Changes in v2
> - New patch file created for v2.
>
> common/bloblist.c | 2 +-
> test/bloblist.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/common/bloblist.c b/common/bloblist.c
> index 193122a8af..be2050611a 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -384,7 +384,7 @@ int bloblist_check(ulong addr, uint size)
> return log_msg_ret("Bad magic", -ENOENT);
> if (hdr->version != BLOBLIST_VERSION)
> return log_msg_ret("Bad version", -EPROTONOSUPPORT);
> - if (!hdr->total_size || (size && hdr->total_size != size))
> + if (!hdr->total_size || (size && hdr->total_size > size))
> return log_msg_ret("Bad total size", -EFBIG);
> if (hdr->used_size > hdr->total_size)
> return log_msg_ret("Bad used size", -ENOENT);
> diff --git a/test/bloblist.c b/test/bloblist.c
> index 17d9dd03d0..7dab9addf8 100644
> --- a/test/bloblist.c
> +++ b/test/bloblist.c
> @@ -207,7 +207,7 @@ static int bloblist_test_checksum(struct unit_test_state
> *uts)
> hdr->flags++;
>
> hdr->total_size--;
> - ut_asserteq(-EFBIG, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
> + ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
> hdr->total_size++;
>
> hdr->spare++;
> --
> 2.25.1
>
Reviewed-by: Ilias Apalodimas
Re: [PATCH v3 2/7] rpi5: Use devicetree as alternative way to read IO base addresses
On 18/12/2023 22:03, Ivan T. Ivanov wrote:
From: Dmitry Malkin
MBOX and Watchdog on RPi5/bcm2712 has a different base IO offsets.
Find them via devicetree blob passed by bootloader.
Signed-off-by: Dmitry Malkin
Signed-off-by: Ivan T. Ivanov
Reviewed-by: Matthias Brugger
---
arch/arm/mach-bcm283x/include/mach/base.h | 5 ++-
arch/arm/mach-bcm283x/include/mach/mbox.h | 3 +-
arch/arm/mach-bcm283x/include/mach/sdhci.h | 3 +-
arch/arm/mach-bcm283x/include/mach/timer.h | 3 +-
arch/arm/mach-bcm283x/include/mach/wdog.h | 3 +-
arch/arm/mach-bcm283x/init.c | 43 ++
6 files changed, 43 insertions(+), 17 deletions(-)
diff --git a/arch/arm/mach-bcm283x/include/mach/base.h
b/arch/arm/mach-bcm283x/include/mach/base.h
index 4ccaf69693..6de99e7ea1 100644
--- a/arch/arm/mach-bcm283x/include/mach/base.h
+++ b/arch/arm/mach-bcm283x/include/mach/base.h
@@ -6,7 +6,10 @@
#ifndef _BCM283x_BASE_H_
#define _BCM283x_BASE_H_
-extern unsigned long rpi_bcm283x_base;
+extern unsigned long rpi_mbox_base;
+extern unsigned long rpi_timer_base;
+extern unsigned long rpi_sdhci_base;
+extern unsigned long rpi_wdog_base;
#ifdef CONFIG_ARMV7_LPAE
#ifdef CONFIG_TARGET_RPI_4_32B
diff --git a/arch/arm/mach-bcm283x/include/mach/mbox.h
b/arch/arm/mach-bcm283x/include/mach/mbox.h
index 490664f878..35d4e2f075 100644
--- a/arch/arm/mach-bcm283x/include/mach/mbox.h
+++ b/arch/arm/mach-bcm283x/include/mach/mbox.h
@@ -38,8 +38,7 @@
/* Raw mailbox HW */
-#define BCM2835_MBOX_PHYSADDR ({ BUG_ON(!rpi_bcm283x_base); \
-rpi_bcm283x_base + 0xb880; })
+#define BCM2835_MBOX_PHYSADDR rpi_mbox_base
struct bcm2835_mbox_regs {
u32 read;
diff --git a/arch/arm/mach-bcm283x/include/mach/sdhci.h
b/arch/arm/mach-bcm283x/include/mach/sdhci.h
index 7323690687..e837c679c4 100644
--- a/arch/arm/mach-bcm283x/include/mach/sdhci.h
+++ b/arch/arm/mach-bcm283x/include/mach/sdhci.h
@@ -8,8 +8,7 @@
#include
-#define BCM2835_SDHCI_PHYSADDR ({ BUG_ON(!rpi_bcm283x_base); \
- rpi_bcm283x_base + 0x0030; })
+#define BCM2835_SDHCI_PHYSADDR rpi_sdhci_base
int bcm2835_sdhci_init(u32 regbase, u32 emmc_freq);
diff --git a/arch/arm/mach-bcm283x/include/mach/timer.h b/arch/arm/mach-bcm283x/include/mach/timer.h
index 5567dbd7f3..60500a256d 100644
--- a/arch/arm/mach-bcm283x/include/mach/timer.h
+++ b/arch/arm/mach-bcm283x/include/mach/timer.h
@@ -11,8 +11,7 @@
#include
#endif
-#define BCM2835_TIMER_PHYSADDR ({ BUG_ON(!rpi_bcm283x_base); \
- rpi_bcm283x_base + 0x3000; })
+#define BCM2835_TIMER_PHYSADDR rpi_timer_base
#define BCM2835_TIMER_CS_M3 (1 << 3)
#define BCM2835_TIMER_CS_M2 (1 << 2)
diff --git a/arch/arm/mach-bcm283x/include/mach/wdog.h
b/arch/arm/mach-bcm283x/include/mach/wdog.h
index 9942666720..b950560674 100644
--- a/arch/arm/mach-bcm283x/include/mach/wdog.h
+++ b/arch/arm/mach-bcm283x/include/mach/wdog.h
@@ -8,8 +8,7 @@
#include
-#define BCM2835_WDOG_PHYSADDR ({ BUG_ON(!rpi_bcm283x_base); \
-rpi_bcm283x_base + 0x0010; })
+#define BCM2835_WDOG_PHYSADDR rpi_wdog_base
struct bcm2835_wdog_regs {
u32 unknown0[7];
diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c
index af23b9711a..1c5c748484 100644
--- a/arch/arm/mach-bcm283x/init.c
+++ b/arch/arm/mach-bcm283x/init.c
@@ -151,7 +151,11 @@ static void rpi_update_mem_map(void)
static void rpi_update_mem_map(void) {}
#endif
-unsigned long rpi_bcm283x_base = 0x3f00;
+/* Default bcm283x devices addresses */
+unsigned long rpi_mbox_base = 0x3f00b880;
+unsigned long rpi_sdhci_base = 0x3f30;
+unsigned long rpi_wdog_base = 0x3f10;
+unsigned long rpi_timer_base = 0x3f003000;
int arch_cpu_init(void)
{
@@ -162,22 +166,45 @@ int arch_cpu_init(void)
int mach_cpu_init(void)
{
- int ret, soc_offset;
+ int ret, soc, offset;
u64 io_base, size;
rpi_update_mem_map();
/* Get IO base from device tree */
- soc_offset = fdt_path_offset(gd->fdt_blob, "/soc");
- if (soc_offset < 0)
- return soc_offset;
+ soc = fdt_path_offset(gd->fdt_blob, "/soc");
+ if (soc < 0)
+ return soc;
- ret = fdt_read_range((void *)gd->fdt_blob, soc_offset, 0, NULL,
- &io_base, &size);
+ ret = fdt_read_range((void *)gd->fdt_blob, soc, 0, NULL,
+&io_base, &size);
if (ret)
return ret;
- rpi_bcm283x_base = io_base;
+ rpi_mbox_base = io_base + 0x00b880;
+ rpi_sdhci_base = io_base + 0x30;
+ rpi_wdog_base = io_base + 0x10;
+ rpi_timer_base = io_base + 0x003000;
+
+ offset = fdt_node_offset_by_compatible(gd->fdt_blob, soc,
+ "brcm,bcm2835-mbox");
+ if (offse
Re: [PATCH v2 3/9] bloblist: refactor of bloblist_reloc()
Hi Raymond,
On Thu, 21 Dec 2023 at 02:41, Raymond Mao wrote:
>
> The current bloblist pointer and size can be retrieved from global
> data, so we don't need to pass them from the function arguments.
> This change also help to remove all external access of gd->bloblist
> outside of bloblist module.
>
> Signed-off-by: Raymond Mao
> ---
[...]
> }
> }
>
> -void bloblist_reloc(void *to, uint to_size, void *from, uint from_size)
> +void bloblist_reloc(void *to, uint to_size)
> {
> struct bloblist_hdr *hdr;
>
> - memcpy(to, from, from_size);
> + memcpy(to, gd->bloblist, gd->bloblist->total_size);
> hdr = to;
> - hdr->total_size = to_size;
> + if (to_size < gd->bloblist->total_size)
What's the size of *to? Is it equal to to_size?
Because if to_size can be smaller that gd->bloblist->total_size the
memcpy above is wrong
> + hdr->total_size = gd->bloblist->total_size;
> + else
> + hdr->total_size = to_size;
> + gd->bloblist = to;
> }
>
> int bloblist_init(void)
> diff --git a/common/board_f.c b/common/board_f.c
> index d4d7d01f8f..00b0430889 100644
> --- a/common/board_f.c
[...]
/Ilias
Re: [PATCH v2 6/9] qemu-arm: Get bloblist from boot arguments
> #endif
>
> +/* Boot parameters saved from start.S */
> +extern unsigned long saved_args[];
> +
> int board_init(void)
> {
> return 0;
> @@ -144,6 +148,35 @@ void *board_fdt_blob_setup(int *err)
> return (void *)CFG_SYS_SDRAM_BASE;
> }
>
> +int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
> +{
> + int ret = -ENOENT;
> + unsigned long reg_fdt;
> + unsigned long reg_zero;
> +
> + if (!IS_ENABLED(CONFIG_OF_BOARD) || !IS_ENABLED(CONFIG_BLOBLIST))
> + return -ENOENT;
> +
> + ret = bloblist_check(saved_args[3], size);
> + if (ret)
> + return ret;
> +
> + if (IS_ENABLED(CONFIG_ARM64)) {
> + reg_fdt = saved_args[0];
> + reg_zero = saved_args[2];
> + } else {
> + reg_fdt = saved_args[2];
> + reg_zero = saved_args[0];
I think it's better if we fix up the order in the low-level asm code.
Store the variables in the 'correct' order there and get rid of this
if
[...]
/Ilias
Re: [PATCH v3 3/7] rpi5: Use devicetree to retrieve board revision
On 18/12/2023 22:03, Ivan T. Ivanov wrote:
Firmware on RPi5 return error on board revision query
through firmware interface, but on the other hand it fills
"linux,revision" in "system" node, so use it to detect board
revision.
system {
linux,revision = <0xc04170>;
linux,serial = <0x6cf44e80 0x3c533ede>;
};
Signed-off-by: Ivan T. Ivanov
Reviewed-by: Matthias Brugger
---
board/raspberrypi/rpi/rpi.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index cd823ad746..2851ebc985 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -171,6 +171,11 @@ static const struct rpi_model rpi_models_new_scheme[] = {
DTB_DIR "bcm2711-rpi-cm4.dtb",
true,
},
+ [0x17] = {
+ "5 Model B",
+ DTB_DIR "bcm2712-rpi-5-b.dtb",
+ true,
+ },
};
static const struct rpi_model rpi_models_old_scheme[] = {
@@ -429,15 +434,27 @@ static void get_board_revision(void)
int ret;
const struct rpi_model *models;
uint32_t models_count;
+ ofnode node;
BCM2835_MBOX_INIT_HDR(msg);
BCM2835_MBOX_INIT_TAG(&msg->get_board_rev, GET_BOARD_REV);
ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &msg->hdr);
if (ret) {
- printf("bcm2835: Could not query board revision\n");
/* Ignore error; not critical */
- return;
+ node = ofnode_path("/system");
+ if (!ofnode_valid(node)) {
+ printf("bcm2835: Could not find /system node\n");
+ return;
+ }
+
+ ret = ofnode_read_u32(node, "linux,revision", &revision);
+ if (ret) {
+ printf("bcm2835: Could not find linux,revision\n");
+ return;
+ }
+ } else {
+ revision = msg->get_board_rev.body.resp.rev;
}
/*
@@ -451,7 +468,6 @@ static void get_board_revision(void)
*
http://www.raspberrypi.org/forums/viewtopic.php?f=63&t=98367&start=250
* http://www.raspberrypi.org/forums/viewtopic.php?f=31&t=20594
*/
- revision = msg->get_board_rev.body.resp.rev;
if (revision & 0x80) {
rev_scheme = 1;
rev_type = (revision >> 4) & 0xff;
Re: [PATCH v3 7/7] pci: pcie-brcmstb: Add bcm2712 PCIe controller support
On 18/12/2023 22:03, Ivan T. Ivanov wrote:
PCIe controller have minor register map difference compared
to bcm2711 variant. Handle this using device specific register
offset.
Signed-off-by: Ivan T. Ivanov
Reviewed-by: Matthias Brugger
---
drivers/pci/pcie_brcmstb.c | 23 +++
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pcie_brcmstb.c b/drivers/pci/pcie_brcmstb.c
index cd45f0bee9..d63e715b2e 100644
--- a/drivers/pci/pcie_brcmstb.c
+++ b/drivers/pci/pcie_brcmstb.c
@@ -90,7 +90,6 @@
#define PCIE_MEM_WIN0_LIMIT_HI(win) \
PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI + ((win) * 8)
-#define PCIE_MISC_HARD_PCIE_HARD_DEBUG 0x4204
#define PCIE_HARD_DEBUG_SERDES_IDDQ_MASK 0x0800
#define PCIE_MSI_INTR2_CLR0x4508
@@ -131,6 +130,10 @@
#define SSC_STATUS_PLL_LOCK_MASK 0x800
#define SSC_STATUS_PLL_LOCK_SHIFT 11
+struct pcie_cfg_data {
+ unsigned long hard_debug_offs;
+};
+
/**
* struct brcm_pcie - the PCIe controller state
* @base: Base address of memory mapped IO registers of the controller
@@ -141,6 +144,7 @@
struct brcm_pcie {
void __iomem*base;
+ struct pcie_cfg_data *cfg;
int gen;
boolssc;
};
@@ -458,7 +462,7 @@ static int brcm_pcie_probe(struct udevice *dev)
/* Take the bridge out of reset */
clrbits_le32(base + PCIE_RGR1_SW_INIT_1, RGR1_SW_INIT_1_INIT_MASK);
- clrbits_le32(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG,
+ clrbits_le32(base + pcie->cfg->hard_debug_offs,
PCIE_HARD_DEBUG_SERDES_IDDQ_MASK);
/* Wait for SerDes to be stable */
@@ -599,7 +603,7 @@ static int brcm_pcie_remove(struct udevice *dev)
setbits_le32(base + PCIE_RGR1_SW_INIT_1, RGR1_SW_INIT_1_PERST_MASK);
/* Turn off SerDes */
- setbits_le32(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG,
+ setbits_le32(base + pcie->cfg->hard_debug_offs,
PCIE_HARD_DEBUG_SERDES_IDDQ_MASK);
/* Shutdown bridge */
@@ -620,6 +624,8 @@ static int brcm_pcie_of_to_plat(struct udevice *dev)
if (!pcie->base)
return -EINVAL;
+ pcie->cfg = (struct pcie_cfg_data *)dev_get_driver_data(dev);
+
pcie->ssc = ofnode_read_bool(dn, "brcm,enable-ssc");
ret = ofnode_read_u32(dn, "max-link-speed", &max_link_speed);
@@ -636,8 +642,17 @@ static const struct dm_pci_ops brcm_pcie_ops = {
.write_config = brcm_pcie_write_config,
};
+static const struct pcie_cfg_data bcm2711_cfg = {
+ .hard_debug_offs= 0x4204
+};
+
+static const struct pcie_cfg_data bcm2712_cfg = {
+ .hard_debug_offs= 0x4304
+};
+
static const struct udevice_id brcm_pcie_ids[] = {
- { .compatible = "brcm,bcm2711-pcie" },
+ { .compatible = "brcm,bcm2711-pcie", .data = (ulong)&bcm2711_cfg },
+ { .compatible = "brcm,bcm2712-pcie", .data = (ulong)&bcm2712_cfg },
{ }
};
Re: Proposal: U-Boot memory management
Hi Simon I'll respond to the rest more thoroughly but I since I caught this early, [...] > > 5. Avoid calling efi_allocate_pages() and efi_allocate_pool() outside > boot-time services. This solves the problem 6. If memory is needed by > an app, allocate it with malloc() and see 3. There are only two > efi_allocate_pages() (smbios and efi_runtime). There are more calls of > efi_allocate_pool(), but most of these seem easy to fix up. For > example, efi_init_event_log() allocates a buffer, but this can be > allocated in normal malloc() space or in a bloblist. The TCG event log is only valid in the EFI world and is described by the EFI spec extensions [0]. I prefer it to remain as is > > 6. Don't worry too much about whether EFI will be used for booting. > The cost is likely not that great: use bootstage to measure it as is > done for driver model. Try to minmise the cost of its tables, > particularly for execution time, but otherwise just rely on the > ability to disable EFI_LOADER. > > – > > Regards, > Simon [0] https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf Thanks /Ilias
Re: [PATCH v3] arch: arm: Kconfig: Enable BOOTSTD_FULL for Rockchip SoCs
Hi all, On Mon, Dec 11, 2023 at 6:27 PM Tom Rini wrote: > > On Mon, Dec 11, 2023 at 11:22:45AM -0700, Simon Glass wrote: > > Hi Tom, > [snip] > > > I think in hind-sight too much stuff is omitted without BOOTSTD_FULL. > > > The option itself then enables other stuff too by default, but some > > > parts of the bootflow command itself should be visible even without FULL > > > to make things easier on the user. > > > > At the time the goal was to avoid growth compared to the distro > > scripts. We could perhaps add some more things in with BOOTSTD_FULL > > but still have it as an option? > > Right. But now that we've tried this, some of the feedback has been that > it's just too minimal right now. Like looking at the help message for > bootflow, list and info should probably always be available. And maybe > the flags for "scan" should be re-thought? Too late to change things now > but "bootflow scan -b" should maybe how it's always been for "scan and > boot". What would be the preferred approach for this patch? Is it to update the default capabilities of bootflow or we can have this patch? Thanks Shantur > > -- > Tom
Re: [PATCH v3] arch: arm: Kconfig: Enable BOOTSTD_FULL for Rockchip SoCs
On Fri, Dec 22, 2023 at 12:05:39PM +, Shantur Rathore wrote: > Hi all, > > On Mon, Dec 11, 2023 at 6:27 PM Tom Rini wrote: > > > > On Mon, Dec 11, 2023 at 11:22:45AM -0700, Simon Glass wrote: > > > Hi Tom, > > [snip] > > > > I think in hind-sight too much stuff is omitted without BOOTSTD_FULL. > > > > The option itself then enables other stuff too by default, but some > > > > parts of the bootflow command itself should be visible even without FULL > > > > to make things easier on the user. > > > > > > At the time the goal was to avoid growth compared to the distro > > > scripts. We could perhaps add some more things in with BOOTSTD_FULL > > > but still have it as an option? > > > > Right. But now that we've tried this, some of the feedback has been that > > it's just too minimal right now. Like looking at the help message for > > bootflow, list and info should probably always be available. And maybe > > the flags for "scan" should be re-thought? Too late to change things now > > but "bootflow scan -b" should maybe how it's always been for "scan and > > boot". > > What would be the preferred approach for this patch? > Is it to update the default capabilities of bootflow or we can have this > patch? Well, I don't see a problem with just adding this to the platforms which enable BOOTSTD_FULL until we can rework what's included/excluded by that flag, and there's an issue filed for it now. -- Tom signature.asc Description: PGP signature
Re: [PATCH v3] arch: arm: Kconfig: Enable BOOTSTD_FULL for Rockchip SoCs
On Fri, Dec 22, 2023 at 12:07 PM Tom Rini wrote: > > On Fri, Dec 22, 2023 at 12:05:39PM +, Shantur Rathore wrote: > > Hi all, > > > > On Mon, Dec 11, 2023 at 6:27 PM Tom Rini wrote: > > > > > > On Mon, Dec 11, 2023 at 11:22:45AM -0700, Simon Glass wrote: > > > > Hi Tom, > > > [snip] > > > > > I think in hind-sight too much stuff is omitted without BOOTSTD_FULL. > > > > > The option itself then enables other stuff too by default, but some > > > > > parts of the bootflow command itself should be visible even without > > > > > FULL > > > > > to make things easier on the user. > > > > > > > > At the time the goal was to avoid growth compared to the distro > > > > scripts. We could perhaps add some more things in with BOOTSTD_FULL > > > > but still have it as an option? > > > > > > Right. But now that we've tried this, some of the feedback has been that > > > it's just too minimal right now. Like looking at the help message for > > > bootflow, list and info should probably always be available. And maybe > > > the flags for "scan" should be re-thought? Too late to change things now > > > but "bootflow scan -b" should maybe how it's always been for "scan and > > > boot". > > > > What would be the preferred approach for this patch? > > Is it to update the default capabilities of bootflow or we can have this > > patch? > > Well, I don't see a problem with just adding this to the platforms which > enable BOOTSTD_FULL until we can rework what's included/excluded by that > flag, and there's an issue filed for it now. > > -- > Tom Great, can we have this merged in please then [0] [0] - https://patchwork.ozlabs.org/project/uboot/patch/20231119172310.1307942-...@shantur.com/ Regards, Shantur
Re: [PATCH v3 0/7] rpi5: initial support
Hi Ivan, On 2023-12-18 21:03, Ivan T. Ivanov wrote: Hi, These patches are adding basic support for RPi5. They are based on v2 series from Dmitry Malkin[1]. With them I am able to _start_ current openSUSE Tumbleweed without modification. They are still a lot of things to be added to the upstream Linux before it runs flawlessly on this device, but at least in U-Booot SD controller used for uSD card and Frameboffer and HDMI0 devices are working fine now. It seems that PCIe controller is working fine too, but I have not tested it too much. Serial console and reset are also functional. Hopefully this will help others add missing pieces more easily. So I've given this a go, and the basics (serial) worked out of the box (thanks for that!) after performing the memory-map change you described in your reply to patch 1. However, I don't get anything on the PCIe front: U-Boot> pci enum PCIe BRCM: link down PCIe BRCM: link down despite having an nvme device connected and enabled (the RPi kernel finds it). I'm guessing I must be missing something. How did you test PCIe? Thanks, M. -- Jazz is not dead. It just smells funny...
Re: [PATCH v2 7/8] dts: meson-gxbb: Switch to using upstream DT
On Fri, Dec 22, 2023 at 11:42:07AM +0530, Sumit Garg wrote: > Although there were still some variations in board DTS files based on > meson-gxbb SoC but I think those were minor differences from upstream > and shouldn't impact boot on these devices. > > So switch to upstream DT via mirroring amlogic/ directory from > devicetree-rebasing/src/arm64/amlogic/ directory. And thereby directly > building DTB from there including *-u-boot.dtsi files from > arch/$(ARCH)/dts/ directory. > > Reviewed-by: Neil Armstrong > Signed-off-by: Sumit Garg > --- > configs/nanopi-k2_defconfig | 3 ++- > configs/odroid-c2_defconfig | 3 ++- > configs/p200_defconfig| 3 ++- > configs/p201_defconfig| 3 ++- > configs/videostrong-kii-pro_defconfig | 3 ++- > configs/wetek-hub_defconfig | 3 ++- > configs/wetek-play2_defconfig | 3 ++- > dts/arch/arm64/Makefile | 9 + > dts/arch/arm64/amlogic| 1 + > 9 files changed, 24 insertions(+), 7 deletions(-) > create mode 12 dts/arch/arm64/amlogic Do we need all of the Makefile portions of this? One of the things that works today is that trees listed in ONFIG_DEFAULT_DEVICE_TREE (and a few other variables) will be automatically built. We might just need the new Makefiles to just have: include $(srctree)/scripts/Makefile.dts and then the trees a given config will need will be built. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/8] CI: Exclude devicetree-rebasing subtree for CONFIG checks
On Fri, Dec 22, 2023 at 11:42:01AM +0530, Sumit Garg wrote: > Since devicetree-rebasing is an external repo with its own coding style, > exclude it from Azure and gitlab CI CONFIG checks. > > Signed-off-by: Sumit Garg Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 3/8] scripts/Makefile.lib: Statically define *-u-boot.dtsi files location
On Fri, Dec 22, 2023 at 11:42:03AM +0530, Sumit Garg wrote: > Allow u-boot to build DTB from a different directory tree such that > *-u-boot.dtsi files can be included from a common location. Currently > that location is arch/$(ARCH)/dts/, so statically define that common > location. > > This is needed for platform owners to start building DTB files from > devicetree-rebasing directory but still being able to include > *-u-boot.dtsi files. > > Signed-off-by: Sumit Garg Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 0/7] rpi5: initial support
On 12-22 12:19, Marc Zyngier wrote: > > Hi Ivan, > > On 2023-12-18 21:03, Ivan T. Ivanov wrote: > > Hi, > > > > These patches are adding basic support for RPi5. > > They are based on v2 series from Dmitry Malkin[1]. > > > > With them I am able to _start_ current openSUSE > > Tumbleweed without modification. They are still > > a lot of things to be added to the upstream Linux > > before it runs flawlessly on this device, but at > > least in U-Booot SD controller used for uSD card > > and Frameboffer and HDMI0 devices are working fine > > now. It seems that PCIe controller is working fine > > too, but I have not tested it too much. > > > > Serial console and reset are also functional. > > > > Hopefully this will help others add missing pieces > > more easily. > > So I've given this a go, and the basics (serial) worked > out of the box (thanks for that!) after performing the > memory-map change you described in your reply to patch 1. > > However, I don't get anything on the PCIe front: > > U-Boot> pci enum > PCIe BRCM: link down > PCIe BRCM: link down > > despite having an nvme device connected and enabled > (the RPi kernel finds it). > > I'm guessing I must be missing something. How did you > test PCIe? > No, you are not missing anything. I have not tested it for real. That is why I said "it seems". Sorry. Yesterday I started looking more closely at what's missing and unfortunately a lot more changes are needed. Regards, Ivan
Re: [PATCH v2 5/8] doc: devicetree: Updates for devicetree-rebasing subtree
On Fri, Dec 22, 2023 at 11:42:05AM +0530, Sumit Garg wrote: > Encourage SoC/board maintainers to migrate to using devicetree-rebasing > subtree and maintain a regular sync with Linux kernel devicetree files > and bindings. > > Along with that add documentation regarding how to run DT bindings > schema checks. > > Signed-off-by: Sumit Garg > --- > doc/develop/devicetree/control.rst | 108 +++-- > 1 file changed, 86 insertions(+), 22 deletions(-) [snip] > @@ -81,18 +94,21 @@ Failing that, you could write one from scratch yourself! > Configuration > - > > -Use:: > +Traditionally, U-Boot placed copies of devicetree source files from Linux > +kernel into `arch//dts/.dts` which can be selected via:: > > - #define CONFIG_DEFAULT_DEVICE_TREE"" > +#define CONFIG_DEFAULT_DEVICE_TREE "" Oh, oof. This never got updated for Kconfig rather than config.h. Can you please include re-wording this to be setting when prompted for DEFAULT_DEVICE_TREE by Kconfig? And the rest of the text you're updating as well to match, thanks. -- Tom signature.asc Description: PGP signature
Re: [RFC PATCH 00/16] Introduce ICSSG Ethernet driver
On 22/12/2023 12:26, MD Danish Anwar wrote:
> Hi Roger,
>
> On 20/12/23 3:29 pm, Roger Quadros wrote:
>> Hi,
>>
>> On 19/12/2023 12:11, MD Danish Anwar wrote:
>>> Introduce ICSSG PRUETH support in uboot. The ICSSG driver is used in TI
>>> AM654 SR2.0.
>>>
>>> The ICSSG PRU Sub-system runs on EMAC firmware. This series Introduces
>>> support for ICSSG driver in uboot. This series also adds the driver's
>>> dependencies.
>>>
>>> The ICSSG2 node is added in device tree overlay so that it remains in
>>> sync with linux kernel.
>>>
>>> The series introduces device tree and config changes and AM65x
>>> to enable ICSSG driver. The series also enables SPL_LOAD_FIT_APPLY_OVERLAY
>>> for AM65x in order to load overlay over spl.
>>>
>>> This series has been tested on AM65x SR2.0, and the ICSSG interface is
>>> able to ping / dhcp and boot kernel using tftp in uboot.
>>>
>>> To use ICSSG2 ethernet, the ICSSG firmware needs to be loaded to PRU RPROC
>>> cores and RPROC cores need to be booted with the firmware. This step is
>>> done inside driver in kernel as kernel supports APIs like
>>> rproc_set_firmware() and rproc_fw_boot(). But as u-boot doesn't have these
>>> APIs, the same needs to be done via u-boot cmds.
>>>
>>> To make sure icssg-eth works we need to do below steps.
>>>
>>> 1. Initialize rproc cores i.e. rproc_init()
>>> 2. Load $firmware_file from partition '1:2' (root) on device (mmc in this
>>>example)
>>> 3. Load the firmware file to rproc cores passing. i.e. rproc_load()
>>>taking rproc_id, loadaddr and file size as arguments.
>>> 4. Start rproc cores. i.e. rproc_start() taking rproc_id as arguments
>>>
>>> The above steps are done by running the below commands at u-boot prompt.
>>>
>>> => setenv start_icssg2 'rproc start 14; rproc start 15; rproc start 16;
>>> rproc start 17; rproc start 18; rproc start 19'
>>> => setenv stop_icssg2 'rproc stop 14; rproc stop 15; rproc stop 16; rproc
>>> stop 17; rproc stop 18; rproc stop 19'
>>> => setenv firmware_dir '/lib/firmware/ti-pruss'
>>> => setenv get_firmware_mmc 'load mmc ${bootpart} ${loadaddr}
>>> ${firmware_dir}/${firmware_file}'
>>>
>>> => setenv init_icssg2 'setenv ethact icssg2-eth; setenv autoload no; rproc
>>> init; setenv loadaddr 0x8000; \
>>> setenv firmware_file am65x-sr2-pru0-prueth-fw.elf; run
>>> get_firmware_mmc; rproc load 14 0x8000 ${filesize}; \
>>> setenv loadaddr 0x8900; setenv firmware_file
>>> am65x-sr2-rtu0-prueth-fw.elf; run get_firmware_mmc; rproc load 15
>>> 0x8900 ${filesize}; \
>>> setenv loadaddr 0x9000; setenv firmware_file
>>> am65x-sr2-txpru0-prueth-fw.elf; run get_firmware_mmc; rproc load 16
>>> 0x9000 ${filesize}; \
>>> setenv loadaddr 0x8000; setenv firmware_file
>>> am65x-sr2-pru1-prueth-fw.elf; run get_firmware_mmc; rproc load 17
>>> 0x8000 ${filesize}; \
>>> setenv loadaddr 0x8900; setenv firmware_file
>>> am65x-sr2-rtu1-prueth-fw.elf; run get_firmware_mmc; rproc load 18
>>> 0x8900 ${filesize}; \
>>> setenv loadaddr 0x9000; setenv firmware_file
>>> am65x-sr2-txpru1-prueth-fw.elf; run get_firmware_mmc; rproc load 19
>>> 0x9000 ${filesize}; \
>>> run start_icssg2;'
>>
>> A whole bunch of commands are required to get ethernet functional.
>> This is not at all user friendly and will be a maintenance nightmare.
>> What worries me is tracking the 6 different rproc cores and the 6 different
>> firmware files to start 1 ethernet device.
>> This will get even more interesting when we have to deal with different
>> ICSSG instances on different boards.
>>
>> What is preventing the driver from starting the rproc cores it needs so user
>> doesn't need to care about it?
>> All the necessary information is in the Device tree. At least this is how it
>> is done on Linux.
>>
>
> I tried removing the need for these commands and implementing them
> inside the driver only. I am able to load the firmware from driver using
> the fs_loader API request_firmware_into_buf(). It requires changes to
> dt. A DT node called fs-loader needs to be added also CONFIG_FS_LOADER
> needs to enabled. In the DT node we need to specify the storage media
> that we are using i.e. mmc, ospi, usb. It's upto user to modify the
> storage media, the driver will take the media from DT and try to laod
> firmware from their.
>
> For loading firmwares to rproc cores, rproc_load() API is needed. Now
> this API takes rproc_id, loadaddr and firmware_size as arguments.
> loadaddr is fixed for all three pru cores. firmware_size is obtained
> from request_firmware_into_buf() but I couldn't find a way to get the
> rproc_id. For now based on the ICSSG instance and slice number I am
> figuring out the rproc_id and passing it to rproc_load() and
> rproc_start() APIs. Please let me know if you have any other / better
> way of finding rproc_id.
>
> Below is the entire diff to remove these commands and move their
> functionality to driver. Please have a look and let me know
Re: [PATCH v3 0/7] rpi5: initial support
On 2023-12-22 12:33, Ivan T. Ivanov wrote: On 12-22 12:19, Marc Zyngier wrote: Hi Ivan, On 2023-12-18 21:03, Ivan T. Ivanov wrote: > Hi, > > These patches are adding basic support for RPi5. > They are based on v2 series from Dmitry Malkin[1]. > > With them I am able to _start_ current openSUSE > Tumbleweed without modification. They are still > a lot of things to be added to the upstream Linux > before it runs flawlessly on this device, but at > least in U-Booot SD controller used for uSD card > and Frameboffer and HDMI0 devices are working fine > now. It seems that PCIe controller is working fine > too, but I have not tested it too much. > > Serial console and reset are also functional. > > Hopefully this will help others add missing pieces > more easily. So I've given this a go, and the basics (serial) worked out of the box (thanks for that!) after performing the memory-map change you described in your reply to patch 1. However, I don't get anything on the PCIe front: U-Boot> pci enum PCIe BRCM: link down PCIe BRCM: link down despite having an nvme device connected and enabled (the RPi kernel finds it). I'm guessing I must be missing something. How did you test PCIe? No, you are not missing anything. I have not tested it for real. That is why I said "it seems". Sorry. Yesterday I started looking more closely at what's missing and unfortunately a lot more changes are needed. Ah, fair enough. I was slightly surprised that it appeared to be working out of the box, which would have been a first on the ARM side... Happy to test whatever patches you come up with! Thanks, M. -- Jazz is not dead. It just smells funny...
Re: [PATCH v7 2/2] schemas: Add some common reserved-memory usages
On Thu, 21 Dec 2023 at 17:50, Chiu, Chasel wrote: > > > Hi Ard, > > Please see my reply below inline and let me know your thoughts. > > Thanks, > Chasel > > > > -Original Message- > > From: Ard Biesheuvel > > Sent: Thursday, December 21, 2023 6:31 AM > > To: Chiu, Chasel > > Cc: Simon Glass ; devicet...@vger.kernel.org; Mark > > Rutland > > ; Rob Herring ; Tan, Lean Sheng > > ; lkml ; Dhaval > > Sharma ; Brune, Maximilian > > ; Yunhui Cui ; > > Dong, Guo ; Tom Rini ; ron minnich > > ; Guo, Gua ; linux- > > a...@vger.kernel.org; U-Boot Mailing List > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory > > usages > > > > On Tue, 28 Nov 2023 at 21:31, Chiu, Chasel wrote: > > > > > > > > > > > > > > > > -Original Message- > > > > From: Ard Biesheuvel > > > > Sent: Tuesday, November 28, 2023 10:08 AM > > > > To: Chiu, Chasel > > > > Cc: Simon Glass ; devicet...@vger.kernel.org; Mark > > > > Rutland ; Rob Herring ; Tan, > > > > Lean Sheng ; lkml > > > > ; Dhaval Sharma ; > > > > Brune, Maximilian ; Yunhui Cui > > > > ; Dong, Guo ; Tom Rini > > > > ; ron minnich ; Guo, Gua > > > > ; linux- a...@vger.kernel.org; U-Boot Mailing > > > > List > > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory > > > > usages > > > > > > > > You are referring to a 2000 line patch so it is not 100% clear where to > > > > look tbh. > > > > > > > > > > > > On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel > > > > wrote: > > > > > > > > > > > > > > > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, line > > > > > 268 is for > > > > related example code. > > > > > > > > > > > > > That refers to a 'memory-allocation' node, right? How does that > > > > relate to the 'reserved-memory' node? > > > > > > > > And crucially, how does this clarify in which way "runtime-code" and > > > > "runtime- data" reservations are being used? > > > > > > > > Since the very beginning of this discussion, I have been asking > > > > repeatedly for examples that describe the wider context in which these > > reservations are used. > > > > The "runtime" into runtime-code and runtime-data means that these > > > > regions have a special significance to the operating system, not > > > > just to the next bootloader stage. So I want to understand exactly > > > > why it is necessary to describe these regions in a way where the > > > > operating system might be expected to interpret this information and act > > upon it. > > > > > > > > > > > > > I think runtime code and data today are mainly for supporting UEFI runtime > > services - some BIOS functions for OS to utilize, OS may follow below ACPI > > spec to > > treat them as reserved range: > > > https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces.html# > > > uefi-memory-types-and-mapping-to-acpi-address-range-types > > > > > > Like I mentioned earlier, that PR is still in early phase and has not > > > reflected all > > the required changes yet, but the idea is to build > > gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes. > > > UEFI generic Payload has DxeMain integrated, however Memory Types are > > platform-specific, for example, some platforms may need bigger runtime > > memory > > for their implementation, that's why we want such FDT reserved-memory node > > to > > tell DxeMain. > > > > > > > > The Payload flow will be like this: > > > Payload creates built-in default MemoryTypes table -> > > > FDT reserved-memory node to override if required (this also ensures > > > the > > same memory map cross boots so ACPI S4 works) -> > > > Build gEfiMemoryTypeInformationGuid HOB by "platfom specific" > > MemoryTypes Table -> > > > DxeMain/GCD to consume this MemoryTypes table and setup memory > > service -> > > > Install memory types table to UEFI system table.Configuration > > > table... > > > > > > Note: if Payload built-in default MemoryTypes table works fine for the > > > platform, then FDT reserved-memory node does not need to provide such > > 'usage' compatible strings. (optional) This FDT node could allow > > flexibility/compatibility without rebuilding Payload binary. > > > > > > Not sure if I answered all your questions, please highlight which area > > > you need > > more information. > > > > > > > The gEfiMemoryTypeInformationGuid HOB typically carries platform defaults, > > and > > the actual memory type information is kept in a non-volatile EFI variable, > > which > > gets updated when the memory usage changes. Is this different for > > UefiPayloadPkg? > > > > (For those among the cc'ees less versed in EFI/EDK2: when you get the > > 'config > > changed -rebooting' message from the boot firmware, it typically means that > > this > > memory type table has changed, and a reboot is necessary.) > > > > So the platform init needs to read this variable, or get the information in > > a > > different way. I assume it is the payload, not the platform init that > > updates the > > v
Re: [PATCH v2 7/8] dts: meson-gxbb: Switch to using upstream DT
On Fri, 22 Dec 2023 at 18:01, Tom Rini wrote: > > On Fri, Dec 22, 2023 at 11:42:07AM +0530, Sumit Garg wrote: > > > Although there were still some variations in board DTS files based on > > meson-gxbb SoC but I think those were minor differences from upstream > > and shouldn't impact boot on these devices. > > > > So switch to upstream DT via mirroring amlogic/ directory from > > devicetree-rebasing/src/arm64/amlogic/ directory. And thereby directly > > building DTB from there including *-u-boot.dtsi files from > > arch/$(ARCH)/dts/ directory. > > > > Reviewed-by: Neil Armstrong > > Signed-off-by: Sumit Garg > > --- > > configs/nanopi-k2_defconfig | 3 ++- > > configs/odroid-c2_defconfig | 3 ++- > > configs/p200_defconfig| 3 ++- > > configs/p201_defconfig| 3 ++- > > configs/videostrong-kii-pro_defconfig | 3 ++- > > configs/wetek-hub_defconfig | 3 ++- > > configs/wetek-play2_defconfig | 3 ++- > > dts/arch/arm64/Makefile | 9 + > > dts/arch/arm64/amlogic| 1 + > > 9 files changed, 24 insertions(+), 7 deletions(-) > > create mode 12 dts/arch/arm64/amlogic > > Do we need all of the Makefile portions of this? One of the things that > works today is that trees listed in ONFIG_DEFAULT_DEVICE_TREE (and a few > other variables) will be automatically built. We might just need the new > Makefiles to just have: > include $(srctree)/scripts/Makefile.dts > and then the trees a given config will need will be built. That sounds fine for Amlogic SoC and many others too with CONFIG_DEFAULT_DEVICE_TREE="" CONFIG_OF_LIST="" options available. However, if in future we have truely generalized U-Boot (like efforts for Qcom platform) then CONFIG_OF_LIST="" may become an overloaded choice. For now, let me drop the Makefile portions. -Sumit > > -- > Tom
Re: [PATCH v2 0/8] An effort to bring DT bindings compliance within U-Boot
On Fri, 22 Dec 2023 at 13:48, Krzysztof Kozlowski wrote: > > On 22/12/2023 07:12, Sumit Garg wrote: > > Changes in v2: > > -- > > - Patch #1: excluded gitab CI config check and added commit description. > > - Patch #3: s/UBOOT_DTSI_LOC/u_boot_dtsi_loc/ > > - Patch #4: s/DEVICE_TREE_LOC/dt_dir/ and s/U-boot/U-Boot/ > > - Patch #5: s/U-boot/U-Boot/ > > - Patch #6 and #7: Picked up review tags > > > > Prerequisite > > > > > > This patch series requires devicetree-rebasing git repo to be added as a > > subtree to the main U-Boot repo via: > > > > $ git subtree add --prefix devicetree-rebasing \ > > > > git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git > > \ > > v6.6-dts --squash > > > > Background > > -- > > > > This effort started while I was reviewing patch series corresponding to > > Qcom platforms [1] which was about to import modified devicetree source > > files from Linux kernel. I suppose keeping devicetree files sync with > > Linux kernel without any DT bindings schema validation has been a pain > > for U-Boot SoC/platform maintainers. There has been past discussions > > about a single DT repo but that hasn't come up and Linux kernel remained > > the place where DT source files as well as bindings are placed and > > maintained. > > Thanks for doing this. > > I really suggest to store information that kernel DTS is directly > re-used, thus DTS backward and forward compatibility matters, also in > Linux kernel sources. The point is that sub-arch maintainers should be > aware of it. I don't think that as DT maintainers we can efficiently > keep an eye on it. Maybe create a subsystem profile and include it to > maintainer entries of such affected platforms? > >From U-Boot point of view, currently we have the config option: "CONFIG_OF_UPSTREAM=y" per platform which means directly re-use of kernel DTS. So U-Boot sub-arch maintainers should be aware of platforms which get converted to re-use kernel DTS. I suppose we have to relay information to kernel sub-arch maintainers who aren't the same as maintaining U-Boot counterparts. How about adding U-Boot ML to CC for whichever DT change gets submitted in the kernel? Otherwise adding U-Boot sub-arch maintainers as reviewers for corresponding kernel DT changes works too if that's acceptable. -Sumit > Best regards, > Krzysztof > > Best regards, > Krzysztof >
Re: [PATCH v2 5/8] doc: devicetree: Updates for devicetree-rebasing subtree
On Fri, 22 Dec 2023 at 18:05, Tom Rini wrote: > > On Fri, Dec 22, 2023 at 11:42:05AM +0530, Sumit Garg wrote: > > > Encourage SoC/board maintainers to migrate to using devicetree-rebasing > > subtree and maintain a regular sync with Linux kernel devicetree files > > and bindings. > > > > Along with that add documentation regarding how to run DT bindings > > schema checks. > > > > Signed-off-by: Sumit Garg > > --- > > doc/develop/devicetree/control.rst | 108 +++-- > > 1 file changed, 86 insertions(+), 22 deletions(-) > [snip] > > @@ -81,18 +94,21 @@ Failing that, you could write one from scratch yourself! > > Configuration > > - > > > > -Use:: > > +Traditionally, U-Boot placed copies of devicetree source files from Linux > > +kernel into `arch//dts/.dts` which can be selected via:: > > > > - #define CONFIG_DEFAULT_DEVICE_TREE"" > > +#define CONFIG_DEFAULT_DEVICE_TREE "" > > Oh, oof. This never got updated for Kconfig rather than config.h. Can > you please include re-wording this to be setting when prompted > for DEFAULT_DEVICE_TREE by Kconfig? And the rest of the text you're > updating as well to match, thanks. > Sure but how about changing it to say via board defconfig as CONFIG_DEFAULT_DEVICE_TREE="", since AFAIK most people use defconfig to configure U-Boot. -Sumit > -- > Tom
Re: [PATCH v2 7/8] dts: meson-gxbb: Switch to using upstream DT
On Fri, Dec 22, 2023 at 06:50:32PM +0530, Sumit Garg wrote: > On Fri, 22 Dec 2023 at 18:01, Tom Rini wrote: > > > > On Fri, Dec 22, 2023 at 11:42:07AM +0530, Sumit Garg wrote: > > > > > Although there were still some variations in board DTS files based on > > > meson-gxbb SoC but I think those were minor differences from upstream > > > and shouldn't impact boot on these devices. > > > > > > So switch to upstream DT via mirroring amlogic/ directory from > > > devicetree-rebasing/src/arm64/amlogic/ directory. And thereby directly > > > building DTB from there including *-u-boot.dtsi files from > > > arch/$(ARCH)/dts/ directory. > > > > > > Reviewed-by: Neil Armstrong > > > Signed-off-by: Sumit Garg > > > --- > > > configs/nanopi-k2_defconfig | 3 ++- > > > configs/odroid-c2_defconfig | 3 ++- > > > configs/p200_defconfig| 3 ++- > > > configs/p201_defconfig| 3 ++- > > > configs/videostrong-kii-pro_defconfig | 3 ++- > > > configs/wetek-hub_defconfig | 3 ++- > > > configs/wetek-play2_defconfig | 3 ++- > > > dts/arch/arm64/Makefile | 9 + > > > dts/arch/arm64/amlogic| 1 + > > > 9 files changed, 24 insertions(+), 7 deletions(-) > > > create mode 12 dts/arch/arm64/amlogic > > > > Do we need all of the Makefile portions of this? One of the things that > > works today is that trees listed in ONFIG_DEFAULT_DEVICE_TREE (and a few > > other variables) will be automatically built. We might just need the new > > Makefiles to just have: > > include $(srctree)/scripts/Makefile.dts > > and then the trees a given config will need will be built. > > That sounds fine for Amlogic SoC and many others too with > CONFIG_DEFAULT_DEVICE_TREE="" CONFIG_OF_LIST="" options available. > However, if in future we have truely generalized U-Boot (like efforts > for Qcom platform) then CONFIG_OF_LIST="" may become an overloaded > choice. I don't object to listing them when needed, I'm just not totally sure on how the use case really works out. But we can cross that when we get there, too. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 5/8] doc: devicetree: Updates for devicetree-rebasing subtree
On Fri, Dec 22, 2023 at 07:18:05PM +0530, Sumit Garg wrote: > On Fri, 22 Dec 2023 at 18:05, Tom Rini wrote: > > > > On Fri, Dec 22, 2023 at 11:42:05AM +0530, Sumit Garg wrote: > > > > > Encourage SoC/board maintainers to migrate to using devicetree-rebasing > > > subtree and maintain a regular sync with Linux kernel devicetree files > > > and bindings. > > > > > > Along with that add documentation regarding how to run DT bindings > > > schema checks. > > > > > > Signed-off-by: Sumit Garg > > > --- > > > doc/develop/devicetree/control.rst | 108 +++-- > > > 1 file changed, 86 insertions(+), 22 deletions(-) > > [snip] > > > @@ -81,18 +94,21 @@ Failing that, you could write one from scratch > > > yourself! > > > Configuration > > > - > > > > > > -Use:: > > > +Traditionally, U-Boot placed copies of devicetree source files from Linux > > > +kernel into `arch//dts/.dts` which can be selected via:: > > > > > > - #define CONFIG_DEFAULT_DEVICE_TREE"" > > > +#define CONFIG_DEFAULT_DEVICE_TREE "" > > > > Oh, oof. This never got updated for Kconfig rather than config.h. Can > > you please include re-wording this to be setting when prompted > > for DEFAULT_DEVICE_TREE by Kconfig? And the rest of the text you're > > updating as well to match, thanks. > > > > Sure but how about changing it to say via board defconfig as > CONFIG_DEFAULT_DEVICE_TREE="", since AFAIK most people use > defconfig to configure U-Boot. Well, do kernel docs tell people to modify defconfigs, or to use the config menu/etc? -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 7/8] dts: meson-gxbb: Switch to using upstream DT
On Fri, 22 Dec 2023 at 19:24, Tom Rini wrote: > > On Fri, Dec 22, 2023 at 06:50:32PM +0530, Sumit Garg wrote: > > On Fri, 22 Dec 2023 at 18:01, Tom Rini wrote: > > > > > > On Fri, Dec 22, 2023 at 11:42:07AM +0530, Sumit Garg wrote: > > > > > > > Although there were still some variations in board DTS files based on > > > > meson-gxbb SoC but I think those were minor differences from upstream > > > > and shouldn't impact boot on these devices. > > > > > > > > So switch to upstream DT via mirroring amlogic/ directory from > > > > devicetree-rebasing/src/arm64/amlogic/ directory. And thereby directly > > > > building DTB from there including *-u-boot.dtsi files from > > > > arch/$(ARCH)/dts/ directory. > > > > > > > > Reviewed-by: Neil Armstrong > > > > Signed-off-by: Sumit Garg > > > > --- > > > > configs/nanopi-k2_defconfig | 3 ++- > > > > configs/odroid-c2_defconfig | 3 ++- > > > > configs/p200_defconfig| 3 ++- > > > > configs/p201_defconfig| 3 ++- > > > > configs/videostrong-kii-pro_defconfig | 3 ++- > > > > configs/wetek-hub_defconfig | 3 ++- > > > > configs/wetek-play2_defconfig | 3 ++- > > > > dts/arch/arm64/Makefile | 9 + > > > > dts/arch/arm64/amlogic| 1 + > > > > 9 files changed, 24 insertions(+), 7 deletions(-) > > > > create mode 12 dts/arch/arm64/amlogic > > > > > > Do we need all of the Makefile portions of this? One of the things that > > > works today is that trees listed in ONFIG_DEFAULT_DEVICE_TREE (and a few > > > other variables) will be automatically built. We might just need the new > > > Makefiles to just have: > > > include $(srctree)/scripts/Makefile.dts > > > and then the trees a given config will need will be built. > > > > That sounds fine for Amlogic SoC and many others too with > > CONFIG_DEFAULT_DEVICE_TREE="" CONFIG_OF_LIST="" options available. > > However, if in future we have truely generalized U-Boot (like efforts > > for Qcom platform) then CONFIG_OF_LIST="" may become an overloaded > > choice. > > I don't object to listing them when needed, I'm just not totally sure on > how the use case really works out. But we can cross that when we get > there, too. Agree and I have similar concerns about it. -Sumit > > -- > Tom
Re: [PATCH v2 5/8] doc: devicetree: Updates for devicetree-rebasing subtree
On Fri, 22 Dec 2023 at 19:26, Tom Rini wrote: > > On Fri, Dec 22, 2023 at 07:18:05PM +0530, Sumit Garg wrote: > > On Fri, 22 Dec 2023 at 18:05, Tom Rini wrote: > > > > > > On Fri, Dec 22, 2023 at 11:42:05AM +0530, Sumit Garg wrote: > > > > > > > Encourage SoC/board maintainers to migrate to using devicetree-rebasing > > > > subtree and maintain a regular sync with Linux kernel devicetree files > > > > and bindings. > > > > > > > > Along with that add documentation regarding how to run DT bindings > > > > schema checks. > > > > > > > > Signed-off-by: Sumit Garg > > > > --- > > > > doc/develop/devicetree/control.rst | 108 +++-- > > > > 1 file changed, 86 insertions(+), 22 deletions(-) > > > [snip] > > > > @@ -81,18 +94,21 @@ Failing that, you could write one from scratch > > > > yourself! > > > > Configuration > > > > - > > > > > > > > -Use:: > > > > +Traditionally, U-Boot placed copies of devicetree source files from > > > > Linux > > > > +kernel into `arch//dts/.dts` which can be selected via:: > > > > > > > > - #define CONFIG_DEFAULT_DEVICE_TREE"" > > > > +#define CONFIG_DEFAULT_DEVICE_TREE "" > > > > > > Oh, oof. This never got updated for Kconfig rather than config.h. Can > > > you please include re-wording this to be setting when prompted > > > for DEFAULT_DEVICE_TREE by Kconfig? And the rest of the text you're > > > updating as well to match, thanks. > > > > > > > Sure but how about changing it to say via board defconfig as > > CONFIG_DEFAULT_DEVICE_TREE="", since AFAIK most people use > > defconfig to configure U-Boot. > > Well, do kernel docs tell people to modify defconfigs, or to use the > config menu/etc? > Okay I see your point, I will use wording to select DEFAULT_DEVICE_TREE by Kconfig instead. -Sumit > -- > Tom
[PATCH v4] dts: rockpro64: Disable usb regulators-always-on
USB port regulators should be controlled by PHYs
so we remove always-on property and let phy manage the
regulator.
phy-supply isn't configured for TypeC port in upstream and
now that we are removing always-on, we need to add the
phy-supply until its fixed upstream.
Signed-off-by: Shantur Rathore
---
Changes in v4:
- otg port must be configured with phy-supply instead of host port node
Changes in v3:
- Split up patches as seperate series
Changes in v2:
- As requested, added fix for regulator-always-on in RockPro64
arch/arm/dts/rk3399-rockpro64-u-boot.dtsi | 12
1 file changed, 12 insertions(+)
diff --git a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
index 732727d9b0..0ac9fa9a03 100644
--- a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
@@ -22,6 +22,18 @@
};
};
+&u2phy0_otg {
+ phy-supply = <&vcc5v0_typec>;
+};
+
+&vcc5v0_host {
+ /delete-property/ regulator-always-on;
+};
+
+&vcc5v0_typec {
+ /delete-property/ regulator-always-on;
+};
+
&vdd_center {
regulator-min-microvolt = <95>;
regulator-max-microvolt = <95>;
--
2.40.1
[PATCH 1/1] bootmeth: pass size to efi_binary_run()
If we call efi_binary_run() with size parameter set to zero, we get an error
Not a PE-COFF file
Fill the missing value.
Fixes: 1373ffde52e1 ("Merge tag 'v2024.01-rc5' into next")
Fixes: 7017fc54a5bc ("bootmeth: use efi_loader interfaces instead of bootefi
command")
Signed-off-by: Heinrich Schuchardt
---
boot/bootmeth_efi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index 00060f7d25..c4eb331d69 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -454,12 +454,12 @@ static int distro_efi_boot(struct udevice *dev, struct
bootflow *bflow)
if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) {
log_debug("Booting with built-in fdt\n");
- if (efi_binary_run(map_sysmem(kernel, 0), 0,
+ if (efi_binary_run(map_sysmem(kernel, 0), bflow->size,
EFI_FDT_USE_INTERNAL))
return log_msg_ret("run", -EINVAL);
} else {
log_debug("Booting with external fdt\n");
- if (efi_binary_run(map_sysmem(kernel, 0), 0,
+ if (efi_binary_run(map_sysmem(kernel, 0), bflow->size,
map_sysmem(fdt, 0)))
return log_msg_ret("run", -EINVAL);
}
--
2.43.0
Re: [PATCH 1/1] bootmeth: pass size to efi_binary_run()
On Fri, 22 Dec 2023 at 17:02, Heinrich Schuchardt wrote:
>
> If we call efi_binary_run() with size parameter set to zero, we get an error
>
> Not a PE-COFF file
>
> Fill the missing value.
>
> Fixes: 1373ffde52e1 ("Merge tag 'v2024.01-rc5' into next")
> Fixes: 7017fc54a5bc ("bootmeth: use efi_loader interfaces instead of bootefi
> command")
> Signed-off-by: Heinrich Schuchardt
> ---
> boot/bootmeth_efi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
> index 00060f7d25..c4eb331d69 100644
> --- a/boot/bootmeth_efi.c
> +++ b/boot/bootmeth_efi.c
> @@ -454,12 +454,12 @@ static int distro_efi_boot(struct udevice *dev, struct
> bootflow *bflow)
>
> if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) {
> log_debug("Booting with built-in fdt\n");
> - if (efi_binary_run(map_sysmem(kernel, 0), 0,
> + if (efi_binary_run(map_sysmem(kernel, 0), bflow->size,
>EFI_FDT_USE_INTERNAL))
> return log_msg_ret("run", -EINVAL);
> } else {
> log_debug("Booting with external fdt\n");
> - if (efi_binary_run(map_sysmem(kernel, 0), 0,
> + if (efi_binary_run(map_sysmem(kernel, 0), bflow->size,
>map_sysmem(fdt, 0)))
> return log_msg_ret("run", -EINVAL);
> }
> --
> 2.43.0
>
Reviewed-by: Ilias Apalodimas
Re: [PATCH v2 1/9] bloblist: add API to check the register conventions
On Fri, 22 Dec 2023 at 05:55, Ilias Apalodimas wrote: > ... > The function looks correct, but the name is a bit off. There are no > registers conventions check AFAICT. We are just comparing 2 addresses. > Am I missing anything? > The function name is from the spec. Below is the section describes the expectation of arg[0-3]: https://github.com/FirmwareHandoff/firmware_handoff/blob/main/source/register_conventions.rst Thanks and regards, Raymond
Re: [PATCH v2 3/9] bloblist: refactor of bloblist_reloc()
Hi Ilias,
On Fri, 22 Dec 2023 at 06:12, Ilias Apalodimas
wrote:
> Hi Raymond,
>
> On Thu, 21 Dec 2023 at 02:41, Raymond Mao wrote:
> >
> > The current bloblist pointer and size can be retrieved from global
> > data, so we don't need to pass them from the function arguments.
> > This change also help to remove all external access of gd->bloblist
> > outside of bloblist module.
> >
> > Signed-off-by: Raymond Mao
> > ---
>
> [...]
>
> > }
> > }
> >
> > -void bloblist_reloc(void *to, uint to_size, void *from, uint from_size)
> > +void bloblist_reloc(void *to, uint to_size)
> > {
> > struct bloblist_hdr *hdr;
> >
> > - memcpy(to, from, from_size);
> > + memcpy(to, gd->bloblist, gd->bloblist->total_size);
> > hdr = to;
> > - hdr->total_size = to_size;
> > + if (to_size < gd->bloblist->total_size)
>
> What's the size of *to? Is it equal to to_size?
> Because if to_size can be smaller that gd->bloblist->total_size the
> memcpy above is wrong
>
to_size should be 0 (use the total_size) or a value larger than total_size.
I think I should keep the below line from the function header.
>> - * @to_size: New size for bloblist (must be larger than from_size)
I will refactor this part.
> > + hdr->total_size = gd->bloblist->total_size;
> > + else
> > + hdr->total_size = to_size;
> > + gd->bloblist = to;
> > }
> >
> > int bloblist_init(void)
> > diff --git a/common/board_f.c b/common/board_f.c
> > index d4d7d01f8f..00b0430889 100644
> > --- a/common/board_f.c
>
> [...]
>
> /Ilias
>
Re: [PATCH v2 6/9] qemu-arm: Get bloblist from boot arguments
On Fri, 22 Dec 2023 at 06:19, Ilias Apalodimas
wrote:
> > #endif
> >
> > +/* Boot parameters saved from start.S */
> > +extern unsigned long saved_args[];
> > +
> > int board_init(void)
> > {
> > return 0;
> > @@ -144,6 +148,35 @@ void *board_fdt_blob_setup(int *err)
> > return (void *)CFG_SYS_SDRAM_BASE;
> > }
> >
> > +int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
> > +{
> > + int ret = -ENOENT;
> > + unsigned long reg_fdt;
> > + unsigned long reg_zero;
> > +
> > + if (!IS_ENABLED(CONFIG_OF_BOARD) || !IS_ENABLED(CONFIG_BLOBLIST))
> > + return -ENOENT;
> > +
> > + ret = bloblist_check(saved_args[3], size);
> > + if (ret)
> > + return ret;
> > +
> > + if (IS_ENABLED(CONFIG_ARM64)) {
> > + reg_fdt = saved_args[0];
> > + reg_zero = saved_args[2];
> > + } else {
> > + reg_fdt = saved_args[2];
> > + reg_zero = saved_args[0];
>
> I think it's better if we fix up the order in the low-level asm code.
> Store the variables in the 'correct' order there and get rid of this
> if
>
Yes I agree. I can swap the order for aarch32.
Thanks and regards,
Raymond
Re: [PATCH 1/1] bootmeth: pass size to efi_binary_run()
On Fri, 22 Dec 2023 16:01:56 +0100, Heinrich Schuchardt wrote: > If we call efi_binary_run() with size parameter set to zero, we get an error > > Not a PE-COFF file > > Fill the missing value. > > > [...] Applied to u-boot/next, thanks! -- Tom
Re: [PATCH v2 0/8] An effort to bring DT bindings compliance within U-Boot
On 22/12/2023 14:43, Sumit Garg wrote: > On Fri, 22 Dec 2023 at 13:48, Krzysztof Kozlowski > wrote: >> >> On 22/12/2023 07:12, Sumit Garg wrote: >>> Changes in v2: >>> -- >>> - Patch #1: excluded gitab CI config check and added commit description. >>> - Patch #3: s/UBOOT_DTSI_LOC/u_boot_dtsi_loc/ >>> - Patch #4: s/DEVICE_TREE_LOC/dt_dir/ and s/U-boot/U-Boot/ >>> - Patch #5: s/U-boot/U-Boot/ >>> - Patch #6 and #7: Picked up review tags >>> >>> Prerequisite >>> >>> >>> This patch series requires devicetree-rebasing git repo to be added as a >>> subtree to the main U-Boot repo via: >>> >>> $ git subtree add --prefix devicetree-rebasing \ >>> >>> git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git >>> \ >>> v6.6-dts --squash >>> >>> Background >>> -- >>> >>> This effort started while I was reviewing patch series corresponding to >>> Qcom platforms [1] which was about to import modified devicetree source >>> files from Linux kernel. I suppose keeping devicetree files sync with >>> Linux kernel without any DT bindings schema validation has been a pain >>> for U-Boot SoC/platform maintainers. There has been past discussions >>> about a single DT repo but that hasn't come up and Linux kernel remained >>> the place where DT source files as well as bindings are placed and >>> maintained. >> >> Thanks for doing this. >> >> I really suggest to store information that kernel DTS is directly >> re-used, thus DTS backward and forward compatibility matters, also in >> Linux kernel sources. The point is that sub-arch maintainers should be >> aware of it. I don't think that as DT maintainers we can efficiently >> keep an eye on it. Maybe create a subsystem profile and include it to >> maintainer entries of such affected platforms? >> > > From U-Boot point of view, currently we have the config option: > "CONFIG_OF_UPSTREAM=y" per platform which means directly re-use of > kernel DTS. So U-Boot sub-arch maintainers should be aware of > platforms which get converted to re-use kernel DTS. I was speaking about kernel. > > I suppose we have to relay information to kernel sub-arch maintainers > who aren't the same as maintaining U-Boot counterparts. How about > adding U-Boot ML to CC for whichever DT change gets submitted in the And every other project? Just setup lei filters. > kernel? Otherwise adding U-Boot sub-arch maintainers as reviewers for > corresponding kernel DT changes works too if that's acceptable. You just entirely ignored my proposal without addressing it... ok let it be. No, CC-ing U-boot maintainers changes nothing because as I said, I want kernel maintainers and contributors to be aware. Best regards, Krzysztof
Re: [PATCH 1/1] bootmeth: pass size to efi_binary_run()
On Fri, Dec 22, 2023 at 3:37 PM Tom Rini wrote: > > On Fri, 22 Dec 2023 16:01:56 +0100, Heinrich Schuchardt wrote: > > > If we call efi_binary_run() with size parameter set to zero, we get an error > > > > Not a PE-COFF file > > > > Fill the missing value. > > > > > > [...] > > Applied to u-boot/next, thanks! Is this a fix that should land for 2024.01?
[PATCH 0/1] MAINTAINERS: fix folders within glob pattern
From: Anthony Loiseau This patch fixes few folders I think mishandled within MAINTAINERS file. For example, files within drivers/clk/tegra/ were not affilated to ARM TEGRA because rule "F: drivers/*/tegra*" does not match them. See MAINTAINERS file embedded documentation on top. The same for tools/env/* files which where not affiliated to Environment section despite the "F: tools/env*" rule. It worth be noted that despite I successfully tested this update for Arm Tegra and Environment sections, Arm TI related updates does nothing. My feeling is that those updates are good and wanted but another issue remains. It looks like all sections maintained by Tom Rini are entirely ignored, I don't know why and did not dig into it deeply. Example of test: ./scripts/get_maintainer.pl -f drivers/clk/tegra/Makefile now lists Arm Tegra maintainers were it did not. Cc: Tom Rini Cc: Thierry Reding Cc: Svyatoslav Ryhel Cc: Tom Rini Cc: Joe Hershberger Signed-off-by: Anthony Loiseau Anthony Loiseau (1): MAINTAINERS: fix folders within glob pattern MAINTAINERS | 5 + 1 file changed, 5 insertions(+) -- 2.11.0
[PATCH 1/1] MAINTAINERS: fix folders within glob pattern
From: Anthony Loiseau A "F: foo*" entry does not match any foo*/ folder nor its subtree, another "F: foo*/" entry is needed for that. Add missing foo*/ entries where an existing folder was ignored, so this folder and its subtree is properly covered. Arm tegra, Arm TI and Environment sections are affected. Cc: Tom Rini Cc: Thierry Reding Cc: Svyatoslav Ryhel Cc: Tom Rini Cc: Joe Hershberger Signed-off-by: Anthony Loiseau --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 969514468c..8c57d142d7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -675,6 +675,7 @@ F: arch/arm/dts/tegra* F: arch/arm/include/asm/arch-tegra*/ F: arch/arm/mach-tegra/ F: drivers/*/tegra* +F: drivers/*/tegra*/ ARM TI M: Tom Rini @@ -690,6 +691,7 @@ F: arch/arm/include/asm/arch-omap*/ F: arch/arm/include/asm/ti-common/ F: board/ti/ F: drivers/dma/ti* +F: drivers/dma/ti*/ F: drivers/firmware/ti_sci.* F: drivers/gpio/omap_gpio.c F: drivers/memory/ti-aemif.c @@ -701,6 +703,7 @@ F: drivers/phy/omap-usb2-phy.c F: drivers/phy/phy-ti-am654.c F: drivers/phy/ti-pipe3-phy.c F: drivers/ram/k3* +F: drivers/ram/k3*/ F: drivers/remoteproc/ipu_rproc.c F: drivers/remoteproc/k3_system_controller.c F: drivers/remoteproc/pruc_rpoc.c @@ -1029,8 +1032,10 @@ ENVIRONMENT M: Joe Hershberger S: Maintained F: env/ +F: include/env/ F: include/env* F: test/env/ +F: tools/env/ F: tools/env* F: tools/mkenvimage.c -- 2.11.0
Re: [PATCH v2 0/8] An effort to bring DT bindings compliance within U-Boot
On Fri, Dec 22, 2023 at 04:38:01PM +0100, Krzysztof Kozlowski wrote: > On 22/12/2023 14:43, Sumit Garg wrote: > > On Fri, 22 Dec 2023 at 13:48, Krzysztof Kozlowski > > wrote: > >> > >> On 22/12/2023 07:12, Sumit Garg wrote: > >>> Changes in v2: > >>> -- > >>> - Patch #1: excluded gitab CI config check and added commit description. > >>> - Patch #3: s/UBOOT_DTSI_LOC/u_boot_dtsi_loc/ > >>> - Patch #4: s/DEVICE_TREE_LOC/dt_dir/ and s/U-boot/U-Boot/ > >>> - Patch #5: s/U-boot/U-Boot/ > >>> - Patch #6 and #7: Picked up review tags > >>> > >>> Prerequisite > >>> > >>> > >>> This patch series requires devicetree-rebasing git repo to be added as a > >>> subtree to the main U-Boot repo via: > >>> > >>> $ git subtree add --prefix devicetree-rebasing \ > >>> > >>> git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git > >>> \ > >>> v6.6-dts --squash > >>> > >>> Background > >>> -- > >>> > >>> This effort started while I was reviewing patch series corresponding to > >>> Qcom platforms [1] which was about to import modified devicetree source > >>> files from Linux kernel. I suppose keeping devicetree files sync with > >>> Linux kernel without any DT bindings schema validation has been a pain > >>> for U-Boot SoC/platform maintainers. There has been past discussions > >>> about a single DT repo but that hasn't come up and Linux kernel remained > >>> the place where DT source files as well as bindings are placed and > >>> maintained. > >> > >> Thanks for doing this. > >> > >> I really suggest to store information that kernel DTS is directly > >> re-used, thus DTS backward and forward compatibility matters, also in > >> Linux kernel sources. The point is that sub-arch maintainers should be > >> aware of it. I don't think that as DT maintainers we can efficiently > >> keep an eye on it. Maybe create a subsystem profile and include it to > >> maintainer entries of such affected platforms? > >> > > > > From U-Boot point of view, currently we have the config option: > > "CONFIG_OF_UPSTREAM=y" per platform which means directly re-use of > > kernel DTS. So U-Boot sub-arch maintainers should be aware of > > platforms which get converted to re-use kernel DTS. > > I was speaking about kernel. > > > > > I suppose we have to relay information to kernel sub-arch maintainers > > who aren't the same as maintaining U-Boot counterparts. How about > > adding U-Boot ML to CC for whichever DT change gets submitted in the > > And every other project? Just setup lei filters. > > > kernel? Otherwise adding U-Boot sub-arch maintainers as reviewers for > > corresponding kernel DT changes works too if that's acceptable. > > You just entirely ignored my proposal without addressing it... ok let it > be. No, CC-ing U-boot maintainers changes nothing because as I said, I > want kernel maintainers and contributors to be aware. I don't think that adding U-Boot platform maintainers as reviewers for the platforms in the kernel is a terrible idea. Certainly kernel platform maintainers for which U-Boot platform maintainers are added to the MAINTAINERS entry will be aware. That said, something like your "strict dts compliance" maintainer profile entry [1] would be helpful I think to denote which platforms' dts are being shared in this manner 1: ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES M: Krzysztof Kozlowski R: Alim Akhtar L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) L: linux-samsung-...@vger.kernel.org S: Maintained P: Documentation/process/maintainer-soc-clean-dts.rst Q: https://patchwork.kernel.org/project/linux-samsung-soc/list/ signature.asc Description: PGP signature
Re: [PATCH v2 3/9] bloblist: refactor of bloblist_reloc()
Hi Raymond,
On Fri, 22 Dec 2023 at 17:30, Raymond Mao wrote:
>
> Hi Ilias,
>
> On Fri, 22 Dec 2023 at 06:12, Ilias Apalodimas
> wrote:
>>
>> Hi Raymond,
>>
>> On Thu, 21 Dec 2023 at 02:41, Raymond Mao wrote:
>> >
>> > The current bloblist pointer and size can be retrieved from global
>> > data, so we don't need to pass them from the function arguments.
>> > This change also help to remove all external access of gd->bloblist
>> > outside of bloblist module.
>> >
>> > Signed-off-by: Raymond Mao
>> > ---
>>
>> [...]
>>
>> > }
>> > }
>> >
>> > -void bloblist_reloc(void *to, uint to_size, void *from, uint from_size)
>> > +void bloblist_reloc(void *to, uint to_size)
>> > {
>> > struct bloblist_hdr *hdr;
>> >
>> > - memcpy(to, from, from_size);
>> > + memcpy(to, gd->bloblist, gd->bloblist->total_size);
>> > hdr = to;
>> > - hdr->total_size = to_size;
>> > + if (to_size < gd->bloblist->total_size)
>>
>> What's the size of *to? Is it equal to to_size?
>> Because if to_size can be smaller that gd->bloblist->total_size the
>> memcpy above is wrong
>
> to_size should be 0 (use the total_size) or a value larger than total_size.
> I think I should keep the below line from the function header.
The point here is, are we certain that the *to is big enough? Or we'll
end up overflowing ?
Thanks
/Ilias
> >> - * @to_size: New size for bloblist (must be larger than from_size)
> I will refactor this part.
>
>>
>> > + hdr->total_size = gd->bloblist->total_size;
>> > + else
>> > + hdr->total_size = to_size;
>> > + gd->bloblist = to;
>> > }
>> >
>> > int bloblist_init(void)
>> > diff --git a/common/board_f.c b/common/board_f.c
>> > index d4d7d01f8f..00b0430889 100644
>> > --- a/common/board_f.c
>>
>> [...]
>>
>> /Ilias
Re: [PATCH v2 0/8] An effort to bring DT bindings compliance within U-Boot
On Fri, Dec 22, 2023 at 04:38:01PM +0100, Krzysztof Kozlowski wrote: > On 22/12/2023 14:43, Sumit Garg wrote: > > On Fri, 22 Dec 2023 at 13:48, Krzysztof Kozlowski > > wrote: > >> > >> On 22/12/2023 07:12, Sumit Garg wrote: > >>> Changes in v2: > >>> -- > >>> - Patch #1: excluded gitab CI config check and added commit description. > >>> - Patch #3: s/UBOOT_DTSI_LOC/u_boot_dtsi_loc/ > >>> - Patch #4: s/DEVICE_TREE_LOC/dt_dir/ and s/U-boot/U-Boot/ > >>> - Patch #5: s/U-boot/U-Boot/ > >>> - Patch #6 and #7: Picked up review tags > >>> > >>> Prerequisite > >>> > >>> > >>> This patch series requires devicetree-rebasing git repo to be added as a > >>> subtree to the main U-Boot repo via: > >>> > >>> $ git subtree add --prefix devicetree-rebasing \ > >>> > >>> git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git > >>> \ > >>> v6.6-dts --squash > >>> > >>> Background > >>> -- > >>> > >>> This effort started while I was reviewing patch series corresponding to > >>> Qcom platforms [1] which was about to import modified devicetree source > >>> files from Linux kernel. I suppose keeping devicetree files sync with > >>> Linux kernel without any DT bindings schema validation has been a pain > >>> for U-Boot SoC/platform maintainers. There has been past discussions > >>> about a single DT repo but that hasn't come up and Linux kernel remained > >>> the place where DT source files as well as bindings are placed and > >>> maintained. > >> > >> Thanks for doing this. > >> > >> I really suggest to store information that kernel DTS is directly > >> re-used, thus DTS backward and forward compatibility matters, also in > >> Linux kernel sources. The point is that sub-arch maintainers should be > >> aware of it. I don't think that as DT maintainers we can efficiently > >> keep an eye on it. Maybe create a subsystem profile and include it to > >> maintainer entries of such affected platforms? > >> > > > > From U-Boot point of view, currently we have the config option: > > "CONFIG_OF_UPSTREAM=y" per platform which means directly re-use of > > kernel DTS. So U-Boot sub-arch maintainers should be aware of > > platforms which get converted to re-use kernel DTS. > > I was speaking about kernel. > > > > > I suppose we have to relay information to kernel sub-arch maintainers > > who aren't the same as maintaining U-Boot counterparts. How about > > adding U-Boot ML to CC for whichever DT change gets submitted in the > > And every other project? Just setup lei filters. > > > kernel? Otherwise adding U-Boot sub-arch maintainers as reviewers for > > corresponding kernel DT changes works too if that's acceptable. > > You just entirely ignored my proposal without addressing it... ok let it > be. No, CC-ing U-boot maintainers changes nothing because as I said, I > want kernel maintainers and contributors to be aware. Maybe an underlying question is, what kernel maintainers aren't aware, but should have been already? Then we can figure out how to address that. For example, with your Samsung hat on you weren't aware that exynos 4/5/7 DTS files are cared about by U-Boot, but are now aware. Samsung platforms are only recently becoming more active in U-Boot as well, so that's all understandable. On the other hand TI folks know, and I expect the K3 families to switch over to this series ASAP, and STM32 and all of the AMD/Xilinx stuff too. -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/1] bootmeth: pass size to efi_binary_run()
On Fri, 22 Dec 2023 at 17:43, Peter Robinson wrote: > > On Fri, Dec 22, 2023 at 3:37 PM Tom Rini wrote: > > > > On Fri, 22 Dec 2023 16:01:56 +0100, Heinrich Schuchardt wrote: > > > > > If we call efi_binary_run() with size parameter set to zero, we get an > > > error > > > > > > Not a PE-COFF file > > > > > > Fill the missing value. > > > > > > > > > [...] > > > > Applied to u-boot/next, thanks! > > Is this a fix that should land for 2024.01? I was about to ask the same. I think it should go into -master as well Thanks /Ilias
Re: [PATCH 1/1] bootmeth: pass size to efi_binary_run()
On Fri, Dec 22, 2023 at 05:46:11PM +0200, Ilias Apalodimas wrote: > On Fri, 22 Dec 2023 at 17:43, Peter Robinson wrote: > > > > On Fri, Dec 22, 2023 at 3:37 PM Tom Rini wrote: > > > > > > On Fri, 22 Dec 2023 16:01:56 +0100, Heinrich Schuchardt wrote: > > > > > > > If we call efi_binary_run() with size parameter set to zero, we get an > > > > error > > > > > > > > Not a PE-COFF file > > > > > > > > Fill the missing value. > > > > > > > > > > > > [...] > > > > > > Applied to u-boot/next, thanks! > > > > Is this a fix that should land for 2024.01? > > I was about to ask the same. I think it should go into -master as well This specifically fixes the merge of -rc5 in to -next however. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 0/8] An effort to bring DT bindings compliance within U-Boot
On 22/12/2023 16:45, Conor Dooley wrote: >>> >>> I suppose we have to relay information to kernel sub-arch maintainers >>> who aren't the same as maintaining U-Boot counterparts. How about >>> adding U-Boot ML to CC for whichever DT change gets submitted in the >> >> And every other project? Just setup lei filters. >> >>> kernel? Otherwise adding U-Boot sub-arch maintainers as reviewers for >>> corresponding kernel DT changes works too if that's acceptable. >> >> You just entirely ignored my proposal without addressing it... ok let it >> be. No, CC-ing U-boot maintainers changes nothing because as I said, I >> want kernel maintainers and contributors to be aware. > > I don't think that adding U-Boot platform maintainers as reviewers for > the platforms in the kernel is a terrible idea. Certainly kernel > platform maintainers for which U-Boot platform maintainers are added to > the MAINTAINERS entry will be aware. That said, something like your The point is it does not solve my concern here. I did not express problem that U-Boot maintainers are not aware. They can easily be aware by setting simple lei filters. The problem I want to solve is the kernel maintainers to be aware. Best regards, Krzysztof
Re: [PATCH v2 0/8] An effort to bring DT bindings compliance within U-Boot
On 22/12/2023 16:46, Tom Rini wrote: > On Fri, Dec 22, 2023 at 04:38:01PM +0100, Krzysztof Kozlowski wrote: >> On 22/12/2023 14:43, Sumit Garg wrote: >>> On Fri, 22 Dec 2023 at 13:48, Krzysztof Kozlowski >>> wrote: On 22/12/2023 07:12, Sumit Garg wrote: > Changes in v2: > -- > - Patch #1: excluded gitab CI config check and added commit description. > - Patch #3: s/UBOOT_DTSI_LOC/u_boot_dtsi_loc/ > - Patch #4: s/DEVICE_TREE_LOC/dt_dir/ and s/U-boot/U-Boot/ > - Patch #5: s/U-boot/U-Boot/ > - Patch #6 and #7: Picked up review tags > > Prerequisite > > > This patch series requires devicetree-rebasing git repo to be added as a > subtree to the main U-Boot repo via: > > $ git subtree add --prefix devicetree-rebasing \ > > git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git > \ > v6.6-dts --squash > > Background > -- > > This effort started while I was reviewing patch series corresponding to > Qcom platforms [1] which was about to import modified devicetree source > files from Linux kernel. I suppose keeping devicetree files sync with > Linux kernel without any DT bindings schema validation has been a pain > for U-Boot SoC/platform maintainers. There has been past discussions > about a single DT repo but that hasn't come up and Linux kernel remained > the place where DT source files as well as bindings are placed and > maintained. Thanks for doing this. I really suggest to store information that kernel DTS is directly re-used, thus DTS backward and forward compatibility matters, also in Linux kernel sources. The point is that sub-arch maintainers should be aware of it. I don't think that as DT maintainers we can efficiently keep an eye on it. Maybe create a subsystem profile and include it to maintainer entries of such affected platforms? >>> >>> From U-Boot point of view, currently we have the config option: >>> "CONFIG_OF_UPSTREAM=y" per platform which means directly re-use of >>> kernel DTS. So U-Boot sub-arch maintainers should be aware of >>> platforms which get converted to re-use kernel DTS. >> >> I was speaking about kernel. >> >>> >>> I suppose we have to relay information to kernel sub-arch maintainers >>> who aren't the same as maintaining U-Boot counterparts. How about >>> adding U-Boot ML to CC for whichever DT change gets submitted in the >> >> And every other project? Just setup lei filters. >> >>> kernel? Otherwise adding U-Boot sub-arch maintainers as reviewers for >>> corresponding kernel DT changes works too if that's acceptable. >> >> You just entirely ignored my proposal without addressing it... ok let it >> be. No, CC-ing U-boot maintainers changes nothing because as I said, I >> want kernel maintainers and contributors to be aware. > > Maybe an underlying question is, what kernel maintainers aren't aware, > but should have been already? Then we can figure out how to address None of them is aware. > that. For example, with your Samsung hat on you weren't aware that > exynos 4/5/7 DTS files are cared about by U-Boot, but are now aware. Hm, I am still not aware of this. I mean, you wrote it above, but it is the first time I see using directly usptream DTS for U-Boot on Samsung platforms. Did anyone test it actually? I certainly did not. I think this patchset did not remove U-Boot-tree Samsung DTS, did it? Best regards, Krzysztof
Re: [PATCH v2 0/8] An effort to bring DT bindings compliance within U-Boot
On Fri, Dec 22, 2023 at 04:55:54PM +0100, Krzysztof Kozlowski wrote: > On 22/12/2023 16:46, Tom Rini wrote: > > On Fri, Dec 22, 2023 at 04:38:01PM +0100, Krzysztof Kozlowski wrote: > >> On 22/12/2023 14:43, Sumit Garg wrote: > >>> On Fri, 22 Dec 2023 at 13:48, Krzysztof Kozlowski > >>> wrote: > > On 22/12/2023 07:12, Sumit Garg wrote: > > Changes in v2: > > -- > > - Patch #1: excluded gitab CI config check and added commit description. > > - Patch #3: s/UBOOT_DTSI_LOC/u_boot_dtsi_loc/ > > - Patch #4: s/DEVICE_TREE_LOC/dt_dir/ and s/U-boot/U-Boot/ > > - Patch #5: s/U-boot/U-Boot/ > > - Patch #6 and #7: Picked up review tags > > > > Prerequisite > > > > > > This patch series requires devicetree-rebasing git repo to be added as a > > subtree to the main U-Boot repo via: > > > > $ git subtree add --prefix devicetree-rebasing \ > > > > git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git > > \ > > v6.6-dts --squash > > > > Background > > -- > > > > This effort started while I was reviewing patch series corresponding to > > Qcom platforms [1] which was about to import modified devicetree source > > files from Linux kernel. I suppose keeping devicetree files sync with > > Linux kernel without any DT bindings schema validation has been a pain > > for U-Boot SoC/platform maintainers. There has been past discussions > > about a single DT repo but that hasn't come up and Linux kernel remained > > the place where DT source files as well as bindings are placed and > > maintained. > > Thanks for doing this. > > I really suggest to store information that kernel DTS is directly > re-used, thus DTS backward and forward compatibility matters, also in > Linux kernel sources. The point is that sub-arch maintainers should be > aware of it. I don't think that as DT maintainers we can efficiently > keep an eye on it. Maybe create a subsystem profile and include it to > maintainer entries of such affected platforms? > > >>> > >>> From U-Boot point of view, currently we have the config option: > >>> "CONFIG_OF_UPSTREAM=y" per platform which means directly re-use of > >>> kernel DTS. So U-Boot sub-arch maintainers should be aware of > >>> platforms which get converted to re-use kernel DTS. > >> > >> I was speaking about kernel. > >> > >>> > >>> I suppose we have to relay information to kernel sub-arch maintainers > >>> who aren't the same as maintaining U-Boot counterparts. How about > >>> adding U-Boot ML to CC for whichever DT change gets submitted in the > >> > >> And every other project? Just setup lei filters. > >> > >>> kernel? Otherwise adding U-Boot sub-arch maintainers as reviewers for > >>> corresponding kernel DT changes works too if that's acceptable. > >> > >> You just entirely ignored my proposal without addressing it... ok let it > >> be. No, CC-ing U-boot maintainers changes nothing because as I said, I > >> want kernel maintainers and contributors to be aware. > > > > Maybe an underlying question is, what kernel maintainers aren't aware, > > but should have been already? Then we can figure out how to address > > None of them is aware. > > > that. For example, with your Samsung hat on you weren't aware that > > exynos 4/5/7 DTS files are cared about by U-Boot, but are now aware. > > Hm, I am still not aware of this. I mean, you wrote it above, but it is > the first time I see using directly usptream DTS for U-Boot on Samsung > platforms. > > Did anyone test it actually? I certainly did not. I think this patchset > did not remove U-Boot-tree Samsung DTS, did it? With this literal patchset, only the amlogic platforms Sumit is changing have been tested, yes. In general, the U-Boot guideline has been "resync your DTS files from the kernel as often as possible" as well as "start with DTS files from the kernel, not hand-crafted". And some SoCs/vendors have been better about following these rules than others. There's a good number of commits under arch/arm/dts/ today syncing up with various states of v6.6. Which I guess emphasises my question, what kernel maintainers weren't aware that U-Boot has been consuming their DTS files as-is (as much as possible) for a number of years now? -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 12/32] board: dragonboard410c: upstream DT compat
Hi Sumit,
[...]
>> - if (init == USB_INIT_HOST) {
>> - /* Start USB Hub */
>> - dm_gpio_set_dir_flags(&hub_reset,
>> - GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
>> - mdelay(100);
>> - /* Switch usb to host connectors */
>> - dm_gpio_set_dir_flags(&usb_sel,
>> - GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
>> - mdelay(100);
>> - } else { /* Device */
>> - /* Disable hub */
>> - dm_gpio_set_dir_flags(&hub_reset, GPIOD_IS_OUT);
>> - /* Switch back to device connector */
>> - dm_gpio_set_dir_flags(&usb_sel, GPIOD_IS_OUT);
>> + /* Select "default" or "device" pinctrl */
>> + switch (init) {
>> + case USB_INIT_HOST:
>> + pinctrl_select_state(usb, "default");
>> + break;
>> + case USB_INIT_DEVICE:
>> + pinctrl_select_state(usb, "device");
>> + break;
>> + default:
>> + debug("Unknown usb_init_type %d\n", init);
>> + break;
>
> Can this pinctrl configuration move to the corresponding USB driver instead?
Possibly, this is definitely something where DT is currently lacking,
similar discussions in Linux resulted in the USB onboard_hub driver, it
would be nice to add support for that U-Boot at some point.
I don't think putting the pinctrl code directly into the USB driver is
the right way to go, as it definitely wouldn't be accepted in upstream
DT bindings.
>
> -Sumit
>
--
// Caleb (they/them)
Re: [PATCH v2 3/9] bloblist: refactor of bloblist_reloc()
Hi Ilias,
On Fri, 22 Dec 2023 at 10:46, Ilias Apalodimas
wrote:
> Hi Raymond,
>
> On Fri, 22 Dec 2023 at 17:30, Raymond Mao wrote:
> >
> > Hi Ilias,
> >
> > On Fri, 22 Dec 2023 at 06:12, Ilias Apalodimas <
> ilias.apalodi...@linaro.org> wrote:
> >>
> >> Hi Raymond,
> >>
> >> On Thu, 21 Dec 2023 at 02:41, Raymond Mao
> wrote:
> >> >
> >> > The current bloblist pointer and size can be retrieved from global
> >> > data, so we don't need to pass them from the function arguments.
> >> > This change also help to remove all external access of gd->bloblist
> >> > outside of bloblist module.
> >> >
> >> > Signed-off-by: Raymond Mao
> >> > ---
> >>
> >> [...]
> >>
> >> > }
> >> > }
> >> >
> >> > -void bloblist_reloc(void *to, uint to_size, void *from, uint
> from_size)
> >> > +void bloblist_reloc(void *to, uint to_size)
> >> > {
> >> > struct bloblist_hdr *hdr;
> >> >
> >> > - memcpy(to, from, from_size);
> >> > + memcpy(to, gd->bloblist, gd->bloblist->total_size);
> >> > hdr = to;
> >> > - hdr->total_size = to_size;
> >> > + if (to_size < gd->bloblist->total_size)
> >>
> >> What's the size of *to? Is it equal to to_size?
> >> Because if to_size can be smaller that gd->bloblist->total_size the
> >> memcpy above is wrong
> >
> > to_size should be 0 (use the total_size) or a value larger than
> total_size.
> > I think I should keep the below line from the function header.
>
> The point here is, are we certain that the *to is big enough? Or we'll
> end up overflowing ?
>
Yes, this needs to be checked before copying.
Thanks and regards,
Raymond
Re: [PATCH 0/7] Add SE HMBSC board support
[snip]
Sumit, could you rebase this series on my generic board support? [1] in
it's current form this series conflicts, and includes some of the major
anti-patterns I'm trying to move away from in mach-snapdragon.
>>>
>>> Although, I haven't gone through your series but I was expecting those
>>> conflicts. Let's work together to make this series compatible on top
>>> of yours.
>>>
You should not have to introduce a new CONFIG_TARGET_XYZ variable, and
from what I can tell you shouldn't even need to add the board/schneider
directory at all, you can just set the following in your defconfig:
CONFIG_SYS_BOARD="dragonboard410c"
>>>
>>> This is simply a misnomer, its HMIBSC board. I suppose we should
>>> rather separate the SoC specific bits into mach-snapdragon and let
>>> different boards use them.
>>
>> Is there any reason why you can't just use the existing db410c board
>> code as-is? I don't see why you want to duplicate it.
>
> I don't want to duplicate it but rather we should make the common
> parts as part of arch/arm/mach-snapdragon/ and let derivative boards
> use them. The HMIBSC board is an industrial board which doesn't need
> any generic distro boot but rather FIT booting with OTA updates via
> RAUC [1] along with U-boot environment protection. Doesn't that make
> it different from db410c?
Right, your series does this board specific configuration in the board
header file (include/configs/hmibsc.h) and in the defconfig.
The actual board code in board/schneider/hmibsc/hmibsc.c is directly
copied from board/qualcomm/dragonboard410c/dragonboard410c.c with just
the copyright changed, the serial number code removed, and a style fix
to a comment. Ignoring for a moment the copyright issue which would
definitely need to be fixed, what I'm suggesting that you do is just use
the board code from db410c.
The way I have designed the generic board support is so that two similar
boards can share the same board code but with different config headers,
from what I can tell this would work just fine for you.
--- board/qualcomm/dragonboard410c/dragonboard410c.c
+++ board/schneider/hmibsc/hmibsc.c
@@ -2,5 +2,5 @@
/*
- * Board init file for Dragonboard 410C
+ * Board init file for SE HMIBSC
*
- * (C) Copyright 2015 Mateusz Kulikowski
+ * (C) Copyright 2023 Sumit Garg
*/
@@ -8,3 +8,2 @@
#include
-#include
#include
@@ -137,7 +136,2 @@
{
- char serial[16];
-
- memset(serial, 0, 16);
- snprintf(serial, 13, "%x", msm_board_serial());
- env_set("serial#", serial);
return 0;
@@ -145,3 +139,4 @@
-/* Fixup of DTB for Linux Kernel
+/*
+ * Fixup of DTB for Linux Kernel
* 1. Fixup installed DRAM.
@@ -165,3 +160,2 @@
-
if (!eth_env_get_enetaddr("btaddr", mac)) {
@@ -169,5 +163,6 @@
-/* The BD address is same as WLAN MAC address but with
- * least significant bit flipped.
- */
+ /*
+* The BD address is same as WLAN MAC address but with
+* least significant bit flipped.
+*/
mac[0] ^= 0x01;
>
> [1] https://rauc.io/
>
>>
>> The only reason the db410c and db820c have their board code is because
>> they're old platforms and already supported. For adding new support
>> there needs to be some very strong justification to have board-specific
>> C code.
>>
>> I think it would be nice to make the db410c code go away, or be toggled
>> at runtime, probably most of it will just work and not break any other
>> boards anyway. The db820c code is just part of what should be in the
>> pinctrl driver...
>>
>> Let's move away from this old model and towards having more generic
>> U-Boot images. This will snowball towards making future bringup even easier.
>
> As I have mentioned in the other thread, we should give alternatives
> to the board code as well. How would you handle the following?
>
> - Fastboot mode entry on a button press.
This can be nicely handled through CONFIG_AUTOBOOT. I currently use
AUTOBOOT_STOP_STR but that's quite limited (only works for the power
button with "\r"). Probably the right solution here is to use
CONFIG_AUTOBOOT_USE_MENUKEY and introduce support for
CONFIG_AUTOBOOT_MENUBUTTON to allow specifying a named button instead of
an ASCII code. One would then configure "menucmd" in the U-Boot
environment to launch fastboot.
This lets us drop all the weird non-standard keycode handling in board
code and just configure it in the defconfig instead. I plan to implement
this eventually but would for sure appreciate it if someone beat me to it.
> - Configure MAC address for network support.
I believe you mentioned elsewhere some board with the MAC address on an
i2c EEPROM? The NVMEM framework solves exactly this issue, and U-Boot
already supports it, just add support to your ethernet driver to
retrieve its MAC address via NVMEM.
> - Setup board serial number.
Same as above, the quick and dirty way to go would be to have
mach-snapdragon check fo
Re: [PATCH 00/13] Complete decoupling of zboot logic from commands
On Sun, Dec 03, 2023 at 05:29:25PM -0700, Simon Glass wrote: > This series refactors the zboot code to allow it to be used with > CONFIG_COMMAND disabled. > > A new zboot_run() function is used to boot a zimage. > > This is cmde (part e of CMDLINE refactoring) > It depends on dm/cmdd-working > which depends on dm/bootstda-working > which depends on dm/cmdc-working Just FYI, at this point all of the required series are now in -next so this could come in via the x86 tree now. -- Tom signature.asc Description: PGP signature
[PATCH 1/1] lib: smbios: remove redundant next_header()
next_header() and get_next_header() only differ in how the const attribute
is used. One function taking a const parameter and returning a non-const is
good enough.
Fixes: 3d49ee8510d3 ("efi_loader: add SMBIOS table measurement")
Signed-off-by: Heinrich Schuchardt
---
lib/smbios-parser.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/lib/smbios-parser.c b/lib/smbios-parser.c
index b578c30840..f4de350e6e 100644
--- a/lib/smbios-parser.c
+++ b/lib/smbios-parser.c
@@ -50,14 +50,7 @@ static u8 *find_next_header(u8 *pos)
return pos;
}
-static struct smbios_header *get_next_header(struct smbios_header *curr)
-{
- u8 *pos = ((u8 *)curr) + curr->length;
-
- return (struct smbios_header *)find_next_header(pos);
-}
-
-static const struct smbios_header *next_header(const struct smbios_header
*curr)
+static struct smbios_header *get_next_header(const struct smbios_header *curr)
{
u8 *pos = ((u8 *)curr) + curr->length;
@@ -73,7 +66,7 @@ const struct smbios_header *smbios_header(const struct
smbios_entry *entry, int
if (header->type == type)
return header;
- header = next_header(header);
+ header = get_next_header(header);
}
return NULL;
--
2.43.0
[PATCH 1/1] lib: smbios: verify_checksum() is duplicate
The function verify_checksum() duplicates what table_compute_checksum()
does. Replace it.
Fixes: 415eab0655a8 ("smbios: add parsing API")
Signed-off-by: Heinrich Schuchardt
---
lib/smbios-parser.c | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/lib/smbios-parser.c b/lib/smbios-parser.c
index f4de350e6e..a29a06398d 100644
--- a/lib/smbios-parser.c
+++ b/lib/smbios-parser.c
@@ -5,23 +5,9 @@
#define LOG_CATEGORY LOGC_BOOT
+#include
#include
-static inline int verify_checksum(const struct smbios_entry *e)
-{
- /*
-* Checksums for SMBIOS tables are calculated to have a value, so that
-* the sum over all bytes yields zero (using unsigned 8 bit arithmetic).
-*/
- u8 *byte = (u8 *)e;
- u8 sum = 0;
-
- for (int i = 0; i < e->length; i++)
- sum += byte[i];
-
- return sum;
-}
-
const struct smbios_entry *smbios_entry(u64 address, u32 size)
{
const struct smbios_entry *entry = (struct smbios_entry
*)(uintptr_t)address;
@@ -32,7 +18,7 @@ const struct smbios_entry *smbios_entry(u64 address, u32 size)
if (memcmp(entry->anchor, "_SM_", 4))
return NULL;
- if (verify_checksum(entry))
+ if (table_compute_checksum(entry, entry->length))
return NULL;
return entry;
--
2.43.0
Re: [PATCH 1/1] lib: smbios: remove redundant next_header()
Hi Heinrich,
On Fri, 22 Dec 2023 at 19:54, Heinrich Schuchardt wrote:
>
> next_header() and get_next_header() only differ in how the const attribute
> is used. One function taking a const parameter and returning a non-const is
> good enough.
>
> Fixes: 3d49ee8510d3 ("efi_loader: add SMBIOS table measurement")\
This doesn't fix a bug. It correctly cleans up code, so I think the
Fixes tag must be omitted.
> Signed-off-by: Heinrich Schuchardt
> ---
> lib/smbios-parser.c | 11 ++-
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/lib/smbios-parser.c b/lib/smbios-parser.c
> index b578c30840..f4de350e6e 100644
> --- a/lib/smbios-parser.c
> +++ b/lib/smbios-parser.c
> @@ -50,14 +50,7 @@ static u8 *find_next_header(u8 *pos)
> return pos;
> }
>
> -static struct smbios_header *get_next_header(struct smbios_header *curr)
> -{
> - u8 *pos = ((u8 *)curr) + curr->length;
> -
> - return (struct smbios_header *)find_next_header(pos);
> -}
> -
> -static const struct smbios_header *next_header(const struct smbios_header
> *curr)
> +static struct smbios_header *get_next_header(const struct smbios_header
> *curr)
> {
> u8 *pos = ((u8 *)curr) + curr->length;
>
> @@ -73,7 +66,7 @@ const struct smbios_header *smbios_header(const struct
> smbios_entry *entry, int
> if (header->type == type)
> return header;
>
> - header = next_header(header);
> + header = get_next_header(header);
> }
>
> return NULL;
> --
> 2.43.0
>
Other than that
Reviewed-by: Ilias Apalodimas
[PATCH 1/1] smbios: type2: contained object handles
The type 2 structure must include information about the contained objects.
It is fine to set the number of contained object handles to 0.
Add the missing field.
Fixes: 721e992a8af5 ("x86: Add SMBIOS table support")
Signed-off-by: Heinrich Schuchardt
---
include/smbios.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/smbios.h b/include/smbios.h
index e601283d29..88c19ae062 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -139,6 +139,7 @@ struct __packed smbios_type2 {
u8 chassis_location;
u16 chassis_handle;
u8 board_type;
+ u8 number_contained_objects;
char eos[SMBIOS_STRUCT_EOS_BYTES];
};
--
2.43.0
RE: [PATCH v7 2/2] schemas: Add some common reserved-memory usages
Please see my reply below inline. Thanks, Chasel > -Original Message- > From: Ard Biesheuvel > Sent: Friday, December 22, 2023 4:48 AM > To: Chiu, Chasel > Cc: Simon Glass ; devicet...@vger.kernel.org; Mark Rutland > ; Rob Herring ; Tan, Lean Sheng > ; lkml ; Dhaval > Sharma ; Brune, Maximilian > ; Yunhui Cui ; > Dong, Guo ; Tom Rini ; ron minnich > ; Guo, Gua ; linux- > a...@vger.kernel.org; U-Boot Mailing List > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory > usages > > On Thu, 21 Dec 2023 at 17:50, Chiu, Chasel wrote: > > > > > > Hi Ard, > > > > Please see my reply below inline and let me know your thoughts. > > > > Thanks, > > Chasel > > > > > > > -Original Message- > > > From: Ard Biesheuvel > > > Sent: Thursday, December 21, 2023 6:31 AM > > > To: Chiu, Chasel > > > Cc: Simon Glass ; devicet...@vger.kernel.org; Mark > > > Rutland ; Rob Herring ; Tan, > > > Lean Sheng ; lkml > > > ; Dhaval Sharma ; > > > Brune, Maximilian ; Yunhui Cui > > > ; Dong, Guo ; Tom Rini > > > ; ron minnich ; Guo, Gua > > > ; linux- a...@vger.kernel.org; U-Boot Mailing > > > List > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory > > > usages > > > > > > On Tue, 28 Nov 2023 at 21:31, Chiu, Chasel wrote: > > > > > > > > > > > > > > > > > > > > > -Original Message- > > > > > From: Ard Biesheuvel > > > > > Sent: Tuesday, November 28, 2023 10:08 AM > > > > > To: Chiu, Chasel > > > > > Cc: Simon Glass ; devicet...@vger.kernel.org; > > > > > Mark Rutland ; Rob Herring > > > > > ; Tan, Lean Sheng ; > > > > > lkml ; Dhaval Sharma > > > > > ; Brune, Maximilian > > > > > ; Yunhui Cui > > > > > ; Dong, Guo ; Tom > > > > > Rini ; ron minnich ; > > > > > Guo, Gua ; linux- a...@vger.kernel.org; > > > > > U-Boot Mailing List > > > > > Subject: Re: [PATCH v7 2/2] schemas: Add some common > > > > > reserved-memory usages > > > > > > > > > > You are referring to a 2000 line patch so it is not 100% clear where > > > > > to look > tbh. > > > > > > > > > > > > > > > On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel > wrote: > > > > > > > > > > > > > > > > > > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, > > > > > > line > > > > > > 268 is for > > > > > related example code. > > > > > > > > > > > > > > > > That refers to a 'memory-allocation' node, right? How does that > > > > > relate to the 'reserved-memory' node? > > > > > > > > > > And crucially, how does this clarify in which way "runtime-code" > > > > > and > > > > > "runtime- data" reservations are being used? > > > > > > > > > > Since the very beginning of this discussion, I have been asking > > > > > repeatedly for examples that describe the wider context in which > > > > > these > > > reservations are used. > > > > > The "runtime" into runtime-code and runtime-data means that > > > > > these regions have a special significance to the operating > > > > > system, not just to the next bootloader stage. So I want to > > > > > understand exactly why it is necessary to describe these regions > > > > > in a way where the operating system might be expected to > > > > > interpret this information and act > > > upon it. > > > > > > > > > > > > > > > > > I think runtime code and data today are mainly for supporting UEFI > > > > runtime > > > services - some BIOS functions for OS to utilize, OS may follow > > > below ACPI spec to treat them as reserved range: > > > > https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces.h > > > > tml# uefi-memory-types-and-mapping-to-acpi-address-range-types > > > > > > > > Like I mentioned earlier, that PR is still in early phase and has > > > > not reflected all > > > the required changes yet, but the idea is to build > > > gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes. > > > > UEFI generic Payload has DxeMain integrated, however Memory Types > > > > are > > > platform-specific, for example, some platforms may need bigger > > > runtime memory for their implementation, that's why we want such FDT > > > reserved-memory node to tell DxeMain. > > > > > > > > > > > The Payload flow will be like this: > > > > Payload creates built-in default MemoryTypes table -> > > > > FDT reserved-memory node to override if required (this also > > > > ensures the > > > same memory map cross boots so ACPI S4 works) -> > > > > Build gEfiMemoryTypeInformationGuid HOB by "platfom specific" > > > MemoryTypes Table -> > > > > DxeMain/GCD to consume this MemoryTypes table and setup > > > > memory > > > service -> > > > > Install memory types table to UEFI system table.Configuration > > > > table... > > > > > > > > Note: if Payload built-in default MemoryTypes table works fine for > > > > the platform, then FDT reserved-memory node does not need to > > > > provide such > > > 'usage' compatible strings. (optional) This FDT node could allow > > > flexibility/compatibility without rebuilding Payload binary. > > > > > > > > Not sure if I answ
[PATCH v13 00/24] Modernize U-Boot shell
Hi! During 2021 summer, Sean Anderson wrote a contribution to add a new shell, based on LIL, to U-Boot [1, 2]. While one of the goals of this contribution was to address the fact actual U-Boot shell, which is based on Busybox hush, is old there was a discussion about adding a new shell versus updating the actual one [3, 4]. So, in this series, with Harald Seiler, we updated the actual U-Boot shell to reflect what is currently in Busybox source code. Basically, this contribution is about taking a snapshot of Busybox shell/hush.c file (as it exists in commit 37460f5da) and adapt it to suit U-Boot needs. This contribution was written to be as backward-compatible as possible to avoid breaking the existing. So, the modern hush flavor offers the same as the actual, that is to say: 1. Variable expansion. 2. Instruction lists (;, && and ||). 3. If, then and else. 4. Loops (for, while and until). No new features offered by Busybox hush were implemented (e.g. functions). It is possible to change the parser at runtime using the "cli" command: => cli print old => cli set modern => cli print modern => cli set old The default parser is the old one. Note that to use both parser, you would need to set both CONFIG_HUSH_MODERN_PARSER and CONFIG_HUSH_OLD_PARSER. In terms of testing, new unit tests were added to ut to ensure the new behavior is the same as the old one and it does not add regression. Nonetheless, if old behavior was buggy and fixed upstream, the fix is then added to U-Boot [5]. In sandbox, all of these tests pass smoothly: => printenv board board=sandbox => ut hush Running 20 hush tests ... Failures: 0 => cli set modern => ut hush Running 20 hush tests ... Failures: 0 Thanks to the effort of Harald Seiler, I was successful booting a board: => printenv fdtfile fdtfile=amlogic/meson-gxl-s905x-libretech-cc.dtb => cli get old => boot ... root@lepotato:~# root@lepotato:~# reboot ... => cli set modern => cli get modern => printenv fdtfile fdtfile=amlogic/meson-gxl-s905x-libretech-cc.dtb => boot ... root@lepotato:~# This contribution indeed adds a lot of code and there were concern about its size [6, 7]. With regard to the amount of code added, the cli_hush_upstream.c is 13030 lines long but it seems a smaller subset is really used: gcc -D__U_BOOT__ -E common/cli_hush_upstream.c | wc -l 2870 Despite this, it is better to still have the whole upstream code for the sake of easing maintenance. With regard to memory size, I conducted some experiments for version 8 of this series and for a subset of arm64 boards and found the worst case to be 4K [8]. Tom Rini conducted more research on this and also found the increase to be acceptable [9]. If you want to review it - your review will really be appreciated - here are some information regarding the commits: * commits marked as "test:" deal with unit tests. * commit "cli: Add Busybox upstream hush.c file." copies Busybox shell/hush.c into U-Boot tree, this explain why this commit contains around 12000 additions. * commit "cli: Port Busybox 2021 hush to U-Boot." modifies previously added file to permit us to use this as new shell. The really good idea of #include'ing Busybox code into a wrapper file to define some particular functions while minimizing modifications to upstream code comes from Harald Seiler. * commit "cmd: Add new parser command" adds a new command which permits selecting parser at runtime. I am not really satisfied with the fact it calls cli_init() and cli_loop() each time the parser is set, so your reviews would be welcomed. * Other commits focus on enabling features we need (e.g. if). Changes since: v2: * Added a small fix to compile sandbox with NO_SDL=1. * Added a command to change parser at runtime. * Added 2021 parser function to all run_command*(). v3: * Various bug fixes pointed by the CI. * Added upstream busybox hush commits until 6th February 2022. v4: * Various cleaning. * Modified python test to accept failure output when the test are designed to fail. * Bumped upstream busybox hush commits until 24h March 2022. v5: * Bumped upstream busybox hush commits until 30th January 2023. * Fix how hush interprets '<' and '>', indeed we needed to escape them but I removed this behavior as tests are handled by test command and not hush itself. This permitted to have the ut fdt to pass. * Fix a problem with how exit was handled. This was reported by the ut exit test. v6: * There was no v6 and I got mixed up with version. v7: * Bumped upstream busybox hush commits until 9th May 2023. * Renamed parser command to change parser at runtime to cli and added documentation. * Added better separation of patches. * Removed code about __gnu_thumb1_case_si as it was merged in another series. * Various cleaning. v8: * Bumped upstream busybox hush commits until 25th May 2023. v9: * Bumped upstream busybox hush commits until 2nd October 2023. v10: * Fixed a build error in commit adding cli command. * Add
[PATCH v13 01/24] test: Add framework to test hush behavior
Introduce a new subcommand to ut: ut hush.
For the moment, this command does nothing, future commits will add tests which
will be run on command call.
Note that CONFIG_HUSH_PARSER must be defined to compile this new subcommand.
Signed-off-by: Francis Laniel
Reviewed-by: Simon Glass
---
include/test/hush.h | 15 +++
include/test/suites.h | 1 +
test/Makefile | 3 +++
test/cmd_ut.c | 6 ++
test/hush/Makefile | 6 ++
test/hush/cmd_ut_hush.c | 20
6 files changed, 51 insertions(+)
create mode 100644 include/test/hush.h
create mode 100644 test/hush/Makefile
create mode 100644 test/hush/cmd_ut_hush.c
diff --git a/include/test/hush.h b/include/test/hush.h
new file mode 100644
index 00..cca66544a0
--- /dev/null
+++ b/include/test/hush.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * (C) Copyright 2021
+ * Francis Laniel, Amarula Solutions, francis.lan...@amarulasolutions.com
+ */
+
+#ifndef __TEST_HUSH_H__
+#define __TEST_HUSH_H__
+
+#include
+
+/* Declare a new environment test */
+#define HUSH_TEST(_name, _flags) UNIT_TEST(_name, _flags, hush_test)
+
+#endif /* __TEST_HUSH_H__ */
diff --git a/include/test/suites.h b/include/test/suites.h
index 49224d8ea2..365d5f20df 100644
--- a/include/test/suites.h
+++ b/include/test/suites.h
@@ -43,6 +43,7 @@ int do_ut_env(struct cmd_tbl *cmdtp, int flag, int argc, char
*const argv[]);
int do_ut_exit(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
int do_ut_fdt(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
int do_ut_font(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
+int do_ut_hush(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
int do_ut_lib(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
int do_ut_loadm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
int do_ut_log(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]);
diff --git a/test/Makefile b/test/Makefile
index 6b8a1506f5..9aeef02f9e 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -17,6 +17,9 @@ obj-$(CONFIG_FUZZ) += fuzz/
ifndef CONFIG_SANDBOX_VPL
obj-$(CONFIG_UNIT_TEST) += lib/
endif
+ifneq ($(CONFIG_HUSH_PARSER),)
+obj-$(CONFIG_$(SPL_)CMDLINE) += hush/
+endif
obj-$(CONFIG_$(SPL_)CMDLINE) += print_ut.o
obj-$(CONFIG_$(SPL_)CMDLINE) += str_ut.o
obj-$(CONFIG_UT_TIME) += time_ut.o
diff --git a/test/cmd_ut.c b/test/cmd_ut.c
index 1b934b2329..0677ce0cd1 100644
--- a/test/cmd_ut.c
+++ b/test/cmd_ut.c
@@ -121,6 +121,9 @@ static struct cmd_tbl cmd_ut_sub[] = {
#ifdef CONFIG_CMD_ADDRMAP
U_BOOT_CMD_MKENT(addrmap, CONFIG_SYS_MAXARGS, 1, do_ut_addrmap, "", ""),
#endif
+#if CONFIG_IS_ENABLED(HUSH_PARSER)
+ U_BOOT_CMD_MKENT(hush, CONFIG_SYS_MAXARGS, 1, do_ut_hush, "", ""),
+#endif
#ifdef CONFIG_CMD_LOADM
U_BOOT_CMD_MKENT(loadm, CONFIG_SYS_MAXARGS, 1, do_ut_loadm, "", ""),
#endif
@@ -216,6 +219,9 @@ U_BOOT_LONGHELP(ut,
#ifdef CONFIG_CONSOLE_TRUETYPE
"\nfont - font command"
#endif
+#if CONFIG_IS_ENABLED(HUSH_PARSER)
+ "\nhush - Test hush behavior"
+#endif
#ifdef CONFIG_CMD_LOADM
"\nloadm - loadm command parameters and loading memory blob"
#endif
diff --git a/test/hush/Makefile b/test/hush/Makefile
new file mode 100644
index 00..dfa2a92615
--- /dev/null
+++ b/test/hush/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# (C) Copyright 2021
+# Francis Laniel, Amarula Solutions, francis.lan...@amarulasolutions.com
+
+obj-y += cmd_ut_hush.o
diff --git a/test/hush/cmd_ut_hush.c b/test/hush/cmd_ut_hush.c
new file mode 100644
index 00..48a1adbf28
--- /dev/null
+++ b/test/hush/cmd_ut_hush.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2021
+ * Francis Laniel, Amarula Solutions, francis.lan...@amarulasolutions.com
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+int do_ut_hush(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
+{
+ struct unit_test *tests = UNIT_TEST_SUITE_START(hush_test);
+ const int n_ents = UNIT_TEST_SUITE_COUNT(hush_test);
+
+ return cmd_ut_category("hush", "hush_test_",
+ tests, n_ents, argc, argv);
+}
--
2.34.1
[PATCH v13 02/24] test: hush: Test hush if/else
As asked in commit 9c6bf1715f6a ("test/py: hush_if_test: Add tests to cover
octal/hex values"), this commit translates test_hush_if_test.py to a C test.
Signed-off-by: Francis Laniel
Reviewed-by: Simon Glass
---
test/hush/Makefile | 1 +
test/hush/if.c | 316 +
2 files changed, 317 insertions(+)
create mode 100644 test/hush/if.c
diff --git a/test/hush/Makefile b/test/hush/Makefile
index dfa2a92615..a3c9ae5106 100644
--- a/test/hush/Makefile
+++ b/test/hush/Makefile
@@ -4,3 +4,4 @@
# Francis Laniel, Amarula Solutions, francis.lan...@amarulasolutions.com
obj-y += cmd_ut_hush.o
+obj-y += if.o
diff --git a/test/hush/if.c b/test/hush/if.c
new file mode 100644
index 00..b7200e32ec
--- /dev/null
+++ b/test/hush/if.c
@@ -0,0 +1,316 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2021
+ * Francis Laniel, Amarula Solutions, francis.lan...@amarulasolutions.com
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+/*
+ * All tests will execute the following:
+ * if condition_to_test; then
+ * true
+ * else
+ * false
+ * fi
+ * If condition is true, command returns 1, 0 otherwise.
+ */
+const char *if_format = "if %s; then true; else false; fi";
+
+static int hush_test_if_base(struct unit_test_state *uts)
+{
+ char if_formatted[128];
+
+ sprintf(if_formatted, if_format, "true");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "false");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ return 0;
+}
+HUSH_TEST(hush_test_if_base, 0);
+
+static int hush_test_if_basic_operators(struct unit_test_state *uts)
+{
+ char if_formatted[128];
+
+ sprintf(if_formatted, if_format, "test aaa = aaa");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test aaa = bbb");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test aaa != bbb");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test aaa != aaa");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test aaa < bbb");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test bbb < aaa");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test bbb > aaa");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test aaa > bbb");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 123 -eq 123");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 123 -eq 456");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 123 -ne 456");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 123 -ne 123");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 123 -lt 456");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 123 -lt 123");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 456 -lt 123");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 123 -le 456");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 123 -le 123");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 456 -le 123");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 456 -gt 123");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 123 -gt 123");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 123 -gt 456");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 456 -ge 123");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 123 -ge 123");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 123 -ge 456");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ return 0;
+}
+HUSH_TEST(hush_test_if_basic_operators, 0);
+
+static int hush_test_if_octal(struct unit_test_state *uts)
+{
+ char if_formatted[128];
+
+ sprintf(if_formatted, if_format, "test 010 -eq 010");
+ ut_assertok(run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 010 -eq 011");
+ ut_asserteq(1, run_command(if_formatted, 0));
+
+ sprintf(if_formatted, if_format, "test 010 -ne 011");
+ ut_assertok(run_command(if_fo
[PATCH v13 03/24] test/py: hush_if_test: Remove the test file
5804ebfeb1ce ("test: hush: Test hush if/else") translated this test to a C test,
so this python file is no more needed.
Signed-off-by: Francis Laniel
Reviewed-by: Simon Glass
---
test/py/tests/test_hush_if_test.py | 197 -
1 file changed, 197 deletions(-)
delete mode 100644 test/py/tests/test_hush_if_test.py
diff --git a/test/py/tests/test_hush_if_test.py
b/test/py/tests/test_hush_if_test.py
deleted file mode 100644
index 3b4b6fcaf4..00
--- a/test/py/tests/test_hush_if_test.py
+++ /dev/null
@@ -1,197 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-# Copyright (c) 2015-2016, NVIDIA CORPORATION. All rights reserved.
-
-# Test operation of the "if" shell command.
-
-import os
-import os.path
-import pytest
-
-# TODO: These tests should be converted to a C test.
-# For more information please take a look at the thread
-# https://lists.denx.de/pipermail/u-boot/2019-October/388732.html
-
-pytestmark = pytest.mark.buildconfigspec('hush_parser')
-
-# The list of "if test" conditions to test.
-subtests = (
-# Base if functionality.
-
-('true', True),
-('false', False),
-
-# Basic operators.
-
-('test aaa = aaa', True),
-('test aaa = bbb', False),
-
-('test aaa != bbb', True),
-('test aaa != aaa', False),
-
-('test aaa < bbb', True),
-('test bbb < aaa', False),
-
-('test bbb > aaa', True),
-('test aaa > bbb', False),
-
-('test 123 -eq 123', True),
-('test 123 -eq 456', False),
-
-('test 123 -ne 456', True),
-('test 123 -ne 123', False),
-
-('test 123 -lt 456', True),
-('test 123 -lt 123', False),
-('test 456 -lt 123', False),
-
-('test 123 -le 456', True),
-('test 123 -le 123', True),
-('test 456 -le 123', False),
-
-('test 456 -gt 123', True),
-('test 123 -gt 123', False),
-('test 123 -gt 456', False),
-
-('test 456 -ge 123', True),
-('test 123 -ge 123', True),
-('test 123 -ge 456', False),
-
-# Octal tests
-
-('test 010 -eq 010', True),
-('test 010 -eq 011', False),
-
-('test 010 -ne 011', True),
-('test 010 -ne 010', False),
-
-# Hexadecimal tests
-
-('test 0x200 -gt 0x201', False),
-('test 0x200 -gt 0x200', False),
-('test 0x200 -gt 0x1ff', True),
-
-# Mixed tests
-
-('test 010 -eq 10', False),
-('test 010 -ne 10', True),
-('test 0xa -eq 10', True),
-('test 0xa -eq 012', True),
-
-('test 200 -gt 0x1ff', False),
-('test 0x200 -gt 1ff', True),
-('test 0x200 -lt 1ff', False),
-('test 0x200 -eq 200', False),
-('test 0x200 -ne 200', True),
-
-('test -z ""', True),
-('test -z "aaa"', False),
-
-('test -n "aaa"', True),
-('test -n ""', False),
-
-# Inversion of simple tests.
-
-('test ! aaa = aaa', False),
-('test ! aaa = bbb', True),
-('test ! ! aaa = aaa', True),
-('test ! ! aaa = bbb', False),
-
-# Binary operators.
-
-('test aaa != aaa -o bbb != bbb', False),
-('test aaa != aaa -o bbb = bbb', True),
-('test aaa = aaa -o bbb != bbb', True),
-('test aaa = aaa -o bbb = bbb', True),
-
-('test aaa != aaa -a bbb != bbb', False),
-('test aaa != aaa -a bbb = bbb', False),
-('test aaa = aaa -a bbb != bbb', False),
-('test aaa = aaa -a bbb = bbb', True),
-
-# Inversion within binary operators.
-
-('test ! aaa != aaa -o ! bbb != bbb', True),
-('test ! aaa != aaa -o ! bbb = bbb', True),
-('test ! aaa = aaa -o ! bbb != bbb', True),
-('test ! aaa = aaa -o ! bbb = bbb', False),
-
-('test ! ! aaa != aaa -o ! ! bbb != bbb', False),
-('test ! ! aaa != aaa -o ! ! bbb = bbb', True),
-('test ! ! aaa = aaa -o ! ! bbb != bbb', True),
-('test ! ! aaa = aaa -o ! ! bbb = bbb', True),
-)
-
-def exec_hush_if(u_boot_console, expr, result):
-"""Execute a shell "if" command, and validate its result."""
-
-config = u_boot_console.config.buildconfig
-maxargs = int(config.get('config_sys_maxargs', '0'))
-args = len(expr.split(' ')) - 1
-if args > maxargs:
-u_boot_console.log.warning('CONFIG_SYS_MAXARGS too low; need ' +
-str(args))
-pytest.skip()
-
-cmd = 'if ' + expr + '; then echo true; else echo false; fi'
-response = u_boot_console.run_command(cmd)
-assert response.strip() == str(result).lower()
-
-@pytest.mark.buildconfigspec('cmd_echo')
-@pytest.mark.parametrize('expr,result', subtests)
-def test_hush_if_test(u_boot_console, expr, result):
-"""Test a single "if test" condition."""
-
-exec_hush_if(u_boot_console, expr, result)
-
-def test_hush_z(u_boot_console):
-"""Test the -z operator"""
-u_boot_console.run_command('setenv ut_var_nonexistent')
-u_boot_console.run_command('setenv ut_var_exists 1')
-exec_hush_if(u_boot_console, 'test -z "$ut_var_nonexistent"', True)
-exec_hush_if(u_boot_console, 'test -z "$ut_var_exists"', False)
-u_boot_console.run_command('se
[PATCH v13 04/24] test: hush: Test hush variable expansion
Verifies shell variables are replaced by their values.
Reviewed-by: Simon Glass
Signed-off-by: Francis Laniel
---
test/hush/Makefile | 1 +
test/hush/dollar.c | 167 +++
test/py/tests/test_ut.py | 8 +-
3 files changed, 175 insertions(+), 1 deletion(-)
create mode 100644 test/hush/dollar.c
diff --git a/test/hush/Makefile b/test/hush/Makefile
index a3c9ae5106..feb4f71956 100644
--- a/test/hush/Makefile
+++ b/test/hush/Makefile
@@ -5,3 +5,4 @@
obj-y += cmd_ut_hush.o
obj-y += if.o
+obj-y += dollar.o
diff --git a/test/hush/dollar.c b/test/hush/dollar.c
new file mode 100644
index 00..defb2c3fd0
--- /dev/null
+++ b/test/hush/dollar.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2021
+ * Francis Laniel, Amarula Solutions, francis.lan...@amarulasolutions.com
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+static int hush_test_simple_dollar(struct unit_test_state *uts)
+{
+ console_record_reset_enable();
+ ut_assertok(run_command("echo $dollar_foo", 0));
+ ut_assert_nextline_empty();
+ ut_assert_console_end();
+
+ ut_assertok(run_command("echo ${dollar_foo}", 0));
+ ut_assert_nextline_empty();
+ ut_assert_console_end();
+
+ ut_assertok(run_command("dollar_foo=bar", 0));
+
+ ut_assertok(run_command("echo $dollar_foo", 0));
+ ut_assert_nextline("bar");
+ ut_assert_console_end();
+
+ ut_assertok(run_command("echo ${dollar_foo}", 0));
+ ut_assert_nextline("bar");
+ ut_assert_console_end();
+
+ ut_assertok(run_command("dollar_foo=\\$bar", 0));
+
+ ut_assertok(run_command("echo $dollar_foo", 0));
+ ut_assert_nextline("$bar");
+ ut_assert_console_end();
+
+ ut_assertok(run_command("dollar_foo='$bar'", 0));
+
+ ut_assertok(run_command("echo $dollar_foo", 0));
+ ut_assert_nextline("$bar");
+ ut_assert_console_end();
+
+ ut_asserteq(1, run_command("dollar_foo=bar quux", 0));
+ /* Next line contains error message */
+ ut_assert_skipline();
+ ut_assert_console_end();
+
+ ut_asserteq(1, run_command("dollar_foo='bar quux", 0));
+ /* Next line contains error message */
+ ut_assert_skipline();
+ ut_assert_console_end();
+
+ ut_asserteq(1, run_command("dollar_foo=bar quux\"", 0));
+ /* Two next lines contain error message */
+ ut_assert_skipline();
+ ut_assert_skipline();
+ ut_assert_console_end();
+
+ ut_assertok(run_command("dollar_foo='bar \"quux'", 0));
+
+ ut_assertok(run_command("echo $dollar_foo", 0));
+ /*
+* This one is buggy.
+* ut_assert_nextline("bar \"quux");
+* ut_assert_console_end();
+*
+* So, let's reset output:
+*/
+ console_record_reset_enable();
+
+ ut_asserteq(1, run_command("dollar_foo=\"bar 'quux\"", 0));
+ /* Next line contains error message */
+ ut_assert_skipline();
+ ut_assert_console_end();
+
+ ut_assertok(run_command("dollar_foo='bar quux'", 0));
+ ut_assertok(run_command("echo $dollar_foo", 0));
+ ut_assert_nextline("bar quux");
+ ut_assert_console_end();
+
+ puts("Beware: this test set local variable dollar_foo and it cannot be
unset!");
+
+ return 0;
+}
+HUSH_TEST(hush_test_simple_dollar, 0);
+
+static int hush_test_env_dollar(struct unit_test_state *uts)
+{
+ env_set("env_foo", "bar");
+ console_record_reset_enable();
+
+ ut_assertok(run_command("echo $env_foo", 0));
+ ut_assert_nextline("bar");
+ ut_assert_console_end();
+
+ ut_assertok(run_command("echo ${env_foo}", 0));
+ ut_assert_nextline("bar");
+ ut_assert_console_end();
+
+ /* Environment variables have priority over local variable */
+ ut_assertok(run_command("env_foo=quux", 0));
+ ut_assertok(run_command("echo ${env_foo}", 0));
+ ut_assert_nextline("bar");
+ ut_assert_console_end();
+
+ /* Clean up setting the variable */
+ env_set("env_foo", NULL);
+
+ puts("Beware: this test set local variable env_foo and it cannot be
unset!");
+
+ return 0;
+}
+HUSH_TEST(hush_test_env_dollar, 0);
+
+static int hush_test_command_dollar(struct unit_test_state *uts)
+{
+ console_record_reset_enable();
+
+ ut_assertok(run_command("dollar_bar=\"echo bar\"", 0));
+
+ ut_assertok(run_command("$dollar_bar", 0));
+ ut_assert_nextline("bar");
+ ut_assert_console_end();
+
+ ut_assertok(run_command("${dollar_bar}", 0));
+ ut_assert_nextline("bar");
+ ut_assert_console_end();
+
+ ut_assertok(run_command("dollar_bar=\"echo\nbar\"", 0));
+
+ ut_assertok(run_command("$dollar_bar", 0));
+ ut_assert_nextline("bar");
+ ut_assert_console_end();
+
+ ut_assertok(run_command("dollar_bar='echo bar\n'", 0));
+
+ ut_assertok(run_command("$dol
[PATCH v13 05/24] test: hush: Test hush commands list
Verifies behavior of commands separated by ';', '&&' and '||'.
Signed-off-by: Francis Laniel
Reviewed-by: Simon Glass
---
test/hush/Makefile | 1 +
test/hush/list.c | 79 ++
2 files changed, 80 insertions(+)
create mode 100644 test/hush/list.c
diff --git a/test/hush/Makefile b/test/hush/Makefile
index feb4f71956..ff4fe7700b 100644
--- a/test/hush/Makefile
+++ b/test/hush/Makefile
@@ -6,3 +6,4 @@
obj-y += cmd_ut_hush.o
obj-y += if.o
obj-y += dollar.o
+obj-y += list.o
diff --git a/test/hush/list.c b/test/hush/list.c
new file mode 100644
index 00..052cf2783c
--- /dev/null
+++ b/test/hush/list.c
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2021
+ * Francis Laniel, Amarula Solutions, francis.lan...@amarulasolutions.com
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+static int hush_test_semicolon(struct unit_test_state *uts)
+{
+ /* A; B = B truth table. */
+ ut_asserteq(1, run_command("false; false", 0));
+ ut_assertok(run_command("false; true", 0));
+ ut_assertok(run_command("true; true", 0));
+ ut_asserteq(1, run_command("true; false", 0));
+
+ return 0;
+}
+HUSH_TEST(hush_test_semicolon, 0);
+
+static int hush_test_and(struct unit_test_state *uts)
+{
+ /* A && B truth table. */
+ ut_asserteq(1, run_command("false && false", 0));
+ ut_asserteq(1, run_command("false && true", 0));
+ ut_assertok(run_command("true && true", 0));
+ ut_asserteq(1, run_command("true && false", 0));
+
+ return 0;
+}
+HUSH_TEST(hush_test_and, 0);
+
+static int hush_test_or(struct unit_test_state *uts)
+{
+ /* A || B truth table. */
+ ut_asserteq(1, run_command("false || false", 0));
+ ut_assertok(run_command("false || true", 0));
+ ut_assertok(run_command("true || true", 0));
+ ut_assertok(run_command("true || false", 0));
+
+ return 0;
+}
+HUSH_TEST(hush_test_or, 0);
+
+static int hush_test_and_or(struct unit_test_state *uts)
+{
+ /* A && B || C truth table. */
+ ut_asserteq(1, run_command("false && false || false", 0));
+ ut_asserteq(1, run_command("false && false || true", 0));
+ ut_asserteq(1, run_command("false && true || true", 0));
+ ut_asserteq(1, run_command("false && true || false", 0));
+ ut_assertok(run_command("true && true || false", 0));
+ ut_asserteq(1, run_command("true && false || false", 0));
+ ut_assertok(run_command("true && false || true", 0));
+ ut_assertok(run_command("true && true || true", 0));
+
+ return 0;
+}
+HUSH_TEST(hush_test_and_or, 0);
+
+static int hush_test_or_and(struct unit_test_state *uts)
+{
+ /* A || B && C truth table. */
+ ut_asserteq(1, run_command("false || false && false", 0));
+ ut_asserteq(1, run_command("false || false && true", 0));
+ ut_assertok(run_command("false || true && true", 0));
+ ut_asserteq(1, run_command("false || true && false", 0));
+ ut_assertok(run_command("true || true && false", 0));
+ ut_assertok(run_command("true || false && false", 0));
+ ut_assertok(run_command("true || false && true", 0));
+ ut_assertok(run_command("true || true && true", 0));
+
+ return 0;
+}
+HUSH_TEST(hush_test_or_and, 0);
--
2.34.1
[PATCH v13 06/24] test: hush: Test hush loops
The added tests verifies correct behavior of for, while and until loops.
Signed-off-by: Francis Laniel
Reviewed-by: Simon Glass
---
test/hush/Makefile | 1 +
test/hush/loop.c | 65 ++
2 files changed, 66 insertions(+)
create mode 100644 test/hush/loop.c
diff --git a/test/hush/Makefile b/test/hush/Makefile
index ff4fe7700b..a2d98815e5 100644
--- a/test/hush/Makefile
+++ b/test/hush/Makefile
@@ -7,3 +7,4 @@ obj-y += cmd_ut_hush.o
obj-y += if.o
obj-y += dollar.o
obj-y += list.o
+obj-y += loop.o
diff --git a/test/hush/loop.c b/test/hush/loop.c
new file mode 100644
index 00..ca777e38fe
--- /dev/null
+++ b/test/hush/loop.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2021
+ * Francis Laniel, Amarula Solutions, francis.lan...@amarulasolutions.com
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+static int hush_test_for(struct unit_test_state *uts)
+{
+ console_record_reset_enable();
+
+ ut_assertok(run_command("for loop_i in foo bar quux quux; do echo
$loop_i; done", 0));
+ ut_assert_nextline("foo");
+ ut_assert_nextline("bar");
+ ut_assert_nextline("quux");
+ ut_assert_nextline("quux");
+ ut_assert_console_end();
+
+ puts("Beware: this test set local variable loop_i and it cannot be
unset!");
+
+ return 0;
+}
+HUSH_TEST(hush_test_for, 0);
+
+static int hush_test_while(struct unit_test_state *uts)
+{
+ console_record_reset_enable();
+
+ /* Exit status is that of test, so 1 since test is false to quit the
loop. */
+ ut_asserteq(1, run_command("while test -z \"$loop_foo\"; do echo bar;
loop_foo=quux; done", 0));
+ ut_assert_nextline("bar");
+ ut_assert_console_end();
+
+ puts("Beware: this test set local variable loop_foo and it cannot be
unset!");
+
+ return 0;
+}
+HUSH_TEST(hush_test_while, 0);
+
+static int hush_test_until(struct unit_test_state *uts)
+{
+ console_record_reset_enable();
+ env_set("loop_bar", "bar");
+
+ /*
+* WARNING We have to use environment variable because it is not
possible
+* resetting local variable.
+*/
+ ut_assertok(run_command("until test -z \"$loop_bar\"; do echo quux;
setenv loop_bar; done", 0));
+ ut_assert_nextline("quux");
+ ut_assert_console_end();
+
+ /*
+* Loop normally resets foo environment variable, but we reset it here
in
+* case the test failed.
+*/
+ env_set("loop_bar", NULL);
+ return 0;
+}
+HUSH_TEST(hush_test_until, 0);
--
2.34.1
[PATCH v13 09/24] cli: Add menu for hush parser
For the moment, the menu contains only entry: HUSH_OLD_PARSER which is the default. The goal is to prepare the field to add a new hush parser which guarantees actual behavior is still correct. Reviewed-by: Simon Glass Signed-off-by: Francis Laniel --- cmd/Kconfig | 13 + common/Makefile | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index 5a7678f0ac..15715ac6ad 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -22,6 +22,19 @@ config HUSH_PARSER If disabled, you get the old, much simpler behaviour with a somewhat smaller memory footprint. +menu "Hush flavor to use" +depends on HUSH_PARSER + +config HUSH_OLD_PARSER + bool "Use hush old parser" + default y + help + This option enables the old flavor of hush based on hush Busybox from + 2005. + + It is actually the default U-Boot shell when decided to use hush as shell. +endmenu + config CMDLINE_EDITING bool "Enable command line editing" default y diff --git a/common/Makefile b/common/Makefile index 1495436d5d..3bb33b4e36 100644 --- a/common/Makefile +++ b/common/Makefile @@ -9,7 +9,7 @@ obj-y += init/ obj-y += main.o obj-y += exports.o obj-y += cli_getch.o cli_simple.o cli_readline.o -obj-$(CONFIG_HUSH_PARSER) += cli_hush.o +obj-$(CONFIG_HUSH_OLD_PARSER) += cli_hush.o obj-$(CONFIG_AUTOBOOT) += autoboot.o obj-y += version.o -- 2.34.1
[PATCH v13 08/24] cli: Port upstream Busybox hush to U-Boot
Adds new file cli_hush_upstream.c, it is a copy of Busybox hush file as it was
of
time to commit 37460f5da.
This commit modifies Busybox hush to not compile some part specific to Busybox
and adds some code needed by U-Boot.
The modifications consists mainly on adding code #if(n)def guards.
For the moment, this refurbished flavor of hush only permits running command
without any keywords (i.e., if and for are not recognized) or variable expansion
(i.e., echo $foo prints foo and not value stored in variable foo).
A new file was also added to define some functions specific to U-Boot.
Signed-off-by: Francis Laniel
Signed-off-by: Harald Seiler
---
common/cli_hush_modern.c | 274
common/cli_hush_upstream.c | 502 -
2 files changed, 774 insertions(+), 2 deletions(-)
create mode 100644 common/cli_hush_modern.c
diff --git a/common/cli_hush_modern.c b/common/cli_hush_modern.c
new file mode 100644
index 00..34278fdca2
--- /dev/null
+++ b/common/cli_hush_modern.c
@@ -0,0 +1,274 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * This file defines the compilation unit for the new hush shell version. The
+ * actual implementation from upstream BusyBox can be found in
+ * `cli_hush_upstream.c` which is included at the end of this file.
+ *
+ * This "wrapper" technique is used to keep the changes to the upstream version
+ * as minmal as possible. Instead, all defines and redefines necessary are
done
+ * here, outside the upstream sources. This will hopefully make upgrades to
+ * newer revisions much easier.
+ *
+ * Copyright (c) 2021, Harald Seiler, DENX Software Engineering, h...@denx.de
+ */
+
+#include /* readline */
+#include
+#include /* malloc, free, realloc*/
+#include /* isalpha, isdigit */
+#include
+#include
+#include
+#include
+#include /* find_cmd */
+#include
+
+/*
+ * BusyBox Version: UPDATE THIS WHEN PULLING NEW UPSTREAM REVISION!
+ */
+#define BB_VER "1.34.0.git37460f5daff9"
+
+/*
+ * Define hush features by the names used upstream.
+ */
+#define ENABLE_HUSH_INTERACTIVE1
+#define ENABLE_FEATURE_EDITING 1
+/* No MMU in U-Boot */
+#define BB_MMU 0
+#define USE_FOR_NOMMU(...) __VA_ARGS__
+#define USE_FOR_MMU(...)
+
+/*
+ * Size-saving "small" ints (arch-dependent)
+ */
+#if CONFIG_IS_ENABLED(X86) || CONFIG_IS_ENABLED(X86_64) ||
CONFIG_IS_ENABLED(MIPS)
+/* add other arches which benefit from this... */
+typedef signed char smallint;
+typedef unsigned char smalluint;
+#else
+/* for arches where byte accesses generate larger code: */
+typedef int smallint;
+typedef unsigned smalluint;
+#endif
+
+/*
+ * Alignment defines used by BusyBox.
+ */
+#define ALIGN1 __attribute__((aligned(1)))
+#define ALIGN2 __attribute__((aligned(2)))
+#define ALIGN4 __attribute__((aligned(4)))
+#define ALIGN8 __attribute__((aligned(8)))
+#define ALIGN_PTR __attribute__((aligned(sizeof(void*
+
+/*
+ * Miscellaneous compiler/platform defines.
+ */
+#define FAST_FUNC /* not used in U-Boot */
+#define UNUSED_PARAM __always_unused
+#define ALWAYS_INLINE __always_inline
+#define NOINLINE noinline
+
+/*
+ * Defines to provide equivalents to what libc/BusyBox defines.
+ */
+#define EOF(-1)
+#define EXIT_SUCCESS 0
+#define EXIT_FAILURE 1
+
+/*
+ * Stubs to provide libc/BusyBox functions based on U-Boot equivalents where it
+ * makes sense.
+ */
+#define utoa simple_itoa
+
+static void __noreturn xfunc_die(void)
+{
+ panic("HUSH died!");
+}
+
+#define bb_error_msg_and_die(format, ...) do { \
+panic("HUSH: " format, __VA_ARGS__); \
+} while (0);
+
+#define bb_simple_error_msg_and_die(msg) do { \
+panic_str("HUSH: " msg); \
+} while (0);
+
+/* fdprintf() is used for debug output. */
+static int __maybe_unused fdprintf(int fd, const char *format, ...)
+{
+ va_list args;
+ uint i;
+
+ assert(fd == 2);
+
+ va_start(args, format);
+ i = vprintf(format, args);
+ va_end(args);
+
+ return i;
+}
+
+static void bb_verror_msg(const char *s, va_list p, const char* strerr)
+{
+ /* TODO: what to do with strerr arg? */
+ vprintf(s, p);
+}
+
+static void bb_error_msg(const char *s, ...)
+{
+ va_list p;
+
+ va_start(p, s);
+ bb_verror_msg(s, p, NULL);
+ va_end(p);
+}
+
+static void *xmalloc(size_t size)
+{
+ void *p = NULL;
+ if (!(p = malloc(size)))
+ panic("out of memory");
+ return p;
+}
+
+static void *xzalloc(size_t size)
+{
+ void *p = xmalloc(size);
+ memset(p, 0, size);
+ return p;
+}
+
+static void *xrealloc(void *ptr, size_t size)
+{
+ void *p = NULL;
+ if (!(p = realloc(ptr, size)))
+ panic("out of memory");
+ return p;
+}
+
+#define xstrdupstrdup
+#
[PATCH v13 10/24] global_data.h: add GD_FLG_HUSH_OLD_PARSER flag
This flag is used to indicate we are using the hush parser.
Reviewed-by: Simon Glass
Signed-off-by: Francis Laniel
---
common/cli.c | 2 ++
include/asm-generic/global_data.h | 4
2 files changed, 6 insertions(+)
diff --git a/common/cli.c b/common/cli.c
index 3916a7b10a..e5fe1060d0 100644
--- a/common/cli.c
+++ b/common/cli.c
@@ -268,6 +268,8 @@ void cli_loop(void)
void cli_init(void)
{
#ifdef CONFIG_HUSH_PARSER
+ if (!(gd->flags & GD_FLG_HUSH_OLD_PARSER))
+ gd->flags |= GD_FLG_HUSH_OLD_PARSER;
u_boot_hush_start();
#endif
diff --git a/include/asm-generic/global_data.h
b/include/asm-generic/global_data.h
index e8c6412e3f..0a9b6bd92a 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -697,6 +697,10 @@ enum gd_flags {
* @GD_FLG_BLOBLIST_READY: bloblist is ready for use
*/
GD_FLG_BLOBLIST_READY = 0x80,
+ /**
+* @GD_FLG_HUSH_OLD_PARSER: Use hush old parser.
+*/
+ GD_FLG_HUSH_OLD_PARSER = 0x100,
};
#endif /* __ASSEMBLY__ */
--
2.34.1
[PATCH v13 11/24] cmd: Add new cli command
This command can be used to print the current parser with 'cli get'.
It can also be used to set the current parser with 'cli set'.
For the moment, only one value is valid for set: old.
Signed-off-by: Francis Laniel
---
cmd/Makefile | 2 +
cmd/cli.c | 114 ++
common/cli.c | 3 +-
doc/usage/cmd/cli.rst | 59 ++
doc/usage/index.rst | 1 +
5 files changed, 178 insertions(+), 1 deletion(-)
create mode 100644 cmd/cli.c
create mode 100644 doc/usage/cmd/cli.rst
diff --git a/cmd/Makefile b/cmd/Makefile
index 5ed0e4011d..477b86cf23 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -229,6 +229,8 @@ obj-$(CONFIG_CMD_AVB) += avb.o
# Foundries.IO SCP03
obj-$(CONFIG_CMD_SCP03) += scp03.o
+obj-$(CONFIG_HUSH_PARSER) += cli.o
+
obj-$(CONFIG_ARM) += arm/
obj-$(CONFIG_RISCV) += riscv/
obj-$(CONFIG_SANDBOX) += sandbox/
diff --git a/cmd/cli.c b/cmd/cli.c
new file mode 100644
index 00..86c6471aa4
--- /dev/null
+++ b/cmd/cli.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include
+#include
+#include
+#include
+#include
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static const char *gd_flags_to_parser_name(void)
+{
+ if (gd->flags & GD_FLG_HUSH_OLD_PARSER)
+ return "old";
+ return NULL;
+}
+
+static int do_cli_get(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])
+{
+ const char *current = gd_flags_to_parser_name();
+
+ if (!current) {
+ printf("current cli value is not valid, this should not
happen!\n");
+ return CMD_RET_FAILURE;
+ }
+
+ printf("%s\n", current);
+
+ return CMD_RET_SUCCESS;
+}
+
+static int parser_string_to_gd_flags(const char *parser)
+{
+ if (!strcmp(parser, "old"))
+ return GD_FLG_HUSH_OLD_PARSER;
+ return -1;
+}
+
+static void reset_parser_gd_flags(void)
+{
+ gd->flags &= ~GD_FLG_HUSH_OLD_PARSER;
+}
+
+static int do_cli_set(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ char *parser_name;
+ int parser_flag;
+
+ if (argc < 2)
+ return CMD_RET_USAGE;
+
+ parser_name = argv[1];
+
+ parser_flag = parser_string_to_gd_flags(parser_name);
+ if (parser_flag == -1) {
+ printf("Bad value for parser name: %s\n", parser_name);
+ return CMD_RET_USAGE;
+ }
+
+ if (parser_flag == GD_FLG_HUSH_OLD_PARSER &&
+ !CONFIG_IS_ENABLED(HUSH_OLD_PARSER)) {
+ printf("Want to set current parser to old, but its code was not
compiled!\n");
+ return CMD_RET_FAILURE;
+ }
+
+ reset_parser_gd_flags();
+ gd->flags |= parser_flag;
+
+ cli_init();
+ cli_loop();
+
+ /* cli_loop() should never return. */
+ return CMD_RET_FAILURE;
+}
+
+static struct cmd_tbl parser_sub[] = {
+ U_BOOT_CMD_MKENT(get, 1, 1, do_cli_get, "", ""),
+ U_BOOT_CMD_MKENT(set, 2, 1, do_cli_set, "", ""),
+};
+
+static int do_cli(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ struct cmd_tbl *cp;
+
+ if (argc < 2)
+ return CMD_RET_USAGE;
+
+ /* drop initial "parser" arg */
+ argc--;
+ argv++;
+
+ cp = find_cmd_tbl(argv[0], parser_sub, ARRAY_SIZE(parser_sub));
+ if (cp)
+ return cp->cmd(cmdtp, flag, argc, argv);
+
+ return CMD_RET_USAGE;
+}
+
+#if CONFIG_IS_ENABLED(SYS_LONGHELP)
+static char cli_help_text[] =
+ "get - print current cli\n"
+ "set - set the current cli, possible value is: old"
+ ;
+#endif
+
+U_BOOT_CMD(cli, 3, 1, do_cli,
+ "cli",
+#if CONFIG_IS_ENABLED(SYS_LONGHELP)
+ cli_help_text
+#endif
+);
diff --git a/common/cli.c b/common/cli.c
index e5fe1060d0..d419671e8c 100644
--- a/common/cli.c
+++ b/common/cli.c
@@ -268,7 +268,8 @@ void cli_loop(void)
void cli_init(void)
{
#ifdef CONFIG_HUSH_PARSER
- if (!(gd->flags & GD_FLG_HUSH_OLD_PARSER))
+ if (!(gd->flags & GD_FLG_HUSH_OLD_PARSER)
+ && CONFIG_IS_ENABLED(HUSH_OLD_PARSER))
gd->flags |= GD_FLG_HUSH_OLD_PARSER;
u_boot_hush_start();
#endif
diff --git a/doc/usage/cmd/cli.rst b/doc/usage/cmd/cli.rst
new file mode 100644
index 00..89ece3203d
--- /dev/null
+++ b/doc/usage/cmd/cli.rst
@@ -0,0 +1,59 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+cli command
+===
+
+Synopis
+---
+
+::
+
+cli get
+cli set cli_flavor
+
+Description
+---
+
+The cli command permits getting and changing the current parser at runtime.
+
+cli get
+~~~
+
+It shows the current value of the parser used by the CLI.
+
+cli set
+~~~
+
+It permits setting the value of the parser used by the CLI.
+
+Possible values are old and 2021.
+Note that, to use a specific parser its code should have been compiled, that
+is to say y
[PATCH v13 12/24] cli: Enables using modern hush parser as command line parser
If one defines HUSH_MODERN_PARSER, it is then possible to use modern parser
with:
=> cli get
old
=> cli set modern
=> cli get
modern
Reviewed-by: Simon Glass
Signed-off-by: Francis Laniel
---
cmd/Kconfig | 12
cmd/Makefile | 2 +-
cmd/cli.c | 28 ++---
common/Makefile | 1 +
common/cli.c | 38 +++
common/cli_hush_modern.c | 3 ++
common/cli_hush_upstream.c| 46 +---
doc/usage/cmd/cli.rst | 19 ++--
include/asm-generic/global_data.h | 4 +++
include/cli_hush.h| 51 +--
10 files changed, 184 insertions(+), 20 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 15715ac6ad..e25578cde3 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -33,6 +33,18 @@ config HUSH_OLD_PARSER
2005.
It is actually the default U-Boot shell when decided to use hush as
shell.
+
+config HUSH_MODERN_PARSER
+ bool "Use hush modern parser"
+ help
+ This option enables the new flavor of hush based on hush upstream
+ Busybox.
+
+ This parser is experimental and not well tested.
+
+config HUSH_SELECTABLE
+ bool
+ default y if HUSH_OLD_PARSER && HUSH_MODERN_PARSER
endmenu
config CMDLINE_EDITING
diff --git a/cmd/Makefile b/cmd/Makefile
index 477b86cf23..e2a2b16ab2 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -229,7 +229,7 @@ obj-$(CONFIG_CMD_AVB) += avb.o
# Foundries.IO SCP03
obj-$(CONFIG_CMD_SCP03) += scp03.o
-obj-$(CONFIG_HUSH_PARSER) += cli.o
+obj-$(CONFIG_HUSH_SELECTABLE) += cli.o
obj-$(CONFIG_ARM) += arm/
obj-$(CONFIG_RISCV) += riscv/
diff --git a/cmd/cli.c b/cmd/cli.c
index 86c6471aa4..b93cc952ed 100644
--- a/cmd/cli.c
+++ b/cmd/cli.c
@@ -12,6 +12,8 @@ static const char *gd_flags_to_parser_name(void)
{
if (gd->flags & GD_FLG_HUSH_OLD_PARSER)
return "old";
+ if (gd->flags & GD_FLG_HUSH_MODERN_PARSER)
+ return "modern";
return NULL;
}
@@ -34,18 +36,31 @@ static int parser_string_to_gd_flags(const char *parser)
{
if (!strcmp(parser, "old"))
return GD_FLG_HUSH_OLD_PARSER;
+ if (!strcmp(parser, "modern"))
+ return GD_FLG_HUSH_MODERN_PARSER;
+ return -1;
+}
+
+static int gd_flags_to_parser_config(int flag)
+{
+ if (gd->flags & GD_FLG_HUSH_OLD_PARSER)
+ return CONFIG_VAL(HUSH_OLD_PARSER);
+ if (gd->flags & GD_FLG_HUSH_MODERN_PARSER)
+ return CONFIG_VAL(HUSH_MODERN_PARSER);
return -1;
}
static void reset_parser_gd_flags(void)
{
gd->flags &= ~GD_FLG_HUSH_OLD_PARSER;
+ gd->flags &= ~GD_FLG_HUSH_MODERN_PARSER;
}
static int do_cli_set(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
{
char *parser_name;
+ int parser_config;
int parser_flag;
if (argc < 2)
@@ -59,9 +74,14 @@ static int do_cli_set(struct cmd_tbl *cmdtp, int flag, int
argc,
return CMD_RET_USAGE;
}
- if (parser_flag == GD_FLG_HUSH_OLD_PARSER &&
- !CONFIG_IS_ENABLED(HUSH_OLD_PARSER)) {
- printf("Want to set current parser to old, but its code was not
compiled!\n");
+ parser_config = gd_flags_to_parser_config(parser_flag);
+ switch (parser_config) {
+ case -1:
+ printf("Bad value for parser flags: %d\n", parser_flag);
+ return CMD_RET_FAILURE;
+ case 0:
+ printf("Want to set current parser to %s, but its code was not
compiled!\n",
+ parser_name);
return CMD_RET_FAILURE;
}
@@ -102,7 +122,7 @@ static int do_cli(struct cmd_tbl *cmdtp, int flag, int argc,
#if CONFIG_IS_ENABLED(SYS_LONGHELP)
static char cli_help_text[] =
"get - print current cli\n"
- "set - set the current cli, possible value is: old"
+ "set - set the current cli, possible value are: old, modern"
;
#endif
diff --git a/common/Makefile b/common/Makefile
index 3bb33b4e36..f010c2a1b9 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -10,6 +10,7 @@ obj-y += main.o
obj-y += exports.o
obj-y += cli_getch.o cli_simple.o cli_readline.o
obj-$(CONFIG_HUSH_OLD_PARSER) += cli_hush.o
+obj-$(CONFIG_HUSH_MODERN_PARSER) += cli_hush_modern.o
obj-$(CONFIG_AUTOBOOT) += autoboot.o
obj-y += version.o
diff --git a/common/cli.c b/common/cli.c
index d419671e8c..b3eb512b62 100644
--- a/common/cli.c
+++ b/common/cli.c
@@ -43,12 +43,15 @@ int run_command(const char *cmd, int flag)
return 1;
return 0;
-#else
+#elif CONFIG_IS_ENABLED(HUSH_OLD_PARSER)
int hush_flags = FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP;
if (flag & CMD_FLAG_ENV)
hush_flags |= FLAG_CONT_ON_NEWLINE;
return parse_string
[PATCH v13 14/24] cli: hush_modern: Add functions to be called from run_command()
run_command() is called internally by the command run and it can also be called
directly from U-Boot code, e.g. to do unit tests.
This commit adds this path to go to modern hush.
Signed-off-by: Francis Laniel
Reviewed-by: Simon Glass
---
common/cli_hush_upstream.c | 66 --
1 file changed, 63 insertions(+), 3 deletions(-)
diff --git a/common/cli_hush_upstream.c b/common/cli_hush_upstream.c
index ff4a57c6e3..7f6b058e6c 100644
--- a/common/cli_hush_upstream.c
+++ b/common/cli_hush_upstream.c
@@ -1011,6 +1011,7 @@ struct globals {
#ifdef __U_BOOT__
int flag_repeat;
int do_repeat;
+ int run_command_flags;
#endif /* __U_BOOT__ */
char *ifs_whitespace; /* = G.ifs or malloced */
#ifndef __U_BOOT__
@@ -3004,7 +3005,24 @@ static int i_getch(struct in_str *i)
if (i->p && *i->p != '\0') {
ch = (unsigned char)*i->p++;
goto out;
+#ifndef __U_BOOT__
}
+#else /* __U_BOOT__ */
+ /*
+* There are two ways for command to be called:
+* 1. The first one is when they are typed by the user.
+* 2. The second one is through run_command() (NOTE command run
+* internally calls run_command()).
+*
+* In the second case, we do not get input from the user, so once we
+* get a '\0', it means we need to stop.
+* NOTE G.run_command_flags is only set on run_command call stack, so
+* we use this to know if we come from user input or run_command().
+*/
+ } else if (i->p && *i->p == '\0' && G.run_command_flags){
+ return EOF;
+ }
+#endif /* __U_BOOT__ */
#endif
#ifndef __U_BOOT__
/* peek_buf[] is an int array, not char. Can contain EOF. */
@@ -3163,7 +3181,6 @@ static void setup_file_in_str(struct in_str *i)
#endif /* !__U_BOOT__ */
}
-#ifndef __U_BOOT__
static void setup_string_in_str(struct in_str *i, const char *s)
{
memset(i, 0, sizeof(*i));
@@ -3171,7 +3188,6 @@ static void setup_string_in_str(struct in_str *i, const
char *s)
i->p = s;
}
-#endif /* !__U_BOOT__ */
/*
* o_string support
@@ -7910,7 +7926,11 @@ static int run_and_free_list(struct pipe *pi);
* NUL: parse all, execute, return
* ';': parse till ';' or newline, execute, repeat till EOF
*/
+#ifndef __U_BOOT__
static void parse_and_run_stream(struct in_str *inp, int end_trigger)
+#else /* __U_BOOT__ */
+static int parse_and_run_stream(struct in_str *inp, int end_trigger)
+#endif /* __U_BOOT__ */
{
/* Why we need empty flag?
* An obscure corner case "false; ``; echo $?":
@@ -7919,7 +7939,11 @@ static void parse_and_run_stream(struct in_str *inp, int
end_trigger)
* this breaks "false; echo `echo $?`" case.
*/
bool empty = 1;
+#ifndef __U_BOOT__
while (1) {
+#else /* __U_BOOT__ */
+ do {
+#endif /* __U_BOOT__ */
struct pipe *pipe_list;
#if ENABLE_HUSH_INTERACTIVE
@@ -7966,21 +7990,57 @@ static void parse_and_run_stream(struct in_str *inp,
int end_trigger)
empty = 0;
if (G_flag_return_in_progress == 1)
break;
+#ifndef __U_BOOT__
}
+#else /* __U_BOOT__ */
+ /*
+* This do/while is needed by run_command to avoid looping on a command
+* with syntax error.
+*/
+ } while (!(G.run_command_flags & FLAG_EXIT_FROM_LOOP));
+
+ return G.last_exitcode;
+#endif /* __U_BOOT__ */
}
#ifndef __U_BOOT__
static void parse_and_run_string(const char *s)
+#else /* __U_BOOT__ */
+static int parse_and_run_string(const char *s)
+#endif /* __U_BOOT__ */
{
struct in_str input;
//IF_HUSH_LINENO_VAR(unsigned sv = G.parse_lineno;)
setup_string_in_str(&input, s);
+#ifndef __U_BOOT__
parse_and_run_stream(&input, '\0');
+#else /* __U_BOOT__ */
+ return parse_and_run_stream(&input, '\0');
+#endif /* __U_BOOT__ */
//IF_HUSH_LINENO_VAR(G.parse_lineno = sv;)
}
-#endif /* !__U_BOOT__ */
+#ifdef __U_BOOT__
+int parse_string_outer(const char *cmd, int flags)
+{
+ int ret;
+ int old_flags;
+
+ /*
+* Keep old values of run_command to be able to restore them once
+* command was executed.
+*/
+ old_flags = G.run_command_flags;
+ G.run_command_flags = flags;
+
+ ret = parse_and_run_string(cmd);
+
+ G.run_command_flags = old_flags;
+
+ return ret;
+}
+#endif /* __U_BOOT__ */
#ifndef __U_BOOT__
static void parse_and_run_file(HFILE *fp)
#else /* __U_BOOT__ */
--
2.34.1
[PATCH v13 13/24] cli: hush_modern: Enable variables expansion for modern hush
Enables variables expansion for modern hush, both for local and environment
variables.
So the following commands:
foo=bar
echo $foo
setenv bar foo
echo $bar
leads to "bar" and "foo" being printed on console output.
Signed-off-by: Francis Laniel
Reviewed-by: Simon Glass
---
common/cli_hush_modern.c | 17 +++
common/cli_hush_upstream.c | 91 +++---
2 files changed, 103 insertions(+), 5 deletions(-)
diff --git a/common/cli_hush_modern.c b/common/cli_hush_modern.c
index 626fed089b..15bb1f0d92 100644
--- a/common/cli_hush_modern.c
+++ b/common/cli_hush_modern.c
@@ -207,6 +207,23 @@ static const char* endofname(const char *name)
return name;
}
+/**
+ * list_size() - returns the number of elements in char ** before NULL.
+ *
+ * Argument must contain NULL to signalize its end.
+ *
+ * @list The list to count the number of element.
+ * @return The number of element in list.
+ */
+static size_t list_size(char **list)
+{
+ size_t size;
+
+ for (size = 0; list[size] != NULL; size++);
+
+ return size;
+}
+
struct in_str;
static int u_boot_cli_readline(struct in_str *i);
diff --git a/common/cli_hush_upstream.c b/common/cli_hush_upstream.c
index 4b6ab20f3b..ff4a57c6e3 100644
--- a/common/cli_hush_upstream.c
+++ b/common/cli_hush_upstream.c
@@ -3485,7 +3485,6 @@ static int o_get_last_ptr(o_string *o, int n)
return ((int)(uintptr_t)list[n-1]) + string_start;
}
-#ifndef __U_BOOT__
/*
* Globbing routines.
*
@@ -3740,8 +3739,10 @@ static int glob_needed(const char *s)
*/
static int perform_glob(o_string *o, int n)
{
+#ifndef __U_BOOT__
glob_t globdata;
int gr;
+#endif /* __U_BOOT__ */
char *pattern;
debug_printf_glob("start perform_glob: n:%d o->data:%p\n", n, o->data);
@@ -3750,13 +3751,16 @@ static int perform_glob(o_string *o, int n)
pattern = o->data + o_get_last_ptr(o, n);
debug_printf_glob("glob pattern '%s'\n", pattern);
if (!glob_needed(pattern)) {
+#ifndef __U_BOOT__
literal:
+#endif /* __U_BOOT__ */
/* unbackslash last string in o in place, fix length */
o->length = unbackslash(pattern) - o->data;
debug_printf_glob("glob pattern '%s' is literal\n", pattern);
return o_save_ptr_helper(o, n);
}
+#ifndef __U_BOOT__
memset(&globdata, 0, sizeof(globdata));
/* Can't use GLOB_NOCHECK: it does not unescape the string.
* If we glob "*.\*" and don't find anything, we need
@@ -3792,16 +3796,22 @@ static int perform_glob(o_string *o, int n)
if (DEBUG_GLOB)
debug_print_list("perform_glob returning", o, n);
return n;
+#else /* __U_BOOT__ */
+ /*
+* NOTE We only use perform glob to call unbackslash to remove backslash
+* from string once expanded.
+* So, it seems OK to return this if no previous return was done.
+*/
+ return o_save_ptr_helper(o, n);
+#endif /* __U_BOOT__ */
}
-#endif /* !__U_BOOT__ */
#endif /* !HUSH_BRACE_EXPANSION */
/* If o->o_expflags & EXP_FLAG_GLOB, glob the string so far remembered.
* Otherwise, just finish current list[] and start new */
static int o_save_ptr(o_string *o, int n)
{
-#ifndef __U_BOOT__
if (o->o_expflags & EXP_FLAG_GLOB) {
/* If o->has_empty_slot, list[n] was already globbed
* (if it was requested back then when it was filled)
@@ -3809,7 +3819,6 @@ static int o_save_ptr(o_string *o, int n)
if (!o->has_empty_slot)
return perform_glob(o, n); /* o_save_ptr_helper is
inside */
}
-#endif /* !__U_BOOT__ */
return o_save_ptr_helper(o, n);
}
@@ -5455,7 +5464,20 @@ static int parse_dollar(o_string *as_string,
nommu_addchr(as_string, ch);
if (ch == '}')
break;
+#ifndef __U_BOOT__
if (!isalnum(ch) && ch != '_') {
+#else /* __U_BOOT__ */
+ /*
+* In several places in U-Boot, we use variable like
+* foo# (e.g. serial#), particularly in env.
+* So, we need to authorize # to appear inside
+* variable name and then expand this variable.
+* NOTE Having # in variable name is not permitted in
+* upstream hush but expansion will be done (even though
+* the result will be empty).
+*/
+ if (!isalnum(ch) && ch != '_' && ch != '#') {
+#endif /* __U_BOOT__ */
unsigned end_ch;
#ifndef __U_BOOT__
unsigned char last_ch;
@@ -7030,7 +7052,20 @@ static NOINLINE int expand_one_var(o_string *output, int
n,
}
#endif /* !__U_BOOT__ */
default:
+#ifndef __U_BOOT__
[PATCH v13 15/24] cli: add modern hush as parser for run_command*()
Enables using, in code, modern hush as parser for run_command function family.
It also enables the command run to be used by CLI user of modern hush.
Reviewed-by: Simon Glass
Signed-off-by: Francis Laniel
---
common/cli.c | 67 --
common/cli_hush_upstream.c | 2 +-
test/boot/bootflow.c | 16 -
3 files changed, 66 insertions(+), 19 deletions(-)
diff --git a/common/cli.c b/common/cli.c
index b3eb512b62..a34938294e 100644
--- a/common/cli.c
+++ b/common/cli.c
@@ -25,6 +25,14 @@
#include
#ifdef CONFIG_CMDLINE
+
+static inline bool use_hush_old(void)
+{
+ return IS_ENABLED(CONFIG_HUSH_SELECTABLE) ?
+ gd->flags & GD_FLG_HUSH_OLD_PARSER :
+ IS_ENABLED(CONFIG_HUSH_OLD_PARSER);
+}
+
/*
* Run a command using the selected parser.
*
@@ -43,15 +51,30 @@ int run_command(const char *cmd, int flag)
return 1;
return 0;
-#elif CONFIG_IS_ENABLED(HUSH_OLD_PARSER)
- int hush_flags = FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP;
-
- if (flag & CMD_FLAG_ENV)
- hush_flags |= FLAG_CONT_ON_NEWLINE;
- return parse_string_outer(cmd, hush_flags);
-#else /* HUSH_MODERN_PARSER */
- /* Not yet implemented. */
- return 1;
+#else
+ if (use_hush_old()) {
+ int hush_flags = FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP;
+
+ if (flag & CMD_FLAG_ENV)
+ hush_flags |= FLAG_CONT_ON_NEWLINE;
+ return parse_string_outer(cmd, hush_flags);
+ }
+ /*
+* Possible values for flags are the following:
+* FLAG_EXIT_FROM_LOOP: This flags ensures we exit from loop in
+* parse_and_run_stream() after first iteration while normal
+* behavior, * i.e. when called from cli_loop(), is to loop
+* infinitely.
+* FLAG_PARSE_SEMICOLON: modern Hush parses ';' and does not stop
+* first time it sees one. So, I think we do not need this flag.
+* FLAG_REPARSING: For the moment, I do not understand the goal
+* of this flag.
+* FLAG_CONT_ON_NEWLINE: This flag seems to be used to continue
+* parsing even when reading '\n' when coming from
+* run_command(). In this case, modern Hush reads until it finds
+* '\0'. So, I think we do not need this flag.
+*/
+ return parse_string_outer_modern(cmd, FLAG_EXIT_FROM_LOOP);
#endif
}
@@ -67,12 +90,23 @@ int run_command_repeatable(const char *cmd, int flag)
#ifndef CONFIG_HUSH_PARSER
return cli_simple_run_command(cmd, flag);
#else
+ int ret;
+
+ if (use_hush_old()) {
+ ret = parse_string_outer(cmd,
+FLAG_PARSE_SEMICOLON
+| FLAG_EXIT_FROM_LOOP);
+ } else {
+ ret = parse_string_outer_modern(cmd,
+ FLAG_PARSE_SEMICOLON
+ | FLAG_EXIT_FROM_LOOP);
+ }
+
/*
* parse_string_outer() returns 1 for failure, so clean up
* its result.
*/
- if (parse_string_outer(cmd,
- FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP))
+ if (ret)
return -1;
return 0;
@@ -111,12 +145,11 @@ int run_command_list(const char *cmd, int len, int flag)
buff[len] = '\0';
}
#ifdef CONFIG_HUSH_PARSER
-#if CONFIG_IS_ENABLED(HUSH_OLD_PARSER)
- rcode = parse_string_outer(buff, FLAG_PARSE_SEMICOLON);
-#else /* HUSH_MODERN_PARSER */
- /* Not yet implemented. */
- rcode = 1;
-#endif
+ if (use_hush_old()) {
+ rcode = parse_string_outer(buff, FLAG_PARSE_SEMICOLON);
+ } else {
+ rcode = parse_string_outer_modern(buff, FLAG_PARSE_SEMICOLON);
+ }
#else
/*
* This function will overwrite any \n it sees with a \0, which
diff --git a/common/cli_hush_upstream.c b/common/cli_hush_upstream.c
index 7f6b058e6c..11870c51e8 100644
--- a/common/cli_hush_upstream.c
+++ b/common/cli_hush_upstream.c
@@ -8022,7 +8022,7 @@ static int parse_and_run_string(const char *s)
}
#ifdef __U_BOOT__
-int parse_string_outer(const char *cmd, int flags)
+int parse_string_outer_modern(const char *cmd, int flags)
{
int ret;
int old_flags;
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index a9b555c779..104f49deef 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -710,7 +710,21 @@ static int bootflow_scan_menu_boot(struct unit_test_state
*uts)
ut_assert_skip_to_line("(2 bootflows, 2 valid)");
ut_assert_nextline("Selected: Armbian");
- ut_assert_skip_to_line("Boot failed (err=-14)");
+
+ if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ /*
+* With old hush, despite booti failing to boot, i.e. returning
+* CMD_RET_FAILURE, run_c
[PATCH v13 16/24] test: hush: Fix instructions list tests for modern hush
Modifies the expected result for modern hush.
Indeed, there were bugs in actual U-Boot hush which were fixed in upstream
Busybox.
As modern hush is based on upstream Busybox, these bugs no longer exist.
Reviewed-by: Simon Glass
Signed-off-by: Francis Laniel
---
test/hush/list.c | 69 +---
1 file changed, 65 insertions(+), 4 deletions(-)
diff --git a/test/hush/list.c b/test/hush/list.c
index 052cf2783c..73ae36e88f 100644
--- a/test/hush/list.c
+++ b/test/hush/list.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
static int hush_test_semicolon(struct unit_test_state *uts)
{
@@ -46,12 +47,43 @@ static int hush_test_or(struct unit_test_state *uts)
}
HUSH_TEST(hush_test_or, 0);
+DECLARE_GLOBAL_DATA_PTR;
+
static int hush_test_and_or(struct unit_test_state *uts)
{
/* A && B || C truth table. */
ut_asserteq(1, run_command("false && false || false", 0));
- ut_asserteq(1, run_command("false && false || true", 0));
- ut_asserteq(1, run_command("false && true || true", 0));
+
+ if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ ut_asserteq(1, run_command("false && false || true", 0));
+ } else if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+ /*
+* This difference seems to come from a bug solved in Busybox
+* hush.
+*
+* Indeed, the following expression can be seen like this:
+* (false && false) || true
+* So, (false && false) returns 1, the second false is not
+* executed, and true is executed because of ||.
+*/
+ ut_assertok(run_command("false && false || true", 0));
+ }
+
+ if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ ut_asserteq(1, run_command("false && true || true", 0));
+ } else if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+ /*
+* This difference seems to come from a bug solved in Busybox
+* hush.
+*
+* Indeed, the following expression can be seen like this:
+* (false && true) || true
+* So, (false && true) returns 1, the true is not executed, and
+* true is executed because of ||.
+*/
+ ut_assertok(run_command("false && true || true", 0));
+ }
+
ut_asserteq(1, run_command("false && true || false", 0));
ut_assertok(run_command("true && true || false", 0));
ut_asserteq(1, run_command("true && false || false", 0));
@@ -69,8 +101,37 @@ static int hush_test_or_and(struct unit_test_state *uts)
ut_asserteq(1, run_command("false || false && true", 0));
ut_assertok(run_command("false || true && true", 0));
ut_asserteq(1, run_command("false || true && false", 0));
- ut_assertok(run_command("true || true && false", 0));
- ut_assertok(run_command("true || false && false", 0));
+
+ if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ ut_assertok(run_command("true || true && false", 0));
+ } else if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+ /*
+* This difference seems to come from a bug solved in Busybox
+* hush.
+*
+* Indeed, the following expression can be seen like this:
+* (true || true) && false
+* So, (true || true) returns 0, the second true is not
+* executed, and then false is executed because of &&.
+*/
+ ut_asserteq(1, run_command("true || true && false", 0));
+ }
+
+ if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ ut_assertok(run_command("true || false && false", 0));
+ } else if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+ /*
+* This difference seems to come from a bug solved in Busybox
+* hush.
+*
+* Indeed, the following expression can be seen like this:
+* (true || false) && false
+* So, (true || false) returns 0, the false is not executed, and
+* then false is executed because of &&.
+*/
+ ut_asserteq(1, run_command("true || false && false", 0));
+ }
+
ut_assertok(run_command("true || false && true", 0));
ut_assertok(run_command("true || true && true", 0));
--
2.34.1
[PATCH v13 17/24] test: hush: Fix variable expansion tests for modern hush
Modifies the expected result for modern hush.
Indeed, there were bugs in actual U-Boot hush which were fixed in upstream
Busybox.
As modern hush is based on upstream Busybox, these bugs no longer exist.
Reviewed-by: Simon Glass
Signed-off-by: Francis Laniel
---
test/hush/dollar.c | 79 --
1 file changed, 69 insertions(+), 10 deletions(-)
diff --git a/test/hush/dollar.c b/test/hush/dollar.c
index defb2c3fd0..7ab7ea7084 100644
--- a/test/hush/dollar.c
+++ b/test/hush/dollar.c
@@ -9,6 +9,9 @@
#include
#include
#include
+#include
+
+DECLARE_GLOBAL_DATA_PTR;
static int hush_test_simple_dollar(struct unit_test_state *uts)
{
@@ -51,13 +54,29 @@ static int hush_test_simple_dollar(struct unit_test_state
*uts)
ut_asserteq(1, run_command("dollar_foo='bar quux", 0));
/* Next line contains error message */
ut_assert_skipline();
- ut_assert_console_end();
+
+ if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+ /*
+* For some strange reasons, the console is not empty after
+* running above command.
+* So, we reset it to not have side effects for other tests.
+*/
+ console_record_reset_enable();
+ } else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ ut_assert_console_end();
+ }
ut_asserteq(1, run_command("dollar_foo=bar quux\"", 0));
/* Two next lines contain error message */
ut_assert_skipline();
ut_assert_skipline();
- ut_assert_console_end();
+
+ if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+ /* See above comments. */
+ console_record_reset_enable();
+ } else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ ut_assert_console_end();
+ }
ut_assertok(run_command("dollar_foo='bar \"quux'", 0));
@@ -71,17 +90,35 @@ static int hush_test_simple_dollar(struct unit_test_state
*uts)
*/
console_record_reset_enable();
- ut_asserteq(1, run_command("dollar_foo=\"bar 'quux\"", 0));
- /* Next line contains error message */
- ut_assert_skipline();
- ut_assert_console_end();
+ if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+ /*
+* Old parser returns an error because it waits for closing
+* '\'', but this behavior is wrong as the '\'' is surrounded by
+* '"', so no need to wait for a closing one.
+*/
+ ut_assertok(run_command("dollar_foo=\"bar 'quux\"", 0));
+
+ ut_assertok(run_command("echo $dollar_foo", 0));
+ ut_assert_nextline("bar 'quux");
+ ut_assert_console_end();
+ } else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ ut_asserteq(1, run_command("dollar_foo=\"bar 'quux\"", 0));
+ /* Next line contains error message */
+ ut_assert_skipline();
+ ut_assert_console_end();
+ }
ut_assertok(run_command("dollar_foo='bar quux'", 0));
ut_assertok(run_command("echo $dollar_foo", 0));
ut_assert_nextline("bar quux");
ut_assert_console_end();
- puts("Beware: this test set local variable dollar_foo and it cannot be
unset!");
+ if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+ /* Reset local variable. */
+ ut_assertok(run_command("dollar_foo=", 0));
+ } else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ puts("Beware: this test set local variable dollar_foo and it
cannot be unset!");
+ }
return 0;
}
@@ -109,7 +146,12 @@ static int hush_test_env_dollar(struct unit_test_state
*uts)
/* Clean up setting the variable */
env_set("env_foo", NULL);
- puts("Beware: this test set local variable env_foo and it cannot be
unset!");
+ if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+ /* Reset local variable. */
+ ut_assertok(run_command("env_foo=", 0));
+ } else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ puts("Beware: this test set local variable env_foo and it
cannot be unset!");
+ }
return 0;
}
@@ -144,7 +186,18 @@ static int hush_test_command_dollar(struct unit_test_state
*uts)
ut_assertok(run_command("dollar_bar='echo bar\\n'", 0));
ut_assertok(run_command("$dollar_bar", 0));
- ut_assert_nextline("barn");
+
+ if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+ /*
+* This difference seems to come from a bug solved in Busybox
+* hush.
+* Behavior of hush 2021 is coherent with bash and other shells.
+*/
+ ut_assert_nextline("bar\\n");
+ } else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ ut_assert_nextline("barn");
+ }
+
ut_assert_console_end();
ut_
[PATCH v13 18/24] cli: hush_modern: Enable using < and > as string compare operators
In Busybox hush, '<' and '>' are used as redirection operators.
For example, cat foo > bar will write content of file foo inside file bar.
In U-Boot, we do not have file system, so we can hardly redirect command output
inside a file.
But, in actual U-Boot hush, these operators ('<' and '>') are used as string
compare operators.
For example, test aaa < bbb returns 0 as aaa is before bbb in the dictionary.
Busybox hush also permits this, but operators need to be escaped ('\<' and
'\>').
Indeed, if escaping is needed it permits the developer to think about its code,
as in a lot of case, we want to compare integers (using '-lt' or '-gt') rather
than strings.
As testing in U-Boot is handled by the test command, we will stick with the
original behaviour and not adapt to Busybox one.
Nonetheless, if one day we decide to implement test with '[[ ]]', we will then
stick to upstream Busybox behavior.
Signed-off-by: Francis Laniel
Reviewed-by: Simon Glass
---
common/cli_hush_upstream.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/common/cli_hush_upstream.c b/common/cli_hush_upstream.c
index 11870c51e8..a56ea6c5c8 100644
--- a/common/cli_hush_upstream.c
+++ b/common/cli_hush_upstream.c
@@ -6159,7 +6159,28 @@ static struct pipe *parse_stream(char **pstring,
if (parse_redirect(&ctx, redir_fd, redir_style, input))
goto parse_error_exitcode1;
continue; /* get next char */
-#endif /* !__U_BOOT__ */
+#else /* __U_BOOT__ */
+ /*
+* In U-Boot, '<' and '>' can be used in test command
to test if
+* a string is, alphabetically, before or after another.
+* In 2021 Busybox hush, we will keep the same behavior
and so not treat
+* them as redirection operator.
+*
+* Indeed, in U-Boot, tests are handled by the test
command and not by the
+* shell code.
+* So, better to give this character as input to test
command.
+*
+* NOTE In my opinion, when you use '<' or '>' I am
almost sure
+* you wanted to use "-gt" or "-lt" in place, so
thinking to
+* escape these will make you should check your code
(sh syntax
+* at this level is, for me, error prone).
+*/
+ case '>':
+ fallthrough;
+ case '<':
+ o_addQchr(&ctx.word, ch);
+ continue;
+#endif /* __U_BOOT__ */
case '#':
if (ctx.word.length == 0 && !ctx.word.has_quoted_part) {
/* skip "#comment" */
--
2.34.1
[PATCH v13 19/24] cli: hush_modern: Enable if keyword
Adds support for "if then else" construct both for command line interface and
through run_command().
Signed-off-by: Francis Laniel
Reviewed-by: Simon Glass
---
common/cli_hush_modern.c | 11 +++
common/cli_hush_upstream.c | 12
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/common/cli_hush_modern.c b/common/cli_hush_modern.c
index 15bb1f0d92..6360747f21 100644
--- a/common/cli_hush_modern.c
+++ b/common/cli_hush_modern.c
@@ -33,6 +33,7 @@
*/
#define ENABLE_HUSH_INTERACTIVE1
#define ENABLE_FEATURE_EDITING 1
+#define ENABLE_HUSH_IF 1
/* No MMU in U-Boot */
#define BB_MMU 0
#define USE_FOR_NOMMU(...) __VA_ARGS__
@@ -124,6 +125,11 @@ static void bb_error_msg(const char *s, ...)
va_end(p);
}
+static void bb_simple_error_msg(const char *s)
+{
+ bb_error_msg("%s", s);
+}
+
static void *xmalloc(size_t size)
{
void *p = NULL;
@@ -147,6 +153,11 @@ static void *xrealloc(void *ptr, size_t size)
return p;
}
+static void *xmemdup(const void *s, int n)
+{
+ return memcpy(xmalloc(n), s, n);
+}
+
#define xstrdupstrdup
#define xstrndup strndup
diff --git a/common/cli_hush_upstream.c b/common/cli_hush_upstream.c
index a56ea6c5c8..06af538a48 100644
--- a/common/cli_hush_upstream.c
+++ b/common/cli_hush_upstream.c
@@ -1487,7 +1487,6 @@ static void msg_and_die_if_script(unsigned lineno, const
char *fmt, ...)
die_if_script();
}
-#ifndef __U_BOOT__
static void syntax_error(unsigned lineno UNUSED_PARAM, const char *msg)
{
if (msg)
@@ -1496,7 +1495,6 @@ static void syntax_error(unsigned lineno UNUSED_PARAM,
const char *msg)
bb_simple_error_msg("syntax error");
die_if_script();
}
-#endif /* !__U_BOOT__ */
static void syntax_error_at(unsigned lineno UNUSED_PARAM, const char *msg)
{
@@ -3961,7 +3959,6 @@ static void debug_print_tree(struct pipe *pi, int lvl)
[PIPE_OR ] = "OR" ,
[PIPE_BG ] = "BG" ,
};
-#ifndef __U_BOOT__
static const char *RES[] = {
[RES_NONE ] = "NONE" ,
# if ENABLE_HUSH_IF
@@ -3991,7 +3988,6 @@ static void debug_print_tree(struct pipe *pi, int lvl)
[RES_ ] = "" ,
[RES_SNTX ] = "SNTX" ,
};
-#endif /* !__U_BOOT__ */
static const char *const CMDTYPE[] = {
"{}",
"()",
@@ -4009,10 +4005,8 @@ static void debug_print_tree(struct pipe *pi, int lvl)
lvl*2, "",
pin,
pi->num_cmds,
-#ifdef __U_BOOT__
(IF_HAS_KEYWORDS(pi->pi_inverted ? "! " :) ""),
RES[pi->res_word],
-#endif /* !__U_BOOT__ */
pi->followup, PIPE[pi->followup]
);
prn = 0;
@@ -9832,6 +9826,7 @@ static NOINLINE int run_pipe(struct pipe *pi)
rcode = 1; /* exitcode if redir failed */
#ifndef __U_BOOT__
if (setup_redirects(command, &squirrel) == 0) {
+#endif /* !__U_BOOT__ */
debug_printf_exec(": run_list\n");
//FIXME: we need to pass squirrel down into run_list()
//for SH_STANDALONE case, or else this construct:
@@ -9840,10 +9835,11 @@ static NOINLINE int run_pipe(struct pipe *pi)
//and in SH_STANDALONE mode, "find" is not execed,
//therefore CLOEXEC on saved fd does not help.
rcode = run_list(command->group) & 0xff;
+#ifndef __U_BOOT__
}
restore_redirects(squirrel);
- IF_HAS_KEYWORDS(if (pi->pi_inverted) rcode = !rcode;)
#endif /* !__U_BOOT__ */
+ IF_HAS_KEYWORDS(if (pi->pi_inverted) rcode = !rcode;)
debug_leave();
debug_printf_exec("run_pipe: return %d\n", rcode);
return rcode;
@@ -10362,12 +10358,12 @@ static int run_list(struct pipe *pi)
break;
if (G_flag_return_in_progress == 1)
break;
+#endif /* !__U_BOOT__ */
IF_HAS_KEYWORDS(rword = pi->res_word;)
debug_printf_exec(": rword=%d cond_code=%d last_rword=%d\n",
rword, cond_code, last_rword);
-#endif /* !__U_BOOT__ */
sv_errexit_depth = G.errexit_depth;
if (
#if ENABLE_HUSH_IF
--
2.34.1
[PATCH v13 20/24] cli: hush_modern: Enable loops
Enables the use of for, while and until loops for command line as
well as with run_command().
Signed-off-by: Francis Laniel
Reviewed-by: Simon Glass
---
common/cli_hush_modern.c | 1 +
common/cli_hush_upstream.c | 15 ++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/common/cli_hush_modern.c b/common/cli_hush_modern.c
index 6360747f21..fb578c3853 100644
--- a/common/cli_hush_modern.c
+++ b/common/cli_hush_modern.c
@@ -34,6 +34,7 @@
#define ENABLE_HUSH_INTERACTIVE1
#define ENABLE_FEATURE_EDITING 1
#define ENABLE_HUSH_IF 1
+#define ENABLE_HUSH_LOOPS 1
/* No MMU in U-Boot */
#define BB_MMU 0
#define USE_FOR_NOMMU(...) __VA_ARGS__
diff --git a/common/cli_hush_upstream.c b/common/cli_hush_upstream.c
index 06af538a48..748edc9d2f 100644
--- a/common/cli_hush_upstream.c
+++ b/common/cli_hush_upstream.c
@@ -10348,7 +10348,7 @@ static int run_list(struct pipe *pi)
#ifndef __U_BOOT__
for (; pi; pi = IF_HUSH_LOOPS(rword == RES_DONE ? loop_top : )
pi->next) {
#else /* __U_BOOT__ */
- for (; pi; pi = pi->next) {
+ for (; pi; pi = rword == RES_DONE ? loop_top : pi->next) {
#endif /* __U_BOOT__ */
int r;
int sv_errexit_depth;
@@ -10450,7 +10450,20 @@ static int run_list(struct pipe *pi)
}
/* Insert next value from for_lcur */
/* note: *for_lcur already has quotes removed, $var
expanded, etc */
+#ifndef __U_BOOT__
set_local_var(xasprintf("%s=%s", pi->cmds[0].argv[0],
*for_lcur++), /*flag:*/ 0);
+#else /* __U_BOOT__ */
+ /* We cannot use xasprintf, so we emulate it. */
+ char *full_var;
+ char *var = pi->cmds[0].argv[0];
+ char *val = *for_lcur++;
+
+ /* + 1 to take into account =. */
+ full_var = xmalloc(strlen(var) + strlen(val) + 1);
+ sprintf(full_var, "%s=%s", var, val);
+
+ set_local_var_modern(full_var, /*flag:*/ 0);
+#endif /* __U_BOOT__ */
continue;
}
if (rword == RES_IN) {
--
2.34.1
[PATCH v13 21/24] test: hush: Fix loop tests for modern hush
Modifies return code got from while loop as modern hush always returns 0 from
while loop.
Reviewed-by: Simon Glass
Signed-off-by: Francis Laniel
---
test/hush/loop.c | 34 ++
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/test/hush/loop.c b/test/hush/loop.c
index ca777e38fe..80a9071a80 100644
--- a/test/hush/loop.c
+++ b/test/hush/loop.c
@@ -9,6 +9,9 @@
#include
#include
#include
+#include
+
+DECLARE_GLOBAL_DATA_PTR;
static int hush_test_for(struct unit_test_state *uts)
{
@@ -21,7 +24,12 @@ static int hush_test_for(struct unit_test_state *uts)
ut_assert_nextline("quux");
ut_assert_console_end();
- puts("Beware: this test set local variable loop_i and it cannot be
unset!");
+ if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+ /* Reset local variable. */
+ ut_assertok(run_command("loop_i=", 0));
+ } else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ puts("Beware: this test set local variable loop_i and it cannot
be unset!");
+ }
return 0;
}
@@ -31,12 +39,30 @@ static int hush_test_while(struct unit_test_state *uts)
{
console_record_reset_enable();
- /* Exit status is that of test, so 1 since test is false to quit the
loop. */
- ut_asserteq(1, run_command("while test -z \"$loop_foo\"; do echo bar;
loop_foo=quux; done", 0));
+ if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+ /*
+* Hush 2021 always returns 0 from while loop...
+* You can see code snippet near this line to have a better
+* understanding:
+* debug_printf_exec(": while expr is false: breaking
(exitcode:EXIT_SUCCESS)\n");
+*/
+ ut_assertok(run_command("while test -z \"$loop_foo\"; do echo
bar; loop_foo=quux; done", 0));
+ } else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ /*
+* Exit status is that of test, so 1 since test is false to quit
+* the loop.
+*/
+ ut_asserteq(1, run_command("while test -z \"$loop_foo\"; do
echo bar; loop_foo=quux; done", 0));
+ }
ut_assert_nextline("bar");
ut_assert_console_end();
- puts("Beware: this test set local variable loop_foo and it cannot be
unset!");
+ if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
+ /* Reset local variable. */
+ ut_assertok(run_command("loop_foo=", 0));
+ } else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
+ puts("Beware: this test set local variable loop_foo and it
cannot be unset!");
+ }
return 0;
}
--
2.34.1
[PATCH v13 22/24] cli: modern_hush: Add upstream commits up to 2nd October 2023.
This commit adds the following hush busybox upstream commits:
791b222dd55d ("sleep: fix "sleep -- ARGS"")
5353df91cba7 ("Update applet size estimates")
e41e481fd571 ("hush: fix a compile failure")
07a95cfcabb0 ("ash: disable check for "good" function name, bash does not check
this")
e5692e2342c6 ("hush: quote values in "readonly" output")
96769486e20f ("shell: move varcmp() to shell_common.h and use it in hush")
bab8828b0dad ("hush: fix expansion of space in "a=${a:+$a }c" construct")
b5be8da350b5 ("hush: make "false" built-in")
6824298ab4d3 ("hush: fix ELIF cmd1;cmd2 THEN ... not executing cmd2, closes
15571")
3a7f00eadcf4 ("hush: add comment about abort on syntax error %{^}")
acae889dd972 ("ash,hush: tab completion of functions and aliases")
90b607d79a13 ("hush: quote variable values printed by "set" (match ash
behavior)")
6748e6494c22 ("hush (NOMMU): fix LINENO in execed children")
fd5fb2d2b596 ("hush: speed up "big heredoc" code")
1409432d072e ("hush: add TODO comment")
93ae7464e6e4 ("hush: restore SIGHUP handling, this time explain why we do what
we do")
1fdb33bd07e5 ("hush: restore tty pgrp on SIGHUP")
6101b6d3eaa0 ("hush: remove special handling of SIGHUP")
93e0898c663a ("shell: fix SIGWINCH and SIGCHLD (in hush) interrupting line
input, closes 15256")
969e00816835 ("hush: code shrink")
27be0e8cfeb6 ("shell: fix compile failures in some configs")
7d1c7d833785 ("ash,hush: use HOME for tab completion and prompts")
21afddefd258 ("hush: fix "error: invalid preprocessing directive ##"")
e53c7dbafc78 ("hush: fix set -n to act immediately, not just after run_list()")
574b9c446da1 ("hush: fix var_LINENO3.tests failure")
49bcf9f40cff ("hush: speed up ${x//\*/|} too")
53b2fdcdba4c ("*: add NOINLINEs where code noticeably shrinks")
7c3e96d4b3d4 ("shell: use more compact SHELL_ASH / HUSH config defines. no code
changes")
62f1eed1e191 ("hush: in a comment, document what -i might be doing")
aaf3d5ba74c5 ("shell: tweak --help")
db5546ca1018 ("libbb: code shrink: introduce and use [_]exit_SUCCESS()")
931c55f9e2b4 ("libbb: invert the meaning of SETUP_ENV_NO_CHDIR ->
SETUP_ENV_CHDIR")
12566e7f9b5e ("ash,hush: fix handling of SIGINT while waiting for interactive
input")
987be932ed3c ("*: slap on a few ALIGN_PTR where appropriate")
Signed-off-by: Francis Laniel
---
common/cli_hush_modern.c | 20 +-
common/cli_hush_upstream.c | 407 ++---
2 files changed, 304 insertions(+), 123 deletions(-)
diff --git a/common/cli_hush_modern.c b/common/cli_hush_modern.c
index fb578c3853..9a1bc977d5 100644
--- a/common/cli_hush_modern.c
+++ b/common/cli_hush_modern.c
@@ -26,7 +26,7 @@
/*
* BusyBox Version: UPDATE THIS WHEN PULLING NEW UPSTREAM REVISION!
*/
-#define BB_VER "1.34.0.git37460f5daff9"
+#define BB_VER "1.35.0.git7d1c7d833785"
/*
* Define hush features by the names used upstream.
@@ -236,6 +236,24 @@ static size_t list_size(char **list)
return size;
}
+static int varcmp(const char *p, const char *q)
+{
+ int c, d;
+
+ while ((c = *p) == (d = *q)) {
+ if (c == '\0' || c == '=')
+ goto out;
+ p++;
+ q++;
+ }
+ if (c == '=')
+ c = '\0';
+ if (d == '=')
+ d = '\0';
+out:
+ return c - d;
+}
+
struct in_str;
static int u_boot_cli_readline(struct in_str *i);
diff --git a/common/cli_hush_upstream.c b/common/cli_hush_upstream.c
index 748edc9d2f..ca40bbb2cd 100644
--- a/common/cli_hush_upstream.c
+++ b/common/cli_hush_upstream.c
@@ -91,7 +91,7 @@
* in word = GLOB, quoting should be significant on char-by-char basis: a*cd"*"
*/
//config:config HUSH
-//config: bool "hush (68 kb)"
+//config: bool "hush (70 kb)"
//config: default y
//config: select SHELL_HUSH
//config: help
@@ -339,7 +339,7 @@
* therefore we don't show them either.
*/
//usage:#define hush_trivial_usage
-//usage: "[-enxl] [-c 'SCRIPT' [ARG0 ARGS] | FILE [ARGS] | -s [ARGS]]"
+//usage: "[-enxl] [-c 'SCRIPT' [ARG0 ARGS] | FILE ARGS | -s ARGS]"
//usage:#define hush_full_usage "\n\n"
//usage: "Unix shell interpreter"
@@ -374,7 +374,7 @@
# define F_DUPFD_CLOEXEC F_DUPFD
#endif
-#if ENABLE_FEATURE_SH_EMBEDDED_SCRIPTS && !(ENABLE_ASH || ENABLE_SH_IS_ASH ||
ENABLE_BASH_IS_ASH)
+#if ENABLE_FEATURE_SH_EMBEDDED_SCRIPTS && !ENABLE_SHELL_ASH
# include "embedded_scripts.h"
#else
# define NUM_SCRIPTS 0
@@ -573,7 +573,7 @@ enum {
#define NULL_O_STRING { NULL }
#ifndef debug_printf_parse
-static const char *const assignment_flag[] = {
+static const char *const assignment_flag[] ALIGN_PTR = {
"MAYBE_ASSIGNMENT",
"DEFINITELY_ASSIGNMENT",
"NOT_ASSIGNMENT",
@@ -957,6 +957,7 @@ struct globals {
#if ENABLE_HUSH_INTERACTIVE
smallint promptmode; /* 0: PS1, 1: PS2 */
#endif
+ /* set by signal handler if SIGINT is received _and_ its trap is not
set */
smal
[PATCH v13 23/24] cmd: Set modern hush as default shell
Signed-off-by: Francis Laniel --- cmd/Kconfig | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index e25578cde3..26ad03 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -27,21 +27,17 @@ depends on HUSH_PARSER config HUSH_OLD_PARSER bool "Use hush old parser" - default y help This option enables the old flavor of hush based on hush Busybox from 2005. - It is actually the default U-Boot shell when decided to use hush as shell. - config HUSH_MODERN_PARSER bool "Use hush modern parser" + default y help This option enables the new flavor of hush based on hush upstream Busybox. - This parser is experimental and not well tested. - config HUSH_SELECTABLE bool default y if HUSH_OLD_PARSER && HUSH_MODERN_PARSER -- 2.34.1
[PATCH v13 24/24] configs: Use old hush for several boards
The keymile board family is not compatible with modern hush parser. Indeed, This boards used set_local_var() to store some variables as local shell. They then used get_local_var() to retrieve the variables values. Sadly, this two functions do not exist with CONFIG_HUSH_MODERN_PARSER. A patch was proposed to use environment variables rather than local variables but it does not tackle the problem, so complementary work is needed to make this boards use CONFIG_HUSH_MODERN_PARSER. Also, with CONFIG_HUSH_MODERN_PARSER, kirkwoord sheevaplug and phytec bk4r1 hit their board limits, so better to stick with old hush. Cc: Holger Brunck Link: https://marc.info/?l=u-boot&m=165541917618725&w=2 Signed-off-by: Francis Laniel --- configs/kmcent2_defconfig | 1 + configs/kmcoge5ne_defconfig| 1 + configs/kmeter1_defconfig | 1 + configs/kmopti2_defconfig | 1 + configs/kmsupx5_defconfig | 1 + configs/kmtepr2_defconfig | 1 + configs/pg_wcom_expu1_defconfig| 1 + configs/pg_wcom_expu1_update_defconfig | 1 + configs/pg_wcom_seli8_defconfig| 1 + configs/pg_wcom_seli8_update_defconfig | 1 + configs/socfpga_secu1_defconfig| 1 + configs/tuge1_defconfig| 1 + configs/tuxx1_defconfig| 1 + 13 files changed, 13 insertions(+) diff --git a/configs/kmcent2_defconfig b/configs/kmcent2_defconfig index 2cf9565fc9..ac272b3840 100644 --- a/configs/kmcent2_defconfig +++ b/configs/kmcent2_defconfig @@ -111,3 +111,4 @@ CONFIG_BCH=y CONFIG_PANIC_HANG=y CONFIG_LZO=y CONFIG_POST=y +CONFIG_HUSH_OLD_PARSER=y diff --git a/configs/kmcoge5ne_defconfig b/configs/kmcoge5ne_defconfig index 257ceeca90..ace5080690 100644 --- a/configs/kmcoge5ne_defconfig +++ b/configs/kmcoge5ne_defconfig @@ -202,3 +202,4 @@ CONFIG_QE=y CONFIG_SYS_NS16550=y CONFIG_BCH=y CONFIG_POST=y +CONFIG_HUSH_OLD_PARSER=y diff --git a/configs/kmeter1_defconfig b/configs/kmeter1_defconfig index 46e0370e35..56b83c085d 100644 --- a/configs/kmeter1_defconfig +++ b/configs/kmeter1_defconfig @@ -173,3 +173,4 @@ CONFIG_DM_ETH_PHY=y CONFIG_QE_UEC=y CONFIG_QE=y CONFIG_SYS_NS16550=y +CONFIG_HUSH_OLD_PARSER=y diff --git a/configs/kmopti2_defconfig b/configs/kmopti2_defconfig index c6c021adde..08c7602f5d 100644 --- a/configs/kmopti2_defconfig +++ b/configs/kmopti2_defconfig @@ -183,3 +183,4 @@ CONFIG_QE_UEC=y # CONFIG_PINCTRL_FULL is not set CONFIG_QE=y CONFIG_SYS_NS16550=y +CONFIG_HUSH_OLD_PARSER=y diff --git a/configs/kmsupx5_defconfig b/configs/kmsupx5_defconfig index 25642e7018..72db26f320 100644 --- a/configs/kmsupx5_defconfig +++ b/configs/kmsupx5_defconfig @@ -166,3 +166,4 @@ CONFIG_QE_UEC=y # CONFIG_PINCTRL_FULL is not set CONFIG_QE=y CONFIG_SYS_NS16550=y +CONFIG_HUSH_OLD_PARSER=y diff --git a/configs/kmtepr2_defconfig b/configs/kmtepr2_defconfig index ea37a29060..ed908d3c77 100644 --- a/configs/kmtepr2_defconfig +++ b/configs/kmtepr2_defconfig @@ -182,3 +182,4 @@ CONFIG_QE_UEC=y # CONFIG_PINCTRL_FULL is not set CONFIG_QE=y CONFIG_SYS_NS16550=y +CONFIG_HUSH_OLD_PARSER=y diff --git a/configs/pg_wcom_expu1_defconfig b/configs/pg_wcom_expu1_defconfig index 455b439151..0337447f79 100644 --- a/configs/pg_wcom_expu1_defconfig +++ b/configs/pg_wcom_expu1_defconfig @@ -105,3 +105,4 @@ CONFIG_DM_SERIAL=y CONFIG_SYS_NS16550=y CONFIG_LZO=y CONFIG_POST=y +CONFIG_HUSH_OLD_PARSER=y diff --git a/configs/pg_wcom_expu1_update_defconfig b/configs/pg_wcom_expu1_update_defconfig index 269116cd0d..e5daa306ab 100644 --- a/configs/pg_wcom_expu1_update_defconfig +++ b/configs/pg_wcom_expu1_update_defconfig @@ -103,3 +103,4 @@ CONFIG_DM_SERIAL=y CONFIG_SYS_NS16550=y CONFIG_LZO=y CONFIG_POST=y +CONFIG_HUSH_OLD_PARSER=y diff --git a/configs/pg_wcom_seli8_defconfig b/configs/pg_wcom_seli8_defconfig index 678bc10070..e86a17abdf 100644 --- a/configs/pg_wcom_seli8_defconfig +++ b/configs/pg_wcom_seli8_defconfig @@ -105,3 +105,4 @@ CONFIG_DM_SERIAL=y CONFIG_SYS_NS16550=y CONFIG_LZO=y CONFIG_POST=y +CONFIG_HUSH_OLD_PARSER=y diff --git a/configs/pg_wcom_seli8_update_defconfig b/configs/pg_wcom_seli8_update_defconfig index 7c7b001903..886334a043 100644 --- a/configs/pg_wcom_seli8_update_defconfig +++ b/configs/pg_wcom_seli8_update_defconfig @@ -103,3 +103,4 @@ CONFIG_DM_SERIAL=y CONFIG_SYS_NS16550=y CONFIG_LZO=y CONFIG_POST=y +CONFIG_HUSH_OLD_PARSER=y diff --git a/configs/socfpga_secu1_defconfig b/configs/socfpga_secu1_defconfig index 6a4106a559..0f8a65c42f 100644 --- a/configs/socfpga_secu1_defconfig +++ b/configs/socfpga_secu1_defconfig @@ -113,3 +113,4 @@ CONFIG_DESIGNWARE_WATCHDOG=y CONFIG_WDT=y CONFIG_SYS_TIMER_COUNTS_DOWN=y # CONFIG_GZIP is not set +CONFIG_HUSH_OLD_PARSER=y diff --git a/configs/tuge1_defconfig b/configs/tuge1_defconfig index 9ff5d1599f..5c4d33235e 100644 --- a/configs/tuge1_defconfig +++ b/configs/tuge1_defconfig @@ -166,3 +166,4 @@ CONFIG_QE_UEC=y # CONFIG_PINCTRL_FULL is not set CONFIG_QE=y CONFIG_SYS_NS16550=y +CONFIG_H
Re: [PATCH v13 15/24] cli: add modern hush as parser for run_command*()
Hi!
Le vendredi 22 décembre 2023, 22:02:35 CET Francis Laniel a écrit :
> Enables using, in code, modern hush as parser for run_command function
> family. It also enables the command run to be used by CLI user of modern
> hush.
>
> Reviewed-by: Simon Glass
> Signed-off-by: Francis Laniel
> ---
> common/cli.c | 67 --
> common/cli_hush_upstream.c | 2 +-
> test/boot/bootflow.c | 16 -
> 3 files changed, 66 insertions(+), 19 deletions(-)
>
> diff --git a/common/cli.c b/common/cli.c
> index b3eb512b62..a34938294e 100644
> --- a/common/cli.c
> +++ b/common/cli.c
> @@ -25,6 +25,14 @@
> #include
>
> #ifdef CONFIG_CMDLINE
> +
> +static inline bool use_hush_old(void)
> +{
> + return IS_ENABLED(CONFIG_HUSH_SELECTABLE) ?
> + gd->flags & GD_FLG_HUSH_OLD_PARSER :
> + IS_ENABLED(CONFIG_HUSH_OLD_PARSER);
> +}
> +
> /*
> * Run a command using the selected parser.
> *
> @@ -43,15 +51,30 @@ int run_command(const char *cmd, int flag)
> return 1;
>
> return 0;
> -#elif CONFIG_IS_ENABLED(HUSH_OLD_PARSER)
> - int hush_flags = FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP;
> -
> - if (flag & CMD_FLAG_ENV)
> - hush_flags |= FLAG_CONT_ON_NEWLINE;
> - return parse_string_outer(cmd, hush_flags);
> -#else /* HUSH_MODERN_PARSER */
> - /* Not yet implemented. */
> - return 1;
> +#else
> + if (use_hush_old()) {
> + int hush_flags = FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP;
> +
> + if (flag & CMD_FLAG_ENV)
> + hush_flags |= FLAG_CONT_ON_NEWLINE;
> + return parse_string_outer(cmd, hush_flags);
> + }
> + /*
> + * Possible values for flags are the following:
> + * FLAG_EXIT_FROM_LOOP: This flags ensures we exit from loop in
> + * parse_and_run_stream() after first iteration while normal
> + * behavior, * i.e. when called from cli_loop(), is to loop
> + * infinitely.
> + * FLAG_PARSE_SEMICOLON: modern Hush parses ';' and does not stop
> + * first time it sees one. So, I think we do not need this flag.
> + * FLAG_REPARSING: For the moment, I do not understand the goal
> + * of this flag.
> + * FLAG_CONT_ON_NEWLINE: This flag seems to be used to continue
> + * parsing even when reading '\n' when coming from
> + * run_command(). In this case, modern Hush reads until it finds
> + * '\0'. So, I think we do not need this flag.
> + */
> + return parse_string_outer_modern(cmd, FLAG_EXIT_FROM_LOOP);
> #endif
> }
>
> @@ -67,12 +90,23 @@ int run_command_repeatable(const char *cmd, int flag)
> #ifndef CONFIG_HUSH_PARSER
> return cli_simple_run_command(cmd, flag);
> #else
> + int ret;
> +
> + if (use_hush_old()) {
> + ret = parse_string_outer(cmd,
> + FLAG_PARSE_SEMICOLON
> + | FLAG_EXIT_FROM_LOOP);
> + } else {
> + ret = parse_string_outer_modern(cmd,
> + FLAG_PARSE_SEMICOLON
> + | FLAG_EXIT_FROM_LOOP);
> + }
> +
> /*
>* parse_string_outer() returns 1 for failure, so clean up
>* its result.
>*/
> - if (parse_string_outer(cmd,
> -FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP))
> + if (ret)
> return -1;
>
> return 0;
> @@ -111,12 +145,11 @@ int run_command_list(const char *cmd, int len, int
> flag) buff[len] = '\0';
> }
> #ifdef CONFIG_HUSH_PARSER
> -#if CONFIG_IS_ENABLED(HUSH_OLD_PARSER)
> - rcode = parse_string_outer(buff, FLAG_PARSE_SEMICOLON);
> -#else /* HUSH_MODERN_PARSER */
> - /* Not yet implemented. */
> - rcode = 1;
> -#endif
> + if (use_hush_old()) {
> + rcode = parse_string_outer(buff, FLAG_PARSE_SEMICOLON);
> + } else {
> + rcode = parse_string_outer_modern(buff, FLAG_PARSE_SEMICOLON);
> + }
> #else
> /*
>* This function will overwrite any \n it sees with a \0, which
> diff --git a/common/cli_hush_upstream.c b/common/cli_hush_upstream.c
> index 7f6b058e6c..11870c51e8 100644
> --- a/common/cli_hush_upstream.c
> +++ b/common/cli_hush_upstream.c
> @@ -8022,7 +8022,7 @@ static int parse_and_run_string(const char *s)
> }
>
> #ifdef __U_BOOT__
> -int parse_string_outer(const char *cmd, int flags)
> +int parse_string_outer_modern(const char *cmd, int flags)
> {
> int ret;
> int old_flags;
> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> index a9b555c779..104f49deef 100644
> --- a/test/boot/bootflow.c
> +++ b/test/boot/bootflow.c
> @@ -710,7 +710,21 @@ static int bootflow_scan_menu_boot(struct
> unit_test_state *uts) ut_assert_skip_to_line("(2 bootflows, 2 valid)");
>
> ut_assert_nextline("Selected: Armbian");
> - ut_assert_skip_to_line("Boot failed (err=-14)");
> +
> +
Re: [PATCH v13 15/24] cli: add modern hush as parser for run_command*()
On Fri, Dec 22, 2023 at 10:10:42PM +0100, Francis Laniel wrote:
> Hi!
>
>
> Le vendredi 22 décembre 2023, 22:02:35 CET Francis Laniel a écrit :
> > Enables using, in code, modern hush as parser for run_command function
> > family. It also enables the command run to be used by CLI user of modern
> > hush.
> >
> > Reviewed-by: Simon Glass
> > Signed-off-by: Francis Laniel
[snip]
> > diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> > index a9b555c779..104f49deef 100644
> > --- a/test/boot/bootflow.c
> > +++ b/test/boot/bootflow.c
> > @@ -710,7 +710,21 @@ static int bootflow_scan_menu_boot(struct
> > unit_test_state *uts) ut_assert_skip_to_line("(2 bootflows, 2 valid)");
> >
> > ut_assert_nextline("Selected: Armbian");
> > - ut_assert_skip_to_line("Boot failed (err=-14)");
> > +
> > + if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
> > + /*
> > +* With old hush, despite booti failing to boot, i.e. returning
> > +* CMD_RET_FAILURE, run_command() returns 0 which leads
> bootflow_boot(),
> > as + * we are using bootmeth_script here, to return -EFAULT.
> > +*/
> > + ut_assert_skip_to_line("Boot failed (err=-14)");
> > + } else if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
> > + /*
> > +* While with modern one, run_command() propagates
> CMD_RET_FAILURE
> > returned + * by booti, so we get 1 here.
> > +*/
> > + ut_assert_skip_to_line("Boot failed (err=1)");
> > + }
>
> I would like to give a bit of context here.
> With the following patch:
> diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
> index c169c835e3..cc5adda0a3 100644
> --- a/test/py/tests/test_ut.py
> +++ b/test/py/tests/test_ut.py
> @@ -173,7 +173,7 @@ else
> fi
> fi
> booti ${kernel_addr_r} ${ramdisk_addr_r} ${fdt_addr_r}
> -
> +echo $?
> # Recompile with:
> # mkimage -C none -A arm -T script -d /boot/boot.cmd /boot/boot.scr
> ''' % (mmc_dev)
> We can easily see that booti is failing while running the test:
> $ ./test/py/test.py -o log_cli=true -s --build -v -k
> 'test_ut[ut_bootstd_bootflow_scan_menu_boot'
> ...
> Aborting!
> Failed to load '/boot/dtb/rockchip/overlay/-fixup.scr'
> 1
>
> The problem with old hush, is that the 1 returned here, which corresponds to
> CMD_RET_FAILURE, is not propagated as the return value of run_command().
> So, this lead the -EFAULT branch here to be taken:
> int bootflow_boot(struct bootflow *bflow)
> {
> /* ... */
>
> ret = bootmeth_boot(bflow->method, bflow);
> if (ret)
> return log_msg_ret("boot", ret);
>
> /*
>* internal error, should not get here since we should have booted
>* something or returned an error
>*/
>
> return log_msg_ret("end", -EFAULT);
> }
>
> With modern hush, CMD_RET_FAILURE is propagated as the return value of
> run_command().
> As a consequence, we return with log_mst_ret("boot", 1), which leaded to this
> test to fail.
> The above modification consists in adapting the expected output to the
> current
> shell flavor.
> I think this is the good thing to do, as I find modern hush behavior better
> than the old one, i.e. it propagates CMD_RET_FAILURE as return of
> run_command().
Oh very nice, thanks for digging in to this and explaining!
--
Tom
signature.asc
Description: PGP signature
[PATCH v3 0/9] Handoff bloblist from previous boot stage
This patch set depends on another series: "[PATCH v3 00/14] Support Firmware Handoff spec via bloblist". This patch set implements Qemu-Arm platform custom functions to retrieve the bloblist (aka. Transfer List) from previous loader via boot arguments when CONFIG_OF_BOARD option is enabled and all boot arguments are compliant to the register conventions defined in the Firmware Handoff spec v0.9. Qemu-Arm platform custom function will load the FDT from the bloblist if it exists. Otherwise it fallbacks to get the FDT from the specified memory address. If a platform vendor wish to have different behaviors for loading bloblist or FDT from the previous boot stage, it is required to implement the custom functions board_bloblist_from_boot_arg() and board_fdt_blob_setup(). Raymond Mao (9): bloblist: add API to check the register conventions bloblist: check bloblist with specified buffer size bloblist: refactor of bloblist_reloc() arm: armv7: save boot arguments arm: armv8: save boot arguments qemu-arm: Get bloblist from boot arguments bloblist: Load the bloblist from the previous loader fdt: update the document and Kconfig description qemu-arm: get FDT from bloblist arch/arm/cpu/armv7/start.S | 13 ++ arch/arm/cpu/armv8/start.S | 14 ++ board/emulation/qemu-arm/qemu-arm.c | 42 - common/bloblist.c | 72 - common/board_f.c| 8 +--- configs/qemu_arm64_defconfig| 3 ++ doc/develop/devicetree/control.rst | 6 +-- dts/Kconfig | 7 ++- include/bloblist.h | 36 +-- test/bloblist.c | 8 ++-- 10 files changed, 166 insertions(+), 43 deletions(-) -- 2.25.1
[PATCH v3 1/9] bloblist: add API to check the register conventions
Add bloblist_check_reg_conv() to check whether the bloblist is compliant
to the register conventions defined in Firmware Handoff specification.
This API can be used for all Arm platforms.
Signed-off-by: Raymond Mao
---
Changes in v2
- Refactor of bloblist_check_reg_conv().
Changes in v3
- bloblist_check_reg_conv() returns -ENOENT if OF_BOARD is disabled.
common/bloblist.c | 13 +
include/bloblist.h | 12
2 files changed, 25 insertions(+)
diff --git a/common/bloblist.c b/common/bloblist.c
index 625e480f6b..ba17dd851a 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -542,3 +542,16 @@ int bloblist_maybe_init(void)
return 0;
}
+
+int bloblist_check_reg_conv(ulong rfdt, ulong rzero)
+{
+ if (!IS_ENABLED(CONFIG_OF_BOARD))
+ return -ENOENT;
+
+ if (rzero || rfdt != (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT, 0)) {
+ gd->bloblist = NULL; /* Reset the gd bloblist pointer */
+ return -EIO;
+ }
+
+ return 0;
+}
diff --git a/include/bloblist.h b/include/bloblist.h
index 84fc943819..bd32e38a06 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -461,4 +461,16 @@ static inline int bloblist_maybe_init(void)
}
#endif /* BLOBLIST */
+/**
+ * bloblist_check_reg_conv() - Check whether the bloblist is compliant to
+ *the register conventions according to the
+ *Firmware Handoff spec.
+ *
+ * @rfdt: Register that holds the FDT base address.
+ * @rzero: Register that must be zero.
+ * Return: 0 if OK, -EIO if the bloblist is not compliant to the register
+ *conventions, -ENOENT if OF_BOARD is disabled.
+ */
+int bloblist_check_reg_conv(ulong rfdt, ulong rzero);
+
#endif /* __BLOBLIST_H */
--
2.25.1
[PATCH v3 2/9] bloblist: check bloblist with specified buffer size
Instead of expecting the bloblist total size to be the same as the
pre-allocated buffer size, practically we are more interested in
whether the pre-allocated buffer size is bigger than the bloblist
total size.
Signed-off-by: Raymond Mao
Reviewed-by: Ilias Apalodimas
---
Changes in v2
- New patch file created for v2.
common/bloblist.c | 2 +-
test/bloblist.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/bloblist.c b/common/bloblist.c
index ba17dd851a..db3fbb20cf 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -384,7 +384,7 @@ int bloblist_check(ulong addr, uint size)
return log_msg_ret("Bad magic", -ENOENT);
if (hdr->version != BLOBLIST_VERSION)
return log_msg_ret("Bad version", -EPROTONOSUPPORT);
- if (!hdr->total_size || (size && hdr->total_size != size))
+ if (!hdr->total_size || (size && hdr->total_size > size))
return log_msg_ret("Bad total size", -EFBIG);
if (hdr->used_size > hdr->total_size)
return log_msg_ret("Bad used size", -ENOENT);
diff --git a/test/bloblist.c b/test/bloblist.c
index 17d9dd03d0..7dab9addf8 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -207,7 +207,7 @@ static int bloblist_test_checksum(struct unit_test_state
*uts)
hdr->flags++;
hdr->total_size--;
- ut_asserteq(-EFBIG, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+ ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
hdr->total_size++;
hdr->spare++;
--
2.25.1
[PATCH v3 3/9] bloblist: refactor of bloblist_reloc()
The current bloblist pointer and size can be retrieved from global
data, so we don't need to pass them from the function arguments.
This change also help to remove all external access of gd->bloblist
outside of bloblist module.
Signed-off-by: Raymond Mao
---
Changes in v2
- New patch file created for v2.
Changes in v3
- Check the space size before copying the bloblist.
- Add return code of bloblist_reloc().
common/bloblist.c | 10 --
common/board_f.c | 8 ++--
include/bloblist.h | 8
test/bloblist.c| 6 ++
4 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/common/bloblist.c b/common/bloblist.c
index db3fbb20cf..5ad1db137a 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -472,13 +472,19 @@ void bloblist_show_list(void)
}
}
-void bloblist_reloc(void *to, uint to_size, void *from, uint from_size)
+int bloblist_reloc(void *to, uint to_size)
{
struct bloblist_hdr *hdr;
- memcpy(to, from, from_size);
+ if (to_size < gd->bloblist->total_size)
+ return -ENOSPC;
+
+ memcpy(to, gd->bloblist, gd->bloblist->total_size);
hdr = to;
hdr->total_size = to_size;
+ gd->bloblist = to;
+
+ return 0;
}
int bloblist_init(void)
diff --git a/common/board_f.c b/common/board_f.c
index d4d7d01f8f..00b0430889 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -676,13 +676,9 @@ static int reloc_bloblist(void)
return 0;
}
if (gd->new_bloblist) {
- int size = CONFIG_BLOBLIST_SIZE;
-
debug("Copying bloblist from %p to %p, size %x\n",
- gd->bloblist, gd->new_bloblist, size);
- bloblist_reloc(gd->new_bloblist, CONFIG_BLOBLIST_SIZE_RELOC,
- gd->bloblist, size);
- gd->bloblist = gd->new_bloblist;
+ gd->bloblist, gd->new_bloblist, gd->bloblist->total_size);
+ bloblist_reloc(gd->new_bloblist, CONFIG_BLOBLIST_SIZE_RELOC);
}
#endif
diff --git a/include/bloblist.h b/include/bloblist.h
index bd32e38a06..c1dab11f78 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -418,11 +418,11 @@ const char *bloblist_tag_name(enum bloblist_tag_t tag);
* bloblist_reloc() - Relocate the bloblist and optionally resize it
*
* @to: Pointer to new bloblist location (must not overlap old location)
- * @to_size: New size for bloblist (must be larger than from_size)
- * @from: Pointer to bloblist to relocate
- * @from_size: Size of bloblist to relocate
+ * @to_size: New size for bloblist
+ * Return: 0 if OK, -ENOSPC if the new size is small than the bloblist total
+ *size.
*/
-void bloblist_reloc(void *to, uint to_size, void *from, uint from_size);
+int bloblist_reloc(void *to, uint to_size);
/**
* bloblist_init() - Init the bloblist system with a single bloblist
diff --git a/test/bloblist.c b/test/bloblist.c
index 7dab9addf8..1c60bbac36 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -376,13 +376,12 @@ static int bloblist_test_reloc(struct unit_test_state
*uts)
{
const uint large_size = TEST_BLOBLIST_SIZE;
const uint small_size = 0x20;
- void *old_ptr, *new_ptr;
+ void *new_ptr;
void *blob1, *blob2;
ulong new_addr;
ulong new_size;
ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0));
- old_ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE);
/* Add one blob and then one that won't fit */
blob1 = bloblist_add(TEST_TAG, small_size, 0);
@@ -394,8 +393,7 @@ static int bloblist_test_reloc(struct unit_test_state *uts)
new_addr = TEST_ADDR + TEST_BLOBLIST_SIZE;
new_size = TEST_BLOBLIST_SIZE + 0x100;
new_ptr = map_sysmem(new_addr, TEST_BLOBLIST_SIZE);
- bloblist_reloc(new_ptr, new_size, old_ptr, TEST_BLOBLIST_SIZE);
- gd->bloblist = new_ptr;
+ ut_assertok(bloblist_reloc(new_ptr, new_size));
/* Check the old blob is there and that we can now add the bigger one */
ut_assertnonnull(bloblist_find(TEST_TAG, small_size));
--
2.25.1
[PATCH v3 4/9] arm: armv7: save boot arguments
Save boot arguments r[0-3] into an array for handover of bloblist from
previous boot stage.
Signed-off-by: Raymond Mao
---
Changes in v2
- New patch file created for v2.
Changes in v3
- Swap value of r0 with r2.
arch/arm/cpu/armv7/start.S | 13 +
1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index 69e281b086..2ca63ca32c 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -152,9 +152,22 @@ ENDPROC(c_runtime_cpu_setup)
*
*/
WEAK(save_boot_params)
+#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST))
+ ldr r12, =saved_args
+ /* Intentionally swapping r0 with r2 */
+ stm r12, {r2, r1, r0, r3}
+#endif
b save_boot_params_ret@ back to my caller
ENDPROC(save_boot_params)
+.section .data
+.global saved_args
+saved_args:
+ .rept 4
+ .word 0
+ .endr
+END(saved_args)
+
#ifdef CONFIG_ARMV7_LPAE
WEAK(switch_to_hypervisor)
b switch_to_hypervisor_ret
--
2.25.1
[PATCH v3 5/9] arm: armv8: save boot arguments
Save boot arguments x[0-3] into an array for handover of bloblist from previous boot stage. Signed-off-by: Raymond Mao --- Changes in v2 - New patch file created for v2. arch/arm/cpu/armv8/start.S | 14 ++ 1 file changed, 14 insertions(+) diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index 6cc1d26e5e..8e704f590e 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -370,5 +370,19 @@ ENTRY(c_runtime_cpu_setup) ENDPROC(c_runtime_cpu_setup) WEAK(save_boot_params) +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST)) + adr x9, saved_args + stp x0, x1, [x9] + /* Increment the address by 16 bytes for the next pair of values */ + stp x2, x3, [x9, #16] +#endif b save_boot_params_ret/* back to my caller */ ENDPROC(save_boot_params) + +.section .data +.global saved_args +saved_args: + .rept 4 + .xword 0 + .endr +END(saved_args) -- 2.25.1
[PATCH v3 6/9] qemu-arm: Get bloblist from boot arguments
Add platform custom function to get bloblist from boot arguments.
Check whether boot arguments aligns with the register conventions
defined in FW Handoff spec v0.9.
Add bloblist related options into qemu default config.
Signed-off-by: Raymond Mao
---
Changes in v2
- Remove low level code for copying boot arguments.
- Refactor board_fdt_blob_setup() and remove direct access of gd->bloblist.
Changes in v3
- Optimize board_bloblist_from_boot_arg().
board/emulation/qemu-arm/qemu-arm.c | 30 +
configs/qemu_arm64_defconfig| 3 +++
2 files changed, 33 insertions(+)
diff --git a/board/emulation/qemu-arm/qemu-arm.c
b/board/emulation/qemu-arm/qemu-arm.c
index 942f1fff57..e225011bf0 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -4,6 +4,7 @@
*/
#include
+#include
#include
#include
#include
@@ -102,6 +103,15 @@ static struct mm_region qemu_arm64_mem_map[] = {
struct mm_region *mem_map = qemu_arm64_mem_map;
#endif
+/*
+ * Boot parameters saved from start.S
+ * saved_args[0]: FDT base address
+ * saved_args[1]: Bloblist signature
+ * saved_args[2]: must be 0
+ * saved_args[3]: Bloblist base address
+ */
+extern unsigned long saved_args[];
+
int board_init(void)
{
return 0;
@@ -144,6 +154,26 @@ void *board_fdt_blob_setup(int *err)
return (void *)CFG_SYS_SDRAM_BASE;
}
+int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
+{
+ int ret = -ENOENT;
+
+ if (!IS_ENABLED(CONFIG_OF_BOARD) || !IS_ENABLED(CONFIG_BLOBLIST))
+ return -ENOENT;
+
+ ret = bloblist_check(saved_args[3], size);
+ if (ret)
+ return ret;
+
+ /* Check the register conventions */
+ ret = bloblist_check_reg_conv(saved_args[0], saved_args[2]);
+ if (!ret)
+ /* Relocate the bloblist to the fixed address */
+ ret = bloblist_reloc((void *)addr, CONFIG_BLOBLIST_SIZE);
+
+ return ret;
+}
+
void enable_caches(void)
{
icache_enable();
diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
index c010c25a92..418f48001c 100644
--- a/configs/qemu_arm64_defconfig
+++ b/configs/qemu_arm64_defconfig
@@ -69,3 +69,6 @@ CONFIG_USB_EHCI_HCD=y
CONFIG_USB_EHCI_PCI=y
CONFIG_SEMIHOSTING=y
CONFIG_TPM=y
+CONFIG_BLOBLIST=y
+CONFIG_BLOBLIST_ADDR=0x40004000
+CONFIG_BLOBLIST_SIZE=0x4000
--
2.25.1
[PATCH v3 7/9] bloblist: Load the bloblist from the previous loader
During bloblist initialization, when CONFIG_OF_BOARD is defined,
invoke the platform custom function to load the bloblist via boot
arguments from the previous loader.
If the bloblist exists, copy it into the fixed bloblist memory region.
Signed-off-by: Raymond Mao
---
common/bloblist.c | 47 --
include/bloblist.h | 16
2 files changed, 45 insertions(+), 18 deletions(-)
diff --git a/common/bloblist.c b/common/bloblist.c
index 5ad1db137a..e66afcde64 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -492,31 +492,38 @@ int bloblist_init(void)
bool fixed = IS_ENABLED(CONFIG_BLOBLIST_FIXED);
int ret = -ENOENT;
ulong addr, size;
- bool expected;
-
- /**
-* We don't expect to find an existing bloblist in the first phase of
-* U-Boot that runs. Also we have no way to receive the address of an
-* allocated bloblist from a previous stage, so it must be at a fixed
+ /*
+* If U-Boot is not in the first phase, an existing bloblist must be
+* at a fixed address.
+*/
+ bool from_addr = fixed && !u_boot_first_phase();
+ /*
+* If U-Boot is in the first phase that a board specific routine should
+* install the bloblist passed from previous loader to this fixed
* address.
*/
- expected = fixed && !u_boot_first_phase();
+ bool from_board = fixed && IS_ENABLED(CONFIG_OF_BOARD) &&
+ u_boot_first_phase();
+
if (spl_prev_phase() == PHASE_TPL && !IS_ENABLED(CONFIG_TPL_BLOBLIST))
- expected = false;
+ from_addr = false;
if (fixed)
addr = IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED,
CONFIG_BLOBLIST_ADDR);
size = CONFIG_BLOBLIST_SIZE;
- if (expected) {
+
+ if (from_board)
+ ret = board_bloblist_from_boot_arg(addr, size);
+ else if (from_addr)
ret = bloblist_check(addr, size);
- if (ret) {
- log_warning("Expected bloblist at %lx not found
(err=%d)\n",
- addr, ret);
- } else {
- /* Get the real size, if it is not what we expected */
- size = gd->bloblist->total_size;
- }
- }
+
+ if (ret)
+ log_warning("Bloblist at %lx not found (err=%d)\n",
+ addr, ret);
+ else
+ /* Get the real size */
+ size = gd->bloblist->total_size;
+
if (ret) {
if (CONFIG_IS_ENABLED(BLOBLIST_ALLOC)) {
void *ptr = memalign(BLOBLIST_ALIGN, size);
@@ -525,7 +532,8 @@ int bloblist_init(void)
return log_msg_ret("alloc", -ENOMEM);
addr = map_to_sysmem(ptr);
} else if (!fixed) {
- return log_msg_ret("!fixed", ret);
+ return log_msg_ret("BLOBLIST_FIXED is not enabled",
+ ret);
}
log_debug("Creating new bloblist size %lx at %lx\n", size,
addr);
@@ -538,6 +546,9 @@ int bloblist_init(void)
return log_msg_ret("ini", ret);
gd->flags |= GD_FLG_BLOBLIST_READY;
+ bloblist_show_stats();
+ bloblist_show_list();
+
return 0;
}
diff --git a/include/bloblist.h b/include/bloblist.h
index c1dab11f78..2f4246576e 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -445,6 +445,22 @@ int bloblist_reloc(void *to, uint to_size);
*/
int bloblist_init(void);
+#if (IS_ENABLED(CONFIG_ARCH_QEMU) && IS_ENABLED(CONFIG_ARM))
+/* Board custom function for qemu-arm */
+int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size);
+#else
+/*
+ * A board need to implement this custom function if it needs to retrieve
+ * bloblist from a previous loader
+ */
+static inline
+int board_bloblist_from_boot_arg(unsigned long __always_unused addr,
+unsigned long __always_unused size)
+{
+ return -1;
+}
+#endif
+
#if CONFIG_IS_ENABLED(BLOBLIST)
/**
* bloblist_maybe_init() - Init the bloblist system if not already done
--
2.25.1
[PATCH v3 8/9] fdt: update the document and Kconfig description
Update the document and Kconfig to describe the behavior of board specific custom functions when CONFIG_OF_BOARD is defined. Signed-off-by: Raymond Mao --- doc/develop/devicetree/control.rst | 6 +++--- dts/Kconfig| 7 +-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/doc/develop/devicetree/control.rst b/doc/develop/devicetree/control.rst index cbb65c9b17..e061f4e812 100644 --- a/doc/develop/devicetree/control.rst +++ b/doc/develop/devicetree/control.rst @@ -104,9 +104,9 @@ in u-boot.bin so you can still just flash u-boot.bin onto your board. If you are using CONFIG_SPL_FRAMEWORK, then u-boot.img will be built to include the device tree binary. -If CONFIG_OF_BOARD is defined, a board-specific routine will provide the -devicetree at runtime, for example if an earlier bootloader stage creates -it and passes it to U-Boot. +If CONFIG_OF_BOARD is defined, board-specific routines will provide the +bloblist and devicetree at runtime, for example if an earlier bootloader stage +creates it and passes it to U-Boot. If CONFIG_SANDBOX is defined, then it will be read from a file on startup. Use the -d flag to U-Boot to specify the file to read, -D for the diff --git a/dts/Kconfig b/dts/Kconfig index 00c0aeff89..12d61dc748 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -110,8 +110,11 @@ config OF_BOARD default y if SANDBOX || OF_HAS_PRIOR_STAGE help If this option is enabled, the device tree is provided at runtime by - a custom function called board_fdt_blob_setup(). The board must - implement this function if it wishes to provide special behaviour. + a custom function called board_fdt_blob_setup(). + If this option is enabled, bloblist is provided at runtime by a + custom function called board_bloblist_from_boot_arg(). + The board must implement these functions if it wishes to provide + special behaviour. With this option, the device tree build by U-Boot may be overridden or ignored. See OF_HAS_PRIOR_STAGE. -- 2.25.1
[PATCH v3 9/9] qemu-arm: get FDT from bloblist
Get devicetree from a bloblist if it exists.
If not, fallback to get FDT from the specified memory address.
Signed-off-by: Raymond Mao
---
Changes in v2
- Refactor of board_fdt_blob_setup().
board/emulation/qemu-arm/qemu-arm.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/board/emulation/qemu-arm/qemu-arm.c
b/board/emulation/qemu-arm/qemu-arm.c
index e225011bf0..d326668caf 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -149,9 +149,17 @@ int dram_init_banksize(void)
void *board_fdt_blob_setup(int *err)
{
+ void *fdt = NULL;
*err = 0;
- /* QEMU loads a generated DTB for us at the start of RAM. */
- return (void *)CFG_SYS_SDRAM_BASE;
+
+ /* Check if a DTB exists in bloblist */
+ if (IS_ENABLED(CONFIG_BLOBLIST) && !bloblist_maybe_init())
+ fdt = bloblist_find(BLOBLISTT_CONTROL_FDT, 0);
+ if (!fdt)
+ /* QEMU loads a generated DTB for us at the start of RAM. */
+ return (void *)CFG_SYS_SDRAM_BASE;
+
+ return fdt;
}
int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
--
2.25.1
[PATCH v4] arch: arm: Kconfig: Enable BOOTSTD_FULL for Rockchip SoCs
Rockchip SoCs can support wide range of bootflows. Without full bootflow commands, it can be difficult to figure out issues if any, hence enable by default. Reviewed-by: Simon Glass Signed-off-by: Shantur Rathore --- Changes in v4: - Replace BOOTSTD_DEFAULT with BOOTSTD_FULL as previous version didn't work on next arch/arm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index d812685c98..a0265b36ad 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1985,7 +1985,7 @@ config ARCH_ROCKCHIP imply ADC imply CMD_DM imply DEBUG_UART_BOARD_INIT - imply BOOTSTD_DEFAULTS + imply BOOTSTD_FULL imply FAT_WRITE imply SARADC_ROCKCHIP imply SPL_SYSRESET -- 2.40.1
Re: [PATCH v3] arch: arm: Kconfig: Enable BOOTSTD_FULL for Rockchip SoCs
Hi, On Fri, Dec 22, 2023 at 12:09 PM Shantur Rathore wrote: > > On Fri, Dec 22, 2023 at 12:07 PM Tom Rini wrote: > > > > On Fri, Dec 22, 2023 at 12:05:39PM +, Shantur Rathore wrote: > > > Hi all, > > > > > > On Mon, Dec 11, 2023 at 6:27 PM Tom Rini wrote: > > > > > > > > On Mon, Dec 11, 2023 at 11:22:45AM -0700, Simon Glass wrote: > > > > > Hi Tom, > > > > [snip] > > > > > > I think in hind-sight too much stuff is omitted without > > > > > > BOOTSTD_FULL. > > > > > > The option itself then enables other stuff too by default, but some > > > > > > parts of the bootflow command itself should be visible even without > > > > > > FULL > > > > > > to make things easier on the user. > > > > > > > > > > At the time the goal was to avoid growth compared to the distro > > > > > scripts. We could perhaps add some more things in with BOOTSTD_FULL > > > > > but still have it as an option? > > > > > > > > Right. But now that we've tried this, some of the feedback has been that > > > > it's just too minimal right now. Like looking at the help message for > > > > bootflow, list and info should probably always be available. And maybe > > > > the flags for "scan" should be re-thought? Too late to change things now > > > > but "bootflow scan -b" should maybe how it's always been for "scan and > > > > boot". > > > > > > What would be the preferred approach for this patch? > > > Is it to update the default capabilities of bootflow or we can have this > > > patch? > > > > Well, I don't see a problem with just adding this to the platforms which > > enable BOOTSTD_FULL until we can rework what's included/excluded by that > > flag, and there's an issue filed for it now. > > > > -- > > Tom > > Great, can we have this merged in please then [0] > > [0] - > https://patchwork.ozlabs.org/project/uboot/patch/20231119172310.1307942-...@shantur.com/ > I tried this patch over the next branch and it failed to build. Sent v4 here : https://patchwork.ozlabs.org/project/uboot/patch/2023134358.563121-...@shantur.com/ Regards, Shantur
