Re: [PATCH v2 4/6] efi_loader: support boot from URI device path
On Fri, 15 Sept 2023 at 15:32, Ilias Apalodimas wrote: > > Hi Kojima-san, > > > > > + efi_handle_t *image_handle) > > > > +{ > > > > + efi_status_t ret; > > > > + efi_handle_t bm_handle; > > > > + struct efi_handler *handler; > > > > + struct efi_device_path *file_path; > > > > + struct efi_device_path *device_path; > > > > + > > > > + if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&bm_handle)) { > > > > + log_warning("DM_TAG_EFI not found\n"); > > > > + return EFI_INVALID_PARAMETER; > > > > + } > > > > + > > > > + ret = efi_search_protocol(bm_handle, > > > > + &efi_simple_file_system_protocol_guid, > > > > &handler); > > > > + if (ret != EFI_SUCCESS) > > > > + return ret; > > > > + > > > > + ret = efi_search_protocol(bm_handle, &efi_guid_device_path, > > > > &handler); > > > > + if (ret != EFI_SUCCESS) > > > > + return ret; > > > > + > > > > + ret = efi_protocol_open(handler, (void **)&device_path, efi_root, > > > > NULL, > > > > + EFI_OPEN_PROTOCOL_GET_PROTOCOL); > > > > + if (ret != EFI_SUCCESS) > > > > + return ret; > > > > + > > > > + file_path = expand_media_path(device_path); > > > > + ret = EFI_CALL(efi_load_image(true, efi_root, file_path, NULL, 0, > > > > + image_handle)); > > > > + > > > > + efi_free_pool(file_path); > > > > + > > > > + return ret; > > > > +} > > > > > > We need to decide what we want here. There were recently some patches > > > from > > > Raymond [0] which piggybacked on your earlier eficonfig work. I think the > > > last patch of this series hasn't been merged due to a test failing, but we > > > should fix it. > > > That patch has a different approach. Everytime a disk appears it will add > > > a boot option if the default filepath is found and that's how we fixed the > > > > efi_bootmgr_update_media_device_boot_option() automatically adds the > > boot option if the device handle installed EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, > > but it does not check whether the default file path is found or not. > > When the user selects the automatically created boot option, > > expand_media_path() > > is called and efibootmgr tries to boot with the default file. > > > > Ah correct. On the function above though, we are we open coding a > different version of efi_open_protocol()? Can't we call that instead of > efi_search_protocol()/efi_protocol_open()? I will call efi_open_protocol() instead. Thanks. > > > > behaviour of efibootmgr to adhere to the EFI spec. This patch is doing > > > the > > > opposite, trying to detect the BOOTAA64.EFI etc on the fly. I think I > > > prefer the approach you have here, but we should end up with one solution > > > to solve both. > > > > This HTTP Boot case is slightly different from the case where the user > > selects > > the automatically added boot option. > > In this case, user selects the URI device path boot option. The > > efibootmgr downloads the > > file, mount the image, and try to boot with the default file name on the > > fly. > > When the patch[0] is merged, it is possible to boot the downloaded iso > > image from the > > automatically added "blkmap" boot option, but I think efibootmgr needs > > to boot with > > the URI device path user selected that this series does, not boot > > from the "blkmap" boot option. > > > > Indeed, the used must still select the 'http://' boot option. > It's been a while and I don't remember the eficonfig details. Do you > remember why we decided to store the load options after scanning back then? We just followed the EDK2 reference implementation. > IOW if we pick this up, can we also use it on the efibootmgr code and scan > all disks on the fly instead of adding boot options? Yes, it is possible, but I'm not sure scanning all the disks on the fly is a good idea. For users, it is hard to control which device is selected to boot. Now come to think of it, we add the one load option for each disk when the disk is detected, then try to boot with the default file by scanning the default boot file in the selected disk. In current implementation, the following load options are automatically created. Boot: attributes: A-- (0x0001) label: virtio 0:1 file_path: /HD(1,GPT,b6b38698-f211-4d78-b208-f68e5523e588,0x800,0x10) data: : c1 ac c1 38 c0 9f f0 41 b9 01 fa 74 d6 d6 e4 de ...8...A...t Boot0001: attributes: A-- (0x0001) label: virtio 0:2 file_path: /HD(2,GPT,98533ff2-11d5-428c-b323-9afaf6b4abd8,0x100800,0x1122000) data: : c1 ac c1 38 c0 9f f0 41 b9 01 fa 74 d6 d6 e4 de ...8...A...t For this disk, only the block device "virtio 0" load option is enough, we can search for the default boot file on the fly. This is what EDK2 does. Thanks, Masahisa Kojima > > > > > > > > + > > > > +/** > > > >
Re: [PATCH v5 10/21] boot: Rename Android-boot text
On jeu., sept. 14, 2023 at 10:55, Simon Glass wrote: > Phrases like 'Enable support for' are pointless since this is an option > which enables things. Drop that part so it is easier to follow. > > Signed-off-by: Simon Glass Reviewed-by: Mattijs Korpershoek > --- > > (no changes since v1) > > boot/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/boot/Kconfig b/boot/Kconfig > index f8b8d608951..017f7117d57 100644 > --- a/boot/Kconfig > +++ b/boot/Kconfig > @@ -3,7 +3,7 @@ menu "Boot options" > menu "Boot images" > > config ANDROID_BOOT_IMAGE > - bool "Enable support for Android Boot Images" > + bool "Android Boot Images" > default y if FASTBOOT > help > This enables support for booting images which use the Android > -- > 2.42.0.459.ge4e396fd5e-goog
Re: [PATCHv9 00/15] net/lwip: add lwip library for the network stack
Hi Maxim, On Thu, 14 Sept 2023 at 19:20, Maxim Uvarov wrote: > > changelog: > v9: - added first patch describing git submodule for lwip. So > the build procedure is: > git submodule init > git submodule update > make > - reworked a little bit dhcp cmd state polling > - fixed review comments for v8 > v8: - comments for previous review > - removed lwip timeout callback pointer > - made lwip timeouts works, that also allowed to remove > static vars. > - setenv for filesize tftp and wget has to be in hex. > - Makefile changes always compile it tftp,dns,wget,ping due > to it can be used not only by CONFIG_CMD_. > - Kconfig changes - simplify lwIP settings and support only > one configuration. > - tested with mini debian.iso load over http or tftp, mount > and boot it (qemu, arm64). > v7: - more review fixes. > - support of multiply eth devices, were "ethact" selects the > active device. > v6: - fixed review comments for v5 (thanks Ilias and Simon). > v5: - fixed Iliases comments and split big patch on the small > ones. > v4: - tested with tests/py/ did some minor fixes (out of tree > build, variables set after downloads). > - accounted review comments for documentation. > - implemented dns command > - corrected wget command to not use serverip variable and use just > url string. > v3: - use lwip commands for ping,tftp,wget,dhcp if this patch > applied. Drop CONFIG_LIB_LWIP_REPLACE_ option. > - docs: use rst variant and drop references to RFC. Something is wrong with the current implementation and I suspect it's partially due to callbacks. If you spawn a qemu with '-device virtio-net-pci,netdev=net0 -netdev tap,id=net0', assign ip addresses and try a tftp this initially works => tftp synquacer_fw.capsule eth0: virtio-net#33 52:52:52:52:52:52 active TFTP from server 10.10.10.2; our IP address is 10.10.10.1 Filename 'synquacer_fw.capsule'. Load address: 0x4020 Loading:### [...] However, if you do a ping first the output looks like this => ping 10.10.10.2 eth0: virtio-net#33 52:52:52:52:52:52 active Using virtio-net#33 device pinging addr: 10.10.10.2 host 10.10.10.2 a alive 1 ms => tftp synquacer_fw.capsule init already done for virtio-net#33 TFTP from server 10.10.10.2; our IP address is 10.10.10.1 Filename 'synquacer_fw.capsule'. Load address: 0x4020 Loading:ping failed; host �Hi> is not alive host 10.10.10.2 a alive 0 ms => Thanks /Ilias > > > Maxim Uvarov (15): > submodule: add lwIP as git submodule > net/lwip: add doc/develop/net_lwip.rst > net/lwip: integrate lwIP library > net/lwip: implement dns cmd > net/lwip: implement dhcp cmd > net/lwip: implement tftp cmd > net/lwip: implement wget cmd > net/lwip: implement ping cmd > net/lwip: add lwIP configuration > net/lwip: implement lwIP port to U-Boot > net/lwip: update .gitignore with lwIP > net/lwip: connection between cmd and lwip apps > net/lwip: replace original net commands with lwip > net/lwip: split net.h to net.h, arp.h and eth.h > net/lwip: drop old net/wget > > .gitmodules | 3 + > boot/bootmeth_efi.c | 18 +- > boot/bootmeth_pxe.c | 21 +- > cmd/Makefile | 1 + > cmd/net-lwip.c| 286 + > cmd/net.c | 86 + > cmd/pxe.c | 19 +- > doc/develop/index.rst | 1 + > doc/develop/net_lwip.rst | 75 + > include/net.h | 197 +--- > include/net/arp.h | 7 + > include/net/eth.h | 190 +++ > include/net/lwip.h| 73 + > include/net/ulwip.h | 64 > include/net/wget.h| 22 -- > net/Kconfig | 3 + > net/Makefile | 2 +- > net/eth-uclass.c | 8 + > net/lwip/.gitignore | 8 + > net/lwip/Kconfig | 25 ++ > net/lwip/Makefile | 70 > net/lwip/apps/dhcp/lwip-dhcp.c| 85 + > net/lwip/apps/dns/lwip-dns.c | 63 > net/lwip/apps/http/Makefile | 6 + > net/lwip/apps/http/lwip-wget.c| 105 ++ > net/lwip/apps/ping/Makefile | 12 + > net/lwip/apps/ping/lwip_ping.c| 38 +++ > net/lwip/apps/ping/lwip_ping.h| 15 + > net/lwip/apps/ping/ping.h | 19 ++ > net/lwip/apps/tftp/Makefile | 7 + > net/lwip/
Re: [PATCH v2 4/6] efi_loader: support boot from URI device path
Kojima-san [...] > > We just followed the EDK2 reference implementation. > > > IOW if we pick this up, can we also use it on the efibootmgr code and scan > > all disks on the fly instead of adding boot options? > > Yes, it is possible, but I'm not sure scanning all the disks on the fly > is a good idea. For users, it is hard to control which device is selected > to boot. > > Now come to think of it, we add the one load option for each disk when the > disk is detected, then try to boot with the default file by scanning > the default boot file > in the selected disk. > > In current implementation, the following load options are automatically > created. > > Boot: > attributes: A-- (0x0001) > label: virtio 0:1 > file_path: /HD(1,GPT,b6b38698-f211-4d78-b208-f68e5523e588,0x800,0x10) > data: > : c1 ac c1 38 c0 9f f0 41 b9 01 fa 74 d6 d6 e4 de > ...8...A...t > Boot0001: > attributes: A-- (0x0001) > label: virtio 0:2 > file_path: > /HD(2,GPT,98533ff2-11d5-428c-b323-9afaf6b4abd8,0x100800,0x1122000) > data: > : c1 ac c1 38 c0 9f f0 41 b9 01 fa 74 d6 d6 e4 de > ...8...A...t Yep, that's exactly what I was thinking as an alternative. On the automatically scan boot option, just search for '1234567' on the load options. Keep in mind that patch from Raymon is failing on an erofs selftest. But when I was looking at it, I came to the conclusion, that due to the EFI subsystem coming up earlier a print changes and that test gets utterly confused. IOW that patch seems fine, but you'll probably need to adjust a selftest as well. Thanks /Ilias > > For this disk, only the block device "virtio 0" load option is enough, > we can search > for the default boot file on the fly. This is what EDK2 does. > > Thanks, > Masahisa Kojima > > > > > > > > > > > > > > + > > > > > +/** > > > > > + * load_default_file_from_blk_dev() - load the default file > > > > > + * > > > > > + * @blk pointer to the UCLASS_BLK udevice > > > > > + * @handle: pointer to handle for newly installed image > > > > > + * Return: status code > > > > > + */ > > > > > +static efi_status_t load_default_file_from_blk_dev(struct udevice > > > > > *blk, > > > > > +efi_handle_t *handle) > > > > > +{ > > > > > + efi_status_t ret; > > > > > + struct udevice *partition; > > > > > + > > > > > + /* image that has no partition table but a file system */ > > > > > + ret = try_load_default_file(blk, handle); > > > > > + if (ret == EFI_SUCCESS) > > > > > + return ret; > > > > > + > > > > > + /* try the partitions */ > > > > > + device_foreach_child(partition, blk) { > > > > > + enum uclass_id id; > > > > > + > > > > > + id = device_get_uclass_id(partition); > > > > > + if (id != UCLASS_PARTITION) > > > > > + continue; > > > > > + > > > > > + ret = try_load_default_file(partition, handle); > > > > > + if (ret == EFI_SUCCESS) > > > > > + return ret; > > > > > + } > > > > > + > > > > > + return EFI_NOT_FOUND; > > > > > +} > > > > > + > > > > > +/** > > > > > + * try_load_from_uri_path() - Handle the URI device path > > > > > + * > > > > > + * @uridp: uri device path > > > > > + * @lo_label label of load option > > > > > + * @handle: pointer to handle for newly installed image > > > > > + * Return: status code > > > > > + */ > > > > > +static efi_status_t try_load_from_uri_path(struct > > > > > efi_device_path_uri *uridp, > > > > > +u16 *lo_label, > > > > > +efi_handle_t *handle) > > > > > +{ > > > > > + char *s; > > > > > + int uri_len; > > > > > + int image_size; > > > > > + efi_status_t ret; > > > > > + ulong image_addr; > > > > > + > > > > > + s = env_get("loadaddr"); > > > > > + if (!s) { > > > > > + log_err("Error: loadaddr is not set\n"); > > > > > + return EFI_INVALID_PARAMETER; > > > > > + } > > > > > + image_addr = hextoul(s, NULL); > > > > > + image_size = wget_with_dns(image_addr, uridp->uri); > > > > > + if (image_size < 0) > > > > > + return EFI_INVALID_PARAMETER; > > > > > + > > > > > + /* > > > > > + * If the file extension is ".iso" or ".img", mount it and try > > > > > to load > > > > > + * the default file. > > > > > > > > Don't we have a better way to validate isos and efi apps instead of > > > > the extension? The efi is checked against a valid PE/COFF image so I > > > > guess > > > > we'll really on the mount to fail for isos? > > > > > > EDK2 reference implementation checks the file type with the following > > > priority. > > > 1) "Content-Type" header in HTTP response message > > > 2) Filename Extensions > > > Documentation is here[1]. > > > > > > Since "Content-Type" is not available in the current U-
Re: [PATCHv9 00/15] net/lwip: add lwip library for the network stack
On Fri, 15 Sept 2023 at 14:07, Ilias Apalodimas wrote: > Hi Maxim, > > On Thu, 14 Sept 2023 at 19:20, Maxim Uvarov > wrote: > > > > changelog: > > v9: - added first patch describing git submodule for lwip. So > > the build procedure is: > > git submodule init > > git submodule update > > make > > - reworked a little bit dhcp cmd state polling > > - fixed review comments for v8 > > v8: - comments for previous review > > - removed lwip timeout callback pointer > > - made lwip timeouts works, that also allowed to remove > > static vars. > > - setenv for filesize tftp and wget has to be in hex. > > - Makefile changes always compile it tftp,dns,wget,ping due > > to it can be used not only by CONFIG_CMD_. > > - Kconfig changes - simplify lwIP settings and support only > > one configuration. > > - tested with mini debian.iso load over http or tftp, mount > > and boot it (qemu, arm64). > > v7: - more review fixes. > > - support of multiply eth devices, were "ethact" selects the > > active device. > > v6: - fixed review comments for v5 (thanks Ilias and Simon). > > v5: - fixed Iliases comments and split big patch on the small > > ones. > > v4: - tested with tests/py/ did some minor fixes (out of tree > > build, variables set after downloads). > > - accounted review comments for documentation. > > - implemented dns command > > - corrected wget command to not use serverip variable and > use just > > url string. > > v3: - use lwip commands for ping,tftp,wget,dhcp if this patch > > applied. Drop CONFIG_LIB_LWIP_REPLACE_ option. > > - docs: use rst variant and drop references to RFC. > > Something is wrong with the current implementation and I suspect it's > partially due to callbacks. > > If you spawn a qemu with '-device virtio-net-pci,netdev=net0 -netdev > tap,id=net0', assign ip addresses and try a tftp this initially works > => tftp synquacer_fw.capsule > eth0: virtio-net#33 52:52:52:52:52:52 active > TFTP from server 10.10.10.2; our IP address is 10.10.10.1 > Filename 'synquacer_fw.capsule'. > Load address: 0x4020 > Loading:### > [...] > > However, if you do a ping first the output looks like this > > => ping 10.10.10.2 > eth0: virtio-net#33 52:52:52:52:52:52 active > Using virtio-net#33 device > pinging addr: 10.10.10.2 > host 10.10.10.2 a alive > 1 ms > => tftp synquacer_fw.capsule > init already done for virtio-net#33 > TFTP from server 10.10.10.2; our IP address is 10.10.10.1 > Filename 'synquacer_fw.capsule'. > Load address: 0x4020 > Loading:ping failed; host �Hi> is not alive > host 10.10.10.2 a alive > 0 ms > => > > Thanks > /Ilias > Thanks, I will get it fixed. > > > > > > Maxim Uvarov (15): > > submodule: add lwIP as git submodule > > net/lwip: add doc/develop/net_lwip.rst > > net/lwip: integrate lwIP library > > net/lwip: implement dns cmd > > net/lwip: implement dhcp cmd > > net/lwip: implement tftp cmd > > net/lwip: implement wget cmd > > net/lwip: implement ping cmd > > net/lwip: add lwIP configuration > > net/lwip: implement lwIP port to U-Boot > > net/lwip: update .gitignore with lwIP > > net/lwip: connection between cmd and lwip apps > > net/lwip: replace original net commands with lwip > > net/lwip: split net.h to net.h, arp.h and eth.h > > net/lwip: drop old net/wget > > > > .gitmodules | 3 + > > boot/bootmeth_efi.c | 18 +- > > boot/bootmeth_pxe.c | 21 +- > > cmd/Makefile | 1 + > > cmd/net-lwip.c| 286 + > > cmd/net.c | 86 + > > cmd/pxe.c | 19 +- > > doc/develop/index.rst | 1 + > > doc/develop/net_lwip.rst | 75 + > > include/net.h | 197 +--- > > include/net/arp.h | 7 + > > include/net/eth.h | 190 +++ > > include/net/lwip.h| 73 + > > include/net/ulwip.h | 64 > > include/net/wget.h| 22 -- > > net/Kconfig | 3 + > > net/Makefile | 2 +- > > net/eth-uclass.c | 8 + > > net/lwip/.gitignore | 8 + > > net/lwip/Kconfig | 25 ++ > > net/lwip/Makefile | 70 > > net/lwip/apps/dhcp/lwip-dhcp.c| 85 + > > net/lwip/apps/dns/lwip-dns.c | 63 > > net/lwip/apps/http/Makefile
[PATCH] arm64: zynqmp: Corrected pcap_prog register address
From: Lukas Funke Currently the pcap_prog struct variable is pointing to 0x3004 which is incorrect according to [1]. The variable should point to 0x3000. [1] https://www.xilinx.com/htmldocs/registers/ug1087/ug1087-zynq-ultrascale-registers.html#csu___pcap_prog.html Signed-off-by: Lukas Funke --- arch/arm/mach-zynqmp/include/mach/hardware.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h b/arch/arm/mach-zynqmp/include/mach/hardware.h index 634bf169c6..8cb6494e52 100644 --- a/arch/arm/mach-zynqmp/include/mach/hardware.h +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h @@ -166,7 +166,7 @@ struct csu_regs { u32 jtag_dap_cfg; u32 idcode; u32 version; - u32 reserved2[3055]; + u32 reserved2[3054]; u32 pcap_prog; }; -- 2.30.2
Re: [RFC PATCH 0/5] Allow for removal of DT nodes and properties
+CC Jose who's maintaining the metadata spec from Arm side. On Fri, 15 Sept 2023 at 02:38, Tom Rini wrote: > > On Thu, Sep 14, 2023 at 04:41:43PM -0600, Simon Glass wrote: > > Hi Rob, > > > > On Wed, 13 Sept 2023 at 16:39, Rob Herring wrote: > [snip] > > > I don't think we should decide what to do here based on said > > > certification program. It can adapt to what's decided. Probably, the > > > /option nodes will be stripped out or ignored for certification. > > > > > > I accepted u-boot options node schema into dtschema, but now have > > > second thoughts on that. Now I'm getting more u-boot specific > > > (perhaps, not clear) stuff and widevine stuff internal to a TEE. > > > > Where should these bindings go such that ARM / Linaro are not trying > > to remove them? I would be OK with moving them out somewhere else, but > > how are people supposed to deal with such fragmentation? My > > understanding was that dt-schema was an attempt to set up a neutral > > area where bindings could be accepted that were not just for > > Linux...did that change? > > Well, part of the problem here is that I've been talking with Ilias more > about what's intended here and the fwu-* stuff that Rob rejected is > indeed not right. We should drop it and replace it with something that > really addresses the underlying problem (which is how do you know > how/where to find some GUIDs) and I think we think it's something that > can be shared between projects too and so be easier to convince Rob that > the next form of it is right (or the right direction). Tom, I gave the discussion we had last night a bit of thought (thanks for that) and talked with Jose. The spec as is written right now recommends a GPT-based partition. It then has various GPT UUIDs that indicate where to find the metadata as well as the various firmware banks that participate in the whole scheme. If you have that GPT, then the only thing you really need is to find the metadata UUID and handle the various banks. However, the synquacer which boots from a NOR has no GPT. What that fwu node entry does, in order to abstract the rest of the implementation, is map UUIIDs <-> offset + length. That way the core code still thinks it's trying to discover UUIDs but the eventual reads/writes end up on a NOR. That box boots via SCP [0] , which has no understanding of device trees, instead they hardcode the UUIDS, offsets etc for the Synquacer. But if they ever wanted to switch over having that information would come in handy. Jose and I discussed this a bit. I'll backpaddle and have someone look into upstreaming the fwu part. There are a few changes required, since we already have some MTD information in the DT and those entries would make more sense there instead of inventing a new node, but we can discuss this when the patches are sent. Once those are sent, we can add a recommendation on the spec, pointing to that DT entry for any future early stage boot loaders that want to implement it, pointing to the DT entry. That only solves a fraction of the problem though. Other nodes, like the EFI public key etc. still need to be removed. [0] https://github.com/ARM-software/SCP-firmware Thanks /Ilias > > -- > Tom
[PATCH] include: env: ti: mmc: Add 'ti' directory prefix for overlays
DTSOs are stored in vendor specific directories in Linux. Add the vendor prefix to the path to load the overlay correctly. Signed-off-by: Ravi Gunasekaran --- include/env/ti/mmc.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/env/ti/mmc.env b/include/env/ti/mmc.env index 0256a2d2aa..3e6b0287a3 100644 --- a/include/env/ti/mmc.env +++ b/include/env/ti/mmc.env @@ -70,7 +70,7 @@ get_overlay_mmc= fdt resize 0x10; for overlay in $name_overlays; do; - load mmc ${bootpart} ${dtboaddr} ${bootdir}/dtb/${overlay} && + load mmc ${bootpart} ${dtboaddr} ${bootdir}/dtb/ti/${overlay} && fdt apply ${dtboaddr}; done; get_kern_mmc=load mmc ${bootpart} ${loadaddr} base-commit: 2fe4b54556ea6271237b35de68dc458bfceab94c -- 2.17.1
[PATCH v3 0/7] Add EFI HTTP boot support
This series adds the EFI HTTP boot support. User can add the URI device path with "efidebug boot add" command. efibootmgr handles the URI device path, download the specified file using wget, mount the downloaded image with blkmap, then boot with the default file(e.g. EFI/BOOT/BOOTAA64.EFI). This version still does not include the test. To enable EFI HTTP boot, we need to enable the following Kconfig options. CONFIG_CMD_DNS CONFIG_CMD_WGET CONFIG_BLKMAP On the Socionext Developerbox, enter the following commands then debian installer is downloaded into "loadaddr" and installer automatically starts. => dhcp => setenv serverip 192.168.1.1 => efidebug boot add -u 3 debian-netinst http://ftp.riken.jp/Linux/debian/debian-cd/12.1.0/arm64/iso-cd/debian-12.1.0-arm64-netinst.iso => efidebug boot order 3 => bootefi bootmgr Note that this debian installer can not proceed the installation bacause RAM disk of installer image is not recogniged by the kernel. I'm still investigating this issue, but drivers/nvdimm/of_pmem.c in linux will be one of the solution to recognize RAM disk from kernel. (In EDK2, the equivalent solution is called ACPI NFIT.) On QEMU, I can not make DNS work from the QEMU guest. The following commands work on qemu_arm64(manually set the http server ip in URI). => dhcp => setenv gatewayip 10.0.2.2 => setenv httpserverip 134.160.38.1 => efidebug boot add -u 3 debian-netinst http://134.160.38.1/Linux/debian/debian-cd/12.1.0/arm64/iso-cd/debian-12.1.0-arm64-netinst.iso => efidebug boot order 3 => bootefi bootmgr [TODO] - add test - stricter wget uri check - omit the dns process if the given uri has ip address -> this will be supported when the lwip migration completes - uri device path support in eficonfig [change log] v2 -> v3 - Patch#6 is added, reserve the whole ramdisk memory region - remove .efi file extension check for PE-COFF image - use "if IS_ENABLED(..)" as much as possible - 1024 should be sizeof(net_boot_file_name) - call net_set_state(NETLOOP_FAIL) when wget encounters error - describe DNS ip address host name limitation in document v1 -> v2 - carve out the network handling(wget and dns code) under net/wget.c - carve out ramdisk creation code under drivers/block/blkmap_helper.c - wget supports the valid range check to store the received blocks using lmb - support when the downloaded image have no partiton table but a file system - not start the .efi file in try_load_entry() - call efi_check_pe() for .efi file to check the file is PE-COFF image - add documentation for EFI HTTP Boot Masahisa Kojima (7): net: wget: prevent overwriting reserved memory net: wget: add wget with dns utility function blk: blkmap: add ramdisk creation utility function efi_loader: support boot from URI device path efi_loader: set EFI HTTP Boot download buffer as reserved cmd: efidebug: add uri device path doc: uefi: add HTTP Boot support cmd/efidebug.c| 50 + doc/develop/uefi/uefi.rst | 30 + drivers/block/Makefile| 1 + drivers/block/blkmap.c| 15 --- drivers/block/blkmap_helper.c | 53 + include/blkmap.h | 29 + include/efi_loader.h | 2 + include/net.h | 3 + lib/efi_loader/efi_bootmgr.c | 194 lib/efi_loader/efi_dt_fixup.c | 2 +- net/wget.c| 206 -- 11 files changed, 562 insertions(+), 23 deletions(-) create mode 100644 drivers/block/blkmap_helper.c base-commit: 9f201d9bf3dd895d1d5e3a57b562e4f107ed5118 prerequisite-patch-id: 386e2563ffafe7f80fda169076d2835ea071b3f9 -- 2.34.1
[PATCH v3 1/7] net: wget: prevent overwriting reserved memory
This introduces the valid range check to store the received blocks using lmb. The same logic is implemented in tftp. Signed-off-by: Masahisa Kojima --- net/wget.c | 80 +- 1 file changed, 73 insertions(+), 7 deletions(-) diff --git a/net/wget.c b/net/wget.c index 2dbfeb1a1d..9690e4539c 100644 --- a/net/wget.c +++ b/net/wget.c @@ -4,16 +4,20 @@ * Copyright Duncan Hare 2017 */ +#include #include #include #include #include #include +#include #include #include #include #include +DECLARE_GLOBAL_DATA_PTR; + static const char bootfile1[] = "GET "; static const char bootfile3[] = " HTTP/1.0\r\n\r\n"; static const char http_eom[] = "\r\n\r\n"; @@ -55,6 +59,29 @@ static unsigned int retry_tcp_ack_num; /* TCP retry acknowledge number*/ static unsigned int retry_tcp_seq_num; /* TCP retry sequence number */ static int retry_len; /* TCP retry length */ +static ulong wget_load_size; + +/** + * wget_init_max_size() - initialize maximum load size + * + * Return: 0 if success, -1 if fails + */ +static int wget_init_load_size(void) +{ + struct lmb lmb; + phys_size_t max_size; + + lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob); + + max_size = lmb_get_free_size(&lmb, image_load_addr); + if (!max_size) + return -1; + + wget_load_size = max_size; + + return 0; +} + /** * store_block() - store block in memory * @src: source of data @@ -63,10 +90,25 @@ static int retry_len; /* TCP retry length */ */ static inline int store_block(uchar *src, unsigned int offset, unsigned int len) { + ulong store_addr = image_load_addr + offset; ulong newsize = offset + len; uchar *ptr; - ptr = map_sysmem(image_load_addr + offset, len); + if (IS_ENABLED(CONFIG_LMB)) { + ulong end_addr = image_load_addr + wget_load_size; + + if (!end_addr) + end_addr = ULONG_MAX; + + if (store_addr < image_load_addr || + store_addr + len > end_addr) { + puts("\nwget error: "); + puts("trying to overwrite reserved memory...\n"); + return -1; + } + } + + ptr = map_sysmem(store_addr, len); memcpy(ptr, src, len); unmap_sysmem(ptr); @@ -240,25 +282,39 @@ static void wget_connected(uchar *pkt, unsigned int tcp_seq_num, net_boot_file_size = 0; - if (len > hlen) - store_block(pkt + hlen, 0, len - hlen); + if (len > hlen) { + if (store_block(pkt + hlen, 0, len - hlen) != 0) { + wget_loop_state = NETLOOP_FAIL; + wget_fail("wget: store error\n", tcp_seq_num, tcp_ack_num, action); + net_set_state(NETLOOP_FAIL); + return; + } + } debug_cond(DEBUG_WGET, "wget: Connected Pkt %p hlen %x\n", pkt, hlen); for (i = 0; i < pkt_q_idx; i++) { + int err; + ptr1 = map_sysmem( (phys_addr_t)(pkt_q[i].pkt), pkt_q[i].len); - store_block(ptr1, - pkt_q[i].tcp_seq_num - - initial_data_seq_num, - pkt_q[i].len); + err = store_block(ptr1, + pkt_q[i].tcp_seq_num - + initial_data_seq_num, + pkt_q[i].len); unmap_sysmem(ptr1); debug_cond(DEBUG_WGET, "wget: Connctd pkt Q %p len %x\n", pkt_q[i].pkt, pkt_q[i].len); + if (err) { + wget_loop_state = NETLOOP_FAIL; + wget_fail("wget: store error\n", tcp_seq_num, tcp_ack_num, action); + net_set_state(NETLOOP_FAIL); + return; + } } } } @@ -330,6 +386,7 @@ static void wget_handler(uchar *pkt, u16 dport, len) != 0) { wget_fail("wget: store error\n",
[PATCH v3 2/7] net: wget: add wget with dns utility function
Current wget takes the target uri in this format: ":" e.g.) 192.168.1.1:/bar The http server ip address must be resolved before calling wget. This commit adds the utility function runs wget with dhs. User can call wget with the uri like "http://foo/bar";. Signed-off-by: Masahisa Kojima --- include/net.h | 2 ++ net/wget.c| 54 +++ 2 files changed, 56 insertions(+) diff --git a/include/net.h b/include/net.h index e254df7d7f..b9cba5244a 100644 --- a/include/net.h +++ b/include/net.h @@ -926,4 +926,6 @@ void eth_set_enable_bootdevs(bool enable); static inline void eth_set_enable_bootdevs(bool enable) {} #endif +int wget_with_dns(ulong dst_addr, char *uri); + #endif /* __NET_H__ */ diff --git a/net/wget.c b/net/wget.c index 9690e4539c..bdf940a315 100644 --- a/net/wget.c +++ b/net/wget.c @@ -15,6 +15,7 @@ #include #include #include +#include DECLARE_GLOBAL_DATA_PTR; @@ -504,3 +505,56 @@ void wget_start(void) wget_send(TCP_SYN, 0, 0, 0); } + +#if (IS_ENABLED(CONFIG_CMD_DNS)) +int wget_with_dns(ulong dst_addr, char *uri) +{ + int ret; + char *s, *host_name, *file_name, *str_copy; + + /* +* Download file using wget. +* +* U-Boot wget takes the target uri in this format. +* ":" e.g.) 192.168.1.1:/sample/test.iso +* Need to resolve the http server ip address before starting wget. +*/ + str_copy = strdup(uri); + if (!str_copy) + return -ENOMEM; + + s = str_copy + strlen("http://";); + host_name = strsep(&s, "/"); + if (!s) { + log_err("Error: invalied uri, no file path\n"); + ret = -EINVAL; + goto out; + } + file_name = s; + + /* TODO: If the given uri has ip address for the http server, skip dns */ + net_dns_resolve = host_name; + net_dns_env_var = "httpserverip"; + if (net_loop(DNS) < 0) { + log_err("Error: dns lookup of %s failed, check setup\n", net_dns_resolve); + ret = -EINVAL; + goto out; + } + s = env_get("httpserverip"); + if (!s) { + ret = -EINVAL; + goto out; + } + + strlcpy(net_boot_file_name, s, sizeof(net_boot_file_name)); + strlcat(net_boot_file_name, ":/", sizeof(net_boot_file_name)); /* append '/' which is removed by strsep() */ + strlcat(net_boot_file_name, file_name, sizeof(net_boot_file_name)); + image_load_addr = dst_addr; + ret = net_loop(WGET); + +out: + free(str_copy); + + return ret; +} +#endif -- 2.34.1
[PATCH v3 3/7] blk: blkmap: add ramdisk creation utility function
User needs to call several functions to create the ramdisk with blkmap. This adds the utility function to create blkmap device and mount the ramdisk. Signed-off-by: Masahisa Kojima Reviewed-by: Simon Glass --- drivers/block/Makefile| 1 + drivers/block/blkmap.c| 15 -- drivers/block/blkmap_helper.c | 53 +++ include/blkmap.h | 29 +++ 4 files changed, 83 insertions(+), 15 deletions(-) create mode 100644 drivers/block/blkmap_helper.c diff --git a/drivers/block/Makefile b/drivers/block/Makefile index a161d145fd..c3ccfc03e5 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -15,6 +15,7 @@ endif obj-$(CONFIG_SANDBOX) += sandbox.o host-uclass.o host_dev.o obj-$(CONFIG_$(SPL_TPL_)BLOCK_CACHE) += blkcache.o obj-$(CONFIG_BLKMAP) += blkmap.o +obj-$(CONFIG_BLKMAP) += blkmap_helper.o obj-$(CONFIG_EFI_MEDIA) += efi-media-uclass.o obj-$(CONFIG_EFI_MEDIA_SANDBOX) += sb_efi_media.o diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c index 2bb0acc20f..4e95997f61 100644 --- a/drivers/block/blkmap.c +++ b/drivers/block/blkmap.c @@ -66,21 +66,6 @@ struct blkmap_slice { void (*destroy)(struct blkmap *bm, struct blkmap_slice *bms); }; -/** - * struct blkmap - Block map - * - * Data associated with a blkmap. - * - * @label: Human readable name of this blkmap - * @blk: Underlying block device - * @slices: List of slices associated with this blkmap - */ -struct blkmap { - char *label; - struct udevice *blk; - struct list_head slices; -}; - static bool blkmap_slice_contains(struct blkmap_slice *bms, lbaint_t blknr) { return (blknr >= bms->blknr) && (blknr < (bms->blknr + bms->blkcnt)); diff --git a/drivers/block/blkmap_helper.c b/drivers/block/blkmap_helper.c new file mode 100644 index 00..0f80035f57 --- /dev/null +++ b/drivers/block/blkmap_helper.c @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * blkmap helper function + * + * Copyright (c) 2023, Linaro Limited + */ + +#include +#include +#include +#include + +int blkmap_create_ramdisk(const char *label, ulong image_addr, int image_size, + struct udevice **devp) +{ + int ret; + lbaint_t blknum; + struct blkmap *bm; + struct blk_desc *desc; + struct udevice *bm_dev; + + ret = blkmap_create(label, &bm_dev); + if (ret) { + log_err("failed to create blkmap\n"); + return ret; + } + + bm = dev_get_plat(bm_dev); + desc = dev_get_uclass_plat(bm->blk); + blknum = image_size >> desc->log2blksz; + ret = blkmap_map_pmem(bm_dev, 0, blknum, image_addr); + if (ret) { + log_err("Unable to map %#llx at block %d : %d\n", + (unsigned long long)image_addr, 0, ret); + goto err; + } + log_info("Block %d+0x" LBAF " mapped to %#llx\n", 0, blknum, +(unsigned long long)image_addr); + + ret = device_probe(bm->blk); + if (ret) + goto err; + + if (devp) + *devp = bm_dev; + + return 0; + +err: + blkmap_destroy(bm_dev); + + return ret; +} diff --git a/include/blkmap.h b/include/blkmap.h index af54583c7d..0d87e6db6b 100644 --- a/include/blkmap.h +++ b/include/blkmap.h @@ -7,6 +7,23 @@ #ifndef _BLKMAP_H #define _BLKMAP_H +#include + +/** + * struct blkmap - Block map + * + * Data associated with a blkmap. + * + * @label: Human readable name of this blkmap + * @blk: Underlying block device + * @slices: List of slices associated with this blkmap + */ +struct blkmap { + char *label; + struct udevice *blk; + struct list_head slices; +}; + /** * blkmap_map_linear() - Map region of other block device * @@ -74,4 +91,16 @@ int blkmap_create(const char *label, struct udevice **devp); */ int blkmap_destroy(struct udevice *dev); +/** + * blkmap_create_ramdisk() - Create new ramdisk with blkmap + * + * @label: Label of the new blkmap + * @image_addr: Target memory start address of this mapping + * @image_size: Target memory size of this mapping + * @devp: Updated with the address of the created blkmap device + * Returns: 0 on success, negative error code on failure + */ +int blkmap_create_ramdisk(const char *label, ulong image_addr, int image_size, + struct udevice **devp); + #endif /* _BLKMAP_H */ -- 2.34.1
[PATCH v3 4/7] efi_loader: support boot from URI device path
This supports to boot from the URI device path. When user selects the URI device path, bootmgr downloads the file using wget into the address specified by loadaddr env variable. If the file is .iso or .img file, mount the image with blkmap then try to boot with the default file(e.g. EFI/BOOT/BOOTAA64.EFI). If the file is .efi file, load and start the downloaded file. Signed-off-by: Masahisa Kojima --- lib/efi_loader/efi_bootmgr.c | 189 +++ 1 file changed, 189 insertions(+) diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a40762c74c..1a287ad89d 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -7,10 +7,14 @@ #define LOG_CATEGORY LOGC_EFI +#include +#include #include #include +#include #include #include +#include #include #include #include @@ -168,6 +172,182 @@ out: return ret; } +/** + * mount_image() - mount the image with blkmap + * + * @lo_label u16 label string of load option + * @image_addr:image address + * @image_size image size + * Return: pointer to the UCLASS_BLK udevice, NULL if failed + */ +static struct udevice *mount_image(u16 *lo_label, ulong image_addr, int image_size) +{ + int err; + struct blkmap *bm; + struct udevice *bm_dev; + char *label = NULL, *p; + + label = efi_alloc(utf16_utf8_strlen(lo_label) + 1); + if (!label) + return NULL; + + p = label; + utf16_utf8_strcpy(&p, lo_label); + err = blkmap_create_ramdisk(label, image_addr, image_size, &bm_dev); + if (err) { + efi_free_pool(label); + return NULL; + } + bm = dev_get_plat(bm_dev); + + efi_free_pool(label); + + return bm->blk; +} + +/** + * try_load_default_file() - try to load the default file + * + * Search the device having EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, + * then try to load with the default boot file(e.g. EFI/BOOT/BOOTAA64.EFI). + * + * @devpointer to the UCLASS_BLK or UCLASS_PARTITION udevice + * @image_handle: pointer to handle for newly installed image + * Return: status code + */ +static efi_status_t try_load_default_file(struct udevice *dev, + efi_handle_t *image_handle) +{ + efi_status_t ret; + efi_handle_t bm_handle; + struct efi_handler *handler; + struct efi_device_path *file_path; + struct efi_device_path *device_path; + + if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&bm_handle)) { + log_warning("DM_TAG_EFI not found\n"); + return EFI_INVALID_PARAMETER; + } + + ret = efi_search_protocol(bm_handle, + &efi_simple_file_system_protocol_guid, &handler); + if (ret != EFI_SUCCESS) + return ret; + + ret = EFI_CALL(bs->open_protocol(bm_handle, &efi_guid_device_path, +(void **)&device_path, efi_root, NULL, +EFI_OPEN_PROTOCOL_GET_PROTOCOL)); + if (ret != EFI_SUCCESS) + return ret; + + file_path = expand_media_path(device_path); + ret = EFI_CALL(efi_load_image(true, efi_root, file_path, NULL, 0, + image_handle)); + + efi_free_pool(file_path); + + return ret; +} + +/** + * load_default_file_from_blk_dev() - load the default file + * + * @blkpointer to the UCLASS_BLK udevice + * @handle:pointer to handle for newly installed image + * Return: status code + */ +static efi_status_t load_default_file_from_blk_dev(struct udevice *blk, + efi_handle_t *handle) +{ + efi_status_t ret; + struct udevice *partition; + + /* image that has no partition table but a file system */ + ret = try_load_default_file(blk, handle); + if (ret == EFI_SUCCESS) + return ret; + + /* try the partitions */ + device_foreach_child(partition, blk) { + enum uclass_id id; + + id = device_get_uclass_id(partition); + if (id != UCLASS_PARTITION) + continue; + + ret = try_load_default_file(partition, handle); + if (ret == EFI_SUCCESS) + return ret; + } + + return EFI_NOT_FOUND; +} + +/** + * try_load_from_uri_path() - Handle the URI device path + * + * @uridp: uri device path + * @lo_label label of load option + * @handle:pointer to handle for newly installed image + * Return: status code + */ +static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp, + u16 *lo_label, + efi_handle_t *handle) +{ + char *s; + int uri_len; + int image_size; + efi_st
[PATCH v3 5/7] efi_loader: set EFI HTTP Boot download buffer as reserved
The buffer used to download the ISO image file must be reserved to avoid the unintended access to the image. For PE-COFF file case, this memory reservation is done in LoadImage Boot Service. Signed-off-by: Masahisa Kojima --- include/efi_loader.h | 2 ++ lib/efi_loader/efi_bootmgr.c | 5 + lib/efi_loader/efi_dt_fixup.c | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/include/efi_loader.h b/include/efi_loader.h index 4a29ddaef4..c4207edc91 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -554,6 +554,8 @@ void efi_runtime_detach(void); /* efi_convert_pointer() - convert pointer to virtual address */ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition, void **address); +/* add reserved memory to memory map */ +void efi_reserve_memory(u64 addr, u64 size, bool nomap); /* Carve out DT reserved memory ranges */ void efi_carve_out_dt_rsv(void *fdt); /* Purge unused kaslr-seed */ diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 1a287ad89d..d26c607815 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -326,6 +326,11 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp, return EFI_INVALID_PARAMETER; ret = load_default_file_from_blk_dev(blk, handle); + if (ret != EFI_SUCCESS) + return ret; + + /* whole ramdisk must be reserved */ + efi_reserve_memory(image_addr, image_size, true); } else if (efi_check_pe((void *)image_addr, image_size, NULL) == EFI_SUCCESS) { efi_handle_t mem_handle = NULL; struct efi_device_path *file_path = NULL; diff --git a/lib/efi_loader/efi_dt_fixup.c b/lib/efi_loader/efi_dt_fixup.c index 838023c78f..edc515b9ff 100644 --- a/lib/efi_loader/efi_dt_fixup.c +++ b/lib/efi_loader/efi_dt_fixup.c @@ -22,7 +22,7 @@ const efi_guid_t efi_guid_dt_fixup_protocol = EFI_DT_FIXUP_PROTOCOL_GUID; * @nomap: indicates that the memory range shall not be accessed by the * UEFI payload */ -static void efi_reserve_memory(u64 addr, u64 size, bool nomap) +void efi_reserve_memory(u64 addr, u64 size, bool nomap) { int type; efi_uintn_t ret; -- 2.34.1
[PATCH v3 6/7] cmd: efidebug: add uri device path
This adds the URI device path option for 'boot add' subcommand. User can add the URI load option for downloading ISO image file or EFI application through network. Currently HTTP is only supported. Signed-off-by: Masahisa Kojima --- cmd/efidebug.c | 50 +++ include/net.h | 1 + net/wget.c | 72 ++ 3 files changed, 123 insertions(+) diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 0be3af3e76..f2fd6ba71d 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -829,6 +830,52 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, argc -= 1; argv += 1; break; +#if (IS_ENABLED(CONFIG_BLKMAP) && IS_ENABLED(CONFIG_CMD_WGET) && IS_ENABLED(CONFIG_CMD_DNS)) + case 'u': + { + char *pos; + int uridp_len; + struct efi_device_path_uri *uridp; + + if (argc < 3 || lo.label) { + r = CMD_RET_USAGE; + goto out; + } + id = (int)hextoul(argv[1], &endp); + if (*endp != '\0' || id > 0x) + return CMD_RET_USAGE; + + efi_create_indexed_name(var_name16, sizeof(var_name16), + "Boot", id); + + label = efi_convert_string(argv[2]); + if (!label) + return CMD_RET_FAILURE; + lo.label = label; + + uridp_len = sizeof(struct efi_device_path) + strlen(argv[3]) + 1; + fp_free = efi_alloc(uridp_len + sizeof(END)); + uridp = (struct efi_device_path_uri *)fp_free; + uridp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; + uridp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_URI; + uridp->dp.length = uridp_len; + if (!wget_validate_uri(argv[3])) { + printf("ERROR: invalid URI\n"); + r = CMD_RET_FAILURE; + goto out; + } + + strcpy(uridp->uri, argv[3]); + pos = (char *)uridp + uridp_len; + memcpy(pos, &END, sizeof(END)); + fp_size += uridp_len + sizeof(END); + file_path = (struct efi_device_path *)uridp; + argc -= 3; + argv += 3; + break; + } +#endif + default: r = CMD_RET_USAGE; goto out; @@ -1492,6 +1539,9 @@ static char efidebug_help_text[] = " -b|-B[:] \n" " -i|-I [:] \n" " (-b, -i for short form device path)\n" +#if (IS_ENABLED(CONFIG_BLKMAP) && IS_ENABLED(CONFIG_CMD_WGET) && IS_ENABLED(CONFIG_CMD_DNS)) + " -u \n" +#endif " -s ''\n" "efidebug boot rm [ [ [...]]]\n" " - delete UEFI Boot variables\n" diff --git a/include/net.h b/include/net.h index b9cba5244a..4b42672070 100644 --- a/include/net.h +++ b/include/net.h @@ -927,5 +927,6 @@ static inline void eth_set_enable_bootdevs(bool enable) {} #endif int wget_with_dns(ulong dst_addr, char *uri); +bool wget_validate_uri(char *uri); #endif /* __NET_H__ */ diff --git a/net/wget.c b/net/wget.c index bdf940a315..0bc0bf33bb 100644 --- a/net/wget.c +++ b/net/wget.c @@ -558,3 +558,75 @@ out: return ret; } #endif + +/** + * wget_validate_uri() - validate the uri for wget + * + * @uri: uri string + * Return: true on success, false on failure + */ +bool wget_validate_uri(char *uri) +{ + char c; + bool ret = true; + char *str_copy, *s, *authority; + + /* TODO: strict uri conformance check */ + + /* +* Uri is expected to be correctly percent encoded. +* This is the minimum check, control codes(0x1-0x19, 0x7F, except '\0') +* and space character(0x20) are not allowed. +*/ + for (c = 0x1; c < 0x21; c++) { + if (strchr(uri, c)) { + printf("invalid character is used\n"); + return false; + } + } + if (strchr(uri, 0x7f)) { + printf("invalid character is used\n"); + return false; + } + + /* +* This follows the current U-Boot wget implementation. +* scheme: only "http:" is supported +* authority: +* - user information: not supported +* - host: supported +* - port: not supported(always
[PATCH v3 7/7] doc: uefi: add HTTP Boot support
This adds the description about HTTP Boot. Signed-off-by: Masahisa Kojima --- doc/develop/uefi/uefi.rst | 30 ++ 1 file changed, 30 insertions(+) diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index a7a41f2fac..65eea89265 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -594,6 +594,36 @@ UEFI variables. Booting according to these variables is possible via:: As of U-Boot v2020.10 UEFI variables cannot be set at runtime. The U-Boot command 'efidebug' can be used to set the variables. +UEFI HTTP Boot +~~ + +HTTP Boot provides the capability for system deployment and configuration +over the network. HTTP Boot can be activated by specifying:: + +CONFIG_CMD_DNS +CONFIG_CMD_WGET +CONFIG_BLKMAP + +Set up the load option specifying the target URI:: + +efidebug boot add -u 1 netinst http://foo/bar + +When this load option is selected as boot selection, resolve the +host ip address by dns, then download the file with wget. +If the downloaded file extension is .iso or .img file, efibootmgr tries to +mount the image and boot with the default file(e.g. EFI/BOOT/BOOTAA64.EFI). +If the downloaded file is PE-COFF image, load the downloaded file and +start it. + +There is a limitation that current implementation tries to resolve +the IP address as a host name. If the uri is like "http://192.168.1.1/foobar";, +the dns process tries to resolve the host "192.168.1.1" and it will +end up with "host not found". + +We need to preset the "httpserverip" environment variable to proceed the wget:: + +setenv httpserverip 192.168.1.1 + Executing the built in hello world application ~~ -- 2.34.1
Re: [PATCH v2] global: Use proper project name U-Boot (next2)
On 9/8/23 09:11, Michal Simek wrote: Use proper project name in README, rst and comment. Done in connection to commit bb922ca3eb4b ("global: Use proper project name U-Boot (next)"). Signed-off-by: Michal Simek --- Changes in v2: - Revert change in tools/binman/entries.rst reported by Simon board/cobra5272/README | 18 +- board/emulation/qemu-ppce500/qemu-ppce500.c | 2 +- doc/board/xilinx/zynq.rst | 2 +- doc/board/xilinx/zynqmp-r5.rst | 4 ++-- doc/imx/mkimage/imximage.txt| 2 +- doc/usage/environment.rst | 2 +- doc/usage/semihosting.rst | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/board/cobra5272/README b/board/cobra5272/README index 11abcfacdb6e..0b07148b73a3 100644 --- a/board/cobra5272/README +++ b/board/cobra5272/README @@ -1,6 +1,6 @@ File: README.COBRA5272 Author: Florian Schlote for Sentec elektronik (li...@sentec-elektronik.de) -Contents: This is the README of u-boot (Universal bootloader) for our +Contents: This is the README of U-Boot (Universal bootloader) for our COBRA5272 board. Version: v01.00 Date: Tue Mar 30 00:28:33 CEST 2004 @@ -31,7 +31,7 @@ Please refer to u-boot README (general info, u-boot-x-x-x/README), to u-boot-x-x-x/board/cobra5272/README and to the comments in u-boot-x-x-x/include/configs/cobra5272.h -Configuring u-boot is done by commenting/uncommenting preprocessor defines. +Configuring U-Boot is done by commenting/uncommenting preprocessor defines. Default configuration is @@ -48,10 +48,10 @@ Default configuration is #--- -# u-boot FLASH version & RAM version +# U-Boot FLASH version & RAM version #--- -The u-boot bootloader for Coldfire processors can be configured +The U-Boot bootloader for Coldfire processors can be configured 1. as a standalone bootloader residing in flash & relocating itself to RAM on startup automatically => "FLASH version" @@ -60,7 +60,7 @@ The u-boot bootloader for Coldfire processors can be configured prestage bootloader ("chainloading") & is running only from the RAM address it is linked to => "RAM version" - This version may be very helpful when installing u-boot for the first time + This version may be very helpful when installing U-Boot for the first time since it can be used to make available s. th. like a "bootstrap mechanism". @@ -71,7 +71,7 @@ How to build the different images: Flash version -- -Compile u-boot +Compile U-Boot in dir ./u-boot-x-x-x/ @@ -81,14 +81,14 @@ please first check: CONFIG_MONITOR_IS_IN_RAM has to be not present in the file - => u-boot as single bootloader starting from flash + => U-Boot as single bootloader starting from flash in configs/cobra5272_defconfig CONFIG_TEXT_BASE should be CONFIG_TEXT_BASE=0xffe0 - => linking address for u-boot as single bootloader stored in flash + => linking address for U-Boot as single bootloader stored in flash then: @@ -116,7 +116,7 @@ please modify the settings: CONFIG_MONITOR_IS_IN_RAM=y - => u-boot as RAM version, chainloaded by another bootloader or using bdm cable + => U-Boot as RAM version, chainloaded by another bootloader or using bdm cable in configs/cobra5272_defconfig CONFIG_TEXT_BASE should be diff --git a/board/emulation/qemu-ppce500/qemu-ppce500.c b/board/emulation/qemu-ppce500/qemu-ppce500.c index a39bcb4fa0c7..40fd9e5e02b1 100644 --- a/board/emulation/qemu-ppce500/qemu-ppce500.c +++ b/board/emulation/qemu-ppce500/qemu-ppce500.c @@ -318,7 +318,7 @@ ulong get_bus_freq(ulong dummy) int cpu_numcores(void) { /* -* The QEMU u-boot target only needs to drive the first core, +* The QEMU U-Boot target only needs to drive the first core, * spinning and device tree nodes get driven by QEMU itself */ return 1; diff --git a/doc/board/xilinx/zynq.rst b/doc/board/xilinx/zynq.rst index 438912fe42c2..76d67bd62ee9 100644 --- a/doc/board/xilinx/zynq.rst +++ b/doc/board/xilinx/zynq.rst @@ -83,7 +83,7 @@ Mainline status --- - Added basic board configurations support. -- Added zynq u-boot bsp code - arch/arm/mach-zynq +- Added zynq U-Boot bsp code - arch/arm/mach-zynq - Added zynq boards named - zc70x, zed, microzed, zc770_xm010/xm011/xm012/xm013 - Added zynq drivers: diff --git a/doc/board/xilinx/zynqmp-r5.rst b/doc/board/xilinx/zynqmp-r5.rst index 2cd368b0308e..266d07d1193f 100644 --- a/doc/board/xilinx/zynqmp-r5.rst +++ b/doc/board/xilinx/zynqmp-r5.rst @@ -26,7 +26,7 @@ configure and build armv7 toolchain:: Notes ^ -Output fragment is u-boot. +Output fragment is U-Boot. L
[PATCH] common: Drop linux/printk.h from common header
This old patch was marked as deferred. Bring it back to life, to continue towards the removal of common.h Move this out of the common header and include it only where needed. Signed-off-by: Simon Glass --- arch/arm/lib/gic-v3-its.c | 1 + arch/arm/mach-imx/imx8/fdt.c | 1 + arch/arm/mach-k3/common.c | 1 + arch/arm/mach-meson/board-gx.c | 1 + arch/arm/mach-omap2/am33xx/board.c | 1 + arch/arm/mach-omap2/utils.c| 1 + arch/arm/mach-rockchip/boot_mode.c | 1 + arch/arm/mach-rockchip/rk3188/rk3188.c | 1 + arch/arm/mach-rockchip/rk3399/rk3399.c | 1 + arch/arm/mach-socfpga/misc.c | 1 + arch/arm/mach-stm32mp/bsec.c | 1 + arch/arm/mach-stm32mp/cmd_stm32key.c | 1 + arch/arm/mach-stm32mp/cmd_stm32prog/cmd_stm32prog.c| 1 + arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.c| 1 + arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.h| 2 ++ arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog_serial.c | 1 + arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog_usb.c| 1 + arch/arm/mach-stm32mp/cpu.c| 3 ++- arch/arm/mach-stm32mp/include/mach/stm32mp1_smc.h | 1 + arch/arm/mach-tegra/cboot.c| 1 + arch/arm/mach-tegra/ivc.c | 1 + arch/arm/mach-tegra/tegra124/xusb-padctl.c | 1 + arch/arm/mach-tegra/tegra20/clock.c| 1 + arch/arm/mach-tegra/tegra210/xusb-padctl.c | 1 + arch/arm/mach-tegra/tegra30/clock.c| 1 + arch/arm/mach-tegra/xusb-padctl-common.c | 1 + arch/arm/mach-uniphier/boot-device/boot-device.c | 1 + arch/arm/mach-uniphier/clk/dpll-ld4.c | 1 + arch/arm/mach-uniphier/clk/dpll-pro4.c | 1 + arch/arm/mach-uniphier/dram/umc-ld4.c | 1 + arch/arm/mach-uniphier/dram/umc-pro4.c | 1 + arch/arm/mach-uniphier/dram/umc-sld8.c | 1 + arch/arm/mach-uniphier/memconf.c | 1 + arch/arm/mach-uniphier/spl_board_init.c| 1 + arch/riscv/lib/smp.c | 1 + arch/x86/cpu/tangier/pinmux.c | 1 + arch/x86/cpu/tangier/sdram.c | 1 + board/dhelectronics/dh_stm32mp1/board.c| 1 + board/mediatek/mt8518/mt8518_ap1.c | 1 + board/mscc/ocelot/ocelot.c | 1 + board/nvidia/jetson-tk1/jetson-tk1.c | 1 + board/pine64/pinebook-pro-rk3399/pinebook-pro-rk3399.c | 1 + board/samsung/common/exynos5-dt.c | 1 + board/samsung/common/misc.c| 1 + board/samsung/goni/goni.c | 1 + board/samsung/odroid/odroid.c | 1 + board/st/common/stm32mp_dfu.c | 1 + board/st/stm32mp1/stm32mp1.c | 1 + board/sunxi/board.c| 1 + board/synopsys/hsdk/clk-lib.c | 1 + board/synopsys/hsdk/env-lib.c | 1 + board/ti/am65x/evm.c | 1 + board/ti/common/board_detect.c | 1 + board/ti/j721e/evm.c | 1 + board/toradex/apalis-tk1/apalis-tk1.c | 1 + cmd/adc.c | 1 + cmd/fastboot.c | 1 + cmd/nvedit.c | 1 + cmd/onenand.c | 1 + cmd/regulator.c| 1 + cmd/thordown.c | 1 + cmd/tpm_test.c | 1 + cmd/usb_gadget_sdp.c | 1 + cmd/usb_mass_storage.c | 1 + common/dfu.c | 1 + common/spl/spl_dfu.c | 1 + common/spl/spl_fit.c | 1 + common/spl/spl_opensbi.c | 1 + common/spl/spl_sdp.c | 1 + common/update.c| 1 + disk/part_efi.c| 1 + drivers/adc/adc-uclass.c | 1 + drivers/adc/exynos-adc.c | 1 + drivers/adc/meson-saradc.c | 1 + drivers/adc/rockchip-saradc.c | 1 + drivers/adc/sandbox.c | 1 + drivers/ata/dwc_ahci.c | 1 + drivers/bio
RE: [PATCH] spi: zynqmp_qspi: Workaround for small data cache issue
Hi Michal, > -Original Message- > From: Simek, Michal > Sent: Thursday, September 14, 2023 7:34 PM > To: Abbarapu, Venkatesh ; u- > b...@lists.denx.de > Cc: ja...@amarulasolutions.com; g...@xilinx.com; Ashok Reddy Soma > > Subject: Re: [PATCH] spi: zynqmp_qspi: Workaround for small data cache > issue > > > > On 9/14/23 12:48, Venkatesh Yadav Abbarapu wrote: > > Cache related issues are seen with small sized data reads. > > Due to this, proper data is not read. Also some times sf probe fails > > randomly. To workaround this issue, invalidate dcache after read DMA > > is triggered. > > > > Signed-off-by: Ashok Reddy Soma > > Signed-off-by: Venkatesh Yadav Abbarapu > > > --- > > drivers/spi/zynqmp_gqspi.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/spi/zynqmp_gqspi.c b/drivers/spi/zynqmp_gqspi.c > > index a506d15227..ec59ef5804 100644 > > --- a/drivers/spi/zynqmp_gqspi.c > > +++ b/drivers/spi/zynqmp_gqspi.c > > @@ -707,6 +707,8 @@ static int zynqmp_qspi_start_dma(struct > zynqmp_qspi_priv *priv, > > return -ETIMEDOUT; > > } > > > > + invalidate_dcache_range(addr, addr + size); > > + > > writel(GQSPI_DMA_DST_I_STS_DONE, &dma_regs->dmaisr); > > > > debug("buf:0x%lx, rxbuf:0x%lx, *buf:0x%x len: 0x%x\n", > > I got two patches. Which one is correct? > Can you please send v2? There are two patches. Will send v2 as series. Thanks Venkatesh > > M
[PATCH v2 0/2] Fix issues for small sized data reads
Change flush cache to invalidate cache and also fix cache related issues are seen with small sized data reads. Due to this, proper data is not read. Also some times sf probe fails randomly. Ashok Reddy Soma (1): spi: zynqmp_qspi: Change flush cache to invalidate cache Venkatesh Yadav Abbarapu (1): spi: zynqmp_qspi: Workaround for small data cache issue drivers/spi/zynqmp_gqspi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- 2.17.1
[PATCH v2 1/2] spi: zynqmp_qspi: Change flush cache to invalidate cache
From: Ashok Reddy Soma Before DMA read, ideally cache should be invalidated, so that data from memory will be updated to cache after DMA is completed. But flush_dcache_range is being used which is incorrect. Change flush_dcache_range to invalidate_dcache_range. Signed-off-by: Ashok Reddy Soma Signed-off-by: Venkatesh Yadav Abbarapu --- drivers/spi/zynqmp_gqspi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/zynqmp_gqspi.c b/drivers/spi/zynqmp_gqspi.c index c4aee279aa..a506d15227 100644 --- a/drivers/spi/zynqmp_gqspi.c +++ b/drivers/spi/zynqmp_gqspi.c @@ -690,7 +690,7 @@ static int zynqmp_qspi_start_dma(struct zynqmp_qspi_priv *priv, writel(GQSPI_DMA_DST_I_STS_MASK, &dma_regs->dmaier); addr = (unsigned long)buf; size = roundup(priv->len, GQSPI_DMA_ALIGN); - flush_dcache_range(addr, addr + size); + invalidate_dcache_range(addr, addr + size); while (priv->len) { zynqmp_qspi_calc_exp(priv, &gen_fifo_cmd); -- 2.17.1
[PATCH v2 2/2] spi: zynqmp_qspi: Workaround for small data cache issue
Cache related issues are seen with small sized data reads. Due to this, proper data is not read. Also some times sf probe fails randomly. To workaround this issue, invalidate dcache after read DMA is triggered. Signed-off-by: Ashok Reddy Soma Signed-off-by: Venkatesh Yadav Abbarapu --- drivers/spi/zynqmp_gqspi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/spi/zynqmp_gqspi.c b/drivers/spi/zynqmp_gqspi.c index a506d15227..ec59ef5804 100644 --- a/drivers/spi/zynqmp_gqspi.c +++ b/drivers/spi/zynqmp_gqspi.c @@ -707,6 +707,8 @@ static int zynqmp_qspi_start_dma(struct zynqmp_qspi_priv *priv, return -ETIMEDOUT; } + invalidate_dcache_range(addr, addr + size); + writel(GQSPI_DMA_DST_I_STS_DONE, &dma_regs->dmaisr); debug("buf:0x%lx, rxbuf:0x%lx, *buf:0x%x len: 0x%x\n", -- 2.17.1
[PATCH] net: phy: xilinx-gmii2rgmii: Removed hardcoded phy address 0 for bridge
Current code expects bridge phy address at 0 which is not correct expectation because bridge phy address is configurable. That's why update the code to read reg property to figure it out where bridge is and use it in phy creation code. Signed-off-by: Michal Simek Signed-off-by: Tejas Bhumkar --- drivers/net/phy/phy.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index ae21acb059..d64a83b83d 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -838,7 +838,10 @@ static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus, ofnode_for_each_subnode(node, dev_ofnode(dev)) { node = ofnode_by_compatible(node, "xlnx,gmii-to-rgmii-1.0"); if (ofnode_valid(node)) { - phydev = phy_device_create(bus, 0, + int gmiirgmii_phyaddr; + + gmiirgmii_phyaddr = ofnode_read_u32_default(node, "reg", 0); + phydev = phy_device_create(bus, gmiirgmii_phyaddr, PHY_GMII2RGMII_ID, false); if (phydev) phydev->node = node; -- 2.27.0
Re: [PATCH v1 1/2] meson-a1: dts: add hw rng node
Hi Neil, On Thu, Jun 22, 2023 at 06:19:33PM +0200, neil.armstr...@linaro.org wrote: > On 21/06/2023 15:29, Alexey Romanov wrote: > > Add support for hardware random number generator > > of Amlogic Meson SoCs. > > > > Signed-off-by: Alexey Romanov > > --- > > arch/arm/dts/meson-a1.dtsi | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/arch/arm/dts/meson-a1.dtsi b/arch/arm/dts/meson-a1.dtsi > > index f3560cbc3a4..1f57c137384 100644 > > --- a/arch/arm/dts/meson-a1.dtsi > > +++ b/arch/arm/dts/meson-a1.dtsi > > @@ -140,6 +140,11 @@ > > clock-names = "xtal", "pclk", "baud"; > > status = "disabled"; > > }; > > + > > + hwrng: rng@5118 { > > + compatible = "amlogic,meson-rng"; > > + reg = <0x0 0x5118 0x0 0x4>; > > + }; > > }; > > gic: interrupt-controller@ff901000 { > > Weird, you introduced amlogic,meson-rng-s4 previously but you don't use it > here ? This is A1 (not S4). > > Anyway please add those to Linux first, them sync the DT to u-boot afterwards, Applied into v6.7/arm64-dt here: https://lore.kernel.org/all/20230823213630.12936-13-ddroko...@sberdevices.ru/ > > Thanks, > Neil -- Thank you, Alexey
Re: [PATCH v1 2/2] meson-a1: dts: add ao secure node
Hi Neil, On Wed, Jun 21, 2023 at 04:29:53PM +0300, Alexey Romanov wrote: > ao-secure node can be used to get information about the board, > so, for example, using show_board_info() we can get following > information for board with Meson A1 SoC: > > SoC: Amlogic Meson A1 (A113L) Revision 2c:a (1:a) > > Signed-off-by: Alexey Romanov > --- > arch/arm/dts/meson-a1.dtsi | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/arm/dts/meson-a1.dtsi b/arch/arm/dts/meson-a1.dtsi > index 1f57c137384..149cc0e23a3 100644 > --- a/arch/arm/dts/meson-a1.dtsi > +++ b/arch/arm/dts/meson-a1.dtsi > @@ -145,6 +145,12 @@ > compatible = "amlogic,meson-rng"; > reg = <0x0 0x5118 0x0 0x4>; > }; > + > + sec_AO: ao-secure@5a20 { > + compatible = "amlogic,meson-gx-ao-secure", > "syscon"; > + reg = <0x0 0x5a20 0x0 0x140>; > + amlogic,has-chip-id; > + }; > }; > > gic: interrupt-controller@ff901000 { > -- > 2.38.1 > Applied into v6.7/arm64-dt here: https://lore.kernel.org/all/20230823213630.12936-14-ddroko...@sberdevices.ru/ -- Thank you, Alexey
Re: [PATCH] arm64: zynqmp: Corrected pcap_prog register address
On 9/15/23 11:39, lukas.funke-...@weidmueller.com wrote: From: Lukas Funke Currently the pcap_prog struct variable is pointing to 0x3004 which is incorrect according to [1]. The variable should point to 0x3000. [1] https://www.xilinx.com/htmldocs/registers/ug1087/ug1087-zynq-ultrascale-registers.html#csu___pcap_prog.html Signed-off-by: Lukas Funke --- arch/arm/mach-zynqmp/include/mach/hardware.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h b/arch/arm/mach-zynqmp/include/mach/hardware.h index 634bf169c6..8cb6494e52 100644 --- a/arch/arm/mach-zynqmp/include/mach/hardware.h +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h @@ -166,7 +166,7 @@ struct csu_regs { u32 jtag_dap_cfg; u32 idcode; u32 version; - u32 reserved2[3055]; + u32 reserved2[3054]; u32 pcap_prog; }; Good catch. Applied. M
Re: [PATCH] pinctrl: zynqmp: Display the tristate configuration for all pins
On 9/14/23 12:06, Venkatesh Yadav Abbarapu wrote: Read the tristate config for all the pins and display it. ZynqMP> pinmux status MIO1 MIO1: slew:fast bias:enabled pull:up input:cmos drive:12mA volt:1.8 tri_state:enabled Signed-off-by: Venkatesh Yadav Abbarapu --- drivers/pinctrl/pinctrl-zynqmp.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/pinctrl-zynqmp.c b/drivers/pinctrl/pinctrl-zynqmp.c index 02626a7561..d98834b1aa 100644 --- a/drivers/pinctrl/pinctrl-zynqmp.c +++ b/drivers/pinctrl/pinctrl-zynqmp.c @@ -533,6 +533,8 @@ static int zynqmp_pinctrl_get_pin_muxing(struct udevice *dev, &pinmux.drive_strength); zynqmp_pm_pinctrl_get_config(selector, PM_PINCTRL_CONFIG_VOLTAGE_STATUS, &pinmux.volt_sts); + zynqmp_pm_pinctrl_get_config(selector, PM_PINCTRL_CONFIG_TRI_STATE, +&pinmux.tri_state); switch (pinmux.drive_strength) { case PM_PINCTRL_DRIVE_STRENGTH_2MA: @@ -553,13 +555,15 @@ static int zynqmp_pinctrl_get_pin_muxing(struct udevice *dev, return -EINVAL; } - snprintf(buf, size, "slew:%s\tbias:%s\tpull:%s\tinput:%s\tdrive:%dmA\tvolt:%s", + snprintf(buf, size, + "slew:%s\tbias:%s\tpull:%s\tinput:%s\tdrive:%dmA\tvolt:%s\ttri_state:%s", pinmux.slew ? "slow" : "fast", pinmux.bias ? "enabled" : "disabled", pinmux.pull_ctrl ? "up" : "down", pinmux.input_type ? "schmitt" : "cmos", pinmux.drive_strength, -pinmux.volt_sts ? "1.8" : "3.3"); +pinmux.volt_sts ? "1.8" : "3.3", +pinmux.tri_state ? "enabled" : "disabled"); return 0; } When I run it on SOM I get some lines truncated. MIO12 : slew:slow bias:enabled pull:up input:cmos drive:4mA volt:1.8 tri_state:disabled MIO13 : slew:slow bias:disabled pull:up input:cmos drive:4mA volt:1.8 tri_state:disable MIO14 : slew:slow bias:disabled pull:up input:cmos drive:4mA volt:1.8 tri_state:disable it is because of buf limit is 80 chars (PINMUX_SIZE) It means we have 2 options. 1. keep this patch as it is and send one more which extend PINMUX_SIZE limit 2. change enabled/disabled to something shorter The longest line is slew:show\tbias:disabled\tpull:down\tinput:schmitt\tdriver:12mA\tvolt:1.8\ttri_state:disabled\0 which is based on my calculation 88 chars. It means if you want to increase that macro 90 chars should be enough. Please let me know which way you want to go. Thanks, Michal
[PATCH v2 0/3] trace: Fix flyrecord alignment issue
Hi, sandbox is getting bigger and bigger and I have reached the case that adding some more functions ends up in CI loop failure. After some investigation I found that flyrecord header have incorrect information about data offset which is caused by incorrect alignment calculation. That's why this series is fixing alignment calculation. I have done it via 3 patches where the first two are just preparing code for actual fixup. Thanks, Michal Changes in v2: - s/start_addr/start_ofs/g' Michal Simek (3): trace: Use 64bit variable for start and len trace: Move trace_clocks description above record offset calculation trace: Fix alignment logic in flyrecord header tools/proftool.c | 39 ++- 1 file changed, 34 insertions(+), 5 deletions(-) -- 2.36.1
[PATCH v2 1/3] trace: Use 64bit variable for start and len
tputq() requires variables to have 64bit width that's why make them 64bit to clean alignment requirement. Signed-off-by: Michal Simek Reviewed-by: Simon Glass --- (no changes since v1) tools/proftool.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/proftool.c b/tools/proftool.c index 101bcb63334e..869a2a32c510 100644 --- a/tools/proftool.c +++ b/tools/proftool.c @@ -1493,7 +1493,8 @@ static int write_pages(struct twriter *tw, enum out_format_t out_format, static int write_flyrecord(struct twriter *tw, enum out_format_t out_format, int *missing_countp, int *skip_countp) { - int start, ret, len; + unsigned long long start, len; + int ret; FILE *fout = tw->fout; char str[200]; -- 2.36.1
[PATCH v2 2/3] trace: Move trace_clocks description above record offset calculation
Flyrecord tracing data are page aligned that's why it is necessary to calculate alignment properly. Because trace_clocks description is the part of record length it is necessary to have information about length earlier. Signed-off-by: Michal Simek Reviewed-by: Simon Glass --- (no changes since v1) tools/proftool.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/proftool.c b/tools/proftool.c index 869a2a32c510..7c95a94482fc 100644 --- a/tools/proftool.c +++ b/tools/proftool.c @@ -1500,6 +1500,10 @@ static int write_flyrecord(struct twriter *tw, enum out_format_t out_format, tw->ptr += fprintf(fout, "flyrecord%c", 0); + snprintf(str, sizeof(str), +"[local] global counter uptime perf mono mono_raw boot x86-tsc\n"); + len = strlen(str); + /* trace data */ start = ALIGN(tw->ptr + 16, TRACE_PAGE_SIZE); tw->ptr += tputq(fout, start); @@ -1510,9 +1514,6 @@ static int write_flyrecord(struct twriter *tw, enum out_format_t out_format, return -1; tw->ptr += ret; - snprintf(str, sizeof(str), -"[local] global counter uptime perf mono mono_raw boot x86-tsc\n"); - len = strlen(str); tw->ptr += tputq(fout, len); tw->ptr += tputs(fout, str); -- 2.36.1
[PATCH v2 3/3] trace: Fix alignment logic in flyrecord header
Current alignment which is using 16 bytes is not correct in connection to trace_clocks description and it's length. That's why use start_addr variable and record proper size based on used entries. Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format"). Signed-off-by: Michal Simek Reviewed-by: Simon Glass --- Changes in v2: - s/start_addr/start_ofs/g' tools/proftool.c | 31 +-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/tools/proftool.c b/tools/proftool.c index 7c95a94482fc..fca45e4a5af6 100644 --- a/tools/proftool.c +++ b/tools/proftool.c @@ -1493,19 +1493,43 @@ static int write_pages(struct twriter *tw, enum out_format_t out_format, static int write_flyrecord(struct twriter *tw, enum out_format_t out_format, int *missing_countp, int *skip_countp) { - unsigned long long start, len; + unsigned long long start, start_ofs, len; int ret; FILE *fout = tw->fout; char str[200]; + /* Record start pointer */ + start_ofs = tw->ptr; + debug("Start of flyrecord header at: 0x%llx\n", start_ofs); + tw->ptr += fprintf(fout, "flyrecord%c", 0); + /* flyrecord\0 - allocated 10 bytes */ + start_ofs += 10; + + /* +* 8 bytes that are a 64-bit word containing the offset into the file +* that holds the data for the CPU. +* +* 8 bytes that are a 64-bit word containing the size of the CPU +* data at that offset. +*/ + start_ofs += 16; + snprintf(str, sizeof(str), "[local] global counter uptime perf mono mono_raw boot x86-tsc\n"); len = strlen(str); + /* trace clock length - 8 bytes */ + start_ofs += 8; + /* trace clock data */ + start_ofs += len; + + debug("Calculated flyrecord header end at: 0x%llx, trace clock len: 0x%llx\n", + start_ofs, len); + /* trace data */ - start = ALIGN(tw->ptr + 16, TRACE_PAGE_SIZE); + start = ALIGN(start_ofs, TRACE_PAGE_SIZE); tw->ptr += tputq(fout, start); /* use a placeholder for the size */ @@ -1517,6 +1541,9 @@ static int write_flyrecord(struct twriter *tw, enum out_format_t out_format, tw->ptr += tputq(fout, len); tw->ptr += tputs(fout, str); + debug("End of flyrecord header at: 0x%x, offset: 0x%llx\n", + tw->ptr, start); + debug("trace text base %lx, map file %lx\n", text_base, text_offset); ret = write_pages(tw, out_format, missing_countp, skip_countp); -- 2.36.1
Re: [PATCH 3/5] armv8: fsl-layerscape: create bypass smmu mapping for MC
On 9/6/2023 11:09 PM, Robin Murphy wrote: > On 2023-09-06 19:10, Laurentiu Tudor wrote: >> >> >> On 9/6/2023 8:21 PM, Robin Murphy wrote: >>> On 2023-09-06 17:01, Laurentiu Tudor wrote: MC being a plain DMA master as any other device in the SoC and being live at OS boot time, as soon as the SMMU is probed it will immediately start triggering faults because there is no mapping in the SMMU for the MC. Pre-create such a mapping in the SMMU, being the OS's responsibility to preserve it. >>> >>> Does U-Boot enable the SMMU? AFAICS the only thing it knows how to do >>> is explicitly turn it *off*, therefore programming other registers >>> appears to be a complete waste of time. >> >> No, it doesn't enable SMMU but it does mark a SMR as valid for MC FW. >> And the ARM SMMU driver subtly preserves it, see [1] (it's late and I >> might be wrong, but I'll double check tomorrow). :-) > > No, that sets the SMR valid bit *if* the corresponding entry is > allocated and marked as valid in the software state in smmu->smrs, which > at probe time it isn't, because that's only just been allocated and is > still zero-initialised. Unless, that is, > arm_smmu_rmr_install_bypass_smr() found a reserved region and > preallocated an entry to honour it. But even those entries are still > constructed from scratch; we can't do anything with the existing > SMR/S2CR register contents in general since they may be uninitialised > random reset values, so we don't even look. > > Pay no attention to the qcom_smmu_cfg_probe() hack either - that only > exists on the promise that the relevant platforms couldn't have their > firmware updated to use proper RMRs. > > You're already doing the right thing in patch #2, so there's no need to > waste code on doing a pointless wrong thing as well. > Spent quite a while (sorry!) trying to remember why this was needed but don't have a clear answer. I suspect that the hack was used with early iort rmr implementations. Anyway, I retested without it both DT and ACPI and looks good. Will drop the patch and resubmit the series. --- Thanks & Best Regards, Laurentiu > >>> All that should matter to the OS, and that it is responsible for >>> upholding, is the reserved memory regions from patch #2. For >>> instance, if the OS is Linux, literally the first thing >>> arm_smmu_device_reset() does is rewrite all the S2CRs and SMRs >>> without so much as looking. >> >> [1] >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/arm/arm-smmu/arm-smmu.c#n894 >> >> >> --- >> Best Regards, Laurentiu >> >>> Signed-off-by: Laurentiu Tudor --- arch/arm/cpu/armv8/fsl-layerscape/soc.c | 26 --- .../asm/arch-fsl-layerscape/immap_lsch3.h | 9 +++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c index 3bfdc3f77431..870b99838ab5 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c @@ -376,6 +376,18 @@ void bypass_smmu(void) val = (in_le32(SMMU_NSCR0) | SCR0_CLIENTPD_MASK) & ~(SCR0_USFCFG_MASK); out_le32(SMMU_NSCR0, val); } + +void setup_smmu_mc_bypass(int icid, int mask) +{ +u32 val; + +val = SMMU_SMR_VALID_MASK | (icid << SMMU_SMR_ID_SHIFT) | +(mask << SMMU_SMR_MASK_SHIFT); +out_le32(SMMU_REG_SMR(0), val); +val = SMMU_S2CR_EXIDVALID_VALID_MASK | SMMU_S2CR_TYPE_BYPASS_MASK; +out_le32(SMMU_REG_S2CR(0), val); +} + void fsl_lsch3_early_init_f(void) { erratum_rcw_src(); @@ -402,10 +414,18 @@ void fsl_lsch3_early_init_f(void) bypass_smmu(); #endif -#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS1028A) || \ -defined(CONFIG_ARCH_LS2080A) || defined(CONFIG_ARCH_LX2160A) || \ -defined(CONFIG_ARCH_LX2162A) +#ifdef CONFIG_ARCH_LS1028A +set_icids(); +#endif + +#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS2080A) +set_icids(); +setup_smmu_mc_bypass(0x300, 0); +#endif + +#if defined(CONFIG_ARCH_LX2160A) || defined(CONFIG_ARCH_LX2162A) set_icids(); +setup_smmu_mc_bypass(0x4000, 0); #endif } diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h index ca5e33379ba9..bec5355adaed 100644 --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h @@ -190,6 +190,15 @@ #define SCR0_CLIENTPD_MASK0x0001 #define SCR0_USFCFG_MASK0x0400 +#define SMMU_REG_SMR(n)(SMMU_BASE + 0x800 + ((n) << 2)) +#define SMMU_REG_S2CR(n)(SMMU
Re: [PATCH] include: env: ti: mmc: Add 'ti' directory prefix for overlays
On 15:42-20230915, Ravi Gunasekaran wrote: > DTSOs are stored in vendor specific directories in Linux. what is dtso? kernel generates dtbo > Add the vendor prefix to the path to load the overlay correctly. > > Signed-off-by: Ravi Gunasekaran > --- > include/env/ti/mmc.env | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/env/ti/mmc.env b/include/env/ti/mmc.env > index 0256a2d2aa..3e6b0287a3 100644 > --- a/include/env/ti/mmc.env > +++ b/include/env/ti/mmc.env > @@ -70,7 +70,7 @@ get_overlay_mmc= > fdt resize 0x10; > for overlay in $name_overlays; > do; > - load mmc ${bootpart} ${dtboaddr} ${bootdir}/dtb/${overlay} && > + load mmc ${bootpart} ${dtboaddr} ${bootdir}/dtb/ti/${overlay} && > fdt apply ${dtboaddr}; > done; > get_kern_mmc=load mmc ${bootpart} ${loadaddr} > > base-commit: 2fe4b54556ea6271237b35de68dc458bfceab94c we should move to stdboot and simplify this. -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
USB: error messages on DWC3 gadget endpoint dequeue
Hi Marek, I was testing fastboot image download over usb for imx8mp (from usb recovery patch of verdin-imx8mp) and i am having error messages on endpoint request dequeue function of DWC3 gadget controller. However, download is working fine, so this message may not be an error. They are happening because fastboot tx before sending a new usb request dequeue the same request, maybe to be sure it does not send it twice. Can I just ignore these messages? Maybe change its log level to dbg instead of error? What do you think? The messages I am seeing are below and are the ones with "... was not queued to ep1in-bulk". U-Boot 2023.10-rc3-00028-gc99d052c76 (Sep 14 2023 - 15:59:22 -0300) CPU: Freescale i.MX8MP[8] rev1.1 1600 MHz (running at 1200 MHz) CPU: Industrial temperature grade (-40C to 105C) at 47C Reset cause: POR DRAM: 4 GiB Core: 168 devices, 28 uclasses, devicetree: separate WDT: Started watchdog@3028 with servicing every 1000ms (60s timeout) MMC: FSL_SDHC: 1, FSL_SDHC: 2 Loading Environment from MMC... OK In:serial@3088 Out: serial@3088 Err: serial@3088 Model: Toradex 0058 Verdin iMX8M Plus Quad 4GB WB IT V1.1A Serial#: 15007098 Carrier: Toradex Dahlia V1.1C, Serial# 10952631 SEC0: RNG instantiated Net: eth1: ethernet@30be, eth0: ethernet@30bf [PRIME] Hit any key to stop autoboot: 0 Verdin iMX8MP # Verdin iMX8MP # fastboot 0 dwc3-generic-peripheral usb@3810: request ffb29c40 was not queued to ep1in-bulk Starting download of 3084 bytes dwc3-generic-peripheral usb@3810: request ffb29c40 was not queued to ep1in-bulk downloading of 3084 bytes finished dwc3-generic-peripheral usb@3810: request ffb29c40 was not queued to ep1in-bulk dwc3-generic-peripheral usb@3810: request ffb29c40 was not queued to ep1in-bulk Starting download of 86 bytes dwc3-generic-peripheral usb@3810: request ffb29c40 was not queued to ep1in-bulk downloading of 86 bytes finished dwc3-generic-peripheral usb@3810: request ffb29c40 was not queued to ep1in-bulk dwc3-generic-peripheral usb@3810: request ffb29c40 was not queued to ep1in-bulk Starting download of 47173400 bytes dwc3-generic-peripheral usb@3810: request ffb29c40 was not queued to ep1in-bulk ... ... ... ... ... ... ... ... ... downloading of 47173400 bytes finished dwc3-generic-peripheral usb@3810: request ffb29c40 was not queued to ep1in-bulk dwc3-generic-peripheral usb@3810: request ffb29c40 was not queued to ep1in-bulk Thanks, Joao Paulo
Re: USB: error messages on DWC3 gadget endpoint dequeue
On 15/09/2023 15.05, João Paulo Silva Gonçalves wrote: > Hi Marek, > > I was testing fastboot image download over usb for imx8mp (from usb > recovery patch of verdin-imx8mp) and i am having error messages on > endpoint request dequeue function of DWC3 gadget controller. However, > download is working fine, so this message may not be an error. They are > happening because fastboot tx before sending a new usb request dequeue > the same request, maybe to be sure it does not send it twice. Can I > just ignore these messages? Maybe change its log level to dbg instead > of error? What do you think? The messages I am seeing are below and are > the ones with "... was not queued to ep1in-bulk". We apply this internally (sorry if it's whitespace damaged), but I never fully understood the problem nor how the referenced kernel thread was resolved, which is why I haven't sent it upstream yet. dwc3: gadget: Handle dequeuing of non queued request gracefully Trying to dequeue an request that is currently not queued should be a no-op and be handled gracefully. Checking on list/queue empty indicate whether the request is queue or not. Handling this gracefully allows for race condition free synchronization between the complete callback being called to to a completed transfer and trying to call usb_ep_dequeue() at the same time. Inspired by: https://patchwork.kernel.org/project/linux-usb/patch/20191106144553.16956-1-alexandru.ardel...@analog.com/ diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index eb416b832aa..378d19d8e99 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1113,6 +1113,9 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, spin_lock_irqsave(&dwc->lock, flags); + if (list_empty(&dep->request_list) && list_empty(&dep->req_queued)) + goto out0; + list_for_each_entry(r, &dep->request_list, list) { if (r == req) break; Rasmus
[PATCH] net: eth-uclass: Setup ROM source only when ROM reading passes
There is no reason to setup ROM source if read_rom_hwaddr hook doesn't exist or reading mac address fails. It is ending up with confusion about mac address source. It is nicely visible if you put mac address to DT as local-mac-address = [ff ff ff ff ff ff]; but also save ethaddr to variables setenv -f ethaddr 02:18:31:7e:3e:01 Before this patch U-Boot prints that source is DT Address in ROM is ff:ff:ff:ff:ff:ff Address in environment is 02:18:31:7e:3e:01 After that source is DT: Address in DT isff:ff:ff:ff:ff:ff Address in environment is 02:18:31:7e:3e:01 Signed-off-by: Michal Simek --- net/eth-uclass.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/net/eth-uclass.c b/net/eth-uclass.c index c393600fabcd..e40b869e1c31 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -588,10 +588,14 @@ static int eth_post_probe(struct udevice *dev) /* Check if the device has a valid MAC address in device tree */ if (!eth_dev_get_mac_address(dev, pdata->enetaddr) || !is_valid_ethaddr(pdata->enetaddr)) { - source = "ROM"; /* Check if the device has a MAC address in ROM */ - if (eth_get_ops(dev)->read_rom_hwaddr) - eth_get_ops(dev)->read_rom_hwaddr(dev); + if (eth_get_ops(dev)->read_rom_hwaddr) { + int ret; + + ret = eth_get_ops(dev)->read_rom_hwaddr(dev); + if (!ret) + source = "ROM"; + } } eth_env_get_enetaddr_by_index("eth", dev_seq(dev), env_enetaddr); -- 2.36.1
[PATCH v2] net: eth-uclass: Setup ROM source only when ROM reading passes
There is no reason to setup ROM source if read_rom_hwaddr hook doesn't exist or reading mac address fails. It is ending up with confusion about mac address source. It is nicely visible if you put mac address to DT as local-mac-address = [ff ff ff ff ff ff]; but also save ethaddr to variables setenv -f ethaddr 02:18:31:7e:3e:01 Before this patch U-Boot prints that source is ROM Address in ROM is ff:ff:ff:ff:ff:ff Address in environment is 02:18:31:7e:3e:01 After that source is DT: Address in DT isff:ff:ff:ff:ff:ff Address in environment is 02:18:31:7e:3e:01 Signed-off-by: Michal Simek --- Changes in v2: - Fix commit message net/eth-uclass.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/net/eth-uclass.c b/net/eth-uclass.c index c393600fabcd..e40b869e1c31 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -588,10 +588,14 @@ static int eth_post_probe(struct udevice *dev) /* Check if the device has a valid MAC address in device tree */ if (!eth_dev_get_mac_address(dev, pdata->enetaddr) || !is_valid_ethaddr(pdata->enetaddr)) { - source = "ROM"; /* Check if the device has a MAC address in ROM */ - if (eth_get_ops(dev)->read_rom_hwaddr) - eth_get_ops(dev)->read_rom_hwaddr(dev); + if (eth_get_ops(dev)->read_rom_hwaddr) { + int ret; + + ret = eth_get_ops(dev)->read_rom_hwaddr(dev); + if (!ret) + source = "ROM"; + } } eth_env_get_enetaddr_by_index("eth", dev_seq(dev), env_enetaddr); -- 2.36.1
Re: [PATCH] Makefile: Force regeneration of env.txt
On 9/10/23 5:36 PM, Simon Glass wrote: Hi Andrew, On Tue, 5 Sept 2023 at 12:15, Andrew Davis wrote: On 9/5/23 1:09 PM, Andrew Davis wrote: If the source .env file changes to one that is also older than the generated env.txt file then it is not regenerated. This means when switching board configs we do not regenerate the env. This can be tested easily with: $ make j721e_evm_a72_defconfig $ make # this may fail to complete but that is okay for this test $ make am64x_evm_a53_defconfig $ make $ vim include/generated/env.txt Note this is still the J721e env not the AM64 config as expected. There is probably a better way to detect if the dependency name changed, but that may involve extra files and hashing contents, so let's just force it for now. Signed-off-by: Andrew Davis --- This is basically a revert of: 36fc832927eb ("Makefile: Fix incorrect FORCE deps on env rules") But without changing the `include/generated/env.in` rule. The more I think about it, that should also be changed to forced as right now any changes to include/config.h or other kconfig options are not reflected in the env.in file after updates. That rule should depend on the generated config file so that any change in kconfig variables will cause it to regenerate, that might be what we need for this target instead of FORCE. Andrew Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 9be24c4ec61..d195590d4b0 100644 --- a/Makefile +++ b/Makefile @@ -1830,7 +1830,7 @@ quiet_cmd_envc = ENVC$@ touch $@ ; \ fi -include/generated/env.txt: $(wildcard $(ENV_FILE)) +include/generated/env.txt: $(wildcard $(ENV_FILE)) FORCE $(call cmd,envc) I thought that FORCE was only for if_changed - see makefiles.rst.txt Could it depend on autoconf.h instead? That works, sending v2, thanks! Andrew # Write out the resulting environment, converted to a C string Regards, Simon
[PATCH v2] Makefile: Force regeneration of env.txt
If the source .env file changes to one that is also older than the generated env.txt file then the .env file is not regenerated. This means when switching board configs we do not regenerate the env. This can be tested with: $ make j721e_evm_a72_defconfig $ make # this may fail to complete but that is okay for this test $ make am64x_evm_a53_defconfig $ make $ vim include/generated/env.txt Note this is still the J721e env not the AM64 config as expected. As ENV_FILE is set based on configuration, regenerate anytime autoconf.h changes. Signed-off-by: Andrew Davis --- Changes for v2: - Depend on autoconf.h as suggested by Simon Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 9be24c4ec61..dba3c145368 100644 --- a/Makefile +++ b/Makefile @@ -1830,7 +1830,7 @@ quiet_cmd_envc = ENVC$@ touch $@ ; \ fi -include/generated/env.txt: $(wildcard $(ENV_FILE)) +include/generated/env.txt: $(wildcard $(ENV_FILE)) include/generated/autoconf.h $(call cmd,envc) # Write out the resulting environment, converted to a C string -- 2.39.2
Re: USB: error messages on DWC3 gadget endpoint dequeue
Hi Rasmus, Make sense to me. At least now I know that these messages are not a critical error and will not give me problems on the future. Thanks for the patch too. Regards, João Paulo On Fri, 2023-09-15 at 15:47 +0200, Rasmus Villemoes wrote: > This message originated from outside your organization > > On 15/09/2023 15.05, João Paulo Silva Gonçalves wrote: > > Hi Marek, > > > > I was testing fastboot image download over usb for imx8mp (from usb > > recovery patch of verdin-imx8mp) and i am having error messages on > > endpoint request dequeue function of DWC3 gadget controller. However, > > download is working fine, so this message may not be an error. They are > > happening because fastboot tx before sending a new usb request dequeue > > the same request, maybe to be sure it does not send it twice. Can I > > just ignore these messages? Maybe change its log level to dbg instead > > of error? What do you think? The messages I am seeing are below and are > > the ones with "... was not queued to ep1in-bulk". > > We apply this internally (sorry if it's whitespace damaged), but I never > fully understood the problem nor how the referenced kernel thread was > resolved, which is why I haven't sent it upstream yet. > > dwc3: gadget: Handle dequeuing of non queued request gracefully > > Trying to dequeue an request that is currently not queued should be > a no-op > and be handled gracefully. > > Checking on list/queue empty indicate whether the request is queue > or not. > Handling this gracefully allows for race condition free synchronization > between the complete callback being called to to a completed > transfer and > trying to call usb_ep_dequeue() at the same time. > > Inspired by: > https://patchwork.kernel.org/project/linux-usb/patch/20191106144553.16956-1-alexandru.ardel...@analog.com/ > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index eb416b832aa..378d19d8e99 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -1113,6 +1113,9 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, > > spin_lock_irqsave(&dwc->lock, flags); > > + if (list_empty(&dep->request_list) && list_empty(&dep->req_queued)) > + goto out0; > + > list_for_each_entry(r, &dep->request_list, list) { > if (r == req) > break; > > Rasmus
[RFT PATCH 0/2] mmc: meson-gx: improve MMC reliabilty
Amlogic MMC on the GX (and later) SoCs has been problematic for years, especially with u-boot. Linux has been fairly stable for a few years. It is using a fixed phase setting with Core = 180, Tx = 0 and Rx = 0 (the latter cannot be set starting from the v3 MMC IPs) Still the results were not good with those settings with u-boot, on some sm1 based platforms. U-boot then started using a 270 core phase for sm1 only. This worked for most sm1 platforms but problems persist on others. The proposal with this patchset is to use 270 for the ID phase, 180 otherwise. This works well on the platforms I have tested (Libretech's boards and VIM3L) It would be great if others could test this and report whether this work for them or not. If the results are good, this might be ported to Linux as well (... but the situation is less critical there) Jerome Brunet (2): mmc: meson-gx: clean up and align on Linux settings mmc: meson-gx: set 270 core phase during the identification drivers/mmc/meson_gx_mmc.c | 50 ++ drivers/mmc/meson_gx_mmc.h | 9 +-- 2 files changed, 31 insertions(+), 28 deletions(-) -- 2.40.1
[RFT PATCH 1/2] mmc: meson-gx: clean up and align on Linux settings
* Remove obsolete comments * Set core phase to 180 regardless of the SoC like Linux * Enable always-on clock AML mmc driver has been working okay(ish) for a few years The purpose of this patch is to bring u-boot closer to what Linux is doing Signed-off-by: Jerome Brunet --- drivers/mmc/meson_gx_mmc.c | 45 -- drivers/mmc/meson_gx_mmc.h | 9 ++-- 2 files changed, 26 insertions(+), 28 deletions(-) diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index fcf4f03d1e24..c6168792cbae 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -17,13 +17,14 @@ #include #include "meson_gx_mmc.h" -bool meson_gx_mmc_is_compatible(struct udevice *dev, - enum meson_gx_mmc_compatible family) -{ - enum meson_gx_mmc_compatible compat = dev_get_driver_data(dev); - - return compat == family; -} +struct meson_gx_mmc_version_data meson_gx_mmc_version[] = { + [MMC_COMPATIBLE_V2] = { + .clk_always_on = BIT(24), + }, + [MMC_COMPATIBLE_V3] = { + .clk_always_on = BIT(28), + }, +}; static inline void *get_regbase(const struct mmc *mmc) { @@ -44,13 +45,17 @@ static inline void meson_write(struct mmc *mmc, uint32_t val, int offset) static void meson_mmc_config_clock(struct mmc *mmc) { + struct meson_mmc_plat *pdata = mmc->priv; uint32_t meson_mmc_clk = 0; unsigned int clk, clk_src, clk_div; if (!mmc->clock) return; - /* TOFIX This should use the proper clock taken from DT */ + /* Clk always on */ + meson_mmc_clk |= pdata->version->clk_always_on; + meson_mmc_clk |= CLK_CO_PHASE_180; + meson_mmc_clk |= CLK_TX_PHASE_000; /* 1GHz / CLK_MAX_DIV = 15,9 MHz */ if (mmc->clock > 1600) { @@ -62,20 +67,6 @@ static void meson_mmc_config_clock(struct mmc *mmc) } clk_div = DIV_ROUND_UP(clk, mmc->clock); - /* -* SM1 SoCs doesn't work fine over 50MHz with CLK_CO_PHASE_180 -* If CLK_CO_PHASE_270 is used, it's more stable than other. -* Other SoCs use CLK_CO_PHASE_180 by default. -* It needs to find what is a proper value about each SoCs. -*/ - if (meson_gx_mmc_is_compatible(mmc->dev, MMC_COMPATIBLE_SM1)) - meson_mmc_clk |= CLK_CO_PHASE_270; - else - meson_mmc_clk |= CLK_CO_PHASE_180; - - /* 180 phase tx clock */ - meson_mmc_clk |= CLK_TX_PHASE_000; - /* clock settings */ meson_mmc_clk |= clk_src; meson_mmc_clk |= clk_div; @@ -243,6 +234,7 @@ static const struct dm_mmc_ops meson_dm_mmc_ops = { static int meson_mmc_of_to_plat(struct udevice *dev) { + enum meson_gx_mmc_compatible compat = dev_get_driver_data(dev); struct meson_mmc_plat *pdata = dev_get_plat(dev); fdt_addr_t addr; @@ -251,6 +243,7 @@ static int meson_mmc_of_to_plat(struct udevice *dev) return -EINVAL; pdata->regbase = (void *)addr; + pdata->version = &meson_gx_mmc_version[compat]; return 0; } @@ -277,7 +270,7 @@ static int meson_mmc_probe(struct udevice *dev) cfg->voltages = MMC_VDD_33_34 | MMC_VDD_32_33 | MMC_VDD_31_32 | MMC_VDD_165_195; cfg->host_caps = MMC_MODE_8BIT | MMC_MODE_4BIT | - MMC_MODE_HS_52MHz | MMC_MODE_HS; +SD_HS | MMC_MODE_HS_52MHz | MMC_MODE_HS; cfg->f_min = DIV_ROUND_UP(SD_EMMC_CLKSRC_24M, CLK_MAX_DIV); cfg->f_max = 1; /* 100 MHz */ cfg->b_max = 511; /* max 512 - 1 blocks */ @@ -321,9 +314,9 @@ int meson_mmc_bind(struct udevice *dev) } static const struct udevice_id meson_mmc_match[] = { - { .compatible = "amlogic,meson-gx-mmc", .data = MMC_COMPATIBLE_GX }, - { .compatible = "amlogic,meson-axg-mmc", .data = MMC_COMPATIBLE_GX }, - { .compatible = "amlogic,meson-sm1-mmc", .data = MMC_COMPATIBLE_SM1 }, + { .compatible = "amlogic,meson-gx-mmc", .data = MMC_COMPATIBLE_V2 }, + { .compatible = "amlogic,meson-axg-mmc", .data = MMC_COMPATIBLE_V3 }, + { .compatible = "amlogic,meson-sm1-mmc", .data = MMC_COMPATIBLE_V3 }, { /* sentinel */ } }; diff --git a/drivers/mmc/meson_gx_mmc.h b/drivers/mmc/meson_gx_mmc.h index 8974b78f559d..3ec913d1b5ef 100644 --- a/drivers/mmc/meson_gx_mmc.h +++ b/drivers/mmc/meson_gx_mmc.h @@ -10,8 +10,8 @@ #include enum meson_gx_mmc_compatible { - MMC_COMPATIBLE_GX, - MMC_COMPATIBLE_SM1, + MMC_COMPATIBLE_V2, + MMC_COMPATIBLE_V3, }; #define SDIO_PORT_A0 @@ -84,7 +84,12 @@ enum meson_gx_mmc_compatible { #define MESON_SD_EMMC_CMD_RSP2 0x64 #define MESON_SD_EMMC_CMD_RSP3 0x68 +struct meson_gx_mmc_version_data { + uint32_t clk_always_on; +}; + struct meson_mmc_plat { + struct meson_gx_mmc_version_data *version;
[RFT PATCH 2/2] mmc: meson-gx: set 270 core phase during the identification
It has been reported that some devices have problems with a 180 degree core phase. Setting 270 helped some of these devices. Other continue to struggle (while it works fine with 180 in Linux ... :sigh:) Poking around the HW, it seems that setting a 270 core phase during the identification, then using 180 for the rest of the operations, helps the device operate correctly. Signed-off-by: Jerome Brunet --- drivers/mmc/meson_gx_mmc.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index c6168792cbae..284be2b9dca4 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -54,9 +54,14 @@ static void meson_mmc_config_clock(struct mmc *mmc) /* Clk always on */ meson_mmc_clk |= pdata->version->clk_always_on; - meson_mmc_clk |= CLK_CO_PHASE_180; meson_mmc_clk |= CLK_TX_PHASE_000; + /* Core phase according to mode */ + if (mmc->selected_mode == MMC_LEGACY) + meson_mmc_clk |= CLK_CO_PHASE_270; + else + meson_mmc_clk |= CLK_CO_PHASE_180; + /* 1GHz / CLK_MAX_DIV = 15,9 MHz */ if (mmc->clock > 1600) { clk = SD_EMMC_CLKSRC_DIV2; -- 2.40.1
[PATCH 1/2] spl: make SYS_SPL_MALLOC depend on !(SPL_STACK_R && SPL_SYS_MALLOC_SIMPLE)
Currently, one can have both SYS_SPL_MALLOC=y and SPL_SYS_MALLOC_SIMPLE=y. However, while the former does make board_init_r() in spl.c call mem_malloc_init(), that has no effect at all, because that just updates a few bookkeeping variables, but as the linker map shows, the latter setting has (as expected) caused most of dlmalloc.o to be garbage-collected. That is, those bookkeeping variables are not used for anything. IOWs, with SYS_SPL_MALLOC=y and SPL_SYS_MALLOC_SIMPLE=y, the value of CONFIG_SYS_SPL_MALLOC_SIZE is irrelevant, and one still only has the small, SRAM-backed, malloc arena available. Now I want to change that so that the mem_malloc_init() instead updates the gd->malloc* variables to point at the SDRAM area. However, there's a small complication, namely when SPL_STACK_R=y is also in the mix. In that case, the "simple" malloc arena is indeed updated to point at the SDRAM area carved out of the new stack (see spl_relocate_stack_gd()). So that case works in the sense that one does get a "large" "simple" malloc arena (of size SPL_STACK_R_MALLOC_SIMPLE_LEN) - but CONFIG_SYS_SPL_MALLOC_SIZE is still irrelevant. Once I change the mem_malloc_init() logic, this would then break, because the gd->malloc* variables would be updated again. Also, it doesn't really make sense to allow the .config to essentially specify two different SDRAM-backed malloc arenas. So since CONFIG_SYS_SPL_MALLOC and its dependent options are no-ops currently when SPL_STACK_R && SPL_SYS_MALLOC_SIMPLE, simply forbid that combination. Simple grepping suggests that these boards/configs are affected, in that they become a tiny bit smaller, and the defconfig will need refreshing: am62ax_evm_r5_defconfig am62x_evm_r5_defconfig am64x_evm_a53_defconfig am64x_evm_r5_defconfig am65x_evm_a53_defconfig am65x_hs_evm_a53_defconfig iot2050_defconfig j7200_evm_a72_defconfig j721e_evm_a72_defconfig j721s2_evm_a72_defconfig j721s2_evm_r5_defconfig verdin-am62_r5_defconfig Signed-off-by: Rasmus Villemoes --- common/spl/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/common/spl/Kconfig b/common/spl/Kconfig index c5dd476db5..fca9ada337 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -405,6 +405,7 @@ config SPL_SEPARATE_BSS config SYS_SPL_MALLOC bool "Enable malloc pool in SPL" depends on SPL_FRAMEWORK + depends on !(SPL_STACK_R && SPL_SYS_MALLOC_SIMPLE) config HAS_CUSTOM_SPL_MALLOC_START bool "For the SPL malloc pool, define a custom starting address" -- 2.37.2
[PATCH 0/2] make CONFIG_SPL_SYS_MALLOC_SIMPLE && CONFIG_SYS_SPL_MALLOC actually work
Currently, setting both CONFIG_SPL_SYS_MALLOC_SIMPLE and CONFIG_SYS_SPL_MALLOC (but not CONFIG_SPL_STACK_R) doesn't work as expected: The SIMPLE option means that all malloc etc. calls are directed at build-time to the implementation in malloc_simple.c, but the mem_alloc_init() call which is done in board_init_f() currently calls into dlmalloc.c, and updates the variables describing the dlmalloc arena - which is of course completely unused. And the malloc() implementation continues to hand out allocations from the initial SPL_SYS_MALLOC_F_LEN bytes. These two patches are an attempt at making the combination in $subject actually work as one would expect: define an area of sdram to be used as a malloc arena, but still manage that using the malloc_simple() implementation. Since this now changes the mem_alloc_init() from being a harmless no-op call to actually doing something, there's a risk of some boards breaking. The solution for those boards will probably be to just drop CONFIG_SYS_SPL_MALLOC, since that hasn't actually done anything. Rasmus Villemoes (2): spl: make SYS_SPL_MALLOC depend on !(SPL_STACK_R && SPL_SYS_MALLOC_SIMPLE) malloc_simple: add mem_malloc_init_simple() common/dlmalloc.c | 2 +- common/malloc_simple.c | 7 +++ common/spl/Kconfig | 1 + include/malloc.h | 7 +-- 4 files changed, 14 insertions(+), 3 deletions(-) -- 2.37.2
[PATCH 2/2] malloc_simple: add mem_malloc_init_simple()
I was running out of malloc() in SPL, and the message told me to look at CONFIG_SYS_SPL_MALLOC_SIZE. So I did, and bumped it quite a bit, but that had no effect whatsoever. The reason for that was that I also have CONFIG_SPL_SYS_MALLOC_SIMPLE=y. So while board_init_r() in spl.c duly calls mem_malloc_init(SYS_SPL_MALLOC_START, CONFIG_SYS_SPL_MALLOC_SIZE); that doesn't actually do anything, because that function just sets some variables in dlmalloc.c, and (as the linker map shows), the rest of dlmalloc.o has been garbage-collected. I don't want to have the full dlmalloc implementation in SPL - code size is still precious. However, once SDRAM is initialized, the heap is practically infinite, if only CONFIG_SYS_SPL_MALLOC_SIZE actually had an effect. So just as CONFIG_SPL_SYS_MALLOC_SIMPLE redirects malloc() and friends at build-time to the _simple variants, add a _simple variant of mem_malloc_init() which will actually update the bookkeeping variables relevant to the actual and active malloc() implementation. Signed-off-by: Rasmus Villemoes --- common/dlmalloc.c | 2 +- common/malloc_simple.c | 7 +++ include/malloc.h | 7 +-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index dcecdb8623..d42b26410f 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -619,7 +619,7 @@ void *sbrk(ptrdiff_t increment) return (void *)old; } -void mem_malloc_init(ulong start, ulong size) +void mem_dlmalloc_init(ulong start, ulong size) { mem_malloc_start = start; mem_malloc_end = start + size; diff --git a/common/malloc_simple.c b/common/malloc_simple.c index 0a004d40e1..9ecf05cf2e 100644 --- a/common/malloc_simple.c +++ b/common/malloc_simple.c @@ -17,6 +17,13 @@ DECLARE_GLOBAL_DATA_PTR; +void mem_malloc_init_simple(ulong start, ulong size) +{ + gd->malloc_base = start; + gd->malloc_ptr = 0; + gd->malloc_limit = size; +} + static void *alloc_simple(size_t bytes, int align) { ulong addr, new_ptr; diff --git a/include/malloc.h b/include/malloc.h index 161ccbd129..f59942115b 100644 --- a/include/malloc.h +++ b/include/malloc.h @@ -899,6 +899,7 @@ void malloc_disable_testing(void); #define malloc malloc_simple #define realloc realloc_simple #define memalign memalign_simple +#define mem_malloc_init mem_malloc_init_simple #if IS_ENABLED(CONFIG_VALGRIND) #define free free_simple #else @@ -908,6 +909,9 @@ void *calloc(size_t nmemb, size_t size); void *realloc_simple(void *ptr, size_t size); #else +#define mem_malloc_init mem_dlmalloc_init +void mem_dlmalloc_init(ulong start, ulong size); + # ifdef USE_DL_PREFIX # define cALLOcdlcalloc # define fREe dlfree @@ -955,6 +959,7 @@ int initf_malloc(void); /* Simple versions which can be used when space is tight */ void *malloc_simple(size_t size); void *memalign_simple(size_t alignment, size_t bytes); +void mem_malloc_init_simple(ulong start, ulong size); #pragma GCC visibility push(hidden) # if __STD_C @@ -997,8 +1002,6 @@ extern ulong mem_malloc_start; extern ulong mem_malloc_end; extern ulong mem_malloc_brk; -void mem_malloc_init(ulong start, ulong size); - #ifdef __cplusplus }; /* end of extern "C" */ #endif -- 2.37.2
Re: [PATCH v5 00/21] Kconfig: Tidy up some options
On Thu, Sep 14, 2023 at 10:55:38AM -0600, Simon Glass wrote: > The view from 'make menuconfig' is confusing in places. This series aims > to improve the top-level menu and also the boot menu. > > It also groups FDT-fixup options tegether, at least the ones I could fine. > > Finally this series marks the distro scripts as deprecated, so people have > a pointer to standard boot. > > Changes in v5: > - Add back the Makefile condition, for efi-app builds > > Changes in v4: > - Just move these options to driver/video for later consideration > - Allow TIMESTAMP without FIT > - Add new patch to drop SPL/TPL_RAM_SUPPORT option for SPL_LOAD_FIT_ADDRESS > - Add new patch to make ARCH_FIXUP_FDT_MEMORY depend on OF_LIBFDT > > Changes in v3: > - Drop comment about an update/ directory > - Drop extra newline and quote > - Add new patch to drop CMD_MTDPARTS condition for FDT_FIXUP_PARTITIONS > - Drop patch 'Move SYS_RX_ETH_BUFFER into the network menu' > - Drop patch 'video: Move BMP options and code to video directory' > > Changes in v2: > - Add new patch to move bmp code to drivers/video > - Fix FMU typo in the subject > - Drop now-unnecessary depends on FWU_MULTI_BANK_UPDATE > - Mention in the DISTRO_DEFAULTS option that it is script-based > - Expand and rewrite the commit message > - Use the word 'Mark' instead of 'Make' to improve the English > > Simon Glass (21): > lib: rational: Move the Kconfigs into the correct place > Kconfig: Move API into general setup > video: Move bmp code to drivers/video > video: Move the BMP options > FWU: Avoid showing an unselectable menu option > test: Move POST under a renamed Testing section > boot: Move fdt_support to boot/ > Move fdt_simplefb to boot/ > boot: Move some other fdt-fixup options to the same menu > boot: Rename Android-boot text > Kconfig: Create a menu for FIT > spl: Tidy up load address in spl_ram > spl: Drop SPL/TPL_RAM_SUPPORT option for SPL_LOAD_FIT_ADDRESS > Kconfig: Move SPL_FIT under FIT > boot: Make standard boot a menu > Kconfig: Move TEXT_BASE et al under general setup > Mark DISTRO_DEFAULTS as deprecated > Make ARCH_FIXUP_FDT_MEMORY depend on OF_LIBFDT > boot: Join FDT_FIXUP_PARTITIONS with related options > boot: Drop CMD_MTDPARTS condition for FDT_FIXUP_PARTITIONS > boot: Join ARCH_FIXUP_FDT_MEMORY with related options Thanks for iterating on this so many times. Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH] common: Drop linux/printk.h from common header
On Thu, Sep 14, 2023 at 06:21:46PM -0600, Simon Glass wrote: > This old patch was marked as deferred. Bring it back to life, to continue > towards the removal of common.h > > Move this out of the common header and include it only where needed. > > Signed-off-by: Simon Glass [snip] > 317 files changed, 322 insertions(+), 3 deletions(-) So, going back to the original series here, how did you determine that the files changed here need ? I can see how to get there with some grep'ing but I think my original complaint was that we added the new header too widely. -- Tom signature.asc Description: PGP signature
Re: [PATCH] common: Drop linux/printk.h from common header
Hi Tom, On Fri, 15 Sept 2023 at 13:48, Tom Rini wrote: > > On Thu, Sep 14, 2023 at 06:21:46PM -0600, Simon Glass wrote: > > > This old patch was marked as deferred. Bring it back to life, to continue > > towards the removal of common.h > > > > Move this out of the common header and include it only where needed. > > > > Signed-off-by: Simon Glass > [snip] > > 317 files changed, 322 insertions(+), 3 deletions(-) > > So, going back to the original series here, how did you determine that > the files changed here need ? I can see how to get > there with some grep'ing but I think my original complaint was that we > added the new header too widely. Basically I used ctags to get the symbols from the header file, then grepped files for them. For that one, it is: KERN_ALERT,KERN_CONT,KERN_CRIT,KERN_DEBUG,KERN_EMERG,KERN_ERR,KERN_INFO,KERN_NOTICE,KERN_WARNING,__KERNEL_PRINTK__,__printk,no_printk,pr_alert,pr_cont,pr_crit,pr_debug,pr_debug,pr_devel,pr_devel,pr_emerg,pr_err,pr_fmt,pr_info,pr_notice,pr_warn,pr_warning,printk,printk_once I was a little miffed that a comment along these lines from Heinrich, with no actual evidence, and no response to my follow-up questions, caused the series to be dropped. It isn't easy to do these sorts of clean-ups. A lot of files include things like net.h and image.h which bring in many headers...but we should clean up those header files up too (particularly image.h). So I think the standard should be: "include what you use". Regards, Simon
Re: [PATCH] MAINTAINERS: ufs: Change Bhupesh's email address
On 9/15/23 20:14, Bhupesh Sharma wrote: Set my current personal email in the MAINTAINERS file. Why this change so soon ? Did linaro raise any concerns about the maintainership ?
Re: [PATCH] common: Drop linux/printk.h from common header
On Fri, Sep 15, 2023 at 02:01:33PM -0600, Simon Glass wrote: > Hi Tom, > > On Fri, 15 Sept 2023 at 13:48, Tom Rini wrote: > > > > On Thu, Sep 14, 2023 at 06:21:46PM -0600, Simon Glass wrote: > > > > > This old patch was marked as deferred. Bring it back to life, to continue > > > towards the removal of common.h > > > > > > Move this out of the common header and include it only where needed. > > > > > > Signed-off-by: Simon Glass > > [snip] > > > 317 files changed, 322 insertions(+), 3 deletions(-) > > > > So, going back to the original series here, how did you determine that > > the files changed here need ? I can see how to get > > there with some grep'ing but I think my original complaint was that we > > added the new header too widely. > > Basically I used ctags to get the symbols from the header file, then > grepped files for them. For that one, it is: > > KERN_ALERT,KERN_CONT,KERN_CRIT,KERN_DEBUG,KERN_EMERG,KERN_ERR,KERN_INFO,KERN_NOTICE,KERN_WARNING,__KERNEL_PRINTK__,__printk,no_printk,pr_alert,pr_cont,pr_crit,pr_debug,pr_debug,pr_devel,pr_devel,pr_emerg,pr_err,pr_fmt,pr_info,pr_notice,pr_warn,pr_warning,printk,printk_once OK, good. > I was a little miffed that a comment along these lines from Heinrich, > with no actual evidence, and no response to my follow-up questions, > caused the series to be dropped. It isn't easy to do these sorts of > clean-ups. Well, I know earlier on I had concerns and spotted specific cases where it was off. > A lot of files include things like net.h and image.h which bring in > many headers...but we should clean up those header files up too > (particularly image.h). So I think the standard should be: "include > what you use". Yes, we should first get rid of common.h and then see what we can remove next. -- Tom signature.asc Description: PGP signature
Re: bootstd: CACHE Misaligned operation errors (Marvell Armada 385)
Hi Tom, Hi Simon, On Wed, Sep 13, 2023 at 9:53 PM Tony Dinh wrote: > > Hi Simon, > > On Wed, Sep 13, 2023 at 8:38 PM Simon Glass wrote: > > > > Hi Tom, > > > > On Wed, 13 Sept 2023 at 14:14, Tom Rini wrote: > > > > > > On Wed, Sep 13, 2023 at 12:56:53PM -0700, Tony Dinh wrote: > > > > Hi Tom, > > > > > > > > On Wed, Sep 13, 2023 at 9:22 AM Tom Rini wrote: > > > > > > > > > > On Tue, Sep 12, 2023 at 12:38:00PM -0700, Tony Dinh wrote: > > > > > > > > > > > I've been testing the boostd for a few Marvell boards and seeing > > > > > > this > > > > > > error on the Thecus N2350 (Marvell Armada 385, dual-core CPU). The > > > > > > "bootflow scan scsi" command triggered the "CACHE: Misaligned > > > > > > operation at range" error. However, this error did not affect the > > > > > > result of the scan, i.e. the bootflow for scsi partition was created > > > > > > correctly, and u-boot is running normally. > > > > > > > > > > > > Enabling CONFIG_SYS_DCACHE_OFF got rid of the errors altogether. > > > > > > Perhaps this is a case where the DCACHE is not required and should > > > > > > be > > > > > > turned off? > > > > > > > > > > > > Please see the log after the break below. > > > > > > > > > > Can you please try -next ? There's at least one SCSI related cache > > > > > alignment fix there that's not in master, thanks. > > > > > > > > Unfortunately I got the same errors. This time the ranges are > > > > different, of course. > > > > > > > > master: > > > > > > > > N2350 > bootflow scan scsi > > > > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388] > > > > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388] > > > > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388] > > > > ERROR: v7_outer_cache_inval_range - start address is not aligned - > > > > 0x3fb99f88 > > > > ERROR: v7_outer_cache_inval_range - stop address is not aligned - > > > > 0x3fb9a388 > > > > > > > > next: > > > > > > > > N2350 > bootflow scan scsi > > > > CACHE: Misaligned operation at range [3fb80388, 3fb80788] > > > > CACHE: Misaligned operation at range [3fb80388, 3fb80788] > > > > CACHE: Misaligned operation at range [3fb80388, 3fb80788] > > > > ERROR: v7_outer_cache_inval_range - start address is not aligned - > > > > 0x3fb80388 > > > > ERROR: v7_outer_cache_inval_range - stop address is not aligned - > > > > 0x3fb80788 > > > > > > Can you debug to where these calls are so we can align these buffers? > > > See 02660defdc8a ("scsi: Cache align temporary buffer") for an example. > > > > I wonder if we need to use memalign() when allocating memory to read things > > from the media? But I am not sure which file time is being read, or which > > bootmeth it is. > > Looks like we probably need to align the buffer tempbuff. > > /drivers/scsi/scsi.c > static int scsi_detect_dev(struct udevice *dev, int target, int lun, > struct blk_desc *dev_desc) > { > unsigned char perq, modi; > lbaint_t capacity; > unsigned long blksz; > struct scsi_cmd *pccb = (struct scsi_cmd *)&tempccb; > int count, err; > > pccb->target = target; > pccb->lun = lun; > pccb->pdata = (unsigned char *)&tempbuff; > pccb->datalen = 512; > > If you look at the log I posted previously, this error shows up during > "bootflow scan scsi". > Taking the hint from Simon. I turned on log_debug and can see where the alignment is not correct. It is fs.c fs_read_alloc(). The memalign() call here probably needs to be revised to take into consideration ARCH_DMA_MINALIGN somehow? It is 64 for armv7. diff --git a/fs/fs.c b/fs/fs.c index 2b815b1db0..b70281532e 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -1019,9 +1019,12 @@ int fs_read_alloc(const char *fname, ulong size, uint align, void **bufp) int ret; buf = memalign(align, size + 1); + log_debug("aligned buf addr 0x%x\n", (unsigned int)buf); + if (!buf) return log_msg_ret("buf", -ENOMEM); addr = map_to_sysmem(buf); + log_debug("aligned buf sysmem addr 0x%x\n", (unsigned int)addr); ret = fs_read(fname, addr, 0, size, &bytes_read); if (ret) { Please see the log below after the break. All the best, Tony Log: bootmeth_try_file()/boot/boot.scr - err=0 fs_devread() <2, 0, 1024> fs_devread() <8, 0, 32> fs_devread() <8216, 256, 128> bootmeth_alloc_file()- script file size 4b7 fs_read_alloc() aligned buf addr 0x3eb4b608 fs_read_alloc() aligned buf sysmem addr 0x3eb4b608 fs_devread() <12312, 0, 8> fs_devread() <551551112, 256, 128> fs_devread() <551592072, 0, 1207> scsi_read buffer addr=0x3eb4b608 scsi_read_ext: startblk 20e0a88a, blccnt 0 buffer 3EB4B608 scsi_exec pdata=0x3eb4b608 ahci_scsi_exec ahci_scsi_exec cmd = 0x48 ahci_scsi_exec pdata=0x3eb4b608 ata_scsiop_read_write ERROR: v7_outer_cache_inval_range - start address is not aligned - 0x3eb4b608 ERROR: v7_outer_cache_inval_range - stop address is not aligned - 0x3eb4ba08 ata_scsiop_read_writ
[PATCH] MAINTAINERS: ufs: Change Bhupesh's email address
Set my current personal email in the MAINTAINERS file. Signed-off-by: Bhupesh Sharma --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 536df4a896..b1f5ae0591 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1597,7 +1597,7 @@ T:git https://source.denx.de/u-boot/custodians/u-boot-ubi.git F: drivers/mtd/ubi/ UFS -M: Bhupesh Sharma +M: Bhupesh Sharma M: Neha Malcom Francis S: Maintained F: drivers/ufs/ -- 2.38.1
[PATCH] doc: usage: load: document part as hexadecimal
From: Mickaël Tansorier `part` option is in hexadecimal, so information is missing in usage documentation. Callgraph for `part` parsing is : do_load -> fs_set_blk_dev -> part_get_info_by_dev_and_name_or_num -> blk_get_device_part_str -> hextoul (This is why it is hexadecimal) Signed-off-by: Mickaël Tansorier Reviewed-by: Yoann Congal --- doc/usage/cmd/load.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/usage/cmd/load.rst b/doc/usage/cmd/load.rst index 1efee773179..2c892ee1cb7 100644 --- a/doc/usage/cmd/load.rst +++ b/doc/usage/cmd/load.rst @@ -40,7 +40,7 @@ bytes pos number of bytes to skip -addr, bytes, pos are hexadecimal numbers. +part, addr, bytes, pos are hexadecimal numbers. Example --- -- 2.30.2
Re: bootstd: CACHE Misaligned operation errors (Marvell Armada 385)
On Fri, Sep 15, 2023 at 6:32 PM Tony Dinh wrote: > > Hi Tom, Hi Simon, > > On Wed, Sep 13, 2023 at 9:53 PM Tony Dinh wrote: > > > > Hi Simon, > > > > On Wed, Sep 13, 2023 at 8:38 PM Simon Glass wrote: > > > > > > Hi Tom, > > > > > > On Wed, 13 Sept 2023 at 14:14, Tom Rini wrote: > > > > > > > > On Wed, Sep 13, 2023 at 12:56:53PM -0700, Tony Dinh wrote: > > > > > Hi Tom, > > > > > > > > > > On Wed, Sep 13, 2023 at 9:22 AM Tom Rini wrote: > > > > > > > > > > > > On Tue, Sep 12, 2023 at 12:38:00PM -0700, Tony Dinh wrote: > > > > > > > > > > > > > I've been testing the boostd for a few Marvell boards and seeing > > > > > > > this > > > > > > > error on the Thecus N2350 (Marvell Armada 385, dual-core CPU). The > > > > > > > "bootflow scan scsi" command triggered the "CACHE: Misaligned > > > > > > > operation at range" error. However, this error did not affect the > > > > > > > result of the scan, i.e. the bootflow for scsi partition was > > > > > > > created > > > > > > > correctly, and u-boot is running normally. > > > > > > > > > > > > > > Enabling CONFIG_SYS_DCACHE_OFF got rid of the errors altogether. > > > > > > > Perhaps this is a case where the DCACHE is not required and > > > > > > > should be > > > > > > > turned off? > > > > > > > > > > > > > > Please see the log after the break below. > > > > > > > > > > > > Can you please try -next ? There's at least one SCSI related cache > > > > > > alignment fix there that's not in master, thanks. > > > > > > > > > > Unfortunately I got the same errors. This time the ranges are > > > > > different, of course. > > > > > > > > > > master: > > > > > > > > > > N2350 > bootflow scan scsi > > > > > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388] > > > > > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388] > > > > > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388] > > > > > ERROR: v7_outer_cache_inval_range - start address is not aligned - > > > > > 0x3fb99f88 > > > > > ERROR: v7_outer_cache_inval_range - stop address is not aligned - > > > > > 0x3fb9a388 > > > > > > > > > > next: > > > > > > > > > > N2350 > bootflow scan scsi > > > > > CACHE: Misaligned operation at range [3fb80388, 3fb80788] > > > > > CACHE: Misaligned operation at range [3fb80388, 3fb80788] > > > > > CACHE: Misaligned operation at range [3fb80388, 3fb80788] > > > > > ERROR: v7_outer_cache_inval_range - start address is not aligned - > > > > > 0x3fb80388 > > > > > ERROR: v7_outer_cache_inval_range - stop address is not aligned - > > > > > 0x3fb80788 > > > > > > > > Can you debug to where these calls are so we can align these buffers? > > > > See 02660defdc8a ("scsi: Cache align temporary buffer") for an example. > > > > > > I wonder if we need to use memalign() when allocating memory to read > > > things from the media? But I am not sure which file time is being read, > > > or which bootmeth it is. > > > > Looks like we probably need to align the buffer tempbuff. > > > > /drivers/scsi/scsi.c > > static int scsi_detect_dev(struct udevice *dev, int target, int lun, > > struct blk_desc *dev_desc) > > { > > unsigned char perq, modi; > > lbaint_t capacity; > > unsigned long blksz; > > struct scsi_cmd *pccb = (struct scsi_cmd *)&tempccb; > > int count, err; > > > > pccb->target = target; > > pccb->lun = lun; > > pccb->pdata = (unsigned char *)&tempbuff; > > pccb->datalen = 512; > > > > If you look at the log I posted previously, this error shows up during > > "bootflow scan scsi". > > > > Taking the hint from Simon. I turned on log_debug and can see where > the alignment is not correct. It is fs.c fs_read_alloc(). The > memalign() call here probably needs to be revised to take into > consideration ARCH_DMA_MINALIGN somehow? It is 64 for armv7. > > diff --git a/fs/fs.c b/fs/fs.c > index 2b815b1db0..b70281532e 100644 > --- a/fs/fs.c > +++ b/fs/fs.c > @@ -1019,9 +1019,12 @@ int fs_read_alloc(const char *fname, ulong > size, uint align, void **bufp) > int ret; > > buf = memalign(align, size + 1); > + log_debug("aligned buf addr 0x%x\n", (unsigned int)buf); > + > if (!buf) > return log_msg_ret("buf", -ENOMEM); > addr = map_to_sysmem(buf); > + log_debug("aligned buf sysmem addr 0x%x\n", (unsigned int)addr); > > ret = fs_read(fname, addr, 0, size, &bytes_read); > if (ret) { > > Please see the log below after the break. Actually, it looks like the fix should be in bootmeth_script.c. diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c index 0269e0f9b0..68e77aa50a 100644 --- a/boot/bootmeth_script.c +++ b/boot/bootmeth_script.c @@ -99,7 +99,7 @@ static int script_read_bootflow_file(struct udevice *bootstd, if (!bflow->subdir) return log_msg_ret("prefix", -ENOMEM); - ret = bootmeth_alloc_file(bflow, 0x1, 1); + ret = bootmeth_alloc_file(bflow, 0x1, ARCH_DMA_MINALIGN); All the best, Tony > > > Log: > >bootmeth_try_file()/boot/boo