Re: [PATCH v2 4/6] usb: Add environment based device blocklist
On 3/17/24 7:15 PM, Janne Grunau wrote:
Hej,
Hi,
On Sun, Mar 17, 2024, at 17:18, Marek Vasut wrote:
On 3/17/24 12:07 PM, Janne Grunau via B4 Relay wrote:
From: Janne Grunau
Add the environment variable "usb_blocklist" to prevent USB devices
listed in it from being used. This allows to ignore devices which
trigger bugs in u-boot's USB stack or are undesirable for other reasons.
Devices emulating keyboards are one example. U-boot currently supports
only one USB keyboard device. Most commonly, people run into this with
Yubikeys, so let's ignore those in the default environment.
Based on previous USB keyboard specific patches for the same purpose.
Link:
https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
Signed-off-by: Janne Grunau
---
common/usb.c | 56
+++
doc/usage/environment.rst | 12 ++
include/env_default.h | 11 ++
3 files changed, 79 insertions(+)
diff --git a/common/usb.c b/common/usb.c
index 836506dcd9..73af5be066 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device *dev,
int addr, bool do_read,
return 0;
}
+static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
+{
+ printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
+ blocklist);
+ return 0;
This could be static void without return 0 at the end.
the return is there to break out of the while loop on parsing errors in a
single statement. This probably won't be necessary after using strsep and
sscanf in the parsing function but see below.
Ahh, now I see it. But then, shouldn't this return -ENODEV here already ?
+}
+
+static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
+{
+ ulong vid, pid;
+ char *end;
+ const char *block_str = env_get("usb_blocklist");
+ const char *cur = block_str;
+
+ /* parse "usb_blocklist" strictly */
+ while (cur && cur[0] != '\0') {
Have a look at strsep() , namely strsep(block_str, ","); This will split
the string up for you at "," delimiters.
Example is in drivers/dfu/dfu.c dfu_config_interfaces() .
strsep() is probably a good idea even if it alone won't make the code that much
simpler for strict parsing.
And then, on each token, you can try and run sscanf(token, "%04x:%04x",
vid, pid);, that will parse the token format for you. See e.g.
test/lib/sscanf.c for further examples.
That should simplify the parsing a lot.
It would but sscanf() is optional and is only selected by CONFIG_XEN so I
assumed there would be concerns over binary size increase if USB_HOST would
require sscanf.
Good point, lets postpone sscanf() . Also, looking at it, sscanf would
work for numbers, but not for the special characters. So ... do you want
to try tweaking this further with strsep() or shall we go with this
implementation ?
RE: [PATCH] efi_loader: accept append write with valid size and data
Hi Heinrich,
> -Original Message-
> From: Heinrich Schuchardt
> Sent: Friday, March 15, 2024 4:58 PM
> To: Kojima, Masahisa/小島 雅久
> Cc: u-boot@lists.denx.de; Ilias Apalodimas
> Subject: Re: [PATCH] efi_loader: accept append write with valid size and data
>
> On 3/15/24 08:03, Ilias Apalodimas wrote:
> > Hi Kojima-san
> >
> > On Fri, 15 Mar 2024 at 02:10, wrote:
> >>
> >> Hi Ilias,
> >>
> >>> -Original Message-
> >>> From: Ilias Apalodimas
> >>> Sent: Thursday, March 14, 2024 10:54 PM
> >>> To: Kojima, Masahisa/小島 雅久
> >>> Cc: u-boot@lists.denx.de; Heinrich Schuchardt
> >>> Subject: Re: [PATCH] efi_loader: accept append write with valid size and
> data
> >>>
> >>> Hi Kojima-san
> >>>
> >>> Apologies for the late reply
> >>>
> >>> On Mon, 4 Mar 2024 at 08:10, Masahisa Kojima
> >>> wrote:
>
> Current "variables" efi_selftest result is inconsistent
> between the U-Boot file storage and the tee-based StandaloneMM
> RPMB secure storage.
> U-Boot file storage implementation does not accept SetVariale
> call to non-existent variable with EFI_VARIABLE_APPEND_WRITE
> attribute and valid size and data, however it is accepted
> in EDK II StandaloneMM implementation.
> >>>
> >>> Yes,
> >>>
>
> Since UEFI specification does not clearly describe the behavior
> of the append write to non-existent variable, let's update
> the U-Boot file storage implementation to get aligned with
> the EDK II reference implementation.
>
> Signed-off-by: Masahisa Kojima
> ---
> lib/efi_loader/efi_variable.c | 13 +-
> lib/efi_selftest/efi_selftest_variables.c | 30
> >>> +--
> 2 files changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/lib/efi_loader/efi_variable.c
> b/lib/efi_loader/efi_variable.c
> index 40f7a0fb10..1693c3387b 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -282,11 +282,8 @@ efi_status_t efi_set_variable_int(const u16
> >>> *variable_name,
> }
> time = var->time;
> } else {
> - if (delete || append)
> - /*
> -* Trying to delete or to update a non-existent
> -* variable.
> -*/
> + if (delete)
> + /* Trying to delete a non-existent variable. */
> return EFI_NOT_FOUND;
> >>>
> >>> I am not sure about this tbh. The UEFI spec says
> >>> "When the EFI_VARIABLE_APPEND_WRITE attribute is set, then a
> >>> SetVariable() call with a DataSize of zero will not cause any change
> >>> to the variable value (the timestamp
> >>> associated with the variable may be updated however, even if no new
> >>> data value is provided;see the description
> >>> of the EFI_VARIABLE_AUTHENTICATION_2 descriptor below). In this
> case
> >>> the DataSize will not be zero
> >>> since the EFI_VARIABLE_AUTHENTICATION_2 descriptor will be
> populated)"
> >>>
> >>> I think this assumes the variable exists. Is there a different chapter
> >>> in the spec that describes the behavior?
> >>
> >> No, I could not find it.
> >> I'm also not sure what the UEFI spec expects.
> >
> > I don't have any objections to aligning this with EDK2. But can we add
> > a comment about it as well?
> > Heinrich any preference here?
> >
> > [...]
> >
> >>> non-existent variable returns wrong code\n");
> + efi_st_error("Variable was not deleted\n");
> return EFI_ST_FAILURE;
> >>>
> >>> This used to fail as well [0]. Does it work now?
> >>
> >> Yes. With this patch applied, both U-Boot file storage and StMM
> >> pass the efi selftest.
> >
> > Yes, that's obviously a hack. The reason we didn't bother fixing it
> > back then is because stmm is not running in our CI, but I should have
> > added a comment on that instead of having it on email history...
>
> Hello Kojima,
>
> Thank you for pointing out the inconsistencies.
>
> I have created https://mantis.uefi.org/mantis/view.php?id=2447 to
> discuss the expected behavior.
Thank you.
My company is not the member of uefi and probably I could not create the
account.
>
> The logic in MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c is:
>
> If the variable does not exist and EFI_VARIABLE_APPEND_WRITE is set and
> the size is non-zero, the variable is created.
> If the variable does not exist and EFI_VARIABLE_APPEND_WRITE is set and
> the size is zero, the variable is not created and EFI_SUCCESS is returned.
>
> I am not able to derive this from the specification.
>
> In our selftest we should test the following four cases of
> EFI_VARIABLE_APPEND_WRITE:
>
> variable existing, size non-zero
> variable existing, size zero
> variable non-existent, size non-zero
> variable
[PATCH] rpi: Copy eth MAC address from fw DT to loaded DT
Raspberry Pi B models before model 4 don't have an EEPROM nor an OTP to
store the permanent factory MAC address of the NIC.
So the firmware that runs initially computes the factory MAC address of
the board and patches the DTB to give that information to the next
stage.
The MAC is put in the standard property `local-mac-address` which is
inserted in the `ethernet0` node of the firmware provided FDT.
If the next stage is Linux, Linux uses this MAC if no other MAC was
provided by another mechanism.
There is also another way to give the MAC to the Linux kernel: using the
boot param `smsc95xx.macaddr`.
When CONFIG_MISC_INIT_R=y, U-Boot requests directly the MAC from the
running firmware in the GPU through the Raspberry Pi Mailbox. It then
stores it in ${usbethaddr} environment variable.
In U-Boot, the MAC is then often given to Linux like this:
> setenv bootargs [...] smsc95xx.macaddr="${usbethaddr}" [...]
It works because the smsc95xx driver in Linux will take this MAC if the
parameter was specified. If there is no MAC information provided, Linux
will generate a random MAC address at each boot.
This patch extends commit 6d0642494993 ("rpi: Copy properties from
firmware dtb to the loaded dtb") by making U-Boot copy the
`local-mac-address` property from the firmware FDT to the loaded FDT.
It makes it then possible not to specify the kernel boot param
`smsc95xx.macaddr` at all anymore, and still have Linux pick up the
correct factory MAC address, without generating a random one at each
boot.
This is meaningful in a setup where the user configures U-Boot to give a
fresh FDT (not the firmware provided one) to the Linux kernel, but still
wants the most important firmware provided modifications to be copied
(CONFIG_OF_BOARD_SETUP=y) through ft_board_setup() call.
Cc: Matthias Brugger
Cc: Peter Robinson
Cc: Antoine Mazeas
Signed-off-by: Martin Wetterwald
---
board/raspberrypi/rpi/rpi.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index 2851ebc985..b36a893047 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -566,6 +566,9 @@ void update_fdt_from_fw(void *fdt, void *fw_fdt)
/* address of the PHY device as provided by the firmware */
copy_property(fdt, fw_fdt, "ethernet0/mdio@e14/ethernet-phy@1", "reg");
+
+ /* MAC address of the NIC as provided by the firmware */
+ copy_property(fdt, fw_fdt, "ethernet0", "local-mac-address");
}
int ft_board_setup(void *blob, struct bd_info *bd)
--
2.44.0
Re: [PATCH] ARM: dts: renesas: Stop using the -u-boot DTs for build
On Sun, Mar 17, 2024 at 1:24 AM Marek Vasut
wrote:
>
> The U-Boot build system can automatically paste -u-boot.dtsi at the
> end of matching .dts during build. Stop emulating this behavior and
> rename the -u-boot.dts files to -u-boot.dtsi, drop "#include...dts"
> from those new u-boot.dtsi files, and update board configuration
> accordingly.
>
> The rename, '#include...dts` scrubbing and configuration update has
> been done using the following script:
> ```
> $ find . -name r[78]\*-u-boot.dts | sort -u | while read line ; do \
> git mv ${line%-u-boot.dts}-u-boot.dts ${line%-u-boot.dts}-u-boot.dtsi ; \
> done
> $ sed -i '/^#include.*dts"/ d' `find . -name r[78]\*-u-boot.dtsi`
> $ sed -i 's@-u-boot@@g' `git grep -li renesas configs`
> ```
> The Salvator-X and ULCB board files have been updated manually.
>
> Signed-off-by: Marek Vasut
Seems like the right thing to do. Beacon should boards do this already.
Acked-by: Adam Ford
> ---
> Cc: Adam Ford
> Cc: Biju Das
> Cc: Lad Prabhakar
> Cc: Paul Barker
> Cc: Ralph Siemsen
> ---
> arch/arm/dts/Makefile | 58 +--
> ...boot.dts => r7s72100-gr-peach-u-boot.dtsi} | 1 -
> dts => r8a774a1-hihope-rzg2m-u-boot.dtsi} | 1 -
> dts => r8a774b1-hihope-rzg2n-u-boot.dtsi} | 1 -
> ...-u-boot.dts => r8a774c0-ek874-u-boot.dtsi} | 1 -
> dts => r8a774e1-hihope-rzg2h-u-boot.dtsi} | 1 -
> ...r-u-boot.dts => r8a7790-lager-u-boot.dtsi} | 1 -
> ...t-u-boot.dts => r8a7790-stout-u-boot.dtsi} | 1 -
> ...u-boot.dts => r8a7791-koelsch-u-boot.dtsi} | 1 -
> ...-u-boot.dts => r8a7791-porter-u-boot.dtsi} | 1 -
> ...u-boot.dts => r8a7792-blanche-u-boot.dtsi} | 1 -
> ...se-u-boot.dts => r8a7793-gose-u-boot.dtsi} | 1 -
> ...alt-u-boot.dts => r8a7794-alt-u-boot.dtsi} | 1 -
> ...lk-u-boot.dts => r8a7794-silk-u-boot.dtsi} | 1 -
> ...ot.dts => r8a77950-salvator-x-u-boot.dtsi} | 1 -
> ...b-u-boot.dts => r8a77950-ulcb-u-boot.dtsi} | 1 -
> ...ot.dts => r8a77960-salvator-x-u-boot.dtsi} | 1 -
> ...b-u-boot.dts => r8a77960-ulcb-u-boot.dtsi} | 1 -
> ...ot.dts => r8a77965-salvator-x-u-boot.dtsi} | 1 -
> ...b-u-boot.dts => r8a77965-ulcb-u-boot.dtsi} | 1 -
> ...-u-boot.dts => r8a77970-eagle-u-boot.dtsi} | 1 -
> ...-u-boot.dts => r8a77970-v3msk-u-boot.dtsi} | 1 -
> ...u-boot.dts => r8a77980-condor-u-boot.dtsi} | 1 -
> ...-u-boot.dts => r8a77980-v3hsk-u-boot.dtsi} | 1 -
> ...-u-boot.dts => r8a77990-ebisu-u-boot.dtsi} | 1 -
> ...-u-boot.dts => r8a77995-draak-u-boot.dtsi} | 1 -
> ...u-boot.dts => r8a779a0-falcon-u-boot.dtsi} | 1 -
> ...u-boot.dts => r8a779f0-spider-u-boot.dtsi} | 1 -
> ...ot.dts => r8a779g0-white-hawk-u-boot.dtsi} | 1 -
> ...oot.dts => r8a779h0-gray-hawk-u-boot.dtsi} | 1 -
> board/renesas/salvator-x/salvator-x.c | 6 +-
> board/renesas/ulcb/ulcb.c | 6 +-
> configs/alt_defconfig | 2 +-
> configs/blanche_defconfig | 2 +-
> configs/gose_defconfig| 2 +-
> configs/grpeach_defconfig | 2 +-
> configs/hihope_rzg2_defconfig | 4 +-
> configs/koelsch_defconfig | 2 +-
> configs/lager_defconfig | 2 +-
> configs/porter_defconfig | 2 +-
> configs/r8a77970_eagle_defconfig | 2 +-
> configs/r8a77970_v3msk_defconfig | 2 +-
> configs/r8a77980_condor_defconfig | 2 +-
> configs/r8a77980_v3hsk_defconfig | 2 +-
> configs/r8a77990_ebisu_defconfig | 2 +-
> configs/r8a77995_draak_defconfig | 2 +-
> configs/r8a779a0_falcon_defconfig | 2 +-
> configs/r8a779f0_spider_defconfig | 2 +-
> configs/r8a779g0_whitehawk_defconfig | 2 +-
> configs/r8a779h0_grayhawk_defconfig | 2 +-
> configs/rcar3_salvator-x_defconfig| 4 +-
> configs/rcar3_ulcb_defconfig | 4 +-
> configs/silinux_ek874_defconfig | 2 +-
> configs/silk_defconfig| 2 +-
> configs/stout_defconfig | 2 +-
> 55 files changed, 61 insertions(+), 90 deletions(-)
> rename arch/arm/dts/{r7s72100-gr-peach-u-boot.dts =>
> r7s72100-gr-peach-u-boot.dtsi} (97%)
> rename arch/arm/dts/{r8a774a1-hihope-rzg2m-u-boot.dts =>
> r8a774a1-hihope-rzg2m-u-boot.dtsi} (91%)
> rename arch/arm/dts/{r8a774b1-hihope-rzg2n-u-boot.dts =>
> r8a774b1-hihope-rzg2n-u-boot.dtsi} (91%)
> rename arch/arm/dts/{r8a774c0-ek874-u-boot.dts =>
> r8a774c0-ek874-u-boot.dtsi} (95%)
> rename arch/arm/dts/{r8a774e1-hihope-rzg2h-u-boot.dts =>
> r8a774e1-hihope-rzg2h-u-boot.dtsi} (91%)
> rename arch/arm/dts/{r8a7790-lager-u-boot.dts => r8a7790-lager-u-boot.dtsi}
> (91%)
> rename arch/arm/dts/{r8a7790-stout-u-boot.dts => r8a7790-stout-u-boot.dtsi}
> (91%)
> rename arch/arm/dts/{r8a7791-koelsch-u-boot.dts =>
> r8a7791-koelsch-u-boot.dtsi} (90%)
Re: [PATCH v1] arm: imx: imx8m: soc: Fix NPU/VPU fdt disable fixup
On Fri, Mar 15, 2024 at 11:45 AM Vitor Soares wrote: > > From: Vitor Soares > > On imx8m[m|p|q].dtsi, upstream Linux uses different names for NPU/VPU > IP block nodes. It leads variants without such HW block having it > enabled by default. > > This patch adds the upstream Linux node's paths to the disable list while > keep the compatibility with downstream Linux. > > Signed-off-by: Vitor Soares Applied for u-boot-imx/master, thanks.
Re: [PATCH] apalis-imx8: Fix sc_misc_otp_fuse_read() error check
On Tue, Mar 12, 2024 at 9:59 PM Fabio Estevam wrote:
>
> Commit bfb3409d676f ("imx: toradex/apalis-imx8: correct SCU API usage")
> made an incorrect logic change in the error code check of
> sc_misc_otp_fuse_read():
>
> - if (scierr == SC_ERR_NONE) {
> + if (scierr) {
> /* QP has one A72 core disabled */
> is_quadplus = ((val >> 4) & 0x3) != 0x0;
> }
>
> The other changes in this commit are correct.
>
> sc_misc_otp_fuse_read() returns 0 on a successful fuse read.
>
> This inversion causes board_mem_get_layout() to report incorrect RAM size.
>
> Go back the original error check logic to fix the problem.
>
> Fixes: bfb3409d676f ("imx: toradex/apalis-imx8: correct SCU API usage")
> Signed-off-by: Fabio Estevam
Applied for u-boot-imx/master, thanks.
Re: [PATCH] colibri-imx8x: Fix sc_misc_otp_fuse_read() error check
On Tue, Mar 12, 2024 at 9:36 PM Fabio Estevam wrote:
>
> Commit aa6e698a7acd ("imx: toradex/colibri-imx8x: correct SCU API usage")
> made an incorrect logic change in the error code check of
> sc_misc_otp_fuse_read():
>
> - if (sc_err == SC_ERR_NONE) {
> + if (sc_err) {
> /* DX has two A35 cores disabled */
> return (val & 0xf) != 0x0;
> }
>
> The other changes in this commit are correct.
>
> sc_misc_otp_fuse_read() returns 0 on a successful fuse read.
>
> This inversion causes board_mem_get_layout() to report incorrect RAM size.
>
> Go back the original error check logic to fix the problem.
>
> Fixes: aa6e698a7acd ("imx: toradex/colibri-imx8x: correct SCU API usage")
> Reported-by: Hiago De Franco
> Signed-off-by: Fabio Estevam
Applied for u-boot-imx/master, thanks.
Re: [PATCH] imx8m*_venice: move venice to OF_UPSTREAM
On Tue, Mar 12, 2024 at 10:16 PM Fabio Estevam wrote:
>
> Hi Tim,
>
> On Tue, Mar 12, 2024 at 4:05 PM Tim Harvey wrote:
> >
> > Move to imx8m{m,n,p}-venice to OF_UPSTREAM:
> > - replace the non-upstream generic imx8m{m,n,p}-venice dt with one of the
> >dt's from the OF_LIST
> > - handle the fact that dtbs now have a 'freescale/' prefix
> > - imply OF_UPSTREAM
> > - remove rudundant files from arch/arm/dts leaving only the
> >*-u-boot.dtsi files
> >
> > Signed-off-by: Tim Harvey
> ...
> > 33 files changed, 13 insertions(+), 10658 deletions(-)
>
> This diff looks great :-)
>
> Reviewed-by: Fabio Estevam
Applied for u-boot-imx/next, thanks.
Re: [PATCH v2 0/5] Add RAUC boot logic for phycore_imx8mp
On Tue, Mar 12, 2024 at 6:00 AM Leonard Anderweit wrote: > > This series adds RAUC boot logic for the phycore_imx8mp. > The first patch converts the environment from a CFG_EXTRA_ENV_SETTINGS #define > to a text environment for better readability and maintainability. > The second patch moves the default bootcmd from the defconfig to the board > environment. > The third patch enables the redundant environment on phycore_imx8mp. > Patch 4 adds RAUC boot logic common to all phytec boards. > Patch 5 adds the RAUC boot logic to phycore_imx8mp. > > v2: > - rebase on next Applied series for u-boot-imx/next, thanks.
Re: [PATCH] board: phytec: define get_som_type also when SoM detection is disabled
On Tue, Mar 12, 2024 at 6:39 AM Benjamin Hahn wrote:
>
> define the phytec_get_som_type function also when the SoM detection is
> disabled.
>
> Fixes:
> commit 110d321a56c3 ("board: phytec: common: phytec_som_detection: Add
> phytec_get_som_type")
>
> Signed-off-by: Benjamin Hahn
Applied for u-boot-imx/master, thanks.
Re: [PATCH v1] configs: colibri-imx7: enable watchdog
On Thu, Mar 7, 2024 at 12:23 PM Parth Pancholi wrote: > > From: Parth Pancholi > > Enable watchdog functionality for Toradex's Colibri iMX7 (NAND/EMMC) > modules. > > Signed-off-by: Parth Pancholi Applied for u-boot-imx/next, thanks.
Re: [PATCH] drivers: imx_tmu: Select polling-rate from cpu-thermal devicetree node
On Mon, Mar 4, 2024 at 8:49 AM Benjamin Hahn wrote: > > The polling rate is already specified in some devicetrees, like > imx8mp.dtsi for example, but was not selected so far. For the > trippoints, the cpu-thermal node is used. Also get the polling rate from > this node. Use the default of 5000ms if the polling rate should not be > specified in the devicetree. > > NOTE: The polling rate from the devicetree will be used after this > patch. In imx8*.dtsi devicetrees the polling delay is set to 2000ms for > example. > > Signed-off-by: Benjamin Hahn Applied for u-boot-imx/next, thanks.
[GIT PULL] Please pull u-boot-imx-next-20240317
Hi Tom, Please pull this material for next from u-boot-imx, thank The following changes since commit 099c94b7613bb10d97936447f5136f3a36694325: Merge tag 'u-boot-rockchip-20240315' of https://source.denx.de/u-boot/custodians/u-boot-rockchip into next (2024-03-15 09:15:31 -0400) are available in the Git repository at: https://gitlab.denx.de/u-boot/custodians/u-boot-imx.git tags/u-boot-imx-next-20240317 for you to fetch changes up to 86b79cf131b64eadae023a127921893d30503093: imx8m*_venice: move venice to OF_UPSTREAM (2024-03-17 18:39:54 -0300) u-boot-imx-next-20240317 -- CI: https://source.denx.de/u-boot/custodians/u-boot-imx/-/pipelines/19975 - Select polling-rate from cpu-thermal devicetree node on imx_tmu. - Re-organize the U-Boot environment and add RAUC logic for phycore_imx8mp. - Enable watchdog on colibri-imx7. - Move imx8mm-venice to use OF_UPSTREAM. Benjamin Hahn (1): drivers: imx_tmu: Select polling-rate from cpu-thermal devicetree node Leonard Anderweit (5): phycore_imx8mp: Move environment from include/config to board phycore_imx8mp: Move default bootcmd to board env configs: phycore-imx8mp_defconfig: Use redundant environment include: env: Add phytec RAUC boot logic board: phytec: phycore_imx8mp: Add RAUC boot logic to environment Parth Pancholi (1): configs: colibri-imx7: enable watchdog Tim Harvey (1): imx8m*_venice: move venice to OF_UPSTREAM arch/arm/dts/Makefile | 17 - arch/arm/dts/imx7d-colibri-eval-v3-u-boot.dtsi |4 + arch/arm/dts/imx8mm-venice-gw700x.dtsi | 525 --- arch/arm/dts/imx8mm-venice-gw71xx-0x.dts | 19 - arch/arm/dts/imx8mm-venice-gw71xx.dtsi | 239 - arch/arm/dts/imx8mm-venice-gw72xx-0x.dts | 19 - arch/arm/dts/imx8mm-venice-gw72xx.dtsi | 400 - arch/arm/dts/imx8mm-venice-gw73xx-0x.dts | 19 - arch/arm/dts/imx8mm-venice-gw73xx.dtsi | 452 -- arch/arm/dts/imx8mm-venice-gw7901.dts | 1137 arch/arm/dts/imx8mm-venice-gw7902.dts | 1052 -- arch/arm/dts/imx8mm-venice-gw7903.dts | 869 -- arch/arm/dts/imx8mm-venice-gw7904.dts | 928 --- arch/arm/dts/imx8mm-venice-gw7905-0x.dts | 28 - arch/arm/dts/imx8mm-venice-gw7905.dtsi | 303 --- arch/arm/dts/imx8mm-venice.dts | 169 arch/arm/dts/imx8mn-venice-gw7902.dts | 980 arch/arm/dts/imx8mn-venice.dts | 169 arch/arm/dts/imx8mp-venice-gw702x.dtsi | 587 arch/arm/dts/imx8mp-venice-gw71xx-2x.dts | 19 - arch/arm/dts/imx8mp-venice-gw71xx.dtsi | 236 - arch/arm/dts/imx8mp-venice-gw72xx-2x.dts | 19 - arch/arm/dts/imx8mp-venice-gw72xx.dtsi | 378 arch/arm/dts/imx8mp-venice-gw73xx-2x.dts | 19 - arch/arm/dts/imx8mp-venice-gw73xx.dtsi | 421 - arch/arm/dts/imx8mp-venice-gw74xx.dts | 1125 --- arch/arm/dts/imx8mp-venice-gw7905-2x.dts | 28 - arch/arm/dts/imx8mp-venice-gw7905.dtsi | 309 --- arch/arm/dts/imx8mp-venice.dts | 183 arch/arm/mach-imx/imx8m/Kconfig|3 + board/gateworks/venice/venice.c|7 +- board/phytec/phycore_imx8mp/phycore_imx8mp.env | 62 ++ configs/colibri_imx7_defconfig |2 + configs/colibri_imx7_emmc_defconfig|2 + configs/imx8mm_venice_defconfig|4 +- configs/imx8mn_venice_defconfig|4 +- configs/imx8mp_venice_defconfig|4 +- configs/phycore-imx8mp_defconfig |4 +- drivers/thermal/imx_tmu.c |6 +- include/configs/phycore_imx8mp.h | 43 - include/env/phytec/rauc.env| 52 ++ 41 files changed, 141 insertions(+), 10705 deletions(-) delete mode 100644 arch/arm/dts/imx8mm-venice-gw700x.dtsi delete mode 100644 arch/arm/dts/imx8mm-venice-gw71xx-0x.dts delete mode 100644 arch/arm/dts/imx8mm-venice-gw71xx.dtsi delete mode 100644 arch/arm/dts/imx8mm-venice-gw72xx-0x.dts delete mode 100644 arch/arm/dts/imx8mm-venice-gw72xx.dtsi delete mode 100644 arch/arm/dts/imx8mm-venice-gw73xx-0x.dts delete mode 100644 arch/arm/dts/imx8mm-venice-gw73xx.dtsi delete mode 100644 arch/arm/dts/imx8mm-venice-gw7901.dts delete mode 100644 arch/arm/dts/imx8mm-venice-gw7902.dts delete mode 100644 arch/arm/dts/imx8mm-venice-gw7903.dts delete mode 100644 arch/arm/dts/imx8mm-venice-gw7904.dts delete mode 100644 arch/arm/dts/imx8mm-venice-gw7905-0x.dts delete mode 100644 arch/arm/dts/imx8mm-venice-gw7905.dtsi delete mode 100644 arch/arm/dts/imx8mm-venice.dts delete mode 100644 arch/arm/dts
[GIT PULL] Please pull u-boot-imx-master-20240317
Hi Tom, Please pull these fixes from u-boot-imx, thanks. The following changes since commit 86fd291a7990af84e96808f48eff2219dd4ef496: Merge tag 'efi-2024-04-rc5' of https://source.denx.de/u-boot/custodians/u-boot-efi (2024-03-13 20:39:46 -0400) are available in the Git repository at: https://gitlab.denx.de/u-boot/custodians/u-boot-imx.git tags/u-boot-imx-master-20240317 for you to fetch changes up to e648c4a3455a4d1880efe121602ed90a0bc9b53f: arm: imx: imx8m: soc: Fix NPU/VPU fdt disable fixup (2024-03-17 18:00:04 -0300) u-boot-imx-master-20240317 -- CI: https://source.denx.de/u-boot/custodians/u-boot-imx/-/pipelines/19974 - Fix build error when SoM detection on Phytec board. - Fix sc_misc_otp_fuse_read() error check on colibri-imx8x/apalis-imx8. - Fix NPU/VPU fdt disable fixup on i.MX8M. Benjamin Hahn (1): board: phytec: define get_som_type also when SoM detection is disabled Fabio Estevam (2): colibri-imx8x: Fix sc_misc_otp_fuse_read() error check apalis-imx8: Fix sc_misc_otp_fuse_read() error check Vitor Soares (1): arm: imx: imx8m: soc: Fix NPU/VPU fdt disable fixup arch/arm/mach-imx/imx8m/soc.c | 18 ++ board/phytec/common/phytec_som_detection.c | 5 + board/toradex/apalis-imx8/apalis-imx8.c | 2 +- board/toradex/colibri-imx8x/colibri-imx8x.c | 2 +- 4 files changed, 21 insertions(+), 6 deletions(-)
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
Should be usb_denylist, since "block devices" is ambiguous meaning if
it is a list of data chunks (blocks) or if it blocks (deny). There is
no question what a denylist is.
On Sun, Mar 17, 2024 at 4:07 AM Janne Grunau via B4 Relay
wrote:
>
> From: Janne Grunau
>
> Add the environment variable "usb_blocklist" to prevent USB devices
> listed in it from being used. This allows to ignore devices which
> trigger bugs in u-boot's USB stack or are undesirable for other reasons.
> Devices emulating keyboards are one example. U-boot currently supports
> only one USB keyboard device. Most commonly, people run into this with
> Yubikeys, so let's ignore those in the default environment.
>
> Based on previous USB keyboard specific patches for the same purpose.
>
> Link:
> https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
> Signed-off-by: Janne Grunau
> ---
> common/usb.c | 56
> +++
> doc/usage/environment.rst | 12 ++
> include/env_default.h | 11 ++
> 3 files changed, 79 insertions(+)
>
> diff --git a/common/usb.c b/common/usb.c
> index 836506dcd9..73af5be066 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device *dev,
> int addr, bool do_read,
> return 0;
> }
>
> +static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
> +{
> + printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
> + blocklist);
> + return 0;
> +}
> +
> +static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
> +{
> + ulong vid, pid;
> + char *end;
> + const char *block_str = env_get("usb_blocklist");
> + const char *cur = block_str;
> +
> + /* parse "usb_blocklist" strictly */
> + while (cur && cur[0] != '\0') {
> + vid = simple_strtoul(cur, , 0);
> + if (cur == end || end[0] != ':')
> + return usb_blocklist_parse_error(block_str,
> +cur - block_str);
> +
> + cur = end + 1;
> + pid = simple_strtoul(cur, , 0);
> + if (cur == end && end[0] != '*')
> + return usb_blocklist_parse_error(block_str,
> +cur - block_str);
> +
> + if (end[0] == '*') {
> + /* use out of range idProduct as wildcard indication
> */
> + pid = U16_MAX + 1;
> + end++;
> + }
> + if (end[0] != ',' && end[0] != '\0')
> + return usb_blocklist_parse_error(block_str,
> +cur - block_str);
> +
> + if (id_vendor == vid && (pid > U16_MAX || id_product == pid))
> {
> + printf("Ignoring USB device 0x%x:0x%x\n",id_vendor,
> + id_product);
> + debug("Ignoring USB device 0x%x:0x%x\n",id_vendor,
> + id_product);
> + return 1;
> + }
> + if (end[0] == '\0')
> + break;
> + cur = end + 1;
> + }
> +
> + return 0;
> +}
> +
> int usb_select_config(struct usb_device *dev)
> {
> unsigned char *tmpbuf = NULL;
> @@ -1099,6 +1150,11 @@ int usb_select_config(struct usb_device *dev)
> le16_to_cpus(>descriptor.idProduct);
> le16_to_cpus(>descriptor.bcdDevice);
>
> + /* ignore devices from usb_blocklist */
> + if (usb_device_is_blocked(dev->descriptor.idVendor,
> + dev->descriptor.idProduct))
> + return -ENODEV;
> +
> /*
> * Kingston DT Ultimate 32GB USB 3.0 seems to be extremely sensitive
> * about this first Get Descriptor request. If there are any other
> diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
> index ebf75fa948..e42702adb2 100644
> --- a/doc/usage/environment.rst
> +++ b/doc/usage/environment.rst
> @@ -366,6 +366,18 @@ tftpwindowsize
> This means the count of blocks we can receive before
> sending ack to server.
>
> +usb_blocklist
> +Block USB devices from being bound to an USB device driver. This can be
> used
> +to ignore devices which causes crashes in u-boot's USB stack or are
> +undesirable for other reasons.
> +The default environment blocks Yubico devices since they emulate USB
> +keyboards. U-boot currently supports only a single USB keyboard device
> and
> +it is undesirable that a Yubikey is used as keyboard.
> +Devices are matched by idVendor and idProduct. The variable contains a
> comma
> +separated list of idVendor:idProduct pairs as hexadecimal numbers joined
> +by a colon. '*' functions as a wildcard for idProduct
[PATCH 3/3] rockchip: io-domain: Add support for RK3328
Port the RK3328 part of the Rockchip IO-domain driver from linux.
This differs from linux version in that pmu io iodomain bit is enabled
in the write ops instead of in an init ops as in linux, this way we can
avoid keeping a full state of all supply that have been configured.
Enable by default on all RK3328 boards, skip rk3328-evb because this
target is typically also used on miscellaneous boards and boxes not
fully supported by U-Boot.
Signed-off-by: Jonas Karlman
---
arch/arm/mach-rockchip/rk3328/syscon_rk3328.c | 3 ++
configs/evb-rk3328_defconfig | 1 +
drivers/misc/Kconfig | 2 +-
drivers/misc/rockchip-io-domain.c | 38 +++
4 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-rockchip/rk3328/syscon_rk3328.c
b/arch/arm/mach-rockchip/rk3328/syscon_rk3328.c
index daf74a0e2d37..d2f267e63534 100644
--- a/arch/arm/mach-rockchip/rk3328/syscon_rk3328.c
+++ b/arch/arm/mach-rockchip/rk3328/syscon_rk3328.c
@@ -17,4 +17,7 @@ U_BOOT_DRIVER(rockchip_rk3328_grf) = {
.name = "rockchip_rk3328_grf",
.id = UCLASS_SYSCON,
.of_match = rk3328_syscon_ids,
+#if CONFIG_IS_ENABLED(OF_REAL)
+ .bind = dm_scan_fdt_dev,
+#endif
};
diff --git a/configs/evb-rk3328_defconfig b/configs/evb-rk3328_defconfig
index 75a0e0f286bd..53ad6777ec50 100644
--- a/configs/evb-rk3328_defconfig
+++ b/configs/evb-rk3328_defconfig
@@ -57,6 +57,7 @@ CONFIG_FASTBOOT_BUF_ADDR=0x800800
CONFIG_FASTBOOT_CMD_OEM_FORMAT=y
CONFIG_ROCKCHIP_GPIO=y
CONFIG_SYS_I2C_ROCKCHIP=y
+# CONFIG_ROCKCHIP_IODOMAIN is not set
CONFIG_MMC_DW=y
CONFIG_MMC_DW_ROCKCHIP=y
CONFIG_PHY_MOTORCOMM=y
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index f11ce72525f5..860420d3e3e3 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -104,7 +104,7 @@ config ROCKCHIP_OTP
config ROCKCHIP_IODOMAIN
bool "Rockchip IO-domain driver support"
depends on DM_REGULATOR && ARCH_ROCKCHIP
- default y if ROCKCHIP_RK3568
+ default y if ROCKCHIP_RK3328 || ROCKCHIP_RK3568
help
Enable support for IO-domains in Rockchip SoCs. It is necessary
for the IO-domain setting of the SoC to match the voltage supplied
diff --git a/drivers/misc/rockchip-io-domain.c
b/drivers/misc/rockchip-io-domain.c
index 0ffea32ef07f..04d4d07c4127 100644
--- a/drivers/misc/rockchip-io-domain.c
+++ b/drivers/misc/rockchip-io-domain.c
@@ -27,6 +27,10 @@
#define MAX_VOLTAGE_1_8198
#define MAX_VOLTAGE_3_3360
+#define RK3328_SOC_CON40x410
+#define RK3328_SOC_CON4_VCCIO2 BIT(7)
+#define RK3328_SOC_VCCIO2_SUPPLY_NUM 1
+
#define RK3399_PMUGRF_CON0 0x180
#define RK3399_PMUGRF_CON0_VSELBIT(8)
#define RK3399_PMUGRF_VSEL_SUPPLY_NUM 9
@@ -95,6 +99,22 @@ static int rockchip_iodomain_write(struct regmap *grf, uint
offset, int idx, int
return regmap_write(grf, offset, val);
}
+static int rk3328_iodomain_write(struct regmap *grf, uint offset, int idx, int
uV)
+{
+ int ret = rockchip_iodomain_write(grf, offset, idx, uV);
+
+ if (!ret && idx == RK3328_SOC_VCCIO2_SUPPLY_NUM) {
+ /*
+* set vccio2 iodomain to also use this framework
+* instead of a special gpio.
+*/
+ u32 val = RK3328_SOC_CON4_VCCIO2 | (RK3328_SOC_CON4_VCCIO2 <<
16);
+ ret = regmap_write(grf, RK3328_SOC_CON4, val);
+ }
+
+ return ret;
+}
+
static int rk3399_pmu_iodomain_write(struct regmap *grf, uint offset, int idx,
int uV)
{
int ret = rockchip_iodomain_write(grf, offset, idx, uV);
@@ -111,6 +131,20 @@ static int rk3399_pmu_iodomain_write(struct regmap *grf,
uint offset, int idx, i
return ret;
}
+static const struct rockchip_iodomain_soc_data soc_data_rk3328 = {
+ .grf_offset = 0x410,
+ .supply_names = {
+ "vccio1-supply",
+ "vccio2-supply",
+ "vccio3-supply",
+ "vccio4-supply",
+ "vccio5-supply",
+ "vccio6-supply",
+ "pmuio-supply",
+ },
+ .write = rk3328_iodomain_write,
+};
+
static const struct rockchip_iodomain_soc_data soc_data_rk3399 = {
.grf_offset = 0xe640,
.supply_names = {
@@ -156,6 +190,10 @@ static const struct rockchip_iodomain_soc_data
soc_data_rk3568_pmu = {
};
static const struct udevice_id rockchip_iodomain_ids[] = {
+ {
+ .compatible = "rockchip,rk3328-io-voltage-domain",
+ .data = (ulong)_data_rk3328,
+ },
{
.compatible = "rockchip,rk3399-io-voltage-domain",
.data = (ulong)_data_rk3399,
--
2.43.2
[PATCH 1/3] rockchip: rk3328: Sort imply statements alphabetically
Sort imply statements under ROCKCHIP_RK3328 alphabetically and remove ENABLE_ARM_SOC_BOOT0_HOOK, DEBUG_UART_BOARD_INIT and SYS_NS16550, they are already implyed or selected by ARCH_ROCKCHIP. Signed-off-by: Jonas Karlman --- arch/arm/mach-rockchip/Kconfig | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index 3d6a76a793e7..b723c72f09c4 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -191,18 +191,15 @@ config ROCKCHIP_RK3328 select SUPPORT_TPL select TPL select TPL_NEEDS_SEPARATE_STACK if TPL + imply MISC + imply MISC_INIT_R imply ROCKCHIP_COMMON_BOARD + imply ROCKCHIP_EFUSE imply ROCKCHIP_SDRAM_COMMON imply SPL_ROCKCHIP_COMMON_BOARD + imply SPL_SEPARATE_BSS imply SPL_SERIAL imply TPL_SERIAL - imply SPL_SEPARATE_BSS - select ENABLE_ARM_SOC_BOOT0_HOOK - select DEBUG_UART_BOARD_INIT - select SYS_NS16550 - imply MISC - imply ROCKCHIP_EFUSE - imply MISC_INIT_R help The Rockchip RK3328 is a ARM-based SoC with a quad-core Cortex-A53. including NEON and GPU, 1MB L2 cache, Mali-T7 graphics, two -- 2.43.2
[PATCH 2/3] rockchip: rk3328: Enable ARMv8 crypto extensions
The RK3328 SoC support ARMv8 Cryptography Extensions and use of the ARMv8 crypto extensions help speed up FIT checksum validation in SPL. Imply ARMV8_SET_SMPEN and ARMV8_CRYPTO to take advantage of the crypto extensions for SHA256 when validating checksum of FIT images. Also imply OF_LIVE to help speed up init of U-Boot proper. Signed-off-by: Jonas Karlman --- arch/arm/mach-rockchip/Kconfig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index b723c72f09c4..fee463e6d92f 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -191,8 +191,11 @@ config ROCKCHIP_RK3328 select SUPPORT_TPL select TPL select TPL_NEEDS_SEPARATE_STACK if TPL + imply ARMV8_CRYPTO + imply ARMV8_SET_SMPEN imply MISC imply MISC_INIT_R + imply OF_LIVE imply ROCKCHIP_COMMON_BOARD imply ROCKCHIP_EFUSE imply ROCKCHIP_SDRAM_COMMON -- 2.43.2
[PATCH 0/3] rockchip: rk3328: Add IO-domain driver and speed up boot
This series adds support for RK3328 to the IO-domain driver, it also enabled ARMv8 crypto extensions and OF_LIVE to speed up boot on rk3328 boards. Before this series init time is around 4 seconds on a Rock64 v2.0: => bootstage report Timer summary in microseconds (9 records): MarkElapsed Stage 0 0 reset 337,470337,470 board_init_f 956,864619,394 board_init_r 2,780,894 1,824,030 eth_common_init 3,935,118 1,154,224 eth_initialize 3,935,373255 main_loop 3,940,666 5,293 cli_loop Accumulated time: 12,313 dm_r 327,343 dm_f After this series init time is around 1.1 seconds on same Rock64 v2.0: => bootstage report Timer summary in microseconds (10 records): MarkElapsed Stage 0 0 reset 265,803265,803 board_init_f 904,140638,337 board_init_r 979,531 75,391 eth_common_init 1,148,105168,574 eth_initialize 1,148,308203 main_loop 1,298,735150,427 cli_loop Accumulated time: 6,793 of_live 17,891 dm_r 344,821 dm_f Do not fully understand why OF_LIVE have such big impact on init time. It does not have such big impact on e.g. a RK3308 board. Jonas Karlman (3): rockchip: rk3328: Sort imply statements alphabetically rockchip: rk3328: Enable ARMv8 crypto extensions rockchip: io-domain: Add support for RK3328 arch/arm/mach-rockchip/Kconfig| 14 +++ arch/arm/mach-rockchip/rk3328/syscon_rk3328.c | 3 ++ configs/evb-rk3328_defconfig | 1 + drivers/misc/Kconfig | 2 +- drivers/misc/rockchip-io-domain.c | 38 +++ 5 files changed, 50 insertions(+), 8 deletions(-) -- 2.43.2
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
Hej,
On Sun, Mar 17, 2024, at 17:18, Marek Vasut wrote:
> On 3/17/24 12:07 PM, Janne Grunau via B4 Relay wrote:
>> From: Janne Grunau
>>
>> Add the environment variable "usb_blocklist" to prevent USB devices
>> listed in it from being used. This allows to ignore devices which
>> trigger bugs in u-boot's USB stack or are undesirable for other reasons.
>> Devices emulating keyboards are one example. U-boot currently supports
>> only one USB keyboard device. Most commonly, people run into this with
>> Yubikeys, so let's ignore those in the default environment.
>>
>> Based on previous USB keyboard specific patches for the same purpose.
>>
>> Link:
>> https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
>> Signed-off-by: Janne Grunau
>> ---
>> common/usb.c | 56
>> +++
>> doc/usage/environment.rst | 12 ++
>> include/env_default.h | 11 ++
>> 3 files changed, 79 insertions(+)
>>
>> diff --git a/common/usb.c b/common/usb.c
>> index 836506dcd9..73af5be066 100644
>> --- a/common/usb.c
>> +++ b/common/usb.c
>> @@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device *dev,
>> int addr, bool do_read,
>> return 0;
>> }
>>
>> +static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
>> +{
>> +printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
>> + blocklist);
>> +return 0;
>
> This could be static void without return 0 at the end.
the return is there to break out of the while loop on parsing errors in a
single statement. This probably won't be necessary after using strsep and
sscanf in the parsing function but see below.
>> +}
>> +
>> +static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
>> +{
>> +ulong vid, pid;
>> +char *end;
>> +const char *block_str = env_get("usb_blocklist");
>> +const char *cur = block_str;
>> +
>> +/* parse "usb_blocklist" strictly */
>> +while (cur && cur[0] != '\0') {
>
> Have a look at strsep() , namely strsep(block_str, ","); This will split
> the string up for you at "," delimiters.
>
> Example is in drivers/dfu/dfu.c dfu_config_interfaces() .
strsep() is probably a good idea even if it alone won't make the code that much
simpler for strict parsing.
> And then, on each token, you can try and run sscanf(token, "%04x:%04x",
> vid, pid);, that will parse the token format for you. See e.g.
> test/lib/sscanf.c for further examples.
>
> That should simplify the parsing a lot.
It would but sscanf() is optional and is only selected by CONFIG_XEN so I
assumed there would be concerns over binary size increase if USB_HOST would
require sscanf.
Janne
Re: [PATCH 1/3] boot: fdt: Change type of env_get_bootm_low() to phys_addr_t
On 17.03.24 17:58, Marek Vasut wrote:
On 3/17/24 10:08 AM, Heinrich Schuchardt wrote:
On 3/17/24 07:16, Marek Vasut wrote:
Change type of ulong env_get_bootm_low() to phys_addr_t
env_get_bootm_low().
The PPC/LS systems already treat env_get_bootm_low() result as
phys_addr_t,
while the function itself still returns ulong. This is potentially
dangerous
on 64bit systems, where ulong might not be large enough to hold the
content
Isn't long always 64bit when building with gcc or llvm?
C99 spec says:
5.2.4.2.1 Sizes of integer types
...
- maximum value for an object of type unsigned long int
ULONG_MAX 4294967295 // 2^32 - 1
...
Simpler table:
https://en.wikipedia.org/wiki/C_data_types
So, to answer the question, it might, but we cannot rely on that.
Also, phys_addr_t represents physical addresses, which this is.
[...]
@@ -553,8 +554,8 @@ int boot_ramdisk_high(struct lmb *lmb, ulong
rd_data, ulong rd_len,
initrd_high = env_get_bootm_mapsize() + env_get_bootm_low();
}
- debug("## initrd_high = 0x%08lx, copy_to_ram = %d\n",
- initrd_high, initrd_copy_to_ram);
+ debug("## initrd_high = 0x%p, copy_to_ram = %d\n",
+ (void *)(uintptr_t)initrd_high, initrd_copy_to_ram);
Void * may be narrower than phys_addr_t.
When would that happen, on PAE systems ?
Shouldn't we convert to unsigned long long for printing.
Printing phys_addr_t always confuses me. I was under the impression that
turning the value into uintptr_t (platform pointer type represented as
integer) and then actually using that as a pointer for printing should
not suffer from any type width problems. Does it ?
As you already remarked on LPAE this may happen.
Though you may not be able load initrd outside the address range of
void* the variables might exceed it.
Best regards
Heinrich
Since this is a debug print, I can just upcast it to u64.
Re: [PATCH 2/3] boot: fdt: Drop lmb_alloc*() typecasts
On 3/17/24 10:16 AM, Heinrich Schuchardt wrote: On 3/17/24 07:16, Marek Vasut wrote: The lmb_alloc_base() returns phys_addr_t , map_sysmem() accepts phys_addr_t as first parameter. Declare $addr as phys_addr_t and %s/$addr/addr/ I'm not sure this helps readability, I want to delimit the variable name somehow, so I switched this to 'addr' here and for 'usable' in 3/3. get rid of the casts. Reported-by: Laurent Pinchart Signed-off-by: Marek Vasut --- Cc: Heinrich Schuchardt Cc: Kuninori Morimoto Cc: Laurent Pinchart Cc: Simon Glass Cc: Tom Rini --- boot/image-fdt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/boot/image-fdt.c b/boot/image-fdt.c index c2571b22244..c37442c9130 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -162,6 +162,7 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size) void *of_start = NULL; phys_addr_t start, size, usable; char *fdt_high; + phys_addr_t addr; Please, keep variables in the narrowest scope where they are used. Have a look at the entire function, this is the narrowest scope, $addr is used in both branches of the conditional now, in the same way too.
Re: [PATCH 3/3] boot: fdt: Move usable variable below updated comment
On 3/17/24 12:29 PM, Laurent Pinchart wrote: Hi Marek, Thank you for the patch. On Sun, Mar 17, 2024 at 07:16:31AM +0100, Marek Vasut wrote: Move the variable below comment which explains what the variable means. Update the comment. No functional change. Suggested-by: Laurent Pinchart Signed-off-by: Marek Vasut --- Cc: Heinrich Schuchardt Cc: Kuninori Morimoto Cc: Laurent Pinchart Cc: Simon Glass Cc: Tom Rini --- boot/image-fdt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/boot/image-fdt.c b/boot/image-fdt.c index c37442c9130..16cd15e7e44 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -218,12 +218,12 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size) if (start + size < low) continue; - usable = min(start + size, low + mapsize); - /* * At least part of this DRAM bank is usable, try -* using it for LMB allocation. +* using the the DRAM bank up to $usable address s/the the/the/ Is it a u-boot convention to write $variable to refer to a variable in comments ? I don't think there is any convention so far, but some convention would be good to discern what is a variable and what is a regular word, esp. since 'usable' appears in both forms in the comment. Kerneldoc @usable could be an option, shell $usable could also be an option.
Re: [PATCH 2/3] boot: fdt: Drop lmb_alloc*() typecasts
On 3/17/24 12:26 PM, Laurent Pinchart wrote:
Hi Marek,
Thank you for the patch.
On Sun, Mar 17, 2024 at 07:16:30AM +0100, Marek Vasut wrote:
The lmb_alloc_base() returns phys_addr_t , map_sysmem() accepts
phys_addr_t as first parameter. Declare $addr as phys_addr_t and
s/$addr/addr/ ?
get rid of the casts.
Reported-by: Laurent Pinchart
Signed-off-by: Marek Vasut
---
Cc: Heinrich Schuchardt
Cc: Kuninori Morimoto
Cc: Laurent Pinchart
Cc: Simon Glass
Cc: Tom Rini
---
boot/image-fdt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index c2571b22244..c37442c9130 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -162,6 +162,7 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree,
ulong *of_size)
void*of_start = NULL;
phys_addr_t start, size, usable;
char*fdt_high;
+ phys_addr_t addr;
phys_addr_t low;
phys_size_t mapsize;
ulong of_len = 0;
@@ -186,7 +187,6 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree,
ulong *of_size)
fdt_high = env_get("fdt_high");
if (fdt_high) {
ulong desired_addr = hextoul(fdt_high, NULL);
- ulong addr;
I would keep addr declared here. With that,
I already replied to Heinrich on both parts, let's continue the
discussion there.
Re: [PATCH 1/3] boot: fdt: Change type of env_get_bootm_low() to phys_addr_t
On 3/17/24 12:18 PM, Laurent Pinchart wrote:
Hi Marek,
Thank you for the patch.
On Sun, Mar 17, 2024 at 07:16:29AM +0100, Marek Vasut wrote:
Change type of ulong env_get_bootm_low() to phys_addr_t env_get_bootm_low().
The PPC/LS systems already treat env_get_bootm_low() result as phys_addr_t,
while the function itself still returns ulong. This is potentially dangerous
on 64bit systems, where ulong might not be large enough to hold the content
of "bootm_low" environment variable. Fix it by using phys_addr_t, similar to
what env_get_bootm_size() does, which returns phys_size_t .
Reported-by: Laurent Pinchart
Signed-off-by: Marek Vasut
---
Cc: Heinrich Schuchardt
Cc: Kuninori Morimoto
Cc: Laurent Pinchart
Cc: Simon Glass
Cc: Tom Rini
---
boot/bootm.c | 2 +-
boot/image-board.c | 11 ++-
boot/image-fdt.c | 9 +
include/image.h| 2 +-
4 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/boot/bootm.c b/boot/bootm.c
index d071537d692..2e818264f5f 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -242,7 +242,7 @@ static int boot_get_kernel(const char *addr_fit, struct
bootm_headers *images,
#ifdef CONFIG_LMB
static void boot_start_lmb(struct bootm_headers *images)
{
- ulong mem_start;
+ phys_addr_t mem_start;
phys_size_t mem_size;
mem_start = env_get_bootm_low();
diff --git a/boot/image-board.c b/boot/image-board.c
index 75f6906cd56..447ced2678a 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -107,12 +107,13 @@ static int on_loadaddr(const char *name, const char
*value, enum env_op op,
}
U_BOOT_ENV_CALLBACK(loadaddr, on_loadaddr);
-ulong env_get_bootm_low(void)
+phys_addr_t env_get_bootm_low(void)
{
char *s = env_get("bootm_low");
+ phys_size_t tmp;
if (s) {
- ulong tmp = hextoul(s, NULL);
+ tmp = (phys_addr_t)simple_strtoull(s, NULL, 16);
return tmp;
Can't you write
return (phys_addr_t)simple_strtoull(s, NULL, 16);
and avoid the temporary variable ?
Fixed in V2, thanks
Re: [PATCH 1/3] boot: fdt: Change type of env_get_bootm_low() to phys_addr_t
On 3/17/24 10:08 AM, Heinrich Schuchardt wrote:
On 3/17/24 07:16, Marek Vasut wrote:
Change type of ulong env_get_bootm_low() to phys_addr_t
env_get_bootm_low().
The PPC/LS systems already treat env_get_bootm_low() result as
phys_addr_t,
while the function itself still returns ulong. This is potentially
dangerous
on 64bit systems, where ulong might not be large enough to hold the
content
Isn't long always 64bit when building with gcc or llvm?
C99 spec says:
5.2.4.2.1 Sizes of integer types
...
- maximum value for an object of type unsigned long int
ULONG_MAX 4294967295 // 2^32 - 1
...
Simpler table:
https://en.wikipedia.org/wiki/C_data_types
So, to answer the question, it might, but we cannot rely on that.
Also, phys_addr_t represents physical addresses, which this is.
[...]
@@ -553,8 +554,8 @@ int boot_ramdisk_high(struct lmb *lmb, ulong
rd_data, ulong rd_len,
initrd_high = env_get_bootm_mapsize() + env_get_bootm_low();
}
- debug("## initrd_high = 0x%08lx, copy_to_ram = %d\n",
- initrd_high, initrd_copy_to_ram);
+ debug("## initrd_high = 0x%p, copy_to_ram = %d\n",
+ (void *)(uintptr_t)initrd_high, initrd_copy_to_ram);
Void * may be narrower than phys_addr_t.
When would that happen, on PAE systems ?
Shouldn't we convert to unsigned long long for printing.
Printing phys_addr_t always confuses me. I was under the impression that
turning the value into uintptr_t (platform pointer type represented as
integer) and then actually using that as a pointer for printing should
not suffer from any type width problems. Does it ?
Since this is a debug print, I can just upcast it to u64.
iMX8MP - Falcon Mode in QSPI Boot Implementation
Dear U-Boot community list members, I have a NXP i.MX8M Plus EVB which I'm trying to enable Falcon mode in secure boot from QSPI on it. I saw that there is no defconfig file for this boot method (using FlexSPI) so I took imx8mp_evk_defconfig and modified it manually to the attached configuration file (Which does not contain secure boot configurations). I encountered an issue where I'm able to load all the FIT image components (ATF, DTB and Kernel images) into RAM, but after the ATF finished its run, the Kernel doesn't start (When I used the Falcon mode in SD/eMMC boot mode - The Kernel Started its initialization right after ATF run). Can you please advise what could be the problem? Or what in your opinion is the best (and fastest) way to fix and solve this issue? Thank you in advance, Nitzan Tavor. Nitzan Tavor Software Team Leader | R Solutions Your Goal. Our Creation www.abra-it.com This message contains confidential information and is intended only for the individual named. If you are not the named addressee you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately by e-mail if you have received this e-mail by mistake and delete this e-mail from your system. E-mail transmission cannot be guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender therefore does not accept liability for any errors or omissions in the contents of this message, which arise as a result of e-mail transmission. If verification is required please request a hard-copy version. imx8mp_evk_fspi_falcon_defconfig Description: imx8mp_evk_fspi_falcon_defconfig
Re: [PATCH v2 6/6] usb: kbd: Add probe quirk for Apple and Keychron keyboards
On 3/17/24 12:07 PM, Janne Grunau via B4 Relay wrote: From: Janne Grunau Those keyboards do not return the current device state. Polling will timeout unless there are key presses. This is not a problem during operation but the inital device state query during probing will fail. Skip this step in usb_kbd_probe_dev() to make these devices useable. Not all Apple keyboards behave like this. A keyboard with USB vendor/product ID 05ac:0221 is reported to work with the current code. Unfortunately some Keychron keyboards "re-use" Apple's vendor ID and show the same behavior (Keychron C2, 05ac:024f for example). Reviewed-by: Marek Vasut Signed-off-by: Janne Grunau --- common/usb_kbd.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 8cc3345075..43c7668671 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -31,6 +31,10 @@ #define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_2021 0x029a #define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_20210x029f +#define USB_VENDOR_ID_KEYCHRON 0x3434 + +#define USB_HID_QUIRK_POLL_NO_REPORT_IDLE (1 << 0) Use BIT(0) instead of (1 << 0) With that fixed: Reviewed-by: Marek Vasut
Re: [PATCH v2 5/6] usb: kbd: support Apple Magic Keyboards (2021)
On 3/17/24 12:07 PM, Janne Grunau via B4 Relay wrote: From: Janne Grunau Apple USB keyboards (Magic Keyboard from 2021 (product id 0x029c)) carry the HID keyboard boot protocol on the second interface descriptor. Probe via vendor and product IDs since the class/subclass/protocol match uses the first interface descriptor. Probe the two first interface descriptors for the HID keyboard boot protocol. USB configuration descriptor for reference: Reviewed-by: Marek Vasut Thank you for the detailed commit message, that is real helpful !
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
On 3/17/24 12:07 PM, Janne Grunau via B4 Relay wrote:
From: Janne Grunau
Add the environment variable "usb_blocklist" to prevent USB devices
listed in it from being used. This allows to ignore devices which
trigger bugs in u-boot's USB stack or are undesirable for other reasons.
Devices emulating keyboards are one example. U-boot currently supports
only one USB keyboard device. Most commonly, people run into this with
Yubikeys, so let's ignore those in the default environment.
Based on previous USB keyboard specific patches for the same purpose.
Link:
https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
Signed-off-by: Janne Grunau
---
common/usb.c | 56 +++
doc/usage/environment.rst | 12 ++
include/env_default.h | 11 ++
3 files changed, 79 insertions(+)
diff --git a/common/usb.c b/common/usb.c
index 836506dcd9..73af5be066 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device *dev,
int addr, bool do_read,
return 0;
}
+static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
+{
+ printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
+ blocklist);
+ return 0;
This could be static void without return 0 at the end.
+}
+
+static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
+{
+ ulong vid, pid;
+ char *end;
+ const char *block_str = env_get("usb_blocklist");
+ const char *cur = block_str;
+
+ /* parse "usb_blocklist" strictly */
+ while (cur && cur[0] != '\0') {
Have a look at strsep() , namely strsep(block_str, ","); This will split
the string up for you at "," delimiters.
Example is in drivers/dfu/dfu.c dfu_config_interfaces() .
And then, on each token, you can try and run sscanf(token, "%04x:%04x",
vid, pid);, that will parse the token format for you. See e.g.
test/lib/sscanf.c for further examples.
That should simplify the parsing a lot.
[...]
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
On 3/17/24 12:34 PM, Janne Grunau wrote:
Hej,
On Sun, Mar 17, 2024, at 12:07, Janne Grunau via B4 Relay wrote:
From: Janne Grunau
Add the environment variable "usb_blocklist" to prevent USB devices
listed in it from being used. This allows to ignore devices which
trigger bugs in u-boot's USB stack or are undesirable for other reasons.
Devices emulating keyboards are one example. U-boot currently supports
only one USB keyboard device. Most commonly, people run into this with
Yubikeys, so let's ignore those in the default environment.
Based on previous USB keyboard specific patches for the same purpose.
Link:
https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
Signed-off-by: Janne Grunau
---
common/usb.c | 56
+++
doc/usage/environment.rst | 12 ++
include/env_default.h | 11 ++
3 files changed, 79 insertions(+)
diff --git a/common/usb.c b/common/usb.c
index 836506dcd9..73af5be066 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device
*dev, int addr, bool do_read,
return 0;
}
+static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
+{
+ printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
+ blocklist);
+ return 0;
+}
+
+static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
+{
+ ulong vid, pid;
+ char *end;
+ const char *block_str = env_get("usb_blocklist");
+ const char *cur = block_str;
+
+ /* parse "usb_blocklist" strictly */
+ while (cur && cur[0] != '\0') {
+ vid = simple_strtoul(cur, , 0);
+ if (cur == end || end[0] != ':')
+ return usb_blocklist_parse_error(block_str,
+cur - block_str);
+
+ cur = end + 1;
+ pid = simple_strtoul(cur, , 0);
+ if (cur == end && end[0] != '*')
+ return usb_blocklist_parse_error(block_str,
+cur - block_str);
+
+ if (end[0] == '*') {
+ /* use out of range idProduct as wildcard indication */
+ pid = U16_MAX + 1;
+ end++;
+ }
+ if (end[0] != ',' && end[0] != '\0')
+ return usb_blocklist_parse_error(block_str,
+cur - block_str);
+
+ if (id_vendor == vid && (pid > U16_MAX || id_product == pid)) {
+ printf("Ignoring USB device 0x%x:0x%x\n",id_vendor,
+ id_product);
this is a leftover from testing, already removed locally
You could turn this into dev_dbg()
Re: [PATCH v2 3/6] usb: xhci: Abort transfers with unallocated rings
On 3/17/24 12:07 PM, Janne Grunau via B4 Relay wrote: From: Janne Grunau Discovered while trying to use the second interface in the USB keyboard driver necessary on Apple USB keyboards. Signed-off-by: Janne Grunau Reviewed-by: Marek Vasut
Re: [PATCH 0/4] configs: apple: Switch to standard boot + small adjustments
On Sun, Mar 17, 2024 at 7:54 AM Janne Grunau via B4 Relay wrote: > > This series contains a few misc config changes for Apple silicon > systems: > - switch from the deprecated distro boot scripts to standard boot > - allows EFI console resizing based on the video console size > - enables 16x32 bitmap fonts as Apple devices come with high DPI > displays > - enables 64-bit LBA addressing for USB storage devices larger than 2TB > > Signed-off-by: Janne Grunau > --- > Hector Martin (1): > apple_m1_defconfig: Turn on CONFIG_SYS_64BIT_LBA > > Janne Grunau (3): > configs: apple: Use "vidconsole,serial" as stdout/stderr > configs: apple: Enable CMD_SELECT_FONT and FONT_16X32 > arm: apple: Switch to standard boot > > arch/arm/Kconfig | 2 +- > configs/apple_m1_defconfig | 3 +++ > include/configs/apple.h| 24 > 3 files changed, 8 insertions(+), 21 deletions(-) > --- > base-commit: f3c979dd0053c082d2df170446923e7ce5edbc2d > change-id: 20240317-apple_config-981fcd9028b9 > LGTM. Reviewed-by: Neal Gompa -- 真実はいつも一つ！/ Always, there's only one truth!
[PATCH 1/4] apple_m1_defconfig: Turn on CONFIG_SYS_64BIT_LBA
From: Hector Martin This makes USB HDDs >2TiB work. The only reason this hasn't bitten us for the internal NVMe yet is the 4K sector size, because the largest SSD Apple sells is 8TB and we can handle up to 16TiB with that sector size. Close call. Signed-off-by: Hector Martin Signed-off-by: Janne Grunau --- configs/apple_m1_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig index e00d72e8be..31d966f0ab 100644 --- a/configs/apple_m1_defconfig +++ b/configs/apple_m1_defconfig @@ -10,6 +10,7 @@ CONFIG_SYS_PBSIZE=276 CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_BOARD_LATE_INIT=y # CONFIG_NET is not set +CONFIG_SYS_64BIT_LBA=y CONFIG_APPLE_SPI_KEYB=y # CONFIG_MMC is not set CONFIG_NVME_APPLE=y -- 2.44.0
[PATCH 3/4] configs: apple: Enable CMD_SELECT_FONT and FONT_16X32
From: Janne Grunau Apple devices have high DPI displays so the larger fonts are preferable for improved readability. This does not yet change the used font based on the display's pixel density so the standard 8x16 font is still used by default. Signed-off-by: Janne Grunau --- configs/apple_m1_defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig index 31d966f0ab..c30aec7c55 100644 --- a/configs/apple_m1_defconfig +++ b/configs/apple_m1_defconfig @@ -9,6 +9,7 @@ CONFIG_SYS_PBSIZE=276 # CONFIG_DISPLAY_CPUINFO is not set CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_BOARD_LATE_INIT=y +CONFIG_CMD_SELECT_FONT=y # CONFIG_NET is not set CONFIG_SYS_64BIT_LBA=y CONFIG_APPLE_SPI_KEYB=y @@ -19,6 +20,7 @@ CONFIG_USB_XHCI_DWC3=y CONFIG_USB_XHCI_PCI=y CONFIG_USB_DWC3=y CONFIG_USB_KEYBOARD=y +CONFIG_VIDEO_FONT_16X32=y CONFIG_SYS_WHITE_ON_BLACK=y CONFIG_NO_FB_CLEAR=y CONFIG_VIDEO_SIMPLE=y -- 2.44.0
[PATCH 2/4] configs: apple: Use "vidconsole,serial" as stdout/stderr
From: Janne Grunau The display size querying in efi_console relies on this order. The display should be the primary output device and should be used to display more than 80x25 chars. Signed-off-by: Janne Grunau --- include/configs/apple.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/configs/apple.h b/include/configs/apple.h index 0576bc04c9..a70440b3ad 100644 --- a/include/configs/apple.h +++ b/include/configs/apple.h @@ -6,8 +6,8 @@ /* Environment */ #define ENV_DEVICE_SETTINGS \ "stdin=serial,usbkbd,spikbd\0" \ - "stdout=serial,vidconsole\0" \ - "stderr=serial,vidconsole\0" + "stdout=vidconsole,serial\0" \ + "stderr=vidconsole,serial\0" #if IS_ENABLED(CONFIG_CMD_NVME) #define BOOT_TARGET_NVME(func) func(NVME, nvme, 0) -- 2.44.0
[PATCH 4/4] arm: apple: Switch to standard boot
From: Janne Grunau Use standard boot instead of the distro boot scripts. Signed-off-by: Janne Grunau --- arch/arm/Kconfig| 2 +- include/configs/apple.h | 20 ++-- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 01d6556c42..ad89abde41 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1034,7 +1034,7 @@ config ARCH_APPLE select USB imply CMD_DM imply CMD_GPT - imply DISTRO_DEFAULTS + imply BOOTSTD_DEFAULTS imply OF_HAS_PRIOR_STAGE config ARCH_OWL diff --git a/include/configs/apple.h b/include/configs/apple.h index a70440b3ad..1e08b11448 100644 --- a/include/configs/apple.h +++ b/include/configs/apple.h @@ -9,26 +9,10 @@ "stdout=vidconsole,serial\0" \ "stderr=vidconsole,serial\0" -#if IS_ENABLED(CONFIG_CMD_NVME) - #define BOOT_TARGET_NVME(func) func(NVME, nvme, 0) -#else - #define BOOT_TARGET_NVME(func) -#endif - -#if IS_ENABLED(CONFIG_CMD_USB) - #define BOOT_TARGET_USB(func) func(USB, usb, 0) -#else - #define BOOT_TARGET_USB(func) -#endif - -#define BOOT_TARGET_DEVICES(func) \ - BOOT_TARGET_NVME(func) \ - BOOT_TARGET_USB(func) - -#include +#define BOOT_TARGETS "nvme usb" #define CFG_EXTRA_ENV_SETTINGS \ ENV_DEVICE_SETTINGS \ - BOOTENV + "boot_targets=" BOOT_TARGETS "\0" #endif -- 2.44.0
[PATCH 0/4] configs: apple: Switch to standard boot + small adjustments
This series contains a few misc config changes for Apple silicon systems: - switch from the deprecated distro boot scripts to standard boot - allows EFI console resizing based on the video console size - enables 16x32 bitmap fonts as Apple devices come with high DPI displays - enables 64-bit LBA addressing for USB storage devices larger than 2TB Signed-off-by: Janne Grunau --- Hector Martin (1): apple_m1_defconfig: Turn on CONFIG_SYS_64BIT_LBA Janne Grunau (3): configs: apple: Use "vidconsole,serial" as stdout/stderr configs: apple: Enable CMD_SELECT_FONT and FONT_16X32 arm: apple: Switch to standard boot arch/arm/Kconfig | 2 +- configs/apple_m1_defconfig | 3 +++ include/configs/apple.h| 24 3 files changed, 8 insertions(+), 21 deletions(-) --- base-commit: f3c979dd0053c082d2df170446923e7ce5edbc2d change-id: 20240317-apple_config-981fcd9028b9 Best regards, -- Janne Grunau
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
Hej,
On Sun, Mar 17, 2024, at 12:07, Janne Grunau via B4 Relay wrote:
> From: Janne Grunau
>
> Add the environment variable "usb_blocklist" to prevent USB devices
> listed in it from being used. This allows to ignore devices which
> trigger bugs in u-boot's USB stack or are undesirable for other reasons.
> Devices emulating keyboards are one example. U-boot currently supports
> only one USB keyboard device. Most commonly, people run into this with
> Yubikeys, so let's ignore those in the default environment.
>
> Based on previous USB keyboard specific patches for the same purpose.
>
> Link:
> https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
> Signed-off-by: Janne Grunau
> ---
> common/usb.c | 56
> +++
> doc/usage/environment.rst | 12 ++
> include/env_default.h | 11 ++
> 3 files changed, 79 insertions(+)
>
> diff --git a/common/usb.c b/common/usb.c
> index 836506dcd9..73af5be066 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device
> *dev, int addr, bool do_read,
> return 0;
> }
>
> +static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
> +{
> + printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
> +blocklist);
> + return 0;
> +}
> +
> +static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
> +{
> + ulong vid, pid;
> + char *end;
> + const char *block_str = env_get("usb_blocklist");
> + const char *cur = block_str;
> +
> + /* parse "usb_blocklist" strictly */
> + while (cur && cur[0] != '\0') {
> + vid = simple_strtoul(cur, , 0);
> + if (cur == end || end[0] != ':')
> + return usb_blocklist_parse_error(block_str,
> + cur - block_str);
> +
> + cur = end + 1;
> + pid = simple_strtoul(cur, , 0);
> + if (cur == end && end[0] != '*')
> + return usb_blocklist_parse_error(block_str,
> + cur - block_str);
> +
> + if (end[0] == '*') {
> + /* use out of range idProduct as wildcard indication */
> + pid = U16_MAX + 1;
> + end++;
> + }
> + if (end[0] != ',' && end[0] != '\0')
> + return usb_blocklist_parse_error(block_str,
> + cur - block_str);
> +
> + if (id_vendor == vid && (pid > U16_MAX || id_product == pid)) {
> + printf("Ignoring USB device 0x%x:0x%x\n",id_vendor,
> + id_product);
this is a leftover from testing, already removed locally
Janne
Re: [PATCH 3/3] boot: fdt: Move usable variable below updated comment
Hi Marek, Thank you for the patch. On Sun, Mar 17, 2024 at 07:16:31AM +0100, Marek Vasut wrote: > Move the variable below comment which explains what the variable means. > Update the comment. No functional change. > > Suggested-by: Laurent Pinchart > Signed-off-by: Marek Vasut > --- > Cc: Heinrich Schuchardt > Cc: Kuninori Morimoto > Cc: Laurent Pinchart > Cc: Simon Glass > Cc: Tom Rini > --- > boot/image-fdt.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/boot/image-fdt.c b/boot/image-fdt.c > index c37442c9130..16cd15e7e44 100644 > --- a/boot/image-fdt.c > +++ b/boot/image-fdt.c > @@ -218,12 +218,12 @@ int boot_relocate_fdt(struct lmb *lmb, char > **of_flat_tree, ulong *of_size) > if (start + size < low) > continue; > > - usable = min(start + size, low + mapsize); > - > /* >* At least part of this DRAM bank is usable, try > - * using it for LMB allocation. > + * using the the DRAM bank up to $usable address s/the the/the/ Is it a u-boot convention to write $variable to refer to a variable in comments ? Reviewed-by: Laurent Pinchart > + * limit for LMB allocation. >*/ > + usable = min(start + size, low + mapsize); > addr = lmb_alloc_base(lmb, of_len, 0x1000, usable); > of_start = map_sysmem(addr, of_len); > /* Allocation succeeded, use this block. */ -- Regards, Laurent Pinchart
Re: [PATCH v2 0/6] USB keyboard improvements for asahi / desktop systems
On Sun, Mar 17, 2024 at 4:07 AM Janne Grunau via B4 Relay wrote: > > Apple USB Keyboards from 2021 need quirks to be useable. The boot HID > keyboard protocol is unfortunately not described in the first interface > descriptor but the second. This needs several changes. The USB keyboard > driver has to look at all (2) interface descriptors during probing. > Since I didn't want to rebuild the USB driver probe code the Apple > keyboards are bound to the keyboard driver via USB vendor and product > IDs. > To make the keyboards useable on Apple silicon devices the xhci driver > needs to initializes rings for the endpoints of the first two interface > descriptors. If this is causes concerns regarding regressions or memory > use the USB_MAX_ACTIVE_INTERFACES define could be turned into a CONFIG > option. > Even after this changes the keyboards still do not probe successfully > since they apparently do not behave HID standard compliant. They only > generate reports on key events. This leads the final check whether the > keyboard is operational to fail unless the user presses keys during the > probe. Skip this check for known keyboards. > Keychron seems to emulate Apple keyboards (some models even "re-use" > Apple's USB vendor ID) so apply this quirk as well. > > Some devices like Yubikeys emulate a keyboard. since u-boot only binds a > single keyboard block this kind of devices from the USB keyboard driver. > > Signed-off-by: Janne Grunau > --- > Changes in v2: > - rewritten commit message for "[PATCH 2/6] usb: xhci: Set up endpoints for > the first 2 interfaces" > - Replaced the usb keyboard Yubikey block with an env based USB device > blocklist > - Use "-EINVAL" as return value in "[PATCH 3/6] usb: xhci: Abort transfers > with unallocated rings" > - added "Reviewed-by:" tags > - Link to v1: > https://lore.kernel.org/r/20240221-asahi-keyboards-v1-0-814b2e741...@jannau.net > > --- > Janne Grunau (6): > usb: xhci: refactor xhci_set_configuration > usb: xhci: Set up endpoints for the first 2 interfaces > usb: xhci: Abort transfers with unallocated rings > usb: Add environment based device blocklist > usb: kbd: support Apple Magic Keyboards (2021) > usb: kbd: Add probe quirk for Apple and Keychron keyboards > > common/usb.c | 56 +++ > common/usb_kbd.c | 61 +++-- > doc/usage/environment.rst| 12 + > drivers/usb/host/xhci-ring.c | 5 ++ > drivers/usb/host/xhci.c | 126 > +++ > include/env_default.h| 11 > include/usb.h| 6 +++ > 7 files changed, 227 insertions(+), 50 deletions(-) > --- > base-commit: 37345abb97ef0dd9c50a03b2a72617612dcae585 > change-id: 20240218-asahi-keyboards-f2ddaf0022b2 > Series LGTM. Thanks for this! :) Reviewed-by: Neal Gompa -- 真実はいつも一つ！/ Always, there's only one truth!
Re: [PATCH 2/3] boot: fdt: Drop lmb_alloc*() typecasts
Hi Marek,
Thank you for the patch.
On Sun, Mar 17, 2024 at 07:16:30AM +0100, Marek Vasut wrote:
> The lmb_alloc_base() returns phys_addr_t , map_sysmem() accepts
> phys_addr_t as first parameter. Declare $addr as phys_addr_t and
s/$addr/addr/ ?
> get rid of the casts.
>
> Reported-by: Laurent Pinchart
> Signed-off-by: Marek Vasut
> ---
> Cc: Heinrich Schuchardt
> Cc: Kuninori Morimoto
> Cc: Laurent Pinchart
> Cc: Simon Glass
> Cc: Tom Rini
> ---
> boot/image-fdt.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> index c2571b22244..c37442c9130 100644
> --- a/boot/image-fdt.c
> +++ b/boot/image-fdt.c
> @@ -162,6 +162,7 @@ int boot_relocate_fdt(struct lmb *lmb, char
> **of_flat_tree, ulong *of_size)
> void*of_start = NULL;
> phys_addr_t start, size, usable;
> char*fdt_high;
> + phys_addr_t addr;
> phys_addr_t low;
> phys_size_t mapsize;
> ulong of_len = 0;
> @@ -186,7 +187,6 @@ int boot_relocate_fdt(struct lmb *lmb, char
> **of_flat_tree, ulong *of_size)
> fdt_high = env_get("fdt_high");
> if (fdt_high) {
> ulong desired_addr = hextoul(fdt_high, NULL);
> - ulong addr;
I would keep addr declared here. With that,
Reviewed-by: Laurent Pinchart
>
> if (desired_addr == ~0UL) {
> /* All ones means use fdt in place */
> @@ -224,8 +224,8 @@ int boot_relocate_fdt(struct lmb *lmb, char
> **of_flat_tree, ulong *of_size)
>* At least part of this DRAM bank is usable, try
>* using it for LMB allocation.
>*/
> - of_start = map_sysmem((ulong)lmb_alloc_base(lmb,
> - of_len, 0x1000, usable), of_len);
> + addr = lmb_alloc_base(lmb, of_len, 0x1000, usable);
> + of_start = map_sysmem(addr, of_len);
> /* Allocation succeeded, use this block. */
> if (of_start != NULL)
> break;
--
Regards,
Laurent Pinchart
Re: [PATCH 1/3] boot: fdt: Change type of env_get_bootm_low() to phys_addr_t
Hi Marek,
Thank you for the patch.
On Sun, Mar 17, 2024 at 07:16:29AM +0100, Marek Vasut wrote:
> Change type of ulong env_get_bootm_low() to phys_addr_t env_get_bootm_low().
> The PPC/LS systems already treat env_get_bootm_low() result as phys_addr_t,
> while the function itself still returns ulong. This is potentially dangerous
> on 64bit systems, where ulong might not be large enough to hold the content
> of "bootm_low" environment variable. Fix it by using phys_addr_t, similar to
> what env_get_bootm_size() does, which returns phys_size_t .
>
> Reported-by: Laurent Pinchart
> Signed-off-by: Marek Vasut
> ---
> Cc: Heinrich Schuchardt
> Cc: Kuninori Morimoto
> Cc: Laurent Pinchart
> Cc: Simon Glass
> Cc: Tom Rini
> ---
> boot/bootm.c | 2 +-
> boot/image-board.c | 11 ++-
> boot/image-fdt.c | 9 +
> include/image.h| 2 +-
> 4 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/boot/bootm.c b/boot/bootm.c
> index d071537d692..2e818264f5f 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -242,7 +242,7 @@ static int boot_get_kernel(const char *addr_fit, struct
> bootm_headers *images,
> #ifdef CONFIG_LMB
> static void boot_start_lmb(struct bootm_headers *images)
> {
> - ulong mem_start;
> + phys_addr_t mem_start;
> phys_size_t mem_size;
>
> mem_start = env_get_bootm_low();
> diff --git a/boot/image-board.c b/boot/image-board.c
> index 75f6906cd56..447ced2678a 100644
> --- a/boot/image-board.c
> +++ b/boot/image-board.c
> @@ -107,12 +107,13 @@ static int on_loadaddr(const char *name, const char
> *value, enum env_op op,
> }
> U_BOOT_ENV_CALLBACK(loadaddr, on_loadaddr);
>
> -ulong env_get_bootm_low(void)
> +phys_addr_t env_get_bootm_low(void)
> {
> char *s = env_get("bootm_low");
> + phys_size_t tmp;
>
> if (s) {
> - ulong tmp = hextoul(s, NULL);
> + tmp = (phys_addr_t)simple_strtoull(s, NULL, 16);
> return tmp;
Can't you write
return (phys_addr_t)simple_strtoull(s, NULL, 16);
and avoid the temporary variable ?
Reviewed-by: Laurent Pinchart
> }
>
> @@ -538,7 +539,7 @@ int boot_ramdisk_high(struct lmb *lmb, ulong rd_data,
> ulong rd_len,
> ulong *initrd_start, ulong *initrd_end)
> {
> char*s;
> - ulong initrd_high;
> + phys_addr_t initrd_high;
> int initrd_copy_to_ram = 1;
>
> s = env_get("initrd_high");
> @@ -553,8 +554,8 @@ int boot_ramdisk_high(struct lmb *lmb, ulong rd_data,
> ulong rd_len,
> initrd_high = env_get_bootm_mapsize() + env_get_bootm_low();
> }
>
> - debug("## initrd_high = 0x%08lx, copy_to_ram = %d\n",
> - initrd_high, initrd_copy_to_ram);
> + debug("## initrd_high = 0x%p, copy_to_ram = %d\n",
> + (void *)(uintptr_t)initrd_high, initrd_copy_to_ram);
>
> if (rd_data) {
> if (!initrd_copy_to_ram) { /* zero-copy ramdisk support */
> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> index 5e4aa9de0d2..c2571b22244 100644
> --- a/boot/image-fdt.c
> +++ b/boot/image-fdt.c
> @@ -160,9 +160,10 @@ int boot_relocate_fdt(struct lmb *lmb, char
> **of_flat_tree, ulong *of_size)
> {
> void*fdt_blob = *of_flat_tree;
> void*of_start = NULL;
> - u64 start, size, usable;
> + phys_addr_t start, size, usable;
> char*fdt_high;
> - ulong mapsize, low;
> + phys_addr_t low;
> + phys_size_t mapsize;
> ulong of_len = 0;
> int bank;
> int err;
> @@ -217,7 +218,7 @@ int boot_relocate_fdt(struct lmb *lmb, char
> **of_flat_tree, ulong *of_size)
> if (start + size < low)
> continue;
>
> - usable = min(start + size, (u64)(low + mapsize));
> + usable = min(start + size, low + mapsize);
>
> /*
>* At least part of this DRAM bank is usable, try
> @@ -233,7 +234,7 @@ int boot_relocate_fdt(struct lmb *lmb, char
> **of_flat_tree, ulong *of_size)
>* Reduce the mapping size in the next bank
>* by the size of attempt in current bank.
>*/
> - mapsize -= usable - max(start, (u64)low);
> + mapsize -= usable - max(start, low);
> if (!mapsize)
> break;
> }
> diff --git a/include/image.h b/include/image.h
> index 21de70f0c9e..acffd17e0df 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -946,7 +946,7 @@ static inline void image_set_name(struct legacy_img_hdr
> *hdr, const char *name)
> int image_check_hcrc(const struct legacy_img_hdr *hdr);
> int image_check_dcrc(const struct legacy_img_hdr *hdr);
> #ifndef USE_HOSTCC
> -ulong env_get_bootm_low(void);
> +phys_addr_t env_get_bootm_low(void);
>
[PATCH v2 0/6] USB keyboard improvements for asahi / desktop systems
Apple USB Keyboards from 2021 need quirks to be useable. The boot HID keyboard protocol is unfortunately not described in the first interface descriptor but the second. This needs several changes. The USB keyboard driver has to look at all (2) interface descriptors during probing. Since I didn't want to rebuild the USB driver probe code the Apple keyboards are bound to the keyboard driver via USB vendor and product IDs. To make the keyboards useable on Apple silicon devices the xhci driver needs to initializes rings for the endpoints of the first two interface descriptors. If this is causes concerns regarding regressions or memory use the USB_MAX_ACTIVE_INTERFACES define could be turned into a CONFIG option. Even after this changes the keyboards still do not probe successfully since they apparently do not behave HID standard compliant. They only generate reports on key events. This leads the final check whether the keyboard is operational to fail unless the user presses keys during the probe. Skip this check for known keyboards. Keychron seems to emulate Apple keyboards (some models even "re-use" Apple's USB vendor ID) so apply this quirk as well. Some devices like Yubikeys emulate a keyboard. since u-boot only binds a single keyboard block this kind of devices from the USB keyboard driver. Signed-off-by: Janne Grunau --- Changes in v2: - rewritten commit message for "[PATCH 2/6] usb: xhci: Set up endpoints for the first 2 interfaces" - Replaced the usb keyboard Yubikey block with an env based USB device blocklist - Use "-EINVAL" as return value in "[PATCH 3/6] usb: xhci: Abort transfers with unallocated rings" - added "Reviewed-by:" tags - Link to v1: https://lore.kernel.org/r/20240221-asahi-keyboards-v1-0-814b2e741...@jannau.net --- Janne Grunau (6): usb: xhci: refactor xhci_set_configuration usb: xhci: Set up endpoints for the first 2 interfaces usb: xhci: Abort transfers with unallocated rings usb: Add environment based device blocklist usb: kbd: support Apple Magic Keyboards (2021) usb: kbd: Add probe quirk for Apple and Keychron keyboards common/usb.c | 56 +++ common/usb_kbd.c | 61 +++-- doc/usage/environment.rst| 12 + drivers/usb/host/xhci-ring.c | 5 ++ drivers/usb/host/xhci.c | 126 +++ include/env_default.h| 11 include/usb.h| 6 +++ 7 files changed, 227 insertions(+), 50 deletions(-) --- base-commit: 37345abb97ef0dd9c50a03b2a72617612dcae585 change-id: 20240218-asahi-keyboards-f2ddaf0022b2 Best regards, -- Janne Grunau
[PATCH v2 5/6] usb: kbd: support Apple Magic Keyboards (2021)
From: Janne Grunau
Apple USB keyboards (Magic Keyboard from 2021 (product id 0x029c)) carry
the HID keyboard boot protocol on the second interface descriptor.
Probe via vendor and product IDs since the class/subclass/protocol match
uses the first interface descriptor.
Probe the two first interface descriptors for the HID keyboard boot
protocol.
USB configuration descriptor for reference:
| Bus 003 Device 002: ID 05ac:029c Apple, Inc. Magic Keyboard
| Device Descriptor:
| bLength18
| bDescriptorType 1
| bcdUSB 2.00
| bDeviceClass0 [unknown]
| bDeviceSubClass 0 [unknown]
| bDeviceProtocol 0
| bMaxPacketSize064
| idVendor 0x05ac Apple, Inc.
| idProduct 0x029c Magic Keyboard
| bcdDevice3.90
| iManufacturer 1 Apple Inc.
| iProduct2 Magic Keyboard
| iSerial 3 ...
| bNumConfigurations 1
| Configuration Descriptor:
| bLength 9
| bDescriptorType 2
| wTotalLength 0x003b
| bNumInterfaces 2
| bConfigurationValue 1
| iConfiguration 4 Keyboard
| bmAttributes 0xa0
| (Bus Powered)
| Remote Wakeup
| MaxPower 500mA
| Interface Descriptor:
| bLength 9
| bDescriptorType 4
| bInterfaceNumber0
| bAlternateSetting 0
| bNumEndpoints 1
| bInterfaceClass 3 Human Interface Device
| bInterfaceSubClass 0 [unknown]
| bInterfaceProtocol 0
| iInterface 5 Device Management
| HID Device Descriptor:
| bLength 9
| bDescriptorType33
| bcdHID 1.10
| bCountryCode0 Not supported
| bNumDescriptors 1
| bDescriptorType34 Report
| wDescriptorLength 83
| Report Descriptors:
| ** UNAVAILABLE **
| Endpoint Descriptor:
| bLength 7
| bDescriptorType 5
| bEndpointAddress 0x81 EP 1 IN
| bmAttributes3
| Transfer TypeInterrupt
| Synch Type None
| Usage Type Data
| wMaxPacketSize 0x0010 1x 16 bytes
| bInterval 8
| Interface Descriptor:
| bLength 9
| bDescriptorType 4
| bInterfaceNumber1
| bAlternateSetting 0
| bNumEndpoints 1
| bInterfaceClass 3 Human Interface Device
| bInterfaceSubClass 1 Boot Interface Subclass
| bInterfaceProtocol 1 Keyboard
| iInterface 6 Keyboard / Boot
| HID Device Descriptor:
| bLength 9
| bDescriptorType33
| bcdHID 1.10
| bCountryCode 13 International (ISO)
| bNumDescriptors 1
| bDescriptorType34 Report
| wDescriptorLength 207
| Report Descriptors:
| ** UNAVAILABLE **
| Endpoint Descriptor:
| bLength 7
| bDescriptorType 5
| bEndpointAddress 0x82 EP 2 IN
| bmAttributes3
| Transfer TypeInterrupt
| Synch Type None
| Usage Type Data
| wMaxPacketSize 0x0010 1x 16 bytes
| bInterval 8
Signed-off-by: Janne Grunau
---
common/usb_kbd.c | 39 +++
1 file changed, 35 insertions(+), 4 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index 4cbc9acb73..8cc3345075 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -23,6 +23,14 @@
#include
+/*
+ * USB vendor and product IDs used for quirks.
+ */
+#define USB_VENDOR_ID_APPLE0x05ac
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_20210x029c
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_20210x029a
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_2021 0x029f
+
/*
* If overwrite_console returns 1, the stdin, stderr and stdout
* are switched to the serial port, else the settings in the
@@ -106,6 +114,8 @@ struct usb_kbd_pdata {
unsigned long last_report;
struct int_queue *intq;
+ uint32_tifnum;
+
uint32_trepeat_delay;
uint32_tusb_in_pointer;
@@ -150,8 +160,8 @@ static void usb_kbd_put_queue(struct usb_kbd_pdata *data,
u8 c)
*/
static void usb_kbd_setled(struct usb_device *dev)
{
- struct usb_interface *iface = >config.if_desc[0];
struct usb_kbd_pdata *data = dev->privptr;
+ struct usb_interface *iface = >config.if_desc[data->ifnum];
[PATCH v2 1/6] usb: xhci: refactor xhci_set_configuration
From: Janne Grunau
In the next step endpoints for multiple interfaces are set up. Move most
of the per endpoint initialization to separate function to avoid another
identation level.
Reviewed-by: Marek Vasut
Signed-off-by: Janne Grunau
---
drivers/usb/host/xhci.c | 119 +---
1 file changed, 73 insertions(+), 46 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d13cbff9b3..534c4b973f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -475,67 +475,34 @@ static int xhci_configure_endpoints(struct usb_device
*udev, bool ctx_change)
}
/**
- * Configure the endpoint, programming the device contexts.
+ * Fill endpoint contexts for interface descriptor ifdesc.
*
- * @param udev pointer to the USB device structure
- * Return: returns the status of the xhci_configure_endpoints
+ * @param udev pointer to the USB device structure
+ * @param ctrl pointer to the xhci pravte device structure
+ * @param virt_dev pointer to the xhci virtual device structure
+ * @param ifdesc pointer to the USB interface config descriptor
+ * Return: returns the status of xhci_init_ep_contexts_if
*/
-static int xhci_set_configuration(struct usb_device *udev)
+static int xhci_init_ep_contexts_if(struct usb_device *udev,
+ struct xhci_ctrl *ctrl,
+ struct xhci_virt_device *virt_dev,
+ struct usb_interface *ifdesc
+ )
{
- struct xhci_container_ctx *in_ctx;
- struct xhci_container_ctx *out_ctx;
- struct xhci_input_control_ctx *ctrl_ctx;
- struct xhci_slot_ctx *slot_ctx;
struct xhci_ep_ctx *ep_ctx[MAX_EP_CTX_NUM];
int cur_ep;
- int max_ep_flag = 0;
int ep_index;
unsigned int dir;
unsigned int ep_type;
- struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
- int num_of_ep;
- int ep_flag = 0;
u64 trb_64 = 0;
- int slot_id = udev->slot_id;
- struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
- struct usb_interface *ifdesc;
u32 max_esit_payload;
unsigned int interval;
unsigned int mult;
unsigned int max_burst;
unsigned int avg_trb_len;
unsigned int err_count = 0;
+ int num_of_ep = ifdesc->no_of_ep;
- out_ctx = virt_dev->out_ctx;
- in_ctx = virt_dev->in_ctx;
-
- num_of_ep = udev->config.if_desc[0].no_of_ep;
- ifdesc = >config.if_desc[0];
-
- ctrl_ctx = xhci_get_input_control_ctx(in_ctx);
- /* Initialize the input context control */
- ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG);
- ctrl_ctx->drop_flags = 0;
-
- /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
- for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
- ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
- ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
- if (max_ep_flag < ep_flag)
- max_ep_flag = ep_flag;
- }
-
- xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size);
-
- /* slot context */
- xhci_slot_copy(ctrl, in_ctx, out_ctx);
- slot_ctx = xhci_get_slot_ctx(ctrl, in_ctx);
- slot_ctx->dev_info &= ~(cpu_to_le32(LAST_CTX_MASK));
- slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(max_ep_flag + 1) | 0);
-
- xhci_endpoint_copy(ctrl, in_ctx, out_ctx, 0);
-
- /* filling up ep contexts */
for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
struct usb_endpoint_descriptor *endpt_desc = NULL;
struct usb_ss_ep_comp_descriptor *ss_ep_comp_desc = NULL;
@@ -561,7 +528,8 @@ static int xhci_set_configuration(struct usb_device *udev)
avg_trb_len = max_esit_payload;
ep_index = xhci_get_ep_index(endpt_desc);
- ep_ctx[ep_index] = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
+ ep_ctx[ep_index] = xhci_get_ep_ctx(ctrl, virt_dev->in_ctx,
+ ep_index);
/* Allocate the ep rings */
virt_dev->eps[ep_index].ring = xhci_ring_alloc(ctrl, 1, true);
@@ -614,6 +582,65 @@ static int xhci_set_configuration(struct usb_device *udev)
}
}
+ return 0;
+}
+
+/**
+ * Configure the endpoint, programming the device contexts.
+ *
+ * @param udev pointer to the USB device structure
+ * Return: returns the status of the xhci_configure_endpoints
+ */
+static int xhci_set_configuration(struct usb_device *udev)
+{
+ struct xhci_container_ctx *out_ctx;
+ struct xhci_container_ctx *in_ctx;
+ struct xhci_input_control_ctx *ctrl_ctx;
+ struct xhci_slot_ctx *slot_ctx;
+ int err;
+ int cur_ep;
+ int max_ep_flag = 0;
+ struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
+ int num_of_ep;
+ int ep_flag
[PATCH v2 6/6] usb: kbd: Add probe quirk for Apple and Keychron keyboards
From: Janne Grunau
Those keyboards do not return the current device state. Polling will
timeout unless there are key presses. This is not a problem during
operation but the inital device state query during probing will fail.
Skip this step in usb_kbd_probe_dev() to make these devices useable.
Not all Apple keyboards behave like this. A keyboard with USB
vendor/product ID 05ac:0221 is reported to work with the current code.
Unfortunately some Keychron keyboards "re-use" Apple's vendor ID and
show the same behavior (Keychron C2, 05ac:024f for example).
Reviewed-by: Marek Vasut
Signed-off-by: Janne Grunau
---
common/usb_kbd.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index 8cc3345075..43c7668671 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -31,6 +31,10 @@
#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_20210x029a
#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_2021 0x029f
+#define USB_VENDOR_ID_KEYCHRON 0x3434
+
+#define USB_HID_QUIRK_POLL_NO_REPORT_IDLE (1 << 0)
+
/*
* If overwrite_console returns 1, the stdin, stderr and stdout
* are switched to the serial port, else the settings in the
@@ -474,6 +478,7 @@ static int usb_kbd_probe_dev(struct usb_device *dev,
unsigned int ifnum)
struct usb_interface *iface;
struct usb_endpoint_descriptor *ep;
struct usb_kbd_pdata *data;
+ unsigned int quirks = 0;
int epNum;
if (dev->descriptor.bNumConfigurations != 1)
@@ -506,6 +511,15 @@ static int usb_kbd_probe_dev(struct usb_device *dev,
unsigned int ifnum)
debug("USB KBD: found interrupt EP: 0x%x\n", ep->bEndpointAddress);
+ switch (dev->descriptor.idVendor) {
+ case USB_VENDOR_ID_APPLE:
+ case USB_VENDOR_ID_KEYCHRON:
+ quirks |= USB_HID_QUIRK_POLL_NO_REPORT_IDLE;
+ break;
+ default:
+ break;
+ }
+
data = malloc(sizeof(struct usb_kbd_pdata));
if (!data) {
printf("USB KBD: Error allocating private data\n");
@@ -546,6 +560,14 @@ static int usb_kbd_probe_dev(struct usb_device *dev,
unsigned int ifnum)
usb_set_idle(dev, iface->desc.bInterfaceNumber, 0, 0);
#endif
+ /*
+* Apple and Keychron keyboards do not report the device state. Reports
+* are only returned during key presses.
+*/
+ if (quirks & USB_HID_QUIRK_POLL_NO_REPORT_IDLE) {
+ debug("USB KBD: quirk: skip testing device state\n");
+ return 1;
+ }
debug("USB KBD: enable interrupt pipe...\n");
#ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE
data->intq = create_int_queue(dev, data->intpipe, 1,
--
2.44.0
[PATCH v2 2/6] usb: xhci: Set up endpoints for the first 2 interfaces
From: Janne Grunau
The xhci driver currently only does the necessary initialization for
endpoints found in the first interface descriptor. Apple USB keyboards
(released 2021) use the second interface descriptor for the HID keyboard
boot protocol. To allow USB drivers to use endpoints from other
interface descriptors the xhci driver needs to ensure these endpoints
are initialized as well.
Use USB_MAX_ACTIVE_INTERFACES to control how many interface descriptors
are considered during endpoint initialisation.
For now define it to 2 as that is sufficient for supporting the Apple
keyboards.
Reviewed-by: Marek Vasut
Signed-off-by: Janne Grunau
---
drivers/usb/host/xhci.c | 31 +++
include/usb.h | 6 ++
2 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 534c4b973f..741e186ee0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -606,24 +606,28 @@ static int xhci_set_configuration(struct usb_device *udev)
int slot_id = udev->slot_id;
struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
struct usb_interface *ifdesc;
+ unsigned int ifnum;
+ unsigned int max_ifnum = min((unsigned int)USB_MAX_ACTIVE_INTERFACES,
+(unsigned int)udev->config.no_of_if);
out_ctx = virt_dev->out_ctx;
in_ctx = virt_dev->in_ctx;
- num_of_ep = udev->config.if_desc[0].no_of_ep;
- ifdesc = >config.if_desc[0];
-
ctrl_ctx = xhci_get_input_control_ctx(in_ctx);
/* Initialize the input context control */
ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG);
ctrl_ctx->drop_flags = 0;
- /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
- for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
- ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
- ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
- if (max_ep_flag < ep_flag)
- max_ep_flag = ep_flag;
+ for (ifnum = 0; ifnum < max_ifnum; ifnum++) {
+ ifdesc = >config.if_desc[ifnum];
+ num_of_ep = ifdesc->no_of_ep;
+ /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
+ for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
+ ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
+ ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
+ if (max_ep_flag < ep_flag)
+ max_ep_flag = ep_flag;
+ }
}
xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size);
@@ -637,9 +641,12 @@ static int xhci_set_configuration(struct usb_device *udev)
xhci_endpoint_copy(ctrl, in_ctx, out_ctx, 0);
/* filling up ep contexts */
- err = xhci_init_ep_contexts_if(udev, ctrl, virt_dev, ifdesc);
- if (err < 0)
- return err;
+ for (ifnum = 0; ifnum < max_ifnum; ifnum++) {
+ ifdesc = >config.if_desc[ifnum];
+ err = xhci_init_ep_contexts_if(udev, ctrl, virt_dev, ifdesc);
+ if (err < 0)
+ return err;
+ }
return xhci_configure_endpoints(udev, false);
}
diff --git a/include/usb.h b/include/usb.h
index 09e3f0cb30..3aafdc8bfd 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -49,6 +49,12 @@ extern bool usb_started; /* flag for the started/stopped USB
status */
*/
#define USB_TIMEOUT_MS(pipe) (usb_pipebulk(pipe) ? 5000 : 1000)
+/*
+ * The xhcd hcd driver prepares only a limited number interfaces / endpoints.
+ * Define this limit so that drivers do not exceed it.
+ */
+#define USB_MAX_ACTIVE_INTERFACES 2
+
/* device request (setup) */
struct devrequest {
__u8requesttype;
--
2.44.0
[PATCH v2 4/6] usb: Add environment based device blocklist
From: Janne Grunau
Add the environment variable "usb_blocklist" to prevent USB devices
listed in it from being used. This allows to ignore devices which
trigger bugs in u-boot's USB stack or are undesirable for other reasons.
Devices emulating keyboards are one example. U-boot currently supports
only one USB keyboard device. Most commonly, people run into this with
Yubikeys, so let's ignore those in the default environment.
Based on previous USB keyboard specific patches for the same purpose.
Link:
https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
Signed-off-by: Janne Grunau
---
common/usb.c | 56 +++
doc/usage/environment.rst | 12 ++
include/env_default.h | 11 ++
3 files changed, 79 insertions(+)
diff --git a/common/usb.c b/common/usb.c
index 836506dcd9..73af5be066 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device *dev,
int addr, bool do_read,
return 0;
}
+static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
+{
+ printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
+ blocklist);
+ return 0;
+}
+
+static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
+{
+ ulong vid, pid;
+ char *end;
+ const char *block_str = env_get("usb_blocklist");
+ const char *cur = block_str;
+
+ /* parse "usb_blocklist" strictly */
+ while (cur && cur[0] != '\0') {
+ vid = simple_strtoul(cur, , 0);
+ if (cur == end || end[0] != ':')
+ return usb_blocklist_parse_error(block_str,
+cur - block_str);
+
+ cur = end + 1;
+ pid = simple_strtoul(cur, , 0);
+ if (cur == end && end[0] != '*')
+ return usb_blocklist_parse_error(block_str,
+cur - block_str);
+
+ if (end[0] == '*') {
+ /* use out of range idProduct as wildcard indication */
+ pid = U16_MAX + 1;
+ end++;
+ }
+ if (end[0] != ',' && end[0] != '\0')
+ return usb_blocklist_parse_error(block_str,
+cur - block_str);
+
+ if (id_vendor == vid && (pid > U16_MAX || id_product == pid)) {
+ printf("Ignoring USB device 0x%x:0x%x\n",id_vendor,
+ id_product);
+ debug("Ignoring USB device 0x%x:0x%x\n",id_vendor,
+ id_product);
+ return 1;
+ }
+ if (end[0] == '\0')
+ break;
+ cur = end + 1;
+ }
+
+ return 0;
+}
+
int usb_select_config(struct usb_device *dev)
{
unsigned char *tmpbuf = NULL;
@@ -1099,6 +1150,11 @@ int usb_select_config(struct usb_device *dev)
le16_to_cpus(>descriptor.idProduct);
le16_to_cpus(>descriptor.bcdDevice);
+ /* ignore devices from usb_blocklist */
+ if (usb_device_is_blocked(dev->descriptor.idVendor,
+ dev->descriptor.idProduct))
+ return -ENODEV;
+
/*
* Kingston DT Ultimate 32GB USB 3.0 seems to be extremely sensitive
* about this first Get Descriptor request. If there are any other
diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
index ebf75fa948..e42702adb2 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -366,6 +366,18 @@ tftpwindowsize
This means the count of blocks we can receive before
sending ack to server.
+usb_blocklist
+Block USB devices from being bound to an USB device driver. This can be
used
+to ignore devices which causes crashes in u-boot's USB stack or are
+undesirable for other reasons.
+The default environment blocks Yubico devices since they emulate USB
+keyboards. U-boot currently supports only a single USB keyboard device and
+it is undesirable that a Yubikey is used as keyboard.
+Devices are matched by idVendor and idProduct. The variable contains a
comma
+separated list of idVendor:idProduct pairs as hexadecimal numbers joined
+by a colon. '*' functions as a wildcard for idProduct to block all devices
+with the specified idVendor.
+
vlan
When set to a value < 4095 the traffic over
Ethernet is encapsulated/received over 802.1q
diff --git a/include/env_default.h b/include/env_default.h
index 2ca4a087d3..ba4c7516b4 100644
--- a/include/env_default.h
+++ b/include/env_default.h
@@ -99,6 +99,17 @@ const char default_environment[] = {
#ifdef CONFIG_SYS_SOC
"soc=" CONFIG_SYS_SOC "\0"
#endif
+#ifdef
[PATCH v2 3/6] usb: xhci: Abort transfers with unallocated rings
From: Janne Grunau Discovered while trying to use the second interface in the USB keyboard driver necessary on Apple USB keyboards. Signed-off-by: Janne Grunau --- drivers/usb/host/xhci-ring.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index b60661fe05..910c5f3352 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -685,6 +685,9 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe, reset_ep(udev, ep_index); ring = virt_dev->eps[ep_index].ring; + if (!ring) + return -EINVAL; + /* * How much data is (potentially) left before the 64KB boundary? * XHCI Spec puts restriction( TABLE 49 and 6.4.1 section of XHCI Spec) @@ -871,6 +874,8 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe, ep_index = usb_pipe_ep_index(pipe); ep_ring = virt_dev->eps[ep_index].ring; + if (!ep_ring) + return -EINVAL; /* * Check to see if the max packet size for the default control -- 2.44.0
Re: [PATCH v3 7/7] efi_selftest: Update StrToFat() unit test after CP473 map extension
On 3/16/24 22:50, Janne Grunau via B4 Relay wrote:
From: Janne Grunau
Test that Unicode code points which map to CP437 code points 1-31 are
converted to '_'. This ensures no FAT file names do not contain chars
which are control characters in other code pages (CP 1250 for example).
Signed-off-by: Janne Grunau
---
lib/efi_selftest/efi_selftest_unicode_collation.c | 12
1 file changed, 12 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_unicode_collation.c
b/lib/efi_selftest/efi_selftest_unicode_collation.c
index 32c99caf35..ad7dfa9fb9 100644
--- a/lib/efi_selftest/efi_selftest_unicode_collation.c
+++ b/lib/efi_selftest/efi_selftest_unicode_collation.c
@@ -220,6 +220,18 @@ static int test_str_to_fat(void)
return EFI_ST_FAILURE;
}
+ /*
+* Test unicode code points which map to CP 437 0x01 - 0x1f are
+* converted to '_'.
+*/
+ boottime->set_mem(fat, 16, 0);
+ ret = unicode_collation_protocol->str_to_fat(unicode_collation_protocol,
+ u"\u263a\u2666\u2022\u25d8\u2642\u2194\u00b6\u203c", 8, fat);
+ if (!ret || efi_st_strcmp_16_8(u"", fat)) {
+ efi_st_error("str_to_fat returned %u, \"%s\"\n", ret, fat);
+ return EFI_ST_FAILURE;
+ }
+
Reviewed-by: Heinrich Schuchardt
return EFI_ST_SUCCESS;
}
Re: [PATCH v3 6/7] efi_selftest: Add geometric shapes character selftest
On 3/16/24 22:50, Janne Grunau via B4 Relay wrote:
From: Janne Grunau
Draw symbols from Unicode's "Geometric shapes" page which translate to
code page 437 code points 1-31. These are used by UEFI applications to
draw user interfaces using EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.
The output has to be checked manually on the screen for correctness.
Signed-off-by: Janne Grunau
---
lib/efi_selftest/efi_selftest_textoutput.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_textoutput.c
b/lib/efi_selftest/efi_selftest_textoutput.c
index b56fd2ab76..2208a9877a 100644
--- a/lib/efi_selftest/efi_selftest_textoutput.c
+++ b/lib/efi_selftest/efi_selftest_textoutput.c
@@ -60,6 +60,13 @@
u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2534"
u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
u"\u2500\u2500\u2500\u2500\u2518\n";
+ const u16 shapes[] =
+u"Geometric shapes as described\n"
+u"U+25B2 \u25B2 - Black up-pointing triangle\n"
+u"U+25BA \u25BA - Black right-pointing pointer\n"
+u"U+25BC \u25BC - Black down-pointing triangle\n"
+u"U+25C4 \u25C4 - Black left-pointing pointer\n";
Please, indent the strings by two tabs.
Otherwise
Reviewed-by: Heinrich Schuchardt
+
/* SetAttribute */
efi_st_printf("\nColor palette\n");
for (foreground = 0; foreground < 0x10; ++foreground) {
@@ -160,6 +167,12 @@ u"\u2500\u2500\u2500\u2500\u2518\n";
return EFI_ST_FAILURE;
}
efi_st_printf("\n");
+ ret = con_out->output_string(con_out, shapes);
+ if (ret != EFI_ST_SUCCESS) {
+ efi_st_error("OutputString failed for geometric shapes\n");
+ return EFI_ST_FAILURE;
+ }
+ efi_st_printf("\n");
return EFI_ST_SUCCESS;
}
Re: [PATCH v3 5/7] efi_selftest: Add box drawing character selftest
On 3/16/24 22:50, Janne Grunau via B4 Relay wrote:
From: Andre Przywara
UEFI applications rely on Unicode output capability, and might use that
for drawing pseudo-graphical interfaces using Unicode defined box
drawing characters.
Add a simple test to display the most basic box characters, which would
need to be checked manually on the screen for correctness.
Signed-off-by: Andre Przywara
Suggested-by: Heinrich Schuchardt
Signed-off-by: Janne Grunau
---
lib/efi_selftest/efi_selftest_textoutput.c | 20
1 file changed, 20 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_textoutput.c
b/lib/efi_selftest/efi_selftest_textoutput.c
index 917903473d..b56fd2ab76 100644
--- a/lib/efi_selftest/efi_selftest_textoutput.c
+++ b/lib/efi_selftest/efi_selftest_textoutput.c
@@ -46,6 +46,20 @@ u"U+03BB \u03BB - Greek small letter lambda\n"
u"U+03C2 \u03C2 - Greek small letter final sigma\n"
u"U+1F19 \u1F19 - Greek capital letter epsilon with dasia\n";
+ const u16 boxes[] =
+u"This should render as four boxes with text\n"
+u"\u250c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u252c\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2510\n\u2502"
+u" left top\u2502 right top \u2502\n\u251c\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u253c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2524\n\u2502 "
+u"left bottom \u2502 right bottom \u2502\n\u2514\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2534"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2518\n";
Please, indent the strings by two tabs.
Otherwise:
Reviewed-by: Heinrich Schuchardt
+
/* SetAttribute */
efi_st_printf("\nColor palette\n");
for (foreground = 0; foreground < 0x10; ++foreground) {
@@ -140,6 +154,12 @@ u"U+1F19 \u1F19 - Greek capital letter epsilon with
dasia\n";
return EFI_ST_FAILURE;
}
efi_st_printf("\n");
+ ret = con_out->output_string(con_out, boxes);
+ if (ret != EFI_ST_SUCCESS) {
+ efi_st_error("OutputString failed for box drawing chars\n");
+ return EFI_ST_FAILURE;
+ }
+ efi_st_printf("\n");
return EFI_ST_SUCCESS;
}
Re: [PATCH v3 1/7] lib: charset: Fix and extend utf8_to_utf32_stream() documentation
On 3/16/24 22:50, Janne Grunau via B4 Relay wrote: From: Janne Grunau Suggested-by: Heinrich Schuchardt Reviewed-by: Heinrich Schuchardt Signed-off-by: Janne Grunau This patch was already merged. No need to resend it. Best regards Heinrich --- include/charset.h | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/include/charset.h b/include/charset.h index 44034c71d3..f1050c903d 100644 --- a/include/charset.h +++ b/include/charset.h @@ -324,11 +324,21 @@ int utf_to_cp(s32 *c, const u16 *codepage); int utf8_to_cp437_stream(u8 c, char *buffer); /** - * utf8_to_utf32_stream() - convert UTF-8 stream to UTF-32 + * utf8_to_utf32_stream() - convert UTF-8 byte stream to Unicode code points + * + * The function is called for each byte @c in a UTF-8 stream. The byte is + * appended to the temporary storage @buffer until the UTF-8 stream in + * @buffer describes a Unicode code point. + * + * When a new code point has been decoded it is returned and buffer[0] is + * set to '\0', otherwise the return value is 0. + * + * The buffer must be at least 5 characters long. Before the first function + * invocation buffer[0] must be set to '\0'." * * @c:next UTF-8 character to convert * @buffer: buffer, at least 5 characters - * Return: next codepage 437 character or 0 + * Return: Unicode code point or 0 */ int utf8_to_utf32_stream(u8 c, char *buffer);
Re: [PATCH v3 4/7] efi_selftest: Add international characters test
On 3/16/24 22:50, Janne Grunau via B4 Relay wrote:
From: Andre Przywara
UEFI relies entirely on unicode output, which actual fonts displayed on
the screen might not be ready for.
Add a test displaying some international characters, to reveal missing
glyphs, especially in our builtin fonts.
This would be needed to be manually checked on the screen for
correctness.
Signed-off-by: Andre Przywara
Suggested-by: Heinrich Schuchardt
Signed-off-by: Janne Grunau
---
lib/efi_selftest/efi_selftest_textoutput.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_textoutput.c
b/lib/efi_selftest/efi_selftest_textoutput.c
index cc44b38bc2..917903473d 100644
--- a/lib/efi_selftest/efi_selftest_textoutput.c
+++ b/lib/efi_selftest/efi_selftest_textoutput.c
@@ -31,6 +31,21 @@ static int execute(void)
0xD804, 0xDC22,
0};
+ const u16 text[] =
+u"This should render international characters as described\n"
+u"U+00D6 \u00D6 - Latin capital letter O with diaresis\n"
+u"U+00DF \u00DF - Latin small letter sharp s\n"
+u"U+00E5 \u00E5 - Latin small letter a with ring above\n"
+u"U+00E9 \u00E9 - Latin small letter e with acute\n"
+u"U+00F1 \u00F1 - Latin small letter n with tilde\n"
+u"U+00F6 \u00F6 - Latin small letter o with diaresis\n"
+u"The following characters will render as '?' with bitmap fonts\n"
+u"U+00F8 \u00F8 - Latin small letter o with stroke\n"
+u"U+03AC \u03AC - Greek small letter alpha with tonus\n"
+u"U+03BB \u03BB - Greek small letter lambda\n"
+u"U+03C2 \u03C2 - Greek small letter final sigma\n"
+u"U+1F19 \u1F19 - Greek capital letter epsilon with dasia\n";
The strings should be indented by two tabs.
Otherwise:
Reviewed-by: Heinrich Schuchardt
+
/* SetAttribute */
efi_st_printf("\nColor palette\n");
for (foreground = 0; foreground < 0x10; ++foreground) {
@@ -119,6 +134,12 @@ static int execute(void)
return EFI_ST_FAILURE;
}
efi_st_printf("\n");
+ ret = con_out->output_string(con_out, text);
+ if (ret != EFI_ST_SUCCESS) {
+ efi_st_error("OutputString failed for international chars\n");
+ return EFI_ST_FAILURE;
+ }
+ efi_st_printf("\n");
return EFI_ST_SUCCESS;
}
Re: [PATCH 2/3] boot: fdt: Drop lmb_alloc*() typecasts
On 3/17/24 07:16, Marek Vasut wrote:
The lmb_alloc_base() returns phys_addr_t , map_sysmem() accepts
phys_addr_t as first parameter. Declare $addr as phys_addr_t and
%s/$addr/addr/
get rid of the casts.
Reported-by: Laurent Pinchart
Signed-off-by: Marek Vasut
---
Cc: Heinrich Schuchardt
Cc: Kuninori Morimoto
Cc: Laurent Pinchart
Cc: Simon Glass
Cc: Tom Rini
---
boot/image-fdt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index c2571b22244..c37442c9130 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -162,6 +162,7 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree,
ulong *of_size)
void*of_start = NULL;
phys_addr_t start, size, usable;
char*fdt_high;
+ phys_addr_t addr;
Please, keep variables in the narrowest scope where they are used.
Best regards
Heinrich
phys_addr_t low;
phys_size_t mapsize;
ulong of_len = 0;
@@ -186,7 +187,6 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree,
ulong *of_size)
fdt_high = env_get("fdt_high");
if (fdt_high) {
ulong desired_addr = hextoul(fdt_high, NULL);
- ulong addr;
if (desired_addr == ~0UL) {
/* All ones means use fdt in place */
@@ -224,8 +224,8 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree,
ulong *of_size)
* At least part of this DRAM bank is usable, try
* using it for LMB allocation.
*/
- of_start = map_sysmem((ulong)lmb_alloc_base(lmb,
- of_len, 0x1000, usable), of_len);
+ addr = lmb_alloc_base(lmb, of_len, 0x1000, usable);
+ of_start = map_sysmem(addr, of_len);
/* Allocation succeeded, use this block. */
if (of_start != NULL)
break;
Re: [PATCH 1/3] boot: fdt: Change type of env_get_bootm_low() to phys_addr_t
On 3/17/24 07:16, Marek Vasut wrote:
Change type of ulong env_get_bootm_low() to phys_addr_t env_get_bootm_low().
The PPC/LS systems already treat env_get_bootm_low() result as phys_addr_t,
while the function itself still returns ulong. This is potentially dangerous
on 64bit systems, where ulong might not be large enough to hold the content
Isn't long always 64bit when building with gcc or llvm?
of "bootm_low" environment variable. Fix it by using phys_addr_t, similar to
what env_get_bootm_size() does, which returns phys_size_t .
Reported-by: Laurent Pinchart
Signed-off-by: Marek Vasut
---
Cc: Heinrich Schuchardt
Cc: Kuninori Morimoto
Cc: Laurent Pinchart
Cc: Simon Glass
Cc: Tom Rini
---
boot/bootm.c | 2 +-
boot/image-board.c | 11 ++-
boot/image-fdt.c | 9 +
include/image.h| 2 +-
4 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/boot/bootm.c b/boot/bootm.c
index d071537d692..2e818264f5f 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -242,7 +242,7 @@ static int boot_get_kernel(const char *addr_fit, struct
bootm_headers *images,
#ifdef CONFIG_LMB
static void boot_start_lmb(struct bootm_headers *images)
{
- ulong mem_start;
+ phys_addr_t mem_start;
phys_size_t mem_size;
mem_start = env_get_bootm_low();
Please, remove the conversion to phys_addr_t in the
lmb_init_and_reserve_range() invocation.
diff --git a/boot/image-board.c b/boot/image-board.c
index 75f6906cd56..447ced2678a 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -107,12 +107,13 @@ static int on_loadaddr(const char *name, const char
*value, enum env_op op,
}
U_BOOT_ENV_CALLBACK(loadaddr, on_loadaddr);
-ulong env_get_bootm_low(void)
+phys_addr_t env_get_bootm_low(void)
{
char *s = env_get("bootm_low");
+ phys_size_t tmp;
if (s) {
- ulong tmp = hextoul(s, NULL);
+ tmp = (phys_addr_t)simple_strtoull(s, NULL, 16);
In C the conversion is superfluous. Please, remove '(phys_addr_t)'.
Why do we need tmp?
return simple_strtoull(s, NULL, 16);
return tmp;
}
@@ -538,7 +539,7 @@ int boot_ramdisk_high(struct lmb *lmb, ulong rd_data, ulong
rd_len,
ulong *initrd_start, ulong *initrd_end)
{
char*s;
- ulong initrd_high;
+ phys_addr_t initrd_high;
int initrd_copy_to_ram = 1;
s = env_get("initrd_high");
@@ -553,8 +554,8 @@ int boot_ramdisk_high(struct lmb *lmb, ulong rd_data, ulong
rd_len,
initrd_high = env_get_bootm_mapsize() + env_get_bootm_low();
}
- debug("## initrd_high = 0x%08lx, copy_to_ram = %d\n",
- initrd_high, initrd_copy_to_ram);
+ debug("## initrd_high = 0x%p, copy_to_ram = %d\n",
+ (void *)(uintptr_t)initrd_high, initrd_copy_to_ram);
Void * may be narrower than phys_addr_t. Shouldn't we convert to
unsigned long long for printing.
Best regards
Heinrich
if (rd_data) {
if (!initrd_copy_to_ram) { /* zero-copy ramdisk support */
diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index 5e4aa9de0d2..boot/image-fdt.c
@@ -160,9 +160,10 @@c2571b22244 100644
--- a/boot/image-fdt.c
+++ b/ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong
*of_size)
{
void*fdt_blob = *of_flat_tree;
void*of_start = NULL;
- u64 start, size, usable;
+ phys_addr_t start, size, usable;
char*fdt_high;
- ulong mapsize, low;
+ phys_addr_t low;
+ phys_size_t mapsize;
ulong of_len = 0;
int bank;
int err;
@@ -217,7 +218,7 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree,
ulong *of_size)
if (start + size < low)
continue;
- usable = min(start + size, (u64)(low + mapsize));
+ usable = min(start + size, low + mapsize);
/*
* At least part of this DRAM bank is usable, try
@@ -233,7 +234,7 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree,
ulong *of_size)
* Reduce the mapping size in the next bank
* by the size of attempt in current bank.
*/
- mapsize -= usable - max(start, (u64)low);
+ mapsize -= usable - max(start, low);
if (!mapsize)
break;
}
diff --git a/include/image.h b/include/image.h
index 21de70f0c9e..acffd17e0df 100644
--- a/include/image.h
+++ b/include/image.h
@@ -946,7 +946,7 @@ static inline void image_set_name(struct legacy_img_hdr
*hdr, const char *name)
int image_check_hcrc(const struct legacy_img_hdr *hdr);
int image_check_dcrc(const struct legacy_img_hdr *hdr);
#ifndef USE_HOSTCC
-ulong
[PATCH] ARM: dts: renesas: Stop using the -u-boot DTs for build
The U-Boot build system can automatically paste -u-boot.dtsi at the
end of matching .dts during build. Stop emulating this behavior and
rename the -u-boot.dts files to -u-boot.dtsi, drop "#include...dts"
from those new u-boot.dtsi files, and update board configuration
accordingly.
The rename, '#include...dts` scrubbing and configuration update has
been done using the following script:
```
$ find . -name r[78]\*-u-boot.dts | sort -u | while read line ; do \
git mv ${line%-u-boot.dts}-u-boot.dts ${line%-u-boot.dts}-u-boot.dtsi ; \
done
$ sed -i '/^#include.*dts"/ d' `find . -name r[78]\*-u-boot.dtsi`
$ sed -i 's@-u-boot@@g' `git grep -li renesas configs`
```
The Salvator-X and ULCB board files have been updated manually.
Signed-off-by: Marek Vasut
---
Cc: Adam Ford
Cc: Biju Das
Cc: Lad Prabhakar
Cc: Paul Barker
Cc: Ralph Siemsen
---
arch/arm/dts/Makefile | 58 +--
...boot.dts => r7s72100-gr-peach-u-boot.dtsi} | 1 -
dts => r8a774a1-hihope-rzg2m-u-boot.dtsi} | 1 -
dts => r8a774b1-hihope-rzg2n-u-boot.dtsi} | 1 -
...-u-boot.dts => r8a774c0-ek874-u-boot.dtsi} | 1 -
dts => r8a774e1-hihope-rzg2h-u-boot.dtsi} | 1 -
...r-u-boot.dts => r8a7790-lager-u-boot.dtsi} | 1 -
...t-u-boot.dts => r8a7790-stout-u-boot.dtsi} | 1 -
...u-boot.dts => r8a7791-koelsch-u-boot.dtsi} | 1 -
...-u-boot.dts => r8a7791-porter-u-boot.dtsi} | 1 -
...u-boot.dts => r8a7792-blanche-u-boot.dtsi} | 1 -
...se-u-boot.dts => r8a7793-gose-u-boot.dtsi} | 1 -
...alt-u-boot.dts => r8a7794-alt-u-boot.dtsi} | 1 -
...lk-u-boot.dts => r8a7794-silk-u-boot.dtsi} | 1 -
...ot.dts => r8a77950-salvator-x-u-boot.dtsi} | 1 -
...b-u-boot.dts => r8a77950-ulcb-u-boot.dtsi} | 1 -
...ot.dts => r8a77960-salvator-x-u-boot.dtsi} | 1 -
...b-u-boot.dts => r8a77960-ulcb-u-boot.dtsi} | 1 -
...ot.dts => r8a77965-salvator-x-u-boot.dtsi} | 1 -
...b-u-boot.dts => r8a77965-ulcb-u-boot.dtsi} | 1 -
...-u-boot.dts => r8a77970-eagle-u-boot.dtsi} | 1 -
...-u-boot.dts => r8a77970-v3msk-u-boot.dtsi} | 1 -
...u-boot.dts => r8a77980-condor-u-boot.dtsi} | 1 -
...-u-boot.dts => r8a77980-v3hsk-u-boot.dtsi} | 1 -
...-u-boot.dts => r8a77990-ebisu-u-boot.dtsi} | 1 -
...-u-boot.dts => r8a77995-draak-u-boot.dtsi} | 1 -
...u-boot.dts => r8a779a0-falcon-u-boot.dtsi} | 1 -
...u-boot.dts => r8a779f0-spider-u-boot.dtsi} | 1 -
...ot.dts => r8a779g0-white-hawk-u-boot.dtsi} | 1 -
...oot.dts => r8a779h0-gray-hawk-u-boot.dtsi} | 1 -
board/renesas/salvator-x/salvator-x.c | 6 +-
board/renesas/ulcb/ulcb.c | 6 +-
configs/alt_defconfig | 2 +-
configs/blanche_defconfig | 2 +-
configs/gose_defconfig| 2 +-
configs/grpeach_defconfig | 2 +-
configs/hihope_rzg2_defconfig | 4 +-
configs/koelsch_defconfig | 2 +-
configs/lager_defconfig | 2 +-
configs/porter_defconfig | 2 +-
configs/r8a77970_eagle_defconfig | 2 +-
configs/r8a77970_v3msk_defconfig | 2 +-
configs/r8a77980_condor_defconfig | 2 +-
configs/r8a77980_v3hsk_defconfig | 2 +-
configs/r8a77990_ebisu_defconfig | 2 +-
configs/r8a77995_draak_defconfig | 2 +-
configs/r8a779a0_falcon_defconfig | 2 +-
configs/r8a779f0_spider_defconfig | 2 +-
configs/r8a779g0_whitehawk_defconfig | 2 +-
configs/r8a779h0_grayhawk_defconfig | 2 +-
configs/rcar3_salvator-x_defconfig| 4 +-
configs/rcar3_ulcb_defconfig | 4 +-
configs/silinux_ek874_defconfig | 2 +-
configs/silk_defconfig| 2 +-
configs/stout_defconfig | 2 +-
55 files changed, 61 insertions(+), 90 deletions(-)
rename arch/arm/dts/{r7s72100-gr-peach-u-boot.dts =>
r7s72100-gr-peach-u-boot.dtsi} (97%)
rename arch/arm/dts/{r8a774a1-hihope-rzg2m-u-boot.dts =>
r8a774a1-hihope-rzg2m-u-boot.dtsi} (91%)
rename arch/arm/dts/{r8a774b1-hihope-rzg2n-u-boot.dts =>
r8a774b1-hihope-rzg2n-u-boot.dtsi} (91%)
rename arch/arm/dts/{r8a774c0-ek874-u-boot.dts => r8a774c0-ek874-u-boot.dtsi}
(95%)
rename arch/arm/dts/{r8a774e1-hihope-rzg2h-u-boot.dts =>
r8a774e1-hihope-rzg2h-u-boot.dtsi} (91%)
rename arch/arm/dts/{r8a7790-lager-u-boot.dts => r8a7790-lager-u-boot.dtsi}
(91%)
rename arch/arm/dts/{r8a7790-stout-u-boot.dts => r8a7790-stout-u-boot.dtsi}
(91%)
rename arch/arm/dts/{r8a7791-koelsch-u-boot.dts =>
r8a7791-koelsch-u-boot.dtsi} (90%)
rename arch/arm/dts/{r8a7791-porter-u-boot.dts => r8a7791-porter-u-boot.dtsi}
(92%)
rename arch/arm/dts/{r8a7792-blanche-u-boot.dts =>
r8a7792-blanche-u-boot.dtsi} (89%)
rename arch/arm/dts/{r8a7793-gose-u-boot.dts => r8a7793-gose-u-boot.dtsi} (91%)
rename arch/arm/dts/{r8a7794-alt-u-boot.dts => r8a7794-alt-u-boot.dtsi} (96%)
rename
[PATCH 3/3] boot: fdt: Move usable variable below updated comment
Move the variable below comment which explains what the variable means. Update the comment. No functional change. Suggested-by: Laurent Pinchart Signed-off-by: Marek Vasut --- Cc: Heinrich Schuchardt Cc: Kuninori Morimoto Cc: Laurent Pinchart Cc: Simon Glass Cc: Tom Rini --- boot/image-fdt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/boot/image-fdt.c b/boot/image-fdt.c index c37442c9130..16cd15e7e44 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -218,12 +218,12 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size) if (start + size < low) continue; - usable = min(start + size, low + mapsize); - /* * At least part of this DRAM bank is usable, try -* using it for LMB allocation. +* using the the DRAM bank up to $usable address +* limit for LMB allocation. */ + usable = min(start + size, low + mapsize); addr = lmb_alloc_base(lmb, of_len, 0x1000, usable); of_start = map_sysmem(addr, of_len); /* Allocation succeeded, use this block. */ -- 2.43.0
[PATCH 2/3] boot: fdt: Drop lmb_alloc*() typecasts
The lmb_alloc_base() returns phys_addr_t , map_sysmem() accepts
phys_addr_t as first parameter. Declare $addr as phys_addr_t and
get rid of the casts.
Reported-by: Laurent Pinchart
Signed-off-by: Marek Vasut
---
Cc: Heinrich Schuchardt
Cc: Kuninori Morimoto
Cc: Laurent Pinchart
Cc: Simon Glass
Cc: Tom Rini
---
boot/image-fdt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index c2571b22244..c37442c9130 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -162,6 +162,7 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree,
ulong *of_size)
void*of_start = NULL;
phys_addr_t start, size, usable;
char*fdt_high;
+ phys_addr_t addr;
phys_addr_t low;
phys_size_t mapsize;
ulong of_len = 0;
@@ -186,7 +187,6 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree,
ulong *of_size)
fdt_high = env_get("fdt_high");
if (fdt_high) {
ulong desired_addr = hextoul(fdt_high, NULL);
- ulong addr;
if (desired_addr == ~0UL) {
/* All ones means use fdt in place */
@@ -224,8 +224,8 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree,
ulong *of_size)
* At least part of this DRAM bank is usable, try
* using it for LMB allocation.
*/
- of_start = map_sysmem((ulong)lmb_alloc_base(lmb,
- of_len, 0x1000, usable), of_len);
+ addr = lmb_alloc_base(lmb, of_len, 0x1000, usable);
+ of_start = map_sysmem(addr, of_len);
/* Allocation succeeded, use this block. */
if (of_start != NULL)
break;
--
2.43.0
[PATCH 1/3] boot: fdt: Change type of env_get_bootm_low() to phys_addr_t
Change type of ulong env_get_bootm_low() to phys_addr_t env_get_bootm_low().
The PPC/LS systems already treat env_get_bootm_low() result as phys_addr_t,
while the function itself still returns ulong. This is potentially dangerous
on 64bit systems, where ulong might not be large enough to hold the content
of "bootm_low" environment variable. Fix it by using phys_addr_t, similar to
what env_get_bootm_size() does, which returns phys_size_t .
Reported-by: Laurent Pinchart
Signed-off-by: Marek Vasut
---
Cc: Heinrich Schuchardt
Cc: Kuninori Morimoto
Cc: Laurent Pinchart
Cc: Simon Glass
Cc: Tom Rini
---
boot/bootm.c | 2 +-
boot/image-board.c | 11 ++-
boot/image-fdt.c | 9 +
include/image.h| 2 +-
4 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/boot/bootm.c b/boot/bootm.c
index d071537d692..2e818264f5f 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -242,7 +242,7 @@ static int boot_get_kernel(const char *addr_fit, struct
bootm_headers *images,
#ifdef CONFIG_LMB
static void boot_start_lmb(struct bootm_headers *images)
{
- ulong mem_start;
+ phys_addr_t mem_start;
phys_size_t mem_size;
mem_start = env_get_bootm_low();
diff --git a/boot/image-board.c b/boot/image-board.c
index 75f6906cd56..447ced2678a 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -107,12 +107,13 @@ static int on_loadaddr(const char *name, const char
*value, enum env_op op,
}
U_BOOT_ENV_CALLBACK(loadaddr, on_loadaddr);
-ulong env_get_bootm_low(void)
+phys_addr_t env_get_bootm_low(void)
{
char *s = env_get("bootm_low");
+ phys_size_t tmp;
if (s) {
- ulong tmp = hextoul(s, NULL);
+ tmp = (phys_addr_t)simple_strtoull(s, NULL, 16);
return tmp;
}
@@ -538,7 +539,7 @@ int boot_ramdisk_high(struct lmb *lmb, ulong rd_data, ulong
rd_len,
ulong *initrd_start, ulong *initrd_end)
{
char*s;
- ulong initrd_high;
+ phys_addr_t initrd_high;
int initrd_copy_to_ram = 1;
s = env_get("initrd_high");
@@ -553,8 +554,8 @@ int boot_ramdisk_high(struct lmb *lmb, ulong rd_data, ulong
rd_len,
initrd_high = env_get_bootm_mapsize() + env_get_bootm_low();
}
- debug("## initrd_high = 0x%08lx, copy_to_ram = %d\n",
- initrd_high, initrd_copy_to_ram);
+ debug("## initrd_high = 0x%p, copy_to_ram = %d\n",
+ (void *)(uintptr_t)initrd_high, initrd_copy_to_ram);
if (rd_data) {
if (!initrd_copy_to_ram) { /* zero-copy ramdisk support */
diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index 5e4aa9de0d2..c2571b22244 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -160,9 +160,10 @@ int boot_relocate_fdt(struct lmb *lmb, char
**of_flat_tree, ulong *of_size)
{
void*fdt_blob = *of_flat_tree;
void*of_start = NULL;
- u64 start, size, usable;
+ phys_addr_t start, size, usable;
char*fdt_high;
- ulong mapsize, low;
+ phys_addr_t low;
+ phys_size_t mapsize;
ulong of_len = 0;
int bank;
int err;
@@ -217,7 +218,7 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree,
ulong *of_size)
if (start + size < low)
continue;
- usable = min(start + size, (u64)(low + mapsize));
+ usable = min(start + size, low + mapsize);
/*
* At least part of this DRAM bank is usable, try
@@ -233,7 +234,7 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree,
ulong *of_size)
* Reduce the mapping size in the next bank
* by the size of attempt in current bank.
*/
- mapsize -= usable - max(start, (u64)low);
+ mapsize -= usable - max(start, low);
if (!mapsize)
break;
}
diff --git a/include/image.h b/include/image.h
index 21de70f0c9e..acffd17e0df 100644
--- a/include/image.h
+++ b/include/image.h
@@ -946,7 +946,7 @@ static inline void image_set_name(struct legacy_img_hdr
*hdr, const char *name)
int image_check_hcrc(const struct legacy_img_hdr *hdr);
int image_check_dcrc(const struct legacy_img_hdr *hdr);
#ifndef USE_HOSTCC
-ulong env_get_bootm_low(void);
+phys_addr_t env_get_bootm_low(void);
phys_size_t env_get_bootm_size(void);
phys_size_t env_get_bootm_mapsize(void);
#endif
--
2.43.0
Re: [PATCH 1/3] mmc: Convert hs400_tuning flag from u8 to bool
On 2/20/24 3:41 PM, Paul Barker wrote: On 20/02/2024 11:27, Marek Vasut wrote: On 2/20/24 11:57, Paul Barker wrote: On 20/02/2024 08:37, Marek Vasut wrote: This hs400_tuning is a flag, make it bool. No functional change. This will be useful in the following patch, which adds another more generic flag, where the compiler can better use the space now reserved for the u8 to store more flags in it. The minimum size for a bool is one byte so there likely won't be any improvement in struct size from using bool instead of u8 for `hs400_tuning` here and `tuning` added in the next patch. I still think it's a good change to make though, bool is the right type for an on/off flag. The compiler does not do boolean packing in structures ? The compiler will only pack booleans if you explicitly say that only one bit of memory is needed, e.g.: bool tuning:1; bool hs400_tuning:1; Otherwise the assumption is that you may wish to take the address of each field and so each one must have a distinct address in memory. Fixed in V2, thanks !
